| Message ID | 20210730063309.8194-3-rashmi.a@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for eMMC PHY on Intel Thunder Bay | expand |
On Fri, 30 Jul 2021 at 08:33, <rashmi.a@intel.com> wrote: > > From: Rashmi A <rashmi.a@intel.com> > > Intel Thunder Bay SoC eMMC controller is based on Arasan > eMMC 5.1 host controller IP > > Signed-off-by: Rashmi A <rashmi.a@intel.com> > Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> Rashmi, is it safe to apply this separately from the phy driver/dt changes? Then I can queue this via my mmc tree, if you like. Kind regards Uffe > --- > drivers/mmc/host/sdhci-of-arasan.c | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c > index 839965f7c717..6f202fb7a546 100644 > --- a/drivers/mmc/host/sdhci-of-arasan.c > +++ b/drivers/mmc/host/sdhci-of-arasan.c > @@ -185,6 +185,13 @@ static const struct sdhci_arasan_soc_ctl_map intel_lgm_sdxc_soc_ctl_map = { > .hiword_update = false, > }; > > +static const struct sdhci_arasan_soc_ctl_map thunderbay_soc_ctl_map = { > + .baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 }, > + .clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 }, > + .support64b = { .reg = 0x4, .width = 1, .shift = 24 }, > + .hiword_update = false, > +}; > + > static const struct sdhci_arasan_soc_ctl_map intel_keembay_soc_ctl_map = { > .baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 }, > .clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 }, > @@ -430,6 +437,15 @@ static const struct sdhci_pltfm_data sdhci_arasan_cqe_pdata = { > SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN, > }; > > +static const struct sdhci_pltfm_data sdhci_arasan_thunderbay_pdata = { > + .ops = &sdhci_arasan_cqe_ops, > + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, > + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN | > + SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN | > + SDHCI_QUIRK2_STOP_WITH_TC | > + SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400, > +}; > + > #ifdef CONFIG_PM_SLEEP > /** > * sdhci_arasan_suspend - Suspend method for the driver > @@ -1098,6 +1114,12 @@ static struct sdhci_arasan_of_data sdhci_arasan_generic_data = { > .clk_ops = &arasan_clk_ops, > }; > > +static const struct sdhci_arasan_of_data sdhci_arasan_thunderbay_data = { > + .soc_ctl_map = &thunderbay_soc_ctl_map, > + .pdata = &sdhci_arasan_thunderbay_pdata, > + .clk_ops = &arasan_clk_ops, > +}; > + > static const struct sdhci_pltfm_data sdhci_keembay_emmc_pdata = { > .ops = &sdhci_arasan_cqe_ops, > .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | > @@ -1231,6 +1253,10 @@ static const struct of_device_id sdhci_arasan_of_match[] = { > .compatible = "intel,keembay-sdhci-5.1-sdio", > .data = &intel_keembay_sdio_data, > }, > + { > + .compatible = "intel,thunderbay-sdhci-5.1", > + .data = &sdhci_arasan_thunderbay_data, > + }, > /* Generic compatible below here */ > { > .compatible = "arasan,sdhci-8.9a", > @@ -1582,7 +1608,8 @@ static int sdhci_arasan_probe(struct platform_device *pdev) > > if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-emmc") || > of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd") || > - of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sdio")) { > + of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sdio") || > + of_device_is_compatible(np, "intel,thunderbay-sdhci-5.1")) { > sdhci_arasan_update_clockmultiplier(host, 0x0); > sdhci_arasan_update_support64b(host, 0x0); > > -- > 2.17.1 >
> -----Original Message----- > From: Ulf Hansson <ulf.hansson@linaro.org> > Sent: Friday, August 6, 2021 6:36 PM > To: A, Rashmi <rashmi.a@intel.com> > Cc: linux-drivers-review-request@eclists.intel.com; Michal Simek > <michal.simek@xilinx.com>; linux-mmc <linux-mmc@vger.kernel.org>; Linux > ARM <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- > kernel@vger.kernel.org>; Kishon <kishon@ti.com>; Vinod Koul > <vkoul@kernel.org>; Andy Shevchenko > <andriy.shevchenko@linux.intel.com>; linux-phy@lists.infradead.org; Mark > Gross <mgross@linux.intel.com>; kris.pan@linux.intel.com; Zhou, Furong > <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa > <mallikarjunappa.sangannavar@intel.com>; Hunter, Adrian > <adrian.hunter@intel.com>; Vaidya, Mahesh R > <mahesh.r.vaidya@intel.com>; Srikandan, Nandhini > <nandhini.srikandan@intel.com>; Demakkanavar, Kenchappa > <kenchappa.demakkanavar@intel.com> > Subject: Re: [PATCH 2/3] mmc: sdhci-of-arasan: Add intel Thunder Bay SOC > support to the arasan eMMC driver > > On Fri, 30 Jul 2021 at 08:33, <rashmi.a@intel.com> wrote: > > > > From: Rashmi A <rashmi.a@intel.com> > > > > Intel Thunder Bay SoC eMMC controller is based on Arasan eMMC 5.1 host > > controller IP > > > > Signed-off-by: Rashmi A <rashmi.a@intel.com> > > Reviewed-by: Adrian Hunter <adrian.hunter@intel.com> > > Rashmi, is it safe to apply this separately from the phy driver/dt changes? > Then I can queue this via my mmc tree, if you like. No, the phy driver/dt changes must go together with "mmc: sdhci-of-arasan: Add intel Thunder Bay SOC support to the arasan eMMC driver" patch. Regards Rashmi > > Kind regards > Uffe > > > --- > > drivers/mmc/host/sdhci-of-arasan.c | 29 > ++++++++++++++++++++++++++++- > > 1 file changed, 28 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/sdhci-of-arasan.c > > b/drivers/mmc/host/sdhci-of-arasan.c > > index 839965f7c717..6f202fb7a546 100644 > > --- a/drivers/mmc/host/sdhci-of-arasan.c > > +++ b/drivers/mmc/host/sdhci-of-arasan.c > > @@ -185,6 +185,13 @@ static const struct sdhci_arasan_soc_ctl_map > intel_lgm_sdxc_soc_ctl_map = { > > .hiword_update = false, > > }; > > > > +static const struct sdhci_arasan_soc_ctl_map thunderbay_soc_ctl_map = { > > + .baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 }, > > + .clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 }, > > + .support64b = { .reg = 0x4, .width = 1, .shift = 24 }, > > + .hiword_update = false, > > +}; > > + > > static const struct sdhci_arasan_soc_ctl_map intel_keembay_soc_ctl_map > = { > > .baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 }, > > .clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 }, @@ > > -430,6 +437,15 @@ static const struct sdhci_pltfm_data > sdhci_arasan_cqe_pdata = { > > SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN, > > }; > > > > +static const struct sdhci_pltfm_data sdhci_arasan_thunderbay_pdata = { > > + .ops = &sdhci_arasan_cqe_ops, > > + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | > SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, > > + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN | > > + SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN | > > + SDHCI_QUIRK2_STOP_WITH_TC | > > + SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400, > > +}; > > + > > #ifdef CONFIG_PM_SLEEP > > /** > > * sdhci_arasan_suspend - Suspend method for the driver @@ -1098,6 > > +1114,12 @@ static struct sdhci_arasan_of_data > sdhci_arasan_generic_data = { > > .clk_ops = &arasan_clk_ops, > > }; > > > > +static const struct sdhci_arasan_of_data sdhci_arasan_thunderbay_data = > { > > + .soc_ctl_map = &thunderbay_soc_ctl_map, > > + .pdata = &sdhci_arasan_thunderbay_pdata, > > + .clk_ops = &arasan_clk_ops, > > +}; > > + > > static const struct sdhci_pltfm_data sdhci_keembay_emmc_pdata = { > > .ops = &sdhci_arasan_cqe_ops, > > .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | @@ -1231,6 > > +1253,10 @@ static const struct of_device_id sdhci_arasan_of_match[] = { > > .compatible = "intel,keembay-sdhci-5.1-sdio", > > .data = &intel_keembay_sdio_data, > > }, > > + { > > + .compatible = "intel,thunderbay-sdhci-5.1", > > + .data = &sdhci_arasan_thunderbay_data, > > + }, > > /* Generic compatible below here */ > > { > > .compatible = "arasan,sdhci-8.9a", @@ -1582,7 +1608,8 > > @@ static int sdhci_arasan_probe(struct platform_device *pdev) > > > > if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-emmc") || > > of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd") || > > - of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sdio")) { > > + of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sdio") || > > + of_device_is_compatible(np, "intel,thunderbay-sdhci-5.1")) > > + { > > sdhci_arasan_update_clockmultiplier(host, 0x0); > > sdhci_arasan_update_support64b(host, 0x0); > > > > -- > > 2.17.1 > >
On 09-08-21, 05:16, A, Rashmi wrote: > > > > Rashmi, is it safe to apply this separately from the phy driver/dt changes? > > Then I can queue this via my mmc tree, if you like. > No, the phy driver/dt changes must go together with "mmc: sdhci-of-arasan: Add intel Thunder Bay SOC support to the arasan eMMC driver" patch. Why is that? What could happen, emmc driver will complain about phy not found and bail right?
> -----Original Message----- > From: Vinod Koul <vkoul@kernel.org> > Sent: Monday, August 9, 2021 2:12 PM > To: A, Rashmi <rashmi.a@intel.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Michal Simek > <michal.simek@xilinx.com>; linux-mmc <linux-mmc@vger.kernel.org>; Linux > ARM <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- > kernel@vger.kernel.org>; Kishon <kishon@ti.com>; Andy Shevchenko > <andriy.shevchenko@linux.intel.com>; linux-phy@lists.infradead.org; Mark > Gross <mgross@linux.intel.com>; kris.pan@linux.intel.com; Zhou, Furong > <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa > <mallikarjunappa.sangannavar@intel.com>; Hunter, Adrian > <adrian.hunter@intel.com>; Vaidya, Mahesh R > <mahesh.r.vaidya@intel.com>; Srikandan, Nandhini > <nandhini.srikandan@intel.com>; Demakkanavar, Kenchappa > <kenchappa.demakkanavar@intel.com> > Subject: Re: [PATCH 2/3] mmc: sdhci-of-arasan: Add intel Thunder Bay SOC > support to the arasan eMMC driver > > On 09-08-21, 05:16, A, Rashmi wrote: > > > > > > > Rashmi, is it safe to apply this separately from the phy driver/dt changes? > > > Then I can queue this via my mmc tree, if you like. > > No, the phy driver/dt changes must go together with "mmc: sdhci-of- > arasan: Add intel Thunder Bay SOC support to the arasan eMMC driver" > patch. > > Why is that? > > What could happen, emmc driver will complain about phy not found and bail > right? This is right, but ideally both mmc:phy and mmc: sdhci-of-arasan driver code changes should go together > > -- > ~Vinod
On Mon, 9 Aug 2021 at 13:17, A, Rashmi <rashmi.a@intel.com> wrote: > > > > > -----Original Message----- > > From: Vinod Koul <vkoul@kernel.org> > > Sent: Monday, August 9, 2021 2:12 PM > > To: A, Rashmi <rashmi.a@intel.com> > > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Michal Simek > > <michal.simek@xilinx.com>; linux-mmc <linux-mmc@vger.kernel.org>; Linux > > ARM <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- > > kernel@vger.kernel.org>; Kishon <kishon@ti.com>; Andy Shevchenko > > <andriy.shevchenko@linux.intel.com>; linux-phy@lists.infradead.org; Mark > > Gross <mgross@linux.intel.com>; kris.pan@linux.intel.com; Zhou, Furong > > <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa > > <mallikarjunappa.sangannavar@intel.com>; Hunter, Adrian > > <adrian.hunter@intel.com>; Vaidya, Mahesh R > > <mahesh.r.vaidya@intel.com>; Srikandan, Nandhini > > <nandhini.srikandan@intel.com>; Demakkanavar, Kenchappa > > <kenchappa.demakkanavar@intel.com> > > Subject: Re: [PATCH 2/3] mmc: sdhci-of-arasan: Add intel Thunder Bay SOC > > support to the arasan eMMC driver > > > > On 09-08-21, 05:16, A, Rashmi wrote: > > > > > > > > > > Rashmi, is it safe to apply this separately from the phy driver/dt changes? > > > > Then I can queue this via my mmc tree, if you like. > > > No, the phy driver/dt changes must go together with "mmc: sdhci-of- > > arasan: Add intel Thunder Bay SOC support to the arasan eMMC driver" > > patch. > > > > Why is that? > > > > What could happen, emmc driver will complain about phy not found and bail > > right? > This is right, but ideally both mmc:phy and mmc: sdhci-of-arasan driver code changes should go together If patches are well written and can be standalone, we (maintainers) ideally prefer to queue things on a per subsystem basis, because it's just easier. That said, I also noticed that a new compatible string was added, "intel,thunderbay-sdhci-5.1". This needs to be documented in Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml, in a separate patch, preceding $subject patch. Kind regards Uffe
> -----Original Message----- > From: Ulf Hansson <ulf.hansson@linaro.org> > Sent: Monday, August 9, 2021 5:43 PM > To: A, Rashmi <rashmi.a@intel.com> > Cc: Vinod Koul <vkoul@kernel.org>; Michal Simek > <michal.simek@xilinx.com>; linux-mmc <linux-mmc@vger.kernel.org>; Linux > ARM <linux-arm-kernel@lists.infradead.org>; Linux Kernel Mailing List <linux- > kernel@vger.kernel.org>; Kishon <kishon@ti.com>; Andy Shevchenko > <andriy.shevchenko@linux.intel.com>; linux-phy@lists.infradead.org; Mark > Gross <mgross@linux.intel.com>; kris.pan@linux.intel.com; Zhou, Furong > <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa > <mallikarjunappa.sangannavar@intel.com>; Hunter, Adrian > <adrian.hunter@intel.com>; Vaidya, Mahesh R > <mahesh.r.vaidya@intel.com>; Srikandan, Nandhini > <nandhini.srikandan@intel.com>; Demakkanavar, Kenchappa > <kenchappa.demakkanavar@intel.com> > Subject: Re: [PATCH 2/3] mmc: sdhci-of-arasan: Add intel Thunder Bay SOC > support to the arasan eMMC driver > > On Mon, 9 Aug 2021 at 13:17, A, Rashmi <rashmi.a@intel.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Vinod Koul <vkoul@kernel.org> > > > Sent: Monday, August 9, 2021 2:12 PM > > > To: A, Rashmi <rashmi.a@intel.com> > > > Cc: Ulf Hansson <ulf.hansson@linaro.org>; Michal Simek > > > <michal.simek@xilinx.com>; linux-mmc <linux-mmc@vger.kernel.org>; > > > Linux ARM <linux-arm-kernel@lists.infradead.org>; Linux Kernel > > > Mailing List <linux- kernel@vger.kernel.org>; Kishon > > > <kishon@ti.com>; Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com>; linux-phy@lists.infradead.org; > > > Mark Gross <mgross@linux.intel.com>; kris.pan@linux.intel.com; Zhou, > > > Furong <furong.zhou@intel.com>; Sangannavar, Mallikarjunappa > > > <mallikarjunappa.sangannavar@intel.com>; Hunter, Adrian > > > <adrian.hunter@intel.com>; Vaidya, Mahesh R > > > <mahesh.r.vaidya@intel.com>; Srikandan, Nandhini > > > <nandhini.srikandan@intel.com>; Demakkanavar, Kenchappa > > > <kenchappa.demakkanavar@intel.com> > > > Subject: Re: [PATCH 2/3] mmc: sdhci-of-arasan: Add intel Thunder Bay > > > SOC support to the arasan eMMC driver > > > > > > On 09-08-21, 05:16, A, Rashmi wrote: > > > > > > > > > > > > > Rashmi, is it safe to apply this separately from the phy driver/dt > changes? > > > > > Then I can queue this via my mmc tree, if you like. > > > > No, the phy driver/dt changes must go together with "mmc: > > > > sdhci-of- > > > arasan: Add intel Thunder Bay SOC support to the arasan eMMC driver" > > > patch. > > > > > > Why is that? > > > > > > What could happen, emmc driver will complain about phy not found and > > > bail right? > > This is right, but ideally both mmc:phy and mmc: sdhci-of-arasan > > driver code changes should go together > > If patches are well written and can be standalone, we (maintainers) ideally > prefer to queue things on a per subsystem basis, because it's just easier. > > That said, I also noticed that a new compatible string was added, > "intel,thunderbay-sdhci-5.1". This needs to be documented in > Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml, in a separate > patch, preceding $subject patch. > I acknowledge your comments. I will submit a separate patch to document device tree bindings. After that mmc: sdhci-of-arasan driver patch could be queued. Regards Rashmi
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c index 839965f7c717..6f202fb7a546 100644 --- a/drivers/mmc/host/sdhci-of-arasan.c +++ b/drivers/mmc/host/sdhci-of-arasan.c @@ -185,6 +185,13 @@ static const struct sdhci_arasan_soc_ctl_map intel_lgm_sdxc_soc_ctl_map = { .hiword_update = false, }; +static const struct sdhci_arasan_soc_ctl_map thunderbay_soc_ctl_map = { + .baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 }, + .clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 }, + .support64b = { .reg = 0x4, .width = 1, .shift = 24 }, + .hiword_update = false, +}; + static const struct sdhci_arasan_soc_ctl_map intel_keembay_soc_ctl_map = { .baseclkfreq = { .reg = 0x0, .width = 8, .shift = 14 }, .clockmultiplier = { .reg = 0x4, .width = 8, .shift = 14 }, @@ -430,6 +437,15 @@ static const struct sdhci_pltfm_data sdhci_arasan_cqe_pdata = { SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN, }; +static const struct sdhci_pltfm_data sdhci_arasan_thunderbay_pdata = { + .ops = &sdhci_arasan_cqe_ops, + .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC, + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN | + SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN | + SDHCI_QUIRK2_STOP_WITH_TC | + SDHCI_QUIRK2_CAPS_BIT63_FOR_HS400, +}; + #ifdef CONFIG_PM_SLEEP /** * sdhci_arasan_suspend - Suspend method for the driver @@ -1098,6 +1114,12 @@ static struct sdhci_arasan_of_data sdhci_arasan_generic_data = { .clk_ops = &arasan_clk_ops, }; +static const struct sdhci_arasan_of_data sdhci_arasan_thunderbay_data = { + .soc_ctl_map = &thunderbay_soc_ctl_map, + .pdata = &sdhci_arasan_thunderbay_pdata, + .clk_ops = &arasan_clk_ops, +}; + static const struct sdhci_pltfm_data sdhci_keembay_emmc_pdata = { .ops = &sdhci_arasan_cqe_ops, .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | @@ -1231,6 +1253,10 @@ static const struct of_device_id sdhci_arasan_of_match[] = { .compatible = "intel,keembay-sdhci-5.1-sdio", .data = &intel_keembay_sdio_data, }, + { + .compatible = "intel,thunderbay-sdhci-5.1", + .data = &sdhci_arasan_thunderbay_data, + }, /* Generic compatible below here */ { .compatible = "arasan,sdhci-8.9a", @@ -1582,7 +1608,8 @@ static int sdhci_arasan_probe(struct platform_device *pdev) if (of_device_is_compatible(np, "intel,keembay-sdhci-5.1-emmc") || of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sd") || - of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sdio")) { + of_device_is_compatible(np, "intel,keembay-sdhci-5.1-sdio") || + of_device_is_compatible(np, "intel,thunderbay-sdhci-5.1")) { sdhci_arasan_update_clockmultiplier(host, 0x0); sdhci_arasan_update_support64b(host, 0x0);