[PATCH v4 0/3] i.MX7 PCI support

[PATCH v4 0/3] i.MX7 PCI support

[Andrey Smirnov]

Hello, everyone:

This is a fourth iteration of the code that adds PCI-subsystem bits
necessary for enabling PCI support on i.MX7.

Changes since v3 (can be found at [version3]):
	- Move all of the reset_control_assert's into imx6_pcie_assert_core_reset
	- Documented required reset and power domain DT bindings

Changes since v2 (can be found at [version2]):

	- Collected Reviewed-by for patch #2 from Lucas
	- Reset logic implementation moved out into a reset controller
          driver (see [reset1])
	- Removed unused leftover code

Changes since v1 (can be found at [version1]):

	- All GPC related code moved into a separate driver (see [gpc1])
	- Removed GPIO probe deferral logging
	- Fixed section mismatch warning
	- Minor reformatting of fsl,imx6q-pcie.txt(as per Rob
          Herring's request)

[version3] https://lkml.org/lkml/2017/2/6/565
[version2] https://lkml.org/lkml/2017/2/1/510
[version1] https://lkml.org/lkml/2017/1/19/488
[gpc1] https://lkml.org/lkml/2017/2/6/551
[reset1] https://lkml.org/lkml/2017/2/6/554

Andrey Smirnov (3):
  PCI: imx6: Fix a typo in error message
  PCI: imx6: Allow probe deferal by reset GPIO
  PCI: imx6: Add code to support i.MX7D

 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 +-
 drivers/pci/host/pci-imx6.c                        | 131 ++++++++++++++++-----
 include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
 3 files changed, 118 insertions(+), 30 deletions(-)

-- 
2.9.3

[PATCH v4 0/3] i.MX7 PCI support

[Andrey Smirnov]

Hello, everyone:

This is a fourth iteration of the code that adds PCI-subsystem bits
necessary for enabling PCI support on i.MX7.

Changes since v3 (can be found at [version3]):
	- Move all of the reset_control_assert's into imx6_pcie_assert_core_reset
	- Documented required reset and power domain DT bindings

Changes since v2 (can be found at [version2]):

	- Collected Reviewed-by for patch #2 from Lucas
	- Reset logic implementation moved out into a reset controller
          driver (see [reset1])
	- Removed unused leftover code

Changes since v1 (can be found at [version1]):

	- All GPC related code moved into a separate driver (see [gpc1])
	- Removed GPIO probe deferral logging
	- Fixed section mismatch warning
	- Minor reformatting of fsl,imx6q-pcie.txt(as per Rob
          Herring's request)

[version3] https://lkml.org/lkml/2017/2/6/565
[version2] https://lkml.org/lkml/2017/2/1/510
[version1] https://lkml.org/lkml/2017/1/19/488
[gpc1] https://lkml.org/lkml/2017/2/6/551
[reset1] https://lkml.org/lkml/2017/2/6/554

Andrey Smirnov (3):
  PCI: imx6: Fix a typo in error message
  PCI: imx6: Allow probe deferal by reset GPIO
  PCI: imx6: Add code to support i.MX7D

 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 +-
 drivers/pci/host/pci-imx6.c                        | 131 ++++++++++++++++-----
 include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
 3 files changed, 118 insertions(+), 30 deletions(-)

-- 
2.9.3

[PATCH v4 1/3] PCI: imx6: Fix a typo in error message

[Andrey Smirnov]

Cc: yurovsky@gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

Acked-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/host/pci-imx6.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index c8cefb0..50a1291 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -678,8 +678,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
 							   "pcie_inbound_axi");
 		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
-			dev_err(dev,
-				"pcie_incbound_axi clock missing or invalid\n");
+			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
 			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
 		}
 	}
-- 
2.9.3

[PATCH v4 1/3] PCI: imx6: Fix a typo in error message

[Andrey Smirnov]

Cc: yurovsky@gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

Acked-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/host/pci-imx6.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index c8cefb0..50a1291 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -678,8 +678,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
 							   "pcie_inbound_axi");
 		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
-			dev_err(dev,
-				"pcie_incbound_axi clock missing or invalid\n");
+			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
 			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
 		}
 	}
-- 
2.9.3


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 1/3] PCI: imx6: Fix a typo in error message

[Andrey Smirnov]

Cc: yurovsky at gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org

Acked-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/host/pci-imx6.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index c8cefb0..50a1291 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -678,8 +678,7 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
 							   "pcie_inbound_axi");
 		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
-			dev_err(dev,
-				"pcie_incbound_axi clock missing or invalid\n");
+			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
 			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
 		}
 	}
-- 
2.9.3

[PATCH v4 2/3] PCI: imx6: Allow probe deferal by reset GPIO

[Andrey Smirnov]

Some designs implement reset GPIO via a GPIO expander connected to a
peripheral bus. One such example would be i.MX7 Sabre board where said
GPIO is provided by SPI shift register connected to a bitbanged SPI
bus. In order to support such designs allow reset GPIO request to defer
probing of the driver.

Cc: yurovsky@gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/host/pci-imx6.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 50a1291..3ef8093 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -611,7 +611,7 @@ static int __init imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
 	return 0;
 }
 
-static int __init imx6_pcie_probe(struct platform_device *pdev)
+static int imx6_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct imx6_pcie *imx6_pcie;
@@ -653,6 +653,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 			dev_err(dev, "unable to get reset gpio\n");
 			return ret;
 		}
+	} else if (imx6_pcie->reset_gpio == -EPROBE_DEFER) {
+		return imx6_pcie->reset_gpio;
 	}
 
 	/* Fetch clocks */
@@ -746,11 +748,12 @@ static struct platform_driver imx6_pcie_driver = {
 		.name	= "imx6q-pcie",
 		.of_match_table = imx6_pcie_of_match,
 	},
+	.probe    = imx6_pcie_probe,
 	.shutdown = imx6_pcie_shutdown,
 };
 
 static int __init imx6_pcie_init(void)
 {
-	return platform_driver_probe(&imx6_pcie_driver, imx6_pcie_probe);
+	return platform_driver_register(&imx6_pcie_driver);
 }
 device_initcall(imx6_pcie_init);
-- 
2.9.3

[PATCH v4 2/3] PCI: imx6: Allow probe deferal by reset GPIO

[Andrey Smirnov]

Some designs implement reset GPIO via a GPIO expander connected to a
peripheral bus. One such example would be i.MX7 Sabre board where said
GPIO is provided by SPI shift register connected to a bitbanged SPI
bus. In order to support such designs allow reset GPIO request to defer
probing of the driver.

Cc: yurovsky at gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 drivers/pci/host/pci-imx6.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 50a1291..3ef8093 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -611,7 +611,7 @@ static int __init imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
 	return 0;
 }
 
-static int __init imx6_pcie_probe(struct platform_device *pdev)
+static int imx6_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct imx6_pcie *imx6_pcie;
@@ -653,6 +653,8 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 			dev_err(dev, "unable to get reset gpio\n");
 			return ret;
 		}
+	} else if (imx6_pcie->reset_gpio == -EPROBE_DEFER) {
+		return imx6_pcie->reset_gpio;
 	}
 
 	/* Fetch clocks */
@@ -746,11 +748,12 @@ static struct platform_driver imx6_pcie_driver = {
 		.name	= "imx6q-pcie",
 		.of_match_table = imx6_pcie_of_match,
 	},
+	.probe    = imx6_pcie_probe,
 	.shutdown = imx6_pcie_shutdown,
 };
 
 static int __init imx6_pcie_init(void)
 {
-	return platform_driver_probe(&imx6_pcie_driver, imx6_pcie_probe);
+	return platform_driver_register(&imx6_pcie_driver);
 }
 device_initcall(imx6_pcie_init);
-- 
2.9.3

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Andrey Smirnov]

Add various bits of code needed to support i.MX7D variant of the IP.

Cc: yurovsky@gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
 drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
 include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
 3 files changed, 112 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index 83aeb1f..11db2ab 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
 and thus inherits all the common properties defined in designware-pcie.txt.
 
 Required properties:
-- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
+- compatible:
+	- "fsl,imx6q-pcie"
+	- "fsl,imx6sx-pcie",
+	- "fsl,imx6qp-pcie"
+	- "fsl,imx7d-pcie"
 - reg: base address and length of the PCIe controller
 - interrupts: A list of interrupt outputs of the controller. Must contain an
   entry for each entry in the interrupt-names property.
@@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
 - clock names: Must include the following additional entries:
 	- "pcie_inbound_axi"
 
+Additional required properties for imx7d-pcie:
+- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
+- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
+- reset-names: Must contain the following entires:
+  	       - "pciephy"
+	       - "apps"
+
 Example:
 
 	pcie@0x01000000 {
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 3ef8093..723805c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -17,6 +17,7 @@
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
 #include <linux/module.h>
 #include <linux/of_gpio.h>
 #include <linux/of_device.h>
@@ -27,6 +28,7 @@
 #include <linux/signal.h>
 #include <linux/types.h>
 #include <linux/interrupt.h>
+#include <linux/reset.h>
 
 #include "pcie-designware.h"
 
@@ -36,6 +38,7 @@ enum imx6_pcie_variants {
 	IMX6Q,
 	IMX6SX,
 	IMX6QP,
+	IMX7D,
 };
 
 struct imx6_pcie {
@@ -47,6 +50,8 @@ struct imx6_pcie {
 	struct clk		*pcie_inbound_axi;
 	struct clk		*pcie;
 	struct regmap		*iomuxc_gpr;
+	struct reset_control	*pciephy_reset;
+	struct reset_control	*apps_reset;
 	enum imx6_pcie_variants variant;
 	u32			tx_deemph_gen1;
 	u32			tx_deemph_gen2_3p5db;
@@ -56,6 +61,11 @@ struct imx6_pcie {
 	int			link_gen;
 };
 
+/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
+#define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
+#define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
+#define PHY_PLL_LOCK_WAIT_USLEEP_MAX	200
+
 /* PCIe Root Complex registers (memory-mapped) */
 #define PCIE_RC_LCR				0x7c
 #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
@@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
 	u32 val, gpr1, gpr12;
 
 	switch (imx6_pcie->variant) {
+	case IMX7D:
+		reset_control_assert(imx6_pcie->pciephy_reset);
+		reset_control_assert(imx6_pcie->apps_reset);
+		break;
 	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
@@ -333,11 +347,33 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
 		break;
+	case IMX7D:
+		break;
 	}
 
 	return ret;
 }
 
+static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
+{
+	u32 val;
+	unsigned int retries;
+	struct pcie_port *pp = &imx6_pcie->pp;
+	struct device *dev = pp->dev;
+
+	for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
+		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
+
+		if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
+			return;
+
+		usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
+			     PHY_PLL_LOCK_WAIT_USLEEP_MAX);
+	}
+
+	dev_err(dev, "PCIe PLL lock timeout\n");
+}
+
 static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 {
 	struct pcie_port *pp = &imx6_pcie->pp;
@@ -381,6 +417,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 	}
 
 	switch (imx6_pcie->variant) {
+	case IMX7D:
+		reset_control_deassert(imx6_pcie->pciephy_reset);
+		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
+		break;
 	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
 				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
@@ -407,35 +447,44 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 
 static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
 {
-	if (imx6_pcie->variant == IMX6SX)
+	switch (imx6_pcie->variant) {
+	case IMX7D:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
+		break;
+	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
 				   IMX6SX_GPR12_PCIE_RX_EQ_2);
+		/* FALLTHROUGH */
+	default:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
 
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
+		/* configure constant input signal to the pcie ctrl and phy */
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
+
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
+				   imx6_pcie->tx_deemph_gen1 << 0);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
+				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
+				   imx6_pcie->tx_deemph_gen2_6db << 12);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_SWING_FULL,
+				   imx6_pcie->tx_swing_full << 18);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_SWING_LOW,
+				   imx6_pcie->tx_swing_low << 25);
+		break;
+	}
 
-	/* configure constant input signal to the pcie ctrl and phy */
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 			IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
-
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
-			   imx6_pcie->tx_deemph_gen1 << 0);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
-			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
-			   imx6_pcie->tx_deemph_gen2_6db << 12);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_SWING_FULL,
-			   imx6_pcie->tx_swing_full << 18);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_SWING_LOW,
-			   imx6_pcie->tx_swing_low << 25);
 }
 
 static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
@@ -498,8 +547,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
 	dw_pcie_writel_rc(pp, PCIE_RC_LCR, tmp);
 
 	/* Start LTSSM. */
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+	if (imx6_pcie->variant == IMX7D)
+		reset_control_deassert(imx6_pcie->apps_reset);
+	else
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
 
 	ret = imx6_pcie_wait_for_link(imx6_pcie);
 	if (ret) {
@@ -676,13 +728,31 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(imx6_pcie->pcie);
 	}
 
-	if (imx6_pcie->variant == IMX6SX) {
+	switch (imx6_pcie->variant) {
+	case IMX6SX:
 		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
 							   "pcie_inbound_axi");
 		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
 			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
 			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
 		}
+		break;
+	case IMX7D:
+		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
+								  "pciephy");
+		if (IS_ERR(imx6_pcie->pciephy_reset)) {
+			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
+			return PTR_ERR(imx6_pcie->pciephy_reset);
+		}
+
+		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
+		if (IS_ERR(imx6_pcie->apps_reset)) {
+			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
+			return PTR_ERR(imx6_pcie->apps_reset);
+		}
+		break;
+	default:
+		break;
 	}
 
 	/* Grab GPR config register range */
@@ -740,6 +810,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
 	{ .compatible = "fsl,imx6q-pcie",  .data = (void *)IMX6Q,  },
 	{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
 	{ .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
+	{ .compatible = "fsl,imx7d-pcie",  .data = (void *)IMX7D,  },
 	{},
 };
 
diff --git a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
index 4585d61..abbd524 100644
--- a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
@@ -44,4 +44,8 @@
 
 #define IMX7D_GPR5_CSI_MUX_CONTROL_MIPI		(0x1 << 4)
 
+#define IMX7D_GPR12_PCIE_PHY_REFCLK_SEL		BIT(5)
+
+#define IMX7D_GPR22_PCIE_PHY_PLL_LOCKED		BIT(31)
+
 #endif /* __LINUX_IMX7_IOMUXC_GPR_H */
-- 
2.9.3

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Andrey Smirnov]

Add various bits of code needed to support i.MX7D variant of the IP.

Cc: yurovsky at gmail.com
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: linux-arm-kernel at lists.infradead.org
Cc: devicetree at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
 drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
 include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
 3 files changed, 112 insertions(+), 26 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index 83aeb1f..11db2ab 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
 and thus inherits all the common properties defined in designware-pcie.txt.
 
 Required properties:
-- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
+- compatible:
+	- "fsl,imx6q-pcie"
+	- "fsl,imx6sx-pcie",
+	- "fsl,imx6qp-pcie"
+	- "fsl,imx7d-pcie"
 - reg: base address and length of the PCIe controller
 - interrupts: A list of interrupt outputs of the controller. Must contain an
   entry for each entry in the interrupt-names property.
@@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
 - clock names: Must include the following additional entries:
 	- "pcie_inbound_axi"
 
+Additional required properties for imx7d-pcie:
+- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
+- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
+- reset-names: Must contain the following entires:
+  	       - "pciephy"
+	       - "apps"
+
 Example:
 
 	pcie at 0x01000000 {
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 3ef8093..723805c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -17,6 +17,7 @@
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
+#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
 #include <linux/module.h>
 #include <linux/of_gpio.h>
 #include <linux/of_device.h>
@@ -27,6 +28,7 @@
 #include <linux/signal.h>
 #include <linux/types.h>
 #include <linux/interrupt.h>
+#include <linux/reset.h>
 
 #include "pcie-designware.h"
 
@@ -36,6 +38,7 @@ enum imx6_pcie_variants {
 	IMX6Q,
 	IMX6SX,
 	IMX6QP,
+	IMX7D,
 };
 
 struct imx6_pcie {
@@ -47,6 +50,8 @@ struct imx6_pcie {
 	struct clk		*pcie_inbound_axi;
 	struct clk		*pcie;
 	struct regmap		*iomuxc_gpr;
+	struct reset_control	*pciephy_reset;
+	struct reset_control	*apps_reset;
 	enum imx6_pcie_variants variant;
 	u32			tx_deemph_gen1;
 	u32			tx_deemph_gen2_3p5db;
@@ -56,6 +61,11 @@ struct imx6_pcie {
 	int			link_gen;
 };
 
+/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
+#define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
+#define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
+#define PHY_PLL_LOCK_WAIT_USLEEP_MAX	200
+
 /* PCIe Root Complex registers (memory-mapped) */
 #define PCIE_RC_LCR				0x7c
 #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
@@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
 	u32 val, gpr1, gpr12;
 
 	switch (imx6_pcie->variant) {
+	case IMX7D:
+		reset_control_assert(imx6_pcie->pciephy_reset);
+		reset_control_assert(imx6_pcie->apps_reset);
+		break;
 	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
@@ -333,11 +347,33 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
 		break;
+	case IMX7D:
+		break;
 	}
 
 	return ret;
 }
 
+static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
+{
+	u32 val;
+	unsigned int retries;
+	struct pcie_port *pp = &imx6_pcie->pp;
+	struct device *dev = pp->dev;
+
+	for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
+		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
+
+		if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
+			return;
+
+		usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
+			     PHY_PLL_LOCK_WAIT_USLEEP_MAX);
+	}
+
+	dev_err(dev, "PCIe PLL lock timeout\n");
+}
+
 static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 {
 	struct pcie_port *pp = &imx6_pcie->pp;
@@ -381,6 +417,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 	}
 
 	switch (imx6_pcie->variant) {
+	case IMX7D:
+		reset_control_deassert(imx6_pcie->pciephy_reset);
+		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
+		break;
 	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
 				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
@@ -407,35 +447,44 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
 
 static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
 {
-	if (imx6_pcie->variant == IMX6SX)
+	switch (imx6_pcie->variant) {
+	case IMX7D:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
+		break;
+	case IMX6SX:
 		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
 				   IMX6SX_GPR12_PCIE_RX_EQ_2);
+		/* FALLTHROUGH */
+	default:
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
 
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
+		/* configure constant input signal to the pcie ctrl and phy */
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
+
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
+				   imx6_pcie->tx_deemph_gen1 << 0);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
+				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
+				   imx6_pcie->tx_deemph_gen2_6db << 12);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_SWING_FULL,
+				   imx6_pcie->tx_swing_full << 18);
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
+				   IMX6Q_GPR8_TX_SWING_LOW,
+				   imx6_pcie->tx_swing_low << 25);
+		break;
+	}
 
-	/* configure constant input signal to the pcie ctrl and phy */
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
 			IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
-
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
-			   imx6_pcie->tx_deemph_gen1 << 0);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
-			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
-			   imx6_pcie->tx_deemph_gen2_6db << 12);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_SWING_FULL,
-			   imx6_pcie->tx_swing_full << 18);
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
-			   IMX6Q_GPR8_TX_SWING_LOW,
-			   imx6_pcie->tx_swing_low << 25);
 }
 
 static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
@@ -498,8 +547,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
 	dw_pcie_writel_rc(pp, PCIE_RC_LCR, tmp);
 
 	/* Start LTSSM. */
-	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
-			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
+	if (imx6_pcie->variant == IMX7D)
+		reset_control_deassert(imx6_pcie->apps_reset);
+	else
+		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
 
 	ret = imx6_pcie_wait_for_link(imx6_pcie);
 	if (ret) {
@@ -676,13 +728,31 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 		return PTR_ERR(imx6_pcie->pcie);
 	}
 
-	if (imx6_pcie->variant == IMX6SX) {
+	switch (imx6_pcie->variant) {
+	case IMX6SX:
 		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
 							   "pcie_inbound_axi");
 		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
 			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
 			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
 		}
+		break;
+	case IMX7D:
+		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
+								  "pciephy");
+		if (IS_ERR(imx6_pcie->pciephy_reset)) {
+			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
+			return PTR_ERR(imx6_pcie->pciephy_reset);
+		}
+
+		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
+		if (IS_ERR(imx6_pcie->apps_reset)) {
+			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
+			return PTR_ERR(imx6_pcie->apps_reset);
+		}
+		break;
+	default:
+		break;
 	}
 
 	/* Grab GPR config register range */
@@ -740,6 +810,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
 	{ .compatible = "fsl,imx6q-pcie",  .data = (void *)IMX6Q,  },
 	{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
 	{ .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
+	{ .compatible = "fsl,imx7d-pcie",  .data = (void *)IMX7D,  },
 	{},
 };
 
diff --git a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
index 4585d61..abbd524 100644
--- a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
+++ b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
@@ -44,4 +44,8 @@
 
 #define IMX7D_GPR5_CSI_MUX_CONTROL_MIPI		(0x1 << 4)
 
+#define IMX7D_GPR12_PCIE_PHY_REFCLK_SEL		BIT(5)
+
+#define IMX7D_GPR22_PCIE_PHY_PLL_LOCKED		BIT(31)
+
 #endif /* __LINUX_IMX7_IOMUXC_GPR_H */
-- 
2.9.3

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Dienstag, den 07.02.2017, 07:50 -0800 schrieb Andrey Smirnov:
> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky@gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

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

> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"
> +	       - "apps"
> +
>  Example:
>  
>  	pcie@0x01000000 {
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 3ef8093..723805c 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -17,6 +17,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_device.h>
> @@ -27,6 +28,7 @@
>  #include <linux/signal.h>
>  #include <linux/types.h>
>  #include <linux/interrupt.h>
> +#include <linux/reset.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -36,6 +38,7 @@ enum imx6_pcie_variants {
>  	IMX6Q,
>  	IMX6SX,
>  	IMX6QP,
> +	IMX7D,
>  };
>  
>  struct imx6_pcie {
> @@ -47,6 +50,8 @@ struct imx6_pcie {
>  	struct clk		*pcie_inbound_axi;
>  	struct clk		*pcie;
>  	struct regmap		*iomuxc_gpr;
> +	struct reset_control	*pciephy_reset;
> +	struct reset_control	*apps_reset;
>  	enum imx6_pcie_variants variant;
>  	u32			tx_deemph_gen1;
>  	u32			tx_deemph_gen2_3p5db;
> @@ -56,6 +61,11 @@ struct imx6_pcie {
>  	int			link_gen;
>  };
>  
> +/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> +#define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MAX	200
> +
>  /* PCIe Root Complex registers (memory-mapped) */
>  #define PCIE_RC_LCR				0x7c
>  #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> @@ -333,11 +347,33 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>  		break;
> +	case IMX7D:
> +		break;
>  	}
>  
>  	return ret;
>  }
>  
> +static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> +{
> +	u32 val;
> +	unsigned int retries;
> +	struct pcie_port *pp = &imx6_pcie->pp;
> +	struct device *dev = pp->dev;
> +
> +	for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
> +		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
> +
> +		if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
> +			return;
> +
> +		usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
> +			     PHY_PLL_LOCK_WAIT_USLEEP_MAX);
> +	}
> +
> +	dev_err(dev, "PCIe PLL lock timeout\n");
> +}
> +
>  static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  {
>  	struct pcie_port *pp = &imx6_pcie->pp;
> @@ -381,6 +417,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	}
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_deassert(imx6_pcie->pciephy_reset);
> +		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
> @@ -407,35 +447,44 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  
>  static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
>  {
> -	if (imx6_pcie->variant == IMX6SX)
> +	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
> +		break;
> +	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_2);
> +		/* FALLTHROUGH */
> +	default:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
>  
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> +		/* configure constant input signal to the pcie ctrl and phy */
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> +
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> +				   imx6_pcie->tx_deemph_gen1 << 0);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> +				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> +				   imx6_pcie->tx_deemph_gen2_6db << 12);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_FULL,
> +				   imx6_pcie->tx_swing_full << 18);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_LOW,
> +				   imx6_pcie->tx_swing_low << 25);
> +		break;
> +	}
>  
> -	/* configure constant input signal to the pcie ctrl and phy */
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  			IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> -
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> -			   imx6_pcie->tx_deemph_gen1 << 0);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> -			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> -			   imx6_pcie->tx_deemph_gen2_6db << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_FULL,
> -			   imx6_pcie->tx_swing_full << 18);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_LOW,
> -			   imx6_pcie->tx_swing_low << 25);
>  }
>  
>  static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
> @@ -498,8 +547,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	dw_pcie_writel_rc(pp, PCIE_RC_LCR, tmp);
>  
>  	/* Start LTSSM. */
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> +	if (imx6_pcie->variant == IMX7D)
> +		reset_control_deassert(imx6_pcie->apps_reset);
> +	else
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
>  
>  	ret = imx6_pcie_wait_for_link(imx6_pcie);
>  	if (ret) {
> @@ -676,13 +728,31 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(imx6_pcie->pcie);
>  	}
>  
> -	if (imx6_pcie->variant == IMX6SX) {
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
>  							   "pcie_inbound_axi");
>  		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
>  			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
>  			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
>  		}
> +		break;
> +	case IMX7D:
> +		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
> +								  "pciephy");
> +		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> +			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
> +			return PTR_ERR(imx6_pcie->pciephy_reset);
> +		}
> +
> +		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
> +		if (IS_ERR(imx6_pcie->apps_reset)) {
> +			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
> +			return PTR_ERR(imx6_pcie->apps_reset);
> +		}
> +		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Grab GPR config register range */
> @@ -740,6 +810,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
>  	{ .compatible = "fsl,imx6q-pcie",  .data = (void *)IMX6Q,  },
>  	{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
>  	{ .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
> +	{ .compatible = "fsl,imx7d-pcie",  .data = (void *)IMX7D,  },
>  	{},
>  };
>  
> diff --git a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> index 4585d61..abbd524 100644
> --- a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> +++ b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> @@ -44,4 +44,8 @@
>  
>  #define IMX7D_GPR5_CSI_MUX_CONTROL_MIPI		(0x1 << 4)
>  
> +#define IMX7D_GPR12_PCIE_PHY_REFCLK_SEL		BIT(5)
> +
> +#define IMX7D_GPR22_PCIE_PHY_PLL_LOCKED		BIT(31)
> +
>  #endif /* __LINUX_IMX7_IOMUXC_GPR_H */

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Dienstag, den 07.02.2017, 07:50 -0800 schrieb Andrey Smirnov:
> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky@gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

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

> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"
> +	       - "apps"
> +
>  Example:
>  
>  	pcie@0x01000000 {
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 3ef8093..723805c 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -17,6 +17,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_device.h>
> @@ -27,6 +28,7 @@
>  #include <linux/signal.h>
>  #include <linux/types.h>
>  #include <linux/interrupt.h>
> +#include <linux/reset.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -36,6 +38,7 @@ enum imx6_pcie_variants {
>  	IMX6Q,
>  	IMX6SX,
>  	IMX6QP,
> +	IMX7D,
>  };
>  
>  struct imx6_pcie {
> @@ -47,6 +50,8 @@ struct imx6_pcie {
>  	struct clk		*pcie_inbound_axi;
>  	struct clk		*pcie;
>  	struct regmap		*iomuxc_gpr;
> +	struct reset_control	*pciephy_reset;
> +	struct reset_control	*apps_reset;
>  	enum imx6_pcie_variants variant;
>  	u32			tx_deemph_gen1;
>  	u32			tx_deemph_gen2_3p5db;
> @@ -56,6 +61,11 @@ struct imx6_pcie {
>  	int			link_gen;
>  };
>  
> +/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> +#define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MAX	200
> +
>  /* PCIe Root Complex registers (memory-mapped) */
>  #define PCIE_RC_LCR				0x7c
>  #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> @@ -333,11 +347,33 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>  		break;
> +	case IMX7D:
> +		break;
>  	}
>  
>  	return ret;
>  }
>  
> +static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> +{
> +	u32 val;
> +	unsigned int retries;
> +	struct pcie_port *pp = &imx6_pcie->pp;
> +	struct device *dev = pp->dev;
> +
> +	for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
> +		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
> +
> +		if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
> +			return;
> +
> +		usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
> +			     PHY_PLL_LOCK_WAIT_USLEEP_MAX);
> +	}
> +
> +	dev_err(dev, "PCIe PLL lock timeout\n");
> +}
> +
>  static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  {
>  	struct pcie_port *pp = &imx6_pcie->pp;
> @@ -381,6 +417,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	}
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_deassert(imx6_pcie->pciephy_reset);
> +		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
> @@ -407,35 +447,44 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  
>  static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
>  {
> -	if (imx6_pcie->variant == IMX6SX)
> +	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
> +		break;
> +	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_2);
> +		/* FALLTHROUGH */
> +	default:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
>  
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> +		/* configure constant input signal to the pcie ctrl and phy */
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> +
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> +				   imx6_pcie->tx_deemph_gen1 << 0);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> +				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> +				   imx6_pcie->tx_deemph_gen2_6db << 12);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_FULL,
> +				   imx6_pcie->tx_swing_full << 18);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_LOW,
> +				   imx6_pcie->tx_swing_low << 25);
> +		break;
> +	}
>  
> -	/* configure constant input signal to the pcie ctrl and phy */
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  			IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> -
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> -			   imx6_pcie->tx_deemph_gen1 << 0);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> -			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> -			   imx6_pcie->tx_deemph_gen2_6db << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_FULL,
> -			   imx6_pcie->tx_swing_full << 18);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_LOW,
> -			   imx6_pcie->tx_swing_low << 25);
>  }
>  
>  static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
> @@ -498,8 +547,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	dw_pcie_writel_rc(pp, PCIE_RC_LCR, tmp);
>  
>  	/* Start LTSSM. */
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> +	if (imx6_pcie->variant == IMX7D)
> +		reset_control_deassert(imx6_pcie->apps_reset);
> +	else
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
>  
>  	ret = imx6_pcie_wait_for_link(imx6_pcie);
>  	if (ret) {
> @@ -676,13 +728,31 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(imx6_pcie->pcie);
>  	}
>  
> -	if (imx6_pcie->variant == IMX6SX) {
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
>  							   "pcie_inbound_axi");
>  		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
>  			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
>  			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
>  		}
> +		break;
> +	case IMX7D:
> +		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
> +								  "pciephy");
> +		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> +			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
> +			return PTR_ERR(imx6_pcie->pciephy_reset);
> +		}
> +
> +		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
> +		if (IS_ERR(imx6_pcie->apps_reset)) {
> +			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
> +			return PTR_ERR(imx6_pcie->apps_reset);
> +		}
> +		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Grab GPR config register range */
> @@ -740,6 +810,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
>  	{ .compatible = "fsl,imx6q-pcie",  .data = (void *)IMX6Q,  },
>  	{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
>  	{ .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
> +	{ .compatible = "fsl,imx7d-pcie",  .data = (void *)IMX7D,  },
>  	{},
>  };
>  
> diff --git a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> index 4585d61..abbd524 100644
> --- a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> +++ b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> @@ -44,4 +44,8 @@
>  
>  #define IMX7D_GPR5_CSI_MUX_CONTROL_MIPI		(0x1 << 4)
>  
> +#define IMX7D_GPR12_PCIE_PHY_REFCLK_SEL		BIT(5)
> +
> +#define IMX7D_GPR22_PCIE_PHY_PLL_LOCKED		BIT(31)
> +
>  #endif /* __LINUX_IMX7_IOMUXC_GPR_H */

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Dienstag, den 07.02.2017, 07:50 -0800 schrieb Andrey Smirnov:
> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky@gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

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

> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"
> +	       - "apps"
> +
>  Example:
>  
>  	pcie@0x01000000 {
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 3ef8093..723805c 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -17,6 +17,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_device.h>
> @@ -27,6 +28,7 @@
>  #include <linux/signal.h>
>  #include <linux/types.h>
>  #include <linux/interrupt.h>
> +#include <linux/reset.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -36,6 +38,7 @@ enum imx6_pcie_variants {
>  	IMX6Q,
>  	IMX6SX,
>  	IMX6QP,
> +	IMX7D,
>  };
>  
>  struct imx6_pcie {
> @@ -47,6 +50,8 @@ struct imx6_pcie {
>  	struct clk		*pcie_inbound_axi;
>  	struct clk		*pcie;
>  	struct regmap		*iomuxc_gpr;
> +	struct reset_control	*pciephy_reset;
> +	struct reset_control	*apps_reset;
>  	enum imx6_pcie_variants variant;
>  	u32			tx_deemph_gen1;
>  	u32			tx_deemph_gen2_3p5db;
> @@ -56,6 +61,11 @@ struct imx6_pcie {
>  	int			link_gen;
>  };
>  
> +/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> +#define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MAX	200
> +
>  /* PCIe Root Complex registers (memory-mapped) */
>  #define PCIE_RC_LCR				0x7c
>  #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> @@ -333,11 +347,33 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>  		break;
> +	case IMX7D:
> +		break;
>  	}
>  
>  	return ret;
>  }
>  
> +static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> +{
> +	u32 val;
> +	unsigned int retries;
> +	struct pcie_port *pp = &imx6_pcie->pp;
> +	struct device *dev = pp->dev;
> +
> +	for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
> +		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
> +
> +		if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
> +			return;
> +
> +		usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
> +			     PHY_PLL_LOCK_WAIT_USLEEP_MAX);
> +	}
> +
> +	dev_err(dev, "PCIe PLL lock timeout\n");
> +}
> +
>  static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  {
>  	struct pcie_port *pp = &imx6_pcie->pp;
> @@ -381,6 +417,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	}
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_deassert(imx6_pcie->pciephy_reset);
> +		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
> @@ -407,35 +447,44 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  
>  static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
>  {
> -	if (imx6_pcie->variant == IMX6SX)
> +	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
> +		break;
> +	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_2);
> +		/* FALLTHROUGH */
> +	default:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
>  
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> +		/* configure constant input signal to the pcie ctrl and phy */
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> +
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> +				   imx6_pcie->tx_deemph_gen1 << 0);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> +				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> +				   imx6_pcie->tx_deemph_gen2_6db << 12);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_FULL,
> +				   imx6_pcie->tx_swing_full << 18);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_LOW,
> +				   imx6_pcie->tx_swing_low << 25);
> +		break;
> +	}
>  
> -	/* configure constant input signal to the pcie ctrl and phy */
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  			IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> -
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> -			   imx6_pcie->tx_deemph_gen1 << 0);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> -			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> -			   imx6_pcie->tx_deemph_gen2_6db << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_FULL,
> -			   imx6_pcie->tx_swing_full << 18);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_LOW,
> -			   imx6_pcie->tx_swing_low << 25);
>  }
>  
>  static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
> @@ -498,8 +547,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	dw_pcie_writel_rc(pp, PCIE_RC_LCR, tmp);
>  
>  	/* Start LTSSM. */
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> +	if (imx6_pcie->variant == IMX7D)
> +		reset_control_deassert(imx6_pcie->apps_reset);
> +	else
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
>  
>  	ret = imx6_pcie_wait_for_link(imx6_pcie);
>  	if (ret) {
> @@ -676,13 +728,31 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(imx6_pcie->pcie);
>  	}
>  
> -	if (imx6_pcie->variant == IMX6SX) {
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
>  							   "pcie_inbound_axi");
>  		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
>  			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
>  			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
>  		}
> +		break;
> +	case IMX7D:
> +		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
> +								  "pciephy");
> +		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> +			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
> +			return PTR_ERR(imx6_pcie->pciephy_reset);
> +		}
> +
> +		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
> +		if (IS_ERR(imx6_pcie->apps_reset)) {
> +			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
> +			return PTR_ERR(imx6_pcie->apps_reset);
> +		}
> +		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Grab GPR config register range */
> @@ -740,6 +810,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
>  	{ .compatible = "fsl,imx6q-pcie",  .data = (void *)IMX6Q,  },
>  	{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
>  	{ .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
> +	{ .compatible = "fsl,imx7d-pcie",  .data = (void *)IMX7D,  },
>  	{},
>  };
>  
> diff --git a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> index 4585d61..abbd524 100644
> --- a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> +++ b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> @@ -44,4 +44,8 @@
>  
>  #define IMX7D_GPR5_CSI_MUX_CONTROL_MIPI		(0x1 << 4)
>  
> +#define IMX7D_GPR12_PCIE_PHY_REFCLK_SEL		BIT(5)
> +
> +#define IMX7D_GPR22_PCIE_PHY_PLL_LOCKED		BIT(31)
> +
>  #endif /* __LINUX_IMX7_IOMUXC_GPR_H */



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Dienstag, den 07.02.2017, 07:50 -0800 schrieb Andrey Smirnov:
> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky at gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: devicetree at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

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

> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"
> +	       - "apps"
> +
>  Example:
>  
>  	pcie at 0x01000000 {
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 3ef8093..723805c 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -17,6 +17,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_device.h>
> @@ -27,6 +28,7 @@
>  #include <linux/signal.h>
>  #include <linux/types.h>
>  #include <linux/interrupt.h>
> +#include <linux/reset.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -36,6 +38,7 @@ enum imx6_pcie_variants {
>  	IMX6Q,
>  	IMX6SX,
>  	IMX6QP,
> +	IMX7D,
>  };
>  
>  struct imx6_pcie {
> @@ -47,6 +50,8 @@ struct imx6_pcie {
>  	struct clk		*pcie_inbound_axi;
>  	struct clk		*pcie;
>  	struct regmap		*iomuxc_gpr;
> +	struct reset_control	*pciephy_reset;
> +	struct reset_control	*apps_reset;
>  	enum imx6_pcie_variants variant;
>  	u32			tx_deemph_gen1;
>  	u32			tx_deemph_gen2_3p5db;
> @@ -56,6 +61,11 @@ struct imx6_pcie {
>  	int			link_gen;
>  };
>  
> +/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> +#define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MAX	200
> +
>  /* PCIe Root Complex registers (memory-mapped) */
>  #define PCIE_RC_LCR				0x7c
>  #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> @@ -333,11 +347,33 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>  		break;
> +	case IMX7D:
> +		break;
>  	}
>  
>  	return ret;
>  }
>  
> +static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> +{
> +	u32 val;
> +	unsigned int retries;
> +	struct pcie_port *pp = &imx6_pcie->pp;
> +	struct device *dev = pp->dev;
> +
> +	for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
> +		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
> +
> +		if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
> +			return;
> +
> +		usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
> +			     PHY_PLL_LOCK_WAIT_USLEEP_MAX);
> +	}
> +
> +	dev_err(dev, "PCIe PLL lock timeout\n");
> +}
> +
>  static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  {
>  	struct pcie_port *pp = &imx6_pcie->pp;
> @@ -381,6 +417,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	}
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_deassert(imx6_pcie->pciephy_reset);
> +		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
> @@ -407,35 +447,44 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  
>  static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
>  {
> -	if (imx6_pcie->variant == IMX6SX)
> +	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
> +		break;
> +	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_2);
> +		/* FALLTHROUGH */
> +	default:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
>  
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> +		/* configure constant input signal to the pcie ctrl and phy */
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> +
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> +				   imx6_pcie->tx_deemph_gen1 << 0);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> +				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> +				   imx6_pcie->tx_deemph_gen2_6db << 12);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_FULL,
> +				   imx6_pcie->tx_swing_full << 18);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_LOW,
> +				   imx6_pcie->tx_swing_low << 25);
> +		break;
> +	}
>  
> -	/* configure constant input signal to the pcie ctrl and phy */
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  			IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> -
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> -			   imx6_pcie->tx_deemph_gen1 << 0);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> -			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> -			   imx6_pcie->tx_deemph_gen2_6db << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_FULL,
> -			   imx6_pcie->tx_swing_full << 18);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_LOW,
> -			   imx6_pcie->tx_swing_low << 25);
>  }
>  
>  static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
> @@ -498,8 +547,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	dw_pcie_writel_rc(pp, PCIE_RC_LCR, tmp);
>  
>  	/* Start LTSSM. */
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> +	if (imx6_pcie->variant == IMX7D)
> +		reset_control_deassert(imx6_pcie->apps_reset);
> +	else
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
>  
>  	ret = imx6_pcie_wait_for_link(imx6_pcie);
>  	if (ret) {
> @@ -676,13 +728,31 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(imx6_pcie->pcie);
>  	}
>  
> -	if (imx6_pcie->variant == IMX6SX) {
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
>  							   "pcie_inbound_axi");
>  		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
>  			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
>  			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
>  		}
> +		break;
> +	case IMX7D:
> +		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
> +								  "pciephy");
> +		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> +			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
> +			return PTR_ERR(imx6_pcie->pciephy_reset);
> +		}
> +
> +		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
> +		if (IS_ERR(imx6_pcie->apps_reset)) {
> +			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
> +			return PTR_ERR(imx6_pcie->apps_reset);
> +		}
> +		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Grab GPR config register range */
> @@ -740,6 +810,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
>  	{ .compatible = "fsl,imx6q-pcie",  .data = (void *)IMX6Q,  },
>  	{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
>  	{ .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
> +	{ .compatible = "fsl,imx7d-pcie",  .data = (void *)IMX7D,  },
>  	{},
>  };
>  
> diff --git a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> index 4585d61..abbd524 100644
> --- a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> +++ b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> @@ -44,4 +44,8 @@
>  
>  #define IMX7D_GPR5_CSI_MUX_CONTROL_MIPI		(0x1 << 4)
>  
> +#define IMX7D_GPR12_PCIE_PHY_REFCLK_SEL		BIT(5)
> +
> +#define IMX7D_GPR22_PCIE_PHY_PLL_LOCKED		BIT(31)
> +
>  #endif /* __LINUX_IMX7_IOMUXC_GPR_H */

Re: [PATCH v4 2/3] PCI: imx6: Allow probe deferal by reset GPIO

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

Hi Andrey,

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.10-rc7 next-20170207]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrey-Smirnov/PCI-imx6-Fix-a-typo-in-error-message/20170208-014815
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

>> WARNING: vmlinux.o(.text+0x4d99fc): Section mismatch in reference from the function imx6_pcie_probe() to the function .init.text:hook_fault_code()
   The function imx6_pcie_probe() references
   the function __init hook_fault_code().
   This is often because imx6_pcie_probe lacks a __init
   annotation or the annotation of hook_fault_code is wrong.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 60380 bytes --]

Re: [PATCH v4 2/3] PCI: imx6: Allow probe deferal by reset GPIO

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]

Hi Andrey,

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.10-rc7 next-20170207]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrey-Smirnov/PCI-imx6-Fix-a-typo-in-error-message/20170208-014815
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

>> WARNING: vmlinux.o(.text+0x4d99fc): Section mismatch in reference from the function imx6_pcie_probe() to the function .init.text:hook_fault_code()
   The function imx6_pcie_probe() references
   the function __init hook_fault_code().
   This is often because imx6_pcie_probe lacks a __init
   annotation or the annotation of hook_fault_code is wrong.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 60380 bytes --]

[-- Attachment #3: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 2/3] PCI: imx6: Allow probe deferal by reset GPIO

[kbuild test robot]

Hi Andrey,

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.10-rc7 next-20170207]
[cannot apply to pci/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Andrey-Smirnov/PCI-imx6-Fix-a-typo-in-error-message/20170208-014815
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All warnings (new ones prefixed by >>):

>> WARNING: vmlinux.o(.text+0x4d99fc): Section mismatch in reference from the function imx6_pcie_probe() to the function .init.text:hook_fault_code()
   The function imx6_pcie_probe() references
   the function __init hook_fault_code().
   This is often because imx6_pcie_probe lacks a __init
   annotation or the annotation of hook_fault_code is wrong.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 60380 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170208/4de16b82/attachment-0001.gz>

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lee Jones]

On Tue, 07 Feb 2017, Andrey Smirnov wrote:

> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky@gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----

>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +

Acked-by: Lee Jones <lee.jones@linaro.org>

>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"
> +	       - "apps"
> +
>  Example:
>  
>  	pcie@0x01000000 {
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 3ef8093..723805c 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -17,6 +17,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_device.h>
> @@ -27,6 +28,7 @@
>  #include <linux/signal.h>
>  #include <linux/types.h>
>  #include <linux/interrupt.h>
> +#include <linux/reset.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -36,6 +38,7 @@ enum imx6_pcie_variants {
>  	IMX6Q,
>  	IMX6SX,
>  	IMX6QP,
> +	IMX7D,
>  };
>  
>  struct imx6_pcie {
> @@ -47,6 +50,8 @@ struct imx6_pcie {
>  	struct clk		*pcie_inbound_axi;
>  	struct clk		*pcie;
>  	struct regmap		*iomuxc_gpr;
> +	struct reset_control	*pciephy_reset;
> +	struct reset_control	*apps_reset;
>  	enum imx6_pcie_variants variant;
>  	u32			tx_deemph_gen1;
>  	u32			tx_deemph_gen2_3p5db;
> @@ -56,6 +61,11 @@ struct imx6_pcie {
>  	int			link_gen;
>  };
>  
> +/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> +#define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MAX	200
> +
>  /* PCIe Root Complex registers (memory-mapped) */
>  #define PCIE_RC_LCR				0x7c
>  #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> @@ -333,11 +347,33 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>  		break;
> +	case IMX7D:
> +		break;
>  	}
>  
>  	return ret;
>  }
>  
> +static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> +{
> +	u32 val;
> +	unsigned int retries;
> +	struct pcie_port *pp = &imx6_pcie->pp;
> +	struct device *dev = pp->dev;
> +
> +	for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
> +		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
> +
> +		if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
> +			return;
> +
> +		usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
> +			     PHY_PLL_LOCK_WAIT_USLEEP_MAX);
> +	}
> +
> +	dev_err(dev, "PCIe PLL lock timeout\n");
> +}
> +
>  static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  {
>  	struct pcie_port *pp = &imx6_pcie->pp;
> @@ -381,6 +417,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	}
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_deassert(imx6_pcie->pciephy_reset);
> +		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
> @@ -407,35 +447,44 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  
>  static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
>  {
> -	if (imx6_pcie->variant == IMX6SX)
> +	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
> +		break;
> +	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_2);
> +		/* FALLTHROUGH */
> +	default:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
>  
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> +		/* configure constant input signal to the pcie ctrl and phy */
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> +
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> +				   imx6_pcie->tx_deemph_gen1 << 0);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> +				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> +				   imx6_pcie->tx_deemph_gen2_6db << 12);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_FULL,
> +				   imx6_pcie->tx_swing_full << 18);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_LOW,
> +				   imx6_pcie->tx_swing_low << 25);
> +		break;
> +	}
>  
> -	/* configure constant input signal to the pcie ctrl and phy */
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  			IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> -
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> -			   imx6_pcie->tx_deemph_gen1 << 0);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> -			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> -			   imx6_pcie->tx_deemph_gen2_6db << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_FULL,
> -			   imx6_pcie->tx_swing_full << 18);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_LOW,
> -			   imx6_pcie->tx_swing_low << 25);
>  }
>  
>  static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
> @@ -498,8 +547,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	dw_pcie_writel_rc(pp, PCIE_RC_LCR, tmp);
>  
>  	/* Start LTSSM. */
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> +	if (imx6_pcie->variant == IMX7D)
> +		reset_control_deassert(imx6_pcie->apps_reset);
> +	else
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
>  
>  	ret = imx6_pcie_wait_for_link(imx6_pcie);
>  	if (ret) {
> @@ -676,13 +728,31 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(imx6_pcie->pcie);
>  	}
>  
> -	if (imx6_pcie->variant == IMX6SX) {
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
>  							   "pcie_inbound_axi");
>  		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
>  			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
>  			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
>  		}
> +		break;
> +	case IMX7D:
> +		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
> +								  "pciephy");
> +		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> +			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
> +			return PTR_ERR(imx6_pcie->pciephy_reset);
> +		}
> +
> +		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
> +		if (IS_ERR(imx6_pcie->apps_reset)) {
> +			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
> +			return PTR_ERR(imx6_pcie->apps_reset);
> +		}
> +		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Grab GPR config register range */
> @@ -740,6 +810,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
>  	{ .compatible = "fsl,imx6q-pcie",  .data = (void *)IMX6Q,  },
>  	{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
>  	{ .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
> +	{ .compatible = "fsl,imx7d-pcie",  .data = (void *)IMX7D,  },
>  	{},
>  };
>  
> diff --git a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> index 4585d61..abbd524 100644
> --- a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> +++ b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> @@ -44,4 +44,8 @@
>  
>  #define IMX7D_GPR5_CSI_MUX_CONTROL_MIPI		(0x1 << 4)
>  
> +#define IMX7D_GPR12_PCIE_PHY_REFCLK_SEL		BIT(5)
> +
> +#define IMX7D_GPR22_PCIE_PHY_PLL_LOCKED		BIT(31)
> +
>  #endif /* __LINUX_IMX7_IOMUXC_GPR_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lee Jones]

On Tue, 07 Feb 2017, Andrey Smirnov wrote:

> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky at gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: devicetree at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----

>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +

Acked-by: Lee Jones <lee.jones@linaro.org>

>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"
> +	       - "apps"
> +
>  Example:
>  
>  	pcie at 0x01000000 {
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 3ef8093..723805c 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -17,6 +17,7 @@
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> +#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
>  #include <linux/of_device.h>
> @@ -27,6 +28,7 @@
>  #include <linux/signal.h>
>  #include <linux/types.h>
>  #include <linux/interrupt.h>
> +#include <linux/reset.h>
>  
>  #include "pcie-designware.h"
>  
> @@ -36,6 +38,7 @@ enum imx6_pcie_variants {
>  	IMX6Q,
>  	IMX6SX,
>  	IMX6QP,
> +	IMX7D,
>  };
>  
>  struct imx6_pcie {
> @@ -47,6 +50,8 @@ struct imx6_pcie {
>  	struct clk		*pcie_inbound_axi;
>  	struct clk		*pcie;
>  	struct regmap		*iomuxc_gpr;
> +	struct reset_control	*pciephy_reset;
> +	struct reset_control	*apps_reset;
>  	enum imx6_pcie_variants variant;
>  	u32			tx_deemph_gen1;
>  	u32			tx_deemph_gen2_3p5db;
> @@ -56,6 +61,11 @@ struct imx6_pcie {
>  	int			link_gen;
>  };
>  
> +/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> +#define PHY_PLL_LOCK_WAIT_MAX_RETRIES	2000
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MIN	50
> +#define PHY_PLL_LOCK_WAIT_USLEEP_MAX	200
> +
>  /* PCIe Root Complex registers (memory-mapped) */
>  #define PCIE_RC_LCR				0x7c
>  #define PCIE_RC_LCR_MAX_LINK_SPEEDS_GEN1	0x1
> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> @@ -333,11 +347,33 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
>  				   IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
>  		break;
> +	case IMX7D:
> +		break;
>  	}
>  
>  	return ret;
>  }
>  
> +static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
> +{
> +	u32 val;
> +	unsigned int retries;
> +	struct pcie_port *pp = &imx6_pcie->pp;
> +	struct device *dev = pp->dev;
> +
> +	for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
> +		regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
> +
> +		if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
> +			return;
> +
> +		usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
> +			     PHY_PLL_LOCK_WAIT_USLEEP_MAX);
> +	}
> +
> +	dev_err(dev, "PCIe PLL lock timeout\n");
> +}
> +
>  static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  {
>  	struct pcie_port *pp = &imx6_pcie->pp;
> @@ -381,6 +417,10 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  	}
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_deassert(imx6_pcie->pciephy_reset);
> +		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR5,
>  				   IMX6SX_GPR5_PCIE_BTNRST_RESET, 0);
> @@ -407,35 +447,44 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
>  
>  static void imx6_pcie_init_phy(struct imx6_pcie *imx6_pcie)
>  {
> -	if (imx6_pcie->variant == IMX6SX)
> +	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX7D_GPR12_PCIE_PHY_REFCLK_SEL, 0);
> +		break;
> +	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_MASK,
>  				   IMX6SX_GPR12_PCIE_RX_EQ_2);
> +		/* FALLTHROUGH */
> +	default:
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
>  
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 0 << 10);
> +		/* configure constant input signal to the pcie ctrl and phy */
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> +
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> +				   imx6_pcie->tx_deemph_gen1 << 0);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> +				   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> +				   imx6_pcie->tx_deemph_gen2_6db << 12);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_FULL,
> +				   imx6_pcie->tx_swing_full << 18);
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> +				   IMX6Q_GPR8_TX_SWING_LOW,
> +				   imx6_pcie->tx_swing_low << 25);
> +		break;
> +	}
>  
> -	/* configure constant input signal to the pcie ctrl and phy */
>  	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  			IMX6Q_GPR12_DEVICE_TYPE, PCI_EXP_TYPE_ROOT_PORT << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_LOS_LEVEL, 9 << 4);
> -
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN1,
> -			   imx6_pcie->tx_deemph_gen1 << 0);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_3P5DB,
> -			   imx6_pcie->tx_deemph_gen2_3p5db << 6);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_DEEMPH_GEN2_6DB,
> -			   imx6_pcie->tx_deemph_gen2_6db << 12);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_FULL,
> -			   imx6_pcie->tx_swing_full << 18);
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR8,
> -			   IMX6Q_GPR8_TX_SWING_LOW,
> -			   imx6_pcie->tx_swing_low << 25);
>  }
>  
>  static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
> @@ -498,8 +547,11 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
>  	dw_pcie_writel_rc(pp, PCIE_RC_LCR, tmp);
>  
>  	/* Start LTSSM. */
> -	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> -			IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
> +	if (imx6_pcie->variant == IMX7D)
> +		reset_control_deassert(imx6_pcie->apps_reset);
> +	else
> +		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> +				   IMX6Q_GPR12_PCIE_CTL_2, 1 << 10);
>  
>  	ret = imx6_pcie_wait_for_link(imx6_pcie);
>  	if (ret) {
> @@ -676,13 +728,31 @@ static int imx6_pcie_probe(struct platform_device *pdev)
>  		return PTR_ERR(imx6_pcie->pcie);
>  	}
>  
> -	if (imx6_pcie->variant == IMX6SX) {
> +	switch (imx6_pcie->variant) {
> +	case IMX6SX:
>  		imx6_pcie->pcie_inbound_axi = devm_clk_get(dev,
>  							   "pcie_inbound_axi");
>  		if (IS_ERR(imx6_pcie->pcie_inbound_axi)) {
>  			dev_err(dev, "pcie_inbound_axi clock missing or invalid\n");
>  			return PTR_ERR(imx6_pcie->pcie_inbound_axi);
>  		}
> +		break;
> +	case IMX7D:
> +		imx6_pcie->pciephy_reset = devm_reset_control_get(dev,
> +								  "pciephy");
> +		if (IS_ERR(imx6_pcie->pciephy_reset)) {
> +			dev_err(dev, "Failed to get PCIEPHY reset contol\n");
> +			return PTR_ERR(imx6_pcie->pciephy_reset);
> +		}
> +
> +		imx6_pcie->apps_reset = devm_reset_control_get(dev, "apps");
> +		if (IS_ERR(imx6_pcie->apps_reset)) {
> +			dev_err(dev, "Failed to get PCIE APPS reset contol\n");
> +			return PTR_ERR(imx6_pcie->apps_reset);
> +		}
> +		break;
> +	default:
> +		break;
>  	}
>  
>  	/* Grab GPR config register range */
> @@ -740,6 +810,7 @@ static const struct of_device_id imx6_pcie_of_match[] = {
>  	{ .compatible = "fsl,imx6q-pcie",  .data = (void *)IMX6Q,  },
>  	{ .compatible = "fsl,imx6sx-pcie", .data = (void *)IMX6SX, },
>  	{ .compatible = "fsl,imx6qp-pcie", .data = (void *)IMX6QP, },
> +	{ .compatible = "fsl,imx7d-pcie",  .data = (void *)IMX7D,  },
>  	{},
>  };
>  
> diff --git a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> index 4585d61..abbd524 100644
> --- a/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> +++ b/include/linux/mfd/syscon/imx7-iomuxc-gpr.h
> @@ -44,4 +44,8 @@
>  
>  #define IMX7D_GPR5_CSI_MUX_CONTROL_MIPI		(0x1 << 4)
>  
> +#define IMX7D_GPR12_PCIE_PHY_REFCLK_SEL		BIT(5)
> +
> +#define IMX7D_GPR22_PCIE_PHY_PLL_LOCKED		BIT(31)
> +
>  #endif /* __LINUX_IMX7_IOMUXC_GPR_H */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v4 0/3] i.MX7 PCI support

[Bjorn Helgaas]

On Tue, Feb 07, 2017 at 07:50:24AM -0800, Andrey Smirnov wrote:
> Hello, everyone:
> 
> This is a fourth iteration of the code that adds PCI-subsystem bits
> necessary for enabling PCI support on i.MX7.
> 
> Changes since v3 (can be found at [version3]):
> 	- Move all of the reset_control_assert's into imx6_pcie_assert_core_reset
> 	- Documented required reset and power domain DT bindings
> 
> Changes since v2 (can be found at [version2]):
> 
> 	- Collected Reviewed-by for patch #2 from Lucas
> 	- Reset logic implementation moved out into a reset controller
>           driver (see [reset1])
> 	- Removed unused leftover code
> 
> Changes since v1 (can be found at [version1]):
> 
> 	- All GPC related code moved into a separate driver (see [gpc1])
> 	- Removed GPIO probe deferral logging
> 	- Fixed section mismatch warning
> 	- Minor reformatting of fsl,imx6q-pcie.txt(as per Rob
>           Herring's request)
> 
> [version3] https://lkml.org/lkml/2017/2/6/565
> [version2] https://lkml.org/lkml/2017/2/1/510
> [version1] https://lkml.org/lkml/2017/1/19/488
> [gpc1] https://lkml.org/lkml/2017/2/6/551
> [reset1] https://lkml.org/lkml/2017/2/6/554
> 
> Andrey Smirnov (3):
>   PCI: imx6: Fix a typo in error message
>   PCI: imx6: Allow probe deferal by reset GPIO
>   PCI: imx6: Add code to support i.MX7D
> 
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 +-
>  drivers/pci/host/pci-imx6.c                        | 131 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 118 insertions(+), 30 deletions(-)

I applied patches 1 & 3 (with reviewed-by and ack from Lucas and Lee)
to pci/host-imx6 for v4.11, thanks!

I didn't apply 2 because of the section mismatch found by the autobuilder:

  >> WARNING: drivers/built-in.o(.data+0x6948c): Section mismatch in reference from the variable imx6_pcie_driver to the function .init.text:imx6_pcie_probe()
     The variable imx6_pcie_driver references
     the function __init imx6_pcie_probe()
     If the reference is valid then annotate the
     variable with or __refdata (see linux/init.h) or name the variable:

Please repost it after you figure out how to deal with that.

Bjorn

Re: [PATCH v4 0/3] i.MX7 PCI support

[Bjorn Helgaas]

On Tue, Feb 07, 2017 at 07:50:24AM -0800, Andrey Smirnov wrote:
> Hello, everyone:
> 
> This is a fourth iteration of the code that adds PCI-subsystem bits
> necessary for enabling PCI support on i.MX7.
> 
> Changes since v3 (can be found at [version3]):
> 	- Move all of the reset_control_assert's into imx6_pcie_assert_core_reset
> 	- Documented required reset and power domain DT bindings
> 
> Changes since v2 (can be found at [version2]):
> 
> 	- Collected Reviewed-by for patch #2 from Lucas
> 	- Reset logic implementation moved out into a reset controller
>           driver (see [reset1])
> 	- Removed unused leftover code
> 
> Changes since v1 (can be found at [version1]):
> 
> 	- All GPC related code moved into a separate driver (see [gpc1])
> 	- Removed GPIO probe deferral logging
> 	- Fixed section mismatch warning
> 	- Minor reformatting of fsl,imx6q-pcie.txt(as per Rob
>           Herring's request)
> 
> [version3] https://lkml.org/lkml/2017/2/6/565
> [version2] https://lkml.org/lkml/2017/2/1/510
> [version1] https://lkml.org/lkml/2017/1/19/488
> [gpc1] https://lkml.org/lkml/2017/2/6/551
> [reset1] https://lkml.org/lkml/2017/2/6/554
> 
> Andrey Smirnov (3):
>   PCI: imx6: Fix a typo in error message
>   PCI: imx6: Allow probe deferal by reset GPIO
>   PCI: imx6: Add code to support i.MX7D
> 
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 +-
>  drivers/pci/host/pci-imx6.c                        | 131 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 118 insertions(+), 30 deletions(-)

I applied patches 1 & 3 (with reviewed-by and ack from Lucas and Lee)
to pci/host-imx6 for v4.11, thanks!

I didn't apply 2 because of the section mismatch found by the autobuilder:

  >> WARNING: drivers/built-in.o(.data+0x6948c): Section mismatch in reference from the variable imx6_pcie_driver to the function .init.text:imx6_pcie_probe()
     The variable imx6_pcie_driver references
     the function __init imx6_pcie_probe()
     If the reference is valid then annotate the
     variable with or __refdata (see linux/init.h) or name the variable:

Please repost it after you figure out how to deal with that.

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 0/3] i.MX7 PCI support

[Bjorn Helgaas]

On Tue, Feb 07, 2017 at 07:50:24AM -0800, Andrey Smirnov wrote:
> Hello, everyone:
> 
> This is a fourth iteration of the code that adds PCI-subsystem bits
> necessary for enabling PCI support on i.MX7.
> 
> Changes since v3 (can be found at [version3]):
> 	- Move all of the reset_control_assert's into imx6_pcie_assert_core_reset
> 	- Documented required reset and power domain DT bindings
> 
> Changes since v2 (can be found at [version2]):
> 
> 	- Collected Reviewed-by for patch #2 from Lucas
> 	- Reset logic implementation moved out into a reset controller
>           driver (see [reset1])
> 	- Removed unused leftover code
> 
> Changes since v1 (can be found at [version1]):
> 
> 	- All GPC related code moved into a separate driver (see [gpc1])
> 	- Removed GPIO probe deferral logging
> 	- Fixed section mismatch warning
> 	- Minor reformatting of fsl,imx6q-pcie.txt(as per Rob
>           Herring's request)
> 
> [version3] https://lkml.org/lkml/2017/2/6/565
> [version2] https://lkml.org/lkml/2017/2/1/510
> [version1] https://lkml.org/lkml/2017/1/19/488
> [gpc1] https://lkml.org/lkml/2017/2/6/551
> [reset1] https://lkml.org/lkml/2017/2/6/554
> 
> Andrey Smirnov (3):
>   PCI: imx6: Fix a typo in error message
>   PCI: imx6: Allow probe deferal by reset GPIO
>   PCI: imx6: Add code to support i.MX7D
> 
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 +-
>  drivers/pci/host/pci-imx6.c                        | 131 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 118 insertions(+), 30 deletions(-)

I applied patches 1 & 3 (with reviewed-by and ack from Lucas and Lee)
to pci/host-imx6 for v4.11, thanks!

I didn't apply 2 because of the section mismatch found by the autobuilder:

  >> WARNING: drivers/built-in.o(.data+0x6948c): Section mismatch in reference from the variable imx6_pcie_driver to the function .init.text:imx6_pcie_probe()
     The variable imx6_pcie_driver references
     the function __init imx6_pcie_probe()
     If the reference is valid then annotate the
     variable with or __refdata (see linux/init.h) or name the variable:

Please repost it after you figure out how to deal with that.

Bjorn

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky@gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain

This domain is just the PHY? Seems like this needs a separate PHY 
driver.

> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"

And for this too.

> +	       - "apps"
> +
>  Example:
>  
>  	pcie@0x01000000 {

[...]

> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,

So the difference with i.MX7D is not really that it has a reset or not, 
but some platforms use a reset driver and some do not. The latter should 
be fixed.

Rob

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> Cc: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain

This domain is just the PHY? Seems like this needs a separate PHY 
driver.

> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"

And for this too.

> +	       - "apps"
> +
>  Example:
>  
>  	pcie@0x01000000 {

[...]

> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,

So the difference with i.MX7D is not really that it has a reset or not, 
but some platforms use a reset driver and some do not. The latter should 
be fixed.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> Add various bits of code needed to support i.MX7D variant of the IP.
> 
> Cc: yurovsky at gmail.com
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: devicetree at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>  3 files changed, 112 insertions(+), 26 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index 83aeb1f..11db2ab 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>  and thus inherits all the common properties defined in designware-pcie.txt.
>  
>  Required properties:
> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> +- compatible:
> +	- "fsl,imx6q-pcie"
> +	- "fsl,imx6sx-pcie",
> +	- "fsl,imx6qp-pcie"
> +	- "fsl,imx7d-pcie"
>  - reg: base address and length of the PCIe controller
>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>    entry for each entry in the interrupt-names property.
> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>  - clock names: Must include the following additional entries:
>  	- "pcie_inbound_axi"
>  
> +Additional required properties for imx7d-pcie:
> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain

This domain is just the PHY? Seems like this needs a separate PHY 
driver.

> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> +- reset-names: Must contain the following entires:
> +  	       - "pciephy"

And for this too.

> +	       - "apps"
> +
>  Example:
>  
>  	pcie at 0x01000000 {

[...]

> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>  	u32 val, gpr1, gpr12;
>  
>  	switch (imx6_pcie->variant) {
> +	case IMX7D:
> +		reset_control_assert(imx6_pcie->pciephy_reset);
> +		reset_control_assert(imx6_pcie->apps_reset);
> +		break;
>  	case IMX6SX:
>  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,

So the difference with i.MX7D is not really that it has a reset or not, 
but some platforms use a reset driver and some do not. The latter should 
be fixed.

Rob

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Bjorn Helgaas]

On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > Cc: yurovsky@gmail.com
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >  3 files changed, 112 insertions(+), 26 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > index 83aeb1f..11db2ab 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >  
> >  Required properties:
> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> > +- compatible:
> > +	- "fsl,imx6q-pcie"
> > +	- "fsl,imx6sx-pcie",
> > +	- "fsl,imx6qp-pcie"
> > +	- "fsl,imx7d-pcie"
> >  - reg: base address and length of the PCIe controller
> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >    entry for each entry in the interrupt-names property.
> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >  - clock names: Must include the following additional entries:
> >  	- "pcie_inbound_axi"
> >  
> > +Additional required properties for imx7d-pcie:
> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> 
> This domain is just the PHY? Seems like this needs a separate PHY 
> driver.
> 
> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> > +- reset-names: Must contain the following entires:
> > +  	       - "pciephy"
> 
> And for this too.
> 
> > +	       - "apps"
> > +
> >  Example:
> >  
> >  	pcie@0x01000000 {
> 
> [...]
> 
> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	u32 val, gpr1, gpr12;
> >  
> >  	switch (imx6_pcie->variant) {
> > +	case IMX7D:
> > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > +		reset_control_assert(imx6_pcie->apps_reset);
> > +		break;
> >  	case IMX6SX:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> 
> So the difference with i.MX7D is not really that it has a reset or not, 
> but some platforms use a reset driver and some do not. The latter should 
> be fixed.

I have this patch queued for v4.11.  Are these things that should be
fixed first?  If so, I can drop this.

Bjorn

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Bjorn Helgaas]

On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > Cc: yurovsky@gmail.com
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >  3 files changed, 112 insertions(+), 26 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > index 83aeb1f..11db2ab 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >  
> >  Required properties:
> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> > +- compatible:
> > +	- "fsl,imx6q-pcie"
> > +	- "fsl,imx6sx-pcie",
> > +	- "fsl,imx6qp-pcie"
> > +	- "fsl,imx7d-pcie"
> >  - reg: base address and length of the PCIe controller
> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >    entry for each entry in the interrupt-names property.
> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >  - clock names: Must include the following additional entries:
> >  	- "pcie_inbound_axi"
> >  
> > +Additional required properties for imx7d-pcie:
> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> 
> This domain is just the PHY? Seems like this needs a separate PHY 
> driver.
> 
> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> > +- reset-names: Must contain the following entires:
> > +  	       - "pciephy"
> 
> And for this too.
> 
> > +	       - "apps"
> > +
> >  Example:
> >  
> >  	pcie@0x01000000 {
> 
> [...]
> 
> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	u32 val, gpr1, gpr12;
> >  
> >  	switch (imx6_pcie->variant) {
> > +	case IMX7D:
> > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > +		reset_control_assert(imx6_pcie->apps_reset);
> > +		break;
> >  	case IMX6SX:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> 
> So the difference with i.MX7D is not really that it has a reset or not, 
> but some platforms use a reset driver and some do not. The latter should 
> be fixed.

I have this patch queued for v4.11.  Are these things that should be
fixed first?  If so, I can drop this.

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Bjorn Helgaas]

On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > Cc: yurovsky at gmail.com
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: devicetree at vger.kernel.org
> > Cc: linux-kernel at vger.kernel.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >  3 files changed, 112 insertions(+), 26 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > index 83aeb1f..11db2ab 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >  
> >  Required properties:
> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> > +- compatible:
> > +	- "fsl,imx6q-pcie"
> > +	- "fsl,imx6sx-pcie",
> > +	- "fsl,imx6qp-pcie"
> > +	- "fsl,imx7d-pcie"
> >  - reg: base address and length of the PCIe controller
> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >    entry for each entry in the interrupt-names property.
> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >  - clock names: Must include the following additional entries:
> >  	- "pcie_inbound_axi"
> >  
> > +Additional required properties for imx7d-pcie:
> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> 
> This domain is just the PHY? Seems like this needs a separate PHY 
> driver.
> 
> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> > +- reset-names: Must contain the following entires:
> > +  	       - "pciephy"
> 
> And for this too.
> 
> > +	       - "apps"
> > +
> >  Example:
> >  
> >  	pcie at 0x01000000 {
> 
> [...]
> 
> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	u32 val, gpr1, gpr12;
> >  
> >  	switch (imx6_pcie->variant) {
> > +	case IMX7D:
> > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > +		reset_control_assert(imx6_pcie->apps_reset);
> > +		break;
> >  	case IMX6SX:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> 
> So the difference with i.MX7D is not really that it has a reset or not, 
> but some platforms use a reset driver and some do not. The latter should 
> be fixed.

I have this patch queued for v4.11.  Are these things that should be
fixed first?  If so, I can drop this.

Bjorn

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > Add various bits of code needed to support i.MX7D variant of the IP.

> > 
> > [...]
> > 
> > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > >  	u32 val, gpr1, gpr12;
> > >  
> > >  	switch (imx6_pcie->variant) {
> > > +	case IMX7D:
> > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > +		break;
> > >  	case IMX6SX:
> > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > 
> > So the difference with i.MX7D is not really that it has a reset or not, 
> > but some platforms use a reset driver and some do not. The latter should 
> > be fixed.
> 
> I have this patch queued for v4.11.  Are these things that should be
> fixed first?  If so, I can drop this.

Well, depends if you trust things will get fixed later and if the PHY 
in fact should be separate as that affects the binding. It would affect 
how the driver changes are done as instead of "if (IMX7D) ...", you'd 
have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
how much churn you want there.

Rob

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > Add various bits of code needed to support i.MX7D variant of the IP.

> > 
> > [...]
> > 
> > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > >  	u32 val, gpr1, gpr12;
> > >  
> > >  	switch (imx6_pcie->variant) {
> > > +	case IMX7D:
> > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > +		break;
> > >  	case IMX6SX:
> > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > 
> > So the difference with i.MX7D is not really that it has a reset or not, 
> > but some platforms use a reset driver and some do not. The latter should 
> > be fixed.
> 
> I have this patch queued for v4.11.  Are these things that should be
> fixed first?  If so, I can drop this.

Well, depends if you trust things will get fixed later and if the PHY 
in fact should be separate as that affects the binding. It would affect 
how the driver changes are done as instead of "if (IMX7D) ...", you'd 
have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
how much churn you want there.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > Add various bits of code needed to support i.MX7D variant of the IP.

> > 
> > [...]
> > 
> > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > >  	u32 val, gpr1, gpr12;
> > >  
> > >  	switch (imx6_pcie->variant) {
> > > +	case IMX7D:
> > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > +		break;
> > >  	case IMX6SX:
> > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > 
> > So the difference with i.MX7D is not really that it has a reset or not, 
> > but some platforms use a reset driver and some do not. The latter should 
> > be fixed.
> 
> I have this patch queued for v4.11.  Are these things that should be
> fixed first?  If so, I can drop this.

Well, depends if you trust things will get fixed later and if the PHY 
in fact should be separate as that affects the binding. It would affect 
how the driver changes are done as instead of "if (IMX7D) ...", you'd 
have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
how much churn you want there.

Rob

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Bjorn Helgaas]

On Wed, Feb 15, 2017 at 03:26:24PM -0600, Rob Herring wrote:
> On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> > On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > > Add various bits of code needed to support i.MX7D variant of the IP.
> 
> > > 
> > > [...]
> > > 
> > > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > > >  	u32 val, gpr1, gpr12;
> > > >  
> > > >  	switch (imx6_pcie->variant) {
> > > > +	case IMX7D:
> > > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > > +		break;
> > > >  	case IMX6SX:
> > > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > > 
> > > So the difference with i.MX7D is not really that it has a reset or not, 
> > > but some platforms use a reset driver and some do not. The latter should 
> > > be fixed.
> > 
> > I have this patch queued for v4.11.  Are these things that should be
> > fixed first?  If so, I can drop this.
> 
> Well, depends if you trust things will get fixed later and if the PHY 
> in fact should be separate as that affects the binding. It would affect 
> how the driver changes are done as instead of "if (IMX7D) ...", you'd 
> have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
> how much churn you want there.

I dropped it for now, not that I don't trust it will get fixed, but it
sounds like not completely trivial changes and will affect the binding
as well, so the intermediate state sounds a little messy.

Bjorn

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Bjorn Helgaas]

On Wed, Feb 15, 2017 at 03:26:24PM -0600, Rob Herring wrote:
> On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> > On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > > Add various bits of code needed to support i.MX7D variant of the IP.
> 
> > > 
> > > [...]
> > > 
> > > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > > >  	u32 val, gpr1, gpr12;
> > > >  
> > > >  	switch (imx6_pcie->variant) {
> > > > +	case IMX7D:
> > > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > > +		break;
> > > >  	case IMX6SX:
> > > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > > 
> > > So the difference with i.MX7D is not really that it has a reset or not, 
> > > but some platforms use a reset driver and some do not. The latter should 
> > > be fixed.
> > 
> > I have this patch queued for v4.11.  Are these things that should be
> > fixed first?  If so, I can drop this.
> 
> Well, depends if you trust things will get fixed later and if the PHY 
> in fact should be separate as that affects the binding. It would affect 
> how the driver changes are done as instead of "if (IMX7D) ...", you'd 
> have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
> how much churn you want there.

I dropped it for now, not that I don't trust it will get fixed, but it
sounds like not completely trivial changes and will affect the binding
as well, so the intermediate state sounds a little messy.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Bjorn Helgaas]

On Wed, Feb 15, 2017 at 03:26:24PM -0600, Rob Herring wrote:
> On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> > On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > > Add various bits of code needed to support i.MX7D variant of the IP.
> 
> > > 
> > > [...]
> > > 
> > > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > > >  	u32 val, gpr1, gpr12;
> > > >  
> > > >  	switch (imx6_pcie->variant) {
> > > > +	case IMX7D:
> > > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > > +		break;
> > > >  	case IMX6SX:
> > > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > > 
> > > So the difference with i.MX7D is not really that it has a reset or not, 
> > > but some platforms use a reset driver and some do not. The latter should 
> > > be fixed.
> > 
> > I have this patch queued for v4.11.  Are these things that should be
> > fixed first?  If so, I can drop this.
> 
> Well, depends if you trust things will get fixed later and if the PHY 
> in fact should be separate as that affects the binding. It would affect 
> how the driver changes are done as instead of "if (IMX7D) ...", you'd 
> have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
> how much churn you want there.

I dropped it for now, not that I don't trust it will get fixed, but it
sounds like not completely trivial changes and will affect the binding
as well, so the intermediate state sounds a little messy.

Bjorn

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Bjorn Helgaas]

On Wed, Feb 15, 2017 at 03:26:24PM -0600, Rob Herring wrote:
> On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> > On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > > Add various bits of code needed to support i.MX7D variant of the IP.
> 
> > > 
> > > [...]
> > > 
> > > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > > >  	u32 val, gpr1, gpr12;
> > > >  
> > > >  	switch (imx6_pcie->variant) {
> > > > +	case IMX7D:
> > > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > > +		break;
> > > >  	case IMX6SX:
> > > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > > 
> > > So the difference with i.MX7D is not really that it has a reset or not, 
> > > but some platforms use a reset driver and some do not. The latter should 
> > > be fixed.
> > 
> > I have this patch queued for v4.11.  Are these things that should be
> > fixed first?  If so, I can drop this.
> 
> Well, depends if you trust things will get fixed later and if the PHY 
> in fact should be separate as that affects the binding. It would affect 
> how the driver changes are done as instead of "if (IMX7D) ...", you'd 
> have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
> how much churn you want there.

I dropped it for now, not that I don't trust it will get fixed, but it
sounds like not completely trivial changes and will affect the binding
as well, so the intermediate state sounds a little messy.

Bjorn

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Andrey Smirnov]

On Wed, Feb 15, 2017 at 9:17 AM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
>> Add various bits of code needed to support i.MX7D variant of the IP.
>>
>> Cc: yurovsky@gmail.com
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>>  3 files changed, 112 insertions(+), 26 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> index 83aeb1f..11db2ab 100644
>> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>>  and thus inherits all the common properties defined in designware-pcie.txt.
>>
>>  Required properties:
>> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
>> +- compatible:
>> +     - "fsl,imx6q-pcie"
>> +     - "fsl,imx6sx-pcie",
>> +     - "fsl,imx6qp-pcie"
>> +     - "fsl,imx7d-pcie"
>>  - reg: base address and length of the PCIe controller
>>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>>    entry for each entry in the interrupt-names property.
>> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>>  - clock names: Must include the following additional entries:
>>       - "pcie_inbound_axi"
>>
>> +Additional required properties for imx7d-pcie:
>> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
>
> This domain is just the PHY? Seems like this needs a separate PHY
> driver.

PCIE_PHY is the name of the power domain corresponding to PGC_PCIE
(which is what that property is expected to point to) as per
Frescale/NXP datasheet (p. 822 in v0.1 of i.MX7 Application Processors
Manual). I was never able to find any clear language indicating what
parts of DesignWare's IP core and Freescale's/NXP's PCIE PHY it powers
in the manual. However, experiments with hardware show that when that
domain remains non-powered any attempt to access registers of DW's IP
block result in system hanging, so it seemed to me that the two are
not independent of each other enough to be represented as individual
DT nodes.

>
>> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
>> +- reset-names: Must contain the following entires:
>> +            - "pciephy"
>
> And for this too.
>
>> +            - "apps"
>> +
>>  Example:
>>
>>       pcie@0x01000000 {
>
> [...]
>
>> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>>       u32 val, gpr1, gpr12;
>>
>>       switch (imx6_pcie->variant) {
>> +     case IMX7D:
>> +             reset_control_assert(imx6_pcie->pciephy_reset);
>> +             reset_control_assert(imx6_pcie->apps_reset);
>> +             break;
>>       case IMX6SX:
>>               regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>>                                  IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>
> So the difference with i.MX7D is not really that it has a reset or not,
> but some platforms use a reset driver and some do not. The latter should
> be fixed.

That depends on what variant of the SoC you are comparing it to. 6QP,
6SX do have reset and helper signals wire to bits in registers in
IOMUX, 6Q howerver doesn't have a reset line wire and have to do some
trickery as per comment in the driver several lines below:

"... As there is no dedicated reset signal wired up for MX6QDL, we
need to manually force LTSSM into "detect" state before completely
disabling LTSSM, which is a prerequisite for core configuration..."

If memory serves me well part of that 6Q trickery code is the reason
for driver using hook_fault_code().

That is not to say that all of this code could not be encapsulated as
a reset controller, and I agree, doing so might make the driver
better. At the same time I don't have the hardware to test all of
those platforms and I am hoping we can agree this kind of change to be
out of scope of this series.

Thanks,
Andrey Smironv

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Andrey Smirnov]

On Wed, Feb 15, 2017 at 9:17 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
>> Add various bits of code needed to support i.MX7D variant of the IP.
>>
>> Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> Cc: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
>> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>>  3 files changed, 112 insertions(+), 26 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> index 83aeb1f..11db2ab 100644
>> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>>  and thus inherits all the common properties defined in designware-pcie.txt.
>>
>>  Required properties:
>> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
>> +- compatible:
>> +     - "fsl,imx6q-pcie"
>> +     - "fsl,imx6sx-pcie",
>> +     - "fsl,imx6qp-pcie"
>> +     - "fsl,imx7d-pcie"
>>  - reg: base address and length of the PCIe controller
>>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>>    entry for each entry in the interrupt-names property.
>> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>>  - clock names: Must include the following additional entries:
>>       - "pcie_inbound_axi"
>>
>> +Additional required properties for imx7d-pcie:
>> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
>
> This domain is just the PHY? Seems like this needs a separate PHY
> driver.

PCIE_PHY is the name of the power domain corresponding to PGC_PCIE
(which is what that property is expected to point to) as per
Frescale/NXP datasheet (p. 822 in v0.1 of i.MX7 Application Processors
Manual). I was never able to find any clear language indicating what
parts of DesignWare's IP core and Freescale's/NXP's PCIE PHY it powers
in the manual. However, experiments with hardware show that when that
domain remains non-powered any attempt to access registers of DW's IP
block result in system hanging, so it seemed to me that the two are
not independent of each other enough to be represented as individual
DT nodes.

>
>> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
>> +- reset-names: Must contain the following entires:
>> +            - "pciephy"
>
> And for this too.
>
>> +            - "apps"
>> +
>>  Example:
>>
>>       pcie@0x01000000 {
>
> [...]
>
>> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>>       u32 val, gpr1, gpr12;
>>
>>       switch (imx6_pcie->variant) {
>> +     case IMX7D:
>> +             reset_control_assert(imx6_pcie->pciephy_reset);
>> +             reset_control_assert(imx6_pcie->apps_reset);
>> +             break;
>>       case IMX6SX:
>>               regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>>                                  IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>
> So the difference with i.MX7D is not really that it has a reset or not,
> but some platforms use a reset driver and some do not. The latter should
> be fixed.

That depends on what variant of the SoC you are comparing it to. 6QP,
6SX do have reset and helper signals wire to bits in registers in
IOMUX, 6Q howerver doesn't have a reset line wire and have to do some
trickery as per comment in the driver several lines below:

"... As there is no dedicated reset signal wired up for MX6QDL, we
need to manually force LTSSM into "detect" state before completely
disabling LTSSM, which is a prerequisite for core configuration..."

If memory serves me well part of that 6Q trickery code is the reason
for driver using hook_fault_code().

That is not to say that all of this code could not be encapsulated as
a reset controller, and I agree, doing so might make the driver
better. At the same time I don't have the hardware to test all of
those platforms and I am hoping we can agree this kind of change to be
out of scope of this series.

Thanks,
Andrey Smironv
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Andrey Smirnov]

On Wed, Feb 15, 2017 at 9:17 AM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
>> Add various bits of code needed to support i.MX7D variant of the IP.
>>
>> Cc: yurovsky@gmail.com
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>>  3 files changed, 112 insertions(+), 26 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> index 83aeb1f..11db2ab 100644
>> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>>  and thus inherits all the common properties defined in designware-pcie.txt.
>>
>>  Required properties:
>> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
>> +- compatible:
>> +     - "fsl,imx6q-pcie"
>> +     - "fsl,imx6sx-pcie",
>> +     - "fsl,imx6qp-pcie"
>> +     - "fsl,imx7d-pcie"
>>  - reg: base address and length of the PCIe controller
>>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>>    entry for each entry in the interrupt-names property.
>> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>>  - clock names: Must include the following additional entries:
>>       - "pcie_inbound_axi"
>>
>> +Additional required properties for imx7d-pcie:
>> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
>
> This domain is just the PHY? Seems like this needs a separate PHY
> driver.

PCIE_PHY is the name of the power domain corresponding to PGC_PCIE
(which is what that property is expected to point to) as per
Frescale/NXP datasheet (p. 822 in v0.1 of i.MX7 Application Processors
Manual). I was never able to find any clear language indicating what
parts of DesignWare's IP core and Freescale's/NXP's PCIE PHY it powers
in the manual. However, experiments with hardware show that when that
domain remains non-powered any attempt to access registers of DW's IP
block result in system hanging, so it seemed to me that the two are
not independent of each other enough to be represented as individual
DT nodes.

>
>> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
>> +- reset-names: Must contain the following entires:
>> +            - "pciephy"
>
> And for this too.
>
>> +            - "apps"
>> +
>>  Example:
>>
>>       pcie@0x01000000 {
>
> [...]
>
>> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>>       u32 val, gpr1, gpr12;
>>
>>       switch (imx6_pcie->variant) {
>> +     case IMX7D:
>> +             reset_control_assert(imx6_pcie->pciephy_reset);
>> +             reset_control_assert(imx6_pcie->apps_reset);
>> +             break;
>>       case IMX6SX:
>>               regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>>                                  IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>
> So the difference with i.MX7D is not really that it has a reset or not,
> but some platforms use a reset driver and some do not. The latter should
> be fixed.

That depends on what variant of the SoC you are comparing it to. 6QP,
6SX do have reset and helper signals wire to bits in registers in
IOMUX, 6Q howerver doesn't have a reset line wire and have to do some
trickery as per comment in the driver several lines below:

"... As there is no dedicated reset signal wired up for MX6QDL, we
need to manually force LTSSM into "detect" state before completely
disabling LTSSM, which is a prerequisite for core configuration..."

If memory serves me well part of that 6Q trickery code is the reason
for driver using hook_fault_code().

That is not to say that all of this code could not be encapsulated as
a reset controller, and I agree, doing so might make the driver
better. At the same time I don't have the hardware to test all of
those platforms and I am hoping we can agree this kind of change to be
out of scope of this series.

Thanks,
Andrey Smironv

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Andrey Smirnov]

On Wed, Feb 15, 2017 at 9:17 AM, Rob Herring <robh@kernel.org> wrote:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
>> Add various bits of code needed to support i.MX7D variant of the IP.
>>
>> Cc: yurovsky at gmail.com
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Lee Jones <lee.jones@linaro.org>
>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> Cc: linux-arm-kernel at lists.infradead.org
>> Cc: devicetree at vger.kernel.org
>> Cc: linux-kernel at vger.kernel.org
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> ---
>>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>>  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>>  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>>  3 files changed, 112 insertions(+), 26 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> index 83aeb1f..11db2ab 100644
>> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>>  and thus inherits all the common properties defined in designware-pcie.txt.
>>
>>  Required properties:
>> -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
>> +- compatible:
>> +     - "fsl,imx6q-pcie"
>> +     - "fsl,imx6sx-pcie",
>> +     - "fsl,imx6qp-pcie"
>> +     - "fsl,imx7d-pcie"
>>  - reg: base address and length of the PCIe controller
>>  - interrupts: A list of interrupt outputs of the controller. Must contain an
>>    entry for each entry in the interrupt-names property.
>> @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>>  - clock names: Must include the following additional entries:
>>       - "pcie_inbound_axi"
>>
>> +Additional required properties for imx7d-pcie:
>> +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
>
> This domain is just the PHY? Seems like this needs a separate PHY
> driver.

PCIE_PHY is the name of the power domain corresponding to PGC_PCIE
(which is what that property is expected to point to) as per
Frescale/NXP datasheet (p. 822 in v0.1 of i.MX7 Application Processors
Manual). I was never able to find any clear language indicating what
parts of DesignWare's IP core and Freescale's/NXP's PCIE PHY it powers
in the manual. However, experiments with hardware show that when that
domain remains non-powered any attempt to access registers of DW's IP
block result in system hanging, so it seemed to me that the two are
not independent of each other enough to be represented as individual
DT nodes.

>
>> +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
>> +- reset-names: Must contain the following entires:
>> +            - "pciephy"
>
> And for this too.
>
>> +            - "apps"
>> +
>>  Example:
>>
>>       pcie at 0x01000000 {
>
> [...]
>
>> @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>>       u32 val, gpr1, gpr12;
>>
>>       switch (imx6_pcie->variant) {
>> +     case IMX7D:
>> +             reset_control_assert(imx6_pcie->pciephy_reset);
>> +             reset_control_assert(imx6_pcie->apps_reset);
>> +             break;
>>       case IMX6SX:
>>               regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>>                                  IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>
> So the difference with i.MX7D is not really that it has a reset or not,
> but some platforms use a reset driver and some do not. The latter should
> be fixed.

That depends on what variant of the SoC you are comparing it to. 6QP,
6SX do have reset and helper signals wire to bits in registers in
IOMUX, 6Q howerver doesn't have a reset line wire and have to do some
trickery as per comment in the driver several lines below:

"... As there is no dedicated reset signal wired up for MX6QDL, we
need to manually force LTSSM into "detect" state before completely
disabling LTSSM, which is a prerequisite for core configuration..."

If memory serves me well part of that 6Q trickery code is the reason
for driver using hook_fault_code().

That is not to say that all of this code could not be encapsulated as
a reset controller, and I agree, doing so might make the driver
better. At the same time I don't have the hardware to test all of
those platforms and I am hoping we can agree this kind of change to be
out of scope of this series.

Thanks,
Andrey Smironv

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > Cc: yurovsky@gmail.com
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >  3 files changed, 112 insertions(+), 26 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > index 83aeb1f..11db2ab 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >  
> >  Required properties:
> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> > +- compatible:
> > +	- "fsl,imx6q-pcie"
> > +	- "fsl,imx6sx-pcie",
> > +	- "fsl,imx6qp-pcie"
> > +	- "fsl,imx7d-pcie"
> >  - reg: base address and length of the PCIe controller
> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >    entry for each entry in the interrupt-names property.
> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >  - clock names: Must include the following additional entries:
> >  	- "pcie_inbound_axi"
> >  
> > +Additional required properties for imx7d-pcie:
> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> 
> This domain is just the PHY? Seems like this needs a separate PHY 
> driver.
> 
No, it's called the PHY power domain, as that is probably the part that
draws the most power, but the PCIe core also looses it's state when this
domain is powered down. So it's probably the complete core that is
inside this domain.

> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> > +- reset-names: Must contain the following entires:
> > +  	       - "pciephy"
> 
> And for this too.
> 
> > +	       - "apps"
> > +
> >  Example:
> >  
> >  	pcie@0x01000000 {
> 
> [...]
> 
> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	u32 val, gpr1, gpr12;
> >  
> >  	switch (imx6_pcie->variant) {
> > +	case IMX7D:
> > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > +		reset_control_assert(imx6_pcie->apps_reset);
> > +		break;
> >  	case IMX6SX:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> 
> So the difference with i.MX7D is not really that it has a reset or not, 
> but some platforms use a reset driver and some do not. The latter should 
> be fixed.

The resets on anything before i.MX7 are not in a separate reset driver,
but are just some signals from the PCIe core wired into a syscon (IOMUX
GPR) area. While we could invent a reset controller for those, I don't
see how this would improve things. Especially as the reset on i.MX6
seems to be some side-effect of the "power-down" signal of the core, so
not strictly a reset.

Also I don't see why we should change the binding for the driver with a
long history of deployed DTs. That seems like a total waste of manpower.

Regards,
Lucas

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> > Cc: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > Cc: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> > Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
> > Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >  3 files changed, 112 insertions(+), 26 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > index 83aeb1f..11db2ab 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >  
> >  Required properties:
> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> > +- compatible:
> > +	- "fsl,imx6q-pcie"
> > +	- "fsl,imx6sx-pcie",
> > +	- "fsl,imx6qp-pcie"
> > +	- "fsl,imx7d-pcie"
> >  - reg: base address and length of the PCIe controller
> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >    entry for each entry in the interrupt-names property.
> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >  - clock names: Must include the following additional entries:
> >  	- "pcie_inbound_axi"
> >  
> > +Additional required properties for imx7d-pcie:
> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> 
> This domain is just the PHY? Seems like this needs a separate PHY 
> driver.
> 
No, it's called the PHY power domain, as that is probably the part that
draws the most power, but the PCIe core also looses it's state when this
domain is powered down. So it's probably the complete core that is
inside this domain.

> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> > +- reset-names: Must contain the following entires:
> > +  	       - "pciephy"
> 
> And for this too.
> 
> > +	       - "apps"
> > +
> >  Example:
> >  
> >  	pcie@0x01000000 {
> 
> [...]
> 
> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	u32 val, gpr1, gpr12;
> >  
> >  	switch (imx6_pcie->variant) {
> > +	case IMX7D:
> > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > +		reset_control_assert(imx6_pcie->apps_reset);
> > +		break;
> >  	case IMX6SX:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> 
> So the difference with i.MX7D is not really that it has a reset or not, 
> but some platforms use a reset driver and some do not. The latter should 
> be fixed.

The resets on anything before i.MX7 are not in a separate reset driver,
but are just some signals from the PCIe core wired into a syscon (IOMUX
GPR) area. While we could invent a reset controller for those, I don't
see how this would improve things. Especially as the reset on i.MX6
seems to be some side-effect of the "power-down" signal of the core, so
not strictly a reset.

Also I don't see why we should change the binding for the driver with a
long history of deployed DTs. That seems like a total waste of manpower.

Regards,
Lucas

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > Cc: yurovsky@gmail.com
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: devicetree@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >  3 files changed, 112 insertions(+), 26 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > index 83aeb1f..11db2ab 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >  
> >  Required properties:
> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> > +- compatible:
> > +	- "fsl,imx6q-pcie"
> > +	- "fsl,imx6sx-pcie",
> > +	- "fsl,imx6qp-pcie"
> > +	- "fsl,imx7d-pcie"
> >  - reg: base address and length of the PCIe controller
> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >    entry for each entry in the interrupt-names property.
> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >  - clock names: Must include the following additional entries:
> >  	- "pcie_inbound_axi"
> >  
> > +Additional required properties for imx7d-pcie:
> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> 
> This domain is just the PHY? Seems like this needs a separate PHY 
> driver.
> 
No, it's called the PHY power domain, as that is probably the part that
draws the most power, but the PCIe core also looses it's state when this
domain is powered down. So it's probably the complete core that is
inside this domain.

> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> > +- reset-names: Must contain the following entires:
> > +  	       - "pciephy"
> 
> And for this too.
> 
> > +	       - "apps"
> > +
> >  Example:
> >  
> >  	pcie@0x01000000 {
> 
> [...]
> 
> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	u32 val, gpr1, gpr12;
> >  
> >  	switch (imx6_pcie->variant) {
> > +	case IMX7D:
> > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > +		reset_control_assert(imx6_pcie->apps_reset);
> > +		break;
> >  	case IMX6SX:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> 
> So the difference with i.MX7D is not really that it has a reset or not, 
> but some platforms use a reset driver and some do not. The latter should 
> be fixed.

The resets on anything before i.MX7 are not in a separate reset driver,
but are just some signals from the PCIe core wired into a syscon (IOMUX
GPR) area. While we could invent a reset controller for those, I don't
see how this would improve things. Especially as the reset on i.MX6
seems to be some side-effect of the "power-down" signal of the core, so
not strictly a reset.

Also I don't see why we should change the binding for the driver with a
long history of deployed DTs. That seems like a total waste of manpower.

Regards,
Lucas


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > Cc: yurovsky at gmail.com
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Lee Jones <lee.jones@linaro.org>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: devicetree at vger.kernel.org
> > Cc: linux-kernel at vger.kernel.org
> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> > ---
> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >  3 files changed, 112 insertions(+), 26 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > index 83aeb1f..11db2ab 100644
> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >  
> >  Required properties:
> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> > +- compatible:
> > +	- "fsl,imx6q-pcie"
> > +	- "fsl,imx6sx-pcie",
> > +	- "fsl,imx6qp-pcie"
> > +	- "fsl,imx7d-pcie"
> >  - reg: base address and length of the PCIe controller
> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >    entry for each entry in the interrupt-names property.
> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >  - clock names: Must include the following additional entries:
> >  	- "pcie_inbound_axi"
> >  
> > +Additional required properties for imx7d-pcie:
> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> 
> This domain is just the PHY? Seems like this needs a separate PHY 
> driver.
> 
No, it's called the PHY power domain, as that is probably the part that
draws the most power, but the PCIe core also looses it's state when this
domain is powered down. So it's probably the complete core that is
inside this domain.

> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
> > +- reset-names: Must contain the following entires:
> > +  	       - "pciephy"
> 
> And for this too.
> 
> > +	       - "apps"
> > +
> >  Example:
> >  
> >  	pcie at 0x01000000 {
> 
> [...]
> 
> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> >  	u32 val, gpr1, gpr12;
> >  
> >  	switch (imx6_pcie->variant) {
> > +	case IMX7D:
> > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > +		reset_control_assert(imx6_pcie->apps_reset);
> > +		break;
> >  	case IMX6SX:
> >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> 
> So the difference with i.MX7D is not really that it has a reset or not, 
> but some platforms use a reset driver and some do not. The latter should 
> be fixed.

The resets on anything before i.MX7 are not in a separate reset driver,
but are just some signals from the PCIe core wired into a syscon (IOMUX
GPR) area. While we could invent a reset controller for those, I don't
see how this would improve things. Especially as the reset on i.MX6
seems to be some side-effect of the "power-down" signal of the core, so
not strictly a reset.

Also I don't see why we should change the binding for the driver with a
long history of deployed DTs. That seems like a total waste of manpower.

Regards,
Lucas

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Mittwoch, den 15.02.2017, 15:57 -0600 schrieb Bjorn Helgaas:
> On Wed, Feb 15, 2017 at 03:26:24PM -0600, Rob Herring wrote:
> > On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> > > On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > > > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > > > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > > > 
> > > > [...]
> > > > 
> > > > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > > > >  	u32 val, gpr1, gpr12;
> > > > >  
> > > > >  	switch (imx6_pcie->variant) {
> > > > > +	case IMX7D:
> > > > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > > > +		break;
> > > > >  	case IMX6SX:
> > > > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > > > 
> > > > So the difference with i.MX7D is not really that it has a reset or not, 
> > > > but some platforms use a reset driver and some do not. The latter should 
> > > > be fixed.
> > > 
> > > I have this patch queued for v4.11.  Are these things that should be
> > > fixed first?  If so, I can drop this.
> > 
> > Well, depends if you trust things will get fixed later and if the PHY 
> > in fact should be separate as that affects the binding. It would affect 
> > how the driver changes are done as instead of "if (IMX7D) ...", you'd 
> > have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
> > how much churn you want there.
> 
> I dropped it for now, not that I don't trust it will get fixed, but it
> sounds like not completely trivial changes and will affect the binding
> as well, so the intermediate state sounds a little messy.

As I pointed out in direct reply to Rob, I honestly think the binding is
fine as is and properly reflects the hardware. But I guess he'll comment
on that, so JFYI.

Regards,
Lucas

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Mittwoch, den 15.02.2017, 15:57 -0600 schrieb Bjorn Helgaas:
> On Wed, Feb 15, 2017 at 03:26:24PM -0600, Rob Herring wrote:
> > On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> > > On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > > > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > > > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > > > 
> > > > [...]
> > > > 
> > > > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > > > >  	u32 val, gpr1, gpr12;
> > > > >  
> > > > >  	switch (imx6_pcie->variant) {
> > > > > +	case IMX7D:
> > > > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > > > +		break;
> > > > >  	case IMX6SX:
> > > > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > > > 
> > > > So the difference with i.MX7D is not really that it has a reset or not, 
> > > > but some platforms use a reset driver and some do not. The latter should 
> > > > be fixed.
> > > 
> > > I have this patch queued for v4.11.  Are these things that should be
> > > fixed first?  If so, I can drop this.
> > 
> > Well, depends if you trust things will get fixed later and if the PHY 
> > in fact should be separate as that affects the binding. It would affect 
> > how the driver changes are done as instead of "if (IMX7D) ...", you'd 
> > have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
> > how much churn you want there.
> 
> I dropped it for now, not that I don't trust it will get fixed, but it
> sounds like not completely trivial changes and will affect the binding
> as well, so the intermediate state sounds a little messy.

As I pointed out in direct reply to Rob, I honestly think the binding is
fine as is and properly reflects the hardware. But I guess he'll comment
on that, so JFYI.

Regards,
Lucas

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Mittwoch, den 15.02.2017, 15:57 -0600 schrieb Bjorn Helgaas:
> On Wed, Feb 15, 2017 at 03:26:24PM -0600, Rob Herring wrote:
> > On Wed, Feb 15, 2017 at 11:38:50AM -0600, Bjorn Helgaas wrote:
> > > On Wed, Feb 15, 2017 at 11:17:00AM -0600, Rob Herring wrote:
> > > > On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> > > > > Add various bits of code needed to support i.MX7D variant of the IP.
> > 
> > > > 
> > > > [...]
> > > > 
> > > > > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
> > > > >  	u32 val, gpr1, gpr12;
> > > > >  
> > > > >  	switch (imx6_pcie->variant) {
> > > > > +	case IMX7D:
> > > > > +		reset_control_assert(imx6_pcie->pciephy_reset);
> > > > > +		reset_control_assert(imx6_pcie->apps_reset);
> > > > > +		break;
> > > > >  	case IMX6SX:
> > > > >  		regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > > > >  				   IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
> > > > 
> > > > So the difference with i.MX7D is not really that it has a reset or not, 
> > > > but some platforms use a reset driver and some do not. The latter should 
> > > > be fixed.
> > > 
> > > I have this patch queued for v4.11.  Are these things that should be
> > > fixed first?  If so, I can drop this.
> > 
> > Well, depends if you trust things will get fixed later and if the PHY 
> > in fact should be separate as that affects the binding. It would affect 
> > how the driver changes are done as instead of "if (IMX7D) ...", you'd 
> > have "if (imx6_pcie->apps_reset) ..." for example. That part depends on
> > how much churn you want there.
> 
> I dropped it for now, not that I don't trust it will get fixed, but it
> sounds like not completely trivial changes and will affect the binding
> as well, so the intermediate state sounds a little messy.

As I pointed out in direct reply to Rob, I honestly think the binding is
fine as is and properly reflects the hardware. But I guess he'll comment
on that, so JFYI.

Regards,
Lucas

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Thu, Feb 16, 2017 at 3:12 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
>> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
>> > Add various bits of code needed to support i.MX7D variant of the IP.
>> >
>> > Cc: yurovsky@gmail.com
>> > Cc: Lucas Stach <l.stach@pengutronix.de>
>> > Cc: Bjorn Helgaas <bhelgaas@google.com>
>> > Cc: Rob Herring <robh+dt@kernel.org>
>> > Cc: Mark Rutland <mark.rutland@arm.com>
>> > Cc: Lee Jones <lee.jones@linaro.org>
>> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> > Cc: linux-arm-kernel@lists.infradead.org
>> > Cc: devicetree@vger.kernel.org
>> > Cc: linux-kernel@vger.kernel.org
>> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> > ---
>> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>> >  3 files changed, 112 insertions(+), 26 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > index 83aeb1f..11db2ab 100644
>> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>> >  and thus inherits all the common properties defined in designware-pcie.txt.
>> >
>> >  Required properties:
>> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
>> > +- compatible:
>> > +   - "fsl,imx6q-pcie"
>> > +   - "fsl,imx6sx-pcie",
>> > +   - "fsl,imx6qp-pcie"
>> > +   - "fsl,imx7d-pcie"
>> >  - reg: base address and length of the PCIe controller
>> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
>> >    entry for each entry in the interrupt-names property.
>> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>> >  - clock names: Must include the following additional entries:
>> >     - "pcie_inbound_axi"
>> >
>> > +Additional required properties for imx7d-pcie:
>> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
>>
>> This domain is just the PHY? Seems like this needs a separate PHY
>> driver.
>>
> No, it's called the PHY power domain, as that is probably the part that
> draws the most power, but the PCIe core also looses it's state when this
> domain is powered down. So it's probably the complete core that is
> inside this domain.

A shared domain doesn't mean the phy and core should be 1 node. It is
the separate reset and clock for the PHY that tell me they should be
separate. And I'm pretty sure the DW block and PHY are separate. If
the PHY registers were part of the same register range, then I'd say
they should be one.

>> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
>> > +- reset-names: Must contain the following entires:
>> > +          - "pciephy"
>>
>> And for this too.
>>
>> > +          - "apps"
>> > +
>> >  Example:
>> >
>> >     pcie@0x01000000 {
>>
>> [...]
>>
>> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>> >     u32 val, gpr1, gpr12;
>> >
>> >     switch (imx6_pcie->variant) {
>> > +   case IMX7D:
>> > +           reset_control_assert(imx6_pcie->pciephy_reset);
>> > +           reset_control_assert(imx6_pcie->apps_reset);
>> > +           break;
>> >     case IMX6SX:
>> >             regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> >                                IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>>
>> So the difference with i.MX7D is not really that it has a reset or not,
>> but some platforms use a reset driver and some do not. The latter should
>> be fixed.
>
> The resets on anything before i.MX7 are not in a separate reset driver,
> but are just some signals from the PCIe core wired into a syscon (IOMUX
> GPR) area. While we could invent a reset controller for those, I don't
> see how this would improve things. Especially as the reset on i.MX6
> seems to be some side-effect of the "power-down" signal of the core, so
> not strictly a reset.
>
> Also I don't see why we should change the binding for the driver with a
> long history of deployed DTs. That seems like a total waste of manpower.

We can debate whether or not we change existing platforms. Maybe that
doesn't make sense now. But best practices should be considered when
adding new bindings rather than just extending existing bindings. We
didn't split out PHYs at one time and now we generally do. I'm not so
concerned with just adding i.MX7D, but really the next chip (and the
next).

Rob

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Thu, Feb 16, 2017 at 3:12 AM, Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
>> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
>> > Add various bits of code needed to support i.MX7D variant of the IP.
>> >
>> > Cc: yurovsky-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
>> > Cc: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> > Cc: Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> > Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
>> > Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > Cc: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
>> > Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>> > Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> > Signed-off-by: Andrey Smirnov <andrew.smirnov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> > ---
>> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>> >  3 files changed, 112 insertions(+), 26 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > index 83aeb1f..11db2ab 100644
>> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>> >  and thus inherits all the common properties defined in designware-pcie.txt.
>> >
>> >  Required properties:
>> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
>> > +- compatible:
>> > +   - "fsl,imx6q-pcie"
>> > +   - "fsl,imx6sx-pcie",
>> > +   - "fsl,imx6qp-pcie"
>> > +   - "fsl,imx7d-pcie"
>> >  - reg: base address and length of the PCIe controller
>> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
>> >    entry for each entry in the interrupt-names property.
>> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>> >  - clock names: Must include the following additional entries:
>> >     - "pcie_inbound_axi"
>> >
>> > +Additional required properties for imx7d-pcie:
>> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
>>
>> This domain is just the PHY? Seems like this needs a separate PHY
>> driver.
>>
> No, it's called the PHY power domain, as that is probably the part that
> draws the most power, but the PCIe core also looses it's state when this
> domain is powered down. So it's probably the complete core that is
> inside this domain.

A shared domain doesn't mean the phy and core should be 1 node. It is
the separate reset and clock for the PHY that tell me they should be
separate. And I'm pretty sure the DW block and PHY are separate. If
the PHY registers were part of the same register range, then I'd say
they should be one.

>> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
>> > +- reset-names: Must contain the following entires:
>> > +          - "pciephy"
>>
>> And for this too.
>>
>> > +          - "apps"
>> > +
>> >  Example:
>> >
>> >     pcie@0x01000000 {
>>
>> [...]
>>
>> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>> >     u32 val, gpr1, gpr12;
>> >
>> >     switch (imx6_pcie->variant) {
>> > +   case IMX7D:
>> > +           reset_control_assert(imx6_pcie->pciephy_reset);
>> > +           reset_control_assert(imx6_pcie->apps_reset);
>> > +           break;
>> >     case IMX6SX:
>> >             regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> >                                IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>>
>> So the difference with i.MX7D is not really that it has a reset or not,
>> but some platforms use a reset driver and some do not. The latter should
>> be fixed.
>
> The resets on anything before i.MX7 are not in a separate reset driver,
> but are just some signals from the PCIe core wired into a syscon (IOMUX
> GPR) area. While we could invent a reset controller for those, I don't
> see how this would improve things. Especially as the reset on i.MX6
> seems to be some side-effect of the "power-down" signal of the core, so
> not strictly a reset.
>
> Also I don't see why we should change the binding for the driver with a
> long history of deployed DTs. That seems like a total waste of manpower.

We can debate whether or not we change existing platforms. Maybe that
doesn't make sense now. But best practices should be considered when
adding new bindings rather than just extending existing bindings. We
didn't split out PHYs at one time and now we generally do. I'm not so
concerned with just adding i.MX7D, but really the next chip (and the
next).

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Thu, Feb 16, 2017 at 3:12 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
>> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
>> > Add various bits of code needed to support i.MX7D variant of the IP.
>> >
>> > Cc: yurovsky@gmail.com
>> > Cc: Lucas Stach <l.stach@pengutronix.de>
>> > Cc: Bjorn Helgaas <bhelgaas@google.com>
>> > Cc: Rob Herring <robh+dt@kernel.org>
>> > Cc: Mark Rutland <mark.rutland@arm.com>
>> > Cc: Lee Jones <lee.jones@linaro.org>
>> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> > Cc: linux-arm-kernel@lists.infradead.org
>> > Cc: devicetree@vger.kernel.org
>> > Cc: linux-kernel@vger.kernel.org
>> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> > ---
>> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>> >  3 files changed, 112 insertions(+), 26 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > index 83aeb1f..11db2ab 100644
>> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>> >  and thus inherits all the common properties defined in designware-pcie.txt.
>> >
>> >  Required properties:
>> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
>> > +- compatible:
>> > +   - "fsl,imx6q-pcie"
>> > +   - "fsl,imx6sx-pcie",
>> > +   - "fsl,imx6qp-pcie"
>> > +   - "fsl,imx7d-pcie"
>> >  - reg: base address and length of the PCIe controller
>> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
>> >    entry for each entry in the interrupt-names property.
>> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>> >  - clock names: Must include the following additional entries:
>> >     - "pcie_inbound_axi"
>> >
>> > +Additional required properties for imx7d-pcie:
>> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
>>
>> This domain is just the PHY? Seems like this needs a separate PHY
>> driver.
>>
> No, it's called the PHY power domain, as that is probably the part that
> draws the most power, but the PCIe core also looses it's state when this
> domain is powered down. So it's probably the complete core that is
> inside this domain.

A shared domain doesn't mean the phy and core should be 1 node. It is
the separate reset and clock for the PHY that tell me they should be
separate. And I'm pretty sure the DW block and PHY are separate. If
the PHY registers were part of the same register range, then I'd say
they should be one.

>> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
>> > +- reset-names: Must contain the following entires:
>> > +          - "pciephy"
>>
>> And for this too.
>>
>> > +          - "apps"
>> > +
>> >  Example:
>> >
>> >     pcie@0x01000000 {
>>
>> [...]
>>
>> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>> >     u32 val, gpr1, gpr12;
>> >
>> >     switch (imx6_pcie->variant) {
>> > +   case IMX7D:
>> > +           reset_control_assert(imx6_pcie->pciephy_reset);
>> > +           reset_control_assert(imx6_pcie->apps_reset);
>> > +           break;
>> >     case IMX6SX:
>> >             regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> >                                IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>>
>> So the difference with i.MX7D is not really that it has a reset or not,
>> but some platforms use a reset driver and some do not. The latter should
>> be fixed.
>
> The resets on anything before i.MX7 are not in a separate reset driver,
> but are just some signals from the PCIe core wired into a syscon (IOMUX
> GPR) area. While we could invent a reset controller for those, I don't
> see how this would improve things. Especially as the reset on i.MX6
> seems to be some side-effect of the "power-down" signal of the core, so
> not strictly a reset.
>
> Also I don't see why we should change the binding for the driver with a
> long history of deployed DTs. That seems like a total waste of manpower.

We can debate whether or not we change existing platforms. Maybe that
doesn't make sense now. But best practices should be considered when
adding new bindings rather than just extending existing bindings. We
didn't split out PHYs at one time and now we generally do. I'm not so
concerned with just adding i.MX7D, but really the next chip (and the
next).

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Rob Herring]

On Thu, Feb 16, 2017 at 3:12 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
>> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
>> > Add various bits of code needed to support i.MX7D variant of the IP.
>> >
>> > Cc: yurovsky at gmail.com
>> > Cc: Lucas Stach <l.stach@pengutronix.de>
>> > Cc: Bjorn Helgaas <bhelgaas@google.com>
>> > Cc: Rob Herring <robh+dt@kernel.org>
>> > Cc: Mark Rutland <mark.rutland@arm.com>
>> > Cc: Lee Jones <lee.jones@linaro.org>
>> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
>> > Cc: linux-arm-kernel at lists.infradead.org
>> > Cc: devicetree at vger.kernel.org
>> > Cc: linux-kernel at vger.kernel.org
>> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
>> > ---
>> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
>> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
>> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
>> >  3 files changed, 112 insertions(+), 26 deletions(-)
>> >
>> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > index 83aeb1f..11db2ab 100644
>> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
>> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
>> >  and thus inherits all the common properties defined in designware-pcie.txt.
>> >
>> >  Required properties:
>> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
>> > +- compatible:
>> > +   - "fsl,imx6q-pcie"
>> > +   - "fsl,imx6sx-pcie",
>> > +   - "fsl,imx6qp-pcie"
>> > +   - "fsl,imx7d-pcie"
>> >  - reg: base address and length of the PCIe controller
>> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
>> >    entry for each entry in the interrupt-names property.
>> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
>> >  - clock names: Must include the following additional entries:
>> >     - "pcie_inbound_axi"
>> >
>> > +Additional required properties for imx7d-pcie:
>> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
>>
>> This domain is just the PHY? Seems like this needs a separate PHY
>> driver.
>>
> No, it's called the PHY power domain, as that is probably the part that
> draws the most power, but the PCIe core also looses it's state when this
> domain is powered down. So it's probably the complete core that is
> inside this domain.

A shared domain doesn't mean the phy and core should be 1 node. It is
the separate reset and clock for the PHY that tell me they should be
separate. And I'm pretty sure the DW block and PHY are separate. If
the PHY registers were part of the same register range, then I'd say
they should be one.

>> > +- resets: Must contain phandles to PCIE related reset lines exposed by SRC IP block
>> > +- reset-names: Must contain the following entires:
>> > +          - "pciephy"
>>
>> And for this too.
>>
>> > +          - "apps"
>> > +
>> >  Example:
>> >
>> >     pcie at 0x01000000 {
>>
>> [...]
>>
>> > @@ -251,6 +261,10 @@ static void imx6_pcie_assert_core_reset(struct imx6_pcie *imx6_pcie)
>> >     u32 val, gpr1, gpr12;
>> >
>> >     switch (imx6_pcie->variant) {
>> > +   case IMX7D:
>> > +           reset_control_assert(imx6_pcie->pciephy_reset);
>> > +           reset_control_assert(imx6_pcie->apps_reset);
>> > +           break;
>> >     case IMX6SX:
>> >             regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
>> >                                IMX6SX_GPR12_PCIE_TEST_POWERDOWN,
>>
>> So the difference with i.MX7D is not really that it has a reset or not,
>> but some platforms use a reset driver and some do not. The latter should
>> be fixed.
>
> The resets on anything before i.MX7 are not in a separate reset driver,
> but are just some signals from the PCIe core wired into a syscon (IOMUX
> GPR) area. While we could invent a reset controller for those, I don't
> see how this would improve things. Especially as the reset on i.MX6
> seems to be some side-effect of the "power-down" signal of the core, so
> not strictly a reset.
>
> Also I don't see why we should change the binding for the driver with a
> long history of deployed DTs. That seems like a total waste of manpower.

We can debate whether or not we change existing platforms. Maybe that
doesn't make sense now. But best practices should be considered when
adding new bindings rather than just extending existing bindings. We
didn't split out PHYs at one time and now we generally do. I'm not so
concerned with just adding i.MX7D, but really the next chip (and the
next).

Rob

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Dienstag, den 21.02.2017, 10:38 -0600 schrieb Rob Herring:
> On Thu, Feb 16, 2017 at 3:12 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
> >> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> >> > Add various bits of code needed to support i.MX7D variant of the IP.
> >> >
> >> > Cc: yurovsky@gmail.com
> >> > Cc: Lucas Stach <l.stach@pengutronix.de>
> >> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> >> > Cc: Rob Herring <robh+dt@kernel.org>
> >> > Cc: Mark Rutland <mark.rutland@arm.com>
> >> > Cc: Lee Jones <lee.jones@linaro.org>
> >> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> >> > Cc: linux-arm-kernel@lists.infradead.org
> >> > Cc: devicetree@vger.kernel.org
> >> > Cc: linux-kernel@vger.kernel.org
> >> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >> > ---
> >> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >> >  3 files changed, 112 insertions(+), 26 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > index 83aeb1f..11db2ab 100644
> >> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >> >
> >> >  Required properties:
> >> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> >> > +- compatible:
> >> > +   - "fsl,imx6q-pcie"
> >> > +   - "fsl,imx6sx-pcie",
> >> > +   - "fsl,imx6qp-pcie"
> >> > +   - "fsl,imx7d-pcie"
> >> >  - reg: base address and length of the PCIe controller
> >> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >> >    entry for each entry in the interrupt-names property.
> >> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >> >  - clock names: Must include the following additional entries:
> >> >     - "pcie_inbound_axi"
> >> >
> >> > +Additional required properties for imx7d-pcie:
> >> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> >>
> >> This domain is just the PHY? Seems like this needs a separate PHY
> >> driver.
> >>
> > No, it's called the PHY power domain, as that is probably the part that
> > draws the most power, but the PCIe core also looses it's state when this
> > domain is powered down. So it's probably the complete core that is
> > inside this domain.
> 
> A shared domain doesn't mean the phy and core should be 1 node. It is
> the separate reset and clock for the PHY that tell me they should be
> separate. And I'm pretty sure the DW block and PHY are separate. If
> the PHY registers were part of the same register range, then I'd say
> they should be one.

Then we are on the same page of _not_ splitting out the PHY. :)
The DW PCIe PHY has no separate register range on i.MX. In fact the PHY
registers are only accessible through a indirection register in the PCIe
host controller register range.

Regards,
Lucas

Re: [PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Dienstag, den 21.02.2017, 10:38 -0600 schrieb Rob Herring:
> On Thu, Feb 16, 2017 at 3:12 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
> >> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> >> > Add various bits of code needed to support i.MX7D variant of the IP.
> >> >
> >> > Cc: yurovsky@gmail.com
> >> > Cc: Lucas Stach <l.stach@pengutronix.de>
> >> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> >> > Cc: Rob Herring <robh+dt@kernel.org>
> >> > Cc: Mark Rutland <mark.rutland@arm.com>
> >> > Cc: Lee Jones <lee.jones@linaro.org>
> >> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> >> > Cc: linux-arm-kernel@lists.infradead.org
> >> > Cc: devicetree@vger.kernel.org
> >> > Cc: linux-kernel@vger.kernel.org
> >> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >> > ---
> >> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >> >  3 files changed, 112 insertions(+), 26 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > index 83aeb1f..11db2ab 100644
> >> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >> >
> >> >  Required properties:
> >> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> >> > +- compatible:
> >> > +   - "fsl,imx6q-pcie"
> >> > +   - "fsl,imx6sx-pcie",
> >> > +   - "fsl,imx6qp-pcie"
> >> > +   - "fsl,imx7d-pcie"
> >> >  - reg: base address and length of the PCIe controller
> >> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >> >    entry for each entry in the interrupt-names property.
> >> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >> >  - clock names: Must include the following additional entries:
> >> >     - "pcie_inbound_axi"
> >> >
> >> > +Additional required properties for imx7d-pcie:
> >> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> >>
> >> This domain is just the PHY? Seems like this needs a separate PHY
> >> driver.
> >>
> > No, it's called the PHY power domain, as that is probably the part that
> > draws the most power, but the PCIe core also looses it's state when this
> > domain is powered down. So it's probably the complete core that is
> > inside this domain.
> 
> A shared domain doesn't mean the phy and core should be 1 node. It is
> the separate reset and clock for the PHY that tell me they should be
> separate. And I'm pretty sure the DW block and PHY are separate. If
> the PHY registers were part of the same register range, then I'd say
> they should be one.

Then we are on the same page of _not_ splitting out the PHY. :)
The DW PCIe PHY has no separate register range on i.MX. In fact the PHY
registers are only accessible through a indirection register in the PCIe
host controller register range.

Regards,
Lucas

[PATCH v4 3/3] PCI: imx6: Add code to support i.MX7D

[Lucas Stach]

Am Dienstag, den 21.02.2017, 10:38 -0600 schrieb Rob Herring:
> On Thu, Feb 16, 2017 at 3:12 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Mittwoch, den 15.02.2017, 11:17 -0600 schrieb Rob Herring:
> >> On Tue, Feb 07, 2017 at 07:50:27AM -0800, Andrey Smirnov wrote:
> >> > Add various bits of code needed to support i.MX7D variant of the IP.
> >> >
> >> > Cc: yurovsky at gmail.com
> >> > Cc: Lucas Stach <l.stach@pengutronix.de>
> >> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> >> > Cc: Rob Herring <robh+dt@kernel.org>
> >> > Cc: Mark Rutland <mark.rutland@arm.com>
> >> > Cc: Lee Jones <lee.jones@linaro.org>
> >> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> >> > Cc: linux-arm-kernel at lists.infradead.org
> >> > Cc: devicetree at vger.kernel.org
> >> > Cc: linux-kernel at vger.kernel.org
> >> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> >> > ---
> >> >  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  13 ++-
> >> >  drivers/pci/host/pci-imx6.c                        | 121 ++++++++++++++++-----
> >> >  include/linux/mfd/syscon/imx7-iomuxc-gpr.h         |   4 +
> >> >  3 files changed, 112 insertions(+), 26 deletions(-)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > index 83aeb1f..11db2ab 100644
> >> > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> >> > @@ -4,7 +4,11 @@ This PCIe host controller is based on the Synopsis Designware PCIe IP
> >> >  and thus inherits all the common properties defined in designware-pcie.txt.
> >> >
> >> >  Required properties:
> >> > -- compatible: "fsl,imx6q-pcie", "fsl,imx6sx-pcie", "fsl,imx6qp-pcie"
> >> > +- compatible:
> >> > +   - "fsl,imx6q-pcie"
> >> > +   - "fsl,imx6sx-pcie",
> >> > +   - "fsl,imx6qp-pcie"
> >> > +   - "fsl,imx7d-pcie"
> >> >  - reg: base address and length of the PCIe controller
> >> >  - interrupts: A list of interrupt outputs of the controller. Must contain an
> >> >    entry for each entry in the interrupt-names property.
> >> > @@ -34,6 +38,13 @@ Additional required properties for imx6sx-pcie:
> >> >  - clock names: Must include the following additional entries:
> >> >     - "pcie_inbound_axi"
> >> >
> >> > +Additional required properties for imx7d-pcie:
> >> > +- power-domains: Must be set to a phandle pointing to PCIE_PHY power domain
> >>
> >> This domain is just the PHY? Seems like this needs a separate PHY
> >> driver.
> >>
> > No, it's called the PHY power domain, as that is probably the part that
> > draws the most power, but the PCIe core also looses it's state when this
> > domain is powered down. So it's probably the complete core that is
> > inside this domain.
> 
> A shared domain doesn't mean the phy and core should be 1 node. It is
> the separate reset and clock for the PHY that tell me they should be
> separate. And I'm pretty sure the DW block and PHY are separate. If
> the PHY registers were part of the same register range, then I'd say
> they should be one.

Then we are on the same page of _not_ splitting out the PHY. :)
The DW PCIe PHY has no separate register range on i.MX. In fact the PHY
registers are only accessible through a indirection register in the PCIe
host controller register range.

Regards,
Lucas