Re: [PATCH v2 1/2] mmc: dw_mmc: implement option for configuring DMA threshold

[Kaustabh Chakraborty]

On 2026-04-14 16:50 +08:00, Shawn Lin wrote:
> 在 2026/04/14 星期二 16:36, Kaustabh Chakraborty 写道:
>> Some controllers, such as certain Exynos SDIO ones, are unable to
>> perform DMA transfers of small amount of bytes properly. Following the
>> device tree schema, implement the property to define the DMA transfer
>> threshold (from a hard coded value of 16 bytes) so that lesser number of
>> bytes can be transferred safely skipping DMA in such controllers. The
>> value of 16 bytes stays as the default for controllers which do not
>> define it. This value can be overridden by implementation-specific init
>> sequences.
>> 
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@xxxxxxxxxxx>
>> ---
>>   drivers/mmc/host/dw_mmc.c | 5 +++--
>>   drivers/mmc/host/dw_mmc.h | 2 ++
>>   2 files changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 20193ee7b73eb..9dd9fed4ccf49 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -40,7 +40,6 @@
>>   				 SDMMC_INT_RESP_ERR | SDMMC_INT_HLE)
>>   #define DW_MCI_ERROR_FLAGS	(DW_MCI_DATA_ERROR_FLAGS | \
>>   				 DW_MCI_CMD_ERROR_FLAGS)
>> -#define DW_MCI_DMA_THRESHOLD	16
>>   
>>   #define DW_MCI_FREQ_MAX	200000000	/* unit: HZ */
>>   #define DW_MCI_FREQ_MIN	100000		/* unit: HZ */
>> @@ -821,7 +820,7 @@ static int dw_mci_pre_dma_transfer(struct dw_mci *host,
>>   	 * non-word-aligned buffers or lengths. Also, we don't bother
>>   	 * with all the DMA setup overhead for short transfers.
>>   	 */
>> -	if (data->blocks * data->blksz < DW_MCI_DMA_THRESHOLD)
>> +	if (data->blocks * data->blksz < host->dma_threshold)
>>   		return -EINVAL;
>>   
>>   	if (data->blksz & 3)
>> @@ -3245,6 +3244,8 @@ int dw_mci_probe(struct dw_mci *host)
>>   		goto err_clk_ciu;
>>   	}
>>   
>> +	host->dma_threshold = 16;
>
> I'd prefer to set it in dw_mci_alloc_host() instead of picking up
> a random place to put it, for better code management.

Okay, that function is in -next I see.

>
>> +
>>   	if (host->rstc) {
>>   		reset_control_assert(host->rstc);
>>   		usleep_range(10, 50);
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 42e58be74ce09..fc7601fba849f 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -164,6 +164,8 @@ struct dw_mci {
>>   	void __iomem		*fifo_reg;
>>   	u32			data_addr_override;
>>   	bool			wm_aligned;
>> +	/* Configurable data byte threshold value for DMA transfer. */
>
> No here, there is a long section of comment before struct dw_mci{ } that
> describes each member of it, please add it there.

Ah, you mean the documenting comment. Shouldn't have missed in either
way.

>
>> +	u32			dma_threshold;
>>   
>>   	struct scatterlist	*sg;
>>   	struct sg_mapping_iter	sg_miter;
>>