[PATCH] usb: phy: am335x: Enable USB remote wakeup using PHY wakeup

[PATCH] usb: phy: am335x: Enable USB remote wakeup using PHY wakeup

[George Cherian]

USB remote wakeup using PHY wakeup is supported only in standby mode.
Enabling the PHY_WKUP will break DS0 mode of system suspend.
If the same is enabled while entering DS0, AM33xx never stays in DS0,
it returns immediately from DS0. By default make the PHY wakeup
disabled, using sysfs entry enable the same manually to get
the remote wakeup working from standby.

echo enabled > /sys/bus/platform/device/<usb phy id>/power/wakeup
This will enable the PHY wakeup while going to standby.

PHY wakeup feature is required to wakeup  the system from standby
state. Since AM33xx has a bug in which PHY wakeup should not
be enabled while entering DS0, disable the same by default.
A user wishing to use USB wakeup from standby mode need to enable
the same using the sysfs entries.

Also remove am335x_phy_runtime_(suspend/resume) this driver doesnot really
enable/disable the clocks to the PHY.
Add am335x_phy_(suspend/resume) and use the same for enabling the PHY_WKUP.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/usb/phy/phy-am335x.c | 46 +++++++++++++++++++++++++++++++++-----------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
index 6370e50..3f78eaf 100644
--- a/drivers/usb/phy/phy-am335x.c
+++ b/drivers/usb/phy/phy-am335x.c
@@ -64,6 +64,19 @@ static int am335x_phy_probe(struct platform_device *pdev)
 	am_phy->usb_phy_gen.phy.shutdown = am335x_shutdown;
 
 	platform_set_drvdata(pdev, am_phy);
+	device_init_wakeup(dev, true);
+
+	/*
+	 * If we leave PHY wakeup enabled then AM33XX wakes up
+	 * immediately from DS0. To avoid this we mark dev->power.can_wakeup
+	 * to false. The same is checked in suspend routine to decide
+	 * on whether to enable PHY wakeup or not.
+	 * PHY wakeup works fine in standby mode, there by allowing us to
+	 * handle remote wakeup, wakeup on disconnect and connect.
+	 */
+
+	device_set_wakeup_enable(dev, false);
+	phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
 
 	return 0;
 
@@ -78,40 +91,51 @@ static int am335x_phy_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_RUNTIME
-
-static int am335x_phy_runtime_suspend(struct device *dev)
+#ifdef CONFIG_PM_SLEEP
+static int am335x_phy_suspend(struct device *dev)
 {
 	struct platform_device	*pdev = to_platform_device(dev);
 	struct am335x_phy *am_phy = platform_get_drvdata(pdev);
 
+	/*
+	 * Enable phy wakeup only if dev->power.can_wakeup is true.
+	 * Make sure to enable wakeup to support remote wakeup	in
+	 * standby mode ( same is not supported in OFF(DS0) mode).
+	 * Enable it by doing
+	 * echo enabled > /sys/bus/platform/devices/<usb-phy-id>/power/wakeup
+	 */
+
 	if (device_may_wakeup(dev))
 		phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, true);
+
 	phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
+
 	return 0;
 }
 
-static int am335x_phy_runtime_resume(struct device *dev)
+static int am335x_phy_resume(struct device *dev)
 {
 	struct platform_device	*pdev = to_platform_device(dev);
 	struct am335x_phy	*am_phy = platform_get_drvdata(pdev);
 
 	phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, true);
+
 	if (device_may_wakeup(dev))
 		phy_ctrl_wkup(am_phy->phy_ctrl, am_phy->id, false);
+
 	return 0;
 }
+#define DEV_PM_OPS     (&am335x_pm_ops)
+#else
+#define DEV_PM_OPS     NULL
+#endif
+
 
 static const struct dev_pm_ops am335x_pm_ops = {
-	SET_RUNTIME_PM_OPS(am335x_phy_runtime_suspend,
-			am335x_phy_runtime_resume, NULL)
+	.suspend = am335x_phy_suspend,
+	.resume = am335x_phy_resume,
 };
 
-#define DEV_PM_OPS	(&am335x_pm_ops)
-#else
-#define DEV_PM_OPS	NULL
-#endif
-
 static const struct of_device_id am335x_phy_ids[] = {
 	{ .compatible = "ti,am335x-usb-phy" },
 	{ }
-- 
1.8.1

Re: [PATCH] usb: phy: am335x: Enable USB remote wakeup using PHY wakeup

[Felipe Balbi]

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

Hi,

On Wed, Dec 18, 2013 at 06:52:09PM +0530, George Cherian wrote:
> USB remote wakeup using PHY wakeup is supported only in standby mode.
> Enabling the PHY_WKUP will break DS0 mode of system suspend.
> If the same is enabled while entering DS0, AM33xx never stays in DS0,
> it returns immediately from DS0. By default make the PHY wakeup
> disabled, using sysfs entry enable the same manually to get
> the remote wakeup working from standby.
> 
> echo enabled > /sys/bus/platform/device/<usb phy id>/power/wakeup
> This will enable the PHY wakeup while going to standby.
> 
> PHY wakeup feature is required to wakeup  the system from standby
> state. Since AM33xx has a bug in which PHY wakeup should not
> be enabled while entering DS0, disable the same by default.
> A user wishing to use USB wakeup from standby mode need to enable
> the same using the sysfs entries.
> 
> Also remove am335x_phy_runtime_(suspend/resume) this driver doesnot really
> enable/disable the clocks to the PHY.
> Add am335x_phy_(suspend/resume) and use the same for enabling the PHY_WKUP.
> 
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
>  drivers/usb/phy/phy-am335x.c | 46 +++++++++++++++++++++++++++++++++-----------
>  1 file changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
> index 6370e50..3f78eaf 100644
> --- a/drivers/usb/phy/phy-am335x.c
> +++ b/drivers/usb/phy/phy-am335x.c
> @@ -64,6 +64,19 @@ static int am335x_phy_probe(struct platform_device *pdev)
>  	am_phy->usb_phy_gen.phy.shutdown = am335x_shutdown;
>  
>  	platform_set_drvdata(pdev, am_phy);
> +	device_init_wakeup(dev, true);
> +
> +	/*
> +	 * If we leave PHY wakeup enabled then AM33XX wakes up
> +	 * immediately from DS0. To avoid this we mark dev->power.can_wakeup
> +	 * to false. The same is checked in suspend routine to decide
> +	 * on whether to enable PHY wakeup or not.
> +	 * PHY wakeup works fine in standby mode, there by allowing us to
> +	 * handle remote wakeup, wakeup on disconnect and connect.
> +	 */
> +
> +	device_set_wakeup_enable(dev, false);
> +	phy_ctrl_power(am_phy->phy_ctrl, am_phy->id, false);
>  
>  	return 0;
>  
> @@ -78,40 +91,51 @@ static int am335x_phy_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PM_RUNTIME
> -
> -static int am335x_phy_runtime_suspend(struct device *dev)
> +#ifdef CONFIG_PM_SLEEP
> +static int am335x_phy_suspend(struct device *dev)
>  {
>  	struct platform_device	*pdev = to_platform_device(dev);
>  	struct am335x_phy *am_phy = platform_get_drvdata(pdev);
>  
> +	/*
> +	 * Enable phy wakeup only if dev->power.can_wakeup is true.
> +	 * Make sure to enable wakeup to support remote wakeup	in
> +	 * standby mode ( same is not supported in OFF(DS0) mode).
> +	 * Enable it by doing
> +	 * echo enabled > /sys/bus/platform/devices/<usb-phy-id>/power/wakeup
> +	 */

Alan, any suggestions on how to handle wakeup correctly ? I mean, it
should be enabled by default during runtime PM and disabled during
system sleep. But when it comes to default state of can_wakeup field,
what should we do ? Should the driver be fiddling with that ? Leave it
to the bus driver ? Any tips ?

cheers

-- 
balbi

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

Re: [PATCH] usb: phy: am335x: Enable USB remote wakeup using PHY wakeup

[Alan Stern]

On Wed, 18 Dec 2013, Felipe Balbi wrote:

> Alan, any suggestions on how to handle wakeup correctly ? I mean, it
> should be enabled by default during runtime PM and disabled during
> system sleep.

During system sleep, it should be set according to the value of 
device_may_wakeup().

>  But when it comes to default state of can_wakeup field,
> what should we do ? Should the driver be fiddling with that ? Leave it
> to the bus driver ? Any tips ?

If the bus driver knows whether the hardware is capable of issuing a
wakeup request, it should set can_wakeup appropriately.  In most cases
the device driver doesn't need to worry about it.

There are a few exceptions, though.  If the bus code doesn't know 
enough about the hardware, or if some hardware has a quirk that the 
driver knows about but the bus code doesn't, for example.

Alan Stern