On 08/09/16 12:56, Ulf Hansson wrote:
> On 16 August 2016 at 11:49, Jon Hunter <jonathanh@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
