On Tue, Jun 3, 2025 at 3:19 PM Krzysztof Kozlowski <k...@kernel.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. > > > > > 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. Bartosz
