casaroli commented on PR #19532:
URL: https://github.com/apache/nuttx/pull/19532#issuecomment-5077957310
Updated the branch: the programming path is now exercised under emulation,
and doing so found and fixed a real bug in it.
**What was wrong.** The guard that refuses to re-program an
already-programmed row checked only whether the row held data *outside* the
bits the field covers:
```c
if ((current & ~touched) != 0) { return -EROFS; }
```
For a field covering the whole row, `touched` is `0xffff`, so that term is
always zero and the guard was bypassed. Writing `0x5a5a` over a row already
holding `0xa5a5` therefore fell through and asked the bootrom to program
`0xffff` into a row whose ECC was already fixed, leaving the driver relying on
the bootrom to refuse. The check is now simply "any non-blank row is off
limits", which is what the ECC constraint actually implies:
```c
if ((uint16_t)(current | value) == current) { continue; } /* already
programmed */
if (current != 0) { return -EROFS; } /* ECC cannot be
recomputed */
```
**How it was found.** I added a functional RP2350 OTP model to my Renode
setup (the 4096 x 24 bit array, the ECC window at `0x40130000`, the raw window
at `0x40134000`, the SW_LOCK registers, and OR-only programming semantics), and
intercepted the bootrom `otp_access()` entry point so programming reaches the
model. That makes the destructive path repeatable without consuming fuses on
real silicon.
19 checks now pass in emulation, covering: a blank row reads zero;
programming a whole row and reading it back; re-writing the same value being a
no-op rather than an error; **a different value being refused with `EROFS`**
(the case that failed before); a sub-row field landing at the right bit
positions; a 32-bit field split across two rows and read back whole;
out-of-range writes rejected; and the factory identity rows left untouched
throughout.
The read path results in the PR description are unchanged and still come
from real hardware — the fix touches only `rp23xx_otp_write_field`, which is
compiled out of the hardware build. Programming has still never been run on
silicon, so the caveat in the description stands: emulation validates this
driver's logic, not the bootrom's programming sequence.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
