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

Multiple updates to SerialUPDI protocol based on discussion 1609 and issues 1621, 1622 and 1623. #1627

Merged
merged 8 commits into from
Jan 28, 2024

Conversation

dbuchwald
Copy link
Contributor

This PR should address the following issues:

The following changes have been made:

Please note: NVM V4 support is highly experimental, implementation was based on pymcuprog but never tested on actual hardware.

@askn37 @MCUdude @stefanrueger - would you be able to check this PR and let me know if you have any questions, comments or concerns? Thanks in advance!

@dbuchwald dbuchwald changed the title Issue 1609 Multiple updates to SerialUPDI protocol based on discussion 1609 and issues 1621, 1622 and 1623. Jan 20, 2024
@askn37
Copy link
Contributor

askn37 commented Jan 21, 2024

Retest using this PR.

run test NVM V0 : PASS

Darwin alicia.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:34 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T8103 arm64

Prepare "-c serialupdi -xrtsdtr=high -P /dev/cu.usbserial-230 -p m4808" and press 'enter' or 'space' to continue. Press any other key to skip
avrdude_bin=/Users/askn/Collaborator/avrdude_pr1627/build_darwin/src/avrdude
avrdude: Version 7.2-20240120 (1b0b263a)
✅ eesave fuse bit set, cleared and verified
✅ the_quick_brown_fox_49152B.hex flash -U write/verify
✅ the_quick_brown_fox_256B.hex eeprom -U write/verify
✅ lorem_ipsum_49152B.srec flash -U write/verify
✅ lorem_ipsum_256B.srec eeprom -U write/verify
✅ 0xff_256B.hex eeprom -U write/verify
✅ the_quick_brown_fox_49152B.hex:a flash -T write/verify
✅ the_quick_brown_fox_256B.hex:a eeprom -T write/verify
✅ lorem_ipsum_49152B.srec flash -T write/verify
✅ lorem_ipsum_256B.srec eeprom -T write/verify
✅ cola-vending-machine.raw flash -T/-U write/verify
✅ holes_pack_my_box_49152B.hex:a flash -U write
✅ holes_the_five_boxing_wizards_49152B.hex flash -T write
✅ holes_pack_my_box_256B.hex:a eeprom -U write
✅ holes_the_five_boxing_wizards_256B.hex eeprom -T write
✅ random_data_64B.bin usersig -T/-U write/read

run test NVM V2 : PASS

Prepare "-c serialupdi -xrtsdtr=high -P /dev/cu.usbserial-230 -p avr128db32" and press 'enter' or 'space' to continue. Press any other key to skip
avrdude_bin=/Users/askn/Collaborator/avrdude_pr1627/build_darwin/src/avrdude
avrdude: Version 7.2-20240120 (1b0b263a)
✅ eesave fuse bit set, cleared and verified
✅ the_quick_brown_fox_131072B.hex flash -U write/verify
✅ the_quick_brown_fox_512B.hex eeprom -U write/verify
✅ lorem_ipsum_131072B.srec flash -U write/verify
✅ lorem_ipsum_512B.srec eeprom -U write/verify
✅ 0xff_512B.hex eeprom -U write/verify
✅ the_quick_brown_fox_131072B.hex:a flash -T write/verify
✅ the_quick_brown_fox_512B.hex:a eeprom -T write/verify
✅ lorem_ipsum_131072B.srec flash -T write/verify
✅ lorem_ipsum_512B.srec eeprom -T write/verify
✅ cola-vending-machine.raw flash -T/-U write/verify
✅ holes_pack_my_box_131072B.hex:a flash -U write
✅ holes_the_five_boxing_wizards_131072B.hex flash -T write
✅ holes_pack_my_box_512B.hex:a eeprom -U write
✅ holes_the_five_boxing_wizards_512B.hex eeprom -T write
✅ random_data_32B.bin usersig -T/-U write/read

run test NVM V3 : PASS

Prepare "-c serialupdi -xrtsdtr=high -P /dev/cu.usbserial-230 -p avr64ea32" and press 'enter' or 'space' to continue. Press any other key to skip
avrdude_bin=/Users/askn/Collaborator/avrdude_pr1627/build_darwin/src/avrdude
avrdude: Version 7.2-20240120 (1b0b263a)
✅ eesave fuse bit set, cleared and verified
✅ the_quick_brown_fox_65536B.hex flash -U write/verify
✅ the_quick_brown_fox_512B.hex eeprom -U write/verify
✅ lorem_ipsum_65536B.srec flash -U write/verify
✅ lorem_ipsum_512B.srec eeprom -U write/verify
✅ 0xff_512B.hex eeprom -U write/verify
✅ the_quick_brown_fox_65536B.hex:a flash -T write/verify
✅ the_quick_brown_fox_512B.hex:a eeprom -T write/verify
✅ lorem_ipsum_65536B.srec flash -T write/verify
✅ lorem_ipsum_512B.srec eeprom -T write/verify
✅ cola-vending-machine.raw flash -T/-U write/verify
✅ holes_pack_my_box_65536B.hex:a flash -U write
✅ holes_the_five_boxing_wizards_65536B.hex flash -T write
✅ holes_pack_my_box_512B.hex:a eeprom -U write
✅ holes_the_five_boxing_wizards_512B.hex eeprom -T write
✅ random_data_64B.bin usersig -T/-U write/read

no test NVM V4 : Haven't gotten it yet

run test NVM V5 : PASS

Prepare "-c serialupdi -xrtsdtr=high -P /dev/cu.usbserial-230 -p avr16eb32" and press 'enter' or 'space' to continue. Press any other key to skip
avrdude_bin=/Users/askn/Collaborator/avrdude_pr1627/build_darwin/src/avrdude
avrdude: Version 7.2-20240120 (1b0b263a)
✅ eesave fuse bit set, cleared and verified
✅ the_quick_brown_fox_16384B.hex flash -U write/verify
✅ the_quick_brown_fox_512B.hex eeprom -U write/verify
✅ lorem_ipsum_16384B.srec flash -U write/verify
✅ lorem_ipsum_512B.srec eeprom -U write/verify
✅ 0xff_512B.hex eeprom -U write/verify
✅ the_quick_brown_fox_16384B.hex:a flash -T write/verify
✅ the_quick_brown_fox_512B.hex:a eeprom -T write/verify
✅ lorem_ipsum_16384B.srec flash -T write/verify
✅ lorem_ipsum_512B.srec eeprom -T write/verify
✅ cola-vending-machine.raw flash -T/-U write/verify
✅ holes_pack_my_box_16384B.hex:a flash -U write
✅ holes_the_five_boxing_wizards_16384B.hex flash -T write
✅ holes_pack_my_box_512B.hex:a eeprom -U write
✅ holes_the_five_boxing_wizards_512B.hex eeprom -T write
✅ random_data_64B.bin usersig -T/-U write/read

That's a very good result for me.

@mcuee mcuee added the bug Something isn't working label Jan 21, 2024
@stefanrueger
Copy link
Collaborator

I have no concerns re the code, but would want to see confirmation that the V4 implementation works as well, so suggest merging after that could be tested. Thanks all round, in particular @askn37 and @dbuchwald, for error reporting, fruitful discussion, careful implementation and thorough testing (with @MCUdude's brand new test tool!)

@MCUdude
Copy link
Collaborator

MCUdude commented Jan 21, 2024

Thanks for the PR @dbuchwald! Tested with an ATmega4808, AVR128DA32, and AVR64DB32, and no issues so far. I'm waiting for an AVR-DU to show up in the mail, but it might take another week before it gets here.

@dbuchwald
Copy link
Contributor Author

@stefanrueger I totally agree that it should be properly tested against NVM v4 device, especially given how the implementation was done (just copied over from pymcuprog, I didn't even have single DU device datasheet to check the EEPROM/FLASH flags reverse).

@MCUdude I'm looking forward to your test results. How did you get DU chip? Do you have any datasheet for it?

@MCUdude
Copy link
Collaborator

MCUdude commented Jan 22, 2024

Do you have any datasheet for it?

Well, I happened to be in contact with someone who thinks Avrdude is a project that's worth supporting. Sorry, I don't have any datasheet for it.

@stefanrueger
Copy link
Collaborator

data sheet for [DU parts]

The atpacks have .atdf files for them from which the avrdude project has extracted the register files (but not the associated bit masks) and put into avrintel.c. So, that is our only knowlede about the parts so far.

@askn37
Copy link
Contributor

askn37 commented Jan 22, 2024

For AVR-DU, the currently available documentation is ATDF, ioavr64du28.h, ioavr64du32.h header files and some IDE configuration definition files. Then, an archive of previously retracted short notes.
https://askn37.github.io/documents/AVR-DU-Product-Brief-DS40002328.pdf

You can tell a lot from header files. This is a good indication of the nature of the hardware. It may be helpful to compare it with header files from other similar chips to see what's similar and what's different.

By convention, when the meaning or use of a register changes, the symbol name changes, even if the IO address or bit position remains the same. This causes the previous code to generate an intentional compilation error to get attention. This trend is most noticeable in hardware peripherals that change frequently, such as CCL, EVSYS, and PORTMUX.

From this comparison, we can infer that NVMCTRL's IO register map is very similar to V3, and the command set and FLASH page size are the same as V2. That's how I interpreted it. I think it's a combination of the Dx specification NOR FLASH metal mask and the Ex writing circuit.


Current AVR-DU (NVM V4) support options.

  1. Don't commit your code to a release. No one is free to run it unless you build it every night or grab the code from a branch and recreate it as needed.

  2. Make relevant code publicly executable. Please clearly indicate that it is experimental or provisional. Anyone has the freedom to try it out and complain if it doesn't work.

@dbuchwald
Copy link
Contributor Author

@askn37 @stefanrueger @MCUdude

Hey, just a short follow-up on the NVM v4 and the reversed bits for NVMCTRL.STATUS register. Following your advice I checked the recently updated atpack from Microchip, and indeed, when comparing v2 and v4 chips (ioavr64dd28.h vs ioavrdu32.h) I can see that the flags are indeed reversed.

ioavr64dd28.h lines 4561-4565:

/* NVMCTRL.STATUS  bit masks and bit positions */
#define NVMCTRL_FBUSY_bm  0x01  /* Flash busy bit mask. */
#define NVMCTRL_FBUSY_bp  0  /* Flash busy bit position. */
#define NVMCTRL_EEBUSY_bm  0x02  /* EEPROM busy bit mask. */
#define NVMCTRL_EEBUSY_bp  1  /* EEPROM busy bit position. */

ioavr64du32.h lines 4210-4214:

/* NVMCTRL.STATUS  bit masks and bit positions */
#define NVMCTRL_EEBUSY_bm  0x01  /* EEPROM busy bit mask. */
#define NVMCTRL_EEBUSY_bp  0  /* EEPROM busy bit position. */
#define NVMCTRL_FLBUSY_bm  0x02  /* Flash busy bit mask. */
#define NVMCTRL_FLBUSY_bp  1  /* Flash busy bit position. */

This suggests that my implementation should be valid, but this can only be confirmed when testing against physical hardware.

Now, for the release forward: I wouldn't remove NVM v4 support. Please remember that when the original SerialUPDI implementation was released back in 2021 it wasn't tested against any NVM v3 chips, as they were not available at the time, so it's pretty much the same scenario...

@stefanrueger
Copy link
Collaborator

@dbuchwald Yes, agreed. I would rather have a candidate implementation for V4 in v7.3 than nothing. Having it out there gets more people trying it. As a physical test might happen soon it makes sense to wait. But if that does not materialise, I would suggest merging the PR anyway.

@stefanrueger stefanrueger merged commit d4ca977 into avrdudes:main Jan 28, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment