All of lore.kernel.org
 help / color / mirror / Atom feed
* [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
@ 2015-07-06 18:27 ` Douglas Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Douglas Anderson @ 2015-07-06 18:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Youn, Felipe Balbi
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz, linux-rockchip,
	Douglas Anderson, Petr Mladek, Alan Stern, devicetree,
	Kever Yang, linux-kernel, Mark Rutland, Kumar Gala, Dan Williams,
	Matthew Garrett, Ian Campbell, Rob Herring, Pratyush Anand,
	Pawel Moll, Peter Chen, Gregory Herrero, Robert Schlabbach

This series of patches, together with
<https://patchwork.kernel.org/patch/6652341/> from Chris Zhong and a
dts change allow us to wake up from a USB device on rk3288 boards.
The patches were tested on rk3288-jerry in the chromeos-3.14 kernel.
The chromeos-3.14 kernel tested included a full set of dwc2 backports
from upstream, so this is expected to function upstream once we get
everything setup there.


Douglas Anderson (3):
  USB: Export usb_wakeup_enabled_descendants()
  Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
  USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

 Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++
 drivers/usb/core/hub.c                         |  7 ++--
 drivers/usb/dwc2/core.h                        |  2 ++
 drivers/usb/dwc2/platform.c                    | 45 ++++++++++++++++++++++++--
 include/linux/usb/hcd.h                        |  5 +++
 5 files changed, 58 insertions(+), 5 deletions(-)

-- 
2.4.3.573.g4eafbef


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

* [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
@ 2015-07-06 18:27 ` Douglas Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Douglas Anderson @ 2015-07-06 18:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Youn, Felipe Balbi
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz, linux-rockchip,
	Douglas Anderson, Petr Mladek, Alan Stern, devicetree,
	Kever Yang, linux-kernel, Mark Rutland, Kumar Gala, Dan Williams,
	Matthew Garrett, Ian Campbell, Rob Herring, Pratyush Anand,
	Pawel Moll, Peter Chen, Gregory Herrero, Robert

This series of patches, together with
<https://patchwork.kernel.org/patch/6652341/> from Chris Zhong and a
dts change allow us to wake up from a USB device on rk3288 boards.
The patches were tested on rk3288-jerry in the chromeos-3.14 kernel.
The chromeos-3.14 kernel tested included a full set of dwc2 backports
from upstream, so this is expected to function upstream once we get
everything setup there.


Douglas Anderson (3):
  USB: Export usb_wakeup_enabled_descendants()
  Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
  USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

 Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++
 drivers/usb/core/hub.c                         |  7 ++--
 drivers/usb/dwc2/core.h                        |  2 ++
 drivers/usb/dwc2/platform.c                    | 45 ++++++++++++++++++++++++--
 include/linux/usb/hcd.h                        |  5 +++
 5 files changed, 58 insertions(+), 5 deletions(-)

-- 
2.4.3.573.g4eafbef

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

* [REPOST PATCH 1/3] USB: Export usb_wakeup_enabled_descendants()
@ 2015-07-06 18:27   ` Douglas Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Douglas Anderson @ 2015-07-06 18:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Youn, Felipe Balbi
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz, linux-rockchip,
	Douglas Anderson, Alan Stern, Petr Mladek, Peter Chen,
	Pratyush Anand, Matthew Garrett, Robert Schlabbach, Dan Williams,
	linux-kernel

In (e583d9d USB: global suspend and remote wakeup don't mix) we
introduced wakeup_enabled_descendants() as a static function.  We'd
like to use this function in USB controller drivers to know if we
should keep the controller on during suspend time, since doing so has
a power impact.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/usb/core/hub.c  | 7 ++++---
 include/linux/usb/hcd.h | 5 +++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 43cb2f2..fdc59db 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3034,13 +3034,14 @@ static int usb_disable_remote_wakeup(struct usb_device *udev)
 }
 
 /* Count of wakeup-enabled devices at or below udev */
-static unsigned wakeup_enabled_descendants(struct usb_device *udev)
+unsigned usb_wakeup_enabled_descendants(struct usb_device *udev)
 {
 	struct usb_hub *hub = usb_hub_to_struct_hub(udev);
 
 	return udev->do_remote_wakeup +
 			(hub ? hub->wakeup_enabled_descendants : 0);
 }
+EXPORT_SYMBOL_GPL(usb_wakeup_enabled_descendants);
 
 /*
  * usb_port_suspend - suspend a usb device's upstream port
@@ -3149,7 +3150,7 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 	 * Therefore we will turn on the suspend feature if udev or any of its
 	 * descendants is enabled for remote wakeup.
 	 */
-	else if (PMSG_IS_AUTO(msg) || wakeup_enabled_descendants(udev) > 0)
+	else if (PMSG_IS_AUTO(msg) || usb_wakeup_enabled_descendants(udev) > 0)
 		status = set_port_feature(hub->hdev, port1,
 				USB_PORT_FEAT_SUSPEND);
 	else {
@@ -3548,7 +3549,7 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
 		}
 		if (udev)
 			hub->wakeup_enabled_descendants +=
-					wakeup_enabled_descendants(udev);
+					usb_wakeup_enabled_descendants(udev);
 	}
 
 	if (hdev->do_remote_wakeup && hub->quirk_check_port_auto_suspend) {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..30d74c9 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -626,11 +626,16 @@ extern wait_queue_head_t usb_kill_urb_queue;
 #define usb_endpoint_out(ep_dir)	(!((ep_dir) & USB_DIR_IN))
 
 #ifdef CONFIG_PM
+extern unsigned usb_wakeup_enabled_descendants(struct usb_device *udev);
 extern void usb_root_hub_lost_power(struct usb_device *rhdev);
 extern int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg);
 extern int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg);
 extern void usb_hcd_resume_root_hub(struct usb_hcd *hcd);
 #else
+static inline unsigned usb_wakeup_enabled_descendants(struct usb_device *udev)
+{
+	return 0;
+}
 static inline void usb_hcd_resume_root_hub(struct usb_hcd *hcd)
 {
 	return;
-- 
2.4.3.573.g4eafbef


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

* [REPOST PATCH 1/3] USB: Export usb_wakeup_enabled_descendants()
@ 2015-07-06 18:27   ` Douglas Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Douglas Anderson @ 2015-07-06 18:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Youn, Felipe Balbi
  Cc: Robert Schlabbach, Pratyush Anand, Heiko Stuebner,
	Andrew Bresticker, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Matthew Garrett, Douglas Anderson, Petr Mladek,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Alan Stern,
	lyz-TNX95d0MmH7DzftRWevZcw, Peter Chen, Chris Zhong,
	Julius Werner, Dan Williams, Alexandru Stan

In (e583d9d USB: global suspend and remote wakeup don't mix) we
introduced wakeup_enabled_descendants() as a static function.  We'd
like to use this function in USB controller drivers to know if we
should keep the controller on during suspend time, since doing so has
a power impact.

Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/usb/core/hub.c  | 7 ++++---
 include/linux/usb/hcd.h | 5 +++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 43cb2f2..fdc59db 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3034,13 +3034,14 @@ static int usb_disable_remote_wakeup(struct usb_device *udev)
 }
 
 /* Count of wakeup-enabled devices at or below udev */
-static unsigned wakeup_enabled_descendants(struct usb_device *udev)
+unsigned usb_wakeup_enabled_descendants(struct usb_device *udev)
 {
 	struct usb_hub *hub = usb_hub_to_struct_hub(udev);
 
 	return udev->do_remote_wakeup +
 			(hub ? hub->wakeup_enabled_descendants : 0);
 }
+EXPORT_SYMBOL_GPL(usb_wakeup_enabled_descendants);
 
 /*
  * usb_port_suspend - suspend a usb device's upstream port
@@ -3149,7 +3150,7 @@ int usb_port_suspend(struct usb_device *udev, pm_message_t msg)
 	 * Therefore we will turn on the suspend feature if udev or any of its
 	 * descendants is enabled for remote wakeup.
 	 */
-	else if (PMSG_IS_AUTO(msg) || wakeup_enabled_descendants(udev) > 0)
+	else if (PMSG_IS_AUTO(msg) || usb_wakeup_enabled_descendants(udev) > 0)
 		status = set_port_feature(hub->hdev, port1,
 				USB_PORT_FEAT_SUSPEND);
 	else {
@@ -3548,7 +3549,7 @@ static int hub_suspend(struct usb_interface *intf, pm_message_t msg)
 		}
 		if (udev)
 			hub->wakeup_enabled_descendants +=
-					wakeup_enabled_descendants(udev);
+					usb_wakeup_enabled_descendants(udev);
 	}
 
 	if (hdev->do_remote_wakeup && hub->quirk_check_port_auto_suspend) {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index c9aa779..30d74c9 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -626,11 +626,16 @@ extern wait_queue_head_t usb_kill_urb_queue;
 #define usb_endpoint_out(ep_dir)	(!((ep_dir) & USB_DIR_IN))
 
 #ifdef CONFIG_PM
+extern unsigned usb_wakeup_enabled_descendants(struct usb_device *udev);
 extern void usb_root_hub_lost_power(struct usb_device *rhdev);
 extern int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg);
 extern int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg);
 extern void usb_hcd_resume_root_hub(struct usb_hcd *hcd);
 #else
+static inline unsigned usb_wakeup_enabled_descendants(struct usb_device *udev)
+{
+	return 0;
+}
 static inline void usb_hcd_resume_root_hub(struct usb_hcd *hcd)
 {
 	return;
-- 
2.4.3.573.g4eafbef

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

* [REPOST PATCH 2/3] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
@ 2015-07-06 18:27   ` Douglas Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Douglas Anderson @ 2015-07-06 18:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Youn, Felipe Balbi
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz, linux-rockchip,
	Douglas Anderson, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Kever Yang, Gregory Herrero,
	devicetree, linux-kernel

Some SoCs with a dwc2 USB controller may need to keep the PHY on to
support remote wakeup.  Allow specifying this as a device tree
property.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index fd132cb..84d258d 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -17,6 +17,9 @@ Refer to clk/clock-bindings.txt for generic clock consumer properties
 Optional properties:
 - phys: phy provider specifier
 - phy-names: shall be "usb2-phy"
+- snps,need-phy-for-wake: if present indicates that the phy needs to be left
+  on for remote wakeup during suspend.
+
 Refer to phy/phy-bindings.txt for generic phy consumer properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
@@ -35,4 +38,5 @@ Example:
 		clock-names = "otg";
 		phys = <&usbphy>;
 		phy-names = "usb2-phy";
+		snps,need-phy-for-wake;
         };
-- 
2.4.3.573.g4eafbef


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

* [REPOST PATCH 2/3] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
@ 2015-07-06 18:27   ` Douglas Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Douglas Anderson @ 2015-07-06 18:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Youn, Felipe Balbi
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Chris Zhong, Heiko Stuebner,
	Julius Werner, Andrew Bresticker, Alexandru Stan,
	lyz-TNX95d0MmH7DzftRWevZcw,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Douglas Anderson, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Kever Yang, Gregory Herrero,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Some SoCs with a dwc2 USB controller may need to keep the PHY on to
support remote wakeup.  Allow specifying this as a device tree
property.

Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt b/Documentation/devicetree/bindings/usb/dwc2.txt
index fd132cb..84d258d 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -17,6 +17,9 @@ Refer to clk/clock-bindings.txt for generic clock consumer properties
 Optional properties:
 - phys: phy provider specifier
 - phy-names: shall be "usb2-phy"
+- snps,need-phy-for-wake: if present indicates that the phy needs to be left
+  on for remote wakeup during suspend.
+
 Refer to phy/phy-bindings.txt for generic phy consumer properties
 - dr_mode: shall be one of "host", "peripheral" and "otg"
   Refer to usb/generic.txt
@@ -35,4 +38,5 @@ Example:
 		clock-names = "otg";
 		phys = <&usbphy>;
 		phy-names = "usb2-phy";
+		snps,need-phy-for-wake;
         };
-- 
2.4.3.573.g4eafbef

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

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

* [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 18:27   ` Douglas Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Douglas Anderson @ 2015-07-06 18:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Youn, Felipe Balbi
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz, linux-rockchip,
	Douglas Anderson, linux-kernel

If the 'snps,need-phy-for-wake' is set in the device tree then:

- We know that we can wakeup, so call device_set_wakeup_capable().
  The USB core will use this knowledge to enable wakeup by default.
- We know that we should keep the PHY on during suspend if something
  on our root hub needs remote wakeup.  This requires the patch (USB:
  Export usb_wakeup_enabled_descendants()).  Note that we don't keep
  the PHY on at suspend time if it's not needed because it would be a
  power draw.

If we later find some users of dwc2 that can support wakeup without
keeping the PHY on we may want to add a way to call
device_set_wakeup_capable() without keeping the PHY on at suspend
time.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
 drivers/usb/dwc2/core.h     |  2 ++
 drivers/usb/dwc2/platform.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 53b8de0..b60a1e8 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -687,6 +687,8 @@ struct dwc2_hsotg {
 	enum usb_dr_mode dr_mode;
 	unsigned int hcd_enabled:1;
 	unsigned int gadget_enabled:1;
+	unsigned int need_phy_for_wake:1;
+	unsigned int phy_off_for_suspend:1;
 
 	struct phy *phy;
 	struct usb_phy *uphy;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 9093530..38fce75 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -42,7 +42,9 @@
 #include <linux/of_device.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/usb.h>
 
+#include <linux/usb/hcd.h>
 #include <linux/usb/of.h>
 
 #include "core.h"
@@ -222,6 +224,10 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
 	hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
 
+	hsotg->need_phy_for_wake =
+		of_property_read_bool(dev->dev.of_node,
+				      "snps,need-phy-for-wake");
+
 	/*
 	 * Attempt to find a generic PHY, then look for an old style
 	 * USB PHY
@@ -265,6 +271,14 @@ static int dwc2_driver_probe(struct platform_device *dev)
 		hsotg->gadget_enabled = 1;
 	}
 
+	/*
+	 * If we need PHY for wakeup we must be wakeup capable.
+	 * When we have a device that can wake without the PHY we
+	 * can adjust this condition.
+	 */
+	if (hsotg->need_phy_for_wake)
+		device_set_wakeup_capable(&dev->dev, true);
+
 	if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
 		retval = dwc2_hcd_init(hsotg, irq);
 		if (retval) {
@@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
 	return retval;
 }
 
+static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
+{
+	struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;
+
+	if (dwc2->lx_state == DWC2_L0)
+		return false;
+
+	/* If the controller isn't allowed to wakeup then we can power off. */
+	if (!device_may_wakeup(dwc2->dev))
+		return true;
+
+	/*
+	 * We don't want to power off the PHY if something under the
+	 * root hub has wakeup enabled.
+	 */
+	if (usb_wakeup_enabled_descendants(root_hub))
+		return false;
+
+	/* No reason to keep the PHY powered, so allow poweroff */
+	return true;
+}
+
 static int __maybe_unused dwc2_suspend(struct device *dev)
 {
 	struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
@@ -290,8 +326,10 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
 	if (dwc2_is_device_mode(dwc2)) {
 		ret = s3c_hsotg_suspend(dwc2);
 	} else {
-		if (dwc2->lx_state == DWC2_L0)
+		if (!dwc2_can_poweroff_phy(dwc2))
 			return 0;
+
+		dwc2->phy_off_for_suspend = true;
 		phy_exit(dwc2->phy);
 		phy_power_off(dwc2->phy);
 
@@ -307,9 +345,12 @@ static int __maybe_unused dwc2_resume(struct device *dev)
 	if (dwc2_is_device_mode(dwc2)) {
 		ret = s3c_hsotg_resume(dwc2);
 	} else {
+		if (!dwc2->phy_off_for_suspend)
+			return ret;
+
 		phy_power_on(dwc2->phy);
 		phy_init(dwc2->phy);
-
+		dwc2->phy_off_for_suspend = false;
 	}
 	return ret;
 }
-- 
2.4.3.573.g4eafbef


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

* [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 18:27   ` Douglas Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Douglas Anderson @ 2015-07-06 18:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman, John Youn, Felipe Balbi
  Cc: Heiko Stuebner, Andrew Bresticker,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Douglas Anderson,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	lyz-TNX95d0MmH7DzftRWevZcw, Chris Zhong, Julius Werner,
	Alexandru Stan

If the 'snps,need-phy-for-wake' is set in the device tree then:

- We know that we can wakeup, so call device_set_wakeup_capable().
  The USB core will use this knowledge to enable wakeup by default.
- We know that we should keep the PHY on during suspend if something
  on our root hub needs remote wakeup.  This requires the patch (USB:
  Export usb_wakeup_enabled_descendants()).  Note that we don't keep
  the PHY on at suspend time if it's not needed because it would be a
  power draw.

If we later find some users of dwc2 that can support wakeup without
keeping the PHY on we may want to add a way to call
device_set_wakeup_capable() without keeping the PHY on at suspend
time.

Signed-off-by: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Signed-off-by: Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
 drivers/usb/dwc2/core.h     |  2 ++
 drivers/usb/dwc2/platform.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 53b8de0..b60a1e8 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -687,6 +687,8 @@ struct dwc2_hsotg {
 	enum usb_dr_mode dr_mode;
 	unsigned int hcd_enabled:1;
 	unsigned int gadget_enabled:1;
+	unsigned int need_phy_for_wake:1;
+	unsigned int phy_off_for_suspend:1;
 
 	struct phy *phy;
 	struct usb_phy *uphy;
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 9093530..38fce75 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -42,7 +42,9 @@
 #include <linux/of_device.h>
 #include <linux/mutex.h>
 #include <linux/platform_device.h>
+#include <linux/usb.h>
 
+#include <linux/usb/hcd.h>
 #include <linux/usb/of.h>
 
 #include "core.h"
@@ -222,6 +224,10 @@ static int dwc2_driver_probe(struct platform_device *dev)
 
 	hsotg->dr_mode = of_usb_get_dr_mode(dev->dev.of_node);
 
+	hsotg->need_phy_for_wake =
+		of_property_read_bool(dev->dev.of_node,
+				      "snps,need-phy-for-wake");
+
 	/*
 	 * Attempt to find a generic PHY, then look for an old style
 	 * USB PHY
@@ -265,6 +271,14 @@ static int dwc2_driver_probe(struct platform_device *dev)
 		hsotg->gadget_enabled = 1;
 	}
 
+	/*
+	 * If we need PHY for wakeup we must be wakeup capable.
+	 * When we have a device that can wake without the PHY we
+	 * can adjust this condition.
+	 */
+	if (hsotg->need_phy_for_wake)
+		device_set_wakeup_capable(&dev->dev, true);
+
 	if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) {
 		retval = dwc2_hcd_init(hsotg, irq);
 		if (retval) {
@@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
 	return retval;
 }
 
+static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
+{
+	struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;
+
+	if (dwc2->lx_state == DWC2_L0)
+		return false;
+
+	/* If the controller isn't allowed to wakeup then we can power off. */
+	if (!device_may_wakeup(dwc2->dev))
+		return true;
+
+	/*
+	 * We don't want to power off the PHY if something under the
+	 * root hub has wakeup enabled.
+	 */
+	if (usb_wakeup_enabled_descendants(root_hub))
+		return false;
+
+	/* No reason to keep the PHY powered, so allow poweroff */
+	return true;
+}
+
 static int __maybe_unused dwc2_suspend(struct device *dev)
 {
 	struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
@@ -290,8 +326,10 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
 	if (dwc2_is_device_mode(dwc2)) {
 		ret = s3c_hsotg_suspend(dwc2);
 	} else {
-		if (dwc2->lx_state == DWC2_L0)
+		if (!dwc2_can_poweroff_phy(dwc2))
 			return 0;
+
+		dwc2->phy_off_for_suspend = true;
 		phy_exit(dwc2->phy);
 		phy_power_off(dwc2->phy);
 
@@ -307,9 +345,12 @@ static int __maybe_unused dwc2_resume(struct device *dev)
 	if (dwc2_is_device_mode(dwc2)) {
 		ret = s3c_hsotg_resume(dwc2);
 	} else {
+		if (!dwc2->phy_off_for_suspend)
+			return ret;
+
 		phy_power_on(dwc2->phy);
 		phy_init(dwc2->phy);
-
+		dwc2->phy_off_for_suspend = false;
 	}
 	return ret;
 }
-- 
2.4.3.573.g4eafbef

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 18:34     ` Felipe Balbi
  0 siblings, 0 replies; 35+ messages in thread
From: Felipe Balbi @ 2015-07-06 18:34 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Greg Kroah-Hartman, John Youn, Felipe Balbi, linux-usb,
	Chris Zhong, Heiko Stuebner, Julius Werner, Andrew Bresticker,
	Alexandru Stan, lyz, linux-rockchip, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Hi,

On Mon, Jul 06, 2015 at 11:27:04AM -0700, Douglas Anderson wrote:
> @@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
>  	return retval;
>  }
>  
> +static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
> +{
> +	struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;

what happens on peripheral only builds ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 18:34     ` Felipe Balbi
  0 siblings, 0 replies; 35+ messages in thread
From: Felipe Balbi @ 2015-07-06 18:34 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Heiko Stuebner, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	John Youn, Greg Kroah-Hartman, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi,
	Andrew Bresticker, lyz-TNX95d0MmH7DzftRWevZcw, Chris Zhong,
	Julius Werner, Alexandru Stan


[-- Attachment #1.1: Type: text/plain, Size: 404 bytes --]

Hi,

On Mon, Jul 06, 2015 at 11:27:04AM -0700, Douglas Anderson wrote:
> @@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
>  	return retval;
>  }
>  
> +static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
> +{
> +	struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;

what happens on peripheral only builds ?

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 18:58     ` Alan Stern
  0 siblings, 0 replies; 35+ messages in thread
From: Alan Stern @ 2015-07-06 18:58 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Greg Kroah-Hartman, John Youn, Felipe Balbi, linux-usb,
	Chris Zhong, Heiko Stuebner, Julius Werner, Andrew Bresticker,
	Alexandru Stan, lyz, linux-rockchip, linux-kernel

On Mon, 6 Jul 2015, Douglas Anderson wrote:

> If the 'snps,need-phy-for-wake' is set in the device tree then:
> 
> - We know that we can wakeup, so call device_set_wakeup_capable().
>   The USB core will use this knowledge to enable wakeup by default.
> - We know that we should keep the PHY on during suspend if something
>   on our root hub needs remote wakeup.  This requires the patch (USB:
>   Export usb_wakeup_enabled_descendants()).  Note that we don't keep
>   the PHY on at suspend time if it's not needed because it would be a
>   power draw.

You know, this is the first time I've run across this optimization.

In principle it applies to any USB host controller, not just to PHYs.  
There's no reason to enable wakeup for a controller if none of the 
attached devices can issue a wakeup request.

I don't know if implementing this in other HCDs would save any power.  
Any ideas?

Alan Stern


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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 18:58     ` Alan Stern
  0 siblings, 0 replies; 35+ messages in thread
From: Alan Stern @ 2015-07-06 18:58 UTC (permalink / raw)
  To: Douglas Anderson
  Cc: Greg Kroah-Hartman, John Youn, Felipe Balbi,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Chris Zhong, Heiko Stuebner,
	Julius Werner, Andrew Bresticker, Alexandru Stan,
	lyz-TNX95d0MmH7DzftRWevZcw,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On Mon, 6 Jul 2015, Douglas Anderson wrote:

> If the 'snps,need-phy-for-wake' is set in the device tree then:
> 
> - We know that we can wakeup, so call device_set_wakeup_capable().
>   The USB core will use this knowledge to enable wakeup by default.
> - We know that we should keep the PHY on during suspend if something
>   on our root hub needs remote wakeup.  This requires the patch (USB:
>   Export usb_wakeup_enabled_descendants()).  Note that we don't keep
>   the PHY on at suspend time if it's not needed because it would be a
>   power draw.

You know, this is the first time I've run across this optimization.

In principle it applies to any USB host controller, not just to PHYs.  
There's no reason to enable wakeup for a controller if none of the 
attached devices can issue a wakeup request.

I don't know if implementing this in other HCDs would save any power.  
Any ideas?

Alan Stern

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

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 19:02       ` Felipe Balbi
  0 siblings, 0 replies; 35+ messages in thread
From: Felipe Balbi @ 2015-07-06 19:02 UTC (permalink / raw)
  To: Alan Stern
  Cc: Douglas Anderson, Greg Kroah-Hartman, John Youn, Felipe Balbi,
	linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz, linux-rockchip,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On Mon, Jul 06, 2015 at 02:58:16PM -0400, Alan Stern wrote:
> On Mon, 6 Jul 2015, Douglas Anderson wrote:
> 
> > If the 'snps,need-phy-for-wake' is set in the device tree then:
> > 
> > - We know that we can wakeup, so call device_set_wakeup_capable().
> >   The USB core will use this knowledge to enable wakeup by default.
> > - We know that we should keep the PHY on during suspend if something
> >   on our root hub needs remote wakeup.  This requires the patch (USB:
> >   Export usb_wakeup_enabled_descendants()).  Note that we don't keep
> >   the PHY on at suspend time if it's not needed because it would be a
> >   power draw.
> 
> You know, this is the first time I've run across this optimization.
> 
> In principle it applies to any USB host controller, not just to PHYs.  
> There's no reason to enable wakeup for a controller if none of the 
> attached devices can issue a wakeup request.
> 
> I don't know if implementing this in other HCDs would save any power.  
> Any ideas?

most likely it would. Enabling wakeup usually boils down to keeping a
tiny part of the controller (or PHY) powered up. Sometimes that lies in
an always-on power domain, so there would be no difference.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 19:02       ` Felipe Balbi
  0 siblings, 0 replies; 35+ messages in thread
From: Felipe Balbi @ 2015-07-06 19:02 UTC (permalink / raw)
  To: Alan Stern
  Cc: Heiko Stuebner, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	John Youn, Greg Kroah-Hartman, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Douglas Anderson, Felipe Balbi,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Bresticker,
	lyz-TNX95d0MmH7DzftRWevZcw, Chris Zhong, Julius Werner,
	Alexandru Stan


[-- Attachment #1.1: Type: text/plain, Size: 1246 bytes --]

On Mon, Jul 06, 2015 at 02:58:16PM -0400, Alan Stern wrote:
> On Mon, 6 Jul 2015, Douglas Anderson wrote:
> 
> > If the 'snps,need-phy-for-wake' is set in the device tree then:
> > 
> > - We know that we can wakeup, so call device_set_wakeup_capable().
> >   The USB core will use this knowledge to enable wakeup by default.
> > - We know that we should keep the PHY on during suspend if something
> >   on our root hub needs remote wakeup.  This requires the patch (USB:
> >   Export usb_wakeup_enabled_descendants()).  Note that we don't keep
> >   the PHY on at suspend time if it's not needed because it would be a
> >   power draw.
> 
> You know, this is the first time I've run across this optimization.
> 
> In principle it applies to any USB host controller, not just to PHYs.  
> There's no reason to enable wakeup for a controller if none of the 
> attached devices can issue a wakeup request.
> 
> I don't know if implementing this in other HCDs would save any power.  
> Any ideas?

most likely it would. Enabling wakeup usually boils down to keeping a
tiny part of the controller (or PHY) powered up. Sometimes that lies in
an always-on power domain, so there would be no difference.

cheers

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 19:32       ` Doug Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Doug Anderson @ 2015-07-06 19:32 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Greg Kroah-Hartman, John Youn, linux-usb, Chris Zhong,
	Heiko Stuebner, Julius Werner, Andrew Bresticker, Alexandru Stan,
	lyz, open list:ARM/Rockchip SoC...,
	linux-kernel

Felipe,

On Mon, Jul 6, 2015 at 11:34 AM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Mon, Jul 06, 2015 at 11:27:04AM -0700, Douglas Anderson wrote:
>> @@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
>>       return retval;
>>  }
>>
>> +static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
>> +{
>> +     struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;
>
> what happens on peripheral only builds ?

The function is only called on if "dwc2_is_device_mode(dwc2)" returns
false.  I think that means we're safe, right?

-Doug

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 19:32       ` Doug Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Doug Anderson @ 2015-07-06 19:32 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Greg Kroah-Hartman, John Youn, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Chris Zhong, Heiko Stuebner, Julius Werner, Andrew Bresticker,
	Alexandru Stan, lyz, open list:ARM/Rockchip SoC...,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Felipe,

On Mon, Jul 6, 2015 at 11:34 AM, Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> wrote:
> Hi,
>
> On Mon, Jul 06, 2015 at 11:27:04AM -0700, Douglas Anderson wrote:
>> @@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
>>       return retval;
>>  }
>>
>> +static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
>> +{
>> +     struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;
>
> what happens on peripheral only builds ?

The function is only called on if "dwc2_is_device_mode(dwc2)" returns
false.  I think that means we're safe, right?

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

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 19:35         ` Felipe Balbi
  0 siblings, 0 replies; 35+ messages in thread
From: Felipe Balbi @ 2015-07-06 19:35 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Felipe Balbi, Greg Kroah-Hartman, John Youn, linux-usb,
	Chris Zhong, Heiko Stuebner, Julius Werner, Andrew Bresticker,
	Alexandru Stan, lyz, open list:ARM/Rockchip SoC...,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 768 bytes --]

On Mon, Jul 06, 2015 at 12:32:56PM -0700, Doug Anderson wrote:
> Felipe,
> 
> On Mon, Jul 6, 2015 at 11:34 AM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Mon, Jul 06, 2015 at 11:27:04AM -0700, Douglas Anderson wrote:
> >> @@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
> >>       return retval;
> >>  }
> >>
> >> +static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
> >> +{
> >> +     struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;
> >
> > what happens on peripheral only builds ?
> 
> The function is only called on if "dwc2_is_device_mode(dwc2)" returns
> false.  I think that means we're safe, right?

heh, missed that. Should be safe, yeah.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 19:35         ` Felipe Balbi
  0 siblings, 0 replies; 35+ messages in thread
From: Felipe Balbi @ 2015-07-06 19:35 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Heiko Stuebner, open list:ARM/Rockchip SoC...,
	John Youn, Greg Kroah-Hartman, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Felipe Balbi,
	Andrew Bresticker, lyz, Chris Zhong, Julius Werner,
	Alexandru Stan


[-- Attachment #1.1: Type: text/plain, Size: 790 bytes --]

On Mon, Jul 06, 2015 at 12:32:56PM -0700, Doug Anderson wrote:
> Felipe,
> 
> On Mon, Jul 6, 2015 at 11:34 AM, Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> wrote:
> > Hi,
> >
> > On Mon, Jul 06, 2015 at 11:27:04AM -0700, Douglas Anderson wrote:
> >> @@ -282,6 +296,28 @@ static int dwc2_driver_probe(struct platform_device *dev)
> >>       return retval;
> >>  }
> >>
> >> +static bool __maybe_unused dwc2_can_poweroff_phy(struct dwc2_hsotg *dwc2)
> >> +{
> >> +     struct usb_device *root_hub = dwc2_hsotg_to_hcd(dwc2)->self.root_hub;
> >
> > what happens on peripheral only builds ?
> 
> The function is only called on if "dwc2_is_device_mode(dwc2)" returns
> false.  I think that means we're safe, right?

heh, missed that. Should be safe, yeah.

-- 
balbi

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 19:39         ` Doug Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Doug Anderson @ 2015-07-06 19:39 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Alan Stern, Greg Kroah-Hartman, John Youn, linux-usb,
	Chris Zhong, Heiko Stuebner, Julius Werner, Andrew Bresticker,
	Alexandru Stan, lyz, open list:ARM/Rockchip SoC...,
	linux-kernel

Hi,

On Mon, Jul 6, 2015 at 12:02 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Mon, Jul 06, 2015 at 02:58:16PM -0400, Alan Stern wrote:
>> On Mon, 6 Jul 2015, Douglas Anderson wrote:
>>
>> > If the 'snps,need-phy-for-wake' is set in the device tree then:
>> >
>> > - We know that we can wakeup, so call device_set_wakeup_capable().
>> >   The USB core will use this knowledge to enable wakeup by default.
>> > - We know that we should keep the PHY on during suspend if something
>> >   on our root hub needs remote wakeup.  This requires the patch (USB:
>> >   Export usb_wakeup_enabled_descendants()).  Note that we don't keep
>> >   the PHY on at suspend time if it's not needed because it would be a
>> >   power draw.
>>
>> You know, this is the first time I've run across this optimization.
>>
>> In principle it applies to any USB host controller, not just to PHYs.
>> There's no reason to enable wakeup for a controller if none of the
>> attached devices can issue a wakeup request.
>>
>> I don't know if implementing this in other HCDs would save any power.
>> Any ideas?
>
> most likely it would. Enabling wakeup usually boils down to keeping a
> tiny part of the controller (or PHY) powered up. Sometimes that lies in
> an always-on power domain, so there would be no difference.

As per Andrew Bresticker (CCed on this email), the optimization made
sense in Tegra.  If you're willing to look into the chromeos-3.10
tree, you can see that Andrew added a usb_port_may_wakeup() call in
<https://chromium-review.googlesource.com/196593>.  He then used it in
the tegra XHCI driver in
<https://chromium-review.googlesource.com/196594>.  Recently I talked
to Andrew and he indicated that rather than add usb_port_may_wakeup()
like he did it probably made sense to just export
usb_wakeup_enabled_descendants().

-Doug

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-06 19:39         ` Doug Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Doug Anderson @ 2015-07-06 19:39 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Alan Stern, Greg Kroah-Hartman, John Youn,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Chris Zhong, Heiko Stuebner,
	Julius Werner, Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Hi,

On Mon, Jul 6, 2015 at 12:02 PM, Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org> wrote:
> On Mon, Jul 06, 2015 at 02:58:16PM -0400, Alan Stern wrote:
>> On Mon, 6 Jul 2015, Douglas Anderson wrote:
>>
>> > If the 'snps,need-phy-for-wake' is set in the device tree then:
>> >
>> > - We know that we can wakeup, so call device_set_wakeup_capable().
>> >   The USB core will use this knowledge to enable wakeup by default.
>> > - We know that we should keep the PHY on during suspend if something
>> >   on our root hub needs remote wakeup.  This requires the patch (USB:
>> >   Export usb_wakeup_enabled_descendants()).  Note that we don't keep
>> >   the PHY on at suspend time if it's not needed because it would be a
>> >   power draw.
>>
>> You know, this is the first time I've run across this optimization.
>>
>> In principle it applies to any USB host controller, not just to PHYs.
>> There's no reason to enable wakeup for a controller if none of the
>> attached devices can issue a wakeup request.
>>
>> I don't know if implementing this in other HCDs would save any power.
>> Any ideas?
>
> most likely it would. Enabling wakeup usually boils down to keeping a
> tiny part of the controller (or PHY) powered up. Sometimes that lies in
> an always-on power domain, so there would be no difference.

As per Andrew Bresticker (CCed on this email), the optimization made
sense in Tegra.  If you're willing to look into the chromeos-3.10
tree, you can see that Andrew added a usb_port_may_wakeup() call in
<https://chromium-review.googlesource.com/196593>.  He then used it in
the tegra XHCI driver in
<https://chromium-review.googlesource.com/196594>.  Recently I talked
to Andrew and he indicated that rather than add usb_port_may_wakeup()
like he did it probably made sense to just export
usb_wakeup_enabled_descendants().

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

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-07 14:28         ` Alan Stern
  0 siblings, 0 replies; 35+ messages in thread
From: Alan Stern @ 2015-07-07 14:28 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Douglas Anderson, Greg Kroah-Hartman, John Youn, linux-usb,
	Chris Zhong, Heiko Stuebner, Julius Werner, Andrew Bresticker,
	Alexandru Stan, lyz, linux-rockchip, linux-kernel

On Mon, 6 Jul 2015, Felipe Balbi wrote:

> > You know, this is the first time I've run across this optimization.
> > 
> > In principle it applies to any USB host controller, not just to PHYs.  
> > There's no reason to enable wakeup for a controller if none of the 
> > attached devices can issue a wakeup request.
> > 
> > I don't know if implementing this in other HCDs would save any power.  
> > Any ideas?
> 
> most likely it would. Enabling wakeup usually boils down to keeping a
> tiny part of the controller (or PHY) powered up. Sometimes that lies in
> an always-on power domain, so there would be no difference.

Doug, how would you feel about reworking the patch that exports
usb_wakeup_enabled_descendants()?  Instead of doing it that way, create
and export a new subroutine in hcd.c called
usb_hcd_wakeup_not_needed(), or something similar.

The idea is that a host controller driver can do something like this:

	do_wakeup = device_may_wakeup(...);
	if (usb_hcd_wakeup_not_needed(hcd))
		do_wakeup = false;

It encapsulates what you want in a form that can easily be used by 
every HCD.  We can then add this call into the HCDs, over time.

(Merging a change like this would be a challenge.  I guess Felipe would
have to put it in a separate branch which Greg could pull, or vice
versa, so that the new routine would be available to patches submitted
to either maintainer.)

Alan Stern


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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-07 14:28         ` Alan Stern
  0 siblings, 0 replies; 35+ messages in thread
From: Alan Stern @ 2015-07-07 14:28 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Heiko Stuebner, linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	John Youn, Greg Kroah-Hartman, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Douglas Anderson, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Andrew Bresticker, lyz-TNX95d0MmH7DzftRWevZcw, Chris Zhong,
	Julius Werner, Alexandru Stan

On Mon, 6 Jul 2015, Felipe Balbi wrote:

> > You know, this is the first time I've run across this optimization.
> > 
> > In principle it applies to any USB host controller, not just to PHYs.  
> > There's no reason to enable wakeup for a controller if none of the 
> > attached devices can issue a wakeup request.
> > 
> > I don't know if implementing this in other HCDs would save any power.  
> > Any ideas?
> 
> most likely it would. Enabling wakeup usually boils down to keeping a
> tiny part of the controller (or PHY) powered up. Sometimes that lies in
> an always-on power domain, so there would be no difference.

Doug, how would you feel about reworking the patch that exports
usb_wakeup_enabled_descendants()?  Instead of doing it that way, create
and export a new subroutine in hcd.c called
usb_hcd_wakeup_not_needed(), or something similar.

The idea is that a host controller driver can do something like this:

	do_wakeup = device_may_wakeup(...);
	if (usb_hcd_wakeup_not_needed(hcd))
		do_wakeup = false;

It encapsulates what you want in a form that can easily be used by 
every HCD.  We can then add this call into the HCDs, over time.

(Merging a change like this would be a challenge.  I guess Felipe would
have to put it in a separate branch which Greg could pull, or vice
versa, so that the new routine would be available to patches submitted
to either maintainer.)

Alan Stern

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-08  0:06           ` Julius Werner
  0 siblings, 0 replies; 35+ messages in thread
From: Julius Werner @ 2015-07-08  0:06 UTC (permalink / raw)
  To: Alan Stern
  Cc: Felipe Balbi, Douglas Anderson, Greg Kroah-Hartman, John Youn,
	linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	LKML

> Doug, how would you feel about reworking the patch that exports
> usb_wakeup_enabled_descendants()?  Instead of doing it that way, create
> and export a new subroutine in hcd.c called
> usb_hcd_wakeup_not_needed(), or something similar.

We have a use case with another host controller (Tegra, which I think
is still in the process of being upstreamed) where we are able to
power down PHYs (and thus reduce power consumption) per port. I think
we should prefer the more flexible 'number of wake devices in subtree'
interface to be able to support cases like that. (And for the simple
case, 'if (usb_hcd_wakeup_not_needed(hcd))' and 'if
(!usb_wakeup_enabled_descendants(hcd->self.root_hub))' look pretty
similar anyway.)

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-08  0:06           ` Julius Werner
  0 siblings, 0 replies; 35+ messages in thread
From: Julius Werner @ 2015-07-08  0:06 UTC (permalink / raw)
  To: Alan Stern
  Cc: Felipe Balbi, Douglas Anderson, Greg Kroah-Hartman, John Youn,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Chris Zhong, Heiko Stuebner,
	Julius Werner, Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	LKML

> Doug, how would you feel about reworking the patch that exports
> usb_wakeup_enabled_descendants()?  Instead of doing it that way, create
> and export a new subroutine in hcd.c called
> usb_hcd_wakeup_not_needed(), or something similar.

We have a use case with another host controller (Tegra, which I think
is still in the process of being upstreamed) where we are able to
power down PHYs (and thus reduce power consumption) per port. I think
we should prefer the more flexible 'number of wake devices in subtree'
interface to be able to support cases like that. (And for the simple
case, 'if (usb_hcd_wakeup_not_needed(hcd))' and 'if
(!usb_wakeup_enabled_descendants(hcd->self.root_hub))' look pretty
similar anyway.)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
  2015-07-08  0:06           ` Julius Werner
  (?)
@ 2015-07-08 15:01           ` Alan Stern
  2015-07-08 19:41               ` Julius Werner
  -1 siblings, 1 reply; 35+ messages in thread
From: Alan Stern @ 2015-07-08 15:01 UTC (permalink / raw)
  To: Julius Werner
  Cc: Felipe Balbi, Douglas Anderson, Greg Kroah-Hartman, John Youn,
	linux-usb, Chris Zhong, Heiko Stuebner, Andrew Bresticker,
	Alexandru Stan, lyz, open list:ARM/Rockchip SoC...,
	LKML

On Tue, 7 Jul 2015, Julius Werner wrote:

> > Doug, how would you feel about reworking the patch that exports
> > usb_wakeup_enabled_descendants()?  Instead of doing it that way, create
> > and export a new subroutine in hcd.c called
> > usb_hcd_wakeup_not_needed(), or something similar.
> 
> We have a use case with another host controller (Tegra, which I think
> is still in the process of being upstreamed) where we are able to
> power down PHYs (and thus reduce power consumption) per port. I think
> we should prefer the more flexible 'number of wake devices in subtree'
> interface to be able to support cases like that. (And for the simple
> case, 'if (usb_hcd_wakeup_not_needed(hcd))' and 'if
> (!usb_wakeup_enabled_descendants(hcd->self.root_hub))' look pretty
> similar anyway.)

Okay, that's a good point.

But I don't see how you will make it work when the root hub itself is
not enabled for wakeup and a non-hub device plugged into one of the
root hub's ports is enabled.

It seems like you would need a usb_hcd_wakeup_not_needed(hcd, port) 
subroutine.

Alan Stern


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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-08 19:41               ` Julius Werner
  0 siblings, 0 replies; 35+ messages in thread
From: Julius Werner @ 2015-07-08 19:41 UTC (permalink / raw)
  To: Alan Stern
  Cc: Julius Werner, Felipe Balbi, Douglas Anderson,
	Greg Kroah-Hartman, John Youn, linux-usb, Chris Zhong,
	Heiko Stuebner, Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	LKML

> But I don't see how you will make it work when the root hub itself is
> not enabled for wakeup and a non-hub device plugged into one of the
> root hub's ports is enabled.
>
> It seems like you would need a usb_hcd_wakeup_not_needed(hcd, port)
> subroutine.

We'd just put that in the Tegra platform driver, I guess. Iterate over
all ports, call usb_wakeup_enabled_descendants() on the connected
device (if any) and disable that port's PHY if it returns 0 or wasn't
connected. Since usb_wakeup_enabled_descendants() also counts
do_remote_wakeup from the device itself and is safe to call even on
non-hubs, that should work for all cases.

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-08 19:41               ` Julius Werner
  0 siblings, 0 replies; 35+ messages in thread
From: Julius Werner @ 2015-07-08 19:41 UTC (permalink / raw)
  To: Alan Stern
  Cc: Julius Werner, Felipe Balbi, Douglas Anderson,
	Greg Kroah-Hartman, John Youn, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Chris Zhong, Heiko Stuebner, Andrew Bresticker, Alexandru Stan,
	lyz, open list:ARM/Rockchip SoC...,
	LKML

> But I don't see how you will make it work when the root hub itself is
> not enabled for wakeup and a non-hub device plugged into one of the
> root hub's ports is enabled.
>
> It seems like you would need a usb_hcd_wakeup_not_needed(hcd, port)
> subroutine.

We'd just put that in the Tegra platform driver, I guess. Iterate over
all ports, call usb_wakeup_enabled_descendants() on the connected
device (if any) and disable that port's PHY if it returns 0 or wasn't
connected. Since usb_wakeup_enabled_descendants() also counts
do_remote_wakeup from the device itself and is safe to call even on
non-hubs, that should work for all cases.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-08 19:58                 ` Alan Stern
  0 siblings, 0 replies; 35+ messages in thread
From: Alan Stern @ 2015-07-08 19:58 UTC (permalink / raw)
  To: Julius Werner
  Cc: Felipe Balbi, Douglas Anderson, Greg Kroah-Hartman, John Youn,
	linux-usb, Chris Zhong, Heiko Stuebner, Andrew Bresticker,
	Alexandru Stan, lyz, open list:ARM/Rockchip SoC...,
	LKML

On Wed, 8 Jul 2015, Julius Werner wrote:

> > But I don't see how you will make it work when the root hub itself is
> > not enabled for wakeup and a non-hub device plugged into one of the
> > root hub's ports is enabled.
> >
> > It seems like you would need a usb_hcd_wakeup_not_needed(hcd, port)
> > subroutine.
> 
> We'd just put that in the Tegra platform driver, I guess. Iterate over
> all ports, call usb_wakeup_enabled_descendants() on the connected
> device (if any) and disable that port's PHY if it returns 0 or wasn't
> connected. Since usb_wakeup_enabled_descendants() also counts
> do_remote_wakeup from the device itself and is safe to call even on
> non-hubs, that should work for all cases.

All right, that seems reasonable.  But remember not to do this if 
wakeup is enabled on the root hub -- in that case all the PHYs must 
remain powered because unplugging and plugging are both wakeup events.

Alan Stern


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

* Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
@ 2015-07-08 19:58                 ` Alan Stern
  0 siblings, 0 replies; 35+ messages in thread
From: Alan Stern @ 2015-07-08 19:58 UTC (permalink / raw)
  To: Julius Werner
  Cc: Felipe Balbi, Douglas Anderson, Greg Kroah-Hartman, John Youn,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Chris Zhong, Heiko Stuebner,
	Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	LKML

On Wed, 8 Jul 2015, Julius Werner wrote:

> > But I don't see how you will make it work when the root hub itself is
> > not enabled for wakeup and a non-hub device plugged into one of the
> > root hub's ports is enabled.
> >
> > It seems like you would need a usb_hcd_wakeup_not_needed(hcd, port)
> > subroutine.
> 
> We'd just put that in the Tegra platform driver, I guess. Iterate over
> all ports, call usb_wakeup_enabled_descendants() on the connected
> device (if any) and disable that port's PHY if it returns 0 or wasn't
> connected. Since usb_wakeup_enabled_descendants() also counts
> do_remote_wakeup from the device itself and is safe to call even on
> non-hubs, that should work for all cases.

All right, that seems reasonable.  But remember not to do this if 
wakeup is enabled on the root hub -- in that case all the PHYs must 
remain powered because unplugging and plugging are both wakeup events.

Alan Stern

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

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

* Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
@ 2015-10-21 16:23   ` Doug Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Doug Anderson @ 2015-10-21 16:23 UTC (permalink / raw)
  To: John Youn
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	Douglas Anderson, Petr Mladek, Alan Stern, devicetree,
	Kever Yang, linux-kernel, Mark Rutland, Kumar Gala, Dan Williams,
	Matthew Garrett, Ian Campbell, Rob Herring, Pratyush Anand,
	Pawel Moll, Peter Chen, Gregory Herrero, Robert Schlabbach,
	Felipe Balbi, Greg Kroah-Hartman

John,

On Mon, Jul 6, 2015 at 11:27 AM, Douglas Anderson <dianders@chromium.org> wrote:
> This series of patches, together with
> <https://patchwork.kernel.org/patch/6652341/> from Chris Zhong and a
> dts change allow us to wake up from a USB device on rk3288 boards.
> The patches were tested on rk3288-jerry in the chromeos-3.14 kernel.
> The chromeos-3.14 kernel tested included a full set of dwc2 backports
> from upstream, so this is expected to function upstream once we get
> everything setup there.
>
>
> Douglas Anderson (3):
>   USB: Export usb_wakeup_enabled_descendants()
>   Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
>   USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
>
>  Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++
>  drivers/usb/core/hub.c                         |  7 ++--
>  drivers/usb/dwc2/core.h                        |  2 ++
>  drivers/usb/dwc2/platform.c                    | 45 ++++++++++++++++++++++++--
>  include/linux/usb/hcd.h                        |  5 +++
>  5 files changed, 58 insertions(+), 5 deletions(-)

This series was posted a while ago.  Do you have any comments on it?
Should I repost it?

Thanks!

-Doug

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

* Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
@ 2015-10-21 16:23   ` Doug Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Doug Anderson @ 2015-10-21 16:23 UTC (permalink / raw)
  To: John Youn
  Cc: Mark Rutland, Heiko Stuebner, Andrew Bresticker, Kever Yang,
	Petr Mladek, Douglas Anderson, Chris Zhong, Pratyush Anand,
	open list:ARM/Rockchip SoC...,
	Alan Stern, lyz, Peter Chen, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Alexandru Stan, Pawel Moll, Ian Campbell, Rob Herring,
	Matthew Garrett, Robert Schlabbach, Dan Williams,
	Gregory Herrero

John,

On Mon, Jul 6, 2015 at 11:27 AM, Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> This series of patches, together with
> <https://patchwork.kernel.org/patch/6652341/> from Chris Zhong and a
> dts change allow us to wake up from a USB device on rk3288 boards.
> The patches were tested on rk3288-jerry in the chromeos-3.14 kernel.
> The chromeos-3.14 kernel tested included a full set of dwc2 backports
> from upstream, so this is expected to function upstream once we get
> everything setup there.
>
>
> Douglas Anderson (3):
>   USB: Export usb_wakeup_enabled_descendants()
>   Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
>   USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
>
>  Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++
>  drivers/usb/core/hub.c                         |  7 ++--
>  drivers/usb/dwc2/core.h                        |  2 ++
>  drivers/usb/dwc2/platform.c                    | 45 ++++++++++++++++++++++++--
>  include/linux/usb/hcd.h                        |  5 +++
>  5 files changed, 58 insertions(+), 5 deletions(-)

This series was posted a while ago.  Do you have any comments on it?
Should I repost it?

Thanks!

-Doug

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

* Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
  2015-10-21 16:23   ` Doug Anderson
@ 2015-10-27  2:05     ` John Youn
  -1 siblings, 0 replies; 35+ messages in thread
From: John Youn @ 2015-10-27  2:05 UTC (permalink / raw)
  To: Doug Anderson, John Youn
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	Petr Mladek, Alan Stern, devicetree, Kever Yang, linux-kernel,
	Mark Rutland, Kumar Gala, Dan Williams, Matthew Garrett,
	Ian Campbell, Rob Herring, Pratyush Anand, Pawel Moll,
	Peter Chen, Gregory Herrero, Robert Schlabbach, Felipe Balbi,
	Greg Kroah-Hartman

On 10/21/2015 9:23 AM, Doug Anderson wrote:
> John,
> 
> On Mon, Jul 6, 2015 at 11:27 AM, Douglas Anderson <dianders@chromium.org> wrote:
>> This series of patches, together with
>> <https://patchwork.kernel.org/patch/6652341/> from Chris Zhong and a
>> dts change allow us to wake up from a USB device on rk3288 boards.
>> The patches were tested on rk3288-jerry in the chromeos-3.14 kernel.
>> The chromeos-3.14 kernel tested included a full set of dwc2 backports
>> from upstream, so this is expected to function upstream once we get
>> everything setup there.
>>
>>
>> Douglas Anderson (3):
>>   USB: Export usb_wakeup_enabled_descendants()
>>   Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
>>   USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
>>
>>  Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++
>>  drivers/usb/core/hub.c                         |  7 ++--
>>  drivers/usb/dwc2/core.h                        |  2 ++
>>  drivers/usb/dwc2/platform.c                    | 45 ++++++++++++++++++++++++--
>>  include/linux/usb/hcd.h                        |  5 +++
>>  5 files changed, 58 insertions(+), 5 deletions(-)
> 
> This series was posted a while ago.  Do you have any comments on it?
> Should I repost it?
> 
> Thanks!
> 
> -Doug
> 


Sorry, I must have missed it earlier.

Could you repost based on latest and I'll review.

Regards,
John



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

* Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
@ 2015-10-27  2:05     ` John Youn
  0 siblings, 0 replies; 35+ messages in thread
From: John Youn @ 2015-10-27  2:05 UTC (permalink / raw)
  To: Doug Anderson, John Youn
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, Chris Zhong, Heiko Stuebner,
	Julius Werner, Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	Petr Mladek, Alan Stern, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Kever Yang, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Mark Rutland,
	Kumar Gala, Dan Williams, Matthew Garrett, Ian Campbell,
	Rob Herring, Pratyush Anand, Pawel Moll

On 10/21/2015 9:23 AM, Doug Anderson wrote:
> John,
> 
> On Mon, Jul 6, 2015 at 11:27 AM, Douglas Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
>> This series of patches, together with
>> <https://patchwork.kernel.org/patch/6652341/> from Chris Zhong and a
>> dts change allow us to wake up from a USB device on rk3288 boards.
>> The patches were tested on rk3288-jerry in the chromeos-3.14 kernel.
>> The chromeos-3.14 kernel tested included a full set of dwc2 backports
>> from upstream, so this is expected to function upstream once we get
>> everything setup there.
>>
>>
>> Douglas Anderson (3):
>>   USB: Export usb_wakeup_enabled_descendants()
>>   Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
>>   USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
>>
>>  Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++
>>  drivers/usb/core/hub.c                         |  7 ++--
>>  drivers/usb/dwc2/core.h                        |  2 ++
>>  drivers/usb/dwc2/platform.c                    | 45 ++++++++++++++++++++++++--
>>  include/linux/usb/hcd.h                        |  5 +++
>>  5 files changed, 58 insertions(+), 5 deletions(-)
> 
> This series was posted a while ago.  Do you have any comments on it?
> Should I repost it?
> 
> Thanks!
> 
> -Doug
> 


Sorry, I must have missed it earlier.

Could you repost based on latest and I'll review.

Regards,
John


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
  2015-10-27  2:05     ` John Youn
@ 2015-10-31  0:01       ` Doug Anderson
  -1 siblings, 0 replies; 35+ messages in thread
From: Doug Anderson @ 2015-10-31  0:01 UTC (permalink / raw)
  To: John Youn
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	Petr Mladek, Alan Stern, devicetree, Kever Yang, linux-kernel,
	Mark Rutland, Kumar Gala, Dan Williams, Matthew Garrett,
	Ian Campbell, Rob Herring, Pratyush Anand, Pawel Moll,
	Peter Chen, Gregory Herrero, Robert Schlabbach, Felipe Balbi,
	Greg Kroah-Hartman

John,

On Mon, Oct 26, 2015 at 7:05 PM, John Youn <John.Youn@synopsys.com> wrote:
> On 10/21/2015 9:23 AM, Doug Anderson wrote:
>> John,
>>
>> On Mon, Jul 6, 2015 at 11:27 AM, Douglas Anderson <dianders@chromium.org> wrote:
>>> This series of patches, together with
>>> <https://patchwork.kernel.org/patch/6652341/> from Chris Zhong and a
>>> dts change allow us to wake up from a USB device on rk3288 boards.
>>> The patches were tested on rk3288-jerry in the chromeos-3.14 kernel.
>>> The chromeos-3.14 kernel tested included a full set of dwc2 backports
>>> from upstream, so this is expected to function upstream once we get
>>> everything setup there.
>>>
>>>
>>> Douglas Anderson (3):
>>>   USB: Export usb_wakeup_enabled_descendants()
>>>   Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
>>>   USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
>>>
>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++
>>>  drivers/usb/core/hub.c                         |  7 ++--
>>>  drivers/usb/dwc2/core.h                        |  2 ++
>>>  drivers/usb/dwc2/platform.c                    | 45 ++++++++++++++++++++++++--
>>>  include/linux/usb/hcd.h                        |  5 +++
>>>  5 files changed, 58 insertions(+), 5 deletions(-)
>>
>> This series was posted a while ago.  Do you have any comments on it?
>> Should I repost it?
>>
>> Thanks!
>>
>> -Doug
>>
>
>
> Sorry, I must have missed it earlier.
>
> Could you repost based on latest and I'll review.

I tried today but something has changed in mainline Linux and the
rk3288 isn't waking up from USB even when I've configured it
correctly.  I can still send up the patches, but they won't actually
produce something that will work upstream.  :(

I'll keep it my back burner to investigate, though if someone else
(like maybe someone from Rockchip) wanted to, they could to it too...

...until we figure out what's going on we should probably consider
these patches abandoned.

-Doug

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

* Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288
@ 2015-10-31  0:01       ` Doug Anderson
  0 siblings, 0 replies; 35+ messages in thread
From: Doug Anderson @ 2015-10-31  0:01 UTC (permalink / raw)
  To: John Youn
  Cc: linux-usb, Chris Zhong, Heiko Stuebner, Julius Werner,
	Andrew Bresticker, Alexandru Stan, lyz,
	open list:ARM/Rockchip SoC...,
	Petr Mladek, Alan Stern, devicetree, Kever Yang, linux-kernel,
	Mark Rutland, Kumar Gala, Dan Williams, Matthew Garrett,
	Ian Campbell, Rob Herring, Pratyush Anand, Pawel Moll

John,

On Mon, Oct 26, 2015 at 7:05 PM, John Youn <John.Youn@synopsys.com> wrote:
> On 10/21/2015 9:23 AM, Doug Anderson wrote:
>> John,
>>
>> On Mon, Jul 6, 2015 at 11:27 AM, Douglas Anderson <dianders@chromium.org> wrote:
>>> This series of patches, together with
>>> <https://patchwork.kernel.org/patch/6652341/> from Chris Zhong and a
>>> dts change allow us to wake up from a USB device on rk3288 boards.
>>> The patches were tested on rk3288-jerry in the chromeos-3.14 kernel.
>>> The chromeos-3.14 kernel tested included a full set of dwc2 backports
>>> from upstream, so this is expected to function upstream once we get
>>> everything setup there.
>>>
>>>
>>> Douglas Anderson (3):
>>>   USB: Export usb_wakeup_enabled_descendants()
>>>   Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB
>>>   USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled
>>>
>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  4 +++
>>>  drivers/usb/core/hub.c                         |  7 ++--
>>>  drivers/usb/dwc2/core.h                        |  2 ++
>>>  drivers/usb/dwc2/platform.c                    | 45 ++++++++++++++++++++++++--
>>>  include/linux/usb/hcd.h                        |  5 +++
>>>  5 files changed, 58 insertions(+), 5 deletions(-)
>>
>> This series was posted a while ago.  Do you have any comments on it?
>> Should I repost it?
>>
>> Thanks!
>>
>> -Doug
>>
>
>
> Sorry, I must have missed it earlier.
>
> Could you repost based on latest and I'll review.

I tried today but something has changed in mainline Linux and the
rk3288 isn't waking up from USB even when I've configured it
correctly.  I can still send up the patches, but they won't actually
produce something that will work upstream.  :(

I'll keep it my back burner to investigate, though if someone else
(like maybe someone from Rockchip) wanted to, they could to it too...

...until we figure out what's going on we should probably consider
these patches abandoned.

-Doug

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

end of thread, other threads:[~2015-10-31  0:01 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-06 18:27 [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288 Douglas Anderson
2015-07-06 18:27 ` Douglas Anderson
2015-07-06 18:27 ` [REPOST PATCH 1/3] USB: Export usb_wakeup_enabled_descendants() Douglas Anderson
2015-07-06 18:27   ` Douglas Anderson
2015-07-06 18:27 ` [REPOST PATCH 2/3] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB Douglas Anderson
2015-07-06 18:27   ` Douglas Anderson
2015-07-06 18:27 ` [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled Douglas Anderson
2015-07-06 18:27   ` Douglas Anderson
2015-07-06 18:34   ` Felipe Balbi
2015-07-06 18:34     ` Felipe Balbi
2015-07-06 19:32     ` Doug Anderson
2015-07-06 19:32       ` Doug Anderson
2015-07-06 19:35       ` Felipe Balbi
2015-07-06 19:35         ` Felipe Balbi
2015-07-06 18:58   ` Alan Stern
2015-07-06 18:58     ` Alan Stern
2015-07-06 19:02     ` Felipe Balbi
2015-07-06 19:02       ` Felipe Balbi
2015-07-06 19:39       ` Doug Anderson
2015-07-06 19:39         ` Doug Anderson
2015-07-07 14:28       ` Alan Stern
2015-07-07 14:28         ` Alan Stern
2015-07-08  0:06         ` Julius Werner
2015-07-08  0:06           ` Julius Werner
2015-07-08 15:01           ` Alan Stern
2015-07-08 19:41             ` Julius Werner
2015-07-08 19:41               ` Julius Werner
2015-07-08 19:58               ` Alan Stern
2015-07-08 19:58                 ` Alan Stern
2015-10-21 16:23 ` [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288 Doug Anderson
2015-10-21 16:23   ` Doug Anderson
2015-10-27  2:05   ` John Youn
2015-10-27  2:05     ` John Youn
2015-10-31  0:01     ` Doug Anderson
2015-10-31  0:01       ` Doug Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.