Fix BK4819 register reads arriving shifted one bit left

Every read came back doubled: seed REG_0C with 0x1248 and the firmware received
0x2490. The command byte's own trailing falling edge was being treated as a data
clock, so bit 15 was shifted away before the guest sampled it and the whole word
landed one place too high.

Each firmware bit is read/raise/lower (BK4819_ReadU16), which means the eighth
command bit is followed by a falling edge before the data phase begins. Skip that
one edge.

Why it went unnoticed: writes were always fine -- 52 registers held exactly what
the firmware wrote -- and the register the firmware polls hardest, REG_0C, was
legitimately 0 in this model. Reading zero and getting zero looks like success.
The skew only surfaced when something tried to report a value through it.

It also explains four failed attempts at the squelch interrupt. The model raised
REG_0C bit 0; the firmware received bit 1. So

    while (BK4819_ReadRegister(BK4819_REG_0C) & 1u)

was never true, the acknowledging write inside it never ran, and 1719 polls saw a
flag the guest could not act on. Every one of those attempts was diagnosed as a
timing or gating problem and was not.

tools/test_bk4819_readback.sh locks it down. It seeds REG_0C -- read ~1700 times
per 30s, so a sample is guaranteed -- with a value carrying bits in both halves,
so a shift either way is unmistakable, and names the direction on failure. Bit 0
is left clear on purpose: with it set the firmware enters an acknowledge loop that
has no timeout, and this test is about alignment only.

Confirmed by A/B: committed code 0x2490, patched 0x1248.
This commit is contained in:
mckero committed 2026-08-29 04:27:00 +01:00
1 parent 655e7883ad
commit ad88ee1519
2 files changed
+115 -1

No files matched your search

+15 -1
View File
@@ -641,6 +641,7 @@ struct BK4819State {
uint8_t cmd; /* register number, once known */ uint8_t cmd; /* register number, once known */
bool have_cmd; bool have_cmd;
bool reading; bool reading;
bool skip_falling; /* the command byte's trailing edge, not a data bit */
uint16_t shift_out; /* bits being clocked out to the guest */ uint16_t shift_out; /* bits being clocked out to the guest */
/* Register file. 128 registers is enough: the number field is seven bits. */ /* Register file. 128 registers is enough: the number field is seven bits. */
@@ -689,6 +690,7 @@ static void bk4819_reset(DeviceState *dev)
s->have_cmd = false; s->have_cmd = false;
s->reading = false; s->reading = false;
s->shift_out = 0; s->shift_out = 0;
s->skip_falling = false;
bk4819_seed_measurements(s); bk4819_seed_measurements(s);
} }
@@ -719,6 +721,7 @@ static void bk4819_set_cs(void *opaque, int line, int level)
s->shift_in = 0; s->shift_in = 0;
s->have_cmd = false; s->have_cmd = false;
s->reading = false; s->reading = false;
s->skip_falling = false;
} }
s->cs = selected; s->cs = selected;
} }
@@ -747,6 +750,15 @@ static void bk4819_set_scl(void *opaque, int line, int level)
s->shift_in = 0; s->shift_in = 0;
if (s->reading) { if (s->reading) {
s->shift_out = s->regs[s->cmd]; s->shift_out = s->regs[s->cmd];
/*
* The command byte's own trailing falling edge must not consume
* bit 15. Each firmware bit is read/raise/lower, so the eighth
* command bit is followed by a falling edge before the data loop
* begins -- and the advance below would shift bit 15 away before
* the guest ever sampled it, delivering the whole word one place
* too high (0x0001 arrived as 0x0002).
*/
s->skip_falling = true;
bk4819_update_sda(s); bk4819_update_sda(s);
} }
} }
@@ -776,7 +788,9 @@ static void bk4819_set_scl(void *opaque, int line, int level)
} }
} }
if (falling && s->have_cmd && s->reading) { if (falling && s->skip_falling) {
s->skip_falling = false;
} else if (falling && s->have_cmd && s->reading) {
/* /*
* Advance on the falling edge so the next bit is settled before the guest * Advance on the falling edge so the next bit is settled before the guest
* samples it. BK4819_ReadU16 sets SCL low, reads, then sets it high. * samples it. BK4819_ReadU16 sets SCL low, reads, then sets it high.
+100
View File
@@ -0,0 +1,100 @@
#!/usr/bin/env bash
# A register read must deliver the value the register holds.
#
# This exists because it did not. Reads came back shifted one place left -- 0x1248
# arrived as 0x2490 -- and the bug was invisible for a long time because writes were
# fine and the registers the firmware polls most were legitimately zero. Reading zero
# and getting zero proves nothing.
#
# The probe seeds REG_0C, which the firmware reads about 1700 times per 30 seconds, so
# a sample is guaranteed. The seed 0x1248 has bits in both halves, so a shift in either
# direction is unmistakable rather than plausible. Bit 0 is deliberately clear: with it
# set the firmware enters an acknowledge loop that has no timeout, and this test is
# about alignment, not interrupt semantics.
set -u
QEMU=${QEMU:-/root/qemu-build/qemu-7.2+dfsg/build/qemu-system-arm}
ELF=${ELF:-/root/uvk5-port/uvk5-sat/build/CW/nr7y.cw.elf}
HERE=$(cd "$(dirname "$0")" && pwd)
SIM=$(dirname "$HERE")
SRC="$SIM/qemu/py32f071.c"
QSRC=/root/qemu-build/qemu-7.2+dfsg/hw/arm/py32f071.c
SEED=0x1248
PORT=1259
SOCK=/tmp/bk-readback.sock
IMG=/tmp/bk-readback.img
for t in "$QEMU" "$ELF"; do
[ -e "$t" ] || { echo "SKIP missing $t"; exit 0; }
done
cp "$SRC" /tmp/bk-readback-orig.c
trap 'cp /tmp/bk-readback-orig.c "$SRC"; cp "$SRC" "$QSRC" 2>/dev/null || true; rm -f "$IMG" "$SOCK"' EXIT
python3 - "$SRC" "$SEED" <<'PY'
import sys
src, seed = sys.argv[1], sys.argv[2]
s = open(src).read()
needle = " s->regs[BK4819_REG_NOISE] = 0x0010;"
if needle not in s:
sys.exit("seed point not found; has bk4819_seed_measurements changed?")
open(src, "w").write(s.replace(needle, f"{needle}\n s->regs[0x0C] = {seed};", 1))
PY
cp "$SRC" "$QSRC"
if (cd /root/qemu-build/qemu-7.2+dfsg/build && ninja qemu-system-arm 2>&1 \
| grep -qE 'FAILED|error:'); then
echo "FAIL build error"
exit 1
fi
gzip -dc "$SIM/assets/pristine/flash-pristine.img.gz" > "$IMG"
rm -f "$SOCK"
timeout 80 "$QEMU" -M "uv-k5-v3,flash-image=$IMG" \
-nographic -monitor none -qmp "unix:$SOCK,server=on,wait=off" \
-kernel "$ELF" -gdb tcp::$PORT 2>/dev/null >/dev/null &
QPID=$!
sleep 24
# A command file, not a pile of -ex flags: a `commands` block cannot survive being
# passed that way, and the failure looks exactly like "the firmware never read it".
cat > /tmp/bk-readback.gdb <<GDB
set confirm off
set pagination off
set height 0
target remote :$PORT
set \$n = 0
break BK4819_ReadRegister
commands
silent
if \$r0 == 0x0c
set \$n = \$n + 1
if \$n <= 1
finish
printf "GOT 0x%04X\n", \$r0
end
end
continue
end
continue
GDB
GOT=$(timeout 45 gdb-multiarch -batch -x /tmp/bk-readback.gdb "$ELF" 2>/dev/null \
| grep -oE 'GOT 0x[0-9A-Fa-f]{4}' | head -1)
kill $QPID 2>/dev/null || true
wait $QPID 2>/dev/null || true
echo " seeded $SEED, firmware received ${GOT:-nothing}"
if [ "$GOT" = "GOT 0x1248" ]; then
echo "PASS reads are bit-aligned"
exit 0
fi
case "$GOT" in
"GOT 0x2490") echo "FAIL shifted one place left; the command byte's trailing falling edge is eating bit 15" ;;
"GOT 0x0924") echo "FAIL shifted one place right; a data bit is being presented twice" ;;
"") echo "FAIL no sample taken; did the firmware boot?" ;;
*) echo "FAIL unexpected value" ;;
esac
exit 1