* 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; 6+ 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] 6+ messages in thread
* [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
@ 2019-07-02 13:27 Leonard Crestez
2019-08-08 15:00 ` Stephen Boyd
0 siblings, 1 reply; 6+ 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 related [flat|nested] 6+ messages in thread* Re: [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
2019-07-02 13:27 Leonard Crestez
@ 2019-08-08 15:00 ` Stephen Boyd
2019-08-08 16:46 ` Leonard Crestez
0 siblings, 1 reply; 6+ 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] 6+ messages in thread* Re: [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs
2019-08-08 15:00 ` Stephen Boyd
@ 2019-08-08 16:46 ` Leonard Crestez
2019-08-08 19:46 ` Stephen Boyd
0 siblings, 1 reply; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2019-08-08 19:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-16 10:32 [PATCH v3 1/2] clk: Add clk_min/max_rate entries in debugfs Leonard Crestez
2019-07-16 17:13 ` Stephen Boyd
-- strict thread matches above, loose matches on Subject: below --
2019-07-02 13:27 Leonard Crestez
2019-08-08 15:00 ` Stephen Boyd
2019-08-08 16:46 ` Leonard Crestez
2019-08-08 19:46 ` Stephen Boyd
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).