linux-kernel - Re: [PATCH v3 1/8] dt-bindings: power: Add T-HEAD TH1520 GPU power sequencer

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <05aa1fad-acf6-4cea-9a20-e54a2a4669b7@samsung.com>
Date: Tue, 3 Jun 2025 20:24:20 +0200
From: Michal Wilczynski <m.wilczynski@...sung.com>
To: Bartosz Golaszewski <brgl@...ev.pl>, Krzysztof Kozlowski
	<krzk@...nel.org>, Ulf Hansson <ulf.hansson@...aro.org>
Cc: Drew Fustini <drew@...7.com>, Guo Ren <guoren@...nel.org>, Fu Wei
	<wefu@...hat.com>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Philipp Zabel
	<p.zabel@...gutronix.de>, Frank Binns <frank.binns@...tec.com>, Matt Coster
	<matt.coster@...tec.com>, Maarten Lankhorst
	<maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,
	Simona Vetter <simona@...ll.ch>, Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
	Alexandre Ghiti <alex@...ti.fr>, Ulf Hansson <ulf.hansson@...aro.org>, Marek
	Szyprowski <m.szyprowski@...sung.com>, linux-riscv@...ts.infradead.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v3 1/8] dt-bindings: power: Add T-HEAD TH1520 GPU power
 sequencer



On 6/3/25 15:35, Bartosz Golaszewski wrote:
> On Tue, Jun 3, 2025 at 3:19 PM Krzysztof Kozlowski <krzk@...nel.org> wrote:
>>
>> On Mon, Jun 02, 2025 at 10:29:13PM GMT, Michal Wilczynski wrote:
>>>>> +description: |
>>>>> +  This binding describes the power sequencer for the T-HEAD TH1520 GPU.
>>>>> +  This sequencer handles the specific power-up and power-down sequences
>>>>> +  required by the GPU, including managing clocks and resets from both the
>>>>> +  sequencer and the GPU device itself.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: thead,th1520-gpu-pwrseq
>>>>> +
>>>>
>>>> Before I review the rest: is this actually a physical device that
>>>> takes care of the power sequencing? Some kind of a power management
>>>> unit for the GPU? If so, I bet it's not called "power sequencer" so
>>>> let's use its actual name as per the datasheet?
>>>
>>> Hi Bart,
>>> Thanks for your feedback.
>>>
>>> The hardware block responsible for powering up the components in the
>>> TH1520 SoC datasheet is called AON (Always On). However, we already have
>>> a DT node named aon that serves as a power domain provider
>>> (Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml).
>>
>> So no. One device, one device node (sometimes with cildren nodes). You
>> do not get another device node just because someone wrote incomplete
>> binding or because driver looks differently.

The child option could potentially be an option here, as it would make
some sense, at least for me - since the child pwrseq DT node would
describe part of the AON block.

>>
>>>
>>> Following the discussion [1] about needing a separate DT node for the
>>> power sequencing capabilities of this AON block, and thinking further
>>> about it, I think the binding should be more generic. The AON block can
>>> manage power sequences for more than just the GPU (e.g. NPU, AUDIO,
>>> DSP).
>>>
>>> The compatible string could be updated like so:
>>> "thead,th1520-aon-pwrseq"
>>
>> Should not be separate node, you already have one for AON.
>>
> 
> Agreed. And as far as implementation goes, you can have the same
> driver be a PM domain AND pwrseq provider. It just has to bind to the
> device node that represents an actual component, not a made-up
> "convenience" node.

Sure - this can be done using existing AON node.

To keep the pwrseq code organized in drivers/power/sequencing/, a
similar approach to our th1520-pd driver interfacing with the AON
firmware library (drivers/firmware/thead,th1520-aon.c) could work.

The idea would be to treat code like pwrseq-thead-aon.c (changed from a
current pwrseq-thead-gpu.c) as a library. It would export its necessary
functions (e.g., for specific sequence init/deinit steps) using
EXPORT_SYMBOL_GPL. The main AON driver would then call these to provide
the pwrseq functionality.

This will introduce a compile-time dependency, as expected.

An alternative would be to keep the driver in drivers/power/sequencing/
as a platform driver and start it up using, for example, an auxiliary
bus. This is similar to what the JH7110 clock driver
(drivers/clk/starfive/clk-starfive-jh7110-sys.c) is doing with a reset
driver. This could offer a cleaner separation of roles if that's
preferred.

Please let me know which way would be preferred.

> 
> Bartosz
> 

Best regards,
-- 
Michal Wilczynski <m.wilczynski@...sung.com>