Re: [PATCH v2 2/4] dt-bindings: firmware: thead,th1520: Add resets for GPU clkgen

[Michal Wilczynski <m.wilczynski@samsung.com>]

On 4/15/25 18:38, Conor Dooley wrote:
> On Mon, Apr 14, 2025 at 08:52:56PM +0200, Michal Wilczynski wrote:
>> Extend the TH1520 AON firmware bindings to describe the GPU clkgen reset
>> line, required for proper GPU clock and reset sequencing.
>>
>> The T-HEAD TH1520 GPU requires coordinated management of two clocks
>> (core and sys) and two resets (GPU core reset and GPU clkgen
>> reset).  Only the clkgen reset is exposed at the AON level, to support
>> SoC-specific initialization handled through a generic PM domain. The GPU
>> core reset remains described in the GPU device node, as from the GPU
>> driver's perspective, there is only a single reset line [1].
>>
>> This follows upstream maintainers' recommendations [2] to abstract
>> SoC specific details into the PM domain layer rather than exposing them
>> to drivers directly.
>>
>> [1] - https://lore.kernel.org/all/816db99d-7088-4c1a-af03-b9a825ac09dc@imgtec.com/
>> [2] - https://lore.kernel.org/all/38d9650fc11a674c8b689d6bab937acf@kernel.org/
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  .../devicetree/bindings/firmware/thead,th1520-aon.yaml        | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
>> index bbc183200400de7aadbb21fea21911f6f4227b09..6ea3029c222df9ba6ea7d423b92ba248cfb02cc0 100644
>> --- a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
>> +++ b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
>> @@ -32,6 +32,13 @@ properties:
>>      items:
>>        - const: aon
>>  
>> +  resets:
>> +    maxItems: 1
>> +
>> +  reset-names:
>> +    items:
>> +      - const: gpu-clkgen
>> +
>>    "#power-domain-cells":
>>      const: 1
>>  
>> @@ -39,6 +46,8 @@ required:
>>    - compatible
>>    - mboxes
>>    - mbox-names
>> +  - resets
>> +  - reset-names
> 
> Given these are new required properties, have you made sure in the
> driver that their absence will not cause problems with older
> devicetrees? I took a brief look at the driver, and it _looked_ like you
> were failing if they were not there? It was a brief look though, tbf.

Hi Conor,

Good point — but in this case, the devicetrees compatible with the
driver haven’t been merged upstream yet. In fact, the TH1520 PM domains
driver currently doesn’t even compile against mainline, since the
required commit [1] didn’t make it into 6.15.

That said, Drew has queued the DT changes for the next release [2], and
you’ve queued [1], so assuming this series lands in 6.16, there won’t be
any older devicetrees to support. As a result, I haven’t added a
fallback path in the driver for missing properties.

If, however this series doesn’t make it in for 6.16, then yes — we’d
need to revisit the driver and add a failure safe path for cases where
these properties aren’t present.

Thanks,
Michał

[1] - https://lore.kernel.org/all/20250407-synergy-staff-b1cec90ffe72@spud/
[2] - https://lore.kernel.org/all/Z%2F6p6MQDS8ZlQv5r@x1/

> 
>>    - "#power-domain-cells"
>>  
>>  additionalProperties: false
>> @@ -49,5 +58,7 @@ examples:
>>          compatible = "thead,th1520-aon";
>>          mboxes = <&mbox_910t 1>;
>>          mbox-names = "aon";
>> +        resets = <&rst 0>;
>> +        reset-names = "gpu-clkgen";
>>          #power-domain-cells = <1>;
>>      };
>>
>> -- 
>> 2.34.1
>>