[PATCH 0/2] Workaround for IMX7d PCI-e PLL lock failure

[PATCH 0/2] Workaround for IMX7d PCI-e PLL lock failure

[Trent Piepho]

This is the workaround for the IMX7d Erratum e10728, failure of
initialize PCIe PLL VCO oscillation resulting in PLL lock failure and
failure of the PCI-e link to come up.

The registers used in the workaround are based on the latest patch in
the NXP kernel, but many things around that have been changed.

This uses a new node of type fsl,imx-pcie-phy to get the PHY's
registers.  The node is found via a phandle added to the PCI-e
controller's node, rather than the incorrect way done in the NXP kernel.

There is no error if the phandle is not preset (since it's needed except
for the imx7d workaround and no existing dtses have it), but if preset
it is an error if something relating to it does not work.

** Should the node be fsl,imx7d-pcie-phy?  snps,dw-pcie-phy?  

There is little to no documenation from NXP and Synopsis about this, so I'm
unsure of the PHY's lineage.

The imx6 PCI-e driver does not use the generic phy layer to interact
with the PHY.  It appears PHY related hardware, like clocks, regulators,
and resets, are part of the fsl,imx6q-pcie node.  But again, the
topology of this hardware is not documented very well.

Another approach would be to add the PHY registers as another bank in
the PCI-e node.  This would match how the PHY reset, clock, etc.  are
done.  However, the PHY is attached to a different AXI master than the
PCI-e controller, so the register range really does not belong there.

Trent Piepho (2):
  ARM: dts: imx7d: Add node for PCIe PHY
  PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     | 11 ++++
 arch/arm/boot/dts/imx7d.dtsi                       |  9 ++++
 drivers/pci/dwc/pci-imx6.c                         | 59 ++++++++++++++++++++++
 3 files changed, 79 insertions(+)

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>

-- 
2.14.4



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

[PATCH 0/2] Workaround for IMX7d PCI-e PLL lock failure

[Trent Piepho]

This is the workaround for the IMX7d Erratum e10728, failure of
initialize PCIe PLL VCO oscillation resulting in PLL lock failure and
failure of the PCI-e link to come up.

The registers used in the workaround are based on the latest patch in
the NXP kernel, but many things around that have been changed.

This uses a new node of type fsl,imx-pcie-phy to get the PHY's
registers.  The node is found via a phandle added to the PCI-e
controller's node, rather than the incorrect way done in the NXP kernel.

There is no error if the phandle is not preset (since it's needed except
for the imx7d workaround and no existing dtses have it), but if preset
it is an error if something relating to it does not work.

** Should the node be fsl,imx7d-pcie-phy?  snps,dw-pcie-phy?  

There is little to no documenation from NXP and Synopsis about this, so I'm
unsure of the PHY's lineage.

The imx6 PCI-e driver does not use the generic phy layer to interact
with the PHY.  It appears PHY related hardware, like clocks, regulators,
and resets, are part of the fsl,imx6q-pcie node.  But again, the
topology of this hardware is not documented very well.

Another approach would be to add the PHY registers as another bank in
the PCI-e node.  This would match how the PHY reset, clock, etc.  are
done.  However, the PHY is attached to a different AXI master than the
PCI-e controller, so the register range really does not belong there.

Trent Piepho (2):
  ARM: dts: imx7d: Add node for PCIe PHY
  PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     | 11 ++++
 arch/arm/boot/dts/imx7d.dtsi                       |  9 ++++
 drivers/pci/dwc/pci-imx6.c                         | 59 ++++++++++++++++++++++
 3 files changed, 79 insertions(+)

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>

-- 
2.14.4

[PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Trent Piepho]

There isn't yet any code in the kernel that uses this device's register,
but there will be some for a PCIe PLL erratum wortkaround.

This adds the PHY as a new node.  The PCI-e controller node gains a
phandle property that points to it.  There is no driver for the PHY at
this point and all the existing code that relates to the PHY is part of
the PCI-e controller driver (and does not need register access, yet).

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 11 +++++++++++
 arch/arm/boot/dts/imx7d.dtsi                             |  9 +++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index cb33421184a0..c7aeda6878ff 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -50,6 +50,7 @@ Additional required properties for imx7d-pcie:
 - reset-names: Must contain the following entires:
 	       - "pciephy"
 	       - "apps"
+- fsl,pcie-phy: A phandle to an fsl,imx-pcie-phy node.
 
 Example:
 
@@ -76,3 +77,13 @@ Example:
 		clocks = <&clks 144>, <&clks 206>, <&clks 189>;
 		clock-names = "pcie", "pcie_bus", "pcie_phy";
 	};
+
+* Freescale i.MX7d PCIe PHY
+
+This is the PHY associated with the IMX7d PCIe controller.  It's used by the
+PCI-e controller via the fsl,pcie-phy phandle.
+
+Required properties:
+- compatible:
+	- "fsl,imx-pcie-phy"
+- reg: base address and length of the PCIe PHY controller
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 200714e3feea..31f5c8576251 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -94,6 +94,14 @@
 	};
 };
 
+&aips2 {
+	pcie_phy: pcie-phy@306d0000 {
+		  compatible = "fsl,imx-pcie-phy";
+		  reg = <0x306d0000 0x10000>;
+		  status = "disabled";
+	};
+};
+
 &aips3 {
 	usbotg2: usb@30b20000 {
 		compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
@@ -167,6 +175,7 @@
 			 <&src IMX7_RESET_PCIE_CTRL_APPS_EN>;
 		reset-names = "pciephy", "apps";
 		status = "disabled";
+		fsl,pcie-phy = <&pcie_phy>;
 	};
 };
 
-- 
2.14.4


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

[PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Trent Piepho]

There isn't yet any code in the kernel that uses this device's register,
but there will be some for a PCIe PLL erratum wortkaround.

This adds the PHY as a new node.  The PCI-e controller node gains a
phandle property that points to it.  There is no driver for the PHY at
this point and all the existing code that relates to the PHY is part of
the PCI-e controller driver (and does not need register access, yet).

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 11 +++++++++++
 arch/arm/boot/dts/imx7d.dtsi                             |  9 +++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
index cb33421184a0..c7aeda6878ff 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -50,6 +50,7 @@ Additional required properties for imx7d-pcie:
 - reset-names: Must contain the following entires:
 	       - "pciephy"
 	       - "apps"
+- fsl,pcie-phy: A phandle to an fsl,imx-pcie-phy node.
 
 Example:
 
@@ -76,3 +77,13 @@ Example:
 		clocks = <&clks 144>, <&clks 206>, <&clks 189>;
 		clock-names = "pcie", "pcie_bus", "pcie_phy";
 	};
+
+* Freescale i.MX7d PCIe PHY
+
+This is the PHY associated with the IMX7d PCIe controller.  It's used by the
+PCI-e controller via the fsl,pcie-phy phandle.
+
+Required properties:
+- compatible:
+	- "fsl,imx-pcie-phy"
+- reg: base address and length of the PCIe PHY controller
diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
index 200714e3feea..31f5c8576251 100644
--- a/arch/arm/boot/dts/imx7d.dtsi
+++ b/arch/arm/boot/dts/imx7d.dtsi
@@ -94,6 +94,14 @@
 	};
 };
 
+&aips2 {
+	pcie_phy: pcie-phy at 306d0000 {
+		  compatible = "fsl,imx-pcie-phy";
+		  reg = <0x306d0000 0x10000>;
+		  status = "disabled";
+	};
+};
+
 &aips3 {
 	usbotg2: usb at 30b20000 {
 		compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
@@ -167,6 +175,7 @@
 			 <&src IMX7_RESET_PCIE_CTRL_APPS_EN>;
 		reset-names = "pciephy", "apps";
 		status = "disabled";
+		fsl,pcie-phy = <&pcie_phy>;
 	};
 };
 
-- 
2.14.4

[PATCH 2/2] PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

[Trent Piepho]

This implements the workound described in the NXP IMX7d erratum e10728.

Initial VCO oscillation may fail under corner conditions such as cold
temperature.  It causes PCIe PLL to fail to lock in the initialization
phase, which results in the PCIe link failing to come up.

The workaround is to disable Duty-cycle Corrector (DCC) calibration
after G_RST.

To do this it is necessary to gain access to the undocumented and
currently unused PCIe PHY register bank.  A new device tree node of type
"fsl,imx-pcie-phy" is created for the PHY block and the existing PCIe
device uses a phandle named "fsl,pcie-phy" to point to it.

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
---
 drivers/pci/dwc/pci-imx6.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
index dcf389b62944..32429afc9380 100644
--- a/drivers/pci/dwc/pci-imx6.c
+++ b/drivers/pci/dwc/pci-imx6.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/of_gpio.h>
 #include <linux/of_device.h>
+#include <linux/of_address.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -58,6 +59,7 @@ struct imx6_pcie {
 	u32			tx_swing_low;
 	int			link_gen;
 	struct regulator	*vpcie;
+	void __iomem		*phy_base;
 };
 
 /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
@@ -98,6 +100,23 @@ struct imx6_pcie {
 #define PCIE_PHY_RX_ASIC_OUT 0x100D
 #define PCIE_PHY_RX_ASIC_OUT_VALID	(1 << 0)
 
+/* iMX7 PCIe PHY registers */
+#define PCIE_PHY_CMN_REG4		0x14
+/* These are probably the bits that *aren't* DCC_FB_EN */
+#define PCIE_PHY_CMN_REG4_DCC_FB_EN	0x29
+
+#define PCIE_PHY_CMN_REG15	        0x54
+#define PCIE_PHY_CMN_REG15_DLY_4	BIT(2)
+#define PCIE_PHY_CMN_REG15_PLL_PD	BIT(5)
+#define PCIE_PHY_CMN_REG15_OVRD_PLL_PD	BIT(7)
+
+#define PCIE_PHY_CMN_REG24		0x90
+#define PCIE_PHY_CMN_REG24_RX_EQ	BIT(6)
+#define PCIE_PHY_CMN_REG24_RX_EQ_SEL	BIT(3)
+
+#define PCIE_PHY_CMN_REG26		0x98
+#define PCIE_PHY_CMN_REG26_ATT_MODE	0xBC
+
 #define PHY_RX_OVRD_IN_LO 0x1005
 #define PHY_RX_OVRD_IN_LO_RX_DATA_EN (1 << 5)
 #define PHY_RX_OVRD_IN_LO_RX_PLL_EN (1 << 3)
@@ -431,6 +450,26 @@ 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);
+
+		/* Workaround for ERR010728, failure of PCI-e PLL VCO to
+		 * oscillate, especially when cold.  This turns off "Duty-cycle
+		 * Corrector" and other mysterious undocumented things.
+		 */
+		if (likely(imx6_pcie->phy_base)) {
+			/* De-assert DCC_FB_EN */
+			writel(PCIE_PHY_CMN_REG4_DCC_FB_EN,
+			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG4);
+			/* Assert RX_EQS and RX_EQS_SEL */
+			writel(PCIE_PHY_CMN_REG24_RX_EQ_SEL
+				| PCIE_PHY_CMN_REG24_RX_EQ,
+			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG24);
+			/* Assert ATT_MODE */
+			writel(PCIE_PHY_CMN_REG26_ATT_MODE,
+			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG26);
+		} else {
+			dev_err(dev, "PHY base address not set\n");
+		}
+
 		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
 		break;
 	case IMX6SX:
@@ -698,6 +737,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct dw_pcie *pci;
 	struct imx6_pcie *imx6_pcie;
+	struct device_node *np;
 	struct resource *dbi_base;
 	struct device_node *node = dev->of_node;
 	int ret;
@@ -717,6 +757,25 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	imx6_pcie->variant =
 		(enum imx6_pcie_variants)of_device_get_match_data(dev);
 
+	/* Find the PHY if one is defined, only imx7d uses it */
+	np = of_parse_phandle(node, "fsl,pcie-phy", 0);
+	if (np) {
+		struct resource res;
+
+		ret = of_address_to_resource(np, 0, &res);
+		if (ret) {
+			dev_err(dev, "Unable to map PCIe PHY\n");
+			return ret;
+		}
+		imx6_pcie->phy_base = devm_ioremap_resource(dev, &res);
+		if (IS_ERR(imx6_pcie->phy_base)) {
+			dev_err(dev, "Unable to map PCIe PHY\n");
+			return PTR_ERR(imx6_pcie->phy_base);
+		}
+	} else {
+		imx6_pcie->phy_base = NULL;
+	}
+
 	dbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
 	if (IS_ERR(pci->dbi_base))
-- 
2.14.4


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

[PATCH 2/2] PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

[Trent Piepho]

This implements the workound described in the NXP IMX7d erratum e10728.

Initial VCO oscillation may fail under corner conditions such as cold
temperature.  It causes PCIe PLL to fail to lock in the initialization
phase, which results in the PCIe link failing to come up.

The workaround is to disable Duty-cycle Corrector (DCC) calibration
after G_RST.

To do this it is necessary to gain access to the undocumented and
currently unused PCIe PHY register bank.  A new device tree node of type
"fsl,imx-pcie-phy" is created for the PHY block and the existing PCIe
device uses a phandle named "fsl,pcie-phy" to point to it.

Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Trent Piepho <tpiepho@impinj.com>
---
 drivers/pci/dwc/pci-imx6.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
index dcf389b62944..32429afc9380 100644
--- a/drivers/pci/dwc/pci-imx6.c
+++ b/drivers/pci/dwc/pci-imx6.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/of_gpio.h>
 #include <linux/of_device.h>
+#include <linux/of_address.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -58,6 +59,7 @@ struct imx6_pcie {
 	u32			tx_swing_low;
 	int			link_gen;
 	struct regulator	*vpcie;
+	void __iomem		*phy_base;
 };
 
 /* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
@@ -98,6 +100,23 @@ struct imx6_pcie {
 #define PCIE_PHY_RX_ASIC_OUT 0x100D
 #define PCIE_PHY_RX_ASIC_OUT_VALID	(1 << 0)
 
+/* iMX7 PCIe PHY registers */
+#define PCIE_PHY_CMN_REG4		0x14
+/* These are probably the bits that *aren't* DCC_FB_EN */
+#define PCIE_PHY_CMN_REG4_DCC_FB_EN	0x29
+
+#define PCIE_PHY_CMN_REG15	        0x54
+#define PCIE_PHY_CMN_REG15_DLY_4	BIT(2)
+#define PCIE_PHY_CMN_REG15_PLL_PD	BIT(5)
+#define PCIE_PHY_CMN_REG15_OVRD_PLL_PD	BIT(7)
+
+#define PCIE_PHY_CMN_REG24		0x90
+#define PCIE_PHY_CMN_REG24_RX_EQ	BIT(6)
+#define PCIE_PHY_CMN_REG24_RX_EQ_SEL	BIT(3)
+
+#define PCIE_PHY_CMN_REG26		0x98
+#define PCIE_PHY_CMN_REG26_ATT_MODE	0xBC
+
 #define PHY_RX_OVRD_IN_LO 0x1005
 #define PHY_RX_OVRD_IN_LO_RX_DATA_EN (1 << 5)
 #define PHY_RX_OVRD_IN_LO_RX_PLL_EN (1 << 3)
@@ -431,6 +450,26 @@ 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);
+
+		/* Workaround for ERR010728, failure of PCI-e PLL VCO to
+		 * oscillate, especially when cold.  This turns off "Duty-cycle
+		 * Corrector" and other mysterious undocumented things.
+		 */
+		if (likely(imx6_pcie->phy_base)) {
+			/* De-assert DCC_FB_EN */
+			writel(PCIE_PHY_CMN_REG4_DCC_FB_EN,
+			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG4);
+			/* Assert RX_EQS and RX_EQS_SEL */
+			writel(PCIE_PHY_CMN_REG24_RX_EQ_SEL
+				| PCIE_PHY_CMN_REG24_RX_EQ,
+			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG24);
+			/* Assert ATT_MODE */
+			writel(PCIE_PHY_CMN_REG26_ATT_MODE,
+			       imx6_pcie->phy_base + PCIE_PHY_CMN_REG26);
+		} else {
+			dev_err(dev, "PHY base address not set\n");
+		}
+
 		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
 		break;
 	case IMX6SX:
@@ -698,6 +737,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct dw_pcie *pci;
 	struct imx6_pcie *imx6_pcie;
+	struct device_node *np;
 	struct resource *dbi_base;
 	struct device_node *node = dev->of_node;
 	int ret;
@@ -717,6 +757,25 @@ static int imx6_pcie_probe(struct platform_device *pdev)
 	imx6_pcie->variant =
 		(enum imx6_pcie_variants)of_device_get_match_data(dev);
 
+	/* Find the PHY if one is defined, only imx7d uses it */
+	np = of_parse_phandle(node, "fsl,pcie-phy", 0);
+	if (np) {
+		struct resource res;
+
+		ret = of_address_to_resource(np, 0, &res);
+		if (ret) {
+			dev_err(dev, "Unable to map PCIe PHY\n");
+			return ret;
+		}
+		imx6_pcie->phy_base = devm_ioremap_resource(dev, &res);
+		if (IS_ERR(imx6_pcie->phy_base)) {
+			dev_err(dev, "Unable to map PCIe PHY\n");
+			return PTR_ERR(imx6_pcie->phy_base);
+		}
+	} else {
+		imx6_pcie->phy_base = NULL;
+	}
+
 	dbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
 	if (IS_ERR(pci->dbi_base))
-- 
2.14.4

Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Shawn Guo]

On Wed, Jul 18, 2018 at 12:44:23PM -0700, Trent Piepho wrote:
> There isn't yet any code in the kernel that uses this device's register,
> but there will be some for a PCIe PLL erratum wortkaround.
> 
> This adds the PHY as a new node.  The PCI-e controller node gains a
> phandle property that points to it.  There is no driver for the PHY at
> this point and all the existing code that relates to the PHY is part of
> the PCI-e controller driver (and does not need register access, yet).
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Trent Piepho <tpiepho@impinj.com>
> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 11 +++++++++++
>  arch/arm/boot/dts/imx7d.dtsi                             |  9 +++++++++
>  2 files changed, 20 insertions(+)

Please have separate patches for bindings and DTS.

Shawn

> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index cb33421184a0..c7aeda6878ff 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -50,6 +50,7 @@ Additional required properties for imx7d-pcie:
>  - reset-names: Must contain the following entires:
>  	       - "pciephy"
>  	       - "apps"
> +- fsl,pcie-phy: A phandle to an fsl,imx-pcie-phy node.
>  
>  Example:
>  
> @@ -76,3 +77,13 @@ Example:
>  		clocks = <&clks 144>, <&clks 206>, <&clks 189>;
>  		clock-names = "pcie", "pcie_bus", "pcie_phy";
>  	};
> +
> +* Freescale i.MX7d PCIe PHY
> +
> +This is the PHY associated with the IMX7d PCIe controller.  It's used by the
> +PCI-e controller via the fsl,pcie-phy phandle.
> +
> +Required properties:
> +- compatible:
> +	- "fsl,imx-pcie-phy"
> +- reg: base address and length of the PCIe PHY controller
> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> index 200714e3feea..31f5c8576251 100644
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -94,6 +94,14 @@
>  	};
>  };
>  
> +&aips2 {
> +	pcie_phy: pcie-phy@306d0000 {
> +		  compatible = "fsl,imx-pcie-phy";
> +		  reg = <0x306d0000 0x10000>;
> +		  status = "disabled";
> +	};
> +};
> +
>  &aips3 {
>  	usbotg2: usb@30b20000 {
>  		compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> @@ -167,6 +175,7 @@
>  			 <&src IMX7_RESET_PCIE_CTRL_APPS_EN>;
>  		reset-names = "pciephy", "apps";
>  		status = "disabled";
> +		fsl,pcie-phy = <&pcie_phy>;
>  	};
>  };
>  
> -- 
> 2.14.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Shawn Guo]

On Wed, Jul 18, 2018 at 12:44:23PM -0700, Trent Piepho wrote:
> There isn't yet any code in the kernel that uses this device's register,
> but there will be some for a PCIe PLL erratum wortkaround.
> 
> This adds the PHY as a new node.  The PCI-e controller node gains a
> phandle property that points to it.  There is no driver for the PHY at
> this point and all the existing code that relates to the PHY is part of
> the PCI-e controller driver (and does not need register access, yet).
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Trent Piepho <tpiepho@impinj.com>
> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 11 +++++++++++
>  arch/arm/boot/dts/imx7d.dtsi                             |  9 +++++++++
>  2 files changed, 20 insertions(+)

Please have separate patches for bindings and DTS.

Shawn

> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index cb33421184a0..c7aeda6878ff 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -50,6 +50,7 @@ Additional required properties for imx7d-pcie:
>  - reset-names: Must contain the following entires:
>  	       - "pciephy"
>  	       - "apps"
> +- fsl,pcie-phy: A phandle to an fsl,imx-pcie-phy node.
>  
>  Example:
>  
> @@ -76,3 +77,13 @@ Example:
>  		clocks = <&clks 144>, <&clks 206>, <&clks 189>;
>  		clock-names = "pcie", "pcie_bus", "pcie_phy";
>  	};
> +
> +* Freescale i.MX7d PCIe PHY
> +
> +This is the PHY associated with the IMX7d PCIe controller.  It's used by the
> +PCI-e controller via the fsl,pcie-phy phandle.
> +
> +Required properties:
> +- compatible:
> +	- "fsl,imx-pcie-phy"
> +- reg: base address and length of the PCIe PHY controller
> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> index 200714e3feea..31f5c8576251 100644
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -94,6 +94,14 @@
>  	};
>  };
>  
> +&aips2 {
> +	pcie_phy: pcie-phy at 306d0000 {
> +		  compatible = "fsl,imx-pcie-phy";
> +		  reg = <0x306d0000 0x10000>;
> +		  status = "disabled";
> +	};
> +};
> +
>  &aips3 {
>  	usbotg2: usb at 30b20000 {
>  		compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> @@ -167,6 +175,7 @@
>  			 <&src IMX7_RESET_PCIE_CTRL_APPS_EN>;
>  		reset-names = "pciephy", "apps";
>  		status = "disabled";
> +		fsl,pcie-phy = <&pcie_phy>;
>  	};
>  };
>  
> -- 
> 2.14.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Lucas Stach]

QW0gTWl0dHdvY2gsIGRlbiAxOC4wNy4yMDE4LCAxMjo0NCAtMDcwMCBzY2hyaWViIFRyZW50IFBp
ZXBobzoKPiBUaGVyZSBpc24ndCB5ZXQgYW55IGNvZGUgaW4gdGhlIGtlcm5lbCB0aGF0IHVzZXMg
dGhpcyBkZXZpY2UncyByZWdpc3RlciwKPiBidXQgdGhlcmUgd2lsbCBiZSBzb21lIGZvciBhIFBD
SWUgUExMIGVycmF0dW0gd29ydGthcm91bmQuCj4gCj4gVGhpcyBhZGRzIHRoZSBQSFkgYXMgYSBu
ZXcgbm9kZS7CoMKgVGhlIFBDSS1lIGNvbnRyb2xsZXIgbm9kZSBnYWlucyBhCj4gcGhhbmRsZSBw
cm9wZXJ0eSB0aGF0IHBvaW50cyB0byBpdC7CoMKgVGhlcmUgaXMgbm8gZHJpdmVyIGZvciB0aGUg
UEhZIGF0Cj4gdGhpcyBwb2ludCBhbmQgYWxsIHRoZSBleGlzdGluZyBjb2RlIHRoYXQgcmVsYXRl
cyB0byB0aGUgUEhZIGlzIHBhcnQgb2YKPiB0aGUgUENJLWUgY29udHJvbGxlciBkcml2ZXIgKGFu
ZCBkb2VzIG5vdCBuZWVkIHJlZ2lzdGVyIGFjY2VzcywgeWV0KS4KPiAKPiA+IENjOiBTaGF3biBH
dW8gPHNoYXduZ3VvQGtlcm5lbC5vcmc+Cj4gPiBDYzogU2FzY2hhIEhhdWVyIDxrZXJuZWxAcGVu
Z3V0cm9uaXguZGU+Cj4gPiBDYzogRmFiaW8gRXN0ZXZhbSA8ZmFiaW8uZXN0ZXZhbUBueHAuY29t
Pgo+ID4gQ2M6IFJpY2hhcmQgWmh1IDxob25neGluZy56aHVAbnhwLmNvbT4KPiA+IENjOiBMdWNh
cyBTdGFjaCA8bC5zdGFjaEBwZW5ndXRyb25peC5kZT4KPiA+IFNpZ25lZC1vZmYtYnk6IFRyZW50
IFBpZXBobyA8dHBpZXBob0BpbXBpbmouY29tPgo+IC0tLQo+IMKgRG9jdW1lbnRhdGlvbi9kZXZp
Y2V0cmVlL2JpbmRpbmdzL3BjaS9mc2wsaW14NnEtcGNpZS50eHQgfCAxMSArKysrKysrKysrKwo+
IMKgYXJjaC9hcm0vYm9vdC9kdHMvaW14N2QuZHRzacKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB8wqDCoDkgKysrKysrKysrCj4gwqAyIGZp
bGVzIGNoYW5nZWQsIDIwIGluc2VydGlvbnMoKykKPiAKPiBkaWZmIC0tZ2l0IGEvRG9jdW1lbnRh
dGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3BjaS9mc2wsaW14NnEtcGNpZS50eHQgYi9Eb2N1bWVu
dGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvcGNpL2ZzbCxpbXg2cS1wY2llLnR4dAo+IGluZGV4
IGNiMzM0MjExODRhMC4uYzdhZWRhNjg3OGZmIDEwMDY0NAo+IC0tLSBhL0RvY3VtZW50YXRpb24v
ZGV2aWNldHJlZS9iaW5kaW5ncy9wY2kvZnNsLGlteDZxLXBjaWUudHh0Cj4gKysrIGIvRG9jdW1l
bnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3BjaS9mc2wsaW14NnEtcGNpZS50eHQKPiBAQCAt
NTAsNiArNTAsNyBAQCBBZGRpdGlvbmFsIHJlcXVpcmVkIHByb3BlcnRpZXMgZm9yIGlteDdkLXBj
aWU6Cj4gwqAtIHJlc2V0LW5hbWVzOiBNdXN0IGNvbnRhaW4gdGhlIGZvbGxvd2luZyBlbnRpcmVz
Ogo+ID4gwqAJwqDCoMKgwqDCoMKgwqAtICJwY2llcGh5Igo+ID4gwqAJwqDCoMKgwqDCoMKgwqAt
ICJhcHBzIgo+ICstIGZzbCxwY2llLXBoeTogQSBwaGFuZGxlIHRvIGFuIGZzbCxpbXgtcGNpZS1w
aHkgbm9kZS4KPiDCoAo+IMKgRXhhbXBsZToKPiDCoAo+IEBAIC03NiwzICs3NywxMyBAQCBFeGFt
cGxlOgo+ID4gwqAJCWNsb2NrcyA9IDwmY2xrcyAxNDQ+LCA8JmNsa3MgMjA2PiwgPCZjbGtzIDE4
OT47Cj4gPiDCoAkJY2xvY2stbmFtZXMgPSAicGNpZSIsICJwY2llX2J1cyIsICJwY2llX3BoeSI7
Cj4gPiDCoAl9Owo+ICsKPiArKiBGcmVlc2NhbGUgaS5NWDdkIFBDSWUgUEhZCj4gKwo+ICtUaGlz
IGlzIHRoZSBQSFkgYXNzb2NpYXRlZCB3aXRoIHRoZSBJTVg3ZCBQQ0llIGNvbnRyb2xsZXIuwqDC
oEl0J3MgdXNlZCBieSB0aGUKPiArUENJLWUgY29udHJvbGxlciB2aWEgdGhlIGZzbCxwY2llLXBo
eSBwaGFuZGxlLgo+ICsKPiArUmVxdWlyZWQgcHJvcGVydGllczoKPiArLSBjb21wYXRpYmxlOgo+
ICsJLSAiZnNsLGlteC1wY2llLXBoeSIKClRoaXMgaXMgdG9vIGdlbmVyaWMuIFBsZWFzZSBjaGFu
Z2UgaXQgdG8gImZzbCxpbXg3LXBjaWUtcGh5Ii4KCj4gKy0gcmVnOiBiYXNlIGFkZHJlc3MgYW5k
IGxlbmd0aCBvZiB0aGUgUENJZSBQSFkgY29udHJvbGxlcgo+IGRpZmYgLS1naXQgYS9hcmNoL2Fy
bS9ib290L2R0cy9pbXg3ZC5kdHNpIGIvYXJjaC9hcm0vYm9vdC9kdHMvaW14N2QuZHRzaQo+IGlu
ZGV4IDIwMDcxNGUzZmVlYS4uMzFmNWM4NTc2MjUxIDEwMDY0NAo+IC0tLSBhL2FyY2gvYXJtL2Jv
b3QvZHRzL2lteDdkLmR0c2kKPiArKysgYi9hcmNoL2FybS9ib290L2R0cy9pbXg3ZC5kdHNpCj4g
QEAgLTk0LDYgKzk0LDE0IEBACj4gPiDCoAl9Owo+IMKgfTsKPiDCoAo+ICsmYWlwczIgewo+ID4g
PiArCXBjaWVfcGh5OiBwY2llLXBoeUAzMDZkMDAwMCB7Cj4gPiArCQnCoMKgY29tcGF0aWJsZSA9
ICJmc2wsaW14LXBjaWUtcGh5IjsKPiA+ICsJCcKgwqByZWcgPSA8MHgzMDZkMDAwMCAweDEwMDAw
PjsKPiA+ICsJCcKgwqBzdGF0dXMgPSAiZGlzYWJsZWQiOwo+ID4gKwl9Owo+ICt9Owo+ICsKPiDC
oCZhaXBzMyB7Cj4gPiA+IMKgCXVzYm90ZzI6IHVzYkAzMGIyMDAwMCB7Cj4gPiDCoAkJY29tcGF0
aWJsZSA9ICJmc2wsaW14N2QtdXNiIiwgImZzbCxpbXgyNy11c2IiOwo+IEBAIC0xNjcsNiArMTc1
LDcgQEAKPiA+IMKgCQkJwqA8JnNyYyBJTVg3X1JFU0VUX1BDSUVfQ1RSTF9BUFBTX0VOPjsKPiA+
IMKgCQlyZXNldC1uYW1lcyA9ICJwY2llcGh5IiwgImFwcHMiOwo+ID4gwqAJCXN0YXR1cyA9ICJk
aXNhYmxlZCI7Cj4gPiArCQlmc2wscGNpZS1waHkgPSA8JnBjaWVfcGh5PjsKPiA+IMKgCX07Cj4g
wqB9Owo+IMKgCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
XwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxpc3RzLmlu
ZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5mby9s
aW51eC1hcm0ta2VybmVsCg==

[PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Lucas Stach]

Am Mittwoch, den 18.07.2018, 12:44 -0700 schrieb Trent Piepho:
> There isn't yet any code in the kernel that uses this device's register,
> but there will be some for a PCIe PLL erratum wortkaround.
> 
> This adds the PHY as a new node.??The PCI-e controller node gains a
> phandle property that points to it.??There is no driver for the PHY at
> this point and all the existing code that relates to the PHY is part of
> the PCI-e controller driver (and does not need register access, yet).
> 
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: Richard Zhu <hongxing.zhu@nxp.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Signed-off-by: Trent Piepho <tpiepho@impinj.com>
> ---
> ?Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 11 +++++++++++
> ?arch/arm/boot/dts/imx7d.dtsi?????????????????????????????|??9 +++++++++
> ?2 files changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> index cb33421184a0..c7aeda6878ff 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
> @@ -50,6 +50,7 @@ Additional required properties for imx7d-pcie:
> ?- reset-names: Must contain the following entires:
> > ?	???????- "pciephy"
> > ?	???????- "apps"
> +- fsl,pcie-phy: A phandle to an fsl,imx-pcie-phy node.
> ?
> ?Example:
> ?
> @@ -76,3 +77,13 @@ Example:
> > ?		clocks = <&clks 144>, <&clks 206>, <&clks 189>;
> > ?		clock-names = "pcie", "pcie_bus", "pcie_phy";
> > ?	};
> +
> +* Freescale i.MX7d PCIe PHY
> +
> +This is the PHY associated with the IMX7d PCIe controller.??It's used by the
> +PCI-e controller via the fsl,pcie-phy phandle.
> +
> +Required properties:
> +- compatible:
> +	- "fsl,imx-pcie-phy"

This is too generic. Please change it to "fsl,imx7-pcie-phy".

> +- reg: base address and length of the PCIe PHY controller
> diff --git a/arch/arm/boot/dts/imx7d.dtsi b/arch/arm/boot/dts/imx7d.dtsi
> index 200714e3feea..31f5c8576251 100644
> --- a/arch/arm/boot/dts/imx7d.dtsi
> +++ b/arch/arm/boot/dts/imx7d.dtsi
> @@ -94,6 +94,14 @@
> > ?	};
> ?};
> ?
> +&aips2 {
> > > +	pcie_phy: pcie-phy at 306d0000 {
> > +		??compatible = "fsl,imx-pcie-phy";
> > +		??reg = <0x306d0000 0x10000>;
> > +		??status = "disabled";
> > +	};
> +};
> +
> ?&aips3 {
> > > ?	usbotg2: usb at 30b20000 {
> > ?		compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
> @@ -167,6 +175,7 @@
> > ?			?<&src IMX7_RESET_PCIE_CTRL_APPS_EN>;
> > ?		reset-names = "pciephy", "apps";
> > ?		status = "disabled";
> > +		fsl,pcie-phy = <&pcie_phy>;
> > ?	};
> ?};
> ?

Re: [PATCH 2/2] PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

[Lucas Stach]

QW0gTWl0dHdvY2gsIGRlbiAxOC4wNy4yMDE4LCAxMjo0NCAtMDcwMCBzY2hyaWViIFRyZW50IFBp
ZXBobzoKPiBUaGlzIGltcGxlbWVudHMgdGhlIHdvcmtvdW5kIGRlc2NyaWJlZCBpbiB0aGUgTlhQ
IElNWDdkIGVycmF0dW0gZTEwNzI4Lgo+IAo+IEluaXRpYWwgVkNPIG9zY2lsbGF0aW9uIG1heSBm
YWlsIHVuZGVyIGNvcm5lciBjb25kaXRpb25zIHN1Y2ggYXMgY29sZAo+IHRlbXBlcmF0dXJlLsKg
wqBJdCBjYXVzZXMgUENJZSBQTEwgdG8gZmFpbCB0byBsb2NrIGluIHRoZSBpbml0aWFsaXphdGlv
bgo+IHBoYXNlLCB3aGljaCByZXN1bHRzIGluIHRoZSBQQ0llIGxpbmsgZmFpbGluZyB0byBjb21l
IHVwLgo+IAo+IFRoZSB3b3JrYXJvdW5kIGlzIHRvIGRpc2FibGUgRHV0eS1jeWNsZSBDb3JyZWN0
b3IgKERDQykgY2FsaWJyYXRpb24KPiBhZnRlciBHX1JTVC4KPiAKPiBUbyBkbyB0aGlzIGl0IGlz
IG5lY2Vzc2FyeSB0byBnYWluIGFjY2VzcyB0byB0aGUgdW5kb2N1bWVudGVkIGFuZAo+IGN1cnJl
bnRseSB1bnVzZWQgUENJZSBQSFkgcmVnaXN0ZXIgYmFuay7CoMKgQSBuZXcgZGV2aWNlIHRyZWUg
bm9kZSBvZiB0eXBlCj4gImZzbCxpbXgtcGNpZS1waHkiIGlzIGNyZWF0ZWQgZm9yIHRoZSBQSFkg
YmxvY2sgYW5kIHRoZSBleGlzdGluZyBQQ0llCj4gZGV2aWNlIHVzZXMgYSBwaGFuZGxlIG5hbWVk
ICJmc2wscGNpZS1waHkiIHRvIHBvaW50IHRvIGl0Lgo+IAo+ID4gQ2M6IFNoYXduIEd1byA8c2hh
d25ndW9Aa2VybmVsLm9yZz4KPiA+IENjOiBTYXNjaGEgSGF1ZXIgPGtlcm5lbEBwZW5ndXRyb25p
eC5kZT4KPiA+IENjOiBGYWJpbyBFc3RldmFtIDxmYWJpby5lc3RldmFtQG54cC5jb20+Cj4gPiBD
YzogUmljaGFyZCBaaHUgPGhvbmd4aW5nLnpodUBueHAuY29tPgo+ID4gQ2M6IEx1Y2FzIFN0YWNo
IDxsLnN0YWNoQHBlbmd1dHJvbml4LmRlPgo+ID4gU2lnbmVkLW9mZi1ieTogVHJlbnQgUGllcGhv
IDx0cGllcGhvQGltcGluai5jb20+Cj4gLS0tCj4gwqBkcml2ZXJzL3BjaS9kd2MvcGNpLWlteDYu
YyB8IDU5ICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysKPiDC
oDEgZmlsZSBjaGFuZ2VkLCA1OSBpbnNlcnRpb25zKCspCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZl
cnMvcGNpL2R3Yy9wY2ktaW14Ni5jIGIvZHJpdmVycy9wY2kvZHdjL3BjaS1pbXg2LmMKPiBpbmRl
eCBkY2YzODliNjI5NDQuLjMyNDI5YWZjOTM4MCAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL3BjaS9k
d2MvcGNpLWlteDYuYwo+ICsrKyBiL2RyaXZlcnMvcGNpL2R3Yy9wY2ktaW14Ni5jCj4gQEAgLTE4
LDYgKzE4LDcgQEAKPiDCoCNpbmNsdWRlIDxsaW51eC9tb2R1bGUuaD4KPiDCoCNpbmNsdWRlIDxs
aW51eC9vZl9ncGlvLmg+Cj4gwqAjaW5jbHVkZSA8bGludXgvb2ZfZGV2aWNlLmg+Cj4gKyNpbmNs
dWRlIDxsaW51eC9vZl9hZGRyZXNzLmg+Cj4gwqAjaW5jbHVkZSA8bGludXgvcGNpLmg+Cj4gwqAj
aW5jbHVkZSA8bGludXgvcGxhdGZvcm1fZGV2aWNlLmg+Cj4gwqAjaW5jbHVkZSA8bGludXgvcmVn
bWFwLmg+Cj4gQEAgLTU4LDYgKzU5LDcgQEAgc3RydWN0IGlteDZfcGNpZSB7Cj4gPiA+IMKgCXUz
MgkJCXR4X3N3aW5nX2xvdzsKPiA+ID4gwqAJaW50CQkJbGlua19nZW47Cj4gPiA+IMKgCXN0cnVj
dCByZWd1bGF0b3IJKnZwY2llOwo+ID4gPiArCXZvaWQgX19pb21lbQkJKnBoeV9iYXNlOwo+IMKg
fTsKPiDCoAo+IMKgLyogUGFyYW1ldGVycyBmb3IgdGhlIHdhaXRpbmcgZm9yIFBDSWUgUEhZIFBM
TCB0byBsb2NrIG9uIGkuTVg3ICovCj4gQEAgLTk4LDYgKzEwMCwyMyBAQCBzdHJ1Y3QgaW14Nl9w
Y2llIHsKPiDCoCNkZWZpbmUgUENJRV9QSFlfUlhfQVNJQ19PVVQgMHgxMDBECj4gPiDCoCNkZWZp
bmUgUENJRV9QSFlfUlhfQVNJQ19PVVRfVkFMSUQJKDEgPDwgMCkKPiDCoAo+ICsvKiBpTVg3IFBD
SWUgUEhZIHJlZ2lzdGVycyAqLwo+ID4gKyNkZWZpbmUgUENJRV9QSFlfQ01OX1JFRzQJCTB4MTQK
PiArLyogVGhlc2UgYXJlIHByb2JhYmx5IHRoZSBiaXRzIHRoYXQgKmFyZW4ndCogRENDX0ZCX0VO
ICovCj4gPiArI2RlZmluZSBQQ0lFX1BIWV9DTU5fUkVHNF9EQ0NfRkJfRU4JMHgyOQo+ICsKPiA+
ICsjZGVmaW5lIFBDSUVfUEhZX0NNTl9SRUcxNQnCoMKgwqDCoMKgwqDCoMKgMHg1NAo+ID4gKyNk
ZWZpbmUgUENJRV9QSFlfQ01OX1JFRzE1X0RMWV80CUJJVCgyKQo+ID4gKyNkZWZpbmUgUENJRV9Q
SFlfQ01OX1JFRzE1X1BMTF9QRAlCSVQoNSkKPiA+ICsjZGVmaW5lIFBDSUVfUEhZX0NNTl9SRUcx
NV9PVlJEX1BMTF9QRAlCSVQoNykKPiArCj4gPiArI2RlZmluZSBQQ0lFX1BIWV9DTU5fUkVHMjQJ
CTB4OTAKPiA+ICsjZGVmaW5lIFBDSUVfUEhZX0NNTl9SRUcyNF9SWF9FUQlCSVQoNikKPiA+ICsj
ZGVmaW5lIFBDSUVfUEhZX0NNTl9SRUcyNF9SWF9FUV9TRUwJQklUKDMpCj4gKwo+ID4gKyNkZWZp
bmUgUENJRV9QSFlfQ01OX1JFRzI2CQkweDk4Cj4gPiArI2RlZmluZSBQQ0lFX1BIWV9DTU5fUkVH
MjZfQVRUX01PREUJMHhCQwo+ICsKPiDCoCNkZWZpbmUgUEhZX1JYX09WUkRfSU5fTE8gMHgxMDA1
Cj4gwqAjZGVmaW5lIFBIWV9SWF9PVlJEX0lOX0xPX1JYX0RBVEFfRU4gKDEgPDwgNSkKPiDCoCNk
ZWZpbmUgUEhZX1JYX09WUkRfSU5fTE9fUlhfUExMX0VOICgxIDw8IDMpCj4gQEAgLTQzMSw2ICs0
NTAsMjYgQEAgc3RhdGljIHZvaWQgaW14Nl9wY2llX2RlYXNzZXJ0X2NvcmVfcmVzZXQoc3RydWN0
IGlteDZfcGNpZSAqaW14Nl9wY2llKQo+ID4gwqAJc3dpdGNoIChpbXg2X3BjaWUtPnZhcmlhbnQp
IHsKPiA+IMKgCWNhc2UgSU1YN0Q6Cj4gPiDCoAkJcmVzZXRfY29udHJvbF9kZWFzc2VydChpbXg2
X3BjaWUtPnBjaWVwaHlfcmVzZXQpOwo+ICsKPiA+ICsJCS8qIFdvcmthcm91bmQgZm9yIEVSUjAx
MDcyOCwgZmFpbHVyZSBvZiBQQ0ktZSBQTEwgVkNPIHRvCj4gPiArCQnCoCogb3NjaWxsYXRlLCBl
c3BlY2lhbGx5IHdoZW4gY29sZC7CoMKgVGhpcyB0dXJucyBvZmYgIkR1dHktY3ljbGUKPiA+ICsJ
CcKgKiBDb3JyZWN0b3IiIGFuZCBvdGhlciBteXN0ZXJpb3VzIHVuZG9jdW1lbnRlZCB0aGluZ3Mu
Cj4gPiArCQnCoCovCj4gPiArCQlpZiAobGlrZWx5KGlteDZfcGNpZS0+cGh5X2Jhc2UpKSB7Cj4g
PiArCQkJLyogRGUtYXNzZXJ0IERDQ19GQl9FTiAqLwo+ID4gKwkJCXdyaXRlbChQQ0lFX1BIWV9D
TU5fUkVHNF9EQ0NfRkJfRU4sCj4gPiArCQkJwqDCoMKgwqDCoMKgwqBpbXg2X3BjaWUtPnBoeV9i
YXNlICsgUENJRV9QSFlfQ01OX1JFRzQpOwo+ID4gKwkJCS8qIEFzc2VydCBSWF9FUVMgYW5kIFJY
X0VRU19TRUwgKi8KPiA+ICsJCQl3cml0ZWwoUENJRV9QSFlfQ01OX1JFRzI0X1JYX0VRX1NFTAo+
ID4gKwkJCQl8IFBDSUVfUEhZX0NNTl9SRUcyNF9SWF9FUSwKPiA+ICsJCQnCoMKgwqDCoMKgwqDC
oGlteDZfcGNpZS0+cGh5X2Jhc2UgKyBQQ0lFX1BIWV9DTU5fUkVHMjQpOwo+ID4gKwkJCS8qIEFz
c2VydCBBVFRfTU9ERSAqLwo+ID4gKwkJCXdyaXRlbChQQ0lFX1BIWV9DTU5fUkVHMjZfQVRUX01P
REUsCj4gPiArCQkJwqDCoMKgwqDCoMKgwqBpbXg2X3BjaWUtPnBoeV9iYXNlICsgUENJRV9QSFlf
Q01OX1JFRzI2KTsKPiA+ICsJCX0gZWxzZSB7Cj4gKwkJCWRldl9lcnIoZGV2LCAiUEhZIGJhc2Ug
YWRkcmVzcyBub3Qgc2V0XG4iKTsKClBsZWFzZSB0dW5lIHRoaXMgZG93biB0byB0aGUgZGV2X3dh
cm4gbGV2ZWwsIGluIGNhc2Ugd2UgYXJlIHJ1bm5pbmcgYQpuZXcga2VybmVsIG9uIGEgb2xkZXIg
RFQuIEFsc28gdGhlIG1lc3NhZ2UgaXNuJ3QgcmVhbGx5IGhlbHBmdWwgdG8gdGhlCnVzZXIuIEl0
IHdvdWxkIGJlIGJldHRlciB0byBoYXZlIHNvbWV0aGluZyBsaWtlICJObyBhY2Nlc3MgdG8gUEhZ
CnJlZ2lzdGVycywgY2FuJ3QgYXBwbHkgd29ya2Fyb3VuZCBmb3IgRVJSMDEwNzI4LiIgTWF5YmUg
ZXZlbiB3aXRoIGEKaGludCB0byB1cGRhdGUgdGhlIERULgoKPiArCQl9Cj4gKwo+ID4gwqAJCWlt
eDdkX3BjaWVfd2FpdF9mb3JfcGh5X3BsbF9sb2NrKGlteDZfcGNpZSk7Cj4gPiDCoAkJYnJlYWs7
Cj4gPiDCoAljYXNlIElNWDZTWDoKPiBAQCAtNjk4LDYgKzczNyw3IEBAIHN0YXRpYyBpbnQgaW14
Nl9wY2llX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpCj4gPiDCoAlzdHJ1Y3Qg
ZGV2aWNlICpkZXYgPSAmcGRldi0+ZGV2Owo+ID4gwqAJc3RydWN0IGR3X3BjaWUgKnBjaTsKPiA+
IMKgCXN0cnVjdCBpbXg2X3BjaWUgKmlteDZfcGNpZTsKPiA+ICsJc3RydWN0IGRldmljZV9ub2Rl
ICpucDsKPiA+IMKgCXN0cnVjdCByZXNvdXJjZSAqZGJpX2Jhc2U7Cj4gPiDCoAlzdHJ1Y3QgZGV2
aWNlX25vZGUgKm5vZGUgPSBkZXYtPm9mX25vZGU7Cj4gPiDCoAlpbnQgcmV0Owo+IEBAIC03MTcs
NiArNzU3LDI1IEBAIHN0YXRpYyBpbnQgaW14Nl9wY2llX3Byb2JlKHN0cnVjdCBwbGF0Zm9ybV9k
ZXZpY2UgKnBkZXYpCj4gPiDCoAlpbXg2X3BjaWUtPnZhcmlhbnQgPQo+ID4gwqAJCShlbnVtIGlt
eDZfcGNpZV92YXJpYW50cylvZl9kZXZpY2VfZ2V0X21hdGNoX2RhdGEoZGV2KTsKPiDCoAo+ID4g
KwkvKiBGaW5kIHRoZSBQSFkgaWYgb25lIGlzIGRlZmluZWQsIG9ubHkgaW14N2QgdXNlcyBpdCAq
Lwo+ID4gKwlucCA9IG9mX3BhcnNlX3BoYW5kbGUobm9kZSwgImZzbCxwY2llLXBoeSIsIDApOwo+
ID4gKwlpZiAobnApIHsKPiA+ICsJCXN0cnVjdCByZXNvdXJjZSByZXM7Cj4gKwo+ID4gKwkJcmV0
ID0gb2ZfYWRkcmVzc190b19yZXNvdXJjZShucCwgMCwgJnJlcyk7Cj4gPiArCQlpZiAocmV0KSB7
Cj4gPiArCQkJZGV2X2VycihkZXYsICJVbmFibGUgdG8gbWFwIFBDSWUgUEhZXG4iKTsKPiA+ICsJ
CQlyZXR1cm4gcmV0Owo+ID4gKwkJfQo+ID4gKwkJaW14Nl9wY2llLT5waHlfYmFzZSA9IGRldm1f
aW9yZW1hcF9yZXNvdXJjZShkZXYsICZyZXMpOwo+ID4gKwkJaWYgKElTX0VSUihpbXg2X3BjaWUt
PnBoeV9iYXNlKSkgewo+ID4gKwkJCWRldl9lcnIoZGV2LCAiVW5hYmxlIHRvIG1hcCBQQ0llIFBI
WVxuIik7Cj4gPiArCQkJcmV0dXJuIFBUUl9FUlIoaW14Nl9wY2llLT5waHlfYmFzZSk7Cj4gPiAr
CQl9Cj4gPiArCX0gZWxzZSB7Cj4gPiArCQlpbXg2X3BjaWUtPnBoeV9iYXNlID0gTlVMTDsKPiAr
CX0KCmlteDZfcGNpZSBpcyB6ZXJvIGluaXRpYWxpemVkIG9uIGFsbG9jYXRpb24sIHNvIG5vIG5l
ZWQgZm9yIHRoZSBlbHNlCnBhdGggc2V0dGluZyB0aGlzIHRvIE5VTEwuCgpSZWdhcmRzLApMdWNh
cwoKPiArCj4gPiDCoAlkYmlfYmFzZSA9IHBsYXRmb3JtX2dldF9yZXNvdXJjZShwZGV2LCBJT1JF
U09VUkNFX01FTSwgMCk7Cj4gPiDCoAlwY2ktPmRiaV9iYXNlID0gZGV2bV9pb3JlbWFwX3Jlc291
cmNlKGRldiwgZGJpX2Jhc2UpOwo+ID4gwqAJaWYgKElTX0VSUihwY2ktPmRiaV9iYXNlKSkKCl9f
X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmxpbnV4LWFybS1r
ZXJuZWwgbWFpbGluZyBsaXN0CmxpbnV4LWFybS1rZXJuZWxAbGlzdHMuaW5mcmFkZWFkLm9yZwpo
dHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LWFybS1rZXJu
ZWwK

[PATCH 2/2] PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

[Lucas Stach]

Am Mittwoch, den 18.07.2018, 12:44 -0700 schrieb Trent Piepho:
> This implements the workound described in the NXP IMX7d erratum e10728.
> 
> Initial VCO oscillation may fail under corner conditions such as cold
> temperature.??It causes PCIe PLL to fail to lock in the initialization
> phase, which results in the PCIe link failing to come up.
> 
> The workaround is to disable Duty-cycle Corrector (DCC) calibration
> after G_RST.
> 
> To do this it is necessary to gain access to the undocumented and
> currently unused PCIe PHY register bank.??A new device tree node of type
> "fsl,imx-pcie-phy" is created for the PHY block and the existing PCIe
> device uses a phandle named "fsl,pcie-phy" to point to it.
> 
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > Cc: Richard Zhu <hongxing.zhu@nxp.com>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Signed-off-by: Trent Piepho <tpiepho@impinj.com>
> ---
> ?drivers/pci/dwc/pci-imx6.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++
> ?1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/pci/dwc/pci-imx6.c b/drivers/pci/dwc/pci-imx6.c
> index dcf389b62944..32429afc9380 100644
> --- a/drivers/pci/dwc/pci-imx6.c
> +++ b/drivers/pci/dwc/pci-imx6.c
> @@ -18,6 +18,7 @@
> ?#include <linux/module.h>
> ?#include <linux/of_gpio.h>
> ?#include <linux/of_device.h>
> +#include <linux/of_address.h>
> ?#include <linux/pci.h>
> ?#include <linux/platform_device.h>
> ?#include <linux/regmap.h>
> @@ -58,6 +59,7 @@ struct imx6_pcie {
> > > ?	u32			tx_swing_low;
> > > ?	int			link_gen;
> > > ?	struct regulator	*vpcie;
> > > +	void __iomem		*phy_base;
> ?};
> ?
> ?/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
> @@ -98,6 +100,23 @@ struct imx6_pcie {
> ?#define PCIE_PHY_RX_ASIC_OUT 0x100D
> > ?#define PCIE_PHY_RX_ASIC_OUT_VALID	(1 << 0)
> ?
> +/* iMX7 PCIe PHY registers */
> > +#define PCIE_PHY_CMN_REG4		0x14
> +/* These are probably the bits that *aren't* DCC_FB_EN */
> > +#define PCIE_PHY_CMN_REG4_DCC_FB_EN	0x29
> +
> > +#define PCIE_PHY_CMN_REG15	????????0x54
> > +#define PCIE_PHY_CMN_REG15_DLY_4	BIT(2)
> > +#define PCIE_PHY_CMN_REG15_PLL_PD	BIT(5)
> > +#define PCIE_PHY_CMN_REG15_OVRD_PLL_PD	BIT(7)
> +
> > +#define PCIE_PHY_CMN_REG24		0x90
> > +#define PCIE_PHY_CMN_REG24_RX_EQ	BIT(6)
> > +#define PCIE_PHY_CMN_REG24_RX_EQ_SEL	BIT(3)
> +
> > +#define PCIE_PHY_CMN_REG26		0x98
> > +#define PCIE_PHY_CMN_REG26_ATT_MODE	0xBC
> +
> ?#define PHY_RX_OVRD_IN_LO 0x1005
> ?#define PHY_RX_OVRD_IN_LO_RX_DATA_EN (1 << 5)
> ?#define PHY_RX_OVRD_IN_LO_RX_PLL_EN (1 << 3)
> @@ -431,6 +450,26 @@ 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);
> +
> > +		/* Workaround for ERR010728, failure of PCI-e PLL VCO to
> > +		?* oscillate, especially when cold.??This turns off "Duty-cycle
> > +		?* Corrector" and other mysterious undocumented things.
> > +		?*/
> > +		if (likely(imx6_pcie->phy_base)) {
> > +			/* De-assert DCC_FB_EN */
> > +			writel(PCIE_PHY_CMN_REG4_DCC_FB_EN,
> > +			???????imx6_pcie->phy_base + PCIE_PHY_CMN_REG4);
> > +			/* Assert RX_EQS and RX_EQS_SEL */
> > +			writel(PCIE_PHY_CMN_REG24_RX_EQ_SEL
> > +				| PCIE_PHY_CMN_REG24_RX_EQ,
> > +			???????imx6_pcie->phy_base + PCIE_PHY_CMN_REG24);
> > +			/* Assert ATT_MODE */
> > +			writel(PCIE_PHY_CMN_REG26_ATT_MODE,
> > +			???????imx6_pcie->phy_base + PCIE_PHY_CMN_REG26);
> > +		} else {
> +			dev_err(dev, "PHY base address not set\n");

Please tune this down to the dev_warn level, in case we are running a
new kernel on a older DT. Also the message isn't really helpful to the
user. It would be better to have something like "No access to PHY
registers, can't apply workaround for ERR010728." Maybe even with a
hint to update the DT.

> +		}
> +
> > ?		imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
> > ?		break;
> > ?	case IMX6SX:
> @@ -698,6 +737,7 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > ?	struct device *dev = &pdev->dev;
> > ?	struct dw_pcie *pci;
> > ?	struct imx6_pcie *imx6_pcie;
> > +	struct device_node *np;
> > ?	struct resource *dbi_base;
> > ?	struct device_node *node = dev->of_node;
> > ?	int ret;
> @@ -717,6 +757,25 @@ static int imx6_pcie_probe(struct platform_device *pdev)
> > ?	imx6_pcie->variant =
> > ?		(enum imx6_pcie_variants)of_device_get_match_data(dev);
> ?
> > +	/* Find the PHY if one is defined, only imx7d uses it */
> > +	np = of_parse_phandle(node, "fsl,pcie-phy", 0);
> > +	if (np) {
> > +		struct resource res;
> +
> > +		ret = of_address_to_resource(np, 0, &res);
> > +		if (ret) {
> > +			dev_err(dev, "Unable to map PCIe PHY\n");
> > +			return ret;
> > +		}
> > +		imx6_pcie->phy_base = devm_ioremap_resource(dev, &res);
> > +		if (IS_ERR(imx6_pcie->phy_base)) {
> > +			dev_err(dev, "Unable to map PCIe PHY\n");
> > +			return PTR_ERR(imx6_pcie->phy_base);
> > +		}
> > +	} else {
> > +		imx6_pcie->phy_base = NULL;
> +	}

imx6_pcie is zero initialized on allocation, so no need for the else
path setting this to NULL.

Regards,
Lucas

> +
> > ?	dbi_base = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > ?	pci->dbi_base = devm_ioremap_resource(dev, dbi_base);
> > ?	if (IS_ERR(pci->dbi_base))

Re: [PATCH 0/2] Workaround for IMX7d PCI-e PLL lock failure

[Lucas Stach]

Hi Trent,

Am Mittwoch, den 18.07.2018, 12:44 -0700 schrieb Trent Piepho:
> This is the workaround for the IMX7d Erratum e10728, failure of
> initialize PCIe PLL VCO oscillation resulting in PLL lock failure and
> failure of the PCI-e link to come up.
> 
> The registers used in the workaround are based on the latest patch in
> the NXP kernel, but many things around that have been changed.
> 
> This uses a new node of type fsl,imx-pcie-phy to get the PHY's
> registers.  The node is found via a phandle added to the PCI-e
> controller's node, rather than the incorrect way done in the NXP kernel.
> 
> There is no error if the phandle is not preset (since it's needed except
> for the imx7d workaround and no existing dtses have it), but if preset
> it is an error if something relating to it does not work.
> 
> ** Should the node be fsl,imx7d-pcie-phy?  snps,dw-pcie-phy?  
> 
> There is little to no documenation from NXP and Synopsis about this, so I'm
> unsure of the PHY's lineage.
> 
> The imx6 PCI-e driver does not use the generic phy layer to interact
> with the PHY.  It appears PHY related hardware, like clocks, regulators,
> and resets, are part of the fsl,imx6q-pcie node.  But again, the
> topology of this hardware is not documented very well.
> 
> Another approach would be to add the PHY registers as another bank in
> the PCI-e node.  This would match how the PHY reset, clock, etc.  are
> done.  However, the PHY is attached to a different AXI master than the
> PCI-e controller, so the register range really does not belong there.

I took some time pondering about the way this is done. There are some
small issues in the implementation (see my replies to the individual
patches), but the overall approach taken looks fine to me.

Regards,
Lucas

[PATCH 0/2] Workaround for IMX7d PCI-e PLL lock failure

[Lucas Stach]

Hi Trent,

Am Mittwoch, den 18.07.2018, 12:44 -0700 schrieb Trent Piepho:
> This is the workaround for the IMX7d Erratum e10728, failure of
> initialize PCIe PLL VCO oscillation resulting in PLL lock failure and
> failure of the PCI-e link to come up.
> 
> The registers used in the workaround are based on the latest patch in
> the NXP kernel, but many things around that have been changed.
> 
> This uses a new node of type fsl,imx-pcie-phy to get the PHY's
> registers.??The node is found via a phandle added to the PCI-e
> controller's node, rather than the incorrect way done in the NXP kernel.
> 
> There is no error if the phandle is not preset (since it's needed except
> for the imx7d workaround and no existing dtses have it), but if preset
> it is an error if something relating to it does not work.
> 
> ** Should the node be fsl,imx7d-pcie-phy???snps,dw-pcie-phy???
> 
> There is little to no documenation from NXP and Synopsis about this, so I'm
> unsure of the PHY's lineage.
> 
> The imx6 PCI-e driver does not use the generic phy layer to interact
> with the PHY.??It appears PHY related hardware, like clocks, regulators,
> and resets, are part of the fsl,imx6q-pcie node.??But again, the
> topology of this hardware is not documented very well.
> 
> Another approach would be to add the PHY registers as another bank in
> the PCI-e node.??This would match how the PHY reset, clock, etc.??are
> done.??However, the PHY is attached to a different AXI master than the
> PCI-e controller, so the register range really does not belong there.

I took some time pondering about the way this is done. There are some
small issues in the implementation (see my replies to the individual
patches), but the overall approach taken looks fine to me.

Regards,
Lucas

Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Trent Piepho]

On Wed, 2018-08-01 at 12:39 +0200, Lucas Stach wrote:
> 
> > +This is the PHY associated with the IMX7d PCIe controller.  It's
> > used by the
> > +PCI-e controller via the fsl,pcie-phy phandle.
> > +
> > +Required properties:
> > +- compatible:
> > +	- "fsl,imx-pcie-phy"
> 
> This is too generic. Please change it to "fsl,imx7-pcie-phy".

Can anyone from NXP tell us if an external PCIe PHY block is present in
other IMX designs?  I suspect that this is generic, but no design other
than imx7d has had any reason to use this PHY register space yet.

If this is specific to this SoC, then maybe it shoudl be fsl,imx7d-
pcie-phy, since the iMX7s has no pcie.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Trent Piepho]

On Wed, 2018-08-01 at 12:39 +0200, Lucas Stach wrote:
> 
> > +This is the PHY associated with the IMX7d PCIe controller.  It's
> > used by the
> > +PCI-e controller via the fsl,pcie-phy phandle.
> > +
> > +Required properties:
> > +- compatible:
> > +	- "fsl,imx-pcie-phy"
> 
> This is too generic. Please change it to "fsl,imx7-pcie-phy".

Can anyone from NXP tell us if an external PCIe PHY block is present in
other IMX designs?  I suspect that this is generic, but no design other
than imx7d has had any reason to use this PHY register space yet.

If this is specific to this SoC, then maybe it shoudl be fsl,imx7d-
pcie-phy, since the iMX7s has no pcie.

RE: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Richard Zhu]

> -----Original Message-----
> From: Trent Piepho [mailto:tpiepho@impinj.com]
> Sent: Thursday, August 2, 2018 1:34 AM
> To: l.stach@pengutronix.de; linux-arm-kernel@lists.infradead.org;
> linux-pci@vger.kernel.org
> Cc: Richard Zhu <hongxing.zhu@nxp.com>; shawnguo@kernel.org;
> kernel@pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>
> Subject: Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY
> 
> On Wed, 2018-08-01 at 12:39 +0200, Lucas Stach wrote:
> >
> > > +This is the PHY associated with the IMX7d PCIe controller.  It's
> > > used by the
> > > +PCI-e controller via the fsl,pcie-phy phandle.
> > > +
> > > +Required properties:
> > > +- compatible:
> > > +	- "fsl,imx-pcie-phy"
> >
> > This is too generic. Please change it to "fsl,imx7-pcie-phy".
> 
> Can anyone from NXP tell us if an external PCIe PHY block is present in other
> IMX designs?  I suspect that this is generic, but no design other than imx7d
> has had any reason to use this PHY register space yet.
>
Hi Trent:
There is a different PCIe PHY block in imx7d PCIe refer to the imx6.
So, the standalone memory region is used to map the imx7d PCIe PHY registers.
Instead of the standalone PHY node, how about to include the PHY registers memory region into PCIe node?
For example:
               pcie: pcie@0x33800000 {
                        compatible = "fsl,imx7d-pcie", "snps,dw-pcie";
                        reg = <0x33800000 0x4000>, <0x306d0000 0x10000>, <0x4ff00000 0x80000>;
                        reg-names = "dbi", "phy", "config";

Richard Zhu
Best Regards!

 
> If this is specific to this SoC, then maybe it shoudl be fsl,imx7d- pcie-phy, since
> the iMX7s has no pcie.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Richard Zhu]

> -----Original Message-----
> From: Trent Piepho [mailto:tpiepho at impinj.com]
> Sent: Thursday, August 2, 2018 1:34 AM
> To: l.stach at pengutronix.de; linux-arm-kernel at lists.infradead.org;
> linux-pci at vger.kernel.org
> Cc: Richard Zhu <hongxing.zhu@nxp.com>; shawnguo at kernel.org;
> kernel at pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>
> Subject: Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY
> 
> On Wed, 2018-08-01 at 12:39 +0200, Lucas Stach wrote:
> >
> > > +This is the PHY associated with the IMX7d PCIe controller.  It's
> > > used by the
> > > +PCI-e controller via the fsl,pcie-phy phandle.
> > > +
> > > +Required properties:
> > > +- compatible:
> > > +	- "fsl,imx-pcie-phy"
> >
> > This is too generic. Please change it to "fsl,imx7-pcie-phy".
> 
> Can anyone from NXP tell us if an external PCIe PHY block is present in other
> IMX designs?  I suspect that this is generic, but no design other than imx7d
> has had any reason to use this PHY register space yet.
>
Hi Trent:
There is a different PCIe PHY block in imx7d PCIe refer to the imx6.
So, the standalone memory region is used to map the imx7d PCIe PHY registers.
Instead of the standalone PHY node, how about to include the PHY registers memory region into PCIe node?
For example:
               pcie: pcie at 0x33800000 {
                        compatible = "fsl,imx7d-pcie", "snps,dw-pcie";
                        reg = <0x33800000 0x4000>, <0x306d0000 0x10000>, <0x4ff00000 0x80000>;
                        reg-names = "dbi", "phy", "config";

Richard Zhu
Best Regards!

 
> If this is specific to this SoC, then maybe it shoudl be fsl,imx7d- pcie-phy, since
> the iMX7s has no pcie.

Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Lucas Stach]

QW0gRG9ubmVyc3RhZywgZGVuIDAyLjA4LjIwMTgsIDAwOjQzICswMDAwIHNjaHJpZWIgUmljaGFy
ZCBaaHU6Cj4gPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQo+ID4gPiA+IEZyb206IFRyZW50
IFBpZXBobyBbbWFpbHRvOnRwaWVwaG9AaW1waW5qLmNvbV0KPiA+IFNlbnQ6IFRodXJzZGF5LCBB
dWd1c3QgMiwgMjAxOCAxOjM0IEFNCj4gPiA+ID4gPiA+IFRvOiBsLnN0YWNoQHBlbmd1dHJvbml4
LmRlOyBsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmc7Cj4gPiBsaW51eC1wY2lA
dmdlci5rZXJuZWwub3JnCj4gPiA+ID4gPiA+IENjOiBSaWNoYXJkIFpodSA8aG9uZ3hpbmcuemh1
QG54cC5jb20+OyBzaGF3bmd1b0BrZXJuZWwub3JnOwo+ID4gPiA+IGtlcm5lbEBwZW5ndXRyb25p
eC5kZTsgRmFiaW8gRXN0ZXZhbSA8ZmFiaW8uZXN0ZXZhbUBueHAuY29tPgo+ID4gU3ViamVjdDog
UmU6IFtQQVRDSCAxLzJdIEFSTTogZHRzOiBpbXg3ZDogQWRkIG5vZGUgZm9yIFBDSWUgUEhZCj4g
PiAKPiA+IE9uIFdlZCwgMjAxOC0wOC0wMSBhdCAxMjozOSArMDIwMCwgTHVjYXMgU3RhY2ggd3Jv
dGU6Cj4gPiA+IAo+ID4gPiA+ICtUaGlzIGlzIHRoZSBQSFkgYXNzb2NpYXRlZCB3aXRoIHRoZSBJ
TVg3ZCBQQ0llIGNvbnRyb2xsZXIuwqDCoEl0J3MKPiA+ID4gPiB1c2VkIGJ5IHRoZQo+ID4gPiA+
ICtQQ0ktZSBjb250cm9sbGVyIHZpYSB0aGUgZnNsLHBjaWUtcGh5IHBoYW5kbGUuCj4gPiA+ID4g
Kwo+ID4gPiA+ICtSZXF1aXJlZCBwcm9wZXJ0aWVzOgo+ID4gPiA+ICstIGNvbXBhdGlibGU6Cj4g
PiA+ID4gKwktICJmc2wsaW14LXBjaWUtcGh5Igo+ID4gPiAKPiA+ID4gVGhpcyBpcyB0b28gZ2Vu
ZXJpYy4gUGxlYXNlIGNoYW5nZSBpdCB0byAiZnNsLGlteDctcGNpZS1waHkiLgo+ID4gCj4gPiBD
YW4gYW55b25lIGZyb20gTlhQIHRlbGwgdXMgaWYgYW4gZXh0ZXJuYWwgUENJZSBQSFkgYmxvY2sg
aXMgcHJlc2VudCBpbiBvdGhlcgo+ID4gSU1YIGRlc2lnbnM/wqDCoEkgc3VzcGVjdCB0aGF0IHRo
aXMgaXMgZ2VuZXJpYywgYnV0IG5vIGRlc2lnbiBvdGhlciB0aGFuIGlteDdkCj4gPiBoYXMgaGFk
IGFueSByZWFzb24gdG8gdXNlIHRoaXMgUEhZIHJlZ2lzdGVyIHNwYWNlIHlldC4KPiA+IAo+IAo+
IEhpIFRyZW50Ogo+IFRoZXJlIGlzIGEgZGlmZmVyZW50IFBDSWUgUEhZIGJsb2NrIGluIGlteDdk
IFBDSWUgcmVmZXIgdG8gdGhlIGlteDYuCj4gU28sIHRoZSBzdGFuZGFsb25lIG1lbW9yeSByZWdp
b24gaXMgdXNlZCB0byBtYXAgdGhlIGlteDdkIFBDSWUgUEhZIHJlZ2lzdGVycy4KPiBJbnN0ZWFk
IG9mIHRoZSBzdGFuZGFsb25lIFBIWSBub2RlLCBob3cgYWJvdXQgdG8gaW5jbHVkZSB0aGUgUEhZ
IHJlZ2lzdGVycyBtZW1vcnkgcmVnaW9uIGludG8gUENJZSBub2RlPwo+IEZvciBleGFtcGxlOgo+
ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgcGNpZTogcGNpZUAweDMzODAwMDAwIHsK
PiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBjb21wYXRp
YmxlID0gImZzbCxpbXg3ZC1wY2llIiwgInNucHMsZHctcGNpZSI7Cj4gwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgcmVnID0gPDB4MzM4MDAwMDAgMHg0MDAw
PiwgPDB4MzA2ZDAwMDAgMHgxMDAwMD4sIDwweDRmZjAwMDAwIDB4ODAwMDA+Owo+IMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHJlZy1uYW1lcyA9ICJkYmki
LCAicGh5IiwgImNvbmZpZyI7CgpJIGRpc2xpa2UgdGhpcyBzb2x1dGlvbiwgYXMgaXQgaXNuJ3Qg
YSBjb3JyZWN0IGRlc2NyaXB0aW9uIG9mIHRoZSBIVy4KSGF2aW5nIGl0IGFzIGEgc2VwYXJhdGUg
bm9kZSB3aXRoIGl0J3Mgb3duIGNvbXBhdGlibGUsIGFzIGRvbmUgaW4gdGhpcwpwYXRjaHNldCwg
YWxsb3dzIHVzIHRvIHN3aXRjaCBvdmVyIHRvIGEgcmVhbCBQSFkgZHJpdmVyIHNob3VsZCB3ZSBl
dmVyCmhhdmUgYSBuZWVkIGZvciBpdCwgYWxsIHdpdGhvdXQgYnJlYWtpbmcgdGhlIERUIGFic3Ry
YWN0aW9uIGFnYWluLgoKUmVnYXJkcywKTHVjYXMgCgpfX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51
eC1hcm0ta2VybmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5v
cmcvbWFpbG1hbi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg==

[PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY

[Lucas Stach]

Am Donnerstag, den 02.08.2018, 00:43 +0000 schrieb Richard Zhu:
> > -----Original Message-----
> > > > From: Trent Piepho [mailto:tpiepho at impinj.com]
> > Sent: Thursday, August 2, 2018 1:34 AM
> > > > > > To: l.stach at pengutronix.de; linux-arm-kernel at lists.infradead.org;
> > linux-pci at vger.kernel.org
> > > > > > Cc: Richard Zhu <hongxing.zhu@nxp.com>; shawnguo at kernel.org;
> > > > kernel at pengutronix.de; Fabio Estevam <fabio.estevam@nxp.com>
> > Subject: Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY
> > 
> > On Wed, 2018-08-01 at 12:39 +0200, Lucas Stach wrote:
> > > 
> > > > +This is the PHY associated with the IMX7d PCIe controller.??It's
> > > > used by the
> > > > +PCI-e controller via the fsl,pcie-phy phandle.
> > > > +
> > > > +Required properties:
> > > > +- compatible:
> > > > +	- "fsl,imx-pcie-phy"
> > > 
> > > This is too generic. Please change it to "fsl,imx7-pcie-phy".
> > 
> > Can anyone from NXP tell us if an external PCIe PHY block is present in other
> > IMX designs???I suspect that this is generic, but no design other than imx7d
> > has had any reason to use this PHY register space yet.
> > 
> 
> Hi Trent:
> There is a different PCIe PHY block in imx7d PCIe refer to the imx6.
> So, the standalone memory region is used to map the imx7d PCIe PHY registers.
> Instead of the standalone PHY node, how about to include the PHY registers memory region into PCIe node?
> For example:
> > ???????????????pcie: pcie at 0x33800000 {
> ????????????????????????compatible = "fsl,imx7d-pcie", "snps,dw-pcie";
> ????????????????????????reg = <0x33800000 0x4000>, <0x306d0000 0x10000>, <0x4ff00000 0x80000>;
> ????????????????????????reg-names = "dbi", "phy", "config";

I dislike this solution, as it isn't a correct description of the HW.
Having it as a separate node with it's own compatible, as done in this
patchset, allows us to switch over to a real PHY driver should we ever
have a need for it, all without breaking the DT abstraction again.

Regards,
Lucas 

Re: [PATCH 0/2] Workaround for IMX7d PCI-e PLL lock failure

[Lorenzo Pieralisi]

On Wed, Jul 18, 2018 at 12:44:22PM -0700, Trent Piepho wrote:
> This is the workaround for the IMX7d Erratum e10728, failure of
> initialize PCIe PLL VCO oscillation resulting in PLL lock failure and
> failure of the PCI-e link to come up.
> 
> The registers used in the workaround are based on the latest patch in
> the NXP kernel, but many things around that have been changed.
> 
> This uses a new node of type fsl,imx-pcie-phy to get the PHY's
> registers.  The node is found via a phandle added to the PCI-e
> controller's node, rather than the incorrect way done in the NXP kernel.
> 
> There is no error if the phandle is not preset (since it's needed except
> for the imx7d workaround and no existing dtses have it), but if preset
> it is an error if something relating to it does not work.
> 
> ** Should the node be fsl,imx7d-pcie-phy?  snps,dw-pcie-phy?  
> 
> There is little to no documenation from NXP and Synopsis about this, so I'm
> unsure of the PHY's lineage.
> 
> The imx6 PCI-e driver does not use the generic phy layer to interact
> with the PHY.  It appears PHY related hardware, like clocks, regulators,
> and resets, are part of the fsl,imx6q-pcie node.  But again, the
> topology of this hardware is not documented very well.
> 
> Another approach would be to add the PHY registers as another bank in
> the PCI-e node.  This would match how the PHY reset, clock, etc.  are
> done.  However, the PHY is attached to a different AXI master than the
> PCI-e controller, so the register range really does not belong there.
> 
> Trent Piepho (2):
>   ARM: dts: imx7d: Add node for PCIe PHY
>   PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

Marked as "changes requested", following Lucas' review, please
respin as appropriate.

Lorenzo

>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     | 11 ++++
>  arch/arm/boot/dts/imx7d.dtsi                       |  9 ++++
>  drivers/pci/dwc/pci-imx6.c                         | 59 ++++++++++++++++++++++
>  3 files changed, 79 insertions(+)
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> 
> -- 
> 2.14.4
> 
> 

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

Re: [PATCH 0/2] Workaround for IMX7d PCI-e PLL lock failure

[Lorenzo Pieralisi]

On Wed, Jul 18, 2018 at 12:44:22PM -0700, Trent Piepho wrote:
> This is the workaround for the IMX7d Erratum e10728, failure of
> initialize PCIe PLL VCO oscillation resulting in PLL lock failure and
> failure of the PCI-e link to come up.
> 
> The registers used in the workaround are based on the latest patch in
> the NXP kernel, but many things around that have been changed.
> 
> This uses a new node of type fsl,imx-pcie-phy to get the PHY's
> registers.  The node is found via a phandle added to the PCI-e
> controller's node, rather than the incorrect way done in the NXP kernel.
> 
> There is no error if the phandle is not preset (since it's needed except
> for the imx7d workaround and no existing dtses have it), but if preset
> it is an error if something relating to it does not work.
> 
> ** Should the node be fsl,imx7d-pcie-phy?  snps,dw-pcie-phy?  
> 
> There is little to no documenation from NXP and Synopsis about this, so I'm
> unsure of the PHY's lineage.
> 
> The imx6 PCI-e driver does not use the generic phy layer to interact
> with the PHY.  It appears PHY related hardware, like clocks, regulators,
> and resets, are part of the fsl,imx6q-pcie node.  But again, the
> topology of this hardware is not documented very well.
> 
> Another approach would be to add the PHY registers as another bank in
> the PCI-e node.  This would match how the PHY reset, clock, etc.  are
> done.  However, the PHY is attached to a different AXI master than the
> PCI-e controller, so the register range really does not belong there.
> 
> Trent Piepho (2):
>   ARM: dts: imx7d: Add node for PCIe PHY
>   PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

Marked as "changes requested", following Lucas' review, please
respin as appropriate.

Lorenzo

>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     | 11 ++++
>  arch/arm/boot/dts/imx7d.dtsi                       |  9 ++++
>  drivers/pci/dwc/pci-imx6.c                         | 59 ++++++++++++++++++++++
>  3 files changed, 79 insertions(+)
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> 
> -- 
> 2.14.4
> 
> 

[PATCH 0/2] Workaround for IMX7d PCI-e PLL lock failure

[Lorenzo Pieralisi]

On Wed, Jul 18, 2018 at 12:44:22PM -0700, Trent Piepho wrote:
> This is the workaround for the IMX7d Erratum e10728, failure of
> initialize PCIe PLL VCO oscillation resulting in PLL lock failure and
> failure of the PCI-e link to come up.
> 
> The registers used in the workaround are based on the latest patch in
> the NXP kernel, but many things around that have been changed.
> 
> This uses a new node of type fsl,imx-pcie-phy to get the PHY's
> registers.  The node is found via a phandle added to the PCI-e
> controller's node, rather than the incorrect way done in the NXP kernel.
> 
> There is no error if the phandle is not preset (since it's needed except
> for the imx7d workaround and no existing dtses have it), but if preset
> it is an error if something relating to it does not work.
> 
> ** Should the node be fsl,imx7d-pcie-phy?  snps,dw-pcie-phy?  
> 
> There is little to no documenation from NXP and Synopsis about this, so I'm
> unsure of the PHY's lineage.
> 
> The imx6 PCI-e driver does not use the generic phy layer to interact
> with the PHY.  It appears PHY related hardware, like clocks, regulators,
> and resets, are part of the fsl,imx6q-pcie node.  But again, the
> topology of this hardware is not documented very well.
> 
> Another approach would be to add the PHY registers as another bank in
> the PCI-e node.  This would match how the PHY reset, clock, etc.  are
> done.  However, the PHY is attached to a different AXI master than the
> PCI-e controller, so the register range really does not belong there.
> 
> Trent Piepho (2):
>   ARM: dts: imx7d: Add node for PCIe PHY
>   PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure

Marked as "changes requested", following Lucas' review, please
respin as appropriate.

Lorenzo

>  .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     | 11 ++++
>  arch/arm/boot/dts/imx7d.dtsi                       |  9 ++++
>  drivers/pci/dwc/pci-imx6.c                         | 59 ++++++++++++++++++++++
>  3 files changed, 79 insertions(+)
> 
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> 
> -- 
> 2.14.4
> 
>