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

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jeff
> Brasen
> Sent: Thursday, January 31, 2019 7:59 AM
> To: edk2-devel@lists.01.org
> Cc: Edgar Handal; Jeff Brasen
> Subject: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Use 16/32-bit IO
> widths
> 
> From: Edgar Handal <ehandal@nvidia.com>
> 
> 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 accessable in
> byte, word, and double word accesses.

Hi,

Thanks for the contribution. The change seems good to me.

Just curious, if the accesses are always slit into byte(8-bit), is there any
issue or performance impact is encountered during your usage?

It will be helpful to get more information on the purpose of the patch.
Thanks.

Best Regards,
Hao Wu

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.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 5aec8c6..82f4493 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -152,19 +152,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,
> @@ -173,7 +190,7 @@ SdMmcHcRwMmio (
>    } else {
>      Status = PciIo->Mem.Write (
>                            PciIo,
> -                          EfiPciIoWidthUint8,
> +                          Width,
>                            BarIndex,
>                            (UINT64) Offset,
>                            Count,
> --
> 2.7.4
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel