Messages in this thread | | | Date | Sun, 26 Jun 2022 23:55:21 +0530 | From | "<Vishal Badole>" <> | Subject | Re: [PATCH] Common clock: To list active consumers of clocks |
| |
On Thu, Jun 23, 2022 at 06:05:48PM -0700, Stephen Boyd wrote: > Quoting <Vishal Badole> (2022-06-22 10:02:20) > > > > From f2e9d78bd0f135206fbfbf2e0178a5782b972939 Mon Sep 17 00:00:00 2001 > > From: Vishal Badole <badolevishal1116@gmail.com> > > Date: Tue, 21 Jun 2022 09:55:51 +0530 > > Subject: [PATCH] Common clock: To list active consumers of clocks > > That patch is still malformed. Please try again. Also, stop sending it > as a reply-to the previous patch. Thanks! > We have applied and checked the patch on top of the mainline and not able to see that it is malformed. We will share revised patch using git send mail. > > > > This feature lists the name of clocks and their consumer devices. > > Using this feature user can easily check which device is using a > > perticular clock. Consumers without dev_id are listed as no_dev_id. > > > > Co-developed-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > > Signed-off-by: Chinmoy Ghosh <chinmoyghosh2001@gmail.com> > > Co-developed-by: Mintu Patel <mintupatel89@gmail.com> > > Signed-off-by: Mintu Patel <mintupatel89@gmail.com> > > Acked-by: Vimal Kumar <vimal.kumar32@gmail.com> > > The acked-by tag is largely for maintainer use. Please remove it. See > Documentation/process/5.Posting.rst for more info. > Agreed, We will update this in the revised patch.
> > Signed-off-by: Vishal Badole <badolevishal1116@gmail.com> > > --- > > drivers/clk/clk.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 59 insertions(+) > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > index ed11918..b191009 100644 > > --- a/drivers/clk/clk.c > > +++ b/drivers/clk/clk.c > > @@ -3018,6 +3018,63 @@ static int clk_summary_show(struct seq_file *s, void *data) > > } > > DEFINE_SHOW_ATTRIBUTE(clk_summary); > > > > +static void clk_consumer_show_one(struct seq_file *s, struct clk_core *core, int level) > > +{ > > + struct clk *clk_user; > > + const char *consumer; > > + > > + hlist_for_each_entry(clk_user, &core->clks, clks_node) { > > + if (!clk_user->dev_id) > > + consumer = "no_dev_id"; > > + else > > + consumer = clk_user->dev_id; > > We can just pass NULL if there isn't a dev_id and get a nice "(NULL)" > print instead of "no_dev_id". > Agreed, we will replace "no_dev_id" with "deviceless" in the revised patch. > > + > > + seq_printf(s, "%*s%-*s %30s %5d %7d ", > > + level * 3 + 1, "", > > + 30 - level * 3, clk_user->core->name, consumer, > > + clk_user->core->enable_count, clk_user->core->prepare_count); > > It would be great to not print the core enable count here and instead > have two levels of enable accounting so we can print the per-user count. > Basically, one in the 'struct clk_core' and one in the 'struct clk'. If > that isn't done then this print is going to duplicate the count for > every 'struct clk' and be meaningless. > We will add enable count member to struct clock to represent per user count and will print that one along with clock and consumer name > > + > > + if (clk_user->core->ops->is_enabled) > > + seq_printf(s, " %8c\n", clk_core_is_enabled(clk_user->core) ? 'Y' : 'N'); > > + else if (!clk_user->core->ops->enable) > > + seq_printf(s, " %8c\n", 'Y'); > > + else > > + seq_printf(s, " %8c\n", '?'); > > I don't think we need any of these prints. They're already covered in > the summary. And the summary should be able to print the users. See > regulator_summary_show_subtree() for inspiration. It looks like they > print "deviceless" for the "no_dev_id" case so maybe just use that > instead of NULL print. > We will remove above prints in the revised patch. We are facing indentation issue whle printing consumer in summary as given below enable prepare protect duty hardware per-user clock count count count rateccuracy phase cycle enable consumer count clk_mcasp0_fixed 0 0 0 24576000 0 50000 Y deviceless 0
In this case it will be better to have a separate debugfs entry as clK_consumer to print clock, consumer and per-user count. > > + } > > +} > > + > > +static void clk_consumer_show_subtree(struct seq_file *s, struct clk_core *c, int level) > > +{ > > + struct clk_core *child; > > + > > + clk_consumer_show_one(s, c, level); > > + > > + hlist_for_each_entry(child, &c->children, child_node) > > + clk_consumer_show_subtree(s, child, level + 1); > > +} > > + > > +static int clk_consumer_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 hardware\n"); > > + seq_puts(s, " clock consumer count count enable\n"); > > + seq_puts(s, "-----------------------------------------------------------------------------------------\n"); > > + clk_prepare_lock(); > > + > > + /*Traversing Linked List to print clock consumer*/ > > Please run scripts/checkpatch.pl, as this comment needs space after /* > and before */ > We will update this in revised patch. > > + > > + for (; *lists; lists++) > > + hlist_for_each_entry(c, *lists, child_node) > > + clk_consumer_show_subtree(s, c, 0); > > + > > + clk_prepare_unlock(); > > + > > + return 0; > > +} > > +DEFINE_SHOW_ATTRIBUTE(clk_consumer); > > + > > static void clk_dump_one(struct seq_file *s, struct clk_core *c, int level) > > { > > int phase;
| |