[PATCH 0/2] clk: meson: s4: two peripheral clock fixes

[PATCH 0/2] clk: meson: s4: two peripheral clock fixes

[christian.koever-draxl]

From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

Two fixes for the S4 peripheral clock controller found while bringing
up an S905W2 TV box:

 - the PWM muxes may pick vid_pll, which is reprogrammed on every HDMI
   mode set and so changes the period of any PWM that runs from it. On
   this board that corrupted the PWM CPU regulator (first cpufreq
   transition hung the SoC) and the 32.768 kHz clock of the SDIO wifi
   chip;
 - the CEC 32k selectors lack CLK_SET_RATE_PARENT, so AO-CEC cannot set
   its 32768 Hz rate and fails to probe.

Tool use (Documentation/process/generated-content.rst):
This work was done with the help of an AI coding assistant, Claude
(Anthropic), used through Claude Code over many sessions. It did a large
part of the hands-on work: it read the vendor kernel and the firmware
interfaces, probed registers on the board, drafted the code and the
changelogs, and ran the builds, checkpatch, dt_binding_check and the
tests on the hardware. I directed the work, reviewed every change and
the final patches, and take responsibility for them. There is no single
prompt to quote; the sessions were of the form "find out why X fails on
this board, fix it, test it on the box".

Tested on an Amediatech X98Q TV box (Amlogic S905W2, S4 family, 2 GiB),
vendor U-Boot and BL31/OP-TEE, Linux 7.3 plus this work.

Christian Stefan Kövér-Draxl (2):
  clk: meson: s4: keep the PWM clocks off vid_pll
  clk: meson: s4: propagate rate requests through the CEC 32k muxes

 drivers/clk/meson/s4-peripherals.c | 40 +++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 12 deletions(-)


base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
-- 
2.56.0


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

[PATCH 1/2] clk: meson: s4: keep the PWM clocks off vid_pll

[christian.koever-draxl]

From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

The PWM driver asks the clock framework for the fastest input it can
get (clk_round_rate() with roughly 0xffff/period), so on S4 every PWM
channel ends up parented to vid_pll, the fastest input of the PWM mux.
vid_pll is derived from the HDMI PLL and is reprogrammed on every
display mode set, which silently changes the period and duty cycle of
whatever PWM runs from it.

On boards that use a PWM regulator for the CPU supply this corrupts
the CPU voltage: on an S905W2 board vddcpu read 699 mV at 1 GHz and
the first cpufreq transition hung the SoC. Boards that feed the
32.768 kHz low power clock of an SDIO wifi chip from a PWM see the chip
intermittently fail to enumerate ("Failed to initialize a
non-removable card").

Drop vid_pll from the PWM parents and keep the remaining inputs on
their register values with a mux table. xtal, fclk_div4 and fclk_div3
cover every PWM rate in use.

Fixes: 57b55c76aaf1 ("clk: meson: S4: add support for Amlogic S4 SoC peripheral clock controller")
Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
---
 drivers/clk/meson/s4-peripherals.c | 38 ++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
index ba41fcd90588..329cd526749e 100644
--- a/drivers/clk/meson/s4-peripherals.c
+++ b/drivers/clk/meson/s4-peripherals.c
@@ -2588,51 +2588,65 @@ static struct clk_regmap s4_spicc0_en = {
 	},
 };
 
-/* PWM Clock */
+/*
+ * PWM Clock
+ *
+ * vid_pll (mux input 1) is left out on purpose. The PWM driver asks for the
+ * fastest clock it can get, which made every PWM pick vid_pll - and vid_pll
+ * is the HDMI PLL, reprogrammed on every display mode set. That silently
+ * changed the period and duty of whatever ran from it: the PWM CPU supply
+ * (a cpufreq voltage change then hung the SoC) and the 32.768kHz clock of an
+ * SDIO wifi chip (which then failed to enumerate).
+ */
+static u32 s4_pwm_parents_table[] = { 0, 2, 3 };
+
 static const struct clk_parent_data s4_pwm_parents[] = {
 	{ .fw_name = "xtal", },
-	{ .hw = &s4_vid_pll.hw },
 	{ .fw_name = "fclk_div4", },
 	{ .fw_name = "fclk_div3", },
 };
 
-static S4_COMP_SEL(pwm_a, CLKCTRL_PWM_CLK_AB_CTRL, 9, 0x3, s4_pwm_parents);
+#define S4_PWM_SEL(_name, _reg, _shift) \
+	MESON_COMP_SEL(s4_, _name, _reg, _shift, 0x3, s4_pwm_parents, \
+		       s4_pwm_parents_table, 0, 0)
+
+static S4_PWM_SEL(pwm_a, CLKCTRL_PWM_CLK_AB_CTRL, 9);
 static S4_COMP_DIV(pwm_a, CLKCTRL_PWM_CLK_AB_CTRL, 0, 8);
 static S4_COMP_GATE(pwm_a, CLKCTRL_PWM_CLK_AB_CTRL, 8);
 
-static S4_COMP_SEL(pwm_b, CLKCTRL_PWM_CLK_AB_CTRL, 25, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_b, CLKCTRL_PWM_CLK_AB_CTRL, 25);
 static S4_COMP_DIV(pwm_b, CLKCTRL_PWM_CLK_AB_CTRL, 16, 8);
 static S4_COMP_GATE(pwm_b, CLKCTRL_PWM_CLK_AB_CTRL, 24);
 
-static S4_COMP_SEL(pwm_c, CLKCTRL_PWM_CLK_CD_CTRL, 9, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_c, CLKCTRL_PWM_CLK_CD_CTRL, 9);
 static S4_COMP_DIV(pwm_c, CLKCTRL_PWM_CLK_CD_CTRL, 0, 8);
 static S4_COMP_GATE(pwm_c, CLKCTRL_PWM_CLK_CD_CTRL, 8);
 
-static S4_COMP_SEL(pwm_d, CLKCTRL_PWM_CLK_CD_CTRL, 25, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_d, CLKCTRL_PWM_CLK_CD_CTRL, 25);
 static S4_COMP_DIV(pwm_d, CLKCTRL_PWM_CLK_CD_CTRL, 16, 8);
 static S4_COMP_GATE(pwm_d, CLKCTRL_PWM_CLK_CD_CTRL, 24);
 
-static S4_COMP_SEL(pwm_e, CLKCTRL_PWM_CLK_EF_CTRL, 9, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_e, CLKCTRL_PWM_CLK_EF_CTRL, 9);
 static S4_COMP_DIV(pwm_e, CLKCTRL_PWM_CLK_EF_CTRL, 0, 8);
 static S4_COMP_GATE(pwm_e, CLKCTRL_PWM_CLK_EF_CTRL, 8);
 
-static S4_COMP_SEL(pwm_f, CLKCTRL_PWM_CLK_EF_CTRL, 25, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_f, CLKCTRL_PWM_CLK_EF_CTRL, 25);
 static S4_COMP_DIV(pwm_f, CLKCTRL_PWM_CLK_EF_CTRL, 16, 8);
 static S4_COMP_GATE(pwm_f, CLKCTRL_PWM_CLK_EF_CTRL, 24);
 
-static S4_COMP_SEL(pwm_g, CLKCTRL_PWM_CLK_GH_CTRL, 9, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_g, CLKCTRL_PWM_CLK_GH_CTRL, 9);
 static S4_COMP_DIV(pwm_g, CLKCTRL_PWM_CLK_GH_CTRL, 0, 8);
 static S4_COMP_GATE(pwm_g, CLKCTRL_PWM_CLK_GH_CTRL, 8);
 
-static S4_COMP_SEL(pwm_h, CLKCTRL_PWM_CLK_GH_CTRL, 25, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_h, CLKCTRL_PWM_CLK_GH_CTRL, 25);
 static S4_COMP_DIV(pwm_h, CLKCTRL_PWM_CLK_GH_CTRL, 16, 8);
 static S4_COMP_GATE(pwm_h, CLKCTRL_PWM_CLK_GH_CTRL, 24);
 
-static S4_COMP_SEL(pwm_i, CLKCTRL_PWM_CLK_IJ_CTRL, 9, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_i, CLKCTRL_PWM_CLK_IJ_CTRL, 9);
 static S4_COMP_DIV(pwm_i, CLKCTRL_PWM_CLK_IJ_CTRL, 0, 8);
 static S4_COMP_GATE(pwm_i, CLKCTRL_PWM_CLK_IJ_CTRL, 8);
 
-static S4_COMP_SEL(pwm_j, CLKCTRL_PWM_CLK_IJ_CTRL, 25, 0x3, s4_pwm_parents);
+static S4_PWM_SEL(pwm_j, CLKCTRL_PWM_CLK_IJ_CTRL, 25);
 static S4_COMP_DIV(pwm_j, CLKCTRL_PWM_CLK_IJ_CTRL, 16, 8);
 static S4_COMP_GATE(pwm_j, CLKCTRL_PWM_CLK_IJ_CTRL, 24);
 
-- 
2.56.0


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

Re: [PATCH 1/2] clk: meson: s4: keep the PWM clocks off vid_pll

[Jerome Brunet]

On dim. 25 oct. 2026 at 20:18, christian.koever-draxl@student.uibk.ac.at wrote:

> From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
>
> The PWM driver asks the clock framework for the fastest input it can
> get (clk_round_rate() with roughly 0xffff/period), so on S4 every PWM
> channel ends up parented to vid_pll, the fastest input of the PWM mux.
> vid_pll is derived from the HDMI PLL and is reprogrammed on every
> display mode set, which silently changes the period and duty cycle of
> whatever PWM runs from it.

While I agree with the general principle of the change,
"The PWM driver asks the clock framework for the fastest input" is a
fairly inaccurate representation of what the PWM driver does and it gets
repeated. Please fix this.

You've disclose the usage AI, it is great. It would be even better if
you could summarize its output a little bit. This is very verbose
for a simple problem: vid_pll is dedicated to another usage in general
and should actually not be provided to PWM devices, eventhough the HW
allows it

>
> On boards that use a PWM regulator for the CPU supply this corrupts
> the CPU voltage: on an S905W2 board vddcpu read 699 mV at 1 GHz and
> the first cpufreq transition hung the SoC. Boards that feed the
> 32.768 kHz low power clock of an SDIO wifi chip from a PWM see the chip
> intermittently fail to enumerate ("Failed to initialize a
> non-removable card").
>
> Drop vid_pll from the PWM parents and keep the remaining inputs on
> their register values with a mux table. xtal, fclk_div4 and fclk_div3
> cover every PWM rate in use.
>
> Fixes: 57b55c76aaf1 ("clk: meson: S4: add support for Amlogic S4 SoC peripheral clock controller")
> Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
> ---
>  drivers/clk/meson/s4-peripherals.c | 38 ++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
> index ba41fcd90588..329cd526749e 100644
> --- a/drivers/clk/meson/s4-peripherals.c
> +++ b/drivers/clk/meson/s4-peripherals.c
> @@ -2588,51 +2588,65 @@ static struct clk_regmap s4_spicc0_en = {
>  	},
>  };
>  
> -/* PWM Clock */
> +/*
> + * PWM Clock
> + *
> + * vid_pll (mux input 1) is left out on purpose. The PWM driver asks for the
> + * fastest clock it can get, which made every PWM pick vid_pll - and vid_pll
> + * is the HDMI PLL, reprogrammed on every display mode set. That silently
> + * changed the period and duty of whatever ran from it: the PWM CPU supply
> + * (a cpufreq voltage change then hung the SoC) and the 32.768kHz clock of an
> + * SDIO wifi chip (which then failed to enumerate).
> + */
> +static u32 s4_pwm_parents_table[] = { 0, 2, 3 };
> +
>  static const struct clk_parent_data s4_pwm_parents[] = {
>  	{ .fw_name = "xtal", },
> -	{ .hw = &s4_vid_pll.hw },
>  	{ .fw_name = "fclk_div4", },
>  	{ .fw_name = "fclk_div3", },
>  };
>  
> -static S4_COMP_SEL(pwm_a, CLKCTRL_PWM_CLK_AB_CTRL, 9, 0x3, s4_pwm_parents);
> +#define S4_PWM_SEL(_name, _reg, _shift) \
> +	MESON_COMP_SEL(s4_, _name, _reg, _shift, 0x3, s4_pwm_parents, \
> +		       s4_pwm_parents_table, 0, 0)
> +
> +static S4_PWM_SEL(pwm_a, CLKCTRL_PWM_CLK_AB_CTRL, 9);
>  static S4_COMP_DIV(pwm_a, CLKCTRL_PWM_CLK_AB_CTRL, 0, 8);
>  static S4_COMP_GATE(pwm_a, CLKCTRL_PWM_CLK_AB_CTRL, 8);
>  
> -static S4_COMP_SEL(pwm_b, CLKCTRL_PWM_CLK_AB_CTRL, 25, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_b, CLKCTRL_PWM_CLK_AB_CTRL, 25);
>  static S4_COMP_DIV(pwm_b, CLKCTRL_PWM_CLK_AB_CTRL, 16, 8);
>  static S4_COMP_GATE(pwm_b, CLKCTRL_PWM_CLK_AB_CTRL, 24);
>  
> -static S4_COMP_SEL(pwm_c, CLKCTRL_PWM_CLK_CD_CTRL, 9, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_c, CLKCTRL_PWM_CLK_CD_CTRL, 9);
>  static S4_COMP_DIV(pwm_c, CLKCTRL_PWM_CLK_CD_CTRL, 0, 8);
>  static S4_COMP_GATE(pwm_c, CLKCTRL_PWM_CLK_CD_CTRL, 8);
>  
> -static S4_COMP_SEL(pwm_d, CLKCTRL_PWM_CLK_CD_CTRL, 25, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_d, CLKCTRL_PWM_CLK_CD_CTRL, 25);
>  static S4_COMP_DIV(pwm_d, CLKCTRL_PWM_CLK_CD_CTRL, 16, 8);
>  static S4_COMP_GATE(pwm_d, CLKCTRL_PWM_CLK_CD_CTRL, 24);
>  
> -static S4_COMP_SEL(pwm_e, CLKCTRL_PWM_CLK_EF_CTRL, 9, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_e, CLKCTRL_PWM_CLK_EF_CTRL, 9);
>  static S4_COMP_DIV(pwm_e, CLKCTRL_PWM_CLK_EF_CTRL, 0, 8);
>  static S4_COMP_GATE(pwm_e, CLKCTRL_PWM_CLK_EF_CTRL, 8);
>  
> -static S4_COMP_SEL(pwm_f, CLKCTRL_PWM_CLK_EF_CTRL, 25, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_f, CLKCTRL_PWM_CLK_EF_CTRL, 25);
>  static S4_COMP_DIV(pwm_f, CLKCTRL_PWM_CLK_EF_CTRL, 16, 8);
>  static S4_COMP_GATE(pwm_f, CLKCTRL_PWM_CLK_EF_CTRL, 24);
>  
> -static S4_COMP_SEL(pwm_g, CLKCTRL_PWM_CLK_GH_CTRL, 9, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_g, CLKCTRL_PWM_CLK_GH_CTRL, 9);
>  static S4_COMP_DIV(pwm_g, CLKCTRL_PWM_CLK_GH_CTRL, 0, 8);
>  static S4_COMP_GATE(pwm_g, CLKCTRL_PWM_CLK_GH_CTRL, 8);
>  
> -static S4_COMP_SEL(pwm_h, CLKCTRL_PWM_CLK_GH_CTRL, 25, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_h, CLKCTRL_PWM_CLK_GH_CTRL, 25);
>  static S4_COMP_DIV(pwm_h, CLKCTRL_PWM_CLK_GH_CTRL, 16, 8);
>  static S4_COMP_GATE(pwm_h, CLKCTRL_PWM_CLK_GH_CTRL, 24);
>  
> -static S4_COMP_SEL(pwm_i, CLKCTRL_PWM_CLK_IJ_CTRL, 9, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_i, CLKCTRL_PWM_CLK_IJ_CTRL, 9);
>  static S4_COMP_DIV(pwm_i, CLKCTRL_PWM_CLK_IJ_CTRL, 0, 8);
>  static S4_COMP_GATE(pwm_i, CLKCTRL_PWM_CLK_IJ_CTRL, 8);
>  
> -static S4_COMP_SEL(pwm_j, CLKCTRL_PWM_CLK_IJ_CTRL, 25, 0x3, s4_pwm_parents);
> +static S4_PWM_SEL(pwm_j, CLKCTRL_PWM_CLK_IJ_CTRL, 25);
>  static S4_COMP_DIV(pwm_j, CLKCTRL_PWM_CLK_IJ_CTRL, 16, 8);
>  static S4_COMP_GATE(pwm_j, CLKCTRL_PWM_CLK_IJ_CTRL, 24);
>  
> -- 
> 2.56.0
>

-- 
Jerome

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

[PATCH 2/2] clk: meson: s4: propagate rate requests through the CEC 32k muxes

[christian.koever-draxl]

From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

The AO-CEC driver asks its core clock for 32768 Hz. On S4 that request
stops at ceca_32k_sel / cecb_32k_sel: both are muxes without
CLK_SET_RATE_PARENT, so it never reaches the dual divider that can
produce the rate, and probe fails with

  meson-ao-cec fe044000.cec: core clock set rate failed
  meson-ao-cec fe044000.cec: CEC controller registration failed

The equivalent G12A mux, ao_cec_sel, carries the flag, which is why CEC
works there. Add it to both S4 selectors.

The other parent of these muxes is the RTC clock, which must not be
reprogrammed on behalf of CEC, so boards should pin the mux parent to
the dual divider with assigned-clock-parents.

Only the CECA path was tested; CECB is the same defect in the
duplicated block.

Fixes: 57b55c76aaf1 ("clk: meson: S4: add support for Amlogic S4 SoC peripheral clock controller")
Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
---
 drivers/clk/meson/s4-peripherals.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
index 329cd526749e..8e303db6154c 100644
--- a/drivers/clk/meson/s4-peripherals.c
+++ b/drivers/clk/meson/s4-peripherals.c
@@ -407,6 +407,7 @@ static struct clk_regmap s4_ceca_32k_sel = {
 			&s4_rtc_clk.hw
 		},
 		.num_parents = 2,
+		.flags = CLK_SET_RATE_PARENT,
 	},
 };
 
@@ -514,6 +515,7 @@ static struct clk_regmap s4_cecb_32k_sel = {
 			&s4_rtc_clk.hw
 		},
 		.num_parents = 2,
+		.flags = CLK_SET_RATE_PARENT,
 	},
 };
 
-- 
2.56.0


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

Re: [PATCH 2/2] clk: meson: s4: propagate rate requests through the CEC 32k muxes

[Jerome Brunet]

On dim. 25 oct. 2026 at 20:18, christian.koever-draxl@student.uibk.ac.at wrote:

> From: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
>
> The AO-CEC driver asks its core clock for 32768 Hz. On S4 that request
> stops at ceca_32k_sel / cecb_32k_sel: both are muxes without
> CLK_SET_RATE_PARENT, so it never reaches the dual divider that can
> produce the rate, and probe fails with
>
>   meson-ao-cec fe044000.cec: core clock set rate failed
>   meson-ao-cec fe044000.cec: CEC controller registration failed
>
> The equivalent G12A mux, ao_cec_sel, carries the flag, which is why CEC
> works there. Add it to both S4 selectors.
>
> The other parent of these muxes is the RTC clock, which must not be
> reprogrammed on behalf of CEC, so boards should pin the mux parent to
> the dual divider with assigned-clock-parents.

Again, the change is OK in itself but if you are going to assign the
mux parents through assigned rate why, why don't you assign the parent
rate too ?

You are doing half the job in DT, half in the driver, that is kind of
odd.

>
> Only the CECA path was tested; CECB is the same defect in the
> duplicated block.
>
> Fixes: 57b55c76aaf1 ("clk: meson: S4: add support for Amlogic S4 SoC peripheral clock controller")
> Signed-off-by: Christian Stefan Kövér-Draxl <ckoever@outlook.de>
> ---
>  drivers/clk/meson/s4-peripherals.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
> index 329cd526749e..8e303db6154c 100644
> --- a/drivers/clk/meson/s4-peripherals.c
> +++ b/drivers/clk/meson/s4-peripherals.c
> @@ -407,6 +407,7 @@ static struct clk_regmap s4_ceca_32k_sel = {
>  			&s4_rtc_clk.hw
>  		},
>  		.num_parents = 2,
> +		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
>  
> @@ -514,6 +515,7 @@ static struct clk_regmap s4_cecb_32k_sel = {
>  			&s4_rtc_clk.hw
>  		},
>  		.num_parents = 2,
> +		.flags = CLK_SET_RATE_PARENT,
>  	},
>  };
>  
> -- 
> 2.56.0
>
>
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

-- 
Jerome

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