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.
[RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for ZynqMP Platform
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 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); + ctrl |= SDHCI_CTRL_EXEC_TUNING; + if (host->quirks2 & SDHCI_QUIRK2_TUNING_WORK_AROUND) + ctrl |= SDHCI_CTRL_TUNED_CLK; + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2); + + mdelay(1); + + arasan_zynqmp_dll_reset(host, sdhci_arasan->device_id); + + /* +* As per the Host Controller spec v3.00, tuning command +* generates Buffer Read Ready interrupt, so enable that. +