| Message ID | 057720844e78e615e46de34a73b16ffaf7dbfc10.1558686047.git.leonard.crestez@nxp.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: Add clk_parent entry in debugfs | expand |
Quoting Leonard Crestez (2019-05-24 01:25:25) > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index aa51756fd4d6..94a93b07dd37 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -3009,10 +3009,21 @@ static int possible_parents_show(struct seq_file *s, void *data) > > return 0; > } > DEFINE_SHOW_ATTRIBUTE(possible_parents); > > +static int current_parent_show(struct seq_file *s, void *data) > +{ > + struct clk_core *core = s->private; > + > + if (core->parent) > + seq_printf(s, "%s\n", core->parent->name); > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(current_parent); Looks OK. > + > static int clk_duty_cycle_show(struct seq_file *s, void *data) > { > struct clk_core *core = s->private; > struct clk_duty *duty = &core->duty; > > @@ -3040,10 +3051,11 @@ static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) > debugfs_create_u32("clk_enable_count", 0444, root, &core->enable_count); > debugfs_create_u32("clk_protect_count", 0444, root, &core->protect_count); > debugfs_create_u32("clk_notifier_count", 0444, root, &core->notifier_count); > debugfs_create_file("clk_duty_cycle", 0444, root, core, > &clk_duty_cycle_fops); > + debugfs_create_file("clk_parent", 0444, root, core, ¤t_parent_fops); Shouldn't we skip creation of this file if core->num_parents == 0? So put this under the if condition below? > > if (core->num_parents > 1) > debugfs_create_file("clk_possible_parents", 0444, root, core, > &possible_parents_fops); >
On 6/7/19 10:05 PM, Stephen Boyd wrote: > Quoting Leonard Crestez (2019-05-24 01:25:25) >> @@ -3040,10 +3051,11 @@ static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) >> debugfs_create_u32("clk_enable_count", 0444, root, &core->enable_count); >> debugfs_create_u32("clk_protect_count", 0444, root, &core->protect_count); >> debugfs_create_u32("clk_notifier_count", 0444, root, &core->notifier_count); >> debugfs_create_file("clk_duty_cycle", 0444, root, core, >> &clk_duty_cycle_fops); >> + debugfs_create_file("clk_parent", 0444, root, core, ¤t_parent_fops); > > Shouldn't we skip creation of this file if core->num_parents == 0? So > put this under the if condition below? It's still useful to determine clk tree structure from debugfs fields, otherwise you'd have to extract by parsing other files. Would you hide clk_rate for fixed-rate? I'd rather have everything available for uniformity, even if it's otherwise constant at runtime. -- Regards, Leonard
Hi Leonard, On Sat, Jun 8, 2019 at 9:26 AM Leonard Crestez <leonard.crestez@nxp.com> wrote: > On 6/7/19 10:05 PM, Stephen Boyd wrote: > > Quoting Leonard Crestez (2019-05-24 01:25:25) > > >> @@ -3040,10 +3051,11 @@ static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) > >> debugfs_create_u32("clk_enable_count", 0444, root, &core->enable_count); > >> debugfs_create_u32("clk_protect_count", 0444, root, &core->protect_count); > >> debugfs_create_u32("clk_notifier_count", 0444, root, &core->notifier_count); > >> debugfs_create_file("clk_duty_cycle", 0444, root, core, > >> &clk_duty_cycle_fops); > >> + debugfs_create_file("clk_parent", 0444, root, core, ¤t_parent_fops); > > > > Shouldn't we skip creation of this file if core->num_parents == 0? So > > put this under the if condition below? > > It's still useful to determine clk tree structure from debugfs fields, > otherwise you'd have to extract by parsing other files. > > Would you hide clk_rate for fixed-rate? I'd rather have everything > available for uniformity, even if it's otherwise constant at runtime. Unless I' missing something, there's a big difference here: all clocks have a rate, but not all clocks have a parent. Gr{oetje,eeting}s, Geert
On 6/8/19 3:19 PM, Geert Uytterhoeven wrote: > Hi Leonard, > > On Sat, Jun 8, 2019 at 9:26 AM Leonard Crestez <leonard.crestez@nxp.com> wrote: >> On 6/7/19 10:05 PM, Stephen Boyd wrote: >>> Quoting Leonard Crestez (2019-05-24 01:25:25) >> >>>> @@ -3040,10 +3051,11 @@ static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) >>>> debugfs_create_u32("clk_enable_count", 0444, root, &core->enable_count); >>>> debugfs_create_u32("clk_protect_count", 0444, root, &core->protect_count); >>>> debugfs_create_u32("clk_notifier_count", 0444, root, &core->notifier_count); >>>> debugfs_create_file("clk_duty_cycle", 0444, root, core, >>>> &clk_duty_cycle_fops); >>>> + debugfs_create_file("clk_parent", 0444, root, core, ¤t_parent_fops); >>> >>> Shouldn't we skip creation of this file if core->num_parents == 0? So >>> put this under the if condition below? >> >> It's still useful to determine clk tree structure from debugfs fields, >> otherwise you'd have to extract by parsing other files. >> >> Would you hide clk_rate for fixed-rate? I'd rather have everything >> available for uniformity, even if it's otherwise constant at runtime. > > Unless I' missing something, there's a big difference here: all clocks > have a rate, but not all clocks have a parent. Sorry, I got confused and thought that condition checked for muxes so the parent of intermediate clk nodes will be hidden. But intermediate nodes have num_parents == 1. Will fix in v2. -- Regards, Leonard
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index aa51756fd4d6..94a93b07dd37 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -3009,10 +3009,21 @@ static int possible_parents_show(struct seq_file *s, void *data) return 0; } DEFINE_SHOW_ATTRIBUTE(possible_parents); +static int current_parent_show(struct seq_file *s, void *data) +{ + struct clk_core *core = s->private; + + if (core->parent) + seq_printf(s, "%s\n", core->parent->name); + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(current_parent); + static int clk_duty_cycle_show(struct seq_file *s, void *data) { struct clk_core *core = s->private; struct clk_duty *duty = &core->duty; @@ -3040,10 +3051,11 @@ static void clk_debug_create_one(struct clk_core *core, struct dentry *pdentry) debugfs_create_u32("clk_enable_count", 0444, root, &core->enable_count); debugfs_create_u32("clk_protect_count", 0444, root, &core->protect_count); debugfs_create_u32("clk_notifier_count", 0444, root, &core->notifier_count); debugfs_create_file("clk_duty_cycle", 0444, root, core, &clk_duty_cycle_fops); + debugfs_create_file("clk_parent", 0444, root, core, ¤t_parent_fops); if (core->num_parents > 1) debugfs_create_file("clk_possible_parents", 0444, root, core, &possible_parents_fops);
This allows to easily determine the parent in shell scripts without parsing more complex files. Add the clk_parent file unconditionally because this information is useful for more than just muxes. Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/clk/clk.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)