avrdudes / avrdude

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

HV UPDI type 2 doesn't work #1163

Closed MCUdude closed 1 year ago

MCUdude commented 1 year ago

Thanks to @mcuee for discovering this bug.

It turns out that the logic for determining if a target supports HV UPDI is broken, at least for type 2 devices (AVR-DD and AVR-EA).

Here's the HV UPDI logic in jtag3.c where I've added two print statements to show what's going on:

    // Generate UPDI high-voltage pulse if user asks for it and hardware supports it
     msg_info("HV UPDI support: %d\n", p->hvupdi_variant);
    LNODEID support;
    if (p->prog_modes & PM_UPDI &&
        PDATA(pgm)->use_hvupdi == true &&
        p->hvupdi_variant != HV_UPDI_VARIANT_1) {
      parm[0] = PARM3_UPDI_HV_NONE;
      for (support = lfirst(pgm->hvupdi_support); support != NULL; support = lnext(support)) {
        msg_info("HV UPDI support list: %d\n", *(int *)ldata(support));
        if(*(int *) ldata(support) == p->hvupdi_variant) {
          pmsg_notice("sending HV pulse to targets %s pin\n",
            p->hvupdi_variant == HV_UPDI_VARIANT_0? "UPDI": "RESET");
          parm[0] = PARM3_UPDI_HV_SIMPLE_PULSE;
          break;
        }
        if (parm[0] == PARM3_UPDI_HV_NONE) {
          pmsg_error("%s does not support sending HV pulse to target %s\n", pgm->desc, p->desc);
          return -1;
        }
      }
      if (jtag3_setparm(pgm, SCOPE_AVR, 3, PARM3_OPT_12V_UPDI_ENABLE, parm, 1) < 0)
        return -1;
    }

And here's the output:

$ ./avrdude -p avr64dd32 -cpickit4_updi -xhvupdi

         Vtarget                      : 0.05 V
         JTAG clock megaAVR/program   : 1000 kHz
         JTAG clock megaAVR/debug     : 100 kHz
         PDI/UPDI clock Xmega/megaAVR : 100 kHz
HV UPDI support: 2
HV UPDI support list: 0
avrdude error: MPLAB(R) PICkit 4 in UPDI mode does not support sending HV pulse to target AVR64DD32
avrdude error: initialization failed, rc=-1
        - double check the connections and try again
        - use -B to set lower ISP clock frequency, e.g. -B 125kHz
        - use -F to override this check

avrdude done.  Thank you.

It happens to work with ATtinys, because these are using HVUPDI type 0. To me, it appears that the linked list isn't being filled, but I don't know enough about linked lists to trace down the bug myself.

mcuee commented 1 year ago

Wow, that is fast to find the root cause. We should be able to fix this. At least @stefanrueger should be able to find the problems. :-)

MCUdude commented 1 year ago

The root cause is pgm->pgm->hvupdi_support. For PICkit4, this is a list that contains 0, 1, 2. For some reason, the for loop only "detects" one field

for (support = lfirst(pgm->hvupdi_support); support != NULL; support = lnext(support))

However, when I "manually" print the contents of pgm->hvupdi_support, I'm getting the full list:

     msg_info("support: %d, %d, %d\n",
       *(int *)ldata(lfirst(pgm->hvupdi_support)),
       *(int *)ldata(lnext(lfirst(pgm->hvupdi_support))),
       *(int *)ldata(lnext(lnext(lfirst(pgm->hvupdi_support)))));

support: 0, 1, 2

MCUdude commented 1 year ago

Sorry, there's nothing wrong with the list. It's the last if statement in the for-loop that causes an early exit. I'll see if I can come up with a solution and submit a PR!

mcuee commented 1 year ago

Yes I can confirm that #1164 fixed the issue.