[edk2] [PATCH v4 6/6] EmbeddedPkg: SiI3132: Enable SCSI pass-through protocol

Jeremy Linton posted 6 patches 7 years, 7 months ago
[edk2] [PATCH v4 6/6] EmbeddedPkg: SiI3132: Enable SCSI pass-through protocol
Posted by Jeremy Linton 7 years, 7 months ago
Now that everything is in place, lets export the protocol,
build the module, and remove the ATAPI unsupported flags.
Now when we detect an ATAPI device on a port we flag it
as such.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
 EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c   | 52 ++++++++++++++--------
 .../Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf      |  2 +
 2 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
index f494655..b98231c 100644
--- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
+++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
@@ -1,7 +1,7 @@
 /** @file
 *  PCIe Sata support for the Silicon Image I3132
 *
-*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
+*  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
 *
 *  This program and the accompanying materials
 *  are licensed and made available under the terms and conditions of the BSD License
@@ -16,6 +16,7 @@
 #include "SataSiI3132.h"
 
 #include <IndustryStandard/Acpi10.h>
+#include <IndustryStandard/Atapi.h>
 
 #include <Library/MemoryAllocationLib.h>
 #include <Library/UefiBootServicesTableLib.h>
@@ -88,7 +89,6 @@ SataSiI3132Constructor (
   )
 {
   SATA_SI3132_INSTANCE    *Instance;
-  EFI_ATA_PASS_THRU_MODE  *AtaPassThruMode;
 
   if (!SataSiI3132Instance) {
     return EFI_INVALID_PARAMETER;
@@ -102,16 +102,15 @@ SataSiI3132Constructor (
   Instance->Signature           = SATA_SII3132_SIGNATURE;
   Instance->PciIo               = PciIo;
 
-  AtaPassThruMode = (EFI_ATA_PASS_THRU_MODE*)AllocatePool (sizeof (EFI_ATA_PASS_THRU_MODE));
-  AtaPassThruMode->Attributes = EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL | EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;
-  AtaPassThruMode->IoAlign = 0x1000;
+  Instance->AtaPassThruMode.Attributes = EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL | EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;
+  Instance->AtaPassThruMode.IoAlign = 0x4;
 
   // Initialize SiI3132 ports
   SataSiI3132PortConstructor (Instance, 0);
   SataSiI3132PortConstructor (Instance, 1);
 
   // Set ATA Pass Thru Protocol
-  Instance->AtaPassThruProtocol.Mode            = AtaPassThruMode;
+  Instance->AtaPassThruProtocol.Mode            = &Instance->AtaPassThruMode;
   Instance->AtaPassThruProtocol.PassThru        = SiI3132AtaPassThru;
   Instance->AtaPassThruProtocol.GetNextPort     = SiI3132GetNextPort;
   Instance->AtaPassThruProtocol.GetNextDevice   = SiI3132GetNextDevice;
@@ -120,6 +119,20 @@ SataSiI3132Constructor (
   Instance->AtaPassThruProtocol.ResetPort       = SiI3132ResetPort;
   Instance->AtaPassThruProtocol.ResetDevice     = SiI3132ResetDevice;
 
+  Instance->ExtScsiPassThruMode.Attributes = EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_PHYSICAL |
+                                             EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_LOGICAL;
+  Instance->ExtScsiPassThruMode.IoAlign = 0x4;
+
+  // Set SCSI Pass Thru Protocol
+  Instance->ExtScsiPassThru.Mode                = &Instance->ExtScsiPassThruMode;
+  Instance->ExtScsiPassThru.PassThru            = SiI3132ScsiPassThru;
+  Instance->ExtScsiPassThru.GetNextTargetLun    = SiI3132GetNextTargetLun;
+  Instance->ExtScsiPassThru.BuildDevicePath     = SiI3132ScsiBuildDevicePath;
+  Instance->ExtScsiPassThru.GetTargetLun        = SiI3132GetTargetLun;
+  Instance->ExtScsiPassThru.ResetChannel        = SiI3132ResetChannel;
+  Instance->ExtScsiPassThru.ResetTargetLun      = SiI3132ResetTargetLun;
+  Instance->ExtScsiPassThru.GetNextTarget       = SiI3132GetNextTarget;
+
   *SataSiI3132Instance = Instance;
 
   return EFI_SUCCESS;
@@ -165,7 +178,9 @@ SataSiI3132PortInitialization (
   UINT32                  Signature;
   EFI_STATUS              Status;
   EFI_PCI_IO_PROTOCOL*    PciIo;
+  BOOLEAN                 Atapi;
 
+  Atapi  = FALSE;
   Status = SiI3132HwResetPort (Port);
   if (EFI_ERROR (Status)) {
     return Status;
@@ -177,33 +192,33 @@ SataSiI3132PortInitialization (
   Status = SATA_PORT_READ32 (Port->RegBase + SII3132_PORT_SSTATUS_REG, &Value32);
   if (!EFI_ERROR (Status) && (Value32 & 0x3)) {
     // Do a soft reset to see if it is a port multiplier
-    SATA_TRACE ("SataSiI3132PortInitialization: soft reset - it is a port multiplier\n");
+    SATA_TRACE ("SataSiI3132PortInitialization: soft reset - is it a port multiplier?\n");
     Status = SiI3132SoftResetCommand (Port, &Signature);
     if (!EFI_ERROR (Status)) {
       if (Signature == SII3132_PORT_SIGNATURE_PMP) {
-        SATA_TRACE ("SataSiI3132PortInitialization(): a Port Multiplier is present");
+        DEBUG ((DEBUG_ERROR, "SataSiI3132PortInitialization(): a Port Multiplier is present"));
         if (FeaturePcdGet (PcdSataSiI3132FeaturePMPSupport)) {
           ASSERT (0); // Not supported yet
         } else {
           return EFI_UNSUPPORTED;
         }
       } else if (Signature == SII3132_PORT_SIGNATURE_ATAPI) {
-        ASSERT (0); // Not supported yet
         SATA_TRACE ("SataSiI3132PortInitialization(): an ATAPI device is present");
-        return EFI_UNSUPPORTED;
+        Atapi = TRUE;
       } else if (Signature == SII3132_PORT_SIGNATURE_ATA) {
         SATA_TRACE ("SataSiI3132PortInitialization(): an ATA device is present");
       } else {
-        SATA_TRACE ("SataSiI3132PortInitialization(): Present device unknown!");
+        DEBUG ((DEBUG_ERROR, "SataSiI3132PortInitialization(): Present device unknown!"));
         ASSERT (0); // Not supported
         return EFI_UNSUPPORTED;
       }
 
       // Create Device
       Device            = (SATA_SI3132_DEVICE*)AllocatePool (sizeof (SATA_SI3132_DEVICE));
-      Device->Index     = Port->Index; //TODO: Could need to be fixed when SATA Port Multiplier support
+      Device->Index     = 0; //TODO: When port multiplers are supported this is the multiplier index
       Device->Port      = Port;
       Device->BlockSize = 0;
+      Device->Atapi     = Atapi;
 
       // Attached the device to the Sata Port
       InsertTailList (&Port->Devices, &Device->Link);
@@ -432,13 +447,12 @@ SataSiI3132DriverBindingStart (
     return Status;
   }
 
-  // Install Ata Pass Thru Protocol
-  Status = gBS->InstallProtocolInterface (
-              &Controller,
-              &gEfiAtaPassThruProtocolGuid,
-              EFI_NATIVE_INTERFACE,
-              &(SataSiI3132Instance->AtaPassThruProtocol)
-              );
+  Status = gBS->InstallMultipleProtocolInterfaces (&Controller,
+                                                   &gEfiAtaPassThruProtocolGuid,
+                                                   &(SataSiI3132Instance->AtaPassThruProtocol),
+                                                   &gEfiExtScsiPassThruProtocolGuid,
+                                                   &(SataSiI3132Instance->ExtScsiPassThru),
+                                                   NULL);
   if (EFI_ERROR (Status)) {
     goto FREE_POOL;
   }
diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf
index 69aaab3..eb6e2bd 100644
--- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf
+++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf
@@ -35,10 +35,12 @@
   SataSiI3132.c
   ComponentName.c
   SiI3132AtaPassThru.c
+  SiI3132ScsiPassThru.c
 
 [Protocols]
   gEfiPciIoProtocolGuid                         # Consumed
   gEfiAtaPassThruProtocolGuid                   # Produced
+  gEfiExtScsiPassThruProtocolGuid               # Produced
 
 [Pcd]
   gEmbeddedTokenSpaceGuid.PcdSataSiI3132FeaturePMPSupport
-- 
2.9.3

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v4 6/6] EmbeddedPkg: SiI3132: Enable SCSI pass-through protocol
Posted by Leif Lindholm 7 years, 7 months ago
On Tue, Mar 07, 2017 at 04:15:11PM -0600, Jeremy Linton wrote:
> Now that everything is in place, lets export the protocol,
> build the module, and remove the ATAPI unsupported flags.
> Now when we detect an ATAPI device on a port we flag it
> as such.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c   | 52 ++++++++++++++--------
>  .../Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf      |  2 +
>  2 files changed, 35 insertions(+), 19 deletions(-)
> 
> diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
> index f494655..b98231c 100644
> --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
> +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132.c
> @@ -1,7 +1,7 @@
>  /** @file
>  *  PCIe Sata support for the Silicon Image I3132
>  *
> -*  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +*  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
>  *
>  *  This program and the accompanying materials
>  *  are licensed and made available under the terms and conditions of the BSD License
> @@ -16,6 +16,7 @@
>  #include "SataSiI3132.h"
>  
>  #include <IndustryStandard/Acpi10.h>
> +#include <IndustryStandard/Atapi.h>
>  
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> @@ -88,7 +89,6 @@ SataSiI3132Constructor (
>    )
>  {
>    SATA_SI3132_INSTANCE    *Instance;
> -  EFI_ATA_PASS_THRU_MODE  *AtaPassThruMode;
>  
>    if (!SataSiI3132Instance) {
>      return EFI_INVALID_PARAMETER;
> @@ -102,16 +102,15 @@ SataSiI3132Constructor (
>    Instance->Signature           = SATA_SII3132_SIGNATURE;
>    Instance->PciIo               = PciIo;
>  
> -  AtaPassThruMode = (EFI_ATA_PASS_THRU_MODE*)AllocatePool (sizeof (EFI_ATA_PASS_THRU_MODE));
> -  AtaPassThruMode->Attributes = EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL | EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;
> -  AtaPassThruMode->IoAlign = 0x1000;
> +  Instance->AtaPassThruMode.Attributes = EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL | EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;
> +  Instance->AtaPassThruMode.IoAlign = 0x4;

Why 0x4? Could we have a #define with a descriptive name instead?

>  
>    // Initialize SiI3132 ports
>    SataSiI3132PortConstructor (Instance, 0);
>    SataSiI3132PortConstructor (Instance, 1);
>  
>    // Set ATA Pass Thru Protocol
> -  Instance->AtaPassThruProtocol.Mode            = AtaPassThruMode;
> +  Instance->AtaPassThruProtocol.Mode            = &Instance->AtaPassThruMode;
>    Instance->AtaPassThruProtocol.PassThru        = SiI3132AtaPassThru;
>    Instance->AtaPassThruProtocol.GetNextPort     = SiI3132GetNextPort;
>    Instance->AtaPassThruProtocol.GetNextDevice   = SiI3132GetNextDevice;
> @@ -120,6 +119,20 @@ SataSiI3132Constructor (
>    Instance->AtaPassThruProtocol.ResetPort       = SiI3132ResetPort;
>    Instance->AtaPassThruProtocol.ResetDevice     = SiI3132ResetDevice;
>  
> +  Instance->ExtScsiPassThruMode.Attributes = EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_PHYSICAL |
> +                                             EFI_EXT_SCSI_PASS_THRU_ATTRIBUTES_LOGICAL;
> +  Instance->ExtScsiPassThruMode.IoAlign = 0x4;

And here?

> +
> +  // Set SCSI Pass Thru Protocol
> +  Instance->ExtScsiPassThru.Mode                = &Instance->ExtScsiPassThruMode;
> +  Instance->ExtScsiPassThru.PassThru            = SiI3132ScsiPassThru;
> +  Instance->ExtScsiPassThru.GetNextTargetLun    = SiI3132GetNextTargetLun;
> +  Instance->ExtScsiPassThru.BuildDevicePath     = SiI3132ScsiBuildDevicePath;
> +  Instance->ExtScsiPassThru.GetTargetLun        = SiI3132GetTargetLun;
> +  Instance->ExtScsiPassThru.ResetChannel        = SiI3132ResetChannel;
> +  Instance->ExtScsiPassThru.ResetTargetLun      = SiI3132ResetTargetLun;
> +  Instance->ExtScsiPassThru.GetNextTarget       = SiI3132GetNextTarget;
> +
>    *SataSiI3132Instance = Instance;
>  
>    return EFI_SUCCESS;
> @@ -165,7 +178,9 @@ SataSiI3132PortInitialization (
>    UINT32                  Signature;
>    EFI_STATUS              Status;
>    EFI_PCI_IO_PROTOCOL*    PciIo;
> +  BOOLEAN                 Atapi;
>  
> +  Atapi  = FALSE;
>    Status = SiI3132HwResetPort (Port);
>    if (EFI_ERROR (Status)) {
>      return Status;
> @@ -177,33 +192,33 @@ SataSiI3132PortInitialization (
>    Status = SATA_PORT_READ32 (Port->RegBase + SII3132_PORT_SSTATUS_REG, &Value32);
>    if (!EFI_ERROR (Status) && (Value32 & 0x3)) {
>      // Do a soft reset to see if it is a port multiplier
> -    SATA_TRACE ("SataSiI3132PortInitialization: soft reset - it is a port multiplier\n");
> +    SATA_TRACE ("SataSiI3132PortInitialization: soft reset - is it a port multiplier?\n");
>      Status = SiI3132SoftResetCommand (Port, &Signature);
>      if (!EFI_ERROR (Status)) {
>        if (Signature == SII3132_PORT_SIGNATURE_PMP) {
> -        SATA_TRACE ("SataSiI3132PortInitialization(): a Port Multiplier is present");
> +        DEBUG ((DEBUG_ERROR, "SataSiI3132PortInitialization(): a Port Multiplier is present"));

What's the dignificance of switching from SATA_TRACE to DEBUG?
Upgrading the verbosity?

>          if (FeaturePcdGet (PcdSataSiI3132FeaturePMPSupport)) {
>            ASSERT (0); // Not supported yet
>          } else {
>            return EFI_UNSUPPORTED;
>          }
>        } else if (Signature == SII3132_PORT_SIGNATURE_ATAPI) {
> -        ASSERT (0); // Not supported yet
>          SATA_TRACE ("SataSiI3132PortInitialization(): an ATAPI device is present");
> -        return EFI_UNSUPPORTED;
> +        Atapi = TRUE;
>        } else if (Signature == SII3132_PORT_SIGNATURE_ATA) {
>          SATA_TRACE ("SataSiI3132PortInitialization(): an ATA device is present");
>        } else {
> -        SATA_TRACE ("SataSiI3132PortInitialization(): Present device unknown!");
> +        DEBUG ((DEBUG_ERROR, "SataSiI3132PortInitialization(): Present device unknown!"));
>          ASSERT (0); // Not supported
>          return EFI_UNSUPPORTED;
>        }
>  
>        // Create Device
>        Device            = (SATA_SI3132_DEVICE*)AllocatePool (sizeof (SATA_SI3132_DEVICE));
> -      Device->Index     = Port->Index; //TODO: Could need to be fixed when SATA Port Multiplier support
> +      Device->Index     = 0; //TODO: When port multiplers are supported this is the multiplier index
>        Device->Port      = Port;
>        Device->BlockSize = 0;
> +      Device->Atapi     = Atapi;
>  
>        // Attached the device to the Sata Port
>        InsertTailList (&Port->Devices, &Device->Link);
> @@ -432,13 +447,12 @@ SataSiI3132DriverBindingStart (
>      return Status;
>    }
>  
> -  // Install Ata Pass Thru Protocol
> -  Status = gBS->InstallProtocolInterface (
> -              &Controller,
> -              &gEfiAtaPassThruProtocolGuid,
> -              EFI_NATIVE_INTERFACE,
> -              &(SataSiI3132Instance->AtaPassThruProtocol)
> -              );
> +  Status = gBS->InstallMultipleProtocolInterfaces (&Controller,
> +                                                   &gEfiAtaPassThruProtocolGuid,
> +                                                   &(SataSiI3132Instance->AtaPassThruProtocol),
> +                                                   &gEfiExtScsiPassThruProtocolGuid,
> +                                                   &(SataSiI3132Instance->ExtScsiPassThru),
> +                                                   NULL);
>    if (EFI_ERROR (Status)) {
>      goto FREE_POOL;
>    }
> diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf
> index 69aaab3..eb6e2bd 100644
> --- a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf
> +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SataSiI3132Dxe.inf
> @@ -35,10 +35,12 @@
>    SataSiI3132.c
>    ComponentName.c
>    SiI3132AtaPassThru.c
> +  SiI3132ScsiPassThru.c
>  
>  [Protocols]
>    gEfiPciIoProtocolGuid                         # Consumed
>    gEfiAtaPassThruProtocolGuid                   # Produced
> +  gEfiExtScsiPassThruProtocolGuid               # Produced
>  
>  [Pcd]
>    gEmbeddedTokenSpaceGuid.PcdSataSiI3132FeaturePMPSupport
> -- 
> 2.9.3
> 
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel