* [PATCH] clk: Properly update prepare/enable count on orphan clock reparent [not found] <CGME20180115115231eucas1p1499f46ca64b65598d03a182ed5670f28@eucas1p1.samsung.com> @ 2018-01-15 11:52 ` Marek Szyprowski 0 siblings, 0 replies; 14+ messages in thread From: Marek Szyprowski @ 2018-01-15 11:52 UTC (permalink / raw) To: linux-clk, linux-arm-kernel Cc: Marek Szyprowski, Stephen Boyd, Michael Turquette, Shawn Guo, Dong Aisheng, Bartlomiej Zolnierkiewicz If orphaned clock has been already prepared/enabled (for example if it or one of its children has CLK_IS_CRITICAL flag), then the prepare/enable counters of the newly assigned parent are not updated correctly. This might later cause warnings during changing clock parents. This patch fixes following warning on Exynos5422-based boards: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 clk_core_disable_lock+0x18/0x24 Modules linked in: CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: pm genpd_power_off_work_fn [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] (warn_slowpath_null) from [<c048d584>] (clk_core_disable_lock+0x18/0x24) [<c048d584>] (clk_core_disable_lock) from [<c048e490>] (clk_core_disable_unprepare+0xc/0x20) [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to 0xeeb01ff8) 1fa0: 00000000 00000000 00000000 00000000 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end trace 503c239fb760f17a ]--- ------------[ cut here ]------------ WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 clk_core_disable_unprepare+0x18/0x20 Modules linked in: CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next-20180115 #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: pm genpd_power_off_work_fn [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] (warn_slowpath_null) from [<c048e49c>] (clk_core_disable_unprepare+0x18/0x20) [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to 0xeeb01ff8) 1fa0: 00000000 00000000 00000000 00000000 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end trace 503c239fb760f17b ]--- ------------[ cut here ]------------ Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting during registration") Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/clk/clk.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 0f686a9dac3e..d33c087d7868 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) if (parent) { /* update the clk tree topology */ flags = clk_enable_lock(); + if (orphan->prepare_count) + clk_core_prepare(parent); + if (orphan->enable_count) + clk_core_enable(parent); clk_reparent(orphan, parent); clk_enable_unlock(flags); __clk_recalc_accuracies(orphan); -- 2.15.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] clk: Properly update prepare/enable count on orphan clock reparent @ 2018-01-15 11:52 ` Marek Szyprowski 0 siblings, 0 replies; 14+ messages in thread From: Marek Szyprowski @ 2018-01-15 11:52 UTC (permalink / raw) To: linux-arm-kernel If orphaned clock has been already prepared/enabled (for example if it or one of its children has CLK_IS_CRITICAL flag), then the prepare/enable counters of the newly assigned parent are not updated correctly. This might later cause warnings during changing clock parents. This patch fixes following warning on Exynos5422-based boards: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 clk_core_disable_lock+0x18/0x24 Modules linked in: CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: pm genpd_power_off_work_fn [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] (warn_slowpath_null) from [<c048d584>] (clk_core_disable_lock+0x18/0x24) [<c048d584>] (clk_core_disable_lock) from [<c048e490>] (clk_core_disable_unprepare+0xc/0x20) [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to 0xeeb01ff8) 1fa0: 00000000 00000000 00000000 00000000 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end trace 503c239fb760f17a ]--- ------------[ cut here ]------------ WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 clk_core_disable_unprepare+0x18/0x20 Modules linked in: CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next-20180115 #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) Workqueue: pm genpd_power_off_work_fn [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] (warn_slowpath_null) from [<c048e49c>] (clk_core_disable_unprepare+0x18/0x20) [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to 0xeeb01ff8) 1fa0: 00000000 00000000 00000000 00000000 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end trace 503c239fb760f17b ]--- ------------[ cut here ]------------ Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting during registration") Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/clk/clk.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 0f686a9dac3e..d33c087d7868 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) if (parent) { /* update the clk tree topology */ flags = clk_enable_lock(); + if (orphan->prepare_count) + clk_core_prepare(parent); + if (orphan->enable_count) + clk_core_enable(parent); clk_reparent(orphan, parent); clk_enable_unlock(flags); __clk_recalc_accuracies(orphan); -- 2.15.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] clk: Properly update prepare/enable count on orphan clock reparent 2018-01-15 11:52 ` Marek Szyprowski @ 2018-01-17 8:05 ` Krzysztof Kozlowski -1 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2018-01-17 8:05 UTC (permalink / raw) To: Marek Szyprowski Cc: linux-clk, linux-arm-kernel, Stephen Boyd, Michael Turquette, Shawn Guo, Dong Aisheng, Bartlomiej Zolnierkiewicz On Mon, Jan 15, 2018 at 12:52 PM, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > If orphaned clock has been already prepared/enabled (for example if it or > one of its children has CLK_IS_CRITICAL flag), then the prepare/enable > counters of the newly assigned parent are not updated correctly. This > might later cause warnings during changing clock parents. > Tested-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] clk: Properly update prepare/enable count on orphan clock reparent @ 2018-01-17 8:05 ` Krzysztof Kozlowski 0 siblings, 0 replies; 14+ messages in thread From: Krzysztof Kozlowski @ 2018-01-17 8:05 UTC (permalink / raw) To: linux-arm-kernel On Mon, Jan 15, 2018 at 12:52 PM, Marek Szyprowski <m.szyprowski@samsung.com> wrote: > If orphaned clock has been already prepared/enabled (for example if it or > one of its children has CLK_IS_CRITICAL flag), then the prepare/enable > counters of the newly assigned parent are not updated correctly. This > might later cause warnings during changing clock parents. > Tested-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] clk: Properly update prepare/enable count on orphan clock reparent 2018-01-15 11:52 ` Marek Szyprowski @ 2018-01-25 22:19 ` Stephen Boyd -1 siblings, 0 replies; 14+ messages in thread From: Stephen Boyd @ 2018-01-25 22:19 UTC (permalink / raw) To: Marek Szyprowski Cc: linux-clk, linux-arm-kernel, Michael Turquette, Shawn Guo, Dong Aisheng, Bartlomiej Zolnierkiewicz On 01/15, Marek Szyprowski wrote: > If orphaned clock has been already prepared/enabled (for example if it or > one of its children has CLK_IS_CRITICAL flag), then the prepare/enable > counters of the newly assigned parent are not updated correctly. This > might later cause warnings during changing clock parents. > > This patch fixes following warning on Exynos5422-based boards: > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 clk_core_disable_lock+0x18/0x24 > Modules linked in: > CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 #106 > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > Workqueue: pm genpd_power_off_work_fn > [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) > [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) > [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) > [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) > [<c01250cc>] (warn_slowpath_null) from [<c048d584>] (clk_core_disable_lock+0x18/0x24) > [<c048d584>] (clk_core_disable_lock) from [<c048e490>] (clk_core_disable_unprepare+0xc/0x20) > [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) > [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) > [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) > [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) > [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) > [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) > [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) > [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) > [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) > [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) > Exception stack(0xeeb01fb0 to 0xeeb01ff8) > 1fa0: 00000000 00000000 00000000 00000000 > 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 > ---[ end trace 503c239fb760f17a ]--- > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 clk_core_disable_unprepare+0x18/0x20 > Modules linked in: > CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next-20180115 #106 > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > Workqueue: pm genpd_power_off_work_fn > [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) > [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) > [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) > [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) > [<c01250cc>] (warn_slowpath_null) from [<c048e49c>] (clk_core_disable_unprepare+0x18/0x20) > [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) > [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) > [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) > [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) > [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) > [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) > [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) > [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) > [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) > [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) > Exception stack(0xeeb01fb0 to 0xeeb01ff8) > 1fa0: 00000000 00000000 00000000 00000000 > 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 > ---[ end trace 503c239fb760f17b ]--- > ------------[ cut here ]------------ > > Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting during registration") > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/clk/clk.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 0f686a9dac3e..d33c087d7868 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) > if (parent) { > /* update the clk tree topology */ > flags = clk_enable_lock(); This is the spinlock? > + if (orphan->prepare_count) > + clk_core_prepare(parent); And this is prepare mutex? Doesn't sound like it will work. > + if (orphan->enable_count) > + clk_core_enable(parent); This would be OK, but then we might touch the hardware again? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] clk: Properly update prepare/enable count on orphan clock reparent @ 2018-01-25 22:19 ` Stephen Boyd 0 siblings, 0 replies; 14+ messages in thread From: Stephen Boyd @ 2018-01-25 22:19 UTC (permalink / raw) To: linux-arm-kernel On 01/15, Marek Szyprowski wrote: > If orphaned clock has been already prepared/enabled (for example if it or > one of its children has CLK_IS_CRITICAL flag), then the prepare/enable > counters of the newly assigned parent are not updated correctly. This > might later cause warnings during changing clock parents. > > This patch fixes following warning on Exynos5422-based boards: > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 clk_core_disable_lock+0x18/0x24 > Modules linked in: > CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 #106 > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > Workqueue: pm genpd_power_off_work_fn > [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) > [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) > [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) > [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) > [<c01250cc>] (warn_slowpath_null) from [<c048d584>] (clk_core_disable_lock+0x18/0x24) > [<c048d584>] (clk_core_disable_lock) from [<c048e490>] (clk_core_disable_unprepare+0xc/0x20) > [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) > [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) > [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) > [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) > [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) > [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) > [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) > [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) > [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) > [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) > Exception stack(0xeeb01fb0 to 0xeeb01ff8) > 1fa0: 00000000 00000000 00000000 00000000 > 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 > ---[ end trace 503c239fb760f17a ]--- > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 clk_core_disable_unprepare+0x18/0x20 > Modules linked in: > CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next-20180115 #106 > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > Workqueue: pm genpd_power_off_work_fn > [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) > [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) > [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) > [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) > [<c01250cc>] (warn_slowpath_null) from [<c048e49c>] (clk_core_disable_unprepare+0x18/0x20) > [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) > [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) > [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) > [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) > [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) > [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) > [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) > [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) > [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) > [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) > Exception stack(0xeeb01fb0 to 0xeeb01ff8) > 1fa0: 00000000 00000000 00000000 00000000 > 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 > ---[ end trace 503c239fb760f17b ]--- > ------------[ cut here ]------------ > > Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting during registration") > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/clk/clk.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 0f686a9dac3e..d33c087d7868 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) > if (parent) { > /* update the clk tree topology */ > flags = clk_enable_lock(); This is the spinlock? > + if (orphan->prepare_count) > + clk_core_prepare(parent); And this is prepare mutex? Doesn't sound like it will work. > + if (orphan->enable_count) > + clk_core_enable(parent); This would be OK, but then we might touch the hardware again? -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] clk: Properly update prepare/enable count on orphan clock reparent 2018-01-25 22:19 ` Stephen Boyd @ 2018-01-26 7:31 ` Marek Szyprowski -1 siblings, 0 replies; 14+ messages in thread From: Marek Szyprowski @ 2018-01-26 7:31 UTC (permalink / raw) To: Stephen Boyd Cc: linux-clk, linux-arm-kernel, Michael Turquette, Shawn Guo, Dong Aisheng, Bartlomiej Zolnierkiewicz Hi Stephen, On 2018-01-25 23:19, Stephen Boyd wrote: > On 01/15, Marek Szyprowski wrote: >> If orphaned clock has been already prepared/enabled (for example if it or >> one of its children has CLK_IS_CRITICAL flag), then the prepare/enable >> counters of the newly assigned parent are not updated correctly. This >> might later cause warnings during changing clock parents. >> >> This patch fixes following warning on Exynos5422-based boards: >> ------------[ cut here ]------------ >> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 clk_core_disable_lock+0x18/0x24 >> Modules linked in: >> CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 #106 >> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >> Workqueue: pm genpd_power_off_work_fn >> [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) >> [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) >> [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) >> [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) >> [<c01250cc>] (warn_slowpath_null) from [<c048d584>] (clk_core_disable_lock+0x18/0x24) >> [<c048d584>] (clk_core_disable_lock) from [<c048e490>] (clk_core_disable_unprepare+0xc/0x20) >> [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) >> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) >> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) >> [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) >> [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >> [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) >> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) >> [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) >> [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) >> [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) >> Exception stack(0xeeb01fb0 to 0xeeb01ff8) >> 1fa0: 00000000 00000000 00000000 00000000 >> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 >> ---[ end trace 503c239fb760f17a ]--- >> ------------[ cut here ]------------ >> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 clk_core_disable_unprepare+0x18/0x20 >> Modules linked in: >> CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next-20180115 #106 >> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >> Workqueue: pm genpd_power_off_work_fn >> [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) >> [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) >> [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) >> [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) >> [<c01250cc>] (warn_slowpath_null) from [<c048e49c>] (clk_core_disable_unprepare+0x18/0x20) >> [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) >> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) >> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) >> [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) >> [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >> [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) >> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) >> [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) >> [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) >> [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) >> Exception stack(0xeeb01fb0 to 0xeeb01ff8) >> 1fa0: 00000000 00000000 00000000 00000000 >> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 >> ---[ end trace 503c239fb760f17b ]--- >> ------------[ cut here ]------------ >> >> Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting during registration") >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/clk/clk.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c >> index 0f686a9dac3e..d33c087d7868 100644 >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) >> if (parent) { >> /* update the clk tree topology */ >> flags = clk_enable_lock(); > This is the spinlock? yes > >> + if (orphan->prepare_count) >> + clk_core_prepare(parent); > And this is prepare mutex? Doesn't sound like it will work. Prepare mutex is taken at the beginning of __clk_core_init() function, so everything is fine here. >> + if (orphan->enable_count) >> + clk_core_enable(parent); > This would be OK, but then we might touch the hardware again? Well, it will touch hardware in one case: if orphaned clock is already prepared and enabled (for example due to having CLK_IS_CRITICAL flag) and the newly registered parent is not (yet?) enabled. Then it will prepare and enable this new parent clock, what is a desired behavior. This code will not temporarily disable any clocks, what I assume was the main reason for the patch mentioned in the commit message. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] clk: Properly update prepare/enable count on orphan clock reparent @ 2018-01-26 7:31 ` Marek Szyprowski 0 siblings, 0 replies; 14+ messages in thread From: Marek Szyprowski @ 2018-01-26 7:31 UTC (permalink / raw) To: linux-arm-kernel Hi Stephen, On 2018-01-25 23:19, Stephen Boyd wrote: > On 01/15, Marek Szyprowski wrote: >> If orphaned clock has been already prepared/enabled (for example if it or >> one of its children has CLK_IS_CRITICAL flag), then the prepare/enable >> counters of the newly assigned parent are not updated correctly. This >> might later cause warnings during changing clock parents. >> >> This patch fixes following warning on Exynos5422-based boards: >> ------------[ cut here ]------------ >> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 clk_core_disable_lock+0x18/0x24 >> Modules linked in: >> CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 #106 >> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >> Workqueue: pm genpd_power_off_work_fn >> [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) >> [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) >> [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) >> [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) >> [<c01250cc>] (warn_slowpath_null) from [<c048d584>] (clk_core_disable_lock+0x18/0x24) >> [<c048d584>] (clk_core_disable_lock) from [<c048e490>] (clk_core_disable_unprepare+0xc/0x20) >> [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) >> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) >> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) >> [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) >> [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >> [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) >> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) >> [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) >> [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) >> [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) >> Exception stack(0xeeb01fb0 to 0xeeb01ff8) >> 1fa0: 00000000 00000000 00000000 00000000 >> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 >> ---[ end trace 503c239fb760f17a ]--- >> ------------[ cut here ]------------ >> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 clk_core_disable_unprepare+0x18/0x20 >> Modules linked in: >> CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next-20180115 #106 >> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >> Workqueue: pm genpd_power_off_work_fn >> [<c0111504>] (unwind_backtrace) from [<c010dbec>] (show_stack+0x10/0x14) >> [<c010dbec>] (show_stack) from [<c09b3f34>] (dump_stack+0x90/0xc8) >> [<c09b3f34>] (dump_stack) from [<c0125060>] (__warn+0xe4/0x110) >> [<c0125060>] (__warn) from [<c01250cc>] (warn_slowpath_null+0x40/0x48) >> [<c01250cc>] (warn_slowpath_null) from [<c048e49c>] (clk_core_disable_unprepare+0x18/0x20) >> [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] (__clk_set_parent_after+0x48/0x4c) >> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] (clk_core_set_parent_nolock+0x224/0x5b4) >> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] (clk_set_parent+0x38/0x6c) >> [<c048f23c>] (clk_set_parent) from [<c049c41c>] (exynos_pd_power+0x74/0x1cc) >> [<c049c41c>] (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >> [<c0550768>] (genpd_power_off) from [<c0550b34>] (genpd_power_off_work_fn+0x2c/0x40) >> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] (process_one_work+0x1d0/0x7bc) >> [<c01432e4>] (process_one_work) from [<c014393c>] (worker_thread+0x34/0x4dc) >> [<c014393c>] (worker_thread) from [<c014a0d0>] (kthread+0x128/0x164) >> [<c014a0d0>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20) >> Exception stack(0xeeb01fb0 to 0xeeb01ff8) >> 1fa0: 00000000 00000000 00000000 00000000 >> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 >> ---[ end trace 503c239fb760f17b ]--- >> ------------[ cut here ]------------ >> >> Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting during registration") >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/clk/clk.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c >> index 0f686a9dac3e..d33c087d7868 100644 >> --- a/drivers/clk/clk.c >> +++ b/drivers/clk/clk.c >> @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) >> if (parent) { >> /* update the clk tree topology */ >> flags = clk_enable_lock(); > This is the spinlock? yes > >> + if (orphan->prepare_count) >> + clk_core_prepare(parent); > And this is prepare mutex? Doesn't sound like it will work. Prepare mutex is taken at the beginning of __clk_core_init() function, so everything is fine here. >> + if (orphan->enable_count) >> + clk_core_enable(parent); > This would be OK, but then we might touch the hardware again? Well, it will touch hardware in one case: if orphaned clock is already prepared and enabled (for example due to having CLK_IS_CRITICAL flag) and the newly registered parent is not (yet?) enabled. Then it will prepare and enable this new parent clock, what is a desired behavior. This code will not temporarily disable any clocks, what I assume was the main reason for the patch mentioned in the commit message. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] clk: Properly update prepare/enable count on orphan clock reparent 2018-01-26 7:31 ` Marek Szyprowski @ 2018-01-26 8:36 ` A.s. Dong -1 siblings, 0 replies; 14+ messages in thread From: A.s. Dong @ 2018-01-26 8:36 UTC (permalink / raw) To: Marek Szyprowski, Stephen Boyd Cc: linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Michael Turquette, Shawn Guo, Bartlomiej Zolnierkiewicz, dl-linux-imx PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBNYXJlayBTenlwcm93c2tpIFtt YWlsdG86bS5zenlwcm93c2tpQHNhbXN1bmcuY29tXQ0KPiBTZW50OiBGcmlkYXksIEphbnVhcnkg MjYsIDIwMTggMzozMSBQTQ0KPiBUbzogU3RlcGhlbiBCb3lkIDxzYm95ZEBjb2RlYXVyb3JhLm9y Zz4NCj4gQ2M6IGxpbnV4LWNsa0B2Z2VyLmtlcm5lbC5vcmc7IGxpbnV4LWFybS1rZXJuZWxAbGlz dHMuaW5mcmFkZWFkLm9yZzsgTWljaGFlbA0KPiBUdXJxdWV0dGUgPG10dXJxdWV0dGVAYmF5bGli cmUuY29tPjsgU2hhd24gR3VvIDxzaGF3bmd1b0BrZXJuZWwub3JnPjsNCj4gQS5zLiBEb25nIDxh aXNoZW5nLmRvbmdAbnhwLmNvbT47IEJhcnRsb21pZWogWm9sbmllcmtpZXdpY3oNCj4gPGIuem9s bmllcmtpZUBzYW1zdW5nLmNvbT4NCj4gU3ViamVjdDogUmU6IFtQQVRDSF0gY2xrOiBQcm9wZXJs eSB1cGRhdGUgcHJlcGFyZS9lbmFibGUgY291bnQgb24gb3JwaGFuDQo+IGNsb2NrIHJlcGFyZW50 DQo+IA0KPiBIaSBTdGVwaGVuLA0KPiANCj4gT24gMjAxOC0wMS0yNSAyMzoxOSwgU3RlcGhlbiBC b3lkIHdyb3RlOg0KPiA+IE9uIDAxLzE1LCBNYXJlayBTenlwcm93c2tpIHdyb3RlOg0KPiA+PiBJ ZiBvcnBoYW5lZCBjbG9jayBoYXMgYmVlbiBhbHJlYWR5IHByZXBhcmVkL2VuYWJsZWQgKGZvciBl eGFtcGxlIGlmDQo+ID4+IGl0IG9yIG9uZSBvZiBpdHMgY2hpbGRyZW4gaGFzIENMS19JU19DUklU SUNBTCBmbGFnKSwgdGhlbiB0aGUNCj4gPj4gcHJlcGFyZS9lbmFibGUgY291bnRlcnMgb2YgdGhl IG5ld2x5IGFzc2lnbmVkIHBhcmVudCBhcmUgbm90IHVwZGF0ZWQNCj4gPj4gY29ycmVjdGx5LiBU aGlzIG1pZ2h0IGxhdGVyIGNhdXNlIHdhcm5pbmdzIGR1cmluZyBjaGFuZ2luZyBjbG9jayBwYXJl bnRzLg0KPiA+Pg0KPiA+PiBUaGlzIHBhdGNoIGZpeGVzIGZvbGxvd2luZyB3YXJuaW5nIG9uIEV4 eW5vczU0MjItYmFzZWQgYm9hcmRzOg0KPiA+PiAtLS0tLS0tLS0tLS1bIGN1dCBoZXJlIF0tLS0t LS0tLS0tLS0NCj4gPj4gV0FSTklORzogQ1BVOiAwIFBJRDogNTkgYXQgZHJpdmVycy9jbGsvY2xr LmM6ODExDQo+ID4+IGNsa19jb3JlX2Rpc2FibGVfbG9jaysweDE4LzB4MjQgTW9kdWxlcyBsaW5r ZWQgaW46DQo+ID4+IENQVTogMCBQSUQ6IDU5IENvbW06IGt3b3JrZXIvMDoxIE5vdCB0YWludGVk IDQuMTUuMC1yYzctbmV4dC0yMDE4MDExNQ0KPiA+PiAjMTA2IEhhcmR3YXJlIG5hbWU6IFNBTVNV TkcgRVhZTk9TIChGbGF0dGVuZWQgRGV2aWNlIFRyZWUpDQo+ID4+IFdvcmtxdWV1ZTogcG0gZ2Vu cGRfcG93ZXJfb2ZmX3dvcmtfZm4gWzxjMDExMTUwND5dDQo+ICh1bndpbmRfYmFja3RyYWNlKQ0K PiA+PiBmcm9tIFs8YzAxMGRiZWM+XSAoc2hvd19zdGFjaysweDEwLzB4MTQpIFs8YzAxMGRiZWM+ XSAoc2hvd19zdGFjaykNCj4gPj4gZnJvbSBbPGMwOWIzZjM0Pl0gKGR1bXBfc3RhY2srMHg5MC8w eGM4KSBbPGMwOWIzZjM0Pl0gKGR1bXBfc3RhY2spDQo+ID4+IGZyb20gWzxjMDEyNTA2MD5dIChf X3dhcm4rMHhlNC8weDExMCkgWzxjMDEyNTA2MD5dIChfX3dhcm4pIGZyb20NCj4gPj4gWzxjMDEy NTBjYz5dICh3YXJuX3Nsb3dwYXRoX251bGwrMHg0MC8weDQ4KSBbPGMwMTI1MGNjPl0NCj4gPj4g KHdhcm5fc2xvd3BhdGhfbnVsbCkgZnJvbSBbPGMwNDhkNTg0Pl0NCj4gPj4gKGNsa19jb3JlX2Rp c2FibGVfbG9jaysweDE4LzB4MjQpIFs8YzA0OGQ1ODQ+XQ0KPiA+PiAoY2xrX2NvcmVfZGlzYWJs ZV9sb2NrKSBmcm9tIFs8YzA0OGU0OTA+XQ0KPiA+PiAoY2xrX2NvcmVfZGlzYWJsZV91bnByZXBh cmUrMHhjLzB4MjApDQo+ID4+IFs8YzA0OGU0OTA+XSAoY2xrX2NvcmVfZGlzYWJsZV91bnByZXBh cmUpIGZyb20gWzxjMDQ4ZWE2MD5dDQo+ID4+IChfX2Nsa19zZXRfcGFyZW50X2FmdGVyKzB4NDgv MHg0YykNCj4gPj4gWzxjMDQ4ZWE2MD5dIChfX2Nsa19zZXRfcGFyZW50X2FmdGVyKSBmcm9tIFs8 YzA0OGVjODg+XQ0KPiA+PiAoY2xrX2NvcmVfc2V0X3BhcmVudF9ub2xvY2srMHgyMjQvMHg1YjQp DQo+ID4+IFs8YzA0OGVjODg+XSAoY2xrX2NvcmVfc2V0X3BhcmVudF9ub2xvY2spIGZyb20gWzxj MDQ4ZjIzYz5dDQo+ID4+IChjbGtfc2V0X3BhcmVudCsweDM4LzB4NmMpIFs8YzA0OGYyM2M+XSAo Y2xrX3NldF9wYXJlbnQpIGZyb20NCj4gPj4gWzxjMDQ5YzQxYz5dIChleHlub3NfcGRfcG93ZXIr MHg3NC8weDFjYykgWzxjMDQ5YzQxYz5dDQo+ID4+IChleHlub3NfcGRfcG93ZXIpIGZyb20gWzxj MDU1MDc2OD5dIChnZW5wZF9wb3dlcl9vZmYrMHgxNjQvMHgyNjQpDQo+ID4+IFs8YzA1NTA3Njg+ XSAoZ2VucGRfcG93ZXJfb2ZmKSBmcm9tIFs8YzA1NTBiMzQ+XQ0KPiA+PiAoZ2VucGRfcG93ZXJf b2ZmX3dvcmtfZm4rMHgyYy8weDQwKQ0KPiA+PiBbPGMwNTUwYjM0Pl0gKGdlbnBkX3Bvd2VyX29m Zl93b3JrX2ZuKSBmcm9tIFs8YzAxNDMyZTQ+XQ0KPiA+PiAocHJvY2Vzc19vbmVfd29yaysweDFk MC8weDdiYykgWzxjMDE0MzJlND5dIChwcm9jZXNzX29uZV93b3JrKSBmcm9tDQo+ID4+IFs8YzAx NDM5M2M+XSAod29ya2VyX3RocmVhZCsweDM0LzB4NGRjKSBbPGMwMTQzOTNjPl0gKHdvcmtlcl90 aHJlYWQpDQo+ID4+IGZyb20gWzxjMDE0YTBkMD5dIChrdGhyZWFkKzB4MTI4LzB4MTY0KSBbPGMw MTRhMGQwPl0gKGt0aHJlYWQpIGZyb20NCj4gPj4gWzxjMDEwMTBiND5dIChyZXRfZnJvbV9mb3Jr KzB4MTQvMHgyMCkgRXhjZXB0aW9uIHN0YWNrKDB4ZWViMDFmYjAgdG8NCj4gMHhlZWIwMWZmOCkN Cj4gPj4gMWZhMDogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgMDAwMDAwMDAg MDAwMDAwMDAgMDAwMDAwMDAgMDAwMDAwMDANCj4gPj4gMWZjMDogMDAwMDAwMDAgMDAwMDAwMDAg MDAwMDAwMDAgMDAwMDAwMDAgMDAwMDAwMDAgMDAwMDAwMDAgMDAwMDAwMDANCj4gPj4gMDAwMDAw MDANCj4gPj4gMWZlMDogMDAwMDAwMDAgMDAwMDAwMDAgMDAwMDAwMDAgMDAwMDAwMDAgMDAwMDAw MTMgMDAwMDAwMDAgLS0tWyBlbmQNCj4gPj4gdHJhY2UgNTAzYzIzOWZiNzYwZjE3YSBdLS0tIC0t LS0tLS0tLS0tLVsgY3V0IGhlcmUgXS0tLS0tLS0tLS0tLQ0KPiA+PiBXQVJOSU5HOiBDUFU6IDAg UElEOiA1OSBhdCBkcml2ZXJzL2Nsay9jbGsuYzo2ODQNCj4gPj4gY2xrX2NvcmVfZGlzYWJsZV91 bnByZXBhcmUrMHgxOC8weDIwDQo+ID4+IE1vZHVsZXMgbGlua2VkIGluOg0KPiA+PiBDUFU6IDAg UElEOiA1OSBDb21tOiBrd29ya2VyLzA6MSBUYWludGVkOiBHICAgICAgICBXICAgICAgICA0LjE1 LjAtcmM3LW5leHQtDQo+IDIwMTgwMTE1ICMxMDYNCj4gPj4gSGFyZHdhcmUgbmFtZTogU0FNU1VO RyBFWFlOT1MgKEZsYXR0ZW5lZCBEZXZpY2UgVHJlZSkNCj4gPj4gV29ya3F1ZXVlOiBwbSBnZW5w ZF9wb3dlcl9vZmZfd29ya19mbiBbPGMwMTExNTA0Pl0NCj4gKHVud2luZF9iYWNrdHJhY2UpDQo+ ID4+IGZyb20gWzxjMDEwZGJlYz5dIChzaG93X3N0YWNrKzB4MTAvMHgxNCkgWzxjMDEwZGJlYz5d IChzaG93X3N0YWNrKQ0KPiA+PiBmcm9tIFs8YzA5YjNmMzQ+XSAoZHVtcF9zdGFjaysweDkwLzB4 YzgpIFs8YzA5YjNmMzQ+XSAoZHVtcF9zdGFjaykNCj4gPj4gZnJvbSBbPGMwMTI1MDYwPl0gKF9f d2FybisweGU0LzB4MTEwKSBbPGMwMTI1MDYwPl0gKF9fd2FybikgZnJvbQ0KPiA+PiBbPGMwMTI1 MGNjPl0gKHdhcm5fc2xvd3BhdGhfbnVsbCsweDQwLzB4NDgpIFs8YzAxMjUwY2M+XQ0KPiA+PiAo d2Fybl9zbG93cGF0aF9udWxsKSBmcm9tIFs8YzA0OGU0OWM+XQ0KPiA+PiAoY2xrX2NvcmVfZGlz YWJsZV91bnByZXBhcmUrMHgxOC8weDIwKQ0KPiA+PiBbPGMwNDhlNDljPl0gKGNsa19jb3JlX2Rp c2FibGVfdW5wcmVwYXJlKSBmcm9tIFs8YzA0OGVhNjA+XQ0KPiA+PiAoX19jbGtfc2V0X3BhcmVu dF9hZnRlcisweDQ4LzB4NGMpDQo+ID4+IFs8YzA0OGVhNjA+XSAoX19jbGtfc2V0X3BhcmVudF9h ZnRlcikgZnJvbSBbPGMwNDhlYzg4Pl0NCj4gPj4gKGNsa19jb3JlX3NldF9wYXJlbnRfbm9sb2Nr KzB4MjI0LzB4NWI0KQ0KPiA+PiBbPGMwNDhlYzg4Pl0gKGNsa19jb3JlX3NldF9wYXJlbnRfbm9s b2NrKSBmcm9tIFs8YzA0OGYyM2M+XQ0KPiA+PiAoY2xrX3NldF9wYXJlbnQrMHgzOC8weDZjKSBb PGMwNDhmMjNjPl0gKGNsa19zZXRfcGFyZW50KSBmcm9tDQo+ID4+IFs8YzA0OWM0MWM+XSAoZXh5 bm9zX3BkX3Bvd2VyKzB4NzQvMHgxY2MpIFs8YzA0OWM0MWM+XQ0KPiA+PiAoZXh5bm9zX3BkX3Bv d2VyKSBmcm9tIFs8YzA1NTA3Njg+XSAoZ2VucGRfcG93ZXJfb2ZmKzB4MTY0LzB4MjY0KQ0KPiA+ PiBbPGMwNTUwNzY4Pl0gKGdlbnBkX3Bvd2VyX29mZikgZnJvbSBbPGMwNTUwYjM0Pl0NCj4gPj4g KGdlbnBkX3Bvd2VyX29mZl93b3JrX2ZuKzB4MmMvMHg0MCkNCj4gPj4gWzxjMDU1MGIzND5dIChn ZW5wZF9wb3dlcl9vZmZfd29ya19mbikgZnJvbSBbPGMwMTQzMmU0Pl0NCj4gPj4gKHByb2Nlc3Nf b25lX3dvcmsrMHgxZDAvMHg3YmMpIFs8YzAxNDMyZTQ+XSAocHJvY2Vzc19vbmVfd29yaykgZnJv bQ0KPiA+PiBbPGMwMTQzOTNjPl0gKHdvcmtlcl90aHJlYWQrMHgzNC8weDRkYykgWzxjMDE0Mzkz Yz5dICh3b3JrZXJfdGhyZWFkKQ0KPiA+PiBmcm9tIFs8YzAxNGEwZDA+XSAoa3RocmVhZCsweDEy OC8weDE2NCkgWzxjMDE0YTBkMD5dIChrdGhyZWFkKSBmcm9tDQo+ID4+IFs8YzAxMDEwYjQ+XSAo cmV0X2Zyb21fZm9yaysweDE0LzB4MjApIEV4Y2VwdGlvbiBzdGFjaygweGVlYjAxZmIwIHRvDQo+ IDB4ZWViMDFmZjgpDQo+ID4+IDFmYTA6ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgIDAwMDAwMDAwIDAwMDAwMDAwIDAwMDAwMDAwIDAwMDAwMDAwDQo+ID4+IDFmYzA6IDAwMDAw MDAwIDAwMDAwMDAwIDAwMDAwMDAwIDAwMDAwMDAwIDAwMDAwMDAwIDAwMDAwMDAwIDAwMDAwMDAw DQo+ID4+IDAwMDAwMDAwDQo+ID4+IDFmZTA6IDAwMDAwMDAwIDAwMDAwMDAwIDAwMDAwMDAwIDAw MDAwMDAwIDAwMDAwMDEzIDAwMDAwMDAwIC0tLVsgZW5kDQo+ID4+IHRyYWNlIDUwM2MyMzlmYjc2 MGYxN2IgXS0tLSAtLS0tLS0tLS0tLS1bIGN1dCBoZXJlIF0tLS0tLS0tLS0tLS0NCj4gPj4NCj4g Pj4gRml4ZXM6IGY4ZjhmMWQwNDQ5NCAoImNsazogRG9uJ3QgdG91Y2ggaGFyZHdhcmUgd2hlbiBy ZXBhcmVudGluZw0KPiA+PiBkdXJpbmcgcmVnaXN0cmF0aW9uIikNCj4gPj4gU2lnbmVkLW9mZi1i eTogTWFyZWsgU3p5cHJvd3NraSA8bS5zenlwcm93c2tpQHNhbXN1bmcuY29tPg0KPiA+PiAtLS0N Cj4gPj4gICBkcml2ZXJzL2Nsay9jbGsuYyB8IDQgKysrKw0KPiA+PiAgIDEgZmlsZSBjaGFuZ2Vk LCA0IGluc2VydGlvbnMoKykNCj4gPj4NCj4gPj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvY2xrL2Ns ay5jIGIvZHJpdmVycy9jbGsvY2xrLmMgaW5kZXgNCj4gPj4gMGY2ODZhOWRhYzNlLi5kMzNjMDg3 ZDc4NjggMTAwNjQ0DQo+ID4+IC0tLSBhL2RyaXZlcnMvY2xrL2Nsay5jDQo+ID4+ICsrKyBiL2Ry aXZlcnMvY2xrL2Nsay5jDQo+ID4+IEBAIC0yOTgyLDYgKzI5ODIsMTAgQEAgc3RhdGljIGludCBf X2Nsa19jb3JlX2luaXQoc3RydWN0IGNsa19jb3JlICpjb3JlKQ0KPiA+PiAgIAkJaWYgKHBhcmVu dCkgew0KPiA+PiAgIAkJCS8qIHVwZGF0ZSB0aGUgY2xrIHRyZWUgdG9wb2xvZ3kgKi8NCj4gPj4g ICAJCQlmbGFncyA9IGNsa19lbmFibGVfbG9jaygpOw0KPiA+IFRoaXMgaXMgdGhlIHNwaW5sb2Nr Pw0KPiANCj4geWVzDQo+IA0KPiA+DQo+ID4+ICsJCQlpZiAob3JwaGFuLT5wcmVwYXJlX2NvdW50 KQ0KPiA+PiArCQkJCWNsa19jb3JlX3ByZXBhcmUocGFyZW50KTsNCj4gPiBBbmQgdGhpcyBpcyBw cmVwYXJlIG11dGV4PyBEb2Vzbid0IHNvdW5kIGxpa2UgaXQgd2lsbCB3b3JrLg0KPiANCj4gUHJl cGFyZSBtdXRleCBpcyB0YWtlbiBhdCB0aGUgYmVnaW5uaW5nIG9mIF9fY2xrX2NvcmVfaW5pdCgp IGZ1bmN0aW9uLCBzbw0KPiBldmVyeXRoaW5nIGlzIGZpbmUgaGVyZS4NCj4gDQoNClRoZW9yZXRp Y2FsbHkgbm8gaXNzdWUgbWF5IGhhcHBlbiwganVzdCBsb29rcyBzdHJhbmdlIHRvIGNsYWltIG11 dGV4IHdpdGhpbg0Kc3BpbmxvY2suDQoNCj4gPj4gKwkJCWlmIChvcnBoYW4tPmVuYWJsZV9jb3Vu dCkNCj4gPj4gKwkJCQljbGtfY29yZV9lbmFibGUocGFyZW50KTsNCj4gPiBUaGlzIHdvdWxkIGJl IE9LLCBidXQgdGhlbiB3ZSBtaWdodCB0b3VjaCB0aGUgaGFyZHdhcmUgYWdhaW4/DQo+IA0KPiBX ZWxsLCBpdCB3aWxsIHRvdWNoIGhhcmR3YXJlIGluIG9uZSBjYXNlOiBpZiBvcnBoYW5lZCBjbG9j ayBpcyBhbHJlYWR5IHByZXBhcmVkDQo+IGFuZCBlbmFibGVkIChmb3IgZXhhbXBsZSBkdWUgdG8g aGF2aW5nIENMS19JU19DUklUSUNBTCBmbGFnKSBhbmQgdGhlIG5ld2x5DQo+IHJlZ2lzdGVyZWQg cGFyZW50IGlzIG5vdCAoeWV0PykgZW5hYmxlZC4gVGhlbiBpdCB3aWxsIHByZXBhcmUgYW5kIGVu YWJsZSB0aGlzDQo+IG5ldyBwYXJlbnQgY2xvY2ssIHdoYXQgaXMgYSBkZXNpcmVkIGJlaGF2aW9y Lg0KPiANCj4gVGhpcyBjb2RlIHdpbGwgbm90IHRlbXBvcmFyaWx5IGRpc2FibGUgYW55IGNsb2Nr cywgd2hhdCBJIGFzc3VtZSB3YXMgdGhlIG1haW4NCj4gcmVhc29uIGZvciB0aGUgcGF0Y2ggbWVu dGlvbmVkIGluIHRoZSBjb21taXQgbWVzc2FnZS4NCj4gDQoNClRoZW4gdGhpcyBjb2RlIHNlZW1z IGJ5cGFzcyB0aGUgQ0xLX09QU19QQVJFTlRfRU5BQkxFIGZlYXR1cmUuDQoNClJlZ2FyZHMNCkRv bmcgQWlzaGVuZw0KDQo+IEJlc3QgcmVnYXJkcw0KPiAtLQ0KPiBNYXJlayBTenlwcm93c2tpLCBQ aEQNCj4gU2Ftc3VuZyBSJkQgSW5zdGl0dXRlIFBvbGFuZA0KDQo= ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] clk: Properly update prepare/enable count on orphan clock reparent @ 2018-01-26 8:36 ` A.s. Dong 0 siblings, 0 replies; 14+ messages in thread From: A.s. Dong @ 2018-01-26 8:36 UTC (permalink / raw) To: linux-arm-kernel > -----Original Message----- > From: Marek Szyprowski [mailto:m.szyprowski at samsung.com] > Sent: Friday, January 26, 2018 3:31 PM > To: Stephen Boyd <sboyd@codeaurora.org> > Cc: linux-clk at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Michael > Turquette <mturquette@baylibre.com>; Shawn Guo <shawnguo@kernel.org>; > A.s. Dong <aisheng.dong@nxp.com>; Bartlomiej Zolnierkiewicz > <b.zolnierkie@samsung.com> > Subject: Re: [PATCH] clk: Properly update prepare/enable count on orphan > clock reparent > > Hi Stephen, > > On 2018-01-25 23:19, Stephen Boyd wrote: > > On 01/15, Marek Szyprowski wrote: > >> If orphaned clock has been already prepared/enabled (for example if > >> it or one of its children has CLK_IS_CRITICAL flag), then the > >> prepare/enable counters of the newly assigned parent are not updated > >> correctly. This might later cause warnings during changing clock parents. > >> > >> This patch fixes following warning on Exynos5422-based boards: > >> ------------[ cut here ]------------ > >> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 > >> clk_core_disable_lock+0x18/0x24 Modules linked in: > >> CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 > >> #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > >> Workqueue: pm genpd_power_off_work_fn [<c0111504>] > (unwind_backtrace) > >> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) > >> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) > >> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from > >> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] > >> (warn_slowpath_null) from [<c048d584>] > >> (clk_core_disable_lock+0x18/0x24) [<c048d584>] > >> (clk_core_disable_lock) from [<c048e490>] > >> (clk_core_disable_unprepare+0xc/0x20) > >> [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] > >> (__clk_set_parent_after+0x48/0x4c) > >> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] > >> (clk_core_set_parent_nolock+0x224/0x5b4) > >> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] > >> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from > >> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] > >> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) > >> [<c0550768>] (genpd_power_off) from [<c0550b34>] > >> (genpd_power_off_work_fn+0x2c/0x40) > >> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] > >> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from > >> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) > >> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from > >> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to > 0xeeb01ff8) > >> 1fa0: 00000000 00000000 00000000 00000000 > >> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > >> 00000000 > >> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end > >> trace 503c239fb760f17a ]--- ------------[ cut here ]------------ > >> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 > >> clk_core_disable_unprepare+0x18/0x20 > >> Modules linked in: > >> CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next- > 20180115 #106 > >> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) > >> Workqueue: pm genpd_power_off_work_fn [<c0111504>] > (unwind_backtrace) > >> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) > >> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) > >> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from > >> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] > >> (warn_slowpath_null) from [<c048e49c>] > >> (clk_core_disable_unprepare+0x18/0x20) > >> [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] > >> (__clk_set_parent_after+0x48/0x4c) > >> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] > >> (clk_core_set_parent_nolock+0x224/0x5b4) > >> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] > >> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from > >> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] > >> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) > >> [<c0550768>] (genpd_power_off) from [<c0550b34>] > >> (genpd_power_off_work_fn+0x2c/0x40) > >> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] > >> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from > >> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) > >> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from > >> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to > 0xeeb01ff8) > >> 1fa0: 00000000 00000000 00000000 00000000 > >> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 > >> 00000000 > >> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end > >> trace 503c239fb760f17b ]--- ------------[ cut here ]------------ > >> > >> Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting > >> during registration") > >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > >> --- > >> drivers/clk/clk.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index > >> 0f686a9dac3e..d33c087d7868 100644 > >> --- a/drivers/clk/clk.c > >> +++ b/drivers/clk/clk.c > >> @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) > >> if (parent) { > >> /* update the clk tree topology */ > >> flags = clk_enable_lock(); > > This is the spinlock? > > yes > > > > >> + if (orphan->prepare_count) > >> + clk_core_prepare(parent); > > And this is prepare mutex? Doesn't sound like it will work. > > Prepare mutex is taken at the beginning of __clk_core_init() function, so > everything is fine here. > Theoretically no issue may happen, just looks strange to claim mutex within spinlock. > >> + if (orphan->enable_count) > >> + clk_core_enable(parent); > > This would be OK, but then we might touch the hardware again? > > Well, it will touch hardware in one case: if orphaned clock is already prepared > and enabled (for example due to having CLK_IS_CRITICAL flag) and the newly > registered parent is not (yet?) enabled. Then it will prepare and enable this > new parent clock, what is a desired behavior. > > This code will not temporarily disable any clocks, what I assume was the main > reason for the patch mentioned in the commit message. > Then this code seems bypass the CLK_OPS_PARENT_ENABLE feature. Regards Dong Aisheng > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] clk: Properly update prepare/enable count on orphan clock reparent 2018-01-26 8:36 ` A.s. Dong @ 2018-01-26 9:06 ` Marek Szyprowski -1 siblings, 0 replies; 14+ messages in thread From: Marek Szyprowski @ 2018-01-26 9:06 UTC (permalink / raw) To: A.s. Dong, Stephen Boyd Cc: linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Michael Turquette, Shawn Guo, Bartlomiej Zolnierkiewicz, dl-linux-imx Hi Aisheng, On 2018-01-26 09:36, A.s. Dong wrote: >> -----Original Message----- >> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] >> Sent: Friday, January 26, 2018 3:31 PM >> To: Stephen Boyd <sboyd@codeaurora.org> >> Cc: linux-clk@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Michael >> Turquette <mturquette@baylibre.com>; Shawn Guo <shawnguo@kernel.org>; >> A.s. Dong <aisheng.dong@nxp.com>; Bartlomiej Zolnierkiewicz >> <b.zolnierkie@samsung.com> >> Subject: Re: [PATCH] clk: Properly update prepare/enable count on orphan >> clock reparent >> >> Hi Stephen, >> >> On 2018-01-25 23:19, Stephen Boyd wrote: >>> On 01/15, Marek Szyprowski wrote: >>>> If orphaned clock has been already prepared/enabled (for example if >>>> it or one of its children has CLK_IS_CRITICAL flag), then the >>>> prepare/enable counters of the newly assigned parent are not updated >>>> correctly. This might later cause warnings during changing clock parents. >>>> >>>> This patch fixes following warning on Exynos5422-based boards: >>>> ------------[ cut here ]------------ >>>> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 >>>> clk_core_disable_lock+0x18/0x24 Modules linked in: >>>> CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 >>>> #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>> Workqueue: pm genpd_power_off_work_fn [<c0111504>] >> (unwind_backtrace) >>>> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) >>>> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) >>>> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from >>>> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] >>>> (warn_slowpath_null) from [<c048d584>] >>>> (clk_core_disable_lock+0x18/0x24) [<c048d584>] >>>> (clk_core_disable_lock) from [<c048e490>] >>>> (clk_core_disable_unprepare+0xc/0x20) >>>> [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] >>>> (__clk_set_parent_after+0x48/0x4c) >>>> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] >>>> (clk_core_set_parent_nolock+0x224/0x5b4) >>>> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] >>>> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from >>>> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] >>>> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >>>> [<c0550768>] (genpd_power_off) from [<c0550b34>] >>>> (genpd_power_off_work_fn+0x2c/0x40) >>>> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] >>>> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from >>>> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) >>>> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from >>>> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to >> 0xeeb01ff8) >>>> 1fa0: 00000000 00000000 00000000 00000000 >>>> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >>>> 00000000 >>>> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end >>>> trace 503c239fb760f17a ]--- ------------[ cut here ]------------ >>>> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 >>>> clk_core_disable_unprepare+0x18/0x20 >>>> Modules linked in: >>>> CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next- >> 20180115 #106 >>>> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>> Workqueue: pm genpd_power_off_work_fn [<c0111504>] >> (unwind_backtrace) >>>> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) >>>> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) >>>> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from >>>> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] >>>> (warn_slowpath_null) from [<c048e49c>] >>>> (clk_core_disable_unprepare+0x18/0x20) >>>> [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] >>>> (__clk_set_parent_after+0x48/0x4c) >>>> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] >>>> (clk_core_set_parent_nolock+0x224/0x5b4) >>>> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] >>>> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from >>>> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] >>>> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >>>> [<c0550768>] (genpd_power_off) from [<c0550b34>] >>>> (genpd_power_off_work_fn+0x2c/0x40) >>>> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] >>>> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from >>>> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) >>>> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from >>>> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to >> 0xeeb01ff8) >>>> 1fa0: 00000000 00000000 00000000 00000000 >>>> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >>>> 00000000 >>>> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end >>>> trace 503c239fb760f17b ]--- ------------[ cut here ]------------ >>>> >>>> Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting >>>> during registration") >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> --- >>>> drivers/clk/clk.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index >>>> 0f686a9dac3e..d33c087d7868 100644 >>>> --- a/drivers/clk/clk.c >>>> +++ b/drivers/clk/clk.c >>>> @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) >>>> if (parent) { >>>> /* update the clk tree topology */ >>>> flags = clk_enable_lock(); >>> This is the spinlock? >> yes >> >>>> + if (orphan->prepare_count) >>>> + clk_core_prepare(parent); >>> And this is prepare mutex? Doesn't sound like it will work. >> Prepare mutex is taken at the beginning of __clk_core_init() function, so >> everything is fine here. >> > Theoretically no issue may happen, just looks strange to claim mutex within > spinlock. Nope, the mutex is taken before entering this block of code. What is strange then? clk_core_prepare() doesn't touch prepare mutex, there is also a clk_core_prepare_lock() function, which takes it. >>>> + if (orphan->enable_count) >>>> + clk_core_enable(parent); >>> This would be OK, but then we might touch the hardware again? >> Well, it will touch hardware in one case: if orphaned clock is already prepared >> and enabled (for example due to having CLK_IS_CRITICAL flag) and the newly >> registered parent is not (yet?) enabled. Then it will prepare and enable this >> new parent clock, what is a desired behavior. >> >> This code will not temporarily disable any clocks, what I assume was the main >> reason for the patch mentioned in the commit message. >> > Then this code seems bypass the CLK_OPS_PARENT_ENABLE feature. Well, CLK_OPS_PARENT_ENABLE doesn't affect preparing/enabling parent clock if child clock has been already prepared/enabled. That's all that this fix do, so I don't know what do you mean by bypassing the CLK_OPS_PARENT_ENABLE feature. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] clk: Properly update prepare/enable count on orphan clock reparent @ 2018-01-26 9:06 ` Marek Szyprowski 0 siblings, 0 replies; 14+ messages in thread From: Marek Szyprowski @ 2018-01-26 9:06 UTC (permalink / raw) To: linux-arm-kernel Hi Aisheng, On 2018-01-26 09:36, A.s. Dong wrote: >> -----Original Message----- >> From: Marek Szyprowski [mailto:m.szyprowski at samsung.com] >> Sent: Friday, January 26, 2018 3:31 PM >> To: Stephen Boyd <sboyd@codeaurora.org> >> Cc: linux-clk at vger.kernel.org; linux-arm-kernel at lists.infradead.org; Michael >> Turquette <mturquette@baylibre.com>; Shawn Guo <shawnguo@kernel.org>; >> A.s. Dong <aisheng.dong@nxp.com>; Bartlomiej Zolnierkiewicz >> <b.zolnierkie@samsung.com> >> Subject: Re: [PATCH] clk: Properly update prepare/enable count on orphan >> clock reparent >> >> Hi Stephen, >> >> On 2018-01-25 23:19, Stephen Boyd wrote: >>> On 01/15, Marek Szyprowski wrote: >>>> If orphaned clock has been already prepared/enabled (for example if >>>> it or one of its children has CLK_IS_CRITICAL flag), then the >>>> prepare/enable counters of the newly assigned parent are not updated >>>> correctly. This might later cause warnings during changing clock parents. >>>> >>>> This patch fixes following warning on Exynos5422-based boards: >>>> ------------[ cut here ]------------ >>>> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 >>>> clk_core_disable_lock+0x18/0x24 Modules linked in: >>>> CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 >>>> #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>> Workqueue: pm genpd_power_off_work_fn [<c0111504>] >> (unwind_backtrace) >>>> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) >>>> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) >>>> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from >>>> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] >>>> (warn_slowpath_null) from [<c048d584>] >>>> (clk_core_disable_lock+0x18/0x24) [<c048d584>] >>>> (clk_core_disable_lock) from [<c048e490>] >>>> (clk_core_disable_unprepare+0xc/0x20) >>>> [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] >>>> (__clk_set_parent_after+0x48/0x4c) >>>> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] >>>> (clk_core_set_parent_nolock+0x224/0x5b4) >>>> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] >>>> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from >>>> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] >>>> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >>>> [<c0550768>] (genpd_power_off) from [<c0550b34>] >>>> (genpd_power_off_work_fn+0x2c/0x40) >>>> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] >>>> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from >>>> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) >>>> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from >>>> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to >> 0xeeb01ff8) >>>> 1fa0: 00000000 00000000 00000000 00000000 >>>> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >>>> 00000000 >>>> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end >>>> trace 503c239fb760f17a ]--- ------------[ cut here ]------------ >>>> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 >>>> clk_core_disable_unprepare+0x18/0x20 >>>> Modules linked in: >>>> CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next- >> 20180115 #106 >>>> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>> Workqueue: pm genpd_power_off_work_fn [<c0111504>] >> (unwind_backtrace) >>>> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) >>>> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) >>>> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from >>>> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] >>>> (warn_slowpath_null) from [<c048e49c>] >>>> (clk_core_disable_unprepare+0x18/0x20) >>>> [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] >>>> (__clk_set_parent_after+0x48/0x4c) >>>> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] >>>> (clk_core_set_parent_nolock+0x224/0x5b4) >>>> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] >>>> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from >>>> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] >>>> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >>>> [<c0550768>] (genpd_power_off) from [<c0550b34>] >>>> (genpd_power_off_work_fn+0x2c/0x40) >>>> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] >>>> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from >>>> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) >>>> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from >>>> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to >> 0xeeb01ff8) >>>> 1fa0: 00000000 00000000 00000000 00000000 >>>> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >>>> 00000000 >>>> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end >>>> trace 503c239fb760f17b ]--- ------------[ cut here ]------------ >>>> >>>> Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting >>>> during registration") >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>> --- >>>> drivers/clk/clk.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index >>>> 0f686a9dac3e..d33c087d7868 100644 >>>> --- a/drivers/clk/clk.c >>>> +++ b/drivers/clk/clk.c >>>> @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core *core) >>>> if (parent) { >>>> /* update the clk tree topology */ >>>> flags = clk_enable_lock(); >>> This is the spinlock? >> yes >> >>>> + if (orphan->prepare_count) >>>> + clk_core_prepare(parent); >>> And this is prepare mutex? Doesn't sound like it will work. >> Prepare mutex is taken at the beginning of __clk_core_init() function, so >> everything is fine here. >> > Theoretically no issue may happen, just looks strange to claim mutex within > spinlock. Nope, the mutex is taken before entering this block of code. What is strange then? clk_core_prepare() doesn't touch prepare mutex, there is also a clk_core_prepare_lock() function, which takes it. >>>> + if (orphan->enable_count) >>>> + clk_core_enable(parent); >>> This would be OK, but then we might touch the hardware again? >> Well, it will touch hardware in one case: if orphaned clock is already prepared >> and enabled (for example due to having CLK_IS_CRITICAL flag) and the newly >> registered parent is not (yet?) enabled. Then it will prepare and enable this >> new parent clock, what is a desired behavior. >> >> This code will not temporarily disable any clocks, what I assume was the main >> reason for the patch mentioned in the commit message. >> > Then this code seems bypass the CLK_OPS_PARENT_ENABLE feature. Well, CLK_OPS_PARENT_ENABLE doesn't affect preparing/enabling parent clock if child clock has been already prepared/enabled. That's all that this fix do, so I don't know what do you mean by bypassing the CLK_OPS_PARENT_ENABLE feature. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] clk: Properly update prepare/enable count on orphan clock reparent 2018-01-26 9:06 ` Marek Szyprowski @ 2018-01-26 9:14 ` Marek Szyprowski -1 siblings, 0 replies; 14+ messages in thread From: Marek Szyprowski @ 2018-01-26 9:14 UTC (permalink / raw) To: A.s. Dong, Stephen Boyd Cc: linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Michael Turquette, Shawn Guo, Bartlomiej Zolnierkiewicz, dl-linux-imx Hi Aisheng and Stephen, On 2018-01-26 10:06, Marek Szyprowski wrote: > Hi Aisheng, > > On 2018-01-26 09:36, A.s. Dong wrote: >>> -----Original Message----- >>> From: Marek Szyprowski [mailto:m.szyprowski@samsung.com] >>> Sent: Friday, January 26, 2018 3:31 PM >>> To: Stephen Boyd <sboyd@codeaurora.org> >>> Cc: linux-clk@vger.kernel.org; linux-arm-kernel@lists.infradead.org; >>> Michael >>> Turquette <mturquette@baylibre.com>; Shawn Guo <shawnguo@kernel.org>; >>> A.s. Dong <aisheng.dong@nxp.com>; Bartlomiej Zolnierkiewicz >>> <b.zolnierkie@samsung.com> >>> Subject: Re: [PATCH] clk: Properly update prepare/enable count on >>> orphan >>> clock reparent >>> >>> Hi Stephen, >>> >>> On 2018-01-25 23:19, Stephen Boyd wrote: >>>> On 01/15, Marek Szyprowski wrote: >>>>> If orphaned clock has been already prepared/enabled (for example if >>>>> it or one of its children has CLK_IS_CRITICAL flag), then the >>>>> prepare/enable counters of the newly assigned parent are not updated >>>>> correctly. This might later cause warnings during changing clock >>>>> parents. >>>>> >>>>> This patch fixes following warning on Exynos5422-based boards: >>>>> ------------[ cut here ]------------ >>>>> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 >>>>> clk_core_disable_lock+0x18/0x24 Modules linked in: >>>>> CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 >>>>> #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>>> Workqueue: pm genpd_power_off_work_fn [<c0111504>] >>> (unwind_backtrace) >>>>> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) >>>>> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) >>>>> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from >>>>> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] >>>>> (warn_slowpath_null) from [<c048d584>] >>>>> (clk_core_disable_lock+0x18/0x24) [<c048d584>] >>>>> (clk_core_disable_lock) from [<c048e490>] >>>>> (clk_core_disable_unprepare+0xc/0x20) >>>>> [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] >>>>> (__clk_set_parent_after+0x48/0x4c) >>>>> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] >>>>> (clk_core_set_parent_nolock+0x224/0x5b4) >>>>> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] >>>>> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from >>>>> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] >>>>> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >>>>> [<c0550768>] (genpd_power_off) from [<c0550b34>] >>>>> (genpd_power_off_work_fn+0x2c/0x40) >>>>> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] >>>>> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from >>>>> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) >>>>> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from >>>>> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to >>> 0xeeb01ff8) >>>>> 1fa0: 00000000 00000000 00000000 00000000 >>>>> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >>>>> 00000000 >>>>> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end >>>>> trace 503c239fb760f17a ]--- ------------[ cut here ]------------ >>>>> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 >>>>> clk_core_disable_unprepare+0x18/0x20 >>>>> Modules linked in: >>>>> CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W 4.15.0-rc7-next- >>> 20180115 #106 >>>>> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>>> Workqueue: pm genpd_power_off_work_fn [<c0111504>] >>> (unwind_backtrace) >>>>> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) >>>>> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) >>>>> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from >>>>> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] >>>>> (warn_slowpath_null) from [<c048e49c>] >>>>> (clk_core_disable_unprepare+0x18/0x20) >>>>> [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] >>>>> (__clk_set_parent_after+0x48/0x4c) >>>>> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] >>>>> (clk_core_set_parent_nolock+0x224/0x5b4) >>>>> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] >>>>> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from >>>>> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] >>>>> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >>>>> [<c0550768>] (genpd_power_off) from [<c0550b34>] >>>>> (genpd_power_off_work_fn+0x2c/0x40) >>>>> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] >>>>> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from >>>>> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) >>>>> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from >>>>> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to >>> 0xeeb01ff8) >>>>> 1fa0: 00000000 00000000 00000000 00000000 >>>>> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >>>>> 00000000 >>>>> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end >>>>> trace 503c239fb760f17b ]--- ------------[ cut here ]------------ >>>>> >>>>> Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting >>>>> during registration") >>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>>> --- >>>>> drivers/clk/clk.c | 4 ++++ >>>>> 1 file changed, 4 insertions(+) >>>>> >>>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index >>>>> 0f686a9dac3e..d33c087d7868 100644 >>>>> --- a/drivers/clk/clk.c >>>>> +++ b/drivers/clk/clk.c >>>>> @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core >>>>> *core) >>>>> if (parent) { >>>>> /* update the clk tree topology */ >>>>> flags = clk_enable_lock(); >>>> This is the spinlock? >>> yes >>> >>>>> + if (orphan->prepare_count) >>>>> + clk_core_prepare(parent); >>>> And this is prepare mutex? Doesn't sound like it will work. >>> Prepare mutex is taken at the beginning of __clk_core_init() >>> function, so >>> everything is fine here. >>> >> Theoretically no issue may happen, just looks strange to claim mutex >> within >> spinlock. > > Nope, the mutex is taken before entering this block of code. What is > strange > then? clk_core_prepare() doesn't touch prepare mutex, there is also > a clk_core_prepare_lock() function, which takes it. Okay, I got your concerns. To make this part a bit easier to understand, I will move "if (orphan->prepare_count) clk_core_prepare(parent);" to be called before "flags = clk_enable_lock();". >>>>> + if (orphan->enable_count) >>>>> + clk_core_enable(parent); >>>> This would be OK, but then we might touch the hardware again? >>> Well, it will touch hardware in one case: if orphaned clock is >>> already prepared >>> and enabled (for example due to having CLK_IS_CRITICAL flag) and the >>> newly >>> registered parent is not (yet?) enabled. Then it will prepare and >>> enable this >>> new parent clock, what is a desired behavior. >>> >>> This code will not temporarily disable any clocks, what I assume was >>> the main >>> reason for the patch mentioned in the commit message. >>> >> Then this code seems bypass the CLK_OPS_PARENT_ENABLE feature. > > Well, CLK_OPS_PARENT_ENABLE doesn't affect preparing/enabling parent > clock if > child clock has been already prepared/enabled. That's all that this > fix do, so > I don't know what do you mean by bypassing the CLK_OPS_PARENT_ENABLE > feature. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] clk: Properly update prepare/enable count on orphan clock reparent @ 2018-01-26 9:14 ` Marek Szyprowski 0 siblings, 0 replies; 14+ messages in thread From: Marek Szyprowski @ 2018-01-26 9:14 UTC (permalink / raw) To: linux-arm-kernel Hi Aisheng and Stephen, On 2018-01-26 10:06, Marek Szyprowski wrote: > Hi Aisheng, > > On 2018-01-26 09:36, A.s. Dong wrote: >>> -----Original Message----- >>> From: Marek Szyprowski [mailto:m.szyprowski at samsung.com] >>> Sent: Friday, January 26, 2018 3:31 PM >>> To: Stephen Boyd <sboyd@codeaurora.org> >>> Cc: linux-clk at vger.kernel.org; linux-arm-kernel at lists.infradead.org; >>> Michael >>> Turquette <mturquette@baylibre.com>; Shawn Guo <shawnguo@kernel.org>; >>> A.s. Dong <aisheng.dong@nxp.com>; Bartlomiej Zolnierkiewicz >>> <b.zolnierkie@samsung.com> >>> Subject: Re: [PATCH] clk: Properly update prepare/enable count on >>> orphan >>> clock reparent >>> >>> Hi Stephen, >>> >>> On 2018-01-25 23:19, Stephen Boyd wrote: >>>> On 01/15, Marek Szyprowski wrote: >>>>> If orphaned clock has been already prepared/enabled (for example if >>>>> it or one of its children has CLK_IS_CRITICAL flag), then the >>>>> prepare/enable counters of the newly assigned parent are not updated >>>>> correctly. This might later cause warnings during changing clock >>>>> parents. >>>>> >>>>> This patch fixes following warning on Exynos5422-based boards: >>>>> ------------[ cut here ]------------ >>>>> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:811 >>>>> clk_core_disable_lock+0x18/0x24 Modules linked in: >>>>> CPU: 0 PID: 59 Comm: kworker/0:1 Not tainted 4.15.0-rc7-next-20180115 >>>>> #106 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>>> Workqueue: pm genpd_power_off_work_fn [<c0111504>] >>> (unwind_backtrace) >>>>> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) >>>>> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) >>>>> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from >>>>> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] >>>>> (warn_slowpath_null) from [<c048d584>] >>>>> (clk_core_disable_lock+0x18/0x24) [<c048d584>] >>>>> (clk_core_disable_lock) from [<c048e490>] >>>>> (clk_core_disable_unprepare+0xc/0x20) >>>>> [<c048e490>] (clk_core_disable_unprepare) from [<c048ea60>] >>>>> (__clk_set_parent_after+0x48/0x4c) >>>>> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] >>>>> (clk_core_set_parent_nolock+0x224/0x5b4) >>>>> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] >>>>> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from >>>>> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] >>>>> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >>>>> [<c0550768>] (genpd_power_off) from [<c0550b34>] >>>>> (genpd_power_off_work_fn+0x2c/0x40) >>>>> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] >>>>> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from >>>>> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) >>>>> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from >>>>> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to >>> 0xeeb01ff8) >>>>> 1fa0: 00000000 00000000 00000000 00000000 >>>>> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >>>>> 00000000 >>>>> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end >>>>> trace 503c239fb760f17a ]--- ------------[ cut here ]------------ >>>>> WARNING: CPU: 0 PID: 59 at drivers/clk/clk.c:684 >>>>> clk_core_disable_unprepare+0x18/0x20 >>>>> Modules linked in: >>>>> CPU: 0 PID: 59 Comm: kworker/0:1 Tainted: G W??????? 4.15.0-rc7-next- >>> 20180115 #106 >>>>> Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) >>>>> Workqueue: pm genpd_power_off_work_fn [<c0111504>] >>> (unwind_backtrace) >>>>> from [<c010dbec>] (show_stack+0x10/0x14) [<c010dbec>] (show_stack) >>>>> from [<c09b3f34>] (dump_stack+0x90/0xc8) [<c09b3f34>] (dump_stack) >>>>> from [<c0125060>] (__warn+0xe4/0x110) [<c0125060>] (__warn) from >>>>> [<c01250cc>] (warn_slowpath_null+0x40/0x48) [<c01250cc>] >>>>> (warn_slowpath_null) from [<c048e49c>] >>>>> (clk_core_disable_unprepare+0x18/0x20) >>>>> [<c048e49c>] (clk_core_disable_unprepare) from [<c048ea60>] >>>>> (__clk_set_parent_after+0x48/0x4c) >>>>> [<c048ea60>] (__clk_set_parent_after) from [<c048ec88>] >>>>> (clk_core_set_parent_nolock+0x224/0x5b4) >>>>> [<c048ec88>] (clk_core_set_parent_nolock) from [<c048f23c>] >>>>> (clk_set_parent+0x38/0x6c) [<c048f23c>] (clk_set_parent) from >>>>> [<c049c41c>] (exynos_pd_power+0x74/0x1cc) [<c049c41c>] >>>>> (exynos_pd_power) from [<c0550768>] (genpd_power_off+0x164/0x264) >>>>> [<c0550768>] (genpd_power_off) from [<c0550b34>] >>>>> (genpd_power_off_work_fn+0x2c/0x40) >>>>> [<c0550b34>] (genpd_power_off_work_fn) from [<c01432e4>] >>>>> (process_one_work+0x1d0/0x7bc) [<c01432e4>] (process_one_work) from >>>>> [<c014393c>] (worker_thread+0x34/0x4dc) [<c014393c>] (worker_thread) >>>>> from [<c014a0d0>] (kthread+0x128/0x164) [<c014a0d0>] (kthread) from >>>>> [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xeeb01fb0 to >>> 0xeeb01ff8) >>>>> 1fa0: 00000000 00000000 00000000 00000000 >>>>> 1fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 >>>>> 00000000 >>>>> 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ---[ end >>>>> trace 503c239fb760f17b ]--- ------------[ cut here ]------------ >>>>> >>>>> Fixes: f8f8f1d04494 ("clk: Don't touch hardware when reparenting >>>>> during registration") >>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >>>>> --- >>>>> ?? drivers/clk/clk.c | 4 ++++ >>>>> ?? 1 file changed, 4 insertions(+) >>>>> >>>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index >>>>> 0f686a9dac3e..d33c087d7868 100644 >>>>> --- a/drivers/clk/clk.c >>>>> +++ b/drivers/clk/clk.c >>>>> @@ -2982,6 +2982,10 @@ static int __clk_core_init(struct clk_core >>>>> *core) >>>>> ?????????? if (parent) { >>>>> ?????????????? /* update the clk tree topology */ >>>>> ?????????????? flags = clk_enable_lock(); >>>> This is the spinlock? >>> yes >>> >>>>> +??????????? if (orphan->prepare_count) >>>>> +??????????????? clk_core_prepare(parent); >>>> And this is prepare mutex? Doesn't sound like it will work. >>> Prepare mutex is taken at the beginning of __clk_core_init() >>> function, so >>> everything is fine here. >>> >> Theoretically no issue may happen, just looks strange to claim mutex >> within >> spinlock. > > Nope, the mutex is taken before entering this block of code. What is > strange > then? clk_core_prepare() doesn't touch prepare mutex, there is also > a clk_core_prepare_lock() function, which takes it. Okay, I got your concerns. To make this part a bit easier to understand, I will move "if (orphan->prepare_count) clk_core_prepare(parent);" to be called before "flags = clk_enable_lock();". >>>>> +??????????? if (orphan->enable_count) >>>>> +??????????????? clk_core_enable(parent); >>>> This would be OK, but then we might touch the hardware again? >>> Well, it will touch hardware in one case: if orphaned clock is >>> already prepared >>> and enabled (for example due to having CLK_IS_CRITICAL flag) and the >>> newly >>> registered parent is not (yet?) enabled. Then it will prepare and >>> enable this >>> new parent clock, what is a desired behavior. >>> >>> This code will not temporarily disable any clocks, what I assume was >>> the main >>> reason for the patch mentioned in the commit message. >>> >> Then this code seems bypass the CLK_OPS_PARENT_ENABLE feature. > > Well, CLK_OPS_PARENT_ENABLE doesn't affect preparing/enabling parent > clock if > child clock has been already prepared/enabled. That's all that this > fix do, so > I don't know what do you mean by bypassing the CLK_OPS_PARENT_ENABLE > feature. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-01-26 9:14 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20180115115231eucas1p1499f46ca64b65598d03a182ed5670f28@eucas1p1.samsung.com>
2018-01-15 11:52 ` [PATCH] clk: Properly update prepare/enable count on orphan clock reparent Marek Szyprowski
2018-01-15 11:52 ` Marek Szyprowski
2018-01-17 8:05 ` Krzysztof Kozlowski
2018-01-17 8:05 ` Krzysztof Kozlowski
2018-01-25 22:19 ` Stephen Boyd
2018-01-25 22:19 ` Stephen Boyd
2018-01-26 7:31 ` Marek Szyprowski
2018-01-26 7:31 ` Marek Szyprowski
2018-01-26 8:36 ` A.s. Dong
2018-01-26 8:36 ` A.s. Dong
2018-01-26 9:06 ` Marek Szyprowski
2018-01-26 9:06 ` Marek Szyprowski
2018-01-26 9:14 ` Marek Szyprowski
2018-01-26 9:14 ` Marek Szyprowski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.