| Message ID | 20240929-fix_glitch_free-v1-0-22f9c36b7edf@amlogic.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | clk: Fix issues related to CLK_IGNORE_UNUSED failures and amlogic glitch free mux | expand |
On Sun 29 Sep 2024 at 14:10, Chuan Liu via B4 Relay <devnull+chuan.liu.amlogic.com@kernel.org> wrote: > If CLK_OPS_PARENT_ENABLE is configured for clock, > clk_core_disable_unprepare() is called in clk_disable_unused_subtree(). > Even clocks that are configured with CLK_IGNORE_UNUSED are disabled, > resulting in the failure of CLK_IGNORE_UNUSED. > > To ensure that amlogic glitch free mux can switch clock channels > properly, add flag CLK_OPS_PARENT_ENABLE to glitch free mux. The issue > that CLK_OPS_PARENT_ENABLE in CCF causes CLK_IGNORE_UNUSED to fail is > also exposed. The problem is that you are mixing problems together and it makes things rather difficult to follow. There are 2 distinct problem, there should have been to distinct patchset, even if you reference the CCF one in the Amlogic change. CLK_IGNORE_UNUSED is no guarantee that a clock will stay on, no matter what happens in the clock tree. I explained that to you several times, and it is the very reason why you are being asked to justify each usage of the flag. Most of the time, using it is wrong. That being said, there seems to be problems with CLK_OPS_PARENT_ENABLE in clk_disable_unused_subtree(). As it is, I think * a clock with CLK_IGNORE_UNUSED and CLK_OPS_PARENT_ENABLE, would 'leak' an enable, essentially making the parent subtree critical. * All parents of a CLK_OPS_PARENT_ENABLE clock would have its CLK_IGNORE_UNUSED ignored as a result of the enable/disable sequence (note that in any other circumstance, enable/disable should indeed disable an CLK_IGNORE_UNUSED clock). * Parent of ignored clocks may still get disabled. I'll be sending a proposal to address these problems soon. > > glitch free mux channel switchover failure issue(Test vpu_clk on S4): > step 1: > $ cat /sys/kernel/debug/clk/vpu/clk_parent > vpu_0 > $ cat /sys/kernel/debug/clk/vpu_0/clk_rate > 399999994 > $ cat /sys/kernel/debug/clk/vpu_1/clk_rate > 666666656 > $ echo 1 > /sys/kernel/debug/clk/vpu/clk_prepare_enable > $ cat /sys/kernel/debug/meson-clk-msr/clks/cts_vpu_clk > 399987500 +/-12500Hz > > step 2: > $ echo 0 > /sys/kernel/debug/clk/vpu/clk_prepare_enable > $ echo 1 > /sys/kernel/debug/clk/vpu/clk_parent > $ cat /sys/kernel/debug/clk/vpu/clk_parent > vpu_1 > $ cat /sys/kernel/debug/clk/vpu/clk_rate > 666666656 > $ echo 1 > /sys/kernel/debug/clk/vpu/clk_prepare_enable > $ cat /sys/kernel/debug/meson-clk-msr/clks/cts_vpu_clk > 0 +/-3125Hz > > In step2, vpu_0 is disabled, and the vpu is not switched to vpu_1. At > this time, the vpu is still connected to vpu_0 and vpu_0 is disabled at > this time, resulting in the clk-measure not measuring the clock. > If keep the display on and managed at all time is critical, then may you should consider having the module managing it as built-in. You would not need CLK_IGNORE_USED at all because device would be present before clk_disable_unused() is executed. Past the late init, if no device actively use the clock, disabling it is the sane thing to do because nothing says it will ever be used. > Signed-off-by: Chuan Liu <chuan.liu@amlogic.com> > --- > Chuan Liu (2): > clk: Fix the CLK_IGNORE_UNUSED failure issue > clk: meson: Fix glitch free mux related issues > > drivers/clk/clk.c | 67 ++++++++++++++++++++++++++++++++++++-- > drivers/clk/meson/a1-peripherals.c | 12 +++---- > drivers/clk/meson/axg.c | 16 +++++---- > drivers/clk/meson/c3-peripherals.c | 6 ++-- > drivers/clk/meson/g12a.c | 18 ++++++---- > drivers/clk/meson/gxbb.c | 18 ++++++---- > drivers/clk/meson/s4-peripherals.c | 32 +++++++++--------- > 7 files changed, 122 insertions(+), 47 deletions(-) > --- > base-commit: e736da1956cf0880a02ec5023f3487eea7611b5f > change-id: 20240929-fix_glitch_free-290c88923c31 > > Best regards,
If CLK_OPS_PARENT_ENABLE is configured for clock, clk_core_disable_unprepare() is called in clk_disable_unused_subtree(). Even clocks that are configured with CLK_IGNORE_UNUSED are disabled, resulting in the failure of CLK_IGNORE_UNUSED. To ensure that amlogic glitch free mux can switch clock channels properly, add flag CLK_OPS_PARENT_ENABLE to glitch free mux. The issue that CLK_OPS_PARENT_ENABLE in CCF causes CLK_IGNORE_UNUSED to fail is also exposed. glitch free mux channel switchover failure issue(Test vpu_clk on S4): step 1: $ cat /sys/kernel/debug/clk/vpu/clk_parent vpu_0 $ cat /sys/kernel/debug/clk/vpu_0/clk_rate 399999994 $ cat /sys/kernel/debug/clk/vpu_1/clk_rate 666666656 $ echo 1 > /sys/kernel/debug/clk/vpu/clk_prepare_enable $ cat /sys/kernel/debug/meson-clk-msr/clks/cts_vpu_clk 399987500 +/-12500Hz step 2: $ echo 0 > /sys/kernel/debug/clk/vpu/clk_prepare_enable $ echo 1 > /sys/kernel/debug/clk/vpu/clk_parent $ cat /sys/kernel/debug/clk/vpu/clk_parent vpu_1 $ cat /sys/kernel/debug/clk/vpu/clk_rate 666666656 $ echo 1 > /sys/kernel/debug/clk/vpu/clk_prepare_enable $ cat /sys/kernel/debug/meson-clk-msr/clks/cts_vpu_clk 0 +/-3125Hz In step2, vpu_0 is disabled, and the vpu is not switched to vpu_1. At this time, the vpu is still connected to vpu_0 and vpu_0 is disabled at this time, resulting in the clk-measure not measuring the clock. Signed-off-by: Chuan Liu <chuan.liu@amlogic.com> --- Chuan Liu (2): clk: Fix the CLK_IGNORE_UNUSED failure issue clk: meson: Fix glitch free mux related issues drivers/clk/clk.c | 67 ++++++++++++++++++++++++++++++++++++-- drivers/clk/meson/a1-peripherals.c | 12 +++---- drivers/clk/meson/axg.c | 16 +++++---- drivers/clk/meson/c3-peripherals.c | 6 ++-- drivers/clk/meson/g12a.c | 18 ++++++---- drivers/clk/meson/gxbb.c | 18 ++++++---- drivers/clk/meson/s4-peripherals.c | 32 +++++++++--------- 7 files changed, 122 insertions(+), 47 deletions(-) --- base-commit: e736da1956cf0880a02ec5023f3487eea7611b5f change-id: 20240929-fix_glitch_free-290c88923c31 Best regards,