WIP: stm32/qspi: Detect transfer errors and pass back up to spiflash driver.#6992
WIP: stm32/qspi: Detect transfer errors and pass back up to spiflash driver.#6992dpgeorge merged 2 commits intomicropython:masterfrom
Conversation
|
Thanks, this looks like a good improvement but I guess needs a bit more work before it's mergeable. |
|
Actually this looks quite mergeable as-is, and I think we should put it in (it's not a good idea to silently ignore errors). @andrewleech did you have any further updates to this? |
|
I believe I had this merged in our project and used it as-is. |
| // Read direct from flash for first part | ||
| rest = cache->block * SECTOR_SIZE - addr; | ||
| mp_spiflash_read_data(self, addr, rest, dest); | ||
| ret = mp_spiflash_read_data(self, addr, rest, dest); |
There was a problem hiding this comment.
it should check for ret!=0 and return here, or the loop will keep going
| } | ||
| } | ||
| #endif | ||
| return ret; |
There was a problem hiding this comment.
this should be return 0, in case USE_WR_DELAY is disabled
| // Wait for write to finish | ||
| while (!(QUADSPI->SR & QUADSPI_SR_TCF)) { | ||
| if (QUADSPI->SR & QUADSPI_SR_TEF) { | ||
| ret = -MP_EIO; |
There was a problem hiding this comment.
This can just return -MP_EIO, no need to clear TC because it's not set yet (otherwise this loop would exit). Also TC is cleared on entry to all functions, so it should be OK.
| while (!(QUADSPI->SR & QUADSPI_SR_FTF)) { | ||
| if (QUADSPI->SR & QUADSPI_SR_TEF) { | ||
| ret = -MP_EIO; | ||
| len = 0; |
There was a problem hiding this comment.
len will be decremented below, then this will loop for a very long time
|
For the above comments, I'm fixing them during merge. |
This changes the signatures of QSPI write_cmd_data, write_cmd_addr_data and read_cmd_qaddr_qdata so they return an error code. The softqspi and stm32 hardware qspi driver are updated to follow this new signature. Also the spiflash driver is updated to use these new return values. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
I also fixed this in a second commit. |
3 participants
This is an initial attempt to catch and pass up errors in QSPI Flash transfers.
It only has support for stm32 qspi driver.
The regular SPI driver should also likely be extended to pass up errors (if these are detectable).
I haven't looked at what's needed to make other ports compatible with this.
Also,
qspi_read_cmd()was already directly returning the read value as auint32_t. Returning an error code instead of this is not a good way to handle this, likely the return value should be the error code like other functions here now have and the read value passed back in a pointer.