rewrite of stlinkv2 driver - take 2

stefaandesmet2003 commented 3 years ago

Hi, for you to consider this PR. It's the original work from @mjagdis (PR #108) , updated for automatic merge in current repository.

Modifications in this PR :

adds SWIM high-speed mode, resulting in faster read & write times
on write, only rewrites changes to flash. improves flash longevity and programming times
uses fast block programming (avoid unnecessary erase) if flash block is already erased (all zeroes)
replaced the fixed number of retries with a dynamic mechanism based on the read status from the stlinkv2. And removes a number of undocumented delays. This will improve the read/write times and timeout behaviour in general.
adds a debugging mode (make DEBUG=1). This can help identify the cause of issues opened on this repo. (basically you can ask the issue author to post a log of the debug info to have a better understanding of his issue)

For info, the flash write time is at least 4x faster (1s vs 4s for a 8k flash) than the current base repo version.

Because of the substantial changes, it might be useful to perform some tests on different STM8S & STM8L to confirm this change doesn't break any support. But i hope you will find the changes interesting enough to consider. Cheers, Stefaan

spth commented 3 years ago

I hope to find time for testing next week.

spth commented 3 years ago

I started with some very basic testing: writing the flash of an STM8AF5288 using the integrated ST-Link/v2 of the STM8A-Discovery. So far it looks good. For a 2KB "Hello, world", stm8flash from the branch takes about 70 ms (about 600 ms in master). For a 19 KB stdcbench 0.7, stm8flash from the branch takes about 500 ms (about 4400 ms in master). In the past, I've sometimes run a large number of experiments on STM8 boards, which took days (compiling the code, uploading, running, processing the results). The stm8flash from the branch could have saved over an hour there. It also helps with quick turnaround times during development.

spth commented 3 years ago

How much testing have you done already? Which STM8 devices, which ST-Link/v2 (or 2.1 or 3), which memory (flash, eeprom, ram, opt)?

spth commented 3 years ago

Two more good results: Simple timer / LED blink examples (just 110B) written to flash using the integrated Nucleo board ST-Link/v2.1 on STM8L152R8 and STM8S208RB both work.

I did however, also find a problem: When specifying a wrong device (tested using ST-Link/v2.1 with -p STM8L152R8 when the hardware is STM8S208RB) stm8flash from the branch hangs. With master we get an error message ("Tries exceeded") instead. While that error message is a bit non-descriptive, it still seems far better than hanging to me.

stefaandesmet2003 commented 3 years ago

How much testing have you done already? Which STM8 devices, which ST-Link/v2 (or 2.1 or 3), which memory (flash, eeprom, ram, opt)?

I did some basic testing on STM8S103 & STM8S105 with a clone ST-Link/v2. Basic, meaning write & read/verify working bin files. I didn't try with invalid input parameters, or corner cases on opt bytes etc. I did write all zeroes & all ones to flash & eeprom to test writing speed.

stefaandesmet2003 commented 3 years ago

I started with some very basic testing: writing the flash of an STM8AF5288 using the integrated ST-Link/v2 of the STM8A-Discovery. So far it looks good. For a 2KB "Hello, world", stm8flash from the branch takes about 70 ms (about 600 ms in master). For a 19 KB stdcbench 0.7, stm8flash from the branch takes about 500 ms (about 4400 ms in master). In the past, I've sometimes run a large number of experiments on STM8 boards, which took days (compiling the code, uploading, running, processing the results). The stm8flash from the branch could have saved over an hour there. It also helps with quick turnaround times during development.

Thanks for testing. This driver will only rewrite flash blocks that have changed. So repeatedly writing the same bin file will only read the flash. But for turnaround time during development it's definitely a good thing to have.

stefaandesmet2003 commented 3 years ago

Two more good results: Simple timer / LED blink examples (just 110B) written to flash using the integrated Nucleo board ST-Link/v2.1 on STM8L152R8 and STM8S208RB both work.

I did however, also find a problem: When specifying a wrong device (tested using ST-Link/v2.1 with -p STM8L152R8 when the hardware is STM8S208RB) stm8flash from the branch hangs. With master we get an error message ("Tries exceeded") instead. While that error message is a bit non-descriptive, it still seems far better than hanging to me.

I assume the hangup was on flash reading? (could you maybe paste some debug output?) I believe the reason for your problem is that there is currently no fallback/timeout in msg_rcv if stlink returns no data or not enough data :

static void msg_recv(programmer_t *pgm, unsigned char *buf, unsigned int length) {
    int n;

    while (length > 0) {
        n = msg_transfer(pgm, buf, length, LIBUSB_ENDPOINT_IN);
        length -= n;
        buf += n;

        if (length > 0) {
            DEBUG_PRINT("    short read - %d bytes more needed\n", length);
            usleep(1000000);
        }
    }
}

I will rework: the READ_STATUS command to stlink returns the number of bytes available for the subsequent READ_BUF command, but is currently not used. So we'll use that for msg_recv (and only read the actual number of bytes available, instead of hanging) and return that value to main.c which already handles this type of error. I believe this way there won't be a need for additional timeout mechanism like the original TRY() do you agree?

spth commented 3 years ago

I had tried to write. But, as you mentioned, writing would do a read first to check if the block really needs to be written, so it might have happened then.

stefaandesmet2003 commented 3 years ago

I had tried to write. But, as you mentioned, writing would do a read first to check if the block really needs to be written, so it might have happened then.

Did a quick test, and my guess above was wrong. the problem is that you specify a device with a wrong flash block size. The device waits for all bytes in the block to be sent before starting the erase/flash cycle, while the driver hangs waiting for the FLASH_IAPSR.EOP bit to be set. I have reproduced the same when trying to flash S105 (block size 128) with -p stm8s103?3 (block size 64): driver sends 64 bytes and waits for the EOP flag, and device waits for another 64 bytes.. Master branch has a timeout there in stlink2_wait_until_transfer_completes. Will push an update soon.

spth commented 3 years ago

My original plan was to do a bit more testing, but I might not find time. And today, I didn't find my ST-Link/v3 either. So I only did another few quick tests using an ST-Link/V2 instead.

stefaandesmet2003 commented 3 years ago

Ok thanks! I'm still here if issues should arise from this merge.

spth commented 3 years ago

We have a user report that this broke reset on ST-Link V3 for them:

https://github.com/vdudouyt/stm8flash/issues/148

stefaandesmet2003 commented 3 years ago

do you happen to have a document about the stlinkv3 usb interface protocol?

vdudouyt / stm8flash

rewrite of stlinkv2 driver - take 2 #144