Re: [RFC PATCH 2/4] PM / Domains: Add support for explicit control of PM domains

[Jon Hunter]

On 10/04/17 05:09, Rajendra Nayak wrote:
> Hey Jon,
> 
> On 03/28/2017 07:44 PM, Jon Hunter wrote:
>> The current generic PM domain framework (GenDP) only allows a single
>> PM domain to be associated with a given device. There are several
>> use-cases for various system-on-chip devices where it is necessary for
>> a PM domain consumer to control more than one PM domain where the PM
>> domains:
>> i).  Do not conform to a parent-child relationship so are not nested
>> ii). May not be powered on and off at the same time so need independent
>>      control.
>>
>> To support the above, add new APIs for GenPD to allow consumers to get,
>> power-on, power-off and put PM domains so that they can be explicitly
>> controlled by the consumer.
> 
> thanks for working on this RFC.
> 
> []..
>   
>> +/**
>> + * pm_genpd_get - Get a generic I/O PM domain by name
>> + * @name: Name of the PM domain.
>> + *
>> + * Look-ups a PM domain by name. If found, increment the device
>> + * count for PM domain to ensure that the PM domain cannot be
>> + * removed, increment the suspended count so that it can still
>> + * be turned off (when not in-use) and return a pointer to its
>> + * generic_pm_domain structure. If not found return ERR_PTR().
>> + */
>> +struct generic_pm_domain *pm_genpd_get(const char *name)
>> +{
>> +	struct generic_pm_domain *gpd, *genpd = ERR_PTR(-EEXIST);
>> +
>> +	if (!name)
>> +		return ERR_PTR(-EINVAL);
>> +
>> +	mutex_lock(&gpd_list_lock);
>> +	list_for_each_entry(gpd, &gpd_list, gpd_list_node) {
>> +		if (!strcmp(gpd->name, name)) {
>> +			genpd_lock(gpd);
>> +			gpd->device_count++;
> 
> There apis' should also take a device pointer as a parameter,
> so we can track all the devices belonging to a powerdomain.
> That would also mean keeping the genpd->dev_list updated instead of
> only incrementing the device_count here.

I had contemplated that and I am happy to do that if that is what the
consensus wants. However, my only reservation about doing that was it
only allows devices to call the APIs, but maybe that is ok. I was trying
to keep it similar to the clk and regulator APIs.

>> +			gpd->suspended_count++;
>> +			genpd_unlock(gpd);
>> +			genpd = gpd;
>> +			break;
>> +		}
>> +	}
>> +	mutex_unlock(&gpd_list_lock);
>> +
>> +	return genpd;
> 
> Instead of returning a pointer to generic_pm_domain to all
> consumers (who are then free to poke around it) we should hide
> all internal structures handled by the framework and only expose
> some kind of a handle to all the consumers.
> That would also mean having a clear split of the headers to
> distinguish between what's accessible to consumers vs providers.

OK, I will take a look at that.

Cheers
Jon

-- 
nvpublic