[PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Fabio Estevam]

From: Fabio Estevam <fabio.estevam@nxp.com>

Commit 5c5fb40de8f14 ("PCI: imx6: Add support for active-low reset GPIO")
cause regressions on some boards like MX6 Gateworks Ventana, for example.

The reason for the breakage is that this commit sets the gpio polarity
in the wrong logic level.

Also, the commit log is wrong because active-low reset GPIO is what the
driver used to support since the beginning.

So keep the old behavior that ignores the gpio polarity specified in
the device tree and treat the PCI reset GPIO as active-low.

Cc: <stable@vger.kernel.org> # 4.5
Reported-by: Krzysztof Hałasa <khalasa@piap.pl>
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes since v1:
- Improve wording of commit log (Tim)

 drivers/pci/host/pci-imx6.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index eb5a275..2f817fa 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -32,7 +32,7 @@
 #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
 
 struct imx6_pcie {
-	struct gpio_desc	*reset_gpio;
+	int			reset_gpio;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie;
@@ -309,10 +309,10 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 	usleep_range(200, 500);
 
 	/* Some boards don't have PCIe reset GPIO. */
-	if (imx6_pcie->reset_gpio) {
-		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
+	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
+		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
 		msleep(100);
-		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
 	}
 	return 0;
 
@@ -523,6 +523,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 {
 	struct imx6_pcie *imx6_pcie;
 	struct pcie_port *pp;
+	struct device_node *np = pdev->dev.of_node;
 	struct resource *dbi_base;
 	struct device_node *node = pdev->dev.of_node;
 	int ret;
@@ -544,8 +545,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
-							GPIOD_OUT_LOW);
+	imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
+	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
+		ret = devm_gpio_request_one(&pdev->dev, imx6_pcie->reset_gpio,
+					    GPIOF_OUT_INIT_LOW, "PCIe reset");
+		if (ret) {
+			dev_err(&pdev->dev, "unable to get reset gpio\n");
+			return ret;
+		}
+	}
 
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
-- 
1.9.1

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Tim Harvey]

On Mon, Mar 28, 2016 at 2:45 PM, Fabio Estevam <festevam@gmail.com> wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> Commit 5c5fb40de8f14 ("PCI: imx6: Add support for active-low reset GPIO")
> cause regressions on some boards like MX6 Gateworks Ventana, for example.
>
> The reason for the breakage is that this commit sets the gpio polarity
> in the wrong logic level.
>
> Also, the commit log is wrong because active-low reset GPIO is what the
> driver used to support since the beginning.
>
> So keep the old behavior that ignores the gpio polarity specified in
> the device tree and treat the PCI reset GPIO as active-low.
>
> Cc: <stable@vger.kernel.org> # 4.5
> Reported-by: Krzysztof Hałasa <khalasa@piap.pl>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v1:
> - Improve wording of commit log (Tim)
>
>  drivers/pci/host/pci-imx6.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index eb5a275..2f817fa 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -32,7 +32,7 @@
>  #define to_imx6_pcie(x)        container_of(x, struct imx6_pcie, pp)
>
>  struct imx6_pcie {
> -       struct gpio_desc        *reset_gpio;
> +       int                     reset_gpio;
>         struct clk              *pcie_bus;
>         struct clk              *pcie_phy;
>         struct clk              *pcie;
> @@ -309,10 +309,10 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
>         usleep_range(200, 500);
>
>         /* Some boards don't have PCIe reset GPIO. */
> -       if (imx6_pcie->reset_gpio) {
> -               gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
> +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +               gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
>                 msleep(100);
> -               gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
> +               gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
>         }
>         return 0;
>
> @@ -523,6 +523,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  {
>         struct imx6_pcie *imx6_pcie;
>         struct pcie_port *pp;
> +       struct device_node *np = pdev->dev.of_node;
>         struct resource *dbi_base;
>         struct device_node *node = pdev->dev.of_node;
>         int ret;
> @@ -544,8 +545,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>                 return PTR_ERR(pp->dbi_base);
>
>         /* Fetch GPIOs */
> -       imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
> -                                                       GPIOD_OUT_LOW);
> +       imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +               ret = devm_gpio_request_one(&pdev->dev, imx6_pcie->reset_gpio,
> +                                           GPIOF_OUT_INIT_LOW, "PCIe reset");
> +               if (ret) {
> +                       dev_err(&pdev->dev, "unable to get reset gpio\n");
> +                       return ret;
> +               }
> +       }
>
>         /* Fetch clocks */
>         imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
> --
> 1.9.1
>

Fabio/Krzysztof - thanks for finding this regression that breaks PCI
on most IMX6. I verified reverting this resolves Gateworks Ventana PCI
breakage in v4.5.

Acked-by: Tim Harvey <tharvey@gateworks.com>

Regards,

Tim

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Petr Štetiar]

Tim Harvey <tharvey@gateworks.com> [2016-03-29 06:43:19]:

> > Commit 5c5fb40de8f14 ("PCI: imx6: Add support for active-low reset GPIO")
> > cause regressions on some boards like MX6 Gateworks Ventana, for example.

Sorry for the problems.

> > The reason for the breakage is that this commit sets the gpio polarity
> > in the wrong logic level.

It's just a nitpick :-) but this commit doesn't set the polarity in the wrong
logic level, 

> > -       if (imx6_pcie->reset_gpio) {
> > -               gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
> > +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > +               gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
> >                 msleep(100);
> > -               gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
> > +               gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);

as you can see, the polarity was simply wrong from the beginning[1]. My patch
has just probably made the error more exposed.

1. https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/pci/host/pci-imx6.c?id=bb38919ec56e0758c3ae56dfc091dcde1391353e

-- ynezz

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Fabio Estevam]

Hi Petr,

On Wed, Mar 30, 2016 at 9:21 AM, Petr Štetiar <ynezz@true.cz> wrote:

> It's just a nitpick :-) but this commit doesn't set the polarity in the wrong
> logic level,

Yes, it does. That's why it causes regressions.

> as you can see, the polarity was simply wrong from the beginning[1]. My patch
> has just probably made the error more exposed.

No, polarity was active low since the beginning.

In order to make the Toradex board to work without breaking old dtb's
is to introduce a property like 'phy-reset-active-high' and handle it
in the driver.

Take a look at the FEC driver for a an example:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/freescale/fec_main.c?id=962d8cdc3133435aed2928637f73e272128a326c

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Petr Štetiar]

Fabio Estevam <festevam@gmail.com> [2016-03-30 09:36:35]:

Hi Fabio,

> In order to make the Toradex board to work without breaking old dtb's is to
> introduce a property like 'phy-reset-active-high' and handle it in the
> driver.

do we really need to do this?  With Krzysztof's patch it should work for both
cases correctly. If I understand gpiolib correctly, you don't need to
introduce reset-active-high, if you use gpiod_* functions, then the GPIO
polarity is handled correctly inside the gpiolib framework[1] and all you need
is to define pin's polarity correctly via gpio pin definition. Or am I missing
something?

Then we need to just fix the broken DTBs.

1. http://lxr.free-electrons.com/source/drivers/gpio/gpiolib.c#L1759

-- ynezz

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Fabio Estevam]

Hi Petr,

On Wed, Mar 30, 2016 at 10:00 AM, Petr Štetiar <ynezz@true.cz> wrote:

> do we really need to do this?  With Krzysztof's patch it should work for both
> cases correctly. If I understand gpiolib correctly, you don't need to
> introduce reset-active-high, if you use gpiod_* functions, then the GPIO
> polarity is handled correctly inside the gpiolib framework[1] and all you need
> is to define pin's polarity correctly via gpio pin definition. Or am I missing
> something?
>
> Then we need to just fix the broken DTBs.

If we apply Krzysztof's patch then we break PCI in imx6qdl-sabresd.

In this board we have:

reset-gpio = <&gpio7 12 0>;

, which is wrong. Of course, we should fix this to be 'reset-gpio =
<&gpio7 12 GPIO_ACTIVE_LOW>;'

However we should not break old dtb's. That's why reverting the patch
is the only solution we have so that old dtb's still work.

We have had this same problem for the FEC PHY reset. The solution
there was to add a new property to treat the active-high case.

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Petr Štetiar]

Fabio Estevam <festevam@gmail.com> [2016-03-30 10:07:16]:

> If we apply Krzysztof's patch then we break PCI in imx6qdl-sabresd.
> 
> In this board we have:
> 
> reset-gpio = <&gpio7 12 0>;
> 
> , which is wrong. Of course, we should fix this to be 'reset-gpio =
> <&gpio7 12 GPIO_ACTIVE_LOW>;'
> 
> However we should not break old dtb's. That's why reverting the patch
> is the only solution we have so that old dtb's still work.

But this only apply for stable right? I understand this and it's fine with me.

> We have had this same problem for the FEC PHY reset. The solution
> there was to add a new property to treat the active-high case.

In next we should fix it correctly, apply Krzysztof's patch and fix broken
DTBs, right?

-- ynezz

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Lucas Stach]

Am Mittwoch, den 30.03.2016, 15:16 +0200 schrieb Petr Štetiar:
> Fabio Estevam <festevam@gmail.com> [2016-03-30 10:07:16]:
> 
> > If we apply Krzysztof's patch then we break PCI in imx6qdl-sabresd.
> > 
> > In this board we have:
> > 
> > reset-gpio = <&gpio7 12 0>;
> > 
> > , which is wrong. Of course, we should fix this to be 'reset-gpio =
> > <&gpio7 12 GPIO_ACTIVE_LOW>;'
> > 
> > However we should not break old dtb's. That's why reverting the patch
> > is the only solution we have so that old dtb's still work.
> 
> But this only apply for stable right? I understand this and it's fine with me.
> 
> > We have had this same problem for the FEC PHY reset. The solution
> > there was to add a new property to treat the active-high case.
> 
> In next we should fix it correctly, apply Krzysztof's patch and fix broken
> DTBs, right?
> 
We can and should fix broken DTs, but keeping DT stability means we need
to be able to support old (unfixed) DTs with future kernels.

So we can not just fix the DTs and move on, as kernel and DT are two
separate entities, even if they are currently distributed through the
same source tree.

Regards,
Lucas

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Fabio Estevam]

Hi Lucas,

On Wed, Mar 30, 2016 at 10:20 AM, Lucas Stach <l.stach@pengutronix.de> wrote:

> We can and should fix broken DTs, but keeping DT stability means we need
> to be able to support old (unfixed) DTs with future kernels.
>
> So we can not just fix the DTs and move on, as kernel and DT are two
> separate entities, even if they are currently distributed through the
> same source tree.

Correct.

Could you send your Acked-by for this patch then?

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Lucas Stach]

Am Montag, den 28.03.2016, 18:45 -0300 schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Commit 5c5fb40de8f14 ("PCI: imx6: Add support for active-low reset GPIO")
> cause regressions on some boards like MX6 Gateworks Ventana, for example.
> 
> The reason for the breakage is that this commit sets the gpio polarity
> in the wrong logic level.
> 
> Also, the commit log is wrong because active-low reset GPIO is what the
> driver used to support since the beginning.
> 
> So keep the old behavior that ignores the gpio polarity specified in
> the device tree and treat the PCI reset GPIO as active-low.
> 
> Cc: <stable@vger.kernel.org> # 4.5
> Reported-by: Krzysztof Hałasa <khalasa@piap.pl>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>

We need to keep compatibility to existing DTs that are setting the GPIO
active polarity in the wrong way, so this revert is correct:

Acked-by: Lucas Stach <l.stach@pengutronix.de>

> ---
> Changes since v1:
> - Improve wording of commit log (Tim)
> 
>  drivers/pci/host/pci-imx6.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index eb5a275..2f817fa 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -32,7 +32,7 @@
>  #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
>  
>  struct imx6_pcie {
> -	struct gpio_desc	*reset_gpio;
> +	int			reset_gpio;
>  	struct clk		*pcie_bus;
>  	struct clk		*pcie_phy;
>  	struct clk		*pcie;
> @@ -309,10 +309,10 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
>  	usleep_range(200, 500);
>  
>  	/* Some boards don't have PCIe reset GPIO. */
> -	if (imx6_pcie->reset_gpio) {
> -		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
> +	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
>  		msleep(100);
> -		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
> +		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
>  	}
>  	return 0;
>  
> @@ -523,6 +523,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  {
>  	struct imx6_pcie *imx6_pcie;
>  	struct pcie_port *pp;
> +	struct device_node *np = pdev->dev.of_node;
>  	struct resource *dbi_base;
>  	struct device_node *node = pdev->dev.of_node;
>  	int ret;
> @@ -544,8 +545,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(pp->dbi_base);
>  
>  	/* Fetch GPIOs */
> -	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
> -							GPIOD_OUT_LOW);
> +	imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> +	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +		ret = devm_gpio_request_one(&pdev->dev, imx6_pcie->reset_gpio,
> +					    GPIOF_OUT_INIT_LOW, "PCIe reset");
> +		if (ret) {
> +			dev_err(&pdev->dev, "unable to get reset gpio\n");
> +			return ret;
> +		}
> +	}
>  
>  	/* Fetch clocks */
>  	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Krzysztof Hałasa]

Fabio Estevam <festevam@gmail.com> writes:

> In order to make the Toradex board to work without breaking old dtb's
> is to introduce a property like 'phy-reset-active-high' and handle it
> in the driver.

Well, I don't know. Most DTBs are distributed along the kernel on the
same boot medium. Both ways are far from the ideal. Reverting for now is
ok, but long-term I'd rather fix the buggy DTSs and remove extra stuff.

Old DTBs aren't compatible with new kernels anyway and I guess it will
be the case in the future again (e.g. IIRC pre-4.2 IMX6 DTBs don't boot
on 4.2+ - or was it 4.1?).
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Lucas Stach]

Am Donnerstag, den 31.03.2016, 07:07 +0200 schrieb Krzysztof Hałasa:
> Fabio Estevam <festevam@gmail.com> writes:
> 
> > In order to make the Toradex board to work without breaking old dtb's
> > is to introduce a property like 'phy-reset-active-high' and handle it
> > in the driver.
> 
> Well, I don't know. Most DTBs are distributed along the kernel on the
> same boot medium. Both ways are far from the ideal. Reverting for now is
> ok, but long-term I'd rather fix the buggy DTSs and remove extra stuff.
> 
> Old DTBs aren't compatible with new kernels anyway and I guess it will
> be the case in the future again (e.g. IIRC pre-4.2 IMX6 DTBs don't boot
> on 4.2+ - or was it 4.1?).

Sorry, but I strongly object to the argument of "DTs are not stable
anyways, so we might just continue to break them". We are trying to keep
them stable.

I think you are referring to the GPC interrupt domain change with your
above statement about DT stability breaking on 4.1/4.2. This is not the
case. While I only catched the boot regression in the -rc phase of this
kernel, the final released kernel will boot just fine with an old DTB.

The i.MX platform tries to keep DTs stable. No exceptions.

Regards,
Lucas

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Krzysztof Hałasa]

Lucas Stach <l.stach@pengutronix.de> writes:

> I think you are referring to the GPC interrupt domain change with your
> above statement about DT stability breaking on 4.1/4.2. This is not the
> case. While I only catched the boot regression in the -rc phase of this
> kernel, the final released kernel will boot just fine with an old DTB.

It seems it was the other way around:
    BIG FAT WARNING: because the DTs were so far lying by not
    exposing the fact that the GPC block is actually the first
    interrupt controller in the chain, kernels with this patch
    applied wont have any suspend-resume facility when booted
    with old DTs, and old kernels with updated DTs won't even boot.

... perhaps combined with something else. I don't remember.

Nevertheless, the ugly DTS entries are just this - ugly. We should have
a way to remove them eventually.
-- 
Krzysztof Halasa

Industrial Research Institute for Automation and Measurements PIAP
Al. Jerozolimskie 202, 02-486 Warsaw, Poland

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Fabio Estevam]

Hi Bjorn,

On Wed, Mar 30, 2016 at 10:29 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Montag, den 28.03.2016, 18:45 -0300 schrieb Fabio Estevam:
>> From: Fabio Estevam <fabio.estevam@nxp.com>
>>
>> Commit 5c5fb40de8f14 ("PCI: imx6: Add support for active-low reset GPIO")
>> cause regressions on some boards like MX6 Gateworks Ventana, for example.
>>
>> The reason for the breakage is that this commit sets the gpio polarity
>> in the wrong logic level.
>>
>> Also, the commit log is wrong because active-low reset GPIO is what the
>> driver used to support since the beginning.
>>
>> So keep the old behavior that ignores the gpio polarity specified in
>> the device tree and treat the PCI reset GPIO as active-low.
>>
>> Cc: <stable@vger.kernel.org> # 4.5
>> Reported-by: Krzysztof Hałasa <khalasa@piap.pl>
>> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
>
> We need to keep compatibility to existing DTs that are setting the GPIO
> active polarity in the wrong way, so this revert is correct:
>
> Acked-by: Lucas Stach <l.stach@pengutronix.de>

Any chance to get this into 4.6-rc3 to fix the regression?

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Bjorn Helgaas]

Hi Fabio,

On Mon, Mar 28, 2016 at 06:45:36PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Commit 5c5fb40de8f14 ("PCI: imx6: Add support for active-low reset GPIO")
> cause regressions on some boards like MX6 Gateworks Ventana, for example.
> 
> The reason for the breakage is that this commit sets the gpio polarity
> in the wrong logic level.
> 
> Also, the commit log is wrong because active-low reset GPIO is what the
> driver used to support since the beginning.
> 
> So keep the old behavior that ignores the gpio polarity specified in
> the device tree and treat the PCI reset GPIO as active-low.
> 
> Cc: <stable@vger.kernel.org> # 4.5
> Reported-by: Krzysztof Hałasa <khalasa@piap.pl>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>

Applied with acks from Tim and Lucas to for-linus for v4.6, thanks!

If I understand correctly, the revert fixes Gateworks Ventana, but
breaks Toradex Apalis Ixora.  It would be really nice to get Petr's
patches to fix that system in at the same time.  Looks like we just
need a minor documentation update there?

Bjorn

> ---
> Changes since v1:
> - Improve wording of commit log (Tim)
> 
>  drivers/pci/host/pci-imx6.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index eb5a275..2f817fa 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -32,7 +32,7 @@
>  #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
>  
>  struct imx6_pcie {
> -	struct gpio_desc	*reset_gpio;
> +	int			reset_gpio;
>  	struct clk		*pcie_bus;
>  	struct clk		*pcie_phy;
>  	struct clk		*pcie;
> @@ -309,10 +309,10 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
>  	usleep_range(200, 500);
>  
>  	/* Some boards don't have PCIe reset GPIO. */
> -	if (imx6_pcie->reset_gpio) {
> -		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
> +	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
>  		msleep(100);
> -		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
> +		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
>  	}
>  	return 0;
>  
> @@ -523,6 +523,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  {
>  	struct imx6_pcie *imx6_pcie;
>  	struct pcie_port *pp;
> +	struct device_node *np = pdev->dev.of_node;
>  	struct resource *dbi_base;
>  	struct device_node *node = pdev->dev.of_node;
>  	int ret;
> @@ -544,8 +545,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(pp->dbi_base);
>  
>  	/* Fetch GPIOs */
> -	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
> -							GPIOD_OUT_LOW);
> +	imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> +	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +		ret = devm_gpio_request_one(&pdev->dev, imx6_pcie->reset_gpio,
> +					    GPIOF_OUT_INIT_LOW, "PCIe reset");
> +		if (ret) {
> +			dev_err(&pdev->dev, "unable to get reset gpio\n");
> +			return ret;
> +		}
> +	}
>  
>  	/* Fetch clocks */
>  	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Fabio Estevam]

On Tue, Apr 5, 2016 at 6:31 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:

> Applied with acks from Tim and Lucas to for-linus for v4.6, thanks!
>
> If I understand correctly, the revert fixes Gateworks Ventana, but
> breaks Toradex Apalis Ixora.  It would be really nice to get Petr's
> patches to fix that system in at the same time.  Looks like we just
> need a minor documentation update there?

Yes, that's correct.

Re: [PATCH v2] Revert "PCI: imx6: Add support for active-low reset GPIO"

[Fabio Estevam]

Bjorn,

On Tue, Apr 5, 2016 at 6:31 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:

> Applied with acks from Tim and Lucas to for-linus for v4.6, thanks!

Any chance for this patch getting applied to 4.6-rc5?