All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: add clock panic dump in tree-view
@ 2018-05-10  8:50 ` Shawn Lin
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn Lin @ 2018-05-10  8:50 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd; +Cc: linux-rockchip, linux-clk, Shawn Lin

Sometimes it's useful and for debugging to check the whole system clock
configuration in tree-view upon panic.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 Documentation/admin-guide/kernel-parameters.txt |  3 ++
 drivers/clk/clk.c                               | 67 +++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 3487be7..7bbdb6f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -519,6 +519,9 @@
 			debug and development, but should not be needed on a
 			platform with proper driver support.  For more
 			information, see Documentation/clk.txt.
+	clk_panic_dump
+			[CLK]
+			Dump the whole clock tree-view upon panic.
 
 	clock=		[BUGS=X86-32, HW] gettimeofday clocksource override.
 			[Deprecated]
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 9ae92aa..adb5537 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2802,6 +2802,73 @@ static inline void clk_debug_unregister(struct clk_core *core)
 }
 #endif
 
+static bool clk_pdump_enable;
+static int __init clk_pdump_setup(char *__unused)
+{
+	clk_pdump_enable  = true;
+	return 1;
+}
+__setup("clk_panic_dump", clk_pdump_setup);
+
+static void clk_pdump_show_one(struct clk_core *c, int level)
+{
+	if (!c)
+		return;
+
+	pr_err("%*s%-*s %11d %12d %11lu %10lu %-3d\n",
+		level * 3 + 1, "", 30 - level * 3, c->name,
+		c->enable_count, c->prepare_count, clk_core_get_rate(c),
+		clk_core_get_accuracy(c), clk_core_get_phase(c));
+}
+
+static void clk_pdump_show_subtree(struct clk_core *c, int level)
+{
+	struct clk_core *child;
+
+	if (!c)
+		return;
+
+	clk_pdump_show_one(c, level);
+
+	hlist_for_each_entry(child, &c->children, child_node)
+		clk_pdump_show_subtree(child, level + 1);
+}
+
+static int clk_panic_dump(struct notifier_block *this, unsigned long ev,
+			  void *ptr)
+{
+	struct clk_core *c;
+
+	if (!clk_pdump_enable)
+		return 0;
+
+	pr_err("clock panic dump:\n");
+	pr_err("   clock                         enable_cnt  prepare_cnt        rate   accuracy   phase\n");
+	pr_err("----------------------------------------------------------------------------------------\n");
+
+	clk_prepare_lock();
+
+	hlist_for_each_entry(c, &clk_root_list, child_node)
+		clk_pdump_show_subtree(c, 0);
+	hlist_for_each_entry(c, &clk_orphan_list, child_node)
+		clk_pdump_show_subtree(c, 0);
+
+	clk_prepare_unlock();
+
+	return 0;
+}
+
+static struct notifier_block clk_pdump_block = {
+	.notifier_call = clk_panic_dump,
+};
+
+static int clk_register_pdump(void)
+{
+	atomic_notifier_chain_register(&panic_notifier_list, &clk_pdump_block);
+	return 0;
+}
+late_initcall_sync(clk_register_pdump);
+
 /**
  * __clk_core_init - initialize the data structures in a struct clk_core
  * @core:	clk_core being initialized
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] clk: add clock panic dump in tree-view
@ 2018-05-10  8:50 ` Shawn Lin
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn Lin @ 2018-05-10  8:50 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Shawn Lin,
	linux-clk-u79uwXL29TY76Z2rM5mHXA

Sometimes it's useful and for debugging to check the whole system clock
configuration in tree-view upon panic.

Signed-off-by: Shawn Lin <shawn.lin-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
---

 Documentation/admin-guide/kernel-parameters.txt |  3 ++
 drivers/clk/clk.c                               | 67 +++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 3487be7..7bbdb6f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -519,6 +519,9 @@
 			debug and development, but should not be needed on a
 			platform with proper driver support.  For more
 			information, see Documentation/clk.txt.
+	clk_panic_dump
+			[CLK]
+			Dump the whole clock tree-view upon panic.
 
 	clock=		[BUGS=X86-32, HW] gettimeofday clocksource override.
 			[Deprecated]
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 9ae92aa..adb5537 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2802,6 +2802,73 @@ static inline void clk_debug_unregister(struct clk_core *core)
 }
 #endif
 
+static bool clk_pdump_enable;
+static int __init clk_pdump_setup(char *__unused)
+{
+	clk_pdump_enable  = true;
+	return 1;
+}
+__setup("clk_panic_dump", clk_pdump_setup);
+
+static void clk_pdump_show_one(struct clk_core *c, int level)
+{
+	if (!c)
+		return;
+
+	pr_err("%*s%-*s %11d %12d %11lu %10lu %-3d\n",
+		level * 3 + 1, "", 30 - level * 3, c->name,
+		c->enable_count, c->prepare_count, clk_core_get_rate(c),
+		clk_core_get_accuracy(c), clk_core_get_phase(c));
+}
+
+static void clk_pdump_show_subtree(struct clk_core *c, int level)
+{
+	struct clk_core *child;
+
+	if (!c)
+		return;
+
+	clk_pdump_show_one(c, level);
+
+	hlist_for_each_entry(child, &c->children, child_node)
+		clk_pdump_show_subtree(child, level + 1);
+}
+
+static int clk_panic_dump(struct notifier_block *this, unsigned long ev,
+			  void *ptr)
+{
+	struct clk_core *c;
+
+	if (!clk_pdump_enable)
+		return 0;
+
+	pr_err("clock panic dump:\n");
+	pr_err("   clock                         enable_cnt  prepare_cnt        rate   accuracy   phase\n");
+	pr_err("----------------------------------------------------------------------------------------\n");
+
+	clk_prepare_lock();
+
+	hlist_for_each_entry(c, &clk_root_list, child_node)
+		clk_pdump_show_subtree(c, 0);
+	hlist_for_each_entry(c, &clk_orphan_list, child_node)
+		clk_pdump_show_subtree(c, 0);
+
+	clk_prepare_unlock();
+
+	return 0;
+}
+
+static struct notifier_block clk_pdump_block = {
+	.notifier_call = clk_panic_dump,
+};
+
+static int clk_register_pdump(void)
+{
+	atomic_notifier_chain_register(&panic_notifier_list, &clk_pdump_block);
+	return 0;
+}
+late_initcall_sync(clk_register_pdump);
+
 /**
  * __clk_core_init - initialize the data structures in a struct clk_core
  * @core:	clk_core being initialized
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] clk: add clock panic dump in tree-view
@ 2018-05-16  8:09   ` Stephen Boyd
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2018-05-16  8:09 UTC (permalink / raw)
  To: Michael Turquette, Shawn Lin; +Cc: linux-rockchip, linux-clk, Shawn Lin

Quoting Shawn Lin (2018-05-10 01:50:05)
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentat=
ion/admin-guide/kernel-parameters.txt
> index 3487be7..7bbdb6f 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -519,6 +519,9 @@
>                         debug and development, but should not be needed o=
n a
>                         platform with proper driver support.  For more
>                         information, see Documentation/clk.txt.
> +       clk_panic_dump
> +                       [CLK]
> +                       Dump the whole clock tree-view upon panic.

Typically when the system panics it's because something has gone wrong.
When a clk has gone wrong, typically the system hangs or crashes hard
and printing out panic info just doesn't happen.

I'm not sure why clks are so special here that we need to dump the
information about the clk tree at the point of a panic. Is this more of
a debug patch to dump out clk tree info by calling panic() in certain
places near where you get odd system hangs?

>  =

>         clock=3D          [BUGS=3DX86-32, HW] gettimeofday clocksource ov=
erride.
>                         [Deprecated]
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 9ae92aa..adb5537 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2802,6 +2802,73 @@ static inline void clk_debug_unregister(struct clk=
_core *core)
>  }
>  #endif
>  =

> +static bool clk_pdump_enable;
> +static int __init clk_pdump_setup(char *__unused)
> +{
> +       clk_pdump_enable  =3D true;
> +       return 1;
> +}
> +__setup("clk_panic_dump", clk_pdump_setup);
> +
> +static void clk_pdump_show_one(struct clk_core *c, int level)
> +{
> +       if (!c)
> +               return;
> +
> +       pr_err("%*s%-*s %11d %12d %11lu %10lu %-3d\n",
> +               level * 3 + 1, "", 30 - level * 3, c->name,
> +               c->enable_count, c->prepare_count, clk_core_get_rate(c),
> +               clk_core_get_accuracy(c), clk_core_get_phase(c));
> +}
> +
> +static void clk_pdump_show_subtree(struct clk_core *c, int level)
> +{
> +       struct clk_core *child;
> +
> +       if (!c)
> +               return;
> +
> +       clk_pdump_show_one(c, level);
> +
> +       hlist_for_each_entry(child, &c->children, child_node)
> +               clk_pdump_show_subtree(child, level + 1);
> +}
> +
> +static int clk_panic_dump(struct notifier_block *this, unsigned long ev,
> +                         void *ptr)
> +{
> +       struct clk_core *c;
> +
> +       if (!clk_pdump_enable)
> +               return 0;

Why register the notifier in late init if the commandline doesn't have
the parameter set?

> +
> +       pr_err("clock panic dump:\n");
> +       pr_err("   clock                         enable_cnt  prepare_cnt =
       rate   accuracy   phase\n");
> +       pr_err("---------------------------------------------------------=
-------------------------------\n");
> +
> +       clk_prepare_lock();

Why are we taking locks in the panic path?

> +
> +       hlist_for_each_entry(c, &clk_root_list, child_node)
> +               clk_pdump_show_subtree(c, 0);
> +       hlist_for_each_entry(c, &clk_orphan_list, child_node)
> +               clk_pdump_show_subtree(c, 0);
> +
> +       clk_prepare_unlock();
> +
> +       return 0;
> +}
> +
> +static struct notifier_block clk_pdump_block =3D {
> +       .notifier_call =3D clk_panic_dump,
> +};
> +
> +static int clk_register_pdump(void)
> +{
> +       atomic_notifier_chain_register(&panic_notifier_list, &clk_pdump_b=
lock);
> +       return 0;
> +}
> +late_initcall_sync(clk_register_pdump);

Why sync?

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] clk: add clock panic dump in tree-view
@ 2018-05-16  8:09   ` Stephen Boyd
  0 siblings, 0 replies; 6+ messages in thread
From: Stephen Boyd @ 2018-05-16  8:09 UTC (permalink / raw)
  To: Michael Turquette
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Shawn Lin,
	linux-clk-u79uwXL29TY76Z2rM5mHXA

Quoting Shawn Lin (2018-05-10 01:50:05)
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 3487be7..7bbdb6f 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -519,6 +519,9 @@
>                         debug and development, but should not be needed on a
>                         platform with proper driver support.  For more
>                         information, see Documentation/clk.txt.
> +       clk_panic_dump
> +                       [CLK]
> +                       Dump the whole clock tree-view upon panic.

Typically when the system panics it's because something has gone wrong.
When a clk has gone wrong, typically the system hangs or crashes hard
and printing out panic info just doesn't happen.

I'm not sure why clks are so special here that we need to dump the
information about the clk tree at the point of a panic. Is this more of
a debug patch to dump out clk tree info by calling panic() in certain
places near where you get odd system hangs?

>  
>         clock=          [BUGS=X86-32, HW] gettimeofday clocksource override.
>                         [Deprecated]
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 9ae92aa..adb5537 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2802,6 +2802,73 @@ static inline void clk_debug_unregister(struct clk_core *core)
>  }
>  #endif
>  
> +static bool clk_pdump_enable;
> +static int __init clk_pdump_setup(char *__unused)
> +{
> +       clk_pdump_enable  = true;
> +       return 1;
> +}
> +__setup("clk_panic_dump", clk_pdump_setup);
> +
> +static void clk_pdump_show_one(struct clk_core *c, int level)
> +{
> +       if (!c)
> +               return;
> +
> +       pr_err("%*s%-*s %11d %12d %11lu %10lu %-3d\n",
> +               level * 3 + 1, "", 30 - level * 3, c->name,
> +               c->enable_count, c->prepare_count, clk_core_get_rate(c),
> +               clk_core_get_accuracy(c), clk_core_get_phase(c));
> +}
> +
> +static void clk_pdump_show_subtree(struct clk_core *c, int level)
> +{
> +       struct clk_core *child;
> +
> +       if (!c)
> +               return;
> +
> +       clk_pdump_show_one(c, level);
> +
> +       hlist_for_each_entry(child, &c->children, child_node)
> +               clk_pdump_show_subtree(child, level + 1);
> +}
> +
> +static int clk_panic_dump(struct notifier_block *this, unsigned long ev,
> +                         void *ptr)
> +{
> +       struct clk_core *c;
> +
> +       if (!clk_pdump_enable)
> +               return 0;

Why register the notifier in late init if the commandline doesn't have
the parameter set?

> +
> +       pr_err("clock panic dump:\n");
> +       pr_err("   clock                         enable_cnt  prepare_cnt        rate   accuracy   phase\n");
> +       pr_err("----------------------------------------------------------------------------------------\n");
> +
> +       clk_prepare_lock();

Why are we taking locks in the panic path?

> +
> +       hlist_for_each_entry(c, &clk_root_list, child_node)
> +               clk_pdump_show_subtree(c, 0);
> +       hlist_for_each_entry(c, &clk_orphan_list, child_node)
> +               clk_pdump_show_subtree(c, 0);
> +
> +       clk_prepare_unlock();
> +
> +       return 0;
> +}
> +
> +static struct notifier_block clk_pdump_block = {
> +       .notifier_call = clk_panic_dump,
> +};
> +
> +static int clk_register_pdump(void)
> +{
> +       atomic_notifier_chain_register(&panic_notifier_list, &clk_pdump_block);
> +       return 0;
> +}
> +late_initcall_sync(clk_register_pdump);

Why sync?

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] clk: add clock panic dump in tree-view
@ 2018-05-17  2:32     ` Shawn Lin
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn Lin @ 2018-05-17  2:32 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: Michael Turquette, shawn.lin, linux-rockchip, linux-clk

Hi Stephen,

On 2018/5/16 16:09, Stephen Boyd wrote:
> Quoting Shawn Lin (2018-05-10 01:50:05)
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 3487be7..7bbdb6f 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -519,6 +519,9 @@
>>                          debug and development, but should not be needed on a
>>                          platform with proper driver support.  For more
>>                          information, see Documentation/clk.txt.
>> +       clk_panic_dump
>> +                       [CLK]
>> +                       Dump the whole clock tree-view upon panic.
> 
> Typically when the system panics it's because something has gone wrong.
> When a clk has gone wrong, typically the system hangs or crashes hard
> and printing out panic info just doesn't happen.
> 
> I'm not sure why clks are so special here that we need to dump the
> information about the clk tree at the point of a panic. Is this more of
> a debug patch to dump out clk tree info by calling panic() in certain
> places near where you get odd system hangs?

Yes, it's a debug patch when system in a odd hangds/hard locked. If it
crashes seriously, anything wrt. panic info is unusable, but if the
system still has a chance to dump panic info, for instance stack trace,
it would also be able to dump the clk tree.

Btw, I should marked this patch as RFC but forgot it. So if this patch
is acceptable from your point, if I address the your comment about the
code?

> 
>>   
>>          clock=          [BUGS=X86-32, HW] gettimeofday clocksource override.
>>                          [Deprecated]
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 9ae92aa..adb5537 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -2802,6 +2802,73 @@ static inline void clk_debug_unregister(struct clk_core *core)
>>   }
>>   #endif
>>   
>> +static bool clk_pdump_enable;
>> +static int __init clk_pdump_setup(char *__unused)
>> +{
>> +       clk_pdump_enable  = true;
>> +       return 1;
>> +}
>> +__setup("clk_panic_dump", clk_pdump_setup);
>> +
>> +static void clk_pdump_show_one(struct clk_core *c, int level)
>> +{
>> +       if (!c)
>> +               return;
>> +
>> +       pr_err("%*s%-*s %11d %12d %11lu %10lu %-3d\n",
>> +               level * 3 + 1, "", 30 - level * 3, c->name,
>> +               c->enable_count, c->prepare_count, clk_core_get_rate(c),
>> +               clk_core_get_accuracy(c), clk_core_get_phase(c));
>> +}
>> +
>> +static void clk_pdump_show_subtree(struct clk_core *c, int level)
>> +{
>> +       struct clk_core *child;
>> +
>> +       if (!c)
>> +               return;
>> +
>> +       clk_pdump_show_one(c, level);
>> +
>> +       hlist_for_each_entry(child, &c->children, child_node)
>> +               clk_pdump_show_subtree(child, level + 1);
>> +}
>> +
>> +static int clk_panic_dump(struct notifier_block *this, unsigned long ev,
>> +                         void *ptr)
>> +{
>> +       struct clk_core *c;
>> +
>> +       if (!clk_pdump_enable)
>> +               return 0;
> 
> Why register the notifier in late init if the commandline doesn't have
> the parameter set?
> 

Ok, will fix.

>> +
>> +       pr_err("clock panic dump:\n");
>> +       pr_err("   clock                         enable_cnt  prepare_cnt        rate   accuracy   phase\n");
>> +       pr_err("----------------------------------------------------------------------------------------\n");
>> +
>> +       clk_prepare_lock();
> 
> Why are we taking locks in the panic path?

ok, it's unnecessary in the special path indeed.

> 
>> +
>> +       hlist_for_each_entry(c, &clk_root_list, child_node)
>> +               clk_pdump_show_subtree(c, 0);
>> +       hlist_for_each_entry(c, &clk_orphan_list, child_node)
>> +               clk_pdump_show_subtree(c, 0);
>> +
>> +       clk_prepare_unlock();
>> +
>> +       return 0;
>> +}
>> +
>> +static struct notifier_block clk_pdump_block = {
>> +       .notifier_call = clk_panic_dump,
>> +};
>> +
>> +static int clk_register_pdump(void)
>> +{
>> +       atomic_notifier_chain_register(&panic_notifier_list, &clk_pdump_block);
>> +       return 0;
>> +}
>> +late_initcall_sync(clk_register_pdump);
> 
> Why sync?

Oh, sync is no needed.

> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] clk: add clock panic dump in tree-view
@ 2018-05-17  2:32     ` Shawn Lin
  0 siblings, 0 replies; 6+ messages in thread
From: Shawn Lin @ 2018-05-17  2:32 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Michael Turquette, shawn.lin-TNX95d0MmH7DzftRWevZcw,
	linux-clk-u79uwXL29TY76Z2rM5mHXA

Hi Stephen,

On 2018/5/16 16:09, Stephen Boyd wrote:
> Quoting Shawn Lin (2018-05-10 01:50:05)
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 3487be7..7bbdb6f 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -519,6 +519,9 @@
>>                          debug and development, but should not be needed on a
>>                          platform with proper driver support.  For more
>>                          information, see Documentation/clk.txt.
>> +       clk_panic_dump
>> +                       [CLK]
>> +                       Dump the whole clock tree-view upon panic.
> 
> Typically when the system panics it's because something has gone wrong.
> When a clk has gone wrong, typically the system hangs or crashes hard
> and printing out panic info just doesn't happen.
> 
> I'm not sure why clks are so special here that we need to dump the
> information about the clk tree at the point of a panic. Is this more of
> a debug patch to dump out clk tree info by calling panic() in certain
> places near where you get odd system hangs?

Yes, it's a debug patch when system in a odd hangds/hard locked. If it
crashes seriously, anything wrt. panic info is unusable, but if the
system still has a chance to dump panic info, for instance stack trace,
it would also be able to dump the clk tree.

Btw, I should marked this patch as RFC but forgot it. So if this patch
is acceptable from your point, if I address the your comment about the
code?

> 
>>   
>>          clock=          [BUGS=X86-32, HW] gettimeofday clocksource override.
>>                          [Deprecated]
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 9ae92aa..adb5537 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -2802,6 +2802,73 @@ static inline void clk_debug_unregister(struct clk_core *core)
>>   }
>>   #endif
>>   
>> +static bool clk_pdump_enable;
>> +static int __init clk_pdump_setup(char *__unused)
>> +{
>> +       clk_pdump_enable  = true;
>> +       return 1;
>> +}
>> +__setup("clk_panic_dump", clk_pdump_setup);
>> +
>> +static void clk_pdump_show_one(struct clk_core *c, int level)
>> +{
>> +       if (!c)
>> +               return;
>> +
>> +       pr_err("%*s%-*s %11d %12d %11lu %10lu %-3d\n",
>> +               level * 3 + 1, "", 30 - level * 3, c->name,
>> +               c->enable_count, c->prepare_count, clk_core_get_rate(c),
>> +               clk_core_get_accuracy(c), clk_core_get_phase(c));
>> +}
>> +
>> +static void clk_pdump_show_subtree(struct clk_core *c, int level)
>> +{
>> +       struct clk_core *child;
>> +
>> +       if (!c)
>> +               return;
>> +
>> +       clk_pdump_show_one(c, level);
>> +
>> +       hlist_for_each_entry(child, &c->children, child_node)
>> +               clk_pdump_show_subtree(child, level + 1);
>> +}
>> +
>> +static int clk_panic_dump(struct notifier_block *this, unsigned long ev,
>> +                         void *ptr)
>> +{
>> +       struct clk_core *c;
>> +
>> +       if (!clk_pdump_enable)
>> +               return 0;
> 
> Why register the notifier in late init if the commandline doesn't have
> the parameter set?
> 

Ok, will fix.

>> +
>> +       pr_err("clock panic dump:\n");
>> +       pr_err("   clock                         enable_cnt  prepare_cnt        rate   accuracy   phase\n");
>> +       pr_err("----------------------------------------------------------------------------------------\n");
>> +
>> +       clk_prepare_lock();
> 
> Why are we taking locks in the panic path?

ok, it's unnecessary in the special path indeed.

> 
>> +
>> +       hlist_for_each_entry(c, &clk_root_list, child_node)
>> +               clk_pdump_show_subtree(c, 0);
>> +       hlist_for_each_entry(c, &clk_orphan_list, child_node)
>> +               clk_pdump_show_subtree(c, 0);
>> +
>> +       clk_prepare_unlock();
>> +
>> +       return 0;
>> +}
>> +
>> +static struct notifier_block clk_pdump_block = {
>> +       .notifier_call = clk_panic_dump,
>> +};
>> +
>> +static int clk_register_pdump(void)
>> +{
>> +       atomic_notifier_chain_register(&panic_notifier_list, &clk_pdump_block);
>> +       return 0;
>> +}
>> +late_initcall_sync(clk_register_pdump);
> 
> Why sync?

Oh, sync is no needed.

> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-17  2:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-10  8:50 [PATCH] clk: add clock panic dump in tree-view Shawn Lin
2018-05-10  8:50 ` Shawn Lin
2018-05-16  8:09 ` Stephen Boyd
2018-05-16  8:09   ` Stephen Boyd
2018-05-17  2:32   ` Shawn Lin
2018-05-17  2:32     ` Shawn Lin

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.