Linux-USB Archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] usb: ohci-platform: fix usb disconnect issue after s4
@ 2022-08-30 11:14 Yinbo Zhu
  2022-08-30 12:59 ` Greg Kroah-Hartman
  2022-08-30 14:30 ` Alan Stern
  0 siblings, 2 replies; 3+ messages in thread
From: Yinbo Zhu @ 2022-08-30 11:14 UTC (permalink / raw
  To: Alan Stern, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Greg Kroah-Hartman, Patchwork Bot
  Cc: zhuyinbo

Avoid retaining bogus hardware states during resume-from-hibernation.
Previously we had reset the hardware as part of preparing to reinstate
the snapshot image. But we can do better now with the new PM framework,
since we know exactly which resume operations are from hibernation

According to the commit "cd1965db0" and "6ec4beb5c" that the flag
"hibernated" is for resume-from-hibernation and it should be true when
usb resume from disk.

When this flag "hibernated" is set, the drivers will reset the hardware
to get rid of any existing state and make sure resume from hibernation
re-enumerates everything for ohci.

Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
---
 drivers/usb/host/ohci-platform.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
index 0adae6265127..e733da2cd3b7 100644
--- a/drivers/usb/host/ohci-platform.c
+++ b/drivers/usb/host/ohci-platform.c
@@ -289,7 +289,7 @@ static int ohci_platform_suspend(struct device *dev)
 	return ret;
 }
 
-static int ohci_platform_resume(struct device *dev)
+static int ohci_platform_renew(struct device *dev, bool hibernated)
 {
 	struct usb_hcd *hcd = dev_get_drvdata(dev);
 	struct usb_ohci_pdata *pdata = dev_get_platdata(dev);
@@ -301,7 +301,7 @@ static int ohci_platform_resume(struct device *dev)
 			return err;
 	}
 
-	ohci_resume(hcd, false);
+	ohci_resume(hcd, hibernated);
 
 	pm_runtime_disable(dev);
 	pm_runtime_set_active(dev);
@@ -309,6 +309,16 @@ static int ohci_platform_resume(struct device *dev)
 
 	return 0;
 }
+
+static int ohci_platform_resume(struct device *dev)
+{
+	return ohci_platform_renew(dev, false);
+}
+
+static int ohci_platform_restore(struct device *dev)
+{
+	return ohci_platform_renew(dev, true);
+}
 #endif /* CONFIG_PM_SLEEP */
 
 static const struct of_device_id ohci_platform_ids[] = {
@@ -325,8 +335,16 @@ static const struct platform_device_id ohci_platform_table[] = {
 };
 MODULE_DEVICE_TABLE(platform, ohci_platform_table);
 
-static SIMPLE_DEV_PM_OPS(ohci_platform_pm_ops, ohci_platform_suspend,
-	ohci_platform_resume);
+#ifdef CONFIG_PM_SLEEP
+static const struct dev_pm_ops ohci_platform_pm_ops = {
+	.suspend = ohci_platform_suspend,
+	.resume = ohci_platform_resume,
+	.freeze = ohci_platform_suspend,
+	.thaw = ohci_platform_resume,
+	.poweroff = ohci_platform_suspend,
+	.restore = ohci_platform_restore,
+};
+#endif
 
 static struct platform_driver ohci_platform_driver = {
 	.id_table	= ohci_platform_table,
@@ -335,7 +353,9 @@ static struct platform_driver ohci_platform_driver = {
 	.shutdown	= usb_hcd_platform_shutdown,
 	.driver		= {
 		.name	= "ohci-platform",
+#ifdef CONFIG_PM_SLEEP
 		.pm	= &ohci_platform_pm_ops,
+#endif
 		.of_match_table = ohci_platform_ids,
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	}
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] usb: ohci-platform: fix usb disconnect issue after s4
  2022-08-30 11:14 [PATCH v1] usb: ohci-platform: fix usb disconnect issue after s4 Yinbo Zhu
@ 2022-08-30 12:59 ` Greg Kroah-Hartman
  2022-08-30 14:30 ` Alan Stern
  1 sibling, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2022-08-30 12:59 UTC (permalink / raw
  To: Yinbo Zhu; +Cc: Alan Stern, linux-usb, linux-kernel, Patchwork Bot

On Tue, Aug 30, 2022 at 07:14:49PM +0800, Yinbo Zhu wrote:
> Avoid retaining bogus hardware states during resume-from-hibernation.
> Previously we had reset the hardware as part of preparing to reinstate
> the snapshot image. But we can do better now with the new PM framework,
> since we know exactly which resume operations are from hibernation
> 
> According to the commit "cd1965db0" and "6ec4beb5c" that the flag
> "hibernated" is for resume-from-hibernation and it should be true when
> usb resume from disk.

When writing commit ids in changelogs, please use the recommended
format.  For this, that paragraph would read:

According to commit cd1965db054e ("USB: ohci: move
ohci_pci_{suspend,resume} to ohci-hcd.c") and commit 6ec4beb5c701 ("USB:
new flag for resume-from-hibernation"), the flag "hibernated" is for...

> When this flag "hibernated" is set, the drivers will reset the hardware
> to get rid of any existing state and make sure resume from hibernation
> re-enumerates everything for ohci.
> 
> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>

What commit id does this fix?


> ---
>  drivers/usb/host/ohci-platform.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
> index 0adae6265127..e733da2cd3b7 100644
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -289,7 +289,7 @@ static int ohci_platform_suspend(struct device *dev)
>  	return ret;
>  }
>  
> -static int ohci_platform_resume(struct device *dev)
> +static int ohci_platform_renew(struct device *dev, bool hibernated)

I hate functions like this as it's now impossible to read the caller and
understand what is going on.

>  {
>  	struct usb_hcd *hcd = dev_get_drvdata(dev);
>  	struct usb_ohci_pdata *pdata = dev_get_platdata(dev);
> @@ -301,7 +301,7 @@ static int ohci_platform_resume(struct device *dev)
>  			return err;
>  	}
>  
> -	ohci_resume(hcd, false);
> +	ohci_resume(hcd, hibernated);
>  
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_active(dev);
> @@ -309,6 +309,16 @@ static int ohci_platform_resume(struct device *dev)
>  
>  	return 0;
>  }
> +
> +static int ohci_platform_resume(struct device *dev)
> +{
> +	return ohci_platform_renew(dev, false);

See, what does "false" here mean?

You can wrap the ohci_platform_renew() function with two helpers that
are ohci_platform_renew_hibernated() and ohci_platform_renew() or
something like that, which would explain what the difference is here.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1] usb: ohci-platform: fix usb disconnect issue after s4
  2022-08-30 11:14 [PATCH v1] usb: ohci-platform: fix usb disconnect issue after s4 Yinbo Zhu
  2022-08-30 12:59 ` Greg Kroah-Hartman
@ 2022-08-30 14:30 ` Alan Stern
  1 sibling, 0 replies; 3+ messages in thread
From: Alan Stern @ 2022-08-30 14:30 UTC (permalink / raw
  To: Yinbo Zhu
  Cc: Greg Kroah-Hartman, linux-usb, linux-kernel, Greg Kroah-Hartman,
	Patchwork Bot

On Tue, Aug 30, 2022 at 07:14:49PM +0800, Yinbo Zhu wrote:
> Avoid retaining bogus hardware states during resume-from-hibernation.
> Previously we had reset the hardware as part of preparing to reinstate
> the snapshot image. But we can do better now with the new PM framework,
> since we know exactly which resume operations are from hibernation
> 
> According to the commit "cd1965db0" and "6ec4beb5c" that the flag
> "hibernated" is for resume-from-hibernation and it should be true when
> usb resume from disk.
> 
> When this flag "hibernated" is set, the drivers will reset the hardware
> to get rid of any existing state and make sure resume from hibernation
> re-enumerates everything for ohci.

What is the "usb disconnect issue" you mention in the Subject line that 
this patch is supposed to fix?

> Signed-off-by: Yinbo Zhu <zhuyinbo@loongson.cn>
> ---
>  drivers/usb/host/ohci-platform.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
> index 0adae6265127..e733da2cd3b7 100644
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -289,7 +289,7 @@ static int ohci_platform_suspend(struct device *dev)
>  	return ret;
>  }
>  
> -static int ohci_platform_resume(struct device *dev)
> +static int ohci_platform_renew(struct device *dev, bool hibernated)

How about calling this routine ohci_platform_resume_maybe_hibernated() 
instead?

Alan Stern

>  {
>  	struct usb_hcd *hcd = dev_get_drvdata(dev);
>  	struct usb_ohci_pdata *pdata = dev_get_platdata(dev);
> @@ -301,7 +301,7 @@ static int ohci_platform_resume(struct device *dev)
>  			return err;
>  	}
>  
> -	ohci_resume(hcd, false);
> +	ohci_resume(hcd, hibernated);
>  
>  	pm_runtime_disable(dev);
>  	pm_runtime_set_active(dev);
> @@ -309,6 +309,16 @@ static int ohci_platform_resume(struct device *dev)
>  
>  	return 0;
>  }
> +
> +static int ohci_platform_resume(struct device *dev)
> +{
> +	return ohci_platform_renew(dev, false);
> +}
> +
> +static int ohci_platform_restore(struct device *dev)
> +{
> +	return ohci_platform_renew(dev, true);
> +}
>  #endif /* CONFIG_PM_SLEEP */
>  
>  static const struct of_device_id ohci_platform_ids[] = {
> @@ -325,8 +335,16 @@ static const struct platform_device_id ohci_platform_table[] = {
>  };
>  MODULE_DEVICE_TABLE(platform, ohci_platform_table);
>  
> -static SIMPLE_DEV_PM_OPS(ohci_platform_pm_ops, ohci_platform_suspend,
> -	ohci_platform_resume);
> +#ifdef CONFIG_PM_SLEEP
> +static const struct dev_pm_ops ohci_platform_pm_ops = {
> +	.suspend = ohci_platform_suspend,
> +	.resume = ohci_platform_resume,
> +	.freeze = ohci_platform_suspend,
> +	.thaw = ohci_platform_resume,
> +	.poweroff = ohci_platform_suspend,
> +	.restore = ohci_platform_restore,
> +};
> +#endif
>  
>  static struct platform_driver ohci_platform_driver = {
>  	.id_table	= ohci_platform_table,
> @@ -335,7 +353,9 @@ static struct platform_driver ohci_platform_driver = {
>  	.shutdown	= usb_hcd_platform_shutdown,
>  	.driver		= {
>  		.name	= "ohci-platform",
> +#ifdef CONFIG_PM_SLEEP
>  		.pm	= &ohci_platform_pm_ops,
> +#endif
>  		.of_match_table = ohci_platform_ids,
>  		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>  	}
> -- 
> 2.31.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-08-30 14:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-30 11:14 [PATCH v1] usb: ohci-platform: fix usb disconnect issue after s4 Yinbo Zhu
2022-08-30 12:59 ` Greg Kroah-Hartman
2022-08-30 14:30 ` Alan Stern

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).