ElTangas / jtag2updi

UPDI programmer software for Arduino (targets Tiny AVR-0/1/2, Mega AVR-0 and AVR-DA/DB MCUs)
MIT License
329 stars 90 forks source link

Support AVR-DA, add debug channel, add timeouts #31

Closed SpenceKonde closed 4 years ago

SpenceKonde commented 4 years ago

Sorry for this all being in one giant commit, rather than smaller ones with informative commit messages >.<

SpenceKonde commented 4 years ago

Oh, and it adds support for NO_ACK_WRITE, ie, for burst writes, it can turn off the response signature

Note also that AVRdude is hardcoded to never write more than 256 bytes at a time, so for these parts it only ever writes a half page at a time, rather than a whole page :-/ doesn't look like there's a way to override that without recompiling avrdude, either...

ElTangas commented 4 years ago

Cool, I see you put a lot of work into this, I'm sure it's working fine but I'll just run some tests.

ElTangas commented 4 years ago

BTW, once this is done, it should be an easy step to get jtag2updi running on the AVR-DA, right? Maybe you already tested it?

SpenceKonde commented 4 years ago

Didn't test because my DACore is still in a very rudimentary state because I havent had a way to upload until now. I think adding support would be very easy though - change test for XAVR to accept avr arch=104 too, add a couple of extra blocks for settings to sys.h, dbg.h and parts.h, and add test for DA series to init since clock speed is set differently... and then should just work


Spence Konde Azzy’S Electronics

New products! Check them out at tindie.com/stores/DrAzzy GitHub: github.com/SpenceKonde ATTinyCore: Arduino support for almost every ATTiny microcontroller Contact: spencekonde@gmail.com

On Mon, May 18, 2020, 03:55 ElTangas notifications@github.com wrote:

BTW, once this is done, it should be an easy step to get jtag2updi running on the AVR-DA, right? Maybe you already tested it?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ElTangas/jtag2updi/pull/31#issuecomment-630010664, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTXEW44KMP4FASCLFPAB4DRSDSWDANCNFSM4NDNVKWQ .

ElTangas commented 4 years ago

I've found several issues on testing.

  1. avrdude terminal mode (i.e. -t option) is not working; once in terminal mode all commands give timeout errors.

  2. LGT clones are hanging on the "get parameter" command:

Programmer Type : JTAGMKII_PDI Description : JTAGv2 to UPDI bridge avrdude: jtagmkII_getparm() avrdude: jtagmkII_getparm(): Sending get parameter command (parm 0x01): avrdude: jtagmkII_send(): sending 2 bytes avrdude: Send: . [1b] . [03] . [00] . [02] . [00] . [00] . [00] . [0e] . [03] . [01] . [d3] . [99] avrdude: jtagmkII_recv(): avrdude: Recv: . [1b] avrdude: Recv: . [03] avrdude: Recv: . [00] avrdude: Recv: . [03] avrdude: Recv: . [c1] avrdude: Recv: . [00] avrdude: Recv: . [00] avrdude: Recv: . [0e]

then it hangs and starts sending garbage to the serial port (seems like some kind of buffer overrun or endless loop).

  1. Part definitions using "has_pdi" are not working, thus breaking compatibility with versions of avrdude other than the patched one used on the Arduino IDE.
SpenceKonde commented 4 years ago
  1. This is expected breakage - the timeout system assumes that if it doesn't receive any command from the host for 250ms, that avrdude must have malfunctioned, and, upon 4 consecutive timeouts (1 second without a command) resets itself to the initial state to await fresh communication from a new invocation of avrdude. There is a design decision in how to handle terminal modewhile also preventing jtag2updi from getting stuck waiting for communication from the host, as it previously could.
  2. This could be - I do not have LGT clones to test with (I'm very much down on these, as they're deceptively marketed as having an atmega328p on them, leading to confused newbies trying to use them and discovering that they aren't quite a '328p). It's probably something silly; does it generate a warning about the part not being defined? That would indicate that it's not being caught by parts.h as a '328-like part (though I thought that it had the same define as a '328p?), and just adding that will probably fix it. If not, then it sounds like this is down to some weird difference between the LGT and real ATmega parts...
  3. I have not tested this - my objective was to get an upload working for Arduino, and it wasn't until late in the development process that I even realized that there was even a way for AVRDude to be told to do the page erase instead of a proper chip erase. Unfortunate that this mode exists IMO. Is this broken even on non-DA parts now? I didn't think I changed anything that would have broken this... I am actually a little concerned about how this will behave on DA-series parts, since AVRdude has a hardcoded limit of how much it writes in one write command of 256b, when these have a 512b page size. Hopefully avrdude is smart enough to not to send a page erase prior to the second "half-page" write!

As an aside, I've gotten reports that my version seems to be faster at writing, probably because of the no-ack-write mode.

SpenceKonde commented 4 years ago

My plan of action on 1. is to add an optional SUPPORT_TERMINAL_MODE define, and if set, it will disable the host timeout entirely (I could make it wait a much longer period of time, but since I would expect that build option to only be used by people who are planning to use terminal mode (ie, advanced users who understand what disabling the timeouts means, ie, that they'll lose the automatic recovery in the event of host<->jtag2updi communication problems), I don't see that as necessary)

Not sure how to deal with 2 since I don't have the hardware to test (nor do I even know how I would get an LGT-based clone, since the only way I've heard of people ending up with them is trying to buy a board with a normal '328p from an unscrupulous seller who is actually shipping LGT-based clones - then they discover that things don't work right and come crying on the forums about it). I think it's unlikely to be a buffer overrun type problem - I don't suppose you have a record of what "garbage" it sends to the serial port?

Let me know regarding whether 3. is broken with non-DA parts too, or only DA-series parts.

ElTangas commented 4 years ago

I see. Since I didn't implement a comprehensive debug system in the original version, avrdude terminal mode was very important for me during development. I don't think it's a good idea to break it by design.

I'll take a closer look at the other issues and try to get more details.

Would it be feasible to add compile options to disable the timeout system completely (like you have with the debug system)? If some quirk with the timer is responsible for the LGT incompatibility, maybe this would solve both problems (1 & 2)?

In other words, I prefer that the timeout feature is optional.

ElTangas commented 4 years ago

I need clarification on this snippet from JTAG2::enter_progmode()

    case 0x08:
      if (nvm_version == 1) {
        // For NVM version 1 parts, there's a page buffer.
        // It might have data in it if something else was writing to the flash when
        // we so rudely interrupted, so better clear the page buffer, just in case.
        uint8_t NVM_Status = UPDI::lds_b(NVM::NVM_base + NVM::STATUS);
        uint8_t NVM_Cmnd = UPDI::lds_b(NVM::NVM_base + NVM::CTRLA);
        if (NVM_Status || NVM_Cmnd ) {
          #if defined(DEBUG_ON)
          DBG::debug('d', NVM_Status,NVM_Cmnd);
          #endif
          UPDI::sts_b(NVM::NVM_base + NVM::STATUS, 0);
          UPDI::sts_b(NVM::NVM_base + NVM::CTRLA, 0);
        }
        UPDI::sts_b(NVM::NVM_base | NVM::CTRLA, NVM::PBC);

Is there any need to make sure NVM::STATUS and NVM::CTRLA are zero? If so, why read them first and not just write zero? If it's for debugging purposes, then the whole thing should be enclosed in a conditional compile block. The datasheet says STATUS is read only, does writing to it even has any effect?

ElTangas commented 4 years ago
Let me know regarding whether 3. is broken with non-DA parts too, or only DA-series parts.

The problem seems to be limited to DA parts.

I believe the bug might be located in the JTAG2::erase() function, since case 5 is called only if page erase is enabled (i.e. if has_pdi is defined instead of has_updi).

      case 5:
        if (nvm_version == 1) {
          const uint16_t address = packet.body[2] | (packet.body[3] << 8);
          NVM::wait<false>();
          UPDI::sts_b(address, 0xFF);
          NVM::command<false>(NVM::ER);
          set_status(RSP_OK);
        } else {
          const uint32_t address = (((uint32_t)packet.body[4]) << 16) | (((uint16_t) packet.body[3]) << 8) | packet.body[2];
          NVM_v2::wait<false>();
          NVM_v2::command<false>(NVM_v2::FLPER);
          UPDI::sts_b_l(address, 0xFF);
          NVM_v2::command<false>(NVM_v2::NOOP);
          set_status(RSP_OK);
        }

Once avrdude requests the page erase, all subsequent commands fail. From the avrdude log:

avrdude: writing flash (3676 bytes):

Writing | avrdude: jtagmkII_page_erase(.., flash, 0x0)
avrdude: jtagmkII_page_erase(): Sending xmega erase command: 
avrdude: jtagmkII_send(): sending 6 bytes
avrdude: Send: . [1b] . [0e] . [00] . [06] . [00] . [00] . [00] . [0e] 4 [34] . [04] . [00] . [00] . [80] . [00] o [6f] . [d9] 
avrdude: jtagmkII_recv():
avrdude: Recv: . [1b] 
avrdude: Recv: . [0e] 
avrdude: Recv: . [00] 
avrdude: Recv: . [01] 
avrdude: Recv: . [00] 
avrdude: Recv: . [00] 
avrdude: Recv: . [00] 
avrdude: Recv: . [0e] 
avrdude: Recv: . [80] 
avrdude: Recv: . [7f] 
avrdude: Recv: 2 [32] 

avrdude: jtagmkII_recv(): Got message seqno 14 (command_sequence == 14)
avrdude: Recv: . [80] 

Raw message:
0x80 
OK

avrdude: jtagmkII_paged_write(.., flash, 512, 512)
avrdude: jtagmkII_paged_write(): block_size at addr 0 is 256
avrdude: jtagmkII_paged_write(): Sending write memory command: 
avrdude: jtagmkII_send(): sending 266 bytes
avrdude: Send: . [1b] . [0f] . [00] . [0a] . [01] . [00] . [00] . [0e] . [04] . [c0] . [00] . [01] . [00] . [00] . [00] . [00] . [80] . [00] R [52] . [c1] . [00] . [00] k [6b] . [c1] . [00] . [00] i [69] . [c1] . [00] . [00] g [67] . [c1] . [00] . [00] e [65] . [c1] . [00] . [00] c [63] . [c1] . [00] . [00] a [61] . [c1] . [00] . [00] _ [5f] . [c1] . [00] . [00] ] [5d] . [c1] . [00] . [00] [ [5b] . [c1] . [00] . [00] Y [59] . [c1] . [00] . [00] W [57] . [c1] . [00] . [00] U [55] . [c1] . [00] . [00] S [53] . [c1] . [00] . [00] Q [51] . [c1] . [00] . [00] O [4f] . [c1] . [00] . [00] M [4d] . [c1] . [00] . [00] K [4b] . [c1] . [00] . [00] I [49] . [c1] . [00] . [00] G [47] . [c1] . [00] . [00] E [45] . [c1] . [00] . [00] C [43] . [c1] . [00] . [00] A [41] . [c1] . [00] . [00] ? [3f] . [c1] . [00] . [00] = [3d] . [c1] . [00] . [00] ; [3b] . [c1] . [00] . [00] . [86] . [01] . [01] . [00] . [06] . [01] . [01] . [00] . [06] . [01] . [00] . [00] . [00] . [00] . [00] . [00] J [4a] T [54] A [41] G [47] I [49] C [43] E [45]   [20] m [6d] k [6b] I [49] I [49] . [00] e [65] s [73] a [61] r [72] E [45] M [4d] V [56] N [4e] @ [40] . [03] . [a0] . [01] . [cf] . [00] g [67] . [00] 3 [33] . [00] " [22] . [00] . [10] . [00] . [8a] . [00]   [20] g [67] o [6f] r [72] P [50] M [4d] V [56] N [4e] . [00] . [00] . [89] . [11] . [12] # [23] . [9b] 2 [32] $ [24] F [46] . [ad] W [57] 6 [36] e [65] . [bf] t [74] H [48] . [8c] . [c1] . [9d] Z [5a] . [af] . [d3] . [be] l [6c] . [ca] . [e5] . [db] ~ [7e] . [e9] . [f7] . [f8] . [81] . [10] . [08] . [01] . [93] 3 [33] . [1a] " [22] . [a5] V [56] , [2c] G [47] . [b7] u [75] > [3e] d [64] . [c9] . [9c] @ [40] . [8d] . [db] . [bf] R [52] . [ae] . [ed] . [da] d [64] . [cb] . [ff] . [f9] v [76] . [e8] . [02] ! [21] . [8b] 0 [30] . [10] . [02] . [99] . [13] & [26] g [67] . [af] v [76] 4 [34] D [44] . [bd] U [55] J [4a] . [ad] . [c3] . [bc] X [58] . [8e] . [d1] . [9f] n [6e] . [eb] . [e7] . [e3] . [d0] 
avrdude: jtagmkII_recv():
avrdude: Recv: . [1b] 
avrdude: Recv: . [0f] 
avrdude: Recv: . [00] 
avrdude: Recv: . [01] 
avrdude: Recv: . [00] 
avrdude: Recv: . [00] 
avrdude: Recv: . [00] 
avrdude: Recv: . [0e] 
avrdude: Recv: . [ab] 
avrdude: Recv: . [11] 
avrdude: Recv: , [2c] 

avrdude: jtagmkII_recv(): Got message seqno 15 (command_sequence == 15)
avrdude: Recv: . [ab] 

Raw message:
0xab 
No target power

avrdude: jtagmkII_paged_write(): bad response to write memory command: RSP_NO_TARGET_POWER
######avrdude: jtagmkII_write_byte(.., flash, 0x0, ...)
avrdude: jtagmkII_program_disable(): Sending leave progmode command: 
avrdude: jtagmkII_send(): sending 1 bytes
avrdude: Send: . [1b] . [10] . [00] . [01] . [00] . [00] . [00] . [0e] . [15] . [2e] . [9a] 
avrdude: jtagmkII_recv():
avrdude: Recv: . [1b] 
avrdude: Recv: . [10] 
avrdude: Recv: . [00] 
avrdude: Recv: . [01] 
avrdude: Recv: . [00] 
avrdude: Recv: . [00] 
avrdude: Recv: . [00] 
avrdude: Recv: . [0e] 
avrdude: Recv: . [ab] 
avrdude: Recv: . [db] 
avrdude: Recv: . [c6] 

avrdude: jtagmkII_recv(): Got message seqno 16 (command_sequence == 16)
avrdude: Recv: . [ab] 

Raw message:
0xab 
No target power

avrdude: jtagmkII_program_disable(): bad response to leave progmode command: RSP_NO_TARGET_POWER
 ***failed;  
SpenceKonde commented 4 years ago

Yeah - will make the defines DISABLE_HOST_TIMEOUT and DISABLE_TARGET_TIMEOUT - the latter should probably not be needed, but the option will be there.

ElTangas commented 4 years ago

It's incredibly weird, it's as if the NVM_v2::FLPER is not working as described in the datasheet. The page gets erased, but the NVM is left in some sort of weird state and subsequent writes don't work correctly (?) edit: yes, it seems this command does not set busy flags in the status register! I'll investigate more tomorrow.

At least I've confirmed that avrdude works as expected: one 512 byte page erase is followed by 2 consecutive 256 byte writes to consecutive half pages, so I think it'll be ok on that end.

SpenceKonde commented 4 years ago

I have a theory about what's going on with the page erase - let me test something

The no target power error is what I return when we're waiting for a response from the target, but it never gives one.

ElTangas commented 4 years ago

I've fixed it, the problem was the wait() and command() functions in NVM_v2, you need to use lds_b_l and sts_b_l to read/write to the NVM controller (even though its address is just 16 bit long).

This is because if you use the 16 bit address functions, the 3rd byte is not cleared, whatever was there remains.

SpenceKonde commented 4 years ago

Added commit to address some issues discussed above: There are now DISABLE_HOST_TIMEOUT and DISABLE_TARGET_TIMEOUT. I think only the former should be needed to make terminal mode work. Removed the checking NVMCTRL.STATUS and NVMCTRL.CTRLA upon entering programming mode for NVM v1 parts. This issue and workaround is only relevant for NVM v2 parts. Added comments describing reasoning for this. Put in your fix for has_pdi (looks like it would be needed for other situations too...) Added clock setting and defines for DA-series parts (untested, though I know that code works for setting the clock frequency)

SpenceKonde commented 4 years ago

I also have a few LGT clones on order....

ElTangas commented 4 years ago

Thank you. I will review changes shortly.

ElTangas commented 4 years ago

In your recent revision you added this comment:

        //NVM v2 devices can get into a state where the command and status registers both read as an invalid result (in my testing, I have only seen 0xFF).
        // In this case, to restore functionality, both registers should be written 0.
        // If debug is enabled, report this so that the circumstances that led to this condition can be investigated.
        uint8_t NVM_Status = UPDI::lds_b(NVM_v2::NVM_base + NVM_v2::STATUS);
        uint8_t NVM_Cmnd = UPDI::lds_b(NVM_v2::NVM_base + NVM_v2::CTRLA);

Are you sure the invalid register values you are reading are not caused by the fact you are using the 16 bit address functions instead of the long address functions, thus reading from an incorrect part of memory instead of the NVM registers? I think lds_b_l() should be used everywhere the 3rd address bit might have an unknown value.

edit: actually, I was observing strange readings on the satus register, like 0xFF and thinking WTF? Then it hit me: this was uninitialized flash, I was not really reading the NVM status register at all. That was the hint that allowed me to find that earlier bug when using "has_pdi".

ElTangas commented 4 years ago

Terminal mode is working with timeouts disabled, but only for reading. It still fails for writing and explaining why will require diving into some quirks of avrdude. Here we go...

As you know, jtag2updi waits for commands from avrdude then calls the appropriate functions to handle them using a switch...case statement. Probably you found this part strange:

case JTAG2::CMND_RESET:
case JTAG2::CMND_ENTER_PROGMODE:
    JTAG2::enter_progmode();
    break;

Why on earth would reset and enter program mode act the same? So you changed CMND_RESET to do nothing because apparently it's not very useful on normal avrdude usage. Well, there is a reason: terminal mode quirks on avrdude's side. Check out the command sequence avrdude sends when you try to write memory in terminal mode:

avrdude> write eeprom 0x00 0x00
>>> write eeprom 0x00 0x00
avrdude: jtagmkII_write_byte(.., eeprom, 0x0, ...)
avrdude: jtagmkII_program_disable(): Sending leave progmode command: 0x80 (1 bytes msg)
avrdude: jtagmkII_reset(): Sending reset command: 0x80 (1 bytes msg)
avrdude: jtagmkII_write_byte(): Sending write memory command: 0xa5 (2 bytes msg)
avrdude: jtagmkII_write_byte(): bad response to write memory command: RSP_ILLEGAL_MCU_STATE
avrdude (write): error writing 0x00 at 0x00000, rc=-1
write operation not supported on memory type "eeprom"

As you can see avrdude starts by requesting programming mode exit, then sends a reset, then the write command, which of course fails because the target chip is not in programming mode anymore. Yeah...

Fortunately fixing this should be just a matter of having JTAG2::CMND_RESET and JTAG2::CMND_ENTER_PROGMODE act the same.

There is another problem, the erase command is not working when in terminal mode. I will investigate further tomorrow.

edit: anyway, this is looking good, should be ready to commit soon.

SpenceKonde commented 4 years ago

Yeah, I think you did a more successful job than I at debugging the 0xFF readings from NVMCTRL registers! I'll implement that, and also fix the issue about the reset command (that said, wouldn't terminal mode also fail on an actual JTAGICE MkII, since it's being told to leave programming mode and not reenter it...

I don't suppose there's a way to test if it's in terminal mode? One of my goals in this was to reduce the number of times it reset the target MCU to the minimum, because once the new code is uploaded, extra resets are a nuisance, since if you;'re connected to serial port while doing this (this is key to my workflow), you don't want any "startup message" to be sent multiple times if possible...

SpenceKonde commented 4 years ago

That said, I will keep that check there, since we DO need to check that CTRLA doesn't already have a command in it, and we DO need to check for errors in the status register and write them 0 if we find any (that's a difference between NVMCTRL v2 and v1

Is there any reason I can't add an extra byte to the response to enter progmode? (you mentioned this sort of thing being okay before). I'd like to be able to get the REV ID back without having to connect to the debugging console...

ElTangas commented 4 years ago
I don't suppose there's a way to test if it's in terminal mode? One of my goals in this 
was to reduce the number of times it reset the target MCU to the minimum, because
 once the new code is uploaded, extra resets are a nuisance, since if you;'re connected
 to serial port while doing this (this is key to my workflow), you don't want any "startup
 message" to be sent multiple times if possible...

I understand. OTOH, I like to debug using terminal mode and avrdude verbosity lvl 4. But I think a compromise solution is possible. Since terminal mode doesn't work anyway when timeouts are enabled, you can make it so that when host timeout is enabled, JTAG2::CMND_RESET is a nop; when disabled, it acts as JTAG2::CMND_ENTER_PROGMODE.

Is there any reason I can't add an extra byte to the response to enter progmode? 
(you mentioned this sort of thing being okay before). I'd like to be able to get 
the REV ID back without having to connect to the debugging console...

I think there is no problem, avrdude seems to accept extra data graciously. While debugging the has_pdi issue I added the NVM status register to the response from the erase command and it was ok.

edit:

You just need to increase the packet.size field to whatever is needed. Also, note that the JTAG2::set_status() function sets packet.size to 1, so if you need to change the size, it has to be done after this call.

Now I notice you tried to send the SIB here:

void JTAG2::set_device_descriptor() {
  flash_pagesize = packet.body[244];
  eeprom_pagesize = packet.body[246];
  // Now they've told us what we're talking to, and we will try to connect to it
  /* Initialize or enable UPDI */
  UPDI_io::put(UPDI_io::double_break);
  UPDI::stcs(UPDI::reg::Control_A, 0x06);
  uint8_t sib[16];
  UPDI::read_sib(sib);
  #if defined(DEBUG_ON)
  DBG::debug(sib, 16, 1);
  #endif

  if (sib[10] == '2') {
    nvm_version = 2;
  } else {
    nvm_version = 1;
  }
  packet.size_word[0] = 20;         // this line needs to go after set_status()
  packet.body[1] = 'S';
  packet.body[2] = 'I';
  packet.body[3] = 'B';
  for (uint8_t i = 0; i < 16; i++) {
    packet.body[i + 4] = sib[i];
  }
  JTAG2::ConnectedTo |= 0x01; //now connected to target
  set_status(RSP_OK);
  //  <--- here
}

It won't work like this (see comments I added to the code).

BTW, good idea connecting to the UPDI at this time, I was connecting at sign on which I agree is too soon.

ElTangas commented 4 years ago

I noticed you added this variable

uint8_t JTAG2::ConnectedTo;

From its usage, I believe it's meant to contain connection flags to host and target in bit0 and bit1? If this is the case then there is a bug I think: From JTAG2::sign_on()

  JTAG2::ConnectedTo |= 0x01; //now connected to host

From JTAG2::set_device_descriptor()

  JTAG2::ConnectedTo |= 0x01; //now connected to target

The same bit is being set in both cases.

SpenceKonde commented 4 years ago

Yeah, I caught that one last night, will update pr later today, have addressed the other things you mentioned too


Spence Konde Azzy’S Electronics

New products! Check them out at tindie.com/stores/DrAzzy GitHub: github.com/SpenceKonde ATTinyCore: Arduino support for almost every ATTiny microcontroller Contact: spencekonde@gmail.com

On Thu, May 21, 2020, 11:33 ElTangas notifications@github.com wrote:

I noticed you added this variable

uint8_t JTAG2::ConnectedTo;

From its usage, I believe it's meant to contain connection flags to host and target in bit0 and bit1? If this is the case then there is a bug I think: From JTAG2::sign_on()

JTAG2::ConnectedTo |= 0x01; //now connected to host

From JTAG2::set_device_descriptor()

JTAG2::ConnectedTo |= 0x01; //now connected to target

The same bit is being set in both cases.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ElTangas/jtag2updi/pull/31#issuecomment-632156697, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABTXEW3AD7XY5RO72OWILILRSVCU5ANCNFSM4NDNVKWQ .

Dlloydev commented 4 years ago

BTW, good idea connecting to the UPDI at this time, I was connecting at sign on which I agree is too soon.

Hello, I've been following this with interest as I've been developing a HV programmer based on a tinyAVR and am currently using a HV programmer based on the ATmega328P (Arduino Nano).

I've followed the "UPDI Enable Sequence by 12V Programming" as per documentation except that I've been using a fast double-break instead of a fast break in the sequence. For the older version of jtag2updi, this sequence has worked very well without issue. The programmer also has a mode that includes power-cycle just proir to sign-on and after the final commands are completed. This mode also worked without issue. It allows having the updi pin as an output driving an LED then uploading a new "sketch" to change the LEDs blink frequency on the fly.

Now, for some reason, the UPDI enable sequence by 12V programming will not work, unless the fast double-break is removed. I even tried a fast single-break, but needed to remove it. As it stands everything works perfectly without any break, but the sequence isn't in agreement with documentation.

Note: I've kept the whole sequence to well within the 8.8ms requirement. The "fast" break timing is well within specification. The sequence ends with a sync which I think puts the UPDI in default 4MHz autobaud.

ElTangas commented 4 years ago

@Dlloydev I think you should wait until @SpenceKonde submits the version that's in the works, then compile with the timeout feature disabled and see if it helps.

My objective is to break the least things possible that are currently working... I think we are getting there.

But anyway, you say it's working but using an undocumented sequence? The sequence I use to enable UPDI (low voltage) is a double break, which is not the documented method either, but is convenient, because it works whatever the previous state of the UPDI unit was.

I copied that from pyupdi, that is authored by someone from Atmel, so I was never very worried that it's undocumented...

Dlloydev commented 4 years ago

The issue with having to remove the double-break began prior to the new revisions from Dr. Azzy ... When I found the issue, I was using a clone of your repo dated May 5 that has the change "Disable UPDI on exit".

I've been using the double-break all along for the same reason ... that it works whatever the previous state of the UPDI unit was. The sequence I use is: HV pulse-->double-break-->sync. I tried HV pulse-->break-->sync with no luck. Strangely, now just HV pulse-->sync works fine, even with the latest version from Dr. Azzy.

Note that I've been using 1K series resistance in the UPDI signal line because I found that with 4.7K, the lower level voltage gets "lifted" when UPDI is at 4MHz. I caught this when I had the threshold setting at 1.2V on my logic analyzer ... 1.8V threshold works fine. A scope trace showed the minimum voltage to vary approx 0.6V to 0.8V, but would be closer to 0V with 2.2K or lower. I think the lifted signal was due to the effect of the UPDI's internal pullup resistor being effectively in series with the 4.7K. This is probably not an issue, but the signal is definitely "sharper" with the 2.2K or lower resistor.

ElTangas commented 4 years ago

Maybe you are using chips of a new silicon revision. I think the sequence for HW programming has been changed in newer hardware. (edit: and is not documented in the datasheet...) Let me find the link discussing this... Read this thread, in particular the post marked as solution https://www.avrfreaks.net/comment/2788846 maybe it helps.

Dlloydev commented 4 years ago

I've tested both hardware revisions (even just now with a new revision 1606 and an old revision 1604). Here's the thread where I've gone through the MCU revisions earlier in the HV programmer's development.

So, in summary, the HV programmer can work with all UPDI pin configurations when I work with cloned jtag2updi firmware dated previous to May 5 and I use a fast double-break as part of the enable sequence.

The HV programmer works with all UPDI pin configurations when I work with cloned jtag2updi firmware dated May 5 and newer and I remove the fast double-break from the enable sequence.

Here's the response from avrdude when I try to burn the bootloader on the 1606 when the enable sequence includes the double-break:

`Arduino: 1.8.12 (Windows 10), Board: "ATtiny3216/1616/1606/816/806/416/406 (optiboot), ATtiny1606, 20 MHz, Enabled (default timer), 1.8V, Disabled, Disabled, Closer to 5v, UPDI (powercycle to enter bootloader), TX:7, RX:6"

C:\Users\dlloy\AppData\Local\Arduino15\packages\arduino\tools\avrdude\6.3.0-arduino17/bin/avrdude -CC:\Users\dlloy\AppData\Local\Arduino15\packages\megaTinyCore\hardware\megaavr\2.0.2/avrdude.conf -v -pattiny1606 -cjtag2updi -PCOM10 -e -Ufuse0:w:0x00:m -Ufuse1:w:0b00000000:m -Ufuse2:w:0x02:m -Ufuse4:w:0x00:m -Ufuse5:w:0b11000100:m -Ufuse6:w:0x03:m -Ufuse7:w:0x00:m -Ufuse8:w:0x2:m -Uflash:w:C:\Users\dlloy\AppData\Local\Arduino15\packages\megaTinyCore\hardware\megaavr\2.0.2/bootloaders/optiboot_x/optiboot_txy6.hex:i

avrdude: Version 6.3-20190619 Copyright (c) 2000-2005 Brian Dean, http://www.bdmicro.com/ Copyright (c) 2007-2014 Joerg Wunsch

     System wide configuration file is "C:\Users\dlloy\AppData\Local\Arduino15\packages\megaTinyCore\hardware\megaavr\2.0.2/avrdude.conf"

     Using Port                    : COM10
     Using Programmer              : jtag2updi

JTAG ICE mkII sign-on message: Communications protocol version: 1 M_MCU: boot-loader FW version: 1 firmware version: 6.00 hardware version: 1 S_MCU: boot-loader FW version: 1 firmware version: 6.00 hardware version: 1 Serial number: 00:00:00:00:00:00 Device ID: JTAGICE mkII AVR Part : ATtiny1606 Chip Erase delay : 0 us PAGEL : P00 BS2 : P00 RESET disposition : dedicated RETRY pulse : SCK serial program mode : yes parallel program mode : yes Timeout : 0 StabDelay : 0 CmdexeDelay : 0 SyncLoops : 0 ByteDelay : 0 PollIndex : 0 PollValue : 0x00 Memory Detail :

                              Block Poll               Page                       Polled
       Memory Type Mode Delay Size  Indx Paged  Size   Size #Pages MinW  MaxW   ReadBack
       ----------- ---- ----- ----- ---- ------ ------ ---- ------ ----- ----- ---------
       signature      0     0     0    0 no          3    0      0     0     0 0x00 0x00
       prodsig        0     0     0    0 no         61   61      0     0     0 0x00 0x00
       fuses          0     0     0    0 no          9    0      0     0     0 0x00 0x00
       fuse0          0     0     0    0 no          1    0      0     0     0 0x00 0x00
       fuse1          0     0     0    0 no          1    0      0     0     0 0x00 0x00
       fuse2          0     0     0    0 no          1    0      0     0     0 0x00 0x00
       fuse4          0     0     0    0 no          1    0      0     0     0 0x00 0x00
       fuse5          0     0     0    0 no          1    0      0     0     0 0x00 0x00
       fuse6          0     0     0    0 no          1    0      0     0     0 0x00 0x00
       fuse7          0     0     0    0 no          1    0      0     0     0 0x00 0x00
       fuse8          0     0     0    0 no          1    0      0     0     0 0x00 0x00
       lock           0     0     0    0 no          1    0      0     0     0 0x00 0x00
       data           0     0     0    0 no          0    0      0     0     0 0x00 0x00
       usersig        0     0     0    0 no         32   32      0     0     0 0x00 0x00
       flash          0     0     0    0 no      16384   64      0     0     0 0x00 0x00
       eeprom         0     0     0    0 no        256   32      0     0     0 0x00 0x00

     Programmer Type : JTAGMKII_PDI
     Description     : JTAGv2 to UPDI bridge
     M_MCU hardware version: 1
     M_MCU firmware version: 6.00
     S_MCU hardware version: 1
     S_MCU firmware version: 6.00
     Serial number:          00:00:00:00:00:00
     Vtarget         : 5.0 V

avrdude: jtagmkII_set_devdescr(): bad response to set device descriptor command: RSP_NO_TARGET_POWER avrdude: jtagmkII_initialize(): Cannot locate "flash" and "boot" memories in description avrdude: AVR device initialized and ready to accept instructions

Reading | avrdude: jtagmkII_program_enable(): bad response to enter progmode command: RSP_ILLEGAL_MCU_STATE avrdude: jtagmkII_program_enable(): bad response to enter progmode command: RSP_ILLEGAL_MCU_STATE avrdude: jtagmkII_read_byte(): bad response to read memory command: RSP_ILLEGAL_MCU_STATE avr_read(): error reading address 0x0000 read operation not supported for memory "signature" avrdude: error reading signature data for part "ATtiny1606", rc=-2 avrdude: error reading signature data, rc=-2 avrdude: jtagmkII_program_disable(): bad response to leave progmode command: RSP_ILLEGAL_MCU_STATE

avrdude done. Thank you.

Error while burning bootloader.`

EDIT: The HV enable sequence uses the most strict timing information I could find and I've also measured the timings to be well within specified ranges, the total sequence is about 4ms duration, well within the "OE Timer Protection for GPIO Config" specifation of 8.8ms (from 33.3.2.4 in the ATtiny806/1606 datasheet).

EDIT: Thanks for the link ... I wish I had that specific ATtiny3217 to do some tests on. The closest I've tested to this is the 817 on the ATTINT817-XMINI board.

ElTangas commented 4 years ago

What to say... the May 5 version has only minor differences from the previous one, basically it just turns off the UPDI unit on exit. I don't see how that could break the 12V init sequence?

I would advise you to reverse the changes slowly, and see when it starts working again, so that we can get some clues. In particular, try reversing the change to the JTAG2::delay_exec() function.

Anyway, this should be discussed in another place, maybe you can create an issue? Edit: what I mean is, this issue must be tested and solved on the May 5 version, and not on SpenceKonde version which is not even ready to be committed, and has many important changes that would make debugging your issue unnecessarily difficult.

Dlloydev commented 4 years ago

Yes, I agree ... I'll see if I can isolate anything on the May 5 version report my findings in a separate issue. EDIT: Resolved: I couldn't quite narrow it down, but I believe the MCU state is now different (than April 7 version) at the point of sign-on. I've decided to use a HV sequence that only provides the 12V pulse just prior to the existing long double-break and sync. Although the long break isn't documented in the enable sequence, it doesn't preclude this. Thank you for your help and for developing the jtag2updi firmware!

ElTangas commented 4 years ago

@SpenceKonde Can you address this small issue: https://github.com/ElTangas/jtag2updi/issues/34 ?

ElTangas commented 4 years ago

@Dlloydev Glad you could get it to work, though it would be of course more satisfying to know the exact cause.

Please check if your fix also works with @SpenceKonde version once it's updated.

SpenceKonde commented 4 years ago

Yeah that one is already fixed in my codebase....

But I've managed to fuck up the progmode entry. Why the hell does avrdude have to send two enter progmode commands?!

ElTangas commented 4 years ago
Why the hell does avrdude have to send two enter progmode commands?!

Yeah, avrdude has a number of annoying quirks...

Anyway, I found the usual addressing bug cropping up on JTAG2::read_mem()

This is because non-flash memory is handled by the NVM v1 code which uses 16 bit addressing, so if you read/write flash then read another type of memory during the same session (e.g. the signature, fuses, etc...), incorrect data will be read (on DA series chips).

This is very noticeable in terminal mode, but can happen in command line mode also.

SpenceKonde commented 4 years ago

I think this has addressed all the outstanding issues! And added a few small things, reduced the reset thrash (a little), and prevented timeouts when trying to reset from hanging jtag2updi!

I now have Arduino IDE uploading to the new parts too! So now I can get to the fun part of making the Arduino API functions work. I'm where I thought I would be 3 weeks ago, buuuuut....

SpenceKonde commented 4 years ago

IME 4.7k is an excessive value of resistor - I use 470, and official Microchip proigrammers seem to use values around 1k.... (also, pyupdi needs a lower value resistors for some serial adapters, if you weren't aware)

ElTangas commented 4 years ago
  if (initial_status!=0x08){
    UPDI::CPU_reset_on();
    // Now we have time to enter program mode (this mode also disables the WDT)
    // Previously a reset was done here WHICH WOULD GUARANTEE THAT IT WAS 
    // IN NORMAL MODE!! But then we checked anyway, and went into a meaningless
    // switch case statement... because we already KNOW that it would be in 
    // normal mode, because we just reset it into normal mode.... 
    // So, since we know we'd be in normal mode, and we want to be in programming mode
    // Write NVM unlock key (allows read access to all addressing space)

There was a reason for this, contrary to what you say a reset is not guaranteed to put the MCU in normal mode, imagine there is a key loaded in the UPDI unit.

Crazy unlikely at this point in the code, but is it impossible? Truth is, a reset can put the MCU in any mode depending on whether a UPDI key is loaded.

ElTangas commented 4 years ago

Ok, I will run (hopefully) final tests, that involve programming chips that have the WDT enabled at maximum reset rate, this was a challenge for early jtag2updi versions.

SpenceKonde commented 4 years ago

Wonderful. I just fixed a silly issue with the define tests in sys.h for some of the untested parts (megaAVR 0-series and DA-series) - nothing that impacts the main targets though. Now compiles for 0-series megaAVR,.

ElTangas commented 4 years ago

Actually I found a preexisting bug now. I will correct it after committing your code.