[PATCH v4 1/6] usb: dwc2: fix recovery after PM domain off

[Fabrice Gasnier]

When the power domain containing the DWC2 controller is disabled during
suspend, its registers are lost. Reading registers to back them up from
dwc2_suspend() after clock gating can hang STM32MP1 in both host and
device mode. Host suspend enters clock gating directly; in device mode,
this can happen if the host suspended the USB bus before system suspend.

Check GUSBCFG.TOUTCAL after restoring the controller's mode to detect
register loss, then reinitialize the controller and role-specific state.
For host mode, reinitialize the HCD, discard stale register backups, and
restore HCD accessibility after setup. For gadget mode, force a
disconnect, disable non-EP0 endpoints, clear stale low-power state, and
mark gadget speed unknown. Pass the register-loss state to gadget resume
so recovery also runs when the host had suspended the bus.

Host resume observes HPRT0_CONNSTS cleared after register loss, so it
does not skip the recovery path. Ordinary resume and USB-reset behavior
remain unchanged.

Fixes: ba6e518d136b ("usb: dwc2: Implement recovery after PM domain off")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@xxxxxxxxxxx>
---
Changes in v4:
- Disconnect the gadget and clear stale endpoint and low-power state.
  Sashiko comment "usb: dwc2: incomplete gadget state cleanup in
  dwc2_hsotg_resume()"
- Reinitialize host state and clear stale backups after register loss.
  Sashiko comment "usb: dwc2: incomplete host mode recovery in
  dwc2_resume() after PM domain off"
- Revisit commit message

Changes in v3:
- Add a note in the commit message since Sashiko comment on
  _dwc2_hcd_resume() check on HPRT0_CONNSTS bit.
- Add need_restore flag and pass it to dwc2_hsotg_resume() to avoid
  possible interrupt race with lx_state update as raised by Sashiko,
  instead of updating lx_state to DWC2_L0.

Changes in v2:
- In case registers have been lost, ensure the device mode resume takes
  place by updating the lx_state to DWC2_L0, as reported by Sashiko. Note
  in case of error in dwc2_core_init() choice is made to leave the low
  level HW init, as it is the expected state out of the resume routine.
---
 drivers/usb/dwc2/core.c     |  1 -
 drivers/usb/dwc2/core.h     |  9 ++++---
 drivers/usb/dwc2/gadget.c   | 18 +++++++++++--
 drivers/usb/dwc2/hcd.c      | 47 +++++++++++++++++++++++++++++++++
 drivers/usb/dwc2/platform.c | 64 ++++++++++++++++++---------------------------
 5 files changed, 93 insertions(+), 46 deletions(-)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 6a0135e06b00..50627c2baf1d 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -43,7 +43,6 @@ int dwc2_backup_global_registers(struct dwc2_hsotg *hsotg)
 	/* Backup global regs */
 	gr = &hsotg->gr_backup;
 
-	gr->gintsts = dwc2_readl(hsotg, GINTSTS);
 	gr->gotgctl = dwc2_readl(hsotg, GOTGCTL);
 	gr->gintmsk = dwc2_readl(hsotg, GINTMSK);
 	gr->gahbcfg = dwc2_readl(hsotg, GAHBCFG);
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 015a5fb7842d..32662cb3d3eb 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -668,7 +668,6 @@ struct dwc2_hw_params {
 /**
  * struct dwc2_gregs_backup - Holds global registers state before
  * entering partial power down
- * @gintsts:		Backup of GINTSTS register
  * @gotgctl:		Backup of GOTGCTL register
  * @gintmsk:		Backup of GINTMSK register
  * @gahbcfg:		Backup of GAHBCFG register
@@ -685,7 +684,6 @@ struct dwc2_hw_params {
  * @valid:		True if registers values backuped.
  */
 struct dwc2_gregs_backup {
-	u32 gintsts;
 	u32 gotgctl;
 	u32 gintmsk;
 	u32 gahbcfg;
@@ -1415,7 +1413,7 @@ void dwc2_dump_global_registers(struct dwc2_hsotg *hsotg);
 	IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
 int dwc2_hsotg_remove(struct dwc2_hsotg *hsotg);
 int dwc2_hsotg_suspend(struct dwc2_hsotg *dwc2);
-int dwc2_hsotg_resume(struct dwc2_hsotg *dwc2);
+int dwc2_hsotg_resume(struct dwc2_hsotg *dwc2, bool need_restore);
 int dwc2_gadget_init(struct dwc2_hsotg *hsotg);
 void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg *dwc2,
 				       bool reset);
@@ -1451,7 +1449,7 @@ static inline int dwc2_hsotg_remove(struct dwc2_hsotg *dwc2)
 { return 0; }
 static inline int dwc2_hsotg_suspend(struct dwc2_hsotg *dwc2)
 { return 0; }
-static inline int dwc2_hsotg_resume(struct dwc2_hsotg *dwc2)
+static inline int dwc2_hsotg_resume(struct dwc2_hsotg *dwc2, bool need_restore)
 { return 0; }
 static inline int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
 { return 0; }
@@ -1502,6 +1500,7 @@ static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg) {}
 #if IS_ENABLED(CONFIG_USB_DWC2_HOST) || IS_ENABLED(CONFIG_USB_DWC2_DUAL_ROLE)
 int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg);
 int dwc2_hcd_get_future_frame_number(struct dwc2_hsotg *hsotg, int us);
+int dwc2_hcd_restore(struct dwc2_hsotg *hsotg);
 void dwc2_hcd_connect(struct dwc2_hsotg *hsotg);
 void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force);
 void dwc2_hcd_start(struct dwc2_hsotg *hsotg);
@@ -1529,6 +1528,8 @@ static inline int dwc2_hcd_get_frame_number(struct dwc2_hsotg *hsotg)
 static inline int dwc2_hcd_get_future_frame_number(struct dwc2_hsotg *hsotg,
 						   int us)
 { return 0; }
+static inline int dwc2_hcd_restore(struct dwc2_hsotg *hsotg)
+{ return 0; }
 static inline void dwc2_hcd_connect(struct dwc2_hsotg *hsotg) {}
 static inline void dwc2_hcd_disconnect(struct dwc2_hsotg *hsotg, bool force) {}
 static inline void dwc2_hcd_start(struct dwc2_hsotg *hsotg) {}
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 6c8a9f05fff0..225442471aa9 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -5200,11 +5200,12 @@ int dwc2_hsotg_suspend(struct dwc2_hsotg *hsotg)
 	return 0;
 }
 
-int dwc2_hsotg_resume(struct dwc2_hsotg *hsotg)
+int dwc2_hsotg_resume(struct dwc2_hsotg *hsotg, bool need_restore)
 {
 	unsigned long flags;
+	int ep;
 
-	if (hsotg->lx_state == DWC2_L2)
+	if (!need_restore && hsotg->lx_state == DWC2_L2)
 		return 0;
 
 	if (hsotg->driver) {
@@ -5212,6 +5213,19 @@ int dwc2_hsotg_resume(struct dwc2_hsotg *hsotg)
 			 hsotg->driver->driver.name);
 
 		spin_lock_irqsave(&hsotg->lock, flags);
+		if (need_restore) {
+			hsotg->hibernated = 0;
+			hsotg->in_ppd = 0;
+			hsotg->bus_suspended = false;
+			dwc2_hsotg_disconnect(hsotg);
+			for (ep = 1; ep < hsotg->num_of_eps; ep++) {
+				if (hsotg->eps_in[ep])
+					dwc2_hsotg_ep_disable(&hsotg->eps_in[ep]->ep);
+				if (hsotg->eps_out[ep])
+					dwc2_hsotg_ep_disable(&hsotg->eps_out[ep]->ep);
+			}
+			hsotg->gadget.speed = USB_SPEED_UNKNOWN;
+		}
 		dwc2_hsotg_core_init_disconnected(hsotg, false);
 		if (hsotg->enabled) {
 			/* Enable ACG feature in device mode,if supported */
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index d5275cd658d4..3f9adfa7c630 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -2302,6 +2302,53 @@ static void dwc2_hcd_reinit(struct dwc2_hsotg *hsotg)
 	dwc2_core_host_init(hsotg);
 }
 
+/**
+ * dwc2_hcd_restore - reinitialize host state after register loss
+ * @hsotg: Programming view of the DWC_otg controller
+ *
+ * Return: 0 on success, or a negative error code on failure.
+ */
+int dwc2_hcd_restore(struct dwc2_hsotg *hsotg)
+{
+	struct usb_hcd *hcd = dwc2_hsotg_to_hcd(hsotg);
+	unsigned long flags;
+	int ret;
+
+	clear_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+	dwc2_disable_global_interrupts(hsotg);
+
+	if (!IS_ERR_OR_NULL(hsotg->uphy))
+		usb_phy_set_suspend(hsotg->uphy, false);
+
+	ret = dwc2_vbus_supply_init(hsotg);
+	if (ret)
+		return ret;
+
+	ret = dwc2_core_init(hsotg, true);
+	if (ret) {
+		dwc2_vbus_supply_exit(hsotg);
+		return ret;
+	}
+
+	spin_lock_irqsave(&hsotg->lock, flags);
+	dwc2_hcd_reinit(hsotg);
+	hsotg->gr_backup.valid = false;
+	hsotg->hr_backup.valid = false;
+	hsotg->dr_backup.valid = false;
+	hsotg->hibernated = 0;
+	hsotg->in_ppd = 0;
+	hsotg->bus_suspended = false;
+	hsotg->lx_state = DWC2_L0;
+	hsotg->flags.b.port_connect_status_change = 1;
+	hsotg->flags.b.port_suspend_change = 1;
+	set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
+	spin_unlock_irqrestore(&hsotg->lock, flags);
+
+	dwc2_enable_global_interrupts(hsotg);
+
+	return 0;
+}
+
 static void dwc2_hc_init_split(struct dwc2_hsotg *hsotg,
 			       struct dwc2_host_chan *chan,
 			       struct dwc2_qtd *qtd, struct dwc2_hcd_urb *urb)
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index ef0d73077034..72214593f431 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -692,14 +692,6 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
 		regulator_disable(dwc2->usb33d);
 	}
 
-	if (is_device_mode)
-		ret = dwc2_gadget_backup_critical_registers(dwc2);
-	else
-		ret = dwc2_host_backup_critical_registers(dwc2);
-
-	if (ret)
-		return ret;
-
 	if (dwc2->ll_hw_enabled &&
 	    (is_device_mode || dwc2_host_can_poweroff_phy(dwc2))) {
 		ret = __dwc2_lowlevel_hw_disable(dwc2);
@@ -709,27 +701,10 @@ static int __maybe_unused dwc2_suspend(struct device *dev)
 	return ret;
 }
 
-static int dwc2_restore_critical_registers(struct dwc2_hsotg *hsotg)
-{
-	struct dwc2_gregs_backup *gr;
-
-	gr = &hsotg->gr_backup;
-
-	if (!gr->valid) {
-		dev_err(hsotg->dev, "No valid register backup, failed to restore\n");
-		return -EINVAL;
-	}
-
-	if (gr->gintsts & GINTSTS_CURMODE_HOST)
-		return dwc2_host_restore_critical_registers(hsotg);
-
-	return dwc2_gadget_restore_critical_registers(hsotg, DWC2_RESTORE_DCTL |
-						      DWC2_RESTORE_DCFG);
-}
-
 static int __maybe_unused dwc2_resume(struct device *dev)
 {
 	struct dwc2_hsotg *dwc2 = dev_get_drvdata(dev);
+	bool need_restore;
 	int ret = 0;
 
 	if (!dwc2->ll_hw_enabled)
@@ -742,18 +717,6 @@ static int __maybe_unused dwc2_resume(struct device *dev)
 	}
 	dwc2->phy_off_for_suspend = false;
 
-	/*
-	 * During suspend it's possible that the power domain for the
-	 * DWC2 controller is disabled and all register values get lost.
-	 * In case the GUSBCFG register is not initialized, it's clear the
-	 * registers must be restored.
-	 */
-	if (!(dwc2_readl(dwc2, GUSBCFG) & GUSBCFG_TOUTCAL_MASK)) {
-		ret = dwc2_restore_critical_registers(dwc2);
-		if (ret)
-			return ret;
-	}
-
 	if (dwc2->params.activate_stm_id_vb_detection) {
 		unsigned long flags;
 		u32 ggpio, gotgctl;
@@ -786,8 +749,31 @@ static int __maybe_unused dwc2_resume(struct device *dev)
 		dwc2_drd_resume(dwc2);
 	}
 
+	/*
+	 * During suspend it's possible that the power domain for the
+	 * DWC2 controller is disabled and all register values get lost.
+	 * In case the GUSBCFG register is not initialized, it's clear the
+	 * registers must be restored. So invoke here the necessary init
+	 * as done by dwc2_hcd_init() during probe.
+	 */
+	need_restore = !(dwc2_readl(dwc2, GUSBCFG) & GUSBCFG_TOUTCAL_MASK);
+	if (need_restore) {
+		if (dwc2->dr_mode != USB_DR_MODE_PERIPHERAL) {
+			if (dwc2_is_device_mode(dwc2)) {
+				ret = dwc2_core_init(dwc2, true);
+				if (ret)
+					return ret;
+				dwc2_enable_global_interrupts(dwc2);
+			} else {
+				ret = dwc2_hcd_restore(dwc2);
+				if (ret)
+					return ret;
+			}
+		}
+	}
+
 	if (dwc2_is_device_mode(dwc2))
-		ret = dwc2_hsotg_resume(dwc2);
+		ret = dwc2_hsotg_resume(dwc2, need_restore);
 
 	return ret;
 }

-- 
2.43.0