| Message ID | 20190105001439.7098-1-linus.walleij@linaro.org |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
| Series | ata: ahci-imx: Covert to use GPIO descriptor | expand |
Hello! On 05.01.2019 3:14, Linus Walleij wrote: > This converts the i.MX AHCI driver to use a GPIO descriptor > instead of parsing the device tree by itself. > > This driver is quite obviously device tree only, and the > GPIO line is treated as optional, so let's keep it as > optional. > > None of the device trees in the kernek use this GPIO Kernel. :-) > facility today, so it is hard to test. > > Cc: Egor Starkov <egor.starkov@ge.com> > Cc: Richard Zhu <hongxing.zhu@nxp.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/ata/ahci_imx.c | 24 ++++++++---------------- > 1 file changed, 8 insertions(+), 16 deletions(-) > > diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c > index b00799d208f5..1c9d35c2f89c 100644 > --- a/drivers/ata/ahci_imx.c > +++ b/drivers/ata/ahci_imx.c [...] > @@ -1044,19 +1043,12 @@ static int imx8_sata_probe(struct device *dev, struct imx_ahci_priv *imxpriv) > } > > /* Fetch GPIO, then enable the external OSC */ > - imxpriv->clkreq_gpio = of_get_named_gpio(np, "clkreq-gpio", 0); > - if (gpio_is_valid(imxpriv->clkreq_gpio)) { > - ret = devm_gpio_request_one(dev, imxpriv->clkreq_gpio, > - GPIOF_OUT_INIT_LOW, > - "SATA CLKREQ"); > - if (ret == -EBUSY) { > - dev_info(dev, "clkreq had been initialized.\n"); > - } else if (ret) { > - dev_err(dev, "%d unable to get clkreq.\n", ret); > - return ret; > - } > - } else if (imxpriv->clkreq_gpio == -EPROBE_DEFER) { > - return imxpriv->clkreq_gpio; > + imxpriv->clkreq_gpiod = devm_gpiod_get_optional(dev, "clkreq", GPIOD_OUT_LOW); > + if (IS_ERR(imxpriv->clkreq_gpiod)) > + return PTR_ERR(imxpriv->clkreq_gpiod); > + if (imxpriv->clkreq_gpiod) { > + gpiod_set_consumer_name(imxpriv->clkreq_gpiod, "SATA CLKREQ"); > + dev_info(dev, "clkreq had been initialized.\n"); Hm, this message was printed on -EBUSY error, now you print it on success... MBR, Sergei
Hi: > -----Original Message----- > From: Sergei Shtylyov [mailto:sergei.shtylyov@cogentembedded.com] > Sent: 2019年1月5日 16:30 > To: Linus Walleij <linus.walleij@linaro.org>; Jens Axboe <axboe@kernel.dk>; > Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > Cc: linux-ide@vger.kernel.org; Egor Starkov <egor.starkov@ge.com>; Richard > Zhu <hongxing.zhu@nxp.com> > Subject: Re: [PATCH] ata: ahci-imx: Covert to use GPIO descriptor > > Hello! > > On 05.01.2019 3:14, Linus Walleij wrote: > > > This converts the i.MX AHCI driver to use a GPIO descriptor instead of > > parsing the device tree by itself. > > > > This driver is quite obviously device tree only, and the GPIO line is > > treated as optional, so let's keep it as optional. > > > > None of the device trees in the kernek use this GPIO > > Kernel. :-) > > > facility today, so it is hard to test. > > > > Cc: Egor Starkov <egor.starkov@ge.com> > > Cc: Richard Zhu <hongxing.zhu@nxp.com> > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > --- > > drivers/ata/ahci_imx.c | 24 ++++++++---------------- > > 1 file changed, 8 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index > > b00799d208f5..1c9d35c2f89c 100644 > > --- a/drivers/ata/ahci_imx.c > > +++ b/drivers/ata/ahci_imx.c > [...] > > @@ -1044,19 +1043,12 @@ static int imx8_sata_probe(struct device *dev, > struct imx_ahci_priv *imxpriv) > > } > > > > /* Fetch GPIO, then enable the external OSC */ > > - imxpriv->clkreq_gpio = of_get_named_gpio(np, "clkreq-gpio", 0); > > - if (gpio_is_valid(imxpriv->clkreq_gpio)) { > > - ret = devm_gpio_request_one(dev, imxpriv->clkreq_gpio, > > - GPIOF_OUT_INIT_LOW, > > - "SATA CLKREQ"); > > - if (ret == -EBUSY) { > > - dev_info(dev, "clkreq had been initialized.\n"); > > - } else if (ret) { > > - dev_err(dev, "%d unable to get clkreq.\n", ret); > > - return ret; > > - } > > - } else if (imxpriv->clkreq_gpio == -EPROBE_DEFER) { > > - return imxpriv->clkreq_gpio; > > + imxpriv->clkreq_gpiod = devm_gpiod_get_optional(dev, "clkreq", > GPIOD_OUT_LOW); > > + if (IS_ERR(imxpriv->clkreq_gpiod)) > > + return PTR_ERR(imxpriv->clkreq_gpiod); > > + if (imxpriv->clkreq_gpiod) { > > + gpiod_set_consumer_name(imxpriv->clkreq_gpiod, "SATA > CLKREQ"); > > + dev_info(dev, "clkreq had been initialized.\n"); > > Hm, this message was printed on -EBUSY error, now you print it on > success... > [Richard Zhu] Correct. The clkreq gpio maybe initialized(requested/configured) by other module, when the external OSC is shared by ATA and the other module. Thus, it's not a real error when devm_gpio_request_one return -EBUSY in this scenario. That message is used to tell us the clkreq# had been initialized by another module already. Best Regards Richard Zhu > MBR, Sergei
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index b00799d208f5..1c9d35c2f89c 100644 --- a/drivers/ata/ahci_imx.c +++ b/drivers/ata/ahci_imx.c @@ -22,8 +22,8 @@ #include <linux/platform_device.h> #include <linux/regmap.h> #include <linux/ahci_platform.h> +#include <linux/gpio/consumer.h> #include <linux/of_device.h> -#include <linux/of_gpio.h> #include <linux/mfd/syscon.h> #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h> #include <linux/libata.h> @@ -111,7 +111,7 @@ struct imx_ahci_priv { struct clk *phy_pclk0; struct clk *phy_pclk1; void __iomem *phy_base; - int clkreq_gpio; + struct gpio_desc *clkreq_gpiod; struct regmap *gpr; bool no_device; bool first_time; @@ -991,7 +991,6 @@ static struct scsi_host_template ahci_platform_sht = { static int imx8_sata_probe(struct device *dev, struct imx_ahci_priv *imxpriv) { - int ret; struct resource *phy_res; struct platform_device *pdev = imxpriv->ahci_pdev; struct device_node *np = dev->of_node; @@ -1044,19 +1043,12 @@ static int imx8_sata_probe(struct device *dev, struct imx_ahci_priv *imxpriv) } /* Fetch GPIO, then enable the external OSC */ - imxpriv->clkreq_gpio = of_get_named_gpio(np, "clkreq-gpio", 0); - if (gpio_is_valid(imxpriv->clkreq_gpio)) { - ret = devm_gpio_request_one(dev, imxpriv->clkreq_gpio, - GPIOF_OUT_INIT_LOW, - "SATA CLKREQ"); - if (ret == -EBUSY) { - dev_info(dev, "clkreq had been initialized.\n"); - } else if (ret) { - dev_err(dev, "%d unable to get clkreq.\n", ret); - return ret; - } - } else if (imxpriv->clkreq_gpio == -EPROBE_DEFER) { - return imxpriv->clkreq_gpio; + imxpriv->clkreq_gpiod = devm_gpiod_get_optional(dev, "clkreq", GPIOD_OUT_LOW); + if (IS_ERR(imxpriv->clkreq_gpiod)) + return PTR_ERR(imxpriv->clkreq_gpiod); + if (imxpriv->clkreq_gpiod) { + gpiod_set_consumer_name(imxpriv->clkreq_gpiod, "SATA CLKREQ"); + dev_info(dev, "clkreq had been initialized.\n"); } return 0;
This converts the i.MX AHCI driver to use a GPIO descriptor instead of parsing the device tree by itself. This driver is quite obviously device tree only, and the GPIO line is treated as optional, so let's keep it as optional. None of the device trees in the kernek use this GPIO facility today, so it is hard to test. Cc: Egor Starkov <egor.starkov@ge.com> Cc: Richard Zhu <hongxing.zhu@nxp.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/ata/ahci_imx.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)