From: Chuan Liu <chuan.liu@amlogic.com>

glitch free mux has two clock channels (channel 0 and channel 1) with
the same configuration. When the frequency needs to be changed, the two
channels ping-pong to ensure clock continuity and suppress glitch.

Channel 0 of glitch free mux is not only the clock source for the mux,
but also the working clock for glitch free mux. Therefore, when glitch
free mux switches, it is necessary to ensure that channel 0 has a clock
input, otherwise glitch free mux will not work and cannot switch to the
target channel.

Add flag CLK_SET_RATE_GATE to channels 0 and 1 to implement Ping-Pong
switchover to suppress glitch.

glitch free mux Add flag CLK_OPS_PARENT_ENABLE to ensure that channel 0
has clock input when switching channels.

Change-Id: I6fa6ff92f7b2e0a13dd7a27feb0e8568be3ca9f9
Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
---
 drivers/clk/meson/a1-peripherals.c | 12 ++++++------
 drivers/clk/meson/axg.c            | 16 ++++++++++------
 drivers/clk/meson/c3-peripherals.c |  6 +++---
 drivers/clk/meson/g12a.c           | 18 +++++++++++-------
 drivers/clk/meson/gxbb.c           | 18 +++++++++++-------
 drivers/clk/meson/s4-peripherals.c | 32 ++++++++++++++++----------------
 6 files changed, 57 insertions(+), 45 deletions(-)

diff --git a/drivers/clk/meson/a1-peripherals.c b/drivers/clk/meson/a1-peripherals.c
index 7aa6abb2eb1f..7f515e002adb 100644
--- a/drivers/clk/meson/a1-peripherals.c
+++ b/drivers/clk/meson/a1-peripherals.c
@@ -423,7 +423,7 @@ static struct clk_regmap dspa_a = {
 			&dspa_a_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -471,7 +471,7 @@ static struct clk_regmap dspa_b = {
 			&dspa_b_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -489,7 +489,7 @@ static struct clk_regmap dspa_sel = {
 			&dspa_b.hw,
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -569,7 +569,7 @@ static struct clk_regmap dspb_a = {
 			&dspb_a_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -617,7 +617,7 @@ static struct clk_regmap dspb_b = {
 			&dspb_b_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -635,7 +635,7 @@ static struct clk_regmap dspb_sel = {
 			&dspb_b.hw,
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 1b08daf579b2..e2d3266f4b45 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -1077,7 +1077,8 @@ static struct clk_regmap axg_vpu_0 = {
 		 * We want to avoid CCF to disable the VPU clock if
 		 * display has been set by Bootloader
 		 */
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1126,7 +1127,8 @@ static struct clk_regmap axg_vpu_1 = {
 		 * We want to avoid CCF to disable the VPU clock if
 		 * display has been set by Bootloader
 		 */
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1144,7 +1146,7 @@ static struct clk_regmap axg_vpu = {
 			&axg_vpu_1.hw
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -1194,7 +1196,8 @@ static struct clk_regmap axg_vapb_0 = {
 			&axg_vapb_0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1242,7 +1245,8 @@ static struct clk_regmap axg_vapb_1 = {
 			&axg_vapb_1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1260,7 +1264,7 @@ static struct clk_regmap axg_vapb_sel = {
 			&axg_vapb_1.hw
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
diff --git a/drivers/clk/meson/c3-peripherals.c b/drivers/clk/meson/c3-peripherals.c
index 7dcbf4ebee07..27343a73a521 100644
--- a/drivers/clk/meson/c3-peripherals.c
+++ b/drivers/clk/meson/c3-peripherals.c
@@ -1364,7 +1364,7 @@ static struct clk_regmap hcodec_0 = {
 			&hcodec_0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1411,7 +1411,7 @@ static struct clk_regmap hcodec_1 = {
 			&hcodec_1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1431,7 +1431,7 @@ static struct clk_regmap hcodec = {
 		.ops = &clk_regmap_mux_ops,
 		.parent_data = hcodec_parent_data,
 		.num_parents = ARRAY_SIZE(hcodec_parent_data),
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index d3539fe9f7af..21a25001e904 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -2746,7 +2746,8 @@ static struct clk_regmap g12a_vpu_0 = {
 		.ops = &clk_regmap_gate_ops,
 		.parent_hws = (const struct clk_hw *[]) { &g12a_vpu_0_div.hw },
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -2790,7 +2791,8 @@ static struct clk_regmap g12a_vpu_1 = {
 		.ops = &clk_regmap_gate_ops,
 		.parent_hws = (const struct clk_hw *[]) { &g12a_vpu_1_div.hw },
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -2812,7 +2814,7 @@ static struct clk_regmap g12a_vpu = {
 			&g12a_vpu_1.hw,
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -3035,7 +3037,8 @@ static struct clk_regmap g12a_vapb_0 = {
 			&g12a_vapb_0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -3083,7 +3086,8 @@ static struct clk_regmap g12a_vapb_1 = {
 			&g12a_vapb_1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -3105,7 +3109,7 @@ static struct clk_regmap g12a_vapb_sel = {
 			&g12a_vapb_1.hw,
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -4039,7 +4043,7 @@ static struct clk_regmap g12a_mali = {
 		.ops = &clk_regmap_mux_ops,
 		.parent_hws = g12a_mali_parent_hws,
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index 262c318edbd5..812b3e20c366 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -1132,7 +1132,7 @@ static struct clk_regmap gxbb_mali = {
 		.ops = &clk_regmap_mux_ops,
 		.parent_hws = gxbb_mali_parent_hws,
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -1543,7 +1543,8 @@ static struct clk_regmap gxbb_vpu_0 = {
 		.ops = &clk_regmap_gate_ops,
 		.parent_hws = (const struct clk_hw *[]) { &gxbb_vpu_0_div.hw },
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1591,7 +1592,8 @@ static struct clk_regmap gxbb_vpu_1 = {
 		.ops = &clk_regmap_gate_ops,
 		.parent_hws = (const struct clk_hw *[]) { &gxbb_vpu_1_div.hw },
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1613,7 +1615,7 @@ static struct clk_regmap gxbb_vpu = {
 			&gxbb_vpu_1.hw
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -1674,7 +1676,8 @@ static struct clk_regmap gxbb_vapb_0 = {
 			&gxbb_vapb_0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1726,7 +1729,8 @@ static struct clk_regmap gxbb_vapb_1 = {
 			&gxbb_vapb_1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
+		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
+			 CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1748,7 +1752,7 @@ static struct clk_regmap gxbb_vapb_sel = {
 			&gxbb_vapb_1.hw
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_NO_REPARENT,
+		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
index c930cf0614a0..cf10be40141d 100644
--- a/drivers/clk/meson/s4-peripherals.c
+++ b/drivers/clk/meson/s4-peripherals.c
@@ -1404,7 +1404,7 @@ static struct clk_regmap s4_mali_mux = {
 		.ops = &clk_regmap_mux_ops,
 		.parent_hws = s4_mali_parent_hws,
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -1466,7 +1466,7 @@ static struct clk_regmap s4_vdec_p0 = {
 			&s4_vdec_p0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1516,7 +1516,7 @@ static struct clk_regmap s4_vdec_p1 = {
 			&s4_vdec_p1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1536,7 +1536,7 @@ static struct clk_regmap s4_vdec_mux = {
 		.ops = &clk_regmap_mux_ops,
 		.parent_hws = s4_vdec_mux_parent_hws,
 		.num_parents = ARRAY_SIZE(s4_vdec_mux_parent_hws),
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -1586,7 +1586,7 @@ static struct clk_regmap s4_hevcf_p0 = {
 			&s4_hevcf_p0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1636,7 +1636,7 @@ static struct clk_regmap s4_hevcf_p1 = {
 			&s4_hevcf_p1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1656,7 +1656,7 @@ static struct clk_regmap s4_hevcf_mux = {
 		.ops = &clk_regmap_mux_ops,
 		.parent_hws = s4_hevcf_mux_parent_hws,
 		.num_parents = ARRAY_SIZE(s4_hevcf_mux_parent_hws),
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -1712,7 +1712,7 @@ static struct clk_regmap s4_vpu_0 = {
 		.ops = &clk_regmap_gate_ops,
 		.parent_hws = (const struct clk_hw *[]) { &s4_vpu_0_div.hw },
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1756,7 +1756,7 @@ static struct clk_regmap s4_vpu_1 = {
 		.ops = &clk_regmap_gate_ops,
 		.parent_hws = (const struct clk_hw *[]) { &s4_vpu_1_div.hw },
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1774,7 +1774,7 @@ static struct clk_regmap s4_vpu = {
 			&s4_vpu_1.hw,
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -1921,7 +1921,7 @@ static struct clk_regmap s4_vpu_clkc_p0 = {
 			&s4_vpu_clkc_p0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1969,7 +1969,7 @@ static struct clk_regmap s4_vpu_clkc_p1 = {
 			&s4_vpu_clkc_p1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -1989,7 +1989,7 @@ static struct clk_regmap s4_vpu_clkc_mux = {
 		.ops = &clk_regmap_mux_ops,
 		.parent_hws = s4_vpu_mux_parent_hws,
 		.num_parents = ARRAY_SIZE(s4_vpu_mux_parent_hws),
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 
@@ -2049,7 +2049,7 @@ static struct clk_regmap s4_vapb_0 = {
 			&s4_vapb_0_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -2097,7 +2097,7 @@ static struct clk_regmap s4_vapb_1 = {
 			&s4_vapb_1_div.hw
 		},
 		.num_parents = 1,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
 	},
 };
 
@@ -2115,7 +2115,7 @@ static struct clk_regmap s4_vapb = {
 			&s4_vapb_1.hw
 		},
 		.num_parents = 2,
-		.flags = CLK_SET_RATE_PARENT,
+		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
 	},
 };
 

-- 
2.42.0

Hello,

On Sun, Sep 29, 2024 at 8:10 AM Chuan Liu via B4 Relay
<devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>
> From: Chuan Liu <chuan.liu@amlogic.com>
>
> glitch free mux has two clock channels (channel 0 and channel 1) with
> the same configuration. When the frequency needs to be changed, the two
> channels ping-pong to ensure clock continuity and suppress glitch.
You describe the solution to this below:
> Add flag CLK_SET_RATE_GATE to channels 0 and 1 to implement Ping-Pong
> switchover to suppress glitch.
It would be great to have this change in a separate patch.
The clocks to which you're adding CLK_SET_RATE_GATE aren't switched at
runtime in mainline kernels (at least I think so).

> Channel 0 of glitch free mux is not only the clock source for the mux,
> but also the working clock for glitch free mux. Therefore, when glitch
> free mux switches, it is necessary to ensure that channel 0 has a clock
> input, otherwise glitch free mux will not work and cannot switch to the
> target channel.
[...]
> glitch free mux Add flag CLK_OPS_PARENT_ENABLE to ensure that channel 0
> has clock input when switching channels.
This describes a second problem. I think it's best to have this in a
separate commit/patch.
Also you're updating some mali clocks (e.g. on G12 and GX) but not all
of them (Meson8b for example is missing).

I still have some questions to the CLK_OPS_PARENT_ENABLE approach -
please share your findings on this.

There's multiple clocks involved in a glitch-free mux hierarchy:
- a number of clock inputs (e.g. fclk, xtal, ...)
- two muxes (one for every channel of the glitch-free mux)
- two dividers (one for every channel of the glitch-free mux)
- two gates (one for every channel of the glitch-free mux)
- the glitch-free mux

When switching from channel 0 (which is active and enabled) CCF
(common clock framework) will:
a) on channel 1:
- find the best input clock
- choose the best input clock in the mux
- set the divider
- enable the gate
b) switch the glitch-free mux
c) on channel 2:
- disable the gate

To me it's not clear at which level the problem occurs (glitch-free
mux, gate, divider, mux, input clock).
Also I don't understand why enabling the clocks with
CLK_OPS_PARENT_ENABLE solves any problem since CCF is doing things
automatically for us.
Can you please explain (preferably with an example) what problem is
solved with this approach?

Last but not least: if we're running into bugs when
CLK_OPS_PARENT_ENABLE is missing then that patch should carry a Fixes
tag.


Best regards,
Martin

Hi Martin,


On 2024/10/1 4:08, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
>
> Hello,
>
> On Sun, Sep 29, 2024 at 8:10 AM Chuan Liu via B4 Relay
> <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>> From: Chuan Liu <chuan.liu@amlogic.com>
>>
>> glitch free mux has two clock channels (channel 0 and channel 1) with
>> the same configuration. When the frequency needs to be changed, the two
>> channels ping-pong to ensure clock continuity and suppress glitch.
> You describe the solution to this below:
>> Add flag CLK_SET_RATE_GATE to channels 0 and 1 to implement Ping-Pong
>> switchover to suppress glitch.
> It would be great to have this change in a separate patch.
> The clocks to which you're adding CLK_SET_RATE_GATE aren't switched at
> runtime in mainline kernels (at least I think so).


Okay, I will separate it into two patches and submit it in the next version.


>
>> Channel 0 of glitch free mux is not only the clock source for the mux,
>> but also the working clock for glitch free mux. Therefore, when glitch
>> free mux switches, it is necessary to ensure that channel 0 has a clock
>> input, otherwise glitch free mux will not work and cannot switch to the
>> target channel.
> [...]
>> glitch free mux Add flag CLK_OPS_PARENT_ENABLE to ensure that channel 0
>> has clock input when switching channels.
> This describes a second problem. I think it's best to have this in a
> separate commit/patch.
> Also you're updating some mali clocks (e.g. on G12 and GX) but not all
> of them (Meson8b for example is missing).


Yes, M8 missed it, I will complete it in the next version.


>
> I still have some questions to the CLK_OPS_PARENT_ENABLE approach -
> please share your findings on this.
>
> There's multiple clocks involved in a glitch-free mux hierarchy:
> - a number of clock inputs (e.g. fclk, xtal, ...)
> - two muxes (one for every channel of the glitch-free mux)
> - two dividers (one for every channel of the glitch-free mux)
> - two gates (one for every channel of the glitch-free mux)
> - the glitch-free mux
>
> When switching from channel 0 (which is active and enabled) CCF
> (common clock framework) will:
> a) on channel 1:
> - find the best input clock
> - choose the best input clock in the mux
> - set the divider
> - enable the gate
> b) switch the glitch-free mux
> c) on channel 2:
> - disable the gate
>
> To me it's not clear at which level the problem occurs (glitch-free
> mux, gate, divider, mux, input clock).
> Also I don't understand why enabling the clocks with
> CLK_OPS_PARENT_ENABLE solves any problem since CCF is doing things
> automatically for us.
> Can you please explain (preferably with an example) what problem is
> solved with this approach?


If CLK_OPS_PARENT_ENABLE is configured in mux, 'new_parent' and
'old_parent' will be enabled first when __clk_set_parent_before() is
called. And disable them at __clk_set_parent_after(). Our glitch free
only has two clock sources, so adding this flag ensures that both
channels 0 and 1 are enabled when mux switches.

In fact, we just need to make sure that channel 0 is enabled. The
purpose of CLK_OPS_PARENT_ENABLE may not be to solve our situation,
but adding this flag does solve our current problem.


>
> Last but not least: if we're running into bugs when
> CLK_OPS_PARENT_ENABLE is missing then that patch should carry a Fixes
> tag.


Thanks for the heads-up. I'll keep an eye on it in the next version.


>
> Best regards,
> Martin

On Tue 08 Oct 2024 at 13:44, Chuan Liu <chuan.liu@amlogic.com> wrote:

> Hi Martin,
>
>
> On 2024/10/1 4:08, Martin Blumenstingl wrote:
>> [ EXTERNAL EMAIL ]
>>
>> Hello,
>>
>> On Sun, Sep 29, 2024 at 8:10 AM Chuan Liu via B4 Relay
>> <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>>> From: Chuan Liu <chuan.liu@amlogic.com>
>>>
>>> glitch free mux has two clock channels (channel 0 and channel 1) with
>>> the same configuration. When the frequency needs to be changed, the two
>>> channels ping-pong to ensure clock continuity and suppress glitch.
>> You describe the solution to this below:
>>> Add flag CLK_SET_RATE_GATE to channels 0 and 1 to implement Ping-Pong
>>> switchover to suppress glitch.
>> It would be great to have this change in a separate patch.
>> The clocks to which you're adding CLK_SET_RATE_GATE aren't switched at
>> runtime in mainline kernels (at least I think so).
>
>
> Okay, I will separate it into two patches and submit it in the next version.
>
>
>>
>>> Channel 0 of glitch free mux is not only the clock source for the mux,
>>> but also the working clock for glitch free mux. Therefore, when glitch
>>> free mux switches, it is necessary to ensure that channel 0 has a clock
>>> input, otherwise glitch free mux will not work and cannot switch to the
>>> target channel.
>> [...]
>>> glitch free mux Add flag CLK_OPS_PARENT_ENABLE to ensure that channel 0
>>> has clock input when switching channels.
>> This describes a second problem. I think it's best to have this in a
>> separate commit/patch.
>> Also you're updating some mali clocks (e.g. on G12 and GX) but not all
>> of them (Meson8b for example is missing).
>
>
> Yes, M8 missed it, I will complete it in the next version.
>
>
>>
>> I still have some questions to the CLK_OPS_PARENT_ENABLE approach -
>> please share your findings on this.
>>
>> There's multiple clocks involved in a glitch-free mux hierarchy:
>> - a number of clock inputs (e.g. fclk, xtal, ...)
>> - two muxes (one for every channel of the glitch-free mux)
>> - two dividers (one for every channel of the glitch-free mux)
>> - two gates (one for every channel of the glitch-free mux)
>> - the glitch-free mux
>>
>> When switching from channel 0 (which is active and enabled) CCF
>> (common clock framework) will:
>> a) on channel 1:
>> - find the best input clock
>> - choose the best input clock in the mux
>> - set the divider
>> - enable the gate
>> b) switch the glitch-free mux
>> c) on channel 2:
>> - disable the gate
>>
>> To me it's not clear at which level the problem occurs (glitch-free
>> mux, gate, divider, mux, input clock).
>> Also I don't understand why enabling the clocks with
>> CLK_OPS_PARENT_ENABLE solves any problem since CCF is doing things
>> automatically for us.
>> Can you please explain (preferably with an example) what problem is
>> solved with this approach?
>
>
> If CLK_OPS_PARENT_ENABLE is configured in mux, 'new_parent' and
> 'old_parent' will be enabled first when __clk_set_parent_before() is
> called. And disable them at __clk_set_parent_after(). Our glitch free
> only has two clock sources, so adding this flag ensures that both
> channels 0 and 1 are enabled when mux switches.
>
> In fact, we just need to make sure that channel 0 is enabled. The
> purpose of CLK_OPS_PARENT_ENABLE may not be to solve our situation,
> but adding this flag does solve our current problem.

This is last point is important.

It is OK to use a side effect of CLK_OPS_PARENT_ENABLE but it needs to
be documented somehow, so what really matters is still known 2y from now.

Make sure the information appears in the code comments at least once.

>
>
>>
>> Last but not least: if we're running into bugs when
>> CLK_OPS_PARENT_ENABLE is missing then that patch should carry a Fixes
>> tag.
>
>
> Thanks for the heads-up. I'll keep an eye on it in the next version.
>
>
>>
>> Best regards,
>> Martin

-- 
Jerome

Hi Jerome & Martin:

Sorry for the imprecise description of the glitch-free mux earlier.

Recently, while troubleshooting a CPU hang issue caused by glitches,
I realized there was a discrepancy from our previous understanding,
so I'd like to clarify it here.

On 10/8/2024 2:02 PM, Jerome Brunet wrote:

> [ EXTERNAL EMAIL ]
>
> On Tue 08 Oct 2024 at 13:44, Chuan Liu <chuan.liu@amlogic.com> wrote:
>
>> Hi Martin,
>>
>>
>> On 2024/10/1 4:08, Martin Blumenstingl wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> Hello,
>>>
>>> On Sun, Sep 29, 2024 at 8:10 AM Chuan Liu via B4 Relay
>>> <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>>>> From: Chuan Liu <chuan.liu@amlogic.com>
>>>>
>>>> glitch free mux has two clock channels (channel 0 and channel 1) with
>>>> the same configuration. When the frequency needs to be changed, the two
>>>> channels ping-pong to ensure clock continuity and suppress glitch.
>>> You describe the solution to this below:
>>>> Add flag CLK_SET_RATE_GATE to channels 0 and 1 to implement Ping-Pong
>>>> switchover to suppress glitch.
>>> It would be great to have this change in a separate patch.
>>> The clocks to which you're adding CLK_SET_RATE_GATE aren't switched at
>>> runtime in mainline kernels (at least I think so).
>>
>> Okay, I will separate it into two patches and submit it in the next version.
>>
>>
>>>> Channel 0 of glitch free mux is not only the clock source for the mux,
>>>> but also the working clock for glitch free mux. Therefore, when glitch
>>>> free mux switches, it is necessary to ensure that channel 0 has a clock
>>>> input, otherwise glitch free mux will not work and cannot switch to the
>>>> target channel.
>>> [...]
>>>> glitch free mux Add flag CLK_OPS_PARENT_ENABLE to ensure that channel 0
>>>> has clock input when switching channels.
>>> This describes a second problem. I think it's best to have this in a
>>> separate commit/patch.
>>> Also you're updating some mali clocks (e.g. on G12 and GX) but not all
>>> of them (Meson8b for example is missing).
>>
>> Yes, M8 missed it, I will complete it in the next version.
>>
>>
>>> I still have some questions to the CLK_OPS_PARENT_ENABLE approach -
>>> please share your findings on this.
>>>
>>> There's multiple clocks involved in a glitch-free mux hierarchy:
>>> - a number of clock inputs (e.g. fclk, xtal, ...)
>>> - two muxes (one for every channel of the glitch-free mux)
>>> - two dividers (one for every channel of the glitch-free mux)
>>> - two gates (one for every channel of the glitch-free mux)
>>> - the glitch-free mux
>>>
>>> When switching from channel 0 (which is active and enabled) CCF
>>> (common clock framework) will:
>>> a) on channel 1:
>>> - find the best input clock
>>> - choose the best input clock in the mux
>>> - set the divider
>>> - enable the gate
>>> b) switch the glitch-free mux
>>> c) on channel 2:
>>> - disable the gate
>>>
>>> To me it's not clear at which level the problem occurs (glitch-free
>>> mux, gate, divider, mux, input clock).
>>> Also I don't understand why enabling the clocks with
>>> CLK_OPS_PARENT_ENABLE solves any problem since CCF is doing things
>>> automatically for us.
>>> Can you please explain (preferably with an example) what problem is
>>> solved with this approach?
>>
>> If CLK_OPS_PARENT_ENABLE is configured in mux, 'new_parent' and
>> 'old_parent' will be enabled first when __clk_set_parent_before() is
>> called. And disable them at __clk_set_parent_after(). Our glitch free
>> only has two clock sources, so adding this flag ensures that both
>> channels 0 and 1 are enabled when mux switches.
>>
>> In fact, we just need to make sure that channel 0 is enabled. The

It is indeed necessary to enable the glitch-free mux on *both* channel 0 
and channel 1 to allow proper switching. multiple original channel clock 
cycles and new channel clock cycles will be filtered during mux 
switching. An example of the clock waveform is shown below: __ __ __ __ 
__ __ __ __ ori: ↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑ ^ 1 * cycle 
original channel. _ _ _ _ _ _ _ _ _ _ _ _ new: ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ 
|_↑ |_↑ |_↑ |_↑ |_↑ |_↑ ^ 5 * cycles new channel. __ __ _ _ _ _ out: ↑ 
|__↑ |______________________↑ |_↑ |_↑ |_↑ |_↑ ^ ^ start switching mux. 
switch to new channel.

>> purpose of CLK_OPS_PARENT_ENABLE may not be to solve our situation,
>> but adding this flag does solve our current problem.
> This is last point is important.
>
> It is OK to use a side effect of CLK_OPS_PARENT_ENABLE but it needs to
> be documented somehow, so what really matters is still known 2y from now.
>
> Make sure the information appears in the code comments at least once.
>
>>
>>> Last but not least: if we're running into bugs when
>>> CLK_OPS_PARENT_ENABLE is missing then that patch should carry a Fixes
>>> tag.
>>
>> Thanks for the heads-up. I'll keep an eye on it in the next version.
>>
>>
>>> Best regards,
>>> Martin
> --
> Jerome

On 9/28/2025 2:05 PM, Chuan Liu wrote:
> Hi Jerome & Martin:
>
> Sorry for the imprecise description of the glitch-free mux earlier.
>
> Recently, while troubleshooting a CPU hang issue caused by glitches,
> I realized there was a discrepancy from our previous understanding,
> so I'd like to clarify it here.
>
> On 10/8/2024 2:02 PM, Jerome Brunet wrote:
>
>> [ EXTERNAL EMAIL ]
>>
>> On Tue 08 Oct 2024 at 13:44, Chuan Liu <chuan.liu@amlogic.com> wrote:
>>
>>> Hi Martin,
>>>
>>>
>>> On 2024/10/1 4:08, Martin Blumenstingl wrote:
>>>> [ EXTERNAL EMAIL ]
>>>>
>>>> Hello,
>>>>
>>>> On Sun, Sep 29, 2024 at 8:10 AM Chuan Liu via B4 Relay
>>>> <devnull+chuan.liu.amlogic.com@kernel.org> wrote:
>>>>> From: Chuan Liu <chuan.liu@amlogic.com>
>>>>>
>>>>> glitch free mux has two clock channels (channel 0 and channel 1) with
>>>>> the same configuration. When the frequency needs to be changed, 
>>>>> the two
>>>>> channels ping-pong to ensure clock continuity and suppress glitch.
>>>> You describe the solution to this below:
>>>>> Add flag CLK_SET_RATE_GATE to channels 0 and 1 to implement Ping-Pong
>>>>> switchover to suppress glitch.
>>>> It would be great to have this change in a separate patch.
>>>> The clocks to which you're adding CLK_SET_RATE_GATE aren't switched at
>>>> runtime in mainline kernels (at least I think so).
>>>
>>> Okay, I will separate it into two patches and submit it in the next 
>>> version.
>>>
>>>
>>>>> Channel 0 of glitch free mux is not only the clock source for the 
>>>>> mux,
>>>>> but also the working clock for glitch free mux. Therefore, when 
>>>>> glitch
>>>>> free mux switches, it is necessary to ensure that channel 0 has a 
>>>>> clock
>>>>> input, otherwise glitch free mux will not work and cannot switch 
>>>>> to the
>>>>> target channel.
>>>> [...]
>>>>> glitch free mux Add flag CLK_OPS_PARENT_ENABLE to ensure that 
>>>>> channel 0
>>>>> has clock input when switching channels.
>>>> This describes a second problem. I think it's best to have this in a
>>>> separate commit/patch.
>>>> Also you're updating some mali clocks (e.g. on G12 and GX) but not all
>>>> of them (Meson8b for example is missing).
>>>
>>> Yes, M8 missed it, I will complete it in the next version.
>>>
>>>
>>>> I still have some questions to the CLK_OPS_PARENT_ENABLE approach -
>>>> please share your findings on this.
>>>>
>>>> There's multiple clocks involved in a glitch-free mux hierarchy:
>>>> - a number of clock inputs (e.g. fclk, xtal, ...)
>>>> - two muxes (one for every channel of the glitch-free mux)
>>>> - two dividers (one for every channel of the glitch-free mux)
>>>> - two gates (one for every channel of the glitch-free mux)
>>>> - the glitch-free mux
>>>>
>>>> When switching from channel 0 (which is active and enabled) CCF
>>>> (common clock framework) will:
>>>> a) on channel 1:
>>>> - find the best input clock
>>>> - choose the best input clock in the mux
>>>> - set the divider
>>>> - enable the gate
>>>> b) switch the glitch-free mux
>>>> c) on channel 2:
>>>> - disable the gate
>>>>
>>>> To me it's not clear at which level the problem occurs (glitch-free
>>>> mux, gate, divider, mux, input clock).
>>>> Also I don't understand why enabling the clocks with
>>>> CLK_OPS_PARENT_ENABLE solves any problem since CCF is doing things
>>>> automatically for us.
>>>> Can you please explain (preferably with an example) what problem is
>>>> solved with this approach?
>>>
>>> If CLK_OPS_PARENT_ENABLE is configured in mux, 'new_parent' and
>>> 'old_parent' will be enabled first when __clk_set_parent_before() is
>>> called. And disable them at __clk_set_parent_after(). Our glitch free
>>> only has two clock sources, so adding this flag ensures that both
>>> channels 0 and 1 are enabled when mux switches.
>>>
>>> In fact, we just need to make sure that channel 0 is enabled. The
>
> It is indeed necessary to enable the glitch-free mux on *both* channel 
> 0 and channel 1 to allow proper switching. multiple original channel 
> clock cycles and new channel clock cycles will be filtered during mux 
> switching. An example of the clock waveform is shown below: __ __ __ 
> __ __ __ __ __ ori: ↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑ ^ 1 * 
> cycle original channel. _ _ _ _ _ _ _ _ _ _ _ _ new: ↑ |_↑ |_↑ |_↑ |_↑ 
> |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ ^ 5 * cycles new channel. __ __ _ _ _ 
> _ out: ↑ |__↑ |______________________↑ |_↑ |_↑ |_↑ |_↑ ^ ^ start 
> switching mux. switch to new channel.
>

Sorry, there seems to be something wrong with the following format parsing.


An example of the clock waveform is shown below:


        __    __    __    __    __    __    __    __
ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
                   ^
                   1 * cycle original channel.
        _   _   _   _   _   _   _   _   _   _   _   _
new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
                                       ^
                                       5 * cycles new channel.
        __    __                        _   _   _   _
out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
              ^                        ^
              start switching mux.     switch to new channel.


>>> purpose of CLK_OPS_PARENT_ENABLE may not be to solve our situation,
>>> but adding this flag does solve our current problem.
>> This is last point is important.
>>
>> It is OK to use a side effect of CLK_OPS_PARENT_ENABLE but it needs to
>> be documented somehow, so what really matters is still known 2y from 
>> now.
>>
>> Make sure the information appears in the code comments at least once.
>>
>>>
>>>> Last but not least: if we're running into bugs when
>>>> CLK_OPS_PARENT_ENABLE is missing then that patch should carry a Fixes
>>>> tag.
>>>
>>> Thanks for the heads-up. I'll keep an eye on it in the next version.
>>>
>>>
>>>> Best regards,
>>>> Martin
>> -- 
>> Jerome

Hello,

On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
>
>
> On 9/28/2025 2:05 PM, Chuan Liu wrote:
> > Hi Jerome & Martin:
> >
> > Sorry for the imprecise description of the glitch-free mux earlier.
> >
> > Recently, while troubleshooting a CPU hang issue caused by glitches,
> > I realized there was a discrepancy from our previous understanding,
> > so I'd like to clarify it here.
[...]
> An example of the clock waveform is shown below:
>
>
>         __    __    __    __    __    __    __    __
> ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
>                    ^
>                    1 * cycle original channel.
>         _   _   _   _   _   _   _   _   _   _   _   _
> new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
>                                        ^
>                                        5 * cycles new channel.
>         __    __                        _   _   _   _
> out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
>               ^                        ^
>               start switching mux.     switch to new channel.
Thank you for the detailed report!
This is indeed problematic behavior. I guess the result is somewhat
random: depending on load (power draw), silicon lottery (quality),
temperature, voltage supply, ... - one may or may not see crashes
caused by this.

Based on the previous discussion on this topic, my suggestion is to
split the original patch:
- one to add CLK_SET_RATE_GATE where needed (I think the meson8b.c
driver already has this where needed) to actually enable the
glitch-free mux behavior
- another one with the CLK_OPS_PARENT_ENABLE change (meson8b.c would
also need to be updated) to prevent the glitch-free mux from
temporarily outputting an electrical low signal. Jerome also asked to
document the behavior so we don't forget why we set this flag

Both patches should get the proper "Fixes" tags.
I think it would also be great if you could include the waveform
example in at least the commit message as it helps understand the
problem.

Let's also give Jerome some time to comment before you send patches.


Best regards,
Martin

On Sun 28 Sep 2025 at 22:55, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:

> Hello,
>
> On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
>>
>>
>> On 9/28/2025 2:05 PM, Chuan Liu wrote:
>> > Hi Jerome & Martin:
>> >
>> > Sorry for the imprecise description of the glitch-free mux earlier.
>> >
>> > Recently, while troubleshooting a CPU hang issue caused by glitches,
>> > I realized there was a discrepancy from our previous understanding,
>> > so I'd like to clarify it here.
> [...]
>> An example of the clock waveform is shown below:
>>
>>
                     1                  2
                     v                  v
>>         __    __    __    __    __    __    __    __
>> ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
>>                    ^
>>                    1 * cycle original channel.
>>         _   _   _   _   _   _   _   _   _   _   _   _
>> new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
>>                                        ^
>>                                        5 * cycles new channel.
>>         __    __                        _   _   _   _
>> out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
>>               ^                        ^
>>               start switching mux.     switch to new channel.

Ok ... but when is it safe to disable the "ori" clock ?
Can you do it at '1' already ? or do you have to wait for '2' ?


> Thank you for the detailed report!
> This is indeed problematic behavior. I guess the result is somewhat
> random: depending on load (power draw), silicon lottery (quality),
> temperature, voltage supply, ... - one may or may not see crashes
> caused by this.
>
> Based on the previous discussion on this topic, my suggestion is to
> split the original patch:
> - one to add CLK_SET_RATE_GATE where needed (I think the meson8b.c
> driver already has this where needed) to actually enable the
> glitch-free mux behavior
> - another one with the CLK_OPS_PARENT_ENABLE change (meson8b.c would
> also need to be updated) to prevent the glitch-free mux from
> temporarily outputting an electrical low signal. Jerome also asked to
> document the behavior so we don't forget why we set this flag

Yes please split the changes and visit all the controllers shipping this
type of muxes.

>
> Both patches should get the proper "Fixes" tags.

... and proper fixes tag maybe different depending on the controller so
there might more that just 2 changes.

> I think it would also be great if you could include the waveform
> example in at least the commit message as it helps understand the
> problem.
>
> Let's also give Jerome some time to comment before you send patches.
>
>
> Best regards,
> Martin

-- 
Jerome

On 9/29/2025 4:48 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Sun 28 Sep 2025 at 22:55, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote:
>
>> Hello,
>>
>> On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
>>>
>>> On 9/28/2025 2:05 PM, Chuan Liu wrote:
>>>> Hi Jerome & Martin:
>>>>
>>>> Sorry for the imprecise description of the glitch-free mux earlier.
>>>>
>>>> Recently, while troubleshooting a CPU hang issue caused by glitches,
>>>> I realized there was a discrepancy from our previous understanding,
>>>> so I'd like to clarify it here.
>> [...]
>>> An example of the clock waveform is shown below:
>>>
>>>
>                       1                  2
>                       v                  v
>>>          __    __    __    __    __    __    __    __
>>> ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
>>>                     ^
>>>                     1 * cycle original channel.
>>>          _   _   _   _   _   _   _   _   _   _   _   _
>>> new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
>>>                                         ^
>>>                                         5 * cycles new channel.
>>>          __    __                        _   _   _   _
>>> out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
>>>                ^                        ^
>>>                start switching mux.     switch to new channel.
> Ok ... but when is it safe to disable the "ori" clock ?
> Can you do it at '1' already ? or do you have to wait for '2' ?


It should wait for "2", because there is a state machine in the
glitch-free mux, this state machine is driven by the working clock
provided by its channel 0.


>
>> Thank you for the detailed report!
>> This is indeed problematic behavior. I guess the result is somewhat
>> random: depending on load (power draw), silicon lottery (quality),
>> temperature, voltage supply, ... - one may or may not see crashes
>> caused by this.
>>
>> Based on the previous discussion on this topic, my suggestion is to
>> split the original patch:
>> - one to add CLK_SET_RATE_GATE where needed (I think the meson8b.c
>> driver already has this where needed) to actually enable the
>> glitch-free mux behavior
>> - another one with the CLK_OPS_PARENT_ENABLE change (meson8b.c would
>> also need to be updated) to prevent the glitch-free mux from
>> temporarily outputting an electrical low signal. Jerome also asked to
>> document the behavior so we don't forget why we set this flag
> Yes please split the changes and visit all the controllers shipping this
> type of muxes.
>
>> Both patches should get the proper "Fixes" tags.
> ... and proper fixes tag maybe different depending on the controller so
> there might more that just 2 changes.
>
>> I think it would also be great if you could include the waveform
>> example in at least the commit message as it helps understand the
>> problem.
>>
>> Let's also give Jerome some time to comment before you send patches.
>>
>>
>> Best regards,
>> Martin
> --
> Jerome

On Mon 29 Sep 2025 at 17:31, Chuan Liu <chuan.liu@amlogic.com> wrote:

> On 9/29/2025 4:48 PM, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On Sun 28 Sep 2025 at 22:55, Martin Blumenstingl
>> <martin.blumenstingl@googlemail.com> wrote:
>>
>>> Hello,
>>>
>>> On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
>>>>
>>>> On 9/28/2025 2:05 PM, Chuan Liu wrote:
>>>>> Hi Jerome & Martin:
>>>>>
>>>>> Sorry for the imprecise description of the glitch-free mux earlier.
>>>>>
>>>>> Recently, while troubleshooting a CPU hang issue caused by glitches,
>>>>> I realized there was a discrepancy from our previous understanding,
>>>>> so I'd like to clarify it here.
>>> [...]
>>>> An example of the clock waveform is shown below:
>>>>
>>>>
>>                       1                  2
>>                       v                  v
>>>>          __    __    __    __    __    __    __    __
>>>> ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
>>>>                     ^
>>>>                     1 * cycle original channel.
>>>>          _   _   _   _   _   _   _   _   _   _   _   _
>>>> new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
>>>>                                         ^
>>>>                                         5 * cycles new channel.
>>>>          __    __                        _   _   _   _
>>>> out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
>>>>                ^                        ^
>>>>                start switching mux.     switch to new channel.
>> Ok ... but when is it safe to disable the "ori" clock ?
>> Can you do it at '1' already ? or do you have to wait for '2' ?
>
>
> It should wait for "2", because there is a state machine in the
> glitch-free mux, this state machine is driven by the working clock
> provided by its channel 0.

Then I don't think the 2 flags are enough to make it safe

Nothing guarantees that CCF will wait for those 5 cycles to turn off
the clock noted 'ori' above.

I think you need new specific ops for this mux

Something that would
* protect both parents before changing the mux
* do the actual change
* wait for it to settle
* remove the protection

>
>
>>
>>> Thank you for the detailed report!
>>> This is indeed problematic behavior. I guess the result is somewhat
>>> random: depending on load (power draw), silicon lottery (quality),
>>> temperature, voltage supply, ... - one may or may not see crashes
>>> caused by this.
>>>
>>> Based on the previous discussion on this topic, my suggestion is to
>>> split the original patch:
>>> - one to add CLK_SET_RATE_GATE where needed (I think the meson8b.c
>>> driver already has this where needed) to actually enable the
>>> glitch-free mux behavior
>>> - another one with the CLK_OPS_PARENT_ENABLE change (meson8b.c would
>>> also need to be updated) to prevent the glitch-free mux from
>>> temporarily outputting an electrical low signal. Jerome also asked to
>>> document the behavior so we don't forget why we set this flag
>> Yes please split the changes and visit all the controllers shipping this
>> type of muxes.
>>
>>> Both patches should get the proper "Fixes" tags.
>> ... and proper fixes tag maybe different depending on the controller so
>> there might more that just 2 changes.
>>
>>> I think it would also be great if you could include the waveform
>>> example in at least the commit message as it helps understand the
>>> problem.
>>>
>>> Let's also give Jerome some time to comment before you send patches.
>>>
>>>
>>> Best regards,
>>> Martin
>> --
>> Jerome

-- 
Jerome

Hi Jerome,


On 9/29/2025 8:55 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Mon 29 Sep 2025 at 17:31, Chuan Liu <chuan.liu@amlogic.com> wrote:
>
>> On 9/29/2025 4:48 PM, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Sun 28 Sep 2025 at 22:55, Martin Blumenstingl
>>> <martin.blumenstingl@googlemail.com> wrote:
>>>
>>>> Hello,
>>>>
>>>> On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
>>>>> On 9/28/2025 2:05 PM, Chuan Liu wrote:
>>>>>> Hi Jerome & Martin:
>>>>>>
>>>>>> Sorry for the imprecise description of the glitch-free mux earlier.
>>>>>>
>>>>>> Recently, while troubleshooting a CPU hang issue caused by glitches,
>>>>>> I realized there was a discrepancy from our previous understanding,
>>>>>> so I'd like to clarify it here.
>>>> [...]
>>>>> An example of the clock waveform is shown below:
>>>>>
>>>>>
>>>                        1                  2
>>>                        v                  v
>>>>>           __    __    __    __    __    __    __    __
>>>>> ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
>>>>>                      ^
>>>>>                      1 * cycle original channel.
>>>>>           _   _   _   _   _   _   _   _   _   _   _   _
>>>>> new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
>>>>>                                          ^
>>>>>                                          5 * cycles new channel.
>>>>>           __    __                        _   _   _   _
>>>>> out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
>>>>>                 ^                        ^
>>>>>                 start switching mux.     switch to new channel.
>>> Ok ... but when is it safe to disable the "ori" clock ?
>>> Can you do it at '1' already ? or do you have to wait for '2' ?
>>
>> It should wait for "2", because there is a state machine in the
>> glitch-free mux, this state machine is driven by the working clock
>> provided by its channel 0.
> Then I don't think the 2 flags are enough to make it safe
>
> Nothing guarantees that CCF will wait for those 5 cycles to turn off
> the clock noted 'ori' above.
>
> I think you need new specific ops for this mux
>
> Something that would
> * protect both parents before changing the mux
> * do the actual change
> * wait for it to settle
> * remove the protection


Got it, thanks for your suggestion. I will try to address it in this way
moving forward, but it may take some time, as I'm currently working on
the bring-up of a new SoC.

By the way, On the new SoC, we have standardized the clock tree and PLL
design at the chip level. As a result, future drivers won't need to
include a large amount of redundant register bit definitions. This
should also help improve the generality, memory footprint, and
performance of our drivers.


>>
>>>> Thank you for the detailed report!
>>>> This is indeed problematic behavior. I guess the result is somewhat
>>>> random: depending on load (power draw), silicon lottery (quality),
>>>> temperature, voltage supply, ... - one may or may not see crashes
>>>> caused by this.
>>>>
>>>> Based on the previous discussion on this topic, my suggestion is to
>>>> split the original patch:
>>>> - one to add CLK_SET_RATE_GATE where needed (I think the meson8b.c
>>>> driver already has this where needed) to actually enable the
>>>> glitch-free mux behavior
>>>> - another one with the CLK_OPS_PARENT_ENABLE change (meson8b.c would
>>>> also need to be updated) to prevent the glitch-free mux from
>>>> temporarily outputting an electrical low signal. Jerome also asked to
>>>> document the behavior so we don't forget why we set this flag
>>> Yes please split the changes and visit all the controllers shipping this
>>> type of muxes.
>>>
>>>> Both patches should get the proper "Fixes" tags.
>>> ... and proper fixes tag maybe different depending on the controller so
>>> there might more that just 2 changes.
>>>
>>>> I think it would also be great if you could include the waveform
>>>> example in at least the commit message as it helps understand the
>>>> problem.
>>>>
>>>> Let's also give Jerome some time to comment before you send patches.
>>>>
>>>>
>>>> Best regards,
>>>> Martin
>>> --
>>> Jerome
> --
> Jerome

Hi Martin:

Thanks for the detailed explanation.


On 9/29/2025 4:55 AM, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
>
> Hello,
>
> On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
>>
>> On 9/28/2025 2:05 PM, Chuan Liu wrote:
>>> Hi Jerome & Martin:
>>>
>>> Sorry for the imprecise description of the glitch-free mux earlier.
>>>
>>> Recently, while troubleshooting a CPU hang issue caused by glitches,
>>> I realized there was a discrepancy from our previous understanding,
>>> so I'd like to clarify it here.
> [...]
>> An example of the clock waveform is shown below:
>>
>>
>>          __    __    __    __    __    __    __    __
>> ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
>>                     ^
>>                     1 * cycle original channel.
>>          _   _   _   _   _   _   _   _   _   _   _   _
>> new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
>>                                         ^
>>                                         5 * cycles new channel.
>>          __    __                        _   _   _   _
>> out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
>>                ^                        ^
>>                start switching mux.     switch to new channel.
> Thank you for the detailed report!
> This is indeed problematic behavior. I guess the result is somewhat
> random: depending on load (power draw), silicon lottery (quality),
> temperature, voltage supply, ... - one may or may not see crashes
> caused by this.


Yes, our glitch-free mux is designed to prevent glitches caused by
excessively short high or low levels in the clock output.


>
> Based on the previous discussion on this topic, my suggestion is to
> split the original patch:
> - one to add CLK_SET_RATE_GATE where needed (I think the meson8b.c
> driver already has this where needed) to actually enable the
> glitch-free mux behavior
> - another one with the CLK_OPS_PARENT_ENABLE change (meson8b.c would
> also need to be updated) to prevent the glitch-free mux from
> temporarily outputting an electrical low signal. Jerome also asked to
> document the behavior so we don't forget why we set this flag
>
> Both patches should get the proper "Fixes" tags.
> I think it would also be great if you could include the waveform
> example in at least the commit message as it helps understand the
> problem.
>
> Let's also give Jerome some time to comment before you send patches.


A V2 version was submitted later with changes based on your suggestions.
Regarding the "Fixes" tag, Jerome had proposed some modifications.

[PATCH v2 0/3] clk: Fix issues related to CLK_IGNORE_UNUSED failures and 
amlogic glitch free mux - Chuan Liu via B4 Relay 
<https://lore.kernel.org/all/20241111-fix_glitch_free-v2-0-0099fd9ad3e5@amlogic.com/>


Adding CLK_OPS_PARENT_ENABLE causes the CLK_IGNORE_UNUSED configuration
of it's parent clocks on the chain to become ineffective, so this patch
depends on fixing that issue before it can proceed.


Jerome and I have submitted patches to address the issue of
CLK_IGNORE_UNUSED becoming ineffective. I originally planned to wait
for progress on this patch and then incorporate Jerome's feedback before
sending the V3 version.

Hi Jerome, sorry if this caused any misunderstanding on your part; I
will provide timely feedback moving forward.


>
>
> Best regards,
> Martin

On Mon 29 Sep 2025 at 11:15, Chuan Liu <chuan.liu@amlogic.com> wrote:

> Hi Martin:
>
> Thanks for the detailed explanation.
>
>
> On 9/29/2025 4:55 AM, Martin Blumenstingl wrote:
>> [ EXTERNAL EMAIL ]
>>
>> Hello,
>>
>> On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
>>>
>>> On 9/28/2025 2:05 PM, Chuan Liu wrote:
>>>> Hi Jerome & Martin:
>>>>
>>>> Sorry for the imprecise description of the glitch-free mux earlier.
>>>>
>>>> Recently, while troubleshooting a CPU hang issue caused by glitches,
>>>> I realized there was a discrepancy from our previous understanding,
>>>> so I'd like to clarify it here.
>> [...]
>>> An example of the clock waveform is shown below:
>>>
>>>
>>>          __    __    __    __    __    __    __    __
>>> ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
>>>                     ^
>>>                     1 * cycle original channel.
>>>          _   _   _   _   _   _   _   _   _   _   _   _
>>> new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
>>>                                         ^
>>>                                         5 * cycles new channel.
>>>          __    __                        _   _   _   _
>>> out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
>>>                ^                        ^
>>>                start switching mux.     switch to new channel.
>> Thank you for the detailed report!
>> This is indeed problematic behavior. I guess the result is somewhat
>> random: depending on load (power draw), silicon lottery (quality),
>> temperature, voltage supply, ... - one may or may not see crashes
>> caused by this.
>
>
> Yes, our glitch-free mux is designed to prevent glitches caused by
> excessively short high or low levels in the clock output.
>
>
>>
>> Based on the previous discussion on this topic, my suggestion is to
>> split the original patch:
>> - one to add CLK_SET_RATE_GATE where needed (I think the meson8b.c
>> driver already has this where needed) to actually enable the
>> glitch-free mux behavior
>> - another one with the CLK_OPS_PARENT_ENABLE change (meson8b.c would
>> also need to be updated) to prevent the glitch-free mux from
>> temporarily outputting an electrical low signal. Jerome also asked to
>> document the behavior so we don't forget why we set this flag
>>
>> Both patches should get the proper "Fixes" tags.
>> I think it would also be great if you could include the waveform
>> example in at least the commit message as it helps understand the
>> problem.
>>
>> Let's also give Jerome some time to comment before you send patches.
>
>
> A V2 version was submitted later with changes based on your suggestions.
> Regarding the "Fixes" tag, Jerome had proposed some modifications.
>
> [PATCH v2 0/3] clk: Fix issues related to CLK_IGNORE_UNUSED failures and
> amlogic glitch free mux - Chuan Liu via B4 Relay
> <https://lore.kernel.org/all/20241111-fix_glitch_free-v2-0-0099fd9ad3e5@amlogic.com/>
>

The comments I've provided on this still stands.

>
> Adding CLK_OPS_PARENT_ENABLE causes the CLK_IGNORE_UNUSED configuration
> of it's parent clocks on the chain to become ineffective, so this patch
> depends on fixing that issue before it can proceed.

Unused clocks are NOT a configuration.

They are by-product of the bootloader. You cannot rely on them. If
anything depends on them, you have a(nother) problem to solve. 

>
>
> Jerome and I have submitted patches to address the issue of
> CLK_IGNORE_UNUSED becoming ineffective. I originally planned to wait
> for progress on this patch and then incorporate Jerome's feedback before
> sending the V3 version.

I've provided a suggestion but this something happening in clock core.
I suggest that you split this out of your series so things that need to
go through Stephen are not mixed with Amlogic stuff.

But again, you cannot rely on the state of clock just because it has
CLK_IGNORE_UNUSED:

* Nothing says it is enabled to begin with
* Nothing says it will stay on if a consumer comes and goes
* ... and yes, it does not survive CCF usage checking down the road.

It is unreliable and it is not meant to be more than that, AFAIK

>
> Hi Jerome, sorry if this caused any misunderstanding on your part; I
> will provide timely feedback moving forward.
>
>
>>
>>
>> Best regards,
>> Martin

-- 
Jerome

On 9/29/2025 8:36 PM, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Mon 29 Sep 2025 at 11:15, Chuan Liu <chuan.liu@amlogic.com> wrote:
>
>> Hi Martin:
>>
>> Thanks for the detailed explanation.
>>
>>
>> On 9/29/2025 4:55 AM, Martin Blumenstingl wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> Hello,
>>>
>>> On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
>>>> On 9/28/2025 2:05 PM, Chuan Liu wrote:
>>>>> Hi Jerome & Martin:
>>>>>
>>>>> Sorry for the imprecise description of the glitch-free mux earlier.
>>>>>
>>>>> Recently, while troubleshooting a CPU hang issue caused by glitches,
>>>>> I realized there was a discrepancy from our previous understanding,
>>>>> so I'd like to clarify it here.
>>> [...]
>>>> An example of the clock waveform is shown below:
>>>>
>>>>
>>>>           __    __    __    __    __    __    __    __
>>>> ori:  ↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑  |__↑
>>>>                      ^
>>>>                      1 * cycle original channel.
>>>>           _   _   _   _   _   _   _   _   _   _   _   _
>>>> new:  ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
>>>>                                          ^
>>>>                                          5 * cycles new channel.
>>>>           __    __                        _   _   _   _
>>>> out:  ↑  |__↑  |______________________↑ |_↑ |_↑ |_↑ |_↑
>>>>                 ^                        ^
>>>>                 start switching mux.     switch to new channel.
>>> Thank you for the detailed report!
>>> This is indeed problematic behavior. I guess the result is somewhat
>>> random: depending on load (power draw), silicon lottery (quality),
>>> temperature, voltage supply, ... - one may or may not see crashes
>>> caused by this.
>>
>> Yes, our glitch-free mux is designed to prevent glitches caused by
>> excessively short high or low levels in the clock output.
>>
>>
>>> Based on the previous discussion on this topic, my suggestion is to
>>> split the original patch:
>>> - one to add CLK_SET_RATE_GATE where needed (I think the meson8b.c
>>> driver already has this where needed) to actually enable the
>>> glitch-free mux behavior
>>> - another one with the CLK_OPS_PARENT_ENABLE change (meson8b.c would
>>> also need to be updated) to prevent the glitch-free mux from
>>> temporarily outputting an electrical low signal. Jerome also asked to
>>> document the behavior so we don't forget why we set this flag
>>>
>>> Both patches should get the proper "Fixes" tags.
>>> I think it would also be great if you could include the waveform
>>> example in at least the commit message as it helps understand the
>>> problem.
>>>
>>> Let's also give Jerome some time to comment before you send patches.
>>
>> A V2 version was submitted later with changes based on your suggestions.
>> Regarding the "Fixes" tag, Jerome had proposed some modifications.
>>
>> [PATCH v2 0/3] clk: Fix issues related to CLK_IGNORE_UNUSED failures and
>> amlogic glitch free mux - Chuan Liu via B4 Relay
>> <https://lore.kernel.org/all/20241111-fix_glitch_free-v2-0-0099fd9ad3e5@amlogic.com/>
>>
> The comments I've provided on this still stands.
>
>> Adding CLK_OPS_PARENT_ENABLE causes the CLK_IGNORE_UNUSED configuration
>> of it's parent clocks on the chain to become ineffective, so this patch
>> depends on fixing that issue before it can proceed.
> Unused clocks are NOT a configuration.
>
> They are by-product of the bootloader. You cannot rely on them. If
> anything depends on them, you have a(nother) problem to solve.
>
>>
>> Jerome and I have submitted patches to address the issue of
>> CLK_IGNORE_UNUSED becoming ineffective. I originally planned to wait
>> for progress on this patch and then incorporate Jerome's feedback before
>> sending the V3 version.
> I've provided a suggestion but this something happening in clock core.
> I suggest that you split this out of your series so things that need to
> go through Stephen are not mixed with Amlogic stuff.
>
> But again, you cannot rely on the state of clock just because it has
> CLK_IGNORE_UNUSED:
>
> * Nothing says it is enabled to begin with
> * Nothing says it will stay on if a consumer comes and goes
> * ... and yes, it does not survive CCF usage checking down the road.
>
> It is unreliable and it is not meant to be more than that, AFAIK


ok, I see what you mean, I will try to do that later.


>> Hi Jerome, sorry if this caused any misunderstanding on your part; I
>> will provide timely feedback moving forward.
>>
>>
>>>
>>> Best regards,
>>> Martin
> --
> Jerome

On Sun 29 Sep 2024 at 14:10, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote:

> From: Chuan Liu <chuan.liu@amlogic.com>
>
> glitch free mux has two clock channels (channel 0 and channel 1) with
> the same configuration. When the frequency needs to be changed, the two
> channels ping-pong to ensure clock continuity and suppress glitch.
>
> Channel 0 of glitch free mux is not only the clock source for the mux,
> but also the working clock for glitch free mux. Therefore, when glitch
> free mux switches, it is necessary to ensure that channel 0 has a clock
> input, otherwise glitch free mux will not work and cannot switch to the
> target channel.
>
> Add flag CLK_SET_RATE_GATE to channels 0 and 1 to implement Ping-Pong
> switchover to suppress glitch.
>
> glitch free mux Add flag CLK_OPS_PARENT_ENABLE to ensure that channel 0
> has clock input when switching channels.

Several 'glitch_free' are not touched by your change. Why ?
I thinking about the mali glitch free mux for example.

>
> Change-Id: I6fa6ff92f7b2e0a13dd7a27feb0e8568be3ca9f9
> Signed-off-by: Chuan Liu <chuan.liu@amlogic.com>
> ---
>  drivers/clk/meson/a1-peripherals.c | 12 ++++++------
>  drivers/clk/meson/axg.c            | 16 ++++++++++------
>  drivers/clk/meson/c3-peripherals.c |  6 +++---
>  drivers/clk/meson/g12a.c           | 18 +++++++++++-------
>  drivers/clk/meson/gxbb.c           | 18 +++++++++++-------
>  drivers/clk/meson/s4-peripherals.c | 32 ++++++++++++++++----------------
>  6 files changed, 57 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/clk/meson/a1-peripherals.c b/drivers/clk/meson/a1-peripherals.c
> index 7aa6abb2eb1f..7f515e002adb 100644
> --- a/drivers/clk/meson/a1-peripherals.c
> +++ b/drivers/clk/meson/a1-peripherals.c
> @@ -423,7 +423,7 @@ static struct clk_regmap dspa_a = {
>  			&dspa_a_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -471,7 +471,7 @@ static struct clk_regmap dspa_b = {
>  			&dspa_b_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -489,7 +489,7 @@ static struct clk_regmap dspa_sel = {
>  			&dspa_b.hw,
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -569,7 +569,7 @@ static struct clk_regmap dspb_a = {
>  			&dspb_a_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -617,7 +617,7 @@ static struct clk_regmap dspb_b = {
>  			&dspb_b_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -635,7 +635,7 @@ static struct clk_regmap dspb_sel = {
>  			&dspb_b.hw,
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
> index 1b08daf579b2..e2d3266f4b45 100644
> --- a/drivers/clk/meson/axg.c
> +++ b/drivers/clk/meson/axg.c
> @@ -1077,7 +1077,8 @@ static struct clk_regmap axg_vpu_0 = {
>  		 * We want to avoid CCF to disable the VPU clock if
>  		 * display has been set by Bootloader
>  		 */
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1126,7 +1127,8 @@ static struct clk_regmap axg_vpu_1 = {
>  		 * We want to avoid CCF to disable the VPU clock if
>  		 * display has been set by Bootloader
>  		 */
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1144,7 +1146,7 @@ static struct clk_regmap axg_vpu = {
>  			&axg_vpu_1.hw
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_NO_REPARENT,
> +		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -1194,7 +1196,8 @@ static struct clk_regmap axg_vapb_0 = {
>  			&axg_vapb_0_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1242,7 +1245,8 @@ static struct clk_regmap axg_vapb_1 = {
>  			&axg_vapb_1_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1260,7 +1264,7 @@ static struct clk_regmap axg_vapb_sel = {
>  			&axg_vapb_1.hw
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_NO_REPARENT,
> +		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> diff --git a/drivers/clk/meson/c3-peripherals.c b/drivers/clk/meson/c3-peripherals.c
> index 7dcbf4ebee07..27343a73a521 100644
> --- a/drivers/clk/meson/c3-peripherals.c
> +++ b/drivers/clk/meson/c3-peripherals.c
> @@ -1364,7 +1364,7 @@ static struct clk_regmap hcodec_0 = {
>  			&hcodec_0_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1411,7 +1411,7 @@ static struct clk_regmap hcodec_1 = {
>  			&hcodec_1_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1431,7 +1431,7 @@ static struct clk_regmap hcodec = {
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_data = hcodec_parent_data,
>  		.num_parents = ARRAY_SIZE(hcodec_parent_data),
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
> index d3539fe9f7af..21a25001e904 100644
> --- a/drivers/clk/meson/g12a.c
> +++ b/drivers/clk/meson/g12a.c
> @@ -2746,7 +2746,8 @@ static struct clk_regmap g12a_vpu_0 = {
>  		.ops = &clk_regmap_gate_ops,
>  		.parent_hws = (const struct clk_hw *[]) { &g12a_vpu_0_div.hw },
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -2790,7 +2791,8 @@ static struct clk_regmap g12a_vpu_1 = {
>  		.ops = &clk_regmap_gate_ops,
>  		.parent_hws = (const struct clk_hw *[]) { &g12a_vpu_1_div.hw },
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -2812,7 +2814,7 @@ static struct clk_regmap g12a_vpu = {
>  			&g12a_vpu_1.hw,
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_NO_REPARENT,
> +		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -3035,7 +3037,8 @@ static struct clk_regmap g12a_vapb_0 = {
>  			&g12a_vapb_0_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -3083,7 +3086,8 @@ static struct clk_regmap g12a_vapb_1 = {
>  			&g12a_vapb_1_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -3105,7 +3109,7 @@ static struct clk_regmap g12a_vapb_sel = {
>  			&g12a_vapb_1.hw,
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_NO_REPARENT,
> +		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -4039,7 +4043,7 @@ static struct clk_regmap g12a_mali = {
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_hws = g12a_mali_parent_hws,
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
> index 262c318edbd5..812b3e20c366 100644
> --- a/drivers/clk/meson/gxbb.c
> +++ b/drivers/clk/meson/gxbb.c
> @@ -1132,7 +1132,7 @@ static struct clk_regmap gxbb_mali = {
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_hws = gxbb_mali_parent_hws,
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -1543,7 +1543,8 @@ static struct clk_regmap gxbb_vpu_0 = {
>  		.ops = &clk_regmap_gate_ops,
>  		.parent_hws = (const struct clk_hw *[]) { &gxbb_vpu_0_div.hw },
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1591,7 +1592,8 @@ static struct clk_regmap gxbb_vpu_1 = {
>  		.ops = &clk_regmap_gate_ops,
>  		.parent_hws = (const struct clk_hw *[]) { &gxbb_vpu_1_div.hw },
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1613,7 +1615,7 @@ static struct clk_regmap gxbb_vpu = {
>  			&gxbb_vpu_1.hw
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_NO_REPARENT,
> +		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -1674,7 +1676,8 @@ static struct clk_regmap gxbb_vapb_0 = {
>  			&gxbb_vapb_0_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1726,7 +1729,8 @@ static struct clk_regmap gxbb_vapb_1 = {
>  			&gxbb_vapb_1_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
> +		.flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED |
> +			 CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1748,7 +1752,7 @@ static struct clk_regmap gxbb_vapb_sel = {
>  			&gxbb_vapb_1.hw
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_NO_REPARENT,
> +		.flags = CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
> index c930cf0614a0..cf10be40141d 100644
> --- a/drivers/clk/meson/s4-peripherals.c
> +++ b/drivers/clk/meson/s4-peripherals.c
> @@ -1404,7 +1404,7 @@ static struct clk_regmap s4_mali_mux = {
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_hws = s4_mali_parent_hws,
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -1466,7 +1466,7 @@ static struct clk_regmap s4_vdec_p0 = {
>  			&s4_vdec_p0_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1516,7 +1516,7 @@ static struct clk_regmap s4_vdec_p1 = {
>  			&s4_vdec_p1_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1536,7 +1536,7 @@ static struct clk_regmap s4_vdec_mux = {
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_hws = s4_vdec_mux_parent_hws,
>  		.num_parents = ARRAY_SIZE(s4_vdec_mux_parent_hws),
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -1586,7 +1586,7 @@ static struct clk_regmap s4_hevcf_p0 = {
>  			&s4_hevcf_p0_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1636,7 +1636,7 @@ static struct clk_regmap s4_hevcf_p1 = {
>  			&s4_hevcf_p1_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1656,7 +1656,7 @@ static struct clk_regmap s4_hevcf_mux = {
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_hws = s4_hevcf_mux_parent_hws,
>  		.num_parents = ARRAY_SIZE(s4_hevcf_mux_parent_hws),
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -1712,7 +1712,7 @@ static struct clk_regmap s4_vpu_0 = {
>  		.ops = &clk_regmap_gate_ops,
>  		.parent_hws = (const struct clk_hw *[]) { &s4_vpu_0_div.hw },
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1756,7 +1756,7 @@ static struct clk_regmap s4_vpu_1 = {
>  		.ops = &clk_regmap_gate_ops,
>  		.parent_hws = (const struct clk_hw *[]) { &s4_vpu_1_div.hw },
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1774,7 +1774,7 @@ static struct clk_regmap s4_vpu = {
>  			&s4_vpu_1.hw,
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -1921,7 +1921,7 @@ static struct clk_regmap s4_vpu_clkc_p0 = {
>  			&s4_vpu_clkc_p0_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1969,7 +1969,7 @@ static struct clk_regmap s4_vpu_clkc_p1 = {
>  			&s4_vpu_clkc_p1_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -1989,7 +1989,7 @@ static struct clk_regmap s4_vpu_clkc_mux = {
>  		.ops = &clk_regmap_mux_ops,
>  		.parent_hws = s4_vpu_mux_parent_hws,
>  		.num_parents = ARRAY_SIZE(s4_vpu_mux_parent_hws),
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };
>  
> @@ -2049,7 +2049,7 @@ static struct clk_regmap s4_vapb_0 = {
>  			&s4_vapb_0_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -2097,7 +2097,7 @@ static struct clk_regmap s4_vapb_1 = {
>  			&s4_vapb_1_div.hw
>  		},
>  		.num_parents = 1,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_GATE,
>  	},
>  };
>  
> @@ -2115,7 +2115,7 @@ static struct clk_regmap s4_vapb = {
>  			&s4_vapb_1.hw
>  		},
>  		.num_parents = 2,
> -		.flags = CLK_SET_RATE_PARENT,
> +		.flags = CLK_SET_RATE_PARENT | CLK_OPS_PARENT_ENABLE,
>  	},
>  };

-- 
Jerome