lkml.org 
[lkml]   [2022]   [Mar]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] USB:Fix ehci infinite suspend-resume loop issue in zhaoxin
From
Date
On 2022/3/15 11:18, Alan Stern wrote:
> On Tue, Mar 15, 2022 at 08:39:09PM +0800, WeitaoWang-oc@zhaoxin.com 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
> .thaw_noirq = NULL,
> .thaw = NULL,
> .poweroff = hcd_pci_suspend,
> .
>

\
 
 \ /
  Last update: 2022-03-16 03:19    [W:0.556 / U:0.072 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site