esp8266 / Arduino

ESP8266 core for Arduino
GNU Lesser General Public License v2.1
16.05k stars 13.33k forks source link

Puya flash chips and gzip binaries #7169

Closed arendst closed 4 years ago

arendst commented 4 years ago

I have a Digoo DG-SP-202 and a Neo both with a puya flash chip id (0x146085) which seem to fail to succesfully OTA a gzipped binary.

The upload goes fine but after a reboot it still comes back with the previous code version.

Is the puya flash fix integrated in the eboot gzipped code?

devyte commented 4 years ago

The PUYA support was implemented in the core, not in the bootloader. I don't really know how the current incarnation would work from the bootloader, unless the bootloader reuses the same flash read/write functions as the core, which I highly doubt is the case. Also, the bootloader sector is almost full, so moving the functions in there isn't straightforward. OTOH, I seem to remember that the bootloader erases the flash before the decompression/copy-over, which if true would mean that no specific support is needed. @TD-er you implemented the current version of the PUYA patch. Can you please take a look?

TD-er commented 4 years ago

I have not yet looked in the gzip bootloader code, so I don't know yet where it does try to save data. I can imagine not only the extraction part does perform write actions, but maybe also some flags set during upload and at boot? The Puya chips have the nasty habit of writing exactly what's been told to write, so small optimizations in the code which just set a single bit to 0 instead of reading, adapting in memory and writing.

It would be an ideal use case to support the Puya chips also, as those are the 1M ones, which would benefit the most from Gzip uploads.

devyte commented 4 years ago

@TD-er it should be enough to just analyze the flash write function to figure out what's being used. If the same hijack as the core is needed in the bootloader as well, then:

TD-er commented 4 years ago

Strictly speaking the write part in void handle_flash_data(void *data_buf, uint32_t length) should work as it tries to erase the sector and writes it. I do see they use different write function signatures and I can't see the internals of those functions. SPIWrite vs. spi_flash_write

But the main difference here may be that the Puya flash write does perform a read before a write and the firmware update routine does only start a next erase and waits until spiflash_is_ready() But I don't know how the PUYA flash does react to that.

Strictly speaking, the Puya flash does not need a call to erase the flash first, when writing data to it. As long as you don't assume only changes from 1 to 0 will be written. So a very simple test could be to disable the erase here when it is Puya flash. That should hardly increase the size of the bootloader as it is only a single read + if statement.

This short investigation is only on the writing part of the firmware. I don't know if somewhere a state is kept on the flash, either during the upload phase, or the extraction phase. If so, then you really must make sure to write the data to this address verbatim as you want it to end up on flash and not just set a single bit to 0 and write it.

Does the boot sequence also assume the last used block of the sketch to be erased? If so, then that may need to be written with 0xFF on Puya chips. For example, is the checksum computed over some block size, or only on the bytes used? Maybe the update is never started if the uploaded firmware is not matching the checksum?

arendst commented 4 years ago

I did some further testing and it seems it's a kind of gamble if it succesfully updates. It sometimes finishes the gzip upload just fine.

I suggest to close this issue for now and let's see once 2.7.0 is released if other users have the same experience as I did.

For all non Puya chips I love the new gzip feature enabling much functionality in one OTA go.

TD-er commented 4 years ago

Maybe you could check the file size of the files that succeed and the ones that fail. Maybe the successful ones have a filesize multiple of 4 bytes?