On Wed, Sep 17, 2014 at 08:25:44PM +0200, Ulf Hansson wrote: > On 16 September 2014 01:36, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > > On Monday, September 15, 2014 09:53:59 AM Dmitry Torokhov wrote: > >> On Sun, Sep 14, 2014 at 06:38:58PM +0200, Rafael J. Wysocki wrote: > >> > On Friday, September 12, 2014 02:05:53 PM Dmitry Torokhov wrote: > >> > > Hi Ulf, > >> > > > >> > > On Tue, Sep 09, 2014 at 01:36:02PM +0200, Ulf Hansson wrote: > >> > > > 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. > > >> > > >> > 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. > > > > >> 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. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
