[PATCH 09/10] PM / Domains: Store the provider in the PM domain structure

[Jon Hunter]

On 08/09/16 12:56, Ulf Hansson wrote:
> On 16 August 2016 at 11:49, Jon Hunter <jonathanh at nvidia.com> wrote:
>> It is possible that a device has more than one provider of PM domains
>> and to support the removal of a PM domain by provider, it is necessary
>> to store a reference to the provider in the PM domain structure.
>> Therefore, store a reference to the firmware node handle in the PM
>> domain structure and populate it when providers (only device-tree based
>> providers are currently supported by PM domains) are registered.
>>
>> Signed-off-by: Jon Hunter <jonathanh at nvidia.com>
>> ---
>>  drivers/base/power/domain.c | 18 ++++++++++++++----
>>  include/linux/pm_domain.h   |  1 +
>>  2 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 72b973539205..0bc145e8e902 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -1539,6 +1539,8 @@ int of_genpd_add_provider_simple(struct device_node *np,
>>                 return -EINVAL;
>>         }
>>
>> +       genpd->provider = &np->fwnode;
>> +
>>         ret = genpd_add_provider(np, genpd_xlate_simple, genpd);
> 
> I guess you want to reset genpd->provider = NULL, when
> genpd_add_provider() fails!?

Yes, will fix.

> Perhaps better to assign genpd->provider when you know
> genpd_add_provider() has succeeded.

Yes seems sane!

>>
>>         mutex_unlock(&gpd_list_lock);
>> @@ -1564,10 +1566,10 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>>         mutex_lock(&gpd_list_lock);
>>
>>         for (i = 0; i < data->num_domains; i++) {
>> -               if (!pm_genpd_present(data->domains[i])) {
>> -                       mutex_unlock(&gpd_list_lock);
>> -                       return -EINVAL;
>> -               }
>> +               if (!pm_genpd_present(data->domains[i]))
>> +                       goto error;
>> +
>> +               data->domains[i]->provider = &np->fwnode;
>>         }
>>
>>         ret = genpd_add_provider(np, genpd_xlate_onecell, data);
>> @@ -1575,6 +1577,14 @@ int of_genpd_add_provider_onecell(struct device_node *np,
>>         mutex_unlock(&gpd_list_lock);
>>
>>         return ret;
>> +
>> +error:
>> +       while (i--)
>> +               data->domains[i]->provider = NULL;
>> +
>> +       mutex_unlock(&gpd_list_lock);
>> +
>> +       return -EINVAL;
>>  }
>>  EXPORT_SYMBOL_GPL(of_genpd_add_provider_onecell);
>>
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index fbdc5c4588ef..4aa285e44eb0 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -51,6 +51,7 @@ struct generic_pm_domain {
>>         struct mutex lock;
>>         struct dev_power_governor *gov;
>>         struct work_struct power_off_work;
>> +       struct fwnode_handle *provider; /* Identity of the domain provider */
>>         const char *name;
>>         atomic_t sd_count;      /* Number of subdomains with power "on" */
>>         enum gpd_status status; /* Current state of the domain */
>> --
>> 2.1.4
>>
> 
> I also think you should extend this change, to also make the
> of_genpd_del_provider() API to reset the genpd->provider = NULL.
> Otherwise you can't track when a provider is removed.

Unfortunately that is not going to work. The function
of_genpd_remove_tail() (patch #10) uses the ->provider member to remove
the last domain for the given provider and of_genpd_del_provider() must
be called before hand.

Cheers
Jon

-- 
nvpublic