Read chip silicon revision and add IO memory on chips with PDI and UPDI #1474
Conversation
|
BTW the |
|
This is indeed interesting feature, @MCUdude, and I'm happy to look into it. Just give me couple of days to play with it and understand the code again :) |
|
Excellent @dbuchwald! |
|
Hello, since this change in https://github.com/MCUdude/avrdude/pull/4/files Have fun and let me know if there are any issues with this implementation. |
Add serialupdi chip silicon revision number read
|
Thank you for the PR @dbuchwald! I've tested it, merged it and it works flawlessly. I think this PR is ready to be merged. Anyone else that wants to give this PR a try? |
|
Looks good for the AVR64EA48. |
|
Good for the ATtiny817 as well. |
This PR is good from my side. Thanks. |
That are set up to communicate with Xmega chips
|
I managed to add the REVID memory to avrdude.conf, and be able to read it using a PDI or JTAG programmer using the EDBG protocol. I'll have to look into how I can accomplish the same using the jtagmkII protocol and the stk500v2 protocol. |
MCUdude
changed the title
|
I've now updated the PR to support Xmegas as well. You can add Supported protocols are jtag3 (PDI and JTAG), stk500v2/xprog (PDI) and jtagmkii (PDI and JTAG). Tested with a Power Debugger, AVRISPmkII, and an AVR dragon in PDI and JTAG mode. Feel free to give it a try using your favorite Xmega-compatible programmer! |
|
One thing I just realized I haven't made sure that Avrdude won't attempt to read the chip revision( |
|
Hmmm. I suspect there is a gating in silicon in that case, but can't confirm without asking an expert... |
|
Sanity check: The following table shows the parts by their
|
@xedbg I tried 0xB7 as well (
I had a look at the family datasheets, and I'm pretty sure I've gotten the sizes right. Feel free to double check! |
Good, I put the table there to ward off inheritance errors. |
MCUdude
changed the title
|
@MCUdude You were right about the merge conflicts. Thanks for dealing with them. I did another sanity cross check of the
Ignoring the classic parts for now (I will deal with them in an automated way for completeness later) there are still differences between the .atdf files and this PR but I think this PR is right. The 0x103f size looks wrong (probably caused by mistaking the last address for the size of a memory). |
|
@stefanrueger I've now resolved the merge conflict, so the PR is now ready. |
|
Wait, why is the size of IO 0x1100 for megaAVR0 and tinyAVR0/1/2? This doesn't comply with the information provided in the datasheet! What's correct, and what isn't? From ioavr128da48.h: /* NVMCTRL - Non-volatile Memory Controller */
#define NVMCTRL_CTRLA _SFR_MEM8(0x1000)
#define NVMCTRL_CTRLB _SFR_MEM8(0x1001)
#define NVMCTRL_STATUS _SFR_MEM8(0x1002)
#define NVMCTRL_INTCTRL _SFR_MEM8(0x1003)
#define NVMCTRL_INTFLAGS _SFR_MEM8(0x1004)
#define NVMCTRL_DATA _SFR_MEM16(0x1006)
#define NVMCTRL_DATAL _SFR_MEM8(0x1006)
#define NVMCTRL_DATAH _SFR_MEM8(0x1007)
#define NVMCTRL_ADDR _SFR_MEM32(0x1008)
#define NVMCTRL_ADDR0 _SFR_MEM8(0x1008)
#define NVMCTRL_ADDR1 _SFR_MEM8(0x1009)
#define NVMCTRL_ADDR2 _SFR_MEM8(0x100A)
#define NVMCTRL_ADDR3 _SFR_MEM8(0x100B)
/* LOCK - Lockbits */
#define LOCK_KEY _SFR_MEM32(0x1040)
#define LOCK_KEY0 _SFR_MEM8(0x1040)
#define LOCK_KEY1 _SFR_MEM8(0x1041)
#define LOCK_KEY2 _SFR_MEM8(0x1042)
#define LOCK_KEY3 _SFR_MEM8(0x1043)From iom4809.h: /* SYSCFG - System Configuration Registers */
#define SYSCFG_REVID _SFR_MEM8(0x0F01)
#define SYSCFG_EXTBRK _SFR_MEM8(0x0F02)
#define SYSCFG_OCDM _SFR_MEM8(0x0F18)
#define SYSCFG_OCDMS _SFR_MEM8(0x0F19)
/* NVMCTRL - Non-volatile Memory Controller */
#define NVMCTRL_CTRLA _SFR_MEM8(0x1000)
#define NVMCTRL_CTRLB _SFR_MEM8(0x1001)
#define NVMCTRL_STATUS _SFR_MEM8(0x1002)
#define NVMCTRL_INTCTRL _SFR_MEM8(0x1003)
#define NVMCTRL_INTFLAGS _SFR_MEM8(0x1004)
#define NVMCTRL_DATA _SFR_MEM16(0x1006)
#define NVMCTRL_DATAL _SFR_MEM8(0x1006)
#define NVMCTRL_DATAH _SFR_MEM8(0x1007)
#define NVMCTRL_ADDR _SFR_MEM16(0x1008)
#define NVMCTRL_ADDRL _SFR_MEM8(0x1008)
#define NVMCTRL_ADDRH _SFR_MEM8(0x1009)
/* SIGROW - Signature row */
#define SIGROW_DEVICEID0 _SFR_MEM8(0x1100)
#define SIGROW_DEVICEID1 _SFR_MEM8(0x1101)
#define SIGROW_DEVICEID2 _SFR_MEM8(0x1102)
#define SIGROW_SERNUM0 _SFR_MEM8(0x1103)
#define SIGROW_SERNUM1 _SFR_MEM8(0x1104)
#define SIGROW_SERNUM2 _SFR_MEM8(0x1105)
#define SIGROW_SERNUM3 _SFR_MEM8(0x1106)
#define SIGROW_SERNUM4 _SFR_MEM8(0x1107)
#define SIGROW_SERNUM5 _SFR_MEM8(0x1108)
#define SIGROW_SERNUM6 _SFR_MEM8(0x1109)
#define SIGROW_SERNUM7 _SFR_MEM8(0x110A)
#define SIGROW_SERNUM8 _SFR_MEM8(0x110B)
#define SIGROW_SERNUM9 _SFR_MEM8(0x110C)
#define SIGROW_OSCCAL32K _SFR_MEM8(0x1114)
#define SIGROW_OSCCAL16M0 _SFR_MEM8(0x1118)
#define SIGROW_OSCCAL16M1 _SFR_MEM8(0x1119)
#define SIGROW_OSCCAL20M0 _SFR_MEM8(0x111A)
#define SIGROW_OSCCAL20M1 _SFR_MEM8(0x111B)
#define SIGROW_TEMPSENSE0 _SFR_MEM8(0x1120)
#define SIGROW_TEMPSENSE1 _SFR_MEM8(0x1121)
#define SIGROW_OSC16ERR3V _SFR_MEM8(0x1122)
#define SIGROW_OSC16ERR5V _SFR_MEM8(0x1123)
#define SIGROW_OSC20ERR3V _SFR_MEM8(0x1124)
#define SIGROW_OSC20ERR5V _SFR_MEM8(0x1125)
#define SIGROW_CHECKSUM1 _SFR_MEM8(0x112F)From iotn3227.h: /* SYSCFG - System Configuration Registers */
#define SYSCFG_REVID _SFR_MEM8(0x0F01)
/* NVMCTRL - Non-volatile Memory Controller */
#define NVMCTRL_CTRLA _SFR_MEM8(0x1000)
#define NVMCTRL_CTRLB _SFR_MEM8(0x1001)
#define NVMCTRL_STATUS _SFR_MEM8(0x1002)
#define NVMCTRL_INTCTRL _SFR_MEM8(0x1003)
#define NVMCTRL_INTFLAGS _SFR_MEM8(0x1004)
#define NVMCTRL_DATA _SFR_MEM16(0x1006)
#define NVMCTRL_DATAL _SFR_MEM8(0x1006)
#define NVMCTRL_DATAH _SFR_MEM8(0x1007)
#define NVMCTRL_ADDR _SFR_MEM16(0x1008)
#define NVMCTRL_ADDRL _SFR_MEM8(0x1008)
#define NVMCTRL_ADDRH _SFR_MEM8(0x1009)
/* SIGROW - Signature row */
#define SIGROW_DEVICEID0 _SFR_MEM8(0x1100)
#define SIGROW_DEVICEID1 _SFR_MEM8(0x1101)
#define SIGROW_DEVICEID2 _SFR_MEM8(0x1102)
#define SIGROW_SERNUM0 _SFR_MEM8(0x1103)
#define SIGROW_SERNUM1 _SFR_MEM8(0x1104)
#define SIGROW_SERNUM2 _SFR_MEM8(0x1105)
#define SIGROW_SERNUM3 _SFR_MEM8(0x1106)
#define SIGROW_SERNUM4 _SFR_MEM8(0x1107)
#define SIGROW_SERNUM5 _SFR_MEM8(0x1108)
#define SIGROW_SERNUM6 _SFR_MEM8(0x1109)
#define SIGROW_SERNUM7 _SFR_MEM8(0x110A)
#define SIGROW_SERNUM8 _SFR_MEM8(0x110B)
#define SIGROW_SERNUM9 _SFR_MEM8(0x110C)
#define SIGROW_OSCCAL16M0 _SFR_MEM8(0x1118)
#define SIGROW_OSCCAL16M1 _SFR_MEM8(0x1119)
#define SIGROW_OSCCAL20M0 _SFR_MEM8(0x111A)
#define SIGROW_OSCCAL20M1 _SFR_MEM8(0x111B) |
|
Hold on, what about my changes? Are they still needed?
Sorry, I might have misunderstood, but it seemed to me there were multiple subjects covered here, including new memory type (SIB), and I still haven’t implemented support for it in serialupdi programmer. On top of that, there are still changes in serialupdi.c related to reading chip revision during initialisation. Should it stay there?
Best regards,
Dawid
|
|
@dbuchwald a separate PR that deals with SIB read has already been merged, and @stefanrueger did the serialupdi implementation, see #1480 |
|
@stefanrueger I suggest we change the size of the IO memory for the megaAVR-0 and tinyAVR-0/1/2 to 0x1100, as the atdf files do. Then we are also able to access the NVM registers as well. from the megaAVR-0 family datasheet: 
from the AVR128DA48 datasheet: 
|
|
Thanks for clarification!
Dawid
|
@dbuchwald PR #1480 introduced these changes: some overcautious sanity checks, returning OK if a read-only memory is written to with its unchanged content (for a future backup/restore extension of AVRDUDE so that there is no error restoring the existing contents to read-only memories). It turned out that the info needed was already there as you pointed out (thank you). If you feel strongly about formatting/programming style (the PR uses a particularly dense writing style) feel free to put in a PR that adjusts the code to your style.
@MCUdude Yes, can do. I would first check whether this creates overlapping memories. These are a nightmare in the I will later, in a separate PR, add |
|
For info, now this PR has the following sizes
|
|
@MCUdude Having looked closer at the [EDIT: the |
This means that the IO memory size can't be larger than 0x1000 bytes on megaAVR0, tinyAVR0/1/2, and AVR-Dx/Ex. Do you want me to update the PR where I change the size to 0x1000? |
Yes, I think this is how it was before. Your first analysis was right, the .atdf are wrong as they cover more than the volatile memories. |
|
@MCUdude If you are happy and have finished testing I can merge |
|
if we extend |
Your call. Please just make sure |
I'll double-check that this won't lead to any overlap what so ever. |
MCUdude
force-pushed
the
megaavr-chiprev
branch
from
ad05b94
to
28d4aa6
Compare
|
I've rolled back the last commit, so now the IO memory correlates with the ATDF files (well, except for the 0x103F/0x1040 typo). The registers for the NVM controller (which I'm pretty sure are volatile) are now included in the IO memory, I'm fine with merging this PR. Too bad I couldn't figure out how to write to the Xmega IO registers... |
Resolves #1472.
This PR allows the jtag2updi or any JTAG3 compatible programmer that speaks UPDI to read the chip silicon revision. pymcuprog already has this feature, and I thought it might be useful in case someone is chasing a silicon bug "ghost" and don't know the revision of their chip.
I've tested this PR with an AVR128DA48, AVR128DB48, ATtiny3217, and AVR64DD32 using the
pkobn_updiprogrammer.I've tested the AVR128DA32 and the ATmega4809 using the jtag2updi and the xplainedmini_updi programmer.
I still haven't been able to figure out how to add this feature to the serialUPDI programmer. I'm having a really hard time navigating the pymcuprog source code. If you have time @dbuchwald, could you please have a look at this?
pymcuprog output:
avrdude output: