public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ruiyu Ni <ruiyu.ni@intel.com>
To: edk2-devel@lists.01.org
Cc: Star Zeng <star.zeng@intel.com>
Subject: [PATCH 5/5] MdeModulePkg/UsbMass: Retry CMD for MediaChanged sense key
Date: Fri,  2 Mar 2018 19:54:37 +0800	[thread overview]
Message-ID: <20180302115437.140240-6-ruiyu.ni@intel.com> (raw)
In-Reply-To: <20180302115437.140240-1-ruiyu.ni@intel.com>

When a USB device reports failure for a CMD and REQUEST SENSE returns
Media Changed key, UsbBootExecCmdWithRetry() stops to retry CMD and
returns EFI_MEDIA_CHANGED to caller.

For this case, the CMD should be retried until success, getting
NoMedia sense key or timeout.
The patch updates UsbBootExecCmdWithRetry() to follow the above
rule so EFI_MEDIA_CHANGED is no longer returned.

UsbBootDetectMedia() is updated accordingly.

Because UsbBootGetParams() is called for new plugged USB storage,
and some USB storage devices may report Media Changed key,
UsbBootGetParams() is updated to treat it as a Success.
This change could fix the issue that some USB storage devices
cannot be detected.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 .../Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c        | 60 ++++++++++++++--------
 .../Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c        | 10 +++-
 2 files changed, 48 insertions(+), 22 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
index dd4b3a5f0a..b84bfd2d72 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
@@ -234,7 +234,7 @@ UsbBootExecCmd (
   @param  Timeout                The timeout used to transfer
 
   @retval EFI_SUCCESS            The command is executed successfully.
-  @retval EFI_MEDIA_CHANGED      The device media has been changed.
+  @retval EFI_NO_MEDIA           The device media is removed.
   @retval Others                 Command execution failed after retrial.
 
 **/
@@ -284,7 +284,7 @@ UsbBootExecCmdWithRetry (
                DataLen,
                Timeout
                );
-    if (Status == EFI_SUCCESS || Status == EFI_MEDIA_CHANGED || Status == EFI_NO_MEDIA) {
+    if (Status == EFI_SUCCESS || Status == EFI_NO_MEDIA) {
       break;
     }
     //
@@ -700,30 +700,42 @@ UsbBootDetectMedia (
   CmdSet = ((EFI_USB_INTERFACE_DESCRIPTOR *) (UsbMass->Context))->InterfaceSubClass;
 
   Status = UsbBootIsUnitReady (UsbMass);
-  if (EFI_ERROR (Status) && (Status != EFI_MEDIA_CHANGED)) {
-    goto ON_ERROR;
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_ERROR, "UsbBootDetectMedia: UsbBootIsUnitReady (%r)\n", Status));
   }
 
-  if ((UsbMass->Pdt != USB_PDT_CDROM) && (CmdSet == USB_MASS_STORE_SCSI)) {
-    //
-    // MODE SENSE is required for the device with PDT of 0x00/0x07/0x0E,
-    // according to Section 4 of USB Mass Storage Specification for Bootability.
-    // MODE SENSE(10) is useless here, while MODE SENSE(6) defined in SCSI
-    // could get the information of Write Protected.
-    // Since not all device support this command, skip if fail.
-    //
-    UsbScsiModeSense (UsbMass);
-  }
+  //
+  // Status could be:
+  //   EFI_SUCCESS: all good.
+  //   EFI_NO_MEDIA: media is not present.
+  //   others: HW error.
+  // For either EFI_NO_MEDIA, or HW error, skip to get WriteProtected and capacity information.
+  //
+  if (!EFI_ERROR (Status)) {
+    if ((UsbMass->Pdt != USB_PDT_CDROM) && (CmdSet == USB_MASS_STORE_SCSI)) {
+      //
+      // MODE SENSE is required for the device with PDT of 0x00/0x07/0x0E,
+      // according to Section 4 of USB Mass Storage Specification for Bootability.
+      // MODE SENSE(10) is useless here, while MODE SENSE(6) defined in SCSI
+      // could get the information of Write Protected.
+      // Since not all device support this command, skip if fail.
+      //
+      UsbScsiModeSense (UsbMass);
+    }
 
-  Status = UsbBootReadCapacity (UsbMass);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((EFI_D_ERROR, "UsbBootDetectMedia: UsbBootReadCapacity (%r)\n", Status));
-    goto ON_ERROR;
+    Status = UsbBootReadCapacity (UsbMass);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_ERROR, "UsbBootDetectMedia: UsbBootReadCapacity (%r)\n", Status));
+    }
   }
 
-  return EFI_SUCCESS;
+  if (EFI_ERROR (Status) && Status != EFI_NO_MEDIA) {
+    //
+    // For NoMedia, BlockIo is still needed.
+    //
+    return Status;
+  }
 
-ON_ERROR:
   //
   // Detect whether it is necessary to reinstall the Block I/O Protocol.
   //
@@ -744,6 +756,10 @@ ON_ERROR:
     //   DriverBindingStart(), which raises to TPL_CALLBACK.
     ASSERT (EfiGetCurrentTpl () == TPL_CALLBACK);
 
+    //
+    // When it is called from DriverBindingStart(), below reinstall fails.
+    // So ignore the return status check.
+    //
     gBS->ReinstallProtocolInterface (
            UsbMass->Controller,
            &gEfiBlockIoProtocolGuid,
@@ -752,7 +768,7 @@ ON_ERROR:
            );
 
     //
-    // Update MediaId after reinstalling Block I/O Protocol.
+    // Reset MediaId after reinstalling Block I/O Protocol.
     //
     if (Media->MediaPresent != OldMedia.MediaPresent) {
       if (Media->MediaPresent) {
@@ -767,6 +783,8 @@ ON_ERROR:
         (Media->LastBlock != OldMedia.LastBlock)) {
       Media->MediaId++;
     }
+
+    Status = Media->MediaPresent ? EFI_MEDIA_CHANGED : EFI_NO_MEDIA;
   }
 
   return Status;
diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c
index 9d1bb25fb3..bb292ed3eb 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassImpl.c
@@ -1,7 +1,7 @@
 /** @file
   USB Mass Storage Driver that manages USB Mass Storage Device and produces Block I/O Protocol.
 
-Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2007 - 2018, 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
@@ -361,6 +361,14 @@ UsbMassInitMedia (
   Media->MediaId          = 1;
 
   Status = UsbBootGetParams (UsbMass);
+  DEBUG ((DEBUG_INFO, "UsbMassInitMedia: UsbBootGetParams (%r)\n", Status));
+  if (Status == EFI_MEDIA_CHANGED) {
+    //
+    // Some USB storage devices may report MEDIA_CHANGED sense key when hot-plugged.
+    // Treat it as SUCCESS
+    //
+    Status = EFI_SUCCESS;
+  }
   return Status;
 }
 
-- 
2.16.1.windows.1



  parent reply	other threads:[~2018-03-02 11:48 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-02 11:54 [PATCH 0/5] Fix UsbMassStorage driver for device compatibility Ruiyu Ni
2018-03-02 11:54 ` [PATCH 1/5] MdeModulePkg/UsbMass: Revert POWER_ON(29h) ASC handling logic Ruiyu Ni
2018-03-02 11:54 ` [PATCH 2/5] MdeModulePkg/UsbMass: Revert "map -r" media change detection fix Ruiyu Ni
2018-03-02 11:54 ` [PATCH 3/5] MdeModulePkg/UsbMass: Add more debug message Ruiyu Ni
2018-03-02 11:54 ` [PATCH 4/5] MdeModulePkg/UsbMass: remove unnecessary RestoreTPL() call Ruiyu Ni
2018-03-02 11:54 ` Ruiyu Ni [this message]
2018-03-03  7:22 ` [PATCH 0/5] Fix UsbMassStorage driver for device compatibility Zeng, Star

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=20180302115437.140240-6-ruiyu.ni@intel.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