Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for ZynqMP Platform

[Adrian Hunter]

On 21/02/18 17:00, Manish Narani wrote:
> Hi Adrian,
> 
>> -----Original Message-----
>> From: Manish Narani
>> Sent: Wednesday, February 21, 2018 11:39 AM
>> To: Adrian Hunter <adrian.hun...@intel.com>; michal.si...@xilinx.com;
>> ulf.hans...@linaro.org; linux-arm-ker...@lists.infradead.org; linux-
>> m...@vger.kernel.org; linux-kernel@vger.kernel.org;
>> devicet...@vger.kernel.org; mark.rutl...@arm.com; robh...@kernel.org
>> Cc: Anirudha Sarangi <anir...@xilinx.com>; Srinivas Goud
>> <sg...@xilinx.com>
>> Subject: RE: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for
>> ZynqMP Platform
>>
>> Hi Adrian,
>>
>>
>>> -----Original Message-----
>>> From: Adrian Hunter [mailto:adrian.hun...@intel.com]
>>> Sent: Friday, February 16, 2018 7:37 PM
>>> To: Manish Narani <mnar...@xilinx.com>; michal.si...@xilinx.com;
>>> ulf.hans...@linaro.org; linux-arm-ker...@lists.infradead.org; linux-
>>> m...@vger.kernel.org; linux-kernel@vger.kernel.org;
>>> devicet...@vger.kernel.org; mark.rutl...@arm.com; robh...@kernel.org
>>> Cc: Anirudha Sarangi <anir...@xilinx.com>; Srinivas Goud
>>> <sg...@xilinx.com>; Manish Narani <mnar...@xilinx.com>
>>> Subject: Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support
>>> for ZynqMP Platform
>>>
>>> On 30/01/18 20:14, Manish Narani wrote:
>>>> This patch adds support of SD auto tuning for ZynqMP platform. Auto
>>>> tuning sequence sends tuning block to card when operating in UHS-1
>>>> modes. This resets the DLL and sends CMD19/CMD21 as a part of the
>>>> auto tuning process. Once the auto tuning process gets completed,
>>>> reset the DLL to load the newly obtained SDHC tuned tap value.
>>>
>>> How is this different from:
>>>     1. reset the dll
>>>     2. call sdhci_execute_tuning
>>>     3. reset the dll
>>>
> Below is my take on your above comments:
> - 'Reset the dll' is a platform specific call inside 
> 'arasan_zynqmp_execute_tuning' which is implemented in sdhci-of-arasan.c 
> - 'arasan_zynqmp_execute_tuning' is called from 'sdhci_execute_tuning' as a 
> platform_execute_tuning routine
> - So to keep 'DLL reset' routine called from sdhci-of-arasan.c, I have 
> implemented the execute_tuning in sdhci-of-arasan.c

I meant something like:

        if (of_device_is_compatible(pdev->dev.of_node, "xlnx,zynqmp-8.9a"))
                host->mmc_host_ops.execute_tuning = 
arasan_zynqmp_execute_tuning;


static int arasan_zynqmp_execute_tuning(struct mmc_host *mmc, u32 opcode)
{
        struct sdhci_host *host = mmc_priv(mmc);
        struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
        struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
        int err;

        arasan_zynqmp_dll_reset(host, sdhci_arasan->device_id);

        err = sdhci_execute_tuning(mmc, opcode);
        if (err)
                return err;

        arasan_zynqmp_dll_reset(host, sdhci_arasan->device_id);

        return 0;
}

> 
> Alternative way (Please review):
> - Define a host->quirk2 bit (SDHCI_QUIRK2_DLL_RESET_NEEDED) in 
> sdhci-of-arasan.c indicating DLL reset needed while tuning operation
> - Call 'dll reset' routine before and after __sdhci_execute_tuning() in 
> sdhci.c when a host->quirk2 bit (SDHCI_QUIRK2_DLL_RESET_NEEDED) is set

We should try to avoid quirks.