[REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[Douglas Anderson]

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

[REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[Douglas Anderson]

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

[REPOST PATCH 1/3] USB: Export usb_wakeup_enabled_descendants()

[Douglas Anderson]

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

[REPOST PATCH 1/3] USB: Export usb_wakeup_enabled_descendants()

[Douglas Anderson]

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

[REPOST PATCH 2/3] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB

[Douglas Anderson]

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

[REPOST PATCH 2/3] Documentation: dt-bindings: Add snps,need-phy-for-wake for dwc2 USB

[Douglas Anderson]

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

[REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Douglas Anderson]

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

[REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Douglas Anderson]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Felipe Balbi]

[-- 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 --]

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Felipe Balbi]

[-- 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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Doug Anderson]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Doug Anderson]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Felipe Balbi]

[-- 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 --]

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Felipe Balbi]

[-- 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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Alan Stern]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Alan Stern]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Felipe Balbi]

[-- 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 --]

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Felipe Balbi]

[-- 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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Doug Anderson]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Doug Anderson]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Alan Stern]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Alan Stern]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Julius Werner]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Julius Werner]

> 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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Alan Stern]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Julius Werner]

> 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.

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Julius Werner]

> 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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Alan Stern]

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

Re: [REPOST PATCH 3/3] USB: dwc2: Don't turn off the usbphy in suspend if wakeup is enabled

[Alan Stern]

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

Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[Doug Anderson]

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

Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[Doug Anderson]

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

Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[John Youn]

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

Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[John Youn]

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

Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[Doug Anderson]

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

Re: [REPOST PATCH 0/3] dwc2 patches to allow wakeup on Rockchip rk3288

[Doug Anderson]

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