Hi Feng,
I see what Baranee is saying.
The UEFI spec says in describing UsbBulkTransfer " If an error other than
timeout occurs during the USB
transfer, then EFI_DEVICE_ERROR is returned and the detailed status code
will be returned in the
Status parameter."
I suggest that the following change would make XhcBulkTransfer the same as
EhcBulkTransfer and would result in consistent behavior of UsbBulkTransfer
on both EHCI and xHCI hardware.
diff --git a/trunk/edk2/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
b/trunk/edk2/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
--- a/trunk/edk2/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c (revision 17702)
+++ b/trunk/edk2/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c (working copy)
@@ -1243,10 +1243,12 @@
if (*TransferResult == EFI_USB_NOERROR) {
Status = EFI_SUCCESS;
- } else if (*TransferResult == EFI_USB_ERR_STALL) {
- RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
- if (EFI_ERROR (RecoveryStatus)) {
- DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint
failed\n"));
+ } else {
+ if (*TransferResult == EFI_USB_ERR_STALL) {
+ RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
+ if (EFI_ERROR (RecoveryStatus)) {
+ DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: XhcRecoverHaltedEndpoint
failed\n"));
+ }
}
Status = EFI_DEVICE_ERROR;
}
Regards,
Eric
> -----Original Message-----
> From: Tian, Feng [mailto:feng.t...@intel.com]
> Sent: Wednesday, June 24, 2015 5:57 PM
> To: edk2-devel@lists.sourceforge.net; Eric Wittmayer; anbazha...@hp.com
> Cc: Tian, Feng
> Subject: RE: [edk2] XHCI question
>
> Hi, Baranee
>
> 1. XhcCheckUrbResult() reports URB transfer state in Urb->Result rather
than
> Urb->Finished.
> 2. XhcCheckUrbResult() is an internal function rather than an exposed
> interface for upper layer.
> 3. XhcBulkTransfer() returns status according to Urb->Result rather than
> others.
>
> Please let me know what's the real issue you met.
>
> Thanks
> Feng
>
> -----Original Message-----
> From: Anbazhagan, Baraneedharan [mailto:anbazha...@hp.com]
> Sent: Thursday, June 25, 2015 06:18
> To: Eric Wittmayer; edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] XHCI question
>
> > > > > Whether XhcCheckUrbResult returns correct error status in case
> > > > > of
> > > failure?
> > > > > XhcCheckUrbResult function header indicates it should report URB
> > > > > transfer state - Urb->Finished (seems to align with EhciDxe) but
> > > > > the implementation is trying to return EFI_STATUS and it doesn't
> > > > > get updated based on EvtTrb-
> > > > > >Completecode. With a failing drive, XhcExecTransfer returns
> > > > > >EFI_SUCCESS
> > > > in
> > > > > case of transaction error.
> > > > >
> > > > Looking at the source code for XhcCheckUrbResult, if you pass in a
> > > > URB Pointer, the
> > > > Urb->Result is updated with the completion code from the event.
> > > > Look for the switch on EvtTrb->Completecode. In the case where
> > > > Urb is passed in, CheckedUrb == Urb.
> > > > EFI_STATUS that is returned indicates if the function actually
> > > > checked any new events.
> > > >
> > > > Regards,
> > > > Eric
> > > >
> > > If return value of XhcCheckUrbResult is a status of whether it
> > > checked any new events or not, then XhcBulkTransfer needs to be
> > > updated to return correct failure status to the caller instead of
> > > checking for
> > EFI_USB_ERR_STALL
> > > and EFI_USB_NOERROR alone .
> > >
> >
> >
> > Error status is returned to the caller in TransferResult ( the last
> > parameter to XhcBulkTransfer ).
> > The check you are talking about is used to recover the endpoint to a
> > running state if it stalled.
> > Currently if there is a transaction error, the caller will find
> > EFI_USB_ERR_TIMEOUT in TransferResult when XhcBulkTransfer
> completes.
> > What do you think should be done differently?
> >
> > Regards,
> > Eric
> >
> >
> >
> It needs to be consistent between USB controller modules - Ehci/Xhci. XHCI
> BulkTransfer returns success if event is checked and caller is expected to
look
> for TransferResult. EHCI module passes error back to caller in Status.
UsbBot
> device driver checks for status on bulk transfer call and it looks for
> TransferResult if status is not success.
>
> -Baranee
>
>
>
----------------------------------------------------------------------------
--
> Monitor 25 network devices or servers for free with OpManager!
> OpManager is web-based network management software that monitors
> network devices and physical & virtual servers, alerts via email & sms for
> fault. Monitor 25 devices for free with no restriction. Download now
> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors
network devices and physical & virtual servers, alerts via email & sms
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
