Message-ID: <9834c7c5-9334-4c78-a2fe-588ff03cf935@amlogic.com>
Date: Sun, 28 Sep 2025 14:40:48 +0800
From: Chuan Liu <chuan.liu@...ogic.com>
To: Jerome Brunet <jbrunet@...libre.com>
Cc: Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Neil Armstrong <neil.armstrong@...aro.org>,
 Kevin Hilman <khilman@...libre.com>, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-amlogic@...ts.infradead.org,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/2] clk: meson: Fix glitch free mux related issues


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@...ogic.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@...nel.org> wrote:
>>>>> From: Chuan Liu <chuan.liu@...ogic.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

Powered by blists - more mailing lists