[PATCH 0/3] PCI: imx6: select the PCIe REF_CLK source on i.MX8MQ

[PATCH 0/3] PCI: imx6: select the PCIe REF_CLK source on i.MX8MQ

[Rudi Heitbaum]

i.MX8MQ can take PCIe REF_CLK either from an off-chip oscillator or from
the internal PLL, but imx8mq_pcie_init_phy() has always selected the
oscillator unconditionally:

	/* TODO: This code assumes external oscillator is being used */

Boards that wire an oscillator to the pad describe it as the "pcie_bus"
clock, but nothing in the driver looks that clock up by name, so it does
not select anything - the unconditional IMX8MQ_GPR_PCIE_REF_USE_PAD is
what actually makes those boards work.

Commit d8574ce57d76 ("PCI: imx6: Add external reference clock input mode
support") added a generic enable_ext_refclk flag, set when an "extref"
clock is supplied, and wired it up for i.MX95. This series extends the
same mechanism to i.MX8MQ, which resolves the TODO above.

Patch 1 allows an optional fifth "extref" clock for fsl,imx8mq-pcie.
Patch 2 makes the driver honour enable_ext_refclk on i.MX8MQ.
Patch 3 names the oscillator on the in-tree i.MX8MQ boards that have one.

Patch 2 must not land before patch 3. On its own it would stop setting
REF_USE_PAD on any board that has not declared an "extref" clock, which
is all of them today. The reverse order is safe: patch 3 only adds a
clock that the driver ignores until patch 2 is applied.

Patch 3 touches arch/arm64/boot/dts/freescale and would normally go via
the i.MX tree rather than the PCI tree. Given the ordering constraint
above, either all three through one tree, or 1 and 3 first with 2
following, would work - whichever the maintainers prefer.

Patch 3 is deliberately additive: each board keeps the "pcie_bus" entry
it already has and gains a fifth clock naming the same oscillator, so
the set of clocks being enabled is unchanged.

Tested on a Google Coral Dev Board (imx8mq-phanbell, not yet enabled in
mainline), where pcie1 takes the off-chip oscillator and pcie0 the
internal PLL. With the extref clock declared, clock-names reads

	pcie pcie_bus pcie_phy pcie_aux extref

pcie1 links at Gen2 and the endpoint enumerates.

The five boards touched by patch 3 are untested - I have no access to
them. The change is intended to preserve their current behaviour
exactly, and review from anyone who has the hardware would be welcome.

Rudi Heitbaum (3):
  dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ
  PCI: imx6: Select the PCIe REF_CLK source on i.MX8MQ
  arm64: dts: imx8mq: Declare the PCIe extref clock

 .../devicetree/bindings/pci/fsl,imx6q-pcie.yaml          | 8 ++++++--
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts             | 8 ++++++--
 .../boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts     | 8 ++++++--
 arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts     | 4 +++-
 arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts  | 8 ++++++--
 arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi      | 8 ++++++--
 drivers/pci/controller/dwc/pci-imx6.c                    | 9 +++++++--
 7 files changed, 40 insertions(+), 13 deletions(-)

-- 
2.53.1

[PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ

[Rudi Heitbaum]

i.MX8MQ can drive PCIe REF_CLK either from the internal PLL or from an
off-chip oscillator. Allow an optional fifth "extref" clock to describe
the latter, matching the name already used for i.MX95.

Signed-off-by: Rudi Heitbaum <rudi@heitbaum.com>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index e8b8131f5f23..8fb0c919ae57 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -46,7 +46,8 @@ properties:
       - description: PCIe PHY clock.
       - description: Additional required clock entry for imx6sx-pcie,
            imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
-      - description: PCIe internal reference clock.
+      - description: PCIe internal reference clock for imx95-pcie, or the
+          external reference clock for imx8mq-pcie.
       - description: PCIe additional external reference clock.
 
   clock-names:
@@ -173,13 +174,16 @@ allOf:
     then:
       properties:
         clocks:
-          maxItems: 4
+          minItems: 4
+          maxItems: 5
         clock-names:
+          minItems: 4
           items:
             - const: pcie
             - const: pcie_bus
             - const: pcie_phy
             - const: pcie_aux
+            - const: extref  # Optional
 
   - if:
       properties:
-- 
2.53.0

[PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed for i.MX8MQ

[Rudi Heitbaum]

i.MX8MQ has two PCIe REF_CLK sources: an off-chip oscillator and the
internal PLL. The driver unconditionally selected the oscillator, which
is what the

	/* TODO: This code assumes external oscillator is being used */

comment refers to. Boards wiring an oscillator to the pad describe it as
the "pcie_bus" clock, but nothing in the driver looks that clock up by
name, so it does not select anything.

Reuse the generic enable_ext_refclk flag, set when an "extref" clock is
supplied, to select between the two, as is already done for i.MX95 in
commit d8574ce57d76 ("PCI: imx6: Add external reference clock input mode
support").

Signed-off-by: Rudi Heitbaum <rudi@heitbaum.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index dcdeba1856f7..5a350b458782 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -445,11 +445,16 @@ static int pcie_phy_write(struct imx_pcie *imx_pcie, int addr, u16 data)
 
 static int imx8mq_pcie_init_phy(struct imx_pcie *imx_pcie)
 {
-	/* TODO: This code assumes external oscillator is being used */
+	bool ext = imx_pcie->enable_ext_refclk;
+
+	/*
+	 * Select the off-chip oscillator as REF_CLK when an "extref" clock is
+	 * supplied, otherwise fall back to the internal PLL.
+	 */
 	regmap_update_bits(imx_pcie->iomuxc_gpr,
 			   imx_pcie_grp_offset(imx_pcie),
 			   IMX8MQ_GPR_PCIE_REF_USE_PAD,
-			   IMX8MQ_GPR_PCIE_REF_USE_PAD);
+			   ext ? IMX8MQ_GPR_PCIE_REF_USE_PAD : 0);
 	/*
 	 * Per the datasheet, the PCIE_VPH is suggested to be 1.8V.  If the
 	 * PCIE_VPH is supplied by 3.3V, the VREG_BYPASS should be cleared
-- 
2.53.0

[PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock

[Rudi Heitbaum]

Boards feeding PCIe REF_CLK from an off-chip oscillator described it as
the "pcie_bus" clock, which the driver never looks up by name and which
therefore selects nothing. The oscillator was used only because the
driver forced IMX8MQ_GPR_PCIE_REF_USE_PAD unconditionally.

Now that the selection follows the "extref" clock, name the oscillator
explicitly on every board that depends on it so the behaviour is
unchanged. The "pcie_bus" entries are left alone.

Endpoint nodes are not touched: they are all disabled and use a separate
binding.

Signed-off-by: Rudi Heitbaum <rudi@heitbaum.com>
---
 arch/arm64/boot/dts/freescale/imx8mq-evk.dts              | 8 ++++++--
 .../boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts      | 8 ++++++--
 arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts      | 4 +++-
 arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts   | 8 ++++++--
 arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi       | 8 ++++++--
 5 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index e7d87ea81b69..3dc0047cd1de 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -374,7 +374,9 @@ &pcie0 {
 	clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
 		 <&pcie0_refclk>,
 		 <&clk IMX8MQ_CLK_PCIE1_PHY>,
-		 <&clk IMX8MQ_CLK_PCIE1_AUX>;
+		 <&clk IMX8MQ_CLK_PCIE1_AUX>,
+		 <&pcie0_refclk>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	vph-supply = <&vgen5_reg>;
 	supports-clkreq;
 	status = "okay";
@@ -402,7 +404,9 @@ &pcie1 {
 	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
 		 <&pcie0_refclk>,
 		 <&clk IMX8MQ_CLK_PCIE2_PHY>,
-		 <&clk IMX8MQ_CLK_PCIE2_AUX>;
+		 <&clk IMX8MQ_CLK_PCIE2_AUX>,
+		 <&pcie0_refclk>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	vpcie-supply = <&reg_pcie1>;
 	vpcie3v3aux-supply = <&reg_pcie1>;
 	vph-supply = <&vgen5_reg>;
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts b/arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts
index d8cf1f27c3ec..8cf6a64db1a7 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-kontron-pitx-imx8m.dts
@@ -247,7 +247,9 @@ &pcie0 {
 	clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
 		 <&pcie0_refclk>,
 		 <&clk IMX8MQ_CLK_PCIE1_PHY>,
-		 <&clk IMX8MQ_CLK_PCIE1_AUX>;
+		 <&clk IMX8MQ_CLK_PCIE1_AUX>,
+		 <&pcie0_refclk>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	status = "okay";
 };
 
@@ -256,7 +258,9 @@ &pcie1 {
 	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
 		 <&pcie1_refclk>,
 		 <&clk IMX8MQ_CLK_PCIE2_PHY>,
-		 <&clk IMX8MQ_CLK_PCIE2_AUX>;
+		 <&clk IMX8MQ_CLK_PCIE2_AUX>,
+		 <&pcie1_refclk>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	fsl,max-link-speed = <1>;
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts b/arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts
index 3ae3824be027..bf931d2f3a87 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-mnt-reform2.dts
@@ -199,7 +199,9 @@ &pcie1 {
 	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
 		 <&pcie1_refclk>,
 		 <&clk IMX8MQ_CLK_PCIE2_PHY>,
-		 <&clk IMX8MQ_CLK_PCIE2_AUX>;
+		 <&clk IMX8MQ_CLK_PCIE2_AUX>,
+		 <&pcie1_refclk>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts b/arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts
index 0165f3a25985..761c43db3026 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-tqma8mq-mba8mx.dts
@@ -97,7 +97,9 @@ &pcie0 {
 	clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
 		 <&pcieclk 3>,
 		 <&pcieclk 2>,
-		 <&clk IMX8MQ_CLK_PCIE1_AUX>;
+		 <&clk IMX8MQ_CLK_PCIE1_AUX>,
+		 <&pcieclk 3>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	status = "okay";
 };
 
@@ -109,7 +111,9 @@ &pcie1 {
 	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
 		 <&pcieclk 1>,
 		 <&pcieclk 0>,
-		 <&clk IMX8MQ_CLK_PCIE2_AUX>;
+		 <&clk IMX8MQ_CLK_PCIE2_AUX>,
+		 <&pcieclk 1>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	status = "okay";
 };
 
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
index c7bbba45f368..31db96adb237 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq-zii-ultra.dtsi
@@ -551,7 +551,9 @@ &pcie0 {
 	clocks = <&clk IMX8MQ_CLK_PCIE1_ROOT>,
 	         <&pcie0_refclk>,
 	         <&clk IMX8MQ_CLK_PCIE1_PHY>,
-	         <&clk IMX8MQ_CLK_PCIE1_AUX>;
+	         <&clk IMX8MQ_CLK_PCIE1_AUX>,
+	         <&pcie0_refclk>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	vph-supply = <&vgen5_reg>;
 	status = "okay";
 };
@@ -563,7 +565,9 @@ &pcie1 {
 	clocks = <&clk IMX8MQ_CLK_PCIE2_ROOT>,
 	         <&pcie1_refclk>,
 	         <&clk IMX8MQ_CLK_PCIE2_PHY>,
-	         <&clk IMX8MQ_CLK_PCIE2_AUX>;
+	         <&clk IMX8MQ_CLK_PCIE2_AUX>,
+	         <&pcie1_refclk>;
+	clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "extref";
 	vph-supply = <&vgen5_reg>;
 	status = "okay";
 };
-- 
2.53.0