Re: [PATCH V2 14/16] WIP: usb: dwc2: Implement recovery after PM domain off

Stefan Wahren <wahrenst@xxxxxxx> · Tue, 13 Aug 2024 01:47:31 +0200

Hi Doug,

Am 28.07.24 um 15:00 schrieb Stefan Wahren:
DO NOT MERGE

According to the dt-bindings there are some platforms, which have a
dedicated USB power domain for DWC2 IP core supply. If the power domain
is switched off during system suspend then all USB register will lose
their settings.

So use the power on/off notifier in order to save & restore the USB
registers during system suspend.
sorry for bothering you with this DWC2 stuff, but it would great if you
can gave some feedback about this patch. I was working a lot to get
suspend to idle working on Raspberry Pi. And this patch is the most
complex part of the series.

Would you agree with this approach or did i miss something?

The problem is that the power domain driver acts independent from dwc2,
so we cannot prevent the USB domain power down except declaring a USB
device as wakeup source. So i decided to use the notifier approach. This
has been successful tested on some older Raspberry Pi boards.

Best regards

Signed-off-by: Stefan Wahren <wahrenst@xxxxxxx>
---

Any feedback is appreciated.

  drivers/usb/dwc2/core.c     | 16 ++++++++++++
  drivers/usb/dwc2/core.h     | 17 +++++++++++++
  drivers/usb/dwc2/gadget.c   | 49 +++++++++++++++++++++++++++++++++++++
  drivers/usb/dwc2/hcd.c      | 49 +++++++++++++++++++++++++++++++++++++
  drivers/usb/dwc2/platform.c | 32 ++++++++++++++++++++++++
  5 files changed, 163 insertions(+)

diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 9919ab725d54..a3263cfdedac 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -391,6 +391,22 @@ int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, int rem_wakeup,
  		return dwc2_gadget_exit_hibernation(hsotg, rem_wakeup, reset);
  }

+int dwc2_enter_poweroff(struct dwc2_hsotg *hsotg)
+{
+	if (dwc2_is_host_mode(hsotg))
+		return dwc2_host_enter_poweroff(hsotg);
+	else
+		return dwc2_gadget_enter_poweroff(hsotg);
+}
+
+int dwc2_exit_poweroff(struct dwc2_hsotg *hsotg)
+{
+	if (dwc2_is_host_mode(hsotg))
+		return dwc2_host_exit_poweroff(hsotg);
+	else
+		return dwc2_gadget_exit_poweroff(hsotg);
+}
+
  /*
   * Do core a soft reset of the core.  Be careful with this because it
   * resets all the internal state machines of the core.
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 2bd74f3033ed..9ab755cc3081 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -9,6 +9,7 @@
  #define __DWC2_CORE_H__

  #include <linux/acpi.h>
+#include <linux/notifier.h>
  #include <linux/phy/phy.h>
  #include <linux/regulator/consumer.h>
  #include <linux/usb/gadget.h>
@@ -1080,6 +1081,8 @@ struct dwc2_hsotg {
  	struct regulator *vbus_supply;
  	struct regulator *usb33d;

+	struct notifier_block genpd_nb;
+
  	spinlock_t lock;
  	void *priv;
  	int     irq;
@@ -1316,6 +1319,8 @@ int dwc2_exit_partial_power_down(struct dwc2_hsotg *hsotg, int rem_wakeup,
  int dwc2_enter_hibernation(struct dwc2_hsotg *hsotg, int is_host);
  int dwc2_exit_hibernation(struct dwc2_hsotg *hsotg, int rem_wakeup,
  		int reset, int is_host);
+int dwc2_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_exit_poweroff(struct dwc2_hsotg *hsotg);
  void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg);
  int dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy);

@@ -1435,6 +1440,8 @@ int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg);
  int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg);
  void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg);
  void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_gadget_exit_poweroff(struct dwc2_hsotg *hsotg);
  static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg)
  { hsotg->fifo_map = 0; }
  #else
@@ -1482,6 +1489,10 @@ static inline int dwc2_hsotg_tx_fifo_average_depth(struct dwc2_hsotg *hsotg)
  { return 0; }
  static inline void dwc2_gadget_init_lpm(struct dwc2_hsotg *hsotg) {}
  static inline void dwc2_gadget_program_ref_clk(struct dwc2_hsotg *hsotg) {}
+static inline int dwc2_gadget_enter_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_gadget_exit_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
  static inline void dwc2_clear_fifo_map(struct dwc2_hsotg *hsotg) {}
  #endif

@@ -1505,6 +1516,8 @@ int dwc2_host_exit_partial_power_down(struct dwc2_hsotg *hsotg,
  void dwc2_host_enter_clock_gating(struct dwc2_hsotg *hsotg);
  void dwc2_host_exit_clock_gating(struct dwc2_hsotg *hsotg, int rem_wakeup);
  bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2);
+int dwc2_host_enter_poweroff(struct dwc2_hsotg *hsotg);
+int dwc2_host_exit_poweroff(struct dwc2_hsotg *hsotg);
  static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg)
  { schedule_work(&hsotg->phy_reset_work); }
  #else
@@ -1544,6 +1557,10 @@ static inline void dwc2_host_exit_clock_gating(struct dwc2_hsotg *hsotg,
  					       int rem_wakeup) {}
  static inline bool dwc2_host_can_poweroff_phy(struct dwc2_hsotg *dwc2)
  { return false; }
+static inline int dwc2_host_enter_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
+static inline int dwc2_host_exit_poweroff(struct dwc2_hsotg *hsotg)
+{ return 0; }
  static inline void dwc2_host_schedule_phy_reset(struct dwc2_hsotg *hsotg) {}

  #endif
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index e7bf9cc635be..38f0112970fe 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -5710,3 +5710,52 @@ void dwc2_gadget_exit_clock_gating(struct dwc2_hsotg *hsotg, int rem_wakeup)
  	hsotg->lx_state = DWC2_L0;
  	hsotg->bus_suspended = false;
  }
+
+int dwc2_gadget_enter_poweroff(struct dwc2_hsotg *hsotg)
+{
+	int ret;
+
+	dev_dbg(hsotg->dev, "Entering device power off.\n");
+
+	/* Backup all registers */
+	ret = dwc2_backup_global_registers(hsotg);
+	if (ret) {
+		dev_err(hsotg->dev, "%s: failed to backup global registers\n",
+			__func__);
+		return ret;
+	}
+
+	ret = dwc2_backup_device_registers(hsotg);
+	if (ret) {
+		dev_err(hsotg->dev, "%s: failed to backup device registers\n",
+			__func__);
+		return ret;
+	}
+
+	dev_dbg(hsotg->dev, "Entering device power off completed.\n");
+	return 0;
+}
+
+int dwc2_gadget_exit_poweroff(struct dwc2_hsotg *hsotg)
+{
+	int ret;
+
+	dev_dbg(hsotg->dev, "Exiting device power off.\n");
+
+	ret = dwc2_restore_global_registers(hsotg);
+	if (ret) {
+		dev_err(hsotg->dev, "%s: failed to restore registers\n",
+			__func__);
+		return ret;
+	}
+
+	ret = dwc2_restore_device_registers(hsotg, 0);
+	if (ret) {
+		dev_err(hsotg->dev, "%s: failed to restore device registers\n",
+			__func__);
+		return ret;
+	}
+
+	dev_dbg(hsotg->dev, "Exiting device power off completed.\n");
+	return 0;
+}
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index cb54390e7de4..22afdafb474e 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -5993,3 +5993,52 @@ void dwc2_host_exit_clock_gating(struct dwc2_hsotg *hsotg, int rem_wakeup)
  			  jiffies + msecs_to_jiffies(71));
  	}
  }
+
+int dwc2_host_enter_poweroff(struct dwc2_hsotg *hsotg)
+{
+	int ret;
+
+	dev_dbg(hsotg->dev, "Entering host power off.\n");
+
+	/* Backup all registers */
+	ret = dwc2_backup_global_registers(hsotg);
+	if (ret) {
+		dev_err(hsotg->dev, "%s: failed to backup global registers\n",
+			__func__);
+		return ret;
+	}
+
+	ret = dwc2_backup_host_registers(hsotg);
+	if (ret) {
+		dev_err(hsotg->dev, "%s: failed to backup host registers\n",
+			__func__);
+		return ret;
+	}
+
+	dev_dbg(hsotg->dev, "Entering host power off completed.\n");
+	return 0;
+}
+
+int dwc2_host_exit_poweroff(struct dwc2_hsotg *hsotg)
+{
+	int ret;
+
+	dev_dbg(hsotg->dev, "Exiting host power off.\n");
+
+	ret = dwc2_restore_global_registers(hsotg);
+	if (ret) {
+		dev_err(hsotg->dev, "%s: failed to restore registers\n",
+			__func__);
+		return ret;
+	}
+
+	ret = dwc2_restore_host_registers(hsotg);
+	if (ret) {
+		dev_err(hsotg->dev, "%s: failed to restore host registers\n",
+			__func__);
+		return ret;
+	}
+
+	dev_dbg(hsotg->dev, "Exiting host power off completed.\n");
+	return 0;
+}
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index 7b84416dfc2b..b97eefc18a6b 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -16,6 +16,7 @@
  #include <linux/platform_device.h>
  #include <linux/phy/phy.h>
  #include <linux/platform_data/s3c-hsotg.h>
+#include <linux/pm_domain.h>
  #include <linux/reset.h>

  #include <linux/usb/of.h>
@@ -307,6 +308,8 @@ static void dwc2_driver_remove(struct platform_device *dev)
  	struct dwc2_gregs_backup *gr;
  	int ret = 0;

+	dev_pm_genpd_remove_notifier(&dev->dev);
+
  	gr = &hsotg->gr_backup;

  	/* Exit Hibernation when driver is removed. */
@@ -421,6 +424,31 @@ int dwc2_check_core_version(struct dwc2_hsotg *hsotg)
  	return 0;
  }

+static int dwc2_power_notifier(struct notifier_block *nb,
+			       unsigned long action, void *data)
+{
+	struct dwc2_hsotg *hsotg = container_of(nb, struct dwc2_hsotg,
+						genpd_nb);
+	int ret;
+
+	switch (action) {
+	case GENPD_NOTIFY_ON:
+		ret = dwc2_exit_poweroff(hsotg);
+		if (ret)
+			dev_err(hsotg->dev, "exit poweroff failed\n");
+		break;
+	case GENPD_NOTIFY_PRE_OFF:
+		ret = dwc2_enter_poweroff(hsotg);
+		if (ret)
+			dev_err(hsotg->dev, "enter poweroff failed\n");
+		break;
+	default:
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
  /**
   * dwc2_driver_probe() - Called when the DWC_otg core is bound to the DWC_otg
   * driver
@@ -620,6 +648,10 @@ static int dwc2_driver_probe(struct platform_device *dev)
  		}
  	}
  #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */
+
+	hsotg->genpd_nb.notifier_call = dwc2_power_notifier;
+	dev_pm_genpd_add_notifier(&dev->dev, &hsotg->genpd_nb);
+
  	return 0;

  #if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \
--
2.34.1










