[PATCH v4 0/3] PCI: imx6: Add external reference clock mode support

[PATCH v4 0/3] PCI: imx6: Add external reference clock mode support

[Richard Zhu]

On i.MX, the PCIe reference clock might come from either internal system
PLL or external clock source. Add the external reference clock source
for reference clock.

Main change in v4:
- Add one more reference clock "extref" to be onhalf the reference clock
  that comes from external crystal oscillator.

Main change in v3:
- Update the logic check external reference clock mode is enabled or
  not in the driver codes.

Main change in v2:
- Fix yamllint warning.
- Refine the driver codes.

[PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock
[PATCH v4 2/3] dt-binding: pci-imx6: Add external reference clock
[PATCH v4 3/3] PCI: imx6: Add external reference clock mode support

Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml      |  7 ++++++-
Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml |  6 ++++++
drivers/pci/controller/dwc/pci-imx6.c                          | 20 +++++++++++++-------
3 files changed, 25 insertions(+), 8 deletions(-)

[PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Richard Zhu]

Add one more reference clock "extref" to be onhalf the reference clock
that comes from external crystal oscillator.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
index 34594972d8db..ee09e0d3bbab 100644
--- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
@@ -105,6 +105,12 @@ properties:
             define it with this name (for instance pipe, core and aux can
             be connected to a single source of the periodic signal).
           const: ref
+        - description:
+            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
+            inputs, one from internal PLL, the other from off chip crystal
+            oscillator. Use extref clock name to be onhalf of the reference
+            clock comes form external crystal oscillator.
+          const: extref
         - description:
             Clock for the PHY registers interface. Originally this is
             a PHY-viewport-based interface, but some platform may have
-- 
2.37.1

Re: [PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Frank Li]

On Thu, Jun 26, 2025 at 03:38:02PM +0800, Richard Zhu wrote:
> Add one more reference clock "extref" to be onhalf the reference clock
> that comes from external crystal oscillator.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> index 34594972d8db..ee09e0d3bbab 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -105,6 +105,12 @@ properties:
>              define it with this name (for instance pipe, core and aux can
>              be connected to a single source of the periodic signal).
>            const: ref
> +        - description:
> +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
> +            inputs, one from internal PLL, the other from off chip crystal
> +            oscillator. Use extref clock name to be onhalf of the reference
> +            clock comes form external crystal oscillator.

typo form, should be 'from'

Frank

> +          const: extref
>          - description:
>              Clock for the PHY registers interface. Originally this is
>              a PHY-viewport-based interface, but some platform may have
> --
> 2.37.1
>

Re: [PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Bjorn Helgaas]

On Thu, Jun 26, 2025 at 03:38:02PM +0800, Richard Zhu wrote:
> Add one more reference clock "extref" to be onhalf the reference clock
> that comes from external crystal oscillator.

s/to be onhalf the reference clock/for a reference clock/

> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> index 34594972d8db..ee09e0d3bbab 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -105,6 +105,12 @@ properties:
>              define it with this name (for instance pipe, core and aux can
>              be connected to a single source of the periodic signal).
>            const: ref
> +        - description:
> +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
> +            inputs, one from internal PLL, the other from off chip crystal
> +            oscillator. Use extref clock name to be onhalf of the reference
> +            clock comes form external crystal oscillator.

Maybe:

  Some dwc wrappers (like i.MX95 PCIes) have two reference clock
  inputs, one from an internal PLL, the other from an off-chip crystal
  oscillator. If present, 'extref' refers to a reference clock from
  an external oscillator.

> +          const: extref
>          - description:
>              Clock for the PHY registers interface. Originally this is
>              a PHY-viewport-based interface, but some platform may have
> -- 
> 2.37.1
> 

Re: [PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Krzysztof Kozlowski]

On Thu, Jun 26, 2025 at 03:38:02PM +0800, Richard Zhu wrote:
> Add one more reference clock "extref" to be onhalf the reference clock
> that comes from external crystal oscillator.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> index 34594972d8db..ee09e0d3bbab 100644
> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> @@ -105,6 +105,12 @@ properties:
>              define it with this name (for instance pipe, core and aux can
>              be connected to a single source of the periodic signal).
>            const: ref
> +        - description:
> +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
> +            inputs, one from internal PLL, the other from off chip crystal
> +            oscillator. Use extref clock name to be onhalf of the reference
> +            clock comes form external crystal oscillator.

How internal PLL can be represented as 'ref' clock? Internal means it is
not outside, so impossible to represent.

Where is the DTS so we can look at big picture?


Best regards,
Krzysztof

Re: [PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Frank Li]

On Fri, Jun 27, 2025 at 08:54:46AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Jun 26, 2025 at 03:38:02PM +0800, Richard Zhu wrote:
> > Add one more reference clock "extref" to be onhalf the reference clock
> > that comes from external crystal oscillator.
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > index 34594972d8db..ee09e0d3bbab 100644
> > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > @@ -105,6 +105,12 @@ properties:
> >              define it with this name (for instance pipe, core and aux can
> >              be connected to a single source of the periodic signal).
> >            const: ref
> > +        - description:
> > +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
> > +            inputs, one from internal PLL, the other from off chip crystal
> > +            oscillator. Use extref clock name to be onhalf of the reference
> > +            clock comes form external crystal oscillator.
>
> How internal PLL can be represented as 'ref' clock? Internal means it is
> not outside, so impossible to represent.

Internal means in side SoC, but outside PCIe controller.

>
> Where is the DTS so we can look at big picture?

imx94 pci's upstream is still on going, which quite similar with imx95.
Just board design choose external crystal.

pcie_ref_clk: clock-pcie-ref {
                compatible = "gpio-gate-clock";
                clocks = <&xtal25m>;
                #clock-cells = <0>;
                enable-gpios = <&pca9670_i2c3 7 GPIO_ACTIVE_LOW>;
};

&pcie0 {
        pinctrl-0 = <&pinctrl_pcie0>;
        pinctrl-names = "default";
        clocks = <&scmi_clk IMX94_CLK_HSIO>,
                 <&scmi_clk IMX94_CLK_HSIOPLL>,
                 <&scmi_clk IMX94_CLK_HSIOPLL_VCO>,
                 <&scmi_clk IMX94_CLK_HSIOPCIEAUX>,
                 <&pcie_ref_clk>;
        clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "ext-ref";
        reset-gpio = <&pcal6416_i2c3_u46 3 GPIO_ACTIVE_LOW>;
        vpcie-supply = <&reg_pcie0>;
        status = "okay";
};

Frank
>
>
> Best regards,
> Krzysztof
>

Re: [PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Krzysztof Kozlowski]

On Fri, Jun 27, 2025 at 04:09:49PM -0400, Frank Li wrote:
> On Fri, Jun 27, 2025 at 08:54:46AM +0200, Krzysztof Kozlowski wrote:
> > On Thu, Jun 26, 2025 at 03:38:02PM +0800, Richard Zhu wrote:
> > > Add one more reference clock "extref" to be onhalf the reference clock
> > > that comes from external crystal oscillator.
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > >  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > index 34594972d8db..ee09e0d3bbab 100644
> > > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > @@ -105,6 +105,12 @@ properties:
> > >              define it with this name (for instance pipe, core and aux can
> > >              be connected to a single source of the periodic signal).
> > >            const: ref
> > > +        - description:
> > > +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
> > > +            inputs, one from internal PLL, the other from off chip crystal
> > > +            oscillator. Use extref clock name to be onhalf of the reference
> > > +            clock comes form external crystal oscillator.
> >
> > How internal PLL can be represented as 'ref' clock? Internal means it is
> > not outside, so impossible to represent.
> 
> Internal means in side SoC, but outside PCIe controller.

So external... It does not matter for PCIe controller whether clock is
coming from SoC or from some crystal.  It is still input pin. Same input
pin.

> 
> >
> > Where is the DTS so we can look at big picture?
> 
> imx94 pci's upstream is still on going, which quite similar with imx95.
> Just board design choose external crystal.
> 
> pcie_ref_clk: clock-pcie-ref {
>                 compatible = "gpio-gate-clock";
>                 clocks = <&xtal25m>;
>                 #clock-cells = <0>;
>                 enable-gpios = <&pca9670_i2c3 7 GPIO_ACTIVE_LOW>;
> };
> 
> &pcie0 {
>         pinctrl-0 = <&pinctrl_pcie0>;
>         pinctrl-names = "default";
>         clocks = <&scmi_clk IMX94_CLK_HSIO>,
>                  <&scmi_clk IMX94_CLK_HSIOPLL>,
>                  <&scmi_clk IMX94_CLK_HSIOPLL_VCO>,
>                  <&scmi_clk IMX94_CLK_HSIOPCIEAUX>,
>                  <&pcie_ref_clk>;
>         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "ext-ref";

So this is totally faked hardware property.

No, it is the same clock signal, not different. You write bindings from
this device point of view, not for your board.

Best regards,
Krzysztof

Re: [PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Frank Li]

On Sat, Jun 28, 2025 at 02:34:12PM +0200, Krzysztof Kozlowski wrote:
> On Fri, Jun 27, 2025 at 04:09:49PM -0400, Frank Li wrote:
> > On Fri, Jun 27, 2025 at 08:54:46AM +0200, Krzysztof Kozlowski wrote:
> > > On Thu, Jun 26, 2025 at 03:38:02PM +0800, Richard Zhu wrote:
> > > > Add one more reference clock "extref" to be onhalf the reference clock
> > > > that comes from external crystal oscillator.
> > > >
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > > index 34594972d8db..ee09e0d3bbab 100644
> > > > --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> > > > @@ -105,6 +105,12 @@ properties:
> > > >              define it with this name (for instance pipe, core and aux can
> > > >              be connected to a single source of the periodic signal).
> > > >            const: ref
> > > > +        - description:
> > > > +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
> > > > +            inputs, one from internal PLL, the other from off chip crystal
> > > > +            oscillator. Use extref clock name to be onhalf of the reference
> > > > +            clock comes form external crystal oscillator.
> > >
> > > How internal PLL can be represented as 'ref' clock? Internal means it is
> > > not outside, so impossible to represent.
> >
> > Internal means in side SoC, but outside PCIe controller.
>
> So external... It does not matter for PCIe controller whether clock is
> coming from SoC or from some crystal.  It is still input pin. Same input
> pin.

It is NOT the same pin. It is TWO pins, there are mux inside in PCI
controller.

There are similar cases in s32 rtc, there are 4 input source[0,1,2,3]
https://lore.kernel.org/imx/20241127144322.GA3454134-robh@kernel.org/
Only one provide.

>
> >
> > >
> > > Where is the DTS so we can look at big picture?
> >
> > imx94 pci's upstream is still on going, which quite similar with imx95.
> > Just board design choose external crystal.
> >
> > pcie_ref_clk: clock-pcie-ref {
> >                 compatible = "gpio-gate-clock";
> >                 clocks = <&xtal25m>;
> >                 #clock-cells = <0>;
> >                 enable-gpios = <&pca9670_i2c3 7 GPIO_ACTIVE_LOW>;
> > };
> >
> > &pcie0 {
> >         pinctrl-0 = <&pinctrl_pcie0>;
> >         pinctrl-names = "default";
> >         clocks = <&scmi_clk IMX94_CLK_HSIO>,
> >                  <&scmi_clk IMX94_CLK_HSIOPLL>,
> >                  <&scmi_clk IMX94_CLK_HSIOPLL_VCO>,
> >                  <&scmi_clk IMX94_CLK_HSIOPCIEAUX>,
> >                  <&pcie_ref_clk>;
> >         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "ext-ref";
>
> So this is totally faked hardware property.
>
> No, it is the same clock signal, not different. You write bindings from
> this device point of view, not for your board.

No the same clock signal. There are two sources, "ext-ref" or "ref".
PCI controller need know which one provide clocks.

There are mux inside PCI controller, DT need provide information which on
provide.

Maybe my example dts miss-lead you. Altherate descript is
  clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "ref", "ext-ref";

  But we thinks if ext-ref provide, "ref" is not neccesary need be turn on.
  So remove it from the list.

Any suggestion?

Frank

>
> Best regards,
> Krzysztof
>

Re: [PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Krzysztof Kozlowski]

On 28/06/2025 17:33, Frank Li wrote:
> On Sat, Jun 28, 2025 at 02:34:12PM +0200, Krzysztof Kozlowski wrote:
>> On Fri, Jun 27, 2025 at 04:09:49PM -0400, Frank Li wrote:
>>> On Fri, Jun 27, 2025 at 08:54:46AM +0200, Krzysztof Kozlowski wrote:
>>>> On Thu, Jun 26, 2025 at 03:38:02PM +0800, Richard Zhu wrote:
>>>>> Add one more reference clock "extref" to be onhalf the reference clock
>>>>> that comes from external crystal oscillator.
>>>>>
>>>>> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
>>>>> ---
>>>>>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
>>>>> index 34594972d8db..ee09e0d3bbab 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
>>>>> @@ -105,6 +105,12 @@ properties:
>>>>>              define it with this name (for instance pipe, core and aux can
>>>>>              be connected to a single source of the periodic signal).
>>>>>            const: ref
>>>>> +        - description:
>>>>> +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
>>>>> +            inputs, one from internal PLL, the other from off chip crystal
>>>>> +            oscillator. Use extref clock name to be onhalf of the reference
>>>>> +            clock comes form external crystal oscillator.
>>>>
>>>> How internal PLL can be represented as 'ref' clock? Internal means it is
>>>> not outside, so impossible to represent.
>>>
>>> Internal means in side SoC, but outside PCIe controller.
>>
>> So external... It does not matter for PCIe controller whether clock is
>> coming from SoC or from some crystal.  It is still input pin. Same input
>> pin.
> 
> It is NOT the same pin. It is TWO pins, there are mux inside in PCI
> controller.
> 
> There are similar cases in s32 rtc, there are 4 input source[0,1,2,3]
> https://lore.kernel.org/imx/20241127144322.GA3454134-robh@kernel.org/
> Only one provide.
> 
>>
>>>
>>>>
>>>> Where is the DTS so we can look at big picture?
>>>
>>> imx94 pci's upstream is still on going, which quite similar with imx95.
>>> Just board design choose external crystal.
>>>
>>> pcie_ref_clk: clock-pcie-ref {
>>>                 compatible = "gpio-gate-clock";
>>>                 clocks = <&xtal25m>;
>>>                 #clock-cells = <0>;
>>>                 enable-gpios = <&pca9670_i2c3 7 GPIO_ACTIVE_LOW>;
>>> };
>>>
>>> &pcie0 {
>>>         pinctrl-0 = <&pinctrl_pcie0>;
>>>         pinctrl-names = "default";
>>>         clocks = <&scmi_clk IMX94_CLK_HSIO>,
>>>                  <&scmi_clk IMX94_CLK_HSIOPLL>,
>>>                  <&scmi_clk IMX94_CLK_HSIOPLL_VCO>,
>>>                  <&scmi_clk IMX94_CLK_HSIOPCIEAUX>,
>>>                  <&pcie_ref_clk>;
>>>         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "ext-ref";
>>
>> So this is totally faked hardware property.
>>
>> No, it is the same clock signal, not different. You write bindings from
>> this device point of view, not for your board.
> 
> No the same clock signal. There are two sources, "ext-ref" or "ref".
> PCI controller need know which one provide clocks.

OK, this should be clearly expressed not some vague play of the words
what is internal and external...

> 
> There are mux inside PCI controller, DT need provide information which on
> provide.
> 
> Maybe my example dts miss-lead you. Altherate descript is
>   clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "ref", "ext-ref";
> 
>   But we thinks if ext-ref provide, "ref" is not neccesary need be turn on.
>   So remove it from the list.

If the ref clock is actually wired it should be there. You describe here
hardware, not what is necessary.

Best regards,
Krzysztof

Re: [PATCH v4 1/3] dt-bindings: PCI: dwc: Add one more reference clock

[Frank Li]

On Mon, Jun 30, 2025 at 10:25:43AM +0200, Krzysztof Kozlowski wrote:
> On 28/06/2025 17:33, Frank Li wrote:
> > On Sat, Jun 28, 2025 at 02:34:12PM +0200, Krzysztof Kozlowski wrote:
> >> On Fri, Jun 27, 2025 at 04:09:49PM -0400, Frank Li wrote:
> >>> On Fri, Jun 27, 2025 at 08:54:46AM +0200, Krzysztof Kozlowski wrote:
> >>>> On Thu, Jun 26, 2025 at 03:38:02PM +0800, Richard Zhu wrote:
> >>>>> Add one more reference clock "extref" to be onhalf the reference clock
> >>>>> that comes from external crystal oscillator.
> >>>>>
> >>>>> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> >>>>> ---
> >>>>>  .../devicetree/bindings/pci/snps,dw-pcie-common.yaml        | 6 ++++++
> >>>>>  1 file changed, 6 insertions(+)
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> >>>>> index 34594972d8db..ee09e0d3bbab 100644
> >>>>> --- a/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> >>>>> +++ b/Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
> >>>>> @@ -105,6 +105,12 @@ properties:
> >>>>>              define it with this name (for instance pipe, core and aux can
> >>>>>              be connected to a single source of the periodic signal).
> >>>>>            const: ref
> >>>>> +        - description:
> >>>>> +            Some dwc wrappers (like i.MX95 PCIes) have two reference clock
> >>>>> +            inputs, one from internal PLL, the other from off chip crystal
> >>>>> +            oscillator. Use extref clock name to be onhalf of the reference
> >>>>> +            clock comes form external crystal oscillator.
> >>>>
> >>>> How internal PLL can be represented as 'ref' clock? Internal means it is
> >>>> not outside, so impossible to represent.
> >>>
> >>> Internal means in side SoC, but outside PCIe controller.
> >>
> >> So external... It does not matter for PCIe controller whether clock is
> >> coming from SoC or from some crystal.  It is still input pin. Same input
> >> pin.
> >
> > It is NOT the same pin. It is TWO pins, there are mux inside in PCI
> > controller.
> >
> > There are similar cases in s32 rtc, there are 4 input source[0,1,2,3]
> > https://lore.kernel.org/imx/20241127144322.GA3454134-robh@kernel.org/
> > Only one provide.
> >
> >>
> >>>
> >>>>
> >>>> Where is the DTS so we can look at big picture?
> >>>
> >>> imx94 pci's upstream is still on going, which quite similar with imx95.
> >>> Just board design choose external crystal.
> >>>
> >>> pcie_ref_clk: clock-pcie-ref {
> >>>                 compatible = "gpio-gate-clock";
> >>>                 clocks = <&xtal25m>;
> >>>                 #clock-cells = <0>;
> >>>                 enable-gpios = <&pca9670_i2c3 7 GPIO_ACTIVE_LOW>;
> >>> };
> >>>
> >>> &pcie0 {
> >>>         pinctrl-0 = <&pinctrl_pcie0>;
> >>>         pinctrl-names = "default";
> >>>         clocks = <&scmi_clk IMX94_CLK_HSIO>,
> >>>                  <&scmi_clk IMX94_CLK_HSIOPLL>,
> >>>                  <&scmi_clk IMX94_CLK_HSIOPLL_VCO>,
> >>>                  <&scmi_clk IMX94_CLK_HSIOPCIEAUX>,
> >>>                  <&pcie_ref_clk>;
> >>>         clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "ext-ref";
> >>
> >> So this is totally faked hardware property.
> >>
> >> No, it is the same clock signal, not different. You write bindings from
> >> this device point of view, not for your board.
> >
> > No the same clock signal. There are two sources, "ext-ref" or "ref".
> > PCI controller need know which one provide clocks.
>
> OK, this should be clearly expressed not some vague play of the words
> what is internal and external...

Does bjorn's suggest description is clear enough?

  "Some dwc wrappers (like i.MX95 PCIes) have two reference clock
  inputs, one from an internal PLL, the other from an off-chip crystal
  oscillator. If present, 'extref' refers to a reference clock from
  an external oscillator."

Frank
>
> >
> > There are mux inside PCI controller, DT need provide information which on
> > provide.
> >
> > Maybe my example dts miss-lead you. Altherate descript is
> >   clock-names = "pcie", "pcie_bus", "pcie_phy", "pcie_aux", "ref", "ext-ref";
> >
> >   But we thinks if ext-ref provide, "ref" is not neccesary need be turn on.
> >   So remove it from the list.
>
> If the ref clock is actually wired it should be there. You describe here
> hardware, not what is necessary.
>
> Best regards,
> Krzysztof

[PATCH v4 2/3] dt-binding: pci-imx6: Add external reference clock mode support

[Richard Zhu]

On i.MX, the PCIe reference clock might come from either internal
system PLL or external clock source.
Add the external reference clock source for reference clock.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
index ca5f2970f217..a45876aba4da 100644
--- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
@@ -219,7 +219,12 @@ allOf:
             - const: pcie_bus
             - const: pcie_phy
             - const: pcie_aux
-            - const: ref
+            - description: PCIe reference clock.
+              oneOf:
+                - description: The controller might be configured clocking
+                    coming in from either an internal system PLL or an
+                    external clock source.
+                  enum: [ref, extref]
 
 unevaluatedProperties: false
 
-- 
2.37.1

Re: [PATCH v4 2/3] dt-binding: pci-imx6: Add external reference clock mode support

[Frank Li]

On Thu, Jun 26, 2025 at 03:38:03PM +0800, Richard Zhu wrote:
> On i.MX, the PCIe reference clock might come from either internal
> system PLL or external clock source.

On i.MX, PCIe has two reference clock inputs: one from the internal PLL and
one from an external clock source. Only one needs to be used, depending on
the board design.

> Add the external reference clock source for reference clock.
					  for external reference clock.

subject missed s, should be dt-bindings.

>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> index ca5f2970f217..a45876aba4da 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> @@ -219,7 +219,12 @@ allOf:
>              - const: pcie_bus
>              - const: pcie_phy
>              - const: pcie_aux
> -            - const: ref
> +            - description: PCIe reference clock.
> +              oneOf:
> +                - description: The controller might be configured clocking
> +                    coming in from either an internal system PLL or an
> +                    external clock source.

description:
  The controller have two reference clock inputs: internal system PLL and
external clock cource. Only one need be used.

> +                  enum: [ref, extref]
>
>  unevaluatedProperties: false
>
> --
> 2.37.1
>

Re: [PATCH v4 2/3] dt-binding: pci-imx6: Add external reference clock mode support

[Bjorn Helgaas]

On Thu, Jun 26, 2025 at 03:38:03PM +0800, Richard Zhu wrote:
> On i.MX, the PCIe reference clock might come from either internal
> system PLL or external clock source.
> Add the external reference clock source for reference clock.

Add blank line between paragraphs or reflow into a single paragraph.

Re: [PATCH v4 2/3] dt-binding: pci-imx6: Add external reference clock mode support

[Krzysztof Kozlowski]

On Thu, Jun 26, 2025 at 03:38:03PM +0800, Richard Zhu wrote:
> On i.MX, the PCIe reference clock might come from either internal
> system PLL or external clock source.
> Add the external reference clock source for reference clock.
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> index ca5f2970f217..a45876aba4da 100644
> --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> @@ -219,7 +219,12 @@ allOf:
>              - const: pcie_bus
>              - const: pcie_phy
>              - const: pcie_aux
> -            - const: ref
> +            - description: PCIe reference clock.
> +              oneOf:
> +                - description: The controller might be configured clocking
> +                    coming in from either an internal system PLL or an
> +                    external clock source.
> +                  enum: [ref, extref]

NAK

As explained in other thread this is the same input and you just
call it differently.

Best regards,
Krzysztof

[PATCH v4 3/3] PCI: imx6: Add external reference clock mode support

[Richard Zhu]

The PCI Express reference clock of i.MX9 PCIes might come from external
clock source. Add the external reference clock mode support.

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 5a38cfaf989b..9309959874c0 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -149,6 +149,7 @@ struct imx_pcie {
 	struct gpio_desc	*reset_gpiod;
 	struct clk_bulk_data	*clks;
 	int			num_clks;
+	bool			enable_ext_refclk;
 	struct regmap		*iomuxc_gpr;
 	u16			msi_ctrl;
 	u32			controller_id;
@@ -241,6 +242,8 @@ static unsigned int imx_pcie_grp_offset(const struct imx_pcie *imx_pcie)
 
 static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
 {
+	bool ext = imx_pcie->enable_ext_refclk;
+
 	/*
 	 * ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
 	 * Through Beacon or PERST# De-assertion
@@ -259,13 +262,12 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
 			IMX95_PCIE_PHY_CR_PARA_SEL,
 			IMX95_PCIE_PHY_CR_PARA_SEL);
 
-	regmap_update_bits(imx_pcie->iomuxc_gpr,
-			   IMX95_PCIE_PHY_GEN_CTRL,
-			   IMX95_PCIE_REF_USE_PAD, 0);
-	regmap_update_bits(imx_pcie->iomuxc_gpr,
-			   IMX95_PCIE_SS_RW_REG_0,
+	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_PHY_GEN_CTRL,
+			   ext ? IMX95_PCIE_REF_USE_PAD : 0,
+			   IMX95_PCIE_REF_USE_PAD);
+	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_0,
 			   IMX95_PCIE_REF_CLKEN,
-			   IMX95_PCIE_REF_CLKEN);
+			   ext ? 0 : IMX95_PCIE_REF_CLKEN);
 
 	return 0;
 }
@@ -1600,7 +1602,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	struct imx_pcie *imx_pcie;
 	struct device_node *np;
 	struct device_node *node = dev->of_node;
-	int ret, domain;
+	int i, ret, domain;
 	u16 val;
 
 	imx_pcie = devm_kzalloc(dev, sizeof(*imx_pcie), GFP_KERNEL);
@@ -1651,6 +1653,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
 	if (imx_pcie->num_clks < 0)
 		return dev_err_probe(dev, imx_pcie->num_clks,
 				     "failed to get clocks\n");
+	imx_pcie->enable_ext_refclk = true;
+	for (i = 0; i < imx_pcie->num_clks; i++)
+		if (strncmp(imx_pcie->clks[i].id, "ref", 3) == 0)
+			imx_pcie->enable_ext_refclk = false;
 
 	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_PHYDRV)) {
 		imx_pcie->phy = devm_phy_get(dev, "pcie-phy");
-- 
2.37.1

Re: [PATCH v4 3/3] PCI: imx6: Add external reference clock mode support

[Frank Li]

On Thu, Jun 26, 2025 at 03:38:04PM +0800, Richard Zhu wrote:
> The PCI Express reference clock of i.MX9 PCIes might come from external
> clock source. Add the external reference clock mode support.
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/pci/controller/dwc/pci-imx6.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 5a38cfaf989b..9309959874c0 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -149,6 +149,7 @@ struct imx_pcie {
>  	struct gpio_desc	*reset_gpiod;
>  	struct clk_bulk_data	*clks;
>  	int			num_clks;
> +	bool			enable_ext_refclk;
>  	struct regmap		*iomuxc_gpr;
>  	u16			msi_ctrl;
>  	u32			controller_id;
> @@ -241,6 +242,8 @@ static unsigned int imx_pcie_grp_offset(const struct imx_pcie *imx_pcie)
>
>  static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
>  {
> +	bool ext = imx_pcie->enable_ext_refclk;
> +
>  	/*
>  	 * ERR051624: The Controller Without Vaux Cannot Exit L23 Ready
>  	 * Through Beacon or PERST# De-assertion
> @@ -259,13 +262,12 @@ static int imx95_pcie_init_phy(struct imx_pcie *imx_pcie)
>  			IMX95_PCIE_PHY_CR_PARA_SEL,
>  			IMX95_PCIE_PHY_CR_PARA_SEL);
>
> -	regmap_update_bits(imx_pcie->iomuxc_gpr,
> -			   IMX95_PCIE_PHY_GEN_CTRL,
> -			   IMX95_PCIE_REF_USE_PAD, 0);
> -	regmap_update_bits(imx_pcie->iomuxc_gpr,
> -			   IMX95_PCIE_SS_RW_REG_0,
> +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_PHY_GEN_CTRL,
> +			   ext ? IMX95_PCIE_REF_USE_PAD : 0,
> +			   IMX95_PCIE_REF_USE_PAD);
> +	regmap_update_bits(imx_pcie->iomuxc_gpr, IMX95_PCIE_SS_RW_REG_0,
>  			   IMX95_PCIE_REF_CLKEN,
> -			   IMX95_PCIE_REF_CLKEN);
> +			   ext ? 0 : IMX95_PCIE_REF_CLKEN);
>
>  	return 0;
>  }
> @@ -1600,7 +1602,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	struct imx_pcie *imx_pcie;
>  	struct device_node *np;
>  	struct device_node *node = dev->of_node;
> -	int ret, domain;
> +	int i, ret, domain;
>  	u16 val;
>
>  	imx_pcie = devm_kzalloc(dev, sizeof(*imx_pcie), GFP_KERNEL);
> @@ -1651,6 +1653,10 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  	if (imx_pcie->num_clks < 0)
>  		return dev_err_probe(dev, imx_pcie->num_clks,
>  				     "failed to get clocks\n");
> +	imx_pcie->enable_ext_refclk = true;
> +	for (i = 0; i < imx_pcie->num_clks; i++)
> +		if (strncmp(imx_pcie->clks[i].id, "ref", 3) == 0)
> +			imx_pcie->enable_ext_refclk = false;
>
>  	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_HAS_PHYDRV)) {
>  		imx_pcie->phy = devm_phy_get(dev, "pcie-phy");
> --
> 2.37.1
>