On 1 June 2015 at 21:42, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > Hi Ulf, > > On Mon, Jun 01, 2015 at 12:18:25PM +0200, Ulf Hansson wrote: >> Other subsystem buses attach PM domains during probe, but prior calling >> the driver's ->probe() method. During the removal phase, detaching the PM >> domain will be done after invoking the driver's ->remove() callback. >> >> Convert the SDIO bus to follow this behavior and add error handling. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> >> --- >> >> A similar patch as $subject patch has been posted and discussed earlier. >> >> According to Aaron Lu, who added the initial support for the ACPI PM domain to >> the SDIO bus, this change is safe. >> http://www.spinics.net/lists/linux-mmc/msg28946.html >> >> --- >> drivers/mmc/core/sdio_bus.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c >> index bee02e6..7e327a6 100644 >> --- a/drivers/mmc/core/sdio_bus.c >> +++ b/drivers/mmc/core/sdio_bus.c >> @@ -137,6 +137,10 @@ static int sdio_bus_probe(struct device *dev) >> if (!id) >> return -ENODEV; >> >> + ret = dev_pm_domain_attach(dev, false); >> + if (ret == -EPROBE_DEFER) >> + return ret; > > Why do we handle only -EPROBE_DEFER? What about other errors? Maybe > dev_pm_domain_attach() is too chatty? Agree. > >> + >> /* Unbound SDIO functions are always suspended. >> * During probe, the function is set active and the usage count >> * is incremented. If the driver supports runtime PM, >> @@ -166,6 +170,7 @@ static int sdio_bus_probe(struct device *dev) >> disable_runtimepm: >> if (func->card->host->caps & MMC_CAP_POWER_OFF_CARD) >> pm_runtime_put_noidle(dev); >> + dev_pm_domain_detach(dev, false); > > If dev_pm_domain_attach() returned -EEXIST (which means that someone > else attached the device to the domain) should we be detaching it? > > I realize that the other buses do the same thing, so these questions are > general. dev_pm_domain_attach() could take a closer look at what errors it gets from acpi_dev_pm_attach() and genpd_dev_pm_attach(). In principle we want the caller of dev_pm_domain_attach() to be able to consider all returned error codes as errors, instead of just -EPROBE_DEFER. On the other hand, the "long term plan" is to have PM domain pointers assigned at device registration point instead of at probe. Since driver core then invokes the ->activate|sync|dismiss() callbacks, it will make the dev_pm_domain_attach|detach() APIs redundant. So, I wonder if it's worth to update the buses/APIs in an intermediate step (even if it would be an improvement), instead of working on adopting the "long term plan". What do you think? Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
