Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
@ 2019-07-02 13:27 Leonard Crestez
  2019-07-02 13:27 ` [PATCH v3 2/2] clk: Assert prepare_lock in clk_core_get_boundaries Leonard Crestez
  2019-08-08 15:00 ` [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs Stephen Boyd
  0 siblings, 2 replies; 8+ messages in thread
From: Leonard Crestez @ 2019-07-02 13:27 UTC (permalink / raw)
  To: Stephen Boyd, Geert Uytterhoeven
  Cc: Michael Turquette, linux-clk, linux-arm-kernel

Add two files to expose min/max clk rates as determined by
clk_core_get_boundaries, taking all consumer requests into account.

This information does not appear to be otherwise exposed to userspace.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

---
 drivers/clk/clk.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Changes since v2:
* Shrink clk_prepare_lock
* Split lock assertion to separate patch
Link to v2: https://patchwork.kernel.org/patch/11021609/

Changes since v1:
* Call clk_prepare_lock/clk_prepare_unlock (Geert)
* Also include in clk_dump, but only with non-default values
Link to v1: https://patchwork.kernel.org/patch/11019873/

Don't add to clk_summary because min/max rates are rarely used and
clk_summary already has too many columns.

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index c0990703ce54..e4e224982ae3 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2894,19 +2894,26 @@ static int clk_summary_show(struct seq_file *s, void *data)
 }
 DEFINE_SHOW_ATTRIBUTE(clk_summary);
 
 static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level)
 {
+	unsigned long min_rate, max_rate;
+
 	if (!c)
 		return;
 
 	/* This should be JSON format, i.e. elements separated with a comma */
 	seq_printf(s, "\"%s\": { ", c->name);
 	seq_printf(s, "\"enable_count\": %d,", c->enable_count);
 	seq_printf(s, "\"prepare_count\": %d,", c->prepare_count);
 	seq_printf(s, "\"protect_count\": %d,", c->protect_count);
 	seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c));
+	clk_core_get_boundaries(c, &min_rate, &max_rate);
+	if (min_rate != 0)
+		seq_printf(s, "\"min_rate\": %lu,", min_rate);
+	if (max_rate != ULONG_MAX)
+		seq_printf(s, "\"max_rate\": %lu,", max_rate);
 	seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c));
 	seq_printf(s, "\"phase\": %d,", clk_core_get_phase(c));
 	seq_printf(s, "\"duty_cycle\": %u",
 		   clk_core_get_scaled_duty_cycle(c, 100000));
 }
@@ -3062,10 +3069,38 @@ static int clk_duty_cycle_show(struct seq_file *s, void *data)
 
 	return 0;
 }
 DEFINE_SHOW_ATTRIBUTE(clk_duty_cycle);
 
+static int clk_min_rate_show(struct seq_file *s, void *data)
+{
+	struct clk_core *core = s->private;
+	unsigned long min_rate, max_rate;
+
+	clk_prepare_lock();
+	clk_core_get_boundaries(core, &min_rate, &max_rate);
+	clk_prepare_unlock();
+	seq_printf(s, "%lu\n", min_rate);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(clk_min_rate);
+
+static int clk_max_rate_show(struct seq_file *s, void *data)
+{
+	struct clk_core *core = s->private;
+	unsigned long min_rate, max_rate;
+
+	clk_prepare_lock();
+	clk_core_get_boundaries(core, &min_rate, &max_rate);
+	clk_prepare_unlock();
+	seq_printf(s, "%lu\n", max_rate);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(clk_max_rate);
+
 static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
 {
 	struct dentry *root;
 
 	if (!core || !pdentry)
@@ -3073,10 +3108,12 @@ static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry)
 
 	root = debugfs_create_dir(core->name, pdentry);
 	core->dentry = root;
 
 	debugfs_create_ulong("clk_rate", 0444, root, &core->rate);
+	debugfs_create_file("clk_min_rate", 0444, root, core, &clk_min_rate_fops);
+	debugfs_create_file("clk_max_rate", 0444, root, core, &clk_max_rate_fops);
 	debugfs_create_ulong("clk_accuracy", 0444, root, &core->accuracy);
 	debugfs_create_u32("clk_phase", 0444, root, &core->phase);
 	debugfs_create_file("clk_flags", 0444, root, core, &clk_flags_fops);
 	debugfs_create_u32("clk_prepare_count", 0444, root, &core->prepare_count);
 	debugfs_create_u32("clk_enable_count", 0444, root, &core->enable_count);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 2/2] clk: Assert prepare_lock in clk_core_get_boundaries
  2019-07-02 13:27 [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs Leonard Crestez
@ 2019-07-02 13:27 ` Leonard Crestez
  2019-08-08 15:00   ` Stephen Boyd
  2019-08-08 15:00 ` [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs Stephen Boyd
  1 sibling, 1 reply; 8+ messages in thread
From: Leonard Crestez @ 2019-07-02 13:27 UTC (permalink / raw)
  To: Stephen Boyd, Geert Uytterhoeven
  Cc: Michael Turquette, linux-clk, linux-arm-kernel

This function iterates the clk consumer list on clk_core so it must be
called under prepare_lock. This is already done by all callers but add a
lockdep assert to check anyway.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>

---
 drivers/clk/clk.c | 2 ++
 1 file changed, 2 insertions(+)

I wouldn't mind if this is dropped as unnecessary.

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index e4e224982ae3..b1c79a58d734 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -591,10 +591,12 @@ static void clk_core_get_boundaries(struct clk_core *core,
 				    unsigned long *min_rate,
 				    unsigned long *max_rate)
 {
 	struct clk *clk_user;
 
+	lockdep_assert_held(&prepare_lock);
+
 	*min_rate = core->min_rate;
 	*max_rate = core->max_rate;
 
 	hlist_for_each_entry(clk_user, &core->clks, clks_node)
 		*min_rate = max(*min_rate, clk_user->min_rate);
-- 
2.17.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
  2019-07-02 13:27 [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs Leonard Crestez
  2019-07-02 13:27 ` [PATCH v3 2/2] clk: Assert prepare_lock in clk_core_get_boundaries Leonard Crestez
@ 2019-08-08 15:00 ` Stephen Boyd
  2019-08-08 16:46   ` Leonard Crestez
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Boyd @ 2019-08-08 15:00 UTC (permalink / raw)
  To: Geert Uytterhoeven, Leonard Crestez
  Cc: Michael Turquette, linux-clk, linux-arm-kernel

Quoting Leonard Crestez (2019-07-02 06:27:09)
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index c0990703ce54..e4e224982ae3 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2894,19 +2894,26 @@ static int clk_summary_show(struct seq_file *s, void *data)
>  }
>  DEFINE_SHOW_ATTRIBUTE(clk_summary);
>  
>  static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level)
>  {
> +       unsigned long min_rate, max_rate;
> +
>         if (!c)
>                 return;
>  
>         /* This should be JSON format, i.e. elements separated with a comma */
>         seq_printf(s, "\"%s\": { ", c->name);
>         seq_printf(s, "\"enable_count\": %d,", c->enable_count);
>         seq_printf(s, "\"prepare_count\": %d,", c->prepare_count);
>         seq_printf(s, "\"protect_count\": %d,", c->protect_count);
>         seq_printf(s, "\"rate\": %lu,", clk_core_get_rate(c));
> +       clk_core_get_boundaries(c, &min_rate, &max_rate);
> +       if (min_rate != 0)
> +               seq_printf(s, "\"min_rate\": %lu,", min_rate);
> +       if (max_rate != ULONG_MAX)
> +               seq_printf(s, "\"max_rate\": %lu,", max_rate);

What are the if conditions about? We always output the values in the
individual files, but for some reason we don't want to do that in the
json output?

>         seq_printf(s, "\"accuracy\": %lu,", clk_core_get_accuracy(c));
>         seq_printf(s, "\"phase\": %d,", clk_core_get_phase(c));
>         seq_printf(s, "\"duty_cycle\": %u",
>                    clk_core_get_scaled_duty_cycle(c, 100000));
>  }

Everything else looks fine, so maybe I'll just remove the if statements
if you don't mind.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 2/2] clk: Assert prepare_lock in clk_core_get_boundaries
  2019-07-02 13:27 ` [PATCH v3 2/2] clk: Assert prepare_lock in clk_core_get_boundaries Leonard Crestez
@ 2019-08-08 15:00   ` Stephen Boyd
  0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-08-08 15:00 UTC (permalink / raw)
  To: Geert Uytterhoeven, Leonard Crestez
  Cc: Michael Turquette, linux-clk, linux-arm-kernel

Quoting Leonard Crestez (2019-07-02 06:27:10)
> This function iterates the clk consumer list on clk_core so it must be
> called under prepare_lock. This is already done by all callers but add a
> lockdep assert to check anyway.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> 
> ---

Applied to clk-next


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
  2019-08-08 15:00 ` [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs Stephen Boyd
@ 2019-08-08 16:46   ` Leonard Crestez
  2019-08-08 19:46     ` Stephen Boyd
  0 siblings, 1 reply; 8+ messages in thread
From: Leonard Crestez @ 2019-08-08 16:46 UTC (permalink / raw)
  To: Stephen Boyd, Geert Uytterhoeven
  Cc: Michael Turquette, linux-clk, linux-arm-kernel

On 8/8/2019 6:00 PM, Stephen Boyd wrote:
> Quoting Leonard Crestez (2019-07-02 06:27:09)
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c

>>   static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level)
>>   {
>> +       clk_core_get_boundaries(c, &min_rate, &max_rate);
>> +       if (min_rate != 0)
>> +               seq_printf(s, "\"min_rate\": %lu,", min_rate);
>> +       if (max_rate != ULONG_MAX)
>> +               seq_printf(s, "\"max_rate\": %lu,", max_rate);
> 
> What are the if conditions about? We always output the values in the
> individual files, but for some reason we don't want to do that in the
> json output?

These if conditions are an easy way to avoid spamming "min_rate": 0, 
"max_rate": 18446744073709551615 in json. If you object to the 
inconsistency a nice solution would to be show "null" in both debugfs 
and json.

Outright hiding min/max files from debugfs is impractical, it would 
require filesystem calls from clk_set_min_rate

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
  2019-08-08 16:46   ` Leonard Crestez
@ 2019-08-08 19:46     ` Stephen Boyd
  0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-08-08 19:46 UTC (permalink / raw)
  To: Geert Uytterhoeven, Leonard Crestez
  Cc: Michael Turquette, linux-clk, linux-arm-kernel

Quoting Leonard Crestez (2019-08-08 09:46:48)
> On 8/8/2019 6:00 PM, Stephen Boyd wrote:
> > Quoting Leonard Crestez (2019-07-02 06:27:09)
> >> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> 
> >>   static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level)
> >>   {
> >> +       clk_core_get_boundaries(c, &min_rate, &max_rate);
> >> +       if (min_rate != 0)
> >> +               seq_printf(s, "\"min_rate\": %lu,", min_rate);
> >> +       if (max_rate != ULONG_MAX)
> >> +               seq_printf(s, "\"max_rate\": %lu,", max_rate);
> > 
> > What are the if conditions about? We always output the values in the
> > individual files, but for some reason we don't want to do that in the
> > json output?
> 
> These if conditions are an easy way to avoid spamming "min_rate": 0, 
> "max_rate": 18446744073709551615 in json. If you object to the 
> inconsistency a nice solution would to be show "null" in both debugfs 
> and json.

Aren't those the min and max values though? I don't see it as spam, it's
just more data that is the "default". Given that json is for machine
parsing maybe the parser of this can ignore them if it wants to when the
values match 0 and ULONG_MAX?


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
  2019-07-16 10:32 Leonard Crestez
@ 2019-07-16 17:13 ` Stephen Boyd
  0 siblings, 0 replies; 8+ messages in thread
From: Stephen Boyd @ 2019-07-16 17:13 UTC (permalink / raw)
  To: Leonard Crestez
  Cc: Geert Uytterhoeven, Michael Turquette, linux-clk, linux-arm-kernel

Quoting Leonard Crestez (2019-07-16 03:32:44)
> Gentle ping.
> 
> This patch shows up on patchwork but not in my inbox so maybe it was 
> lost in spam filters somehow?
> 
> Geert offered a reviewed-by in v2 but I forgot to add it.
> 

I see v3 on the list and in my inbox.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
@ 2019-07-16 10:32 Leonard Crestez
  2019-07-16 17:13 ` Stephen Boyd
  0 siblings, 1 reply; 8+ messages in thread
From: Leonard Crestez @ 2019-07-16 10:32 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Geert Uytterhoeven, Michael Turquette, linux-clk, linux-arm-kernel

Gentle ping.

This patch shows up on patchwork but not in my inbox so maybe it was 
lost in spam filters somehow?

Geert offered a reviewed-by in v2 but I forgot to add it.

--
Regards,
Leonard

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-02 13:27 [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs Leonard Crestez
2019-07-02 13:27 ` [PATCH v3 2/2] clk: Assert prepare_lock in clk_core_get_boundaries Leonard Crestez
2019-08-08 15:00   ` Stephen Boyd
2019-08-08 15:00 ` [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs Stephen Boyd
2019-08-08 16:46   ` Leonard Crestez
2019-08-08 19:46     ` Stephen Boyd
2019-07-16 10:32 Leonard Crestez
2019-07-16 17:13 ` Stephen Boyd

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org linux-clk@archiver.kernel.org
	public-inbox-index linux-clk

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/ public-inbox