Re: [PATCH 1/3] dt-bindings: pwm: amlogic: Add core clock for T7

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3239 bytes --]

On Thu, Sep 24, 2026 at 05:14:28PM +0200, Ronald Claveau via B4 Relay wrote:
> From: Ronald Claveau <linux-kernel-dev@aliel.fr>
> 
> The T7 PWM controller has a system clock (the register access clock)
> in addition to the two input clocks of channels A and B. Add it as a
> third clock, named "core", for the amlogic,t7-pwm compatible, and
> require clock-names for this SoC for clarity.

What additional clarity does that actually bring? I think it's kinda
less clear, because the git history is required to explain why drivers
cannot fetch the existing clocks by name.

> 
> The other compatibles are left unchanged. Add an example showing the
> T7 usage with its fallback to amlogic,meson-s4-pwm.
> 
> Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
> ---
>  .../devicetree/bindings/pwm/pwm-amlogic.yaml       | 37 +++++++++++++++++++---
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
> index c337d85da40f1..9212a40d9fe48 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
> +++ b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
> @@ -62,7 +62,7 @@ properties:
>  
>    clock-names:
>      minItems: 1
> -    maxItems: 2
> +    maxItems: 3
>  
>    power-domains:
>      maxItems: 1
> @@ -135,17 +135,38 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            enum:
> -              - amlogic,meson-s4-pwm
> +            const: amlogic,t7-pwm
>      then:
>        properties:
>          clocks:
>            items:
>              - description: input clock of PWM channel A
>              - description: input clock of PWM channel B
> -        clock-names: false
> +            - description: gate clock for the PWM controller
> +        clock-names:
> +          items:
> +            - const: clk_a
> +            - const: clk_b

I'd rather "channel-a" or similar, we try not to state the obvious in
the names of clocks etc - of course this is a clock!

> +            - const: core
>        required:
>          - clocks
> +        - clock-names
> +    else:
> +      if:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - amlogic,meson-s4-pwm
> +      then:
> +        properties:
> +          clocks:
> +            items:
> +              - description: input clock of PWM channel A
> +              - description: input clock of PWM channel B
> +          clock-names: false

Are you sure that the s4-pwm doesn't also have this core clock? Seems
likely, no?

Cheers,
Conor.

> +        required:
> +          - clocks
>  
>    - if:
>        properties:
> @@ -182,3 +203,11 @@ examples:
>        clocks = <&pwm_src_a>, <&pwm_src_b>;
>        #pwm-cells = <3>;
>      };
> +  - |
> +    pwm@60000 {
> +      compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
> +      reg = <0x60000 0x24>;
> +      clocks = <&clk_a>, <&clk_b>, <&core>;
> +      clock-names = "clk_a", "clk_b", "core";
> +      #pwm-cells = <3>;
> +    };
> 
> -- 
> 2.49.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]