2017-03-22 13:21:11

by Peter De Schrijver

[permalink] [raw]
Subject: [PATCH] clk: Add requested rate to clock summary output

From: Alex Frid <[email protected]>

Added requested rate to clock summary output and to clock dump. This is
useful for clock tree debugging. Also expand the clock name field in the
clock tree debugfs output to provide room for deep multi-tier trees like
on Tegra.

Signed-off-by: Alex Frid <[email protected]>
Signed-off-by: Peter De Schrijver <[email protected]>
---
drivers/clk/clk.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 0b815d1..3f6ae6d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2007,10 +2007,10 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c,
if (!c)
return;

- seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n",
+ seq_printf(s, "%*s%-*s %11d %12d %11lu %11lu %10lu %-3d\n",
level * 3 + 1, "",
- 30 - level * 3, c->name,
- c->enable_count, c->prepare_count, clk_core_get_rate(c),
+ 48 - level * 3, c->name, c->enable_count,
+ c->prepare_count, clk_core_get_rate(c), c->req_rate,
clk_core_get_accuracy(c), clk_core_get_phase(c));
}

@@ -2033,8 +2033,8 @@ static int clk_summary_show(struct seq_file *s, void *data)
struct clk_core *c;
struct hlist_head **lists = (struct hlist_head **)s->private;

- seq_puts(s, " clock enable_cnt prepare_cnt rate accuracy phase\n");
- seq_puts(s, "----------------------------------------------------------------------------------------\n");
+ seq_puts(s, " clock enable_cnt prepare_cnt rate req_rate accuracy phase\n");
+ seq_puts(s, "----------------------------------------------------------------------------------------------------------------------\n");

clk_prepare_lock();

@@ -2070,6 +2070,7 @@ static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level)
seq_printf(s, "\"enable_count\": %d,", c->enable_count);
seq_printf(s, "\"prepare_count\": %d,", c->prepare_count);
seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c));
+ seq_printf(s, "\"req_rate\": %lu,", c->req_rate);
seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c));
seq_printf(s, "\"phase\": %d", clk_core_get_phase(c));
}
--
1.9.1


2017-04-07 11:49:19

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH] clk: Add requested rate to clock summary output

On Wed, Mar 22, 2017 at 03:20:03PM +0200, Peter De Schrijver wrote:
> From: Alex Frid <[email protected]>
>
> Added requested rate to clock summary output and to clock dump. This is
> useful for clock tree debugging. Also expand the clock name field in the
> clock tree debugfs output to provide room for deep multi-tier trees like
> on Tegra.
>

Ping! Any comments on this?

Peter.

> Signed-off-by: Alex Frid <[email protected]>
> Signed-off-by: Peter De Schrijver <[email protected]>
> ---
> drivers/clk/clk.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 0b815d1..3f6ae6d 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2007,10 +2007,10 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c,
> if (!c)
> return;
>
> - seq_printf(s, "%*s%-*s %11d %12d %11lu %10lu %-3d\n",
> + seq_printf(s, "%*s%-*s %11d %12d %11lu %11lu %10lu %-3d\n",
> level * 3 + 1, "",
> - 30 - level * 3, c->name,
> - c->enable_count, c->prepare_count, clk_core_get_rate(c),
> + 48 - level * 3, c->name, c->enable_count,
> + c->prepare_count, clk_core_get_rate(c), c->req_rate,
> clk_core_get_accuracy(c), clk_core_get_phase(c));
> }
>
> @@ -2033,8 +2033,8 @@ static int clk_summary_show(struct seq_file *s, void *data)
> struct clk_core *c;
> struct hlist_head **lists = (struct hlist_head **)s->private;
>
> - seq_puts(s, " clock enable_cnt prepare_cnt rate accuracy phase\n");
> - seq_puts(s, "----------------------------------------------------------------------------------------\n");
> + seq_puts(s, " clock enable_cnt prepare_cnt rate req_rate accuracy phase\n");
> + seq_puts(s, "----------------------------------------------------------------------------------------------------------------------\n");
>
> clk_prepare_lock();
>
> @@ -2070,6 +2070,7 @@ static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level)
> seq_printf(s, "\"enable_count\": %d,", c->enable_count);
> seq_printf(s, "\"prepare_count\": %d,", c->prepare_count);
> seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c));
> + seq_printf(s, "\"req_rate\": %lu,", c->req_rate);
> seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c));
> seq_printf(s, "\"phase\": %d", clk_core_get_phase(c));
> }
> --
> 1.9.1
>

2017-04-12 16:49:41

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH] clk: Add requested rate to clock summary output

On 03/22, Peter De Schrijver wrote:
> From: Alex Frid <[email protected]>
>
> Added requested rate to clock summary output and to clock dump. This is
> useful for clock tree debugging. Also expand the clock name field in the
> clock tree debugfs output to provide room for deep multi-tier trees like
> on Tegra.
>
> Signed-off-by: Alex Frid <[email protected]>
> Signed-off-by: Peter De Schrijver <[email protected]>

We should print out all the consumers (struct clks) and their
requested rates + prepare/enable counts instead. req_rate is sort
of an internal variable that records what the last aggregated
rate was. I'm not sure if we want to expose that to debugfs.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2017-04-13 13:53:18

by Peter De Schrijver

[permalink] [raw]
Subject: Re: [PATCH] clk: Add requested rate to clock summary output

On Wed, Apr 12, 2017 at 09:48:56AM -0700, Stephen Boyd wrote:
> On 03/22, Peter De Schrijver wrote:
> > From: Alex Frid <[email protected]>
> >
> > Added requested rate to clock summary output and to clock dump. This is
> > useful for clock tree debugging. Also expand the clock name field in the
> > clock tree debugfs output to provide room for deep multi-tier trees like
> > on Tegra.
> >
> > Signed-off-by: Alex Frid <[email protected]>
> > Signed-off-by: Peter De Schrijver <[email protected]>
>
> We should print out all the consumers (struct clks) and their
> requested rates + prepare/enable counts instead. req_rate is sort
> of an internal variable that records what the last aggregated
> rate was. I'm not sure if we want to expose that to debugfs.
>

While this certainly would provide more information, I think it would also
make the summary quite large. Hence the just printing the aggregate rate
seems a reasonable compromise. Maybe the consumers and requested rates
can be exposed in a file in the per clock directory. The prepare and enable
counts are not maintained per consumer today, so that's not possible?

Peter.

> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project