* [RFC,usb-next,v2,2/2] usb: core: use phy_exit during suspend if wake up is not supported
@ 2018-03-26 3:21 Chunfeng Yun
0 siblings, 0 replies; 3+ messages in thread
From: Chunfeng Yun @ 2018-03-26 3:21 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-usb, gregkh, matthias.bgg, stern, rogerq, linux-mediatek,
linux-amlogic, j-keerthy, d-gerlach, kishon
On Sat, 2018-03-24 at 15:21 +0100, Martin Blumenstingl wrote:
> If the USB controller can wake up the system (which is the case for
> example with the Mediatek USB3 IP) then we must not call phy_exit during
> suspend to ensure that the USB controller doesn't have to re-enumerate
> the devices during resume.
> However, if the USB controller cannot wake up the system (which is the
> case for example on various TI platforms using a dwc3 controller) then
> we must call phy_exit during suspend. Otherwise the PHY driver keeps the
> clocks enabled, which prevents the system from entering the suspend
> state.
>
> Solve this by introducing two new functions in the PHY wrapper which are
> dedicated to the suspend and resume handling.
> If the controller can wake up the system the new usb_phy_roothub_suspend
> function will simply call usb_phy_roothub_power_off. However, if wake up
> is not supported by the controller it will also call
> usb_phy_roothub_exit.
> The also new usb_phy_roothub_resume function takes care of calling
> usb_phy_roothub_init (if the controller can't wake up the system) in
> addition to usb_phy_roothub_power_on.
>
> Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the HCD")
> Fixes: 178a0bce05cbc1 ("usb: core: hcd: integrate the PHY wrapper into the HCD core")
> Reported-by: Roger Quadros <rogerq@ti.com>
> Suggested-by: Roger Quadros <rogerq@ti.com>
> Suggested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/usb/core/hcd.c | 8 +++++---
> drivers/usb/core/phy.c | 37 +++++++++++++++++++++++++++++++++++++
> drivers/usb/core/phy.h | 5 +++++
> 3 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 15b0418e3b6a..78bae4ecd68b 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2262,7 +2262,8 @@ int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg)
> hcd->state = HC_STATE_SUSPENDED;
>
> if (!PMSG_IS_AUTO(msg))
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev,
> + hcd->phy_roothub);
>
> /* Did we race with a root-hub wakeup event? */
> if (rhdev->do_remote_wakeup) {
> @@ -2302,7 +2303,8 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg)
> }
>
> if (!PMSG_IS_AUTO(msg)) {
> - status = usb_phy_roothub_power_on(hcd->phy_roothub);
> + status = usb_phy_roothub_resume(hcd->self.sysdev,
> + hcd->phy_roothub);
> if (status)
> return status;
> }
> @@ -2344,7 +2346,7 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg)
> }
> } else {
> hcd->state = old_state;
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
> dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
> "resume", status);
> if (status != -ESHUTDOWN)
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index d1861c5a74de..e794cbee97e9 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -155,3 +155,40 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub)
> phy_power_off(roothub_entry->phy);
> }
> EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
> +
> +int usb_phy_roothub_suspend(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub)
> +{
> + usb_phy_roothub_power_off(phy_roothub);
> +
> + /* keep the PHYs initialized so the device can wake up the system */
> + if (device_may_wakeup(controller_dev))
> + return 0;
> +
> + return usb_phy_roothub_exit(phy_roothub);
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
> +
> +int usb_phy_roothub_resume(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub)
> +{
> + int err;
> +
> + /* if the device can't wake up the system _exit was called */
> + if (device_may_wakeup(controller_dev)) {
fix it as Roger suggested before:
if (!device_may_wakeup(controller_dev)) {
> + err = usb_phy_roothub_init(phy_roothub);
> + if (err)
> + return err;
> + }
> +
> + err = usb_phy_roothub_power_on(phy_roothub);
> + if (err) {
> + if (device_may_wakeup(controller_dev))
ditto
After fixing them, it's ok on MTK's platform.
> + usb_phy_roothub_exit(phy_roothub);
> +
> + return err;
Can be removed?
> + }
> +
> + return 0;
return err;
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_resume);
> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
> index eb31253201ad..605555901d44 100644
> --- a/drivers/usb/core/phy.h
> +++ b/drivers/usb/core/phy.h
> @@ -7,3 +7,8 @@ int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
>
> int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
> void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);
> +
> +int usb_phy_roothub_suspend(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub);
> +int usb_phy_roothub_resume(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub);
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread* [RFC,usb-next,v2,2/2] usb: core: use phy_exit during suspend if wake up is not supported
@ 2018-03-26 9:53 Roger Quadros
0 siblings, 0 replies; 3+ messages in thread
From: Roger Quadros @ 2018-03-26 9:53 UTC (permalink / raw)
To: Martin Blumenstingl, linux-usb
Cc: gregkh, matthias.bgg, stern, linux-mediatek, linux-amlogic,
j-keerthy, d-gerlach, kishon, chunfeng.yun
Hi,
On 24/03/18 16:21, Martin Blumenstingl wrote:
> If the USB controller can wake up the system (which is the case for
> example with the Mediatek USB3 IP) then we must not call phy_exit during
> suspend to ensure that the USB controller doesn't have to re-enumerate
> the devices during resume.
> However, if the USB controller cannot wake up the system (which is the
> case for example on various TI platforms using a dwc3 controller) then
> we must call phy_exit during suspend. Otherwise the PHY driver keeps the
> clocks enabled, which prevents the system from entering the suspend
> state.
Actually it doesn't prevent the system from entering suspend but just prevents
the system from reaching lowest power levels in the suspend state.
>
> Solve this by introducing two new functions in the PHY wrapper which are
> dedicated to the suspend and resume handling.
> If the controller can wake up the system the new usb_phy_roothub_suspend
> function will simply call usb_phy_roothub_power_off. However, if wake up
> is not supported by the controller it will also call
> usb_phy_roothub_exit.
> The also new usb_phy_roothub_resume function takes care of calling
> usb_phy_roothub_init (if the controller can't wake up the system) in
> addition to usb_phy_roothub_power_on.
>
> Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the HCD")
> Fixes: 178a0bce05cbc1 ("usb: core: hcd: integrate the PHY wrapper into the HCD core")
> Reported-by: Roger Quadros <rogerq@ti.com>
> Suggested-by: Roger Quadros <rogerq@ti.com>
> Suggested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/usb/core/hcd.c | 8 +++++---
> drivers/usb/core/phy.c | 37 +++++++++++++++++++++++++++++++++++++
> drivers/usb/core/phy.h | 5 +++++
> 3 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 15b0418e3b6a..78bae4ecd68b 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2262,7 +2262,8 @@ int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg)
> hcd->state = HC_STATE_SUSPENDED;
>
> if (!PMSG_IS_AUTO(msg))
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev,
> + hcd->phy_roothub);
>
> /* Did we race with a root-hub wakeup event? */
> if (rhdev->do_remote_wakeup) {
> @@ -2302,7 +2303,8 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg)
> }
>
> if (!PMSG_IS_AUTO(msg)) {
> - status = usb_phy_roothub_power_on(hcd->phy_roothub);
> + status = usb_phy_roothub_resume(hcd->self.sysdev,
> + hcd->phy_roothub);
> if (status)
> return status;
> }
> @@ -2344,7 +2346,7 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg)
> }
> } else {
> hcd->state = old_state;
> - usb_phy_roothub_power_off(hcd->phy_roothub);
> + usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
> dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
> "resume", status);
> if (status != -ESHUTDOWN)
> diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
> index d1861c5a74de..e794cbee97e9 100644
> --- a/drivers/usb/core/phy.c
> +++ b/drivers/usb/core/phy.c
> @@ -155,3 +155,40 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub)
> phy_power_off(roothub_entry->phy);
> }
> EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
> +
> +int usb_phy_roothub_suspend(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub)
> +{
> + usb_phy_roothub_power_off(phy_roothub);
> +
> + /* keep the PHYs initialized so the device can wake up the system */
> + if (device_may_wakeup(controller_dev))
> + return 0;
> +
> + return usb_phy_roothub_exit(phy_roothub);
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
> +
> +int usb_phy_roothub_resume(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub)
> +{
> + int err;
> +
> + /* if the device can't wake up the system _exit was called */
> + if (device_may_wakeup(controller_dev)) {
> + err = usb_phy_roothub_init(phy_roothub);
> + if (err)
> + return err;
> + }
> +
> + err = usb_phy_roothub_power_on(phy_roothub);
> + if (err) {
> + if (device_may_wakeup(controller_dev))
> + usb_phy_roothub_exit(phy_roothub);
> +
> + return err;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(usb_phy_roothub_resume);
> diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
> index eb31253201ad..605555901d44 100644
> --- a/drivers/usb/core/phy.h
> +++ b/drivers/usb/core/phy.h
> @@ -7,3 +7,8 @@ int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
>
> int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
> void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);
> +
> +int usb_phy_roothub_suspend(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub);
> +int usb_phy_roothub_resume(struct device *controller_dev,
> + struct usb_phy_roothub *phy_roothub);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* [RFC,usb-next,v2,2/2] usb: core: use phy_exit during suspend if wake up is not supported
@ 2018-03-24 14:21 Martin Blumenstingl
0 siblings, 0 replies; 3+ messages in thread
From: Martin Blumenstingl @ 2018-03-24 14:21 UTC (permalink / raw)
To: linux-usb
Cc: gregkh, matthias.bgg, stern, rogerq, linux-mediatek,
linux-amlogic, j-keerthy, d-gerlach, kishon, chunfeng.yun,
Martin Blumenstingl
If the USB controller can wake up the system (which is the case for
example with the Mediatek USB3 IP) then we must not call phy_exit during
suspend to ensure that the USB controller doesn't have to re-enumerate
the devices during resume.
However, if the USB controller cannot wake up the system (which is the
case for example on various TI platforms using a dwc3 controller) then
we must call phy_exit during suspend. Otherwise the PHY driver keeps the
clocks enabled, which prevents the system from entering the suspend
state.
Solve this by introducing two new functions in the PHY wrapper which are
dedicated to the suspend and resume handling.
If the controller can wake up the system the new usb_phy_roothub_suspend
function will simply call usb_phy_roothub_power_off. However, if wake up
is not supported by the controller it will also call
usb_phy_roothub_exit.
The also new usb_phy_roothub_resume function takes care of calling
usb_phy_roothub_init (if the controller can't wake up the system) in
addition to usb_phy_roothub_power_on.
Fixes: 07dbff0ddbd86c ("usb: core: add a wrapper for the USB PHYs on the HCD")
Fixes: 178a0bce05cbc1 ("usb: core: hcd: integrate the PHY wrapper into the HCD core")
Reported-by: Roger Quadros <rogerq@ti.com>
Suggested-by: Roger Quadros <rogerq@ti.com>
Suggested-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
drivers/usb/core/hcd.c | 8 +++++---
drivers/usb/core/phy.c | 37 +++++++++++++++++++++++++++++++++++++
drivers/usb/core/phy.h | 5 +++++
3 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 15b0418e3b6a..78bae4ecd68b 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2262,7 +2262,8 @@ int hcd_bus_suspend(struct usb_device *rhdev, pm_message_t msg)
hcd->state = HC_STATE_SUSPENDED;
if (!PMSG_IS_AUTO(msg))
- usb_phy_roothub_power_off(hcd->phy_roothub);
+ usb_phy_roothub_suspend(hcd->self.sysdev,
+ hcd->phy_roothub);
/* Did we race with a root-hub wakeup event? */
if (rhdev->do_remote_wakeup) {
@@ -2302,7 +2303,8 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg)
}
if (!PMSG_IS_AUTO(msg)) {
- status = usb_phy_roothub_power_on(hcd->phy_roothub);
+ status = usb_phy_roothub_resume(hcd->self.sysdev,
+ hcd->phy_roothub);
if (status)
return status;
}
@@ -2344,7 +2346,7 @@ int hcd_bus_resume(struct usb_device *rhdev, pm_message_t msg)
}
} else {
hcd->state = old_state;
- usb_phy_roothub_power_off(hcd->phy_roothub);
+ usb_phy_roothub_suspend(hcd->self.sysdev, hcd->phy_roothub);
dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
"resume", status);
if (status != -ESHUTDOWN)
diff --git a/drivers/usb/core/phy.c b/drivers/usb/core/phy.c
index d1861c5a74de..e794cbee97e9 100644
--- a/drivers/usb/core/phy.c
+++ b/drivers/usb/core/phy.c
@@ -155,3 +155,40 @@ void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub)
phy_power_off(roothub_entry->phy);
}
EXPORT_SYMBOL_GPL(usb_phy_roothub_power_off);
+
+int usb_phy_roothub_suspend(struct device *controller_dev,
+ struct usb_phy_roothub *phy_roothub)
+{
+ usb_phy_roothub_power_off(phy_roothub);
+
+ /* keep the PHYs initialized so the device can wake up the system */
+ if (device_may_wakeup(controller_dev))
+ return 0;
+
+ return usb_phy_roothub_exit(phy_roothub);
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_suspend);
+
+int usb_phy_roothub_resume(struct device *controller_dev,
+ struct usb_phy_roothub *phy_roothub)
+{
+ int err;
+
+ /* if the device can't wake up the system _exit was called */
+ if (device_may_wakeup(controller_dev)) {
+ err = usb_phy_roothub_init(phy_roothub);
+ if (err)
+ return err;
+ }
+
+ err = usb_phy_roothub_power_on(phy_roothub);
+ if (err) {
+ if (device_may_wakeup(controller_dev))
+ usb_phy_roothub_exit(phy_roothub);
+
+ return err;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(usb_phy_roothub_resume);
diff --git a/drivers/usb/core/phy.h b/drivers/usb/core/phy.h
index eb31253201ad..605555901d44 100644
--- a/drivers/usb/core/phy.h
+++ b/drivers/usb/core/phy.h
@@ -7,3 +7,8 @@ int usb_phy_roothub_exit(struct usb_phy_roothub *phy_roothub);
int usb_phy_roothub_power_on(struct usb_phy_roothub *phy_roothub);
void usb_phy_roothub_power_off(struct usb_phy_roothub *phy_roothub);
+
+int usb_phy_roothub_suspend(struct device *controller_dev,
+ struct usb_phy_roothub *phy_roothub);
+int usb_phy_roothub_resume(struct device *controller_dev,
+ struct usb_phy_roothub *phy_roothub);
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-26 9:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-26 3:21 [RFC,usb-next,v2,2/2] usb: core: use phy_exit during suspend if wake up is not supported Chunfeng Yun
-- strict thread matches above, loose matches on Subject: below --
2018-03-26 9:53 Roger Quadros
2018-03-24 14:21 Martin Blumenstingl
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox