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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <96169281-49b2-121f-0b7e-f54aae66db91@nvidia.com>
Date:   Thu, 24 May 2018 15:34:29 +0100
From:   Jon Hunter <jonathanh@...dia.com>
To:     Ulf Hansson <ulf.hansson@...aro.org>
CC:     Rajendra Nayak <rnayak@...eaurora.org>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Linux PM <linux-pm@...r.kernel.org>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        Kevin Hilman <khilman@...nel.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
        Todor Tomov <todor.tomov@...aro.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        <linux-tegra@...r.kernel.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 8/9] PM / Domains: Add support for multi PM domains per
 device to genpd


On 24/05/18 13:17, Ulf Hansson wrote:
> On 24 May 2018 at 11:36, Jon Hunter <jonathanh@...dia.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