[PATCH v2] MdeModulePkg/SdMmcPciHcDxe: Use 16/32-bit IO widths

[PATCH v2] MdeModulePkg/SdMmcPciHcDxe: Use 16/32-bit IO widths

[Jeff Brasen]

From: Edgar Handal <ehandal@nvidia.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1537

Use 16-bit and 32-bit IO widths for SDMMC MMIO to prevent all register
accesses from being split up into 8-bit accesses.

The SDHCI specification states that the registers shall be accessible in
byte, word, and double word accesses. (SD Host Controller Simplified
Specification 4.20 Section 1.2)

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 25 ++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 4c64da3..d73fa10 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -154,19 +154,36 @@ SdMmcHcRwMmio (
   )
 {
   EFI_STATUS                   Status;
+  EFI_PCI_IO_PROTOCOL_WIDTH    Width;
 
   if ((PciIo == NULL) || (Data == NULL))  {
     return EFI_INVALID_PARAMETER;
   }
 
-  if ((Count != 1) && (Count != 2) && (Count != 4) && (Count != 8)) {
-    return EFI_INVALID_PARAMETER;
+  switch (Count) {
+    case 1:
+      Width = EfiPciIoWidthUint8;
+      break;
+    case 2:
+      Width = EfiPciIoWidthUint16;
+      Count = 1;
+      break;
+    case 4:
+      Width = EfiPciIoWidthUint32;
+      Count = 1;
+      break;
+    case 8:
+      Width = EfiPciIoWidthUint32;
+      Count = 2;
+      break;
+    default:
+      return EFI_INVALID_PARAMETER;
   }
 
   if (Read) {
     Status = PciIo->Mem.Read (
                           PciIo,
-                          EfiPciIoWidthUint8,
+                          Width,
                           BarIndex,
                           (UINT64) Offset,
                           Count,
@@ -175,7 +192,7 @@ SdMmcHcRwMmio (
   } else {
     Status = PciIo->Mem.Write (
                           PciIo,
-                          EfiPciIoWidthUint8,
+                          Width,
                           BarIndex,
                           (UINT64) Offset,
                           Count,
-- 
2.7.4

Re: [PATCH v2] MdeModulePkg/SdMmcPciHcDxe: Use 16/32-bit IO widths

[Wu, Hao A]

Thanks Jeff.

Reviewed-by: Hao Wu <hao.a.wu@intel.com>
And pushed via commit f168816c49e388dcd097dd62d766d63f73aaabb3.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Jeff Brasen [mailto:jbrasen@nvidia.com]
> Sent: Wednesday, February 20, 2019 1:07 AM
> To: edk2-devel@lists.01.org
> Cc: Edgar Handal; Jeff Brasen; Wu, Hao A
> Subject: [PATCH v2] MdeModulePkg/SdMmcPciHcDxe: Use 16/32-bit IO
> widths
> 
> From: Edgar Handal <ehandal@nvidia.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1537
> 
> Use 16-bit and 32-bit IO widths for SDMMC MMIO to prevent all register
> accesses from being split up into 8-bit accesses.
> 
> The SDHCI specification states that the registers shall be accessible in
> byte, word, and double word accesses. (SD Host Controller Simplified
> Specification 4.20 Section 1.2)
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 25
> ++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 4c64da3..d73fa10 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -154,19 +154,36 @@ SdMmcHcRwMmio (
>    )
>  {
>    EFI_STATUS                   Status;
> +  EFI_PCI_IO_PROTOCOL_WIDTH    Width;
> 
>    if ((PciIo == NULL) || (Data == NULL))  {
>      return EFI_INVALID_PARAMETER;
>    }
> 
> -  if ((Count != 1) && (Count != 2) && (Count != 4) && (Count != 8)) {
> -    return EFI_INVALID_PARAMETER;
> +  switch (Count) {
> +    case 1:
> +      Width = EfiPciIoWidthUint8;
> +      break;
> +    case 2:
> +      Width = EfiPciIoWidthUint16;
> +      Count = 1;
> +      break;
> +    case 4:
> +      Width = EfiPciIoWidthUint32;
> +      Count = 1;
> +      break;
> +    case 8:
> +      Width = EfiPciIoWidthUint32;
> +      Count = 2;
> +      break;
> +    default:
> +      return EFI_INVALID_PARAMETER;
>    }
> 
>    if (Read) {
>      Status = PciIo->Mem.Read (
>                            PciIo,
> -                          EfiPciIoWidthUint8,
> +                          Width,
>                            BarIndex,
>                            (UINT64) Offset,
>                            Count,
> @@ -175,7 +192,7 @@ SdMmcHcRwMmio (
>    } else {
>      Status = PciIo->Mem.Write (
>                            PciIo,
> -                          EfiPciIoWidthUint8,
> +                          Width,
>                            BarIndex,
>                            (UINT64) Offset,
>                            Count,
> --
> 2.7.4