STMicroelectronics / stm32f4xx-hal-driver

Provides the STM32Cube MCU Component "hal_driver" of the STM32F4 series.
BSD 3-Clause "New" or "Revised" License
119 stars 50 forks source link

Potential design issue/unwanted behavior in/of HAL_SPI_Receive_DMA() #24

Closed lebakassemmerl closed 5 months ago

lebakassemmerl commented 1 year ago

My Setup I am using an STM32F407g as processor and the project I am working on is reading an writing an SD-card via an SPI interface. I am not using one of the example codes flying around in the internet which drive an SD-card and a FAT-filesystem with an STM32 uC. I am using a custom code base with drivers that provides fully asynchronous APIs to the individual peripherals of the uC. And these drivers are using the functions provided by this HAL library.

Design issue

I am making use of the function HAL_SPI_Receive_DMA() in my SPI-driver wich is used by my SD-driver. This function has a behavior which caused me quite a time to figure out why my SD-card isn't working and why the data on the SPI-bus looks quite "random".

The function starts as it follows:

HAL_StatusTypeDef HAL_SPI_Receive_DMA(SPI_HandleTypeDef *hspi, uint8_t *pData, uint16_t Size)
{
  HAL_StatusTypeDef errorcode = HAL_OK;

  /* Check rx dma handle */
  assert_param(IS_SPI_DMA_HANDLE(hspi->hdmarx));

  if (hspi->State != HAL_SPI_STATE_READY)
  {
    errorcode = HAL_BUSY;
    goto error;
  }

  if ((hspi->Init.Direction == SPI_DIRECTION_2LINES) && (hspi->Init.Mode == SPI_MODE_MASTER))
  {
    hspi->State = HAL_SPI_STATE_BUSY_RX;

    /* Check tx dma handle */
    assert_param(IS_SPI_DMA_HANDLE(hspi->hdmatx));

    /* Call transmit-receive function to send Dummy data on Tx line and generate clock on CLK line */
    return HAL_SPI_TransmitReceive_DMA(hspi, pData, pData, Size);
  }

  /* Process Locked */
  __HAL_LOCK(hspi);

  ...
}

So if the SPI-module is configured as master with 2 data-lines, the function calls HAL_SPI_TransmitReceive_DMA() and uses the receive-buffer pData as transmit- and receive-buffer. This is basically not a problem, but it can cause unwanted behavior since current content of this buffer is not known but still sent out. At least in my case this caused the SD-card to do something really odd since it triggered commands where i was actually waiting for a response.

Possible solution Wouldn't it be possible to setup the DMA channels in a way, that the TX-channel always transmits a constant (by neither incrementing the source-address nor the destination address) while the receive channel receives as usual.

Since there are no dedicated DMA functions for that, i imagined something like that (just for explaining my idea).

HAL_StatusTypeDef HAL_SPI_Receive_DMA(SPI_HandleTypeDef *hspi, uint8_t *pData, uint16_t Size)
{
  const uint8_t TX_IDLE = 0xFF;
  ...

  if ((hspi->Init.Direction == SPI_DIRECTION_2LINES) && (hspi->Init.Mode == SPI_MODE_MASTER))
  {
    hspi->State = HAL_SPI_STATE_BUSY_RX;

    // setup TX DMA to send out the same frame 'Size' times
    hspi->hdmatx->Instance->CR &= ~DMA_SxCR_MINC; // disable memory increment
    hspi->hdmatx->Instance->CR |= DMA_MINC_DISABLE;
    hspi->hdmatx->Instance->PAR = (uint32_t)&TX_IDLE;
    hspi->hdmatx->Instance->M0AR = (uint32_t)&hspi->Instance->DR;
    hspi->hdmatx->Instance->NDTR = (uint32_t)Size;
    __HAL_DMA_ENABLE(hspi->hdmatx);

    // setup RX DMA to read in 'Size' bytes
    hspi->hdmarx->Instance->PAR = (uint32_t)&hspi->Instance->DR;
    hspi->hdmarx->Instance->M0AR = (uint32_t)pData;
    hspi->hdmarx->Instance->NDTR = (uint32_t)Size;
    __HAL_DMA_ENABLE(hspi->hdmarx);
  }

  ...
}

My current workaround In my driver I explicitly used the HAL_SPI_TransmitReceive_DMA() function and passed as TX-pointer a constant array filled with 0xFF. However this is not feasible if the SPI-transaction would be several hundred or even thousands of bytes.

HBOSTM commented 1 year ago

Hello @lebakassemmerl ,

Thank you for this contribution. I can't reproduce the problem from my side. So could you please give us more details about how you got this issue? And share part of the code shows this issue.

Best Regards,

HBOSTM commented 1 year ago

Hello @lebakassemmerl,

Please allow me to close this thread as no activity. You may reopen it at anytime if you have any details to share with us in order to help you to solve the issue. Thank you for your comprehension.

With regards,

lebakassemmerl commented 1 year ago

Hello!

Sorry for the very late response, I hope you find the time to have a look at this issue again.

The problem is, as I stated in the description, that the function HAL_SPI_Receive_DMA() calls internally the function HAL_SPI_TransmitReceive_DMA(). This of course make sense, since an SPI master cannot receive any bytes without concurrently sending data on the MOSI / TX line.

However, by simply calling the HAL_SPI_TransmitReceive_DMA() and passing as TX-buffer the pointer to the RX-buffer, causes the SPI module to send out the undefined data / content of the RX-buffer:

    /* Call transmit-receive function to send Dummy data on Tx line and generate clock on CLK line */
    return HAL_SPI_TransmitReceive_DMA(hspi, pData, pData, Size);

This is most of the time probably ok, but in certain situations (e.g. my SD-card driver) this causes the SPI module to send out data that cause the slave to do undefined / unexpected things due to the unknown content of the RX-buffer that was sent out to the MOSI line.

My idea would be to rewrite HAL_SPI_Receive_DMA() to not call HAL_SPI_TransmitReceive_DMA() but provide a separate implementation where the DMA is configured in a way, that as explained in the description: Set the TX-DMA channel to a constant (e.g. 0xFF or 0x00) and disable the increment of it, so that it sends out N times the same value to the MOSI line. However I think one would have to rewrite also the DMA-library in order to achieve this behavior.

Best regards

KRASTM commented 5 months ago

Hello @lebakassemmerl,

I hope you are well, concerning your issue, here is our analysis:

I already checked with our development team, the driver was implemented in comparison with the hardware, in other word the HAL_SPI_Receive_DMA() will keep calling HAL_SPI_TransmitReceive_DMA() except if there is an update but not soon enough.

In your case, as you said you can use the function HAL_SPI_TransmitReceive_DMA() with DMA_MINC_DISABLE for hdma_tx. Otherwise, you may make the necessary change that suits your application.

With regards.

KRASTM commented 5 months ago

Hello @lebakassemmerl,

Please, allow me to close this issue, thank you for your comprehension.

With regards.