Skip to content
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

Incorrect implementation of SerialUPDI communication with NVM Controller in NVM versions V3, V4 and V5 #1621

Closed
dbuchwald opened this issue Jan 17, 2024 · 1 comment · Fixed by #1627
Labels
bug Something isn't working

Comments

@dbuchwald
Copy link
Contributor

As reported by @askn37, current SerialUPDI implementation of AVRDUDE incorrectly maps NVMCTRL STATUS register to offset 0x1002, whereas for NVM controllers V3, V4 and V5 it should have been 0x1006.

On top of that, wait operation and error handling for NVM V2, V3, V4 and V5 needs rework, there are different bit masks indicating errors for each of these versions.

Please note: current testing script hosted here does not detect this issue, so the first step forward would be to write test case that could be used to verify the bug and confirm the fix works.

During the investigation it was found that pymcuprog was updated recently (with new register mapping for new NVM versions), so these updates must be also reflected in scope of this issue.

@MCUdude MCUdude added the bug Something isn't working label Jan 17, 2024
dbuchwald added a commit to dbuchwald/avrdude that referenced this issue Jan 20, 2024
@dbuchwald
Copy link
Contributor Author

@stefanrueger @askn37 @MCUdude

I have just completed implementation of NVM v4 protocol candidate. Based on pymcuprog it's more like NVM v2 + new register layout and reversed bits in NVM_STATUS register (?!):

diff nvmp2.py nvmp4.py
2c2
< NVM controller implementation for P:2.
---
> NVM controller implementation for P:4.
4c4
< Present on, for example, AVR DA, DB, DD
---
> Present on, for example, AVR DU
11c11
< class NvmUpdiP2(NvmUpdi):
---
> class NvmUpdiP4(NvmUpdi):
13c13
<     Version P:2 UPDI NVM properties
---
>     Version P:4 UPDI NVM properties
19,23c19,24
<     NVMCTRL_STATUS = 0x02
<     NVMCTRL_INTCTRL = 0x03
<     NVMCTRL_INTFLAGS = 0x04
<     NVMCTRL_DATA = 0x06 # 16-bit
<     NVMCTRL_ADDR = 0x08 # 24-bit
---
>     NVMCTRL_CTRLC = 0x02
>     NVMCTRL_INTCTRL = 0x04
>     NVMCTRL_INTFLAGS = 0x05
>     NVMCTRL_STATUS = 0x06
>     NVMCTRL_DATA = 0x08 # 16-bit
>     NVMCTRL_ADDR = 0x0C # 24-bit
37c38
<     STATUS_WRITE_ERROR_bm = 0x30
---
>     STATUS_WRITE_ERROR_bm = 0x70
39,40c40,41
<     STATUS_EEPROM_BUSY_bp = 1
<     STATUS_FLASH_BUSY_bp = 0
---
>     STATUS_EEPROM_BUSY_bp = 0
>     STATUS_FLASH_BUSY_bp = 1
98a100
>
120a123
>
182d184
<
185a188
>
188a192
>

Based on this I implemented NVM v4 as NVM v2 and changed register/bit flags to reflect the above.

Please note: this has not been tested yet, as I don't have any DU chips to test it against.

I will now file the official PR for all three issues (#1621, #1622 and #1623) since they all seem resolved now. That being said, I'm still trying to come up with update to testing procedure that would enable invalid status register offset detection...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants