I've submitted a patch to fix this here: https://review.openocd.org/c/openocd/+/7612
I've also submitted three related patches, one to require enabling OTP writes before OTP protection can be set, one to fix a typo in a message and one to improve the message when trying to undo OTP protection (which is impossible). The second patch is shown as having a merge conflict, but AFAICS that's only because it depends on the first patch and Gerrit does not take that into account. And the third patch shows a build failure, but that only seems to be because the first build was aborted because I pushed an update to a commit message before it was completed, a second build (in the same comment?) *was* successful AFAICS. I wanted to comment these things on the patchsets themselves in Gerrit, but I could not find any place where I could just leave a comment about the patchset. I'm really not used to using Gerrit for tracking patches, so my github-PR-mindset is probably approaching this wrong.... --- ** [tickets:#393] STM32F2: Protecting OTP fails (code inverts lock value)** **Status:** new **Milestone:** 0.10.0 **Created:** Thu Apr 27, 2023 07:33 AM UTC by Matthijs Kooijman **Last Updated:** Thu Apr 27, 2023 03:43 PM UTC **Owner:** nobody **Attachments:** - [openocd.log.txt](https://sourceforge.net/p/openocd/tickets/393/attachment/openocd.log.txt) (317.0 kB; text/plain) The STM32F2x flash driver correctly reads protection bytes for the OTP memory, but fails to write them. Looking at the code, it seems the `stm32x_otp_protect` read protect function uses the correct meaning of the lock bytes (0x00 == locked, any other value == unlocked), while `stm32x_otp_protect` actually tries to write 0xff to enable protection, which is incorrect. See the code here: https://github.com/openocd-org/openocd/blob/91bd4313444c5a949ce49d88ab487608df7d6c37/src/flash/nor/stm32f2x.c#L510-L557 The documentation (PM0059: STM32F205/215, STM32F207/217 Flash programming manual) says: > The OTP area is divided into 16 OTP data blocks of 32 bytes and one lock OTP > block of 16 bytes. The OTP data and lock blocks cannot be erased. The lock block contains 16 bytes LOCKBi (0 ≤ i ≤ 15) to lock the corresponding OTP data block (blocks 0 to 15). Each OTP data block can be programmed until the value 0x00 is programmed in the corresponding OTP lock byte. The lock bytes must only contain 0x00 and 0xFF values, otherwise the OTP bytes might not be taken into account correctly. I've tested this on an F4, which uses the same driver (and has OTP that works the same way). Reading the lock bytes works correctly (this chip has one OTP sector locked already): ``` matthijs@dottie:~/docs/MKIT/3Devo/ChildbusBootloader$ openocd -f interface/stlink.cfg -f target/stm32f4x.cfg -c 'init; reset init; flash info 1' Open On-Chip Debugger 0.12.0 [Snip more output] #1 : stm32f2x at 0x1fff7800, size 0x00000200, buswidth 0, chipwidth 0 # 0: 0x00000000 (0x20 0kB) protected # 1: 0x00000020 (0x20 0kB) not protected # 2: 0x00000040 (0x20 0kB) not protected # 3: 0x00000060 (0x20 0kB) not protected [Snip more sectors] STM32F4xx (Low Power) - Rev: Z ``` However locking sector 1 fails: ``` matthijs@dottie:~/docs/MKIT/3Devo/ChildbusBootloader$ openocd -f interface/stlink.cfg -f target/stm32f4x.cfg -c 'init; reset init; flash protect 1 1 1 on' Open On-Chip Debugger 0.12.0 [Snip more output] Error: failed setting protection for blocks 1 to 1 ``` I've attached a complete log with `-d3` to show some more details. Looking at the [code for this](https://github.com/openocd-org/openocd/blob/91bd4313444c5a949ce49d88ab487608df7d6c37/src/flash/nor/stm32f2x.c#L530-L557): ``` for (i = first; first <= last; i++) { retval = target_read_u8(target, lock_base + i, &lock); if (retval != ERROR_OK) return retval; if (lock) continue; lock = 0xff; retval = target_write_u8(target, lock_base + i, lock); if (retval != ERROR_OK) return retval; } return ERROR_OK; } ``` You can see that the `if (lock)` is inverted (it is locked when it is 0, not non-zero), so it skips any unlocked blocks and will only try to lock blocks that are already locked. You would expect it to then just silently fail and not raise an error, but (thanks for gdb to help me here), it turns out the loop is actually an infinite loop, since it checks `first <= last` instead of `i <= last`, so that's another bug. Since `i` does increment, this means it reads all the lock bytes, then system memory and then tries to read past the end of system memory (0x1fff8000) and fails, breaking the loop and returning error. Looks like this code has been broken since when it was first introduced in [f21c12abec](https://github.com/openocd-org/openocd/commit/f21c12abec) and maybe was never tested (which is of course tricky with OTP...). I will see if I can provide a patch to fix this - I have some sacrificial boards that I can use for testing. --- Sent from sourceforge.net because openocd-devel@lists.sourceforge.net is subscribed to https://sourceforge.net/p/openocd/tickets/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/openocd/admin/tickets/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.
