From: "<Vishal Badole>" <badolevishal1116@gmail.com>
To: Stephen Boyd <sboyd@kernel.org>,
mturquette@baylibre.com, inux-clk@vger.kernel.org,
linux-kernel@vger.kernel.org
Cc: chinmoyghosh2001@gmail.com, mintupatel89@gmail.com,
vimal.kumar32@gmail.com
Subject: Re: [PATCH] Common clock: To list active consumers of clocks
Date: Sun, 26 Jun 2022 23:55:21 +0530 [thread overview]
Message-ID: <20220626182517.GA26001@Mahakal> (raw)
In-Reply-To: <20220624010550.582BBC341C7@smtp.kernel.org>
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;
next prev parent reply other threads:[~2022-06-26 18:25 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CAEXpiVQihEadxsNodarz2-wxSAipfpzEaA8zKpnozszC+weYTQ@mail.gmail.com>
2022-06-10 19:40 ` [PATCH 5.4] Common clock: To list active consumers of clocks Stephen Boyd
[not found] ` <CAOUcgRfB-r3aERYeLumExgpTVzsDsBuyOWT+nCJ_OfOv1g0L9g@mail.gmail.com>
2022-06-15 19:23 ` <Vishal Badole>
2022-06-22 16:32 ` [PATCH] " <Vishal Badole>
2022-06-22 19:37 ` Randy Dunlap
2022-06-22 17:02 ` <Vishal Badole>
[not found] ` <20220624010550.582BBC341C7@smtp.kernel.org>
2022-06-26 18:25 ` <Vishal Badole> [this message]
2022-08-02 22:49 ` Elliott, Robert (Servers)
2022-08-08 17:00 ` <Vishal Badole>
2022-08-21 5:07 ` Elliott, Robert (Servers)
2022-08-21 17:59 ` <Vishal Badole>
2022-06-28 5:08 ` [PATCH v3] Common clock: To " Vishal Badole
2022-08-02 18:09 ` Vishal Badole
2022-08-08 16:46 ` <Vishal Badole>
2022-08-21 18:06 ` <Vishal Badole>
2022-08-22 23:50 ` Stephen Boyd
2022-08-26 17:34 ` <Vishal Badole>
2022-11-27 18:00 ` <Vishal Badole>
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220626182517.GA26001@Mahakal \
--to=badolevishal1116@gmail.com \
--cc=chinmoyghosh2001@gmail.com \
--cc=inux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mintupatel89@gmail.com \
--cc=mturquette@baylibre.com \
--cc=sboyd@kernel.org \
--cc=vimal.kumar32@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).