Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.

2018-11-30 Thread Wu, Hao A
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Friday, November 30, 2018 2:48 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add
> V4 64bit SDMA and ADMA2 support.
> 
> Hello Hao,
> 
> Answers to your comments:
> 
> 1. Patch 5 fixes the issue with accessing SD as well as eMMC files.

Hello,

The issue still exists on my side.
Please refer to the comments within the V6 2/2 patch.

> 2. Using Private variable for controller version would mean I can only access 
> it
> in functions having an instance of the installed protocol which is not 
> possible
> in some functions for example where we set clock dividers. We either need
> to use what I have or pass Private or controller version as an argument to
> functions using it.

The solution for this is not very complex.

I checked all the callers of SdMmcHcGetControllerVersion() in this patch:

SdMmcHcClockSupply()
SdMmcHcInitV4Enhancements()
BuildAdmaDescTable()
SdMmcExecTrb()
SdMmcCheckTrbResult()

For SdMmcExecTrb() and SdMmcCheckTrbResult(), they already have the access to
'Private'.

For SdMmcHcInitV4Enhancements() and BuildAdmaDescTable(), their callers all
have the access to 'Private'. So we can either:

1. Add 'Private' to the input parameter
2. Add 'ControllerVersion' to the input parameter

for the above 2 functions. Personally, I prefer the 2nd option.

As for SdMmcHcClockSupply(), among its callers, only SdMmcHcInitClockFreq()
does not have direct access to 'Private'. However, SdMmcHcInitClockFreq() is
able to get the ControllerVersion or 'Private' from its caller.

Hence, ultimately, we can call function SdMmcHcGetControllerVersion() only once
at function SdMmcPciHcDriverBindingStart() right after the
SdMmcHcGetMaxCurrent() call. The result can be saved in the 'ControllerVersion'
field of the "SD_MMC_HC_PRIVATE_DATA" structure.

Also, the type of field 'ControllerVersion' in "SD_MMC_HC_PRIVATE_DATA" can be
updated to UINT16 from UINT32.

> 3. I think there is a bigger issue here. During host controller 
> initialization we
> need to setup 64b addressing which happens before we initialize 64b DMA in
> PCI layer. So what you are suggesting may not be that helpful. Also, what we
> are doing right now is that we go through all slots and if controller on any 
> slot
> does not support 64b, we do not enable 64b DMA in PCI layer. What is the
> likelihood of all controllers on an SOC not supporting similar address width?

I am not sure on this one.

All the devices I own seem to only have 1 slot on the controller. And all the
SD host controllers in a system seem to have the same address width support.

But since this driver is a general one, let us assume there will be such a
case here.

> Also, shouldn't we be enabling 64b DMA in PCI layer if any of the controller
> supports 64b addressing? Wouldn't 64b DMA enabled mean 32b is enabled
> by default?

I think your handling in the proposed v6 1/2 patch is good.

Since the Pci IO protocol is managing all the slots on one SD controller, when
64-bit DMA is enabled in the PCI layer, there will be potential issues if one
or more slots only support 32-bit system addressing.

But since this driver is a general one, let us assume there will be such a
case that controllers may have different address width here.

> 4. Patch 5 has been rebased on tip of edk2.

Thanks.

Best Regards,
Hao Wu

> 
> Thanks
> Ashish
> 
> -Original Message-----
> From: Wu, Hao A 
> Sent: Wednesday, November 28, 2018 12:25 AM
> To: Ashish Singhal ; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4
> 64bit SDMA and ADMA2 support.
> 
> Hello,
> 
> Sorry for the delayed response.
> 
> Apart from inserting the Bugzilla tracker information, several more general
> level
> comments:
> 
> 1. Cannot access the files (content) on SD & eMMC devices
> 
> After applying (rebasing onto the latest codes) your patches, I found that
> though the SD & eMMC devices can be detected, yet the content cannot be
> accessed.
> 
> There are 1 SD card and 1 embedded eMMC device within the system. Under
> Shell, I can see 2 file systems on SD & eMMC devices being detected, but
> when I try to access the files on those file systems by using the 'ls' 
> command,
> it fails, saying "ls: File Not Found - 'FS0:\'". I can confirm that files can 
> be listed
> successfully without applying this patch.
> 
> I also tried the 'dblk' command for display the data via BlockIO protocols
> created on those devices. I found that for SD, the command works fine. But
> for eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".
> 

Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.

2018-11-29 Thread Ashish Singhal
Missed one of your suggested compiler related change in Patch 5. Have posted 
patch 6.

Thanks
Ashish Singhal

-Original Message-
From: Ashish Singhal 
Sent: Thursday, November 29, 2018 11:48 AM
To: 'Wu, Hao A' ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit 
SDMA and ADMA2 support.

Hello Hao,

Answers to your comments:

1. Patch 5 fixes the issue with accessing SD as well as eMMC files.
2. Using Private variable for controller version would mean I can only access 
it in functions having an instance of the installed protocol which is not 
possible in some functions for example where we set clock dividers. We either 
need to use what I have or pass Private or controller version as an argument to 
functions using it.
3. I think there is a bigger issue here. During host controller initialization 
we need to setup 64b addressing which happens before we initialize 64b DMA in 
PCI layer. So what you are suggesting may not be that helpful. Also, what we 
are doing right now is that we go through all slots and if controller on any 
slot does not support 64b, we do not enable 64b DMA in PCI layer. What is the 
likelihood of all controllers on an SOC not supporting similar address width? 
Also, shouldn't we be enabling 64b DMA in PCI layer if any of the controller 
supports 64b addressing? Wouldn't 64b DMA enabled mean 32b is enabled by 
default?
4. Patch 5 has been rebased on tip of edk2.

Thanks
Ashish

-Original Message-
From: Wu, Hao A 
Sent: Wednesday, November 28, 2018 12:25 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit 
SDMA and ADMA2 support.

Hello,

Sorry for the delayed response.

Apart from inserting the Bugzilla tracker information, several more general 
level
comments:

1. Cannot access the files (content) on SD & eMMC devices

After applying (rebasing onto the latest codes) your patches, I found that 
though the SD & eMMC devices can be detected, yet the content cannot be 
accessed.

There are 1 SD card and 1 embedded eMMC device within the system. Under Shell, 
I can see 2 file systems on SD & eMMC devices being detected, but when I try to 
access the files on those file systems by using the 'ls' command, it fails, 
saying "ls: File Not Found - 'FS0:\'". I can confirm that files can be listed 
successfully without applying this patch.

I also tried the 'dblk' command for display the data via BlockIO protocols 
created on those devices. I found that for SD, the command works fine. But for 
eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".

For the hardware I own, the controllers are all version 3.0. I tested on both
IA32 and X64 arch image. The results were the same as described above.

Unfortunately, I do not have access to boards with SD controller with version 
newer than 3.0. So I am not able to perform those tests on my side for Version
4.0 or greater SD controllers.

Do you have any hardware with SD controller of version 3.0? Is it working on 
your side?

Please let me know if additional information is needed.


2. On SdMmcHcGetControllerVersion()

I found that there is a 'ControllerVersion' version in the data structure 
SD_MMC_HC_PRIVATE_DATA. But currently, this field seems never used.

I think we can get the controller version information only once and use this 
field to store it. Hence, this new function can be eliminated and also can 
avoid calling it multiple times.


3. Setting the SD_MMC_HC_64_ADDR_EN bit in SdMmcHcV4Init64BitSupport()

The setting of the SD_MMC_HC_64_ADDR_EN bit is decided by the 'SysBus64V4'
field in the CAP register. For me, additional dependency on the 64-bit DMA 
support in the PCI layer is needed as well.

In function SdMmcPciHcDriverBindingStart():

  //
  // Enable 64-bit DMA support in the PCI layer if this controller
  // supports it.
  //
  if (Support64BitDma) {
Status = PciIo->Attributes (
  PciIo,
  EfiPciIoAttributeOperationEnable,
  EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
  NULL
  );
if (EFI_ERROR (Status)) {
  DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable 
64-bit DMA (%r)\n", Status));
}
  }

If the above PCI attribute setting fails, the PCI layer will not support the 
64-bit DMA.

Hence, I am thinking to introduce a new BOOLEAN field in the 
"SD_MMC_HC_PRIVATE_DATA" data structure (maybe putting the 'Support64BitDma'
into the data structure). If the above PCI operation succeeds, the BOOLEAN 
field will be set to TRUE, otherwise, FALSE.

And the setting of the SD_MMC_HC_64_ADDR_EN bit should depend on the BOOLEAN 
field as well.


4. Please help to rebase the patch upon the latest master branch.

Some changes within the SdMmcPciHcDxe have been pushed recently. Could you help 
to rebase this p

Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.

2018-11-29 Thread Ashish Singhal
Hello Hao,

Answers to your comments:

1. Patch 5 fixes the issue with accessing SD as well as eMMC files.
2. Using Private variable for controller version would mean I can only access 
it in functions having an instance of the installed protocol which is not 
possible in some functions for example where we set clock dividers. We either 
need to use what I have or pass Private or controller version as an argument to 
functions using it.
3. I think there is a bigger issue here. During host controller initialization 
we need to setup 64b addressing which happens before we initialize 64b DMA in 
PCI layer. So what you are suggesting may not be that helpful. Also, what we 
are doing right now is that we go through all slots and if controller on any 
slot does not support 64b, we do not enable 64b DMA in PCI layer. What is the 
likelihood of all controllers on an SOC not supporting similar address width? 
Also, shouldn't we be enabling 64b DMA in PCI layer if any of the controller 
supports 64b addressing? Wouldn't 64b DMA enabled mean 32b is enabled by 
default?
4. Patch 5 has been rebased on tip of edk2.

Thanks
Ashish

-Original Message-
From: Wu, Hao A  
Sent: Wednesday, November 28, 2018 12:25 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit 
SDMA and ADMA2 support.

Hello,

Sorry for the delayed response.

Apart from inserting the Bugzilla tracker information, several more general 
level
comments:

1. Cannot access the files (content) on SD & eMMC devices

After applying (rebasing onto the latest codes) your patches, I found that 
though the SD & eMMC devices can be detected, yet the content cannot be 
accessed.

There are 1 SD card and 1 embedded eMMC device within the system. Under Shell, 
I can see 2 file systems on SD & eMMC devices being detected, but when I try to 
access the files on those file systems by using the 'ls' command, it fails, 
saying "ls: File Not Found - 'FS0:\'". I can confirm that files can be listed 
successfully without applying this patch.

I also tried the 'dblk' command for display the data via BlockIO protocols 
created on those devices. I found that for SD, the command works fine. But for 
eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".

For the hardware I own, the controllers are all version 3.0. I tested on both
IA32 and X64 arch image. The results were the same as described above.

Unfortunately, I do not have access to boards with SD controller with version 
newer than 3.0. So I am not able to perform those tests on my side for Version
4.0 or greater SD controllers.

Do you have any hardware with SD controller of version 3.0? Is it working on 
your side?

Please let me know if additional information is needed.


2. On SdMmcHcGetControllerVersion()

I found that there is a 'ControllerVersion' version in the data structure 
SD_MMC_HC_PRIVATE_DATA. But currently, this field seems never used.

I think we can get the controller version information only once and use this 
field to store it. Hence, this new function can be eliminated and also can 
avoid calling it multiple times.


3. Setting the SD_MMC_HC_64_ADDR_EN bit in SdMmcHcV4Init64BitSupport()

The setting of the SD_MMC_HC_64_ADDR_EN bit is decided by the 'SysBus64V4'
field in the CAP register. For me, additional dependency on the 64-bit DMA 
support in the PCI layer is needed as well.

In function SdMmcPciHcDriverBindingStart():

  //
  // Enable 64-bit DMA support in the PCI layer if this controller
  // supports it.
  //
  if (Support64BitDma) {
Status = PciIo->Attributes (
  PciIo,
  EfiPciIoAttributeOperationEnable,
  EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
  NULL
  );
if (EFI_ERROR (Status)) {
  DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable 
64-bit DMA (%r)\n", Status));
}
  }

If the above PCI attribute setting fails, the PCI layer will not support the 
64-bit DMA.

Hence, I am thinking to introduce a new BOOLEAN field in the 
"SD_MMC_HC_PRIVATE_DATA" data structure (maybe putting the 'Support64BitDma'
into the data structure). If the above PCI operation succeeds, the BOOLEAN 
field will be set to TRUE, otherwise, FALSE.

And the setting of the SD_MMC_HC_64_ADDR_EN bit should depend on the BOOLEAN 
field as well.


4. Please help to rebase the patch upon the latest master branch.

Some changes within the SdMmcPciHcDxe have been pushed recently. Could you help 
to rebase this patch upon the latest changes. Thanks in advance.


Also, some inline comments below.


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ashish Singhal
> Sent: Tuesday, November 20, 2018 4:59 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATC

Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.

2018-11-27 Thread Wu, Hao A
Hello,

Sorry for the delayed response.

Apart from inserting the Bugzilla tracker information, several more general 
level
comments:

1. Cannot access the files (content) on SD & eMMC devices

After applying (rebasing onto the latest codes) your patches, I found that
though the SD & eMMC devices can be detected, yet the content cannot be
accessed.

There are 1 SD card and 1 embedded eMMC device within the system. Under Shell,
I can see 2 file systems on SD & eMMC devices being detected, but when I try to
access the files on those file systems by using the 'ls' command, it fails,
saying "ls: File Not Found - 'FS0:\'". I can confirm that files can be listed
successfully without applying this patch.

I also tried the 'dblk' command for display the data via BlockIO protocols
created on those devices. I found that for SD, the command works fine. But for
eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".

For the hardware I own, the controllers are all version 3.0. I tested on both
IA32 and X64 arch image. The results were the same as described above.

Unfortunately, I do not have access to boards with SD controller with version
newer than 3.0. So I am not able to perform those tests on my side for Version
4.0 or greater SD controllers.

Do you have any hardware with SD controller of version 3.0? Is it working on
your side?

Please let me know if additional information is needed.


2. On SdMmcHcGetControllerVersion()

I found that there is a 'ControllerVersion' version in the data structure
SD_MMC_HC_PRIVATE_DATA. But currently, this field seems never used.

I think we can get the controller version information only once and use this
field to store it. Hence, this new function can be eliminated and also can
avoid calling it multiple times.


3. Setting the SD_MMC_HC_64_ADDR_EN bit in SdMmcHcV4Init64BitSupport()

The setting of the SD_MMC_HC_64_ADDR_EN bit is decided by the 'SysBus64V4'
field in the CAP register. For me, additional dependency on the 64-bit DMA
support in the PCI layer is needed as well.

In function SdMmcPciHcDriverBindingStart():

  //
  // Enable 64-bit DMA support in the PCI layer if this controller
  // supports it.
  //
  if (Support64BitDma) {
Status = PciIo->Attributes (
  PciIo,
  EfiPciIoAttributeOperationEnable,
  EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
  NULL
  );
if (EFI_ERROR (Status)) {
  DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable 
64-bit DMA (%r)\n", Status));
}
  }

If the above PCI attribute setting fails, the PCI layer will not support the
64-bit DMA.

Hence, I am thinking to introduce a new BOOLEAN field in the
"SD_MMC_HC_PRIVATE_DATA" data structure (maybe putting the 'Support64BitDma'
into the data structure). If the above PCI operation succeeds, the BOOLEAN
field will be set to TRUE, otherwise, FALSE.

And the setting of the SD_MMC_HC_64_ADDR_EN bit should depend on the BOOLEAN
field as well.


4. Please help to rebase the patch upon the latest master branch.

Some changes within the SdMmcPciHcDxe have been pushed recently. Could you help
to rebase this patch upon the latest changes. Thanks in advance.


Also, some inline comments below.


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ashish Singhal
> Sent: Tuesday, November 20, 2018 4:59 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4
> 64bit SDMA and ADMA2 support.
> 
> If V4 64 bit address mode is enabled in compatibility register,
> program controller to enable V4 host mode.
> Use appropriate ADMA2 descriptors supporting 64 bit addresses.
> Use appropriate registers for SDMA mode operation.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ashish Singhal 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   4 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 273
> +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  28 ++-
>  3 files changed, 260 insertions(+), 45 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index c683600..22795df 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -2,6 +2,7 @@
> 
>Provides some data structure definitions used by the SD/MMC host
> controller driver.
> 
> +Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
> @@ -144,7 +145,8 @@ typedef struct {
>BOOLEAN Started;
>UINT64  Timeout;
> 
> 

Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.

2018-11-27 Thread Ashish Singhal
Hello,

Any feedback on this patch yet?

Thanks
Ashish

-Original Message-
From: Ashish Singhal  
Sent: Monday, November 19, 2018 1:59 PM
To: edk2-devel@lists.01.org
Cc: Ashish Singhal 
Subject: [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 
support.

If V4 64 bit address mode is enabled in compatibility register, program 
controller to enable V4 host mode.
Use appropriate ADMA2 descriptors supporting 64 bit addresses.
Use appropriate registers for SDMA mode operation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   4 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 273 +
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  28 ++-
 3 files changed, 260 insertions(+), 45 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index c683600..22795df 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,6 +2,7 @@
 
   Provides some data structure definitions used by the SD/MMC host controller 
driver.
 
+Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
 Copyright (c) 2015, Intel Corporation. All rights reserved.  This program 
and the accompanying materials  are licensed and made available under the terms 
and conditions of the BSD License @@ -144,7 +145,8 @@ typedef struct {
   BOOLEAN Started;
   UINT64  Timeout;
 
-  SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
+  SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
+  SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
   EFI_PHYSICAL_ADDRESSAdmaDescPhy;
   VOID*AdmaMap;
   UINT32  AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index e506875..9fef3fb 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -4,6 +4,7 @@
 
   It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
 
+  Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
   Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License @@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet (  }
 
 /**
+  Get the controller version information from the specified slot.
+
+  @param[in]  PciIo   The PCI IO protocol instance.
+  @param[in]  SlotThe slot number of the SD card to send the 
command to.
+  @param[out] Version The buffer to store the version information.
+
+  @retval EFI_SUCCESS The operation executes successfully.
+  @retval Others  The operation fails.
+
+**/
+EFI_STATUS
+SdMmcHcGetControllerVersion (
+  IN EFI_PCI_IO_PROTOCOL  *PciIo,
+  IN UINT8Slot,
+ OUT UINT16   *Version
+  )
+{
+  EFI_STATUSStatus;
+
+  Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof 
+ (UINT16), Version);  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  *Version &= 0xFF;
+
+  return EFI_SUCCESS;
+}
+
+/**
   Software reset the specified SD/MMC host controller and enable all 
interrupts.
 
   @param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA instance.
@@ -776,18 +807,18 @@ SdMmcHcClockSupply (
 
   DEBUG ((DEBUG_INFO, "BaseClkFreq %dMHz Divisor %d ClockFreq %dKhz\n", 
BaseClkFreq, Divisor, ClockFreq));
 
-  Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof 
(ControllerVer), );
+  Status = SdMmcHcGetControllerVersion (PciIo, Slot, );
   if (EFI_ERROR (Status)) {
 return Status;
   }
   //
   // Set SDCLK Frequency Select and Internal Clock Enable fields in Clock 
Control register.
   //
-  if (((ControllerVer & 0xFF) >= SD_MMC_HC_CTRL_VER_300) &&
-  ((ControllerVer & 0xFF) <= SD_MMC_HC_CTRL_VER_420)) {
+  if ((ControllerVer >= SD_MMC_HC_CTRL_VER_300) &&
+  (ControllerVer <= SD_MMC_HC_CTRL_VER_420)) {
 ASSERT (Divisor <= 0x3FF);
 ClockCtrl = ((Divisor & 0xFF) << 8) | ((Divisor & 0x300) >> 2);
-  } else if (((ControllerVer & 0xFF) == 0) || ((ControllerVer & 0xFF) == 1)) {
+  } else if ((ControllerVer == 0) || (ControllerVer == 1)) {
 //
 // Only the most significant bit can be used as divisor.
 //
@@ -935,6 +966,54 @@ SdMmcHcSetBusWidth (  }
 
 /**
+  Configure V4 64 bit system address support at initialization.
+
+  @param[in] PciIo  The PCI IO protocol instance.
+  @param[in] Slot   The slot number of the SD card to send the command 
to.
+  @param[in] Capability The capability of the slot.
+
+  @retval EFI_SUCCESS   The clock is supplied successfully.
+
+**/