From fd1c49e538c90ee887db4b074f2bfb5a29bf384a Mon Sep 17 00:00:00 2001 From: stefanrueger Date: Thu, 23 Nov 2023 17:42:07 +1300 Subject: [PATCH 1/4] Provide avr_get_config_value() and avr_set_config_value() functions Example usage: int ocden = 0; if(avr_get_config_value(pgm, p, "ocden", &ocden) == 0 && ocden) // ocden == 1 means disabled pmsg_warning("OCDEN fuse not programmed, single-byte EEPROM updates not possible\n"); --- src/avrcache.c | 1 - src/avrpart.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++ src/config.c | 1 - src/libavrdude.h | 5 +++ src/term.c | 1 - 5 files changed, 107 insertions(+), 3 deletions(-) diff --git a/src/avrcache.c b/src/avrcache.c index fca48b2dc..4d70c5050 100644 --- a/src/avrcache.c +++ b/src/avrcache.c @@ -29,7 +29,6 @@ #include "avrdude.h" #include "libavrdude.h" -#include "avrintel.h" /* * Provides an API for cached bytewise access diff --git a/src/avrpart.c b/src/avrpart.c index ead74f5e7..86f1f20f6 100644 --- a/src/avrpart.c +++ b/src/avrpart.c @@ -493,6 +493,108 @@ AVRMEM_ALIAS *avr_find_memalias(const AVRPART *p, const AVRMEM *m_orig) { return NULL; } +const Configitem_t *avr_locate_config(const Configitem_t *cfg, int nc, const char *name) { + const Configitem_t *match = NULL; + int matches = 0, n1; + + if(!cfg || !name || !(n1 = *name)) + return NULL; + + size_t l = strlen(name); + for(int i = 0; i < nc; i++) { + if(n1 == *cfg[i].name && !strncmp(cfg[i].name, name, l)) { // Partial initial match + match = cfg+i; + matches++; + if(cfg[i].name[l] == 0) // Exact match; return straight away + return match; + } + } + + return matches == 1? match: NULL; +} + +// Return memory associated with config item and fill in pointer to Configitem_t record +static AVRMEM *avr_locate_config_mem_c_value(const PROGRAMMER *pgm, const AVRPART *p, + const char *cname, const Configitem_t **cp, int *valp) { + + int id = p->mcuid; + + if(id < 0 || id >= (int) (sizeof uP_table/sizeof *uP_table)) { + pmsg_error("%s does not have a valid mcuid (%d)\n", p->desc, id); + return NULL; + } + + int nc = uP_table[id].nconfigs; + const Configitem_t *cfg = uP_table[id].cfgtable; + if(nc < 1 || !cfg) { + pmsg_error("%s does not have config information in avrintel.c\n", p->desc); + return NULL; + } + + const Configitem_t *c = avr_locate_config(cfg, nc, cname); + if(!c) { + pmsg_error("%s does not have a unique config item matched by %s\n", p->desc, cname); + return NULL; + } + + AVRMEM *mem = str_starts(c->memstr, "lock")? avr_locate_lock(p): avr_locate_fuse_by_offset(p, c->memoffset); + if(!mem) + mem = avr_locate_mem(p, c->memstr); + if(!mem) { + pmsg_error("%s does not have the memory %s needed for config item %s\n", p->desc, c->memstr, cname); + return NULL; + } + + if(mem->size < 1 || mem->size > 4) { + pmsg_error("cannot handle size %d of %s's memory %s for config item %s\n", mem->size, p->desc, c->memstr, cname); + return NULL; + } + + int fusel = 0; + for(int i = 0; i < mem->size; i++) + if(led_read_byte(pgm, p, mem, i, (unsigned char *) &fusel + i) < 0) { + pmsg_error("cannot read from %s's %s memory\n", p->desc, mem->desc); + return NULL; + } + + *cp = c; + *valp = fusel; + return mem; +} + +int avr_get_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, int *valuep) { + const Configitem_t *c; + int fusel; + + if(!avr_locate_config_mem_c_value(pgm, p, cname, &c, &fusel)) + return -1; + + *valuep = (fusel & c->mask) >> c->lsh; + return 0; +} + +int avr_set_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, int value) { + AVRMEM *mem; + const Configitem_t *c; + int fusel; + + if(!(mem=avr_locate_config_mem_c_value(pgm, p, cname, &c, &fusel))) + return -1; + + int newval = (fusel & ~c->mask) | ((value << c->lsh) & c->mask); + + if(newval != fusel) { + for(int i = 0; i < mem->size; i++) + if(led_write_byte(pgm, p, mem, i, ((unsigned char *) &newval)[i]) < 0) { + pmsg_error("cannot write to %s's %s memory\n", p->desc, mem->desc); + return -1; + } + } + + return 0; +} + + static char *print_num(const char *fmt, int n) { return str_sprintf(n<10? "%d": fmt, n); } diff --git a/src/config.c b/src/config.c index 90fdc59cb..048aaba59 100644 --- a/src/config.c +++ b/src/config.c @@ -32,7 +32,6 @@ #include "avrdude.h" #include "libavrdude.h" #include "config.h" -#include "avrintel.h" #include "config_gram.h" diff --git a/src/libavrdude.h b/src/libavrdude.h index 55d9d28df..e1944d4bd 100644 --- a/src/libavrdude.h +++ b/src/libavrdude.h @@ -25,6 +25,7 @@ #include #include #include +#include "avrintel.h" typedef uint32_t pinmask_t; /* @@ -1419,6 +1420,10 @@ typedef struct { extern "C" { #endif +const Configitem_t *avr_locate_config(const Configitem_t *cfg, int nc, const char *name); +int avr_get_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, int *valuep); +int avr_set_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, int value); + int setport_from_serialadapter(char **portp, const SERIALADAPTER *ser, const char *sernum); int setport_from_vid_pid(char **portp, int vid, int pid, const char *sernum); int list_available_serialports(LISTID programmers); diff --git a/src/term.c b/src/term.c index 7e3f967e7..8531f3f53 100644 --- a/src/term.c +++ b/src/term.c @@ -35,7 +35,6 @@ #include #include "libavrdude.h" -#include "avrintel.h" #if defined(HAVE_LIBREADLINE) #include From 879ea06545746edca8362d3164343cae9a4051d6 Mon Sep 17 00:00:00 2001 From: stefanrueger Date: Thu, 23 Nov 2023 17:44:54 +1300 Subject: [PATCH 2/4] Use avr_get_config_value() to determine OCDEN fuse setting --- src/jtagmkI.c | 20 +++----------------- src/jtagmkII.c | 20 +++----------------- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/src/jtagmkI.c b/src/jtagmkI.c index 687a3dece..e920534b9 100644 --- a/src/jtagmkI.c +++ b/src/jtagmkI.c @@ -63,17 +63,6 @@ struct pdata #define PDATA(pgm) ((struct pdata *)(pgm->cookie)) -/* - * The OCDEN fuse is bit 7 of the high fuse (hfuse). In order to - * perform memory operations on MTYPE_SPM and MTYPE_EEPROM, OCDEN - * needs to be programmed. - * - * OCDEN should probably rather be defined via the configuration, but - * if this ever changes to a different fuse byte for one MCU, quite - * some code here needs to be generalized anyway. - */ -#define OCDEN (1 << 7) - /* * Table of baud rates supported by the mkI ICE, accompanied by their * internal parameter value. @@ -550,12 +539,9 @@ static int jtagmkI_initialize(const PROGRAMMER *pgm, const AVRPART *p) { if (jtagmkI_reset(pgm) < 0) return -1; - AVRMEM *hf = avr_locate_hfuse(p); - if (!hf || jtagmkI_read_byte(pgm, p, hf, 1, &b) < 0) - return -1; - if ((b & OCDEN) != 0) - pmsg_warning("OCDEN fuse not programmed, " - "single-byte EEPROM updates not possible\n"); + int ocden = 0; + if(avr_get_config_value(pgm, p, "ocden", &ocden) == 0 && ocden) // ocden == 1 means disabled + pmsg_warning("OCDEN fuse not programmed, single-byte EEPROM updates not possible\n"); return 0; } diff --git a/src/jtagmkII.c b/src/jtagmkII.c index 24801c6a7..f17122140 100644 --- a/src/jtagmkII.c +++ b/src/jtagmkII.c @@ -95,17 +95,6 @@ struct pdata #define PDATA(pgm) ((struct pdata *)(pgm->cookie)) -/* - * The OCDEN fuse is bit 7 of the high fuse (hfuse). In order to - * perform memory operations on MTYPE_SPM and MTYPE_EEPROM, OCDEN - * needs to be programmed. - * - * OCDEN should probably rather be defined via the configuration, but - * if this ever changes to a different fuse byte for one MCU, quite - * some code here needs to be generalized anyway. - */ -#define OCDEN (1 << 7) - #define RC(x) { x, #x }, static struct { unsigned int code; @@ -1325,12 +1314,9 @@ static int jtagmkII_initialize(const PROGRAMMER *pgm, const AVRPART *p) { } if ((pgm->flag & PGM_FL_IS_JTAG) && !(p->prog_modes & (PM_PDI | PM_UPDI))) { - AVRMEM *hf = avr_locate_hfuse(p); - if (!hf || jtagmkII_read_byte(pgm, p, hf, 1, &b) < 0) - return -1; - if ((b & OCDEN) != 0) - pmsg_warning("OCDEN fuse not programmed, " - "single-byte EEPROM updates not possible\n"); + int ocden = 0; + if(avr_get_config_value(pgm, p, "ocden", &ocden) == 0 && ocden) // ocden == 1 means disabled + pmsg_warning("OCDEN fuse not programmed, single-byte EEPROM updates not possible\n"); } if (pgm->read_chip_rev && p->prog_modes & (PM_PDI | PM_UPDI)) { From 65901cf57096369f50bd9419fb2798ac9ce79e35 Mon Sep 17 00:00:00 2001 From: stefanrueger Date: Fri, 24 Nov 2023 15:49:12 +1300 Subject: [PATCH 3/4] Fix access of internal uP_table given the part --- src/avrpart.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/avrpart.c b/src/avrpart.c index 86f1f20f6..4abdcf6e6 100644 --- a/src/avrpart.c +++ b/src/avrpart.c @@ -517,15 +517,20 @@ const Configitem_t *avr_locate_config(const Configitem_t *cfg, int nc, const cha static AVRMEM *avr_locate_config_mem_c_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, const Configitem_t **cp, int *valp) { - int id = p->mcuid; - - if(id < 0 || id >= (int) (sizeof uP_table/sizeof *uP_table)) { - pmsg_error("%s does not have a valid mcuid (%d)\n", p->desc, id); + int idx = -1; + + if(p->mcuid >= 0) + idx = upidxmcuid(p->mcuid); + if(idx < 0 && p->desc && *p->desc) + idx = upidxname(p->desc); + if(idx < 0) { + pmsg_error("uP_table neither knows mcuid %d nor part %s\n", + p->mcuid, p->desc && *p->desc? p->desc: "???"); return NULL; } - int nc = uP_table[id].nconfigs; - const Configitem_t *cfg = uP_table[id].cfgtable; + int nc = uP_table[idx].nconfigs; + const Configitem_t *cfg = uP_table[idx].cfgtable; if(nc < 1 || !cfg) { pmsg_error("%s does not have config information in avrintel.c\n", p->desc); return NULL; From f4de3787515f99e343a8493bc00b535720d020bb Mon Sep 17 00:00:00 2001 From: stefanrueger Date: Sun, 26 Nov 2023 19:09:42 +1300 Subject: [PATCH 4/4] Provide avr_locate_upidx(), avr_locate_configitems(), avr_locate_configlist() --- src/avrpart.c | 114 ++++++++++++++++++++++++++++++++++++----------- src/libavrdude.h | 7 ++- src/term.c | 30 +++++++------ 3 files changed, 110 insertions(+), 41 deletions(-) diff --git a/src/avrpart.c b/src/avrpart.c index 4abdcf6e6..be6d828ca 100644 --- a/src/avrpart.c +++ b/src/avrpart.c @@ -493,50 +493,105 @@ AVRMEM_ALIAS *avr_find_memalias(const AVRPART *p, const AVRMEM *m_orig) { return NULL; } -const Configitem_t *avr_locate_config(const Configitem_t *cfg, int nc, const char *name) { - const Configitem_t *match = NULL; - int matches = 0, n1; +// Return index in uP_table for part or -1 +int avr_locate_upidx(const AVRPART *p) { + int idx = -1; + + if(!p) + return -1; + if(p->mcuid >= 0) + idx = upidxmcuid(p->mcuid); + if(idx < 0 && p->desc && *p->desc) + idx = upidxname(p->desc); + + if(idx < 0) + pmsg_error("uP_table neither knows mcuid %d nor part %s\n", + p->mcuid, p->desc && *p->desc? p->desc: "???"); + + return idx; +} - if(!cfg || !name || !(n1 = *name)) +// Return pointer to config table for the part and set number of config bitfields +const Configitem_t *avr_locate_configitems(const AVRPART *p, int *nc) { + int idx = avr_locate_upidx(p); + if(idx < 0) return NULL; - size_t l = strlen(name); + *nc = uP_table[idx].nconfigs; + return uP_table[idx].cfgtable; +} + + +/* + * Return pointer to a configuration bitfield that uniquely matches the + * argument name. Return NULL if none matches or more than one do. + * + * The caller provides a matching function which can be str_eq, str_starts, + * str_matched_by etc. If name is the full name of a configuration bitfield + * then a pointer to that is returned irrespective of the matching function. + */ +const Configitem_t *avr_locate_config(const Configitem_t *cfg, int nc, const char *name, + int (*match)(const char *, const char*)) { + + if(!cfg || nc < 1 || !name || !match) + return NULL; + + const Configitem_t *ret = NULL; + int nmatches = 0; + for(int i = 0; i < nc; i++) { - if(n1 == *cfg[i].name && !strncmp(cfg[i].name, name, l)) { // Partial initial match - match = cfg+i; - matches++; - if(cfg[i].name[l] == 0) // Exact match; return straight away - return match; + if(match(cfg[i].name, name)) { + if(match == str_eq || str_eq(cfg[i].name, name)) // Full name specified: return straight away + return cfg+i; + nmatches++, ret = cfg+i; } } - return matches == 1? match: NULL; + return nmatches == 1? ret: NULL; +} + +/* + * Return a NULL terminated malloc'd list of pointers to config bitfields + * + * The caller provides a matching function which can be str_eq, str_starts, + * str_matched_by etc. If name is a full, existing config name then the + * returned list is confined to this specific entry irrespective of the + * matching function. + */ +const Configitem_t **avr_locate_configlist(const Configitem_t *cfg, int nc, const char *name, + int (*match)(const char *, const char*)) { + + const Configitem_t **ret = cfg_malloc(__func__, sizeof cfg*(nc>0? nc+1: 1)), **r = ret; + + if(cfg && name && match) { + for(int i = 0; i < nc; i++) + if(match(cfg[i].name, name)) { + if(match == str_eq || str_eq(cfg[i].name, name)) { // Full name specified: return straight away + ret[0] = cfg+i; + ret[1] = NULL; + return ret; + } + *r++ = cfg+i; + } + } + *r = NULL; + + return ret; } // Return memory associated with config item and fill in pointer to Configitem_t record static AVRMEM *avr_locate_config_mem_c_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, const Configitem_t **cp, int *valp) { - int idx = -1; + int nc = 0; + const Configitem_t *cfg = avr_locate_configitems(p, &nc); - if(p->mcuid >= 0) - idx = upidxmcuid(p->mcuid); - if(idx < 0 && p->desc && *p->desc) - idx = upidxname(p->desc); - if(idx < 0) { - pmsg_error("uP_table neither knows mcuid %d nor part %s\n", - p->mcuid, p->desc && *p->desc? p->desc: "???"); + if(!cfg || nc < 1) { + pmsg_error("avrintel.c does not hold configuration information for %s\n", p->desc); return NULL; } - int nc = uP_table[idx].nconfigs; - const Configitem_t *cfg = uP_table[idx].cfgtable; - if(nc < 1 || !cfg) { - pmsg_error("%s does not have config information in avrintel.c\n", p->desc); - return NULL; - } - - const Configitem_t *c = avr_locate_config(cfg, nc, cname); + const Configitem_t *c = avr_locate_config(cfg, nc, cname, str_contains); if(!c) { pmsg_error("%s does not have a unique config item matched by %s\n", p->desc, cname); return NULL; @@ -567,6 +622,7 @@ static AVRMEM *avr_locate_config_mem_c_value(const PROGRAMMER *pgm, const AVRPAR return mem; } +// Initialise *valuep with configuration value of named configuration bitfield int avr_get_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, int *valuep) { const Configitem_t *c; int fusel; @@ -578,6 +634,7 @@ int avr_get_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cn return 0; } +// Set configuration value of named configuration bitfield to value int avr_set_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, int value) { AVRMEM *mem; const Configitem_t *c; @@ -586,6 +643,9 @@ int avr_set_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cn if(!(mem=avr_locate_config_mem_c_value(pgm, p, cname, &c, &fusel))) return -1; + if((value << c->lsh) & ~c->mask) + pmsg_warning("value 0x%02x has bits set outside bitfield mask 0x%02x\n", value, c->mask >> c->lsh); + int newval = (fusel & ~c->mask) | ((value << c->lsh) & c->mask); if(newval != fusel) { diff --git a/src/libavrdude.h b/src/libavrdude.h index e1944d4bd..ebf343f4e 100644 --- a/src/libavrdude.h +++ b/src/libavrdude.h @@ -1420,7 +1420,12 @@ typedef struct { extern "C" { #endif -const Configitem_t *avr_locate_config(const Configitem_t *cfg, int nc, const char *name); +int avr_locate_upidx(const AVRPART *p); +const Configitem_t *avr_locate_configitems(const AVRPART *p, int *nc); +const Configitem_t *avr_locate_config(const Configitem_t *cfg, int nc, const char *name, + int (*match)(const char *, const char*)); +const Configitem_t **avr_locate_configlist(const Configitem_t *cfg, int nc, const char *name, + int (*match)(const char *, const char*)); int avr_get_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, int *valuep); int avr_set_config_value(const PROGRAMMER *pgm, const AVRPART *p, const char *cname, int value); diff --git a/src/term.c b/src/term.c index 8531f3f53..96d1ee7fb 100644 --- a/src/term.c +++ b/src/term.c @@ -1999,29 +1999,33 @@ static char *tokenize(char *s, int *argcp, char ***argvp) { static int do_cmd(const PROGRAMMER *pgm, const AVRPART *p, int argc, char *argv[]) { - int i; int hold, matches; size_t len; len = strlen(argv[0]); matches = 0; - for (i=0; i 1? "ambiguous": "invalid"); + pmsg_error("(cmd) command %s is %s", argv[0], matches > 1? "ambiguous": "invalid"); + if(matches > 1) + for(int ch = ':', i = 0; i < NCMDS; i++) + if(*(void (**)(void)) ((char *) pgm + cmd[i].fnoff)) + if(len && strncasecmp(argv[0], cmd[i].name, len)==0) + msg_error("%c %s", ch, cmd[i].name), ch = ','; + msg_error("\n"); + return -1; }