From 25ca91371d6d856cf274ef655d8315c624c81f52 Mon Sep 17 00:00:00 2001 From: Stefan Rueger Date: Tue, 22 Nov 2022 21:32:42 +0000 Subject: [PATCH 1/4] Pad pages with input file contents before avr_write() --- src/avr.c | 109 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 87 insertions(+), 22 deletions(-) diff --git a/src/avr.c b/src/avr.c index dee653c0..a9b607ae 100644 --- a/src/avr.c +++ b/src/avr.c @@ -828,12 +828,13 @@ int avr_write_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, int wsize = m->size; if (size < wsize) { wsize = size; - } - else if (size > wsize) { + } else if (size > wsize) { pmsg_warning("%d bytes requested, but memory region is only %d bytes\n", size, wsize); imsg_warning("Only %d bytes will actually be written\n", wsize); } + if(wsize <= 0) + return wsize; if ((p->prog_modes & PM_TPI) && m->page_size > 1 && pgm->cmd_tpi) { unsigned int chunk; /* number of words for each write command */ @@ -901,50 +902,114 @@ int avr_write_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, int /* * the programmer supports a paged mode write */ - int need_write, failure; + int need_write, failure, nset; unsigned int pageaddr; unsigned int npages, nwritten; - /* quickly scan number of pages to be written to first */ - for (pageaddr = 0, npages = 0; - pageaddr < wsize; - pageaddr += m->page_size) { - /* check whether this page must be written to */ - for (i = pageaddr; - i < pageaddr + m->page_size; - i++) - if ((m->tags[i] & TAG_ALLOCATED) != 0) { + /* + * Not all paged memory looks like NOR memory to AVRDUDE, particularly + * - EEPROM + * - when talking to a bootloader + * - handling write via a part-programmer combo that can do page erase + * + * Hence, read in from the chip all pages with holes to fill them in. The + * small cost of doing so is outweighed by the benefit of not potentially + * overwriting bytes with 0xff outside the input file. + * + * Also consider that the effective page size for *SPM* erasing of parts + * can be 4 times the page size for SPM writing (eg, ATtiny1634). Thus + * ensure the holes cover the effective page size for SPM programming. + * Benefits -c arduino with input files with holes on 4-page-erase parts. + */ + + AVRMEM *cm = avr_dup_mem(m); + + // Establish and sanity check effective page size + int pgsize = (pgm->prog_modes & PM_SPM) && p->n_page_erase > 0? + p->n_page_erase*cm->page_size: cm->page_size; + if((pgsize & (pgsize-1)) || pgsize < 1) { + pmsg_error("effective page size %d implausible\n", pgsize); + avr_free_mem(cm); + return -1; + } + + uint8_t *spc = cfg_malloc(__func__, cm->page_size); + + // Set cwsize as rounded-up wsize + int cwsize = (wsize + pgsize-1)/pgsize*pgsize; + + for(pageaddr = 0; pageaddr < (unsigned int) cwsize; pageaddr += pgsize) { + for(i = pageaddr, nset = 0; i < pageaddr + pgsize; i++) + if(cm->tags[i] & TAG_ALLOCATED) + nset++; + + if(nset && nset != pgsize) { // Effective page has holes + for(int np=0; np < pgsize/cm->page_size; np++) { // page by page + unsigned int beg = pageaddr + np*cm->page_size; + unsigned int end = beg + cm->page_size; + + for(i = beg; i < end; i++) + if(!(cm->tags[i] & TAG_ALLOCATED)) + break; + + if(i >= end) // Memory page has no holes + continue; + + // Read flash contents to separate memory spc and fill in holes + if(avr_read_page_default(pgm, p, cm, beg, spc) == 0) { + pmsg_notice2("padding %s [0x%04x, 0x%04x]\n", cm->desc, beg, end-1); + for(i = beg; i < end; i++) + if(!(cm->tags[i] & TAG_ALLOCATED)) { + cm->tags[i] |= TAG_ALLOCATED; + cm->buf[i] = spc[i-beg]; + } + } else { + pmsg_notice2("cannot read %s [0x%04x, 0x%04x] to pad page\n", + cm->desc, beg, end-1); + } + } + } + } + + // Quickly scan number of pages to be written to + for(pageaddr = 0, npages = 0; pageaddr < (unsigned int) cwsize; pageaddr += cm->page_size) { + for(i = pageaddr; i < pageaddr + cm->page_size; i++) + if(cm->tags[i] & TAG_ALLOCATED) { npages++; break; } } for (pageaddr = 0, failure = 0, nwritten = 0; - !failure && pageaddr < wsize; - pageaddr += m->page_size) { - /* check whether this page must be written to */ - for (i = pageaddr, need_write = 0; - i < pageaddr + m->page_size; - i++) - if ((m->tags[i] & TAG_ALLOCATED) != 0) { + !failure && pageaddr < (unsigned int) cwsize; + pageaddr += cm->page_size) { + + // Check whether this page must be written to + for (i = pageaddr, need_write = 0; i < pageaddr + cm->page_size; i++) + if ((cm->tags[i] & TAG_ALLOCATED) != 0) { need_write = 1; break; } + if (need_write) { rc = 0; if (auto_erase) - rc = pgm->page_erase(pgm, p, m, pageaddr); + rc = pgm->page_erase(pgm, p, cm, pageaddr); if (rc >= 0) - rc = pgm->paged_write(pgm, p, m, m->page_size, pageaddr, m->page_size); + rc = pgm->paged_write(pgm, p, cm, cm->page_size, pageaddr, cm->page_size); if (rc < 0) /* paged write failed, fall back to byte-at-a-time write below */ failure = 1; } else { - pmsg_debug("avr_write_mem(): skipping page %u: no interesting data\n", pageaddr / m->page_size); + pmsg_debug("avr_write_mem(): skipping page %u: no interesting data\n", pageaddr / cm->page_size); } nwritten++; report_progress(nwritten, npages, NULL); } + + avr_free_mem(cm); + free(spc); + if (!failure) return wsize; /* else: fall back to byte-at-a-time write, for historical reasons */ From b925b5113dd98c48782c9b181e2974b2cc3e92ae Mon Sep 17 00:00:00 2001 From: Stefan Rueger Date: Tue, 22 Nov 2022 21:49:26 +0000 Subject: [PATCH 2/4] Silence compiler warnings --- src/avr.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/src/avr.c b/src/avr.c index a9b607ae..e926cc84 100644 --- a/src/avr.c +++ b/src/avr.c @@ -340,6 +340,10 @@ int avr_read_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, con if (v != NULL) vmem = avr_locate_mem(v, mem->desc); + + if(mem->size < 0) // Sanity check + return -1; + /* * start with all 0xff */ @@ -355,7 +359,7 @@ int avr_read_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, con avr_tpi_setup_rw(pgm, mem, 0, TPI_NVMCMD_NO_OPERATION); /* load bytes */ - for (lastaddr = i = 0; i < mem->size; i++) { + for (lastaddr = i = 0; i < (unsigned long) mem->size; i++) { if (vmem == NULL || (vmem->tags[i] & TAG_ALLOCATED) != 0) { @@ -389,7 +393,7 @@ int avr_read_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, con /* quickly scan number of pages to be written to first */ for (pageaddr = 0, npages = 0; - pageaddr < mem->size; + pageaddr < (unsigned int) mem->size; pageaddr += mem->page_size) { /* check whether this page must be read */ for (i = pageaddr; @@ -406,7 +410,7 @@ int avr_read_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, con } for (pageaddr = 0, failure = 0, nread = 0; - !failure && pageaddr < mem->size; + !failure && pageaddr < (unsigned int) mem->size; pageaddr += mem->page_size) { /* check whether this page must be read */ for (i = pageaddr, need_read = 0; @@ -443,7 +447,7 @@ int avr_read_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, con } } - for (i=0; i < mem->size; i++) { + for (i=0; i < (unsigned long) mem->size; i++) { if (vmem == NULL || (vmem->tags[i] & TAG_ALLOCATED) != 0) { @@ -469,9 +473,9 @@ int avr_read_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, con /* * write a page data at the specified address */ -int avr_write_page(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, - unsigned long addr) -{ +int avr_write_page(const PROGRAMMER *pgm, const AVRPART *p_unused, const AVRMEM *mem, + unsigned long addr) { + unsigned char cmd[4]; unsigned char res[4]; OPCODE * wp, * lext; @@ -718,8 +722,8 @@ int avr_write_byte_default(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM } gettimeofday (&tv, NULL); prog_time = (tv.tv_sec * 1000000) + tv.tv_usec; - } while ((r != data) && - ((prog_time-start_time) < mem->max_write_delay)); + } while (r != data && mem->max_write_delay >= 0 && + prog_time - start_time < (unsigned long) mem->max_write_delay); } /* @@ -865,7 +869,7 @@ int avr_write_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, int wsize = (wsize+chunk-1) / chunk * chunk; /* write words in chunks, low byte first */ - for (lastaddr = i = 0; i < wsize; i += chunk) { + for (lastaddr = i = 0; i < (unsigned int) wsize; i += chunk) { /* check that at least one byte in this chunk is allocated */ for (writeable_chunk = j = 0; !writeable_chunk && j < chunk; j++) { writeable_chunk = m->tags[i+j] & TAG_ALLOCATED; @@ -1023,7 +1027,7 @@ int avr_write_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, int page_tainted = 0; flush_page = 0; - for (i=0; ibuf[i]; report_progress(i, wsize, NULL); @@ -1047,8 +1051,8 @@ int avr_write_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, int } else { page_tainted |= do_write; } - if (i % m->page_size == m->page_size - 1 || - i == wsize - 1) { + if (i % m->page_size == (unsigned int) m->page_size - 1 || + i == (unsigned int) wsize - 1) { /* last byte this page */ flush_page = page_tainted; newpage = 1; From 68c6ffd7fce336548d4113fe4f071c572d138a89 Mon Sep 17 00:00:00 2001 From: Stefan Rueger Date: Thu, 24 Nov 2022 12:14:54 +0000 Subject: [PATCH 3/4] Silence compiler warnings, change comments, remove typos --- src/avrcache.c | 9 ++++++--- src/libavrdude.h | 2 +- src/update.c | 3 +-- src/urclock.c | 4 ++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/avrcache.c b/src/avrcache.c index cca15483..638ac46f 100644 --- a/src/avrcache.c +++ b/src/avrcache.c @@ -135,6 +135,9 @@ int avr_has_paged_access(const PROGRAMMER *pgm, const AVRMEM *mem) { * - Part memory buffer mem is unaffected by this (though temporarily changed) * - Uses read_byte() if memory page size is one, otherwise paged_load() * - Fall back to bytewise read if paged_load() returned an error + * - On failure returns a negative value, on success a non-negative value, which is either + * + The number of bytes read by pgm->paged_load() if that succeeded + * + LIBAVRDUDE_SUCCESS (0) if the fallback of bytewise read succeeded */ int avr_read_page_default(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, int addr, unsigned char *buf) { if(!avr_has_paged_access(pgm, mem) || addr < 0 || addr >= mem->size) @@ -643,8 +646,8 @@ int avr_write_byte_cached(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM // Erase the chip and set the cache accordingly int avr_chip_erase_cached(const PROGRAMMER *pgm, const AVRPART *p) { CacheDesc_t mems[2] = { - { avr_locate_mem(p, "flash"), pgm->cp_flash, 1 }, - { avr_locate_mem(p, "eeprom"), pgm->cp_eeprom, 0 }, + { avr_locate_mem(p, "flash"), pgm->cp_flash, 1, -1, 0 }, + { avr_locate_mem(p, "eeprom"), pgm->cp_eeprom, 0, -1, 0 }, }; int rc; @@ -740,7 +743,7 @@ int avr_page_erase_cached(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM // Free cache(s) discarding any pending writes -int avr_reset_cache(const PROGRAMMER *pgm, const AVRPART *p) { +int avr_reset_cache(const PROGRAMMER *pgm, const AVRPART *p_unused) { AVR_Cache *mems[2] = { pgm->cp_flash, pgm->cp_eeprom, }; for(size_t i = 0; i < sizeof mems/sizeof*mems; i++) { diff --git a/src/libavrdude.h b/src/libavrdude.h index 83556af3..f21bbc9e 100644 --- a/src/libavrdude.h +++ b/src/libavrdude.h @@ -922,7 +922,7 @@ int avr_write_page_default(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM int avr_is_and(const unsigned char *s1, const unsigned char *s2, const unsigned char *s3, size_t n); -// byte-wise cached read/write API +// Bytewise cached read/write API int avr_read_byte_cached(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, unsigned char *value); int avr_write_byte_cached(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, unsigned char data); int avr_chip_erase_cached(const PROGRAMMER *pgm, const AVRPART *p); diff --git a/src/update.c b/src/update.c index 0a7166cd..0afcd9ea 100644 --- a/src/update.c +++ b/src/update.c @@ -560,8 +560,7 @@ int do_op(const PROGRAMMER *pgm, const AVRPART *p, UPDATE *upd, enum updateflags pmsg_info("%d byte%s of %s%s written\n", fs.nbytes, update_plural(fs.nbytes), mem->desc, alias_mem_desc); - // Fall through for (default) auto verify, ie, unless -V was specified - if (!(flags & UF_VERIFY)) + if (!(flags & UF_VERIFY)) // Fall through for auto verify unless -V was specified break; case DEVICE_VERIFY: diff --git a/src/urclock.c b/src/urclock.c index 7bfb2d7f..c15cc665 100644 --- a/src/urclock.c +++ b/src/urclock.c @@ -2260,14 +2260,14 @@ static int urclock_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVR int urclock_write_byte(const PROGRAMMER *pgm_uu, const AVRPART *p_uu, const AVRMEM *mem, unsigned long addr_uu, unsigned char data_uu) { - pmsg_error("bootloader does not implement byte-wise write to %s \n", mem->desc); + pmsg_error("bootloader does not implement bytewise write to %s \n", mem->desc); return -1; } int urclock_read_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, unsigned char *value) { - // Byte-wise read only valid for flash and eeprom + // Bytewise read only valid for flash and eeprom int mchr = avr_mem_is_flash_type(mem)? 'F': 'E'; if(mchr == 'E' && !avr_mem_is_eeprom_type(mem)) { if(!strcmp(mem->desc, "signature") && pgm->read_sig_bytes) { From 8e79b7dc528b97dba14d3da8be2e19f9ad95228c Mon Sep 17 00:00:00 2001 From: Stefan Rueger Date: Thu, 24 Nov 2022 12:16:43 +0000 Subject: [PATCH 4/4] Fix error detection for reading a page to pad unset bytes --- src/avr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/avr.c b/src/avr.c index e926cc84..6d5b975c 100644 --- a/src/avr.c +++ b/src/avr.c @@ -960,7 +960,7 @@ int avr_write_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, int continue; // Read flash contents to separate memory spc and fill in holes - if(avr_read_page_default(pgm, p, cm, beg, spc) == 0) { + if(avr_read_page_default(pgm, p, cm, beg, spc) >= 0) { pmsg_notice2("padding %s [0x%04x, 0x%04x]\n", cm->desc, beg, end-1); for(i = beg; i < end; i++) if(!(cm->tags[i] & TAG_ALLOCATED)) {