On 30.09.21 23:21, Michael Walle wrote: > Am 2021-09-30 18:19, schrieb Frieder Schrempf: >> From: Frieder Schrempf <frieder.schre...@kontron.de> >> >> Currently 'sf update' supports only offsets that are aligned to the >> erase block size of the serial flash. Unaligned offsets result in >> something like: >> >> => sf update ${kernel_addr_r} 0x400 ${filesize} >> device 0 offset 0x400, size 0x11f758 >> SPI flash failed in erase step >> >> In order to support unaligned updates, we simply read the first full >> block and check only the requested part of the block for changes. If >> necessary, the block is erased, the first (unchanged) part of the block >> is written back together with the second part of the block (updated >> data). >> >> Signed-off-by: Frieder Schrempf <frieder.schre...@kontron.de> >> --- >> cmd/sf.c | 17 +++++++++++------ >> 1 file changed, 11 insertions(+), 6 deletions(-) >> >> diff --git a/cmd/sf.c b/cmd/sf.c >> index eac27ed2d7..c54b4b10bb 100644 >> --- a/cmd/sf.c >> +++ b/cmd/sf.c >> @@ -171,30 +171,32 @@ static int do_spi_flash_probe(int argc, char >> *const argv[]) >> static const char *spi_flash_update_block(struct spi_flash *flash, >> u32 offset, >> size_t len, const char *buf, char *cmp_buf, size_t *skipped) >> { >> + u32 offset_aligned = ALIGN_DOWN(offset, flash->sector_size); >> + u32 sector_offset = offset - offset_aligned; >> char *ptr = (char *)buf; >> >> debug("offset=%#x, sector_size=%#x, len=%#zx\n", >> offset, flash->sector_size, len); >> /* Read the entire sector so to allow for rewriting */ >> - if (spi_flash_read(flash, offset, flash->sector_size, cmp_buf)) >> + if (spi_flash_read(flash, offset_aligned, flash->sector_size, >> cmp_buf)) >> return "read"; >> /* Compare only what is meaningful (len) */ >> - if (memcmp(cmp_buf, buf, len) == 0) { >> + if (memcmp(cmp_buf + sector_offset, buf, len) == 0) { >> debug("Skip region %x size %zx: no change\n", >> offset, len); >> *skipped += len; >> return NULL; >> } >> /* Erase the entire sector */ >> - if (spi_flash_erase(flash, offset, flash->sector_size)) >> + if (spi_flash_erase(flash, offset_aligned, flash->sector_size)) >> return "erase"; >> /* If it's a partial sector, copy the data into the temp-buffer */ >> if (len != flash->sector_size) { >> - memcpy(cmp_buf, buf, len); >> + memcpy(cmp_buf + sector_offset, buf, len); >> ptr = cmp_buf; >> } >> /* Write one complete sector */ >> - if (spi_flash_write(flash, offset, flash->sector_size, ptr)) >> + if (spi_flash_write(flash, offset_aligned, flash->sector_size, ptr)) >> return "write"; >> >> return NULL; >> @@ -230,7 +232,10 @@ static int spi_flash_update(struct spi_flash >> *flash, u32 offset, >> ulong last_update = get_timer(0); >> >> for (; buf < end && !err_oper; buf += todo, offset += todo) { >> - todo = min_t(size_t, end - buf, flash->sector_size); >> + if (offset % flash->sector_size) > > do_div() to avoid linking errors on some archs, I guess.
Ok, will fix it. > >> + todo = flash->sector_size - (offset % >> flash->sector_size); > > This is missing the end-buf calculation, no? I.e. if you update just one > sector at an offset and the data you write is smaller than "sector_size > - offset". Indeed, thanks for catching this. > >> + else >> + todo = min_t(size_t, end - buf, flash->sector_size); >> if (get_timer(last_update) > 100) { >> printf(" \rUpdating, %zu%% %lu B/s", >> 100 - (end - buf) / scale, > > -michael