arch/arm/stm32h5: Add OTP support - #20144
Conversation
|
Beat me to it. I have a similar branch which adds this and support for the Can we make it possible to read/write word-by-word without locking blocks? I posted in the // Disable NMI for flash double ECC faults (set ECCNMI_MASK_EN bit)
modifyreg32(STM32_SBS_ECCNMIR, 0, 1); |
|
Hey @darrylring, The differences in flexibility seem to be in the spirit of preventing faults. This PR: Your branch: Mykhailo may join the discussion. There may be a way to catch faults and return an error. Too bad developing this feature is literally expensive. |
|
The main idea behind locking the entire block after write was mostly dictated by this restriction in a reference manual: on page 289 (section 7.3.9) is it clearly stated that an attempt to overwrite already written word may lead to data errors. Thus, there is no other way to make sure we are not corrupting our data. There is a thread in a forum already, proving that the concern is real: https://community.st.com/stm32-mcus-products-25/stm32h503-otp-bug-150915 Lock is an only proper way to prevent double write and possible data corruption. So the problem effectively boils down to this:
This is not theoretical question: it has proven to be an issue already - usually OTP burning is only used during provision phase. The provisioning phase is not only "write to OTP" - often it is way more complex process, and in these cases - provisioning happens on factory-ready devices, before shipping those. If the provisioning fails at some phase - a manufacturer must monitor OTP write status explicitly: did that already happen? If yes - we should skip the OTP write phase (it may corrupt the data). That adds complexity on all levels, and instead of "Just relaunch provisioning" - the manufacturer want to make sure they are not trying to burn through already written memory twice by introducing complex error-handling machinery to prevent a fully manufacturead sample from bricking. I understand you concern here: locking an entire block after writing a single word may be a resource waste, though taking into account the nature of this operation - it is not the highest price for making sure device is not bricked. IT becomes programmer's task to make less commits to the OTP memory, but that is pretty straightforward and native approach. As an option - we may introduce a separate "blind" write-unsafe function that will not be checking the lock, just a blind "write word by given address", and does not lock. We may want to add a comment "use it if you know what you are doing". Adding a "locked" check to reading function is not recommended: it may hit back once already provisioned (and non-locked) devices start using this api. |
|
@liam-geotab @darrylring @mykhailosopiha I think having it integrated on efuse has another advantage: it makes it possible to use OTP from userspace, that is not possible with this current implementation. @mykhailosopiha the STM32H5 is ECC granule = 16 bits, you can do a test: try to write only at 0x08FFF000 and later 0x08FFF004 (you can write 0x08FFF000 and 0x08FFF002 too, you just can't write 0x08FFF000 and later 0x08FFF001). And there is another way to make sure we are not corrupting our data: before attempting to write, read the content if it is 0xFFFF you can write that position without damaging the OTP. @liam-geotab we can merge this PR, but we need Documentation about this feature, otherwise it becomes "Another Hidden Feature of NuttX". |
acassis
left a comment
There was a problem hiding this comment.
@liam-geotab please add some Documentation explaining how to use it, like you added on the Summary testing. It is important to be at STM32H5 documentation, because people will not find this PR easily in the future
|
The STM32H5 will allow you to re-write the same word in OTP but the second write will corrupt the data and ECC. Data corruption is bad, of course, but the ECC corruption (by default) triggers NMIs. The ways to prevent this are:
I do like the idea of checking for 0xFFFF. But then you also hit ECC NMIs by default. The block read/write is definitely the safest approach, but pretty much incompatible with being able to write individual words. It's incompatible with eFuse write, too. Though eFuse read could still be implemented? We were originally intending to write individual words, and so can't really make use of this driver as is. I'm rethinking that, though. |
@darrylring did you try writing adjacent blocks of 4 bytes (32 bits) ? I think it will not raise ECC errors |
It is possible to write adjacent bytes, I did that when was implementing these changes - absolutely fine (If my memory is not playing jokes with me). I guess providing access to user space per-word write is a valuable feature, and api like "write a word" is a good way to give user full control over the content of the OTP. Thou - pay the price of managing that access when doing write. TLDR: In my opinion both APIs should be there - they serve different goals. A driver-like API to read/write words should exist with a "how to not brick your memory" guide. I also believe that having a smart API that does "Write this data chunk to this OTP address and make sure data is not corrupted" should also be present - it resolves pretty complex multi-level error handling under the hood. |
|
Thoughts on having separate configuration options for block read/write and word read/write? Both options have obvious caveats which should definitely be documented, but I think both are useful. |
Add an API for OTP (one-time programmable) memory on stm32h5. There are OTP APIs for non-STM32 platforms. There is no other API for STM32 so far with these names. Implement it in stm32h563xx_flash.c since the progmem abstraction also lives there. int stm32_otp_write(const uint16_t *data, uint16_t len, uint32_t offset); int stm32_otp_read(uint16_t *data, uint16_t len, uint32_t offset); The API allows cross-block reads/writes that don't necessarily start/end at block boundaries. The type of `data` is uint16_t * to express to the caller that the pointer should be 2-aligned. The natural size of OTP words is 16 bits. `len` is uint16_t for no strong reason. Preserve author's work. uint32_t stm32_otp_getlockstatus(void); Get a mask of blocks that are locked. A block being locked is considered as being one-time programmed. Co-authored-by: Mykhailo Sopiha <mykhailosopiha@geotab.com> Signed-off-by: Liam Howatt <liamhowatt@geotab.com>
9e50e40 to
485b5af
Compare
|
I have documented this OTP interface matter-of-factly. @darrylring is there anything in this version that you really want to see changed? Can you foresee anything that forbids wrapping it in the efuse driver in a later PR? I would encourage adding it if you have not been disheartened. I'm on board with your idea for a configuration option that allows multi-write. The current API is opaque enough to support that. The efuse interface could |
Describe the OTP API in stm32h5 platform documentation. Signed-off-by: Liam Howatt <liamhowatt@geotab.com>
485b5af to
f2cce24
Compare
I agree - both are needed. I like what @liam-geotab suggested - the eFuse driver may have it's default per-word read/write, and it may include block read/write API if configured. |
|
Agreed. I will rework my branch based on these changes. |
|
There's probably a better place to continue discussing this, but just thinking that for word read/write (without block locking), we can do something like this:
Read can return one of That should hopefully prevent data corruption in most use cases? |
Suggestion: please open an Issue as (Feature/Improvement) and mark the involved people. |
Summary
Add an API for OTP (one-time programmable) memory on stm32h5. There are OTP APIs for non-STM32 platforms. There is no other API for STM32 so far with these names.
Implement it in stm32h563xx_flash.c since the progmem abstraction also lives there.
The API allows cross-block reads/writes that don't necessarily start/end at block boundaries.
The type of
datais uint16_t * to express to the caller that the pointer should be 2-aligned. The natural size of OTP words is 16 bits.lenis uint16_t for no strong reason. Preserve author's work.Get a mask of blocks that are locked. A block being locked is considered as being one-time programmed. In future stm32 platform support, uint32_t may not be sufficient to represent all blocks. This platform has 32 blocks.
The user can be ignorant of the block sizes but they must be aware of the full size of the OTP area and there is no define for it in a public header. If the user writes half of a block, the unwritten half still gets locked, so the user cannot e.g. write the whole OTP area one word at a time, so they actually do need to be aware of the block size.
If these issues are unacceptable, changes should be requested by reviewers. I am favoring preserving the author's work by default.
Impact
If the new STM32 OTP API is bad, there will be a breaking change later (e.g. when OTP is added for other STM32 platforms) to improve it.
The other impact worth noting is that this permanently sets a device's OTP contents irreversibly, if that wasn't clear.
Testing
nucleo-h563zi:nshwithCONFIG_STM32_PROGMEMenabled.(reset)