linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/4] Allow USB devices to remain runtime-suspended when sleeping
@ 2015-10-02  7:14 Tomeu Vizoso
  2015-10-02  7:14 ` [PATCH v8 1/4] device core: add device_is_bound() Tomeu Vizoso
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Tomeu Vizoso @ 2015-10-02  7:14 UTC (permalink / raw)
  To: linux-pm, Alan Stern, Rafael J. Wysocki, martyn.welch, Ulf Hansson
  Cc: Tomeu Vizoso, David Airlie, Tomas Winkler, linux-usb, linux-acpi,
	linux-kernel, dri-devel, Russell King, Len Brown, Len Brown,
	Kevin Hilman, Tony Lindgren, Greg Kroah-Hartman, linux-omap,
	Pavel Machek, linux-arm-kernel

Hi,

this is v8 of an attempt to make it easier for devices to remain in
runtime PM when the system goes to sleep, mainly to reduce the time
spent resuming devices.

For this, we interpret the absence of all PM callback implementations as
it being safe to do direct_complete, so their ancestors aren't prevented
from remaining runtime-suspended.

Additionally, the prepare() callback of USB devices will return 1 if
runtime PM is enabled and the current wakeup settings are correct.

With these changes, a uvcvideo device (for example) stays in runtime
suspend when the system goes to sleep and is left in that state when the
system resumes, not delaying it unnecessarily.

Thanks,

Tomeu

Changes in v8:
- Add device_is_bound()
- Add dev_pm_domain_set() and update code to use it
- Move no_pm_callbacks field into CONFIG_PM_SLEEP
- Call device_check_pm_callbacks only after a device is bound or unbound

Changes in v7:
- Reduce indentation by adding a label in device_prepare()

Changes in v6:
- Add stub for !CONFIG_PM.
- Move implementation of device_check_pm_callbacks to power/main.c as it
  doesn't belong to CONFIG_PM_SLEEP.
- Take dev->power.lock before modifying flag.

Changes in v5:
- Check for all dev_pm_ops instances associated to a device, updating a
  no_pm_callbacks flag at the times when that could change.

Tomeu Vizoso (4):
  device core: add device_is_bound()
  PM / Domains: add setter for dev.pm_domain
  PM / sleep: Go direct_complete if driver has no callbacks
  USB / PM: Allow USB devices to remain runtime-suspended when sleeping

 arch/arm/mach-omap2/omap_device.c |  7 ++++---
 drivers/acpi/acpi_lpss.c          |  5 +++--
 drivers/acpi/device_pm.c          |  5 +++--
 drivers/base/dd.c                 | 12 ++++++++++--
 drivers/base/power/clock_ops.c    |  5 +++--
 drivers/base/power/common.c       |  8 ++++++++
 drivers/base/power/domain.c       |  6 ++++--
 drivers/base/power/main.c         | 33 +++++++++++++++++++++++++++++++++
 drivers/base/power/power.h        |  6 ++++++
 drivers/gpu/vga/vga_switcheroo.c  | 10 +++++-----
 drivers/misc/mei/pci-me.c         |  5 +++--
 drivers/misc/mei/pci-txe.c        |  5 +++--
 drivers/usb/core/port.c           |  6 ++++++
 drivers/usb/core/usb.c            | 11 ++++++++++-
 include/linux/device.h            |  2 ++
 include/linux/pm.h                |  1 +
 include/linux/pm_domain.h         |  3 +++
 17 files changed, 107 insertions(+), 23 deletions(-)

-- 
2.4.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v8 1/4] device core: add device_is_bound()
  2015-10-02  7:14 [PATCH v8 0/4] Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso
@ 2015-10-02  7:14 ` Tomeu Vizoso
  2015-10-02 13:46   ` Rafael J. Wysocki
  2015-10-02  7:14 ` [PATCH v8 2/4] PM / Domains: add setter for dev.pm_domain Tomeu Vizoso
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Tomeu Vizoso @ 2015-10-02  7:14 UTC (permalink / raw)
  To: linux-pm, Alan Stern, Rafael J. Wysocki, martyn.welch, Ulf Hansson
  Cc: Tomeu Vizoso, Greg Kroah-Hartman, linux-kernel

Adds a function that tells whether a device is already bound to a
driver.

This is needed to warn when there is an attempt to change the PM domain
of a device that has finished probing already. The reason why we want to
enforce that is because in the general case that can cause problems and
also that we can simplify code quite a bit if we can always assume that.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v8:
- Add device_is_bound()

 drivers/base/dd.c      | 9 +++++++--
 include/linux/device.h | 2 ++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index be0eb4639128..f2007fa7316f 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -192,9 +192,14 @@ static int deferred_probe_initcall(void)
 }
 late_initcall(deferred_probe_initcall);
 
+bool device_is_bound(struct device *dev)
+{
+	return klist_node_attached(&dev->p->knode_driver);
+}
+
 static void driver_bound(struct device *dev)
 {
-	if (klist_node_attached(&dev->p->knode_driver)) {
+	if (device_is_bound(dev)) {
 		printk(KERN_WARNING "%s: device %s already bound\n",
 			__func__, kobject_name(&dev->kobj));
 		return;
@@ -545,7 +550,7 @@ static int __device_attach(struct device *dev, bool allow_async)
 
 	device_lock(dev);
 	if (dev->driver) {
-		if (klist_node_attached(&dev->p->knode_driver)) {
+		if (device_is_bound(dev)) {
 			ret = 1;
 			goto out_unlock;
 		}
diff --git a/include/linux/device.h b/include/linux/device.h
index 5d7bc6349930..9b41c05fb479 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1035,6 +1035,8 @@ extern int __must_check driver_attach(struct device_driver *drv);
 extern void device_initial_probe(struct device *dev);
 extern int __must_check device_reprobe(struct device *dev);
 
+extern bool device_is_bound(struct device *dev);
+
 /*
  * Easy functions for dynamically creating devices on the fly
  */
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 2/4] PM / Domains: add setter for dev.pm_domain
  2015-10-02  7:14 [PATCH v8 0/4] Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso
  2015-10-02  7:14 ` [PATCH v8 1/4] device core: add device_is_bound() Tomeu Vizoso
@ 2015-10-02  7:14 ` Tomeu Vizoso
  2015-10-02  7:14 ` [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks Tomeu Vizoso
  2015-10-02  7:14 ` [PATCH v8 4/4] USB / PM: Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso
  3 siblings, 0 replies; 11+ messages in thread
From: Tomeu Vizoso @ 2015-10-02  7:14 UTC (permalink / raw)
  To: linux-pm, Alan Stern, Rafael J. Wysocki, martyn.welch, Ulf Hansson
  Cc: Tomeu Vizoso, David Airlie, Tomas Winkler, linux-acpi,
	linux-kernel, dri-devel, Russell King, Greg Kroah-Hartman,
	Kevin Hilman, Tony Lindgren, Len Brown, linux-omap, Pavel Machek,
	linux-arm-kernel

Adds a function that sets the pointer to dev_pm_domain in struct device
and that warns if the device has already finished probing. The reason
why we want to enforce that is because in the general case that can
cause problems and also that we can simplify code quite a bit if we can
always assume that.

This patch also changes all current code that directly sets the
dev.pm_domain pointer.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v8:
- Add dev_pm_domain_set() and update code to use it

 arch/arm/mach-omap2/omap_device.c |  7 ++++---
 drivers/acpi/acpi_lpss.c          |  5 +++--
 drivers/acpi/device_pm.c          |  5 +++--
 drivers/base/power/clock_ops.c    |  5 +++--
 drivers/base/power/common.c       |  8 ++++++++
 drivers/base/power/domain.c       |  4 ++--
 drivers/gpu/vga/vga_switcheroo.c  | 10 +++++-----
 drivers/misc/mei/pci-me.c         |  5 +++--
 drivers/misc/mei/pci-txe.c        |  5 +++--
 include/linux/pm_domain.h         |  3 +++
 10 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
index 4cb8fd9f741f..204101d11632 100644
--- a/arch/arm/mach-omap2/omap_device.c
+++ b/arch/arm/mach-omap2/omap_device.c
@@ -32,6 +32,7 @@
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include <linux/notifier.h>
@@ -168,7 +169,7 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
 			r->name = dev_name(&pdev->dev);
 	}
 
-	pdev->dev.pm_domain = &omap_device_pm_domain;
+	dev_pm_domain_set(&pdev->dev, &omap_device_pm_domain);
 
 	if (device_active) {
 		omap_device_enable(pdev);
@@ -180,7 +181,7 @@ odbfd_exit1:
 odbfd_exit:
 	/* if data/we are at fault.. load up a fail handler */
 	if (ret)
-		pdev->dev.pm_domain = &omap_device_fail_pm_domain;
+		dev_pm_domain_set(&pdev->dev, &omap_device_fail_pm_domain);
 
 	return ret;
 }
@@ -701,7 +702,7 @@ int omap_device_register(struct platform_device *pdev)
 {
 	pr_debug("omap_device: %s: registering\n", pdev->name);
 
-	pdev->dev.pm_domain = &omap_device_pm_domain;
+	dev_pm_domain_set(&pdev->dev, &omap_device_pm_domain);
 	return platform_device_add(pdev);
 }
 
diff --git a/drivers/acpi/acpi_lpss.c b/drivers/acpi/acpi_lpss.c
index f51bd0d0bc17..cc6e1abc69b3 100644
--- a/drivers/acpi/acpi_lpss.c
+++ b/drivers/acpi/acpi_lpss.c
@@ -17,6 +17,7 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/platform_data/clk-lpss.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/delay.h>
 
@@ -706,7 +707,7 @@ static int acpi_lpss_platform_notify(struct notifier_block *nb,
 
 	switch (action) {
 	case BUS_NOTIFY_ADD_DEVICE:
-		pdev->dev.pm_domain = &acpi_lpss_pm_domain;
+		dev_pm_domain_set(&pdev->dev, &acpi_lpss_pm_domain);
 		if (pdata->dev_desc->flags & LPSS_LTR)
 			return sysfs_create_group(&pdev->dev.kobj,
 						  &lpss_attr_group);
@@ -714,7 +715,7 @@ static int acpi_lpss_platform_notify(struct notifier_block *nb,
 	case BUS_NOTIFY_DEL_DEVICE:
 		if (pdata->dev_desc->flags & LPSS_LTR)
 			sysfs_remove_group(&pdev->dev.kobj, &lpss_attr_group);
-		pdev->dev.pm_domain = NULL;
+		dev_pm_domain_set(&pdev->dev, NULL);
 		break;
 	default:
 		break;
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index 4806b7f856c4..8c5955bf9bbf 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -22,6 +22,7 @@
 #include <linux/export.h>
 #include <linux/mutex.h>
 #include <linux/pm_qos.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 
 #include "internal.h"
@@ -1076,7 +1077,7 @@ static void acpi_dev_pm_detach(struct device *dev, bool power_off)
 	struct acpi_device *adev = ACPI_COMPANION(dev);
 
 	if (adev && dev->pm_domain == &acpi_general_pm_domain) {
-		dev->pm_domain = NULL;
+		dev_pm_domain_set(dev, NULL);
 		acpi_remove_pm_notifier(adev);
 		if (power_off) {
 			/*
@@ -1128,7 +1129,7 @@ int acpi_dev_pm_attach(struct device *dev, bool power_on)
 		return -EBUSY;
 
 	acpi_add_pm_notifier(adev, dev, acpi_pm_notify_work_func);
-	dev->pm_domain = &acpi_general_pm_domain;
+	dev_pm_domain_set(dev, &acpi_general_pm_domain);
 	if (power_on) {
 		acpi_dev_pm_full_power(adev);
 		acpi_device_wakeup(adev, ACPI_STATE_S0, false);
diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index 652b5a367c1f..e80fda6c03a9 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -15,6 +15,7 @@
 #include <linux/clkdev.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 
 #ifdef CONFIG_PM
@@ -346,7 +347,7 @@ static int pm_clk_notify(struct notifier_block *nb,
 		if (error)
 			break;
 
-		dev->pm_domain = clknb->pm_domain;
+		dev_pm_domain_set(dev, clknb->pm_domain);
 		if (clknb->con_ids[0]) {
 			for (con_id = clknb->con_ids; *con_id; con_id++)
 				pm_clk_add(dev, *con_id);
@@ -359,7 +360,7 @@ static int pm_clk_notify(struct notifier_block *nb,
 		if (dev->pm_domain != clknb->pm_domain)
 			break;
 
-		dev->pm_domain = NULL;
+		dev_pm_domain_set(dev, NULL);
 		pm_clk_destroy(dev);
 		break;
 	}
diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index f32b802b98f4..5d308aab22d6 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -128,3 +128,11 @@ void dev_pm_domain_detach(struct device *dev, bool power_off)
 		dev->pm_domain->detach(dev, power_off);
 }
 EXPORT_SYMBOL_GPL(dev_pm_domain_detach);
+
+void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd)
+{
+	WARN(device_is_bound(dev),
+	     "PM domains can only be changed for unbound devices\n");
+	dev->pm_domain = pd;
+}
+EXPORT_SYMBOL_GPL(dev_pm_domain_set);
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 16550c63d611..b75d02aa8d93 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1240,7 +1240,7 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev,
 	}
 
 	dev->power.subsys_data->domain_data = &gpd_data->base;
-	dev->pm_domain = &genpd->domain;
+	dev_pm_domain_set(dev, &genpd->domain);
 
 	spin_unlock_irq(&dev->power.lock);
 
@@ -1259,7 +1259,7 @@ static void genpd_free_dev_data(struct device *dev,
 {
 	spin_lock_irq(&dev->power.lock);
 
-	dev->pm_domain = NULL;
+	dev_pm_domain_set(dev, NULL);
 	dev->power.subsys_data->domain_data = NULL;
 
 	spin_unlock_irq(&dev->power.lock);
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 21060668fd25..2c3208ea6de4 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -665,17 +665,17 @@ int vga_switcheroo_init_domain_pm_ops(struct device *dev,
 		domain->ops.runtime_suspend = vga_switcheroo_runtime_suspend;
 		domain->ops.runtime_resume = vga_switcheroo_runtime_resume;
 
-		dev->pm_domain = domain;
+		dev_pm_domain_set(dev, domain);
 		return 0;
 	}
-	dev->pm_domain = NULL;
+	dev_pm_domain_set(dev, NULL);
 	return -EINVAL;
 }
 EXPORT_SYMBOL(vga_switcheroo_init_domain_pm_ops);
 
 void vga_switcheroo_fini_domain_pm_ops(struct device *dev)
 {
-	dev->pm_domain = NULL;
+	dev_pm_domain_set(dev, NULL);
 }
 EXPORT_SYMBOL(vga_switcheroo_fini_domain_pm_ops);
 
@@ -719,10 +719,10 @@ vga_switcheroo_init_domain_pm_optimus_hdmi_audio(struct device *dev,
 		domain->ops.runtime_resume =
 			vga_switcheroo_runtime_resume_hdmi_audio;
 
-		dev->pm_domain = domain;
+		dev_pm_domain_set(dev, domain);
 		return 0;
 	}
-	dev->pm_domain = NULL;
+	dev_pm_domain_set(dev, NULL);
 	return -EINVAL;
 }
 EXPORT_SYMBOL(vga_switcheroo_init_domain_pm_optimus_hdmi_audio);
diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c
index 27678d8154e0..75fc9c688df8 100644
--- a/drivers/misc/mei/pci-me.c
+++ b/drivers/misc/mei/pci-me.c
@@ -31,6 +31,7 @@
 #include <linux/jiffies.h>
 #include <linux/interrupt.h>
 
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 
 #include <linux/mei.h>
@@ -436,7 +437,7 @@ static inline void mei_me_set_pm_domain(struct mei_device *dev)
 		dev->pg_domain.ops.runtime_resume = mei_me_pm_runtime_resume;
 		dev->pg_domain.ops.runtime_idle = mei_me_pm_runtime_idle;
 
-		pdev->dev.pm_domain = &dev->pg_domain;
+		dev_pm_domain_set(&pdev->dev, &dev->pg_domain);
 	}
 }
 
@@ -448,7 +449,7 @@ static inline void mei_me_set_pm_domain(struct mei_device *dev)
 static inline void mei_me_unset_pm_domain(struct mei_device *dev)
 {
 	/* stop using pm callbacks if any */
-	dev->dev->pm_domain = NULL;
+	dev_pm_domain_set(dev->dev, NULL);
 }
 
 static const struct dev_pm_ops mei_me_pm_ops = {
diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c
index 0882c0201907..71f8a7475717 100644
--- a/drivers/misc/mei/pci-txe.c
+++ b/drivers/misc/mei/pci-txe.c
@@ -27,6 +27,7 @@
 #include <linux/jiffies.h>
 #include <linux/interrupt.h>
 #include <linux/workqueue.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 
 #include <linux/mei.h>
@@ -388,7 +389,7 @@ static inline void mei_txe_set_pm_domain(struct mei_device *dev)
 		dev->pg_domain.ops.runtime_resume = mei_txe_pm_runtime_resume;
 		dev->pg_domain.ops.runtime_idle = mei_txe_pm_runtime_idle;
 
-		pdev->dev.pm_domain = &dev->pg_domain;
+		dev_pm_domain_set(&pdev->dev, &dev->pg_domain);
 	}
 }
 
@@ -400,7 +401,7 @@ static inline void mei_txe_set_pm_domain(struct mei_device *dev)
 static inline void mei_txe_unset_pm_domain(struct mei_device *dev)
 {
 	/* stop using pm callbacks if any */
-	dev->dev->pm_domain = NULL;
+	dev_pm_domain_set(dev->dev, NULL);
 }
 
 static const struct dev_pm_ops mei_txe_pm_ops = {
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index b1cf7e797892..4ae4b14fb7b4 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -306,12 +306,15 @@ static inline int of_genpd_add_provider_onecell(struct device_node *np,
 #ifdef CONFIG_PM
 extern int dev_pm_domain_attach(struct device *dev, bool power_on);
 extern void dev_pm_domain_detach(struct device *dev, bool power_off);
+extern void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd);
 #else
 static inline int dev_pm_domain_attach(struct device *dev, bool power_on)
 {
 	return -ENODEV;
 }
 static inline void dev_pm_domain_detach(struct device *dev, bool power_off) {}
+static inline void dev_pm_domain_set(struct device *dev,
+				     struct dev_pm_domain *pd) {}
 #endif
 
 #endif /* _LINUX_PM_DOMAIN_H */
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks
  2015-10-02  7:14 [PATCH v8 0/4] Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso
  2015-10-02  7:14 ` [PATCH v8 1/4] device core: add device_is_bound() Tomeu Vizoso
  2015-10-02  7:14 ` [PATCH v8 2/4] PM / Domains: add setter for dev.pm_domain Tomeu Vizoso
@ 2015-10-02  7:14 ` Tomeu Vizoso
  2015-10-02  7:48   ` Ulf Hansson
  2015-10-02  7:14 ` [PATCH v8 4/4] USB / PM: Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso
  3 siblings, 1 reply; 11+ messages in thread
From: Tomeu Vizoso @ 2015-10-02  7:14 UTC (permalink / raw)
  To: linux-pm, Alan Stern, Rafael J. Wysocki, martyn.welch, Ulf Hansson
  Cc: Tomeu Vizoso, linux-kernel, Len Brown, Kevin Hilman,
	Greg Kroah-Hartman, Pavel Machek

If a suitable prepare callback cannot be found for a given device and
its driver has no PM callbacks at all, assume that it can go direct to
complete when the system goes to sleep.

The reason for this is that there's lots of devices in a system that do
no PM at all and there's no reason for them to prevent their ancestors
to do direct_complete if they can support it.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---

Changes in v8:
- Move no_pm_callbacks field into CONFIG_PM_SLEEP
- Call device_check_pm_callbacks only after a device is bound or unbound

Changes in v7:
- Reduce indentation by adding a label in device_prepare()

Changes in v6:
- Add stub for !CONFIG_PM.
- Move implementation of device_check_pm_callbacks to power/main.c as it
  doesn't belong to CONFIG_PM_SLEEP.
- Take dev->power.lock before modifying flag.

Changes in v5:
- Check for all dev_pm_ops instances associated to a device, updating a
  no_pm_callbacks flag at the times when that could change.

 drivers/base/dd.c           |  3 +++
 drivers/base/power/domain.c |  2 ++
 drivers/base/power/main.c   | 33 +++++++++++++++++++++++++++++++++
 drivers/base/power/power.h  |  6 ++++++
 include/linux/pm.h          |  1 +
 5 files changed, 45 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index f2007fa7316f..2e3809affb06 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -210,6 +210,8 @@ static void driver_bound(struct device *dev)
 
 	klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
 
+	device_check_pm_callbacks(dev);
+
 	/*
 	 * Make sure the device is no longer in one of the deferred lists and
 	 * kick off retrying all pending devices
@@ -700,6 +702,7 @@ static void __device_release_driver(struct device *dev)
 			dev->pm_domain->dismiss(dev);
 
 		klist_remove(&dev->p->knode_driver);
+		device_check_pm_callbacks(dev);
 		if (dev->bus)
 			blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 						     BUS_NOTIFY_UNBOUND_DRIVER,
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index b75d02aa8d93..9dc1ce9c573b 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -20,6 +20,8 @@
 #include <linux/suspend.h>
 #include <linux/export.h>
 
+#include "power.h"
+
 #define GENPD_RETRY_MAX_MS	250		/* Approximate */
 
 #define GENPD_DEV_CALLBACK(genpd, type, callback, dev)		\
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 1710c26ba097..43b44f8251c7 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -1569,6 +1569,11 @@ static int device_prepare(struct device *dev, pm_message_t state)
 
 	dev->power.wakeup_path = device_may_wakeup(dev);
 
+	if (dev->power.no_pm_callbacks) {
+		ret = 1;	/* Let device go direct_complete */
+		goto unlock;
+	}
+
 	if (dev->pm_domain) {
 		info = "preparing power domain ";
 		callback = dev->pm_domain->ops.prepare;
@@ -1591,6 +1596,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
 	if (callback)
 		ret = callback(dev);
 
+unlock:
 	device_unlock(dev);
 
 	if (ret < 0) {
@@ -1719,3 +1725,30 @@ void dpm_for_each_dev(void *data, void (*fn)(struct device *, void *))
 	device_pm_unlock();
 }
 EXPORT_SYMBOL_GPL(dpm_for_each_dev);
+
+static bool pm_ops_is_empty(const struct dev_pm_ops *ops)
+{
+	if (!ops)
+		return true;
+
+	return !ops->prepare &&
+	       !ops->suspend &&
+	       !ops->suspend_late &&
+	       !ops->suspend_noirq &&
+	       !ops->resume_noirq &&
+	       !ops->resume_early &&
+	       !ops->resume &&
+	       !ops->complete;
+}
+
+void device_check_pm_callbacks(struct device *dev)
+{
+	spin_lock_irq(&dev->power.lock);
+	dev->power.no_pm_callbacks =
+		(!dev->bus || pm_ops_is_empty(dev->bus->pm)) &&
+		(!dev->class || pm_ops_is_empty(dev->class->pm)) &&
+		(!dev->type || pm_ops_is_empty(dev->type->pm)) &&
+		(!dev->pm_domain || pm_ops_is_empty(&dev->pm_domain->ops)) &&
+		(!dev->driver || pm_ops_is_empty(dev->driver->pm));
+	spin_unlock_irq(&dev->power.lock);
+}
diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
index 998fa6b23084..b3e35e825cd8 100644
--- a/drivers/base/power/power.h
+++ b/drivers/base/power/power.h
@@ -37,6 +37,8 @@ extern void device_wakeup_detach_irq(struct device *dev);
 extern void device_wakeup_arm_wake_irqs(void);
 extern void device_wakeup_disarm_wake_irqs(void);
 
+extern void device_check_pm_callbacks(struct device *dev);
+
 #else
 
 static inline int
@@ -58,6 +60,10 @@ static inline void device_wakeup_disarm_wake_irqs(void)
 {
 }
 
+static inline void device_check_pm_callbacks(struct device *dev)
+{
+}
+
 #endif /* CONFIG_PM_SLEEP */
 
 /*
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 35d599e7250d..7943acb74ae4 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -573,6 +573,7 @@ struct dev_pm_info {
 	struct wakeup_source	*wakeup;
 	bool			wakeup_path:1;
 	bool			syscore:1;
+	bool			no_pm_callbacks:1;	/* Owned by the PM core */
 #else
 	unsigned int		should_wakeup:1;
 #endif
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v8 4/4] USB / PM: Allow USB devices to remain runtime-suspended when sleeping
  2015-10-02  7:14 [PATCH v8 0/4] Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso
                   ` (2 preceding siblings ...)
  2015-10-02  7:14 ` [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks Tomeu Vizoso
@ 2015-10-02  7:14 ` Tomeu Vizoso
  3 siblings, 0 replies; 11+ messages in thread
From: Tomeu Vizoso @ 2015-10-02  7:14 UTC (permalink / raw)
  To: linux-pm, Alan Stern, Rafael J. Wysocki, martyn.welch, Ulf Hansson
  Cc: Tomeu Vizoso, Greg Kroah-Hartman, linux-usb, linux-kernel

Have dev_pm_ops.prepare return 1 for USB devices and ports so that USB
devices can remain runtime-suspended when the system goes to a sleep
state, if their wakeup state is correct and they have runtime PM enabled.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---


 drivers/usb/core/port.c |  6 ++++++
 drivers/usb/core/usb.c  | 11 ++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 210618319f10..f49707d73b5a 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -168,12 +168,18 @@ static int usb_port_runtime_suspend(struct device *dev)
 
 	return retval;
 }
+
+static int usb_port_prepare(struct device *dev)
+{
+	return 1;
+}
 #endif
 
 static const struct dev_pm_ops usb_port_pm_ops = {
 #ifdef CONFIG_PM
 	.runtime_suspend =	usb_port_runtime_suspend,
 	.runtime_resume =	usb_port_runtime_resume,
+	.prepare =		usb_port_prepare,
 #endif
 };
 
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 8d5b2f4113cd..cf4dde11db1c 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -316,7 +316,16 @@ static int usb_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
 
 static int usb_dev_prepare(struct device *dev)
 {
-	return 0;		/* Implement eventually? */
+	struct usb_device *udev = to_usb_device(dev);
+
+	if (!pm_runtime_enabled(dev))
+		return 0;
+
+	/* Return 0 if the current wakeup setting is wrong, otherwise 1 */
+	if (udev->do_remote_wakeup != device_may_wakeup(dev))
+		return 0;
+
+	return 1;
 }
 
 static void usb_dev_complete(struct device *dev)
-- 
2.4.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks
  2015-10-02  7:14 ` [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks Tomeu Vizoso
@ 2015-10-02  7:48   ` Ulf Hansson
  2015-10-02  8:10     ` Tomeu Vizoso
  0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2015-10-02  7:48 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: linux-pm, Alan Stern, Rafael J. Wysocki, martyn.welch,
	linux-kernel, Len Brown, Kevin Hilman, Greg Kroah-Hartman,
	Pavel Machek

On 2 October 2015 at 09:14, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> If a suitable prepare callback cannot be found for a given device and
> its driver has no PM callbacks at all, assume that it can go direct to
> complete when the system goes to sleep.
>
> The reason for this is that there's lots of devices in a system that do
> no PM at all and there's no reason for them to prevent their ancestors
> to do direct_complete if they can support it.
>
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
>
> Changes in v8:
> - Move no_pm_callbacks field into CONFIG_PM_SLEEP
> - Call device_check_pm_callbacks only after a device is bound or unbound

Devices that don't use a driver, will not get this feature for "free".
I expect in those cases, they will have to call
device_check_pm_callbacks() themselves, right?
Perhaps we should mention that, at least in the change log.

>
> Changes in v7:
> - Reduce indentation by adding a label in device_prepare()
>
> Changes in v6:
> - Add stub for !CONFIG_PM.
> - Move implementation of device_check_pm_callbacks to power/main.c as it
>   doesn't belong to CONFIG_PM_SLEEP.
> - Take dev->power.lock before modifying flag.
>
> Changes in v5:
> - Check for all dev_pm_ops instances associated to a device, updating a
>   no_pm_callbacks flag at the times when that could change.
>
>  drivers/base/dd.c           |  3 +++
>  drivers/base/power/domain.c |  2 ++
>  drivers/base/power/main.c   | 33 +++++++++++++++++++++++++++++++++
>  drivers/base/power/power.h  |  6 ++++++
>  include/linux/pm.h          |  1 +
>  5 files changed, 45 insertions(+)
>
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index f2007fa7316f..2e3809affb06 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -210,6 +210,8 @@ static void driver_bound(struct device *dev)
>
>         klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
>
> +       device_check_pm_callbacks(dev);
> +
>         /*
>          * Make sure the device is no longer in one of the deferred lists and
>          * kick off retrying all pending devices
> @@ -700,6 +702,7 @@ static void __device_release_driver(struct device *dev)
>                         dev->pm_domain->dismiss(dev);
>
>                 klist_remove(&dev->p->knode_driver);
> +               device_check_pm_callbacks(dev);
>                 if (dev->bus)
>                         blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>                                                      BUS_NOTIFY_UNBOUND_DRIVER,
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index b75d02aa8d93..9dc1ce9c573b 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -20,6 +20,8 @@
>  #include <linux/suspend.h>
>  #include <linux/export.h>
>
> +#include "power.h"
> +

Seems like a leftover from the earlier version of the patch...

>  #define GENPD_RETRY_MAX_MS     250             /* Approximate */
>
>  #define GENPD_DEV_CALLBACK(genpd, type, callback, dev)         \
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 1710c26ba097..43b44f8251c7 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -1569,6 +1569,11 @@ static int device_prepare(struct device *dev, pm_message_t state)
>
>         dev->power.wakeup_path = device_may_wakeup(dev);
>
> +       if (dev->power.no_pm_callbacks) {
> +               ret = 1;        /* Let device go direct_complete */
> +               goto unlock;
> +       }
> +
>         if (dev->pm_domain) {
>                 info = "preparing power domain ";
>                 callback = dev->pm_domain->ops.prepare;
> @@ -1591,6 +1596,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
>         if (callback)
>                 ret = callback(dev);
>
> +unlock:
>         device_unlock(dev);
>
>         if (ret < 0) {
> @@ -1719,3 +1725,30 @@ void dpm_for_each_dev(void *data, void (*fn)(struct device *, void *))
>         device_pm_unlock();
>  }
>  EXPORT_SYMBOL_GPL(dpm_for_each_dev);
> +
> +static bool pm_ops_is_empty(const struct dev_pm_ops *ops)
> +{
> +       if (!ops)
> +               return true;
> +
> +       return !ops->prepare &&
> +              !ops->suspend &&
> +              !ops->suspend_late &&
> +              !ops->suspend_noirq &&
> +              !ops->resume_noirq &&
> +              !ops->resume_early &&
> +              !ops->resume &&
> +              !ops->complete;
> +}
> +
> +void device_check_pm_callbacks(struct device *dev)
> +{
> +       spin_lock_irq(&dev->power.lock);
> +       dev->power.no_pm_callbacks =
> +               (!dev->bus || pm_ops_is_empty(dev->bus->pm)) &&
> +               (!dev->class || pm_ops_is_empty(dev->class->pm)) &&
> +               (!dev->type || pm_ops_is_empty(dev->type->pm)) &&
> +               (!dev->pm_domain || pm_ops_is_empty(&dev->pm_domain->ops)) &&
> +               (!dev->driver || pm_ops_is_empty(dev->driver->pm));
> +       spin_unlock_irq(&dev->power.lock);
> +}
> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
> index 998fa6b23084..b3e35e825cd8 100644
> --- a/drivers/base/power/power.h
> +++ b/drivers/base/power/power.h
> @@ -37,6 +37,8 @@ extern void device_wakeup_detach_irq(struct device *dev);
>  extern void device_wakeup_arm_wake_irqs(void);
>  extern void device_wakeup_disarm_wake_irqs(void);
>
> +extern void device_check_pm_callbacks(struct device *dev);
> +
>  #else
>
>  static inline int
> @@ -58,6 +60,10 @@ static inline void device_wakeup_disarm_wake_irqs(void)
>  {
>  }
>
> +static inline void device_check_pm_callbacks(struct device *dev)
> +{
> +}
> +
>  #endif /* CONFIG_PM_SLEEP */
>
>  /*
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index 35d599e7250d..7943acb74ae4 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -573,6 +573,7 @@ struct dev_pm_info {
>         struct wakeup_source    *wakeup;
>         bool                    wakeup_path:1;
>         bool                    syscore:1;
> +       bool                    no_pm_callbacks:1;      /* Owned by the PM core */
>  #else
>         unsigned int            should_wakeup:1;
>  #endif
> --
> 2.4.3
>

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks
  2015-10-02  7:48   ` Ulf Hansson
@ 2015-10-02  8:10     ` Tomeu Vizoso
  2015-10-02  8:40       ` Ulf Hansson
  0 siblings, 1 reply; 11+ messages in thread
From: Tomeu Vizoso @ 2015-10-02  8:10 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: linux-pm, Alan Stern, Rafael J. Wysocki, martyn.welch,
	linux-kernel, Len Brown, Kevin Hilman, Greg Kroah-Hartman,
	Pavel Machek

On 2 October 2015 at 09:48, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> On 2 October 2015 at 09:14, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> If a suitable prepare callback cannot be found for a given device and
>> its driver has no PM callbacks at all, assume that it can go direct to
>> complete when the system goes to sleep.
>>
>> The reason for this is that there's lots of devices in a system that do
>> no PM at all and there's no reason for them to prevent their ancestors
>> to do direct_complete if they can support it.
>>
>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> ---
>>
>> Changes in v8:
>> - Move no_pm_callbacks field into CONFIG_PM_SLEEP
>> - Call device_check_pm_callbacks only after a device is bound or unbound
>
> Devices that don't use a driver, will not get this feature for "free".
> I expect in those cases, they will have to call
> device_check_pm_callbacks() themselves, right?

You are right, but wonder if we shouldn't go back to calling
device_check_pm_callbacks() from device_pm_add() and
dev_pm_domain_set() so they don't have to.

Thanks,

Tomeu

> Perhaps we should mention that, at least in the change log.
>
>>
>> Changes in v7:
>> - Reduce indentation by adding a label in device_prepare()
>>
>> Changes in v6:
>> - Add stub for !CONFIG_PM.
>> - Move implementation of device_check_pm_callbacks to power/main.c as it
>>   doesn't belong to CONFIG_PM_SLEEP.
>> - Take dev->power.lock before modifying flag.
>>
>> Changes in v5:
>> - Check for all dev_pm_ops instances associated to a device, updating a
>>   no_pm_callbacks flag at the times when that could change.
>>
>>  drivers/base/dd.c           |  3 +++
>>  drivers/base/power/domain.c |  2 ++
>>  drivers/base/power/main.c   | 33 +++++++++++++++++++++++++++++++++
>>  drivers/base/power/power.h  |  6 ++++++
>>  include/linux/pm.h          |  1 +
>>  5 files changed, 45 insertions(+)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index f2007fa7316f..2e3809affb06 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -210,6 +210,8 @@ static void driver_bound(struct device *dev)
>>
>>         klist_add_tail(&dev->p->knode_driver, &dev->driver->p->klist_devices);
>>
>> +       device_check_pm_callbacks(dev);
>> +
>>         /*
>>          * Make sure the device is no longer in one of the deferred lists and
>>          * kick off retrying all pending devices
>> @@ -700,6 +702,7 @@ static void __device_release_driver(struct device *dev)
>>                         dev->pm_domain->dismiss(dev);
>>
>>                 klist_remove(&dev->p->knode_driver);
>> +               device_check_pm_callbacks(dev);
>>                 if (dev->bus)
>>                         blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>>                                                      BUS_NOTIFY_UNBOUND_DRIVER,
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index b75d02aa8d93..9dc1ce9c573b 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -20,6 +20,8 @@
>>  #include <linux/suspend.h>
>>  #include <linux/export.h>
>>
>> +#include "power.h"
>> +
>
> Seems like a leftover from the earlier version of the patch...
>
>>  #define GENPD_RETRY_MAX_MS     250             /* Approximate */
>>
>>  #define GENPD_DEV_CALLBACK(genpd, type, callback, dev)         \
>> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
>> index 1710c26ba097..43b44f8251c7 100644
>> --- a/drivers/base/power/main.c
>> +++ b/drivers/base/power/main.c
>> @@ -1569,6 +1569,11 @@ static int device_prepare(struct device *dev, pm_message_t state)
>>
>>         dev->power.wakeup_path = device_may_wakeup(dev);
>>
>> +       if (dev->power.no_pm_callbacks) {
>> +               ret = 1;        /* Let device go direct_complete */
>> +               goto unlock;
>> +       }
>> +
>>         if (dev->pm_domain) {
>>                 info = "preparing power domain ";
>>                 callback = dev->pm_domain->ops.prepare;
>> @@ -1591,6 +1596,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
>>         if (callback)
>>                 ret = callback(dev);
>>
>> +unlock:
>>         device_unlock(dev);
>>
>>         if (ret < 0) {
>> @@ -1719,3 +1725,30 @@ void dpm_for_each_dev(void *data, void (*fn)(struct device *, void *))
>>         device_pm_unlock();
>>  }
>>  EXPORT_SYMBOL_GPL(dpm_for_each_dev);
>> +
>> +static bool pm_ops_is_empty(const struct dev_pm_ops *ops)
>> +{
>> +       if (!ops)
>> +               return true;
>> +
>> +       return !ops->prepare &&
>> +              !ops->suspend &&
>> +              !ops->suspend_late &&
>> +              !ops->suspend_noirq &&
>> +              !ops->resume_noirq &&
>> +              !ops->resume_early &&
>> +              !ops->resume &&
>> +              !ops->complete;
>> +}
>> +
>> +void device_check_pm_callbacks(struct device *dev)
>> +{
>> +       spin_lock_irq(&dev->power.lock);
>> +       dev->power.no_pm_callbacks =
>> +               (!dev->bus || pm_ops_is_empty(dev->bus->pm)) &&
>> +               (!dev->class || pm_ops_is_empty(dev->class->pm)) &&
>> +               (!dev->type || pm_ops_is_empty(dev->type->pm)) &&
>> +               (!dev->pm_domain || pm_ops_is_empty(&dev->pm_domain->ops)) &&
>> +               (!dev->driver || pm_ops_is_empty(dev->driver->pm));
>> +       spin_unlock_irq(&dev->power.lock);
>> +}
>> diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
>> index 998fa6b23084..b3e35e825cd8 100644
>> --- a/drivers/base/power/power.h
>> +++ b/drivers/base/power/power.h
>> @@ -37,6 +37,8 @@ extern void device_wakeup_detach_irq(struct device *dev);
>>  extern void device_wakeup_arm_wake_irqs(void);
>>  extern void device_wakeup_disarm_wake_irqs(void);
>>
>> +extern void device_check_pm_callbacks(struct device *dev);
>> +
>>  #else
>>
>>  static inline int
>> @@ -58,6 +60,10 @@ static inline void device_wakeup_disarm_wake_irqs(void)
>>  {
>>  }
>>
>> +static inline void device_check_pm_callbacks(struct device *dev)
>> +{
>> +}
>> +
>>  #endif /* CONFIG_PM_SLEEP */
>>
>>  /*
>> diff --git a/include/linux/pm.h b/include/linux/pm.h
>> index 35d599e7250d..7943acb74ae4 100644
>> --- a/include/linux/pm.h
>> +++ b/include/linux/pm.h
>> @@ -573,6 +573,7 @@ struct dev_pm_info {
>>         struct wakeup_source    *wakeup;
>>         bool                    wakeup_path:1;
>>         bool                    syscore:1;
>> +       bool                    no_pm_callbacks:1;      /* Owned by the PM core */
>>  #else
>>         unsigned int            should_wakeup:1;
>>  #endif
>> --
>> 2.4.3
>>
>
> Kind regards
> Uffe
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks
  2015-10-02  8:10     ` Tomeu Vizoso
@ 2015-10-02  8:40       ` Ulf Hansson
  2015-10-02 13:42         ` Rafael J. Wysocki
  0 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2015-10-02  8:40 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: linux-pm, Alan Stern, Rafael J. Wysocki, martyn.welch,
	linux-kernel, Len Brown, Kevin Hilman, Greg Kroah-Hartman,
	Pavel Machek

On 2 October 2015 at 10:10, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> On 2 October 2015 at 09:48, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> On 2 October 2015 at 09:14, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>>> If a suitable prepare callback cannot be found for a given device and
>>> its driver has no PM callbacks at all, assume that it can go direct to
>>> complete when the system goes to sleep.
>>>
>>> The reason for this is that there's lots of devices in a system that do
>>> no PM at all and there's no reason for them to prevent their ancestors
>>> to do direct_complete if they can support it.
>>>
>>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>>> ---
>>>
>>> Changes in v8:
>>> - Move no_pm_callbacks field into CONFIG_PM_SLEEP
>>> - Call device_check_pm_callbacks only after a device is bound or unbound
>>
>> Devices that don't use a driver, will not get this feature for "free".
>> I expect in those cases, they will have to call
>> device_check_pm_callbacks() themselves, right?
>
> You are right, but wonder if we shouldn't go back to calling
> device_check_pm_callbacks() from device_pm_add() and
> dev_pm_domain_set() so they don't have to.

That seems reasonable to me, and I didn't quite understand why you
decided to remove it. :-)

Kind regards
Uffe

[...]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks
  2015-10-02  8:40       ` Ulf Hansson
@ 2015-10-02 13:42         ` Rafael J. Wysocki
  2015-10-05 12:21           ` Tomeu Vizoso
  0 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2015-10-02 13:42 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Tomeu Vizoso, linux-pm, Alan Stern, martyn.welch, linux-kernel,
	Len Brown, Kevin Hilman, Greg Kroah-Hartman, Pavel Machek

On Friday, October 02, 2015 10:40:49 AM Ulf Hansson wrote:
> On 2 October 2015 at 10:10, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> > On 2 October 2015 at 09:48, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >> On 2 October 2015 at 09:14, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
> >>> If a suitable prepare callback cannot be found for a given device and
> >>> its driver has no PM callbacks at all, assume that it can go direct to
> >>> complete when the system goes to sleep.
> >>>
> >>> The reason for this is that there's lots of devices in a system that do
> >>> no PM at all and there's no reason for them to prevent their ancestors
> >>> to do direct_complete if they can support it.
> >>>
> >>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> >>> ---
> >>>
> >>> Changes in v8:
> >>> - Move no_pm_callbacks field into CONFIG_PM_SLEEP
> >>> - Call device_check_pm_callbacks only after a device is bound or unbound
> >>
> >> Devices that don't use a driver, will not get this feature for "free".
> >> I expect in those cases, they will have to call
> >> device_check_pm_callbacks() themselves, right?
> >
> > You are right, but wonder if we shouldn't go back to calling
> > device_check_pm_callbacks() from device_pm_add() and
> > dev_pm_domain_set() so they don't have to.
> 
> That seems reasonable to me, and I didn't quite understand why you
> decided to remove it. :-)

Right.

device_check_pm_callbacks() needs to be called on device registration too
(in case the device doesn't have a driver and doesn't use a PM domain) and
it won't hurt to call it in dev_pm_domain_set() either, although that likely
will be redundant in the majority of cases.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 1/4] device core: add device_is_bound()
  2015-10-02  7:14 ` [PATCH v8 1/4] device core: add device_is_bound() Tomeu Vizoso
@ 2015-10-02 13:46   ` Rafael J. Wysocki
  0 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2015-10-02 13:46 UTC (permalink / raw)
  To: Tomeu Vizoso
  Cc: linux-pm, Alan Stern, martyn.welch, Ulf Hansson,
	Greg Kroah-Hartman, linux-kernel

On Friday, October 02, 2015 09:14:28 AM Tomeu Vizoso wrote:
> Adds a function that tells whether a device is already bound to a
> driver.
> 
> This is needed to warn when there is an attempt to change the PM domain
> of a device that has finished probing already. The reason why we want to
> enforce that is because in the general case that can cause problems and
> also that we can simplify code quite a bit if we can always assume that.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
> ---
> 
> Changes in v8:
> - Add device_is_bound()
> 
>  drivers/base/dd.c      | 9 +++++++--
>  include/linux/device.h | 2 ++
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index be0eb4639128..f2007fa7316f 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -192,9 +192,14 @@ static int deferred_probe_initcall(void)
>  }
>  late_initcall(deferred_probe_initcall);
>  
> +bool device_is_bound(struct device *dev)
> +{
> +	return klist_node_attached(&dev->p->knode_driver);
> +}

It would be useful here to add a comment explaining what locks need to be held
around this for it to be non-racy IMO.

> +
>  static void driver_bound(struct device *dev)
>  {
> -	if (klist_node_attached(&dev->p->knode_driver)) {
> +	if (device_is_bound(dev)) {
>  		printk(KERN_WARNING "%s: device %s already bound\n",
>  			__func__, kobject_name(&dev->kobj));
>  		return;
> @@ -545,7 +550,7 @@ static int __device_attach(struct device *dev, bool allow_async)
>  
>  	device_lock(dev);
>  	if (dev->driver) {
> -		if (klist_node_attached(&dev->p->knode_driver)) {
> +		if (device_is_bound(dev)) {
>  			ret = 1;
>  			goto out_unlock;
>  		}
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 5d7bc6349930..9b41c05fb479 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1035,6 +1035,8 @@ extern int __must_check driver_attach(struct device_driver *drv);
>  extern void device_initial_probe(struct device *dev);
>  extern int __must_check device_reprobe(struct device *dev);
>  
> +extern bool device_is_bound(struct device *dev);
> +
>  /*
>   * Easy functions for dynamically creating devices on the fly
>   */
> 

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks
  2015-10-02 13:42         ` Rafael J. Wysocki
@ 2015-10-05 12:21           ` Tomeu Vizoso
  0 siblings, 0 replies; 11+ messages in thread
From: Tomeu Vizoso @ 2015-10-05 12:21 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Ulf Hansson, linux-pm, Alan Stern, martyn.welch, linux-kernel,
	Len Brown, Kevin Hilman, Greg Kroah-Hartman, Pavel Machek

On 2 October 2015 at 15:42, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Friday, October 02, 2015 10:40:49 AM Ulf Hansson wrote:
>> On 2 October 2015 at 10:10, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> > On 2 October 2015 at 09:48, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>> >> On 2 October 2015 at 09:14, Tomeu Vizoso <tomeu.vizoso@collabora.com> wrote:
>> >>> If a suitable prepare callback cannot be found for a given device and
>> >>> its driver has no PM callbacks at all, assume that it can go direct to
>> >>> complete when the system goes to sleep.
>> >>>
>> >>> The reason for this is that there's lots of devices in a system that do
>> >>> no PM at all and there's no reason for them to prevent their ancestors
>> >>> to do direct_complete if they can support it.
>> >>>
>> >>> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
>> >>> ---
>> >>>
>> >>> Changes in v8:
>> >>> - Move no_pm_callbacks field into CONFIG_PM_SLEEP
>> >>> - Call device_check_pm_callbacks only after a device is bound or unbound
>> >>
>> >> Devices that don't use a driver, will not get this feature for "free".
>> >> I expect in those cases, they will have to call
>> >> device_check_pm_callbacks() themselves, right?
>> >
>> > You are right, but wonder if we shouldn't go back to calling
>> > device_check_pm_callbacks() from device_pm_add() and
>> > dev_pm_domain_set() so they don't have to.
>>
>> That seems reasonable to me, and I didn't quite understand why you
>> decided to remove it. :-)
>
> Right.
>
> device_check_pm_callbacks() needs to be called on device registration too
> (in case the device doesn't have a driver and doesn't use a PM domain) and
> it won't hurt to call it in dev_pm_domain_set() either, although that likely
> will be redundant in the majority of cases.

I got it. I misunderstood your comment from the other day.

Thanks,

Tomeu

> Thanks,
> Rafael
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-10-05 12:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-02  7:14 [PATCH v8 0/4] Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso
2015-10-02  7:14 ` [PATCH v8 1/4] device core: add device_is_bound() Tomeu Vizoso
2015-10-02 13:46   ` Rafael J. Wysocki
2015-10-02  7:14 ` [PATCH v8 2/4] PM / Domains: add setter for dev.pm_domain Tomeu Vizoso
2015-10-02  7:14 ` [PATCH v8 3/4] PM / sleep: Go direct_complete if driver has no callbacks Tomeu Vizoso
2015-10-02  7:48   ` Ulf Hansson
2015-10-02  8:10     ` Tomeu Vizoso
2015-10-02  8:40       ` Ulf Hansson
2015-10-02 13:42         ` Rafael J. Wysocki
2015-10-05 12:21           ` Tomeu Vizoso
2015-10-02  7:14 ` [PATCH v8 4/4] USB / PM: Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).