1
\$\begingroup\$

I am configuring my first SPI transmission on my atmega328p.

Everything seems right (My SPI interrupt ISR(SPI_STC_vect) fires when transmission is done, and I can see all SPI pins on the oscilloscope behave as expected) , except from this example on atmega328p's datasheet on page 137:

datasheet screenshot

It waits untill transmission is done. (When transmission is done, SPIF bit inside SPSR register becomes 1, you can confirm it on datasheet page 141).

When I try and use this example on my code, my code hangs in that loop, and never leaves it.

But if I do this instead:

        #define BIT(x) (0x01 << (x)) // 0<x<7
        #define BIT_GET(p,m) ((p) & (m))
        ...
        while(BIT_GET(SPSR, BIT(7)))
        {
            ;
        }

The code does not hang.

Or, even if I remove the NOT (!) from the original example, it will not hang, like this:

        while((SPSR & (1<<SPIF)))
        ;

(Which I do not know if its right or wrong, it was just a test of mine)

So is the example wrong? I did google it a bit, and no one seemed to have any issue with this example not working.

EDIT: ADDING CODE

int main(void)
{
init_rxtx_function();
init_SPI();

BIT_SET(SREG, BIT(7)); //Global interrupts enabled I=1

swn(SPSR, 2, 1);//prints SPSR: it is 0
BIT_CLEAR(PORTB, BIT(2));//SS low

sw("Before while\n\r");

SPDR = 5;//Configuration write address
        
while(!(SPSR & (1<<SPIF)))
{
    if(inter == 1)
    {
        //pass
    }
    else
    {
        sw("SPSR: ");
        swn(SPSR, 2, 1);//print SPSR
    }
};
        
sw("After while\n\r");
}

/*
inits the SPI as master, changing the default pin overrides.

*/
void init_SPI()
{
    //=====SPI SETUP FOR PT100======

//===SPI pins override====
//MOSI OUTPUT:
BIT_SET(DDRB, BIT(3));//PB3=1
//SCK OUTPUT
BIT_SET(DDRB, BIT(5));//PB5=0   
//SS OUTPUT AND HIGH
BIT_SET(DDRB, BIT(2));//PB2=0
BIT_SET(PORTB, BIT(2));//PORTB2=1


//=====FUNCTION MODE SELECT=====
//Master mode enable
BIT_SET(SPCR, BIT(4));//MSTR=1

//======DATA MODE====
//Modes 1 and 3 supported.
//Select mode 1(CPOL=0, CPHA=1)
BIT_CLEAR(SPCR, BIT(3));//CPOL=0
BIT_SET(SPCR, BIT(2));//CPHA=1

//====DATA ORDER===
//MSB first
BIT_CLEAR(SPCR, BIT(5));//DORD=0

//=====CLOCK RATE SELECT=====
//Fosc/128
BIT_CLEAR(SPSR, BIT(0));//SPI2X=0
BIT_SET(SPCR, BIT(1));//SPR1=1
BIT_SET(SPCR, BIT(0));//SPR0=1

//===SPI interrupt enable===
BIT_SET(SPCR, BIT(7));//SPIE=1

//===SPI ENABLE===
BIT_SET(SPCR, BIT(6));//SPE=1

sw("SPSR: ");//status reg. should be: 0x000000
swn(SPSR, 2, 1);

sw("SPCR: ");//control reg. should be: 11010111
swn(SPCR, 2, 1);
}


/*SPI Serial Transfer Complete Interrupt*/ 
ISR(SPI_STC_vect)
{
    inter = 1;
}

It outputs:

teraterm screenshot

Everything is ok, program stucked in while()

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Please post the whole SPI init and handling code, including the SPI interrupt handler. The answer depends on it. Do you have SPI interrupt enabled, and still wait for SPIF? \$\endgroup\$
    – Justme
    May 19 at 7:06
  • \$\begingroup\$ made some edits :3 \$\endgroup\$ May 19 at 12:36
2
\$\begingroup\$

The answer to your situation is, that on an AVR, many events that can trigger an interrupt flag to be set, are also automatically cleared just by executing the interrupt, so usually the user does not need to explicitly clear interrupt flags in code (there are exceptions though).

So when the SPI transfer is complete, the hardware will set the SPIF bit, and since SPI interrupt (and global interrupts) are enabled, the CPU goes to execute the SPI interrupt vector, which will automatically resets the pending interrupt by clearing the SPIF bit.

Therefore, the main code that is constantly reading the status register in a loop to wait for SPIF bit to go high will (almost) never see the SPIF bit being set and thus will (almost) never continue with execution.

The almost part means that when also other interrupts are being used (UART, timer, etc), and the AVR returns from an interrupt, it will execute one opcode before executing an another pending interrupt, so this way it is possible that sometimes the SPIF bit can be seen high by the main code even if SPI interrupts are used.

This basically means, you can either use polling or interrupts, but when SPI interrupts are enabled, the SPIF flag cannot then be used for polling any more.

In this case, the interrupt code should set a (volatile) variable to indicate to main code that the SPI transfer in interrupt context is complete - either after one byte, or perhaps after a transmission of a full block of several bytes in length if that is what the user wants the program to do.

\$\endgroup\$
3
\$\begingroup\$

According to the same manual 18.5.2

When a serial transfer is complete, the SPIF Flag is set. /--/ the SPIF bit is cleared by first reading the SPI status register with SPIF set, then accessing the SPI data register (SPDR).

These kind of peculiar flag clear procedures, where you clear something by reading are somewhat common with SPI and UART. Their code example is correct, since it loops while the flag is not set, waiting for transmission to be complete.

However, if you were to peek at this code from a debugger, the debugger might also be displaying a memory map or reading the registers through its watch list, as if the registers were any local variable. And thereby destroying the flag. This is a well-known and very annoying phenomenon with debuggers. If you suspect this to be the case, then you could try to not break or single step through the function, but set a breakpoint directly after it and run til there.

Your examples will not hang because they are incorrect, they don't wait for the flag, they just check if it is 1 - it is not, then leave the loop.


General advise about bad practice to avoid:

  • Don't write icky macros to hide bitwise operators. C programmers know what SPSR & (1 << 7) means. They do not know what BIT_GET(SPSR, BIT(7)) means. The only thing you achieved with the macro was to turn the code far less readable.

  • Never bit shift the value 1. Integer constants have a type like variables and in case of 1 it is a signed type. So if you do 1 << 15 on a 8 bitter AVR you actually have an undefined behavior bug, since you are shifting data into the sign bit of a signed int. To avoid bugs like that, always write 1u << n so that the arithmetic is carried out on unsigned types. This is a common rookie mistake, one made by the person who wrote the Atmel datasheet among others.

\$\endgroup\$
1
  • \$\begingroup\$ upvote for mentioning potential bit shifting pitfall \$\endgroup\$
    – Aaron
    May 18 at 16:15
2
\$\begingroup\$

You do need to wait for the transmission to complete, so that if your code wants to put more data output register, it will not corrupt the last few bits of data from the previous transfer. Especially if you have some sort of external driver chip that is being enabled, like RS485/RS422.

In the past, what I have done is set the interrupt for end of data, then inside that interrupt check for end of buffer flag (load the next byte or end of buffer). If that flag is true, then turn on the end of transmission interrupt and turn off the end of data ISR. Then in the end of transmission ISR you can turn off any external driver chips and reset the state machines for the next packet of data to send.

\$\endgroup\$
0
\$\begingroup\$

Ok I asked on AVRfreaks here

The solution was:

I should NOT use both interrupt and polling the flag to check if the transmission has ended.

Because the interrupt will clear the flag first, so the polling will hang waiting for a flag that never (almost never) gets set.

thank you @ki0bk aka Jim

\$\endgroup\$
2
  • 1
    \$\begingroup\$ So exactly what I suspected but did not have the chance to post an answer. And that is the reason why you should not cross post. \$\endgroup\$
    – Justme
    May 19 at 15:20
  • \$\begingroup\$ Make a post rephrasing the answer and Ill mark it as correct answer. I don't get points when marking my answer anyway. \$\endgroup\$ May 20 at 7:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.