public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Anbazhagan, Baraneedharan" <anbazhagan@hp.com>
To: "Tian, Feng" <feng.tian@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: USB hub port reset
Date: Tue, 15 Nov 2016 23:56:11 +0000	[thread overview]
Message-ID: <AT5PR84MB02760E76966AD0F52275DDA1BABF0@AT5PR84MB0276.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <7F1BAD85ADEA444D97065A60D2E97EE566E4FADD@SHSMSX101.ccr.corp.intel.com>

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


  reply	other threads:[~2016-11-15 23:56 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2016-11-16  1:45             ` Tian, Feng
2016-11-16  2:00               ` Anbazhagan, Baraneedharan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AT5PR84MB02760E76966AD0F52275DDA1BABF0@AT5PR84MB0276.NAMPRD84.PROD.OUTLOOK.COM \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox