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: <20251024-unburned-lip-6f142d83ed76@spud>
Date: Fri, 24 Oct 2025 18:06:43 +0100
From: Conor Dooley <conor@...nel.org>
To: Richard Zhu <hongxing.zhu@....com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	bhelgaas@...gle.com, 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.

Is it supposed to be possible to have "ref" and "extref"?
Or "extref" without "ref"?
Neither "ref" or "extref"?
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.

I don't see how this can have been successfully tested.

pw-bot: changes-requested


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ