vdudouyt / stm8flash

program your stm8 devices with SWIM/stlinkv(1,2)
GNU General Public License v2.0
401 stars 182 forks source link

stlinkv2 new driver bug fix #146

Closed stefaandesmet2003 closed 2 years ago

stefaandesmet2003 commented 2 years ago

Hi, I encountered a problem with PR #144 that I submitted earlier. The code raises a swim error 0x05 when writing a series of option bytes with high speed swim. With USE_HIGH_SPEED disabled the error doesn't occur. It is fixed by checking for the EOP bit after each byte flashed. The problem doesn't occur with flash & eeprom memtypes because these writes are handled by block programming and EOP is (already) checked after writing each block. I propose 2 more changes here : -> a dedicated error message (instead of 'tries exceeded') when the flashing fails due to an active write protection (FLASH_IAPSR.WR_PG_DIS bit set). On my test boards this bit is also set when flash programming is attempted under read-out protection. Not sure if this is the case for all STM8 types. -> the code in PR #144 waits for SWIM_CSR.HSIT bit to change to 1 and activate high speed swim. During testing it happened that SWIM_CSR.HSIT remained 0, and then the original code hangs. It is updated here with a retry mechanism; if SWIM_CSR.HSIT remains 0, code continues in low speed swim. I haven't figured out under which circumstances SWIM_CSR.HSIT can remain 0, it is somehow linked to bits in the option bytes, or after a mass erase on removing the ROP.

I would love to include an error/warning message for an active read-out protection, but need your advice. Currently stm8flash will read data from a ROP device without throwing an error, but the returned data are incorrect. According to UM0470 SWIM_CSR.NO_ACCESS may indicate a read-out protection, but not on all STM8 types (not on STM8S103/S105 at least). On STM8S103 & STM8S105 with ROP I read 0x71 on all flash/eeprom bytes. Is this true for other/all types? Other suggestions?

I'm sorry for the trouble, I start to regret taking over the code in PR #108 ..

spth commented 2 years ago

Sorry for taking so long to reply. I just did some very basic testing (on the STM8A-Discovery - STM8AF5288 via integrated ST-Link/V2), and it looks ok.

Regarding the NO_ACCESS bit: I suggest to return an error message when the bit is set on a read attempt (maybe "Bus not accessible - device in HALT, WFI or readout protection"). Does the overwrite-only-if-changed functionality work correctly with ROP active (I've never used ROP so far)?

If you want, I could run some experiments on other STM8 types. On the ST forums, someone reports that the STM8S003 with readout protection active reads all bytes as 0x71, too. We might want to check some STM8AF, STM8AL, STM8L, and preferably also devices with different blocksize. Either way, I think it makes sense to check for NO_ACCESS first and only look at values as a fallback (maybe if the readout is all 0x71 print a warning, that this might indicate an active readout protection). We could also try to read the option byte for memory protection. For non-present ROP, it is most likely 0. With ROP on, we can't read it, but if we get a nonzero value reliably, that might be enough justification to print a warning. Or just read all option bytes, and print the warning if they all have the same value. But I don't know yet of a really reliable way to check for readout protection either.

spth commented 2 years ago

I can't find 0x71 among the opcodes. Looks like no such instruction exists, which might be the reason ST chose that value to return when readout protection is enabled.

Maybe we could read CFG_GCR; according to the STM8S / STM8AF manual, the lowest bit is 0 when SWIM is enabled, so I think that bit could never read 1 when using SWIM.

stefaandesmet2003 commented 2 years ago

clever idea about using CFG_GCR. I tested, unfortunately it reads 0 both with & without ROP. Also the GPIO range & unique id return valid data. But reading 4 bytes at 0x8000 will work. if it returns 0x71717171, it is an invalid reset vector. Reading the option byte is a good alternative, but it's not 100% proof. If it reads 0x71 it might have be set deliberately to this value and then ROP isn't active.