diff mbox series

xhci: Fix USB 3.1 enumeration issues by increasing roothub power-on-good delay

Message ID 20211105160036.549516-1-mathias.nyman@linux.intel.com (mailing list archive)
State Accepted
Commit e1959faf085b004e6c3afaaaa743381f00e7c015
Headers show
Series xhci: Fix USB 3.1 enumeration issues by increasing roothub power-on-good delay | expand

Commit Message

Mathias Nyman Nov. 5, 2021, 4 p.m. UTC
Some USB 3.1 enumeration issues were reported after the hub driver removed
the minimum 100ms limit for the power-on-good delay.

Since commit 90d28fb53d4a ("usb: core: reduce power-on-good delay time of
root hub") the hub driver sets the power-on-delay based on the
bPwrOn2PwrGood value in the hub descriptor.

xhci driver has a 20ms bPwrOn2PwrGood value for both roothubs based
on xhci spec section 5.4.8, but it's clearly not enough for the
USB 3.1 devices, causing enumeration issues.

Tests indicate full 100ms delay is needed.

Reported-by: Walt Jr. Brake <mr.yming81@gmail.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
---
 drivers/usb/host/xhci-hub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Mathias Nyman Nov. 5, 2021, 4:09 p.m. UTC | #1
On 5.11.2021 18.00, Mathias Nyman wrote:
> Some USB 3.1 enumeration issues were reported after the hub driver removed
> the minimum 100ms limit for the power-on-good delay.
> 
> Since commit 90d28fb53d4a ("usb: core: reduce power-on-good delay time of
> root hub") the hub driver sets the power-on-delay based on the
> bPwrOn2PwrGood value in the hub descriptor.
> 
> xhci driver has a 20ms bPwrOn2PwrGood value for both roothubs based
> on xhci spec section 5.4.8, but it's clearly not enough for the
> USB 3.1 devices, causing enumeration issues.
> 
> Tests indicate full 100ms delay is needed.
> 
> Reported-by: Walt Jr. Brake <mr.yming81@gmail.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> 

Walt Jr Brake, just to be sure could you test this one as well?

As Alan suggested this sets 100ms for the USB 3 roothub but
keeps the 20ms for the USB 2 roothub.

Thanks
-Mathias
Greg KH Nov. 5, 2021, 4:15 p.m. UTC | #2
On Fri, Nov 05, 2021 at 06:00:36PM +0200, Mathias Nyman wrote:
> Some USB 3.1 enumeration issues were reported after the hub driver removed
> the minimum 100ms limit for the power-on-good delay.
> 
> Since commit 90d28fb53d4a ("usb: core: reduce power-on-good delay time of
> root hub") the hub driver sets the power-on-delay based on the
> bPwrOn2PwrGood value in the hub descriptor.
> 
> xhci driver has a 20ms bPwrOn2PwrGood value for both roothubs based
> on xhci spec section 5.4.8, but it's clearly not enough for the
> USB 3.1 devices, causing enumeration issues.
> 
> Tests indicate full 100ms delay is needed.
> 
> Reported-by: Walt Jr. Brake <mr.yming81@gmail.com>
> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>

So this needs:
	Fixes: 90d28fb53d4a ("usb: core: reduce power-on-good delay time of root hub")
right?

thanks,

greg k-h
Walt Jr. Brake Nov. 5, 2021, 5:32 p.m. UTC | #3
On 6/11/2021 00:09, Mathias Nyman wrote:
> On 5.11.2021 18.00, Mathias Nyman wrote:
>> Some USB 3.1 enumeration issues were reported after the hub driver removed
>> the minimum 100ms limit for the power-on-good delay.
>>
>> Since commit 90d28fb53d4a ("usb: core: reduce power-on-good delay time of
>> root hub") the hub driver sets the power-on-delay based on the
>> bPwrOn2PwrGood value in the hub descriptor.
>>
>> xhci driver has a 20ms bPwrOn2PwrGood value for both roothubs based
>> on xhci spec section 5.4.8, but it's clearly not enough for the
>> USB 3.1 devices, causing enumeration issues.
>>
>> Tests indicate full 100ms delay is needed.
>>
>> Reported-by: Walt Jr. Brake <mr.yming81@gmail.com>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>
> Walt Jr Brake, just to be sure could you test this one as well?
>
> As Alan suggested this sets 100ms for the USB 3 roothub but
> keeps the 20ms for the USB 2 roothub.
>
> Thanks
> -Mathias

Yes I tested, it works too.
Mathias Nyman Nov. 5, 2021, 7:41 p.m. UTC | #4
On 5.11.2021 19.32, Walt Jr. Brake wrote:
> On 6/11/2021 00:09, Mathias Nyman wrote:
>> On 5.11.2021 18.00, Mathias Nyman wrote:
>>> Some USB 3.1 enumeration issues were reported after the hub driver removed
>>> the minimum 100ms limit for the power-on-good delay.
>>>
>>> Since commit 90d28fb53d4a ("usb: core: reduce power-on-good delay time of
>>> root hub") the hub driver sets the power-on-delay based on the
>>> bPwrOn2PwrGood value in the hub descriptor.
>>>
>>> xhci driver has a 20ms bPwrOn2PwrGood value for both roothubs based
>>> on xhci spec section 5.4.8, but it's clearly not enough for the
>>> USB 3.1 devices, causing enumeration issues.
>>>
>>> Tests indicate full 100ms delay is needed.
>>>
>>> Reported-by: Walt Jr. Brake <mr.yming81@gmail.com>
>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
>>>
>> Walt Jr Brake, just to be sure could you test this one as well?
>>
>> As Alan suggested this sets 100ms for the USB 3 roothub but
>> keeps the 20ms for the USB 2 roothub.
>>
>> Thanks
>> -Mathias
> 
> Yes I tested, it works too.
> 

Thank you
-Mathias
Mathias Nyman Nov. 5, 2021, 7:56 p.m. UTC | #5
On 5.11.2021 18.15, Greg KH wrote:
> On Fri, Nov 05, 2021 at 06:00:36PM +0200, Mathias Nyman wrote:
>> Some USB 3.1 enumeration issues were reported after the hub driver removed
>> the minimum 100ms limit for the power-on-good delay.
>>
>> Since commit 90d28fb53d4a ("usb: core: reduce power-on-good delay time of
>> root hub") the hub driver sets the power-on-delay based on the
>> bPwrOn2PwrGood value in the hub descriptor.
>>
>> xhci driver has a 20ms bPwrOn2PwrGood value for both roothubs based
>> on xhci spec section 5.4.8, but it's clearly not enough for the
>> USB 3.1 devices, causing enumeration issues.
>>
>> Tests indicate full 100ms delay is needed.
>>
>> Reported-by: Walt Jr. Brake <mr.yming81@gmail.com>
>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> 
> So this needs:
> 	Fixes: 90d28fb53d4a ("usb: core: reduce power-on-good delay time of root hub")
> right?

I guess so, not sure on "Fixes" policy here.

This patch fixes an xhci issue revealed by ("usb: core: reduce power-on-good delay time of root hub")
That original patch itself looks correct, but these two patches should really go together to
avoid any enumeration issues.  

Thanks
-Mathias
Greg KH Nov. 6, 2021, 2:41 p.m. UTC | #6
On Fri, Nov 05, 2021 at 09:56:59PM +0200, Mathias Nyman wrote:
> On 5.11.2021 18.15, Greg KH wrote:
> > On Fri, Nov 05, 2021 at 06:00:36PM +0200, Mathias Nyman wrote:
> >> Some USB 3.1 enumeration issues were reported after the hub driver removed
> >> the minimum 100ms limit for the power-on-good delay.
> >>
> >> Since commit 90d28fb53d4a ("usb: core: reduce power-on-good delay time of
> >> root hub") the hub driver sets the power-on-delay based on the
> >> bPwrOn2PwrGood value in the hub descriptor.
> >>
> >> xhci driver has a 20ms bPwrOn2PwrGood value for both roothubs based
> >> on xhci spec section 5.4.8, but it's clearly not enough for the
> >> USB 3.1 devices, causing enumeration issues.
> >>
> >> Tests indicate full 100ms delay is needed.
> >>
> >> Reported-by: Walt Jr. Brake <mr.yming81@gmail.com>
> >> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
> > 
> > So this needs:
> > 	Fixes: 90d28fb53d4a ("usb: core: reduce power-on-good delay time of root hub")
> > right?
> 
> I guess so, not sure on "Fixes" policy here.
> 
> This patch fixes an xhci issue revealed by ("usb: core: reduce power-on-good delay time of root hub")
> That original patch itself looks correct, but these two patches should really go together to
> avoid any enumeration issues.  

Great, I'll use that tag for now so that it will get backported
properly.

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index a3f875eea751..af946c42b6f0 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -257,7 +257,6 @@  static void xhci_common_hub_descriptor(struct xhci_hcd *xhci,
 {
 	u16 temp;
 
-	desc->bPwrOn2PwrGood = 10;	/* xhci section 5.4.9 says 20ms max */
 	desc->bHubContrCurrent = 0;
 
 	desc->bNbrPorts = ports;
@@ -292,6 +291,7 @@  static void xhci_usb2_hub_descriptor(struct usb_hcd *hcd, struct xhci_hcd *xhci,
 	desc->bDescriptorType = USB_DT_HUB;
 	temp = 1 + (ports / 8);
 	desc->bDescLength = USB_DT_HUB_NONVAR_SIZE + 2 * temp;
+	desc->bPwrOn2PwrGood = 10;	/* xhci section 5.4.8 says 20ms */
 
 	/* The Device Removable bits are reported on a byte granularity.
 	 * If the port doesn't exist within that byte, the bit is set to 0.
@@ -344,6 +344,7 @@  static void xhci_usb3_hub_descriptor(struct usb_hcd *hcd, struct xhci_hcd *xhci,
 	xhci_common_hub_descriptor(xhci, desc, ports);
 	desc->bDescriptorType = USB_DT_SS_HUB;
 	desc->bDescLength = USB_DT_SS_HUB_SIZE;
+	desc->bPwrOn2PwrGood = 50;	/* usb 3.1 may fail if less than 100ms */
 
 	/* header decode latency should be zero for roothubs,
 	 * see section 4.23.5.2.