From 72038df3c580c4c326b83c86149d7ac34007532a Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 26 Apr 2018 10:53:00 +0200 Subject: PM / Domains: Fix error path during attach in genpd 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+ # v4.11+ Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 1ea0e2502e8e..ef6cf3d5d2b5 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2246,6 +2246,9 @@ int genpd_dev_pm_attach(struct device *dev) genpd_lock(pd); ret = genpd_power_on(pd, 0); genpd_unlock(pd); + + if (ret) + genpd_remove_device(pd, dev); out: return ret ? -EPROBE_DEFER : 0; } -- cgit v1.2.3 From ed37884584c4b1ee28bb076c756cd6bd29784c7e Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 26 Apr 2018 10:53:01 +0200 Subject: PM / Domains: Drop comment in genpd about legacy Samsung DT binding The parsing of the Samsung specific DT binding is gone, but the comment in the function header remained. Let's drop the comment to avoid confusions. Fixes: 001d50c9a14f (PM / Domains: Remove obsolete "samsung,power-domain" check) Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index ef6cf3d5d2b5..d3703a149cb5 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2185,9 +2185,6 @@ static void genpd_dev_pm_sync(struct device *dev) * Parse device's OF node to find a PM domain specifier. If such is found, * attaches the device to retrieved pm_domain ops. * - * Both generic and legacy Samsung-specific DT bindings are supported to keep - * backwards compatibility with existing DTBs. - * * Returns 0 on successfully attached PM domain or negative error code. Note * that if a power-domain exists for the device, but it cannot be found or * turned on, then return -EPROBE_DEFER to ensure that the device is not -- cgit v1.2.3 From b56d9c9135629632faff44cff153784e76b82550 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 26 Apr 2018 10:53:02 +0200 Subject: PM / Domains: Drop redundant code in genpd while attaching devices The driver core together with the PM core, nowadays deals with deferring all probes during the device system sleep phases. Therefore genpd no longer need to care about this situation, so let's drop the corresponding code. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index d3703a149cb5..d4b96edb027d 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1377,7 +1377,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, struct gpd_timing_data *td) { struct generic_pm_domain_data *gpd_data; - int ret = 0; + int ret; dev_dbg(dev, "%s()\n", __func__); @@ -1390,11 +1390,6 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, genpd_lock(genpd); - if (genpd->prepared_count > 0) { - ret = -EAGAIN; - goto out; - } - ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0; if (ret) goto out; @@ -2194,7 +2189,6 @@ int genpd_dev_pm_attach(struct device *dev) { struct of_phandle_args pd_args; struct generic_pm_domain *pd; - unsigned int i; int ret; if (!dev->of_node) @@ -2220,14 +2214,7 @@ int genpd_dev_pm_attach(struct device *dev) dev_dbg(dev, "adding to PM domain %s\n", pd->name); - for (i = 1; i < GENPD_RETRY_MAX_MS; i <<= 1) { - ret = genpd_add_device(pd, dev, NULL); - if (ret != -EAGAIN) - break; - - mdelay(i); - cond_resched(); - } + ret = genpd_add_device(pd, dev, NULL); mutex_unlock(&gpd_list_lock); if (ret < 0) { -- cgit v1.2.3 From 4f688748c958deb947759773be6dffe6b44d084d Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 26 Apr 2018 10:53:03 +0200 Subject: PM / Domains: Check for existing PM domain in dev_pm_domain_attach() Instead of checking if an existing PM domain pointer has been assigned in genpd_dev_pm_attach() and acpi_dev_pm_attach(), move the check to the common path in dev_pm_domain_attach(), thus potentially avoid one unnecessary check. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/common.c | 3 +++ drivers/base/power/domain.c | 3 --- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index f6a9ad52cbbf..f3cf61f58f25 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -104,6 +104,9 @@ int dev_pm_domain_attach(struct device *dev, bool power_on) { int ret; + if (dev->pm_domain) + return -EEXIST; + ret = acpi_dev_pm_attach(dev, power_on); if (ret) ret = genpd_dev_pm_attach(dev); diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index d4b96edb027d..b816adbe1e62 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2194,9 +2194,6 @@ int genpd_dev_pm_attach(struct device *dev) if (!dev->of_node) return -ENODEV; - if (dev->pm_domain) - return -EEXIST; - ret = of_parse_phandle_with_args(dev->of_node, "power-domains", "#power-domain-cells", 0, &pd_args); if (ret < 0) -- cgit v1.2.3 From 919b7308fcc452cd4e282bab389c33384a9f3790 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 9 May 2018 12:17:52 +0200 Subject: PM / Domains: Allow a better error handling of dev_pm_domain_attach() The callers of dev_pm_domain_attach() currently checks the returned error code for -EPROBE_DEFER and needs to ignore other error codes. This is an unnecessary limitation, which also leads to a rather strange behaviour in the error path. Address this limitation, by changing the return codes from acpi_dev_pm_attach() and genpd_dev_pm_attach(). More precisely, let them return 0, when no PM domain is needed for the device and then return 1, in case the device was successfully attached to its PM domain. In this way, dev_pm_domain_attach(), gets a better understanding of what happens in the attach attempts and also allowing its caller to better act on real errors codes. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/common.c | 7 ++++--- drivers/base/power/domain.c | 19 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index f3cf61f58f25..5e4b481595bd 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -98,7 +98,8 @@ EXPORT_SYMBOL_GPL(dev_pm_put_subsys_data); * Callers must ensure proper synchronization of this function with power * management callbacks. * - * Returns 0 on successfully attached PM domain or negative error code. + * Returns 0 on successfully attached PM domain and when it found that the + * device don't need a PM domain, else a negative error code. */ int dev_pm_domain_attach(struct device *dev, bool power_on) { @@ -108,10 +109,10 @@ int dev_pm_domain_attach(struct device *dev, bool power_on) return -EEXIST; ret = acpi_dev_pm_attach(dev, power_on); - if (ret) + if (!ret) ret = genpd_dev_pm_attach(dev); - return ret; + return ret < 0 ? ret : 0; } EXPORT_SYMBOL_GPL(dev_pm_domain_attach); diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index b816adbe1e62..455ecea6c812 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2180,10 +2180,11 @@ static void genpd_dev_pm_sync(struct device *dev) * Parse device's OF node to find a PM domain specifier. If such is found, * attaches the device to retrieved pm_domain ops. * - * Returns 0 on successfully attached PM domain or negative error code. Note - * that if a power-domain exists for the device, but it cannot be found or - * turned on, then return -EPROBE_DEFER to ensure that the device is not - * probed and to re-try again later. + * Returns 1 on successfully attached PM domain, 0 when the device don't need a + * PM domain or a negative error code in case of failures. Note that if a + * power-domain exists for the device, but it cannot be found or turned on, + * then return -EPROBE_DEFER to ensure that the device is not probed and to + * re-try again later. */ int genpd_dev_pm_attach(struct device *dev) { @@ -2192,12 +2193,12 @@ int genpd_dev_pm_attach(struct device *dev) int ret; if (!dev->of_node) - return -ENODEV; + return 0; ret = of_parse_phandle_with_args(dev->of_node, "power-domains", "#power-domain-cells", 0, &pd_args); if (ret < 0) - return ret; + return 0; mutex_lock(&gpd_list_lock); pd = genpd_get_from_provider(&pd_args); @@ -2218,7 +2219,7 @@ int genpd_dev_pm_attach(struct device *dev) if (ret != -EPROBE_DEFER) dev_err(dev, "failed to add to PM domain %s: %d", pd->name, ret); - goto out; + return ret; } dev->pm_domain->detach = genpd_dev_pm_detach; @@ -2230,8 +2231,8 @@ int genpd_dev_pm_attach(struct device *dev) if (ret) genpd_remove_device(pd, dev); -out: - return ret ? -EPROBE_DEFER : 0; + + return ret ? -EPROBE_DEFER : 1; } EXPORT_SYMBOL_GPL(genpd_dev_pm_attach); -- cgit v1.2.3 From 88a9769e609a7f3b762a2fe88555c5602758346b Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Thu, 26 Apr 2018 10:53:06 +0200 Subject: driver core: Respect all error codes from dev_pm_domain_attach() The limitation of being able to check only for -EPROBE_DEFER from dev_pm_domain_attach() has been removed. Hence let's respect all error codes and bail out accordingly. Signed-off-by: Ulf Hansson Acked-by: Greg Kroah-Hartman Signed-off-by: Rafael J. Wysocki --- drivers/base/platform.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/platform.c b/drivers/base/platform.c index 8075ddc70a17..9460139d9b02 100644 --- a/drivers/base/platform.c +++ b/drivers/base/platform.c @@ -572,17 +572,16 @@ static int platform_drv_probe(struct device *_dev) return ret; ret = dev_pm_domain_attach(_dev, true); - if (ret != -EPROBE_DEFER) { - 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; - } + if (ret) + goto out; + + if (drv->probe) { + ret = drv->probe(dev); + if (ret) + dev_pm_domain_detach(_dev, true); } +out: if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) { dev_warn(_dev, "probe deferral not supported\n"); ret = -ENXIO; -- cgit v1.2.3 From 94ef9b8e2b941ab7e7ce88fa48ce626fa529bf2f Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Mon, 14 May 2018 16:52:37 +0200 Subject: PM / Domains: Don't return -EEXIST at attach when PM domain exists As dev_pm_domain_attach() isn't the only way to assign PM domain pointers to devices, clearly we must allow a device to have the pointer already being assigned. For this reason, return 0 instead of -EEXIST. Reported-by: Krzysztof Kozlowski Signed-off-by: Ulf Hansson Tested-by: Tested-by: Krzysztof Kozlowski Tested-by: Tony Lindgren Signed-off-by: Rafael J. Wysocki --- drivers/base/power/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/base') diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index 5e4b481595bd..390868c2b392 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -106,7 +106,7 @@ int dev_pm_domain_attach(struct device *dev, bool power_on) int ret; if (dev->pm_domain) - return -EEXIST; + return 0; ret = acpi_dev_pm_attach(dev, power_on); if (!ret) -- cgit v1.2.3 From 49072f97d4a3f8f44fe9677e3df94082b29b7e6f Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Tue, 15 May 2018 15:21:43 +0200 Subject: PM / domains: Improve wording of dev_pm_domain_attach() comment Signed-off-by: Geert Uytterhoeven Acked-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/base') diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index 390868c2b392..7ae62b6355b8 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -98,8 +98,8 @@ EXPORT_SYMBOL_GPL(dev_pm_put_subsys_data); * Callers must ensure proper synchronization of this function with power * management callbacks. * - * Returns 0 on successfully attached PM domain and when it found that the - * device don't need a PM domain, else a negative error code. + * Returns 0 on successfully attached PM domain, or when it is found that the + * device doesn't need a PM domain, else a negative error code. */ int dev_pm_domain_attach(struct device *dev, bool power_on) { -- cgit v1.2.3