[PATCH v2] clk: meson: s4: keep the PWM clocks off vid_pll

[PATCH v2] clk: meson: s4: keep the PWM clocks off vid_pll

[christian.koever-draxl]

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

vid_pll is dedicated to the video path and is reprogrammed on display
mode changes, so it should not be offered to the PWM devices, even
though the hardware allows it. On an S905W2 board PWM channels ended up
on it, which broke the PWM CPU regulator and the 32.768 kHz clock of the
SDIO wifi chip.

Drop vid_pll from the PWM parents and keep the remaining inputs on their
register values with a mux table.

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>
---
Changes in v2:
- Rewrote the commit message (Jerome): no claims about how the PWM
  driver picks its parent, only that vid_pll belongs to the video path
  and should not be offered to the PWM devices.
- Dropped "clk: meson: s4: propagate rate requests through the CEC 32k
  muxes": the 32768 Hz rate is now set with assigned-clock-rates in the
  DT next to the parents (Jerome), so the series is down to this patch.
- Link to v1: https://lore.kernel.org/r/20261025191852.208226-2-christian.koever-draxl@student.uibk.ac.at

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.

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

diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
index ba41fcd90588..e5e5ff979efd 100644
--- a/drivers/clk/meson/s4-peripherals.c
+++ b/drivers/clk/meson/s4-peripherals.c
@@ -2588,51 +2588,61 @@ static struct clk_regmap s4_spicc0_en = {
 	},
 };
 
-/* PWM Clock */
+/*
+ * PWM Clock
+ *
+ * vid_pll (mux input 1) is left out: it belongs to the video path and is
+ * reprogrammed on display mode changes.
+ */
+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);
 

base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
-- 
2.56.0


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

Re: [PATCH v2] clk: meson: s4: keep the PWM clocks off vid_pll

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] clk: meson: s4: unused S4_COMP_SEL macro left behind by
  S4_PWM_SEL
--

commit 5666c90897d1a4bb5320168099b837ca1ef8216b
Author: Christian Stefan Kövér-Draxl <ckoever@outlook.de>

clk: meson: s4: keep the PWM clocks off vid_pll

This commit removes vid_pll from the S4 PWM clock parent list and adds a
mux table to preserve the hardware register indices for the remaining
inputs.

> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
> index ba41fcd905881..e5e5ff979efd2 100644
> --- a/drivers/clk/meson/s4-peripherals.c
> +++ b/drivers/clk/meson/s4-peripherals.c
> @@ -2588,51 +2588,61 @@ static struct clk_regmap s4_spicc0_en = {
>  	},
>  };
>  
> -/* PWM Clock */
> +/*
> + * PWM Clock
> + *
> + * vid_pll (mux input 1) is left out: it belongs to the video path and is
> + * reprogrammed on display mode changes.
> + */
> +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);

[Severity: Low]
Does replacing all S4_COMP_SEL() uses with S4_PWM_SEL() leave
S4_COMP_SEL() unused in drivers/clk/meson/s4-peripherals.c, triggering a
-Wunused-macros warning under W=2 builds? Either S4_COMP_SEL() could take
a table argument or the unused macro definition should be removed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106101827.46467-1-christian.koever-draxl@student.uibk.ac.at?part=1

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