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

[Krzysztof Kozlowski <krzk@kernel.org>]

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.

> 
> 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.

Best regards,
Krzysztof