Re: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA Support

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

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Cohen, Eugene
> Sent: Thursday, March 07, 2019 8:54 AM
> To: Ashish Singhal; Wu, Hao A; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b
> DMA Support
> 
> Tested-by: Eugene Cohen <eugene@hp.com>

Eugene,

Thanks for the testing. Really appreciate your help.

Best Regards,
Hao Wu

> 
> Thanks again.
> 
> From: Ashish Singhal <ashishsingha@nvidia.com>
> Sent: Wednesday, March 6, 2019 4:07 PM
> To: Cohen, Eugene <eugene@hp.com>; Wu, Hao A <hao.a.wu@intel.com>;
> edk2-devel@lists.01.org
> Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
> 
> Hi Eugene,
> 
> Thanks for confirming. Can you please validate the v2 patch I sent as well for
> completeness?
> 
> Thanks
> Ashish
> 
> From: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>
> Sent: Wednesday, March 6, 2019 4:05 PM
> To: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Ashish
> Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>;
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
> 
> 
> Ø  I verified the patch on SDHC version 3.00 with 64-bit System Address
> Ø  Support. Hope more configurations are available for testing on Eugene's
> Ø  side.
> 
> This patch works for us.  Tested that the V3 64-bit DMA works and verified
> that addresses above 4GB DMA correctly.
> 
> Thanks for putting this together.
> 
> Feel free to add my Tested-By.
> 
> Eugene
> 
> From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Sent: Tuesday, March 5, 2019 8:01 PM
> To: Ashish Singhal
> <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; edk2-
> devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>
> Subject: RE: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> Support
> 
> Hi Ashish,
> 
> One thing to confirm, for the updated checks within
> SdMmcPciHcDriverBindingStart():
> 
> > if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> > Private->Capability[Slot].SysBus64V3 == 0) ||
> > (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> > Private->Capability[Slot].SysBus64V3 == 0) ||
> > (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> > Private->Capability[Slot].SysBus64V4 == 0)) {
> > Support64BitDma = FALSE;
> > }
> 
> When the SDHC with version greater than 4.10, the check is only performed
> against the 'SysBus64V4' bit. My understanding of the purpose is that:
> 
> 1. For SDHC with version 4.00, the support of V3 mode and V4 mode of
> 64-bit System Address are reflect by bit 'SysBus64V3'. Thus, I can infer
> that the possible support case is both or neither.
> 
> 2. The spec states that SDHC with version greater than 4.10 divides the V3
> mode and V4 mode support into 2 bits (SysBus64V3, SysBus64V4) so that the
> V3 mode support can be optional.
> 
> So based on 1 & 2, we do not even bother to check the 'SysBus64V3' bit
> when HC version >= 4.10. Is that right?
> 
> 
> I verified the patch on SDHC version 3.00 with 64-bit System Address
> Support. Hope more configurations are available for testing on Eugene's
> side.
> 
> 
> Besides, some minor comments below:
> 
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Saturday, March 02, 2019 2:30 AM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Cc: Wu, Hao A; eugene@hp.com<mailto:eugene@hp.com>; Ashish Singhal
> > Subject: [PATCH] MdeModulePkg/SdMmcPciHcDxe: Add V3 64b DMA
> > Support
> 
> Please help to add the below Bugzilla tracker for reference:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1583<https://bugzilla.tianoc
> ore.org/show_bug.cgi?id=1583>
> 
> I have updated the above tracker to match the purpose of the proposed
> patch.
> 
> 
> >
> > Driver was supporting only 32b DMA support for V3 controllers. Add
> > support for 64b DMA as well for completeness.
> >
> > For V4.0 64b support, driver was looking at incorrect capability
> > register bit. Fix for that is present as well.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ashish Singhal
> <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>
> > ---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 10 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 199
> > ++++++++++++++-------
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 29 ++-
> > 4 files changed, 170 insertions(+), 74 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> > index b474f8d..9b7b88c 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> > @@ -6,7 +6,7 @@
> >
> > It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer
> use.
> >
> > - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> > + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> > Copyright (c) 2015 - 2019, 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
> > @@ -666,8 +666,12 @@ SdMmcPciHcDriverBindingStart (
> > // If any of the slots does not support 64b system bus
> > // do not enable 64b DMA in the PCI layer.
> > //
> > - if (Private->Capability[Slot].SysBus64V3 == 0 &&
> > - Private->Capability[Slot].SysBus64V4 == 0) {
> > + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300 &&
> > + Private->Capability[Slot].SysBus64V3 == 0) ||
> > + (Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_400 &&
> > + Private->Capability[Slot].SysBus64V3 == 0) ||
> > + (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410 &&
> > + Private->Capability[Slot].SysBus64V4 == 0)) {
> > Support64BitDma = FALSE;
> > }
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > index 1bb701a..68d8a5c 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > @@ -2,7 +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) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> > Copyright (c) 2015, 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
> > @@ -145,13 +145,15 @@ typedef struct {
> > EFI_PHYSICAL_ADDRESS DataPhy;
> > VOID *DataMap;
> > SD_MMC_HC_TRANSFER_MODE Mode;
> > + SD_MMC_HC_ADMA_LEGTH Length;
> 
> Maybe:
> SD_MMC_HC_ADMA_LENGTH_MODE AdmaLengthMode;
> 
> is better to avoid confusion.
> 
> 
> >
> > EFI_EVENT Event;
> > BOOLEAN Started;
> > UINT64 Timeout;
> >
> > SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
> > - SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
> > + SD_MMC_HC_ADMA_64_V3_DESC_LINE *Adma64V3Desc;
> > + SD_MMC_HC_ADMA_64_V4_DESC_LINE *Adma64V4Desc;
> > EFI_PHYSICAL_ADDRESS AdmaDescPhy;
> > VOID *AdmaMap;
> > UINT32 AdmaPages;
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index d73fa10..a6d2395 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -6,7 +6,7 @@
> >
> > It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer
> use.
> >
> > - Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
> > + Copyright (c) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> > Copyright (c) 2015 - 2019, 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
> > @@ -1010,18 +1010,32 @@ SdMmcHcInitV4Enhancements (
> > if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> > HostCtrl2 = SD_MMC_HC_V4_EN;
> > //
> > - // Check if V4 64bit support is available
> > + // Check if controller version V4.0
> > //
> > - if (Capability.SysBus64V4 != 0) {
> > - HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> > - DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> > + if (ControllerVer == SD_MMC_HC_CTRL_VER_400) {
> > + //
> > + // Check if 64bit support is available
> > + //
> > + if (Capability.SysBus64V3 != 0) {
> > + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> > + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> > + }
> > }
> > //
> > // Check if controller version V4.10 or higher
> > //
> > - if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> > - HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> > - DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> > support\n"));
> > + else if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> > + //
> > + // Check if 64bit support is available
> > + //
> > + if (Capability.SysBus64V4 != 0) {
> > + HostCtrl2 |= SD_MMC_HC_64_ADDR_EN;
> > + DEBUG ((DEBUG_INFO, "Enabled V4 64 bit system bus support\n"));
> > + }
> > + if (ControllerVer >= SD_MMC_HC_CTRL_VER_410) {
> 
> I think the above 'if' statement can be removed.
> 
> 
> > + HostCtrl2 |= SD_MMC_HC_26_DATA_LEN_ADMA_EN;
> > + DEBUG ((DEBUG_INFO, "Enabled V4 26 bit data length ADMA
> > support\n"));
> > + }
> > }
> > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof
> > (HostCtrl2), &HostCtrl2);
> > if (EFI_ERROR (Status)) {
> > @@ -1393,14 +1407,10 @@ BuildAdmaDescTable (
> > EFI_PCI_IO_PROTOCOL *PciIo;
> > EFI_STATUS Status;
> > UINTN Bytes;
> > - BOOLEAN AddressingMode64;
> > - BOOLEAN DataLength26;
> > UINT32 AdmaMaxDataPerLine;
> > UINT32 DescSize;
> > VOID *AdmaDesc;
> >
> > - AddressingMode64 = FALSE;
> > - DataLength26 = FALSE;
> > AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_16B;
> > DescSize = sizeof (SD_MMC_HC_ADMA_32_DESC_LINE);
> > AdmaDesc = NULL;
> > @@ -1410,27 +1420,16 @@ BuildAdmaDescTable (
> > PciIo = Trb->Private->PciIo;
> >
> > //
> > - // Detect whether 64bit addressing is supported.
> > - //
> > - if (ControllerVer >= SD_MMC_HC_CTRL_VER_400) {
> > - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> > SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> > - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> > SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> > - if (!EFI_ERROR (Status)) {
> > - AddressingMode64 = TRUE;
> > - DescSize = sizeof (SD_MMC_HC_ADMA_64_DESC_LINE);
> > - }
> > - }
> > - //
> > // Check for valid ranges in 32bit ADMA Descriptor Table
> > //
> > - if (!AddressingMode64 &&
> > + if ((Trb->Mode == SdMmcAdma32bMode) &&
> > ((Data >= 0x100000000ul) || ((Data + DataLen) > 0x100000000ul))) {
> > return EFI_INVALID_PARAMETER;
> > }
> > //
> > // Check address field alignment
> > //
> > - if (AddressingMode64) {
> > + if (Trb->Mode != SdMmcAdma32bMode) {
> > //
> > // Address field shall be set on 64-bit boundary (Lower 3-bit is always set
> > to 0)
> > //
> > @@ -1445,13 +1444,19 @@ BuildAdmaDescTable (
> > DEBUG ((DEBUG_INFO, "The buffer [0x%x] to construct ADMA desc is not
> > aligned to 4 bytes boundary!\n", Data));
> > }
> > }
> > +
> > //
> > - // Detect whether 26bit data length is supported.
> > + // Configure 64b ADMA.
> > //
> > - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> > SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> > - SD_MMC_HC_26_DATA_LEN_ADMA_EN,
> > SD_MMC_HC_26_DATA_LEN_ADMA_EN);
> > - if (!EFI_ERROR (Status)) {
> > - DataLength26 = TRUE;
> > + if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + DescSize = sizeof (SD_MMC_HC_ADMA_64_V3_DESC_LINE);
> > + }else if (Trb->Mode == SdMmcAdma64bV4Mode) {
> > + DescSize = sizeof (SD_MMC_HC_ADMA_64_V4_DESC_LINE);
> > + }
> > + //
> > + // Configure 26b data length.
> > + //
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > AdmaMaxDataPerLine = ADMA_MAX_DATA_PER_LINE_26B;
> > }
> >
> > @@ -1492,7 +1497,7 @@ BuildAdmaDescTable (
> > return EFI_OUT_OF_RESOURCES;
> > }
> >
> > - if ((!AddressingMode64) &&
> > + if ((Trb->Mode == SdMmcAdma32bMode) &&
> > (UINT64)(UINTN)Trb->AdmaDescPhy > 0x100000000ul) {
> > //
> > // The ADMA doesn't support 64bit addressing.
> > @@ -1511,19 +1516,26 @@ BuildAdmaDescTable (
> >
> > Remaining = DataLen;
> > Address = Data;
> > - if (!AddressingMode64) {
> > + if (Trb->Mode == SdMmcAdma32bMode) {
> > Trb->Adma32Desc = AdmaDesc;
> > - Trb->Adma64Desc = NULL;
> > + Trb->Adma64V3Desc = NULL;
> > + Trb->Adma64V4Desc = NULL;
> > + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + Trb->Adma64V3Desc = AdmaDesc;
> > + Trb->Adma32Desc = NULL;
> > + Trb->Adma64V4Desc = NULL;
> > } else {
> > - Trb->Adma64Desc = AdmaDesc;
> > + Trb->Adma64V4Desc = AdmaDesc;
> > Trb->Adma32Desc = NULL;
> > + Trb->Adma64V3Desc = NULL;
> > }
> 
> Suggest to remove those NULL assignments to:
> Trb->Adma32Desc
> Trb->Adma64Desc
> Trb->Adma64V3Desc
> 
> Since in SdMmcCreateTrb(), 'Trb' is allocated by:
> 
> Trb = AllocateZeroPool (sizeof (SD_MMC_HC_TRB));
> 
> 
> > +
> > for (Index = 0; Index < Entries; Index++) {
> > - if (!AddressingMode64) {
> > + if (Trb->Mode == SdMmcAdma32bMode) {
> > if (Remaining <= AdmaMaxDataPerLine) {
> > Trb->Adma32Desc[Index].Valid = 1;
> > Trb->Adma32Desc[Index].Act = 2;
> > - if (DataLength26) {
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > Trb->Adma32Desc[Index].UpperLength = (UINT16)RShiftU64
> > (Remaining, 16);
> > }
> > Trb->Adma32Desc[Index].LowerLength = (UINT16)(Remaining &
> > MAX_UINT16);
> > @@ -1532,32 +1544,53 @@ BuildAdmaDescTable (
> > } else {
> > Trb->Adma32Desc[Index].Valid = 1;
> > Trb->Adma32Desc[Index].Act = 2;
> > - if (DataLength26) {
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > Trb->Adma32Desc[Index].UpperLength = 0;
> > }
> > Trb->Adma32Desc[Index].LowerLength = 0;
> > Trb->Adma32Desc[Index].Address = (UINT32)Address;
> > }
> > + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + if (Remaining <= AdmaMaxDataPerLine) {
> > + Trb->Adma64V3Desc[Index].Valid = 1;
> > + Trb->Adma64V3Desc[Index].Act = 2;
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > + Trb->Adma64V3Desc[Index].UpperLength = (UINT16)RShiftU64
> > (Remaining, 16);
> > + }
> > + Trb->Adma64V3Desc[Index].LowerLength = (UINT16)(Remaining &
> > MAX_UINT16);
> > + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> > + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> > (Address, 32);
> > + break;
> > + } else {
> > + Trb->Adma64V3Desc[Index].Valid = 1;
> > + Trb->Adma64V3Desc[Index].Act = 2;
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > + Trb->Adma64V3Desc[Index].UpperLength = 0;
> > + }
> > + Trb->Adma64V3Desc[Index].LowerLength = 0;
> > + Trb->Adma64V3Desc[Index].LowerAddress = (UINT32)Address;
> > + Trb->Adma64V3Desc[Index].UpperAddress = (UINT32)RShiftU64
> > (Address, 32);
> > + }
> > } else {
> > if (Remaining <= AdmaMaxDataPerLine) {
> > - Trb->Adma64Desc[Index].Valid = 1;
> > - Trb->Adma64Desc[Index].Act = 2;
> > - if (DataLength26) {
> > - Trb->Adma64Desc[Index].UpperLength = (UINT16)RShiftU64
> > (Remaining, 16);
> > + Trb->Adma64V4Desc[Index].Valid = 1;
> > + Trb->Adma64V4Desc[Index].Act = 2;
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > + Trb->Adma64V4Desc[Index].UpperLength = (UINT16)RShiftU64
> > (Remaining, 16);
> > }
> > - Trb->Adma64Desc[Index].LowerLength = (UINT16)(Remaining &
> > MAX_UINT16);
> > - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> > - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> > 32);
> > + Trb->Adma64V4Desc[Index].LowerLength = (UINT16)(Remaining &
> > MAX_UINT16);
> > + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> > + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> > (Address, 32);
> > break;
> > } else {
> > - Trb->Adma64Desc[Index].Valid = 1;
> > - Trb->Adma64Desc[Index].Act = 2;
> > - if (DataLength26) {
> > - Trb->Adma64Desc[Index].UpperLength = 0;
> > + Trb->Adma64V4Desc[Index].Valid = 1;
> > + Trb->Adma64V4Desc[Index].Act = 2;
> > + if (Trb->Length == SdMmcAdmaLen26b) {
> > + Trb->Adma64V4Desc[Index].UpperLength = 0;
> > }
> > - Trb->Adma64Desc[Index].LowerLength = 0;
> > - Trb->Adma64Desc[Index].LowerAddress = (UINT32)Address;
> > - Trb->Adma64Desc[Index].UpperAddress = (UINT32)RShiftU64 (Address,
> > 32);
> > + Trb->Adma64V4Desc[Index].LowerLength = 0;
> > + Trb->Adma64V4Desc[Index].LowerAddress = (UINT32)Address;
> > + Trb->Adma64V4Desc[Index].UpperAddress = (UINT32)RShiftU64
> > (Address, 32);
> > }
> > }
> >
> > @@ -1568,7 +1601,13 @@ BuildAdmaDescTable (
> > //
> > // Set the last descriptor line as end of descriptor table
> > //
> > - AddressingMode64 ? (Trb->Adma64Desc[Index].End = 1) : (Trb-
> > >Adma32Desc[Index].End = 1);
> > + if (Trb->Mode == SdMmcAdma32bMode) {
> > + Trb->Adma32Desc[Index].End = 1;
> > + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + Trb->Adma64V3Desc[Index].End = 1;
> > + } else {
> > + Trb->Adma64V4Desc[Index].End = 1;
> > + }
> > return EFI_SUCCESS;
> > }
> >
> > @@ -1665,7 +1704,20 @@ SdMmcCreateTrb (
> > if (Trb->DataLen == 0) {
> > Trb->Mode = SdMmcNoData;
> > } else if (Private->Capability[Slot].Adma2 != 0) {
> > - Trb->Mode = SdMmcAdmaMode;
> > + Trb->Mode = SdMmcAdma32bMode;
> > + Trb->Length = SdMmcAdmaLen16b;
> > + if ((Private->ControllerVersion[Slot] == SD_MMC_HC_CTRL_VER_300)
> > &&
> > + (Private->Capability[Slot].SysBus64V3 == 1)) {
> > + Trb->Mode = SdMmcAdma64bV3Mode;
> > + } else if (((Private->ControllerVersion[Slot] ==
> > SD_MMC_HC_CTRL_VER_400) &&
> > + (Private->Capability[Slot].SysBus64V3 == 1)) ||
> > + ((Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410)
> > &&
> > + (Private->Capability[Slot].SysBus64V4 == 1))) {
> > + Trb->Mode = SdMmcAdma64bV4Mode;
> > + }
> > + if (Private->ControllerVersion[Slot] >= SD_MMC_HC_CTRL_VER_410) {
> > + Trb->Length = SdMmcAdmaLen26b;
> > + }
> > Status = BuildAdmaDescTable (Trb, Private->ControllerVersion[Slot]);
> > if (EFI_ERROR (Status)) {
> > PciIo->Unmap (PciIo, Trb->DataMap);
> > @@ -1719,11 +1771,18 @@ SdMmcFreeTrb (
> > Trb->Adma32Desc
> > );
> > }
> > - if (Trb->Adma64Desc != NULL) {
> > + if (Trb->Adma64V3Desc != NULL) {
> > + PciIo->FreeBuffer (
> > + PciIo,
> > + Trb->AdmaPages,
> > + Trb->Adma64V3Desc
> > + );
> > + }
> > + if (Trb->Adma64V4Desc != NULL) {
> > PciIo->FreeBuffer (
> > PciIo,
> > Trb->AdmaPages,
> > - Trb->Adma64Desc
> > + Trb->Adma64V4Desc
> > );
> > }
> > if (Trb->DataMap != NULL) {
> > @@ -1891,27 +1950,35 @@ SdMmcExecTrb (
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > +
> > + if (Private->ControllerVersion[Trb->Slot] >=
> SD_MMC_HC_CTRL_VER_400)
> > {
> > + Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> > SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> > + SD_MMC_HC_64_ADDR_EN, SD_MMC_HC_64_ADDR_EN);
> > + if (!EFI_ERROR (Status)) {
> > + AddressingMode64 = TRUE;
> > + }
> > + }
> > +
> > //
> > // Set Host Control 1 register DMA Select field
> > //
> > - if (Trb->Mode == SdMmcAdmaMode) {
> > + if ((Trb->Mode == SdMmcAdma32bMode) ||
> > + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> > HostCtrl1 = BIT4;
> > Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> > sizeof (HostCtrl1), &HostCtrl1);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > + } else if (Trb->Mode == SdMmcAdma64bV3Mode) {
> > + HostCtrl1 = BIT4|BIT3;
> > + Status = SdMmcHcOrMmio (PciIo, Trb->Slot, SD_MMC_HC_HOST_CTRL1,
> > sizeof (HostCtrl1), &HostCtrl1);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > }
> >
> > SdMmcHcLedOnOff (PciIo, Trb->Slot, TRUE);
> >
> > - if (Private->ControllerVersion[Trb->Slot] >= SD_MMC_HC_CTRL_VER_400)
> > {
> > - Status = SdMmcHcCheckMmioSet(PciIo, Trb->Slot,
> > SD_MMC_HC_HOST_CTRL2, sizeof(UINT16),
> > - SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN,
> > SD_MMC_HC_V4_EN|SD_MMC_HC_64_ADDR_EN);
> > - if (!EFI_ERROR (Status)) {
> > - AddressingMode64 = TRUE;
> > - }
> > - }
> > -
> > if (Trb->Mode == SdMmcSdmaMode) {
> > if ((!AddressingMode64) &&
> > ((UINT64)(UINTN)Trb->DataPhy >= 0x100000000ul)) {
> > @@ -1929,7 +1996,9 @@ SdMmcExecTrb (
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > - } else if (Trb->Mode == SdMmcAdmaMode) {
> > + } else if ((Trb->Mode == SdMmcAdma32bMode) ||
> > + (Trb->Mode == SdMmcAdma64bV3Mode) ||
> > + (Trb->Mode == SdMmcAdma64bV4Mode)) {
> > AdmaAddr = (UINT64)(UINTN)Trb->AdmaDescPhy;
> > Status = SdMmcHcRwMmio (PciIo, Trb->Slot,
> > SD_MMC_HC_ADMA_SYS_ADDR, FALSE, sizeof (AdmaAddr), &AdmaAddr);
> > if (EFI_ERROR (Status)) {
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > index d157f2c..3a05456 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > @@ -2,7 +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) 2018-2019, NVIDIA CORPORATION. All rights reserved.
> > Copyright (c) 2015, 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
> > @@ -80,16 +80,25 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> > ANY KIND, EITHER EXPRESS OR IMPLIED.
> >
> > //
> > // The transfer modes supported by SD Host Controller
> > -// Simplified Spec 3.0 Table 1-2
> > //
> > typedef enum {
> > SdMmcNoData,
> > SdMmcPioMode,
> > SdMmcSdmaMode,
> > - SdMmcAdmaMode
> > + SdMmcAdma32bMode,
> > + SdMmcAdma64bV3Mode,
> > + SdMmcAdma64bV4Mode
> > } SD_MMC_HC_TRANSFER_MODE;
> >
> > //
> > +// The ADMA transfer lengths supported by SD Host Controller
> > +//
> > +typedef enum {
> > + SdMmcAdmaLen16b,
> > + SdMmcAdmaLen26b
> > +} SD_MMC_HC_ADMA_LEGTH;
> 
> Typo 'LEGTH' -> 'LENGTH'
> Also, how about 'SD_MMC_HC_ADMA_LENGTH_MODE'?
> 
> Best Regards,
> Hao Wu
> 
> > +
> > +//
> > // The maximum data length of each descriptor line
> > //
> > #define ADMA_MAX_DATA_PER_LINE_16B SIZE_64KB
> > @@ -122,8 +131,20 @@ typedef struct {
> > UINT32 LowerLength:16;
> > UINT32 LowerAddress;
> > UINT32 UpperAddress;
> > +} SD_MMC_HC_ADMA_64_V3_DESC_LINE;
> > +
> > +typedef struct {
> > + UINT32 Valid:1;
> > + UINT32 End:1;
> > + UINT32 Int:1;
> > + UINT32 Reserved:1;
> > + UINT32 Act:2;
> > + UINT32 UpperLength:10;
> > + UINT32 LowerLength:16;
> > + UINT32 LowerAddress;
> > + UINT32 UpperAddress;
> > UINT32 Reserved1;
> > -} SD_MMC_HC_ADMA_64_DESC_LINE;
> > +} SD_MMC_HC_ADMA_64_V4_DESC_LINE;
> >
> > #define SD_MMC_SDMA_BOUNDARY 512 * 1024
> > #define SD_MMC_SDMA_ROUND_UP(x, n) (((x) + n) & ~(n - 1))
> > --
> > 2.7.4
> ________________________________
> This email message is for the sole use of the intended recipient(s) and may
> contain confidential information.  Any unauthorized review, use, disclosure
> or distribution is prohibited.  If you are not the intended recipient, please
> contact the sender by reply email and destroy all copies of the original
> message.
> ________________________________
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel