On 12/6/2021 7:56 PM, Fabrice Gasnier wrote:
> Override enable bits may not be restored when going to low power mode
> (e.g. when in DWC2_POWER_DOWN_PARAM_NONE).
> These bits are set when probing/initializing drd (role switch). Restore
> them upon resume from low power mode (in case these have been lost).
>
> To achieve this, the last known role is restored upon resume. And the
> override enable bits are always set when configuring aval, bval and vbval.
>
> When resuming, forcing the role should be done only once, or this can cause
> port changes in HOST mode for instance.
> So, only restore FORCEDEVMODE/FORCEHOSTMODE when role_sw is unused
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx>
Acked-by: Minas Harutyunyan <Minas.Harutyunyan@xxxxxxxxxxxx>
> ---
> drivers/usb/dwc2/drd.c | 38 ++++++++++++++++++++++++++++++++++++--
> drivers/usb/dwc2/platform.c | 10 ++++++----
> 2 files changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/dwc2/drd.c b/drivers/usb/dwc2/drd.c
> index 4f453ec..1b39c47 100644
> --- a/drivers/usb/dwc2/drd.c
> +++ b/drivers/usb/dwc2/drd.c
> @@ -13,6 +13,10 @@
> #include <linux/usb/role.h>
> #include "core.h"
>
> +#define dwc2_ovr_gotgctl(gotgctl) \
> + ((gotgctl) |= GOTGCTL_BVALOEN | GOTGCTL_AVALOEN | GOTGCTL_VBVALOEN | \
> + GOTGCTL_DBNCE_FLTR_BYPASS)
> +
> static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
> {
> unsigned long flags;
> @@ -21,8 +25,7 @@ static void dwc2_ovr_init(struct dwc2_hsotg *hsotg)
> spin_lock_irqsave(&hsotg->lock, flags);
>
> gotgctl = dwc2_readl(hsotg, GOTGCTL);
> - gotgctl |= GOTGCTL_BVALOEN | GOTGCTL_AVALOEN | GOTGCTL_VBVALOEN;
> - gotgctl |= GOTGCTL_DBNCE_FLTR_BYPASS;
> + dwc2_ovr_gotgctl(gotgctl);
> gotgctl &= ~(GOTGCTL_BVALOVAL | GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL);
> if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST)
> gotgctl |= GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL;
> @@ -44,6 +47,9 @@ static int dwc2_ovr_avalid(struct dwc2_hsotg *hsotg, bool valid)
> (!valid && !(gotgctl & GOTGCTL_ASESVLD)))
> return -EALREADY;
>
> + /* Always enable overrides to handle the resume case */
> + dwc2_ovr_gotgctl(gotgctl);
> +
> gotgctl &= ~GOTGCTL_BVALOVAL;
> if (valid)
> gotgctl |= GOTGCTL_AVALOVAL | GOTGCTL_VBVALOVAL;
> @@ -63,6 +69,9 @@ static int dwc2_ovr_bvalid(struct dwc2_hsotg *hsotg, bool valid)
> (!valid && !(gotgctl & GOTGCTL_BSESVLD)))
> return -EALREADY;
>
> + /* Always enable overrides to handle the resume case */
> + dwc2_ovr_gotgctl(gotgctl);
> +
> gotgctl &= ~GOTGCTL_AVALOVAL;
> if (valid)
> gotgctl |= GOTGCTL_BVALOVAL | GOTGCTL_VBVALOVAL;
> @@ -196,6 +205,31 @@ void dwc2_drd_suspend(struct dwc2_hsotg *hsotg)
> void dwc2_drd_resume(struct dwc2_hsotg *hsotg)
> {
> u32 gintsts, gintmsk;
> + enum usb_role role;
> +
> + if (hsotg->role_sw) {
> + /* get last known role (as the get ops isn't implemented by this driver) */
> + role = usb_role_switch_get_role(hsotg->role_sw);
> +
> + if (role == USB_ROLE_NONE) {
> + if (hsotg->role_sw_default_mode == USB_DR_MODE_HOST)
> + role = USB_ROLE_HOST;
> + else if (hsotg->role_sw_default_mode == USB_DR_MODE_PERIPHERAL)
> + role = USB_ROLE_DEVICE;
> + }
> +
> + /* restore last role that may have been lost */
> + if (role == USB_ROLE_HOST)
> + dwc2_ovr_avalid(hsotg, true);
> + else if (role == USB_ROLE_DEVICE)
> + dwc2_ovr_bvalid(hsotg, true);
> +
> + dwc2_force_mode(hsotg, role == USB_ROLE_HOST);
> +
> + dev_dbg(hsotg->dev, "resuming %s-session valid\n",
> + role == USB_ROLE_NONE ? "No" :
> + role == USB_ROLE_HOST ? "A" : "B");
> + }
>
> if (hsotg->role_sw && !hsotg->params.external_id_pin_ctl) {
> gintsts = dwc2_readl(hsotg, GINTSTS);
> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
> index c8f18f3..e6a7fc0 100644
> --- a/drivers/usb/dwc2/platform.c
> +++ b/drivers/usb/dwc2/platform.c
> @@ -748,10 +748,12 @@ static int __maybe_unused dwc2_resume(struct device *dev)
> spin_unlock_irqrestore(&dwc2->lock, flags);
> }
>
> - /* Need to restore FORCEDEVMODE/FORCEHOSTMODE */
> - dwc2_force_dr_mode(dwc2);
> -
> - dwc2_drd_resume(dwc2);
> + if (!dwc2->role_sw) {
> + /* Need to restore FORCEDEVMODE/FORCEHOSTMODE */
> + dwc2_force_dr_mode(dwc2);
> + } else {
> + dwc2_drd_resume(dwc2);
> + }
>
> if (dwc2_is_device_mode(dwc2))
> ret = dwc2_hsotg_resume(dwc2);
