Re: [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd

[Ulf Hansson]

On 24 May 2018 at 11:36, Jon Hunter <jonathanh@xxxxxxxxxx> wrote:
>
> On 24/05/18 08:04, Ulf Hansson wrote:
>
> ...
>
>>> Any reason why we could not add a 'boolean' argument to the API to
>>> indicate
>>> whether the new device should be linked? I think that I prefer the API
>>> handles it, but I can see there could be instances where drivers may wish
>>> to
>>> handle it themselves.
>>
>>
>> Coming back to this question. Both Tegra XUSB and Qcom Camera use
>> case, would benefit from doing the linking themselves, as it needs
>> different PM domains to be powered on depending on the current use
>> case - as to avoid wasting power.
>>
>> However, I can understand that you prefer some simplicity over
>> optimizations, as you told us. Then, does it mean that you are
>> insisting on extending the APIs with a boolean for linking, or are you
>> fine with the driver to call device_link_add()?
>
>
> I am fine with the driver calling device_link_add(), but I just wonder if we
> will find a several drivers doing this and then we will end up doing this
> later anyway.

Okay.

>
> The current API is called ...
>
> * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
> * @dev: Device to attach.
> * @index: The index of the PM domain.
>
> This naming and description is a bit misleading, because really it is not
> attaching the device that is passed, but creating a new device to attach a
> PM domain to. So we should consider renaming and changing the description
> and indicate that users need to link the device.

I picked the name to be consistent with the existing
genpd_dev_pm_attach(). Do you have a better suggestion?

I agree, some details is missing to the description, let me try to
improve it. Actually, I was trying to follow existing descriptions
from genpd_dev_pm_attach(), so perhaps that also needs a little
update.

However, do note that, neither genpd_dev_pm_attach() or
genpd_dev_pm_attach_by_id() is supposed to be called by drivers, but
rather only by the driver core. So description may not be so
important.

In regards to good descriptions, for sure the API added in patch9,
dev_pm_domain_attach_by_id(), needs a good one, as this is what
drivers should be using.

>
> Finally, how is a PM domain attached via calling genpd_dev_pm_attach_by_id()
> detached?

Via the existing genpd_dev_pm_detach(), according to what I have
described in the change log. I clarify the description in regards to
this as well.

Kind regards
Uffe