Re: [RFC PATCH v4 1/9] mmc: dw_mmc: Add external dma interface support

[Shawn Lin <shawn.lin@rock-chips.com>]

在 2015/8/6 15:08, Krzysztof Kozlowski 写道:
> On 06.08.2015 15:44, Shawn Lin wrote:
>> DesignWare MMC Controller can supports two types of DMA
>> mode: external dma and internal dma. We get a RK312x platform
>> integrated dw_mmc and ARM pl330 dma controller. This patch add
>> edmac ops to support these platforms. I've tested it on RK312x
>> platform with edmac mode and RK3288 platform with idmac mode.
>>
>> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>>
>> ---
>>
>> Changes in v4:
>> - remove "host->trans_mode" and use "host->use_dma" to indicate
>>    transfer mode.
>> - remove all bt-bindings' changes since we don't need new properities.
>> - check transfer mode at runtime by reading HCON reg
>> - spilt defconfig changes for each sub-architecture
>> - fix the title of cover letter
> How did you fixed the title? It is still empty :)
> 	Subject: [RFC PATCH v4 0/9]
   I mentioned that in ChangeLog-v4 but unfortunately I forgot it.
   Thanks, Krzysztof.  I will be more careful and add it for next version.

>
>> - reuse some code for reducing code size
>>
>> Changes in v3:
>> - choose transfer mode at runtime
>> - remove all CONFIG_MMC_DW_IDMAC config option
>> - add supports-idmac property for some platforms
>>
>> Changes in v2:
>> - Fix typo of dev_info msg
>> - remove unused dmach from declaration of dw_mci_dma_slave
>>
>>   drivers/mmc/host/Kconfig        |  11 +-
>>   drivers/mmc/host/dw_mmc-pltfm.c |   2 +
>>   drivers/mmc/host/dw_mmc.c       | 258 ++++++++++++++++++++++++++++++++--------
>>   include/linux/mmc/dw_mmc.h      |  27 ++++-
>>   4 files changed, 232 insertions(+), 66 deletions(-)
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index 6a0f9c7..a86c0eb 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -607,15 +607,7 @@ config MMC_DW
>>   	help
>>   	  This selects support for the Synopsys DesignWare Mobile Storage IP
>>   	  block, this provides host support for SD and MMC interfaces, in both
>> -	  PIO and external DMA modes.
>> -
>> -config MMC_DW_IDMAC
>> -	bool "Internal DMAC interface"
>> -	depends on MMC_DW
>> -	help
>> -	  This selects support for the internal DMAC block within the Synopsys
>> -	  Designware Mobile Storage IP block. This disables the external DMA
>> -	  interface.
>> +	  PIO, internal DMA mode and external DMA modes.
>>   
>>   config MMC_DW_PLTFM
>>   	tristate "Synopsys Designware MCI Support as platform device"
>> @@ -644,7 +636,6 @@ config MMC_DW_K3
>>   	tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
>>   	depends on MMC_DW
>>   	select MMC_DW_PLTFM
>> -	select MMC_DW_IDMAC
>>   	help
>>   	  This selects support for Hisilicon K3 SoC specific extensions to the
>>   	  Synopsys DesignWare Memory Card Interface driver. Select this option
>> diff --git a/drivers/mmc/host/dw_mmc-pltfm.c b/drivers/mmc/host/dw_mmc-pltfm.c
>> index ec6dbcd..7e1d13b 100644
>> --- a/drivers/mmc/host/dw_mmc-pltfm.c
>> +++ b/drivers/mmc/host/dw_mmc-pltfm.c
>> @@ -59,6 +59,8 @@ int dw_mci_pltfm_register(struct platform_device *pdev,
>>   	host->pdata = pdev->dev.platform_data;
>>   
>>   	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	/* Get registers' physical base address */
>> +	host->phy_regs = (void *)(regs->start);
>>   	host->regs = devm_ioremap_resource(&pdev->dev, regs);
>>   	if (IS_ERR(host->regs))
>>   		return PTR_ERR(host->regs);
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 40e9d8e..5d6cdff 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -56,7 +56,7 @@
>>   #define DW_MCI_FREQ_MAX	200000000	/* unit: HZ */
>>   #define DW_MCI_FREQ_MIN	400000		/* unit: HZ */
>>   
>> -#ifdef CONFIG_MMC_DW_IDMAC
>> +
>>   #define IDMAC_INT_CLR		(SDMMC_IDMAC_INT_AI | SDMMC_IDMAC_INT_NI | \
>>   				 SDMMC_IDMAC_INT_CES | SDMMC_IDMAC_INT_DU | \
>>   				 SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
>> @@ -99,7 +99,6 @@ struct idmac_desc {
>>   
>>   	__le32		des3;	/* buffer 2 physical address */
>>   };
>> -#endif /* CONFIG_MMC_DW_IDMAC */
>>   
>>   static bool dw_mci_reset(struct dw_mci *host);
>>   static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>> @@ -403,7 +402,6 @@ static int dw_mci_get_dma_dir(struct mmc_data *data)
>>   		return DMA_FROM_DEVICE;
>>   }
>>   
>> -#ifdef CONFIG_MMC_DW_IDMAC
>>   static void dw_mci_dma_cleanup(struct dw_mci *host)
>>   {
>>   	struct mmc_data *data = host->data;
>> @@ -441,12 +439,21 @@ static void dw_mci_idmac_stop_dma(struct dw_mci *host)
>>   	mci_writel(host, BMOD, temp);
>>   }
>>   
>> -static void dw_mci_idmac_complete_dma(struct dw_mci *host)
>> +static void dw_mci_dmac_complete_dma(void *arg)
>>   {
>> +	struct dw_mci *host = arg;
> Why changing the argument to void*?

This function will be used as callback hook of dmaengine, and the 
prototype is
"typedef void (*dma_async_tx_callback)(void *dma_async_param);".
w/o this change, we meet a warning  for incompatible pointer case.

>
> Best regards,
> Krzysztof
>
>>   	struct mmc_data *data = host->data;
>>   
>>   	dev_vdbg(host->dev, "DMA complete\n");
>>   
>> +	if (host->use_dma == TRANS_MODE_EDMAC)
>> +		if (data && (data->flags & MMC_DATA_READ))
>> +			/* Invalidate cache after read */
>> +			dma_sync_sg_for_cpu(mmc_dev(host->cur_slot->mmc),
>> +					    data->sg,
>> +					    data->sg_len,
>> +					    DMA_FROM_DEVICE);
>> +
>>   	host->dma_ops->cleanup(host);
>
>
>
>


-- 
Shawn Lin