SAM4S SPI - when receiving several bytes with the PDC, the transfer stalls after one byte of clocks

Go To Last Post
5 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello all,

 

I'm using a SAM4S2A and communicating with two external devices via SPI using manual chip select (PS bit = 0). I'm building code in Keil MDK-ARM 5.27.1.0 and using FreeRTOS.

The SPI_MR setup looks like this:

SPI->SPI_MR = SPI_MR_MSTR                         /* Master mode                                  */
            | SPI_MR_MODFDIS                      /* Mode fault detection is disabled             */
            | SPI_MR_DLYBCS( SPI_COMMON_DLYBCS ); /* Common delay between chip select activations */

The SPI_CS registers for my two peripherals look like this:

SPI->SPI_CSR[0] = SPI_CSR_CPOL
                | SPI_CSR_NCPHA
                | SPI_CSR_BITS_8_BIT
                | SPI_CSR_SCBR       ( SPI_CSR_0_SBCR   )
                | SPI_CSR_DLYBS      ( SPI_CSR_0_DLYBS )
                | SPI_CSR_DLYBCT     ( SPI_CSR_0_DLYBCT );
SPI->SPI_CSR[3] = SPI_CSR_NCPHA
                | SPI_CSR_BITS_8_BIT
                | SPI_CSR_SCBR       ( SPI_CSR_3_SBCR   )
                | SPI_CSR_DLYBS      ( SPI_CSR_3_DLYBS )
                | SPI_CSR_DLYBCT     ( SPI_CSR_3_DLYBCT );

I'm enabling the PDC in my init code, for both transmitting and receiving:

PDC_SPI->PERIPH_PTCR = PERIPH_PTCR_RXTEN | PERIPH_PTCR_TXTEN;

This works great for transmitting. My code for transmitting a packet of 3 bytes looks like this:

static uint8_t spi_write_internal( uint8_t const * buf_p,  uint16_t num_out_bytes )
{
    uint8_t ret_val = 1;

    /*
        Initialize the pointer to the write/read buffer. This is incremented by the PDC as the write happens.
    */
    PDC_SPI->PERIPH_TPR = ( uint32_t )buf_p;

    /*
        Set the transfer count. This is decremented by the PDC as the write happens. The PDC should already be enabled for transmitting. This
        should kick off the transfer.
    */
    PDC_SPI->PERIPH_TCR = num_out_bytes;

    /*
        Enable the completion interrupt; ENDRX indicates all data bits are shifted out - see the datasheet figure 33-9 p. 697.
    */
    SPI->SPI_IER = SPI_IER_TXEMPTY;

    /*
        Wait for the semaphore
    */
    BaseType_t sem_take_result = xSemaphoreTake( spi_sem_h, SPI_TIMEOUT );

    if ( pdFALSE == sem_take_result )
    {
        /*
            We didn't get the semaphore within the expected timeout period. It should have been given by our interrupt handler when the
            transfer was completed. Attempt to give the mutex without checking the error return and bail out.
        */
        Runtime_Err_Report( RUNTIME_ERR_SPI_DRIVER_TAKE_SEM_FAILED );
        ret_val = 0;
    }

    return ret_val;

}

I'm setting the chip select before calling this function:

/*
    Set the peripheral select bits manually
*/
SPI->SPI_MR = ( ( SPI->SPI_MR & ~SPI_MR_PCS_Msk ) | SPI_MR_PCS( periph_select_bits ) );

I'm using an interrupt handler to catch the end of transfer and give the semaphore. Currently I'm using TXEMPTY but I've also done it with ENDRX and that works OK for transmitting, too.

This works great for transmitting, but I'm trying to use it for receiving. Specifically, I want to take the chip select low, clock out a command, and then continue to transmit zeroes while the chip select is still low to read several bytes of data before the chip select goes high.

 

The problem I'm having is that for all my receive transactions, the SPI peripheral only sends the first outgoing byte and then it seems to get into an overflow condition. I can see the peripheral take control of MISO but before any clocks go out, the transmission ends and I never get my completion interrupt.

 

I've tried messing with the CSAAT bits and that will result in the chip select staying low, but no more bytes. I've tried avoiding the OVRES condition by reading SPI_SR and RDR before setting the read counter. It doesn't seem to make a difference whether I disable and re-enable the PDC for each transaction, or not.

Here's my read routine (which fails as described) --

static uint8_t spi_read_internal( uint8_t * buf_p,  uint16_t num_in_bytes )
{
    uint8_t ret_val = 1;

    /*
        Set the transmit data register to send zeroes when we read
    */
    SPI->SPI_TDR = 0;

    /*
        Set up the pointer, which the PDC will increment
    */
    PDC_SPI->PERIPH_RPR = ( uint32_t )buf_p;

    /*
        Set the read count
    */
    PDC_SPI->PERIPH_RCR = num_in_bytes;

    /*
        Enable the completion interrupt; ENDRX indicates all data bits are shifted out - see the datasheet figure 33-9 p. 697.
    */
    SPI->SPI_IER = SPI_IER_TXEMPTY;

    /*
        Wait for the semaphore
    */
    BaseType_t sem_take_result = xSemaphoreTake( spi_sem_h, SPI_TIMEOUT );

    if ( pdFALSE == sem_take_result )
    {
        /*
            We didn't get the semaphore within the expected timeout period. It should have been given by our interrupt handler when the
            transfer was completed.
        */
        Runtime_Err_Report( RUNTIME_ERR_SPI_DRIVER_TAKE_SEM_FAILED );
        ret_val = 0;

    }

    return ret_val;

}

It seems like opening up a single transaction and doing a write-then-read with the PDC should be a common and simple use case, especially since peripherals like the Atmel AT25M01 EEPROM chip expects this. Any suggestions?

 

Thanks,

 

Paul R. Potts

 

Paul R. Potts - paul@thepottshouse.org

Last Edited: Wed. Jun 5, 2019 - 10:54 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Some followup:

 

Out of desperation I changed the receive routine to do busy-waiting like so:
 

/*
    If RDRF is set, read RDR to clear it, so we get fresh data
*/
if ( ( SPI->SPI_SR & SPI_SR_RDRF ) == SPI_SR_RDRF )
{
    ( void )SPI->SPI_RDR;
}

while ( num_in_bytes-- > 0 )
{
    /*
        Wait until the transmit buffer is empty
    */
    while ( ( SPI->SPI_SR & SPI_SR_TDRE ) != SPI_SR_TDRE )
    {
        /*
            DO NOTHING
        */
    }

    *( uint8_t * )&( SPI->SPI_TDR ) = 0;

    /*
        Wait until the receive buffer is not empty
    */
    while ( ( SPI->SPI_SR & SPI_SR_RDRF ) != SPI_SR_RDRF )
    {
        /*
            DO NOTHING
        */
    }

    /*
        Read the byte; low bits only
    */
    *in_buf_p++ = ( uint8_t )SPI->SPI_RDR;

}

This works fine - but I would really like to have the PDC do this. (I guess I could enable interrupt handling for each wait instead of spinning, which would let me put the calling task to sleep, but since the intervals will probably arrive very quickly, that doesn't seem like a big win).

 

Other things I've tried: WDBRX (just seems to make things worse), disabling SPI and re-enabling as suggested in the datasheet on p. 698 (I still got truncated transfers).

 

Paul R. Potts - paul@thepottshouse.org

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's been some years since I have thought about this or tested it, but I believe you must load byte counts in both the TX and RX PDC registers (SPI_TCR and SPI_RCR). The SAM4S SPI must generate the clock, and it only does that when it thinks it has data to transmit on the bus. Refer to Figure 33-7.

 

Your code would probably work fine as-is if your SAM4S were an SPI slave.

Josh @ CIHOLAS Inc - We fill the gaps from chips to apps

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for the suggestion, Josh. I will give that a try.

 

It makes a certain amount of sense that the PDC would need to write in order to read, because of course that's how SPI works at the low level, and that's how I would do it when writing to the SPI peripheral registers directly. But it seems like something it could do itself when configured for reading only - after all, it is acting as a "user interface" on top of the SPI and it manages the reloading and testing flags.

 

It sounds like I need to provide a transmit buffer to use the PDC to receive, which is something I was hoping not to do.

 

I suppose the one byte going out must be the due to the zero I'm loading into the TDR.

 

Thanks,

 

Paul

Paul R. Potts - paul@thepottshouse.org

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OK - here is what I am doing to kick off a SPI read now, using the PDC.

 

With the busy-waiting code, I left it running a torture test of writing data and reading back data to a peripheral, overnight, and it passed without a single read failure. I'm hoping the PDC version will be as reliable.

 

Note that even though I am using only the PDC and not handling any of the SPI peripheral registers directly, other than to enable and disable the TXEMPTY interrupt, I have seen a consistent problem where in some cases the RDRF flag is set upon entry to my read function, and so I get a stray byte and reach my count prematurely. Doing an extra read of RDR before kicking off the PDC transfer seems to fix this problem. I don't think the PDC should be handling the SPI peripheral such that RDRF is set either after completing a read or a write, and suspect that is a hardware bug, but this seems to work around it.

I was seeing this problem before, because it was leading to the OVRES flag set after clocking out my first zero byte, and so I suspected that it was the reason the receive was only sending one byte - I don't think it is; the OVRES doesn't result in no more clocks.

 

Here's the relevant part of the receive function as it stands now. I'll be torture-testing this version.

PDC_SPI->PERIPH_RPR = ( uint32_t )in_buf_p;
PDC_SPI->PERIPH_TPR = ( uint32_t )zero_write_buf_a;

/*
    Even though we are using the PDC to manage the SPI flags and registers and not touching them ourselves, sometimes when we enter this
    routine the RDRF flag is set, indicating stale data in the receive buffer, and our read is returning one byte of old data. This should not
    be happening, but reading RDR to clear RDRF before restarting the PDC seems to do the trick.
*/
( void )SPI->SPI_RDR;

/*
   Set the write and read counts. Use a critical section here. I don't want any possibility that the read and write counts could get out of
   sync because the processor is interrupted between the two loads, and the PDC runs ahead before getting the second count.
*/
taskENTER_CRITICAL();
PDC_SPI->PERIPH_RCR = num_in_bytes;
PDC_SPI->PERIPH_TCR = num_in_bytes;
taskEXIT_CRITICAL();

/*
    Enable the completion interrupt; ENDRX indicates all data bits are shifted out - see the datasheet figure 33-9 p. 697.
*/
SPI->SPI_IER = SPI_IER_TXEMPTY;

Thanks,

 

Paul

 

Paul R. Potts - paul@thepottshouse.org