Re: [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices

["Chiu, Chasel" <chasel.chiu@intel.com>]

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

-----Original Message-----
From: Ni, Ruiyu 
Sent: Friday, June 1, 2018 1:39 PM
To: edk2-devel@lists.01.org
Cc: Chiu, Chasel <chasel.chiu@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH 1/4] MdeModulePkg/AtaAtapiPassThru: Spin up Power up in Standby devices

The patch adds support to certain devices that support PUIS (Power up in Standby).
For those devices that supports SET_FEATURE spin up, SW needs to send SET_FEATURE subcommand to spin up the devices.
For those devices that doesn't support SET_FEATURE spin up, SW needs to send read sectors command to spin up the devices.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c   | 129 ++++++++++++++++++++-
 .../Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h    |   3 +-
 2 files changed, 127 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index e6de5d65bc..6ab7870570 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -1,7 +1,7 @@
 /** @file
   The file for AHCI mode of ATA host controller.
 
-  Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2010 - 2018, Intel Corporation. All rights 
+ reserved.<BR>
   (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License @@ -1841,7 +1841,8 @@ AhciDeviceSetFeature (
   IN UINT8                  Port,
   IN UINT8                  PortMultiplier,
   IN UINT16                 Feature,
-  IN UINT32                 FeatureSpecificData
+  IN UINT32                 FeatureSpecificData,
+  IN UINT64                 Timeout
   )
 {
   EFI_STATUS               Status;
@@ -1868,7 +1869,7 @@ AhciDeviceSetFeature (
              0,
              &AtaCommandBlock,
              &AtaStatusBlock,
-             ATA_ATAPI_TIMEOUT,
+             Timeout,
              NULL
              );
 
@@ -2216,6 +2217,104 @@ Error6:
   return Status;
 }
 
+
+/**
+  Spin-up disk if IDD was incomplete or PUIS feature is enabled
+
+  @param  PciIo               The PCI IO protocol instance.
+  @param  AhciRegisters       The pointer to the EFI_AHCI_REGISTERS.
+  @param  Port                The number of port.
+  @param  PortMultiplier      The multiplier of port.
+  @param  IdentifyData        A pointer to data buffer which is used to contain IDENTIFY data.
+
+**/
+EFI_STATUS
+AhciSpinUpDisk (
+  IN EFI_PCI_IO_PROTOCOL           *PciIo,
+  IN EFI_AHCI_REGISTERS            *AhciRegisters,
+  IN UINT8                         Port,
+  IN UINT8                         PortMultiplier,
+  IN OUT EFI_IDENTIFY_DATA         *IdentifyData
+  )
+{
+  EFI_STATUS               Status;
+  EFI_ATA_COMMAND_BLOCK    AtaCommandBlock;
+  EFI_ATA_STATUS_BLOCK     AtaStatusBlock;
+  UINT8                    Buffer[512];
+
+  if (IdentifyData->AtaData.specific_config == ATA_SPINUP_CFG_REQUIRED_IDD_INCOMPLETE) {
+    //
+    // Use SET_FEATURE subcommand to spin up the device.
+    //
+    Status = AhciDeviceSetFeature (
+               PciIo, AhciRegisters, Port, PortMultiplier,
+               ATA_SUB_CMD_PUIS_SET_DEVICE_SPINUP, 0x00, ATA_SPINUP_TIMEOUT
+               );
+    DEBUG ((DEBUG_INFO, "CMD_PUIS_SET_DEVICE_SPINUP for device at port [%d] PortMultiplier [%d] - %r!\n",
+            Port, PortMultiplier, Status));
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  } else {
+    ASSERT (IdentifyData->AtaData.specific_config == 
+ ATA_SPINUP_CFG_NOT_REQUIRED_IDD_INCOMPLETE);
+
+    //
+    // Use READ_SECTORS to spin up the device if SpinUp SET FEATURE subcommand is not supported
+    //
+    ZeroMem (&AtaCommandBlock, sizeof (EFI_ATA_COMMAND_BLOCK));
+    ZeroMem (&AtaStatusBlock, sizeof (EFI_ATA_STATUS_BLOCK));
+    //
+    // Perform READ SECTORS PIO Data-In command to Read LBA 0
+    //
+    AtaCommandBlock.AtaCommand      = ATA_CMD_READ_SECTORS;
+    AtaCommandBlock.AtaSectorCount  = 0x1;
+
+    Status = AhciPioTransfer (
+               PciIo,
+               AhciRegisters,
+               Port,
+               PortMultiplier,
+               NULL,
+               0,
+               TRUE,
+               &AtaCommandBlock,
+               &AtaStatusBlock,
+               &Buffer,
+               sizeof (Buffer),
+               ATA_SPINUP_TIMEOUT,
+               NULL
+               );
+    DEBUG ((DEBUG_INFO, "Read LBA 0 for device at port [%d] PortMultiplier [%d] - %r!\n",
+            Port, PortMultiplier, Status));
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  }
+
+  //
+  // Read the complete IDENTIFY DEVICE data.
+  //
+  ZeroMem (IdentifyData, sizeof (*IdentifyData));  Status = 
+ AhciIdentify (PciIo, AhciRegisters, Port, PortMultiplier, 
+ IdentifyData);  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Read IDD failed for device at port [%d] PortMultiplier [%d] - %r!\n",
+            Port, PortMultiplier, Status));
+    return Status;
+  }
+
+  DEBUG ((DEBUG_INFO, "IDENTIFY DEVICE: [0] = %016x, [2] = %016x, [83] = %016x, [86] = %016x\n",
+          IdentifyData->AtaData.config, IdentifyData->AtaData.specific_config,
+          IdentifyData->AtaData.command_set_supported_83, 
+ IdentifyData->AtaData.command_set_feature_enb_86));
+  //
+  // Check if IDD is incomplete
+  //
+  if ((IdentifyData->AtaData.config & BIT2) != 0) {
+    return EFI_DEVICE_ERROR;
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   Initialize ATA host controller at AHCI mode.
 
@@ -2458,6 +2557,28 @@ AhciModeInitialization (
           continue;
         }
 
+        DEBUG ((
+          DEBUG_INFO, "IDENTIFY DEVICE: [0] = %016x, [2] = %016x, [83] = %016x, [86] = %016x\n",
+          Buffer.AtaData.config, Buffer.AtaData.specific_config,
+          Buffer.AtaData.command_set_supported_83, Buffer.AtaData.command_set_feature_enb_86
+          ));
+        if ((Buffer.AtaData.config & BIT2) != 0) {
+          //
+          // SpinUp disk if device reported incomplete IDENTIFY DEVICE.
+          //
+          Status = AhciSpinUpDisk (
+                     PciIo,
+                     AhciRegisters,
+                     Port,
+                     0,
+                     &Buffer
+                     );
+          if (EFI_ERROR (Status)) {
+            DEBUG ((DEBUG_ERROR, "Spin up standby device failed - %r\n", Status));
+            continue;
+          }
+        }
+
         DeviceType = EfiIdeHarddisk;
       } else {
         continue;
@@ -2523,7 +2644,7 @@ AhciModeInitialization (
         TransferMode.ModeNumber = (UINT8) SupportedModes->MultiWordDmaMode.Mode;
       }
 
-      Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, 0, 0x03, (UINT32)(*(UINT8 *)&TransferMode));
+      Status = AhciDeviceSetFeature (PciIo, AhciRegisters, Port, 0, 
+ 0x03, (UINT32)(*(UINT8 *)&TransferMode), ATA_ATAPI_TIMEOUT); // 
+ RPPO-SKL-TBD
       if (EFI_ERROR (Status)) {
         DEBUG ((EFI_D_ERROR, "Set transfer Mode Fail, Status = %r\n", Status));
         continue;
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 85e5a55539..31b005f2f6 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -1,7 +1,7 @@
 /** @file
   Header file for ATA/ATAPI PASS THRU driver.
 
-  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2010 - 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 @@ -148,6 +148,7 @@ struct _ATA_NONBLOCK_TASK {  // It means 3 second span.
 //
 #define ATA_ATAPI_TIMEOUT           EFI_TIMER_PERIOD_SECONDS(3)
+#define ATA_SPINUP_TIMEOUT          EFI_TIMER_PERIOD_SECONDS(10)
 
 #define IS_ALIGNED(addr, size)      (((UINTN) (addr) & (size - 1)) == 0)
 
--
2.16.1.windows.1