avrdudes / avr-libc

The AVR-LibC package provides a subset of the standard C library for AVR 8-bit RISC microcontrollers.
https://avrdudes.github.io/avr-libc/
Other
217 stars 49 forks source link

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

Open avrs-admin opened 2 years ago

avrs-admin commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 1 month ago

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 

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 cb3fa932de0b7b13ffb41b13eef4efdfa7bdd089 17 years ago so I don't really understand why it is out of sync with the Microchip definitions.