* [PATCH] clk: Warn about critical clks that fail to enable
@ 2019-12-26 22:13 Stephen Boyd
2019-12-28 15:40 ` Markus Elfring
2019-12-30 17:55 ` Stephen Boyd
0 siblings, 2 replies; 4+ messages in thread
From: Stephen Boyd @ 2019-12-26 22:13 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-kernel, linux-clk, Jerome Brunet, Guenter Roeck
If we don't warn here users of the CLK_IS_CRITICAL flag may not know
that their clk isn't actually enabled because it silently fails to
enable. Let's drop a big WARN_ON in that case so developers find these
problems faster.
Suggested-by: Jerome Brunet <jbrunet@baylibre.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
---
I suspect that this may start warning for other users. Let's see
and revert in case it doesn't work.
drivers/clk/clk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 772258de2d1f..6a9a66dfdeaa 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -3427,13 +3427,13 @@ static int __clk_core_init(struct clk_core *core)
unsigned long flags;
ret = clk_core_prepare(core);
- if (ret)
+ if (WARN_ON(ret))
goto out;
flags = clk_enable_lock();
ret = clk_core_enable(core);
clk_enable_unlock(flags);
- if (ret) {
+ if (WARN_ON(ret)) {
clk_core_unprepare(core);
goto out;
}
base-commit: 12ead77432f2ce32dea797742316d15c5800cb32
--
Sent by a computer, using git, on the internet
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] clk: Warn about critical clks that fail to enable
2019-12-26 22:13 [PATCH] clk: Warn about critical clks that fail to enable Stephen Boyd
@ 2019-12-28 15:40 ` Markus Elfring
2019-12-30 17:55 ` Stephen Boyd
1 sibling, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2019-12-28 15:40 UTC (permalink / raw)
To: Stephen Boyd, Michael Turquette, linux-clk
Cc: linux-kernel, Günter Röck, Jerome Brunet
> If we don't warn here users of the CLK_IS_CRITICAL flag may not know
> that their clk isn't actually enabled because it silently fails to
> enable. Let's drop a big WARN_ON in that case so developers find these
> problems faster.
Would you like to avoid an extra warning for failures with non-critical clocks?
> I suspect that this may start warning for other users.
Will this software development concern trigger any further collateral evolution?
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clk: Warn about critical clks that fail to enable
2019-12-26 22:13 [PATCH] clk: Warn about critical clks that fail to enable Stephen Boyd
2019-12-28 15:40 ` Markus Elfring
@ 2019-12-30 17:55 ` Stephen Boyd
2019-12-30 18:46 ` Guenter Roeck
1 sibling, 1 reply; 4+ messages in thread
From: Stephen Boyd @ 2019-12-30 17:55 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd
Cc: linux-kernel, linux-clk, Jerome Brunet, Guenter Roeck
Quoting Stephen Boyd (2019-12-26 14:13:54)
> If we don't warn here users of the CLK_IS_CRITICAL flag may not know
> that their clk isn't actually enabled because it silently fails to
> enable. Let's drop a big WARN_ON in that case so developers find these
> problems faster.
>
> Suggested-by: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---
>
> I suspect that this may start warning for other users. Let's see
> and revert in case it doesn't work.
>
> drivers/clk/clk.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 772258de2d1f..6a9a66dfdeaa 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3427,13 +3427,13 @@ static int __clk_core_init(struct clk_core *core)
> unsigned long flags;
>
> ret = clk_core_prepare(core);
> - if (ret)
> + if (WARN_ON(ret))
We should probably just print the clk name that failed to be critically
enabled with a pr_warn(). The traceback pointing to this location will
be really hard to understand otherwise.
> goto out;
>
> flags = clk_enable_lock();
> ret = clk_core_enable(core);
> clk_enable_unlock(flags);
> - if (ret) {
> + if (WARN_ON(ret)) {
> clk_core_unprepare(core);
> goto out;
> }
>
> base-commit: 12ead77432f2ce32dea797742316d15c5800cb32
> --
> Sent by a computer, using git, on the internet
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] clk: Warn about critical clks that fail to enable
2019-12-30 17:55 ` Stephen Boyd
@ 2019-12-30 18:46 ` Guenter Roeck
0 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2019-12-30 18:46 UTC (permalink / raw)
To: Stephen Boyd; +Cc: Michael Turquette, linux-kernel, linux-clk, Jerome Brunet
On Mon, Dec 30, 2019 at 09:55:17AM -0800, Stephen Boyd wrote:
> Quoting Stephen Boyd (2019-12-26 14:13:54)
> > If we don't warn here users of the CLK_IS_CRITICAL flag may not know
> > that their clk isn't actually enabled because it silently fails to
> > enable. Let's drop a big WARN_ON in that case so developers find these
> > problems faster.
> >
> > Suggested-by: Jerome Brunet <jbrunet@baylibre.com>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> > ---
> >
> > I suspect that this may start warning for other users. Let's see
> > and revert in case it doesn't work.
> >
> > drivers/clk/clk.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> > index 772258de2d1f..6a9a66dfdeaa 100644
> > --- a/drivers/clk/clk.c
> > +++ b/drivers/clk/clk.c
> > @@ -3427,13 +3427,13 @@ static int __clk_core_init(struct clk_core *core)
> > unsigned long flags;
> >
> > ret = clk_core_prepare(core);
> > - if (ret)
> > + if (WARN_ON(ret))
>
> We should probably just print the clk name that failed to be critically
> enabled with a pr_warn(). The traceback pointing to this location will
> be really hard to understand otherwise.
>
Agreed.
Guenter
> > goto out;
> >
> > flags = clk_enable_lock();
> > ret = clk_core_enable(core);
> > clk_enable_unlock(flags);
> > - if (ret) {
> > + if (WARN_ON(ret)) {
> > clk_core_unprepare(core);
> > goto out;
> > }
> >
> > base-commit: 12ead77432f2ce32dea797742316d15c5800cb32
> > --
> > Sent by a computer, using git, on the internet
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-30 18:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-26 22:13 [PATCH] clk: Warn about critical clks that fail to enable Stephen Boyd
2019-12-28 15:40 ` Markus Elfring
2019-12-30 17:55 ` Stephen Boyd
2019-12-30 18:46 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).