Fix addresses for jtag3_pdi boot and apptable #1887
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It works, neat! |
|
This PR seems to break flash writing when using the Xplained Pro and the ATmega256RFR2: Without this PR writing to flash works just fine |
MCUdude
reviewed
Aug 13, 2024
src/jtag3.c
Outdated
| unsigned long addr) { | ||
|
|
||
| return mem_is_boot(m)? MTYPE_BOOT_FLASH: | ||
| (mem_is_flash(m) && is_pdi(p) && addr >= PDATA(pgm)->boot_start)? MTYPE_BOOT_FLASH: MTYPE_FLASH; |
There was a problem hiding this comment.
It looks like the write issue for classic parts is caused by this return statement. MTYPE_FLASH_PAGE will never be returned.
There was a problem hiding this comment.
No. My initial suggestion caused -cpowerdebugger_jtag -patxmega256a3bu -T 'write flash 0 "Hello"' to fail.
return mem_is_boot(m)? MTYPE_BOOT_FLASH:
(mem_is_flash(m) && is_pdi(p) && addr >= PDATA(pgm)->boot_start)? MTYPE_FLASH: MTYPE_FLASH_PAGE;Can't we just revert back to:
if (p->prog_modes & PM_PDI) {
if (addr >= PDATA(pgm)->boot_start)
return MTYPE_BOOT_FLASH;
else
return MTYPE_FLASH;
} else {
return MTYPE_FLASH_PAGE;
}Since this works perfectly fine, and is IMO easier to read?
There was a problem hiding this comment.
On closer inspection, I think this return statement should be OK. Note the addr < PDATA(pgm)->boot_start:
return mem_is_boot(m)? MTYPE_BOOT_FLASH:
(mem_is_flash(m) && is_pdi(p) && addr < PDATA(pgm)->boot_start)? MTYPE_FLASH: MTYPE_FLASH_PAGE;There was a problem hiding this comment.
I feel like this is some kind of Whack-a-Mole game. Even with the return statement fixed, The powerdebugger_jtag flash write progress suddenly freezes at 97% when flashing an ATxmega256A3BU.
I suggest fixing the issue mentioned in my previous post and then we can look into what's going on this time.
There was a problem hiding this comment.
Note the
addr < PDATA(pgm)->boot_start
Great find. What was I thinking?
There was a problem hiding this comment.
I have redone the fix in a different way and force-pushed that. Now all moles should be hiding incl tpi, isp, pdi, updi...
stefanrueger
force-pushed
the
jtag3_pdi
branch
from
cb7960c
to
e4536c7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Might fix a bug discovered here.
@MCUdude The
jtag3.ccode looks like it would never have worked onapptableandbootmemories what withjtag3_mtype()not even considering the memory just the address andjtag3_memaddr()making no distinction between aflashand anapptable/bootmemory.