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

[Jerome Brunet <jbrunet@baylibre.com>]

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