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

Stefan Wahren wahrenst at gmx.net
Mon Aug 12 23:47:31 UTC 2024


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 at gmx.net>
> ---
>
> 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
>



More information about the dri-devel mailing list