On 18 September 2014 01:43, Dmitry Torokhov [off-list ref] wrote:

On Thu, Sep 18, 2014 at 01:20:49AM +0200, Ulf Hansson wrote:

quoted

On 17 September 2014 22:10, Dmitry Torokhov [off-list ref] wrote:

quoted

On Wed, Sep 17, 2014 at 08:25:44PM +0200, Ulf Hansson wrote:

quoted

On 16 September 2014 01:36, Rafael J. Wysocki [off-list ref] wrote:

quoted

On Monday, September 15, 2014 09:53:59 AM Dmitry Torokhov wrote:

quoted

On Sun, Sep 14, 2014 at 06:38:58PM +0200, Rafael J. Wysocki wrote:

quoted

On Friday, September 12, 2014 02:05:53 PM Dmitry Torokhov wrote:

quoted

Hi Ulf,

On Tue, Sep 09, 2014 at 01:36:02PM +0200, Ulf Hansson wrote:

quoted

To give callers the option of acting on a errors while removing the
pm_domain ops for the device in the ACPI PM domain, let
acpi_dev_pm_detach() return an int to provide the error code.

So how would callers handle the errors? As far as I can see
acpi_dev_pm_detach() is called from ->remove() and ->shutdown() methods, where
there is no meaningful strategy to handle errors as you are past the point of
no return and you keep on tearing down the device.

The benefit is only relevant when ACPI and genpd PM domains would
co-exist. In that case we might be able to skip genpd_dev_pm_detach()
if acpi_dev_pm_detach() succeeds. So, currently there are  no benefit,
but still it doesn't hurt.

It doe snot have any negative material effect, the drawback is purely
from API perspective.

quoted

quoted

quoted

quoted

This is specifically for what patch [3/9] is doing AFAICS.

The existing callers don't need to worry about this.

OK, so I have the very same comment about patch 3 then: we have
dev_pm_domain_detach() returning error. How would the callers handle errors?

Ulf?

I see your point. How about making dev_pm_domain_detach() to be a void
function instead?

Yes, please.

OK!

quoted

quoted

quoted

quoted

WRT this patch: I'd rater we did not just return generic "error code" just
because we do not know who manages PD for the device. Can we add API to check
if we are using ACPI to manage power domains? Then patch #3 could check if it
needs to use ACPI or generic power domain API.

The problem is scalability. If we have other PM domains implementation
in future, each of them need to be checked prior invoking the attach
functions.
Also, how would we distinguish between genpd and a new PM domain XYZ?

I do not think that trying all available methods to detach a pm domain,
i.e.

        err = acpi_dev_pm_detach();
        if (err)
                err = blah_dev_pm_detach();
        if (err)
                err = flab_dev_pm_detach();
        if (err)
                err = gen_dev_pm_detach();

is any better from scalability point of view. If you need to do that you
will probably have to store something like "struct pd_ops *pd_ops" in
your device and call appropriate implementation via it.

No, that's not needed. Go ahead and have look at both ACPI and genpd,
the interesting part is the validation of struct dev_pm_domain pointer
in the struct device. That's all there is to it, no additional data
are required.

OK, so can you simply put the needed method into struct dev_pm_domain and then
dev_pm_domain_detach() would become:

void dev_pm_domain_detach(struct device *dev, bool power_off)
{
        if (dev->pm_domain)
                dev->pm_domain->detach(dev, power_off);
}

Thanks.

Ohh, didn't quite follow that this is what you meant. That would be a
great improvement, I will adopt in the next version.

Kind regards
Uffe

--
Dmitry