diff --git a/src/avr.c b/src/avr.c index 99f5e786..8f7a0e2b 100644 --- a/src/avr.c +++ b/src/avr.c @@ -641,6 +641,46 @@ int avr_bitmask_data(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, return data; } +// Can pgm->write_byte() be skipped, eg, if the wanted data are already there? +int avr_can_skip_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, + unsigned long addr, uint8_t wanted, int *readrc) { + + if(readrc) + *readrc = -99; + + /* + * Check to see if the write is necessary by reading the existing value and + * only write if we are changing the value; we can't use this optimization + * for paged addressing nor for IO memory (as it's not necessarily storage). + * + * For mysterious reasons, on the AT90S1200, this read operation sometimes + * causes the high byte of the same word to be programmed to the value of + * the low byte that has just been programmed before. Avoid that + * optimization on this device. + */ + if(mem->paged || mem_is_io(mem) || (p->flags & AVRPART_IS_AT90S1200)) + return 0; + + // Don't skip write when programming a bootloader + if(is_spm(pgm)) + return 0; + + // Unclear whether this optimisation would work for TPI parts + if(is_tpi(p)) + return 0; + + unsigned char is; + int rc = avr_read_byte_silent(pgm, p, mem, addr, &is); + if(readrc) + *readrc = rc; + if(rc < 0) + return 0; + + int bm = avr_mem_bitmask(p, mem, addr); + + return (is & bm) == (wanted & bm); +} + int avr_write_byte_default(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, unsigned char data) { @@ -653,7 +693,6 @@ int avr_write_byte_default(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM int ready; int tries; unsigned long start, now; - unsigned char b; unsigned short caddr; OPCODE *writeop; int rc; @@ -730,29 +769,19 @@ int avr_write_byte_default(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM goto success; } - int bm = avr_mem_bitmask(p, mem, addr); - - if(!mem->paged && (p->flags & AVRPART_IS_AT90S1200) == 0) { - /* - * Check to see if the write is necessary by reading the existing value and - * only write if we are changing the value; we can't use this optimization - * for paged addressing. - * - * For mysterious reasons, on the AT90S1200, this read operation sometimes - * causes the high byte of the same word to be programmed to the value of - * the low byte that has just been programmed before. Avoid that - * optimization on this device. - */ - rc = pgm->read_byte(pgm, p, mem, addr, &b); + // Check to see if writing could be skipped + int rbrc, skip = avr_can_skip_write_byte(pgm, p, mem, addr, data, &rbrc); + if(rbrc != -99) { // read_byte() was executed + rc = rbrc; if(rc != 0) { if(rc != -1) { rc = -2; goto rcerror; } - // Read operation is not support on this memory + // Read operation is not supported on this memory } else { readok = 1; - if((b & bm) == (data & bm)) + if(skip) goto success; } } @@ -796,6 +825,7 @@ int avr_write_byte_default(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM goto success; } + int bm = avr_mem_bitmask(p, mem, addr); tries = 0; ready = 0; while(!ready) { @@ -882,6 +912,31 @@ rcerror: return rc; } +// Update one byte of data at the specified address, ie, write unless already there +int avr_update_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, + unsigned long addr, unsigned char data) { + + if(pgm->write_byte == avr_write_byte_default) // avr_write_byte_default() already updates + return avr_write_byte(pgm, p, mem, addr, data); + + pmsg_debug("%s(%s, %s, %s, %s, 0x%02x)\n", __func__, pgmid, p->id, mem->desc, + str_ccaddress(addr, mem->size), data); + + if(avr_can_skip_write_byte(pgm, p, mem, addr, data, NULL)) + return 0; + + if(mem_is_readonly(mem) || (pgm->readonly && pgm->readonly(pgm, p, mem, addr))) { + pmsg_error("cannot write to %s memory %s of %s\n", + mem_is_readonly(mem)? "read-only": "write-protected", mem->desc, p->desc); + return -1; + } + + if(!(p->prog_modes & (PM_UPDI | PM_aWire))) // Initialise unused bits in classic & XMEGA parts + data = avr_bitmask_data(pgm, p, mem, addr, data); + + return pgm->write_byte(pgm, p, mem, addr, data); +} + // Write a byte of data at the specified address int avr_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, unsigned char data) { @@ -889,13 +944,12 @@ int avr_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, pmsg_debug("%s(%s, %s, %s, %s, 0x%02x)\n", __func__, pgmid, p->id, mem->desc, str_ccaddress(addr, mem->size), data); - if(mem_is_readonly(mem)) { - unsigned char is; - - if(pgm->read_byte(pgm, p, mem, addr, &is) >= 0 && is == data) + if(mem_is_readonly(mem) || (pgm->readonly && pgm->readonly(pgm, p, mem, addr))) { + if(avr_can_skip_write_byte(pgm, p, mem, addr, data, NULL)) return 0; - pmsg_error("cannot write to read-only memory %s of %s\n", mem->desc, p->desc); + pmsg_error("cannot write to %s memory %s of %s\n", + mem_is_readonly(mem)? "read-only": "write-protected", mem->desc, p->desc); return -1; } @@ -1182,7 +1236,7 @@ int avr_write_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, int continue; if(do_write) { - if(avr_write_byte(pgm, p, m, i, data)) { + if(avr_update_byte(pgm, p, m, i, data)) { msg_error(" *** failed\n"); led_set(pgm, LED_ERR); goto error; @@ -1254,7 +1308,7 @@ int avr_mem_bitmask(const AVRPART *p, const AVRMEM *mem, int addr) { return bitmask; } -// Bitmask for ISP programming (classic parts only) +// Bitmask for verification after ISP programming (classic parts only) static uint8_t get_fuse_bitmask(const AVRMEM *m) { int ret = 0xFF; diff --git a/src/avrcache.c b/src/avrcache.c index 82022c26..639108e5 100644 --- a/src/avrcache.c +++ b/src/avrcache.c @@ -76,7 +76,7 @@ * avr_chip_erase_cached() erases the chip and discards pending writes() to * flash or EEPROM. It presets the flash cache to all 0xff alleviating the * need to read from the device flash. However, if the programmer serves - * bootloaders is_spm(pgm) then the flash cache is reset + * bootloaders, recognised by is_spm(pgm), then the flash cache is reset * instead, necessitating flash memory be fetched from the device on first * read; the reason for this is that bootloaders emulate chip erase and they * won't overwrite themselves (some bootloaders, eg, optiboot ignore chip @@ -142,8 +142,20 @@ int avr_has_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *me mem->size > 0 && mem->size%mem->page_size == 0 && mem_is_paged_type(mem) && !(p && avr_mem_exclude(pgm, p, mem)); } -#define fallback_read_byte (pgm->read_byte != avr_read_byte_cached? led_read_byte: avr_read_byte_default) -#define fallback_write_byte (pgm->write_byte != avr_write_byte_cached? led_write_byte: avr_write_byte_default) +static int read_byte_error(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, uint8_t *value) { + pmsg_error("pgm->read_byte must not be pointing to avr_read_byte_cached()\n"); + return -1; +} + +static int write_byte_error(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, uint8_t data) { + pmsg_error("pgm->write_byte must not be pointing to avr_write_byte_cached()\n"); + return -1; +} + +// Sanity only: guard against a user setting pgm->xyz_byte to avr_xyz_read_byte_cached (AVRDUDE doesn't) +#define fallback_read_byte (pgm->read_byte != avr_read_byte_cached? led_read_byte: read_byte_error) +#define fallback_write_byte (pgm->write_byte != avr_write_byte_cached? led_write_byte: write_byte_error) +#define fallback_update_byte (pgm->write_byte != avr_write_byte_cached? led_update_byte: write_byte_error) /* * Read the page containing addr from the device into buf @@ -670,7 +682,7 @@ int avr_write_byte_cached(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM // Use pgm->write_byte() if not flash/EEPROM/bootrow/usersig or no paged access if(!avr_has_paged_access(pgm, p, mem)) - return fallback_write_byte(pgm, p, mem, addr, data); + return fallback_update_byte(pgm, p, mem, addr, data); // If address is out of range synchronise caches with device and return whether successful if(addr >= (unsigned long) mem->size) diff --git a/src/leds.c b/src/leds.c index f63da62d..25571f9d 100644 --- a/src/leds.c +++ b/src/leds.c @@ -196,11 +196,53 @@ int led_chip_erase(const PROGRAMMER *pgm, const AVRPART *p) { return rc; } +// Programmer specific update byte function with ERR/PGM LED info (ie, only write if data not there) +int led_update_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, unsigned long addr, unsigned char value) { + if(pgm->write_byte == avr_write_byte_default) // avr_write_byte_default() already updates + return led_write_byte(pgm, p, m, addr, value); + + pmsg_debug("%s(%s, %s, %s, %s, 0x%02x)\n", __func__, pgmid, p->id, m->desc, str_ccaddress(addr, m->size), value); + + if(avr_can_skip_write_byte(pgm, p, m, addr, value, NULL)) + return 0; + + if(mem_is_readonly(m) || (pgm->readonly && pgm->readonly(pgm, p, m, addr))) { + pmsg_error("cannot write to %s memory %s of %s\n", + mem_is_readonly(m)? "read-only": "write-protected", m->desc, p->desc); + return -1; + } + + if(!(p->prog_modes & (PM_UPDI | PM_aWire))) // Initialise unused bits in classic & XMEGA parts + value = avr_bitmask_data(pgm, p, m, addr, value); + + led_clr(pgm, LED_ERR); + led_set(pgm, LED_PGM); + + int rc = pgm->write_byte(pgm, p, m, addr, value); + + if(rc < 0) + led_set(pgm, LED_ERR); + led_clr(pgm, LED_PGM); + + return rc; +} + // Programmer specific write byte function with ERR/PGM LED info int led_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, unsigned long addr, unsigned char value) { + pmsg_debug("%s(%s, %s, %s, %s, 0x%02x)\n", __func__, pgmid, p->id, m->desc, str_ccaddress(addr, m->size), value); - if(mem_is_readonly(m)) - return pgm->write_byte(pgm, p, m, addr, value); + if(mem_is_readonly(m) || (pgm->readonly && pgm->readonly(pgm, p, m, addr))) { + if(avr_can_skip_write_byte(pgm, p, m, addr, value, NULL)) + return 0; + + pmsg_error("cannot write to %s memory %s of %s\n", + mem_is_readonly(m)? "read-only": "write-protected", m->desc, p->desc); + return -1; + } + + if(pgm->write_byte != avr_write_byte_default) + if(!(p->prog_modes & (PM_UPDI | PM_aWire))) // Initialise unused bits in classic & XMEGA parts + value = avr_bitmask_data(pgm, p, m, addr, value); led_clr(pgm, LED_ERR); led_set(pgm, LED_PGM); @@ -215,13 +257,11 @@ int led_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, uns } // Programmer specific read byte function with ERR/PGM LED info -int led_read_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, - unsigned long addr, unsigned char *valuep) { - +int led_read_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, unsigned long addr, unsigned char *valp) { led_clr(pgm, LED_ERR); led_set(pgm, LED_PGM); - int rc = pgm->read_byte(pgm, p, m, addr, valuep); + int rc = pgm->read_byte(pgm, p, m, addr, valp); if(rc < 0) led_set(pgm, LED_ERR); diff --git a/src/libavrdude.h b/src/libavrdude.h index 2c1e1c6f..4be2bfff 100644 --- a/src/libavrdude.h +++ b/src/libavrdude.h @@ -1051,7 +1051,7 @@ typedef struct programmer { int (*write_byte)(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, unsigned long addr, unsigned char value); int (*read_byte)(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, - unsigned long addr, unsigned char *value); + unsigned long addr, unsigned char *valp); int (*read_sig_bytes)(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m); int (*read_sib)(const PROGRAMMER *pgm, const AVRPART *p, char *sib); int (*read_chip_rev)(const PROGRAMMER *pgm, const AVRPART *p, unsigned char *chip_rev); @@ -1168,14 +1168,18 @@ extern "C" { uint64_t avr_mstimestamp(void); double avr_timestamp(void); void init_cx(PROGRAMMER *pgm); - int avr_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, - unsigned long addr, unsigned char data); int avr_read_byte_silent(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, unsigned char *datap); int avr_bitmask_data(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, unsigned char data); + int avr_can_skip_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, + unsigned long addr, uint8_t wanted, int *readrc); int avr_write_byte_default(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, unsigned long addr, unsigned char data); + int avr_update_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, + unsigned long addr, unsigned char data); + int avr_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, + unsigned long addr, unsigned char data); int avr_write_mem(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *mem, int size, int auto_erase); int avr_write(const PROGRAMMER *pgm, const AVRPART *p, const char *memstr, int size, int auto_erase); int avr_signature(const PROGRAMMER *pgm, const AVRPART *p); @@ -1743,10 +1747,12 @@ extern "C" { int led_set(const PROGRAMMER *pgm, int led); int led_clr(const PROGRAMMER *pgm, int led); int led_chip_erase(const PROGRAMMER *pgm, const AVRPART *p); + int led_update_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, + unsigned long addr, unsigned char value); int led_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, unsigned long addr, unsigned char value); int led_read_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, - unsigned long addr, unsigned char *value); + unsigned long addr, unsigned char *valp); int led_paged_write(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, unsigned int page_size, unsigned int addr, unsigned int n); int led_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRMEM *m, diff --git a/src/update.c b/src/update.c index 0fb5ac2c..d03e0c3f 100644 --- a/src/update.c +++ b/src/update.c @@ -529,7 +529,7 @@ static int update_avr_write(const PROGRAMMER *pgm, const AVRPART *p, const AVRME if(rc < 0) return -1; - // @@@ has there has been output in the meantime to make the ", x bytes written" look out of place? + // @@@ has there been output in the meantime to make the ", x bytes written" look out of place? if(pbar && !(flags & UF_VERIFY)) pmsg_info("%d byte%s of %s written", fs.nbytes, str_plural(fs.nbytes), m_name); else if(!pbar) diff --git a/tools/test8 b/tools/test8 index e4fa6f8c..71c6d1e9 100755 --- a/tools/test8 +++ b/tools/test8 @@ -14,7 +14,7 @@ Usage() { cat <] Function: test AVRDUDE v 8.0 or later with -c programmer -p part; - also leaves a file bak---.hex for inspection Options: -i Write human-readable contents seeded by to part and exit -r Write random contents seeded by to part and exit