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

[Chuan Liu <chuan.liu@amlogic.com>]

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