linux-kernel - Re: [PATCH 1/4] dt-bindings: clock: mediatek: Add clocks for MT8196 mfgpll

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: <3374975.aeNJFYEL58@workhorse>
Date: Tue, 30 Sep 2025 17:57:00 +0200
From: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
To: Conor Dooley <conor@...nel.org>
Cc: Michael Turquette <mturquette@...libre.com>,
 Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Matthias Brugger <matthias.bgg@...il.com>,
 AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
 Guangjie Song <guangjie.song@...iatek.com>,
 Laura Nao <laura.nao@...labora.com>,
 Nícolas F. R. A. Prado <nfraprado@...labora.com>,
 Yassine Oudjana <y.oudjana@...tonmail.com>, kernel@...labora.com,
 Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
 linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-mediatek@...ts.infradead.org
Subject:
 Re: [PATCH 1/4] dt-bindings: clock: mediatek: Add clocks for MT8196 mfgpll

On Monday, 29 September 2025 19:31:36 Central European Summer Time Conor Dooley wrote:
> On Mon, Sep 29, 2025 at 02:13:20PM +0200, Nicolas Frattaroli wrote:
> > The clock controllers for mfgpll, mfgpll-sc0, and mfgpll-sc1 all need
> > CLK_TOP_MFG_EB to be on if their clock control registers are touched in
> > any way.
> > 
> > This was not known at the time this binding was written, as this
> > dependency only came to light when I started poking at the MFlexGraphics
> > hardware, where this undocumented peculiarity made itself known through
> > SErrors being thrown during register reads.
> > 
> > Add a clocks property to the binding to describe this relationship, and
> > mark it as required for the affected clocks.
> > 
> > Fixes: dd240e95f1be ("dt-bindings: clock: mediatek: Describe MT8196 clock controllers")
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
> > ---
> >  .../bindings/clock/mediatek,mt8196-sys-clock.yaml  | 28 ++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/mediatek,mt8196-sys-clock.yaml b/Documentation/devicetree/bindings/clock/mediatek,mt8196-sys-clock.yaml
> > index 660ab64f390d2e722b7d3e25cf057926da318bc0..41aacd8d5f69050eebdf8392f7b652427632f491 100644
> > --- a/Documentation/devicetree/bindings/clock/mediatek,mt8196-sys-clock.yaml
> > +++ b/Documentation/devicetree/bindings/clock/mediatek,mt8196-sys-clock.yaml
> > @@ -45,6 +45,9 @@ properties:
> >    reg:
> >      maxItems: 1
> >  
> > +  clocks:
> > +    maxItems: 1
> > +
> >    '#clock-cells':
> >      const: 1
> >  
> > @@ -90,6 +93,23 @@ required:
> >  
> >  additionalProperties: false
> >  
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - mediatek,mt8196-mfgpll-pll-ctrl
> > +              - mediatek,mt8196-mfgpll-sc0-pll-ctrl
> > +              - mediatek,mt8196-mfgpll-sc1-pll-ctrl
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: mfg_eb clock
> > +      required:
> > +        - clocks
> 
> Don't you want an else: properties: clocks: false here?

Possibly. I'm never quite sure how strict bindings should be when
it comes to stuff like this. On the one hand, none of the other
compatibles described in it use any clocks that we know of
right now.

On the other, if we have a second set of compatibles that also
needs clocks, but in a different way, would we repeat that for
each such if/then condition? Or would be reformulate this as
some oneOf/anyOf construct specifically for the clock property?

Kind regards,
Nicolas Frattaroli

> 
> > +
> >  examples:
> >    - |
> >      apmixedsys_clk: syscon@...00800 {
> > @@ -104,4 +124,12 @@ examples:
> >          mediatek,hardware-voter = <&scp_hwv>;
> >          #clock-cells = <1>;
> >      };
> > +  - |
> > +    #include <dt-bindings/clock/mediatek,mt8196-clock.h>
> >  
> > +    clock-controller@...10000 {
> > +        compatible = "mediatek,mt8196-mfgpll-pll-ctrl", "syscon";
> > +        reg = <0x4b810000 0x400>;
> > +        clocks = <&topckgen CLK_TOP_MFG_EB>;
> > +        #clock-cells = <1>;
> > +    };
> > 
>