While I was on working adding support for multiple PM domains to genpd, I
stumbled over several problems in the error path related to
dev_pm_domain_attach(). Hence, I decided to fix these problems first, which is
what this series intends to address.
There first patch is material for stable, as it fixes a real bug in genpd,
while the rest may be considered as improvement of the error paths when devices
fails to be attached to their PM domains.
I am seeking acks from subsystem maintainers and suggest with funnel the hole
series through Rafael's linux-pm tree, unless there are objections to that of
course.
Ulf Hansson (11):
PM / Domains: Fix error path during attach in genpd
PM / Domains: Drop comment in genpd about legacy Samsung DT binding
PM / Domains: Drop redundant code in genpd while attaching devices
PM / Domains: Check for existing PM domain in dev_pm_domain_attach()
PM / Domains: Allow a better error handling of dev_pm_domain_attach()
amba: Respect all error codes from dev_pm_domain_attach()
driver core: Respect all error codes from dev_pm_domain_attach()
i2c: Respect all error codes from dev_pm_domain_attach()
mmc: sdio: Respect all error codes from dev_pm_domain_attach()
soundwire: Respect all error codes from dev_pm_domain_attach()
spi: Respect all error codes from dev_pm_domain_attach()
drivers/acpi/device_pm.c | 9 +++------
drivers/amba/bus.c | 4 ++--
drivers/base/platform.c | 17 ++++++++---------
drivers/base/power/common.c | 10 +++++++---
drivers/base/power/domain.c | 45 +++++++++++++++-----------------------------
drivers/i2c/i2c-core-base.c | 2 +-
drivers/mmc/core/sdio_bus.c | 2 +-
drivers/soundwire/bus_type.c | 15 +++++++--------
drivers/spi/spi.c | 11 ++++++-----
include/linux/acpi.h | 2 +-
include/linux/pm_domain.h | 2 +-
11 files changed, 52 insertions(+), 67 deletions(-)
--
2.7.4
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.
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Ulf Hansson <[email protected]>
---
drivers/base/platform.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 8075ddc..9460139 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;
--
2.7.4
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 <[email protected]>
---
drivers/acpi/device_pm.c | 3 ---
drivers/base/power/common.c | 3 +++
drivers/base/power/domain.c | 3 ---
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 3d96e4d..d006300 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1259,9 +1259,6 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)
if (!adev)
return -ENODEV;
- if (dev->pm_domain)
- return -EEXIST;
-
/*
* Only attach the power domain to the first device if the
* companion is shared by multiple. This is to prevent doing power
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index f6a9ad5..f3cf61f 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 d4b96ed..b816adb 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)
--
2.7.4
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.
Cc: Mark Brown <[email protected]>
Signed-off-by: Ulf Hansson <[email protected]>
---
drivers/spi/spi.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 7b213fa..eeab67f 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -356,11 +356,12 @@ static int spi_drv_probe(struct device *dev)
}
ret = dev_pm_domain_attach(dev, true);
- if (ret != -EPROBE_DEFER) {
- ret = sdrv->probe(spi);
- if (ret)
- dev_pm_domain_detach(dev, true);
- }
+ if (ret)
+ return ret;
+
+ ret = sdrv->probe(spi);
+ if (ret)
+ dev_pm_domain_detach(dev, true);
return ret;
}
--
2.7.4
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.
Cc: Wolfram Sang <[email protected]>
Signed-off-by: Ulf Hansson <[email protected]>
---
drivers/i2c/i2c-core-base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 1ba40bb..a17f46a 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -363,7 +363,7 @@ static int i2c_device_probe(struct device *dev)
goto err_clear_wakeup_irq;
status = dev_pm_domain_attach(&client->dev, true);
- if (status == -EPROBE_DEFER)
+ if (status)
goto err_clear_wakeup_irq;
/*
--
2.7.4
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 <[email protected]>
---
drivers/acpi/device_pm.c | 6 +++---
drivers/base/power/common.c | 7 ++++---
drivers/base/power/domain.c | 19 ++++++++++---------
include/linux/acpi.h | 2 +-
include/linux/pm_domain.h | 2 +-
5 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index d006300..a7c2673 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -1257,7 +1257,7 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)
struct acpi_device *adev = ACPI_COMPANION(dev);
if (!adev)
- return -ENODEV;
+ return 0;
/*
* Only attach the power domain to the first device if the
@@ -1265,7 +1265,7 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)
* management twice.
*/
if (!acpi_device_is_first_physical_node(adev, dev))
- return -EBUSY;
+ return 0;
acpi_add_pm_notifier(adev, dev, acpi_pm_notify_work_func);
dev_pm_domain_set(dev, &acpi_general_pm_domain);
@@ -1275,7 +1275,7 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)
}
dev->pm_domain->detach = acpi_dev_pm_detach;
- return 0;
+ return 1;
}
EXPORT_SYMBOL_GPL(acpi_dev_pm_attach);
#endif /* CONFIG_PM */
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index f3cf61f..5e4b481 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 b816adb..455ecea 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);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 15bfb15..c01675b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -899,7 +899,7 @@ static inline int acpi_subsys_runtime_suspend(struct device *dev) { return 0; }
static inline int acpi_subsys_runtime_resume(struct device *dev) { return 0; }
static inline int acpi_dev_pm_attach(struct device *dev, bool power_on)
{
- return -ENODEV;
+ return 0;
}
#endif
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 04dbef9..a1da66e 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -280,7 +280,7 @@ static inline int of_genpd_parse_idle_states(struct device_node *dn,
static inline int genpd_dev_pm_attach(struct device *dev)
{
- return -ENODEV;
+ return 0;
}
static inline
--
2.7.4
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.
Cc: Russell King <[email protected]>
Signed-off-by: Ulf Hansson <[email protected]>
---
drivers/amba/bus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 594c228..79fcbff 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -243,7 +243,7 @@ static int amba_probe(struct device *dev)
break;
ret = dev_pm_domain_attach(dev, true);
- if (ret == -EPROBE_DEFER)
+ if (ret)
break;
ret = amba_get_enable_pclk(pcdev);
@@ -370,7 +370,7 @@ static int amba_device_try_add(struct amba_device *dev, struct resource *parent)
}
ret = dev_pm_domain_attach(&dev->dev, true);
- if (ret == -EPROBE_DEFER) {
+ if (ret) {
iounmap(tmp);
goto err_release;
}
--
2.7.4
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 <[email protected]>
---
drivers/base/power/domain.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index d3703a1..d4b96ed 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) {
--
2.7.4
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 <[email protected]>
---
drivers/mmc/core/sdio_bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 2b32b88..b6d8203 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -139,7 +139,7 @@ static int sdio_bus_probe(struct device *dev)
return -ENODEV;
ret = dev_pm_domain_attach(dev, false);
- if (ret == -EPROBE_DEFER)
+ if (ret)
return ret;
/* Unbound SDIO functions are always suspended.
--
2.7.4
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.
Cc: Vinod Koul <[email protected]>
Cc: Sanyog Kale <[email protected]>
Cc: Pierre-Louis Bossart <[email protected]>
Signed-off-by: Ulf Hansson <[email protected]>
---
drivers/soundwire/bus_type.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/soundwire/bus_type.c b/drivers/soundwire/bus_type.c
index d5f3a70..283b283 100644
--- a/drivers/soundwire/bus_type.c
+++ b/drivers/soundwire/bus_type.c
@@ -83,17 +83,16 @@ static int sdw_drv_probe(struct device *dev)
* attach to power domain but don't turn on (last arg)
*/
ret = dev_pm_domain_attach(dev, false);
- if (ret != -EPROBE_DEFER) {
- ret = drv->probe(slave, id);
- if (ret) {
- dev_err(dev, "Probe of %s failed: %d\n", drv->name, ret);
- dev_pm_domain_detach(dev, false);
- }
- }
-
if (ret)
return ret;
+ ret = drv->probe(slave, id);
+ if (ret) {
+ dev_err(dev, "Probe of %s failed: %d\n", drv->name, ret);
+ dev_pm_domain_detach(dev, false);
+ return ret;
+ }
+
/* device is probed so let's read the properties now */
if (slave->ops && slave->ops->read_prop)
slave->ops->read_prop(slave);
--
2.7.4
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: <[email protected]> # v4.11+
Signed-off-by: Ulf Hansson <[email protected]>
---
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 1ea0e25..ef6cf3d5 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;
}
--
2.7.4
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 <[email protected]>
---
drivers/base/power/domain.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index ef6cf3d5..d3703a1 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
--
2.7.4
On Thu, Apr 26, 2018 at 10:53:07AM +0200, Ulf Hansson wrote:
> 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.
>
> Cc: Wolfram Sang <[email protected]>
> Signed-off-by: Ulf Hansson <[email protected]>
Acked-by: Wolfram Sang <[email protected]>
On Thu, Apr 26, 2018 at 10:53:09AM +0200, Ulf Hansson wrote:
> 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.
Acked-by: Vinod Koul <[email protected]>
--
~Vinod
On Thu, Apr 26, 2018 at 10:53:10AM +0200, Ulf Hansson wrote:
> 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.
Acked-by: Mark Brown <[email protected]>
On Thu, Apr 26, 2018 at 10:53:06AM +0200, Ulf Hansson wrote:
> 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.
If that is really true, nice job!
Acked-by: Greg Kroah-Hartman <[email protected]>
On 26 April 2018 at 10:52, Ulf Hansson <[email protected]> wrote:
> While I was on working adding support for multiple PM domains to genpd, I
> stumbled over several problems in the error path related to
> dev_pm_domain_attach(). Hence, I decided to fix these problems first, which is
> what this series intends to address.
>
> There first patch is material for stable, as it fixes a real bug in genpd,
> while the rest may be considered as improvement of the error paths when devices
> fails to be attached to their PM domains.
>
> I am seeking acks from subsystem maintainers and suggest with funnel the hole
> series through Rafael's linux-pm tree, unless there are objections to that of
> course.
>
> Ulf Hansson (11):
> PM / Domains: Fix error path during attach in genpd
> PM / Domains: Drop comment in genpd about legacy Samsung DT binding
> PM / Domains: Drop redundant code in genpd while attaching devices
> PM / Domains: Check for existing PM domain in dev_pm_domain_attach()
> PM / Domains: Allow a better error handling of dev_pm_domain_attach()
> amba: Respect all error codes from dev_pm_domain_attach()
> driver core: Respect all error codes from dev_pm_domain_attach()
> i2c: Respect all error codes from dev_pm_domain_attach()
> mmc: sdio: Respect all error codes from dev_pm_domain_attach()
> soundwire: Respect all error codes from dev_pm_domain_attach()
> spi: Respect all error codes from dev_pm_domain_attach()
>
> drivers/acpi/device_pm.c | 9 +++------
> drivers/amba/bus.c | 4 ++--
> drivers/base/platform.c | 17 ++++++++---------
> drivers/base/power/common.c | 10 +++++++---
> drivers/base/power/domain.c | 45 +++++++++++++++-----------------------------
> drivers/i2c/i2c-core-base.c | 2 +-
> drivers/mmc/core/sdio_bus.c | 2 +-
> drivers/soundwire/bus_type.c | 15 +++++++--------
> drivers/spi/spi.c | 11 ++++++-----
> include/linux/acpi.h | 2 +-
> include/linux/pm_domain.h | 2 +-
> 11 files changed, 52 insertions(+), 67 deletions(-)
Rafael,
I understand if you are busy so this is not a ping. :-)
However, perhaps I can simplify by sending you a PR with these!?
Kind regards
Uffe
On Wed, May 2, 2018 at 2:31 PM, Ulf Hansson <[email protected]> wrote:
> On 26 April 2018 at 10:52, Ulf Hansson <[email protected]> wrote:
>> While I was on working adding support for multiple PM domains to genpd, I
>> stumbled over several problems in the error path related to
>> dev_pm_domain_attach(). Hence, I decided to fix these problems first, which is
>> what this series intends to address.
>>
>> There first patch is material for stable, as it fixes a real bug in genpd,
>> while the rest may be considered as improvement of the error paths when devices
>> fails to be attached to their PM domains.
>>
>> I am seeking acks from subsystem maintainers and suggest with funnel the hole
>> series through Rafael's linux-pm tree, unless there are objections to that of
>> course.
>>
>> Ulf Hansson (11):
>> PM / Domains: Fix error path during attach in genpd
>> PM / Domains: Drop comment in genpd about legacy Samsung DT binding
>> PM / Domains: Drop redundant code in genpd while attaching devices
>> PM / Domains: Check for existing PM domain in dev_pm_domain_attach()
>> PM / Domains: Allow a better error handling of dev_pm_domain_attach()
>> amba: Respect all error codes from dev_pm_domain_attach()
>> driver core: Respect all error codes from dev_pm_domain_attach()
>> i2c: Respect all error codes from dev_pm_domain_attach()
>> mmc: sdio: Respect all error codes from dev_pm_domain_attach()
>> soundwire: Respect all error codes from dev_pm_domain_attach()
>> spi: Respect all error codes from dev_pm_domain_attach()
>>
>> drivers/acpi/device_pm.c | 9 +++------
>> drivers/amba/bus.c | 4 ++--
>> drivers/base/platform.c | 17 ++++++++---------
>> drivers/base/power/common.c | 10 +++++++---
>> drivers/base/power/domain.c | 45 +++++++++++++++-----------------------------
>> drivers/i2c/i2c-core-base.c | 2 +-
>> drivers/mmc/core/sdio_bus.c | 2 +-
>> drivers/soundwire/bus_type.c | 15 +++++++--------
>> drivers/spi/spi.c | 11 ++++++-----
>> include/linux/acpi.h | 2 +-
>> include/linux/pm_domain.h | 2 +-
>> 11 files changed, 52 insertions(+), 67 deletions(-)
>
> Rafael,
>
> I understand if you are busy so this is not a ping. :-)
>
> However, perhaps I can simplify by sending you a PR with these!?
Yes, please.
I'm going to have a look at the patches anyway, but I'm not expecting
to see anything objectionable in them honestly. :-)
Ulf,
* Ulf Hansson <[email protected]> [180426 09:01]:
> 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.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Ulf Hansson <[email protected]>
> ---
> drivers/base/platform.c | 17 ++++++++---------
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 8075ddc..9460139 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;
> --
Looks like this causes Linux next to not boot for me with device
probes failing with error -17. So that's at least omaps, looks
like kernelci has others failing too.
Reverting for 8c123c14bbba ("driver core: Respect all error codes from
dev_pm_domain_attach()") fixes the issue for me.
Sounds like something is missing, any ideas?
Regards,
Tony
On 14 May 2018 at 17:19, Tony Lindgren <[email protected]> wrote:
> Ulf,
>
> * Ulf Hansson <[email protected]> [180426 09:01]:
>> 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.
>>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Signed-off-by: Ulf Hansson <[email protected]>
>> ---
>> drivers/base/platform.c | 17 ++++++++---------
>> 1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
>> index 8075ddc..9460139 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;
>> --
>
> Looks like this causes Linux next to not boot for me with device
> probes failing with error -17. So that's at least omaps, looks
> like kernelci has others failing too.
Yep, problem also reported for some Exynos5 platforms.
Omap suffers from the similar problem, because of its SoC specific way
of attaching devices to PM domains.
>
> Reverting for 8c123c14bbba ("driver core: Respect all error codes from
> dev_pm_domain_attach()") fixes the issue for me.
>
> Sounds like something is missing, any ideas?
This should solve the problem:
https://patchwork.kernel.org/patch/10398597/
Kind regards
Uffe
* Ulf Hansson <[email protected]> [180514 18:59]:
> On 14 May 2018 at 17:19, Tony Lindgren <[email protected]> wrote:
> > Reverting for 8c123c14bbba ("driver core: Respect all error codes from
> > dev_pm_domain_attach()") fixes the issue for me.
> >
> > Sounds like something is missing, any ideas?
>
> This should solve the problem:
>
> https://patchwork.kernel.org/patch/10398597/
Thanks yeah that fixes it for me too.
Regards,
Tony