Re: [PATCH] PM: domains: create debugfs nodes when adding power domains

[Thierry Strudel]

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