Add memory sram and remove memory data #1571
Conversation
The memory data was introduced for the sole purpose to provide an nvm_data_offset for Microchip programmers. As an address offset it would tell programmers to read/write in data space. It so turned out that this nvm_data_offset always was 0x1000000 for PDI and UPDI parts. This commit removes this artificial data memory and treats the data offset in the code as what it currently is: a constant. At the same time the commit introduces an sram memory with offset and size for virtually all parts known to AVRDUDE.
|
Added handling of SRAM in the same way as IO is handled. Some programmers need the additional offset of diff --git a/src/jtagmkII.c b/src/jtagmkII.c
index c24d27cc..885aae16 100644
--- a/src/jtagmkII.c
+++ b/src/jtagmkII.c
@@ -2223,8 +2223,7 @@ static int jtagmkII_read_byte(const PROGRAMMER *pgm, const AVRPART *p, const AVR
cmd[1] = MTYPE_PRODSIG;
pmsg_notice2("in_sigrow addr 0x%05lx\n", addr);
} else if (mem_is_io(mem) || mem_is_sram(mem)) {
- cmd[1] = MTYPE_FLASH;
- addr += avr_data_offset(p);
+ cmd[1] = MTYPE_SRAM;
} else {
pmsg_error("unknown memory %s\n", mem->desc);
return -1;
@@ -2347,8 +2346,7 @@ static int jtagmkII_write_byte(const PROGRAMMER *pgm, const AVRPART *p, const AV
if (pgm->flag & PGM_FL_IS_DW)
unsupp = 1;
} else if (mem_is_io(mem) || mem_is_sram(mem)) {
- cmd[1] = MTYPE_FLASH; // Works with jtag2updi, does not work with any xmega
- addr += avr_data_offset(p);
+ cmd[1] = MTYPE_SRAM;
} else if(mem_is_readonly(mem)) {
unsigned char is;
if(pgm->read_byte(pgm, p, mem, addr, &is) >= 0 && is == data)@askn37 @mcuee @MCUdude Could you test IO/SRAM read/write in this PR. If that turns out OK, could you make above change and try whether IO/SRAM access is still OK using jtagmkII type programmers? |
I think it was probably expanded for the XMEGA series (25bit BUS). This is because the area above 0x01000000 is treated as a data region, and the area below it is treated as a code region. This also applies to avr-gcc's internal calculations. (architecture numbers are 102, 103, 104)
That's what I discovered while testing FWV=7. avr-gcc compiles the UPDI architecture the same way it compiles the XMEGA architecture. Until then 0x10000000 is indeed a data region. Then, when creating the HEX from ELF, accurately remap the addresses for the UPDI device. The UPDI architecture no longer uses 0x10000000 to indicate a data region. As a result, MTYPE_SRAM and MTYPE_FLASH appear to have been swapped. Furthermore, small flash memories such as BOOTROW are now embedded in the data region. Nothing definitive yet, but since this belongs to a different region (separately from MTYPE_FLASH) it may be necessary to assign MTYPE_FLASH_PAGE.
confirm. Please wait. |
|
First simple test with |
|
There seems to be no support of SRAM by ATmega328PB, using either usbasp or AVRISP mkII. |
|
No issues with ATtiny817 using Xplained Pro built-in UPDI programmer. |
|
AVRISPmkII does not seem to be able to dump SRAM for ATxmega32A4U. |
|
I tried using JTAG2UPDI. The JTAGICEmkII compatible implementation of JTAG2UPDI does not distinguish between memory types in the read direction at all. Simply read the specified 24-bit address (ignoring the 4th octet). The write direction identifies whether the memory type is flash or other. Since NVMCTRL_STATUS is not checked, memory write errors are not returned (if the device is normal, no results other than timeout/USB packet lost on the AVRDUDE side will be returned). Currently, if MTYPE_SRAM is passed, reading is possible, but writing returns RSP_ILLEGAL_MEMORY_TYPE. SRAM reads are passed from AVRDUDE as MTYPE_FLASH. Her fourth octet of the address field contains an invalid value. It's very slow because it's in bytes rather than bulk transfer, but it loads fine. SRAM writes were passed as MTYPE_FLASH from AVRDUDE. Her fourth octet of the address field contains an invalid value. The flash update routine runs byte by byte instead of all at once, so the transfer is much slower, but AVRDUDE does not return an error. (The result of NVMCTRL itself is an update error) Check how the SRAM changes when writing USERROW using a JTAG2UPDI clone modified to support AVR_EA and USERROW key writing. If the implementation is as described in the data sheet, you should be able to observe that a copy of the written USERROW appears at the beginning of the SRAM immediately after writing the USERROW. I used AVR64DD32 as the target (which happened to be connected). It seems that the results were as per the specifications. |
|
Added a commit to sort all memories of parts in canonical order. This was done here rather than in PR #1568 to avoid merge conflicts. |
|
Testing with serialupdi and AVR64EA32. There is no problem with this SRAM operation. |
|
@askn37 @mcuee Thanks for testing!
Do you have a suggestion for how AVRDUDE should handle this for XMEGAs and modern (UPDI) parts? I am not convinced that AVRDUDE got it's handling right. Do you want to suggest a patch or a PR for correct
Correct; there is no method other than bootloaders that can access |
I don't have a clear opinion either. However, I understand that there are some things that need to be done differently with PDI and UPDI. For example, this requires careful consideration.
UPDI also includes OCD functionality, allowing access to SRAM and IO registers. Perhaps that's the difference. |
|
@mcuee @MCUdude @askn37 In absence of a clear steer I propose to consider this PR finished. I realise there might be some inconsistency in how AVRDUDE sets the memory type but
Sooo, I'll merge this PR at the next mergefest unless I hear otherwise |
|
I agree to merge this PR. |
|
I have no objection either. I think I have cleared this theme. |
The memory
datawas introduced for the sole purpose to provide annvm_data_offsetfor Microchip programmers. As an address offset it would tell programmers to read/write in data space. It so turned out that thisnvm_data_offsetalways was0x1000000for PDI and UPDI parts.This PR removes this artificial
datamemory and treats thedataoffset in the code as what it currently is: a constant.At the same time the PR introduces an
srammemory with offset and size for virtually all parts known to AVRDUDE.Here a summary for the
srammemory sizes and offsets (Issue #1559).Known issues
sramis not yet documented (but will be if this PR is accepted in principle)The code base does not yet have code to r/w to thesrammemory