Re: [PATCH v4 3/3] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

2015-07-16 Thread Tomeu Vizoso 
On 16 July 2015 at 02:42, Rafael J. Wysocki r...@rjwysocki.net wrote:
 On Wednesday, July 15, 2015 02:40:08 PM Tomeu Vizoso wrote:
 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.viz...@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))

 Why just enabled and not suspended?

Hmm, the core checks if it's runtime suspended before going
direct_complete, but it's true that the API docs say that it should
return a positive value only if it's runtime suspended.

Is there a reason why the prepare() implementations have to check that
instead of leaving it to the core?

Thanks,

Tomeu

 + 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)


 --
 I speak only for myself.
 Rafael J. Wysocki, Intel Open Source Technology Center.
 --
 To unsubscribe from this list: send the line unsubscribe linux-kernel in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

2015-07-16 Thread Rafael J. Wysocki 
On Thursday, July 16, 2015 02:09:41 PM Tomeu Vizoso wrote:
 On 16 July 2015 at 02:42, Rafael J. Wysocki r...@rjwysocki.net wrote:
  On Wednesday, July 15, 2015 02:40:08 PM Tomeu Vizoso wrote:
  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.viz...@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))
 
  Why just enabled and not suspended?
 
 Hmm, the core checks if it's runtime suspended before going
 direct_complete, but it's true that the API docs say that it should
 return a positive value only if it's runtime suspended.
 
 Is there a reason why the prepare() implementations have to check that
 instead of leaving it to the core?

The concern is that they generally need to check the state of the device
which is meaningless from the suspend suitability (so to speak) perspective
if the device is not (runtime) suspended when the check is made.

But this particular case seems to be exceptional as -prepare doesn't realy
check the device's (hardware) state, but only its software configuration
(if I'm not mistaken).  So checking the RPM status is not needed here.


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 3/3] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

2015-07-15 Thread Tomeu Vizoso 
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.viz...@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

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/3] USB / PM: Allow USB devices to remain runtime-suspended when sleeping

2015-07-15 Thread Rafael J. Wysocki 
On Wednesday, July 15, 2015 02:40:08 PM Tomeu Vizoso wrote:
 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.viz...@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))

Why just enabled and not suspended?

 + 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)
 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html