public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* USB hub port reset
@ 2016-11-13  3:52 Anbazhagan, Baraneedharan
  2016-11-14  1:45 ` Tian, Feng
  0 siblings, 1 reply; 9+ messages in thread
From: Anbazhagan, Baraneedharan @ 2016-11-13  3:52 UTC (permalink / raw)
  To: edk2-devel@lists.01.org, Tian, Feng

EDK2 have reset recovery time of 10ms for hub port based on port status reset bit but USB spec doesn't mention that port status can be used for t6/reset recovery time. Could you please clarify?

USB vendor mentions EDK2 doesn't have reset recovery time on hub port reset and also highlighting EDK2 hub port reset differs from https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L2789

-Baranee


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: USB hub port reset
  2016-11-13  3:52 USB hub port reset Anbazhagan, Baraneedharan
@ 2016-11-14  1:45 ` Tian, Feng
  2016-11-14  3:32   ` Anbazhagan, Baraneedharan
  0 siblings, 1 reply; 9+ messages in thread
From: Tian, Feng @ 2016-11-14  1:45 UTC (permalink / raw)
  To: Anbazhagan, Baraneedharan, edk2-devel@lists.01.org; +Cc: Tian, Feng

Hi, Baranee

Linux wait 10 + 40 ms (TRSTRCY = 10 ms, plus extra 40 ms). Do you mean EDKII should be same with linux to wait more time?

Thanks
Feng

From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
Sent: Sunday, November 13, 2016 11:53 AM
To: edk2-devel@lists.01.org; Tian, Feng <feng.tian@intel.com>
Subject: USB hub port reset

EDK2 have reset recovery time of 10ms for hub port based on port status reset bit but USB spec doesn't mention that port status can be used for t6/reset recovery time. Could you please clarify?

USB vendor mentions EDK2 doesn't have reset recovery time on hub port reset and also highlighting EDK2 hub port reset differs from https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L2789

-Baranee


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: USB hub port reset
  2016-11-14  1:45 ` Tian, Feng
@ 2016-11-14  3:32   ` Anbazhagan, Baraneedharan
  2016-11-14  4:38     ` Tian, Feng
  0 siblings, 1 reply; 9+ messages in thread
From: Anbazhagan, Baraneedharan @ 2016-11-14  3:32 UTC (permalink / raw)
  To: Tian, Feng, edk2-devel@lists.01.org

I didn't mean to have extra delays. Am asking whether EDKII have to be updated for 10ms reset recovery time regardless of port status to align with spec?


>From: Tian, Feng [mailto:feng.tian@intel.com] 
>Sent: Sunday, November 13, 2016 7:46 PM
>To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2-devel@lists.01.org
>Cc: Tian, Feng <feng.tian@intel.com>
>Subject: RE: USB hub port reset
>
>Hi, Baranee
>
>Linux wait 10 + 40 ms (TRSTRCY = 10 ms, plus extra 40 ms). Do you mean EDKII should be same with linux to wait more time?
>
>Thanks
>Feng
>
>From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com] 
>Sent: Sunday, November 13, 2016 11:53 AM
>To: mailto:edk2-devel@lists.01.org; Tian, Feng <mailto:feng.tian@intel.com>
>Subject: USB hub port reset
>
>EDK2 have reset recovery time of 10ms for hub port based on port status reset bit but USB spec doesn't mention that port status can be used for t6/reset recovery time. Could you please clarify?
>
>USB vendor mentions EDK2 doesn't have reset recovery time on hub port reset and also highlighting EDK2 hub port reset differs from https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L2789
>
>-Baranee


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: USB hub port reset
  2016-11-14  3:32   ` Anbazhagan, Baraneedharan
@ 2016-11-14  4:38     ` Tian, Feng
  2016-11-14 13:46       ` Anbazhagan, Baraneedharan
  0 siblings, 1 reply; 9+ messages in thread
From: Tian, Feng @ 2016-11-14  4:38 UTC (permalink / raw)
  To: Anbazhagan, Baraneedharan, edk2-devel@lists.01.org; +Cc: Tian, Feng

I don't catch what you mean. We have had 10ms delay for port reset. Which line of UsbHub.c do you think there is problem? 

Quote from USB2.0 spec:
"Hubs must be able to accept all hub requests and devices must be able to accept a SetAddress() request (refer to
Section 11.24.2 and Section 9.4 respectively) after the reset recovery time 10 ms (TRSTRCY) after the reset is
removed."

Thanks
Feng

-----Original Message-----
From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com] 
Sent: Monday, November 14, 2016 11:33 AM
To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
Subject: RE: USB hub port reset

I didn't mean to have extra delays. Am asking whether EDKII have to be updated for 10ms reset recovery time regardless of port status to align with spec?


>From: Tian, Feng [mailto:feng.tian@intel.com] 
>Sent: Sunday, November 13, 2016 7:46 PM
>To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2-devel@lists.01.org
>Cc: Tian, Feng <feng.tian@intel.com>
>Subject: RE: USB hub port reset
>
>Hi, Baranee
>
>Linux wait 10 + 40 ms (TRSTRCY = 10 ms, plus extra 40 ms). Do you mean EDKII should be same with linux to wait more time?
>
>Thanks
>Feng
>
>From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com] 
>Sent: Sunday, November 13, 2016 11:53 AM
>To: mailto:edk2-devel@lists.01.org; Tian, Feng <mailto:feng.tian@intel.com>
>Subject: USB hub port reset
>
>EDK2 have reset recovery time of 10ms for hub port based on port status reset bit but USB spec doesn't mention that port status can be used for t6/reset recovery time. Could you please clarify?
>
>USB vendor mentions EDK2 doesn't have reset recovery time on hub port reset and also highlighting EDK2 hub port reset differs from https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L2789
>
>-Baranee


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: USB hub port reset
  2016-11-14  4:38     ` Tian, Feng
@ 2016-11-14 13:46       ` Anbazhagan, Baraneedharan
  2016-11-15  4:28         ` Tian, Feng
  0 siblings, 1 reply; 9+ messages in thread
From: Anbazhagan, Baraneedharan @ 2016-11-14 13:46 UTC (permalink / raw)
  To: Tian, Feng, edk2-devel@lists.01.org

In case of XHCI, TRB_TYPE_ADDRESS_DEV/SetAddress occurs within XhcPollPortStatusChange. Seem we don't have 10ms/TRSTRCY for the devices behind hub connected to XHCI or am missing something here?

In EDK2, 10ms(TRSTRCY) delay occurs before clear port status and in case of Linux TRSTRCY delay is after clearing port status.

Thanks
Baranee


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Tian,
> Feng
> Sent: Sunday, November 13, 2016 10:38 PM
> To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2-
> devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>
> Subject: Re: [edk2] USB hub port reset
> 
> I don't catch what you mean. We have had 10ms delay for port reset. Which line of
> UsbHub.c do you think there is problem?
> 
> Quote from USB2.0 spec:
> "Hubs must be able to accept all hub requests and devices must be able to accept a
> SetAddress() request (refer to Section 11.24.2 and Section 9.4 respectively) after the
> reset recovery time 10 ms (TRSTRCY) after the reset is removed."
> 
> Thanks
> Feng
> 
> -----Original Message-----
> From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> Sent: Monday, November 14, 2016 11:33 AM
> To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
> Subject: RE: USB hub port reset
> 
> I didn't mean to have extra delays. Am asking whether EDKII have to be updated for
> 10ms reset recovery time regardless of port status to align with spec?
> 
> 
> >From: Tian, Feng [mailto:feng.tian@intel.com]
> >Sent: Sunday, November 13, 2016 7:46 PM
> >To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>;
> >edk2-devel@lists.01.org
> >Cc: Tian, Feng <feng.tian@intel.com>
> >Subject: RE: USB hub port reset
> >
> >Hi, Baranee
> >
> >Linux wait 10 + 40 ms (TRSTRCY = 10 ms, plus extra 40 ms). Do you mean EDKII
> should be same with linux to wait more time?
> >
> >Thanks
> >Feng
> >
> >From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> >Sent: Sunday, November 13, 2016 11:53 AM
> >To: mailto:edk2-devel@lists.01.org; Tian, Feng
> ><mailto:feng.tian@intel.com>
> >Subject: USB hub port reset
> >
> >EDK2 have reset recovery time of 10ms for hub port based on port status reset bit
> but USB spec doesn't mention that port status can be used for t6/reset recovery
> time. Could you please clarify?
> >
> >USB vendor mentions EDK2 doesn't have reset recovery time on hub port
> >reset and also highlighting EDK2 hub port reset differs from
> >https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L2
> >789
> >
> >-Baranee
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: USB hub port reset
  2016-11-14 13:46       ` Anbazhagan, Baraneedharan
@ 2016-11-15  4:28         ` Tian, Feng
  2016-11-15 23:56           ` Anbazhagan, Baraneedharan
  0 siblings, 1 reply; 9+ messages in thread
From: Tian, Feng @ 2016-11-15  4:28 UTC (permalink / raw)
  To: Anbazhagan, Baraneedharan, edk2-devel@lists.01.org; +Cc: Tian, Feng

Baranee

Supposing are using latest usb code in EdkII trunk.

We send ADDRESS DEVICE CMD in XhcInitializeDeviceSlot(), which will cause XHC issue a USB SET_ADDRESS request to the USB Device.

There are two possible paths:
1. Reset_Change bit gets set after connection without manually port reset.
UsbHubGetPortStatus()->XhcPollPortStatusChange()->(if RESET_C bit is set)->XhcInitializeDeviceSlot()
2. Reset_Change bit gets set after manually Reset Port operation.
UsbHubResetPort()->UsbHubSetPortFeature()->Stall(20)-> UsbHubGetPortStatus()->(invoke above code in XHCI)->(if RESET_C bit is set)->Stall(10)

According to your info, it looks like we need add 10ms delay to XhcInitializeDeviceSlot() like below:

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index e37f674..bb68f34 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2115,6 +2115,7 @@ XhcInitializeDeviceSlot (
   // 8) Issue an Address Device Command for the Device Slot, where the command points to the Input
   //    Context data structure described above.
   //
+  gBS->Stall (10 * 1000);
   ZeroMem (&CmdTrbAddr, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT));
   CmdTrbAddr.PtrLo    = XHC_LOW_32BIT (PhyAddr);
@@ -2321,6 +2322,7 @@ XhcInitializeDeviceSlot64 (
   // 8) Issue an Address Device Command for the Device Slot, where the command points to the Input
   //    Context data structure described above.
   //
+  gBS->Stall (10 * 1000);
   ZeroMem (&CmdTrbAddr, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT_64));
   CmdTrbAddr.PtrLo    = XHC_LOW_32BIT (PhyAddr);

Could you please have a try to see if it solves your problem?

Thanks
Feng

-----Original Message-----
From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com] 
Sent: Monday, November 14, 2016 9:46 PM
To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
Subject: RE: USB hub port reset

In case of XHCI, TRB_TYPE_ADDRESS_DEV/SetAddress occurs within XhcPollPortStatusChange. Seem we don't have 10ms/TRSTRCY for the devices behind hub connected to XHCI or am missing something here?

In EDK2, 10ms(TRSTRCY) delay occurs before clear port status and in case of Linux TRSTRCY delay is after clearing port status.

Thanks
Baranee


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Tian, Feng
> Sent: Sunday, November 13, 2016 10:38 PM
> To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2- 
> devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>
> Subject: Re: [edk2] USB hub port reset
> 
> I don't catch what you mean. We have had 10ms delay for port reset. 
> Which line of UsbHub.c do you think there is problem?
> 
> Quote from USB2.0 spec:
> "Hubs must be able to accept all hub requests and devices must be able 
> to accept a
> SetAddress() request (refer to Section 11.24.2 and Section 9.4 
> respectively) after the reset recovery time 10 ms (TRSTRCY) after the reset is removed."
> 
> Thanks
> Feng
> 
> -----Original Message-----
> From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> Sent: Monday, November 14, 2016 11:33 AM
> To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
> Subject: RE: USB hub port reset
> 
> I didn't mean to have extra delays. Am asking whether EDKII have to be 
> updated for 10ms reset recovery time regardless of port status to align with spec?
> 
> 
> >From: Tian, Feng [mailto:feng.tian@intel.com]
> >Sent: Sunday, November 13, 2016 7:46 PM
> >To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; 
> >edk2-devel@lists.01.org
> >Cc: Tian, Feng <feng.tian@intel.com>
> >Subject: RE: USB hub port reset
> >
> >Hi, Baranee
> >
> >Linux wait 10 + 40 ms (TRSTRCY = 10 ms, plus extra 40 ms). Do you 
> >mean EDKII
> should be same with linux to wait more time?
> >
> >Thanks
> >Feng
> >
> >From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> >Sent: Sunday, November 13, 2016 11:53 AM
> >To: mailto:edk2-devel@lists.01.org; Tian, Feng 
> ><mailto:feng.tian@intel.com>
> >Subject: USB hub port reset
> >
> >EDK2 have reset recovery time of 10ms for hub port based on port 
> >status reset bit
> but USB spec doesn't mention that port status can be used for t6/reset 
> recovery time. Could you please clarify?
> >
> >USB vendor mentions EDK2 doesn't have reset recovery time on hub port 
> >reset and also highlighting EDK2 hub port reset differs from
> >https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#
> >L2
> >789
> >
> >-Baranee
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: USB hub port reset
  2016-11-15  4:28         ` Tian, Feng
@ 2016-11-15 23:56           ` Anbazhagan, Baraneedharan
  2016-11-16  1:45             ` Tian, Feng
  0 siblings, 1 reply; 9+ messages in thread
From: Anbazhagan, Baraneedharan @ 2016-11-15 23:56 UTC (permalink / raw)
  To: Tian, Feng, edk2-devel@lists.01.org

Feng,
Below change solves the problem with a device behind the hub. Do we still need the 10ms delay in UsbHubResetPort function for manual port reset in case of XHCI?

Thanks,
Baranee

-----Original Message-----
From: Tian, Feng [mailto:feng.tian@intel.com] 
Sent: Monday, November 14, 2016 10:28 PM
To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2-devel@lists.01.org
Cc: Tian, Feng <feng.tian@intel.com>
Subject: RE: USB hub port reset

Baranee

Supposing are using latest usb code in EdkII trunk.

We send ADDRESS DEVICE CMD in XhcInitializeDeviceSlot(), which will cause XHC issue a USB SET_ADDRESS request to the USB Device.

There are two possible paths:
1. Reset_Change bit gets set after connection without manually port reset.
UsbHubGetPortStatus()->XhcPollPortStatusChange()->(if RESET_C bit is set)->XhcInitializeDeviceSlot() 2. Reset_Change bit gets set after manually Reset Port operation.
UsbHubResetPort()->UsbHubSetPortFeature()->Stall(20)-> UsbHubGetPortStatus()->(invoke above code in XHCI)->(if RESET_C bit is set)->Stall(10)

According to your info, it looks like we need add 10ms delay to XhcInitializeDeviceSlot() like below:

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index e37f674..bb68f34 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2115,6 +2115,7 @@ XhcInitializeDeviceSlot (
   // 8) Issue an Address Device Command for the Device Slot, where the command points to the Input
   //    Context data structure described above.
   //
+  gBS->Stall (10 * 1000);
   ZeroMem (&CmdTrbAddr, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT));
   CmdTrbAddr.PtrLo    = XHC_LOW_32BIT (PhyAddr);
@@ -2321,6 +2322,7 @@ XhcInitializeDeviceSlot64 (
   // 8) Issue an Address Device Command for the Device Slot, where the command points to the Input
   //    Context data structure described above.
   //
+  gBS->Stall (10 * 1000);
   ZeroMem (&CmdTrbAddr, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT_64));
   CmdTrbAddr.PtrLo    = XHC_LOW_32BIT (PhyAddr);

Could you please have a try to see if it solves your problem?

Thanks
Feng

-----Original Message-----
From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
Sent: Monday, November 14, 2016 9:46 PM
To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
Subject: RE: USB hub port reset

In case of XHCI, TRB_TYPE_ADDRESS_DEV/SetAddress occurs within XhcPollPortStatusChange. Seem we don't have 10ms/TRSTRCY for the devices behind hub connected to XHCI or am missing something here?

In EDK2, 10ms(TRSTRCY) delay occurs before clear port status and in case of Linux TRSTRCY delay is after clearing port status.

Thanks
Baranee


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Tian, Feng
> Sent: Sunday, November 13, 2016 10:38 PM
> To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2- 
> devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>
> Subject: Re: [edk2] USB hub port reset
> 
> I don't catch what you mean. We have had 10ms delay for port reset. 
> Which line of UsbHub.c do you think there is problem?
> 
> Quote from USB2.0 spec:
> "Hubs must be able to accept all hub requests and devices must be able 
> to accept a
> SetAddress() request (refer to Section 11.24.2 and Section 9.4
> respectively) after the reset recovery time 10 ms (TRSTRCY) after the reset is removed."
> 
> Thanks
> Feng
> 
> -----Original Message-----
> From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> Sent: Monday, November 14, 2016 11:33 AM
> To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
> Subject: RE: USB hub port reset
> 
> I didn't mean to have extra delays. Am asking whether EDKII have to be 
> updated for 10ms reset recovery time regardless of port status to align with spec?
> 
> 
> >From: Tian, Feng [mailto:feng.tian@intel.com]
> >Sent: Sunday, November 13, 2016 7:46 PM
> >To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; 
> >edk2-devel@lists.01.org
> >Cc: Tian, Feng <feng.tian@intel.com>
> >Subject: RE: USB hub port reset
> >
> >Hi, Baranee
> >
> >Linux wait 10 + 40 ms (TRSTRCY = 10 ms, plus extra 40 ms). Do you 
> >mean EDKII
> should be same with linux to wait more time?
> >
> >Thanks
> >Feng
> >
> >From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> >Sent: Sunday, November 13, 2016 11:53 AM
> >To: mailto:edk2-devel@lists.01.org; Tian, Feng 
> ><mailto:feng.tian@intel.com>
> >Subject: USB hub port reset
> >
> >EDK2 have reset recovery time of 10ms for hub port based on port 
> >status reset bit
> but USB spec doesn't mention that port status can be used for t6/reset 
> recovery time. Could you please clarify?
> >
> >USB vendor mentions EDK2 doesn't have reset recovery time on hub port 
> >reset and also highlighting EDK2 hub port reset differs from 
> >https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#
> >L2
> >789
> >
> >-Baranee
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: USB hub port reset
  2016-11-15 23:56           ` Anbazhagan, Baraneedharan
@ 2016-11-16  1:45             ` Tian, Feng
  2016-11-16  2:00               ` Anbazhagan, Baraneedharan
  0 siblings, 1 reply; 9+ messages in thread
From: Tian, Feng @ 2016-11-16  1:45 UTC (permalink / raw)
  To: Anbazhagan, Baraneedharan, edk2-devel@lists.01.org; +Cc: Tian, Feng

I would prefer to keep the logic in UsbHubResetPort() unchanged to avoid bring other device identification impacts.

Thanks
Feng

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Anbazhagan, Baraneedharan
Sent: Wednesday, November 16, 2016 7:56 AM
To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2] USB hub port reset

Feng,
Below change solves the problem with a device behind the hub. Do we still need the 10ms delay in UsbHubResetPort function for manual port reset in case of XHCI?

Thanks,
Baranee

-----Original Message-----
From: Tian, Feng [mailto:feng.tian@intel.com]
Sent: Monday, November 14, 2016 10:28 PM
To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2-devel@lists.01.org
Cc: Tian, Feng <feng.tian@intel.com>
Subject: RE: USB hub port reset

Baranee

Supposing are using latest usb code in EdkII trunk.

We send ADDRESS DEVICE CMD in XhcInitializeDeviceSlot(), which will cause XHC issue a USB SET_ADDRESS request to the USB Device.

There are two possible paths:
1. Reset_Change bit gets set after connection without manually port reset.
UsbHubGetPortStatus()->XhcPollPortStatusChange()->(if RESET_C bit is set)->XhcInitializeDeviceSlot() 2. Reset_Change bit gets set after manually Reset Port operation.
UsbHubResetPort()->UsbHubSetPortFeature()->Stall(20)-> UsbHubGetPortStatus()->(invoke above code in XHCI)->(if RESET_C bit is set)->Stall(10)

According to your info, it looks like we need add 10ms delay to XhcInitializeDeviceSlot() like below:

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index e37f674..bb68f34 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2115,6 +2115,7 @@ XhcInitializeDeviceSlot (
   // 8) Issue an Address Device Command for the Device Slot, where the command points to the Input
   //    Context data structure described above.
   //
+  gBS->Stall (10 * 1000);
   ZeroMem (&CmdTrbAddr, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT));
   CmdTrbAddr.PtrLo    = XHC_LOW_32BIT (PhyAddr);
@@ -2321,6 +2322,7 @@ XhcInitializeDeviceSlot64 (
   // 8) Issue an Address Device Command for the Device Slot, where the command points to the Input
   //    Context data structure described above.
   //
+  gBS->Stall (10 * 1000);
   ZeroMem (&CmdTrbAddr, sizeof (CmdTrbAddr));
   PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc->UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT_64));
   CmdTrbAddr.PtrLo    = XHC_LOW_32BIT (PhyAddr);

Could you please have a try to see if it solves your problem?

Thanks
Feng

-----Original Message-----
From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
Sent: Monday, November 14, 2016 9:46 PM
To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
Subject: RE: USB hub port reset

In case of XHCI, TRB_TYPE_ADDRESS_DEV/SetAddress occurs within XhcPollPortStatusChange. Seem we don't have 10ms/TRSTRCY for the devices behind hub connected to XHCI or am missing something here?

In EDK2, 10ms(TRSTRCY) delay occurs before clear port status and in case of Linux TRSTRCY delay is after clearing port status.

Thanks
Baranee


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Tian, Feng
> Sent: Sunday, November 13, 2016 10:38 PM
> To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2- 
> devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>
> Subject: Re: [edk2] USB hub port reset
> 
> I don't catch what you mean. We have had 10ms delay for port reset. 
> Which line of UsbHub.c do you think there is problem?
> 
> Quote from USB2.0 spec:
> "Hubs must be able to accept all hub requests and devices must be able 
> to accept a
> SetAddress() request (refer to Section 11.24.2 and Section 9.4
> respectively) after the reset recovery time 10 ms (TRSTRCY) after the reset is removed."
> 
> Thanks
> Feng
> 
> -----Original Message-----
> From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> Sent: Monday, November 14, 2016 11:33 AM
> To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
> Subject: RE: USB hub port reset
> 
> I didn't mean to have extra delays. Am asking whether EDKII have to be 
> updated for 10ms reset recovery time regardless of port status to align with spec?
> 
> 
> >From: Tian, Feng [mailto:feng.tian@intel.com]
> >Sent: Sunday, November 13, 2016 7:46 PM
> >To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; 
> >edk2-devel@lists.01.org
> >Cc: Tian, Feng <feng.tian@intel.com>
> >Subject: RE: USB hub port reset
> >
> >Hi, Baranee
> >
> >Linux wait 10 + 40 ms (TRSTRCY = 10 ms, plus extra 40 ms). Do you 
> >mean EDKII
> should be same with linux to wait more time?
> >
> >Thanks
> >Feng
> >
> >From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> >Sent: Sunday, November 13, 2016 11:53 AM
> >To: mailto:edk2-devel@lists.01.org; Tian, Feng 
> ><mailto:feng.tian@intel.com>
> >Subject: USB hub port reset
> >
> >EDK2 have reset recovery time of 10ms for hub port based on port 
> >status reset bit
> but USB spec doesn't mention that port status can be used for t6/reset 
> recovery time. Could you please clarify?
> >
> >USB vendor mentions EDK2 doesn't have reset recovery time on hub port 
> >reset and also highlighting EDK2 hub port reset differs from 
> >https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#
> >L2
> >789
> >
> >-Baranee
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: USB hub port reset
  2016-11-16  1:45             ` Tian, Feng
@ 2016-11-16  2:00               ` Anbazhagan, Baraneedharan
  0 siblings, 0 replies; 9+ messages in thread
From: Anbazhagan, Baraneedharan @ 2016-11-16  2:00 UTC (permalink / raw)
  To: Tian, Feng, edk2-devel@lists.01.org

Am OK with that. Need similar change in XhcPeiInitializeDeviceSlot and XhcPeiInitializeDeviceSlot64 for PEI.

-Baranee

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Tian,
> Feng
> Sent: Tuesday, November 15, 2016 7:45 PM
> To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2-
> devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>
> Subject: Re: [edk2] USB hub port reset
> 
> I would prefer to keep the logic in UsbHubResetPort() unchanged to avoid bring
> other device identification impacts.
> 
> Thanks
> Feng
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Anbazhagan, Baraneedharan
> Sent: Wednesday, November 16, 2016 7:56 AM
> To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] USB hub port reset
> 
> Feng,
> Below change solves the problem with a device behind the hub. Do we still need the
> 10ms delay in UsbHubResetPort function for manual port reset in case of XHCI?
> 
> Thanks,
> Baranee
> 
> -----Original Message-----
> From: Tian, Feng [mailto:feng.tian@intel.com]
> Sent: Monday, November 14, 2016 10:28 PM
> To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2-
> devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>
> Subject: RE: USB hub port reset
> 
> Baranee
> 
> Supposing are using latest usb code in EdkII trunk.
> 
> We send ADDRESS DEVICE CMD in XhcInitializeDeviceSlot(), which will cause XHC
> issue a USB SET_ADDRESS request to the USB Device.
> 
> There are two possible paths:
> 1. Reset_Change bit gets set after connection without manually port reset.
> UsbHubGetPortStatus()->XhcPollPortStatusChange()->(if RESET_C bit is set)-
> >XhcInitializeDeviceSlot() 2. Reset_Change bit gets set after manually Reset Port
> operation.
> UsbHubResetPort()->UsbHubSetPortFeature()->Stall(20)->
> UsbHubGetPortStatus()->(invoke above code in XHCI)->(if RESET_C bit is set)-
> >Stall(10)
> 
> According to your info, it looks like we need add 10ms delay to
> XhcInitializeDeviceSlot() like below:
> 
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> index e37f674..bb68f34 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
> @@ -2115,6 +2115,7 @@ XhcInitializeDeviceSlot (
>    // 8) Issue an Address Device Command for the Device Slot, where the command
> points to the Input
>    //    Context data structure described above.
>    //
> +  gBS->Stall (10 * 1000);
>    ZeroMem (&CmdTrbAddr, sizeof (CmdTrbAddr));
>    PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc-
> >UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT));
>    CmdTrbAddr.PtrLo    = XHC_LOW_32BIT (PhyAddr);
> @@ -2321,6 +2322,7 @@ XhcInitializeDeviceSlot64 (
>    // 8) Issue an Address Device Command for the Device Slot, where the command
> points to the Input
>    //    Context data structure described above.
>    //
> +  gBS->Stall (10 * 1000);
>    ZeroMem (&CmdTrbAddr, sizeof (CmdTrbAddr));
>    PhyAddr = UsbHcGetPciAddrForHostAddr (Xhc->MemPool, Xhc-
> >UsbDevContext[SlotId].InputContext, sizeof (INPUT_CONTEXT_64));
>    CmdTrbAddr.PtrLo    = XHC_LOW_32BIT (PhyAddr);
> 
> Could you please have a try to see if it solves your problem?
> 
> Thanks
> Feng
> 
> -----Original Message-----
> From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> Sent: Monday, November 14, 2016 9:46 PM
> To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
> Subject: RE: USB hub port reset
> 
> In case of XHCI, TRB_TYPE_ADDRESS_DEV/SetAddress occurs within
> XhcPollPortStatusChange. Seem we don't have 10ms/TRSTRCY for the devices
> behind hub connected to XHCI or am missing something here?
> 
> In EDK2, 10ms(TRSTRCY) delay occurs before clear port status and in case of Linux
> TRSTRCY delay is after clearing port status.
> 
> Thanks
> Baranee
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Tian, Feng
> > Sent: Sunday, November 13, 2016 10:38 PM
> > To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>; edk2-
> > devel@lists.01.org
> > Cc: Tian, Feng <feng.tian@intel.com>
> > Subject: Re: [edk2] USB hub port reset
> >
> > I don't catch what you mean. We have had 10ms delay for port reset.
> > Which line of UsbHub.c do you think there is problem?
> >
> > Quote from USB2.0 spec:
> > "Hubs must be able to accept all hub requests and devices must be able
> > to accept a
> > SetAddress() request (refer to Section 11.24.2 and Section 9.4
> > respectively) after the reset recovery time 10 ms (TRSTRCY) after the reset is
> removed."
> >
> > Thanks
> > Feng
> >
> > -----Original Message-----
> > From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> > Sent: Monday, November 14, 2016 11:33 AM
> > To: Tian, Feng <feng.tian@intel.com>; edk2-devel@lists.01.org
> > Subject: RE: USB hub port reset
> >
> > I didn't mean to have extra delays. Am asking whether EDKII have to be
> > updated for 10ms reset recovery time regardless of port status to align with spec?
> >
> >
> > >From: Tian, Feng [mailto:feng.tian@intel.com]
> > >Sent: Sunday, November 13, 2016 7:46 PM
> > >To: Anbazhagan, Baraneedharan <anbazhagan@hp.com>;
> > >edk2-devel@lists.01.org
> > >Cc: Tian, Feng <feng.tian@intel.com>
> > >Subject: RE: USB hub port reset
> > >
> > >Hi, Baranee
> > >
> > >Linux wait 10 + 40 ms (TRSTRCY = 10 ms, plus extra 40 ms). Do you
> > >mean EDKII
> > should be same with linux to wait more time?
> > >
> > >Thanks
> > >Feng
> > >
> > >From: Anbazhagan, Baraneedharan [mailto:anbazhagan@hp.com]
> > >Sent: Sunday, November 13, 2016 11:53 AM
> > >To: mailto:edk2-devel@lists.01.org; Tian, Feng
> > ><mailto:feng.tian@intel.com>
> > >Subject: USB hub port reset
> > >
> > >EDK2 have reset recovery time of 10ms for hub port based on port
> > >status reset bit
> > but USB spec doesn't mention that port status can be used for t6/reset
> > recovery time. Could you please clarify?
> > >
> > >USB vendor mentions EDK2 doesn't have reset recovery time on hub port
> > >reset and also highlighting EDK2 hub port reset differs from
> > >https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#
> > >L2
> > >789
> > >
> > >-Baranee
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-11-16  2:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-13  3:52 USB hub port reset Anbazhagan, Baraneedharan
2016-11-14  1:45 ` Tian, Feng
2016-11-14  3:32   ` Anbazhagan, Baraneedharan
2016-11-14  4:38     ` Tian, Feng
2016-11-14 13:46       ` Anbazhagan, Baraneedharan
2016-11-15  4:28         ` Tian, Feng
2016-11-15 23:56           ` Anbazhagan, Baraneedharan
2016-11-16  1:45             ` Tian, Feng
2016-11-16  2:00               ` Anbazhagan, Baraneedharan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox