> -----Original Message-----
> From: Leonard Crestez
> Sent: Monday, September 3, 2018 6:16 PM
> To: Anson Huang <anson.huang@xxxxxxx>; A.s. Dong
> <aisheng.dong@xxxxxxx>
> Cc: Ranjani Vaidyanathan <ranjani.vaidyanathan@xxxxxxx>; Nitin Garg
> <nitin.garg@xxxxxxx>; Jason Liu <jason.hui.liu@xxxxxxx>; Pete Zhang
> <pete.zhang@xxxxxxx>; Eagle Zhou <eagle.zhou@xxxxxxx>; LnxRevLi
> <LnxRevLi@xxxxxxx>; Ulf Hansson <ulf.hansson@xxxxxxxxxx>; v4 . 11+
> <stable@xxxxxxxxxxxxxxx>; Rafael J . Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Subject: [PATCH imx_4.9.y 1/2] PM / Domains: Fix error path during attach in
> genpd
>
> From: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>
> In case the PM domain fails to be powered on in genpd_dev_pm_attach(), it
> returns -EPROBE_DEFER, but keeping the device attached to its PM domain.
> This leads to problems when the next attempt to attach is re-tried. More
> precisely, in that situation an -EEXIST error code is returned, because the device
> already has its PM domain pointer assigned, from the first attempt.
>
> Now, because of the sloppy error handling by the existing callers of
> dev_pm_domain_attach(), probing is allowed to continue when -EEXIST is
> returned. However, in such case there are no guarantees that the PM domain is
> powered on by genpd, which may lead to hangs when buses/drivers tried to
> access their devices.
>
> Let's fix this behaviour, simply by detaching the device when powering on fails
> in genpd_dev_pm_attach().
>
> Cc: v4.11+ <stable@xxxxxxxxxxxxxxx> # v4.11+
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> (cherry picked from commit 72038df3c580c4c326b83c86149d7ac34007532a)
>
> Cherry-picked to imx_4.9.y with minimal conflicts so that we can properly
> handle errors from SCFW pm calls
> Signed-off-by: Leonard Crestez <leonard.crestez@xxxxxxx>
> ---
> drivers/base/power/domain.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index
> 9c3e535795a0..d9681d372997 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1936,10 +1936,13 @@ int genpd_dev_pm_attach(struct device *dev)
> dev->pm_domain->sync = genpd_dev_pm_sync;
>
> mutex_lock(&pd->lock);
> ret = genpd_poweron(pd, 0);
> mutex_unlock(&pd->lock);
> +
> + if (ret)
> + genpd_remove_device(pd, dev);
> out:
> return ret ? -EPROBE_DEFER : 0;
> }
> EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
> #endif /* CONFIG_PM_GENERIC_DOMAINS_OF */
BTW, it looks to me this is not a complete fix.
static int platform_drv_probe(struct device *_dev)
{
ret = dev_pm_domain_attach(_dev, true);
if (ret != -EPROBE_DEFER) { <---- here is risk as it's only hanlle EPROBE_DEFFER. Latest upstream is already changed. However, it may not cause real issue for our case.
if (drv->probe) {
ret = drv->probe(dev);
if (ret)
dev_pm_domain_detach(_dev, true);
} else {
/* don't fail if just dev_pm_domain_attach failed */
ret = 0;
}
}
...
return ret;
}
> --
> 2.17.1
