Unnecessary SD card command in block read/writes

[Lama]

Hi,

When looking into stm32l4xx_hal_sd.c, I observed the following code:

   if(hsd->SdCard.CardType != CARD_SDHC_SDXC)

   {

     add *= 512U;

   }

   /* Set Block Size for Card */

   errorstate = SDMMC_CmdBlockLength(hsd->Instance, BLOCKSIZE);

   if(errorstate != HAL_SD_ERROR_NONE)

   {

     /* Clear all the static flags */

     __HAL_SD_CLEAR_FLAG(hsd, SDMMC_STATIC_FLAGS);

     hsd->ErrorCode |= errorstate;

     hsd->State = HAL_SD_STATE_READY;

     return HAL_ERROR;

   }

The default block size for SDHC and SDXC is 512, so setting block size should be moved into the if-statement avoiding an unnecessary SD card command for the block read/write calls.

@ST could you adopt this change in coming releases?

[Mike_ST]

Yes, I have seen that too at some point. Hope this is not for some compatibility with some cards.

I would say that SDMMC_CmdBlockLength should only be sent at init time.

Anyways, ticket entered.

[Tesla DeLorean]

Definitely some quirk cards out there. Also seem to recall that a) the CSD or some other packet is smaller, b) the performance hit is pretty negligible in the scale of things when Block Count == 1. The SDMMC_CmdBlockLength () could cache the last value sent to the card, and basically fold execution time to zero.

[Tesla DeLorean]

Will also note that no one is hog-tied to the code in the library, you can fix it and clean it to production grade.

Personally I have bigger issues with the old FatFs middleware with known issue on larger volumes, and the broken cache coherency code.

[Tesla DeLorean]

@Mike_ST​ Add this one to your list

https://community.st.com/s/question/0D50X00009XkXGOSA3/sdmmc-stm32f7-sdmmcgetcmdresp1-gets-stuck

I've written code almost identical to this, when the buffer isn't 32-byte aligned it is CRITICAL that you CleanDCache the unaligned pieces/overlap at each end upon entry, otherwise the completion side InvalidateDCache destroys structures/variables falling in those dirty cache lines.

@brk​ Search on this forum is still appallingly awful, a search of "SCB_CleanDCache_by_Addr" didn't find it, I had to come in via Google. I'm not sure how SaleForce catalogues this stuff, but I think Larry Ellison's Cat could have coded it better. #AlmostWorthless

[Brian Kling]

We are currently reviewing a different search tool, should know more by early next week.

[Lama]

Thnx for the ticket. For SDHC and SDHX explicit init of the block size is not even required if I understand the standard correctly, Since standard SD cards are not available a quick fix without side effects would be to only set the block size explicit for this case.

[Lama]

I would prefer clean code and simple code in stead of exceptions for cards that do not comply with the standard. Wrt your point b) I would say in that case the performance hit is the worst: 1 command bus access that is not needed for 512 bytes of data, but maybe I misunderstood.