[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

Some boards do not have a PCIe reset GPIO. To avoid probe
failure on these boards, make the reset GPIO optional as
well.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
 drivers/pci/host/pci-imx6.c                        | 29 +++++++++++-----------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index d5d26d4..b7a2279 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -19,9 +19,9 @@ Required properties:
 	to define the mapping of the PCIe interface to interrupt
 	numbers.
 - num-lanes: number of lanes to use
-- reset-gpio: gpio pin number of power good signal
 
 Optional properties for fsl,imx6q-pcie
+- reset-gpio: gpio pin number of power good signal
 - power-on-gpio: gpio pin number of power-enable signal
 - wake-up-gpio: gpio pin number of incoming wakeup signal
 - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable signal
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index bd70af8..52027ad 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
 
-	gpio_set_value(imx6_pcie->reset_gpio, 0);
-	msleep(100);
-	gpio_set_value(imx6_pcie->reset_gpio, 1);
+	/* Some boards don't have PCIe reset GPIO. */
+	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
+		gpio_set_value(imx6_pcie->reset_gpio, 0);
+		msleep(100);
+		gpio_set_value(imx6_pcie->reset_gpio, 1);
+	}
 
 	return 0;
 }
@@ -439,17 +442,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 
 	/* Fetch GPIOs */
 	imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
-	if (!gpio_is_valid(imx6_pcie->reset_gpio)) {
-		dev_err(&pdev->dev, "no reset-gpio defined\n");
-		ret = -ENODEV;
-	}
-	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");
-		goto err;
+	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");
+			goto err;
+		}
 	}
 
 	imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
-- 
1.8.4.3

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

Some boards do not have a PCIe reset GPIO. To avoid probe
failure on these boards, make the reset GPIO optional as
well.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
 drivers/pci/host/pci-imx6.c                        | 29 +++++++++++-----------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index d5d26d4..b7a2279 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -19,9 +19,9 @@ Required properties:
 	to define the mapping of the PCIe interface to interrupt
 	numbers.
 - num-lanes: number of lanes to use
-- reset-gpio: gpio pin number of power good signal
 
 Optional properties for fsl,imx6q-pcie
+- reset-gpio: gpio pin number of power good signal
 - power-on-gpio: gpio pin number of power-enable signal
 - wake-up-gpio: gpio pin number of incoming wakeup signal
 - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable signal
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index bd70af8..52027ad 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
 
-	gpio_set_value(imx6_pcie->reset_gpio, 0);
-	msleep(100);
-	gpio_set_value(imx6_pcie->reset_gpio, 1);
+	/* Some boards don't have PCIe reset GPIO. */
+	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
+		gpio_set_value(imx6_pcie->reset_gpio, 0);
+		msleep(100);
+		gpio_set_value(imx6_pcie->reset_gpio, 1);
+	}
 
 	return 0;
 }
@@ -439,17 +442,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 
 	/* Fetch GPIOs */
 	imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
-	if (!gpio_is_valid(imx6_pcie->reset_gpio)) {
-		dev_err(&pdev->dev, "no reset-gpio defined\n");
-		ret = -ENODEV;
-	}
-	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");
-		goto err;
+	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");
+			goto err;
+		}
 	}
 
 	imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
-- 
1.8.4.3

[PATCH 2/7] PCI: imx6: Fix waiting for link up

[Marek Vasut]

While waiting for the PHY to report the PCIe link is up, we might
hit a situation where the link training is still in progress, while
the PHY already reports the link is up. Add additional check for
this condition.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 52027ad..4d5be4e 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -48,6 +48,8 @@ struct imx6_pcie {
 #define PL_OFFSET 0x700
 #define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
 #define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c)
+#define PCIE_PHY_DEBUG_R1_XMLH_LINK_IN_TRAINING	(1 << 29)
+#define PCIE_PHY_DEBUG_R1_XMLH_LINK_UP		(1 << 4)
 
 #define PCIE_PHY_CTRL (PL_OFFSET + 0x114)
 #define PCIE_PHY_CTRL_DATA_LOC 0
@@ -338,10 +340,17 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
 {
 	u32 rc, ltssm, rx_valid, temp;
 
-	/* link is debug bit 36, debug register 1 starts at bit 32 */
-	rc = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1) & (0x1 << (36 - 32));
-	if (rc)
-		return -EAGAIN;
+	/*
+	 * Test if the PHY reports that the link is up and also that
+	 * the link training finished. It might happen that the PHY
+	 * reports the link is already up, but the link training bit
+	 * is still set, so make sure to check the training is done
+	 * as well here.
+	 */
+	rc = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1);
+	if ((rc & PCIE_PHY_DEBUG_R1_XMLH_LINK_UP) &&
+	    !(rc & PCIE_PHY_DEBUG_R1_XMLH_LINK_IN_TRAINING))
+		return 1;
 
 	/*
 	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
-- 
1.8.4.3

[PATCH 2/7] PCI: imx6: Fix waiting for link up

[Marek Vasut]

While waiting for the PHY to report the PCIe link is up, we might
hit a situation where the link training is still in progress, while
the PHY already reports the link is up. Add additional check for
this condition.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 52027ad..4d5be4e 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -48,6 +48,8 @@ struct imx6_pcie {
 #define PL_OFFSET 0x700
 #define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
 #define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c)
+#define PCIE_PHY_DEBUG_R1_XMLH_LINK_IN_TRAINING	(1 << 29)
+#define PCIE_PHY_DEBUG_R1_XMLH_LINK_UP		(1 << 4)
 
 #define PCIE_PHY_CTRL (PL_OFFSET + 0x114)
 #define PCIE_PHY_CTRL_DATA_LOC 0
@@ -338,10 +340,17 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
 {
 	u32 rc, ltssm, rx_valid, temp;
 
-	/* link is debug bit 36, debug register 1 starts@bit 32 */
-	rc = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1) & (0x1 << (36 - 32));
-	if (rc)
-		return -EAGAIN;
+	/*
+	 * Test if the PHY reports that the link is up and also that
+	 * the link training finished. It might happen that the PHY
+	 * reports the link is already up, but the link training bit
+	 * is still set, so make sure to check the training is done
+	 * as well here.
+	 */
+	rc = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1);
+	if ((rc & PCIE_PHY_DEBUG_R1_XMLH_LINK_UP) &&
+	    !(rc & PCIE_PHY_DEBUG_R1_XMLH_LINK_IN_TRAINING))
+		return 1;
 
 	/*
 	 * From L0, initiate MAC entry to gen2 if EP/RC supports gen2.
-- 
1.8.4.3

[PATCH V2 3/7] PCI: imx6: Split away the PHY reset

[Marek Vasut]

Split the PCIe PHY reset from the link up function to make
the code a little more structured.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

V2: Drop unused 'temp' variable.

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 4d5be4e..478e87e 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -336,9 +336,26 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 	return;
 }
 
+static void imx6_pcie_reset_phy(struct pcie_port *pp)
+{
+	uint32_t temp;
+
+	pcie_phy_read(pp->dbi_base, PHY_RX_OVRD_IN_LO, &temp);
+	temp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+		 PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+	pcie_phy_write(pp->dbi_base, PHY_RX_OVRD_IN_LO, temp);
+
+	usleep_range(2000, 3000);
+
+	pcie_phy_read(pp->dbi_base, PHY_RX_OVRD_IN_LO, &temp);
+	temp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+		  PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+	pcie_phy_write(pp->dbi_base, PHY_RX_OVRD_IN_LO, temp);
+}
+
 static int imx6_pcie_link_up(struct pcie_port *pp)
 {
-	u32 rc, ltssm, rx_valid, temp;
+	u32 rc, ltssm, rx_valid;
 
 	/*
 	 * Test if the PHY reports that the link is up and also that
@@ -370,21 +387,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
 
 	dev_err(pp->dev, "transition to gen2 is stuck, reset PHY!\n");
 
-	pcie_phy_read(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, &temp);
-	temp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN
-		| PHY_RX_OVRD_IN_LO_RX_PLL_EN);
-	pcie_phy_write(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, temp);
-
-	usleep_range(2000, 3000);
-
-	pcie_phy_read(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, &temp);
-	temp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN
-		| PHY_RX_OVRD_IN_LO_RX_PLL_EN);
-	pcie_phy_write(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, temp);
+	imx6_pcie_reset_phy(pp);
 
 	return 0;
 }
-- 
1.8.4.3

[PATCH V2 3/7] PCI: imx6: Split away the PHY reset

[Marek Vasut]

Split the PCIe PHY reset from the link up function to make
the code a little more structured.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

V2: Drop unused 'temp' variable.

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 4d5be4e..478e87e 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -336,9 +336,26 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 	return;
 }
 
+static void imx6_pcie_reset_phy(struct pcie_port *pp)
+{
+	uint32_t temp;
+
+	pcie_phy_read(pp->dbi_base, PHY_RX_OVRD_IN_LO, &temp);
+	temp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+		 PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+	pcie_phy_write(pp->dbi_base, PHY_RX_OVRD_IN_LO, temp);
+
+	usleep_range(2000, 3000);
+
+	pcie_phy_read(pp->dbi_base, PHY_RX_OVRD_IN_LO, &temp);
+	temp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+		  PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+	pcie_phy_write(pp->dbi_base, PHY_RX_OVRD_IN_LO, temp);
+}
+
 static int imx6_pcie_link_up(struct pcie_port *pp)
 {
-	u32 rc, ltssm, rx_valid, temp;
+	u32 rc, ltssm, rx_valid;
 
 	/*
 	 * Test if the PHY reports that the link is up and also that
@@ -370,21 +387,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
 
 	dev_err(pp->dev, "transition to gen2 is stuck, reset PHY!\n");
 
-	pcie_phy_read(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, &temp);
-	temp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN
-		| PHY_RX_OVRD_IN_LO_RX_PLL_EN);
-	pcie_phy_write(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, temp);
-
-	usleep_range(2000, 3000);
-
-	pcie_phy_read(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, &temp);
-	temp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN
-		| PHY_RX_OVRD_IN_LO_RX_PLL_EN);
-	pcie_phy_write(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, temp);
+	imx6_pcie_reset_phy(pp);
 
 	return 0;
 }
-- 
1.8.4.3

Re: [PATCH V2 3/7] PCI: imx6: Split away the PHY reset

[Jingoo Han]

On Wednesday, December 11, 2013 7:30 PM, Marek Vasut wrote:
> 
> Split the PCIe PHY reset from the link up function to make
> the code a little more structured.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/pci/host/pci-imx6.c | 35 +++++++++++++++++++----------------
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> V2: Drop unused 'temp' variable.

[PATCH V2 3/7] PCI: imx6: Split away the PHY reset

[Jingoo Han]

On Wednesday, December 11, 2013 7:30 PM, Marek Vasut wrote:
> 
> Split the PCIe PHY reset from the link up function to make
> the code a little more structured.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/pci/host/pci-imx6.c | 35 +++++++++++++++++++----------------
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> V2: Drop unused 'temp' variable.

[PATCH 4/7] PCI: imx6: Split away the link up wait loop

[Marek Vasut]

Split the function that waits for the PCIe link to come up from the
rest if the host init function. We will find this change useful in
the subsequent patch, since this will be called twice then.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 478e87e..4d6f6fe 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -304,6 +304,25 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
 			IMX6Q_GPR8_TX_SWING_LOW, 127 << 25);
 }
 
+static int imx6_pcie_wait_for_link(struct pcie_port *pp)
+{
+	int count = 200;
+
+	while (!dw_pcie_link_up(pp)) {
+		usleep_range(100, 1000);
+		if (--count)
+			continue;
+
+		dev_err(pp->dev, "phy link never came up\n");
+		dev_dbg(pp->dev, "DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
+			readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
+			readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static void imx6_pcie_host_init(struct pcie_port *pp)
 {
 	int count = 0;
@@ -320,18 +339,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
 
-	while (!dw_pcie_link_up(pp)) {
-		usleep_range(100, 1000);
-		count++;
-		if (count >= 200) {
-			dev_err(pp->dev, "phy link never came up\n");
-			dev_dbg(pp->dev,
-				"DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
-				readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
-				readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
-			break;
-		}
-	}
+	imx6_pcie_wait_for_link(pp);
 
 	return;
 }
-- 
1.8.4.3

[PATCH 4/7] PCI: imx6: Split away the link up wait loop

[Marek Vasut]

Split the function that waits for the PCIe link to come up from the
rest if the host init function. We will find this change useful in
the subsequent patch, since this will be called twice then.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 478e87e..4d6f6fe 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -304,6 +304,25 @@ static void imx6_pcie_init_phy(struct pcie_port *pp)
 			IMX6Q_GPR8_TX_SWING_LOW, 127 << 25);
 }
 
+static int imx6_pcie_wait_for_link(struct pcie_port *pp)
+{
+	int count = 200;
+
+	while (!dw_pcie_link_up(pp)) {
+		usleep_range(100, 1000);
+		if (--count)
+			continue;
+
+		dev_err(pp->dev, "phy link never came up\n");
+		dev_dbg(pp->dev, "DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
+			readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
+			readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static void imx6_pcie_host_init(struct pcie_port *pp)
 {
 	int count = 0;
@@ -320,18 +339,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
 
-	while (!dw_pcie_link_up(pp)) {
-		usleep_range(100, 1000);
-		count++;
-		if (count >= 200) {
-			dev_err(pp->dev, "phy link never came up\n");
-			dev_dbg(pp->dev,
-				"DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
-				readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
-				readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
-			break;
-		}
-	}
+	imx6_pcie_wait_for_link(pp);
 
 	return;
 }
-- 
1.8.4.3

[PATCH V2 5/7] PCI: imx6: Fix link start operation

[Marek Vasut]

This patch first forces the link into Gen1 mode before starting up
the link and only after the link is up requests the link to start
negotiating possible Gen2 mode operation. This is because without
such sequence, some PCIe switches are not detected at all.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 78 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 6 deletions(-)

V2: Change the link speed reporting from dev_err() to dev_dbg() so it doesn't
    polute the kernel output.

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 4d6f6fe..3028f2c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -44,6 +44,12 @@ struct imx6_pcie {
 	void __iomem		*mem_base;
 };
 
+/* PCIe Root Complex registers (memory-mapped) */
+#define PCIE_RC_LCR				0x7c
+#define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
+#define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2	0x2
+#define PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK	0xf
+
 /* PCIe Port Logic registers (memory-mapped) */
 #define PL_OFFSET 0x700
 #define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
@@ -61,6 +67,9 @@ struct imx6_pcie {
 #define PCIE_PHY_STAT (PL_OFFSET + 0x110)
 #define PCIE_PHY_STAT_ACK_LOC 16
 
+#define PCIE_LINK_WIDTH_SPEED_CONTROL	0x80C
+#define PORT_LOGIC_SPEED_CHANGE		(0x1 << 17)
+
 /* PHY registers (not memory-mapped) */
 #define PCIE_PHY_RX_ASIC_OUT 0x100D
 
@@ -323,11 +332,71 @@ static int imx6_pcie_wait_for_link(struct pcie_port *pp)
 	return 0;
 }
 
-static void imx6_pcie_host_init(struct pcie_port *pp)
+static int imx6_pcie_start_link(struct pcie_port *pp)
 {
-	int count = 0;
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
+	uint32_t tmp;
+	int ret, count;
+
+	/*
+	 * Force Gen1 operation when starting the link. In case the link is
+	 * started in Gen2 mode, there is a possibility the devices on the
+	 * bus will not be detected at all. This happens with PCIe switches.
+	 */
+	tmp = readl(pp->dbi_base + PCIE_RC_LCR);
+	tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
+	tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
+	writel(tmp, pp->dbi_base + PCIE_RC_LCR);
+
+	/* Start LTSSM. */
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+
+	ret = imx6_pcie_wait_for_link(pp);
+	if (ret)
+		return ret;
+
+	/* Allow Gen2 mode after the link is up. */
+	tmp = readl(pp->dbi_base + PCIE_RC_LCR);
+	tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
+	tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2;
+	writel(tmp, pp->dbi_base + PCIE_RC_LCR);
+
+	/*
+	 * Start Directed Speed Change so the best possible speed both link
+	 * partners support can be negotiated.
+	 */
+	tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+	tmp |= PORT_LOGIC_SPEED_CHANGE;
+	writel(tmp, pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+
+	count = 200;
+	while (count--) {
+		tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+		/* Test if the speed change finished. */
+		if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
+			break;
+		usleep_range(100, 1000);
+	}
+
+	/* Make sure link training is finished as well! */
+	if (count)
+		ret = imx6_pcie_wait_for_link(pp);
+	else
+		ret = -EINVAL;
 
+	if (ret) {
+		dev_err(pp->dev, "Failed to bring link up!\n");
+	} else {
+		tmp = readl(pp->dbi_base + 0x80);
+		dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
+	}
+
+	return ret;
+}
+
+static void imx6_pcie_host_init(struct pcie_port *pp)
+{
 	imx6_pcie_assert_core_reset(pp);
 
 	imx6_pcie_init_phy(pp);
@@ -336,10 +405,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 
 	dw_pcie_setup_rc(pp);
 
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
-
-	imx6_pcie_wait_for_link(pp);
+	imx6_pcie_start_link(pp);
 
 	return;
 }
-- 
1.8.4.3

[PATCH V2 5/7] PCI: imx6: Fix link start operation

[Marek Vasut]

This patch first forces the link into Gen1 mode before starting up
the link and only after the link is up requests the link to start
negotiating possible Gen2 mode operation. This is because without
such sequence, some PCIe switches are not detected at all.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 78 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 6 deletions(-)

V2: Change the link speed reporting from dev_err() to dev_dbg() so it doesn't
    polute the kernel output.

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 4d6f6fe..3028f2c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -44,6 +44,12 @@ struct imx6_pcie {
 	void __iomem		*mem_base;
 };
 
+/* PCIe Root Complex registers (memory-mapped) */
+#define PCIE_RC_LCR				0x7c
+#define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
+#define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2	0x2
+#define PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK	0xf
+
 /* PCIe Port Logic registers (memory-mapped) */
 #define PL_OFFSET 0x700
 #define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
@@ -61,6 +67,9 @@ struct imx6_pcie {
 #define PCIE_PHY_STAT (PL_OFFSET + 0x110)
 #define PCIE_PHY_STAT_ACK_LOC 16
 
+#define PCIE_LINK_WIDTH_SPEED_CONTROL	0x80C
+#define PORT_LOGIC_SPEED_CHANGE		(0x1 << 17)
+
 /* PHY registers (not memory-mapped) */
 #define PCIE_PHY_RX_ASIC_OUT 0x100D
 
@@ -323,11 +332,71 @@ static int imx6_pcie_wait_for_link(struct pcie_port *pp)
 	return 0;
 }
 
-static void imx6_pcie_host_init(struct pcie_port *pp)
+static int imx6_pcie_start_link(struct pcie_port *pp)
 {
-	int count = 0;
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
+	uint32_t tmp;
+	int ret, count;
+
+	/*
+	 * Force Gen1 operation when starting the link. In case the link is
+	 * started in Gen2 mode, there is a possibility the devices on the
+	 * bus will not be detected@all. This happens with PCIe switches.
+	 */
+	tmp = readl(pp->dbi_base + PCIE_RC_LCR);
+	tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
+	tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1;
+	writel(tmp, pp->dbi_base + PCIE_RC_LCR);
+
+	/* Start LTSSM. */
+	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+
+	ret = imx6_pcie_wait_for_link(pp);
+	if (ret)
+		return ret;
+
+	/* Allow Gen2 mode after the link is up. */
+	tmp = readl(pp->dbi_base + PCIE_RC_LCR);
+	tmp &= ~PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK;
+	tmp |= PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN2;
+	writel(tmp, pp->dbi_base + PCIE_RC_LCR);
+
+	/*
+	 * Start Directed Speed Change so the best possible speed both link
+	 * partners support can be negotiated.
+	 */
+	tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+	tmp |= PORT_LOGIC_SPEED_CHANGE;
+	writel(tmp, pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+
+	count = 200;
+	while (count--) {
+		tmp = readl(pp->dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
+		/* Test if the speed change finished. */
+		if (!(tmp & PORT_LOGIC_SPEED_CHANGE))
+			break;
+		usleep_range(100, 1000);
+	}
+
+	/* Make sure link training is finished as well! */
+	if (count)
+		ret = imx6_pcie_wait_for_link(pp);
+	else
+		ret = -EINVAL;
 
+	if (ret) {
+		dev_err(pp->dev, "Failed to bring link up!\n");
+	} else {
+		tmp = readl(pp->dbi_base + 0x80);
+		dev_dbg(pp->dev, "Link up, Gen=%i\n", (tmp >> 16) & 0xf);
+	}
+
+	return ret;
+}
+
+static void imx6_pcie_host_init(struct pcie_port *pp)
+{
 	imx6_pcie_assert_core_reset(pp);
 
 	imx6_pcie_init_phy(pp);
@@ -336,10 +405,7 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 
 	dw_pcie_setup_rc(pp);
 
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
-
-	imx6_pcie_wait_for_link(pp);
+	imx6_pcie_start_link(pp);
 
 	return;
 }
-- 
1.8.4.3

[PATCH 6/7] PCI: imx6: Fix bugs in PCIe startup code

[Marek Vasut]

From: Richard Zhu <r65037@freescale.com>

- LTSSM shouldn't be set once in assert_core_reset
- Move peripheral reset just before LTSSM start

From: Richard Zhu <r65037@freescale.com>
Signed-off-by: Richard Zhu <r65037@freescale.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 3028f2c..38df103 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -220,18 +220,9 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
 
-	/* Some boards don't have PCIe reset GPIO. */
-	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value(imx6_pcie->reset_gpio, 0);
-		msleep(100);
-		gpio_set_value(imx6_pcie->reset_gpio, 1);
-	}
-
 	return 0;
 }
 
@@ -275,6 +266,12 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 	/* allow the clocks to stabilize */
 	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);
+		msleep(100);
+		gpio_set_value(imx6_pcie->reset_gpio, 1);
+	}
 	return 0;
 
 err_pcie_axi:
-- 
1.8.4.3

[PATCH 6/7] PCI: imx6: Fix bugs in PCIe startup code

[Marek Vasut]

From: Richard Zhu <r65037@freescale.com>

- LTSSM shouldn't be set once in assert_core_reset
- Move peripheral reset just before LTSSM start

From: Richard Zhu <r65037@freescale.com>
Signed-off-by: Richard Zhu <r65037@freescale.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 3028f2c..38df103 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -220,18 +220,9 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_TEST_PD, 1 << 18);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
 
-	/* Some boards don't have PCIe reset GPIO. */
-	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
-		gpio_set_value(imx6_pcie->reset_gpio, 0);
-		msleep(100);
-		gpio_set_value(imx6_pcie->reset_gpio, 1);
-	}
-
 	return 0;
 }
 
@@ -275,6 +266,12 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 	/* allow the clocks to stabilize */
 	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);
+		msleep(100);
+		gpio_set_value(imx6_pcie->reset_gpio, 1);
+	}
 	return 0;
 
 err_pcie_axi:
-- 
1.8.4.3

[PATCH 7/7] ARM: dts: imx6q-sabrelite: Enable PCI express

[Marek Vasut]

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 arch/arm/boot/dts/imx6q-sabrelite.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index fca8f220..f570ad5 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -190,6 +190,10 @@
 	};
 };
 
+&pcie {
+	status = "okay";
+};
+
 &sata {
 	status = "okay";
 };
-- 
1.8.4.3

[PATCH 7/7] ARM: dts: imx6q-sabrelite: Enable PCI express

[Marek Vasut]

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 arch/arm/boot/dts/imx6q-sabrelite.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
index fca8f220..f570ad5 100644
--- a/arch/arm/boot/dts/imx6q-sabrelite.dts
+++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
@@ -190,6 +190,10 @@
 	};
 };
 
+&pcie {
+	status = "okay";
+};
+
 &sata {
 	status = "okay";
 };
-- 
1.8.4.3

Re: [PATCH 7/7] ARM: dts: imx6q-sabrelite: Enable PCI express

[Shawn Guo]

On Wed, Dec 11, 2013 at 11:30:18AM +0100, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>

Applied, thanks.

> ---
>  arch/arm/boot/dts/imx6q-sabrelite.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index fca8f220..f570ad5 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -190,6 +190,10 @@
>  	};
>  };
>  
> +&pcie {
> +	status = "okay";
> +};
> +
>  &sata {
>  	status = "okay";
>  };
> -- 
> 1.8.4.3
> 

[PATCH 7/7] ARM: dts: imx6q-sabrelite: Enable PCI express

[Shawn Guo]

On Wed, Dec 11, 2013 at 11:30:18AM +0100, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>

Applied, thanks.

> ---
>  arch/arm/boot/dts/imx6q-sabrelite.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx6q-sabrelite.dts b/arch/arm/boot/dts/imx6q-sabrelite.dts
> index fca8f220..f570ad5 100644
> --- a/arch/arm/boot/dts/imx6q-sabrelite.dts
> +++ b/arch/arm/boot/dts/imx6q-sabrelite.dts
> @@ -190,6 +190,10 @@
>  	};
>  };
>  
> +&pcie {
> +	status = "okay";
> +};
> +
>  &sata {
>  	status = "okay";
>  };
> -- 
> 1.8.4.3
> 

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Jingoo Han]

On Wednesday, December 11, 2013 7:30 PM, Marek Vasut wrote:
> 
> Some boards do not have a PCIe reset GPIO. To avoid probe
> failure on these boards, make the reset GPIO optional as
> well.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> ---
>  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
>  drivers/pci/host/pci-imx6.c                        | 29 +++++++++++-----------
>  2 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index d5d26d4..b7a2279 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -19,9 +19,9 @@ Required properties:
>  	to define the mapping of the PCIe interface to interrupt
>  	numbers.
>  - num-lanes: number of lanes to use
> -- reset-gpio: gpio pin number of power good signal
> 
>  Optional properties for fsl,imx6q-pcie
> +- reset-gpio: gpio pin number of power good signal

(+cc Kishon Vijay Abraham I)

As I said earlier,
'reset-gpio' property can be used for 'samsung,exynos5440-pcie'.
Thus, it is not specific for 'fsl,imx6q-pcie'.

Please, modify it as below:

--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -19,6 +19,8 @@ Required properties:
        to define the mapping of the PCIe interface to interrupt
        numbers.
 - num-lanes: number of lanes to use
+
+Optional properties
 - reset-gpio: gpio pin number of power good signal

 Optional properties for fsl,imx6q-pcie

Best regards,
Jingoo Han

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Jingoo Han]

On Wednesday, December 11, 2013 7:30 PM, Marek Vasut wrote:
> 
> Some boards do not have a PCIe reset GPIO. To avoid probe
> failure on these boards, make the reset GPIO optional as
> well.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> ---
>  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
>  drivers/pci/host/pci-imx6.c                        | 29 +++++++++++-----------
>  2 files changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index d5d26d4..b7a2279 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -19,9 +19,9 @@ Required properties:
>  	to define the mapping of the PCIe interface to interrupt
>  	numbers.
>  - num-lanes: number of lanes to use
> -- reset-gpio: gpio pin number of power good signal
> 
>  Optional properties for fsl,imx6q-pcie
> +- reset-gpio: gpio pin number of power good signal

(+cc Kishon Vijay Abraham I)

As I said earlier,
'reset-gpio' property can be used for 'samsung,exynos5440-pcie'.
Thus, it is not specific for 'fsl,imx6q-pcie'.

Please, modify it as below:

--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -19,6 +19,8 @@ Required properties:
        to define the mapping of the PCIe interface to interrupt
        numbers.
 - num-lanes: number of lanes to use
+
+Optional properties
 - reset-gpio: gpio pin number of power good signal

 Optional properties for fsl,imx6q-pcie

Best regards,
Jingoo Han

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Tim Harvey]

On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> wrote:
>
> Some boards do not have a PCIe reset GPIO. To avoid probe
> failure on these boards, make the reset GPIO optional as
> well.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> ---
>  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
>  drivers/pci/host/pci-imx6.c                        | 29 +++++++++++-----------
>  2 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index d5d26d4..b7a2279 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -19,9 +19,9 @@ Required properties:
>         to define the mapping of the PCIe interface to interrupt
>         numbers.
>  - num-lanes: number of lanes to use
> -- reset-gpio: gpio pin number of power good signal
>
>  Optional properties for fsl,imx6q-pcie
> +- reset-gpio: gpio pin number of power good signal
>  - power-on-gpio: gpio pin number of power-enable signal
>  - wake-up-gpio: gpio pin number of incoming wakeup signal
>  - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable signal
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index bd70af8..52027ad 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
>         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>                         IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
>
> -       gpio_set_value(imx6_pcie->reset_gpio, 0);
> -       msleep(100);
> -       gpio_set_value(imx6_pcie->reset_gpio, 1);
> +       /* Some boards don't have PCIe reset GPIO. */
> +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +               gpio_set_value(imx6_pcie->reset_gpio, 0);
> +               msleep(100);
> +               gpio_set_value(imx6_pcie->reset_gpio, 1);
> +       }
>
>         return 0;
>  }

Marek,

Though not the fault of your patch, I noticed while looking at this
that the PCI Express specification is not being properly met with
regards to PERST# and the reference clock.  The spec states that
PERST# must be kept asserted until after the reference clock is stable
(I'm not entirely clear how long of a delay is needed for the clock to
become stable but I think the value is typically the 100ms).  I see in
the current pci-imx6.c code that imx6_pcie_host_init calls
imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed
by imx6_pcie_deassert_core_reset.  Despite the function names,
imx6_pcie_assert_core_reset as shown above asserts then de-asserts
PERST# before the clock is enabled in imx6_pcie_deassert_core_reset.
This seems to me to be a violation of the spec and I believe the
msleep(100) and de-assertion of the option reset_gpio should be done
in imx6_pcie_deassert_core reset after the clock is brought up.

If you agree with my assessment, would you mind resolving this issue
at the same time?  If not, I'm happy to follow-up with a patch to
resolve it after your patch is accepted.

Thanks,

Tim

> @@ -439,17 +442,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>
>         /* Fetch GPIOs */
>         imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> -       if (!gpio_is_valid(imx6_pcie->reset_gpio)) {
> -               dev_err(&pdev->dev, "no reset-gpio defined\n");
> -               ret = -ENODEV;
> -       }
> -       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");
> -               goto err;
> +       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");
> +                       goto err;
> +               }
>         }
>
>         imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
> --
> 1.8.4.3
>

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Tim Harvey]

On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> wrote:
>
> Some boards do not have a PCIe reset GPIO. To avoid probe
> failure on these boards, make the reset GPIO optional as
> well.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Frank Li <lznuaa@gmail.com>
> Cc: Harro Haan <hrhaan@gmail.com>
> Cc: Jingoo Han <jg1.han@samsung.com>
> Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> Cc: Pratyush Anand <pratyush.anand@st.com>
> Cc: Richard Zhu <r65037@freescale.com>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Sean Cross <xobs@kosagi.com>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> Cc: Tim Harvey <tharvey@gateworks.com>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> Cc: Yinghai Lu <yinghai@kernel.org>
> ---
>  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
>  drivers/pci/host/pci-imx6.c                        | 29 +++++++++++-----------
>  2 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index d5d26d4..b7a2279 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -19,9 +19,9 @@ Required properties:
>         to define the mapping of the PCIe interface to interrupt
>         numbers.
>  - num-lanes: number of lanes to use
> -- reset-gpio: gpio pin number of power good signal
>
>  Optional properties for fsl,imx6q-pcie
> +- reset-gpio: gpio pin number of power good signal
>  - power-on-gpio: gpio pin number of power-enable signal
>  - wake-up-gpio: gpio pin number of incoming wakeup signal
>  - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable signal
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index bd70af8..52027ad 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
>         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>                         IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
>
> -       gpio_set_value(imx6_pcie->reset_gpio, 0);
> -       msleep(100);
> -       gpio_set_value(imx6_pcie->reset_gpio, 1);
> +       /* Some boards don't have PCIe reset GPIO. */
> +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> +               gpio_set_value(imx6_pcie->reset_gpio, 0);
> +               msleep(100);
> +               gpio_set_value(imx6_pcie->reset_gpio, 1);
> +       }
>
>         return 0;
>  }

Marek,

Though not the fault of your patch, I noticed while looking at this
that the PCI Express specification is not being properly met with
regards to PERST# and the reference clock.  The spec states that
PERST# must be kept asserted until after the reference clock is stable
(I'm not entirely clear how long of a delay is needed for the clock to
become stable but I think the value is typically the 100ms).  I see in
the current pci-imx6.c code that imx6_pcie_host_init calls
imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed
by imx6_pcie_deassert_core_reset.  Despite the function names,
imx6_pcie_assert_core_reset as shown above asserts then de-asserts
PERST# before the clock is enabled in imx6_pcie_deassert_core_reset.
This seems to me to be a violation of the spec and I believe the
msleep(100) and de-assertion of the option reset_gpio should be done
in imx6_pcie_deassert_core reset after the clock is brought up.

If you agree with my assessment, would you mind resolving this issue
at the same time?  If not, I'm happy to follow-up with a patch to
resolve it after your patch is accepted.

Thanks,

Tim

> @@ -439,17 +442,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>
>         /* Fetch GPIOs */
>         imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
> -       if (!gpio_is_valid(imx6_pcie->reset_gpio)) {
> -               dev_err(&pdev->dev, "no reset-gpio defined\n");
> -               ret = -ENODEV;
> -       }
> -       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");
> -               goto err;
> +       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");
> +                       goto err;
> +               }
>         }
>
>         imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
> --
> 1.8.4.3
>

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> wrote:
> > Some boards do not have a PCIe reset GPIO. To avoid probe
> > failure on these boards, make the reset GPIO optional as
> > well.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Frank Li <lznuaa@gmail.com>
> > Cc: Harro Haan <hrhaan@gmail.com>
> > Cc: Jingoo Han <jg1.han@samsung.com>
> > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > Cc: Pratyush Anand <pratyush.anand@st.com>
> > Cc: Richard Zhu <r65037@freescale.com>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Sean Cross <xobs@kosagi.com>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > Cc: Tim Harvey <tharvey@gateworks.com>
> > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > Cc: Yinghai Lu <yinghai@kernel.org>
> > ---
> > 
> >  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
> >  drivers/pci/host/pci-imx6.c                        | 29
> >  +++++++++++----------- 2 files changed, 16 insertions(+), 15
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > b/Documentation/devicetree/bindings/pci/designware-pcie.txt index
> > d5d26d4..b7a2279 100644
> > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > 
> > @@ -19,9 +19,9 @@ Required properties:
> >         to define the mapping of the PCIe interface to interrupt
> >         numbers.
> >  
> >  - num-lanes: number of lanes to use
> > 
> > -- reset-gpio: gpio pin number of power good signal
> > 
> >  Optional properties for fsl,imx6q-pcie
> > 
> > +- reset-gpio: gpio pin number of power good signal
> > 
> >  - power-on-gpio: gpio pin number of power-enable signal
> >  - wake-up-gpio: gpio pin number of incoming wakeup signal
> >  - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable
> >  signal
> > 
> > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > index bd70af8..52027ad 100644
> > --- a/drivers/pci/host/pci-imx6.c
> > +++ b/drivers/pci/host/pci-imx6.c
> > @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct
> > pcie_port *pp)
> > 
> >         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> >         
> >                         IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> > 
> > -       gpio_set_value(imx6_pcie->reset_gpio, 0);
> > -       msleep(100);
> > -       gpio_set_value(imx6_pcie->reset_gpio, 1);
> > +       /* Some boards don't have PCIe reset GPIO. */
> > +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > +               gpio_set_value(imx6_pcie->reset_gpio, 0);
> > +               msleep(100);
> > +               gpio_set_value(imx6_pcie->reset_gpio, 1);
> > +       }
> > 
> >         return 0;
> >  
> >  }
> 
> Marek,
> 
> Though not the fault of your patch, I noticed while looking at this
> that the PCI Express specification is not being properly met with
> regards to PERST# and the reference clock.  The spec states that
> PERST# must be kept asserted until after the reference clock is stable
> (I'm not entirely clear how long of a delay is needed for the clock to
> become stable but I think the value is typically the 100ms).  I see in
> the current pci-imx6.c code that imx6_pcie_host_init calls
> imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed
> by imx6_pcie_deassert_core_reset.  Despite the function names,
> imx6_pcie_assert_core_reset as shown above asserts then de-asserts
> PERST# before the clock is enabled in imx6_pcie_deassert_core_reset.
> This seems to me to be a violation of the spec and I believe the
> msleep(100) and de-assertion of the option reset_gpio should be done
> in imx6_pcie_deassert_core reset after the clock is brought up.
> 
> If you agree with my assessment, would you mind resolving this issue
> at the same time?  If not, I'm happy to follow-up with a patch to
> resolve it after your patch is accepted.

Is this not resolved by patch 0006 in this series please?

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> wrote:
> > Some boards do not have a PCIe reset GPIO. To avoid probe
> > failure on these boards, make the reset GPIO optional as
> > well.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Frank Li <lznuaa@gmail.com>
> > Cc: Harro Haan <hrhaan@gmail.com>
> > Cc: Jingoo Han <jg1.han@samsung.com>
> > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > Cc: Pratyush Anand <pratyush.anand@st.com>
> > Cc: Richard Zhu <r65037@freescale.com>
> > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Sean Cross <xobs@kosagi.com>
> > Cc: Shawn Guo <shawn.guo@linaro.org>
> > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > Cc: Tim Harvey <tharvey@gateworks.com>
> > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > Cc: Yinghai Lu <yinghai@kernel.org>
> > ---
> > 
> >  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
> >  drivers/pci/host/pci-imx6.c                        | 29
> >  +++++++++++----------- 2 files changed, 16 insertions(+), 15
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > b/Documentation/devicetree/bindings/pci/designware-pcie.txt index
> > d5d26d4..b7a2279 100644
> > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > 
> > @@ -19,9 +19,9 @@ Required properties:
> >         to define the mapping of the PCIe interface to interrupt
> >         numbers.
> >  
> >  - num-lanes: number of lanes to use
> > 
> > -- reset-gpio: gpio pin number of power good signal
> > 
> >  Optional properties for fsl,imx6q-pcie
> > 
> > +- reset-gpio: gpio pin number of power good signal
> > 
> >  - power-on-gpio: gpio pin number of power-enable signal
> >  - wake-up-gpio: gpio pin number of incoming wakeup signal
> >  - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable
> >  signal
> > 
> > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > index bd70af8..52027ad 100644
> > --- a/drivers/pci/host/pci-imx6.c
> > +++ b/drivers/pci/host/pci-imx6.c
> > @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct
> > pcie_port *pp)
> > 
> >         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> >         
> >                         IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> > 
> > -       gpio_set_value(imx6_pcie->reset_gpio, 0);
> > -       msleep(100);
> > -       gpio_set_value(imx6_pcie->reset_gpio, 1);
> > +       /* Some boards don't have PCIe reset GPIO. */
> > +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > +               gpio_set_value(imx6_pcie->reset_gpio, 0);
> > +               msleep(100);
> > +               gpio_set_value(imx6_pcie->reset_gpio, 1);
> > +       }
> > 
> >         return 0;
> >  
> >  }
> 
> Marek,
> 
> Though not the fault of your patch, I noticed while looking at this
> that the PCI Express specification is not being properly met with
> regards to PERST# and the reference clock.  The spec states that
> PERST# must be kept asserted until after the reference clock is stable
> (I'm not entirely clear how long of a delay is needed for the clock to
> become stable but I think the value is typically the 100ms).  I see in
> the current pci-imx6.c code that imx6_pcie_host_init calls
> imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed
> by imx6_pcie_deassert_core_reset.  Despite the function names,
> imx6_pcie_assert_core_reset as shown above asserts then de-asserts
> PERST# before the clock is enabled in imx6_pcie_deassert_core_reset.
> This seems to me to be a violation of the spec and I believe the
> msleep(100) and de-assertion of the option reset_gpio should be done
> in imx6_pcie_deassert_core reset after the clock is brought up.
> 
> If you agree with my assessment, would you mind resolving this issue
> at the same time?  If not, I'm happy to follow-up with a patch to
> resolve it after your patch is accepted.

Is this not resolved by patch 0006 in this series please?

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Tim Harvey]

On Thu, Dec 12, 2013 at 2:22 AM, Marek Vasut <marex@denx.de> wrote:
>
> On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> > On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> wrote:
> > > Some boards do not have a PCIe reset GPIO. To avoid probe
> > > failure on these boards, make the reset GPIO optional as
> > > well.
> > >
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Frank Li <lznuaa@gmail.com>
> > > Cc: Harro Haan <hrhaan@gmail.com>
> > > Cc: Jingoo Han <jg1.han@samsung.com>
> > > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > > Cc: Pratyush Anand <pratyush.anand@st.com>
> > > Cc: Richard Zhu <r65037@freescale.com>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Sean Cross <xobs@kosagi.com>
> > > Cc: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > > Cc: Tim Harvey <tharvey@gateworks.com>
> > > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > > Cc: Yinghai Lu <yinghai@kernel.org>
> > > ---
> > >
> > >  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
> > >  drivers/pci/host/pci-imx6.c                        | 29
> > >  +++++++++++----------- 2 files changed, 16 insertions(+), 15
> > >  deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > b/Documentation/devicetree/bindings/pci/designware-pcie.txt index
> > > d5d26d4..b7a2279 100644
> > > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > >
> > > @@ -19,9 +19,9 @@ Required properties:
> > >         to define the mapping of the PCIe interface to interrupt
> > >         numbers.
> > >
> > >  - num-lanes: number of lanes to use
> > >
> > > -- reset-gpio: gpio pin number of power good signal
> > >
> > >  Optional properties for fsl,imx6q-pcie
> > >
> > > +- reset-gpio: gpio pin number of power good signal
> > >
> > >  - power-on-gpio: gpio pin number of power-enable signal
> > >  - wake-up-gpio: gpio pin number of incoming wakeup signal
> > >  - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable
> > >  signal
> > >
> > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > > index bd70af8..52027ad 100644
> > > --- a/drivers/pci/host/pci-imx6.c
> > > +++ b/drivers/pci/host/pci-imx6.c
> > > @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct
> > > pcie_port *pp)
> > >
> > >         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > >
> > >                         IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> > >
> > > -       gpio_set_value(imx6_pcie->reset_gpio, 0);
> > > -       msleep(100);
> > > -       gpio_set_value(imx6_pcie->reset_gpio, 1);
> > > +       /* Some boards don't have PCIe reset GPIO. */
> > > +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > > +               gpio_set_value(imx6_pcie->reset_gpio, 0);
> > > +               msleep(100);
> > > +               gpio_set_value(imx6_pcie->reset_gpio, 1);
> > > +       }
> > >
> > >         return 0;
> > >
> > >  }
> >
> > Marek,
> >
> > Though not the fault of your patch, I noticed while looking at this
> > that the PCI Express specification is not being properly met with
> > regards to PERST# and the reference clock.  The spec states that
> > PERST# must be kept asserted until after the reference clock is stable
> > (I'm not entirely clear how long of a delay is needed for the clock to
> > become stable but I think the value is typically the 100ms).  I see in
> > the current pci-imx6.c code that imx6_pcie_host_init calls
> > imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed
> > by imx6_pcie_deassert_core_reset.  Despite the function names,
> > imx6_pcie_assert_core_reset as shown above asserts then de-asserts
> > PERST# before the clock is enabled in imx6_pcie_deassert_core_reset.
> > This seems to me to be a violation of the spec and I believe the
> > msleep(100) and de-assertion of the option reset_gpio should be done
> > in imx6_pcie_deassert_core reset after the clock is brought up.
> >
> > If you agree with my assessment, would you mind resolving this issue
> > at the same time?  If not, I'm happy to follow-up with a patch to
> > resolve it after your patch is accepted.
>
> Is this not resolved by patch 0006 in this series please?

Marek,

Yes it is addressed there.  Sorry - I missed that.

Tim

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Tim Harvey]

On Thu, Dec 12, 2013 at 2:22 AM, Marek Vasut <marex@denx.de> wrote:
>
> On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> > On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> wrote:
> > > Some boards do not have a PCIe reset GPIO. To avoid probe
> > > failure on these boards, make the reset GPIO optional as
> > > well.
> > >
> > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Frank Li <lznuaa@gmail.com>
> > > Cc: Harro Haan <hrhaan@gmail.com>
> > > Cc: Jingoo Han <jg1.han@samsung.com>
> > > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > > Cc: Pratyush Anand <pratyush.anand@st.com>
> > > Cc: Richard Zhu <r65037@freescale.com>
> > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Sean Cross <xobs@kosagi.com>
> > > Cc: Shawn Guo <shawn.guo@linaro.org>
> > > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > > Cc: Tim Harvey <tharvey@gateworks.com>
> > > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > > Cc: Yinghai Lu <yinghai@kernel.org>
> > > ---
> > >
> > >  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
> > >  drivers/pci/host/pci-imx6.c                        | 29
> > >  +++++++++++----------- 2 files changed, 16 insertions(+), 15
> > >  deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > b/Documentation/devicetree/bindings/pci/designware-pcie.txt index
> > > d5d26d4..b7a2279 100644
> > > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > >
> > > @@ -19,9 +19,9 @@ Required properties:
> > >         to define the mapping of the PCIe interface to interrupt
> > >         numbers.
> > >
> > >  - num-lanes: number of lanes to use
> > >
> > > -- reset-gpio: gpio pin number of power good signal
> > >
> > >  Optional properties for fsl,imx6q-pcie
> > >
> > > +- reset-gpio: gpio pin number of power good signal
> > >
> > >  - power-on-gpio: gpio pin number of power-enable signal
> > >  - wake-up-gpio: gpio pin number of incoming wakeup signal
> > >  - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable
> > >  signal
> > >
> > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > > index bd70af8..52027ad 100644
> > > --- a/drivers/pci/host/pci-imx6.c
> > > +++ b/drivers/pci/host/pci-imx6.c
> > > @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct
> > > pcie_port *pp)
> > >
> > >         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > >
> > >                         IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> > >
> > > -       gpio_set_value(imx6_pcie->reset_gpio, 0);
> > > -       msleep(100);
> > > -       gpio_set_value(imx6_pcie->reset_gpio, 1);
> > > +       /* Some boards don't have PCIe reset GPIO. */
> > > +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > > +               gpio_set_value(imx6_pcie->reset_gpio, 0);
> > > +               msleep(100);
> > > +               gpio_set_value(imx6_pcie->reset_gpio, 1);
> > > +       }
> > >
> > >         return 0;
> > >
> > >  }
> >
> > Marek,
> >
> > Though not the fault of your patch, I noticed while looking at this
> > that the PCI Express specification is not being properly met with
> > regards to PERST# and the reference clock.  The spec states that
> > PERST# must be kept asserted until after the reference clock is stable
> > (I'm not entirely clear how long of a delay is needed for the clock to
> > become stable but I think the value is typically the 100ms).  I see in
> > the current pci-imx6.c code that imx6_pcie_host_init calls
> > imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed
> > by imx6_pcie_deassert_core_reset.  Despite the function names,
> > imx6_pcie_assert_core_reset as shown above asserts then de-asserts
> > PERST# before the clock is enabled in imx6_pcie_deassert_core_reset.
> > This seems to me to be a violation of the spec and I believe the
> > msleep(100) and de-assertion of the option reset_gpio should be done
> > in imx6_pcie_deassert_core reset after the clock is brought up.
> >
> > If you agree with my assessment, would you mind resolving this issue
> > at the same time?  If not, I'm happy to follow-up with a patch to
> > resolve it after your patch is accepted.
>
> Is this not resolved by patch 0006 in this series please?

Marek,

Yes it is addressed there.  Sorry - I missed that.

Tim

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

On Thursday, December 12, 2013 at 07:16:18 PM, Tim Harvey wrote:
> On Thu, Dec 12, 2013 at 2:22 AM, Marek Vasut <marex@denx.de> wrote:
> > On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> > > On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> wrote:
> > > > Some boards do not have a PCIe reset GPIO. To avoid probe
> > > > failure on these boards, make the reset GPIO optional as
> > > > well.
> > > > 
> > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > Cc: Frank Li <lznuaa@gmail.com>
> > > > Cc: Harro Haan <hrhaan@gmail.com>
> > > > Cc: Jingoo Han <jg1.han@samsung.com>
> > > > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > > > Cc: Pratyush Anand <pratyush.anand@st.com>
> > > > Cc: Richard Zhu <r65037@freescale.com>
> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Cc: Sean Cross <xobs@kosagi.com>
> > > > Cc: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > > > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > > > Cc: Tim Harvey <tharvey@gateworks.com>
> > > > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > > > Cc: Yinghai Lu <yinghai@kernel.org>
> > > > ---
> > > > 
> > > >  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
> > > >  drivers/pci/host/pci-imx6.c                        | 29
> > > >  +++++++++++----------- 2 files changed, 16 insertions(+), 15
> > > >  deletions(-)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > > b/Documentation/devicetree/bindings/pci/designware-pcie.txt index
> > > > d5d26d4..b7a2279 100644
> > > > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > > 
> > > > @@ -19,9 +19,9 @@ Required properties:
> > > >         to define the mapping of the PCIe interface to interrupt
> > > >         numbers.
> > > >  
> > > >  - num-lanes: number of lanes to use
> > > > 
> > > > -- reset-gpio: gpio pin number of power good signal
> > > > 
> > > >  Optional properties for fsl,imx6q-pcie
> > > > 
> > > > +- reset-gpio: gpio pin number of power good signal
> > > > 
> > > >  - power-on-gpio: gpio pin number of power-enable signal
> > > >  - wake-up-gpio: gpio pin number of incoming wakeup signal
> > > >  - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable
> > > >  signal
> > > > 
> > > > diff --git a/drivers/pci/host/pci-imx6.c
> > > > b/drivers/pci/host/pci-imx6.c index bd70af8..52027ad 100644
> > > > --- a/drivers/pci/host/pci-imx6.c
> > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct
> > > > pcie_port *pp)
> > > > 
> > > >         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > > >         
> > > >                         IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> > > > 
> > > > -       gpio_set_value(imx6_pcie->reset_gpio, 0);
> > > > -       msleep(100);
> > > > -       gpio_set_value(imx6_pcie->reset_gpio, 1);
> > > > +       /* Some boards don't have PCIe reset GPIO. */
> > > > +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > > > +               gpio_set_value(imx6_pcie->reset_gpio, 0);
> > > > +               msleep(100);
> > > > +               gpio_set_value(imx6_pcie->reset_gpio, 1);
> > > > +       }
> > > > 
> > > >         return 0;
> > > >  
> > > >  }
> > > 
> > > Marek,
> > > 
> > > Though not the fault of your patch, I noticed while looking at this
> > > that the PCI Express specification is not being properly met with
> > > regards to PERST# and the reference clock.  The spec states that
> > > PERST# must be kept asserted until after the reference clock is stable
> > > (I'm not entirely clear how long of a delay is needed for the clock to
> > > become stable but I think the value is typically the 100ms).  I see in
> > > the current pci-imx6.c code that imx6_pcie_host_init calls
> > > imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed
> > > by imx6_pcie_deassert_core_reset.  Despite the function names,
> > > imx6_pcie_assert_core_reset as shown above asserts then de-asserts
> > > PERST# before the clock is enabled in imx6_pcie_deassert_core_reset.
> > > This seems to me to be a violation of the spec and I believe the
> > > msleep(100) and de-assertion of the option reset_gpio should be done
> > > in imx6_pcie_deassert_core reset after the clock is brought up.
> > > 
> > > If you agree with my assessment, would you mind resolving this issue
> > > at the same time?  If not, I'm happy to follow-up with a patch to
> > > resolve it after your patch is accepted.
> > 
> > Is this not resolved by patch 0006 in this series please?
> 
> Marek,
> 
> Yes it is addressed there.  Sorry - I missed that.

No problem, thanks for checking it :)

Bjorn, would you mind picking 2/7...6/7 of this series so we are done with 
those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact 
on the rest of the series.

Shawn, can you please pick 7/7 ?

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

On Thursday, December 12, 2013 at 07:16:18 PM, Tim Harvey wrote:
> On Thu, Dec 12, 2013 at 2:22 AM, Marek Vasut <marex@denx.de> wrote:
> > On Thursday, December 12, 2013 at 06:10:31 AM, Tim Harvey wrote:
> > > On Wed, Dec 11, 2013 at 2:30 AM, Marek Vasut <marex@denx.de> wrote:
> > > > Some boards do not have a PCIe reset GPIO. To avoid probe
> > > > failure on these boards, make the reset GPIO optional as
> > > > well.
> > > > 
> > > > Signed-off-by: Marek Vasut <marex@denx.de>
> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > Cc: Frank Li <lznuaa@gmail.com>
> > > > Cc: Harro Haan <hrhaan@gmail.com>
> > > > Cc: Jingoo Han <jg1.han@samsung.com>
> > > > Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
> > > > Cc: Pratyush Anand <pratyush.anand@st.com>
> > > > Cc: Richard Zhu <r65037@freescale.com>
> > > > Cc: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Cc: Sean Cross <xobs@kosagi.com>
> > > > Cc: Shawn Guo <shawn.guo@linaro.org>
> > > > Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
> > > > Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
> > > > Cc: Tim Harvey <tharvey@gateworks.com>
> > > > Cc: Troy Kisky <troy.kisky@boundarydevices.com>
> > > > Cc: Yinghai Lu <yinghai@kernel.org>
> > > > ---
> > > > 
> > > >  .../devicetree/bindings/pci/designware-pcie.txt    |  2 +-
> > > >  drivers/pci/host/pci-imx6.c                        | 29
> > > >  +++++++++++----------- 2 files changed, 16 insertions(+), 15
> > > >  deletions(-)
> > > > 
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > > b/Documentation/devicetree/bindings/pci/designware-pcie.txt index
> > > > d5d26d4..b7a2279 100644
> > > > --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > > +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> > > > 
> > > > @@ -19,9 +19,9 @@ Required properties:
> > > >         to define the mapping of the PCIe interface to interrupt
> > > >         numbers.
> > > >  
> > > >  - num-lanes: number of lanes to use
> > > > 
> > > > -- reset-gpio: gpio pin number of power good signal
> > > > 
> > > >  Optional properties for fsl,imx6q-pcie
> > > > 
> > > > +- reset-gpio: gpio pin number of power good signal
> > > > 
> > > >  - power-on-gpio: gpio pin number of power-enable signal
> > > >  - wake-up-gpio: gpio pin number of incoming wakeup signal
> > > >  - disable-gpio: gpio pin number of outgoing rfkill/endpoint disable
> > > >  signal
> > > > 
> > > > diff --git a/drivers/pci/host/pci-imx6.c
> > > > b/drivers/pci/host/pci-imx6.c index bd70af8..52027ad 100644
> > > > --- a/drivers/pci/host/pci-imx6.c
> > > > +++ b/drivers/pci/host/pci-imx6.c
> > > > @@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct
> > > > pcie_port *pp)
> > > > 
> > > >         regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
> > > >         
> > > >                         IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
> > > > 
> > > > -       gpio_set_value(imx6_pcie->reset_gpio, 0);
> > > > -       msleep(100);
> > > > -       gpio_set_value(imx6_pcie->reset_gpio, 1);
> > > > +       /* Some boards don't have PCIe reset GPIO. */
> > > > +       if (gpio_is_valid(imx6_pcie->reset_gpio)) {
> > > > +               gpio_set_value(imx6_pcie->reset_gpio, 0);
> > > > +               msleep(100);
> > > > +               gpio_set_value(imx6_pcie->reset_gpio, 1);
> > > > +       }
> > > > 
> > > >         return 0;
> > > >  
> > > >  }
> > > 
> > > Marek,
> > > 
> > > Though not the fault of your patch, I noticed while looking at this
> > > that the PCI Express specification is not being properly met with
> > > regards to PERST# and the reference clock.  The spec states that
> > > PERST# must be kept asserted until after the reference clock is stable
> > > (I'm not entirely clear how long of a delay is needed for the clock to
> > > become stable but I think the value is typically the 100ms).  I see in
> > > the current pci-imx6.c code that imx6_pcie_host_init calls
> > > imx6_pcie_assert_core_reset first, then imx6_pcie_init_phy, followed
> > > by imx6_pcie_deassert_core_reset.  Despite the function names,
> > > imx6_pcie_assert_core_reset as shown above asserts then de-asserts
> > > PERST# before the clock is enabled in imx6_pcie_deassert_core_reset.
> > > This seems to me to be a violation of the spec and I believe the
> > > msleep(100) and de-assertion of the option reset_gpio should be done
> > > in imx6_pcie_deassert_core reset after the clock is brought up.
> > > 
> > > If you agree with my assessment, would you mind resolving this issue
> > > at the same time?  If not, I'm happy to follow-up with a patch to
> > > resolve it after your patch is accepted.
> > 
> > Is this not resolved by patch 0006 in this series please?
> 
> Marek,
> 
> Yes it is addressed there.  Sorry - I missed that.

No problem, thanks for checking it :)

Bjorn, would you mind picking 2/7...6/7 of this series so we are done with 
those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact 
on the rest of the series.

Shawn, can you please pick 7/7 ?

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Bjorn Helgaas]

On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
> on the rest of the series.

Sure.  I can't really evaluate these myself, so I'm looking for acks
from Richard or Shawn.

I noticed Pratyush, Tim, and Harro had some questions the first time
around, and I don't know whether they got resolved.

Bjorn

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Bjorn Helgaas]

On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
> on the rest of the series.

Sure.  I can't really evaluate these myself, so I'm looking for acks
from Richard or Shawn.

I noticed Pratyush, Tim, and Harro had some questions the first time
around, and I don't know whether they got resolved.

Bjorn

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Bjorn Helgaas]

On Thu, Dec 12, 2013 at 2:07 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
>> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
>> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
>> on the rest of the series.
>
> Sure.  I can't really evaluate these myself, so I'm looking for acks
> from Richard or Shawn.
>
> I noticed Pratyush, Tim, and Harro had some questions the first time
> around, and I don't know whether they got resolved.

By the way, I tried to apply patches 2-6 to my pci/host-imx6 branch,
but patch 6 ("PCI: imx6: Fix bugs in PCIe startup code") didn't apply
cleanly.

I pushed the branch to
http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6
if you want to try it yourself.

Bjorn

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Bjorn Helgaas]

On Thu, Dec 12, 2013 at 2:07 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
>> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
>> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
>> on the rest of the series.
>
> Sure.  I can't really evaluate these myself, so I'm looking for acks
> from Richard or Shawn.
>
> I noticed Pratyush, Tim, and Harro had some questions the first time
> around, and I don't know whether they got resolved.

By the way, I tried to apply patches 2-6 to my pci/host-imx6 branch,
but patch 6 ("PCI: imx6: Fix bugs in PCIe startup code") didn't apply
cleanly.

I pushed the branch to
http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-imx6
if you want to try it yourself.

Bjorn

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

On Thursday, December 12, 2013 at 10:20:30 PM, Bjorn Helgaas wrote:
> On Thu, Dec 12, 2013 at 2:07 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
> >> Bjorn, would you mind picking 2/7...6/7 of this series so we are done
> >> with those? I will re-post 1/7 as a self-standing patch afterwards, it
> >> has no impact on the rest of the series.
> > 
> > Sure.  I can't really evaluate these myself, so I'm looking for acks
> > from Richard or Shawn.
> > 
> > I noticed Pratyush, Tim, and Harro had some questions the first time
> > around, and I don't know whether they got resolved.
> 
> By the way, I tried to apply patches 2-6 to my pci/host-imx6 branch,
> but patch 6 ("PCI: imx6: Fix bugs in PCIe startup code") didn't apply
> cleanly.
> 
> I pushed the branch to
> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host
> -imx6 if you want to try it yourself.

That's because of Fabio's patch. I will rebase and repost, thanks.

Best regards,
Marek Vasut

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

On Thursday, December 12, 2013 at 10:20:30 PM, Bjorn Helgaas wrote:
> On Thu, Dec 12, 2013 at 2:07 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
> >> Bjorn, would you mind picking 2/7...6/7 of this series so we are done
> >> with those? I will re-post 1/7 as a self-standing patch afterwards, it
> >> has no impact on the rest of the series.
> > 
> > Sure.  I can't really evaluate these myself, so I'm looking for acks
> > from Richard or Shawn.
> > 
> > I noticed Pratyush, Tim, and Harro had some questions the first time
> > around, and I don't know whether they got resolved.
> 
> By the way, I tried to apply patches 2-6 to my pci/host-imx6 branch,
> but patch 6 ("PCI: imx6: Fix bugs in PCIe startup code") didn't apply
> cleanly.
> 
> I pushed the branch to
> http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host
> -imx6 if you want to try it yourself.

That's because of Fabio's patch. I will rebase and repost, thanks.

Best regards,
Marek Vasut

Re: [PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Harro Haan]

On 12 December 2013 22:07, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
>> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
>> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
>> on the rest of the series.
>
> Sure.  I can't really evaluate these myself, so I'm looking for acks
> from Richard or Shawn.
>
> I noticed Pratyush, Tim, and Harro had some questions the first time
> around, and I don't know whether they got resolved.

Yes, Marek resolved the build warning comment in [PATCH V2 3/7]. Regards, Harro

[PATCH 1/7] PCI: imx6: Make reset-gpio optional

[Harro Haan]

On 12 December 2013 22:07, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Dec 12, 2013 at 11:25 AM, Marek Vasut <marex@denx.de> wrote:
>> Bjorn, would you mind picking 2/7...6/7 of this series so we are done with
>> those? I will re-post 1/7 as a self-standing patch afterwards, it has no impact
>> on the rest of the series.
>
> Sure.  I can't really evaluate these myself, so I'm looking for acks
> from Richard or Shawn.
>
> I noticed Pratyush, Tim, and Harro had some questions the first time
> around, and I don't know whether they got resolved.

Yes, Marek resolved the build warning comment in [PATCH V2 3/7]. Regards, Harro

[PATCH V2 1/7] PCI: imx6: Make reset-gpio optional

[Marek Vasut]

Some boards do not have a PCIe reset GPIO. To avoid probe
failure on these boards, make the reset GPIO optional as
well.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 .../devicetree/bindings/pci/designware-pcie.txt    |  2 ++
 drivers/pci/host/pci-imx6.c                        | 29 +++++++++++-----------
 2 files changed, 17 insertions(+), 14 deletions(-)

V2: Make the 'reset-gpio' prop shared between imx6 and exynos
    Rebase on top of pci/host-imx6

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index d5d26d4..d6fae13 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -19,6 +19,8 @@ Required properties:
 	to define the mapping of the PCIe interface to interrupt
 	numbers.
 - num-lanes: number of lanes to use
+
+Optional properties:
 - reset-gpio: gpio pin number of power good signal
 
 Optional properties for fsl,imx6q-pcie
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 9fc1cb6..c247602 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -214,9 +214,12 @@ static int imx6_pcie_assert_core_reset(struct pcie_port *pp)
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_REF_CLK_EN, 0 << 16);
 
-	gpio_set_value(imx6_pcie->reset_gpio, 0);
-	msleep(100);
-	gpio_set_value(imx6_pcie->reset_gpio, 1);
+	/* Some boards don't have PCIe reset GPIO. */
+	if (gpio_is_valid(imx6_pcie->reset_gpio)) {
+		gpio_set_value(imx6_pcie->reset_gpio, 0);
+		msleep(100);
+		gpio_set_value(imx6_pcie->reset_gpio, 1);
+	}
 
 	return 0;
 }
@@ -432,17 +435,15 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 
 	/* Fetch GPIOs */
 	imx6_pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
-	if (!gpio_is_valid(imx6_pcie->reset_gpio)) {
-		dev_err(&pdev->dev, "no reset-gpio defined\n");
-		ret = -ENODEV;
-	}
-	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;
+	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->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
-- 
1.8.4.3

[PATCH V2 3/7] PCI: imx6: Split away the PHY reset

[Marek Vasut]

Split the PCIe PHY reset from the link up function to make
the code a little more structured.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

V2: Drop unused 'temp' variable.

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index f5541c3..4983784 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -336,9 +336,26 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 	return;
 }
 
+static void imx6_pcie_reset_phy(struct pcie_port *pp)
+{
+	uint32_t temp;
+
+	pcie_phy_read(pp->dbi_base, PHY_RX_OVRD_IN_LO, &temp);
+	temp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+		 PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+	pcie_phy_write(pp->dbi_base, PHY_RX_OVRD_IN_LO, temp);
+
+	usleep_range(2000, 3000);
+
+	pcie_phy_read(pp->dbi_base, PHY_RX_OVRD_IN_LO, &temp);
+	temp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+		  PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+	pcie_phy_write(pp->dbi_base, PHY_RX_OVRD_IN_LO, temp);
+}
+
 static int imx6_pcie_link_up(struct pcie_port *pp)
 {
-	u32 rc, ltssm, rx_valid, temp;
+	u32 rc, ltssm, rx_valid;
 
 	/*
 	 * Test if the PHY reports that the link is up and also that
@@ -370,21 +387,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
 
 	dev_err(pp->dev, "transition to gen2 is stuck, reset PHY!\n");
 
-	pcie_phy_read(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, &temp);
-	temp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN
-		| PHY_RX_OVRD_IN_LO_RX_PLL_EN);
-	pcie_phy_write(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, temp);
-
-	usleep_range(2000, 3000);
-
-	pcie_phy_read(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, &temp);
-	temp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN
-		| PHY_RX_OVRD_IN_LO_RX_PLL_EN);
-	pcie_phy_write(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, temp);
+	imx6_pcie_reset_phy(pp);
 
 	return 0;
 }
-- 
1.8.4.3

[PATCH V2 3/7] PCI: imx6: Split away the PHY reset

[Marek Vasut]

Split the PCIe PHY reset from the link up function to make
the code a little more structured.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Frank Li <lznuaa@gmail.com>
Cc: Harro Haan <hrhaan@gmail.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Mohit KUMAR <Mohit.KUMAR@st.com>
Cc: Pratyush Anand <pratyush.anand@st.com>
Cc: Richard Zhu <r65037@freescale.com>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Sean Cross <xobs@kosagi.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Siva Reddy Kallam <siva.kallam@samsung.com>
Cc: Srikanth T Shivanand <ts.srikanth@samsung.com>
Cc: Tim Harvey <tharvey@gateworks.com>
Cc: Troy Kisky <troy.kisky@boundarydevices.com>
Cc: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/host/pci-imx6.c | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)

V2: Drop unused 'temp' variable.

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index f5541c3..4983784 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -336,9 +336,26 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 	return;
 }
 
+static void imx6_pcie_reset_phy(struct pcie_port *pp)
+{
+	uint32_t temp;
+
+	pcie_phy_read(pp->dbi_base, PHY_RX_OVRD_IN_LO, &temp);
+	temp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+		 PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+	pcie_phy_write(pp->dbi_base, PHY_RX_OVRD_IN_LO, temp);
+
+	usleep_range(2000, 3000);
+
+	pcie_phy_read(pp->dbi_base, PHY_RX_OVRD_IN_LO, &temp);
+	temp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+		  PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+	pcie_phy_write(pp->dbi_base, PHY_RX_OVRD_IN_LO, temp);
+}
+
 static int imx6_pcie_link_up(struct pcie_port *pp)
 {
-	u32 rc, ltssm, rx_valid, temp;
+	u32 rc, ltssm, rx_valid;
 
 	/*
 	 * Test if the PHY reports that the link is up and also that
@@ -370,21 +387,7 @@ static int imx6_pcie_link_up(struct pcie_port *pp)
 
 	dev_err(pp->dev, "transition to gen2 is stuck, reset PHY!\n");
 
-	pcie_phy_read(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, &temp);
-	temp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN
-		| PHY_RX_OVRD_IN_LO_RX_PLL_EN);
-	pcie_phy_write(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, temp);
-
-	usleep_range(2000, 3000);
-
-	pcie_phy_read(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, &temp);
-	temp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN
-		| PHY_RX_OVRD_IN_LO_RX_PLL_EN);
-	pcie_phy_write(pp->dbi_base,
-		PHY_RX_OVRD_IN_LO, temp);
+	imx6_pcie_reset_phy(pp);
 
 	return 0;
 }
-- 
1.8.4.3