linux-kernel - Re: [PATCH v2 3/3] dt-bindings: phy: rockchip,pcie3-phy: add rockchip,phy-ref-use-pad

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAEEuhpHmW9hji91tbQ3MMhBZELCYc_vA+Ra3oC2W+Rf8LLC6w@mail.gmail.com>
Date: Thu, 7 Aug 2025 10:47:18 +0200
From: Rick Wertenbroek <rick.wertenbroek@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: rick.wertenbroek@...g-vd.ch, dlemoal@...nel.org, 
	alberto.dassatti@...g-vd.ch, Vinod Koul <vkoul@...nel.org>, 
	Kishon Vijay Abraham I <kishon@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Heiko Stuebner <heiko@...ech.de>, linux-phy@...ts.infradead.org, devicetree@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] dt-bindings: phy: rockchip,pcie3-phy: add rockchip,phy-ref-use-pad

On Thu, Aug 7, 2025 at 9:55 AM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 07/08/2025 09:54, Krzysztof Kozlowski wrote:
> > On Wed, Aug 06, 2025 at 03:38:23PM +0200, Rick Wertenbroek wrote:
> >> >From the RK3588 Technical Reference Manual, Part1,
> >> section 6.19 PCIe3PHY_GRF Register Description: "ref_use_pad"
> >>
> >> "Select reference clock connected to ref_pad_clk_p/ref_pad_clk_m.
> >> Selects the external ref_pad_clk_p and ref_pad_clk_m inputs as the
> >> reference clock source when asserted. When de-asserted, ref_alt_clk_p
> >> and ref_alt_clk_m are the sources of the reference clock."
> >>
> >> The hardware reset value for this field is 0x1 (enabled).
> >> Note that this register field is only available on RK3588, not on RK3568.
> >
> > Then you miss restricting it (:false) in one of if:then: blocks.
> >
> > Also, binding cannot be after the user. You need to reorder patches.
> >
> > ...
> >
> >>
> >> +  rockchip,phy-ref-use-pad:
> >> +    description: which PHY should use the external pad as PCIe reference clock.
> >> +      1 means use pad (default), 0 means use internal clock (PLL_PPLL).
> >
> > Can't you deduce it from the presence of clock inputs? IOW, if the
> > clocks are physically connected, is it even reasonable or possible to
> > use internal clock?

Thank you Krzysztof,
But no, if there is no clock, the driver deadlocks, it needs a clock
to probe correctly.

When there is a clock physically connected on the pad, you can still
choose to use it or use the internal clock, this is no problem.
The problem is when you have no clock on the pad (as it defaults to
using the pad) and the loading the driver deadlocks the system...

> >
> >> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >> +    minItems: 2
> >> +    maxItems: 2
> >> +    items:
> >> +      minimum: 0
> >> +      maximum: 1
> >
> > More precise:
> >
> > items:
> >   - description: PHY0 reference clock config
> >   - description: PHY1 reference clock config
> >   enum: [ 0, 1 ]
>
> Eh, no, rather if this stays as int:
>
>     items:
>       - description: PHY0 reference clock config
>         enum: [ 0, 1 ]
>       - description: PHY1 reference clock config
>         enum: [ 0, 1 ]
>     default: [ 1, 1 ]
>
>
> > default: [ 1, 1 ]
> >
> > Anyway, default 1, 1 is pretty non-obvious, so this should be just
> > non-unique-string-array (and property should be like
> > rockchip,phy-ref-clk-source/sel).
> >
> >
> > Best regards,
> > Krzysztof
> >
>
>
> Best regards,
> Krzysztof

I based my patch on patch :
46492d10067660785a09db4ce9244545126a17b8
dt-bindings: phy: rockchip,pcie3-phy: add rockchip,rx-common-refclk-mode

As the option I add is extremely similar, it sets a feature in one of
the PHY registers and only applies to RK3588.
That is why I used the same style as rockchip,rx-common-refclk-mode.

Wouldn't it be confusing or at least incoherent to use enum for
rockchip,phy-ref-use-pad but not for rx-common-refclk-mode ?

As for the name, I used phy-ref-use-pad as it is the name used in the
RK3588 technical reference manual.
(Similarly done on rx-common-refclk-mode).

Best regards,
Rick