[PATCH v1] clk: Add enable-state column to clk summary

[PATCH v1] clk: Add enable-state column to clk summary

[Dmitry Osipenko]

Add "enable state" column to the clk summary. It's handy to know actual
hardware state of all clocks for debugging purposes. In conjunction with
clk_ignore_unused, this tells us what unused clocks are left on after
bootloader without disabling the clocks. It's also s useful debugging
information for cases where firmware touches clocks.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/clk/clk.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f83dac54ed85..07fbdea488bc 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2931,7 +2931,8 @@ static void clk_summary_show_one(struct seq_file *s, struct clk_core *c,
 	else
 		seq_puts(s, "-----");
 
-	seq_printf(s, " %6d\n", clk_core_get_scaled_duty_cycle(c, 100000));
+	seq_printf(s, " %6d", clk_core_get_scaled_duty_cycle(c, 100000));
+	seq_printf(s, " %7d\n", clk_core_is_enabled(c));
 }
 
 static void clk_summary_show_subtree(struct seq_file *s, struct clk_core *c,
@@ -2950,9 +2951,9 @@ 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, "                                 enable  prepare  protect                                duty\n");
-	seq_puts(s, "   clock                          count    count    count        rate   accuracy phase  cycle\n");
-	seq_puts(s, "---------------------------------------------------------------------------------------------\n");
+	seq_puts(s, "                                 enable  prepare  protect                                duty  enable\n");
+	seq_puts(s, "   clock                          count    count    count        rate   accuracy phase  cycle   state\n");
+	seq_puts(s, "-----------------------------------------------------------------------------------------------------\n");
 
 	clk_prepare_lock();
 
-- 
2.27.0

Re: [PATCH v1] clk: Add enable-state column to clk summary

[Stephen Boyd]

Quoting Dmitry Osipenko (2020-11-04 08:56:31)
> Add "enable state" column to the clk summary. It's handy to know actual
> hardware state of all clocks for debugging purposes. In conjunction with
> clk_ignore_unused, this tells us what unused clocks are left on after
> bootloader without disabling the clocks.

Should it be called "boot state" then? That idea sounds OK to me.

> It's also s useful debugging

Stray 's' here.

> information for cases where firmware touches clocks.

Care to explain more? Presumably you mean when firmware is modifying clk
state without notifying the kernel? In which case it should be called
"hardware enable" or something like that and be a "Y/N/?" value
depending on if the value can be read or not and if it is enabled or not?

> 
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Re: [PATCH v1] clk: Add enable-state column to clk summary

[Dmitry Osipenko]

13.11.2020 11:18, Stephen Boyd пишет:
> Quoting Dmitry Osipenko (2020-11-04 08:56:31)
>> Add "enable state" column to the clk summary. It's handy to know actual
>> hardware state of all clocks for debugging purposes. In conjunction with
>> clk_ignore_unused, this tells us what unused clocks are left on after
>> bootloader without disabling the clocks.
> 
> Should it be called "boot state" then? That idea sounds OK to me.
> 
>> It's also s useful debugging
> 
> Stray 's' here.
> 
>> information for cases where firmware touches clocks.
> 
> Care to explain more? Presumably you mean when firmware is modifying clk
> state without notifying the kernel?

This is exactly what I meant.

> In which case it should be called
> "hardware enable" or something like that and be a "Y/N/?" value
> depending on if the value can be read or not and if it is enabled or not?

Indeed, I like the "hardware enable", thank you for the suggestion. The
"Y/N/?" suggestion is also good.

I'll prepare v2, thank you for the review!