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 <rog...@ti.com>
> Suggested-by: Roger Quadros <rog...@ti.com>
> Suggested-by: Chunfeng Yun <chunfeng....@mediatek.com>
> Signed-off-by: Martin Blumenstingl <martin.blumensti...@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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
