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

[Frank Li <Frank.li@nxp.com>]

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
>