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 <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)) {
> +             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);
> 

-- 
cheers,
-roger

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
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

Reply via email to