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

[Chuan Liu]

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

> > +        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.
>
> Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> 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,
Chuan