[PATCH] mmc: sdhci: Fix possible Synchronous Abort using PIO mode

[Jonas Karlman]

Hi Peng,

On 2025-04-10 14:28, Peng Fan wrote:
> Hi Jonas,
> 
> On Thu, Jan 23, 2025 at 09:48:48PM +0000, Jonas Karlman wrote:
>> When MMC_SDHCI_SDMA=y or MMC_SDHCI_ADMA=y and PIO mode is used
>> dma_unmap_single() is called on an unmapped address, 0x0. This may
>> result in a Synchronous Abort:
>>
>>  ## Checking hash(es) for Image atf-1 ... sha256+ OK
>>  CMD_SEND:16
>>                  ARG                      0x00000200
>>                  MMC_RSP_R1,5,6,7         0x00000900
>>  CMD_SEND:18
>>                  ARG                      0x00004005
>>  "Synchronous Abort" handler, esr 0x96000147
>>  elr: 00000000400015bc lr : 0000000040012b4c
>>  x 0: 0000000000008000 x 1: 0000000000092600
>>  x 2: 0000000000000040 x 3: 000000000000003f
>>  x 4: 0000000000000030 x 5: 0000000000000001
>>  x 6: 0000000000000001 x 7: 0000000000000000
>>  x 8: 000000000000000a x 9: 0000000000000090
>>  x10: 0000000043dffc68 x11: 0000000043c00440
>>  x12: 0000000043c00440 x13: ffffffffbfe00000
>>  x14: 000000000000031c x15: 0000000240000000
>>  x16: 000000004001145c x17: 0000000000000032
>>  x18: 0000000043dffef0 x19: 0000000043c00000
>>  x20: 0000000043dffbc8 x21: 0000000000000000
>>  x22: 00000000000f3d95 x23: 0000000000000002
>>  x24: 0000000000000493 x25: 0000000000092600
>>  x26: 0000000000000001 x27: 0000000000000001
>>  x28: 0000000000000008 x29: 0000000043dffab0
>>
>>  Code: d2800082 9ac32042 d1000443 8a230000 (d5087620)
>>  Resetting CPU ...
>>
>>  resetting ...
>>
>> Fix this by only dma_unmap_single() when DMA mode is used and
>> sdhci_prepare_dma() has been called to map host->start_addr.
>>
>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
>> ---
>> drivers/mmc/sdhci.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c
>> index 4833b5158c79..dc7f0724a7b6 100644
>> --- a/drivers/mmc/sdhci.c
r>> +++ b/drivers/mmc/sdhci.c
>> @@ -177,8 +177,10 @@ static int sdhci_transfer_data(struct sdhci_host *host, struct mmc_data *data)
>> 	} while (!(stat & SDHCI_INT_DATA_END));
>>
>> #if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))
>> -	dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
>> -			 mmc_get_dma_dir(data));
>> +	if (host->flags & USE_DMA) {
>> +		dma_unmap_single(host->start_addr, data->blocks * data->blocksize,
>> +				 mmc_get_dma_dir(data));
>> +	}
> 
> Many platforms have MMC_SDHCI_SDMA enabled, but I not see USE_DMA is set.

  #define USE_DMA  (USE_SDMA | USE_ADMA | USE_ADMA64)

So any platform that enable support for xDMA should match USE_DMA.

For Rockchip there is a quirk enabled for SPL that may disable use of
xDMA even when xDMA is enabled at build time.

	/*
	 * Disable use of DMA and force use of PIO mode in SPL to fix an issue
	 * where loading part of TF-A into SRAM using DMA silently fails.
	 */
	if (IS_ENABLED(CONFIG_XPL_BUILD) &&
	    dev_read_bool(dev, "u-boot,spl-fifo-mode"))
		host->flags &= ~USE_DMA;

I.e. xDMA may be enabled at build time, and at runtime device may use
PIO mode.

> 
> So this change will make dma_unmap_single not being called for the platforms.

No, this will fix an uneven call to dma_map_single and dma_unmap_single.

dma_map_single() is currently called in sdhci_prepare_dma():
	if (host->flags & USE_DMA) { sdhci_prepare_dma() }

dma_unmap_single() is currently called in: 
	#if (CONFIG_IS_ENABLED(MMC_SDHCI_SDMA) || CONFIG_IS_ENABLED(MMC_SDHCI_ADMA))

This change just apply same (host->flags & USE_DMA) check that
indirectly wraps the dma_map_single() call.

> 
> Will this introduce any issue?

Not to my knowledge.

sdhci_send_command() initialize host->start_addr = 0, so the current
unconditional call to dma_unmap_single(host->start_addr, ...) in
sdhci_transfer_data() may cause the above Synchronous Abort due to
start_addr being 0.

Most boards may have DRAM 0x0 mapped, however the Rockchip RK3576 where
this issue was observed does not, its DRAM base start at 0x40000000.
Trying to unmap 0x0 ended up with a Synchronous Abort.

Regards,
Jonas

> 
> Regards,
> Peng
> 
>> #endif
>>
>> 	return 0;
>> -- 
>> 2.48.1
>>