linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
@ 2021-11-02  1:03 Maíra Canal
  2022-04-08 14:23 ` Lorenzo Pieralisi
  2022-04-20 23:24 ` Linus Walleij
  0 siblings, 2 replies; 10+ messages in thread
From: Maíra Canal @ 2021-11-02  1:03 UTC (permalink / raw)
  To: hongxing.zhu, l.stach, lorenzo.pieralisi, robh, bhelgaas,
	helgaas, shawnguo, s.hauer
  Cc: kernel, linux-imx, linux-pci, linux-arm-kernel, linux-kernel

Considering the current transition of the GPIO subsystem, remove all
dependencies of the legacy GPIO interface (linux/gpio.h and linux
/of_gpio.h) and replace it with the descriptor-based GPIO approach.

Signed-off-by: Maíra Canal <maira.canal@usp.br>
---
V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard
V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep
---
 drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 80fc98acf097..f08865ac0b40 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -11,13 +11,12 @@
 #include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
 #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
 #include <linux/module.h>
-#include <linux/of_gpio.h>
 #include <linux/of_device.h>
 #include <linux/of_address.h>
 #include <linux/pci.h>
@@ -63,7 +62,7 @@ struct imx6_pcie_drvdata {
 
 struct imx6_pcie {
 	struct dw_pcie		*pci;
-	int			reset_gpio;
+	struct gpio_desc	*reset_gpio;
 	bool			gpio_active_high;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
@@ -526,11 +525,11 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 	usleep_range(200, 500);
 
 	/* Some boards don't have PCIe reset GPIO. */
-	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
+	if (imx6_pcie->reset_gpio) {
+		gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio,
 					imx6_pcie->gpio_active_high);
 		msleep(100);
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
+		gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio,
 					!imx6_pcie->gpio_active_high);
 	}
 
@@ -1025,22 +1024,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pci->dbi_base);
 
 	/* Fetch GPIOs */
-	imx6_pcie->reset_gpio = of_get_named_gpio(node, "reset-gpio", 0);
 	imx6_pcie->gpio_active_high = of_property_read_bool(node,
 						"reset-gpio-active-high");
-	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		ret = devm_gpio_request_one(dev, imx6_pcie->reset_gpio,
-				imx6_pcie->gpio_active_high ?
-					GPIOF_OUT_INIT_HIGH :
-					GPIOF_OUT_INIT_LOW,
-				"PCIe reset");
-		if (ret) {
-			dev_err(dev, "unable to get reset gpio\n");
-			return ret;
-		}
-	} else if (imx6_pcie->reset_gpio == -EPROBE_DEFER) {
-		return imx6_pcie->reset_gpio;
-	}
+	imx6_pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+			imx6_pcie->gpio_active_high ?  GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
+	if (IS_ERR(imx6_pcie->reset_gpio))
+		return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio),
+				"unable to get reset gpio\n");
 
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy");
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2021-11-02  1:03 [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface Maíra Canal
@ 2022-04-08 14:23 ` Lorenzo Pieralisi
  2022-04-08 14:46   ` Maíra Canal
  2022-04-20 23:24 ` Linus Walleij
  1 sibling, 1 reply; 10+ messages in thread
From: Lorenzo Pieralisi @ 2022-04-08 14:23 UTC (permalink / raw)
  To: Maíra Canal, l.stach
  Cc: hongxing.zhu, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel,
	linux-imx, linux-pci, linux-arm-kernel, linux-kernel

On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote:
> Considering the current transition of the GPIO subsystem, remove all
> dependencies of the legacy GPIO interface (linux/gpio.h and linux
> /of_gpio.h) and replace it with the descriptor-based GPIO approach.
> 
> Signed-off-by: Maíra Canal <maira.canal@usp.br>
> ---
> V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard
> V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------
>  1 file changed, 10 insertions(+), 20 deletions(-)

Maira, Lucas,

what's this patch status ? Please let me know.

Thanks,
Lorenzo

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 80fc98acf097..f08865ac0b40 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -11,13 +11,12 @@
>  #include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>  #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
>  #include <linux/module.h>
> -#include <linux/of_gpio.h>
>  #include <linux/of_device.h>
>  #include <linux/of_address.h>
>  #include <linux/pci.h>
> @@ -63,7 +62,7 @@ struct imx6_pcie_drvdata {
>  
>  struct imx6_pcie {
>  	struct dw_pcie		*pci;
> -	int			reset_gpio;
> +	struct gpio_desc	*reset_gpio;
>  	bool			gpio_active_high;
>  	struct clk		*pcie_bus;
>  	struct clk		*pcie_phy;
> @@ -526,11 +525,11 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	usleep_range(200, 500);
>  
>  	/* Some boards don't have PCIe reset GPIO. */
> -	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> -		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> +	if (imx6_pcie->reset_gpio) {
> +		gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio,
>  					imx6_pcie->gpio_active_high);
>  		msleep(100);
> -		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> +		gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio,
>  					!imx6_pcie->gpio_active_high);
>  	}
>  
> @@ -1025,22 +1024,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(pci->dbi_base);
>  
>  	/* Fetch GPIOs */
> -	imx6_pcie->reset_gpio = of_get_named_gpio(node, "reset-gpio", 0);
>  	imx6_pcie->gpio_active_high = of_property_read_bool(node,
>  						"reset-gpio-active-high");
> -	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> -		ret = devm_gpio_request_one(dev, imx6_pcie->reset_gpio,
> -				imx6_pcie->gpio_active_high ?
> -					GPIOF_OUT_INIT_HIGH :
> -					GPIOF_OUT_INIT_LOW,
> -				"PCIe reset");
> -		if (ret) {
> -			dev_err(dev, "unable to get reset gpio\n");
> -			return ret;
> -		}
> -	} else if (imx6_pcie->reset_gpio == -EPROBE_DEFER) {
> -		return imx6_pcie->reset_gpio;
> -	}
> +	imx6_pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> +			imx6_pcie->gpio_active_high ?  GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> +	if (IS_ERR(imx6_pcie->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio),
> +				"unable to get reset gpio\n");
>  
>  	/* Fetch clocks */
>  	imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy");
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2022-04-08 14:23 ` Lorenzo Pieralisi
@ 2022-04-08 14:46   ` Maíra Canal
  2022-04-11 11:59     ` Francesco Dolcini
  2022-04-11 15:25     ` Lorenzo Pieralisi
  0 siblings, 2 replies; 10+ messages in thread
From: Maíra Canal @ 2022-04-08 14:46 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: l.stach, hongxing.zhu, robh, bhelgaas, helgaas, shawnguo,
	s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel,
	linux-kernel

On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote:
> On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote:
> > Considering the current transition of the GPIO subsystem, remove all
> > dependencies of the legacy GPIO interface (linux/gpio.h and linux
> > /of_gpio.h) and replace it with the descriptor-based GPIO approach.
> > 
> > Signed-off-by: Maíra Canal <maira.canal@usp.br>
> > ---
> > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard
> > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------
> >  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> Maira, Lucas,
> 
> what's this patch status ? Please let me know.


Lorenzo,

Thank you for the feedback. Since I sent v3, I didn't get any feedback from the community.

If you have any feedback, I would gladly work on it.

Thanks,
Maíra Canal

> 
> Thanks,
> Lorenzo
> 
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> > index 80fc98acf097..f08865ac0b40 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -11,13 +11,12 @@
> >  #include <linux/bitfield.h>
> >  #include <linux/clk.h>
> >  #include <linux/delay.h>
> > -#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> >  #include <linux/kernel.h>
> >  #include <linux/mfd/syscon.h>
> >  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> >  #include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
> >  #include <linux/module.h>
> > -#include <linux/of_gpio.h>
> >  #include <linux/of_device.h>
> >  #include <linux/of_address.h>
> >  #include <linux/pci.h>
> > @@ -63,7 +62,7 @@ struct imx6_pcie_drvdata {
> >  
> >  struct imx6_pcie {
> >  	struct dw_pcie		*pci;
> > -	int			reset_gpio;
> > +	struct gpio_desc	*reset_gpio;
> >  	bool			gpio_active_high;
> >  	struct clk		*pcie_bus;
> >  	struct clk		*pcie_phy;
> > @@ -526,11 +525,11 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	usleep_range(200, 500);
> >  
> >  	/* Some boards don't have PCIe reset GPIO. */
> > -	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > -		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> > +	if (imx6_pcie->reset_gpio) {
> > +		gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio,
> >  					imx6_pcie->gpio_active_high);
> >  		msleep(100);
> > -		gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> > +		gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio,
> >  					!imx6_pcie->gpio_active_high);
> >  	}
> >  
> > @@ -1025,22 +1024,13 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> >  		return PTR_ERR(pci->dbi_base);
> >  
> >  	/* Fetch GPIOs */
> > -	imx6_pcie->reset_gpio = of_get_named_gpio(node, "reset-gpio", 0);
> >  	imx6_pcie->gpio_active_high = of_property_read_bool(node,
> >  						"reset-gpio-active-high");
> > -	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > -		ret = devm_gpio_request_one(dev, imx6_pcie->reset_gpio,
> > -				imx6_pcie->gpio_active_high ?
> > -					GPIOF_OUT_INIT_HIGH :
> > -					GPIOF_OUT_INIT_LOW,
> > -				"PCIe reset");
> > -		if (ret) {
> > -			dev_err(dev, "unable to get reset gpio\n");
> > -			return ret;
> > -		}
> > -	} else if (imx6_pcie->reset_gpio == -EPROBE_DEFER) {
> > -		return imx6_pcie->reset_gpio;
> > -	}
> > +	imx6_pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > +			imx6_pcie->gpio_active_high ?  GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> > +	if (IS_ERR(imx6_pcie->reset_gpio))
> > +		return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio),
> > +				"unable to get reset gpio\n");
> >  
> >  	/* Fetch clocks */
> >  	imx6_pcie->pcie_phy = devm_clk_get(dev, "pcie_phy");
> > -- 
> > 2.31.1
> > 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2022-04-08 14:46   ` Maíra Canal
@ 2022-04-11 11:59     ` Francesco Dolcini
  2022-04-11 16:03       ` Lucas Stach
  2022-04-11 15:25     ` Lorenzo Pieralisi
  1 sibling, 1 reply; 10+ messages in thread
From: Francesco Dolcini @ 2022-04-11 11:59 UTC (permalink / raw)
  To: Maíra Canal
  Cc: Lorenzo Pieralisi, l.stach, hongxing.zhu, robh, bhelgaas,
	helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci,
	linux-arm-kernel, linux-kernel

On Fri, Apr 08, 2022 at 11:46:32AM -0300, Maíra Canal wrote:
> On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote:
> > > Considering the current transition of the GPIO subsystem, remove all
> > > dependencies of the legacy GPIO interface (linux/gpio.h and linux
> > > /of_gpio.h) and replace it with the descriptor-based GPIO approach.
> > > 
> > > Signed-off-by: Maíra Canal <maira.canal@usp.br>
> > > ---
> > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard
> > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------
> > >  1 file changed, 10 insertions(+), 20 deletions(-)
> > 
> > Maira, Lucas,
> > 
> > what's this patch status ? Please let me know.
> 
> Thank you for the feedback. Since I sent v3, I didn't get any feedback from the community.
> 
> If you have any feedback, I would gladly work on it.

Just for you to know that it will likely conflict with
'PCI: imx6: Fix PERST# start-up sequence' [0] that is also still waiting
for some additional feedback.

Francesco

[0] https://lore.kernel.org/all/20220404081509.94356-1-francesco.dolcini@toradex.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2022-04-08 14:46   ` Maíra Canal
  2022-04-11 11:59     ` Francesco Dolcini
@ 2022-04-11 15:25     ` Lorenzo Pieralisi
  1 sibling, 0 replies; 10+ messages in thread
From: Lorenzo Pieralisi @ 2022-04-11 15:25 UTC (permalink / raw)
  To: Maíra Canal
  Cc: l.stach, hongxing.zhu, robh, bhelgaas, helgaas, shawnguo,
	s.hauer, kernel, linux-imx, linux-pci, linux-arm-kernel,
	linux-kernel, linus.walleij

[+Linus]

On Fri, Apr 08, 2022 at 11:46:32AM -0300, Maíra Canal wrote:
> On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote:
> > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote:
> > > Considering the current transition of the GPIO subsystem, remove all
> > > dependencies of the legacy GPIO interface (linux/gpio.h and linux
> > > /of_gpio.h) and replace it with the descriptor-based GPIO approach.
> > > 
> > > Signed-off-by: Maíra Canal <maira.canal@usp.br>
> > > ---
> > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard
> > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------
> > >  1 file changed, 10 insertions(+), 20 deletions(-)
> > 
> > Maira, Lucas,
> > 
> > what's this patch status ? Please let me know.
> 
> 
> Lorenzo,
> 
> Thank you for the feedback. Since I sent v3, I didn't get any feedback from the community.
> 
> If you have any feedback, I would gladly work on it.

I would ask Linus to have a look please given that it is GPIO code.

Original thread:
https://lore.kernel.org/linux-pci/YYCOTx68LXu1Tn1i@fedora

Thanks,
Lorenzo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2022-04-11 11:59     ` Francesco Dolcini
@ 2022-04-11 16:03       ` Lucas Stach
  2022-04-12 11:02         ` Maíra Canal
  0 siblings, 1 reply; 10+ messages in thread
From: Lucas Stach @ 2022-04-11 16:03 UTC (permalink / raw)
  To: Francesco Dolcini, Maíra Canal, Lorenzo Pieralisi
  Cc: hongxing.zhu, robh, bhelgaas, helgaas, shawnguo, s.hauer, kernel,
	linux-imx, linux-pci, linux-arm-kernel, linux-kernel

Hi Maíra, hi Lorenzo,

Am Montag, dem 11.04.2022 um 13:59 +0200 schrieb Francesco Dolcini:
> On Fri, Apr 08, 2022 at 11:46:32AM -0300, Maíra Canal wrote:
> > On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote:
> > > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote:
> > > > Considering the current transition of the GPIO subsystem, remove all
> > > > dependencies of the legacy GPIO interface (linux/gpio.h and linux
> > > > /of_gpio.h) and replace it with the descriptor-based GPIO approach.
> > > > 
> > > > Signed-off-by: Maíra Canal <maira.canal@usp.br>
> > > > ---
> > > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard
> > > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep
> > > > ---
> > > >  drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------
> > > >  1 file changed, 10 insertions(+), 20 deletions(-)
> > > 
> > > Maira, Lucas,
> > > 
> > > what's this patch status ? Please let me know.

The patch looks fine to me now, however...

> > 
> > Thank you for the feedback. Since I sent v3, I didn't get any feedback from the community.
> > 
> > If you have any feedback, I would gladly work on it.
> 
> Just for you to know that it will likely conflict with
> 'PCI: imx6: Fix PERST# start-up sequence' [0] that is also still waiting
> for some additional feedback.

... they do conflict and I guess the functional fix for the reset
sequence should have priority over this cleanup. Maíra, may I ask you
to respin your patch as soon as Francescos fix is in? I promise to not
let it fall through the cracks again.

Regards,
Lucas


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2022-04-11 16:03       ` Lucas Stach
@ 2022-04-12 11:02         ` Maíra Canal
  0 siblings, 0 replies; 10+ messages in thread
From: Maíra Canal @ 2022-04-12 11:02 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Francesco Dolcini, Lorenzo Pieralisi, hongxing.zhu, robh,
	bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx,
	linux-pci, linux-arm-kernel, linux-kernel

On 04/11, Lucas Stach wrote:
> Hi Maíra, hi Lorenzo,
> 
> Am Montag, dem 11.04.2022 um 13:59 +0200 schrieb Francesco Dolcini:
> > On Fri, Apr 08, 2022 at 11:46:32AM -0300, Maíra Canal wrote:
> > > On Fri, Apr 08, 2022 at 03:23:39PM +0100, Lorenzo Pieralisi wrote:
> > > > On Mon, Nov 01, 2021 at 10:03:11PM -0300, Maíra Canal wrote:
> > > > > Considering the current transition of the GPIO subsystem, remove all
> > > > > dependencies of the legacy GPIO interface (linux/gpio.h and linux
> > > > > /of_gpio.h) and replace it with the descriptor-based GPIO approach.
> > > > > 
> > > > > Signed-off-by: Maíra Canal <maira.canal@usp.br>
> > > > > ---
> > > > > V1 -> V2: Rewrite commit log and subject line to match PCI subsystem standard
> > > > > V2 -> v3: Change gpiod_set_value_cansleep for gpiod_set_raw_value_cansleep
> > > > > ---
> > > > >  drivers/pci/controller/dwc/pci-imx6.c | 30 +++++++++------------------
> > > > >  1 file changed, 10 insertions(+), 20 deletions(-)
> 
> ... they do conflict and I guess the functional fix for the reset
> sequence should have priority over this cleanup. Maíra, may I ask you
> to respin your patch as soon as Francescos fix is in? I promise to not
> let it fall through the cracks again.

Hi Lucas

That's fine. After Francescos patch is applied, I resend the fix.

Thank you for the feedback!

Sincerly,
Maíra Canal

> 
> Regards,
> Lucas
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2021-11-02  1:03 [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface Maíra Canal
  2022-04-08 14:23 ` Lorenzo Pieralisi
@ 2022-04-20 23:24 ` Linus Walleij
  2022-04-25 12:07   ` Lucas Stach
  1 sibling, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2022-04-20 23:24 UTC (permalink / raw)
  To: Maíra Canal
  Cc: hongxing.zhu, l.stach, lorenzo.pieralisi, robh, bhelgaas,
	helgaas, shawnguo, s.hauer, kernel, linux-imx, linux-pci,
	linux-arm-kernel, linux-kernel

Hi Maira and sorry for being slow on reviews.

On Tue, Nov 2, 2021 at 2:04 AM Maíra Canal <maira.canal@usp.br> wrote:

> -               gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> +               gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio,
>                                         !imx6_pcie->gpio_active_high);

Hm I see you got advised to use the raw api. I'm not so sure about
that I like v1 better.

> +       imx6_pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> +                       imx6_pcie->gpio_active_high ?  GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> +       if (IS_ERR(imx6_pcie->reset_gpio))
> +               return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio),
> +                               "unable to get reset gpio\n");

Where is this descriptor coming from? Device trees? Can't we just fix the
DTS file(s) in that case given how wrong they are if they don't set
GPIO_ACTIVE_LOW flag on this IRQ.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2022-04-20 23:24 ` Linus Walleij
@ 2022-04-25 12:07   ` Lucas Stach
  2022-04-25 13:40     ` Linus Walleij
  0 siblings, 1 reply; 10+ messages in thread
From: Lucas Stach @ 2022-04-25 12:07 UTC (permalink / raw)
  To: Linus Walleij, Maíra Canal
  Cc: hongxing.zhu, lorenzo.pieralisi, robh, bhelgaas, helgaas,
	shawnguo, s.hauer, kernel, linux-imx, linux-pci,
	linux-arm-kernel, linux-kernel

Hi Linus,

Am Donnerstag, dem 21.04.2022 um 01:24 +0200 schrieb Linus Walleij:
> Hi Maira and sorry for being slow on reviews.
> 
> On Tue, Nov 2, 2021 at 2:04 AM Maíra Canal <maira.canal@usp.br> wrote:
> 
> > -               gpio_set_value_cansleep(imx6_pcie->reset_gpio,
> > +               gpiod_set_raw_value_cansleep(imx6_pcie->reset_gpio,
> >                                         !imx6_pcie->gpio_active_high);
> 
> Hm I see you got advised to use the raw api. I'm not so sure about
> that I like v1 better.
> 
> > +       imx6_pcie->reset_gpio = devm_gpiod_get_optional(dev, "reset",
> > +                       imx6_pcie->gpio_active_high ?  GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> > +       if (IS_ERR(imx6_pcie->reset_gpio))
> > +               return dev_err_probe(dev, PTR_ERR(imx6_pcie->reset_gpio),
> > +                               "unable to get reset gpio\n");
> 
> Where is this descriptor coming from? Device trees? Can't we just fix the
> DTS file(s) in that case given how wrong they are if they don't set
> GPIO_ACTIVE_LOW flag on this IRQ.

The binding explicitly describes the GPIO as not polarity aware and has
a separate property "reset-gpio-active-high" to avoid breaking old
DTBs. I don't think it's helpful to dismiss this explicit backward
compat just because the driver code looks nicer that way.

Regards,
Lucas


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface
  2022-04-25 12:07   ` Lucas Stach
@ 2022-04-25 13:40     ` Linus Walleij
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2022-04-25 13:40 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Maíra Canal, hongxing.zhu, lorenzo.pieralisi, robh,
	bhelgaas, helgaas, shawnguo, s.hauer, kernel, linux-imx,
	linux-pci, linux-arm-kernel, linux-kernel

On Mon, Apr 25, 2022 at 2:07 PM Lucas Stach <l.stach@pengutronix.de> wrote:

> The binding explicitly describes the GPIO as not polarity aware and has
> a separate property "reset-gpio-active-high" to avoid breaking old
> DTBs. I don't think it's helpful to dismiss this explicit backward
> compat just because the driver code looks nicer that way.

I see. We handle such things a specific way.

Look in drivers/gpio/gpiolib-of.c, especially the function
of_gpio_flags_quirks().

Here we special-case all bindings which for some reason introduced
something necessary custom, like in this case not using the proper
polarity flag.

Add code to this file in the proper place to handle and hide the
old style DTBs using "reset-gpio-active-high" as active high flag
and assuming active low otherwise in this file.

I imagine it begins with

if (IS_ENABLED(CONFIG_PCI_IMX6)) { ... }

Then modify the code in drivers/pci/controller/dwc/pci-imx6.c
to act as if gpiolib handles polarity inversion. Include all changes
to all files in the same patch so this is changed in tandem (one technical
step).

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-04-25 13:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-02  1:03 [PATCH v3] PCI: imx6: Replace legacy gpio interface for gpiod interface Maíra Canal
2022-04-08 14:23 ` Lorenzo Pieralisi
2022-04-08 14:46   ` Maíra Canal
2022-04-11 11:59     ` Francesco Dolcini
2022-04-11 16:03       ` Lucas Stach
2022-04-12 11:02         ` Maíra Canal
2022-04-11 15:25     ` Lorenzo Pieralisi
2022-04-20 23:24 ` Linus Walleij
2022-04-25 12:07   ` Lucas Stach
2022-04-25 13:40     ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).