zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.48k stars 6.41k forks source link

stm32u5: incorrect write width #60724

Closed vaussard closed 1 year ago

vaussard commented 1 year ago

Describe the bug

Hello. I am playing with a nucleo_u575zi_q and noticed that the flash driver did not matched the reference manual when writing. While the manual specifies that only quad-words (128 bits, see Sec. 7.3.7 "Flash main memory programming sequences") can be used, the driver uses double-words (64 bits). The write-block-size is also set to 16 in the device tree, but this parameters is hardcoded to 8 at a couple of places in the driver. I wrote a small test program (see below) to settle who was right and the manual seems correct.

I did a quick workaround (see https://github.com/zephyrproject-rtos/zephyr/commit/7bcb8ebf38d9612511b173bf509e486645ac7e80) and the test program worked as expected. However the L5 seems to be using a width of 64 bits when writing, so a better solution would be needed. Maybe it would be nice to take the write-block-size from the device tree to remove some of the hardcoded constants?

I can propose a better fix if I get some feedback on the desired way to fix this issue.

To Reproduce

See the test program here: https://github.com/zephyrproject-rtos/zephyr/commit/4821b3c65c29956aa3230d1c81bdf1c5c13e4045. It is writing 8-byte and 16-byte blocks of random data in reverse order and reads back to compare. Both flash banks are tested.

Result with master: writing blocks of 8 bytes is allowed, but the readback is either 0xff or wrong. Writing 16 bytes at a time failed in the 1st bank but worked in the 2nd bank (by luck?). Output (edited for brevity):

*** Booting Zephyr OS build zephyr-v3.4.0-1418-g502ecaeac59b ***
Write block size: 16

Testing offset 1040384, write size 8:
---
 - Writing 8 bytes at offset 24
 - Writing 8 bytes at offset 16
 - Writing 8 bytes at offset 8
 - Writing 8 bytes at offset 0
 -> Error [0] = 255 (expected 119)
 -> Error [1] = 255 (expected 233)
(...)
 -> Error [30] = 255 (expected 221)
 -> Error [31] = 255 (expected 193)
FAIL

Testing offset 2088960, write size 8:
---
 - Writing 8 bytes at offset 24
 - Writing 8 bytes at offset 16
 - Writing 8 bytes at offset 8
 - Writing 8 bytes at offset 0
 -> Error [0] = 76 (expected 92)
 -> Error [1] = 145 (expected 39)
(...)
 -> Error [30] = 225 (expected 12)
 -> Error [31] = 65 (expected 222)
FAIL

Testing offset 1040384, write size 16:
---
 - Writing 16 bytes at offset 16
 - Writing 16 bytes at offset 0
 -> Error [16] = 255 (expected 193)
 -> Error [17] = 255 (expected 65)
(...)
 -> Error [30] = 255 (expected 136)
 -> Error [31] = 255 (expected 228)
FAIL

Testing offset 2088960, write size 16:
---
 - Writing 16 bytes at offset 16
 - Writing 16 bytes at offset 0
PASS

Result with my workaround: writing blocks of 8 bytes returns an error. Writing 16 bytes at a time works in both banks. Output:

*** Booting Zephyr OS build zephyr-v3.4.0-1418-g502ecaeac59b ***  
Write block size: 16  

Testing offset 1040384, write size 8:  
---  
- Writing 8 bytes at offset 24  
-> Write error -22  
FAIL  

Testing offset 2088960, write size 8:  
---  
- Writing 8 bytes at offset 24  
-> Write error -22  
FAIL  

Testing offset 1040384, write size 16:  
---  
- Writing 16 bytes at offset 16  
- Writing 16 bytes at offset 0  
PASS  

Testing offset 2088960, write size 16:  
---  
- Writing 16 bytes at offset 16  
- Writing 16 bytes at offset 0  
PASS
erwango commented 1 year ago

@vaussard Thanks for reporting this. Using the device tree constant would be ideal indeed, go ahead.

vaussard commented 1 year ago

Ok I will work on a proper fix and submit a PR to discuss @erwango