[...]
>> >
>> > +/**
>> > + * pm_genpd_uninit - Uninitialize a generic I/O PM domain object.
>> > + * @genpd: PM domain object to initialize.
>> > + */
>> > +void pm_genpd_uninit(struct generic_pm_domain *genpd)
>> > +{
>> > + if (IS_ERR_OR_NULL(genpd))
>> > + return;
>> > +
>> > + mutex_lock(&gpd_list_lock);
>> > + list_del(&genpd->gpd_list_node);
>> > + mutex_unlock(&gpd_list_lock);
>>
>> This is too fragile. You don't protect from the cases below.
>>
>> 1. The genpd may have devices attached to it.
>> 2. The genpd may have subdomains.
>>
>> To deal with these case, that's when it becomes more complex which I
>> guess is the reason to why nobody really cared until now.
>>
>
> Can we not just undo the things which pm_genpd_init does, then let the
> driver to deal with all other uninitialize of e.g. "subdomains" which might
> the driver has initialize before? We know there are no slaves or something
> else which is empty because pm_genpd_init runs INIT_LIST_HEAD, or not?
>
> To make such handling above I would add some:
>
> "pm_genpd_uninit_recursive" function, which cleanup everything from a
> power domain, e.g. subdomains, etc and other lists.
>
> What do you suggest to me for e.g. the raspberrypi power domain driver,
> also simple ignore such error handling?
No, let's give it try so see if can improve the situation.
>
>
> For my current use-case I can remove the failure between pm_genpd_init by
> simple getting all "power states" (the bool is_off for pm_genpd_init),
> before calling pm_genpd_init. In this case I don't have any failure between
> calling pm_genpd_init when another pm_genpd_init was before.
>
> Looks like this:
>
> 1. get all states from firmware to get parameter "is_off", which can fail.
> We should get here the states for the power domains which we want to
> register only.
>
> 2. Then call pm_genpd_init, which cannot be fail between another
> pm_genpd_init. (There are only void functions in the middle, which cannot
> fail). If I call pm_genpd_init, I use "is_off" variable from the step
> of "1.".
>
>
> But then I have still the call of "of_genpd_add_provider_onecell" at the end
> of probing which can fail. So this is also not a valid solution, because
> I need to undo everything before.
Okay, got it.
>
>> Moreover, I think there are some more structures to "uninitialize"
>> besides just unlinking the genpd struct from the gpd list. For example
>> a mutex_destroy() should be done.
>>
>
> yes, of course.
>
> - Alex
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
