From 308263043087c15f58ebaa501bd8da69c9188b68 Mon Sep 17 00:00:00 2001 From: Joerg Wunsch Date: Wed, 15 Jun 2022 23:32:22 +0200 Subject: [PATCH 1/4] Replace internal knowledge in jtag3.c by a public API In certain situations (CRC failure, device locked), that JTAG3 read functions need to return an indication to the caller that it is OK to proceed, and allow erasing the device anyway. Historically, the JTAG3 code passed the respective protocol errors directly (and unexplained) up to the caller, leaving the decision to the caller how to handle the situation. Replace that by a more common return value API. New code should prefer this API instead of any hardcoded return values. --- src/avr.c | 12 ++++++------ src/jtag3.c | 31 +++++++++++++++++++++---------- src/jtag3_private.h | 1 + src/libavrdude.h | 10 ++++++++++ src/main.c | 5 ++--- 5 files changed, 40 insertions(+), 19 deletions(-) diff --git a/src/avr.c b/src/avr.c index d6e78bb5..a8945d86 100644 --- a/src/avr.c +++ b/src/avr.c @@ -439,16 +439,16 @@ int avr_read(PROGRAMMER * pgm, AVRPART * p, char * memtype, (vmem->tags[i] & TAG_ALLOCATED) != 0) { rc = pgm->read_byte(pgm, p, mem, i, mem->buf + i); - if (rc != 0) { + if (rc != LIBAVRDUDE_SUCCESS) { avrdude_message(MSG_INFO, "avr_read(): error reading address 0x%04lx\n", i); - if (rc == -1) { + if (rc == LIBAVRDUDE_GENERAL_FAILURE) { avrdude_message(MSG_INFO, " read operation not supported for memory \"%s\"\n", memtype); - return -2; + return LIBAVRDUDE_NOTSUPPORTED; } avrdude_message(MSG_INFO, " read operation failed for memory \"%s\"\n", memtype); - return rc; + return LIBAVRDUDE_SOFTFAIL; } } report_progress(i, mem->size, NULL); @@ -1035,14 +1035,14 @@ int avr_signature(PROGRAMMER * pgm, AVRPART * p) report_progress (0,1,"Reading"); rc = avr_read(pgm, p, "signature", 0); - if (rc < 0) { + if (rc < LIBAVRDUDE_SUCCESS) { avrdude_message(MSG_INFO, "%s: error reading signature data for part \"%s\", rc=%d\n", progname, p->desc, rc); return rc; } report_progress (1,1,NULL); - return 0; + return LIBAVRDUDE_SUCCESS; } static uint8_t get_fuse_bitmask(AVRMEM * m) { diff --git a/src/jtag3.c b/src/jtag3.c index 6d358dfa..23df86ff 100644 --- a/src/jtag3.c +++ b/src/jtag3.c @@ -313,6 +313,16 @@ static void jtag3_prmsg(PROGRAMMER * pgm, unsigned char * data, size_t len) } } +static int jtag3_errcode(int status) +{ + if (status >= LIBAVRDUDE_SUCCESS) + return status; + if (status == RSP3_FAIL_OCD_LOCKED || + status == RSP3_FAIL_CRC_FAILURE) + return LIBAVRDUDE_SOFTFAIL; + return LIBAVRDUDE_GENERAL_FAILURE; +} + static void jtag3_prevent(PROGRAMMER * pgm, unsigned char * data, size_t len) { int i; @@ -850,7 +860,7 @@ int jtag3_recv(PROGRAMMER * pgm, unsigned char **msg) { putc('\n', stderr); avrdude_message(MSG_NOTICE2, "%s: %s command: timeout/error communicating with programmer (status %d)\n", progname, descr, status); - return -1; + return LIBAVRDUDE_GENERAL_FAILURE; } else if (verbose >= 3) { putc('\n', stderr); jtag3_prmsg(pgm, *resp, status); @@ -871,10 +881,11 @@ int jtag3_recv(PROGRAMMER * pgm, unsigned char **msg) { status = (*resp)[3]; free(*resp); resp = 0; - return -status; + + return jtag3_errcode(status); } - return status; + return LIBAVRDUDE_SUCCESS; } @@ -987,10 +998,10 @@ static int jtag3_program_enable(PROGRAMMER * pgm) free(resp); PDATA(pgm)->prog_enabled = 1; - return 0; + return LIBAVRDUDE_SUCCESS; } - return status; + return jtag3_errcode(status); } static int jtag3_program_disable(PROGRAMMER * pgm) @@ -1921,7 +1932,7 @@ static int jtag3_read_byte(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem, if (!(pgm->flag & PGM_FL_IS_DW)) if ((status = jtag3_program_enable(pgm)) < 0) - return status; + return jtag3_errcode(status); cmd[0] = SCOPE_AVR; cmd[1] = CMD3_READ_MEMORY; @@ -2007,7 +2018,7 @@ static int jtag3_read_byte(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem, if (addr == 0) { if ((status = jtag3_command(pgm, cmd, 12, &resp, "read memory")) < 0) - return status; + return jtag3_errcode(status); signature_cache[0] = resp[4]; signature_cache[1] = resp[5]; @@ -2057,7 +2068,7 @@ static int jtag3_read_byte(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem, } if ((status = jtag3_command(pgm, cmd, 12, &resp, "read memory")) < 0) - return status; + return jtag3_errcode(status); if (resp[1] != RSP3_DATA || status < (pagesize? pagesize: 1) + 4) { @@ -2185,7 +2196,7 @@ static int jtag3_write_byte(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem, cmd[13] = data; if ((status = jtag3_command(pgm, cmd, 14, &resp, "write memory")) < 0) - return status; + return jtag3_errcode(status); free(resp); @@ -2316,7 +2327,7 @@ int jtag3_read_sib(PROGRAMMER * pgm, AVRPART * p, char * sib) u32_to_b4(cmd + 8, AVR_SIBLEN); if ((status = jtag3_command(pgm, cmd, 12, &resp, "read SIB")) < 0) - return status; + return jtag3_errcode(status); memcpy(sib, resp+3, AVR_SIBLEN); sib[AVR_SIBLEN] = 0; // Zero terminate string diff --git a/src/jtag3_private.h b/src/jtag3_private.h index a3e7fb08..6385aa4c 100644 --- a/src/jtag3_private.h +++ b/src/jtag3_private.h @@ -145,6 +145,7 @@ # define RSP3_FAIL_UNSUPP_MEMORY 0x34 /* unsupported memory type */ # define RSP3_FAIL_WRONG_LENGTH 0x35 /* wrong lenth for mem access */ # define RSP3_FAIL_OCD_LOCKED 0x44 /* device is locked */ +# define RSP3_FAIL_CRC_FAILURE 0x47 /* CRC failure in device */ # define RSP3_FAIL_NOT_UNDERSTOOD 0x91 /* ICE events */ diff --git a/src/libavrdude.h b/src/libavrdude.h index ddb72b48..46789f4f 100644 --- a/src/libavrdude.h +++ b/src/libavrdude.h @@ -27,6 +27,16 @@ #include typedef uint32_t pinmask_t; +/* + * Values returned by library functions. + * Some library functions also return a count, i.e. a positive + * number greater than 0. + */ +#define LIBAVRDUDE_SUCCESS 0 +#define LIBAVRDUDE_GENERAL_FAILURE (-1) +#define LIBAVRDUDE_NOTSUPPORTED (-2) // operation not supported +#define LIBAVRDUDE_SOFTFAIL (-3) // returned by avr_signature() if caller + // might proceed with chip erase /* formerly lists.h */ diff --git a/src/main.c b/src/main.c index 253c6e51..5b0a6e38 100644 --- a/src/main.c +++ b/src/main.c @@ -1116,9 +1116,8 @@ int main(int argc, char * argv []) usleep(waittime); if (init_ok) { rc = avr_signature(pgm, p); - if (rc != 0) { - // -68 == -(0x44) == -(RSP3_FAIL_OCD_LOCKED) - if ((rc == -68) && (p->flags & AVRPART_HAS_UPDI) && (attempt < 1)) { + if (rc != LIBAVRDUDE_SUCCESS) { + if ((rc == LIBAVRDUDE_SOFTFAIL) && (p->flags & AVRPART_HAS_UPDI) && (attempt < 1)) { attempt++; if (pgm->read_sib) { // Read SIB and compare FamilyID From ae0e3e2f8e239c5075cd5a8507c9ada021197bcf Mon Sep 17 00:00:00 2001 From: Joerg Wunsch Date: Wed, 22 Jun 2022 23:33:53 +0200 Subject: [PATCH 2/4] Fix a number of logic errors in the previous commits RSP3_FAIL_CRC_FAILURE is 0x43 rather than 0x47. jtag3_errcode() must only be applied to a reason code, not to any general status code. --- src/jtag3.c | 32 +++++++++++++++----------------- src/jtag3_private.h | 2 +- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/jtag3.c b/src/jtag3.c index 23df86ff..4c159ee8 100644 --- a/src/jtag3.c +++ b/src/jtag3.c @@ -313,12 +313,10 @@ static void jtag3_prmsg(PROGRAMMER * pgm, unsigned char * data, size_t len) } } -static int jtag3_errcode(int status) +static int jtag3_errcode(int reason) { - if (status >= LIBAVRDUDE_SUCCESS) - return status; - if (status == RSP3_FAIL_OCD_LOCKED || - status == RSP3_FAIL_CRC_FAILURE) + if (reason == RSP3_FAIL_OCD_LOCKED || + reason == RSP3_FAIL_CRC_FAILURE) return LIBAVRDUDE_SOFTFAIL; return LIBAVRDUDE_GENERAL_FAILURE; } @@ -844,7 +842,7 @@ int jtag3_recv(PROGRAMMER * pgm, unsigned char **msg) { } } - int jtag3_command(PROGRAMMER *pgm, unsigned char *cmd, unsigned int cmdlen, +int jtag3_command(PROGRAMMER *pgm, unsigned char *cmd, unsigned int cmdlen, unsigned char **resp, const char *descr) { int status; @@ -868,9 +866,10 @@ int jtag3_recv(PROGRAMMER * pgm, unsigned char **msg) { avrdude_message(MSG_NOTICE2, "0x%02x (%d bytes msg)\n", (*resp)[1], status); } - c = (*resp)[1]; - if ((c & RSP3_STATUS_MASK) != RSP3_OK) { - if ((c == RSP3_FAILED) && ((*resp)[3] == RSP3_FAIL_OCD_LOCKED)) { + c = (*resp)[1] & RSP3_STATUS_MASK; + if (c != RSP3_OK) { + if ((c == RSP3_FAILED) && ((*resp)[3] == RSP3_FAIL_OCD_LOCKED || + (*resp)[3] == RSP3_FAIL_CRC_FAILURE)) { avrdude_message(MSG_INFO, "%s: Device is locked! Chip erase required to unlock.\n", progname); @@ -881,11 +880,10 @@ int jtag3_recv(PROGRAMMER * pgm, unsigned char **msg) { status = (*resp)[3]; free(*resp); resp = 0; - return jtag3_errcode(status); } - return LIBAVRDUDE_SUCCESS; + return status; } @@ -1001,7 +999,7 @@ static int jtag3_program_enable(PROGRAMMER * pgm) return LIBAVRDUDE_SUCCESS; } - return jtag3_errcode(status); + return status; } static int jtag3_program_disable(PROGRAMMER * pgm) @@ -1932,7 +1930,7 @@ static int jtag3_read_byte(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem, if (!(pgm->flag & PGM_FL_IS_DW)) if ((status = jtag3_program_enable(pgm)) < 0) - return jtag3_errcode(status); + return status; cmd[0] = SCOPE_AVR; cmd[1] = CMD3_READ_MEMORY; @@ -2018,7 +2016,7 @@ static int jtag3_read_byte(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem, if (addr == 0) { if ((status = jtag3_command(pgm, cmd, 12, &resp, "read memory")) < 0) - return jtag3_errcode(status); + return status; signature_cache[0] = resp[4]; signature_cache[1] = resp[5]; @@ -2068,7 +2066,7 @@ static int jtag3_read_byte(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem, } if ((status = jtag3_command(pgm, cmd, 12, &resp, "read memory")) < 0) - return jtag3_errcode(status); + return status; if (resp[1] != RSP3_DATA || status < (pagesize? pagesize: 1) + 4) { @@ -2196,7 +2194,7 @@ static int jtag3_write_byte(PROGRAMMER * pgm, AVRPART * p, AVRMEM * mem, cmd[13] = data; if ((status = jtag3_command(pgm, cmd, 14, &resp, "write memory")) < 0) - return jtag3_errcode(status); + return status; free(resp); @@ -2327,7 +2325,7 @@ int jtag3_read_sib(PROGRAMMER * pgm, AVRPART * p, char * sib) u32_to_b4(cmd + 8, AVR_SIBLEN); if ((status = jtag3_command(pgm, cmd, 12, &resp, "read SIB")) < 0) - return jtag3_errcode(status); + return status; memcpy(sib, resp+3, AVR_SIBLEN); sib[AVR_SIBLEN] = 0; // Zero terminate string diff --git a/src/jtag3_private.h b/src/jtag3_private.h index 6385aa4c..7d0cbb74 100644 --- a/src/jtag3_private.h +++ b/src/jtag3_private.h @@ -144,8 +144,8 @@ # define RSP3_FAIL_WRONG_MODE 0x32 /* progmode vs. non-prog */ # define RSP3_FAIL_UNSUPP_MEMORY 0x34 /* unsupported memory type */ # define RSP3_FAIL_WRONG_LENGTH 0x35 /* wrong lenth for mem access */ +# define RSP3_FAIL_CRC_FAILURE 0x43 /* CRC failure in device */ # define RSP3_FAIL_OCD_LOCKED 0x44 /* device is locked */ -# define RSP3_FAIL_CRC_FAILURE 0x47 /* CRC failure in device */ # define RSP3_FAIL_NOT_UNDERSTOOD 0x91 /* ICE events */ From 89b0aa72e0f605e4408989ae9a579dbffd65688d Mon Sep 17 00:00:00 2001 From: MCUdude Date: Sat, 25 Jun 2022 11:39:16 +0200 Subject: [PATCH 3/4] Attempt to fix EEPROM write issue #1009 --- src/jtag3.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/jtag3.c b/src/jtag3.c index 6d358dfa..bb41431f 100644 --- a/src/jtag3.c +++ b/src/jtag3.c @@ -1761,7 +1761,7 @@ static int jtag3_paged_write(PROGRAMMER * pgm, AVRPART * p, AVRMEM * m, free(cmd); return n_bytes; } - cmd[3] = ( p->flags & AVRPART_HAS_PDI ) ? MTYPE_EEPROM_XMEGA : MTYPE_EEPROM_PAGE; + cmd[3] = ( p->flags & AVRPART_HAS_PDI || p->flags & AVRPART_HAS_UPDI ) ? MTYPE_EEPROM_XMEGA : MTYPE_EEPROM_PAGE; PDATA(pgm)->eeprom_pageaddr = (unsigned long)-1L; } else if (strcmp(m->desc, "usersig") == 0 || strcmp(m->desc, "userrow") == 0) { From a6ea797c1cd51ccebdb44ac5cc5fc71b4c2a8cc7 Mon Sep 17 00:00:00 2001 From: Joerg Wunsch Date: Tue, 28 Jun 2022 22:53:24 +0200 Subject: [PATCH 4/4] PR 996 and 1013 done --- NEWS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS b/NEWS index da4e42d0..d5ce1df4 100644 --- a/NEWS +++ b/NEWS @@ -29,6 +29,8 @@ Changes since version 7.0: to < 80 characters #1000 - Dragon JTAG fix #979 - adding support for all Linux baud rates v.2 #993 + - Replace internal knowledge in jtag3.c by a public API #996 + - JTAG3 UPDI EEPROM fix #1013 * Internals: