[PATCH v2 0/8] add power lost support during system suspend/resume

[PATCH v2 0/8] add power lost support during system suspend/resume

[Xu Yang]

For some SOC, the controller may be powered off during system suspend.
Then, it will not work after system resume without other means. These
patches will add such support if the controller suffer power lost. 

Li Jun (4):
  usb: chipidea: usbmisc: group usbmisc operations for PM
  usb: chipidea: usbmisc: add power lost check for imx6sx
  usb: chipidea: usbmisc: add power lost check for imx7d
  usb: chipidea: usbmisc: add power lost check for imx7ulp

Xu Yang (4):
  usb: chipidea: core: add controller resume support when controller is
    powered off
  usb: chipidea: core: handle suspend/resume for each role
  usb: chipidea: host: add suspend/resume support for host controller
  usb: chipidea: udc: add suspend/resume support for device controller

 drivers/usb/chipidea/ci.h          |   4 +
 drivers/usb/chipidea/ci_hdrc_imx.c |  49 +++------
 drivers/usb/chipidea/ci_hdrc_imx.h |   4 +-
 drivers/usb/chipidea/core.c        |  88 ++++++++++++----
 drivers/usb/chipidea/host.c        |  16 +++
 drivers/usb/chipidea/otg.c         |   2 +-
 drivers/usb/chipidea/otg.h         |   1 +
 drivers/usb/chipidea/udc.c         |  32 ++++++
 drivers/usb/chipidea/usbmisc_imx.c | 160 +++++++++++++++++++++++------
 9 files changed, 270 insertions(+), 86 deletions(-)

-- 
2.34.1

[PATCH v2 1/8] usb: chipidea: core: add controller resume support when controller is powered off

[Xu Yang]

For some SoCs, the controler's power will be off during the system
suspend, and it needs some recovery operation to let the system back
to workable. We add this support in this patch.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- add static modifer for ci_handle_power_lost().
---
 drivers/usb/chipidea/core.c | 80 ++++++++++++++++++++++++++++---------
 drivers/usb/chipidea/otg.c  |  2 +-
 drivers/usb/chipidea/otg.h  |  1 +
 3 files changed, 63 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index ae90fee75a32..80267b973c26 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -637,6 +637,49 @@ static int ci_usb_role_switch_set(struct usb_role_switch *sw,
 	return 0;
 }
 
+static enum ci_role ci_get_role(struct ci_hdrc *ci)
+{
+	enum ci_role role;
+
+	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
+		if (ci->is_otg) {
+			role = ci_otg_role(ci);
+			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
+		} else {
+			/*
+			 * If the controller is not OTG capable, but support
+			 * role switch, the defalt role is gadget, and the
+			 * user can switch it through debugfs.
+			 */
+			role = CI_ROLE_GADGET;
+		}
+	} else {
+		role = ci->roles[CI_ROLE_HOST] ? CI_ROLE_HOST
+					: CI_ROLE_GADGET;
+	}
+
+	return role;
+}
+
+static void ci_handle_power_lost(struct ci_hdrc *ci)
+{
+	enum ci_role role;
+
+	disable_irq_nosync(ci->irq);
+	if (!ci_otg_is_fsm_mode(ci)) {
+		role = ci_get_role(ci);
+
+		if (ci->role != role) {
+			ci_handle_id_switch(ci);
+		} else if (role == CI_ROLE_GADGET) {
+			if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV))
+				usb_gadget_vbus_connect(&ci->gadget);
+		}
+	}
+
+	enable_irq(ci->irq);
+}
+
 static struct usb_role_switch_desc ci_role_switch = {
 	.set = ci_usb_role_switch_set,
 	.get = ci_usb_role_switch_get,
@@ -1134,25 +1177,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
 		}
 	}
 
-	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
-		if (ci->is_otg) {
-			ci->role = ci_otg_role(ci);
-			/* Enable ID change irq */
-			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
-		} else {
-			/*
-			 * If the controller is not OTG capable, but support
-			 * role switch, the defalt role is gadget, and the
-			 * user can switch it through debugfs.
-			 */
-			ci->role = CI_ROLE_GADGET;
-		}
-	} else {
-		ci->role = ci->roles[CI_ROLE_HOST]
-			? CI_ROLE_HOST
-			: CI_ROLE_GADGET;
-	}
-
+	ci->role = ci_get_role(ci);
 	if (!ci_otg_is_fsm_mode(ci)) {
 		/* only update vbus status for peripheral */
 		if (ci->role == CI_ROLE_GADGET) {
@@ -1374,8 +1399,16 @@ static int ci_suspend(struct device *dev)
 static int ci_resume(struct device *dev)
 {
 	struct ci_hdrc *ci = dev_get_drvdata(dev);
+	bool power_lost;
 	int ret;
 
+	/* Since ASYNCLISTADDR (host mode) and ENDPTLISTADDR (device
+	 * mode) share the same register address. We can check if
+	 * controller resume from power lost based on this address
+	 * due to this register will be reset after power lost.
+	 */
+	power_lost = !hw_read(ci, OP_ENDPTLISTADDR, ~0);
+
 	if (device_may_wakeup(dev))
 		disable_irq_wake(ci->irq);
 
@@ -1383,6 +1416,15 @@ static int ci_resume(struct device *dev)
 	if (ret)
 		return ret;
 
+	if (power_lost) {
+		/* shutdown and re-init for phy */
+		ci_usb_phy_exit(ci);
+		ci_usb_phy_init(ci);
+	}
+
+	if (power_lost)
+		ci_handle_power_lost(ci);
+
 	if (ci->supports_runtime_pm) {
 		pm_runtime_disable(dev);
 		pm_runtime_set_active(dev);
diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
index 7b53274ef966..622c3b68aa1e 100644
--- a/drivers/usb/chipidea/otg.c
+++ b/drivers/usb/chipidea/otg.c
@@ -165,7 +165,7 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
 	return 0;
 }
 
-static void ci_handle_id_switch(struct ci_hdrc *ci)
+void ci_handle_id_switch(struct ci_hdrc *ci)
 {
 	enum ci_role role = ci_otg_role(ci);
 
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 5e7a6e571dd2..87629b81e03e 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -14,6 +14,7 @@ int ci_hdrc_otg_init(struct ci_hdrc *ci);
 void ci_hdrc_otg_destroy(struct ci_hdrc *ci);
 enum ci_role ci_otg_role(struct ci_hdrc *ci);
 void ci_handle_vbus_change(struct ci_hdrc *ci);
+void ci_handle_id_switch(struct ci_hdrc *ci);
 static inline void ci_otg_queue_work(struct ci_hdrc *ci)
 {
 	disable_irq_nosync(ci->irq);
-- 
2.34.1

Re: [PATCH v2 1/8] usb: chipidea: core: add controller resume support when controller is powered off

[Peter Chen]

On 22-10-13 23:14:35, Xu Yang wrote:
> For some SoCs, the controler's power will be off during the system
> suspend, and it needs some recovery operation to let the system back
> to workable. We add this support in this patch.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter

> 
> ---
> Changes since v1:
> - add static modifer for ci_handle_power_lost().
> ---
>  drivers/usb/chipidea/core.c | 80 ++++++++++++++++++++++++++++---------
>  drivers/usb/chipidea/otg.c  |  2 +-
>  drivers/usb/chipidea/otg.h  |  1 +
>  3 files changed, 63 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index ae90fee75a32..80267b973c26 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -637,6 +637,49 @@ static int ci_usb_role_switch_set(struct usb_role_switch *sw,
>  	return 0;
>  }
>  
> +static enum ci_role ci_get_role(struct ci_hdrc *ci)
> +{
> +	enum ci_role role;
> +
> +	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> +		if (ci->is_otg) {
> +			role = ci_otg_role(ci);
> +			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> +		} else {
> +			/*
> +			 * If the controller is not OTG capable, but support
> +			 * role switch, the defalt role is gadget, and the
> +			 * user can switch it through debugfs.
> +			 */
> +			role = CI_ROLE_GADGET;
> +		}
> +	} else {
> +		role = ci->roles[CI_ROLE_HOST] ? CI_ROLE_HOST
> +					: CI_ROLE_GADGET;
> +	}
> +
> +	return role;
> +}
> +
> +static void ci_handle_power_lost(struct ci_hdrc *ci)
> +{
> +	enum ci_role role;
> +
> +	disable_irq_nosync(ci->irq);
> +	if (!ci_otg_is_fsm_mode(ci)) {
> +		role = ci_get_role(ci);
> +
> +		if (ci->role != role) {
> +			ci_handle_id_switch(ci);
> +		} else if (role == CI_ROLE_GADGET) {
> +			if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV))
> +				usb_gadget_vbus_connect(&ci->gadget);
> +		}
> +	}
> +
> +	enable_irq(ci->irq);
> +}
> +
>  static struct usb_role_switch_desc ci_role_switch = {
>  	.set = ci_usb_role_switch_set,
>  	.get = ci_usb_role_switch_get,
> @@ -1134,25 +1177,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> -		if (ci->is_otg) {
> -			ci->role = ci_otg_role(ci);
> -			/* Enable ID change irq */
> -			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> -		} else {
> -			/*
> -			 * If the controller is not OTG capable, but support
> -			 * role switch, the defalt role is gadget, and the
> -			 * user can switch it through debugfs.
> -			 */
> -			ci->role = CI_ROLE_GADGET;
> -		}
> -	} else {
> -		ci->role = ci->roles[CI_ROLE_HOST]
> -			? CI_ROLE_HOST
> -			: CI_ROLE_GADGET;
> -	}
> -
> +	ci->role = ci_get_role(ci);
>  	if (!ci_otg_is_fsm_mode(ci)) {
>  		/* only update vbus status for peripheral */
>  		if (ci->role == CI_ROLE_GADGET) {
> @@ -1374,8 +1399,16 @@ static int ci_suspend(struct device *dev)
>  static int ci_resume(struct device *dev)
>  {
>  	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	bool power_lost;
>  	int ret;
>  
> +	/* Since ASYNCLISTADDR (host mode) and ENDPTLISTADDR (device
> +	 * mode) share the same register address. We can check if
> +	 * controller resume from power lost based on this address
> +	 * due to this register will be reset after power lost.
> +	 */
> +	power_lost = !hw_read(ci, OP_ENDPTLISTADDR, ~0);
> +
>  	if (device_may_wakeup(dev))
>  		disable_irq_wake(ci->irq);
>  
> @@ -1383,6 +1416,15 @@ static int ci_resume(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> +	if (power_lost) {
> +		/* shutdown and re-init for phy */
> +		ci_usb_phy_exit(ci);
> +		ci_usb_phy_init(ci);
> +	}
> +
> +	if (power_lost)
> +		ci_handle_power_lost(ci);
> +
>  	if (ci->supports_runtime_pm) {
>  		pm_runtime_disable(dev);
>  		pm_runtime_set_active(dev);
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 7b53274ef966..622c3b68aa1e 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -165,7 +165,7 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
>  	return 0;
>  }
>  
> -static void ci_handle_id_switch(struct ci_hdrc *ci)
> +void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
>  	enum ci_role role = ci_otg_role(ci);
>  
> diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
> index 5e7a6e571dd2..87629b81e03e 100644
> --- a/drivers/usb/chipidea/otg.h
> +++ b/drivers/usb/chipidea/otg.h
> @@ -14,6 +14,7 @@ int ci_hdrc_otg_init(struct ci_hdrc *ci);
>  void ci_hdrc_otg_destroy(struct ci_hdrc *ci);
>  enum ci_role ci_otg_role(struct ci_hdrc *ci);
>  void ci_handle_vbus_change(struct ci_hdrc *ci);
> +void ci_handle_id_switch(struct ci_hdrc *ci);
>  static inline void ci_otg_queue_work(struct ci_hdrc *ci)
>  {
>  	disable_irq_nosync(ci->irq);
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

Re: [PATCH v2 1/8] usb: chipidea: core: add controller resume support when controller is powered off

[Conor Dooley]

On Thu, Oct 13, 2022 at 11:14:35PM +0800, Xu Yang wrote:
> For some SoCs, the controler's power will be off during the system
> suspend, and it needs some recovery operation to let the system back
> to workable. We add this support in this patch.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes since v1:
> - add static modifer for ci_handle_power_lost().
> ---
>  drivers/usb/chipidea/core.c | 80 ++++++++++++++++++++++++++++---------
>  drivers/usb/chipidea/otg.c  |  2 +-
>  drivers/usb/chipidea/otg.h  |  1 +
>  3 files changed, 63 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index ae90fee75a32..80267b973c26 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -637,6 +637,49 @@ static int ci_usb_role_switch_set(struct usb_role_switch *sw,
>  	return 0;
>  }
>  
> +static enum ci_role ci_get_role(struct ci_hdrc *ci)
> +{
> +	enum ci_role role;
> +
> +	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> +		if (ci->is_otg) {
> +			role = ci_otg_role(ci);
> +			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> +		} else {
> +			/*
> +			 * If the controller is not OTG capable, but support
> +			 * role switch, the defalt role is gadget, and the
> +			 * user can switch it through debugfs.
> +			 */
> +			role = CI_ROLE_GADGET;
> +		}
> +	} else {
> +		role = ci->roles[CI_ROLE_HOST] ? CI_ROLE_HOST
> +					: CI_ROLE_GADGET;
> +	}
> +
> +	return role;
> +}
> +
> +static void ci_handle_power_lost(struct ci_hdrc *ci)

Hey,

This appears to have landed in -next and is breaking allmodconfig for
RISC-V:
../drivers/usb/chipidea/core.c:664:13: error: 'ci_handle_power_lost' defined but not used [-Werror=unused-function]
  664 | static void ci_handle_power_lost(struct ci_hdrc *ci)
      |             ^~~~~~~~~~~~~~~~~~~~
  CC [M]  drivers/media/i2c/mt9t001.o
  CC [M]  drivers/net/ethernet/davicom/dm9051.o
  CC [M]  drivers/input/touchscreen/ti_am335x_tsc.o
  CC [M]  drivers/watchdog/wdt_pci.o
cc1: all warnings being treated as errors
make[5]: *** [../scripts/Makefile.build:250: drivers/usb/chipidea/core.o] Error 1
make[4]: *** [../scripts/Makefile.build:500: drivers/usb/chipidea] Error 2
make[3]: *** [../scripts/Makefile.build:500: drivers/usb] Error 2
make[3]: *** Waiting for unfinished jobs....

The only user seems to be wrapped in an #ifdef CONFIG_PM_SLEEP, and
while I haven't had the chance to investigate further yet that's
probably where I'd start looking.
Apologies if it's been reported, I had a quick look on lore but didn't
see anything.

Thanks,
Conor.

> +{
> +	enum ci_role role;
> +
> +	disable_irq_nosync(ci->irq);
> +	if (!ci_otg_is_fsm_mode(ci)) {
> +		role = ci_get_role(ci);
> +
> +		if (ci->role != role) {
> +			ci_handle_id_switch(ci);
> +		} else if (role == CI_ROLE_GADGET) {
> +			if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV))
> +				usb_gadget_vbus_connect(&ci->gadget);
> +		}
> +	}
> +
> +	enable_irq(ci->irq);
> +}
> +
>  static struct usb_role_switch_desc ci_role_switch = {
>  	.set = ci_usb_role_switch_set,
>  	.get = ci_usb_role_switch_get,
> @@ -1134,25 +1177,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> -		if (ci->is_otg) {
> -			ci->role = ci_otg_role(ci);
> -			/* Enable ID change irq */
> -			hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> -		} else {
> -			/*
> -			 * If the controller is not OTG capable, but support
> -			 * role switch, the defalt role is gadget, and the
> -			 * user can switch it through debugfs.
> -			 */
> -			ci->role = CI_ROLE_GADGET;
> -		}
> -	} else {
> -		ci->role = ci->roles[CI_ROLE_HOST]
> -			? CI_ROLE_HOST
> -			: CI_ROLE_GADGET;
> -	}
> -
> +	ci->role = ci_get_role(ci);
>  	if (!ci_otg_is_fsm_mode(ci)) {
>  		/* only update vbus status for peripheral */
>  		if (ci->role == CI_ROLE_GADGET) {
> @@ -1374,8 +1399,16 @@ static int ci_suspend(struct device *dev)
>  static int ci_resume(struct device *dev)
>  {
>  	struct ci_hdrc *ci = dev_get_drvdata(dev);
> +	bool power_lost;
>  	int ret;
>  
> +	/* Since ASYNCLISTADDR (host mode) and ENDPTLISTADDR (device
> +	 * mode) share the same register address. We can check if
> +	 * controller resume from power lost based on this address
> +	 * due to this register will be reset after power lost.
> +	 */
> +	power_lost = !hw_read(ci, OP_ENDPTLISTADDR, ~0);
> +
>  	if (device_may_wakeup(dev))
>  		disable_irq_wake(ci->irq);
>  
> @@ -1383,6 +1416,15 @@ static int ci_resume(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> +	if (power_lost) {
> +		/* shutdown and re-init for phy */
> +		ci_usb_phy_exit(ci);
> +		ci_usb_phy_init(ci);
> +	}
> +
> +	if (power_lost)
> +		ci_handle_power_lost(ci);
> +
>  	if (ci->supports_runtime_pm) {
>  		pm_runtime_disable(dev);
>  		pm_runtime_set_active(dev);
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 7b53274ef966..622c3b68aa1e 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -165,7 +165,7 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
>  	return 0;
>  }
>  
> -static void ci_handle_id_switch(struct ci_hdrc *ci)
> +void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
>  	enum ci_role role = ci_otg_role(ci);
>  
> diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
> index 5e7a6e571dd2..87629b81e03e 100644
> --- a/drivers/usb/chipidea/otg.h
> +++ b/drivers/usb/chipidea/otg.h
> @@ -14,6 +14,7 @@ int ci_hdrc_otg_init(struct ci_hdrc *ci);
>  void ci_hdrc_otg_destroy(struct ci_hdrc *ci);
>  enum ci_role ci_otg_role(struct ci_hdrc *ci);
>  void ci_handle_vbus_change(struct ci_hdrc *ci);
> +void ci_handle_id_switch(struct ci_hdrc *ci);
>  static inline void ci_otg_queue_work(struct ci_hdrc *ci)
>  {
>  	disable_irq_nosync(ci->irq);
> -- 
> 2.34.1
> 
> 

RE: [EXT] Re: [PATCH v2 1/8] usb: chipidea: core: add controller resume support when controller is powered off

[Xu Yang]

Hi Conor,

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Tuesday, October 25, 2022 9:47 PM
> To: Xu Yang <xu.yang_2@nxp.com>
> Cc: peter.chen@kernel.org; Jun Li <jun.li@nxp.com>; gregkh@linuxfoundation.org; linux-usb@vger.kernel.org; dl-linux-
> imx <linux-imx@nxp.com>
> Subject: [EXT] Re: [PATCH v2 1/8] usb: chipidea: core: add controller resume support when controller is powered off
> 
> Caution: EXT Email
> 
> On Thu, Oct 13, 2022 at 11:14:35PM +0800, Xu Yang wrote:
> > For some SoCs, the controler's power will be off during the system
> > suspend, and it needs some recovery operation to let the system back
> > to workable. We add this support in this patch.
> >
> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> >
> > ---
> > Changes since v1:
> > - add static modifer for ci_handle_power_lost().
> > ---
> >  drivers/usb/chipidea/core.c | 80 ++++++++++++++++++++++++++++---------
> >  drivers/usb/chipidea/otg.c  |  2 +-
> >  drivers/usb/chipidea/otg.h  |  1 +
> >  3 files changed, 63 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > index ae90fee75a32..80267b973c26 100644
> > --- a/drivers/usb/chipidea/core.c
> > +++ b/drivers/usb/chipidea/core.c
> > @@ -637,6 +637,49 @@ static int ci_usb_role_switch_set(struct usb_role_switch *sw,
> >       return 0;
> >  }
> >
> > +static enum ci_role ci_get_role(struct ci_hdrc *ci)
> > +{
> > +     enum ci_role role;
> > +
> > +     if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> > +             if (ci->is_otg) {
> > +                     role = ci_otg_role(ci);
> > +                     hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > +             } else {
> > +                     /*
> > +                      * If the controller is not OTG capable, but support
> > +                      * role switch, the defalt role is gadget, and the
> > +                      * user can switch it through debugfs.
> > +                      */
> > +                     role = CI_ROLE_GADGET;
> > +             }
> > +     } else {
> > +             role = ci->roles[CI_ROLE_HOST] ? CI_ROLE_HOST
> > +                                     : CI_ROLE_GADGET;
> > +     }
> > +
> > +     return role;
> > +}
> > +
> > +static void ci_handle_power_lost(struct ci_hdrc *ci)
> 
> Hey,
> 
> This appears to have landed in -next and is breaking allmodconfig for
> RISC-V:
> ../drivers/usb/chipidea/core.c:664:13: error: 'ci_handle_power_lost' defined but not used [-Werror=unused-function]
>   664 | static void ci_handle_power_lost(struct ci_hdrc *ci)
>       |             ^~~~~~~~~~~~~~~~~~~~
>   CC [M]  drivers/media/i2c/mt9t001.o
>   CC [M]  drivers/net/ethernet/davicom/dm9051.o
>   CC [M]  drivers/input/touchscreen/ti_am335x_tsc.o
>   CC [M]  drivers/watchdog/wdt_pci.o
> cc1: all warnings being treated as errors
> make[5]: *** [../scripts/Makefile.build:250: drivers/usb/chipidea/core.o] Error 1
> make[4]: *** [../scripts/Makefile.build:500: drivers/usb/chipidea] Error 2
> make[3]: *** [../scripts/Makefile.build:500: drivers/usb] Error 2
> make[3]: *** Waiting for unfinished jobs....
> 
> The only user seems to be wrapped in an #ifdef CONFIG_PM_SLEEP, and
> while I haven't had the chance to investigate further yet that's
> probably where I'd start looking.
> Apologies if it's been reported, I had a quick look on lore but didn't
> see anything.

Since only ci_resume() will call ci_handle_power_lost(), so it should also
be wrapped in #ifdef CONFIG_PM_SLEEP. Thanks for your report.

Thanks,
Xu Yang

> 
> Thanks,
> Conor.
> 
> > +{
> > +     enum ci_role role;
> > +
> > +     disable_irq_nosync(ci->irq);
> > +     if (!ci_otg_is_fsm_mode(ci)) {
> > +             role = ci_get_role(ci);
> > +
> > +             if (ci->role != role) {
> > +                     ci_handle_id_switch(ci);
> > +             } else if (role == CI_ROLE_GADGET) {
> > +                     if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV))
> > +                             usb_gadget_vbus_connect(&ci->gadget);
> > +             }
> > +     }
> > +
> > +     enable_irq(ci->irq);
> > +}
> > +
> >  static struct usb_role_switch_desc ci_role_switch = {
> >       .set = ci_usb_role_switch_set,
> >       .get = ci_usb_role_switch_get,
> > @@ -1134,25 +1177,7 @@ static int ci_hdrc_probe(struct platform_device *pdev)
> >               }
> >       }
> >
> > -     if (ci->roles[CI_ROLE_HOST] && ci->roles[CI_ROLE_GADGET]) {
> > -             if (ci->is_otg) {
> > -                     ci->role = ci_otg_role(ci);
> > -                     /* Enable ID change irq */
> > -                     hw_write_otgsc(ci, OTGSC_IDIE, OTGSC_IDIE);
> > -             } else {
> > -                     /*
> > -                      * If the controller is not OTG capable, but support
> > -                      * role switch, the defalt role is gadget, and the
> > -                      * user can switch it through debugfs.
> > -                      */
> > -                     ci->role = CI_ROLE_GADGET;
> > -             }
> > -     } else {
> > -             ci->role = ci->roles[CI_ROLE_HOST]
> > -                     ? CI_ROLE_HOST
> > -                     : CI_ROLE_GADGET;
> > -     }
> > -
> > +     ci->role = ci_get_role(ci);
> >       if (!ci_otg_is_fsm_mode(ci)) {
> >               /* only update vbus status for peripheral */
> >               if (ci->role == CI_ROLE_GADGET) {
> > @@ -1374,8 +1399,16 @@ static int ci_suspend(struct device *dev)
> >  static int ci_resume(struct device *dev)
> >  {
> >       struct ci_hdrc *ci = dev_get_drvdata(dev);
> > +     bool power_lost;
> >       int ret;
> >
> > +     /* Since ASYNCLISTADDR (host mode) and ENDPTLISTADDR (device
> > +      * mode) share the same register address. We can check if
> > +      * controller resume from power lost based on this address
> > +      * due to this register will be reset after power lost.
> > +      */
> > +     power_lost = !hw_read(ci, OP_ENDPTLISTADDR, ~0);
> > +
> >       if (device_may_wakeup(dev))
> >               disable_irq_wake(ci->irq);
> >
> > @@ -1383,6 +1416,15 @@ static int ci_resume(struct device *dev)
> >       if (ret)
> >               return ret;
> >
> > +     if (power_lost) {
> > +             /* shutdown and re-init for phy */
> > +             ci_usb_phy_exit(ci);
> > +             ci_usb_phy_init(ci);
> > +     }
> > +
> > +     if (power_lost)
> > +             ci_handle_power_lost(ci);
> > +
> >       if (ci->supports_runtime_pm) {
> >               pm_runtime_disable(dev);
> >               pm_runtime_set_active(dev);
> > diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> > index 7b53274ef966..622c3b68aa1e 100644
> > --- a/drivers/usb/chipidea/otg.c
> > +++ b/drivers/usb/chipidea/otg.c
> > @@ -165,7 +165,7 @@ static int hw_wait_vbus_lower_bsv(struct ci_hdrc *ci)
> >       return 0;
> >  }
> >
> > -static void ci_handle_id_switch(struct ci_hdrc *ci)
> > +void ci_handle_id_switch(struct ci_hdrc *ci)
> >  {
> >       enum ci_role role = ci_otg_role(ci);
> >
> > diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
> > index 5e7a6e571dd2..87629b81e03e 100644
> > --- a/drivers/usb/chipidea/otg.h
> > +++ b/drivers/usb/chipidea/otg.h
> > @@ -14,6 +14,7 @@ int ci_hdrc_otg_init(struct ci_hdrc *ci);
> >  void ci_hdrc_otg_destroy(struct ci_hdrc *ci);
> >  enum ci_role ci_otg_role(struct ci_hdrc *ci);
> >  void ci_handle_vbus_change(struct ci_hdrc *ci);
> > +void ci_handle_id_switch(struct ci_hdrc *ci);
> >  static inline void ci_otg_queue_work(struct ci_hdrc *ci)
> >  {
> >       disable_irq_nosync(ci->irq);
> > --
> > 2.34.1
> >
> >

[PATCH v2 2/8] usb: chipidea: core: handle suspend/resume for each role

[Xu Yang]

There may be a need to handle suspend/resume per role. This patch
will add this support.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- No changes.
---
 drivers/usb/chipidea/ci.h   | 4 ++++
 drivers/usb/chipidea/core.c | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
index a4a3be049910..005c67cb3afb 100644
--- a/drivers/usb/chipidea/ci.h
+++ b/drivers/usb/chipidea/ci.h
@@ -127,12 +127,16 @@ enum ci_revision {
  * struct ci_role_driver - host/gadget role driver
  * @start: start this role
  * @stop: stop this role
+ * @suspend: system suspend handler for this role
+ * @resume: system resume handler for this role
  * @irq: irq handler for this role
  * @name: role name string (host/gadget)
  */
 struct ci_role_driver {
 	int		(*start)(struct ci_hdrc *);
 	void		(*stop)(struct ci_hdrc *);
+	void		(*suspend)(struct ci_hdrc *ci);
+	void		(*resume)(struct ci_hdrc *ci, bool power_lost);
 	irqreturn_t	(*irq)(struct ci_hdrc *);
 	const char	*name;
 };
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 80267b973c26..2b170b434d01 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -1383,6 +1383,10 @@ static int ci_suspend(struct device *dev)
 		return 0;
 	}
 
+	/* Extra routine per role before system suspend */
+	if (ci->role != CI_ROLE_END && ci_role(ci)->suspend)
+		ci_role(ci)->suspend(ci);
+
 	if (device_may_wakeup(dev)) {
 		if (ci_otg_is_fsm_mode(ci))
 			ci_otg_fsm_suspend_for_srp(ci);
@@ -1422,6 +1426,10 @@ static int ci_resume(struct device *dev)
 		ci_usb_phy_init(ci);
 	}
 
+	/* Extra routine per role after system resume */
+	if (ci->role != CI_ROLE_END && ci_role(ci)->resume)
+		ci_role(ci)->resume(ci, power_lost);
+
 	if (power_lost)
 		ci_handle_power_lost(ci);
 
-- 
2.34.1

Re: [PATCH v2 2/8] usb: chipidea: core: handle suspend/resume for each role

[Peter Chen]

On 22-10-13 23:14:36, Xu Yang wrote:
> There may be a need to handle suspend/resume per role. This patch
> will add this support.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
> Changes since v1:
> - No changes.
> ---
>  drivers/usb/chipidea/ci.h   | 4 ++++
>  drivers/usb/chipidea/core.c | 8 ++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index a4a3be049910..005c67cb3afb 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -127,12 +127,16 @@ enum ci_revision {
>   * struct ci_role_driver - host/gadget role driver
>   * @start: start this role
>   * @stop: stop this role
> + * @suspend: system suspend handler for this role
> + * @resume: system resume handler for this role
>   * @irq: irq handler for this role
>   * @name: role name string (host/gadget)
>   */
>  struct ci_role_driver {
>  	int		(*start)(struct ci_hdrc *);
>  	void		(*stop)(struct ci_hdrc *);
> +	void		(*suspend)(struct ci_hdrc *ci);
> +	void		(*resume)(struct ci_hdrc *ci, bool power_lost);
>  	irqreturn_t	(*irq)(struct ci_hdrc *);
>  	const char	*name;
>  };
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 80267b973c26..2b170b434d01 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -1383,6 +1383,10 @@ static int ci_suspend(struct device *dev)
>  		return 0;
>  	}
>  
> +	/* Extra routine per role before system suspend */
> +	if (ci->role != CI_ROLE_END && ci_role(ci)->suspend)
> +		ci_role(ci)->suspend(ci);
> +
>  	if (device_may_wakeup(dev)) {
>  		if (ci_otg_is_fsm_mode(ci))
>  			ci_otg_fsm_suspend_for_srp(ci);
> @@ -1422,6 +1426,10 @@ static int ci_resume(struct device *dev)
>  		ci_usb_phy_init(ci);
>  	}
>  
> +	/* Extra routine per role after system resume */
> +	if (ci->role != CI_ROLE_END && ci_role(ci)->resume)
> +		ci_role(ci)->resume(ci, power_lost);
> +
>  	if (power_lost)
>  		ci_handle_power_lost(ci);
>  
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

[PATCH v2 3/8] usb: chipidea: host: add suspend/resume support for host controller

[Xu Yang]

The controller's power may be powered off during system suspend. This
will add suspend/resume support when the controller suffers power lost.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- wrap suspend/resume functions with CONFIG_PM_SLEEP.
---
 drivers/usb/chipidea/host.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index bc3634a54c6b..ebe7400243b1 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -459,6 +459,18 @@ static void ci_hdrc_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
 	ci_hdrc_free_dma_aligned_buffer(urb);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static void ci_hdrc_host_suspend(struct ci_hdrc *ci)
+{
+	ehci_suspend(ci->hcd, device_may_wakeup(ci->dev));
+}
+
+static void ci_hdrc_host_resume(struct ci_hdrc *ci, bool power_lost)
+{
+	ehci_resume(ci->hcd, power_lost);
+}
+#endif
+
 int ci_hdrc_host_init(struct ci_hdrc *ci)
 {
 	struct ci_role_driver *rdrv;
@@ -472,6 +484,10 @@ int ci_hdrc_host_init(struct ci_hdrc *ci)
 
 	rdrv->start	= host_start;
 	rdrv->stop	= host_stop;
+#ifdef CONFIG_PM_SLEEP
+	rdrv->suspend	= ci_hdrc_host_suspend;
+	rdrv->resume	= ci_hdrc_host_resume;
+#endif
 	rdrv->irq	= host_irq;
 	rdrv->name	= "host";
 	ci->roles[CI_ROLE_HOST] = rdrv;
-- 
2.34.1

Re: [PATCH v2 3/8] usb: chipidea: host: add suspend/resume support for host controller

[Peter Chen]

On 22-10-13 23:14:37, Xu Yang wrote:
> The controller's power may be powered off during system suspend. This
> will add suspend/resume support when the controller suffers power lost.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes since v1:
> - wrap suspend/resume functions with CONFIG_PM_SLEEP.
> ---
>  drivers/usb/chipidea/host.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index bc3634a54c6b..ebe7400243b1 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -459,6 +459,18 @@ static void ci_hdrc_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
>  	ci_hdrc_free_dma_aligned_buffer(urb);
>  }
>  
> +#ifdef CONFIG_PM_SLEEP
> +static void ci_hdrc_host_suspend(struct ci_hdrc *ci)
> +{
> +	ehci_suspend(ci->hcd, device_may_wakeup(ci->dev));
> +}
> +
> +static void ci_hdrc_host_resume(struct ci_hdrc *ci, bool power_lost)
> +{
> +	ehci_resume(ci->hcd, power_lost);
> +}
> +#endif
> +

This code seems doesn't the same with the code at source.codeaurora.org,
please make sure it could work for all platforms.

Peter

>  int ci_hdrc_host_init(struct ci_hdrc *ci)
>  {
>  	struct ci_role_driver *rdrv;
> @@ -472,6 +484,10 @@ int ci_hdrc_host_init(struct ci_hdrc *ci)
>  
>  	rdrv->start	= host_start;
>  	rdrv->stop	= host_stop;
> +#ifdef CONFIG_PM_SLEEP
> +	rdrv->suspend	= ci_hdrc_host_suspend;
> +	rdrv->resume	= ci_hdrc_host_resume;
> +#endif
>  	rdrv->irq	= host_irq;
>  	rdrv->name	= "host";
>  	ci->roles[CI_ROLE_HOST] = rdrv;
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

[PATCH v2 4/8] usb: chipidea: udc: add suspend/resume support for device controller

[Xu Yang]

The controller's power may be powered off during system suspend. This
will add suspend/resume support when the controller suffers power lost.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes since v1:
- wrap suspend/resume functions with CONFIG_PM_SLEEP.
---
 drivers/usb/chipidea/udc.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 8c3e3a635ac2..54c09245ad05 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -2181,6 +2181,34 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
 				     ci->platdata->pins_default);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static void udc_suspend(struct ci_hdrc *ci)
+{
+	/*
+	 * Set OP_ENDPTLISTADDR to be non-zero for
+	 * checking if controller resume from power lost
+	 * in non-host mode.
+	 */
+	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0)
+		hw_write(ci, OP_ENDPTLISTADDR, ~0, ~0);
+}
+
+static void udc_resume(struct ci_hdrc *ci, bool power_lost)
+{
+	if (power_lost) {
+		if (ci->is_otg)
+			hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
+					OTGSC_BSVIS | OTGSC_BSVIE);
+		if (ci->vbus_active)
+			usb_gadget_vbus_disconnect(&ci->gadget);
+	}
+
+	/* Restore value 0 if it was set for power lost check */
+	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0xFFFFFFFF)
+		hw_write(ci, OP_ENDPTLISTADDR, ~0, 0);
+}
+#endif
+
 /**
  * ci_hdrc_gadget_init - initialize device related bits
  * @ci: the controller
@@ -2201,6 +2229,10 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
 
 	rdrv->start	= udc_id_switch_for_device;
 	rdrv->stop	= udc_id_switch_for_host;
+#ifdef CONFIG_PM_SLEEP
+	rdrv->suspend	= udc_suspend;
+	rdrv->resume	= udc_resume;
+#endif
 	rdrv->irq	= udc_irq;
 	rdrv->name	= "gadget";
 
-- 
2.34.1

Re: [PATCH v2 4/8] usb: chipidea: udc: add suspend/resume support for device controller

[Peter Chen]

On 22-10-13 23:14:38, Xu Yang wrote:
> The controller's power may be powered off during system suspend. This
> will add suspend/resume support when the controller suffers power lost.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 

Acked-by: Peter Chen <peter.chen@kernel.org>

Peter
> ---
> Changes since v1:
> - wrap suspend/resume functions with CONFIG_PM_SLEEP.
> ---
>  drivers/usb/chipidea/udc.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 8c3e3a635ac2..54c09245ad05 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -2181,6 +2181,34 @@ static void udc_id_switch_for_host(struct ci_hdrc *ci)
>  				     ci->platdata->pins_default);
>  }
>  
> +#ifdef CONFIG_PM_SLEEP
> +static void udc_suspend(struct ci_hdrc *ci)
> +{
> +	/*
> +	 * Set OP_ENDPTLISTADDR to be non-zero for
> +	 * checking if controller resume from power lost
> +	 * in non-host mode.
> +	 */
> +	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0)
> +		hw_write(ci, OP_ENDPTLISTADDR, ~0, ~0);
> +}
> +
> +static void udc_resume(struct ci_hdrc *ci, bool power_lost)
> +{
> +	if (power_lost) {
> +		if (ci->is_otg)
> +			hw_write_otgsc(ci, OTGSC_BSVIS | OTGSC_BSVIE,
> +					OTGSC_BSVIS | OTGSC_BSVIE);
> +		if (ci->vbus_active)
> +			usb_gadget_vbus_disconnect(&ci->gadget);
> +	}
> +
> +	/* Restore value 0 if it was set for power lost check */
> +	if (hw_read(ci, OP_ENDPTLISTADDR, ~0) == 0xFFFFFFFF)
> +		hw_write(ci, OP_ENDPTLISTADDR, ~0, 0);
> +}
> +#endif
> +
>  /**
>   * ci_hdrc_gadget_init - initialize device related bits
>   * @ci: the controller
> @@ -2201,6 +2229,10 @@ int ci_hdrc_gadget_init(struct ci_hdrc *ci)
>  
>  	rdrv->start	= udc_id_switch_for_device;
>  	rdrv->stop	= udc_id_switch_for_host;
> +#ifdef CONFIG_PM_SLEEP
> +	rdrv->suspend	= udc_suspend;
> +	rdrv->resume	= udc_resume;
> +#endif
>  	rdrv->irq	= udc_irq;
>  	rdrv->name	= "gadget";
>  
> -- 
> 2.34.1
> 

-- 

Thanks,
Peter Chen

[PATCH v2 5/8] usb: chipidea: usbmisc: group usbmisc operations for PM

[Xu Yang]

From: Li Jun <jun.li@nxp.com>

As there maybe more APIs of usbmisc for suspend and resume, group
them into imx_usbmisc_suspend/resume. Besides, introduced .power_lost_check
API, so that proper resume operations can be performed in power lost case.

Signed-off-by: Li Jun <jun.li@nxp.com>

---
Changes since v1:
- No changes.
---
 drivers/usb/chipidea/ci_hdrc_imx.c |  49 ++++--------
 drivers/usb/chipidea/ci_hdrc_imx.h |   4 +-
 drivers/usb/chipidea/usbmisc_imx.c | 119 +++++++++++++++++++++--------
 3 files changed, 106 insertions(+), 66 deletions(-)

diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index 9ffcecd3058c..923f5c00a1d9 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -527,16 +527,19 @@ static void ci_hdrc_imx_shutdown(struct platform_device *pdev)
 	ci_hdrc_imx_remove(pdev);
 }
 
-static int __maybe_unused imx_controller_suspend(struct device *dev)
+static int __maybe_unused imx_controller_suspend(struct device *dev,
+						 pm_message_t msg)
 {
 	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
 	int ret = 0;
 
 	dev_dbg(dev, "at %s\n", __func__);
 
-	ret = imx_usbmisc_hsic_set_clk(data->usbmisc_data, false);
+	ret = imx_usbmisc_suspend(data->usbmisc_data,
+				  PMSG_IS_AUTO(msg) || device_may_wakeup(dev));
 	if (ret) {
-		dev_err(dev, "usbmisc hsic_set_clk failed, ret=%d\n", ret);
+		dev_err(dev,
+			"usbmisc suspend failed, ret=%d\n", ret);
 		return ret;
 	}
 
@@ -549,7 +552,8 @@ static int __maybe_unused imx_controller_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused imx_controller_resume(struct device *dev)
+static int __maybe_unused imx_controller_resume(struct device *dev,
+						pm_message_t msg)
 {
 	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
 	int ret = 0;
@@ -570,22 +574,15 @@ static int __maybe_unused imx_controller_resume(struct device *dev)
 
 	data->in_lpm = false;
 
-	ret = imx_usbmisc_set_wakeup(data->usbmisc_data, false);
+	ret = imx_usbmisc_resume(data->usbmisc_data,
+				 PMSG_IS_AUTO(msg) || device_may_wakeup(dev));
 	if (ret) {
-		dev_err(dev, "usbmisc set_wakeup failed, ret=%d\n", ret);
+		dev_err(dev, "usbmisc resume failed, ret=%d\n", ret);
 		goto clk_disable;
 	}
 
-	ret = imx_usbmisc_hsic_set_clk(data->usbmisc_data, true);
-	if (ret) {
-		dev_err(dev, "usbmisc hsic_set_clk failed, ret=%d\n", ret);
-		goto hsic_set_clk_fail;
-	}
-
 	return 0;
 
-hsic_set_clk_fail:
-	imx_usbmisc_set_wakeup(data->usbmisc_data, true);
 clk_disable:
 	imx_disable_unprepare_clks(dev);
 	return ret;
@@ -601,16 +598,7 @@ static int __maybe_unused ci_hdrc_imx_suspend(struct device *dev)
 		/* The core's suspend doesn't run */
 		return 0;
 
-	if (device_may_wakeup(dev)) {
-		ret = imx_usbmisc_set_wakeup(data->usbmisc_data, true);
-		if (ret) {
-			dev_err(dev, "usbmisc set_wakeup failed, ret=%d\n",
-					ret);
-			return ret;
-		}
-	}
-
-	ret = imx_controller_suspend(dev);
+	ret = imx_controller_suspend(dev, PMSG_SUSPEND);
 	if (ret)
 		return ret;
 
@@ -624,7 +612,7 @@ static int __maybe_unused ci_hdrc_imx_resume(struct device *dev)
 	int ret;
 
 	pinctrl_pm_select_default_state(dev);
-	ret = imx_controller_resume(dev);
+	ret = imx_controller_resume(dev, PMSG_RESUME);
 	if (!ret && data->supports_runtime_pm) {
 		pm_runtime_disable(dev);
 		pm_runtime_set_active(dev);
@@ -637,25 +625,18 @@ static int __maybe_unused ci_hdrc_imx_resume(struct device *dev)
 static int __maybe_unused ci_hdrc_imx_runtime_suspend(struct device *dev)
 {
 	struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
-	int ret;
 
 	if (data->in_lpm) {
 		WARN_ON(1);
 		return 0;
 	}
 
-	ret = imx_usbmisc_set_wakeup(data->usbmisc_data, true);
-	if (ret) {
-		dev_err(dev, "usbmisc set_wakeup failed, ret=%d\n", ret);
-		return ret;
-	}
-
-	return imx_controller_suspend(dev);
+	return imx_controller_suspend(dev, PMSG_AUTO_SUSPEND);
 }
 
 static int __maybe_unused ci_hdrc_imx_runtime_resume(struct device *dev)
 {
-	return imx_controller_resume(dev);
+	return imx_controller_resume(dev, PMSG_AUTO_RESUME);
 }
 
 static const struct dev_pm_ops ci_hdrc_imx_pm_ops = {
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.h b/drivers/usb/chipidea/ci_hdrc_imx.h
index 7daccb9c5006..7135b9a5d913 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.h
+++ b/drivers/usb/chipidea/ci_hdrc_imx.h
@@ -32,9 +32,9 @@ struct imx_usbmisc_data {
 
 int imx_usbmisc_init(struct imx_usbmisc_data *data);
 int imx_usbmisc_init_post(struct imx_usbmisc_data *data);
-int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, bool enabled);
 int imx_usbmisc_hsic_set_connect(struct imx_usbmisc_data *data);
-int imx_usbmisc_hsic_set_clk(struct imx_usbmisc_data *data, bool on);
 int imx_usbmisc_charger_detection(struct imx_usbmisc_data *data, bool connect);
+int imx_usbmisc_suspend(struct imx_usbmisc_data *data, bool wakeup);
+int imx_usbmisc_resume(struct imx_usbmisc_data *data, bool wakeup);
 
 #endif /* __DRIVER_USB_CHIPIDEA_CI_HDRC_IMX_H */
diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index bac0f5458cab..aa815f6d3fe9 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -150,6 +150,8 @@ struct usbmisc_ops {
 	int (*hsic_set_clk)(struct imx_usbmisc_data *data, bool enabled);
 	/* usb charger detection */
 	int (*charger_detection)(struct imx_usbmisc_data *data);
+	/* It's called when system resume from usb power lost */
+	int (*power_lost_check)(struct imx_usbmisc_data *data);
 };
 
 struct imx_usbmisc {
@@ -1009,30 +1011,29 @@ EXPORT_SYMBOL_GPL(imx_usbmisc_init);
 int imx_usbmisc_init_post(struct imx_usbmisc_data *data)
 {
 	struct imx_usbmisc *usbmisc;
+	int ret = 0;
 
 	if (!data)
 		return 0;
 
 	usbmisc = dev_get_drvdata(data->dev);
-	if (!usbmisc->ops->post)
-		return 0;
-	return usbmisc->ops->post(data);
-}
-EXPORT_SYMBOL_GPL(imx_usbmisc_init_post);
-
-int imx_usbmisc_set_wakeup(struct imx_usbmisc_data *data, bool enabled)
-{
-	struct imx_usbmisc *usbmisc;
+	if (usbmisc->ops->post)
+		ret = usbmisc->ops->post(data);
+	if (ret) {
+		dev_err(data->dev, "post init failed, ret=%d\n", ret);
+		return ret;
+	}
 
-	if (!data)
-		return 0;
+	if (usbmisc->ops->set_wakeup)
+		ret = usbmisc->ops->set_wakeup(data, false);
+	if (ret) {
+		dev_err(data->dev, "set_wakeup failed, ret=%d\n", ret);
+		return ret;
+	}
 
-	usbmisc = dev_get_drvdata(data->dev);
-	if (!usbmisc->ops->set_wakeup)
-		return 0;
-	return usbmisc->ops->set_wakeup(data, enabled);
+	return 0;
 }
-EXPORT_SYMBOL_GPL(imx_usbmisc_set_wakeup);
+EXPORT_SYMBOL_GPL(imx_usbmisc_init_post);
 
 int imx_usbmisc_hsic_set_connect(struct imx_usbmisc_data *data)
 {
@@ -1048,20 +1049,6 @@ int imx_usbmisc_hsic_set_connect(struct imx_usbmisc_data *data)
 }
 EXPORT_SYMBOL_GPL(imx_usbmisc_hsic_set_connect);
 
-int imx_usbmisc_hsic_set_clk(struct imx_usbmisc_data *data, bool on)
-{
-	struct imx_usbmisc *usbmisc;
-
-	if (!data)
-		return 0;
-
-	usbmisc = dev_get_drvdata(data->dev);
-	if (!usbmisc->ops->hsic_set_clk || !data->hsic)
-		return 0;
-	return usbmisc->ops->hsic_set_clk(data, on);
-}
-EXPORT_SYMBOL_GPL(imx_usbmisc_hsic_set_clk);
-
 int imx_usbmisc_charger_detection(struct imx_usbmisc_data *data, bool connect)
 {
 	struct imx_usbmisc *usbmisc;
@@ -1094,6 +1081,78 @@ int imx_usbmisc_charger_detection(struct imx_usbmisc_data *data, bool connect)
 }
 EXPORT_SYMBOL_GPL(imx_usbmisc_charger_detection);
 
+int imx_usbmisc_suspend(struct imx_usbmisc_data *data, bool wakeup)
+{
+	struct imx_usbmisc *usbmisc;
+	int ret = 0;
+
+	if (!data)
+		return 0;
+
+	usbmisc = dev_get_drvdata(data->dev);
+
+	if (wakeup && usbmisc->ops->set_wakeup)
+		ret = usbmisc->ops->set_wakeup(data, true);
+	if (ret) {
+		dev_err(data->dev, "set_wakeup failed, ret=%d\n", ret);
+		return ret;
+	}
+
+	if (usbmisc->ops->hsic_set_clk && data->hsic)
+		ret = usbmisc->ops->hsic_set_clk(data, false);
+	if (ret) {
+		dev_err(data->dev, "set_wakeup failed, ret=%d\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_suspend);
+
+int imx_usbmisc_resume(struct imx_usbmisc_data *data, bool wakeup)
+{
+	struct imx_usbmisc *usbmisc;
+	int ret = 0;
+
+	if (!data)
+		return 0;
+
+	usbmisc = dev_get_drvdata(data->dev);
+
+	if (usbmisc->ops->power_lost_check)
+		ret = usbmisc->ops->power_lost_check(data);
+	if (ret > 0) {
+		/* re-init if resume from power lost */
+		ret = imx_usbmisc_init(data);
+		if (ret) {
+			dev_err(data->dev, "re-init failed, ret=%d\n", ret);
+			return ret;
+		}
+	}
+
+	if (wakeup && usbmisc->ops->set_wakeup)
+		ret = usbmisc->ops->set_wakeup(data, false);
+	if (ret) {
+		dev_err(data->dev, "set_wakeup failed, ret=%d\n", ret);
+		return ret;
+	}
+
+	if (usbmisc->ops->hsic_set_clk && data->hsic)
+		ret = usbmisc->ops->hsic_set_clk(data, true);
+	if (ret) {
+		dev_err(data->dev, "set_wakeup failed, ret=%d\n", ret);
+		goto hsic_set_clk_fail;
+	}
+
+	return 0;
+
+hsic_set_clk_fail:
+	if (wakeup && usbmisc->ops->set_wakeup)
+		usbmisc->ops->set_wakeup(data, true);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_resume);
+
 static const struct of_device_id usbmisc_imx_dt_ids[] = {
 	{
 		.compatible = "fsl,imx25-usbmisc",
-- 
2.34.1

[PATCH v2 6/8] usb: chipidea: usbmisc: add power lost check for imx6sx

[Xu Yang]

From: Li Jun <jun.li@nxp.com>

imx6sx mega off can shutdown domain power supply if none of peripheral
in this domain is registered as wakeup source, this patch add related
codes to check if power is lost.

Signed-off-by: Li Jun <jun.li@nxp.com>

---
Changes since v1:
- No changes.
---
 drivers/usb/chipidea/usbmisc_imx.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index aa815f6d3fe9..7bfbfc83cfe3 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -939,6 +939,25 @@ static int usbmisc_imx7ulp_init(struct imx_usbmisc_data *data)
 	return 0;
 }
 
+static int usbmisc_imx6sx_power_lost_check(struct imx_usbmisc_data *data)
+{
+	struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&usbmisc->lock, flags);
+	val = readl(usbmisc->base + data->index * 4);
+	spin_unlock_irqrestore(&usbmisc->lock, flags);
+	/*
+	 * Here use a power on reset value to judge
+	 * if the controller experienced a power lost
+	 */
+	if (val == 0x30001000)
+		return 1;
+	else
+		return 0;
+}
+
 static const struct usbmisc_ops imx25_usbmisc_ops = {
 	.init = usbmisc_imx25_init,
 	.post = usbmisc_imx25_post,
@@ -972,6 +991,7 @@ static const struct usbmisc_ops imx6sx_usbmisc_ops = {
 	.init = usbmisc_imx6sx_init,
 	.hsic_set_connect = usbmisc_imx6_hsic_set_connect,
 	.hsic_set_clk = usbmisc_imx6_hsic_set_clk,
+	.power_lost_check = usbmisc_imx6sx_power_lost_check,
 };
 
 static const struct usbmisc_ops imx7d_usbmisc_ops = {
-- 
2.34.1

[PATCH v2 7/8] usb: chipidea: usbmisc: add power lost check for imx7d

[Xu Yang]

From: Li Jun <jun.li@nxp.com>

imx7d can shutdown domain power supply if none of peripheral in this
domain is registered as wakeup source, this patch add related codes to
check if power is lost.

Signed-off-by: Li Jun <jun.li@nxp.com>

---
Changes since v1:
- No changes.
---
 drivers/usb/chipidea/usbmisc_imx.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index 7bfbfc83cfe3..cc17dcd97856 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -939,6 +939,25 @@ static int usbmisc_imx7ulp_init(struct imx_usbmisc_data *data)
 	return 0;
 }
 
+static int usbmisc_imx7d_power_lost_check(struct imx_usbmisc_data *data)
+{
+	struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
+	unsigned long flags;
+	u32 val;
+
+	spin_lock_irqsave(&usbmisc->lock, flags);
+	val = readl(usbmisc->base);
+	spin_unlock_irqrestore(&usbmisc->lock, flags);
+	/*
+	 * Here use a power on reset value to judge
+	 * if the controller experienced a power lost
+	 */
+	if (val == 0x30001000)
+		return 1;
+	else
+		return 0;
+}
+
 static int usbmisc_imx6sx_power_lost_check(struct imx_usbmisc_data *data)
 {
 	struct imx_usbmisc *usbmisc = dev_get_drvdata(data->dev);
@@ -998,6 +1017,7 @@ static const struct usbmisc_ops imx7d_usbmisc_ops = {
 	.init = usbmisc_imx7d_init,
 	.set_wakeup = usbmisc_imx7d_set_wakeup,
 	.charger_detection = imx7d_charger_detection,
+	.power_lost_check = usbmisc_imx7d_power_lost_check,
 };
 
 static const struct usbmisc_ops imx7ulp_usbmisc_ops = {
-- 
2.34.1

[PATCH v2 8/8] usb: chipidea: usbmisc: add power lost check for imx7ulp

[Xu Yang]

From: Li Jun <jun.li@nxp.com>

imx7ulp can shutdown domain power supply if none of peripheral in this
domain is registered as wakeup source, this patch add related power lost
check API.

Signed-off-by: Li Jun <jun.li@nxp.com>

---
Changes since v1:
- No changes.
---
 drivers/usb/chipidea/usbmisc_imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/chipidea/usbmisc_imx.c b/drivers/usb/chipidea/usbmisc_imx.c
index cc17dcd97856..acdb13316cd0 100644
--- a/drivers/usb/chipidea/usbmisc_imx.c
+++ b/drivers/usb/chipidea/usbmisc_imx.c
@@ -1025,6 +1025,7 @@ static const struct usbmisc_ops imx7ulp_usbmisc_ops = {
 	.set_wakeup = usbmisc_imx7d_set_wakeup,
 	.hsic_set_connect = usbmisc_imx6_hsic_set_connect,
 	.hsic_set_clk = usbmisc_imx6_hsic_set_clk,
+	.power_lost_check = usbmisc_imx7d_power_lost_check,
 };
 
 static inline bool is_imx53_usbmisc(struct imx_usbmisc_data *data)
-- 
2.34.1