Years on, continue to see the same bad timeout code

[Tesla DeLorean]

Posted on May 30, 2018 at 18:04

I've railed on this before, but it keeps getting into the HAL code base. Can someone in charge of managing the software group please explain to the team members who keep using this construct what is wrong with it.. This shouldn't be a lesson that needs to be repeated. It shouldn't be my job to walk the code.

STM32Cube_FW_L4_V1.11.0\Projects\32L4R9IDISCOVERY\Demonstrations\MenuLauncher\Core\Src\sd_diskio_dma.c

DRESULT SD_read(BYTE lun, BYTE *buff, DWORD sector, UINT count)

{  DRESULT res = RES_ERROR;  ReadStatus = 0;  uint32_t timeout;

  if(BSP_SD_ReadBlocks_DMA((uint32_t*)buff,

                           (uint32_t) (sector),                           count) == MSD_OK)  {    /* Wait for the Rading process is completed or a timeout occurs */    timeout = HAL_GetTick() + SD_TIMEOUT;    while((ReadStatus == 0) && (HAL_GetTick() < timeout))    {      __NOP();    }

...

Found this in at least half a dozen files, and might well exists in other timeout loops.

Here done correctly

STM32Cube_FW_L4_V1.11.0\Projects\STM32L4R9I-EVAL\Applications\FatFs\FatFs_uSD_DMA_Standalone\Src\sd_diskio_dma.c

  if(BSP_SD_ReadBlocks_DMA((uint32_t*)buff,

                           (uint32_t) (sector),                           count) == MSD_OK)  {    /* Wait for the Rading process is completed or a timeout occurs */     timeout = HAL_GetTick();    while((ReadStatus == 0) && ((HAL_GetTick() - timeout) < SD_TIMEOUT))    {    }

...

This is not a concept that is hard to grasp, identify the individuals who write those lines of code, they poison the repository. Can we have team members who can sight read code properly check off code from individuals that can't.

[T J]

Posted on May 30, 2018 at 19:14

I think it is difficult to grasp.

This is why Clive is correct: the errant code

timeout = HAL_GetTick() + SD_TIMEOUT;      <- here the result may be 0xFFFFFFFF the highest of all numbers.

the errant test

while((ReadStatus == 0) && (HAL_GetTick() < timeout))

in this example, HAL_GetTick() will forever be less than timeout

Clive's suggestion does fix the problem.

 HAL_GetTick() - timeout    is always relative and accurate even across the FFFFFFFF - 00000000 boundary.

however, I suggest the code be slightly changed for the better understanding as you read it,

only the name has been changed for clarity.

STM32Cube_FW_L4_V1.11.0\Projects\STM32L4R9I-EVAL\Applications\FatFs\FatFs_uSD_DMA_Standalone\Src\sd_diskio_dma.c

  if(BSP_SD_ReadBlocks_DMA((uint32_t*)buff,

                           (uint32_t) (sector),                           count) == MSD_OK)  {    /* Wait for the Rading process is completed or a timeout occurs */     uint32_t startTime = HAL_GetTick();    while((ReadStatus == 0) && ((HAL_GetTick() - startTime) < SD_TIMEOUT))  // works across -ve boundary    {    }

...

[henry.dick]

Posted on May 30, 2018 at 20:05

'

This is not a concept that is hard to grasp,'

My experience with st programmers come with the f1 spl i2c code and all the fixes proposed by st.

Then I ran into an individual who struggled a big way to run code on esp32. This particular individual claimed to have worked on spl.

As a result I have adjusted my expectation for code from st.

[Amel NASRI]

Hello,

Although it is an issue that you reported since a while, it is just now that we reviewed it.

Be ensured that this is tracked internally to take care of your feedback.

-Amel