linux-kernel - Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

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: <969c42d7-0a40-4daf-a074-f2713d0d0412@kernel.org>
Date: Wed, 28 May 2025 13:56:54 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Christian Marangi <ansuelsmth@...il.com>
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>, Philipp Zabel <p.zabel@...gutronix.de>,
 Felix Fietkau <nbd@....name>, linux-clk@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for
 AN7583 clock

On 28/05/2025 10:54, Christian Marangi wrote:
> On Wed, May 28, 2025 at 09:30:37AM +0200, Krzysztof Kozlowski wrote:
>> On 28/05/2025 02:49, Christian Marangi wrote:
>>>    - if:
>>>        properties:
>>>          compatible:
>>> @@ -75,6 +78,17 @@ allOf:
>>>          reg:
>>>            maxItems: 1
>>>  
>>> +      required:
>>> +        - reg
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          const: airoha,an7583-clock
>>> +    then:
>>> +      properties:
>>> +        reg: false
>>
>>
>> No resources here, so this should be part of parent node.
>>
> 
> Ok hope you can help here. This is another case of "MFD" thing.
> 
> I was with the idea that it was O.K. to use this with very different
> devices. (current scenario Clock controller and MDIO controller)
> 
> The node structure I had in mind was
> 
> 		system-controller@...20000 {
> 			compatible = "airoha,an7583-scu", "syscon", "simple-mfd";
> 			reg = <0x0 0x1fb00000 0x0 0x970>;
> 
> 			scuclk: scuclk {
> 				compatible = "airoha,an7583-clock";
> 				#clock-cells = <1>;
> 				#reset-cells = <1>;
> 			};
> 
> 			mdio {
> 				compatible = "airoha,an7583-mdio";
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 
> 				mdio_0: bus@0 {
> 					reg = <0>;
> 					resets = <&scuclk AN7583_MDIO0>;
> 				};
> 
> 				mdio_1: bus@1 {
> 					reg = <1>;
> 					resets = <&scuclk AN7583_MDIO1>;
> 				};
> 			};
> 		};
> 
> But you want
> 
> system-controller@...20000 {
>         compatible = "airoha,an7583-scu", "syscon";
>         reg = <0x0 0x1fb00000 0x0 0x970>;
> 
>         #clock-cells = <1>;
>         #reset-cells = <1>;
> 

mdio could be here just to group the bus (it's pretty common I think),
although not sure if compatible is useful then.

>         mdio_0: bus@0 {
>                 reg = <0>;
>                 resets = <&scuclk AN7583_MDIO0>;
>         };
> 
>         mdio_1: bus@1 {
>                 reg = <1>;
>                 resets = <&scuclk AN7583_MDIO1>;
>         };
> };
> 
> Again sorry if this question keeps coming around and I can totally
> understand if you are getting annoyed by this. The reason I always ask
> this is because it's a total PAIN to implement this with the driver
> structure due to the old "simple-mfd" model.

... and Rob was saying multiple times: be careful when adding
simple-mfd. If it bites back, then I am sorry, but everyone were warned,
weren't they?

What is exactly the pain anyway? You cannot instantiate children from
SCU driver?

> 
> (as again putting everything in a single node conflicts with the OF
> principle of autoprobing stuff with compatible property)

I am not sure if I follow. What principle? Where is this principle
expressed?

And you do not have in your second example additional compatibles, so
even if such principle exists it is not broken: everything autoprobes, I
think.

> 


Best regards,
Krzysztof