avrdudes / avrdude

AVRDUDE is a utility to program AVR microcontrollers
GNU General Public License v2.0
747 stars 138 forks source link

Two keep-alive functions and one memory read extension. #1600

Closed askn37 closed 10 months ago

askn37 commented 11 months ago

The following three separate small patches were created for my own personal needs and may not be of interest to others. First of all, I would like to inform you that something like this exists. (I will think about whether to create a PR later)

One is the "JTAG2UPDI" keepalive. (for interactive mode) Previously, using JTAG2UPDI in interactive mode required disabling host timeout (on the JTAG2UPDI side).

diff --git a/src/jtagmkII.c b/src/jtagmkII.c
index a1722f20..e44fccb4 100644
--- a/src/jtagmkII.c
+++ b/src/jtagmkII.c
@@ -3601,6 +3601,37 @@ static int jtagmkII_flash_clear_pagebuffer32(const PROGRAMMER *pgm) {
     return -1;
 }

+/*** Periodic calls in terminal mode to keep `JTAGMKII_UPDI` enabled. ***/
+/*
+ * GET_SYNC is not guaranteed to be harmless to other program devices.
+ * (JTAG2UPDI and UPDI4AVR are definitely harmless)
+ * So this is probably only available in `jtagmkII_updi_initpgm`.
+ */
+static int jtagmkII_term_keep_alive(const PROGRAMMER *pgm, const AVRPART *p_unused) {
+  unsigned char buf[2], *resp, c = 0xff;
+  int status;
+
+  buf[0] = CMND_GET_SYNC;
+  jtagmkII_send(pgm, buf, 1);
+
+  status = jtagmkII_recv(pgm, &resp);
+  c = resp[0];
+  free(resp);
+
+  if (status <= 0) {
+    msg_notice2("\n");
+    pmsg_error("timeout/error communicating with programmer (status %d)\n", status);
+    return -1;
+  }
+
+  if (c != RSP_OK) {
+    pmsg_error("bad response to `get_sync` command: %s\n", jtagmkII_get_rc(c));
+    return -1;
+  }
+
+  return 0;
+}
+
 #ifdef __OBJC__
 #pragma mark -
 #endif
@@ -3737,6 +3768,7 @@ void jtagmkII_updi_initpgm(PROGRAMMER *pgm) {
   pgm->read_chip_rev  = jtagmkII_read_chip_rev;
   pgm->page_size      = 256;
   pgm->flag           = PGM_FL_IS_PDI;
+  pgm->term_keep_alive= jtagmkII_term_keep_alive;
 }

 const char jtagmkII_dragon_desc[] = "Atmel AVR Dragon in JTAG mode";

One is the "Arduino compatible bootloader" keepalive. (for interactive mode)

diff --git a/src/arduino.c b/src/arduino.c
index 7c705da2..ba559814 100644
--- a/src/arduino.c
+++ b/src/arduino.c
@@ -121,6 +121,33 @@ static void arduino_close(PROGRAMMER * pgm)
   pgm->fd.ifd = -1;
 }

+/*** Periodic call in terminal mode to keep bootloader alive ***/
+static int arduino_term_keep_alive(const PROGRAMMER *pgm, const AVRPART *p_unused) {
+  unsigned char buf[16];
+
+  buf[0] = Cmnd_STK_GET_SYNC;
+  buf[1] = Sync_CRC_EOP;
+
+  serial_send(&pgm->fd, buf, 2);
+
+  if (serial_recv(&pgm->fd, buf, 2) < 0)
+    return -1;
+
+  if (buf[0] != Resp_STK_INSYNC) {
+    msg_error("\n");
+    pmsg_error("protocol expects sync byte 0x%02x but got 0x%02x\n", Resp_STK_INSYNC, buf[0]);
+    return -2;
+  }
+
+  if (buf[1] != Resp_STK_OK) {
+    msg_error("\n");
+    pmsg_error("protocol expects OK byte 0x%02x but got 0x%02x\n", Resp_STK_OK, buf[1]);
+    return -3;
+  }
+
+  return 0;
+}
+
 const char arduino_desc[] = "Arduino programmer";

 void arduino_initpgm(PROGRAMMER *pgm) {
@@ -134,6 +161,7 @@ void arduino_initpgm(PROGRAMMER *pgm) {
   pgm->read_sig_bytes = arduino_read_sig_bytes;
   pgm->open = arduino_open;
   pgm->close = arduino_close;
+  pgm->term_keep_alive = arduino_term_keep_alive;

   disable_trailing_ff_removal(); /* so that arduino bootloader can ignore chip erase */
 }

One is to load USERROW and BOOTROW using an "Arduino compatible bootloader". This only works with PM_UPDI parts. USERROW and (and BOOTROW) are not present in any other parts, so they are unrelated. This only works with bootloader implementations that can read EEPROM. Writing is currently not possible. Even if it were possible, only a small number of bootloader implementations would be able to do it.

diff --git a/src/stk500.c b/src/stk500.c
index aafcccf4..f70eef4f 100644
--- a/src/stk500.c
+++ b/src/stk500.c
@@ -1107,9 +1107,25 @@ static int stk500_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
   int tries;
   unsigned int n;
   int block_size;
-
-  if(set_memchr_a_div(pgm, p, m, &memchr, &a_div) < 0)
-    return -2;
+  int shift_addr = 0;
+
+  if(set_memchr_a_div(pgm, p, m, &memchr, &a_div) < 0) {
+    if((p->prog_modes & PM_UPDI) && (mem_is_userrow(m) || mem_is_bootrow(m))) {
+      // *** This is an experimental trick. ***
+      // Allow reading of USERROW and BOOTROW if the part is an UPDI and SPM bootloader (e.g. optiboot_x/dx).
+      // These areas are always in the same 64KiB data space as the EEPROM.
+      // Therefore, pass the address shifted from the beginning of EEPROM to loadaddr.
+      // Whether this is possible depends on the bootloader implementation.
+      AVRMEM *e;
+      e = avr_locate_eeprom(p);
+      shift_addr = m->offset - e->offset; // This will show a 16-bit wide negative address!
+      memchr = 'E';
+      a_div = 1;
+    }
+    else {
+      return -2;
+    }
+  }

   n = addr + n_bytes;
   for (; addr < n; addr += block_size) {
@@ -1126,7 +1142,7 @@ static int stk500_paged_load(const PROGRAMMER *pgm, const AVRPART *p, const AVRM
     tries = 0;
   retry:
     tries++;
-    stk500_loadaddr(pgm, m, addr, a_div);
+    stk500_loadaddr(pgm, m, addr + shift_addr, a_div);
     buf[0] = Cmnd_STK_READ_PAGE;
     buf[1] = (block_size >> 8) & 0xff;
     buf[2] = block_size & 0xff;
stefanrueger commented 11 months ago

Thanks for your code snippets @askn37.

I had not known that the terminal times out with jtagmkii_updi programmers. Neat.

I don't think further development of the arduino programmer is useful. It has so many deficiencies, almost all of which can be remedied by using urclock instead of arduino. In the long run it might be better to make arduino another id of the urclock programmer and remove the code arduino.c.

The userrow/bootrow is a neat trick. I wonder what the limitations are, eg, will using a linear 16-bit address space work with larger parts?

askn37 commented 11 months ago

@stefanrueger

"urclock" is a good project, but I haven't touched on it yet as the whole thing is too big for me as I am only focusing on AVR-LIBC+PM_UPDI devices. That is a challenge for the future.

(Personally, I also use many-to-one single-wire RS485 and high-speed synchronous serial, so I prefer a single source code that is easy to modify and extend.)

USERROW/USER_SIGNATURE also exists in XMEGA, but is outside of the flat address space and requires the STK500v1 extension. PM_UPDI is fully accessible with a 24-bit flat address and can be read with Cmnd_STK_READ_PAGE + EorF. Rest assured, there are no other AVR chips with USERROW.

You can only write to USERROW if you are using his NVMCTRL version 0, such as "optiboot_x". The 0.5KiB bootloader leaves little room to add code for other versions. Therefore, we restricted the patch to the read direction. This is probably enough to see inside the chip where ICSP is blocked.

stefanrueger commented 11 months ago

"JTAG2UPDI" keepalive looks like a useful enhancement, please submit a PR, so it can be tested by the team.

The USERROW and BOOTROW trick is neat. It would be good if

askn37 commented 11 months ago

Prepare PR for jtagmkII_term_keep_alive. There are at least two users who use -c jtag2updi.

Reading USERROW by the bootloader is closely related to the fact that USERROW is write-only to locked devices. You need to install a user program to check if the data was written correctly, but it's a hassle to physically change the USB port every time. If this is read by the bootloader and the serial port connection is switched remotely (e.g. -xrtsdtr), you can simply use AVRDUDE to complete the test, simplifying script/batch automation. This will help whenever new chips are released in the future.

P.S. I later realized that you can use the same trick to read sernum. However, paged_load doesn't pass, so it doesn't seem worth the trouble to fix it.

P.S. 2 Nano Every will time out unconditionally after up to 60 seconds due to firmware limitations. AVRDUDE cannot detect frame errors until an additional 60 seconds have elapsed. During this time, AVRDUDE is silent. AVRDUDE will eventually exit, but will also throw a malloc error if paged_load is already running. This is a known issue that cannot be avoided.

stefanrueger commented 10 months ago

you can use the same trick to read sernum

Yes. It looks like that at least the UPDI parts can r/w most (all?) memory segments using a 24-bit address and one bootloader r/w routine. The uploader (avrdude -c urclock) knows the mapping of fuses/lock bytes/memories to the single 24-bit address space of the part, so could be modified in a way that the bootloader would not even have to implement different routines for EEPROM and flash.

I have not yet started extending the urboot bootloader to XMEGA and UPDI parts. I kind of hoped (and still do) that some gifted programmer would continue the urboot project for modern parts. Urboot is vvv small, so with a bit of fair wind a version that is able to access all memories could fit into 256 bytes but I am sure that definitely would fit into 512 bytes.

MCUdude commented 10 months ago

Resolved in #1603 and #1604