Messages in this thread | | | Subject | Re: [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd | From | Jon Hunter <> | Date | Thu, 24 May 2018 15:34:29 +0100 |
| |
On 24/05/18 13:17, Ulf Hansson wrote: > On 24 May 2018 at 11:36, Jon Hunter <jonathanh@nvidia.com> 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?
Well, it appears to get more of a 'get' function and so I don't see why we could not have 'genpd_dev_get_by_id()' and then we could have a genpd_dev_put() as well (which would call genpd_dev_pm_detach).
> 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.
OK. Same appears to apply here to the description as I mentioned above. Still seems to be more of a 'get' than an attach. So I wonder if it should be dev_pm_domain_get_by_id() instead?
>> 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.
OK, so this bit is a to-do as that is not yet exposed AFAICT. I see that you said 'although we need to extend it to cover cleanup of the earlier registered device, via calling device_unregister().' So if we do this then that would be fine.
Cheers! Jon
-- nvpublic
| |