Host-upload FPGA SRAM path programs zeros: xi2c_write_long clobbers its own source buffer

[boringethan]

Found during camera-fpga full-frame-readout bring-up (OpenwaterHealth/openmotion-camera-fpga#5), present in 1.8.1-rc.3 and current branches incl. feature/68.

Path: OW_FPGA_BITSTREAM blocks fill bitstream_buffer (host bitstream, CRC-checked) → OW_FPGA_PROG_SRAM reserved=0 → program_sram_fpga(cam, rom_bitstream=true, ...) → fpga_program_sram (crosslink.c:396) → xi2c_write_long(hi2c, addr, cmd, 4, (uint8_t*)bitstream_buffer, bitstream_len) (crosslink.c:408).

Bug: xi2c_write_long (crosslink.c:88) begins with
memset(bitstream_buffer, 0, MAX_BITSTREAM_SIZE) then
memcpy(bitstream_buffer + cmd_len, data, data_len) — when data IS bitstream_buffer (the reserved=0 path), the memset zeroes the source before the copy, so the CrossLink receives the 0x7A command followed by zeros. The embedded-flash path (fpga_configure → ADDR_CAMERA_BITSTREAM) is unaffected because its source is flash, not the shared scratch buffer.

Also observed while working around it (informational): after any DFU session, the app's USB does not re-enumerate until a full power cycle — worth confirming whether that's expected on these boards.

Suggested fix: give xi2c_write_long its own scratch/chunk buffer (it only needs cmd_len+chunk bytes at a time — stream directly from the source with a small prefix buffer for the first chunk), or forbid data==bitstream_buffer and add a separate upload buffer.

Workaround used: DFU-rewrite the dedicated bitstream flash sectors (0x081A0000, 163489 B) and program via the stock path.

[boringethan]

Root-caused and fixed on branch fix/82-sram-upload-path (together with three sibling bugs found during the same bring-up: uploads land at buffer+4 but were streamed from +0; exit_sram_prog sent ISC_ENABLE instead of ISC_DISABLE; program_sram_fpga skipped the USART realign that program_fpga performs). Also: the SDK's send_bitstream_fpga 1KB blocks exceed the 512B comms RX buffer and are always rejected — host tooling must use <=500-byte payloads (SDK issue worth filing). Hardware-validated: 16-camera full-frame capture ran end to end through the fixed path.

[boringethan]

Opened draft PR #84 for the fix/82-sram-upload-path branch (1 commit, e9d3b1a) so the fix is visible, reviewable, and protected from branch cleanup.

Context: the camera-fpga full-frame image readout tooling (OpenwaterHealth/openmotion-camera-fpga#7) uploads bitstreams to FPGA SRAM at capture time and cannot work on stock firmware because of this bug. The patch was validated on both sensor modules as 1.8.1-rc.3-dirty during the 2026-07-05 16-camera capture campaign (32/32 complete frames). A second test setup will need firmware built from this branch (Debug config) — replication recipe in tools/full_frame_capture/RUNBOOK.md on the camera-fpga branch.

[boringethan]

Fixed in PR #120, taking the first of the two suggested approaches.

Only the first chunk needs assembling, because only it carries the command prefix; every later chunk is a straight slice of the caller's data. So chunk 0 is built in a private 8 KB buffer and the remaining chunks stream directly from data — the caller's buffer is never written to at all, which also means the function no longer cares whether data aliases bitstream_buffer.

Chunk sizes and I2C frame flags are deliberately left alone, so the byte stream and the framing on the wire stay exactly what the working flash path already produced. That keeps the change off the path that currently works.

Modelled both implementations over the real 163,489 B length plus the chunk-boundary cases:

case                               bytes  chk   OLD impl     NEW   framing
----------------------------------------------------------------------------------
flash source, 163489 B            163489   20   ok           ok    identical
ALIASED source 163489 B (#82)     163489   20   ZEROS(163488) ok   identical
exactly one chunk (8188+4)          8188    1   ok           ok    identical
one chunk minus 1                   8187    1   ok           ok    identical
one chunk plus 1                    8189    2   ok           ok    identical
exact multiple boundary            16380    2   ok           ok    identical
tiny                                   1    1   ok           ok    identical
tiny aliased                           1    1   ZEROS(0)     ok    identical

The old code emits 163,488 zeros on the aliased path — the reported symptom, reproduced exactly. Builds clean in all four configs; costs 8 KB of RAM_D1 (75.58% → 77.15%). Streaming from data also drops a 163 KB copy out of the flash boot path.

Not yet hardware-verified. Bench check: upload over OW_FPGA_BITSTREAM, OW_FPGA_PROG_SRAM with reserved=0, confirm the FPGA comes up with a working design rather than blank.

Why this got picked up now

It is the pre-flight for permanent NVCM burns. NVCM is one-time programmable, so the exact image wants proving in SRAM before it is fused — and that is the path this bug breaks. The workaround recorded above (DFU-rewrite 0x081A0000) is unavailable on a bootloader unit, since the bootloader clamps its DFU window to the application slot. On a converted module there is currently no way to try a bitstream before committing it permanently, which is what makes this worth fixing rather than working around.

Related on the same thread: openmotion-sdk#196 (bundled NVCM image and released runtime bitstream are different FPGA builds) and openmotion-sdk#158 (success verdict keys on the wrong discriminator).

[boringethan]

Status + plan (bench-verified 2026-08-21, left sensor).

The bug is confirmed, and the fix landscape is now settled — full bench matrix with a corrupt/real A/B is in PR #120's verification comment. Headline findings:

• The xi2c_write_long staging fix alone (PR fix: xi2c_write_long wipes its own source buffer, programming an all-zero FPGA design (#82) #120) is necessary but not sufficient — the host-upload path also needs the bitstream_buffer + 4 source, a real ISC_DISABLE (0x26) on exit, and forced-program semantics.
• All 8 cameras on the bench left sensor are NVCM-burned, so OW_FPGA_PROG_SRAM reserved=0 silently skips and false-passes (returns success in 0.1 s, NVCM image keeps streaming valid histograms). Any verification that saw a ~0.1 s program proved nothing.
• PR fix: xi2c_write_long wipes its own source buffer, programming an all-zero FPGA design (#82) #120 + the PR fix: host-upload FPGA SRAM path programs zeros (xi2c_write_long buffer clobber) #84 fix set, verified end to end today: reserved=3 forced upload of a deliberately corrupted bitstream → real 4.0 s program, no DONE, dead camera; same procedure with the real bitstream → DONE (0x3C byte2 = 0x0F), 159/159 histograms at 40 Hz with exact bin sums (2,457,606). The uploaded bytes are provably what programs.

Plan

1. Land PR fix: host-upload FPGA SRAM path programs zeros (xi2c_write_long buffer clobber) #84 (fix/82-sram-upload-path, e9d3b1a) — it is the complete, twice-bench-validated fix set (2026-07-05 capture campaign + today's run): xi2c_write_long first-chunk staging, bitstream_buffer + 4 source, exit_sram_prog_fpga → real fpga_exit_prog_mode (0x26), reserved==2 (forced flash) / reserved==3 (forced host-upload) semantics, post-program USART realign. Mark it ready for review, rebase onto current next (it cherry-picks cleanly), merge.
2. PR fix: xi2c_write_long wipes its own source buffer, programming an all-zero FPGA design (#82) #120 closed as superseded — its xi2c_write_long variant is functionally equivalent to fix: host-upload FPGA SRAM path programs zeros (xi2c_write_long buffer clobber) #84's; both variants are now hardware-proven, so keeping one complete PR is simpler than merging overlapping halves.
3. Validation recipe for the merged build (the pre-flight for NVCM burns): upload over OW_FPGA_BITSTREAM in ≤500 B blocks (512 B comms RX buffer), then activation key with CRESETB low → IDCODE → ISC_ENABLE → ISC_ERASE (+5.5 s settle) → PROG_SRAM reserved=3 → ISC_DISABLE. Assert the program takes seconds (~4 s for 163,489 B) — 0.1 s means the NVCM gate skipped; assert 0x3C byte2 == 0x0F; confirm 40 Hz histograms with exact bin sums. Do not use reserved=0 as the check on NVCM-burned parts.
4. Ticket stays In review on the board until the fix ships in a release (or Ethan confirms validation), per the usual flow.

Bench state note: left sensor is currently running the verified test build 1.8.2-rc.4-2-g3f502d0 (PR #120 + #84 set, local branch bench/pr120-plus-fixset); it ran 1.8.2-rc.4-5-gf975fcf before today. Right sensor untouched (1.8.2-rc.4-6-gb4d6910).