avrdudes / avrdude

AVRDUDE is a utility to program AVR microcontrollers
GNU General Public License v2.0
696 stars 136 forks source link

ATtiny3216/3217 memory size and page size of userrow/usersig is wrong in avrdude.conf #1183

Closed neutronst4r closed 1 year ago

neutronst4r commented 1 year ago

Hello everyone, in the avrdude.conf (release v7.0) the memory size and page size usersig/userrow is wrong, it should be 64B and not 32B.

The relevant parts in the datasheet (page 17) is this:

In addition to the EEPROM, the ATtiny3216/3217 has one extra page of EEPROM memory that can be used for firmware settings; the User Row (USERROW). This memory supports single-byte read and write as the normal EEPROM. The CPU can write and read this memory as normal EEPROM, and the UPDI can write and read it as a normal EEPROM memory if the part is unlocked. The User Row can be written by the UPDI when the part is locked. USERROW is not affected by a chip erase.

The page size of EEPROM is 64B.

Also the official toolchain resource pack from Atmel creates the memory as a 64B structure:

/* User Row */
typedef struct USERROW_struct
{  
    register8_t USERROW0;  /* User Row Byte 0 */
    [...]                                                                                                                                                                                                        
    register8_t USERROW63;  /* User Row Byte 63 */
} USERROW_t;

files: iotn3216.h and iotn3217.h

I think the error sneaked in because the 3216/3217 are not actually tinys, but megas in disguise. To fix this, one could overwrite the settings directly at the config for the 3216/3217 or one could change the parent family of the chips to the mega variant, which has the correct values, but I don't know enough about avrdude to know, whether the later would break anything else.

mcuee commented 1 year ago

This may not be valid now in git main. https://github.com/avrdudes/avrdude/blob/main/src/avrdude.conf.in

#------------------------------------------------------------
# ATtiny3216
#------------------------------------------------------------

part parent ".avr8x_tiny"
    desc                   = "ATtiny3216";
    id                     = "t3216";
    mcuid                  = 314;
    n_interrupts           = 31;
    signature              = 0x1e 0x95 0x21;

    memory "eeprom"
        size               = 256;
        page_size          = 64;
        offset             = 0x1400;
        readsize           = 256;
    ;

    memory "flash"
        size               = 0x8000;
        page_size          = 128;
        offset             = 0x8000;
        readsize           = 256;
    ;
;

#------------------------------------------------------------
# ATtiny3217
#------------------------------------------------------------

part parent ".avr8x_tiny"
    desc                   = "ATtiny3217";
    id                     = "t3217";
    mcuid                  = 315;
    n_interrupts           = 31;
    signature              = 0x1e 0x95 0x22;

    memory "eeprom"
        size               = 256;
        page_size          = 64;
        offset             = 0x1400;
        readsize           = 256;
    ;

    memory "flash"
        size               = 0x8000;
        page_size          = 128;
        offset             = 0x8000;
        readsize           = 256;
    ;
;
neutronst4r commented 1 year ago

I am not sure what you mean by invalid or if you are agreeing with me or not. So just for clarification: the part, that is wrong is defined in the parent of those two chips (see below), It is wrong in the v7.0 realease and is also wrong in the master branch you linked. I am not sure, if it is wrong for the whole family or just for the two variants 3216 and 3217, but these two definitely have userrow size of 64B and a page size of 64B.

#------------------------------------------------------------
# AVR8X tiny family common values
#------------------------------------------------------------

part parent ".avr8x"
    desc                   = "AVR8X tiny family common values";
    id                     = ".avr8x_tiny";
    family_id              = "tinyAVR";
    # Shared UPDI pin, HV on UPDI pin
    hvupdi_variant         = 0;

    memory "userrow"
        size               = 32;  => should be 64 for 3216/3217
        page_size          = 32; => should be 64 for 3216/3217
        offset             = 0x1300;
        readsize           = 256;
    ;

    memory "usersig"
        alias "userrow";
    ;
;
mcuee commented 1 year ago

I was saying this has been fixed in git main and will be incorporated in the next release.

But maybe I am wrong. So I will remove the invalid flag.

stefanrueger commented 1 year ago

@neutronst4r Thanks for reporting. You are right: git main has this wrong

$ avrdude -p*321[67]*/At | grep userrow.size
.ptmm   ATtiny3216  userrow size    32
.ptmm   ATtiny3217  userrow size    32

You are also right to query whether this might affect other parts. Often, when there is an error in a place, there are similar errors around. Here a list of UPDI ATtiny parts where the eeprom page size differs from the userrow memory size:

for p in $(avrdude -pattiny*/d | grep UPDI | cut -d"'" -f2); do
   echo $p $(avrdude -p$p/At | grep userrow.size | cut -f3-)  $(avrdude -p$p/At | grep eeprom.page_size | cut -f3-);
done | \
awk '{ if($4 != $7) print;}'

ATtiny3216 userrow size 32 eeprom page_size 64
ATtiny3217 userrow size 32 eeprom page_size 64
ATtiny3224 userrow size 32 eeprom page_size 64
ATtiny3226 userrow size 32 eeprom page_size 64
ATtiny3227 userrow size 32 eeprom page_size 64

So we have as further candidates ATtiny3224, ATtiny3226 and ATtiny3227 to look at. Do you have the data sheets, @neutronst4r?

Thanks again for raising.

SpenceKonde commented 1 year ago

Datasheet and headers make clear that 322x has 32b userrow.

#  define USER_SIGNATURES_SIZE      (32)

Datasheet is not clear for 3216/3217, but the header is clear as crystal:

#  define USER_SIGNATURES_SIZE      (64)
neutronst4r commented 1 year ago

Datasheet and headers make clear that 322x has 32b userrow.

Where did you read that?

The datasheet of 3224/3226/3227 (page 40) have similar text as the 3216/3217 (page 17):

The ATtiny3224/3226/3227 has one extra page of EEPROM memory that can be used for firmware settings, the User Row (USERROW).

In both cases it is just one extra page of eeprom memory with special properties. In all cases the page size of eeprom of these chips is 64B.

SpenceKonde commented 1 year ago

Page 2 contains a chart, Table 1, titled "Memory Overview". It shows that all 2-series tinyAVR parts have a 32-byte USERROW.

This is confirmed by the headers which also specify 32 bytes.

The Dx-series is also 32b across the board (on all parts announced or released, at least - DA, DB, and DD. EA will have 64b userrow on all parts regardless of flash size). The product brief for the native-USB DU-series (it was available very uh, briefly before vanishing, a couple of hours in early 2022) indicated that those parts would have a mindblowing 512b USERROW. Of course, we don't know if the product was canceled or they just didn't want to hurt sales of the aging 32u4 during the inevitable 2+ year wait or what.

The common thread appears to be that within each family, the userrow size is likely to be fixed going forward. The only cases where that is violated are the megaAVR 0-series, and tinyAVR 1-series, both of which were released at the dawn of the modern AVR era, when more things were in flux