Re: [PATCH 2/2] clk: meson: Fix glitch free mux related issues
From: Chuan Liu
Date: Mon Sep 29 2025 - 22:05:27 EST
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@xxxxxxxxxxx> wrote:
On 9/29/2025 4:48 PM, Jerome Brunet wrote:Then I don't think the 2 flags are enough to make it safe
[ EXTERNAL EMAIL ]
On Sun 28 Sep 2025 at 22:55, Martin Blumenstingl
<martin.blumenstingl@xxxxxxxxxxxxxx> wrote:
Hello,1 2
On Sun, Sep 28, 2025 at 8:41 AM Chuan Liu <chuan.liu@xxxxxxxxxxx> 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:
v v
Ok ... but when is it safe to disable the "ori" clock ?__ __ __ __ __ __ __ __
ori: ↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑ |__↑
^
1 * cycle original channel.
_ _ _ _ _ _ _ _ _ _ _ _
new: ↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑ |_↑
^
5 * cycles new channel.
__ __ _ _ _ _
out: ↑ |__↑ |______________________↑ |_↑ |_↑ |_↑ |_↑
^ ^
start switching mux. switch to new channel.
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.
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!Yes please split the changes and visit all the controllers shipping this
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
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