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

Commit Message

Fabio Estevam March 28, 2016, 12:49 a.m. UTC

From: Fabio Estevam <[e-mail]>

Commit 5c5fb40de8f14 ("PCI: imx6: Add support for active-low reset GPIO")
has the following issues:

1. It can break old dtb's that didn't take into account the gpio polarity
specified in the device tree (For example: Gateworks Laguna
imx6qdl-gw54xx.dtsi no longer works after this commit)

2. It sets the gpio polarity in the wrong logic level

So revert the commit to avoid such regressions.

Cc: <[e-mail]> # 4.5
Reported-by: Krzysztof Ha?asa <[e-mail]>
Signed-off-by: Fabio Estevam <[e-mail]>
---
 drivers/pci/host/pci-imx6.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

Comments

Tim Harvey March 28, 2016, 10:08 p.m. UTC | #1

On Sun, Mar 27, 2016 at 5:49 PM, Fabio Estevam <[e-mail]> wrote:
> From: Fabio Estevam <[e-mail]>
>
> Commit 5c5fb40de8f14 ("PCI: imx6: Add support for active-low reset GPIO")
> has the following issues:
>

Hi Fabio,

I agree with reverting this - this patch does break PCI for all the
Gateworks Ventana boards.

It's Gateworks 'Ventana' here that uses IMX6 PCI, not Laguna so your
comment is wrong. Also, Ventana boards 'do' have their PCI reset
properly defined as active-low so we probably shouldn't refer to
Gateworks Ventana having gpio polarity not specified in the
device-tree.

It is the fact that the gpio polarity has the wrong logic level in
that patch which breaks things. I think your second point is really
the problem that needs to be mentioned for the reason of the revert.

Other than that you can add a 'Tested-by: Tim Harvey
<[e-mail]>' in reference to Gateworks Ventana PCI.

Thanks,

Tim
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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