PCI: imx6: Fix reset-gpio to work with active low GPIO

PATCH Superseded Petr Štetiar 3 participants 9 messages Nov 16, 2015 – Dec 9, 2015

Subsystems (2)

Arm i.MX and Layerscape PCI

lore

Share & tools

lore

Petr Štetiar

author

2015-11-16 15:19 UTC #0 lore raw

PATCH 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.

In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.

Signed-off-by Petr ?tetiar <[email protected]>

---
 drivers/pci/host/pci-imx6.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

drivers/pci/host/pci-imx6.c +11 -4

index fdb9536..cd7ed71 100644
@@ -293,9 +293,11 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 
 	/* Some boards don't have PCIe reset GPIO. */
 	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value(imx6_pcie->reset_gpio, 0);
+		struct gpio_desc *gd = gpio_to_desc(imx6_pcie->reset_gpio);
+
+		gpiod_set_value(gd, 0);
 		msleep(100);
-		gpio_set_value(imx6_pcie->reset_gpio, 1);
+		gpiod_set_value(gd, 1);
 	}
 	return 0;

@@ -559,6 +561,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 	struct pcie_port *pp;
 	struct device_node *np = pdev->dev.of_node;
 	struct resource *dbi_base;
+	enum of_gpio_flags of_flags;
+	int flags = GPIOF_OUT_INIT_LOW;
 	int ret;
 
 	imx6_pcie = devm_kzalloc(&pdev->dev, sizeof(*imx6_pcie), GFP_KERNEL);

@@ -578,10 +582,13 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
+	imx6_pcie->reset_gpio = of_get_named_gpio_flags(np, "reset-gpio", 0,
+							&of_flags);
 	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
+		if (of_flags & OF_GPIO_ACTIVE_LOW)
+			flags |= GPIOF_ACTIVE_LOW;
 		ret = devm_gpio_request_one(&pdev->dev, imx6_pcie->reset_gpio,
-					    GPIOF_OUT_INIT_LOW, "PCIe reset");
+					    flags, "PCIe reset");
 		if (ret) {
 			dev_err(&pdev->dev, "unable to get reset gpio\n");
 			return ret;

Signature · 1 line

1.7.9.5

Lucas Stach

2015-11-24 13:55 UTC #1 lore raw

PATCH 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

Hi Petr,

sorry for the delay in reviewing this.

Am Montag, den 16.11.2015, 16:19 +0100 schrieb Petr ?tetiar:

Quoted 4 lines

It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.

I see the need for this change, but I'm not completely happy with the implementation.

Quoted 36 lines

In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.

Signed-off-by Petr ?tetiar <[email protected]>

---
 drivers/pci/host/pci-imx6.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

drivers/pci/host/pci-imx6.c +6 -2

index fdb9536..cd7ed71 100644
@@ -293,9 +293,11 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 
 	/* Some boards don't have PCIe reset GPIO. */
 	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value(imx6_pcie->reset_gpio, 0);
+		struct gpio_desc *gd = gpio_to_desc(imx6_pcie->reset_gpio);
+
+		gpiod_set_value(gd, 0);
 		msleep(100);
-		gpio_set_value(imx6_pcie->reset_gpio, 1);
+		gpiod_set_value(gd, 1);
 	}
 	return 0;

@@ -559,6 +561,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 	struct pcie_port *pp;
 	struct device_node *np = pdev->dev.of_node;
 	struct resource *dbi_base;
+	enum of_gpio_flags of_flags;
+	int flags = GPIOF_OUT_INIT_LOW;
 	int ret;
 
 	imx6_pcie = devm_kzalloc(&pdev->dev, sizeof(*imx6_pcie), GFP_KERNEL);

drivers/pci/host/pci-imx6.c static int __init imx6_pcie_probe(struct platform_device *pdev) · line 586

Quoted 6 lines

drivers/pci/host/pci-imx6.c +2 -1

@@ -578,10 +582,13 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
+	imx6_pcie->reset_gpio = of_get_named_gpio_flags(np, "reset-gpio", 0,
+

Rather than adding all this complexity to this function, this should really be switched over to the descriptor based GPIO interface as a whole. So the above should become a single call to "devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW)" and the dependent code changed accordingly.

Regards,
Lucas

Quoted 10 lines

drivers/pci/host/pci-imx6.c +3 -1

 						&of_flags);
 	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
+		if (of_flags & OF_GPIO_ACTIVE_LOW)
+			flags |= GPIOF_ACTIVE_LOW;
 		ret = devm_gpio_request_one(&pdev->dev, imx6_pcie->reset_gpio,
-					    GPIOF_OUT_INIT_LOW, "PCIe reset");
+					    flags, "PCIe reset");
 		if (ret) {
 			dev_err(&pdev->dev, "unable to get reset gpio\n");

return ret;

Signature · 2 lines

Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Petr Štetiar

author

2015-11-27 10:01 UTC #2 lore raw

PATCH v2 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.

In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.

Signed-off-by Petr ?tetiar <[email protected]>

---

Changes since v1:
* reworked to use descriptor based GPIO interface

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

drivers/pci/host/pci-imx6.c +6 ~~-14~~

index 22e8224..c0e121d 100644
@@ -32,7 +32,7 @@
 #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
 
 struct imx6_pcie {
-	int			reset_gpio;
+	struct gpio_desc	*reset_gpio;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie;

@@ -287,10 +287,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 (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value(imx6_pcie->reset_gpio, 0);
+	if (imx6_pcie->reset_gpio) {
+		gpiod_set_value(imx6_pcie->reset_gpio, 0);
 		msleep(100);
-		gpio_set_value(imx6_pcie->reset_gpio, 1);
+		gpiod_set_value(imx6_pcie->reset_gpio, 1);
 	}
 	return 0;

@@ -560,7 +560,6 @@ 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;
 	int ret;

@@ -581,15 +580,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	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;
-		}
-	}
+	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
+							GPIOD_OUT_LOW);
 
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");

Signature · 1 line

1.7.9.5

Lucas Stach

2015-11-27 10:34 UTC #3 lore raw

PATCH v2 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

Am Freitag, den 27.11.2015, 11:01 +0100 schrieb Petr ?tetiar:

Quoted 30 lines

It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.
In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.
Signed-off-by Petr ?tetiar <[email protected]>
---
Changes since v1:
* reworked to use descriptor based GPIO interface
 drivers/pci/host/pci-imx6.c |   20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)
drivers/pci/host/pci-imx6.c +1 -1
index 22e8224..c0e121d 100644
@@ -32,7 +32,7 @@
 #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
 
 struct imx6_pcie {
-	int			reset_gpio;
+	struct gpio_desc	*reset_gpio;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie;

drivers/pci/host/pci-imx6.c static int imx6_pcie_deassert_core_reset(struct pcie_port *pp) · line 291

Quoted 7 lines

drivers/pci/host/pci-imx6.c +2 -2

@@ -287,10 +287,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 (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value(imx6_pcie->reset_gpio, 0);
+	if (imx6_pcie->reset_gpio) {
+		gpiod_set_value(imx6_pcie->reset_gpio, 0);

This now collides with a fix from Fabio that allows this function to sleep. So this should use gpiod_set_value_cansleep() and be rebased to pci/host-imx6, or if Bjorn prefers to pull this in quickly, replace Fabios commit.

Regards,
Lucas

Quoted 32 lines

drivers/pci/host/pci-imx6.c +3 ~~-11~~

 		msleep(100);
-		gpio_set_value(imx6_pcie->reset_gpio, 1);
+		gpiod_set_value(imx6_pcie->reset_gpio, 1);
 	}
 	return 0;

@@ -560,7 +560,6 @@ 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;
 	int ret;

@@ -581,15 +580,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	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;
-		}
-	}
+	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
+							GPIOD_OUT_LOW);
 
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");

Signature · 2 lines

Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Petr Štetiar

author

2015-11-27 10:56 UTC #4 lore raw

PATCH v3 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.

In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.

Signed-off-by Petr ?tetiar <[email protected]>

---

Changes since v1:
* reworked to use descriptor based GPIO interface

Changes since v2:
* rebased on pci/host-imx6 which now uses gpio_set_value_cansleep()

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

drivers/pci/host/pci-imx6.c +6 ~~-14~~

index 3c3b37f..8e9afa5 100644
@@ -32,7 +32,7 @@
 #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
 
 struct imx6_pcie {
-	int			reset_gpio;
+	struct gpio_desc	*reset_gpio;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie;

@@ -287,10 +287,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 (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
+	if (imx6_pcie->reset_gpio) {
+		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
 		msleep(100);
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
 	}
 	return 0;

@@ -560,7 +560,6 @@ 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;
 	int ret;

@@ -581,15 +580,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	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;
-		}
-	}
+	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
+							GPIOD_OUT_LOW);
 
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");

Signature · 1 line

1.7.9.5

Lucas Stach

2015-11-27 10:59 UTC #5 lore raw

PATCH v3 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

Am Freitag, den 27.11.2015, 11:56 +0100 schrieb Petr ?tetiar:

Quoted 8 lines

It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.
In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.
Signed-off-by Petr ?tetiar <[email protected]>

Reviewed-by Lucas Stach <[email protected]>

Quoted 64 lines

---

Changes since v1:
* reworked to use descriptor based GPIO interface

Changes since v2:
* rebased on pci/host-imx6 which now uses gpio_set_value_cansleep()

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

drivers/pci/host/pci-imx6.c +6 ~~-14~~

index 3c3b37f..8e9afa5 100644
@@ -32,7 +32,7 @@
 #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
 
 struct imx6_pcie {
-	int			reset_gpio;
+	struct gpio_desc	*reset_gpio;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie;

@@ -287,10 +287,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 (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
+	if (imx6_pcie->reset_gpio) {
+		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
 		msleep(100);
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
 	}
 	return 0;

@@ -560,7 +560,6 @@ 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;
 	int ret;

@@ -581,15 +580,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	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;
-		}
-	}
+	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
+							GPIOD_OUT_LOW);
 
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");

Signature · 2 lines

Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Bjorn Helgaas

Applied

2015-12-04 20:59 UTC #6 lore raw

PATCH v3 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

In reply to Petr Štetiar

Quoted 8 lines · Petr ?tetiar

It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.
In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.
Signed-off-by Petr ?tetiar <[email protected]>

Applied as follows with Lucas' reviewed-by to pci/host-imx6 for v4.5, thanks!

commit 5c5fb40de8f14391a1238db05cef88754faf9229

Author: Petr ?tetiar <[email protected]>
Date:   Fri Nov 27 11:56:34 2015 +0100

PCI: imx6: Add support for active-low reset GPIO

We previously used of_get_named_gpio(), which ignores the OF flags cell, so the reset GPIO defaulted to "active high." This doesn't work on the Toradex Apalis SoM with Ixora base board, which has an active-low reset GPIO.

Use devm_gpiod_get_optional() instead so we pay attention to the active high/low flag. This also adds support for GPIOs described via ACPI.

[bhelgaas: changelog]

Signed-off-by Petr ?tetiar <[email protected]>

Signed-off-by Bjorn Helgaas <[email protected]>

Reviewed-by Lucas Stach <[email protected]>

drivers/pci/host/pci-imx6.c +6 ~~-14~~

index 3c3b37f..8e9afa5 100644
@@ -32,7 +32,7 @@
 #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
 
 struct imx6_pcie {
-	int			reset_gpio;
+	struct gpio_desc	*reset_gpio;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie;

@@ -287,10 +287,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 (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
+	if (imx6_pcie->reset_gpio) {
+		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
 		msleep(100);
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
 	}
 	return 0;

@@ -560,7 +560,6 @@ 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;
 	int ret;

@@ -581,15 +580,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	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;
-		}
-	}
+	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
+							GPIOD_OUT_LOW);
 
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");

Bjorn Helgaas

2015-12-04 21:02 UTC #7 lore raw

PATCH v3 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

[+cc Jingoo]

Quoted 5 lines · Bjorn Helgaas

Quoted 4 lines · Petr ?tetiar
It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.

Quoted 7 lines

Quoted 4 lines
In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.
Signed-off-by Petr ?tetiar <[email protected]>
Applied as follows with Lucas' reviewed-by to pci/host-imx6 for v4.5, thanks!

Jingoo, I noticed pci-exynos.c also uses of_get_named_gpio(). Do you care about this issue, too, or are you content to ignore the GPIO flags for your "reset-gpio"?

Bjorn

Quoted 71 lines

commit 5c5fb40de8f14391a1238db05cef88754faf9229

Author: Petr ?tetiar <[email protected]>
Date:   Fri Nov 27 11:56:34 2015 +0100

PCI: imx6: Add support for active-low reset GPIO

Use devm_gpiod_get_optional() instead so we pay attention to the active high/low flag. This also adds support for GPIOs described via ACPI.

[bhelgaas: changelog]

Signed-off-by Petr ?tetiar <[email protected]>

Signed-off-by Bjorn Helgaas <[email protected]>

Reviewed-by Lucas Stach <[email protected]>

drivers/pci/host/pci-imx6.c +6 ~~-14~~

index 3c3b37f..8e9afa5 100644
@@ -32,7 +32,7 @@
 #define to_imx6_pcie(x)	container_of(x, struct imx6_pcie, pp)
 
 struct imx6_pcie {
-	int			reset_gpio;
+	struct gpio_desc	*reset_gpio;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie;

@@ -287,10 +287,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 (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
+	if (imx6_pcie->reset_gpio) {
+		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 0);
 		msleep(100);
-		gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
+		gpiod_set_value_cansleep(imx6_pcie->reset_gpio, 1);
 	}
 	return 0;

@@ -560,7 +560,6 @@ 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;
 	int ret;

@@ -581,15 +580,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(pp->dbi_base);
 
 	/* Fetch GPIOs */
-	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;
-		}
-	}
+	imx6_pcie->reset_gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
+							GPIOD_OUT_LOW);
 
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");

Bjorn Helgaas

2015-12-09 15:09 UTC #8 lore raw

PATCH v3 1/1 PCI: imx6: Fix reset-gpio to work with active low GPIO

[Jingoo's response didn't make it to the list because it contained HTML, but I incorporated it below manually]

Quoted 17 lines · Han Jingoo

Quoted 16 lines · Bjorn Helgaas
[+cc Jingoo]
Quoted 12 lines · Bjorn Helgaas
Quoted 8 lines · Petr ?tetiar
It's currently not possible to use reset-gpio on board where the GPIO is active low as the code incorrectly assumes, that reset-gpio will be always active high.
In my case the broken behavior was observed on Toradex Apalis SoM with Ixora base board.
Signed-off-by Petr ?tetiar <[email protected]>
Applied as follows with Lucas' reviewed-by to pci/host-imx6 for v4.5, thanks!

Quoted 6 lines

Quoted 3 lines
Jingoo, I noticed pci-exynos.c also uses of_get_named_gpio(). Do you care about this issue, too, or are you content to ignore the GPIO flags for your "reset-gpio"?
Sorry, I don't care about it.
Also, there is no issue with Exynos platform.

OK, thanks for checking!

Bjorn