Re: USB hub port reset

["Anbazhagan, Baraneedharan" <anbazhagan@hp.com>]

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