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

[linux-kernel-dev]

On 9/24/26 6:35 PM, Conor Dooley wrote:
> On Thu, Sep 24, 2026 at 05:14:28PM +0200, Ronald Claveau via B4 Relay wrote:
>> From: Ronald Claveau <linux-kernel-dev@xxxxxxxx>
>>
>> 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.
> 

Right it's not clear, I will rephrase this.

>>
>> 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@xxxxxxxx>
>> ---
>>  .../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!
> 

Thanks for the suggestion, I didn't want to add "clk_chan_a", I will go
with "channel-a" and "channel-b".

>> +            - 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?
> 

You right, S4, A1, C3 and T7 have this clock, but S4 and A1 clock driver
declare them with "CLK_IGNORE_UNUSED" flag.
I will make a new proposal on V2.

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


-- 
Best regards,
Ronald