Re: [PATCH] mmc: sdhci: disable ADMA when PAGE SIZE is 64K

[Adrian Hunter <adrian.hunter@intel.com>]

On 29/10/2021 14:40, Bough Chen wrote:
>> -----Original Message-----
>> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
>> Sent: 2021年10月27日 13:50
>> To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org
>> Cc: linux-mmc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
>> Subject: Re: [PATCH] mmc: sdhci: disable ADMA when PAGE SIZE is 64K
>>
>> On 27/10/2021 05:47, haibo.chen@nxp.com wrote:
>>> From: Haibo Chen <haibo.chen@nxp.com>
>>>
>>> For ADMA with quirks SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC, it
>> limit
>>> the max segment size as 64K-1. Currently, linux kernel support
>>> 4K/16K/64K PAGE_SIZE. If choose 64K PAGE_SIZE, sdhci in ADMA mode with
>>> the upper quirks will meet issue.
>>>
>>> Though mmc driver give block layer the max segment size through
>>> blk_queue_max_segment_size(), but in this function, it will compare
>>> the argument with PAGE_SIZE, and will choose PAGE_SIZE if the argument
>>> is smaller than PAGE_SIZE. When PAGE_SIZE is 64K, finally the mmc
>>> request queue will get 64K as the max segment size. Once the sg data
>>> length=64K, ADMA will meet issue. For this case, need to disable ADMA
>>> mode.
>>
>> Sorry, I didn't look closely enough at this.
>>
>> For the 64K PAGE_SIZE case, did you try blk_queue_virt_boundary() with a 32K
>> mask instead of blk_queue_max_segment_size() ?
>> e.g. a hack for testing purposes like this:
> 
> I test on my board, this do not work, I still can see sg data_length = 65536.

What about tweaking the logic to write ADMA descriptors?
Something like below for instance?



diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 269c86569402..07c6da1f2f0f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -771,7 +771,19 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
 			len -= offset;
 		}
 
-		BUG_ON(len > 65536);
+		/*
+		 * The block layer forces a minimum segment size of PAGE_SIZE,
+		 * so 'len' can be too big here if PAGE_SIZE >= 64KiB. Write
+		 * multiple descriptors, noting that the ADMA table is sized
+		 * for 4KiB chunks anyway, so it will be big enough.
+		 */
+		while (len > host->max_adma) {
+			int n = 32 * 1024; /* 32KiB*/
+
+			__sdhci_adma_write_desc(host, &desc, addr, n, ADMA2_TRAN_VALID);
+			addr += n;
+			len -= n;
+		}
 
 		/* tran, valid */
 		if (len)
@@ -3968,6 +3980,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
 	 * descriptor for each segment, plus 1 for a nop end descriptor.
 	 */
 	host->adma_table_cnt = SDHCI_MAX_SEGS * 2 + 1;
+	host->max_adma = 65536;
 
 	host->max_timeout_count = 0xE;
 
@@ -4633,10 +4646,12 @@ int sdhci_setup_host(struct sdhci_host *host)
 	 * be larger than 64 KiB though.
 	 */
 	if (host->flags & SDHCI_USE_ADMA) {
-		if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC)
+		if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) {
+			host->max_adma = 65532; /* 32-bit alignment */
 			mmc->max_seg_size = 65535;
-		else
+		} else {
 			mmc->max_seg_size = 65536;
+		}
 	} else {
 		mmc->max_seg_size = mmc->max_req_size;
 	}
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index bb883553d3b4..d7929d725730 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -340,7 +340,8 @@ struct sdhci_adma2_64_desc {
 
 /*
  * Maximum segments assuming a 512KiB maximum requisition size and a minimum
- * 4KiB page size.
+ * 4KiB page size. Note this also allows enough for multiple descriptors in
+ * case of PAGE_SIZE >= 64KiB.
  */
 #define SDHCI_MAX_SEGS		128
 
@@ -543,6 +544,7 @@ struct sdhci_host {
 	unsigned int blocks;	/* remaining PIO blocks */
 
 	int sg_count;		/* Mapped sg entries */
+	int max_adma;		/* Max. length in ADMA descriptor */
 
 	void *adma_table;	/* ADMA descriptor table */
 	void *align_buffer;	/* Bounce buffer */
-- 
2.25.1




> 
> Best Regards
> Haibo Chen
>>
>> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index
>> b15c034b42fb..4c3a9d999500 100644
>> --- a/drivers/mmc/core/queue.c
>> +++ b/drivers/mmc/core/queue.c
>> @@ -376,8 +376,7 @@ static void mmc_setup_queue(struct mmc_queue *mq,
>> struct mmc_card *card)
>>  	 * both blk_queue_max_segment_size().
>>  	 */
>>  	if (!host->can_dma_map_merge)
>> -		blk_queue_max_segment_size(mq->queue,
>> -			round_down(host->max_seg_size, block_size));
>> +		blk_queue_virt_boundary(mq->queue, 0x7fff);
>>
>>  	dma_set_max_seg_size(mmc_dev(host),
>> queue_max_segment_size(mq->queue));
>>
>>
>>
>>
>>>
>>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index
>>> dbe87995596c..47e0c0156b94 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -4187,6 +4187,12 @@ int sdhci_setup_host(struct sdhci_host *host)
>>>  		host->flags &= ~SDHCI_USE_ADMA;
>>>  	}
>>>
>>> +	if ((host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) &&
>>> +		PAGE_SIZE >= 65536 && (host->flags & SDHCI_USE_ADMA)) {
>>> +		DBG("Disabling ADMA as it don't support >= 64K PAGE_SIZE\n");
>>> +		host->flags &= ~SDHCI_USE_ADMA;
>>> +	}
>>> +
>>>  	if (sdhci_can_64bit_dma(host))
>>>  		host->flags |= SDHCI_USE_64_BIT_DMA;
>>>
>>>
>