Re: [PATCH] xhci: Prevent runtime suspend all the time with XHCI_RESET_ON_RESUME quirk

2020-05-04 Thread Kai-Heng Feng



> On May 4, 2020, at 18:13, Mathias Nyman  wrote:
> 
> On 4.5.2020 13.02, Kai-Heng Feng wrote:
>> 
>> 
>>> On May 4, 2020, at 17:47, Oliver Neukum  wrote:
>>> 
>>> Am Montag, den 04.05.2020, 17:19 +0800 schrieb Kai-Heng Feng:
 Etron EJ168 USB 3.0 Host Controller stops working after S3, if it was
 runtime suspended previously:
 [  370.080359] pci :02:00.0: can't change power state from D3cold to 
 D0 (config space inaccessible)
>>> 
>>> Apparently this controller has issues with D3cold
>>> 
 [  370.080477] xhci_hcd :04:00.0: can't change power state from D3cold 
 to D0 (config space inaccessible)
 [  370.080532] pcieport :00:1c.0: DPC: containment event, 
 status:0x1f05 source:0x0200
 [  370.080533] pcieport :00:1c.0: DPC: ERR_FATAL detected
 [  370.080536] xhci_hcd :04:00.0: can't change power state from D3hot 
 to D0 (config space inaccessible)
 [  370.080552] xhci_hcd :04:00.0: AER: can't recover (no 
 error_detected callback)
 [  370.080566] usb usb3: root hub lost power or was reset
 [  370.080566] usb usb4: root hub lost power or was reset
 [  370.080572] xhci_hcd :04:00.0: Host halt failed, -19
 [  370.080574] xhci_hcd :04:00.0: Host not accessible, reset failed.
 [  370.080575] xhci_hcd :04:00.0: PCI post-resume error -19!
 [  370.080586] xhci_hcd :04:00.0: HC died; cleaning up
 
 This can be fixed by not runtime suspend the controller at all.
 
 So instead of conditionally runtime suspend the controller, always
 prevent runtime suspend with XHCI_RESET_ON_RESUME quirk.
>>> 
>>> What does that do to other controllers that can do runtime suspend
>>> under the current scheme?
>> 
>> Ok, I'll add a new quirk specific to this controller.
>> 
>> Kai-Heng
> 
> Host shouldn't runtime suspend by default unless set by userspace, or
> it has XHCI_DEFAULT_PM_RUNTIME_ALLOW quirk set.  

In this case, runtime suspend is indeed enabled by userspace.
However we should still disable it at kernel side to prevent any bug.

Kai-Heng

> 
> -Mathias



Re: [PATCH] xhci: Prevent runtime suspend all the time with XHCI_RESET_ON_RESUME quirk

2020-05-04 Thread Mathias Nyman
On 4.5.2020 13.02, Kai-Heng Feng wrote:
> 
> 
>> On May 4, 2020, at 17:47, Oliver Neukum  wrote:
>>
>> Am Montag, den 04.05.2020, 17:19 +0800 schrieb Kai-Heng Feng:
>>> Etron EJ168 USB 3.0 Host Controller stops working after S3, if it was
>>> runtime suspended previously:
>>> [  370.080359] pci :02:00.0: can't change power state from D3cold to D0 
>>> (config space inaccessible)
>>
>> Apparently this controller has issues with D3cold
>>
>>> [  370.080477] xhci_hcd :04:00.0: can't change power state from D3cold 
>>> to D0 (config space inaccessible)
>>> [  370.080532] pcieport :00:1c.0: DPC: containment event, status:0x1f05 
>>> source:0x0200
>>> [  370.080533] pcieport :00:1c.0: DPC: ERR_FATAL detected
>>> [  370.080536] xhci_hcd :04:00.0: can't change power state from D3hot 
>>> to D0 (config space inaccessible)
>>> [  370.080552] xhci_hcd :04:00.0: AER: can't recover (no error_detected 
>>> callback)
>>> [  370.080566] usb usb3: root hub lost power or was reset
>>> [  370.080566] usb usb4: root hub lost power or was reset
>>> [  370.080572] xhci_hcd :04:00.0: Host halt failed, -19
>>> [  370.080574] xhci_hcd :04:00.0: Host not accessible, reset failed.
>>> [  370.080575] xhci_hcd :04:00.0: PCI post-resume error -19!
>>> [  370.080586] xhci_hcd :04:00.0: HC died; cleaning up
>>>
>>> This can be fixed by not runtime suspend the controller at all.
>>>
>>> So instead of conditionally runtime suspend the controller, always
>>> prevent runtime suspend with XHCI_RESET_ON_RESUME quirk.
>>
>> What does that do to other controllers that can do runtime suspend
>> under the current scheme?
> 
> Ok, I'll add a new quirk specific to this controller.
> 
> Kai-Heng

Host shouldn't runtime suspend by default unless set by userspace, or
it has XHCI_DEFAULT_PM_RUNTIME_ALLOW quirk set.  

-Mathias


Re: [PATCH] xhci: Prevent runtime suspend all the time with XHCI_RESET_ON_RESUME quirk

2020-05-04 Thread Kai-Heng Feng



> On May 4, 2020, at 17:47, Oliver Neukum  wrote:
> 
> Am Montag, den 04.05.2020, 17:19 +0800 schrieb Kai-Heng Feng:
>> Etron EJ168 USB 3.0 Host Controller stops working after S3, if it was
>> runtime suspended previously:
>> [  370.080359] pci :02:00.0: can't change power state from D3cold to D0 
>> (config space inaccessible)
> 
> Apparently this controller has issues with D3cold
> 
>> [  370.080477] xhci_hcd :04:00.0: can't change power state from D3cold 
>> to D0 (config space inaccessible)
>> [  370.080532] pcieport :00:1c.0: DPC: containment event, status:0x1f05 
>> source:0x0200
>> [  370.080533] pcieport :00:1c.0: DPC: ERR_FATAL detected
>> [  370.080536] xhci_hcd :04:00.0: can't change power state from D3hot to 
>> D0 (config space inaccessible)
>> [  370.080552] xhci_hcd :04:00.0: AER: can't recover (no error_detected 
>> callback)
>> [  370.080566] usb usb3: root hub lost power or was reset
>> [  370.080566] usb usb4: root hub lost power or was reset
>> [  370.080572] xhci_hcd :04:00.0: Host halt failed, -19
>> [  370.080574] xhci_hcd :04:00.0: Host not accessible, reset failed.
>> [  370.080575] xhci_hcd :04:00.0: PCI post-resume error -19!
>> [  370.080586] xhci_hcd :04:00.0: HC died; cleaning up
>> 
>> This can be fixed by not runtime suspend the controller at all.
>> 
>> So instead of conditionally runtime suspend the controller, always
>> prevent runtime suspend with XHCI_RESET_ON_RESUME quirk.
> 
> What does that do to other controllers that can do runtime suspend
> under the current scheme?

Ok, I'll add a new quirk specific to this controller.

Kai-Heng

> 
>   Regards
>   Oliver
> 



Re: [PATCH] xhci: Prevent runtime suspend all the time with XHCI_RESET_ON_RESUME quirk

2020-05-04 Thread Oliver Neukum
Am Montag, den 04.05.2020, 17:19 +0800 schrieb Kai-Heng Feng:
> Etron EJ168 USB 3.0 Host Controller stops working after S3, if it was
> runtime suspended previously:
> [  370.080359] pci :02:00.0: can't change power state from D3cold to D0 
> (config space inaccessible)

Apparently this controller has issues with D3cold

> [  370.080477] xhci_hcd :04:00.0: can't change power state from D3cold to 
> D0 (config space inaccessible)
> [  370.080532] pcieport :00:1c.0: DPC: containment event, status:0x1f05 
> source:0x0200
> [  370.080533] pcieport :00:1c.0: DPC: ERR_FATAL detected
> [  370.080536] xhci_hcd :04:00.0: can't change power state from D3hot to 
> D0 (config space inaccessible)
> [  370.080552] xhci_hcd :04:00.0: AER: can't recover (no error_detected 
> callback)
> [  370.080566] usb usb3: root hub lost power or was reset
> [  370.080566] usb usb4: root hub lost power or was reset
> [  370.080572] xhci_hcd :04:00.0: Host halt failed, -19
> [  370.080574] xhci_hcd :04:00.0: Host not accessible, reset failed.
> [  370.080575] xhci_hcd :04:00.0: PCI post-resume error -19!
> [  370.080586] xhci_hcd :04:00.0: HC died; cleaning up
> 
> This can be fixed by not runtime suspend the controller at all.
> 
> So instead of conditionally runtime suspend the controller, always
> prevent runtime suspend with XHCI_RESET_ON_RESUME quirk.

What does that do to other controllers that can do runtime suspend
under the current scheme?

Regards
Oliver