Re: [PATCH 2/2] clk: meson: Fix glitch free mux related issues

[Martin Blumenstingl <martin.blumenstingl@googlemail.com>]

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