> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hun...@intel.com]
> Sent: Friday, February 23, 2018 1:04 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>
> Subject: Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for
> ZynqMP Platform
>
> On 23/02/18 09:29, Manish Narani wrote:
> >
> >
> >> -----Original Message-----
> >> From: Adrian Hunter [mailto:adrian.hun...@intel.com]
> >> Sent: Thursday, February 22, 2018 1:50 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>
> >> Subject: Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning
> >> support for ZynqMP Platform
> >>
> >> 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;
> >>
> > This will need the removal of 'const' from 'static const struct
> mmc_host_ops sdhci_ops = {}' in sdhci.c file. Please confirm.
>
> No, it is not const. You are not looking at the right place. i.e.
>
> $ grep mmc_host_ops drivers/mmc/host/sdhci.h
> struct mmc_host_ops mmc_host_ops; /* MMC host ops */
>
[Manish]
This looks fair. I got your point. I will send another patch for the same.
> >
> > Thanks,
> > Manish
> >>
> >> 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.
