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

Message ID Y3KMEZFv6dpxA+Gv@google.com
State New
Headers
Series [v2] PCI: aardvark: switch to using devm_gpiod_get_optional() |

Commit Message

Dmitry Torokhov Nov. 14, 2022, 6:42 p.m. UTC
  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(-)
  

Comments

Pali Rohár Nov. 15, 2022, 6 p.m. UTC | #1
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
  
Dmitry Torokhov Dec. 7, 2022, 1:31 a.m. UTC | #2
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.
  
Pali Rohár Dec. 7, 2022, 7:57 a.m. UTC | #3
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=
  
Bjorn Helgaas Dec. 7, 2022, 2:33 p.m. UTC | #4
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
  
Dmitry Torokhov Dec. 7, 2022, 6:55 p.m. UTC | #5
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.
  
Bjorn Helgaas Dec. 7, 2022, 8:18 p.m. UTC | #6
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
  
Dmitry Torokhov Dec. 7, 2022, 9:09 p.m. UTC | #7
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.
  
Pali Rohár Dec. 16, 2022, 6:25 p.m. UTC | #8
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
  

Patch

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);