[bug #47937] Incorrect "0" suffix in names of SPI registers and bits for ATmega324PA and ATmega164PA

[avrs-admin]

Mon 16 May 2016 11:59:03 PM CEST

The attached patch fixes the names that avr-libc defines for several SPI-related registers in iom324pa.h and iom164pa.h.  The datasheets call the registers SPCR, SPSR, and SPDR, but avr-libcs's definitions had "0" at the end of the name: SPCR0, SPSR0, and SPDR0.  The names of bits defined in these registers were wrong in the same way.  This patch adds new definitions with the correct names, while leaving the old definitions around for backwards compatibility.

Please apply the patch.  Thanks!

file #37181: spi_fix_m164pa_m324pa.patch

This issue was migrated from https://savannah.nongnu.org/bugs/?47937

[avrs-admin]

Joerg Wunsch <joerg_wunsch>
Tue 17 May 2016 08:35:16 AM CEST

Actuall, the datasheet is inconsistent on this: in the register
summary at the end, the bits do have the 0 suffix, while the
description omits them.

Likewise, the Atmel Studio XML files (where our header files
have been derived from) also uses the 0 suffix.

So please file a bug report with Atmel so they get all their
documentation and description files consistent.

Due to the inconsistencies, I think the way for us to fix it
is by supplying both definitions.

[avrs-admin]

David Grayson
Wed 18 May 2016 03:37:13 AM CEST

Hello, Joerg.  I have seen your name a lot over the years during my AVR-related work, so it is nice to hear from you.

Yes, the datasheets look inconsistent, and the XML files in C:\Program Files (x86)\Atmel\Studio\7.0\packs\atmel\ATmega_DFP\1.0.98\atdf seem to be incorrect.  I plan to write to Atmel support.

As you said, we should supply both definitions.  That's what my patch does; it only adds definitions and does not remove any.

--David

[avrs-admin]

David Grayson
Wed 18 May 2016 04:05:00 AM CEST

While I was researching my bug report to Atmel, I noticed a new part definition file in Atmel Studio 7.0.934: the ATmega324PB.  This part will have two SPI modules, so it makes sense that all of its SPI registers and bits are suffixed with 0 and 1.  I suspect that Atmel is purposely renaming the SPI registers on the old parts for consistency's sake.  They renamed the SPI registers for the ATmega164{A,P,PA} and ATmega324{A,P,PA}, but they have not yet done it for the ATmega644* or ATmega1284*.

I will still contact them anyway, because I noticed an inconsistency: in ATmega164A.atdf they defined a bit to be named SPR01 when they really meant SPR10.  That's definitely an error, but in general their adding of these prefixes might not be an error.

[avrs-admin]

Joerg Wunsch <joerg_wunsch>
Wed 18 May 2016 08:31:59 AM CEST

OK, great if your patch maintains both names.  I'll apply
it shortly then.

Quite possible Atmel tried to maintain some "forward
consistency" (they did it in other situations that way in
the past, e.g. USART bit names), but the datasheet and XML
description ought to match, no matter what.

[avrs-admin]

David Grayson
Mon 23 May 2016 11:30:48 PM CEST

An Atmel tech support representative has told me that he will file a bug report, but did not give any other useful information.

[avrs-admin]

David Grayson
Tue 11 Oct 2016 12:12:58 AM CEST

Hey, it looks like my patch has not been applied to avr-libc.  This is just a reminder to do that.

[avrs-admin]

David Grayson
Mon 28 Aug 2017 10:22:07 PM CEST

Atmel has updated the ATmega324PA datasheet.  It now uses names like "SPCR0" in the tables that define the registers, but they still have plenty of example code and wording in paragraphs that refer to "SPCR".

The ATmega164PA datasheet still does not use the name "SPCR0", so users of that chip will have trouble due to the missing register names in avr-libc.

I submitted this patch 15 months ago and I thought it would be pretty straightforward.  Are there any issues with it?

[bulletmark]

I am not experienced with AVR but I was helping my daughter with some UNI provided atmega324a code copied from her Microchip Studio installation on her Windows laptop to my Linux laptop (where I am using avr-gcc + avrdude) and it would not compile due to problems closely related to this issue. I had to add the following definitions which were missing from /usr/avr/include/avr/iomxx4.h:

#define SPCR0   SPCR                                                                                                            
#define SPIE0   SPIE                                                                                                            
#define SPE0    SPE                                                                                                             
#define DORD0   DORD                                                                                                            
#define MSTR0   MSTR                                                                                                            
#define CPOL0   CPOL                                                                                                            
#define CPHA0   CPHA                                                                                                            
#define SPR01   SPR1                                                                                                            
#define SPR10   SPR1                                                                                                            
#define SPR00   SPR0                                                                                                            
#define SPSR0   SPSR 
#define SPI2X0  SPI2X
#define SPDR0   SPDR
#define SPIF0   SPIF

This is kind of the opposite to this issue as in this case the '0' suffix was missing for atmega324a. I see that this was changed the other way in commit cb3fa93 17 years ago so I don't really understand why it is out of sync with the Microchip definitions.