Devicetree
 help / color / mirror / Atom feed
* [PATCH 0/3] PCI: imx6: select the PCIe REF_CLK source on i.MX8MQ
@ 2026-08-02 11:30 Rudi Heitbaum
  2026-08-02 11:31 ` [PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ Rudi Heitbaum
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Rudi Heitbaum @ 2026-08-02 11:30 UTC (permalink / raw)
  To: Richard Zhu, Lucas Stach, Frank Li, Sascha Hauer
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	Krzysztof Kozlowski, Conor Dooley, Fabio Estevam, linux-pci,
	devicetree, imx, linux-arm-kernel, linux-kernel, rudi

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ
  2026-08-02 11:30 [PATCH 0/3] PCI: imx6: select the PCIe REF_CLK source on i.MX8MQ Rudi Heitbaum
@ 2026-08-02 11:31 ` Rudi Heitbaum
  2026-08-02 11:40   ` sashiko-bot
  2026-08-02 11:32 ` [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed " Rudi Heitbaum
  2026-08-02 11:32 ` [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock Rudi Heitbaum
  2 siblings, 1 reply; 7+ messages in thread
From: Rudi Heitbaum @ 2026-08-02 11:31 UTC (permalink / raw)
  To: Richard Zhu, Lucas Stach, Frank Li, Sascha Hauer
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	Krzysztof Kozlowski, Conor Dooley, Fabio Estevam, linux-pci,
	devicetree, imx, linux-arm-kernel, linux-kernel, rudi

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed for i.MX8MQ
  2026-08-02 11:30 [PATCH 0/3] PCI: imx6: select the PCIe REF_CLK source on i.MX8MQ Rudi Heitbaum
  2026-08-02 11:31 ` [PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ Rudi Heitbaum
@ 2026-08-02 11:32 ` Rudi Heitbaum
  2026-08-02 11:49   ` sashiko-bot
  2026-08-02 11:32 ` [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock Rudi Heitbaum
  2 siblings, 1 reply; 7+ messages in thread
From: Rudi Heitbaum @ 2026-08-02 11:32 UTC (permalink / raw)
  To: Richard Zhu, Lucas Stach, Frank Li, Sascha Hauer
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	Krzysztof Kozlowski, Conor Dooley, Fabio Estevam, linux-pci,
	devicetree, imx, linux-arm-kernel, linux-kernel, rudi

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock
  2026-08-02 11:30 [PATCH 0/3] PCI: imx6: select the PCIe REF_CLK source on i.MX8MQ Rudi Heitbaum
  2026-08-02 11:31 ` [PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ Rudi Heitbaum
  2026-08-02 11:32 ` [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed " Rudi Heitbaum
@ 2026-08-02 11:32 ` Rudi Heitbaum
  2026-08-02 11:49   ` sashiko-bot
  2 siblings, 1 reply; 7+ messages in thread
From: Rudi Heitbaum @ 2026-08-02 11:32 UTC (permalink / raw)
  To: Richard Zhu, Lucas Stach, Frank Li, Sascha Hauer
  Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
	Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas,
	Krzysztof Kozlowski, Conor Dooley, Fabio Estevam, linux-pci,
	devicetree, imx, linux-arm-kernel, linux-kernel, rudi

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ
  2026-08-02 11:31 ` [PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ Rudi Heitbaum
@ 2026-08-02 11:40   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-02 11:40 UTC (permalink / raw)
  To: Rudi Heitbaum; +Cc: linux-pci, robh, Frank.Li, imx, devicetree, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The optional 'extref' clock was added to the Root Complex binding for i.MX8MQ but omitted from the corresponding Endpoint mode binding, resulting in schema validation failure if the endpoint uses the clock.
--

--- Patch [1]: [PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ ---
commit 4169458ec9055f45e1c7d18a9eea850e75cf9b6a
Author: Rudi Heitbaum <rudi@heitbaum.com>

dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ

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

> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> index e8b8131f5f23b..8fb0c919ae572 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml

[ ... ]

> @@ -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

[Severity: Medium]
Will this same clock be used when the hardware is configured in endpoint mode?

Currently, this adds the optional extref clock to the root complex binding for
fsl,imx8mq-pcie, expanding the maximum items to 5. However, the corresponding
endpoint schema in fsl,imx6q-pcie-ep.yaml for fsl,imx8mq-pcie-ep still
restricts clocks to exactly 4 items:

Documentation/devicetree/bindings/pci/fsl,imx6q-pcie-ep.yaml:
  - if:
      properties:
        compatible:
          enum:
            - fsl,imx8mq-pcie-ep
            - fsl,imx95-pcie-ep
    then:
      properties:
        clocks:
          minItems: 4
          maxItems: 4

If an endpoint device tree describes the hardware using this 5th extref clock,
will it fail schema validation because the endpoint binding was not also
updated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785669457.git.rudi@heitbaum.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed for i.MX8MQ
  2026-08-02 11:32 ` [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed " Rudi Heitbaum
@ 2026-08-02 11:49   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-02 11:49 UTC (permalink / raw)
  To: Rudi Heitbaum; +Cc: conor+dt, devicetree, Frank.Li, robh, linux-pci, imx

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch breaks backward compatibility with existing i.MX8MQ device trees by changing the default reference clock source, causing PCIe initialization to fail.
--

--- Patch [2]: [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed for i.MX8MQ ---
commit 6f37370c304d3692e11e97087dc50e68e2389b9e
Author: Rudi Heitbaum <rudi@heitbaum.com>

PCI: imx6: Use the external clock as REF_CLK when needed for i.MX8MQ

This commit updates the i.MX8MQ PCIe PHY initialization to conditionally
select the external oscillator as the reference clock based on the
enable_ext_refclk flag. It falls back to the internal PLL if an "extref"
clock is not supplied.

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 98e1db751132a..9e3b64fecb451 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);

[Severity: High]
Will this change break backward compatibility with existing i.MX8MQ device
trees?

Looking at imx8mq_pcie_init_phy() in drivers/pci/controller/dwc/pci-imx6.c,
if a legacy device tree does not provide a clock named "extref",
imx_pcie->enable_ext_refclk evaluates to false.

This causes the driver to clear IMX8MQ_GPR_PCIE_REF_USE_PAD and fall back to
the internal PLL. On boards that previously relied on the unconditional
external oscillator selection, this regression will cause PCIe initialization
to fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785669457.git.rudi@heitbaum.com?part=2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock
  2026-08-02 11:32 ` [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock Rudi Heitbaum
@ 2026-08-02 11:49   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-02 11:49 UTC (permalink / raw)
  To: Rudi Heitbaum; +Cc: robh, linux-pci, devicetree, conor+dt, Frank.Li, imx

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The requirement for a new 'extref' clock name breaks PCIe on older device trees for i.MX8MQ boards that rely on an external oscillator.
- [High] PCIe endpoint nodes on boards with external oscillators are intentionally omitted from the 'extref' clock update, which will misconfigure the PHY to use the internal PLL and break the link when endpoint mode is enabled.
--

--- Patch [3]: [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock ---
commit ad57d8a5f5ec29362d83c5dccc786006a9f4c2cb
Author: Rudi Heitbaum <rudi@heitbaum.com>

arm64: dts: imx8mq: Declare the PCIe extref clock

This commit updates i.MX8MQ board device trees to explicitly define the
"extref" clock for PCIe nodes, keeping "pcie_bus" entries intact. The
change aligns the device trees with a driver update that relies on the
"extref" clock name to configure the external oscillator.

> arm64: dts: imx8mq: Declare the PCIe extref clock
>     
> 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.

[Severity: Critical]
Does this break backward compatibility for older device trees?

If a user upgrades to a kernel with the new driver behavior but boots with an
older DTB that lacks the newly introduced "extref" clock name, will the driver
in drivers/pci/controller/dwc/pci-imx6.c:imx8mq_pcie_init_phy() clear the
IMX8MQ_GPR_PCIE_REF_USE_PAD bit because "extref" is absent?

Device trees represent a stable ABI, and older deployed DTBs might lose PCIe
functionality if they rely on the external oscillator and this bit is cleared.

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

[Severity: High]
Since endpoint mode initialization also goes through the same PHY setup
routine, will omitting the "extref" clock from endpoint nodes misconfigure
the PHY for endpoint operations?

If a user enables the pcie0_ep or pcie1_ep node on an affected board like the
EVK, the driver will not find the "extref" clock and imx8mq_pcie_init_phy()
will incorrectly clear IMX8MQ_GPR_PCIE_REF_USE_PAD.

Does this mean the PHY will be misconfigured to use the internal PLL instead
of the required external oscillator, breaking the link when endpoint mode is
enabled?

> Signed-off-by: Rudi Heitbaum <rudi@heitbaum.com>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785669457.git.rudi@heitbaum.com?part=3

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-02 11:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-02 11:30 [PATCH 0/3] PCI: imx6: select the PCIe REF_CLK source on i.MX8MQ Rudi Heitbaum
2026-08-02 11:31 ` [PATCH 1/3] dt-bindings: pci: fsl,imx6q-pcie: Add extref clock for i.MX8MQ Rudi Heitbaum
2026-08-02 11:40   ` sashiko-bot
2026-08-02 11:32 ` [PATCH 2/3] PCI: imx6: Use the external clock as REF_CLK when needed " Rudi Heitbaum
2026-08-02 11:49   ` sashiko-bot
2026-08-02 11:32 ` [PATCH 3/3] arm64: dts: imx8mq: Declare the PCIe extref clock Rudi Heitbaum
2026-08-02 11:49   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox