Bootrow r/w does not work for -c serialupdi

[stefanrueger]

Accessing bootrow does not seem to work for -c serialupdi:

$ avrdude -qc serialupdi -p 16eb28 -V -U bootrow:w:'"this is a test"':m && echo OK
avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e943f (probably 16eb28)

avrdude: processing -U bootrow:w:"this is a test":m
avrdude: reading input file "this is a test" for bootrow
         with 15 bytes in 1 section within [0, 14]
         using 1 page and 49 pad bytes
avrdude: writing 15 bytes bootrow ...
avrdude error: invalid memory: <bootrow:64>, 0x000000, 64 (0x0040)
avrdude: 15 bytes of bootrow written

avrdude done.  Thank you.

OK

Serialupdi self-diagnoses an invalid memory for paged write.

It is also remarkable that AVRDUDE thinks the operation went OK. The reason for this is that avr.c uses an ISP-like programming mechanism (load bytes to page buffer then issue an ISP page write command) when the dedicated paged_write() fails

avrdude/src/avr.c

Lines 1147 to 1149 in 41781bd

	if (rc < 0)
	/* paged write failed, fall back to byte-at-a-time write below */
	failure = 1;

As a consequence avr.c deploys the programmer's serialupdi_write_byte() function, indirectly called here

avrdude/src/avr.c

Line 1205 in 41781bd

if(avr_write_byte(pgm, p, m, i, data)) {

which in turn seems to think that the serialupdi byte write to bootrow was successful.

So paged write to bootrow returns a failure; the fallback bytewise write returns OK, but it doesn't look like it was successful:

$ avrdude -qc serialupdi -p 16eb28 -U bootrow:w:'"this is a test"':m && echo OK
avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e943f (probably 16eb28)

avrdude: processing -U bootrow:w:"this is a test":m
avrdude: reading input file "this is a test" for bootrow
         with 15 bytes in 1 section within [0, 14]
         using 1 page and 49 pad bytes
avrdude: writing 15 bytes bootrow ...
avrdude error: invalid memory: <bootrow:64>, 0x000000, 64 (0x0040)
avrdude: 15 bytes of bootrow written
avrdude: verifying bootrow memory against "this is a test"
avrdude warning: verification mismatch
        device 0xff != input 0x74 at addr 0x0000 (error)
avrdude error: verification mismatch

avrdude done.  Thank you.

@dbuchwald I was wondering whether you could solve that riddle? Trace output 16eb28.txt

It's the AVR-DU and AVR-EB parts that have a bootrow memory. I also tried the 64du28, which showed the same behaviour.

[dbuchwald]

Sorry for the delay in reply, I hope to find some time tomorrow to look into it. I took a quick look at the code now and the message you get in the trace:

avrdude serialupdi_paged_write() [serialupdi.c:873] error: invalid memory: <bootrow:64>, 0x000000, 64 (0x0040)

means basically that the bootrow memory is not recognized by serialupdi.c at all. Like not supported, not available. Like an ex parrot, it's not there at all.
If you don't feel like waiting (again, apologies for the delay), you can change the following line in serialupdi_paged_write function in serialupdi.c:

      } else if (mem_is_flash(m)) {

to:

      } else if (mem_is_flash(m) || mem_is_bootrow(m)) {

build and try again. This change is based on the following note from preliminary datasheet which suggests to treat bootrow memory as a regular flash:

https://onlinedocs.microchip.com/oxy/GUID-3E14380F-9711-4707-8991-A4DB432DDBE5-en-US-6/GUID-B0DBDEB5-FEE8-485E-BACC-628BB39DBD4C.html

EDIT: Actually nevermind, please pick up version from the following PR:

#1834

It does make the bootrow update work. There is one strange thing though, and I need to look into it further. Basically - the operations works, but just once. If you want to perform another bootrow update, it will fail, and the data written to (and read back from) bootrow will be invalid. There is a way to work around it, by simply adding -e flag to force flash erase before performing write, but still, I just don't like it.

That being said, I will look into it, but I can't promise it can even be solved. I have seen similar issues in the past - correct data is sent to the device, but strange data is read back from it.

What I did find in the documentation is that when flash memory is to be updated, chip erase is performed by default. Maybe the same approach should be used when writing to bootrow?

[stefanrueger]

@dbuchwald You have nailed it. Thank you.

With your fix now all memories of the new AVR-EB can be read/written including bootrow. Brilliant. Please create a PR in your own time (no rush!) so you get the credit for adapting serialupdi to new chips.

Create random multi-memory file for AVR16EB28

$ avrdude -c dryrun -p 16eb28 -xrandom -xseed=7 -U all:r:16eb28-dry-backup.hex

avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e943f (probably 16eb28)
         reading multiple memories ...
         eeprom          | ################################################## | 100% 0.00 s 
         flash           | ################################################## | 100% 0.00 s 
         fuses           | ################################################## | 100% 0.00 s 
         lock            | ################################################## | 100% 0.00 s 
         prodsig/sigrow  | ################################################## | 100% 0.00 s 
         bootrow         | ################################################## | 100% 0.00 s 
         userrow/usersig | ################################################## | 100% 0.00 s 
         sib             | ################################################## | 100% 0.00 s 
         writing 17204 bytes to output file 16eb28-dry-backup.hex

avrdude done.  Thank you.

Write that file to, and verify it against, a real AVR16EB28 using serialupdi (incl bootrow)

$ avrdude -c serialupdi -p 16eb28 -U all:w:16eb28-dry-backup.hex

avrdude: AVR device initialized and ready to accept instructions
avrdude: device signature = 0x1e943f (probably 16eb28)
avrdude: Note: carrying out an erase cycle as flash memory needs programming (-U all:w:...)
         specify the -D option to disable this feature
avrdude: erasing chip
avrdude: reading 17204 bytes for multiple memories from input file 16eb28-dry-backup.hex
avrdude: 512 bytes eeprom in 1 section [0, 0x1ff]: 64 pages and 0 pad bytes
         writing 512 bytes to eeprom ...
         writing | ################################################## | 100% 4.65 s 
         reading | ################################################## | 100% 0.74 s 
         512 bytes of eeprom verified
avrdude: 16384 bytes flash in 1 section [0, 0x3fff]: 256 pages and 0 pad bytes
         writing 16384 bytes to flash ...
         writing | ################################################## | 100% 10.88 s 
         reading | ################################################## | 100% 3.85 s 
         16384 bytes of flash verified
avrdude: 16 bytes fuses in 1 section [0, 15]
         writing 16 bytes to fuses ..., 16 bytes written, 16 verified
avrdude: 4 bytes lock in 1 section [0, 3]
         writing 4 bytes to lock ..., 4 bytes written, 4 verified
avrdude: 64 bytes bootrow in 1 section [0, 0x3f]: 1 page and 0 pad bytes
         writing 64 bytes to bootrow ...
         writing | ################################################## | 100% 0.04 s 
         reading | ################################################## | 100% 0.01 s 
         64 bytes of bootrow verified
avrdude: 64 bytes userrow/usersig in 1 section [0, 0x3f]: 1 page and 0 pad bytes
         writing 64 bytes to userrow/usersig ...
         writing | ################################################## | 100% 0.26 s 
         reading | ################################################## | 100% 0.02 s 
         64 bytes of userrow/usersig verified

avrdude done.  Thank you.

[dbuchwald]

Please note: double write to bootrow fails, I need to look into it some more. Will let you know soon.

[stefanrueger]

Temrinal works, I guess, b/c the code checks whether a failed write is consistent with NOR-memory writes (cannot set 1's) so the terminal utilises page erase

[dbuchwald]

OK, I will probably need some guidance here. There are basically two ways to go about it - solve the problem on memory definition level or on NVM programmer level, and here is the description/comparison of the approaches:

Memory definition level
Currently, AVRDUDE checks if any of the memories being updated is in the Flash range of the device and if it is, chip erase operation will be performed before any of the writes:
https://github.com/avrdudes/avrdude/blob/main/src/main.c#L1692-L1695

        if(upd->memstr && upd->op == DEVICE_WRITE && memlist_contains_flash(upd->memstr, p)) {
          erase = 1;
          pmsg_info("Note: carrying out an erase cycle as flash memory needs programming (-U %s:w:...)\n", upd->memstr);
          imsg_info("specify the -D option to disable this feature\n");

It's controlled by the memlist_contains_flash function:
https://github.com/avrdudes/avrdude/blob/main/src/update.c#L367-L377
This, in turn, uses the mem_is_in_flash function, which reads memory definitions:
https://github.com/avrdudes/avrdude/blob/main/src/avr.c#L1567

  {"bootrow",     MEM_BOOTROW | MEM_USER_TYPE},

So, changing bootrow memory definition to be also MEM_IN_FLASH, we will force AVRDUDE to erase all flash (including bootrow) prior to any write operation. This will, however, have a side effect of erasing the whole flash when writing to bootrow only, just like writing to any of the MEM_IN_FLASH memories - write to one, erase all of them. Currently writing to flash erases bootrow and that seems to be OK.

Advantages:

• consistent with existing flash memory handling approach

Disadvantages:

• might be confusing to users
• might fail in future if somebody at Microchip decides to produce devices with bootrow which is not part of the flash memory.

NVM Programmer Level
Currently write to bootrow is implemented as a flash write operation (with the latest PR that is). It uses simple NVM write operation which is available in each NVM revision.
That being said, I could change the code so that on compatible devices (NVM v0/v3/v5) there would be separate operation to write to bootrow that would, instead of using the page write operation, it would use the page erase and write operation (FLPERW), as described in AVR16EB14 datasheet:

For page programming, filling the page buffer and writing the page buffer into Flash, User Row, and EEPROM are two separate operations. Before programming a Flash page with the data in the page buffer, the content of the Flash page must be erased (read back 0xFF). Programming an unerased Flash page will corrupt its content. Two options are available to make sure that the Flash page content is programmed correctly: Option 1: The Flash is programmed using one command that handles both erase and write. 1. Make sure the Flash is ready by reading the Flash Busy (FLBUSY) flag in the NVMCTRL.STATUS register. 2. Fill the page buffer. 3. Write the page buffer to Flash with the Flash Page Erase and Page Write (FLPERW) command.

I tested it and it works nicely on my NVM v5 16eb28 chip. The problem is that this approach doesn't address any of NVM v2/v4 devices (and quite honestly I don't know whether I should bother - are there any NVM v2/v4 devices with bootrow?), and is quite specific. This might be an issue down the line with new NVM revisions.

Advantages:

• it would enable bootrow write operation without affecting remaining flash memory
• uses dedicated NVM feature on compatible devices

Disadvantages:

• will complicate codebase significantly (many changes will be required)
• will not provide support for NVM v2/v4 devices - actually not true anymore, I added support using explicit page erase command being executed before page write operation on bootrow
• it will require a lot of testing, since it's NVM version dependent; I don't own any v4 devices

Sorry, I just don't want to make this kind of decision on my own, it seems to go way beyond technical implementation of SerialUPDI programmer. Any feedback would be really appreciated.

[dbuchwald]

Since I wanted to take advantage of some spare time, I created PR for the second approach - implementing separate methods for bootrow handling that will ensure memory erasure prior to write. It either happens using explicit page erase prior to page write or using page erase/page write command on compatible NVM controllers. It would be awesome, though, if it could be tested against any of the v4 devices - I don't have one to check it...

You can now choose which PR you would like to merge @stefanrueger :)

[stefanrueger]

Thanks, @dbuchwald for in-depth analysis.

There may be a third way to go about this:

diff --git a/src/main.c b/src/main.c
index 3e15575f..050ac815 100644
--- a/src/main.c
+++ b/src/main.c
@@ -1681,8 +1681,8 @@ skipopen:
   }
 
   if (uflags & UF_AUTO_ERASE) {
-    if ((p->prog_modes & PM_PDI) && pgm->page_erase && lsize(updates) > 0) {
-      pmsg_info("Note: programmer supports page erase for Xmega devices.\n");
+    if ((p->prog_modes & (PM_PDI | PM_UPDI)) && pgm->page_erase && lsize(updates) > 0) {
+      pmsg_info("Note: programmer supports page erase for Xmega/AVR8X devices.\n");
       imsg_info("Each page will be erased before programming it, but no chip erase is performed.\n");
       imsg_info("To disable page erases, specify the -D option; for a chip-erase, use the -e option.\n");
     } else {

This treats UPDI parts like PDI if the programmer implements page_erase() functionality: Any -U write to memory that makes use of paged_write() will, by default, call page_erase() before calling paged_write(). Users can switch this behaviour off (-D) and/or issue a chip_erase() before programming. This might work in connection with UPDI generally. Needs some testing, but as this is the way XMEGAs go about it it should work for UPDI parts if the programmer implements page erase well. The ramification of above code change should be that avr_write_mem() will normally be called with auto_erase == 1.

Traditionally, AVRDUDE tended to separate page erase from paged write. Though, in my book, it is totally fine if a programmer always erases a page before writing it: there is no universal law that states flash memory must look like NOR-memory (meaning bit cannot be set to 1 while programming).

BTW mem_is_in_flash(m) means that m is a sub-memory of the actual flash memory (including flash itself). This is an assertion on the address range. So, in the current AVRDUDE implementation it would be wrong to subsume bootrow into flash. One could have a new memory feature such as NEEDS_ERASE_BEFORE_WRITE or LOOKS_LIKE_NOR_MEMORY or similar, which is then utilised.

I have to dash, but will have a look at the alternative implementation (thank you @dbuchwald), but this might take a few days.

@dbuchwald Could you check how above third alternative would work with the serialupdi implememtation? We'd need to also see how this might influence other UPDI programmers.