Star, I agree. I will update patch once more to make sure Return status is meaningful. I will also remove ASSERT For Context NULL and return EFI_INVALID_PARAMETER Instead if either Context or MaxLun are NULL.
Mike > -----Original Message----- > From: Zeng, Star > Sent: Thursday, November 16, 2017 3:03 AM > To: Kinney, Michael D <[email protected]>; > [email protected] > Cc: Dong, Eric <[email protected]>; Zeng, Star > <[email protected]> > Subject: RE: [Patch V3 2/2] > MdeModulePkg/UsbMassStorageDxe: Check Get Max LUN > status/value > > Mike, > > After this patch, the return status of UsbBotGetMaxLun > seems confusing and useless. Fortunately, there is no > one using the return status of it. > What do you think? > > > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, November 16, 2017 11:58 AM > To: [email protected] > Cc: Zeng, Star <[email protected]>; Dong, Eric > <[email protected]> > Subject: [Patch V3 2/2] MdeModulePkg/UsbMassStorageDxe: > Check Get Max LUN status/value > > https://bugzilla.tianocore.org/show_bug.cgi?id=767 > > If a USB Mass Storage device does not support the Get > Max LUN command, then the USB I/O Protocol > ControlTransfer() service may return an error. If an > error is returned for this command, then assume that > the device does not support multiple LUNs and return a > maximum LUN value of 0. > > The USB Mass Storage Class Specification states that a > maximum LUN value larger than 0x0F is invalid. Add a > check to make sure this maximum LUN value is in this > valid range, and if it is not, then assume that the > device does not support multiple LUNs and return a > maximum LUN value of 0. > > This change improves compatibility with USB FLASH > drives that do not support the Get Max LUN command or > return an invalid maximum LUN value. > > Cc: Star Zeng <[email protected]> > Cc: Eric Dong <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Michael D Kinney > <[email protected]> > --- > MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c | > 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git > a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c > b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c > index 4bb7222b89..6afe2ae235 100644 > --- > a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c > +++ > b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBot.c > @@ -2,7 +2,7 @@ > Implementation of the USB mass storage Bulk-Only > Transport protocol, > according to USB Mass Storage Class Bulk-Only > Transport, Revision 1.0. > > -Copyright (c) 2007 - 2011, Intel Corporation. All > rights reserved.<BR> > +Copyright (c) 2007 - 2017, Intel Corporation. All > rights reserved.<BR> > This program and the accompanying materials are > licensed and made available under the terms and > conditions of the BSD License which accompanies this > distribution. The full text of the license may be > found at @@ -576,6 +576,18 @@ UsbBotGetMaxLun ( > 1, > &Result > ); > + if (EFI_ERROR (Status) || *MaxLun > USB_BOT_MAX_LUN) > { > + // > + // If the Get LUN request returns an error or the > MaxLun is larger than > + // the maximum LUN value (0x0f) supported by the > USB Mass Storage Class > + // Bulk-Only Transport Spec, then set MaxLun to 0. > + // > + // This improves compatibility with USB FLASH > drives that have a single LUN > + // and either do not return a max LUN value or > return an invalid maximum LUN > + // value. > + // > + *MaxLun = 0; > + } > > return Status; > } > -- > 2.14.2.windows.3 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
