RE: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for ZynqMP Platform
> -Original Message-
> From: Adrian Hunter [mailto:adrian.hun...@intel.com]
> Sent: Friday, February 23, 2018 1:04 PM
> To: Manish Narani ; 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 ; Srinivas Goud
>
> 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 ; 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 ; Srinivas Goud
> >>
> >> 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 ;
> >>>> 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 ; Srinivas Goud
> >>>>
> >>>> 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 ;
> 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 ; Srinivas Goud
> >>>>> ; Manish Narani
> >>>>> 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.
>
> $
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 ; 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 ; Srinivas Goud
>>
>> 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 ; 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 ; Srinivas Goud
>>>>
>>>> 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 ; 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 ; Srinivas Goud
>>>>> ; Manish Narani
>>>>> 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 */
>
> 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.
RE: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for ZynqMP Platform
> -Original Message-
> From: Adrian Hunter [mailto:adrian.hun...@intel.com]
> Sent: Thursday, February 22, 2018 1:50 PM
> To: Manish Narani ; 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 ; Srinivas Goud
>
> 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 ; 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 ; Srinivas Goud
> >>
> >> 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 ; 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 ; Srinivas Goud
> >>> ; Manish Narani
> >>> 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.
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.
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: Thursday, February 22, 2018 1:50 PM
> To: Manish Narani ; 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 ; Srinivas Goud
>
> 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 ; 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 ; Srinivas Goud
> >>
> >> 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 ; 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 ; Srinivas Goud
> >>> ; Manish Narani
> >>> 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.
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.
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 ; 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 ; Srinivas Goud
>>
>> 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 ; 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 ; Srinivas Goud
>>> ; Manish Narani
>>> 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.
RE: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for ZynqMP Platform
Hi Adrian, > -Original Message- > From: Manish Narani > Sent: Wednesday, February 21, 2018 11:39 AM > To: Adrian Hunter ; 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 ; Srinivas Goud > > 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 ; 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 ; Srinivas Goud > > ; Manish Narani > > 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 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 Thanks, Manish > Thanks for your comments. I am looking into this. I will check and let you > know on the same. > > Thanks, > - Manish > > > > > > > Signed-off-by: Manish Narani > > > --- > > > .../devicetree/bindings/mmc/arasan,sdhci.txt | 1 + > > > drivers/mmc/host/sdhci-of-arasan.c | 219 > > - > > > 2 files changed, 219 insertions(+), 1 deletion(-) > > > > > > diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > > > b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > > > index 60481bf..7d29751 100644 > > > --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > > > +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > > > @@ -14,6 +14,7 @@ Required Properties: > > > - "arasan,sdhci-4.9a": generic Arasan SDHCI 4.9a PHY > > > - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY > > > - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC > > > PHY > > > +- "xlnx,zynqmp-8.9a": Xilinx ZynqMP 8.9a PHY > > >For this device it is strongly suggested to include arasan,soc-ctl- > syscon. > > >- reg: From mmc bindings: Register location and length. > > >- clocks: From clock bindings: Handles to clock inputs. > > > diff --git a/drivers/mmc/host/sdhci-of-arasan.c > > > b/drivers/mmc/host/sdhci-of-arasan.c > > > index 0720ea7..7673db4 100644 > > > --- a/drivers/mmc/host/sdhci-of-arasan.c > > > +++ b/drivers/mmc/host/sdhci-of-arasan.c > > > @@ -24,15 +24,18 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include "sdhci-pltfm.h" > > > #include > > > +#include > > > > > > #define SDHCI_ARASAN_VENDOR_REGISTER 0x78 > > > > > > #define VENDOR_ENHANCED_STROBE BIT(0) > > > > > > #define PHY_CLK_TOO_SLOW_HZ40 > > > +#define MAX_TUNING_LOOP40 > > > > > > /* > > > * On some SoCs the
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 ; 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 ; Srinivas Goud
> ; Manish Narani
> 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
>
Thanks for your comments. I am looking into this. I will check and let you know
on the same.
Thanks,
- Manish
> >
> > Signed-off-by: Manish Narani
> > ---
> > .../devicetree/bindings/mmc/arasan,sdhci.txt | 1 +
> > drivers/mmc/host/sdhci-of-arasan.c | 219
> -
> > 2 files changed, 219 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > index 60481bf..7d29751 100644
> > --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> > @@ -14,6 +14,7 @@ Required Properties:
> > - "arasan,sdhci-4.9a": generic Arasan SDHCI 4.9a PHY
> > - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY
> > - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC
> > PHY
> > +- "xlnx,zynqmp-8.9a": Xilinx ZynqMP 8.9a PHY
> >For this device it is strongly suggested to include
> > arasan,soc-ctl-syscon.
> >- reg: From mmc bindings: Register location and length.
> >- clocks: From clock bindings: Handles to clock inputs.
> > diff --git a/drivers/mmc/host/sdhci-of-arasan.c
> > b/drivers/mmc/host/sdhci-of-arasan.c
> > index 0720ea7..7673db4 100644
> > --- a/drivers/mmc/host/sdhci-of-arasan.c
> > +++ b/drivers/mmc/host/sdhci-of-arasan.c
> > @@ -24,15 +24,18 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include "sdhci-pltfm.h"
> > #include
> > +#include
> >
> > #define SDHCI_ARASAN_VENDOR_REGISTER 0x78
> >
> > #define VENDOR_ENHANCED_STROBE BIT(0)
> >
> > #define PHY_CLK_TOO_SLOW_HZ40
> > +#define MAX_TUNING_LOOP40
> >
> > /*
> > * On some SoCs the syscon area has a feature where the upper 16-bits
> > of @@ -88,6 +91,7 @@ struct sdhci_arasan_data {
> > struct sdhci_host *host;
> > struct clk *clk_ahb;
> > struct phy *phy;
> > + u32 device_id;
> > boolis_phy_on;
> >
> > struct clk_hw sdcardclk_hw;
> > @@ -157,6 +161,213 @@ static int sdhci_arasan_syscon_write(struct
> sdhci_host *host,
> > return ret;
> > }
> >
> > +/**
> > + * arasan_zynqmp_dll_reset - Issue the DLL reset.
> > + * @deviceid: Unique Id of device
> > + */
> > +void zynqmp_dll_reset(u8 deviceid)
> > +{
> > + const struct zynqmp_eemi_ops *eemi_ops = get_eemi_ops();
> > +
> > + if (!eemi_ops || !eemi_ops->ioctl)
> > + return;
> > +
> > + /* Issue DLL Reset */
> > + if (deviceid == 0)
> > + eemi_ops->ioctl(NODE_SD_0, IOCTL_SD_DLL_RESET,
> > + PM_DLL_RESET_PULSE, 0, NULL);
> > + else
> > + eemi_ops->ioctl(NODE_SD_1, IOCTL_SD_DLL_RESET,
> > + PM_DLL_RESET_PULSE, 0, NULL); }
> > +
> > +static void arasan_zynqmp_dll_reset(struct sdhci_host *host, u8
> > +deviceid) {
> > + u16 clk;
> > + unsigned long timeout;
> > +
> > + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> > + clk &= ~(SDHCI_CLOCK_CARD_EN | SDHCI_CLOCK_INT_EN);
> > + sdhci_writew(hos
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
>
> Signed-off-by: Manish Narani
> ---
> .../devicetree/bindings/mmc/arasan,sdhci.txt | 1 +
> drivers/mmc/host/sdhci-of-arasan.c | 219
> -
> 2 files changed, 219 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 60481bf..7d29751 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -14,6 +14,7 @@ Required Properties:
> - "arasan,sdhci-4.9a": generic Arasan SDHCI 4.9a PHY
> - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY
> - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY
> +- "xlnx,zynqmp-8.9a": Xilinx ZynqMP 8.9a PHY
>For this device it is strongly suggested to include
> arasan,soc-ctl-syscon.
>- reg: From mmc bindings: Register location and length.
>- clocks: From clock bindings: Handles to clock inputs.
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c
> b/drivers/mmc/host/sdhci-of-arasan.c
> index 0720ea7..7673db4 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -24,15 +24,18 @@
> #include
> #include
> #include
> +#include
> #include
> #include "sdhci-pltfm.h"
> #include
> +#include
>
> #define SDHCI_ARASAN_VENDOR_REGISTER 0x78
>
> #define VENDOR_ENHANCED_STROBE BIT(0)
>
> #define PHY_CLK_TOO_SLOW_HZ40
> +#define MAX_TUNING_LOOP40
>
> /*
> * On some SoCs the syscon area has a feature where the upper 16-bits of
> @@ -88,6 +91,7 @@ struct sdhci_arasan_data {
> struct sdhci_host *host;
> struct clk *clk_ahb;
> struct phy *phy;
> + u32 device_id;
> boolis_phy_on;
>
> struct clk_hw sdcardclk_hw;
> @@ -157,6 +161,213 @@ static int sdhci_arasan_syscon_write(struct sdhci_host
> *host,
> return ret;
> }
>
> +/**
> + * arasan_zynqmp_dll_reset - Issue the DLL reset.
> + * @deviceid: Unique Id of device
> + */
> +void zynqmp_dll_reset(u8 deviceid)
> +{
> + const struct zynqmp_eemi_ops *eemi_ops = get_eemi_ops();
> +
> + if (!eemi_ops || !eemi_ops->ioctl)
> + return;
> +
> + /* Issue DLL Reset */
> + if (deviceid == 0)
> + eemi_ops->ioctl(NODE_SD_0, IOCTL_SD_DLL_RESET,
> + PM_DLL_RESET_PULSE, 0, NULL);
> + else
> + eemi_ops->ioctl(NODE_SD_1, IOCTL_SD_DLL_RESET,
> + PM_DLL_RESET_PULSE, 0, NULL);
> +}
> +
> +static void arasan_zynqmp_dll_reset(struct sdhci_host *host, u8 deviceid)
> +{
> + u16 clk;
> + unsigned long timeout;
> +
> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> + clk &= ~(SDHCI_CLOCK_CARD_EN | SDHCI_CLOCK_INT_EN);
> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> + /* Issue DLL Reset */
> + zynqmp_dll_reset(deviceid);
> +
> + clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> + clk |= SDHCI_CLOCK_INT_EN;
> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> + /* Wait max 20 ms */
> + timeout = 20;
> + while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> + & SDHCI_CLOCK_INT_STABLE)) {
> + if (timeout == 0) {
> + dev_err(mmc_dev(host->mmc),
> + ": Internal clock never stabilised.\n");
> + return;
> + }
> + timeout--;
> + mdelay(1);
> + }
> +
> + clk |= SDHCI_CLOCK_CARD_EN;
> + sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +}
> +
> +static int arasan_zynqmp_execute_tuning(struct sdhci_host *host, u32 opcode)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
> + struct mmc_host *mmc = host->mmc;
> + u16 ctrl;
> + int tuning_loop_counter = MAX_TUNING_LOOP;
> + int err = 0;
> + unsigned long flags;
> + unsigned int tuning_count = 0;
> +
> + spin_lock_irqsave(&host->lock, flags);
> +
> + if (host->tuning_mode == SDHCI_TUNING_MODE_1)
> + tuning_count = host->tuning_count;
> +
> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> +
Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for ZynqMP Platform
On Tue, Jan 30, 2018 at 11:44:33PM +0530, 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. > > Signed-off-by: Manish Narani > --- > .../devicetree/bindings/mmc/arasan,sdhci.txt | 1 + > drivers/mmc/host/sdhci-of-arasan.c | 219 > - > 2 files changed, 219 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > index 60481bf..7d29751 100644 > --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt > @@ -14,6 +14,7 @@ Required Properties: > - "arasan,sdhci-4.9a": generic Arasan SDHCI 4.9a PHY > - "arasan,sdhci-5.1": generic Arasan SDHCI 5.1 PHY > - "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1": rk3399 eMMC PHY > +- "xlnx,zynqmp-8.9a": Xilinx ZynqMP 8.9a PHY This should be a separate patch. >For this device it is strongly suggested to include > arasan,soc-ctl-syscon. >- reg: From mmc bindings: Register location and length. >- clocks: From clock bindings: Handles to clock inputs.
