Workaround for IMX7d PCI-e PLL lock failure
Subsystems (4)
cover Workaround for IMX7d PCI-e PLL lock failure
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(+)
Signature · 1 line
2.14.4
PATCH 1/2 ARM: dts: imx7d: Add node for PCIe PHY
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).
--- Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 11 +++++++++++ arch/arm/boot/dts/imx7d.dtsi | 9 +++++++++ 2 files changed, 20 insertions(+)
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +11 -0
@@ -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
arch/arm/boot/dts/imx7d.dtsi +9 -0
@@ -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>; }; };
Signature · 1 line
2.14.4
PATCH 1/2 ARM: dts: imx7d: Add node for PCIe PHY
Quoted 14 lines · 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 <[email protected]>Cc Sascha Hauer <[email protected]>Cc Fabio Estevam <[email protected]>Cc Richard Zhu <[email protected]>Cc Lucas Stach <[email protected]>Signed-off-by Trent Piepho <[email protected]>
Quoted 4 lines
--- 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
Quoted 62 lines
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +11
-0@@ -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 controllerarch/arm/boot/dts/imx7d.dtsi +9
-0@@ -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>; }; };Signature · 7 lines
2.14.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH 1/2 ARM: dts: imx7d: Add node for PCIe PHY
In reply to Trent PiephoAm Mittwoch, den 18.07.2018, 12:44 -0700 schrieb Trent Piepho:
Quoted 34 lines
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).
Quoted 6 lines
Cc Shawn Guo <[email protected]>Cc Sascha Hauer <[email protected]>Cc Fabio Estevam <[email protected]>Cc Richard Zhu <[email protected]>Cc Lucas Stach <[email protected]>Signed-off-by Trent Piepho <[email protected]>---
?Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt | 11 +++++++++++ ?arch/arm/boot/dts/imx7d.dtsi?????????????????????????????|??9 +++++++++ ?2 files changed, 20 insertions(+)Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +0
-0@@ -50,6 +50,7 @@ Additional required properties for imx7d-pcie:?- reset-names: Must contain the following entires:
Quoted 2 lines
? ???????- "pciephy"
? ???????- "apps"+- fsl,pcie-phy: A phandle to an fsl,imx-pcie-phy node. ?
?Example:
?Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +0
-0@@ -76,3 +77,13 @@ Example: > ? clocks = <&clks 144>, <&clks 206>, <&clks 189>; > ? clock-names = "pcie", "pcie_bus", "pcie_phy";
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt Example: · line 88
Quoted 10 lines
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +9
-0> ? }; + +* 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".
Quoted 28 lines
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +1
-0+- reg: base address and length of the PCIe PHY controllerarch/arm/boot/dts/imx7d.dtsi +0
-0@@ -94,6 +94,14 @@ > ? }; ?};?
+&aips2 {Quoted 5 lines
Quoted 1 line
arch/arm/boot/dts/imx7d.dtsi +1
-0+ pcie_phy: pcie-phy at 306d0000 {(quoted diff) +4
-0+ ??compatible = "fsl,imx-pcie-phy"; + ??reg = <0x306d0000 0x10000>; + ??status = "disabled"; + };arch/arm/boot/dts/imx7d.dtsi +2
-0+}; +?&aips3 {
Quoted 2 lines
Quoted 1 line
? usbotg2: usb at
30b20000{? compatible = "fsl,imx7d-usb", "fsl,imx27-usb";
arch/arm/boot/dts/imx7d.dtsi +0
-0@@ -167,6 +175,7 @@ > ? ?<&src IMX7_RESET_PCIE_CTRL_APPS_EN>; > ? reset-names = "pciephy", "apps"; > ? status = "disabled"; > + fsl,pcie-phy = <&pcie_phy>; > ? }; ?};?
PATCH 1/2 ARM: dts: imx7d: Add node for PCIe PHY
Quoted 10 lines · Lucas Stach
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt Example: · line 88
Quoted 8 lines
+This is the PHY associated with the IMX7d PCIe controller. It's used by the
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +5
-0+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.
PATCH 1/2 ARM: dts: imx7d: Add node for PCIe PHY
Quoted 22 lines
-----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 <[email protected]>; shawnguo at kernel.org; kernel at pengutronix.de; Fabio Estevam <[email protected]>
Subject: Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHYQuoted 10 lines · Lucas Stach
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt Example: · line 88
Quoted 8 lines
+This is the PHY associated with the IMX7d PCIe controller. It's used by the
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +5
-0+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".
Quoted 4 lines
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!
Quoted 2 lines
If this is specific to this SoC, then maybe it shoudl be fsl,imx7d- pcie-phy, since the iMX7s has no pcie.
PATCH 1/2 ARM: dts: imx7d: Add node for PCIe PHY
Am Donnerstag, den 02.08.2018, 00:43 +0000 schrieb Richard Zhu:
Quoted 31 lines
Quoted 21 lines
-----Original Message-----
Quoted 1 line
Quoted 1 line
From: Trent Piepho [mailto:tpiepho at impinj.com]
Sent: Thursday, August 2, 2018 1:34 AM
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
To: l.stach at pengutronix.de; linux-arm-kernel at lists.infradead.org;
linux-pci at vger.kernel.org
Quoted 2 lines
Quoted 2 lines
Quoted 1 line
Quoted 1 line
Cc Richard Zhu <[email protected]>; shawnguo at kernel.org;kernel at pengutronix.de; Fabio Estevam <[email protected]>
Subject: Re: [PATCH 1/2] ARM: dts: imx7d: Add node for PCIe PHY
Quoted 10 lines · Lucas Stach
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt Example: · line 88
Quoted 8 lines
+This is the PHY associated with the IMX7d PCIe controller.??It's used by the
Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt +5
-0+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".
Quoted 4 lines
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:
Quoted 4 lines
Quoted 1 line
???????????????pcie: pcie at
0x33800000{????????????????????????compatible = "fsl,imx7d-pcie", "snps,dw-pcie"; ????????????????????????reg = <
0x338000000x4000>, <0x306d00000x10000>, <0x4ff000000x80000>; ????????????????????????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
PATCH 2/2 PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure
In reply to Trent PiephoThis 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.
--- drivers/pci/dwc/pci-imx6.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)
drivers/pci/dwc/pci-imx6.c +59 -0
@@ -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))
Signature · 1 line
2.14.4
PATCH 2/2 PCI: imx: Add workaround for e10728, IMX7d PCIe PLL failure
Am Mittwoch, den 18.07.2018, 12:44 -0700 schrieb Trent Piepho:
Quoted 89 lines
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.
Quoted 6 lines
Cc Shawn Guo <[email protected]>Cc Sascha Hauer <[email protected]>Cc Fabio Estevam <[email protected]>Cc Richard Zhu <[email protected]>Cc Lucas Stach <[email protected]>Signed-off-by Trent Piepho <[email protected]>---
?drivers/pci/dwc/pci-imx6.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++ ?1 file changed, 59 insertions(+)drivers/pci/dwc/pci-imx6.c +0
-0@@ -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>drivers/pci/dwc/pci-imx6.c +0
-0@@ -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 */drivers/pci/dwc/pci-imx6.c +0
-0@@ -98,6 +100,23 @@ struct imx6_pcie {?#define
PCIE_PHY_RX_ASIC_OUT0x100DQuoted 1 line
?#define
PCIE_PHY_RX_ASIC_OUT_VALID(1 << 0)?
+/* iMX7 PCIe PHY registers */Quoted 1 line
+#define
PCIE_PHY_CMN_REG40x14+/* These are probably the bits that *aren't*
DCC_FB_EN*/Quoted 1 line
+#define
PCIE_PHY_CMN_REG4_DCC_FB_EN0x29+
drivers/pci/dwc/pci-imx6.c struct imx6_pcie { · line 111
Quoted 4 lines
drivers/pci/dwc/pci-imx6.c +4
-0+#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)+
drivers/pci/dwc/pci-imx6.c struct imx6_pcie { · line 115
Quoted 3 lines
drivers/pci/dwc/pci-imx6.c +3
-0+#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)+
drivers/pci/dwc/pci-imx6.c struct imx6_pcie { · line 118
Quoted 2 lines
drivers/pci/dwc/pci-imx6.c +2
-0+#define PCIE_PHY_CMN_REG26 0x98 +#define PCIE_PHY_CMN_REG26_ATT_MODE 0xBC+
?#definePHY_RX_OVRD_IN_LO0x1005
?#definePHY_RX_OVRD_IN_LO_RX_DATA_EN(1 << 5)
?#definePHY_RX_OVRD_IN_LO_RX_PLL_EN(1 << 3)drivers/pci/dwc/pci-imx6.c +0
-0@@ -431,6 +450,26 @@ static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie) > ? switch (imx6_pcie->variant) {Quoted 2 lines
? case IMX7D:
?reset_control_deassert(imx6_pcie->pciephy_reset);+
Quoted 16 lines
drivers/pci/dwc/pci-imx6.c +16
-0+/* 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 {
drivers/pci/dwc/pci-imx6.c static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie) · line 470
Quoted 1 line
drivers/pci/dwc/pci-imx6.c +1
-0+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.
Quoted 34 lines
drivers/pci/dwc/pci-imx6.c +2
-0+ } +Quoted 3 lines
?
imx7d_pcie_wait_for_phy_pll_lock(imx6_pcie);
? break;
? case IMX6SX:drivers/pci/dwc/pci-imx6.c +0
-0@@ -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;Quoted 1 line
? int ret;
drivers/pci/dwc/pci-imx6.c +0
-0@@ -717,6 +757,25 @@ static int imx6_pcie_probe(struct platform_device *pdev)Quoted 2 lines
?
imx6_pcie->variant=
? (enum imx6_pcie_variants)of_device_get_match_data(dev);?
drivers/pci/dwc/pci-imx6.c static int imx6_pcie_probe(struct platform_device *pdev) · line 763
Quoted 4 lines
drivers/pci/dwc/pci-imx6.c +4
-0+/* 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;+
Quoted 12 lines
drivers/pci/dwc/pci-imx6.c +12
-0+ 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 ;
drivers/pci/dwc/pci-imx6.c
Quoted 1 line
drivers/pci/dwc/pci-imx6.c +1
-0+ }
imx6_pcie is zero initialized on allocation, so no need for the else path setting this to NULL.
Regards,
Lucas
Quoted 4 lines
+
Quoted 3 lines
?
dbi_base=platform_get_resource(pdev, IORESOURCE_MEM, 0);
?pci->dbi_base=devm_ioremap_resource(dev, dbi_base);
? if (IS_ERR(pci->dbi_base))
cover Workaround for IMX7d PCI-e PLL lock failure
In reply to Trent PiephoHi Trent,
Am Mittwoch, den 18.07.2018, 12:44 -0700 schrieb Trent Piepho:
Quoted 25 lines
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.
Quoted 4 lines
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
cover Workaround for IMX7d PCI-e PLL lock failure
In reply to Trent PiephoQuoted 25 lines · 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.
Quoted 8 lines
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
Quoted 15 lines
.../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 <[email protected]>Cc Sascha Hauer <[email protected]>Cc Fabio Estevam <[email protected]>Cc Richard Zhu <[email protected]>Cc Lucas Stach <[email protected]>Signature · 1 line
2.14.4