linux-kernel - Re: [PATCH v1 1/2] dt-bindings: firmware: thead,th1520: Add clocks and resets

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: <CAPDyKFpoSwKAmiWyvNt1fVyu6=NU1oVOmQLVuzX_bG=-5KrM2Q@mail.gmail.com>
Date: Wed, 9 Apr 2025 12:41:16 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Michal Wilczynski <m.wilczynski@...sung.com>
Cc: robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, drew@...7.com, 
	guoren@...nel.org, wefu@...hat.com, p.zabel@...gutronix.de, 
	m.szyprowski@...sung.com, linux-kernel@...r.kernel.org, 
	linux-riscv@...ts.infradead.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH v1 1/2] dt-bindings: firmware: thead,th1520: Add clocks
 and resets

On Wed, 9 Apr 2025 at 11:30, Michal Wilczynski <m.wilczynski@...sung.com> wrote:
>
> Prepare for handling GPU clock and reset sequencing through a generic
> power domain by adding clock and reset properties to the TH1520 AON
> firmware bindings.
>
> The T-HEAD TH1520 GPU requires coordinated management of two clocks
> (core and sys) and two resets (GPU and GPU CLKGEN). Due to SoC-specific
> requirements, the CLKGEN reset must be carefully managed alongside clock
> enables to ensure proper GPU operation, as discussed on the mailing list
> [1].
>
> Since the coordination is now handled through a power domain, only the
> programmable clocks (core and sys) are exposed. The GPU MEM clock is
> ignored, as it is not controllable on the TH1520 SoC.
>
> This approach follows upstream maintainers' recommendations [1] to
> avoid SoC-specific details leaking into the GPU driver or clock/reset
> frameworks directly.
>
> [1] - https://lore.kernel.org/all/38d9650fc11a674c8b689d6bab937acf@kernel.org/
>
> Signed-off-by: Michal Wilczynski <m.wilczynski@...sung.com>
> ---
>  .../bindings/firmware/thead,th1520-aon.yaml   | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> index bbc183200400..8075874bcd6b 100644
> --- a/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> +++ b/Documentation/devicetree/bindings/firmware/thead,th1520-aon.yaml
> @@ -25,6 +25,16 @@ properties:
>    compatible:
>      const: thead,th1520-aon
>
> +  clocks:
> +    items:
> +      - description: GPU core clock
> +      - description: GPU sys clock
> +
> +  clock-names:
> +    items:
> +      - const: gpu-core
> +      - const: gpu-sys

These clocks don't look like they belong to the power-domain node, but
rather the GPU's node.

Or is this in fact the correct description of the HW?

> +
>    mboxes:
>      maxItems: 1
>
> @@ -32,13 +42,27 @@ properties:
>      items:
>        - const: aon
>
> +  resets:
> +    items:
> +      - description: GPU reset
> +      - description: GPU CLKGEN reset
> +
> +  reset-names:
> +    items:
> +      - const: gpu
> +      - const: gpu-clkgen
> +

Ditto for the reset.

>    "#power-domain-cells":
>      const: 1
>
>  required:
>    - compatible
> +  - clocks
> +  - clock-names
>    - mboxes
>    - mbox-names
> +  - resets
> +  - reset-names
>    - "#power-domain-cells"
>
>  additionalProperties: false
> @@ -47,7 +71,11 @@ examples:
>    - |
>      aon: aon {
>          compatible = "thead,th1520-aon";
> +        clocks = <&clk_vo 0>, <&clk_vo 1>;
> +        clock-names = "gpu-core", "gpu-sys";
>          mboxes = <&mbox_910t 1>;
>          mbox-names = "aon";
> +        resets = <&rst 0>, <&rst 1>;
> +        reset-names = "gpu", "gpu-clkgen";
>          #power-domain-cells = <1>;
>      };
> --
> 2.34.1
>

That said, it's still possible to make both the clocks and reset being
managed from the genpd provider. I will comment on that separately for
patch2.

Kind regards
Uffe