[v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()

Dmitry Torokhov posted 1 patch 3 years, 4 months ago
drivers/pci/controller/pci-aardvark.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
[v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Dmitry Torokhov 3 years, 4 months ago
Switch the driver to the generic version of gpiod API (and away from
OF-specific variant), so that we can stop exporting
devm_gpiod_get_from_of_node().

Acked-by: Pali Rohár <pali@kernel.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

v2:
 - collected reviewed-by/acked-by tags
 - updated commit description to remove incorrect assumption of why
   devm_gpiod_get_from_of_node() was used in the first place

This is the last user of devm_gpiod_get_from_of_node() in the mainline
(next), it would be great to have it in so that we can remove the API in
the next release cycle.

Thanks!


 drivers/pci/controller/pci-aardvark.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index ba36bbc5897d..5ecfac23c9fc 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -1859,20 +1859,19 @@ static int advk_pcie_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	pcie->reset_gpio = devm_gpiod_get_from_of_node(dev, dev->of_node,
-						       "reset-gpios", 0,
-						       GPIOD_OUT_LOW,
-						       "pcie1-reset");
+	pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
 	ret = PTR_ERR_OR_ZERO(pcie->reset_gpio);
 	if (ret) {
-		if (ret == -ENOENT) {
-			pcie->reset_gpio = NULL;
-		} else {
-			if (ret != -EPROBE_DEFER)
-				dev_err(dev, "Failed to get reset-gpio: %i\n",
-					ret);
-			return ret;
-		}
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get reset-gpio: %i\n",
+				ret);
+		return ret;
+	}
+
+	ret = gpiod_set_consumer_name(pcie->reset_gpio, "pcie1-reset");
+	if (ret) {
+		dev_err(dev, "Failed to set reset gpio name: %d\n", ret);
+		return ret;
 	}
 
 	ret = of_pci_get_max_link_speed(dev->of_node);
-- 
2.38.1.431.g37b22c650d-goog


-- 
Dmitry
Re: [v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Bjorn Helgaas 3 years, 4 months ago
On Mon, Nov 14, 2022 at 10:42:25AM -0800, Dmitry Torokhov wrote:
> Switch the driver to the generic version of gpiod API (and away from
> OF-specific variant), so that we can stop exporting
> devm_gpiod_get_from_of_node().
> 
> Acked-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

This is unrelated to other pending aardvark changes and will help
unblock the API removal, so I applied this to pci/ctrl/aardvark for
v6.2, thanks!

> ---
> 
> v2:
>  - collected reviewed-by/acked-by tags
>  - updated commit description to remove incorrect assumption of why
>    devm_gpiod_get_from_of_node() was used in the first place
> 
> This is the last user of devm_gpiod_get_from_of_node() in the mainline
> (next), it would be great to have it in so that we can remove the API in
> the next release cycle.
> 
> Thanks!
> 
> 
>  drivers/pci/controller/pci-aardvark.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index ba36bbc5897d..5ecfac23c9fc 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -1859,20 +1859,19 @@ static int advk_pcie_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	pcie->reset_gpio = devm_gpiod_get_from_of_node(dev, dev->of_node,
> -						       "reset-gpios", 0,
> -						       GPIOD_OUT_LOW,
> -						       "pcie1-reset");
> +	pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>  	ret = PTR_ERR_OR_ZERO(pcie->reset_gpio);
>  	if (ret) {
> -		if (ret == -ENOENT) {
> -			pcie->reset_gpio = NULL;
> -		} else {
> -			if (ret != -EPROBE_DEFER)
> -				dev_err(dev, "Failed to get reset-gpio: %i\n",
> -					ret);
> -			return ret;
> -		}
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "Failed to get reset-gpio: %i\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	ret = gpiod_set_consumer_name(pcie->reset_gpio, "pcie1-reset");
> +	if (ret) {
> +		dev_err(dev, "Failed to set reset gpio name: %d\n", ret);
> +		return ret;
>  	}
>  
>  	ret = of_pci_get_max_link_speed(dev->of_node);
> -- 
> 2.38.1.431.g37b22c650d-goog
> 
> 
> -- 
> Dmitry
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Pali Rohár 3 years, 3 months ago
On Wednesday 07 December 2022 08:33:51 Bjorn Helgaas wrote:
> On Mon, Nov 14, 2022 at 10:42:25AM -0800, Dmitry Torokhov wrote:
> > Switch the driver to the generic version of gpiod API (and away from
> > OF-specific variant), so that we can stop exporting
> > devm_gpiod_get_from_of_node().
> > 
> > Acked-by: Pali Rohár <pali@kernel.org>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> This is unrelated to other pending aardvark changes and will help
> unblock the API removal, so I applied this to pci/ctrl/aardvark for
> v6.2, thanks!

I'm disappointed that such unimportant change is prioritized and taken
before any other important changes which are fixing real issue and
waiting for applying about half of year.

> > ---
> > 
> > v2:
> >  - collected reviewed-by/acked-by tags
> >  - updated commit description to remove incorrect assumption of why
> >    devm_gpiod_get_from_of_node() was used in the first place
> > 
> > This is the last user of devm_gpiod_get_from_of_node() in the mainline
> > (next), it would be great to have it in so that we can remove the API in
> > the next release cycle.
> > 
> > Thanks!
> > 
> > 
> >  drivers/pci/controller/pci-aardvark.c | 23 +++++++++++------------
> >  1 file changed, 11 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > index ba36bbc5897d..5ecfac23c9fc 100644
> > --- a/drivers/pci/controller/pci-aardvark.c
> > +++ b/drivers/pci/controller/pci-aardvark.c
> > @@ -1859,20 +1859,19 @@ static int advk_pcie_probe(struct platform_device *pdev)
> >  		return ret;
> >  	}
> >  
> > -	pcie->reset_gpio = devm_gpiod_get_from_of_node(dev, dev->of_node,
> > -						       "reset-gpios", 0,
> > -						       GPIOD_OUT_LOW,
> > -						       "pcie1-reset");
> > +	pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> >  	ret = PTR_ERR_OR_ZERO(pcie->reset_gpio);
> >  	if (ret) {
> > -		if (ret == -ENOENT) {
> > -			pcie->reset_gpio = NULL;
> > -		} else {
> > -			if (ret != -EPROBE_DEFER)
> > -				dev_err(dev, "Failed to get reset-gpio: %i\n",
> > -					ret);
> > -			return ret;
> > -		}
> > +		if (ret != -EPROBE_DEFER)
> > +			dev_err(dev, "Failed to get reset-gpio: %i\n",
> > +				ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = gpiod_set_consumer_name(pcie->reset_gpio, "pcie1-reset");
> > +	if (ret) {
> > +		dev_err(dev, "Failed to set reset gpio name: %d\n", ret);
> > +		return ret;
> >  	}
> >  
> >  	ret = of_pci_get_max_link_speed(dev->of_node);
> > -- 
> > 2.38.1.431.g37b22c650d-goog
> > 
> > 
> > -- 
> > Dmitry
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Dmitry Torokhov 3 years, 4 months ago
On Wed, Dec 07, 2022 at 08:33:51AM -0600, Bjorn Helgaas wrote:
> On Mon, Nov 14, 2022 at 10:42:25AM -0800, Dmitry Torokhov wrote:
> > Switch the driver to the generic version of gpiod API (and away from
> > OF-specific variant), so that we can stop exporting
> > devm_gpiod_get_from_of_node().
> > 
> > Acked-by: Pali Rohár <pali@kernel.org>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> 
> This is unrelated to other pending aardvark changes and will help
> unblock the API removal, so I applied this to pci/ctrl/aardvark for
> v6.2, thanks!

Thank you Bjorn!

I wonder if you could also consider picking up the mvebu patch:

	https://lore.kernel.org/all/Y3KbhIi4ZsSO7+Cl@google.com/

pci-mvebu.c is the very last user of of_get_named_gpio_flags() in the
next tree, which we also want to stop exporting.

Thanks.

-- 
Dmitry
Re: [v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Bjorn Helgaas 3 years, 4 months ago
On Wed, Dec 07, 2022 at 10:55:06AM -0800, Dmitry Torokhov wrote:
> ...
> I wonder if you could also consider picking up the mvebu patch:
> 
> 	https://lore.kernel.org/all/Y3KbhIi4ZsSO7+Cl@google.com/
> 
> pci-mvebu.c is the very last user of of_get_named_gpio_flags() in the
> next tree, which we also want to stop exporting.

Sure, but the 0-day bot found a couple issues, which I haven't
bothered to look into:

  https://lore.kernel.org/all/202211151503.HLlq2Z4B-lkp@intel.com/

Bjorn
Re: [v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Dmitry Torokhov 3 years, 4 months ago
On Wed, Dec 07, 2022 at 02:18:07PM -0600, Bjorn Helgaas wrote:
> On Wed, Dec 07, 2022 at 10:55:06AM -0800, Dmitry Torokhov wrote:
> > ...
> > I wonder if you could also consider picking up the mvebu patch:
> > 
> > 	https://lore.kernel.org/all/Y3KbhIi4ZsSO7+Cl@google.com/
> > 
> > pci-mvebu.c is the very last user of of_get_named_gpio_flags() in the
> > next tree, which we also want to stop exporting.
> 
> Sure, but the 0-day bot found a couple issues, which I haven't
> bothered to look into:
> 
>   https://lore.kernel.org/all/202211151503.HLlq2Z4B-lkp@intel.com/

Ugh, indeed, I asumed this was an existing issue and of course I was
wrong. linux/irqchip/chained_irq.h was previously included indirectly
via linux/of_gpio.h and linux/gpio/driver.h.

I sent out v3 tha compiles cleanly.

Thanks.

-- 
Dmitry
Re: [v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Dmitry Torokhov 3 years, 4 months ago
On Mon, Nov 14, 2022 at 10:42:25AM -0800, Dmitry Torokhov wrote:
> Switch the driver to the generic version of gpiod API (and away from
> OF-specific variant), so that we can stop exporting
> devm_gpiod_get_from_of_node().
> 
> Acked-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> v2:
>  - collected reviewed-by/acked-by tags
>  - updated commit description to remove incorrect assumption of why
>    devm_gpiod_get_from_of_node() was used in the first place
> 
> This is the last user of devm_gpiod_get_from_of_node() in the mainline
> (next), it would be great to have it in so that we can remove the API in
> the next release cycle.
> 
> Thanks!

Gentle ping on this one... I'd really like to remove
[devm_]gpiod_get_from_of_node() API from 6.2.

Thanks.

-- 
Dmitry
Re: [v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Pali Rohár 3 years, 4 months ago
On Tuesday 06 December 2022 17:31:11 Dmitry Torokhov wrote:
> On Mon, Nov 14, 2022 at 10:42:25AM -0800, Dmitry Torokhov wrote:
> > Switch the driver to the generic version of gpiod API (and away from
> > OF-specific variant), so that we can stop exporting
> > devm_gpiod_get_from_of_node().
> > 
> > Acked-by: Pali Rohár <pali@kernel.org>
> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > 
> > v2:
> >  - collected reviewed-by/acked-by tags
> >  - updated commit description to remove incorrect assumption of why
> >    devm_gpiod_get_from_of_node() was used in the first place
> > 
> > This is the last user of devm_gpiod_get_from_of_node() in the mainline
> > (next), it would be great to have it in so that we can remove the API in
> > the next release cycle.
> > 
> > Thanks!
> 
> Gentle ping on this one... I'd really like to remove
> [devm_]gpiod_get_from_of_node() API from 6.2.
> 
> Thanks.
> 
> -- 
> Dmitry

Hello Dmitry! You would need to wait with your change. There are more
important fixes and less important cleanups for this aardvark driver
which are waiting in the queue for longer time:
https://patchwork.kernel.org/project/linux-pci/list/?series=&submitter=&state=&q=aardvark&archive=&delegate=
Re: [v2 PATCH] PCI: aardvark: switch to using devm_gpiod_get_optional()
Posted by Pali Rohár 3 years, 4 months ago
On Monday 14 November 2022 10:42:25 Dmitry Torokhov wrote:
> Switch the driver to the generic version of gpiod API (and away from
> OF-specific variant), so that we can stop exporting
> devm_gpiod_get_from_of_node().
> 
> Acked-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> v2:
>  - collected reviewed-by/acked-by tags
>  - updated commit description to remove incorrect assumption of why
>    devm_gpiod_get_from_of_node() was used in the first place
> 
> This is the last user of devm_gpiod_get_from_of_node() in the mainline
> (next), it would be great to have it in so that we can remove the API in
> the next release cycle.
> 
> Thanks!

Just a note that more aardvark patches are waiting on the list.

> 
>  drivers/pci/controller/pci-aardvark.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index ba36bbc5897d..5ecfac23c9fc 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -1859,20 +1859,19 @@ static int advk_pcie_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	pcie->reset_gpio = devm_gpiod_get_from_of_node(dev, dev->of_node,
> -						       "reset-gpios", 0,
> -						       GPIOD_OUT_LOW,
> -						       "pcie1-reset");
> +	pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>  	ret = PTR_ERR_OR_ZERO(pcie->reset_gpio);
>  	if (ret) {
> -		if (ret == -ENOENT) {
> -			pcie->reset_gpio = NULL;
> -		} else {
> -			if (ret != -EPROBE_DEFER)
> -				dev_err(dev, "Failed to get reset-gpio: %i\n",
> -					ret);
> -			return ret;
> -		}
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "Failed to get reset-gpio: %i\n",
> +				ret);
> +		return ret;
> +	}
> +
> +	ret = gpiod_set_consumer_name(pcie->reset_gpio, "pcie1-reset");
> +	if (ret) {
> +		dev_err(dev, "Failed to set reset gpio name: %d\n", ret);
> +		return ret;
>  	}
>  
>  	ret = of_pci_get_max_link_speed(dev->of_node);
> -- 
> 2.38.1.431.g37b22c650d-goog
> 
> 
> -- 
> Dmitry