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

[linux-kernel-dev@aliel.fr]

Hi Chuan,

On 9/30/26 9:13 AM, Chuan Liu wrote:
> Hi Ronald, Cc Conor,
> 
> On 9/29/2026 12:46 AM, Conor Dooley wrote:
>> On Sun, Sep 27, 2026 at 04:28:15PM +0200, Ronald Claveau via B4 Relay
>> wrote:
>>> From: Ronald Claveau <linux-kernel-dev@aliel.fr>
>>>
>>> The T7, A1, C3, and S4 PWM controllers have a system clock in
>>> addition to the two input clocks of channels A and B.
>>>
>>> The channel clocks are acquired by index in the pwm driver,
>>> where the core clock is fetched by name, with optional get
>>> which is silent when not provided.
>>>
>>> Require clock-names and minimum 3 items for S4, and devices
>>> which fallback to S4, where the PWM devices do not work
>>> without all 3 clocks.
>>>
>>> Add an example showing 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        | 21 ++++++++++
>>> ++++++++---
>>>   1 file changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
>>> b/Documentation/devicetree/bindings/pwm/pwm-amlogic.yaml
>>> index c337d85da40f1..e9cd5f1fdf264 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
>>> @@ -143,9 +143,15 @@ allOf:
>>>             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
> 
> This is the APB bus clock gate for the PWM controller. If this clock is
> disabled, the CPU can no longer access the PWM controller's registers.
> 
> This clock serves the same purpose as the "bus" clock in the thermal
> sensor patch you submitted previously [1].
> 
> By the way, in the Amlogic clock driver, many sys_clk/clk81 clocks are
> used to provide the clock for the APB bus of individual controllers.
> Some of them also provide the functional clock for the controller, such
> as I2C.
> 
> Therefore, would it be reasonable to keep the description here
> consistent with the description in the thermal dt-binding mentioned above?
> 
> [1] https://lore.kernel.org/all/20260923-fix-t7-thermal-clock-v2-1-
> bff7a09eb487@aliel.fr/
> 

Thanks for your help and your feedback on this series. I will rename and
change the description in v4.

>>> +        clock-names:
>>> +          items:
>>> +            - const: channel-a
>>> +            - const: channel-b
>>> +            - const: core
> 
> Similarly, would it be more appropriate to name this clock "bus" as well?
> 
>>>         required:
>>>           - clocks
>>> +        - clock-names
>>>       - if:
>>>         properties:
>>> @@ -179,6 +185,15 @@ examples:
>>>       pwm@1000 {
>>>         compatible = "amlogic,meson-s4-pwm";
>>>         reg = <0x1000 0x10>;
>>> -      clocks = <&pwm_src_a>, <&pwm_src_b>;
>>> +      clocks = <&pwm_src_a>, <&pwm_src_b>, <&pwm_src_core>;
>>> +      clock-names = "channel-a", "channel-b", "core";
>>> +      #pwm-cells = <3>;
>>> +    };
>>> +  - |
>>> +    pwm@60000 {
>>
>> If you respin, drop the second example please, since it doesn't actually
>> do anything different anymore.
>>

I will drop this example in the mean time.

>> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>> pw-bot: not-applicable
>>
>>> +      compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
>>> +      reg = <0x60000 0x24>;
>>> +      clocks = <&channel_a>, <&channel_b>, <&core>;
>>> +      clock-names = "channel-a", "channel-b", "core";
>>>         #pwm-cells = <3>;
>>>       };
>>>
>>> -- 
>>> 2.49.0
>>>
>>>
> 


-- 
Best regards,
Ronald