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
