Strange SPI receive routine in Cube example code

[waclawek.jan]

Posted on March 13, 2017 at 00:37

This is supposed to receive one byte in SPI bidirectional mode ([STM32Cube_FW_L4_V1.5.0]\Drivers\BSP\STM32L476G-Discovery\stm32l476g_discovery.c):

&sharpif defined(__ICCARM__)

&sharppragma optimize=none

&sharpendif

/**

 * @brief Receives a Byte from the SPI bus.

 * @retval The received byte value

 */

static uint8_t SPIx_Read(void)

{

 uint8_t receivedbyte;

   __HAL_SPI_ENABLE(&SpiHandle);

   __DSB();

   __DSB();

   __DSB();

   __DSB();

   __DSB();

   __DSB();

   __DSB();

   __DSB();

    __HAL_SPI_DISABLE(&SpiHandle);

 while((SpiHandle.Instance->SR & SPI_FLAG_RXNE) != SPI_FLAG_RXNE);

 /* read the received data */

 receivedbyte = *(__IO uint8_t *)&SpiHandle.Instance->DR;

 /* Wait for the BSY flag reset */

 while((SpiHandle.Instance->SR & SPI_FLAG_BSY) == SPI_FLAG_BSY);

 return receivedbyte;

}

Can ST please comment on:

• Why the &sharppragma? What in this code is supposed to be not-optimized-out? Would ICC optimize out the repeated __DSB()s (sounds not very plausible but one never knows, I don't have ICC)? How about other compilers?
• Why the 8 __DSB()? Is this supposed to be a time waste? Is there something wrong with __NOP()?
• The check for RXNE and BUSY after __HAL_SPI_DISABLE() indicates that the __DSB()s are intended to provide a way to 'start issuing clocks' but disable SPI soon enough so that clocks only for one frame are issued. Correct?
• If above point is true, what if an interrupt happens in middle of the __DSB()s?
• Is the number of __DSB()s dependent on SPI baudrate?
• Where is all this documented in the RM, exactly?

And while at commenting on STM32 SPI issues, can ST please comment also on

https://community.st.com/message/139780-re-stm32f072-spi-bidirectional-mode-strange-behavior?commentID=139780&et=watches.email.thread&sharpcomment-139780 [linkfix Feb.2019 https://community.st.com/s/question/0D50X00009XkZNhSAN/stm32f072-spi-bidirectional-mode-strange-behavior ]

and

https://community.st.com/0D50X00009XkW6HSAV

and

https://community.st.com/0D50X00009XkaArSAJ

threads?

Thanks,

Jan Waclawek

null

[Nesrine M_O]

Posted on March 13, 2017 at 11:37

Hi

Waclawek.Jan

,

Thank you very much for your contribution to the enhancement of our STM32 related solutions.

I am checking your questions and comments. Please be patient,I will come back to you.

-Nesrine-

[Nesrine M_O · Accepted solution]

Posted on March 23, 2017 at 10:41

Hi

Waclawek.Jan

• Why the #pragma? What in this code is supposed to be not-optimized-out? Would ICC optimize out the repeated __DSB()s (sounds not very plausible but one never knows, I don't have ICC)? How about other compilers?

  The pragma has been used to avoid any code optimization, other compilers are missing,this issue has been reported to our team to be considered in next releases.

• Why the 8 __DSB()? Is this supposed to be a time waste? Is there something wrong with __NOP()?

The use of DSB is recommeded but is the same goal as the NOP.

• The check for RXNE and BUSY after __HAL_SPI_DISABLE() indicates that the __DSB()s are intended to provide a way to ''start issuing clocks'' but disable SPI soon enough so that clocks only for one frame are issued. Correct?

  In this very specific cases SPI is used as master with only one line for TX/RX data, so the reception function want generate only one data clock means partners is able to send only one data a

  nd we will a new data clock to send the next data.

• If above point is true, what if an interrupt happens in middle of the __DSB()s?

  Right this is missing ,we maybe must disable IRQ depending application,

  this issue has been reported

  to our team for checking .

• Is the number of __DSB()s dependent on SPI baudrate?

Yes, the number of DSB is SPI baud-rate dependent and the baudrate itself is CLK dependent so the number of DSB is system dependent.

• Where is all this documented in the RM, exactly?

  (The sequence provided is really specific to this mode (where we are transferring the data one by one)

-Nesrine-

[waclawek.jan]

Posted on March 23, 2017 at 21:21

Nesrine,

Thanks for the detailed answer.

While I understand the reason, I am not happy with a software delay which is aimed not only at providing a minimal delay, but to fit into a timing window. This could be concocted into a more controlled sequence with a timer-triggered DMA ... I'll try and report if time permits.

we maybe must disable IRQ depending application,

While I agree that the particular requirements on this delay depend on the application, this code is supposed to be one of the few examples (if not the only one) demonstrating bidirectional transfer on SPI. I'd recommend to add a comment, both explaining the reason for the delay, the calculation of its duration, and the need for disabling/reenabling IRQ where appropriate.

My 2c... 🙂

Thanks again,

Jan