[PATCH v3 0/3] Add T7 PWM core clock support

[PATCH v3 0/3] Add T7 PWM core clock support

[Ronald Claveau via B4 Relay]

The PWM controllers on the Amlogic S4 SoC have a system clock that
controls access to the controller, in addition to the two input
clocks of channels A and B. The A1,C3,T7 which fallback to S4 have
this third clock too.

It is currently not described in the device tree, so it is neither
referenced nor enabled by the driver.

This series adds support for it:

  - patch 1 extends the binding with a third "core" clock for
    amlogic,meson-s4-pwm, and makes clock-names required for it;
  - patch 2 makes the driver get and enable the optional "core"
    clock, leaving existing DTS' unaffected;
  - patch 3 describes the system clock in the A1-C3-S4-T7
    device tree for all the PWM controllers.

`dt_binding_check DT_SCHEMA_FILES=pwm-amlogic.yaml` test is ok

  CHKDT   ./Documentation/devicetree/bindings
warning: python package 'yamllint' not installed, skipping
  STYLE   ./Documentation/devicetree/bindings
  DTEX    Documentation/devicetree/bindings/pwm/pwm-amlogic.example.dts
  DTC [C] Documentation/devicetree/bindings/pwm/pwm-amlogic.example.dtb

Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
Changes in v3:
- PATCH 1: Require the third clock for S4 and all SoC which fallback to it.
- PATCH 3: Add the third clock to A1, C3 and S4.
- Link to v2: https://lore.kernel.org/r/20260925-add-t7-sys-pwm-clk-v2-0-f23a22b71a5c@aliel.fr

Changes in v2:
- PATCH 1: Permits to use "core" clock on S4 SoC and all the ones that fallback to it.
           Rename "clk_a" and "clk_b" to "channel-a" and "channel-b". Thx to Conor's feedback.
- PATCH 3: Rename channel clock-names as done on patch 1.
- Link to v1: https://lore.kernel.org/r/20260924-add-t7-sys-pwm-clk-v1-0-05b91e87f0a9@aliel.fr

---
Ronald Claveau (3):
      dt-bindings: pwm: amlogic: Add core clock for S4 like
      pwm: meson: Add optional core clock for S4 PWM
      arm64: dts: amlogic: a1-c3-s4-t7: Add core clock to PWM controller nodes

 .../devicetree/bindings/pwm/pwm-amlogic.yaml       | 21 +++++++++++++---
 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi        | 28 ++++++++++++++++------
 arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi        | 28 ++++++++++++++++------
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi          | 12 +++++++---
 arch/arm64/boot/dts/amlogic/meson-s4.dtsi          | 20 ++++++++++++----
 drivers/pwm/pwm-meson.c                            |  5 ++++
 6 files changed, 89 insertions(+), 25 deletions(-)
---
base-commit: f5f84daefcd92d7a630066635ecea1433ed5eac7
change-id: 20260924-add-t7-sys-pwm-clk-f0eff81bee21

Best regards,
-- 
Ronald Claveau <linux-kernel-dev@aliel.fr>



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Ronald Claveau via B4 Relay]

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
+        clock-names:
+          items:
+            - const: channel-a
+            - const: channel-b
+            - const: core
       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 {
+      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



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 2/3] pwm: meson: Add optional core clock for S4 PWM

[Ronald Claveau via B4 Relay]

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

On some SoCs (e.g. T7), the PWM controller has a system clock that must
be enabled to access its registers, in addition to the per-channel
input clocks.

Get and enable it with devm_clk_get_optional_enabled() using the
"core" name. The clock is optional, so existing SoCs and device trees
that do not provide it keep working unchanged.

Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 drivers/pwm/pwm-meson.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 20e6cd07127c4..5ab07322e8d15 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -508,8 +508,13 @@ static int meson_pwm_init_channels_s4(struct pwm_chip *chip)
 	struct device *dev = pwmchip_parent(chip);
 	struct device_node *np = dev->of_node;
 	struct meson_pwm *meson = to_meson_pwm(chip);
+	struct clk *core_clk;
 	int i, ret;
 
+	core_clk = devm_clk_get_optional_enabled(dev, "core");
+	if (IS_ERR(core_clk))
+		return dev_err_probe(dev, PTR_ERR(core_clk), "Failed to get core clock\n");
+
 	for (i = 0; i < MESON_NUM_PWMS; i++) {
 		meson->channels[i].clk = of_clk_get(np, i);
 		if (IS_ERR(meson->channels[i].clk))

-- 
2.49.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

[PATCH v3 3/3] arm64: dts: amlogic: a1-c3-s4-t7: Add core clock to PWM controller nodes

[Ronald Claveau via B4 Relay]

From: Ronald Claveau <linux-kernel-dev@aliel.fr>

Describe the system clock of each PWM controller as a third clock,
and add the clock-names property expected by the updated binding:
"channel-a", "channel-b" and "core".

Signed-off-by: Ronald Claveau <linux-kernel-dev@aliel.fr>
---
 arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi | 28 +++++++++++++++++++++-------
 arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi | 28 +++++++++++++++++++++-------
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi   | 12 +++++++++---
 arch/arm64/boot/dts/amlogic/meson-s4.dtsi   | 20 +++++++++++++++-----
 4 files changed, 66 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
index 4e6757a57fb9a..a27e9bba22e5d 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-c3.dtsi
@@ -824,7 +824,9 @@ pwm_mn: pwm@54000 {
 					     "amlogic,meson-s4-pwm";
 				reg = <0x0 0x54000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_M>,
-					 <&clkc_periphs CLKID_PWM_N>;
+					 <&clkc_periphs CLKID_PWM_N>,
+					 <&clkc_periphs CLKID_SYS_PWM_MN>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -843,7 +845,9 @@ pwm_ab: pwm@58000 {
 					     "amlogic,meson-s4-pwm";
 				reg = <0x0 0x58000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_A>,
-					 <&clkc_periphs CLKID_PWM_B>;
+					 <&clkc_periphs CLKID_PWM_B>,
+					 <&clkc_periphs CLKID_SYS_PWM_AB>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -853,7 +857,9 @@ pwm_cd: pwm@5a000 {
 					     "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5a000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_C>,
-					 <&clkc_periphs CLKID_PWM_D>;
+					 <&clkc_periphs CLKID_PWM_D>,
+					 <&clkc_periphs CLKID_SYS_PWM_CD>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -863,7 +869,9 @@ pwm_ef: pwm@5c000 {
 					     "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5c000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_E>,
-					 <&clkc_periphs CLKID_PWM_F>;
+					 <&clkc_periphs CLKID_PWM_F>,
+					 <&clkc_periphs CLKID_SYS_PWM_EF>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -873,7 +881,9 @@ pwm_gh: pwm@5e000 {
 					     "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5e000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_G>,
-					 <&clkc_periphs CLKID_PWM_H>;
+					 <&clkc_periphs CLKID_PWM_H>,
+					 <&clkc_periphs CLKID_SYS_PWM_GH>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -883,7 +893,9 @@ pwm_ij: pwm@60000 {
 					     "amlogic,meson-s4-pwm";
 				reg = <0x0 0x60000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_I>,
-					 <&clkc_periphs CLKID_PWM_J>;
+					 <&clkc_periphs CLKID_PWM_J>,
+					 <&clkc_periphs CLKID_SYS_PWM_IJ>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -893,7 +905,9 @@ pwm_kl: pwm@62000 {
 					     "amlogic,meson-s4-pwm";
 				reg = <0x0 0x62000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_K>,
-					 <&clkc_periphs CLKID_PWM_L>;
+					 <&clkc_periphs CLKID_PWM_L>,
+					 <&clkc_periphs CLKID_SYS_PWM_KL>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
index 8f3f1c0579a0f..8a148af5ed442 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-t7.dtsi
@@ -692,7 +692,9 @@ pwm_ao_ef: pwm@30000 {
 				compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
 				reg = <0x0 0x30000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_AO_E>,
-					 <&clkc_periphs CLKID_PWM_AO_F>;
+					 <&clkc_periphs CLKID_PWM_AO_F>,
+					 <&clkc_periphs CLKID_SYS_PWM_AO_EF>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -701,7 +703,9 @@ pwm_ao_gh: pwm@32000 {
 				compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
 				reg = <0x0 0x32000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_AO_G>,
-					 <&clkc_periphs CLKID_PWM_AO_H>;
+					 <&clkc_periphs CLKID_PWM_AO_H>,
+					 <&clkc_periphs CLKID_SYS_PWM_AO_GH>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -715,7 +719,9 @@ pwm_ab: pwm@58000 {
 				compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
 				reg = <0x0 0x58000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_A>,
-					 <&clkc_periphs CLKID_PWM_B>;
+					 <&clkc_periphs CLKID_PWM_B>,
+					 <&clkc_periphs CLKID_SYS_PWM_AB>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -724,7 +730,9 @@ pwm_cd: pwm@5a000 {
 				compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5a000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_C>,
-					 <&clkc_periphs CLKID_PWM_D>;
+					 <&clkc_periphs CLKID_PWM_D>,
+					 <&clkc_periphs CLKID_SYS_PWM_CD>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -733,7 +741,9 @@ pwm_ef: pwm@5c000 {
 				compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5c000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_E>,
-					 <&clkc_periphs CLKID_PWM_F>;
+					 <&clkc_periphs CLKID_PWM_F>,
+					 <&clkc_periphs CLKID_SYS_PWM_EF>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -742,7 +752,9 @@ pwm_ao_ab: pwm@5e000 {
 				compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5e000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_AO_A>,
-					 <&clkc_periphs CLKID_PWM_AO_B>;
+					 <&clkc_periphs CLKID_PWM_AO_B>,
+					 <&clkc_periphs CLKID_SYS_PWM_AO_AB>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -751,7 +763,9 @@ pwm_ao_cd: pwm@60000 {
 				compatible = "amlogic,t7-pwm", "amlogic,meson-s4-pwm";
 				reg = <0x0 0x60000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_AO_C>,
-					 <&clkc_periphs CLKID_PWM_AO_D>;
+					 <&clkc_periphs CLKID_PWM_AO_D>,
+					 <&clkc_periphs CLKID_SYS_PWM_AO_CD>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
index 6f6a6145cba12..b9761893c666d 100644
--- a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
@@ -544,7 +544,9 @@ pwm_ab: pwm@2400 {
 				reg = <0x0 0x2400 0x0 0x24>;
 				#pwm-cells = <3>;
 				clocks = <&clkc_periphs CLKID_PWM_A>,
-					 <&clkc_periphs CLKID_PWM_B>;
+					 <&clkc_periphs CLKID_PWM_B>,
+					 <&clkc_periphs CLKID_PWM_AB>;
+				clock-names = "channel-a", "channel-b", "core";
 				power-domains = <&pwrc PWRC_I2C_ID>;
 				status = "disabled";
 			};
@@ -555,7 +557,9 @@ pwm_cd: pwm@2800 {
 				reg = <0x0 0x2800 0x0 0x24>;
 				#pwm-cells = <3>;
 				clocks = <&clkc_periphs CLKID_PWM_C>,
-					 <&clkc_periphs CLKID_PWM_D>;
+					 <&clkc_periphs CLKID_PWM_D>,
+					 <&clkc_periphs CLKID_PWM_CD>;
+				clock-names = "channel-a", "channel-b", "core";
 				power-domains = <&pwrc PWRC_I2C_ID>;
 				status = "disabled";
 			};
@@ -655,7 +659,9 @@ pwm_ef: pwm@5400 {
 				reg = <0x0 0x5400 0x0 0x24>;
 				#pwm-cells = <3>;
 				clocks = <&clkc_periphs CLKID_PWM_E>,
-					 <&clkc_periphs CLKID_PWM_F>;
+					 <&clkc_periphs CLKID_PWM_F>,
+					 <&clkc_periphs CLKID_PWM_EF>;
+				clock-names = "channel-a", "channel-b", "core";
 				power-domains = <&pwrc PWRC_I2C_ID>;
 				status = "disabled";
 			};
diff --git a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
index 2a6fbd5308362..ca9abbf857178 100644
--- a/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-s4.dtsi
@@ -685,7 +685,9 @@ pwm_ab: pwm@58000 {
 				compatible = "amlogic,meson-s4-pwm";
 				reg = <0x0 0x58000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_A>,
-					 <&clkc_periphs CLKID_PWM_B>;
+					 <&clkc_periphs CLKID_PWM_B>,
+					 <&clkc_periphs CLKID_PWM_AB>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -694,7 +696,9 @@ pwm_cd: pwm@5a000 {
 				compatible = "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5a000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_C>,
-					 <&clkc_periphs CLKID_PWM_D>;
+					 <&clkc_periphs CLKID_PWM_D>,
+					 <&clkc_periphs CLKID_PWM_CD>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -703,7 +707,9 @@ pwm_ef: pwm@5c000 {
 				compatible = "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5c000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_E>,
-					 <&clkc_periphs CLKID_PWM_F>;
+					 <&clkc_periphs CLKID_PWM_F>,
+					 <&clkc_periphs CLKID_PWM_EF>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -712,7 +718,9 @@ pwm_gh: pwm@5e000 {
 				compatible = "amlogic,meson-s4-pwm";
 				reg = <0x0 0x5e000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_G>,
-					 <&clkc_periphs CLKID_PWM_H>;
+					 <&clkc_periphs CLKID_PWM_H>,
+					 <&clkc_periphs CLKID_PWM_GH>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};
@@ -721,7 +729,9 @@ pwm_ij: pwm@60000 {
 				compatible = "amlogic,meson-s4-pwm";
 				reg = <0x0 0x60000 0x0 0x24>;
 				clocks = <&clkc_periphs CLKID_PWM_I>,
-					 <&clkc_periphs CLKID_PWM_J>;
+					 <&clkc_periphs CLKID_PWM_J>,
+					 <&clkc_periphs CLKID_PWM_IJ>;
+				clock-names = "channel-a", "channel-b", "core";
 				#pwm-cells = <3>;
 				status = "disabled";
 			};

-- 
2.49.0



_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dt-bindings: pwm-amlogic: missing minItems allows validation bypass
--

commit 359e122eac15df570817ba590b91f8958522a038
Author: Ronald Claveau <linux-kernel-dev@aliel.fr>

dt-bindings: pwm: amlogic: Add core clock for S4 like
    
This commit adds a third system core clock to the Amlogic PWM controller 
bindings for S4 and related devices. It updates the conditionals for the 
S4 compatible string to require three clocks instead of two, and updates 
the examples to match.

> 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

[ ... ]

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

[Severity: Medium]
Will dt-schema successfully enforce the minimum 3 items requirement intended 
by the commit message?

Because the global `clocks` property definition has `minItems: 1` and JSON 
Schema's `items` keyword only validates elements that are actually present, a 
device tree node with `compatible = "amlogic,meson-s4-pwm"` and only 1 or 2 
clocks will silently pass validation.

Does this block need an explicit `minItems: 3`?

> +        clock-names:
> +          items:
> +            - const: channel-a
> +            - const: channel-b
> +            - const: core

[Severity: Medium]
Does this also require `minItems: 3`?

Supplying just "channel-a" and "channel-b" will still validate successfully 
without a `minItems` constraint here.

>        required:
>          - clocks
> +        - clock-names

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927-add-t7-sys-pwm-clk-v3-0-18d66abea4d1@aliel.fr?part=1

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Ronald Claveau]

On 9/27/26 4:37 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] dt-bindings: pwm-amlogic: missing minItems allows validation bypass
> --
> 
> commit 359e122eac15df570817ba590b91f8958522a038
> Author: Ronald Claveau <linux-kernel-dev@aliel.fr>
> 
> dt-bindings: pwm: amlogic: Add core clock for S4 like
>     
> This commit adds a third system core clock to the Amlogic PWM controller 
> bindings for S4 and related devices. It updates the conditionals for the 
> S4 compatible string to require three clocks instead of two, and updates 
> the examples to match.
> 
>> 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
> 
> [ ... ]
> 
>> @@ -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
> 
> [Severity: Medium]
> Will dt-schema successfully enforce the minimum 3 items requirement intended 
> by the commit message?
> 
> Because the global `clocks` property definition has `minItems: 1` and JSON 
> Schema's `items` keyword only validates elements that are actually present, a 
> device tree node with `compatible = "amlogic,meson-s4-pwm"` and only 1 or 2 
> clocks will silently pass validation.
> 
> Does this block need an explicit `minItems: 3`?
> 

If I remove one clock, the validation fails

```
  CHKDT   ./Documentation/devicetree/bindings
  LINT    ./Documentation/devicetree/bindings
  STYLE   ./Documentation/devicetree/bindings
  DTEX    Documentation/devicetree/bindings/pwm/pwm-amlogic.example.dts
  DTC [C]
Documentation/devicetree/bindings/thermal/amlogic,thermal.example.dtb
  DTC [C] Documentation/devicetree/bindings/pwm/pwm-amlogic.example.dtb
/home/*redacted*/linux/Documentation/devicetree/bindings/pwm/pwm-amlogic.example.dtb:
pwm@1000 (amlogic,meson-s4-pwm): clock-names: ['channel-a', 'channel-b']
is too short
	from schema $id: http://devicetree.org/schemas/pwm/pwm-amlogic.yaml
/home/*redacted*/linux/Documentation/devicetree/bindings/pwm/pwm-amlogic.example.dtb:
pwm@1000 (amlogic,meson-s4-pwm): clocks: [[4294967295], [4294967295]] is
too short
	from schema $id: http://devicetree.org/schemas/pwm/pwm-amlogic.yaml
/home/*redacted*/linux/Documentation/devicetree/bindings/pwm/pwm-amlogic.example.dtb:
pwm@60000 (amlogic,t7-pwm): clock-names: ['channel-a', 'channel-b'] is
too short
	from schema $id: http://devicetree.org/schemas/pwm/pwm-amlogic.yaml
/home/*redacted*/linux/Documentation/devicetree/bindings/pwm/pwm-amlogic.example.dtb:
pwm@60000 (amlogic,t7-pwm): clocks: [[4294967295], [4294967295]] is too
short
	from schema $id: http://devicetree.org/schemas/pwm/pwm-amlogic.yaml
```

>> +        clock-names:
>> +          items:
>> +            - const: channel-a
>> +            - const: channel-b
>> +            - const: core
> 
> [Severity: Medium]
> Does this also require `minItems: 3`?
> 
> Supplying just "channel-a" and "channel-b" will still validate successfully 
> without a `minItems` constraint here.
> 
>>        required:
>>          - clocks
>> +        - clock-names
> 


-- 
Best regards,
Ronald

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

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

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2692 bytes --]

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
> +        clock-names:
> +          items:
> +            - const: channel-a
> +            - const: channel-b
> +            - const: core
>        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@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
> 
> 

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

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic