#98 Extended erase command doesn't match documentation

[Anders Montonen]

AN4221 describes the erase commands 0x44 as follows:

1. 0x44, 0xbb -> ACK
2. number of pages/mass erase, checksum -> ACK
3. page list, checksum -> ACK

The implementation of stm32_pages_erase combines 2 and 3 above into one message, with only one checksum for the whole packet. The same applies to the no-stretch erase command (0x45).

Is the documentation or the code wrong? I'm implementing the protocol for a device without a ROM I2C bootloader, so I can't verify the actual expected behaviour.

[Anders Montonen]

It seems AN4221 and AN3155 differ in what command 0x44 should look like. The one from AN3155 looks like an older, poorly designed protocol, as it requires you parse the received data in order to know where the checksum is stored, but without validating the checksum (poor as it is) you don't know if the data was valid to begin with.

[Tormod Volden]

AN4221 = "I2C protocol used in the STM32 bootloader"
Figure 14. Erase memory command: host side

AN3155 = "USART protocol used in the STM32 bootloader"
Figure 16. Extended Erase Memory command: host side

I see the difference in the two diagrams. I suppose they cannot be compatible, so if the AN4221 is correct, stm32flash will need a modified algorithm for the I2C case. However I don't know if the current code works or not on I2C.

Maybe Antonio knows if this has been tested.

[Anders Montonen]

I tried with an STM F0 Discovery board (STM32F072) which supports both interfaces, and erasing works fine over USART, but both mass erase and page erase fails over I2C.

[Yann Sionneau]

Hello, I can confirm the I2C bootload protocol implemented is incorrect for erase comands.
I have a fix, but it breaks USART since I didn't know that the current one was the good one for USART.
I'm working on a solution which works for both. I don't have any setup to test I don't break USART case but if someone will be willing to test...
I'll keep you posted as soon as I have a correct fix.

[Yann Sionneau]

Here is the patch. I made special care not to break USART but I don't have the hw for testing.
What I have tested:

• it works well for mass-erase and page-by-page erase over I2C with STM32F723 (it was not working before)

What I didn't test and should be tested before merging:

• That USART flash isn't broken by the patch

Anyone motivated to test the patch for USART use case? :)

PS : patch is done for being applied after this one: https://sourceforge.net/p/stm32flash/tickets/131/#e8d8

[Anonymous]

I can also confirm that the mass (aka extended) erase command 0x44 doesn't work for me over I2C.

It's very easy to implement. The below modification implements the protocol seen in AN4221 (section 2.7 of Rev. 10). However, this almost certainly breaks the USART protocol and possibly some other things, so will need some work and some testing, which I cannot do.

So that this answer can be found on google, the main error I was getting was that trying to write to the chip (in this case the STM32L432KB) resulted in "Page-by-page erase failed." and "Failed to erase memory" error messages, with no further information.

The patch was applied straight on the master repo branch, edc8f2b8

[Tormod Volden]

Thanks for the investigation and patch. About 0002-Fix-bootloader-protocole-for-page-by-page-erase-over.patch:

I don't particularly like stm32_port_is_i2c() matching on name. Can't we just check that port is port_i2c?

I think the error message mentioning the -t option should call it "port timeout" or something, since there are so many different timeouts in play.

Can you please also mention in the commit message what the problem and solution consist of, like a summary of the OP and comments here? It might even be worth mentioning in a code comment that the protocols are different as per the application notes.

*protocol not protocole.

[Tormod Volden]

Oh, I didn't read it carefully enough, I thought it was matching on device node name, and not on an internal name tag, which is OK, I guess.

[Tormod Volden]

Also, why would you get a timeout due to device being busy and stretching the clock after receiving the number of pages? And why wait pages * STM32_PAGEERASE_TIMEOUT at this point? I would think the device gets busy only after receiving the list of pages?

[Tormod Volden]

I have amended Yann's patch accordingly to simply use stm_get_ack() during setup.

I also rebased it to go in before a fix for ticket 131, stripping the timeout comments in the messages for now. Such messages would be part of ticket 131, which is WIP.

The existing code uses a set of flags for each port type instead of explicitly checking for port type in various places, so I introduced a new flag instead of directly checking for I2C port.

This has now been pushed to git, so please confirm that it works. If kernel driver timeout is an issue, please apply the rebased WIP patch in ticket 131.

[Yann Sionneau]

Thanks for the feedback and the merge!
I will try to find some time in the next ~30 days or so to test that it works.
AFAIK I really need to be able to increase the timeout of the i2c master driver (about the -t patch in ticket 131). Let's see when I find the time to test again :)

[Yann Sionneau]

I can confirm that using upstream stm32flash (+ the -t timeout patch) it works to flash my stm32.
So the already merged patch about the i2c protocol is OK!
Thanks!