* [PATCH] PCI: imx6: Fix reset-gpio to work with active low GPIO @ 2015-11-16 15:19 Petr Štetiar 2015-11-24 13:55 ` Lucas Stach 0 siblings, 1 reply; 9+ messages in thread From: Petr Štetiar @ 2015-11-16 15:19 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-pci, Bjorn Helgaas, Lucas Stach, Richard Zhu, 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 <ynezz@true.cz> --- drivers/pci/host/pci-imx6.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index fdb9536..cd7ed71 100644 --- a/drivers/pci/host/pci-imx6.c +++ b/drivers/pci/host/pci-imx6.c @@ -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; -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] PCI: imx6: Fix reset-gpio to work with active low GPIO 2015-11-16 15:19 [PATCH] PCI: imx6: Fix reset-gpio to work with active low GPIO Petr Štetiar @ 2015-11-24 13:55 ` Lucas Stach 2015-11-27 10:01 ` [PATCH v2] " Petr Štetiar 0 siblings, 1 reply; 9+ messages in thread From: Lucas Stach @ 2015-11-24 13:55 UTC (permalink / raw) To: Petr Štetiar; +Cc: linux-arm-kernel, linux-pci, Bjorn Helgaas, Richard Zhu Hi Petr, sorry for the delay in reviewing this. Am Montag, den 16.11.2015, 16:19 +0100 schrieb 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. > I see the need for this change, but I'm not completely happy with the implementation. > In my case the broken behavior was observed on Toradex Apalis SoM with > Ixora base board. > > Signed-off-by: Petr Štetiar <ynezz@true.cz> > --- > drivers/pci/host/pci-imx6.c | 15 +++++++++++---- > 1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index fdb9536..cd7ed71 100644 > --- a/drivers/pci/host/pci-imx6.c > +++ b/drivers/pci/host/pci-imx6.c > @@ -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, > + 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 > &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; -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] PCI: imx6: Fix reset-gpio to work with active low GPIO 2015-11-24 13:55 ` Lucas Stach @ 2015-11-27 10:01 ` Petr Štetiar 2015-11-27 10:34 ` Lucas Stach 0 siblings, 1 reply; 9+ messages in thread From: Petr Štetiar @ 2015-11-27 10:01 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-pci, Bjorn Helgaas, Lucas Stach, Richard Zhu, 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 <ynezz@true.cz> --- Changes since v1: * reworked to use descriptor based GPIO interface drivers/pci/host/pci-imx6.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index 22e8224..c0e121d 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 { - 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"); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] PCI: imx6: Fix reset-gpio to work with active low GPIO 2015-11-27 10:01 ` [PATCH v2] " Petr Štetiar @ 2015-11-27 10:34 ` Lucas Stach 2015-11-27 10:56 ` [PATCH v3] " Petr Štetiar 0 siblings, 1 reply; 9+ messages in thread From: Lucas Stach @ 2015-11-27 10:34 UTC (permalink / raw) To: Petr Štetiar; +Cc: linux-arm-kernel, linux-pci, Bjorn Helgaas, Richard Zhu Am Freitag, den 27.11.2015, 11:01 +0100 schrieb 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 <ynezz@true.cz> > --- > > Changes since v1: > * reworked to use descriptor based GPIO interface > > drivers/pci/host/pci-imx6.c | 20 ++++++-------------- > 1 file changed, 6 insertions(+), 14 deletions(-) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index 22e8224..c0e121d 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 { > - 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); 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 > 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"); -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] PCI: imx6: Fix reset-gpio to work with active low GPIO 2015-11-27 10:34 ` Lucas Stach @ 2015-11-27 10:56 ` Petr Štetiar 2015-11-27 10:59 ` Lucas Stach 2015-12-04 20:59 ` Bjorn Helgaas 0 siblings, 2 replies; 9+ messages in thread From: Petr Štetiar @ 2015-11-27 10:56 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-pci, Bjorn Helgaas, Lucas Stach, Richard Zhu, 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 <ynezz@true.cz> --- 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(-) diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index 3c3b37f..8e9afa5 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 { - 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"); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] PCI: imx6: Fix reset-gpio to work with active low GPIO 2015-11-27 10:56 ` [PATCH v3] " Petr Štetiar @ 2015-11-27 10:59 ` Lucas Stach 2015-12-04 20:59 ` Bjorn Helgaas 1 sibling, 0 replies; 9+ messages in thread From: Lucas Stach @ 2015-11-27 10:59 UTC (permalink / raw) To: Petr Štetiar; +Cc: linux-arm-kernel, linux-pci, Bjorn Helgaas, Richard Zhu Am Freitag, den 27.11.2015, 11:56 +0100 schrieb 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 <ynezz@true.cz> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > --- > > 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(-) > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index 3c3b37f..8e9afa5 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 { > - 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"); -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] PCI: imx6: Fix reset-gpio to work with active low GPIO 2015-11-27 10:56 ` [PATCH v3] " Petr Štetiar 2015-11-27 10:59 ` Lucas Stach @ 2015-12-04 20:59 ` Bjorn Helgaas 2015-12-04 21:02 ` Bjorn Helgaas 1 sibling, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2015-12-04 20:59 UTC (permalink / raw) To: Petr Štetiar Cc: linux-arm-kernel, linux-pci, Bjorn Helgaas, Lucas Stach, Richard Zhu On Fri, Nov 27, 2015 at 11:56:34AM +0100, Petr Štetiar wrote: > 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 <ynezz@true.cz> Applied as follows with Lucas' reviewed-by to pci/host-imx6 for v4.5, thanks! commit 5c5fb40de8f14391a1238db05cef88754faf9229 Author: Petr Štetiar <ynezz@true.cz> 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 <ynezz@true.cz> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index 3c3b37f..8e9afa5 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 { - 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"); ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] PCI: imx6: Fix reset-gpio to work with active low GPIO 2015-12-04 20:59 ` Bjorn Helgaas @ 2015-12-04 21:02 ` Bjorn Helgaas 2015-12-09 15:09 ` Bjorn Helgaas 0 siblings, 1 reply; 9+ messages in thread From: Bjorn Helgaas @ 2015-12-04 21:02 UTC (permalink / raw) To: Petr Štetiar Cc: linux-arm-kernel, linux-pci, Bjorn Helgaas, Lucas Stach, Richard Zhu, Jingoo Han [+cc Jingoo] On Fri, Dec 04, 2015 at 02:59:06PM -0600, Bjorn Helgaas wrote: > On Fri, Nov 27, 2015 at 11:56:34AM +0100, Petr Štetiar wrote: > > 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 <ynezz@true.cz> > > 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 > commit 5c5fb40de8f14391a1238db05cef88754faf9229 > Author: Petr Štetiar <ynezz@true.cz> > 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 <ynezz@true.cz> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> > Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c > index 3c3b37f..8e9afa5 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 { > - 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"); ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3] PCI: imx6: Fix reset-gpio to work with active low GPIO 2015-12-04 21:02 ` Bjorn Helgaas @ 2015-12-09 15:09 ` Bjorn Helgaas 0 siblings, 0 replies; 9+ messages in thread From: Bjorn Helgaas @ 2015-12-09 15:09 UTC (permalink / raw) To: Petr Štetiar Cc: linux-arm-kernel, linux-pci, Bjorn Helgaas, Lucas Stach, Richard Zhu, Jingoo Han [Jingoo's response didn't make it to the list because it contained HTML, but I incorporated it below manually] On Tue, Dec 8, 2015 at 11:28 PM, Han Jingoo wrote: > On Fri, Dec 04, 2015 at 03:02:34PM -0600, Bjorn Helgaas wrote: > > [+cc Jingoo] > > > > On Fri, Dec 04, 2015 at 02:59:06PM -0600, Bjorn Helgaas wrote: > > > On Fri, Nov 27, 2015 at 11:56:34AM +0100, Petr Štetiar wrote: > > > > 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 <ynezz@true.cz> > > > > > > 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"? > > Sorry, I don't care about it. > Also, there is no issue with Exynos platform. OK, thanks for checking! Bjorn ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-12-09 15:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-16 15:19 [PATCH] PCI: imx6: Fix reset-gpio to work with active low GPIO Petr Štetiar 2015-11-24 13:55 ` Lucas Stach 2015-11-27 10:01 ` [PATCH v2] " Petr Štetiar 2015-11-27 10:34 ` Lucas Stach 2015-11-27 10:56 ` [PATCH v3] " Petr Štetiar 2015-11-27 10:59 ` Lucas Stach 2015-12-04 20:59 ` Bjorn Helgaas 2015-12-04 21:02 ` Bjorn Helgaas 2015-12-09 15:09 ` Bjorn Helgaas
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).