-
Notifications
You must be signed in to change notification settings - Fork 145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace internal knowledge in jtag3.c by a public API #996
Conversation
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.
I just tried with avrdude built at this commit on macos but it does not seem to supeseed #982
Whereas with #982:
|
Is there any way I could reproduce the issue with another board? I've got an ATmega4809 Curiosity Nano board (I believe). |
@dl8dtl I believe you need an ATmega32u4 based programmer. |
I think I do have an ATmega32U2 based devboard around, but not an ATmega32U4 one. |
OK, it's not a Curiosity Nano which I've got but an Arduino Nano Every board. It doesn't have the ATmega32U4 but a SAMD11. Is it somehow possible to trigger that wrong CRC fuse setting explicitly to make it return that error? |
But it doesn't use the EDBG protocol … |
Hmm, the AVR128Dx Curiosity Nano boards use EDBG (though with a SAMD chip), and the AVR-Dx controllers have the same syscfg0 layout. |
I can confirm that this PR does not resolve #982. I have an Arduino UNO Wifi Rev2 to test with. With #982 applied:
With this (#996) applied:
|
Please run it with |
Sure! |
From here:
The above analysis assumes the issue is with avrdude, but on the other hand, SAMD21 based EDBG has no issues, can we assume the issue is more likely with the ATmega32U4 FW implementation of EDBG from Arduino side? The issues reported are only for the two Arduino Wifi boards (Uno Wifi and Mega2560 Wifi). |
FYI, I have AVR128DB48 Curiosity Nano (SAMD21 based) and ATmega328PB-xmini (ATmega32U4 based but not using UPDI mode but rather ISP mode). Both do not seem to have issues. So it seems to be specific to the ATmega32U4 based FW implementation of UPDI. It will be good to see if others Xplained Mini board using ATmega32U4 based UPDI interface have the same issues or not. For example, ATtiny817 Xplained Mini. |
I do have that one as well. Maybe it's possible to use that EDBG chip also to attach it to some external MCU. I'll give that a try.
I've got that one as well. You're right, the ATtiny817 does have the same SYSCFG fuses, so I can try it directly. |
Yep, that experiences the same behaviour. So, I've got something to test at tonight. |
I can confirm that this PR (#996) doesn't work with the ATtiny817 Xplained Mini board either (ATtiny817 replaced with an ATtiny3217). |
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.
I found a number of mistakes and confusions, and fixed them. |
Thanks for looking into it @dl8dtl! I can confirm that your latest commit fix the issue, and I'm now able to initialize my ATtiny817 Xplained Mini and my Arduino UNO Wifi Rev2 board. |
Thanks, so it would be interesting to get @umbynos to also verify it, then we can merge it. |
And any new code is encouraged to use the new symbolic return codes:
|
Monday I'll try on my Uno WiFi Rev2. Thanks 😄 |
Ok I tested this on linux with an Arduino Uno WiFi Rev2 and it seems to work. With a normal upload from the IDE and the Arduino CLI. But I did not tested this with bad CRC fuse settings. Could you help me on this? How could I write this fuse settings to my board? |
@umbynos I haven't really used the CRC functionality but according the the ATmega4809 datasheet, the default checksum is 0xFFFF, and the correct checksum has to be calculated and placed in a known location in flash. just write 0x09 to the SSYSCFG0 fuse instead of 0xC9 for the UNO Wifi Rev2 and you should be able to test the CRC fuctionality with any program as long as you don't touch the default CRC checksum of 0xFFFF.
From the datasheet:
|
Write anything but the default of E.g. in terminal mode:
As soon as you finish this session, you cannot connect anymore:
(As an improvement, we could avoid even trying to read the signature in that situation.) |
Ok, after running
But when running the same command again:
So I would say the CRC functionality works, since the board successfully recovers when flashed again from the Arduino IDE or Arduino CLI.
I can confirm that this causes:
I would say this functionality works too, since the board successfully recovers when flashed again from the Arduino IDE or Arduino CLI. 🎉 |
I also tried to revive the uno wifi rev2 from the bad CRC fuse setting without #982 and #996 and:
So I would say this supersedes #982 and fixes the issue. Thanks a lot! |
Thanks for the detailed review, much appreciated! |
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.
This is an alternative implementation proposal meant to supersede
PR #982