| Message ID | 20180718194424.8844-2-tpiepho@impinj.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Workaround for IMX7d PCI-e PLL lock failure | expand |
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
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@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>; > > }; > }; >
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.
> -----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.
Am Donnerstag, den 02.08.2018, 00:43 +0000 schrieb 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"; 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
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>; }; };
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(+)