> -----Original Message-----
> From: Eric Wittmayer [mailto:e...@frescologic.com]
> Sent: Wednesday, June 24, 2015 8:15 PM
> To: 'Tian, Feng'; edk2-devel@lists.sourceforge.net; Anbazhagan, Baraneedharan
> Subject: RE: [edk2] XHCI question
>
> 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.
>
Eric's code change will address the issue and also safe to check for EvTrb !=
NULL before processing it.
Index: XhciSched.c
===================================================================
--- XhciSched.c (revision 17702)
+++ XhciSched.c (working copy)
@@ -1074,7 +1074,11 @@
//
goto EXIT;
}
-
+ if(EvtTrb == NULL)
+ {
+ Status = EFI_DEVICE_ERROR;
+ goto EXIT;
+ }
//
// Only handle COMMAND_COMPLETETION_EVENT and TRANSFER_EVENT.
//
> 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
