| Message ID | 1605680495-37483-1-git-send-email-manish.narani@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci-of-arasan: Add pinctrl support to the driver | expand |
On 18. 11. 20 7:21, Manish Narani wrote: > Driver should be able to handle optional pinctrl setting. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > Signed-off-by: Manish Narani <manish.narani@xilinx.com> > --- > drivers/mmc/host/sdhci-of-arasan.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c > index 829ccef87426..f788cc9d5914 100644 > --- a/drivers/mmc/host/sdhci-of-arasan.c > +++ b/drivers/mmc/host/sdhci-of-arasan.c > @@ -23,6 +23,7 @@ > #include <linux/regmap.h> > #include <linux/of.h> > #include <linux/firmware/xlnx-zynqmp.h> > +#include <linux/pinctrl/consumer.h> > > #include "cqhci.h" > #include "sdhci-pltfm.h" > @@ -135,6 +136,8 @@ struct sdhci_arasan_clk_data { > * @clk_ops: Struct for the Arasan Controller Clock Operations. > * @soc_ctl_base: Pointer to regmap for syscon for soc_ctl registers. > * @soc_ctl_map: Map to get offsets into soc_ctl registers. > + * @pinctrl: Per-device pin control state holder. > + * @pins_default: Pinctrl state for a device. > * @quirks: Arasan deviations from spec. > */ > struct sdhci_arasan_data { > @@ -149,6 +152,8 @@ struct sdhci_arasan_data { > > struct regmap *soc_ctl_base; > const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; > + struct pinctrl *pinctrl; > + struct pinctrl_state *pins_default; > unsigned int quirks; > > /* Controller does not have CD wired and will not function normally without */ > @@ -1619,6 +1624,25 @@ static int sdhci_arasan_probe(struct platform_device *pdev) > goto unreg_clk; > } > > + sdhci_arasan->pinctrl = devm_pinctrl_get(&pdev->dev); > + if (!IS_ERR(sdhci_arasan->pinctrl)) { > + sdhci_arasan->pins_default = > + pinctrl_lookup_state(sdhci_arasan->pinctrl, > + PINCTRL_STATE_DEFAULT); > + if (IS_ERR(sdhci_arasan->pins_default)) { > + dev_err(&pdev->dev, "Missing default pinctrl config\n"); > + ret = PTR_ERR(sdhci_arasan->pins_default); > + goto unreg_clk; > + } > + > + ret = pinctrl_select_state(sdhci_arasan->pinctrl, > + sdhci_arasan->pins_default); > + if (ret) { > + dev_err(&pdev->dev, "could not select default state\n"); > + goto unreg_clk; > + } > + } > + > sdhci_arasan->phy = ERR_PTR(-ENODEV); > if (of_device_is_compatible(pdev->dev.of_node, > "arasan,sdhci-5.1")) { > Ulf: Is there any need to describe in binding doc? I mean all txt based binding have it described. But not quite sure if there is a need to describe it in yaml if only default option is supported. And when this is optional it should be fine also for others SOC. For patch itself. Acked-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal
On Wed, 18 Nov 2020 at 07:22, Manish Narani <manish.narani@xilinx.com> wrote: > > Driver should be able to handle optional pinctrl setting. > > Signed-off-by: Michal Simek <michal.simek@xilinx.com> > Signed-off-by: Manish Narani <manish.narani@xilinx.com> > --- > drivers/mmc/host/sdhci-of-arasan.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c > index 829ccef87426..f788cc9d5914 100644 > --- a/drivers/mmc/host/sdhci-of-arasan.c > +++ b/drivers/mmc/host/sdhci-of-arasan.c > @@ -23,6 +23,7 @@ > #include <linux/regmap.h> > #include <linux/of.h> > #include <linux/firmware/xlnx-zynqmp.h> > +#include <linux/pinctrl/consumer.h> > > #include "cqhci.h" > #include "sdhci-pltfm.h" > @@ -135,6 +136,8 @@ struct sdhci_arasan_clk_data { > * @clk_ops: Struct for the Arasan Controller Clock Operations. > * @soc_ctl_base: Pointer to regmap for syscon for soc_ctl registers. > * @soc_ctl_map: Map to get offsets into soc_ctl registers. > + * @pinctrl: Per-device pin control state holder. > + * @pins_default: Pinctrl state for a device. > * @quirks: Arasan deviations from spec. > */ > struct sdhci_arasan_data { > @@ -149,6 +152,8 @@ struct sdhci_arasan_data { > > struct regmap *soc_ctl_base; > const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; > + struct pinctrl *pinctrl; > + struct pinctrl_state *pins_default; > unsigned int quirks; > > /* Controller does not have CD wired and will not function normally without */ > @@ -1619,6 +1624,25 @@ static int sdhci_arasan_probe(struct platform_device *pdev) > goto unreg_clk; > } > > + sdhci_arasan->pinctrl = devm_pinctrl_get(&pdev->dev); > + if (!IS_ERR(sdhci_arasan->pinctrl)) { > + sdhci_arasan->pins_default = > + pinctrl_lookup_state(sdhci_arasan->pinctrl, > + PINCTRL_STATE_DEFAULT); > + if (IS_ERR(sdhci_arasan->pins_default)) { > + dev_err(&pdev->dev, "Missing default pinctrl config\n"); > + ret = PTR_ERR(sdhci_arasan->pins_default); > + goto unreg_clk; > + } > + > + ret = pinctrl_select_state(sdhci_arasan->pinctrl, > + sdhci_arasan->pins_default); > + if (ret) { > + dev_err(&pdev->dev, "could not select default state\n"); > + goto unreg_clk; > + } > + } Isn't all this already taken care of via pinctrl_bind_pins() called by driver core during probe? [...] Kind regards Uffe
On 18. 11. 20 16:43, Ulf Hansson wrote: > On Wed, 18 Nov 2020 at 07:22, Manish Narani <manish.narani@xilinx.com> wrote: >> >> Driver should be able to handle optional pinctrl setting. >> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> >> Signed-off-by: Manish Narani <manish.narani@xilinx.com> >> --- >> drivers/mmc/host/sdhci-of-arasan.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c >> index 829ccef87426..f788cc9d5914 100644 >> --- a/drivers/mmc/host/sdhci-of-arasan.c >> +++ b/drivers/mmc/host/sdhci-of-arasan.c >> @@ -23,6 +23,7 @@ >> #include <linux/regmap.h> >> #include <linux/of.h> >> #include <linux/firmware/xlnx-zynqmp.h> >> +#include <linux/pinctrl/consumer.h> >> >> #include "cqhci.h" >> #include "sdhci-pltfm.h" >> @@ -135,6 +136,8 @@ struct sdhci_arasan_clk_data { >> * @clk_ops: Struct for the Arasan Controller Clock Operations. >> * @soc_ctl_base: Pointer to regmap for syscon for soc_ctl registers. >> * @soc_ctl_map: Map to get offsets into soc_ctl registers. >> + * @pinctrl: Per-device pin control state holder. >> + * @pins_default: Pinctrl state for a device. >> * @quirks: Arasan deviations from spec. >> */ >> struct sdhci_arasan_data { >> @@ -149,6 +152,8 @@ struct sdhci_arasan_data { >> >> struct regmap *soc_ctl_base; >> const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; >> + struct pinctrl *pinctrl; >> + struct pinctrl_state *pins_default; >> unsigned int quirks; >> >> /* Controller does not have CD wired and will not function normally without */ >> @@ -1619,6 +1624,25 @@ static int sdhci_arasan_probe(struct platform_device *pdev) >> goto unreg_clk; >> } >> >> + sdhci_arasan->pinctrl = devm_pinctrl_get(&pdev->dev); >> + if (!IS_ERR(sdhci_arasan->pinctrl)) { >> + sdhci_arasan->pins_default = >> + pinctrl_lookup_state(sdhci_arasan->pinctrl, >> + PINCTRL_STATE_DEFAULT); >> + if (IS_ERR(sdhci_arasan->pins_default)) { >> + dev_err(&pdev->dev, "Missing default pinctrl config\n"); >> + ret = PTR_ERR(sdhci_arasan->pins_default); >> + goto unreg_clk; >> + } >> + >> + ret = pinctrl_select_state(sdhci_arasan->pinctrl, >> + sdhci_arasan->pins_default); >> + if (ret) { >> + dev_err(&pdev->dev, "could not select default state\n"); >> + goto unreg_clk; >> + } >> + } > > Isn't all this already taken care of via pinctrl_bind_pins() called by > driver core during probe? > Thanks for the hint. Manish: Can you please check it? Thanks, Michal
Hi Uffe/Michal, > -----Original Message----- > From: Michal Simek <michal.simek@xilinx.com> > Sent: Wednesday, November 18, 2020 11:54 PM > To: Ulf Hansson <ulf.hansson@linaro.org>; Manish Narani > <MNARANI@xilinx.com> > Cc: Michal Simek <michals@xilinx.com>; Adrian Hunter > <adrian.hunter@intel.com>; Linux ARM <linux-arm- > kernel@lists.infradead.org>; linux-mmc@vger.kernel.org; Linux Kernel > Mailing List <linux-kernel@vger.kernel.org>; git <git@xilinx.com> > Subject: Re: [PATCH] mmc: sdhci-of-arasan: Add pinctrl support to the driver > > > > On 18. 11. 20 16:43, Ulf Hansson wrote: > > On Wed, 18 Nov 2020 at 07:22, Manish Narani > <manish.narani@xilinx.com> wrote: > >> > >> Driver should be able to handle optional pinctrl setting. > >> > >> Signed-off-by: Michal Simek <michal.simek@xilinx.com> > >> Signed-off-by: Manish Narani <manish.narani@xilinx.com> > >> --- > >> drivers/mmc/host/sdhci-of-arasan.c | 24 ++++++++++++++++++++++++ > >> 1 file changed, 24 insertions(+) > >> > >> diff --git a/drivers/mmc/host/sdhci-of-arasan.c > b/drivers/mmc/host/sdhci-of-arasan.c > >> index 829ccef87426..f788cc9d5914 100644 > >> --- a/drivers/mmc/host/sdhci-of-arasan.c > >> +++ b/drivers/mmc/host/sdhci-of-arasan.c > >> @@ -23,6 +23,7 @@ > >> #include <linux/regmap.h> > >> #include <linux/of.h> > >> #include <linux/firmware/xlnx-zynqmp.h> > >> +#include <linux/pinctrl/consumer.h> > >> > >> #include "cqhci.h" > >> #include "sdhci-pltfm.h" > >> @@ -135,6 +136,8 @@ struct sdhci_arasan_clk_data { > >> * @clk_ops: Struct for the Arasan Controller Clock Operations. > >> * @soc_ctl_base: Pointer to regmap for syscon for soc_ctl registers. > >> * @soc_ctl_map: Map to get offsets into soc_ctl registers. > >> + * @pinctrl: Per-device pin control state holder. > >> + * @pins_default: Pinctrl state for a device. > >> * @quirks: Arasan deviations from spec. > >> */ > >> struct sdhci_arasan_data { > >> @@ -149,6 +152,8 @@ struct sdhci_arasan_data { > >> > >> struct regmap *soc_ctl_base; > >> const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; > >> + struct pinctrl *pinctrl; > >> + struct pinctrl_state *pins_default; > >> unsigned int quirks; > >> > >> /* Controller does not have CD wired and will not function normally > without */ > >> @@ -1619,6 +1624,25 @@ static int sdhci_arasan_probe(struct > platform_device *pdev) > >> goto unreg_clk; > >> } > >> > >> + sdhci_arasan->pinctrl = devm_pinctrl_get(&pdev->dev); > >> + if (!IS_ERR(sdhci_arasan->pinctrl)) { > >> + sdhci_arasan->pins_default = > >> + pinctrl_lookup_state(sdhci_arasan->pinctrl, > >> + PINCTRL_STATE_DEFAULT); > >> + if (IS_ERR(sdhci_arasan->pins_default)) { > >> + dev_err(&pdev->dev, "Missing default pinctrl config\n"); > >> + ret = PTR_ERR(sdhci_arasan->pins_default); > >> + goto unreg_clk; > >> + } > >> + > >> + ret = pinctrl_select_state(sdhci_arasan->pinctrl, > >> + sdhci_arasan->pins_default); > >> + if (ret) { > >> + dev_err(&pdev->dev, "could not select default state\n"); > >> + goto unreg_clk; > >> + } > >> + } > > > > Isn't all this already taken care of via pinctrl_bind_pins() called by > > driver core during probe? > > > > Thanks for the hint. > Manish: Can you please check it? Thanks Uffe. Yes, this is already taken care of via pinctrl_bind_pins() called during probe. This patch is not required to be merged. Thanks, Manish
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c index 829ccef87426..f788cc9d5914 100644 --- a/drivers/mmc/host/sdhci-of-arasan.c +++ b/drivers/mmc/host/sdhci-of-arasan.c @@ -23,6 +23,7 @@ #include <linux/regmap.h> #include <linux/of.h> #include <linux/firmware/xlnx-zynqmp.h> +#include <linux/pinctrl/consumer.h> #include "cqhci.h" #include "sdhci-pltfm.h" @@ -135,6 +136,8 @@ struct sdhci_arasan_clk_data { * @clk_ops: Struct for the Arasan Controller Clock Operations. * @soc_ctl_base: Pointer to regmap for syscon for soc_ctl registers. * @soc_ctl_map: Map to get offsets into soc_ctl registers. + * @pinctrl: Per-device pin control state holder. + * @pins_default: Pinctrl state for a device. * @quirks: Arasan deviations from spec. */ struct sdhci_arasan_data { @@ -149,6 +152,8 @@ struct sdhci_arasan_data { struct regmap *soc_ctl_base; const struct sdhci_arasan_soc_ctl_map *soc_ctl_map; + struct pinctrl *pinctrl; + struct pinctrl_state *pins_default; unsigned int quirks; /* Controller does not have CD wired and will not function normally without */ @@ -1619,6 +1624,25 @@ static int sdhci_arasan_probe(struct platform_device *pdev) goto unreg_clk; } + sdhci_arasan->pinctrl = devm_pinctrl_get(&pdev->dev); + if (!IS_ERR(sdhci_arasan->pinctrl)) { + sdhci_arasan->pins_default = + pinctrl_lookup_state(sdhci_arasan->pinctrl, + PINCTRL_STATE_DEFAULT); + if (IS_ERR(sdhci_arasan->pins_default)) { + dev_err(&pdev->dev, "Missing default pinctrl config\n"); + ret = PTR_ERR(sdhci_arasan->pins_default); + goto unreg_clk; + } + + ret = pinctrl_select_state(sdhci_arasan->pinctrl, + sdhci_arasan->pins_default); + if (ret) { + dev_err(&pdev->dev, "could not select default state\n"); + goto unreg_clk; + } + } + sdhci_arasan->phy = ERR_PTR(-ENODEV); if (of_device_is_compatible(pdev->dev.of_node, "arasan,sdhci-5.1")) {