linux-kernel - RE: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference clock input

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
 <AS8PR04MB88339961A776546B78E7267A8CFDA@AS8PR04MB8833.eurprd04.prod.outlook.com>
Date: Tue, 28 Oct 2025 05:56:51 +0000
From: Hongxing Zhu <hongxing.zhu@....com>
To: Conor Dooley <conor@...nel.org>
CC: "robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
	"bhelgaas@...gle.com" <bhelgaas@...gle.com>, Frank Li <frank.li@....com>,
	"l.stach@...gutronix.de" <l.stach@...gutronix.de>, "lpieralisi@...nel.org"
	<lpieralisi@...nel.org>, "kwilczynski@...nel.org" <kwilczynski@...nel.org>,
	"mani@...nel.org" <mani@...nel.org>, "shawnguo@...nel.org"
	<shawnguo@...nel.org>, "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
	"kernel@...gutronix.de" <kernel@...gutronix.de>, "festevam@...il.com"
	<festevam@...il.com>, "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference
 clock input

> -----Original Message-----
> From: Conor Dooley <conor@...nel.org>
> Sent: 2025年10月28日 1:06
> To: Hongxing Zhu <hongxing.zhu@....com>
> Cc: robh@...nel.org; krzk+dt@...nel.org; conor+dt@...nel.org;
> bhelgaas@...gle.com; Frank Li <frank.li@....com>; l.stach@...gutronix.de;
> lpieralisi@...nel.org; kwilczynski@...nel.org; mani@...nel.org;
> shawnguo@...nel.org; s.hauer@...gutronix.de; kernel@...gutronix.de;
> festevam@...il.com; linux-pci@...r.kernel.org;
> linux-arm-kernel@...ts.infradead.org; devicetree@...r.kernel.org;
> imx@...ts.linux.dev; linux-kernel@...r.kernel.org
> Subject: Re: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external reference
> clock input
> 
> On Mon, Oct 27, 2025 at 06:36:32AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Conor Dooley <conor@...nel.org>
> > > Sent: 2025年10月25日 1:07
> > > To: Hongxing Zhu <hongxing.zhu@....com>
> > > Cc: robh@...nel.org; krzk+dt@...nel.org; conor+dt@...nel.org;
> > > bhelgaas@...gle.com; Frank Li <frank.li@....com>;
> > > l.stach@...gutronix.de; lpieralisi@...nel.org;
> > > kwilczynski@...nel.org; mani@...nel.org; shawnguo@...nel.org;
> > > s.hauer@...gutronix.de; kernel@...gutronix.de; festevam@...il.com;
> > > linux-pci@...r.kernel.org; linux-arm-kernel@...ts.infradead.org;
> > > devicetree@...r.kernel.org; imx@...ts.linux.dev;
> > > linux-kernel@...r.kernel.org
> > > Subject: Re: [PATCH v8 2/3] dt-bindings: PCI: pci-imx6: Add external
> > > reference clock input
> > >
> > > On Fri, Oct 24, 2025 at 10:40:12AM +0800, Richard Zhu wrote:
> > > > i.MX95 PCIes have two reference clock inputs: one from internal
> > > > PLL, the other from off chip crystal oscillator. The "extref"
> > > > clock refers to a reference clock from an external crystal oscillator.
> > > >
> > > > Add external reference clock input for i.MX95 PCIes.
> > > >
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@....com>
> > > > Reviewed-by: Frank Li <Frank.Li@....com>
> > > > ---
> > > >  Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > index ca5f2970f217c..b4c40d0573dce 100644
> > > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml
> > > > @@ -212,14 +212,17 @@ allOf:
> > > >      then:
> > > >        properties:
> > > >          clocks:
> > > > +          minItems: 4
> > > >            maxItems: 5
> > > >          clock-names:
> > > > +          minItems: 4
> > > >            items:
> > > >              - const: pcie
> > >
> > > 1
> > >
> > > >              - const: pcie_bus
> > >
> > > 2
> > >
> > > >              - const: pcie_phy
> > >
> > > 3
> > >
> > > >              - const: pcie_aux
> > >
> > > 4
> > >
> > > >              - const: ref
> > >
> > > 5
> > >
> > > > +            - const: extref  # Optional
> > >
> > > 6
> > >
> > > There are 6 clocks here, but clocks and clock-names in this binding
> > > do not permit 6:
> > > |  clocks:
> > > |    minItems: 3
> > > |    items:
> > > |      - description: PCIe bridge clock.
> > > |      - description: PCIe bus clock.
> > > |      - description: PCIe PHY clock.
> > > |      - description: Additional required clock entry for imx6sx-pcie,
> > > |           imx6sx-pcie-ep, imx8mq-pcie, imx8mq-pcie-ep.
> > > |      - description: PCIe reference clock.
> > > |
> > > |  clock-names:
> > > |    minItems: 3
> > > |    maxItems: 5
> > >
> > > AFAICT, what this patch actually did is make "ref" an optional
> > > clock, but the claim in the patch is that extref is optional. With
> > > this patch applied, you can have a) no reference clocks or b) only "ref".
> "extref"
> > > is never allowed.
> > Hi Conor:
> > Thanks for your review comments.
> > Just same to "extref", the "ref" should be marked as optional clock too.
> 
> Right, your commit message should then mention that.
> 
Hi Conor:
It's my mistake. My previous understanding is wrong.
Only "extref" is an optional clock.

> > > Is it supposed to be possible to have "ref" and "extref"?
> > > Or "extref" without "ref"?
> > > Neither "ref" or "extref"?
> > "ref" and "extref" have an exclusive relationship of choosing one of
> > the two,  and they cannot all exist simultaneously.
> 
> Right, please go test what you've produced, because extref is never
> permitted by this binding.
"ref" and "extref" doesn't have an exclusive relationship. There are two
 options. one is "ref", the other one is "ref, extref".

If "ref" present, the internal system PLL clock is used as PCIe reference
 clock source. If both of them present, the PCIe reference clock would come
 from an off-chip crystal oscillator.

In the end, the maxItem of clocks should be '6'. I would post the another
 patch-set after correct the maxItem of clocks from '5' to '6'.

Sorry again to my misleading.

Best Regards
Richard Zhu
> 
> > > I don't know the answer to that question because you're doing things
> > > that are contradictory in your patch and the commit message isn't clear.
> > >
> > Sorry for causing you confusion.