Re: [PATCH 2/3] mmc: dw_mmc: add dw_mmc-k3 for k3 platform

[zhangfei <zhangfei.gao@linaro.org>]

Dear Jaehoon

Thanks for the reviewing.

On 12/26/2013 12:33 PM, Jaehoon Chung wrote:

>> +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
>
> Only use the IDMAC?

IDMAC has to be set here since we found the controller have some issue 
in non-dma mode.


>> +static unsigned long k3_dwmmc_caps[4] = {
>> +	MMC_CAP_4_BIT_DATA | MMC_CAP_SD_HIGHSPEED,
>> +	MMC_CAP_8_BIT_DATA | MMC_CAP_MMC_HIGHSPEED,
>> +	0,
>> +	0,
>> +};
>
> If supports-highspeed is defined at dt-file,
> then MMC_CAP_SD_HIGHSEEPD and MMC_CAP_MMC_HIGHSPEED should set in dw-mmc.c

Yes, good catch, they can be removed.

>
>> +
>> +static const struct dw_mci_drv_data k3_drv_data = {
>> +	.caps			= k3_dwmmc_caps,
>> +	.set_ios		= dw_mci_k3_set_ios,
>> +	.parse_dt		= dw_mci_k3_parse_dt,
>> +};
>> +
>> +static const struct of_device_id dw_mci_k3_match[] = {
>> +	{ .compatible = "hisilicon,hi4511-dw-mshc", .data = &k3_drv_data, },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, dw_mci_k3_match);
>> +
>> +static int dw_mci_k3_probe(struct platform_device *pdev)
>> +{
>> +	const struct dw_mci_drv_data *drv_data;
>> +	const struct of_device_id *match;
>> +
>> +	match = of_match_node(dw_mci_k3_match, pdev->dev.of_node);
>> +	drv_data = match->data;
>> +
>> +	return dw_mci_pltfm_register(pdev, drv_data);
>> +}
>> +
>> +static int dw_mci_k3_suspend(struct device *dev)
>> +{
>> +	struct dw_mci *host = dev_get_drvdata(dev);
>> +	int ret = 0;
>> +
>> +	ret = dw_mci_suspend(host);
>> +	if (!ret)
>> +		clk_disable_unprepare(host->ciu_clk);
>> +
>> +	return ret;
>> +}
>> +
>> +static int dw_mci_k3_resume(struct device *dev)
>> +{
>> +	struct dw_mci *host = dev_get_drvdata(dev);
>> +	int ret = 0;
>> +
>> +	ret = clk_prepare_enable(host->ciu_clk);
>> +	if (ret) {
>> +		dev_err(host->dev, "failed to enable ciu clock\n");
>> +		return ret;
>> +	}
>> +
>> +	return dw_mci_resume(host);
>> +}
>
> Clock control need in suspend/resume? ciu-clk is used into dw-mmc.c.

The ciu-clk is required here not only for power, but also clk prepare 
have to be triggered in the resume operation as well as init process, 
otherwise sd register access will fail. Some tuning registers accessing 
have been abstracted to clock.

Thanks