Hi,
On 1/22/2018 6:31 PM, Roger Quadros wrote:
> Adding/removing host/gadget controller before .pm_complete()
> causes a lock-up. Let's prevent any dual-role state change
> between .pm_prepare() and .pm_complete() to fix this.
What kind of lock-up are you seeing? Some hardware lockup or software deadlock?
IMO using a freezable_wq for drd_work should address that?
>
> Signed-off-by: Roger Quadros <rog...@ti.com>
> ---
> drivers/usb/dwc3/core.c | 31 +++++++++++++++++++++++++++++++
> drivers/usb/dwc3/core.h | 5 +++++
> drivers/usb/dwc3/drd.c | 10 ++++++----
> 3 files changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 42379cc..85388dd 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1414,6 +1414,33 @@ static int dwc3_runtime_idle(struct device *dev)
> #endif /* CONFIG_PM */
>
> #ifdef CONFIG_PM_SLEEP
> +static int dwc3_prepare(struct device *dev)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + if (dwc->dr_mode == USB_DR_MODE_OTG) {
> + spin_lock_irqsave(&dwc->lock, flags);
> + dwc->dr_keep_role = true;
> + spin_unlock_irqrestore(&dwc->lock, flags);
> + }
> +
> + return 0;
> +}
> +
> +static void dwc3_complete(struct device *dev)
> +{
> + struct dwc3 *dwc = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + if (dwc->dr_mode == USB_DR_MODE_OTG) {
> + spin_lock_irqsave(&dwc->lock, flags);
> + dwc->dr_keep_role = false;
> + spin_unlock_irqrestore(&dwc->lock, flags);
> + dwc3_drd_update(dwc);
> + }
> +}
> +
> static int dwc3_suspend(struct device *dev)
> {
> struct dwc3 *dwc = dev_get_drvdata(dev);
> @@ -1451,6 +1478,10 @@ static const struct dev_pm_ops dwc3_dev_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(dwc3_suspend, dwc3_resume)
> SET_RUNTIME_PM_OPS(dwc3_runtime_suspend, dwc3_runtime_resume,
> dwc3_runtime_idle)
> +#ifdef CONFIG_PM_SLEEP
> + .prepare = dwc3_prepare,
> + .complete = dwc3_complete,
> +#endif
> };
>
> #ifdef CONFIG_OF
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 4a4a4c9..f5eb474 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -786,6 +786,7 @@ struct dwc3_scratchpad_array {
> * @dr_mode: requested mode of operation
> * @current_dr_role: current role of operation when in dual-role mode
> * @desired_dr_role: desired role of operation when in dual-role mode
> + * @dr_keep_role: keep the current dual-role irrespective of ID changes
> * @edev: extcon handle
> * @edev_nb: extcon notifier
> * @hsphy_mode: UTMI phy mode, one of following:
> @@ -901,6 +902,7 @@ struct dwc3 {
> enum usb_dr_mode dr_mode;
> u32 current_dr_role;
> u32 desired_dr_role;
> + bool dr_keep_role;
> struct extcon_dev *edev;
> struct notifier_block edev_nb;
> enum usb_phy_interface hsphy_mode;
> @@ -1227,11 +1229,14 @@ static inline int
> dwc3_send_gadget_generic_command(struct dwc3 *dwc,
> #if IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
> int dwc3_drd_init(struct dwc3 *dwc);
> void dwc3_drd_exit(struct dwc3 *dwc);
> +void dwc3_drd_update(struct dwc3 *dwc);
> #else
> static inline int dwc3_drd_init(struct dwc3 *dwc)
> { return 0; }
> static inline void dwc3_drd_exit(struct dwc3 *dwc)
> { }
> +static inline void dwc3_drd_update(struct dwc3 *dwc);
> +{ }
> #endif
>
> /* power management interface */
> diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
> index cc8ab9a..177a8be 100644
> --- a/drivers/usb/dwc3/drd.c
> +++ b/drivers/usb/dwc3/drd.c
> @@ -13,7 +13,7 @@
> #include "core.h"
> #include "gadget.h"
>
> -static void dwc3_drd_update(struct dwc3 *dwc)
> +void dwc3_drd_update(struct dwc3 *dwc)
> {
> int id;
>
> @@ -31,9 +31,11 @@ static int dwc3_drd_notifier(struct notifier_block *nb,
> {
> struct dwc3 *dwc = container_of(nb, struct dwc3, edev_nb);
>
> - dwc3_set_mode(dwc, event ?
> - DWC3_GCTL_PRTCAP_HOST :
> - DWC3_GCTL_PRTCAP_DEVICE);
> + if (!dwc->dr_keep_role) {
> + dwc3_set_mode(dwc, event ?
> + DWC3_GCTL_PRTCAP_HOST :
> + DWC3_GCTL_PRTCAP_DEVICE);
> + }
>
> return NOTIFY_DONE;
> }
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
