linux-kernel - Re: [RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add support for power domains

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: <b6935ae6-5426-440f-9dbd-775002e110fc@kernel.org>
Date: Tue, 24 Dec 2024 14:32:24 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Michal Wilczynski <m.wilczynski@...sung.com>, mturquette@...libre.com,
 sboyd@...nel.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
 drew@...7.com, guoren@...nel.org, wefu@...hat.com, jassisinghbrar@...il.com,
 paul.walmsley@...ive.com, palmer@...belt.com, aou@...s.berkeley.edu,
 frank.binns@...tec.com, matt.coster@...tec.com,
 maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
 airlied@...il.com, simona@...ll.ch, ulf.hansson@...aro.org,
 jszhang@...nel.org, p.zabel@...gutronix.de, m.szyprowski@...sung.com
Cc: linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org,
 dri-devel@...ts.freedesktop.org, linux-pm@...r.kernel.org
Subject: Re: [RFC PATCH v2 03/19] dt-bindings: power: thead,th1520: Add
 support for power domains

On 24/12/2024 10:31, Michal Wilczynski wrote:
> 
> 
> On 12/23/24 17:09, Krzysztof Kozlowski wrote:
>> On 23/12/2024 13:55, Michal Wilczynski wrote:
>>> +  compatible:
>>> +    const: thead,th1520-pd
>>> +
>>> +  "#power-domain-cells":
>>> +    const: 1
>>> +
>>> +additionalProperties: false
>>> +
>>> +required:
>>> +  - compatible
>>> +  - "#power-domain-cells"
>>> +
>>> +examples:
>>> +  - |
>>> +    firmware {
>>
>> Drop
>>
>>> +        aon: aon {
>>> +            compatible = "thead,th1520-aon";
>>> +            mboxes = <&mbox_910t 1>;
>>> +            mbox-names = "aon";
>>
>> Drop aon node... but the main problem is you do not have any resources
>> in your power-domain device node, assuming your binding is complete.
>> This suggests that this is part of aon, not separate device. Fold the
>> device node into its parent (so everything goes to AON).
> 
> Merging everything to AON node would definitely work. I was looking at
> the other implementations of firmware protocols for example, and that's
> how I figured the current implementation:
> 
> arch/arm/boot/dts/broadcom/bcm2835-rpi.dtsi
> soc {
> 	firmware: firmware {
> 		compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
> 		mboxes = <&mailbox>;

That's a 10 year old binding. Not even reviewed initially by DT maintainers.

> 
> 		firmware_clocks: clocks {
> 			compatible = "raspberrypi,firmware-clocks";
> 			#clock-cells = <1>;
> 		};
> 	};
> 
> 	power: power {
> 		compatible = "raspberrypi,bcm2835-power";
> 		firmware = <&firmware>;
> 		#power-domain-cells = <1>;
> 	};
> };
> 
> This is fairly similar, as the firmware is passed as property, instead
> as in a parent-child relationship. Would you consider it more canonical
> ?

I consider early Raspberry bindings as antipattern. Same as many other
early approaches and that's not an accusation to their authors. Just
rules changed and capacity of DT maintainers increased.

> 
> I would be happy to merge everything to AON node, and merge the
> power-domain driver and AON driver together, but it seemed to me like
> those could use some separation, and since power-domain and the AON
> represent actual HW it's fine to represent them in the device tree.
> 

I said nothing about drivers and I did not suggest merging drivers.
Device nodes should not be split because they have different functions.
This is one single device with multiple functions. Just like hundreds of
others, e.g. clock, power domain and reset controller - all in one. Lack
of separate address space is the main aspect here indicating this is not
a separate device.

So merge the device nodes.



Best regards,
Krzysztof