Report: Incorrect SerialUPDI control of AVR64EA32 #1529
Labels
bug
Something isn't working
Comments
|
@dbuchwald Do you want to have a look? I suspect that 24-bit addresses are needed for the 64k device because the effective address after adding the flash offset exceeds 16 bit. |
|
I will take a look, but might not be able to test it though, as I don't have the AVR64EA32 chip at hand. Anyway, will keep you posted! |
|
OK, so this is what I came up with: according to the logs above, NVM link mode is version 3, which is 16-bit address/page oriented write. I checked it again in the original Microchip's pymcuprog implementation, and this is what they do: And this is what I do: https://github.com/avrdudes/avrdude/blob/main/src/serialupdi.c#L163 So, as long as the chip itself reports to be NVM link version 3, we should and will use 16-bit addresses. The only thing I can suggest now is for @askn37 to try to program the original device using pymcuprog software and see if it works. If it fails, then probably there is some bug in interface/documentation provided by Microchip. |
|
Edit: I realized that the issue is about SerialUPDI. I need to see how I can test this. I have not soldered the pin headers. I will see if I can carry out the test using serialupdi next week. +++++++++++++++++ Just tested again with latest git main and it is still okay. I am using Microchip AVR64EA48 Curiosity Nano bought from Mouser SIngapore. Full debug log here. click for the big full debug log with -vvvv |
mcuee
added
unconfirmed
|
I have changed the label to I will change the lable back to |
|
I can not test this right now but I am not sure if you can get some useful info from my debug log using the |
|
@askn37 Thanks for reporting and for expanding your submission with additional information. Could I ask you to give the complete line, including which |
|
@mcuee if you have access to Curiosity Nano board with AVR64EA32 (or AVR64EA48) chip, you can connect your SerialUPDI programmer like so:
I know it looks flimsy AF, but it does work, good enough to run some tests. That being said, I want to clarify something - my implementation of SerialUPDI in AVRDUDE was always meant to be just a port of Python-based Microchip reference implementation. It was never built directly based on chip datasheets, an for a reason. I could have never dreamt of testing my code against statistically significant sample of the actual microcontrollers, so following manufacturer's reference implementation was the only "natural" thing to do. Bottom line: I really need to know two things:
Please note: English is not my first language, and now rereading the above it sounds like I don't want to handle this issue, but that would be wrong impression. I really do want to help, and I'm happy to experiment with different approaches, but I really need to know that whatever we are trying to do here will not break existing functionality. |
|
Thanks a lot for the tip. I will try to get some compariosons done tomorrow.between pymcuprog and avrdude. No worry, you have been very helpful to avrdude project. |
Unfortunately, it appears that pymcuprog has not been updated since October 2022. Microchip published his AVR16EAxx and AVR32EAxx repositories in April 2023 and began shipping engineering samples to customers. Therefore, it is unlikely that pymcuprog can expect full compatibility with his NVMCTRL version 3. In fact, the first release of Microchip's AVR-LIBC library for his AVR32EAxx had an EEPROM control bug that incorrectly used version 0 code for NVMCTRL version 3. I first discovered it and reported it to Microchip. The repository was properly modified and published in July. I have never tested pymcuprog before. Because debugging Python is not my main job. However, if new issues are discovered, you will need to report them to Microchip again. (By the way, my native language is Japanese. Conversation with a microchip is quite troublesome.) |
|
Just wondering if you can help to check here. Thanks. |
Thank you, @dbuchwald. This is great. Given the choice between a working implementation and one that's conform to a possibly outdated reference implementation, I'd go for the former in a heartbeat. It would be good if you could continue to look after the UPDI implementation @dbuchwald. Many thanks all round, @mcuee for testing and @askn37 for making us aware of this problem. (@askn37 if you have specific code suggestions to try, please feel free to share). |
|
@mcuee if you get some time to replicate the issue, you can also grab the version I just provided via PR above - it basically replaces 16-bit addressing mode for NVM v3 controllers by 24-bit mode. You can test if it works any better. If it works, then we need to look into this "sleep" mode, and as far as I can tell from the datasheet, it might not be trivial thing to do: The UPDI PHY layer runs independently of all sleep modes, and the UPDI is always accessible for a connected debugger independent of the device’s sleep state. If the system enters a sleep mode that turns the system clock off, the UPDI cannot access the system bus and read memories and peripherals. When enabled, the UPDI will request the system clock so that the UPDI always has contact with the rest of the device. Thus, the UPDI PHY layer clock is unaffected by the sleep mode’s settings. By reading the System Domain in Sleep (INSLEEP) bit in the ASI System Status (UPDI.ASI_SYS_STATUS) register, it is possible to monitor if the system domain is in a sleep mode. It is possible to prevent the system clock from stopping when going into a sleep mode by writing to the Request System Clock (CLKREQ) bit in the ASI System Control A (UPDI.ASI_SYS_CTRLA) register. If this bit is set, the system’s sleep mode state is emulated, and the UPDI can access the system bus and read the peripheral registers even in the deepest sleep modes. |
mcuee
added
bug
|
I will label this as bug for now based on the reports by @askn37. But I will try to carry out the test to confirm later. |
|
I can confirm there is an issue in git main and your PR #1531 helps, at least for reading.
PR #1531 is good in terms of verification. |
I managed to hold the pin header and PR #1531 is good as well. git main will fail. |
|
BTW, I need to mention that the nice setial2updi adapter (with pogo-pins or pin headers) is courtesy of @SpenceKonde (aka Dr. Azzy). He is very kind and sent me the following at the end of last year.
|
|
Please give PR #1531 a try. Thanks. My testing shows that it works fine. |
|
I would like to thank you all for your great work. The attached code is a program that allows you to experience sleep failures (rather than address width issues). This applies to tinyAVR-2 and most of its AVR_DA/DB/DD/EA/EB. It can be constructed with AVR-LIBC. When I run it, the LED keeps blinking using PA7, but the CPU remains in a STANDBY sleep state with interrupts disabled. Also, F_CPU is 512Hz. (TCB0, CCL, and EVOUT are used for this LED blinking operation, but tinyAVR-0/1 and megaAVR-0 are not supported because the difference in CCL is troublesome.) Please upload your code to the device using a method other than the problematic SerialUPDI and test it with SerialUPDI. It is important not to reset the CPU at this point. The CPU startup start time is affected by the SUT setting value of FUSE_SYSCFG1. To test without going to sleep, run a SerialUPDI with the reset pad LOW, wait a second or two, then release the reset. SerialUPDI should be able to recognize the CPU. A power-on reset is required if the chip's reset pad functionality is disabled in FUSE_SYSCFG0. Engineering samples of the AVR16EBxx will begin arriving to stakeholders soon. It seems that the application for free distribution has ended. |
Sure, i can take a closer look today. |
And that was the change I made - chips reporting NVM v3 are now handled with 24-bit addressing. That being said, it would be great to find some sort of list of all the devices using this NVM mode to test it with more than one or two... |
If this is not trivial to do, I think we can just document this as a limitation. |
|
@mcuee This is the schematic for the CH340X prototype board. The CH340X is an improved version of the CH340E and also includes the features of the CH340K. CH340K had problems in the past due to VID differences, but CH340X has high compatibility. By adding additional resistors to TNOW, DTR can be extended in two ways during POR. The latest hardware design guide for modernAVR can be found in their respective datasheets. UPDI and RESET have built-in 30kΩ pull-ups that activate on POR, unlike older AVRs. Therefore, only a stabilizing capacitor is required externally. If you add a reset switch, also add a filter resistor. 
His RESET prototype board for AVR follows the latest design guidelines. The difference is that instead of being connected to GND, it is connected to his RTS, which is Low by default. Therefore, the stabilization capacitor is charged by the internal pull-up. When the RTS changes, it discharges with opposite polarity and the potential drops for a short time. It will stay below 0.2VDD until RESET is charged again. Old AVR design guides recommended a 10kΩ RESET pullup. The reason this is no longer mentioned is probably because the internal high frequency RC oscillator is slow and you want to have enough time for POR. Additionally, the UPDI circuit (sub-CPU) operates using a dedicated internal RC oscillator that is separate from the main clock. In the worst case, it may boot slower than the main CPU. To avoid this (preventing abnormalities due to inrush current), FUSE has a startup timer setting. The adjustment range is up to 64ms, so it is best to keep this delay phenomenon in mind. |
No issues, It is probably my fault. :-) I think we can continue here as the topics are all related to serialupdi. |
|
I tried to debug the MinGW 115200bps issue and adding usleep here and there does not help. I think the issue is that ir can not communicate at 115200 bps at all, at least initially with the MinGW build. Adding usleep will not help. |
|
Even at baud rate of 57600 it failed initially but luckily it can recover. |
|
As there is no issue with WIthout As per the following documentation from @SpenceKonde, for DxCore, 57600bps works across different adapters.
|
Looks like this is very specific to MSYS2 mingw compiler. I just tested Arduino-packing build of avrdude 7.2 mingw32 release and my own git main build using cross-compiling, there are no issues with Arduino-packing official build of avrdude 7.2 release My own git main build |
|
Thoughts on silicon revision checking Revision checking is currently implemented in four types: jtag3.c, jtagmkII.c, serialupdi.c, and stk500v2.c. Among these, jtag is called with optional functions such as "pgm->read_chip_rev". STK500/600 is also referenced after "program_enable". Before that, only serialupdi.c is referenced. If you think about it, the revision information should be a clue to identifying the errata, which is her 4th byte in the 3-byte signature extension. Therefore, it stands to reason that there is a close relationship between signatures and amendments. It would be even more useful if they were displayed side by side in the console log. I think it would be better and more consistent to implement serialupdi.c, similar to jtag*.c. I think that's the case based on main.c, what do you think? |
|
Yes, that's right. By the way, I have often read REVID by writing it as below. There is a reason why I got into the habit of checking REVID. This is the same as AVR_DB and later, "AVR" + 5 spaces. However, the SIB of AVR128DA28 rev.A7 that I ordered after that was Same style as tinyAVR/megaAVR, 4 blank spaces + "AVR"? ? ? Apparently AVR128DA28/32/48/64 (DS40002183C) is this old style. AVR64DA28/32/48/64 (DS40002233B) and AVR32DA28/32/48 (DS40002228B) are different in some strange ways. |
|
Guys, I'm sorry, but I feel a bit lost in these discussions, and I find them harder and harder to follow. Apologies if you expect my input on some of the topics brought up here, but at this point I can only react on certain trigger words :) I noticed discussion about RTS/DTR and using it for resetting AVR - sounds like a good idea, but before you make changes to the code that handles it in SerialUPDI, please note that this additional setting (
So, my point is that whatever you decide about using RTS/DTR lines, please keep in mind that there is at least one device in the wild that makes strong assumption as to what these signals are for, and compatibility, if possible, should be maintained. Otherwise, if you need me to look at some code or test something - please tag me in the post, otherwise I might miss it. |
|
I think the only thing for you now is to take a look at the log from @askn37 here to see if you have any concerns or not. There is a warning in his log. @askn37 mentions the following which may or may not be an issue.
|
You are welcome to come out with a PR to make |
So there will be three cases.
@dbuchwald and @stefanrueger |
The only reason this was implemented was to support the dual-mode operation of MCUdude's programmer. I don't remember why it was both RTS and DTR at the same time, but maybe it could be separated. I'm currently going over the very long e-mail discussion between myself and MCUdude about the quirks of serial interfaces, and I found some small things that might help you anticipate potential issues:
Also another thing is that the 300-baud workaround I borrowed from
Again, I'm not trying to say that my implementation is perfect and should not be changed. Feel free to change it any way you want, and if you need my help, I will provide it. What I am saying is that when I worked on it almost two years ago I went through a lot of testing in different environments, and some discrepancies between them were deal-breakers for some pretty good ideas. That being said, context might have changed during these two years, some of these issues might have been fixed. Maybe the MacOS bug has been fixed in the meantime. |
|
*** HEADS UP *** Using DTR/RTS instead of GPIO will definitely have side effects as it is not compliant with regulations such as RS232C. Don't expect 100% compatibility. . . . But it's convenient, isn't it? Arduino IDE Serial Monitor (Windows/macos/Linux)This would be a good standard to use. DTR/RTS is not asserted. The behavior when opened varies depending on the OS and driver. In most cases, RTS=LOW and DTR=LOW when open and RTS=HIGH and DTR=HIGH when closed. So if your target has an auto-reset circuit (series capacitor), it will reset every time you open it again. The CH330/CH340 series was well received, so the manufacturer improved the driver so that RTS=LOW when opened. Screen and MINICOM (*UNIX)When opened, asserts DTR/RTS. The UART switching implementation for "microUPDI adapter" will not work properly because DTR = HIGH and RTS = HIGH. Workarounds include turning off "hardware flow", temporarily setting it to 0bps, and running stty. (I'm not good at it so I don't know the details) TeraTerm (Windows)DTR/RTS is normally not asserted. You can turn DTR/RTS on and off individually as needed using the "hardware flow" or using a macro file. |
To be honest, I think we (avrdude project) do not really need to take care of the serial monitor function. Rather we need to take care of the programming function which may invovle chip reset. There are special cases for serialupdi which takes care of @MCUdude's paticular design. For jtag2updi, it is the same siutation, only certain type of cictuits will work with If we really want to be more generic, I think we need generic So far the following PR seems to work fine for different platforms. The focus is the bootloaders (urclock, optiboot and wiring) programming where proper chip reset is important. The focus is not the UART monitor. |
|
OK, I learned a few things about the way different OS assert DTR/RTS during the low-level
@askn37 Pro Tip: try using Other than this, we probably might want to close the issue as the main problem that NVMCTRL v3 should use 24 bit is solved. |
|
@dbuchwald Thanks for explaining the motivation and history of the rtsdts handling. That is really helpful! |
Ok, I agree with that policy. |
I agree. |
Yes, I agree with you here. |
|
Just want to mention the sleep MinGW issue is indeed specific to MSYS2 mingw compiler. I just built avrdude using MSYS2 clang64 and it works fine. |
|
I'm looking into "serialupdi_initialize". Things I can do right away: The "read chip silicon revision" process that inspired this investigation can be improved by simply moving it to the end of the function. Since "serialupdi_enter_progmode" is called before that, "UPDI_ASI_SYS_STATUS_NVMPROG" will be true and will not be affected. Strange part: "updi_read_sib" is called after checking "UPDI_ASI_SYS_STATUS". This is strange. What I know: For NVMCTRLv0/2, it is safe to run "serialupdi_enter_progmode" when "RSTSYS=0", "INSLEEP=any", "NVMPROG=any", "UROWPROG=0", "LOCKSTATUS=0". v3 also requires "BOOTDONE=1". |
I think this is safe to do. PR is welcome. |
@dbuchwald |
This was referenced Oct 29, 2023
This issue was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Report: Incorrect SerialUPDI control of AVR64EA32
This is a known bug as of commit 481a91d.
case.1 Debug log
case.2 Debug log
Need additional information?
I design my own program writer and create firmware that meets various practical needs. This time, we also acquired control including HV programming using AVR64EA32. I noticed this problem when I was doing comparison testing with other programmers.
NVMCTRL version 3 can correctly interpret 24-bit extended addresses. Although this is clearly stated in the datasheet, NVMCTRL_ADDRESS is a 24-bit register. This allows 16-bit address commands for UPDI access to correspond to LD/ST instructions, and 24-bit address commands to correspond to ELPM/SPM instructions. To treat the flash memory area linearly, you need to set his MSB of this address. In other words, the setting value is 0x800000 or higher. This behavior is equivalent to the AVR_Dx series (NVMCTRL version 2).
This series has a maximum flash capacity of 64KiB, so there is no RAMPZ register. The FLMAP field, which switches the latter bank of data space, also uses only one bit. However, there is no need to manipulate these from UPDI.
REVID register 0xF01 is a register in the IO memory space, so it can only be read when the CPU is active or in his NVMPROG mode. The current implementation of SerialUPDI probably does not take into account the existence of power-saving devices that regularly use CPU sleep.
(Supplement) Use PF6 (RESET pad) to enable high voltage mode. The datasheet states the absolute rating is 9V. Therefore, a suitable high voltage here is 8.2 V when using a Zener diode. However, PICkit4 can exceed this voltage and violate ESD protection.
Further additional information
There are two execution result logs here. Both are displayed when -vvvvq is specified. One result is the target device is asleep, and the other result is the target device is running.
test_run.log
test_sleep.log
The text was updated successfully, but these errors were encountered: