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@xxxxxxxxxxx> 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