On 2022/3/16 10:18, WeitaoWang-oc@xxxxxxxxxxx wrote:
On 2022/3/15 11:18, Alan Stern wrote:
On Tue, Mar 15, 2022 at 08:39:09PM +0800, WeitaoWang-oc@xxxxxxxxxxx wrote:
On 2022/3/14 10:24, Alan Stern wrote:
+ t1 = ehci_readl(ehci, &ehci->regs->status);
+ ehci_writel(ehci, t1 & STS_PCD, &ehci->regs->status);
+ ehci_readl(ehci, &ehci->regs->status);
You should not clear the STS_PCD bit. What if some other port had a
status change at the same time? Then because you cleared the
port-change-detect bit, the system would not realize that the other port
needed to be handled.
I really didn't think about this case.
Leaving the STS_PCD bit turned on will cause the driver to do a little
extra work, but it shouldn't cause any harm.
I have encountered the following situation if EHCI runtime suspend is
enabled by default.
1.Wake from system to disk and boot OS.
You're talking about resuming after hibernation, right?
You're right.
2.EHCI will entry runtime suspend after enumerated by driver during boot
phase of suspend to disk
I'm not sure what you mean by "boot phase of suspend to disk". This is
while the restore kernel is starting up at the beginning of resume from
hibernation, right?
You understood exactly what I was saying.
3.EHCI will be placed to freeze state and ehci_resume is called after image
is loaded.
ehci_resume is called to leave runtime suspend. Going into the freeze
state doesn't require any changes.
4.If PCD flag is set(caused by patch), then HCD_FLAG_RH_RUNNING will be set.
5.Pci_pm_freeze_noirq is called to check ehci root hub state and return
value is -EBUSY. which will cause
quiesce phase of suspend to disk fail.
You're talking about check_root_hub_suspended() in hcd-pci.c, right?
It's right.
You know, I'm not at all certain that the callbacks for freeze and
freeze_noirq should ever return anything other than 0. It's okay for
them to call check_root_hub_suspended(), but they should ignore its
return value.
Can you check if the patch below helps?
Alan Stern
Index: usb-devel/drivers/usb/core/hcd-pci.c
===================================================================
--- usb-devel.orig/drivers/usb/core/hcd-pci.c
+++ usb-devel/drivers/usb/core/hcd-pci.c
@@ -575,6 +575,12 @@ static int hcd_pci_resume(struct device
return resume_common(dev, PM_EVENT_RESUME);
}
+static int hcd_pci_freeze_check(struct device *dev)
+{
+ (void) check_root_hub_suspended(dev);
+ return 0;
+}
+
static int hcd_pci_restore(struct device *dev)
{
return resume_common(dev, PM_EVENT_RESTORE);
@@ -586,6 +592,7 @@ static int hcd_pci_restore(struct device
#define hcd_pci_suspend_noirq NULL
#define hcd_pci_resume_noirq NULL
#define hcd_pci_resume NULL
+#define hcd_pci_freeze_check NULL
#define hcd_pci_restore NULL
#endif /* CONFIG_PM_SLEEP */
@@ -616,8 +623,8 @@ const struct dev_pm_ops usb_hcd_pci_pm_o
.suspend_noirq = hcd_pci_suspend_noirq,
.resume_noirq = hcd_pci_resume_noirq,
.resume = hcd_pci_resume,
- .freeze = check_root_hub_suspended,
- .freeze_noirq = check_root_hub_suspended,
+ .freeze = hcd_pci_freeze_check,
+ .freeze_noirq = hcd_pci_freeze_check,
This patch can fix pci driver's fail check in freeze_noirq phase.
Restoring system state from a hibernation image can continue and success.
Weitao Wang
This patch seems work for our platform, but I don't know if there will
be side-effects for others. So I want to take your previous suggestion
and not to clear PCD, As ehci runtime suspend enabled at OS startup is
not a real requirement. I'll resubmit revised patch without the whitespace
damage.
Thanks for your great help.
Best Wishes
Weitao Wang
.thaw_noirq = NULL,
.thaw = NULL,
.poweroff = hcd_pci_suspend,
.
