From: Thierry Strudel <tstrudel@google.com>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>,
Kevin Hilman <khilman@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Linux PM <linux-pm@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] PM: domains: create debugfs nodes when adding power domains
Date: Fri, 4 Dec 2020 08:29:52 -0800 [thread overview]
Message-ID: <CAMGPabXRezPazh3PD4_sBycNtAQsi_-auu6rMP4=u74yoHJGrg@mail.gmail.com> (raw)
In-Reply-To: <CAPDyKFoH+_7QJepcPwth_EcBKuj1iU7y7i5semd1QZDsVwT0ww@mail.gmail.com>
Hello Uffe,
> >
> > +static void genpd_debug_remove(struct generic_pm_domain *genpd);
> > +
>
> Please avoid these forward declarations. I think it's better to move
> the code around.
I can move up
static void genpd_debug_remove(struct generic_pm_domain *genpd)
but moving
static void genpd_debug_add(struct generic_pm_domain *genpd)
requires moving all those functions implementation:
DEFINE_SHOW_ATTRIBUTE(summary);
DEFINE_SHOW_ATTRIBUTE(status);
DEFINE_SHOW_ATTRIBUTE(sub_domains);
DEFINE_SHOW_ATTRIBUTE(idle_states);
DEFINE_SHOW_ATTRIBUTE(active_time);
DEFINE_SHOW_ATTRIBUTE(total_idle_time);
DEFINE_SHOW_ATTRIBUTE(devices);
DEFINE_SHOW_ATTRIBUTE(perf_state);
are you fine keeping
static void genpd_debug_add(struct generic_pm_domain *genpd)
as a forward declaration ?
>
> > static int genpd_remove(struct generic_pm_domain *genpd)
> > {
> > struct gpd_link *l, *link;
> > @@ -1987,6 +1992,7 @@ static int genpd_remove(struct generic_pm_domain *genpd)
> > kfree(link);
> > }
> >
> > + genpd_debug_remove(genpd);
> > list_del(&genpd->gpd_list_node);
> > genpd_unlock(genpd);
> > cancel_work_sync(&genpd->power_off_work);
> > @@ -3177,36 +3183,44 @@ DEFINE_SHOW_ATTRIBUTE(total_idle_time);
> > DEFINE_SHOW_ATTRIBUTE(devices);
> > DEFINE_SHOW_ATTRIBUTE(perf_state);
> >
> > -static int __init genpd_debug_init(void)
> > +static void genpd_debug_add(struct generic_pm_domain *genpd)
> > {
> > struct dentry *d;
> > - struct generic_pm_domain *genpd;
> >
> > + d = debugfs_create_dir(genpd->name, genpd_debugfs_dir);
>
> What happens if "genpd_debugfs_dir" is NULL, which will be the case
> until the late_initcall(genpd_debug_init) has been executed!?
Good point, I'll return early if NULL and in
static int __init genpd_debug_init(void)
I'll iterate on the list to create the nodes
> Kind regards
> Uffe
Best regards
Thierry
prev parent reply other threads:[~2020-12-04 16:30 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-03 23:35 [PATCH] PM: domains: create debugfs nodes when adding power domains Thierry Strudel
2020-12-04 14:49 ` Ulf Hansson
2020-12-04 16:29 ` Thierry Strudel [this message]
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='CAMGPabXRezPazh3PD4_sBycNtAQsi_-auu6rMP4=u74yoHJGrg@mail.gmail.com' \
--to=tstrudel@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=khilman@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=ulf.hansson@linaro.org \
/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).