Houston, SPI has a problem with avr-gcc 11.1.0

Hello,

I used the Arduino IDE 1.8.15 with megaavr package 1.8.7.
Now I found an error in interaction of Arduino IDE with Nano Every Board and avr-gcc 11.1.0. SPI does not work. The registers are configured wrong.
With original Arduino IDE avr-gcc Toolchain 7.3.0 and 10.3.0 SPI works.
Also the configuration from Arduino IDE with avr-gcc 11.1.0 and MCUdude MegaCoreX Package package works.
Which tells me that it can't be the avr-gcc 11.1.0 toolchain alone.
There must be a problem between SPI Lib and avr-gcc 11.1.0.
I don't get any error messages when compiling, related to SPI.

By the way SPI with avr-gcc 11.1.0 works with the Arduino Mega2560 without problems.

Except for those of the USART Lib.

C:\Arduino IDE Portable\megaAVR0\arduino-1.8.15\portable\packages\arduino\hardware\megaavr\1.8.7\cores\arduino/UART.h: In member function 'virtual void UartClass::begin(long unsigned int)':

C:\Arduino IDE Portable\megaAVR0\arduino-1.8.15\portable\packages\arduino\hardware\megaavr\1.8.7\cores\arduino/UART.h:98:49: error: bitwise operation between different enumeration types 'USART_CMODE_enum' and 'USART_CHSIZE_enum' is deprecated [-Werror=deprecated-enum-enum-conversion]

   98 | #define SERIAL_8N1 (USART_CMODE_ASYNCHRONOUS_gc | USART_CHSIZE_8BIT_gc | USART_PMODE_DISABLED_gc | USART_SBMODE_1BIT_gc)

      |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~

C:\Arduino IDE Portable\megaAVR0\arduino-1.8.15\portable\packages\arduino\hardware\megaavr\1.8.7\cores\arduino/UART.h:156:50: note: in expansion of macro 'SERIAL_8N1'

  156 |     void begin(unsigned long baud) { begin(baud, SERIAL_8N1); }

      |                                                  ^~~~~~~~~~

cc1plus.exe: all warnings being treated as errors

Changed to ... works for now.

//#define SERIAL_8N1 (USART_CMODE_ASYNCHRONOUS_gc | USART_CHSIZE_8BIT_gc | USART_PMODE_DISABLED_gc | USART_SBMODE_1BIT_gc)
constexpr uint8_t SERIAL_8N1 { (static_cast<uint8_t>(USART_CMODE_ASYNCHRONOUS_gc) | USART_CHSIZE_8BIT_gc | USART_PMODE_DISABLED_gc | USART_SBMODE_1BIT_gc) };

Now the question is, if there are no errors in the bit definitions, which registers does the compiled code describe? So it can't be the SPI registers? Something is going completely wrong without being seen. SPI0.CTRLA is not configured and CLRLB completely different.

In addition to the problem, I can not switch a pin. In that case any pin as Salve Select does not go High.

I compared the controller header files of the toolchains. Nothing has changed between 10.3 and 11.1.

What could be the problem?
What else can I do?

My test sketch.

#include<utilities.h>
#include <SPI.h>

void setup()
{
  Serial.begin(250000);
  Serial.println(F("\nµC Reset #### #### ####"));
  SPI.begin();
  showRegister();
  // avr-gcc 11.1.0: SPI0 CTRLA: 0b0000'0000, CTRLB: 0b0000'0001
  // avr-gcc 10.3.0: SPI0 CTRLA: 0b0010'0001, CTRLB: 0b0000'0100
  // avr-gcc  7.3.0: SPI0 CTRLA: 0b0010'0001, CTRLB: 0b0000'0100
}

void loop()
{ }

void showRegister (void)
{
  Serial.print(F("SPI0 CTRLA:    ")); formatBINln(Serial, SPI0.CTRLA, true);
  Serial.print(F("SPI0 CTRLB:    ")); formatBINln(Serial, SPI0.CTRLB, true);
  Serial.print(F("SPI0 INTCTRL:  ")); formatBINln(Serial, SPI0.INTCTRL, true);
  Serial.print(F("SPI0 INTFLAGS: ")); formatBINln(Serial, SPI0.INTFLAGS, true);     
  Serial.print(F("SPI0 DATA:     ")); formatBINln(Serial, SPI0.DATA, true); 
}

utilities.zip (15,9 KB)

If your hypothesis is that the problem is with the library code and MegaCoreX is working, then do a diff between the Arduino megaAVR Boards SPI library and the MegaCoreX SPI library.

You can also check the commit history for the library to see if there is relevant information there:
https://github.com/MCUdude/MegaCoreX/commits/master/megaavr/libraries/SPI

Hello,

the problem is somehow deeper and more hidden. On one side it is due to the new toolchain. On the other side again not. Then it works again with the MCUdude package. Somehow crazy the whole thing. The libs are built too differently. The comparison will take longer ...

In order to make all relevant information available to all who are interested in this subject, I'll share a link to the related discussion at:

Hello,

I am one step further. First I have read the register addresses. They are correct.
(I had at the beginning the quiet suspicion that in the new AVR backend an error could be).

Afterwards I built in own methods into the SPI Lib until I recognized the following effect. If you comment out the begin() method in SPI.cpp

config(DEFAULT_SPI_SETTINGS);

then the register values of CTRLA, CTRLB are correct and my display works. The config method writes the registers with wrong values.

What is going wrong here? :roll_eyes:

Assembler Dumps Files.
Sketch:

#include <SPI.h>

void setup()
{  
  SPI.begin();
}

void loop()
{ }

SPI Lib original with config Methode.
Arduino IDE with original avr-gcc 7.3.0
Arduino IDE with avr-gcc 11.1.0
Arduino IDE with avr-gcc 11.1.0 and MCUdude Package

DumpFiles.zip (37,9 KB)

Hi,

I tried to follow that. Difficult.

SPI.cpp:
config(DEFAULT_SPI_SETTINGS);

HardwareSPI.h:
const SPISettings DEFAULT_SPI_SETTINGS = SPISettings();

Where are the default values for the SPISettings constructor?
SPISettings is a separate class with 3 constructors.
SPISettings is inherited by SPISettingsMegaAVR which also has 3 constructors.

My feeling is that either the config overload is going wrong or the constructs overload.

Hi,

using pin toggle I know that
void init_MightInline(...) and void init_AlwaysInline(...) are called.
ctrla is so far always 0 and ctrlb always 1.
After explicit initialization to 0, both are 0 in the serial output.
After explicit initialization to 5, both are 5 in the serial output.
What can this mean?
The member initialization works, but are not used?

/* member variables containing the desired SPI settings */
  uint8_t ctrla {5};
  uint8_t ctrlb {5};
  friend class SPIClassMegaAVR;

The assignment to ctrla and ctrlb does not work.
ctrla should be at least 0x21 and ctrlb should be at least 0x04.
Why does this not work?

/* Pack into the SPISettings::ctrlb class */
    /* Set mode, disable master slave select, and disable buffering. */
    /* dataMode is register correct, when using SPI_MODE defines     */
    ctrlb = (dataMode)            |
            (SPI_SSD_bm)          |   //0x04  Slave Select Disable bit mask
            (0 << SPI_BUFWR_bp)   |   // 6    Buffer Write Mode bit position
            (0 << SPI_BUFEN_bp);      // 7    Buffer Mode Enable bit position
                
    /* Get Clock related values.*/
    uint8_t clockDiv_mult = (clockDiv & 0x1);
    uint8_t clockDiv_pres = (clockDiv >> 1);
        
    /* Pack into the SPISettings::ctrlb class     */
    /* Set Prescaler, x2, SPI to Master, and Bit Order. */
    
    ctrla = (clockDiv_pres << SPI_PRESC_gp)         |   //  1    Prescaler group position
            (clockDiv_mult << SPI_CLK2X_bp)         |   //  4    Enable Double Speed bit position
            (SPI_ENABLE_bm)                         |   // 0x01  Enable Module bit mask
            (SPI_MASTER_bm)                         |   // 0x20  Master Operation Enable bit mask
            ((bitOrder == LSBFIRST) << SPI_DORD_bp);    //  6    Data Order Setting bit position

When I change this to test, I still get the initialization value 5 output.
Actually something else than 5 should be output?

    /* Pack into the SPISettings::ctrlb class */
    /* Set mode, disable master slave select, and disable buffering. */
    /* dataMode is register correct, when using SPI_MODE defines     
    ctrlb = (dataMode)            |
            (SPI_SSD_bm)          |   //0x04  Slave Select Disable bit mask
            (0 << SPI_BUFWR_bp)   |   // 6    Buffer Write Mode bit position
            (0 << SPI_BUFEN_bp);  */    // 7    Buffer Mode Enable bit position
                
    /* Get Clock related values.*/
    uint8_t clockDiv_mult = (clockDiv & 0x1);
    uint8_t clockDiv_pres = (clockDiv >> 1);
        
    /* Pack into the SPISettings::ctrlb class     */
    /* Set Prescaler, x2, SPI to Master, and Bit Order.
    
    ctrla = (clockDiv_pres << SPI_PRESC_gp)         |   //  1    Prescaler group position
            (clockDiv_mult << SPI_CLK2X_bp)         |   //  4    Enable Double Speed bit position
            (SPI_ENABLE_bm)                         |   // 0x01  Enable Module bit mask
            (SPI_MASTER_bm)                         |   // 0x20  Master Operation Enable bit mask
            ((bitOrder == LSBFIRST) << SPI_DORD_bp); */   //  6    Data Order Setting bit position
            
    ctrla = dataMode | (clockDiv_pres << SPI_PRESC_gp);
    ctrlb = (clockDiv_mult << SPI_CLK2X_bp) | ((bitOrder == LSBFIRST) << SPI_DORD_bp);
    ctrla = 6;
    ctrlb = 6;
  }
  /* member variables containing the desired SPI settings */
  uint8_t ctrla {5};
  uint8_t ctrlb {5};
  friend class SPIClassMegaAVR;

Is it the avr-gcc 11.1.0 broken?
I have tried my own toolchain and the one from Zakkemble.

The serial output does not change when I change the compiler option from -std=gnu++20 to -std=gnu++17. It also does not change if I change -Os to O3, O2 or O1.

Does anyone still have idea to test?

Do you ever do an SPI.startTransaction() in the code that's not working?
I believe that that's when the SPI configuration is actually loaded into the peripheral registers.
(I had a vaguely-related issue I reported here: SPI default clock rate on RP2040 boards is slower than Arduino API Standard. · Issue #245 · arduino/ArduinoCore-mbed · GitHub )

It might be a separate bug with the new API if startTransaction is REQUIRED after begin; AFAIK, a lot of AVR code did the chip-select toggling manually without using the SPI library.

Hi,

my test code is the one from post #1 and #6 respectively.
I read the registers.
SPI.begin configures the SPI registers.
However, for some inexplicable reason, this does not work with avr-gcc 11.1.0.
If I remove the call config(DEFAULT_SPI_SETTINGS) from the SPI.cpp everything is ok again.
Without the config call my hardware works in real life.

You mean SPI.beginTransaction() must be called?
Only what has this to do with the avr-gcc version? This is really crazy. :thinking:

... waiting ...

I have tested it. There is still garbage in the registers.
For example SPI is never switched on with the enable bit.

#include<utilities.h>
#include <SPI.h>

void setup()
{  
  Serial.begin(250000);
  Serial.println(F("\nµC Reset #### #### ####"));

  showRegister(); 
  SPI.begin();
  showRegister(); 
  SPI.beginTransaction(DEFAULT_SPI_SETTINGS);
  showRegister();
  SPI.endTransaction();
  showRegister(); 
}

void loop()
{ }

void showRegister (void)
{
  uint16_t reg16 = 0;
  Serial.print(F("SPI0 CTRLA:    ")); formatBIN(Serial, SPI0.CTRLA, true);    reg16 = (uint16_t)&SPI0.CTRLA;    Serial.print(" ");  formatHEXln(Serial, reg16, true);
  Serial.print(F("SPI0 CTRLB:    ")); formatBIN(Serial, SPI0.CTRLB, true);    reg16 = (uint16_t)&SPI0.CTRLB;    Serial.print(" ");  formatHEXln(Serial, reg16, true);
  Serial.println();
}
µC Reset #### #### ####
SPI0 CTRLA:    0b0000'0000 0x08C0
SPI0 CTRLB:    0b0000'0000 0x08C1

SPI0 CTRLA:    0b0000'0000 0x08C0
SPI0 CTRLB:    0b0000'0001 0x08C1

SPI0 CTRLA:    0b0001'0000 0x08C0
SPI0 CTRLB:    0b0000'0000 0x08C1

SPI0 CTRLA:    0b0001'0000 0x08C0
SPI0 CTRLB:    0b0000'0000 0x08C1

Hello,

I have created today again an assembler dumpfile with avr-gcc 10.3.0 and 11.1.0 and compared as good as I can. For me it looks like the calculation for the members ctrla and ctrlb is completely missing. However, they are used and therefore still have the initialization values. For me the gcc has an error. How do you see this?

AsDump.zip (35,0 KB)

Sketch

#include <SPI.h>

void setup()
{
  SPI.begin();
  SPI.beginTransaction(DEFAULT_SPI_SETTINGS);
  SPI.endTransaction();
}

void loop()
{ }

so... Why are you using version 11.1, anyway? That's about double the last vendor-supported compiler version, and has very recent MAJOR AVR-related patches to add MODE_CC ( https://www.avrfreaks.net/forum/avr-gcc-and-avr-g-are-deprecated-now )

Problems with 11.1 probably need to be treated as if they could be actual compiler bugs, rather than Arduino bugs. :frowning:
(Where are you getting 11.1 binaries, anyway?)

Your assembly listings will be slightly more readable if you add the -C switch to objdump ("demangle C++ symbols")

So, 11.1 divided by 7.3 is 1.5, less than double. :joy:

Why should avr-gcc be obsolete? I do not understand the statement. I have to translate this, I am german native speaker and my english is not the best.

Why I use 11.1? There are always some new cool features I want to try. I'm also aware that if you want to provide code to someone, you'll quickly lose Arduino compatibility. From the other side, I can announce the bug so it can be fixed. The sooner the better.

Thanks for the objdump tip. It makes the function names readable.

gcc

avr-libc (im Terminal)
svn co svn://svn.savannah.nongnu.org/avr-libc/trunk SVN_avr-libc

binutils

Or ready from zakkemble
Only the µC specific files are missing. You can find them here
Devicepacks
rename to .zip and unpack it

new dump files with -C Option
AsDump.zip (69,9 KB)

I wasn't counting Arduino as a "compiler vendor" (maybe I should.) Microchip is still shipping version 5.4 with "Microchip Studio" (Nee "Atmel Studio") (or perhaps "not" - they're pushing "XC8" (which is also currentl gcc 5.4, but I hear is likely to become LLVM-based.)


Why should avr-gcc be obsolete? I do not understand the statement.

Because the majority of the people who work on gcc (the compiler) really don't care very much about 8-bit microcontrollers. And the people who work on 8bit microcontrollers mostly aren't up to working on a very complex multi-target compiler like gcc.
Recently, gcc changed their "front end" (the part that is common to all CPUs) to use a different model of how the Condition codes (Zero, Carry, Negative, etc) are used and preserved. This was a major change, and required that all of the
"back ends" (that convert front-end output to cpu-specific code) also change. There was no one ready and willing to change the AVR backend. Starting with v10. AVR support was "deprecated". and it was scheduled to be removed entirely in v11.
I looked at it, and I couldn't make heads or tails of what needed to be done. (I'm an experienced programmer, but ... not a compiler guru, and I've never worked on GCC.)

Eventually, people contributed about $7,000 to entice someone to do this, someone took the bait and did it, and v11.1 has AVR support again. Read the thread on avr-freaks for the details.
But the number of "highly-competent eyes" looking at v11.1 AVR-GCC is VERY small, and it's likely to be "a while" before any bugs are fixed. Or even noticed. :frowning:

Could you put your .elf and .hex files somewhere accessible as well?

  1 .text         00000cf8  00000000  00000000  000000b4  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
  2 .rodata       00000052  00004cf8  00000cf8  00000dac  2**0
                  CONTENTS, ALLOC, LOAD, READONLY, DATA

I notice that the memory layout has changed since v5/v7 - PROGMEM data used to get put in the .text segment, and it was positioned in between the vectors and the code. Now it's positions AFTER the code. (actually, this looks like a change that shows up with the ATmega4809, rather than the new compiler.)
(AND it ought to be the same with 10.x and 11.x. I'm just plucking at straws.)

Hi,

neither Arduino nor Microchip are compiler vendors. Both use only the avr-gcc. Why Microchip still ships Atmel Studio with the old 5.4 nobody knows. But it doesn't matter with what they deliver their Studio. Microchip does not have C++ in mind. They are focused on assembler and C.

I followed the backend conversion and also donated. I had also written to Arduino as a big profiteer. But there was absolutely no reaction from Arduino. I still resent that. I also wrote to Microchip. They donated the biggest part without which nothing would have happened. They understood at the last minute that otherwise their µC would be thrown out.

"but ... not a compiler guru, and I've never worked on GCC."
I don't understand. If you program with Arduino, you automatically work with avr-gcc. And the Arduino IDE currently uses avr-gcc 7.3.0.

Thanks for your support. Somehow I need to try to submit resilient material to someone. So that this can be fixed. If it would concern only the ATmega4809, then I would know whom I can ask. With a Mega2560 there is no problem with SPI. In the dumpfile I could only see that some vectors are different. But I have no idea about that.

new package: AsDumps.zip (98,3 KB)

However, there is still a connection between SPI API and avr-gcc. MCUdude uses the old SPI API for its ATmega4809 IDE package. This works with the avr-gcc 11.1.0. It all sounds crazy. I know. :wink:

And as said, if you omit the call config(DEFAULT_SPI_SETTINGS) in the SPI.cpp, it also works with avr-gcc 11.1.0

Hi,

I have tried the avr-gcc 11.1.0 now with a AVR128DB48. For this I use the Arduino package from SpenceKonde. In short. SPI with display works. When I look at the SPI lib used it is the same in basic functions. Of course it is a bit adapted for the AVRxDB. But the config(DEFAULT_SPI_SETTINGS) call is the same. Would harden from my point of view the suspicion that there is a problem in connection with the ATmega4809 in the gcc.

Microchip is very much a compiler vendor. They bought Hi-Tech Software in 2009, and sell XC8, XC16, and XC32 toolchains originally supporting 8bit, 16bit, and 32bit PIC processors. XC16 and XC32 are "modified" versions of gcc (much to the frustration of OSSW folk, though the web is full of instructions on how to disable the "license check" or build-your-own mostly-the-same versions.)

XC8 was based on the proprietary Hi-Tech compiler, though (8bit PICs being quite far away from the architectural models that gcc likes.) Support for AVR in XC8 was added after Microchip acquired Atmel, currently by running avr-gcc.
The XC compilers sell for $1000 each, and up.

That Microchip's compiler group didn't fix the avr-gcc problem themselves ought to be embarrassing, and their $5k contribution to the bounty was pretty small change, comparatively...

Surely you recognize the difference between "worked with" and "worked on"

It looks like the code path is picking up init_AlwaysInline() from SPISettings:: in api/HardwareSPI.h instead of from SPISettingsMegaAVR:: in the megaAVR SPI library (SPI.h)

My knowledge of C++ inheritance rules isn't up to figuring out whether that's a compiler issue or source issue. Maybe something to do with "inline" overriding the override?

11.1:

00000486 <SPISettingsMegaAVR::SPISettingsMegaAVR(arduino::SPISettings&)>:
  SPISettingsMegaAVR(SPISettings& x) { SPISettingsMegaAVR(x.getClockFreq(), x.getBitOrder(), x.getDataMode()); }
 486:	fc 01       	movw	r30, r24
    init_AlwaysInline(clock, bitOrder, dataMode);
  }

  // Core developer MUST use an helper function in beginTransaction() to use this data
  void init_AlwaysInline(uint32_t clock, BitOrder bitOrder, SPIMode dataMode) __attribute__((__always_inline__)) {
    this->clockFreq = clock;
 488:	80 e0       	ldi	r24, 0x00	; 0
 48a:	99 e0       	ldi	r25, 0x09	; 9
 48c:	ad e3       	ldi	r26, 0x3D	; 61
 48e:	b0 e0       	ldi	r27, 0x00	; 0
 490:	80 83       	st	Z, r24
 492:	91 83       	std	Z+1, r25	; 0x01
 494:	a2 83       	std	Z+2, r26	; 0x02
 496:	b3 83       	std	Z+3, r27	; 0x03
    this->dataMode = dataMode;
 498:	14 82       	std	Z+4, r1	; 0x04
 49a:	15 82       	std	Z+5, r1	; 0x05
    this->bitOrder = bitOrder;
 49c:	81 e0       	ldi	r24, 0x01	; 1
 49e:	90 e0       	ldi	r25, 0x00	; 0
 4a0:	86 83       	std	Z+6, r24	; 0x06
 4a2:	97 83       	std	Z+7, r25	; 0x07
 4a4:	08 95       	ret

Which matches api/hardwarespi.h:

  // Core developer MUST use an helper function in beginTransaction() to use this data
  void init_AlwaysInline(uint32_t clock, BitOrder bitOrder, SPIMode dataMode) __attribute__((__always_inline__)) {
    this->clockFreq = clock;
    this->dataMode = dataMode;
    this->bitOrder = bitOrder;
  }

Whereas in 10.3:

    :
  // Core developer MUST use an helper function in beginTransaction() to use this data
  void init_AlwaysInline(uint32_t clock, BitOrder bitOrder, SPIMode dataMode) __attribute__((__always_inline__)) {
    this->clockFreq = clock;
 49a:	40 e0       	ldi	r20, 0x00	; 0
 49c:	59 e0       	ldi	r21, 0x09	; 9
 49e:	6d e3       	ldi	r22, 0x3D	; 61
 4a0:	70 e0       	ldi	r23, 0x00	; 0
 4a2:	40 83       	st	Z, r20
 4a4:	51 83       	std	Z+1, r21	; 0x01
 4a6:	62 83       	std	Z+2, r22	; 0x02
 4a8:	73 83       	std	Z+3, r23	; 0x03
    this->dataMode = dataMode;
 4aa:	14 82       	std	Z+4, r1	; 0x04
 4ac:	15 82       	std	Z+5, r1	; 0x05
    this->bitOrder = bitOrder;
 4ae:	81 e0       	ldi	r24, 0x01	; 1
 4b0:	90 e0       	ldi	r25, 0x00	; 0
 4b2:	86 83       	std	Z+6, r24	; 0x06
 4b4:	97 83       	std	Z+7, r25	; 0x07
    clockSetting = F_CPU_CORRECTED / 2;
 4b6:	40 91 00 28 	lds	r20, 0x2800	; 0x802800 <F_CPU_CORRECTED>
 4ba:	50 91 01 28 	lds	r21, 0x2801	; 0x802801 <F_CPU_CORRECTED+0x1>
 4be:	60 91 02 28 	lds	r22, 0x2802	; 0x802802 <F_CPU_CORRECTED+0x2>
 4c2:	70 91 03 28 	lds	r23, 0x2803	; 0x802803 <F_CPU_CORRECTED+0x3>
 4c6:	76 95       	lsr	r23
 4c8:	67 95       	ror	r22
 4ca:	57 95       	ror	r21
 4cc:	47 95       	ror	r20
    clockDiv = 0;
 4ce:	80 e0       	ldi	r24, 0x00	; 0
    while ((clockDiv < 6) && (clock < clockSetting)) {
 4d0:	c4 16       	cp	r12, r20
 4d2:	d5 06       	cpc	r13, r21
 4d4:	e6 06       	cpc	r14, r22
 4d6:	f7 06       	cpc	r15, r23
 4d8:	40 f4       	brcc	.+16     	; 0x4ea <SPISettingsMeg
    :

Which has the clock code from the megaAVR library (but I don't see the mode/bitorder copy in the source. Are the methods BOTH executed somehow?

Hello,

with Pin Toggle I could prove that the methods
init_MightInline and init_AlwaysInline
are processed. And nevertheless something goes wrong in it.

I will summarize all this and post it again in a other german µC forum. There is at least one person who knows more about gcc and ATmega4809. He may be able to see if it is only the ATmega4809 or not. Maybe this will be reported to gcc later. My own guess is that gcc has a problem.

I will report the result here. Thanks for the support. :+1: