public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Chris Co <Christopher.Co@microsoft.com>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [PATCH edk2-platforms 1/7] Silicon/NXP: Add support for iMX SDHC
Date: Wed, 1 Aug 2018 17:15:08 +0100	[thread overview]
Message-ID: <20180801161508.uvfzsbbntzwtiv2d@bivouac.eciton.net> (raw)
In-Reply-To: <20180719041103.9072-2-christopher.co@microsoft.com>

On Thu, Jul 19, 2018 at 04:11:18AM +0000, Chris Co wrote:
> This adds support for using the SD host controller on
> NXP i.MX platforms.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Christopher Co <christopher.co@microsoft.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>

Trying to build the code[1] in this patch throws up issues due to
(initially) a missing dependency on a iMXIoMuxLib mapping. Which is
not introduced in this set at all.

Could you bring these sets together into a single (huge) one,
ensuring that code is introduced in dependency order?

It will be no more unwieldy (I can push patches from the front as they
merit it), and will make my life a _lot_ easier.

If you can also address the items I point out as "address throughout"
below, for all the patches, that will reduce churn.

[1] By hacking the .inf into EmbeddedPkg/EmbeddedPkg.dsc, since the
    i.MX one is also not included in this set.

> ---
>  Silicon/NXP/iMXPlatformPkg/Drivers/SdhcDxe/SdhcDxe.c   | 1356 ++++++++++++++++++++
>  Silicon/NXP/iMXPlatformPkg/Drivers/SdhcDxe/SdhcDxe.inf |   67 +
>  Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h           |  101 ++
>  Silicon/NXP/iMXPlatformPkg/Include/iMXuSdhc.h          |  277 ++++
>  4 files changed, 1801 insertions(+)
> 
> diff --git a/Silicon/NXP/iMXPlatformPkg/Drivers/SdhcDxe/SdhcDxe.c b/Silicon/NXP/iMXPlatformPkg/Drivers/SdhcDxe/SdhcDxe.c
> new file mode 100644
> index 000000000000..5f087f8a28b9
> --- /dev/null
> +++ b/Silicon/NXP/iMXPlatformPkg/Drivers/SdhcDxe/SdhcDxe.c
> @@ -0,0 +1,1356 @@
> +/** @file
> +*
> +*  Copyright (c) Microsoft Corporation. All rights reserved.

Please add copyright year throughout all patches.

> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD License
> +*  which accompanies this distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#include <Uefi.h>
> +#include <Library/BaseLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DevicePathLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DmaLib.h>
> +#include <Library/TimerLib.h>
> +
> +#include <Protocol/EmbeddedExternalDevice.h>
> +#include <Protocol/BlockIo.h>
> +#include <Protocol/DevicePath.h>
> +#include <Protocol/Sdhc.h>

Please sort includes alphabetically.

> +
> +#include <iMXuSdhc.h>
> +#include <iMXGpio.h>
> +
> +typedef struct {
> +    UINT32 IoNumber;
> +    IMX_GPIO_BANK Bank;
> +} IMX_GPIO_PIN;
> +
> +#define PCD_GPIO_PIN_IO_NUMBER(X)   ((X) & 0xFF)
> +#define PCD_GPIO_PIN_BANK(X)        (((X) >> 8) & 0xFF)
> +
> +//
> +// A special value to indicate that GPIO is not the signal source for either
> +// CardDetect or WriteProtect
> +//
> +typedef enum {
> +    USDHC_SIGNAL_GPIO_START = 0x000,
> +    USDHC_SIGNAL_GPIO_END = 0xFF00,
> +    USDHC_SIGNAL_OVERRIDE_PIN_LOW = 0xFF00,

Are the above two lines intentionally assigning the same value?

> +    USDHC_SIGNAL_OVERRIDE_PIN_HIGH = 0xFF01,
> +    USDHC_SIGNAL_INTERNAL_PIN = 0xFFFF
> +} USDHC_SIGNAL_SOURCE;
> +
> +#define USDHC_IS_GPIO_SIGNAL_SOURCE(X)  \
> +    (((X) >= USDHC_SIGNAL_GPIO_START) && ((X) < USDHC_SIGNAL_GPIO_END))
> +
> +typedef struct {
> +    UINT32 SdhcId;
> +    EFI_HANDLE SdhcProtocolHandle;
> +    USDHC_REGISTERS *RegistersBase;
> +    USDHC_SIGNAL_SOURCE CardDetectSignal;
> +    USDHC_SIGNAL_SOURCE WriteProtectSignal;
> +    IMX_GPIO_PIN CardDetectGpioPin;
> +    IMX_GPIO_PIN WriteProtectGpioPin;
> +} USDHC_PRIVATE_CONTEXT;
> +
> +#define LOG_FMT_HELPER(FMT, ...) \
> +    "SDHC%d:" FMT "%a\n", ((SdhcCtx != NULL) ? SdhcCtx->SdhcId : -1), __VA_ARGS__
> +
> +#define LOG_INFO(...) \
> +    DEBUG((DEBUG_INFO | DEBUG_BLKIO, LOG_FMT_HELPER(__VA_ARGS__, "")))
> +
> +#define LOG_TRACE(...) \
> +    DEBUG((DEBUG_VERBOSE | DEBUG_BLKIO, LOG_FMT_HELPER(__VA_ARGS__, "")))
> +
> +#define LOG_ERROR(...) \
> +    DEBUG((DEBUG_ERROR | DEBUG_BLKIO, LOG_FMT_HELPER(__VA_ARGS__, "")))
> +
> +#define LOG_ASSERT(TXT) \
> +    ASSERT(!"Sdhc: " TXT "\n")
> +
> +#ifdef MIN
> +#undef MIN
> +#define MIN(x,y) ((x) > (y) ? (y) : (x))
> +#endif // MIN

Please remove - the MIN macro has existed in Base.h since at least
2007. It does not need local redefinition.

> +
> +//
> +// uSDHC read/write FIFO is 128x32-bit
> +//
> +#define USDHC_FIFO_MAX_WORD_COUNT       128
> +
> +//
> +// Max block count allowed in a single transfer
> +//
> +#define USDHC_MAX_BLOCK_COUNT           0xFFFF
> +
> +//
> +// Number of register maximum polls
> +//
> +#define USDHC_POLL_RETRY_COUNT          100000
> +
> +//
> +// Waits between each registry poll
> +//
> +#define USDHC_POLL_WAIT_US              20
> +
> +//
> +// uSDHC input clock. Ideally, should query it from clock manager
> +//
> +#define USDHC_BASE_CLOCK_FREQ_HZ        198000000
> +
> +#define USDHC_BLOCK_LENGTH_BYTES               512
> +

Can all of the above #defines, enums and structs be moved to a local
"SdhcDxe.h"?

> +VOID
> +DumpState(
> +    IN USDHC_PRIVATE_CONTEXT *SdhcCtx
> +    )

Base indentation is always 2 spaces. (applies throughout)

> +{
> +DEBUG_CODE_BEGIN ();
> +
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +
> +    USDHC_BLK_ATT_REG BlkAtt; BlkAtt.AsUint32 = MmioRead32((UINTN)&Reg->BLK_ATT);

Please never put multiple statements on same line. (applies throughout)

Space between function name and opening parentheses (applies
throughout).

> +    UINT32 CmdArg; CmdArg = MmioRead32((UINTN)&Reg->CMD_ARG);
> +    USDHC_CMD_XFR_TYP_REG CmdXfrTyp; CmdXfrTyp.AsUint32 = MmioRead32((UINTN)&Reg->CMD_XFR_TYP);
> +    USDHC_PROT_CTRL_REG ProtCtrl; ProtCtrl.AsUint32 = MmioRead32((UINTN)&Reg->PROT_CTRL);
> +    USDHC_WTMK_LVL_REG WtmkLvl; WtmkLvl.AsUint32 = MmioRead32((UINTN)&Reg->WTMK_LVL);
> +    USDHC_MIX_CTRL_REG MixCtrl; MixCtrl.AsUint32 = MmioRead32((UINTN)&Reg->MIX_CTRL);
> +    UINT32 IntStatusEn = MmioRead32((UINTN)&Reg->INT_STATUS_EN);
> +    UINT32 IntSignalEn = MmioRead32((UINTN)&Reg->INT_SIGNAL_EN);
> +    UINT32 VendSpec = MmioRead32((UINTN)&Reg->VEND_SPEC);
> +    UINT32 MmcBoot = MmioRead32((UINTN)&Reg->MMC_BOOT);
> +    UINT32 VendSpec2 = MmioRead32((UINTN)&Reg->VEND_SPEC2);
> +    USDHC_INT_STATUS_REG IntStatus; IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS);
> +    USDHC_PRES_STATE_REG PresState; PresState.AsUint32 = MmioRead32((UINTN)&Reg->PRES_STATE);
> +
> +    LOG_INFO(
> +        " - BLK_ATT\t:0x%08x BLKSIZE:0x%x BLKCNT:0x%08x",
> +        BlkAtt.AsUint32,
> +        BlkAtt.Fields.BLKSIZE,
> +        BlkAtt.Fields.BLKCNT);
> +
> +    LOG_INFO(" - CMD_ARG\t:0x%08x", CmdArg);
> +
> +    LOG_INFO(
> +        " - CMD_XFR_TYP\t:0x%08x RSPTYP:%d CCCEN:%d CICEN:%d DPSEL:%d CMDTYP:%d CMDINX:%d",
> +        CmdXfrTyp.AsUint32,
> +        CmdXfrTyp.Fields.RSPTYP,
> +        CmdXfrTyp.Fields.CCCEN,
> +        CmdXfrTyp.Fields.CICEN,
> +        CmdXfrTyp.Fields.DPSEL,
> +        CmdXfrTyp.Fields.CMDTYP,
> +        CmdXfrTyp.Fields.CMDINX);
> +
> +    LOG_INFO(
> +        " - PROT_CTRL\t:0x%08x DTW:%d D3CD:%d CDSS:%d EMODE:%d DMASEL:%d SABGREQ:%d BURST_LEN_EN:%d",
> +        ProtCtrl.AsUint32,
> +        ProtCtrl.Fields.DTW,
> +        ProtCtrl.Fields.D3CD,
> +        ProtCtrl.Fields.CDSS,
> +        ProtCtrl.Fields.EMODE,
> +        ProtCtrl.Fields.DMASEL,
> +        ProtCtrl.Fields.SABGREQ,
> +        ProtCtrl.Fields.BURST_LEN_EN);
> +
> +    LOG_INFO(
> +        " - WTMK_LVL\t:0x%08x RD_WML:%d RD_BRST_LEN:%d WR_WML:%d WR_BRST_LEN:%d",
> +        WtmkLvl.AsUint32,
> +        WtmkLvl.Fields.RD_WML,
> +        WtmkLvl.Fields.RD_BRST_LEN,
> +        WtmkLvl.Fields.WR_WML,
> +        WtmkLvl.Fields.WR_BRST_LEN);
> +
> +    LOG_INFO(
> +        " - MIX_CTRL\t:0x%08x DMAEN:%d BCEN:%d AC12EN:%d DTDSEL:%d MSBSEL:%d AC23EN:%d FBCLK_SEL:%d",
> +        MixCtrl.AsUint32,
> +        MixCtrl.Fields.DMAEN,
> +        MixCtrl.Fields.BCEN,
> +        MixCtrl.Fields.AC12EN,
> +        MixCtrl.Fields.DTDSEL,
> +        MixCtrl.Fields.MSBSEL,
> +        MixCtrl.Fields.AC23EN,
> +        MixCtrl.Fields.FBCLK_SEL);
> +
> +    LOG_INFO(" - INT_STATUS_EN\t:0x%08x", IntStatusEn);
> +    LOG_INFO(" - INT_SIGNAL_EN\t:0x%08x", IntSignalEn);
> +    LOG_INFO(" - VEND_SPEC\t:0x%08x", VendSpec);
> +    LOG_INFO(" - MMC_BOOT\t:0x%08x", MmcBoot);
> +    LOG_INFO(" - VEND_SPEC2\t:0x%08x", VendSpec2);
> +
> +    LOG_INFO(
> +        " - INT_STATUS\t:0x%08x CC:%d TC:%d BWR:%d BRR:%d CTOE:%d CCE:%d CEBE:%d CIE:%d DTOE:%d DCE:%d DEBE:%d",
> +        IntStatus.AsUint32,
> +        IntStatus.Fields.CC,
> +        IntStatus.Fields.TC,
> +        IntStatus.Fields.BWR,
> +        IntStatus.Fields.BRR,
> +        IntStatus.Fields.CTOE,
> +        IntStatus.Fields.CCE,
> +        IntStatus.Fields.CEBE,
> +        IntStatus.Fields.CIE,
> +        IntStatus.Fields.DTOE,
> +        IntStatus.Fields.DCE,
> +        IntStatus.Fields.DEBE);
> +
> +    LOG_INFO(
> +        " - PRES_STATE\t:0x%08x CIHB:%d CDIHB:%d DLA:%d WTA:%d RTA:%d BWEN:%d BREN:%d CINST:%d DLSL:0x%x",
> +        PresState.AsUint32,
> +        PresState.Fields.CIHB,
> +        PresState.Fields.CDIHB,
> +        PresState.Fields.DLA,
> +        PresState.Fields.WTA,
> +        PresState.Fields.RTA,
> +        PresState.Fields.BWEN,
> +        PresState.Fields.BREN,
> +        PresState.Fields.CINST,
> +        PresState.Fields.DLSL);
> +
> +DEBUG_CODE_END ();
> +}
> +
> +EFI_STATUS
> +WaitForReadFifo(
> +    IN USDHC_PRIVATE_CONTEXT *SdhcCtx
> +    )
> +{
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +    USDHC_INT_STATUS_REG IntStatus; IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS);
> +    UINT32 Retry = USDHC_POLL_RETRY_COUNT;
> +
> +    while (!IntStatus.Fields.BRR &&
> +        !(IntStatus.AsUint32 & USDHC_INT_STATUS_ERROR) &&
> +        Retry) {

Would looke better with all tests horizontally aligned.

> +       --Retry;
> +       gBS->Stall(USDHC_POLL_WAIT_US);
> +       IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS);
> +    }
> +
> +    if (IntStatus.AsUint32 & USDHC_INT_STATUS_ERROR) {
> +        LOG_ERROR("Error detected");
> +        DumpState(SdhcCtx);
> +        return EFI_DEVICE_ERROR;
> +    } else if (IntStatus.Fields.BRR) {
> +        MmioWrite32((UINTN)&Reg->INT_STATUS, IntStatus.AsUint32);
> +        return EFI_SUCCESS;
> +    } else {
> +        ASSERT(!Retry);
> +        LOG_ERROR("Time-out waiting on read FIFO");
> +        DumpState(SdhcCtx);
> +        return EFI_TIMEOUT;
> +     }
> +}
> +
> +EFI_STATUS
> +WaitForWriteFifo(
> +    IN USDHC_PRIVATE_CONTEXT *SdhcCtx
> +    )
> +{
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +    USDHC_INT_STATUS_REG IntStatus; IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS) ;
> +    UINT32 Retry = USDHC_POLL_RETRY_COUNT;
> +
> +    while (!IntStatus.Fields.BWR &&
> +        !(IntStatus.AsUint32 & USDHC_INT_STATUS_ERROR) &&
> +        Retry) {
> +       --Retry;
> +       gBS->Stall(USDHC_POLL_WAIT_US);
> +       IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS);
> +    }
> +
> +    if (IntStatus.AsUint32 & USDHC_INT_STATUS_ERROR) {
> +        LOG_ERROR("Error detected");
> +        DumpState(SdhcCtx);
> +        return EFI_DEVICE_ERROR;
> +    } else if (IntStatus.Fields.BWR) {
> +        MmioWrite32((UINTN)&Reg->INT_STATUS, IntStatus.AsUint32);
> +        return EFI_SUCCESS;
> +    } else {
> +        ASSERT(!Retry);
> +        LOG_ERROR("Time-out waiting on write FIFO");
> +        DumpState(SdhcCtx);
> +        return EFI_TIMEOUT;
> +    }
> +}
> +
> +EFI_STATUS
> +WaitForCmdAndOrDataLine(
> +    IN USDHC_PRIVATE_CONTEXT *SdhcCtx,
> +    IN const SD_COMMAND *Cmd
> +    )
> +{
> +    BOOLEAN WaitForDataLine;
> +
> +    //
> +    // Waiting on the DATA lines is the default behavior if no CMD is specified
> +    //
> +    if (Cmd == NULL) {
> +        WaitForDataLine = TRUE;
> +    } else {
> +        //
> +        // Per datasheet, the SDHC can isssue CMD0, CMD12, CMD13 and CMD52
> +        // when the DATA lines are busy during a data transfer. Other commands
> +        // should wait on the DATA lines before issuing
> +        //
> +        switch (Cmd->Index) {
> +        case 0:
> +        case 12:
> +        case 13:
> +        case 52:
> +            WaitForDataLine = FALSE;
> +            break;
> +        default:
> +            WaitForDataLine = TRUE;
> +        }
> +    }
> +
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +    USDHC_PRES_STATE_REG PresState; PresState.AsUint32 = MmioRead32((UINTN)&Reg->PRES_STATE);
> +    USDHC_INT_STATUS_REG IntStatus; IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS) ;
> +    UINT32 Retry = USDHC_POLL_RETRY_COUNT;
> +
> +    while (PresState.Fields.CIHB &&
> +           (!WaitForDataLine || PresState.Fields.CDIHB) &&
> +           !(IntStatus.AsUint32 & USDHC_INT_STATUS_ERROR) &&
> +           Retry) {
> +        gBS->Stall(USDHC_POLL_WAIT_US);
> +        --Retry;
> +        PresState.AsUint32 = MmioRead32((UINTN)&Reg->PRES_STATE);
> +        IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS);
> +    }
> +
> +    if (IntStatus.AsUint32 & USDHC_INT_STATUS_ERROR) {
> +        LOG_ERROR("Error detected");
> +        DumpState(SdhcCtx);
> +        return EFI_DEVICE_ERROR;
> +    } else if (!(PresState.Fields.CIHB &&
> +           (!WaitForDataLine || PresState.Fields.CDIHB))) {
> +           return EFI_SUCCESS;
> +    } else {
> +        ASSERT(!Retry);
> +        LOG_ERROR("Time-out waiting on CMD and/or DATA lines");
> +        DumpState(SdhcCtx);
> +        return EFI_TIMEOUT;
> +    }
> +}
> +
> +EFI_STATUS
> +WaitForCmdResponse(
> +    IN USDHC_PRIVATE_CONTEXT *SdhcCtx
> +    )
> +{
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +    USDHC_INT_STATUS_REG IntStatus; IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS) ;
> +    UINT32 Retry = USDHC_POLL_RETRY_COUNT;
> +
> +    //
> +    // Wait for command to finish execution either with success or failure
> +    //
> +    while (!IntStatus.Fields.CC &&
> +           !(IntStatus.AsUint32 & USDHC_INT_STATUS_ERROR) &&
> +           Retry) {
> +        gBS->Stall(USDHC_POLL_WAIT_US);
> +        --Retry;
> +        IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS);
> +    }
> +
> +    if (IntStatus.AsUint32 & USDHC_INT_STATUS_ERROR) {
> +        LOG_ERROR("Error detected");
> +        DumpState(SdhcCtx);
> +        return EFI_DEVICE_ERROR;
> +    } else if (IntStatus.Fields.CC) {
> +        MmioWrite32((UINTN)&Reg->INT_STATUS, IntStatus.AsUint32);
> +        return EFI_SUCCESS;
> +    } else {
> +        ASSERT(!Retry);
> +        LOG_ERROR("Time-out waiting on command completion");
> +        DumpState(SdhcCtx);
> +        return EFI_TIMEOUT;
> +    }
> +}
> +
> +EFI_STATUS
> +SdhcSetBusWidth(
> +  IN EFI_SDHC_PROTOCOL *This,
> +  IN SD_BUS_WIDTH BusWidth
> +  )
> +{
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +    USDHC_PROT_CTRL_REG ProtCtrl; ProtCtrl.AsUint32 = MmioRead32((UINTN)&Reg->PROT_CTRL);
> +
> +    LOG_TRACE("SdhcSetBusWidth(%d)", BusWidth);
> +
> +    switch (BusWidth) {
> +    case SdBusWidth1Bit:
> +        ProtCtrl.Fields.DTW = USDHC_PROT_CTRL_DTW_1BIT;
> +        break;
> +    case SdBusWidth4Bit:
> +        ProtCtrl.Fields.DTW = USDHC_PROT_CTRL_DTW_4BIT;
> +        break;
> +    case SdBusWidth8Bit:
> +        ProtCtrl.Fields.DTW = USDHC_PROT_CTRL_DTW_8BIT;
> +        break;
> +    default:
> +        LOG_ASSERT("Invalid bus width");
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    MmioWrite32((UINTN)&Reg->PROT_CTRL, ProtCtrl.AsUint32);
> +
> +    return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +SdhcSetClock(
> +    IN EFI_SDHC_PROTOCOL *This,
> +    IN UINT32 TargetFreqHz
> +    )
> +{
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +
> +    LOG_TRACE("SdhcSetClock(%dHz)", TargetFreqHz);
> +
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +    USDHC_SYS_CTRL_REG SysCtrl; SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +
> +    //
> +    // SDCLK = (Base Clock) / (prescaler x divisor)
> +    //
> +    UINT32 Prescaler;
> +    UINT32 Divisor;
> +    UINT32 SDCLK;
> +
> +    USDHC_MIX_CTRL_REG MixCtrl; MixCtrl.AsUint32 = MmioRead32((UINTN)&Reg->MIX_CTRL);
> +
> +    //
> +    // Bruteforce to find the best prescaler and divisor that result
> +    // in SDCLK less than or equal to the requested frequency
> +    //
> +    // Allowed |Base clock divided By
> +    // SDCLKFS |DDR_EN=0   |DDR_EN=1
> +    // 80h      256         512
> +    // 40h      128         256
> +    // 20h      64          128
> +    // 10h      32          64
> +    // 08h      16          32
> +    // 04h      8           16
> +    // 02h      4           8
> +    // 01h      2           4
> +    // 00h      1           2
> +    //
> +    const UINT32 PRESCALER_MIN = (MixCtrl.Fields.DDR_EN ? 2 : 1);
> +    const UINT32 PRESCALER_MAX = (MixCtrl.Fields.DDR_EN ? 512 : 256);;
> +    const UINT32 DIVISOR_MIN = 1;
> +    const UINT32 DIVISOR_MAX = 16;

CONST

> +    UINT32 MinFreqDistance = 0xFFFFFFFF;

MAX_UINT32?

> +    UINT32 FreqDistance;
> +    UINT32 BestPrescaler = 0;
> +    UINT32 BestDivisor = 0;
> +
> +    //
> +    // Bruteforce to find the best prescaler and divisor that result
> +    // in SDCLK less than or equal to the requested frequency
> +    //
> +    for (Prescaler = PRESCALER_MAX; Prescaler >= PRESCALER_MIN; Prescaler /= 2) {
> +        for (Divisor = DIVISOR_MIN; Divisor <= DIVISOR_MAX; ++Divisor) {
> +            SDCLK = USDHC_BASE_CLOCK_FREQ_HZ / (Prescaler * Divisor);
> +
> +            //
> +            // We are not willing to choose clocks higher than the target one
> +            // to avoid exceeding device limits
> +            //
> +            if (SDCLK > TargetFreqHz) {
> +                continue;
> +            } else if (SDCLK == TargetFreqHz) {
> +                BestPrescaler = Prescaler;
> +                BestDivisor = Divisor;
> +                break;
> +            } else {
> +                FreqDistance = TargetFreqHz - SDCLK;
> +                if (FreqDistance < MinFreqDistance) {
> +                    MinFreqDistance = FreqDistance;
> +                    BestPrescaler = Prescaler;
> +                    BestDivisor = Divisor;
> +                }
> +            }
> +        }
> +    }
> +
> +    //
> +    // Wait for clock to become stable before any modifications
> +    //
> +    USDHC_PRES_STATE_REG PresState; PresState.AsUint32 = MmioRead32((UINTN)&Reg->PRES_STATE);
> +    UINT32 Retry = USDHC_POLL_RETRY_COUNT;
> +
> +    while (!PresState.Fields.SDSTB &&
> +           Retry) {
> +        gBS->Stall(USDHC_POLL_WAIT_US);
> +        --Retry;
> +        PresState.AsUint32 = MmioRead32((UINTN)&Reg->PRES_STATE);
> +    }
> +
> +    if (!PresState.Fields.SDSTB) {
> +        ASSERT(!Retry);
> +        LOG_ERROR("Time-out waiting on SD clock to stabilize");
> +        DumpState(SdhcCtx);
> +        return EFI_TIMEOUT;
> +    }
> +
> +    SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +    SysCtrl.Fields.SDCLKFS = BestPrescaler / (MixCtrl.Fields.DDR_EN ? 4 : 2);
> +    SysCtrl.Fields.DVS = BestDivisor - 1;
> +
> +    MmioWrite32((UINTN)&Reg->SYS_CTRL, SysCtrl.AsUint32);
> +
> +    SDCLK = USDHC_BASE_CLOCK_FREQ_HZ / (BestPrescaler * BestDivisor);
> +
> +    LOG_TRACE(
> +        "Current SDCLK:%dHz SDCLKFS:0x%x DVS:0x%x",
> +        SDCLK,
> +        (UINT32)SysCtrl.Fields.SDCLKFS,
> +        (UINT32)SysCtrl.Fields.DVS);
> +
> +    return EFI_SUCCESS;
> +}
> +
> +BOOLEAN
> +SdhcIsCardPresent(
> +    IN EFI_SDHC_PROTOCOL *This
> +    )
> +{
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +    BOOLEAN IsCardPresent;
> +
> +    if (SdhcCtx->CardDetectSignal == USDHC_SIGNAL_INTERNAL_PIN) {
> +        USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +        USDHC_PRES_STATE_REG PresState; PresState.AsUint32 = MmioRead32((UINTN)&Reg->PRES_STATE);
> +        IsCardPresent = (PresState.Fields.CINST == 1);
> +    } else {
> +        IMX_GPIO_VALUE CardDetectLevel;
> +        if (USDHC_IS_GPIO_SIGNAL_SOURCE(SdhcCtx->CardDetectSignal)) {
> +            //
> +            //Read the state of  CD_B pin for the card socket
> +            //
> +            CardDetectLevel =
> +                ImxGpioRead(
> +                    SdhcCtx->CardDetectGpioPin.Bank,
> +                    SdhcCtx->CardDetectGpioPin.IoNumber);
> +        } else if (SdhcCtx->CardDetectSignal == USDHC_SIGNAL_OVERRIDE_PIN_LOW) {
> +            CardDetectLevel = IMX_GPIO_LOW;
> +        } else if (SdhcCtx->CardDetectSignal == USDHC_SIGNAL_OVERRIDE_PIN_HIGH) {
> +            CardDetectLevel = IMX_GPIO_HIGH;
> +        } else {
> +            ASSERT(!"Invalid CardDetect signal source");
> +            CardDetectLevel = IMX_GPIO_LOW;
> +        }
> +
> +        //
> +        // When no card is present,  CD_B is pulled-high, and the SDCard when
> +        // inserted will pull CD_B low
> +        // CD_B=0 means card present, while CD_B=1 means card not present
> +        //
> +        IsCardPresent = (CardDetectLevel == IMX_GPIO_LOW);
> +    }
> +
> +  // Enable if needed while trace debugging, otherwise this will flood the debug
> +  // console due to being called periodically every second for each SDHC
> +#if 0
> +  LOG_TRACE("SdhcIsCardPresent(): %d", IsCardPresent);
> +#endif

That sounds like something to be filtered by setting it to
DEBUG_VERBOSE, which LOG_TRACE already does.

> +
> +    return IsCardPresent;
> +}
> +
> +BOOLEAN
> +SdhcIsReadOnly(
> +    IN EFI_SDHC_PROTOCOL *This
> +    )
> +{
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +    BOOLEAN IsReadOnly;
> +
> +    if (SdhcCtx->WriteProtectSignal == USDHC_SIGNAL_INTERNAL_PIN) {
> +        USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +        USDHC_PRES_STATE_REG PresState; PresState.AsUint32 = MmioRead32((UINTN)&Reg->PRES_STATE);
> +        IsReadOnly = (PresState.Fields.WPSPL == 0);
> +    } else {
> +        IMX_GPIO_VALUE WriteProtectLevel;
> +        if (USDHC_IS_GPIO_SIGNAL_SOURCE(SdhcCtx->WriteProtectSignal)) {
> +            //
> +            //Read the state of  WP pin for the card socket
> +            //
> +            WriteProtectLevel =
> +                ImxGpioRead(
> +                    SdhcCtx->WriteProtectGpioPin.Bank,
> +                    SdhcCtx->WriteProtectGpioPin.IoNumber);
> +        } else if (SdhcCtx->WriteProtectSignal == USDHC_SIGNAL_OVERRIDE_PIN_LOW) {
> +            WriteProtectLevel = IMX_GPIO_LOW;
> +        } else if (SdhcCtx->WriteProtectSignal == USDHC_SIGNAL_OVERRIDE_PIN_HIGH) {
> +            WriteProtectLevel = IMX_GPIO_HIGH;
> +        } else {
> +            ASSERT(!"Invalid WriteProtect signal source");
> +            WriteProtectLevel = IMX_GPIO_LOW;
> +        }
> +
> +        //
> +        // When no card is present,  WP is pulled-high, and the SDCard when
> +        // inserted will pull WP low if WP switch is configured to write enable
> +        // the SDCard, otherwise it WP will stay pulled-high
> +        // WP=0 means write enabled, while WP=1 means write protected
> +        //
> +         IsReadOnly = (WriteProtectLevel != IMX_GPIO_LOW);

Indentation (relative to comment).

> +    }
> +
> +    LOG_TRACE("SdhcIsReadOnly(): %d", IsReadOnly);
> +    return IsReadOnly;
> +}
> +
> +EFI_STATUS
> +SdhcSendCommand(
> +    IN EFI_SDHC_PROTOCOL *This,
> +    IN const SD_COMMAND *Cmd,
> +    IN UINT32 Argument,
> +    IN OPTIONAL const SD_COMMAND_XFR_INFO *XfrInfo
> +    )
> +{
> +    EFI_STATUS Status;
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +
> +    LOG_TRACE(
> +        "SdhcSendCommand(%cCMD%d, %08x)",
> +        ((Cmd->Class == SdCommandClassApp) ? 'A' : ' '),
> +        (UINT32)Cmd->Index,
> +        Argument);
> +
> +    Status = WaitForCmdAndOrDataLine(SdhcCtx, Cmd);
> +    if (Status != EFI_SUCCESS) {
> +        LOG_ERROR("SdhcWaitForCmdAndDataLine failed");
> +        return Status;
> +    }
> +
> +    //
> +    // Clear Interrupt status
> +    //
> +    MmioWrite32((UINTN)&Reg->INT_STATUS, (UINT32)~0);
> +
> +    //
> +    // Setup data transfer command
> +    //
> +    if (XfrInfo) {
> +        if (XfrInfo->BlockCount > USDHC_MAX_BLOCK_COUNT) {
> +            LOG_ERROR(
> +                "Provided %d block count while SDHC max block count is %d",
> +                XfrInfo->BlockCount,
> +                USDHC_MAX_BLOCK_COUNT);
> +            return EFI_INVALID_PARAMETER;
> +        }
> +
> +        //
> +        // Set block size and count
> +        //
> +        USDHC_BLK_ATT_REG BlkAtt = { 0 };
> +        BlkAtt.Fields.BLKSIZE = XfrInfo->BlockSize;
> +        ASSERT (XfrInfo->BlockCount > 0);
> +        BlkAtt.Fields.BLKCNT = XfrInfo->BlockCount;
> +        MmioWrite32((UINTN)&Reg->BLK_ATT, BlkAtt.AsUint32);
> +
> +        //
> +        // Set transfer parameters
> +        //
> +        USDHC_MIX_CTRL_REG MixCtrl = { 0 };
> +        if (Cmd->TransferDirection == SdTransferDirectionRead) {
> +            MixCtrl.Fields.DTDSEL = 1;
> +        }
> +
> +        if (XfrInfo->BlockCount > 1) {
> +            ASSERT((Cmd->TransferType == SdTransferTypeMultiBlock) ||
> +                (Cmd->TransferType == SdTransferTypeMultiBlockNoStop));
> +            MixCtrl.Fields.MSBSEL = 1;
> +            MixCtrl.Fields.BCEN = 1;
> +        }
> +
> +        MmioWrite32((UINTN)&Reg->MIX_CTRL, MixCtrl.AsUint32);
> +
> +        USDHC_WTMK_LVL_REG WtmkLvl = { 0 };
> +
> +#if 0

No #if 0 upstream.

> +        //
> +        // Set FIFO watermarks
> +        //
> +        // Configure FIFO watermark levels to 1/2 the FIFO capacity for read,
> +        // and 1/3 the FIFO capacity for write.
> +        // In case the whole transfer can fit in the FIFO, then use
> +        // the whole transfer length as the FIFO threshold, so we do
> +        // the read/write in one-shot
> +        //
> +
> +        UINT32 FifoThresholdWordCount;
> +        if (Cmd->TransferDirection == SdTransferDirectionRead) {
> +            FifoThresholdWordCount = USDHC_FIFO_MAX_WORD_COUNT / 2;
> +        } else {
> +            FifoThresholdWordCount = USDHC_FIFO_MAX_WORD_COUNT / 3;
> +        }
> +
> +        ASSERT(XfrInfo->BlockSize % sizeof(UINT32) == 0);
> +        UINT32 TransferByteLength = XfrInfo->BlockSize * XfrInfo->BlockCount;
> +        const UINT32 TransferWordCount = TransferByteLength / sizeof(UINT32);
> +        FifoThresholdWordCount = MIN(TransferWordCount, FifoThresholdWordCount);
> +
> +        ASSERT(FifoThresholdWordCount <= 0xFFFF);
> +        const UINT16 Wml = (UINT16)FifoThresholdWordCount;
> +        ASSERT(Wml <= USDHC_FIFO_MAX_WORD_COUNT);
> +
> +        if (Cmd->TransferDirection == SdTransferDirectionRead) {
> +            WtmkLvl.Fields.RD_WML = (UINT8)Wml;
> +            WtmkLvl.Fields.RD_BRST_LEN = MIN(Wml, 8);
> +        } else {
> +            WtmkLvl.Fields.WR_WML = (UINT8)Wml;
> +            WtmkLvl.Fields.WR_BRST_LEN = MIN(Wml, 8);;
> +        }
> +#endif
> +
> +#if 0
> +        WtmkLvl.Fields.RD_WML = 64;
> +        WtmkLvl.Fields.RD_BRST_LEN = 16;
> +        WtmkLvl.Fields.WR_WML = 64;
> +        WtmkLvl.Fields.WR_BRST_LEN = 16;
> +#endif
> +
> +        UINT32 WtmkThreshold = USDHC_BLOCK_LENGTH_BYTES / 4;
> +        if (Cmd->TransferDirection == SdTransferDirectionRead) {
> +            if (WtmkThreshold > USDHC_WTMK_RD_WML_MAX_VAL) {
> +                WtmkThreshold = USDHC_WTMK_RD_WML_MAX_VAL;
> +            }
> +            WtmkLvl.Fields.RD_WML = WtmkThreshold;
> +        } else {
> +            if (WtmkThreshold > USDHC_WTMK_WR_WML_MAX_VAL) {
> +                WtmkThreshold = USDHC_WTMK_WR_WML_MAX_VAL;
> +            }
> +            WtmkLvl.Fields.WR_WML = WtmkThreshold;
> +        }
> +
> +        MmioWrite32((UINTN)&Reg->WTMK_LVL, WtmkLvl.AsUint32);
> +    }
> +
> +    //
> +    // Set CMD parameters
> +    //
> +    USDHC_CMD_XFR_TYP_REG CmdXfrTyp = { 0 };
> +    CmdXfrTyp.Fields.CMDINX = Cmd->Index;
> +
> +    switch (Cmd->ResponseType) {
> +    case SdResponseTypeNone:
> +        break;
> +
> +    case SdResponseTypeR1:
> +    case SdResponseTypeR5:
> +    case SdResponseTypeR6:
> +        CmdXfrTyp.Fields.RSPTYP = USDHC_CMD_XFR_TYP_RSPTYP_RSP_48;
> +        CmdXfrTyp.Fields.CICEN = 1;
> +        CmdXfrTyp.Fields.CCCEN = 1;
> +        break;
> +
> +    case SdResponseTypeR1B:
> +    case SdResponseTypeR5B:
> +        CmdXfrTyp.Fields.RSPTYP = USDHC_CMD_XFR_TYP_RSPTYP_RSP_48_CHK_BSY;
> +        CmdXfrTyp.Fields.CICEN = 1;
> +        CmdXfrTyp.Fields.CCCEN = 1;
> +        break;
> +
> +    case SdResponseTypeR2:
> +        CmdXfrTyp.Fields.RSPTYP = USDHC_CMD_XFR_TYP_RSPTYP_RSP_136;
> +        CmdXfrTyp.Fields.CCCEN = 1;
> +        break;
> +
> +    case SdResponseTypeR3:
> +    case SdResponseTypeR4:
> +        CmdXfrTyp.Fields.RSPTYP = USDHC_CMD_XFR_TYP_RSPTYP_RSP_48;
> +        break;
> +
> +    default:
> +        LOG_ASSERT("SdhcSendCommand(): Invalid response type");
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    if (Cmd->Type == SdCommandTypeAbort) {
> +        CmdXfrTyp.Fields.CMDTYP = USDHC_CMD_XFR_TYP_CMDTYP_ABORT;
> +    }
> +
> +    if (XfrInfo) {
> +        CmdXfrTyp.Fields.DPSEL = 1;
> +    }
> +
> +    //
> +    // Send command and wait for response
> +    //
> +    MmioWrite32((UINTN)&Reg->CMD_ARG, Argument);
> +    MmioWrite32((UINTN)&Reg->CMD_XFR_TYP, CmdXfrTyp.AsUint32);
> +
> +    Status = WaitForCmdResponse(SdhcCtx);
> +    if (EFI_ERROR(Status)) {
> +        LOG_ERROR("WaitForCmdResponse() failed. %r", Status);
> +        return Status;
> +    }
> +
> +    return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +SdhcReceiveResponse(
> +    IN EFI_SDHC_PROTOCOL *This,
> +    IN const SD_COMMAND *Cmd,
> +    OUT UINT32 *Buffer
> +    )
> +{
> +
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +
> +    if (Buffer == NULL) {
> +        LOG_ERROR("Input Buffer is NULL");
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +
> +    switch (Cmd->ResponseType) {
> +    case SdResponseTypeNone:
> +        break;
> +    case SdResponseTypeR1:
> +    case SdResponseTypeR1B:
> +    case SdResponseTypeR3:
> +    case SdResponseTypeR4:
> +    case SdResponseTypeR5:
> +    case SdResponseTypeR5B:
> +    case SdResponseTypeR6:
> +        Buffer[0] = MmioRead32((UINTN)&Reg->CMD_RSP0);
> +        LOG_TRACE(
> +            "SdhcReceiveResponse(Type: %x), Buffer[0]: %08x",
> +            Cmd->ResponseType,
> +            Buffer[0]);
> +        break;
> +    case SdResponseTypeR2:
> +        Buffer[0] = MmioRead32((UINTN)&Reg->CMD_RSP0);
> +        Buffer[1] = MmioRead32((UINTN)&Reg->CMD_RSP1);
> +        Buffer[2] = MmioRead32((UINTN)&Reg->CMD_RSP2);
> +        Buffer[3] = MmioRead32((UINTN)&Reg->CMD_RSP3);
> +
> +        LOG_TRACE(
> +            "SdhcReceiveResponse(Type: %x), Buffer[0-3]: %08x, %08x, %08x, %08x",
> +            Cmd->ResponseType,
> +            Buffer[0],
> +            Buffer[1],
> +            Buffer[2],
> +            Buffer[3]);
> +        break;
> +    default:
> +        LOG_ASSERT("SdhcReceiveResponse(): Invalid response type");
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +SdhcReadBlockData(
> +    IN EFI_SDHC_PROTOCOL *This,
> +    IN UINTN LengthInBytes,
> +    OUT UINT32* Buffer
> +    )
> +{
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +
> +    LOG_TRACE(
> +        "SdhcReadBlockData(LengthInBytes: 0x%x, Buffer: 0x%x)",
> +        LengthInBytes,
> +        Buffer);
> +
> +    ASSERT(Buffer != NULL);
> +    ASSERT(LengthInBytes % sizeof(UINT32) == 0);
> +
> +    UINTN WordIdx = 0;
> +    UINTN NumWords = LengthInBytes / sizeof(UINT32);
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +    USDHC_WTMK_LVL_REG WtmkLvl; WtmkLvl.AsUint32 = MmioRead32((UINTN)&Reg->WTMK_LVL);
> +    UINT32 FifoWords;
> +    EFI_STATUS Status;
> +
> +    ASSERT(WtmkLvl.Fields.RD_WML > 0);
> +
> +    while (WordIdx < NumWords) {
> +        Status = WaitForReadFifo(SdhcCtx);
> +        if (EFI_ERROR(Status)) {
> +            LOG_ERROR(
> +                "WaitForReadFifo() failed at Word%d. %r",
> +                (UINT32)WordIdx,
> +                Status);
> +            return Status;
> +        }
> +
> +        FifoWords = WtmkLvl.Fields.RD_WML;
> +        while (WordIdx < NumWords && FifoWords--) {
> +            Buffer[WordIdx] = MmioRead32((UINTN)&Reg->DATA_BUFF_ACC_PORT);
> +            ++WordIdx;
> +        }
> +    }
> +
> +    return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +SdhcWriteBlockData(
> +    IN EFI_SDHC_PROTOCOL *This,
> +    IN UINTN LengthInBytes,
> +    IN const UINT32* Buffer
> +    )
> +{
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +
> +    LOG_TRACE(
> +        "SdhcWriteBlockData(LengthInBytes: 0x%x, Buffer: 0x%x)",
> +        LengthInBytes,
> +        Buffer);
> +
> +    ASSERT(Buffer != NULL);
> +    ASSERT(LengthInBytes % USDHC_BLOCK_LENGTH_BYTES == 0);
> +
> +    const UINTN BlockWordCount = USDHC_BLOCK_LENGTH_BYTES / sizeof(UINT32);
> +    UINTN NumBlocks = LengthInBytes / USDHC_BLOCK_LENGTH_BYTES;
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +    USDHC_INT_STATUS_REG IntStatus;
> +    USDHC_PRES_STATE_REG PresState;
> +    INT32 Timeout = 100000; // Nothing special about that constant

More of a retry than a timeout?

> +
> +    while (NumBlocks > 0) {
> +        UINTN RemainingWords = BlockWordCount;
> +        IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS);
> +        PresState.AsUint32 = MmioRead32((UINTN)&Reg->PRES_STATE);
> +        while (!PresState.Fields.BWEN && --Timeout);

Does this need a MemoryFence()?

> +        if (Timeout <= 0) {
> +            LOG_ERROR ("Timeout waiting for FIFO PRES_STATE.BWEN flag");
> +            return EFI_TIMEOUT;
> +        }
> +
> +        while (RemainingWords > 0 && !IntStatus.Fields.TC) {
> +            MicroSecondDelay (100);

Does this need a MemoryFence()?

> +            IntStatus.AsUint32 = MmioRead32((UINTN)&Reg->INT_STATUS);
> +            MmioWrite32((UINTN)&Reg->DATA_BUFF_ACC_PORT, *Buffer);
> +            Buffer++;
> +            RemainingWords--;
> +        }
> +        NumBlocks--;
> +    }
> +
> +    return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +SdhcSoftwareReset(
> +    IN EFI_SDHC_PROTOCOL *This,
> +    IN SDHC_RESET_TYPE ResetType
> +    )
> +{
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +
> +    UINT32 Retry;
> +
> +    if (ResetType == SdhcResetTypeAll) {
> +        LOG_TRACE("SdhcSoftwareReset(ALL)");
> +        //
> +        // Software reset for ALL
> +        //
> +        USDHC_SYS_CTRL_REG SysCtrl; SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        SysCtrl.Fields.RSTA = 1;
> +        MmioWrite32((UINTN)&Reg->SYS_CTRL, SysCtrl.AsUint32);
> +        Retry = USDHC_POLL_RETRY_COUNT;
> +        //
> +        // Wait for reset to complete
> +        //
> +        while (SysCtrl.Fields.RSTA && Retry) {
> +            --Retry;
> +            gBS->Stall(USDHC_POLL_WAIT_US);
> +            SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        }
> +
> +        if (SysCtrl.Fields.RSTA) {
> +            ASSERT(!Retry);
> +            LOG_ERROR("Time-out waiting on RSTA for self-clear");
> +            return EFI_TIMEOUT;
> +        }
> +
> +        //
> +        // Disconnect interrupt signals between SDHC and GIC
> +        //
> +        MmioWrite32((UINTN)&Reg->INT_SIGNAL_EN, 0);
> +
> +        //
> +        // Clear and Enable all interrupts
> +        //
> +        MmioWrite32((UINTN)&Reg->INT_STATUS, (UINT32)~0);
> +        MmioWrite32((UINTN)&Reg->INT_STATUS_EN, (UINT32)~0);
> +
> +        LOG_TRACE("Waiting for CMD and DATA lines");
> +
> +        //
> +        // Wait for CMD and DATA lines to become available
> +        //
> +        EFI_STATUS Status = WaitForCmdAndOrDataLine(SdhcCtx, NULL);
> +        if (Status != EFI_SUCCESS) {
> +            LOG_ERROR("SdhcWaitForCmdAndDataLine() failed. %r", Status);
> +            return Status;
> +        }
> +
> +        //
> +        // Send 80 clock ticks to power-up the card
> +        //
> +        SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        SysCtrl.Fields.INITA = 1;
> +        MmioWrite32((UINTN)&Reg->SYS_CTRL, SysCtrl.AsUint32);
> +        Retry = USDHC_POLL_RETRY_COUNT;
> +
> +        //
> +        // Wait for the 80 clock ticks to complete
> +        //
> +        while (SysCtrl.Fields.INITA && Retry) {
> +            --Retry;
> +            gBS->Stall(USDHC_POLL_WAIT_US);
> +            SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        }
> +
> +        if (SysCtrl.Fields.INITA) {
> +            ASSERT(!Retry);
> +            LOG_ERROR("Time-out waiting on INITA for self-clear");
> +            return EFI_TIMEOUT;
> +        }
> +
> +        LOG_TRACE("Card power-up complete");
> +
> +        //
> +        // Set max data-timout counter value
> +        //
> +        SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        SysCtrl.Fields.DTOCV = 0xF;
> +        MmioWrite32((UINTN)&Reg->SYS_CTRL, SysCtrl.AsUint32);
> +
> +        //
> +        // Reset Mixer Control
> +        //
> +        MmioWrite32((UINTN)&Reg->MIX_CTRL, 0);
> +
> +        USDHC_PROT_CTRL_REG ProtCtrl = { 0 };
> +        ProtCtrl.Fields.EMODE = USDHC_PROT_CTRL_EMODE_LITTLE_ENDIAN;
> +        ProtCtrl.Fields.LCTL = 1;
> +        MmioWrite32((UINTN)&Reg->PROT_CTRL, ProtCtrl.AsUint32);
> +
> +        LOG_TRACE("Reset ALL complete");
> +
> +    }else if (ResetType == SdhcResetTypeCmd) {
> +        LOG_TRACE("SdhcSoftwareReset(CMD)");
> +        //
> +        // Software reset for CMD
> +        //
> +        USDHC_SYS_CTRL_REG SysCtrl; SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        SysCtrl.Fields.RSTC = 1;
> +        MmioWrite32((UINTN)&Reg->SYS_CTRL, SysCtrl.AsUint32);
> +        Retry = USDHC_POLL_RETRY_COUNT;
> +
> +        //
> +        // Wait for reset to complete
> +        //
> +        while (SysCtrl.Fields.RSTC && Retry) {
> +            --Retry;
> +            gBS->Stall(USDHC_POLL_WAIT_US);
> +            SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        }
> +
> +        if (SysCtrl.Fields.RSTC) {
> +            ASSERT(!Retry);
> +            LOG_ERROR("Time-out waiting on RSTC for self-clear");
> +            return EFI_TIMEOUT;
> +        }
> +
> +        MmioWrite32((UINTN)&Reg->INT_STATUS, (UINT32)~0);
> +
> +        LOG_TRACE("Reset CMD complete");
> +
> +    } else if (ResetType == SdhcResetTypeData) {
> +        LOG_TRACE("SdhcSoftwareReset(DAT)");
> +        //
> +        // Software reset for DAT
> +        //
> +        USDHC_SYS_CTRL_REG SysCtrl; SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        SysCtrl.Fields.RSTD = 1;
> +        MmioWrite32((UINTN)&Reg->SYS_CTRL, SysCtrl.AsUint32);
> +        Retry = USDHC_POLL_RETRY_COUNT;
> +
> +        //
> +        // Wait for reset to complete
> +        //
> +        while (SysCtrl.Fields.RSTD && Retry) {
> +            --Retry;
> +            gBS->Stall(USDHC_POLL_WAIT_US);
> +            SysCtrl.AsUint32 = MmioRead32((UINTN)&Reg->SYS_CTRL);
> +        }
> +
> +        if (SysCtrl.Fields.RSTD) {
> +            ASSERT(!Retry);
> +            LOG_ERROR("Time-out waiting on RSTD for self-clear");
> +            return EFI_TIMEOUT;
> +        }
> +
> +        MmioWrite32((UINTN)&Reg->INT_STATUS, (UINT32)~0);
> +
> +        LOG_TRACE("Reset DAT complete");
> +
> +    } else {
> +        return EFI_INVALID_PARAMETER;
> +    }
> +
> +    return EFI_SUCCESS;
> +}
> +
> +VOID
> +SdhcCleanup(
> +    IN EFI_SDHC_PROTOCOL *This
> +    )
> +{
> +    if (This->PrivateContext != NULL) {
> +        FreePool(This->PrivateContext);
> +        This->PrivateContext = NULL;
> +    }
> +
> +    FreePool(This);
> +
> +    //
> +    // Any SDHC protocol call to this instance is illegal beyond this point
> +    //
> +}
> +
> +VOID
> +SdhcGetCapabilities(
> +    IN EFI_SDHC_PROTOCOL *This,
> +    OUT SDHC_CAPABILITIES *Capabilities
> +  )
> +{
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = (USDHC_PRIVATE_CONTEXT*)This->PrivateContext;
> +    USDHC_REGISTERS *Reg = SdhcCtx->RegistersBase;
> +
> +    USDHC_HOST_CTRL_CAP_REG Caps; Caps.AsUint32 = MmioRead32((UINTN)&Reg->HOST_CTRL_CAP);
> +
> +    Capabilities->MaximumBlockSize = (UINT32)(512 << Caps.Fields.MBL);
> +    Capabilities->MaximumBlockCount = 0xFFFF; // UINT16_MAX
> +}
> +
> +
> +EFI_SDHC_PROTOCOL gSdhcProtocolTemplate =

g - is this really supposed to be globally visible to external
modules? Or could this be m for module-level visibility?

It looks like it is only used in this file in fact, so could it be STATIC?

> +{

On previous line?

> +    SDHC_PROTOCOL_INTERFACE_REVISION,   // Revision
> +    0,                                  // DeviceId
> +    NULL,                               // PrivateContext
> +    SdhcGetCapabilities,
> +    SdhcSoftwareReset,
> +    SdhcSetClock,
> +    SdhcSetBusWidth,
> +    SdhcIsCardPresent,
> +    SdhcIsReadOnly,
> +    SdhcSendCommand,
> +    SdhcReceiveResponse,
> +    SdhcReadBlockData,
> +    SdhcWriteBlockData,
> +    SdhcCleanup
> +};
> +
> +EFI_STATUS
> +uSdhcDeviceRegister(
> +    IN EFI_HANDLE ImageHandle,
> +    IN UINT32 SdhcId,
> +    IN VOID* RegistersBase,
> +    IN USDHC_SIGNAL_SOURCE CardDetectSignal,
> +    IN USDHC_SIGNAL_SOURCE WriteProtectSignal
> +    )
> +{
> +    EFI_STATUS Status;
> +    EFI_SDHC_PROTOCOL *SdhcProtocol = NULL;
> +    USDHC_PRIVATE_CONTEXT *SdhcCtx = NULL;
> +
> +    if (ImageHandle == NULL ||
> +        RegistersBase == NULL) {
> +        Status = EFI_INVALID_PARAMETER;
> +        goto Exit;
> +    }
> +
> +    //
> +    // Allocate per-device SDHC protocol and private context storage
> +    //
> +
> +    SdhcProtocol = AllocateCopyPool(sizeof(EFI_SDHC_PROTOCOL), &gSdhcProtocolTemplate);

Some very long lines in this function.
Please break at 80 characters (where readability is not obviously
improved by keeping longer line - such as for keeping output strings
searchable).

> +    if (SdhcProtocol == NULL) {
> +        Status =  EFI_OUT_OF_RESOURCES;
> +        goto Exit;
> +    }
> +    SdhcProtocol->SdhcId = SdhcId;
> +    SdhcProtocol->PrivateContext = AllocateZeroPool(sizeof(USDHC_PRIVATE_CONTEXT));
> +    if (SdhcProtocol->PrivateContext == NULL) {
> +        Status = EFI_OUT_OF_RESOURCES;
> +        goto Exit;
> +    }
> +
> +    SdhcCtx = (USDHC_PRIVATE_CONTEXT*)SdhcProtocol->PrivateContext;
> +    SdhcCtx->SdhcId = SdhcId;
> +    SdhcCtx->RegistersBase = (USDHC_REGISTERS*)RegistersBase;
> +    SdhcCtx->CardDetectSignal = CardDetectSignal;
> +    if (USDHC_IS_GPIO_SIGNAL_SOURCE(SdhcCtx->CardDetectSignal)) {
> +        SdhcCtx->CardDetectGpioPin.IoNumber = PCD_GPIO_PIN_IO_NUMBER((UINT16)CardDetectSignal);
> +        SdhcCtx->CardDetectGpioPin.Bank = PCD_GPIO_PIN_BANK(CardDetectSignal);
> +    }
> +
> +    SdhcCtx->WriteProtectSignal= WriteProtectSignal;
> +    if (USDHC_IS_GPIO_SIGNAL_SOURCE(SdhcCtx->WriteProtectSignal)) {
> +        SdhcCtx->WriteProtectGpioPin.IoNumber = PCD_GPIO_PIN_IO_NUMBER((UINT16)WriteProtectSignal);
> +        SdhcCtx->WriteProtectGpioPin.Bank = PCD_GPIO_PIN_BANK(WriteProtectSignal);
> +    }
> +
> +    LOG_INFO(
> +        "Initializing uSDHC%d @%p GPIO CD?:%d WP?:%d",
> +        SdhcId,
> +        RegistersBase,
> +        (USDHC_IS_GPIO_SIGNAL_SOURCE(SdhcCtx->CardDetectSignal) ? 1 : 0),
> +        (USDHC_IS_GPIO_SIGNAL_SOURCE(SdhcCtx->WriteProtectSignal) ? 1 : 0));
> +
> +    if (USDHC_IS_GPIO_SIGNAL_SOURCE(SdhcCtx->CardDetectSignal)) {
> +      LOG_INFO(
> +          "Using GPIO%d_IO%d for CardDetect",
> +          SdhcCtx->CardDetectGpioPin.Bank,
> +          SdhcCtx->CardDetectGpioPin.IoNumber);
> +    }
> +
> +    if (USDHC_IS_GPIO_SIGNAL_SOURCE(SdhcCtx->WriteProtectSignal)) {
> +      LOG_INFO(
> +          "Using GPIO%d_IO%d for WriteProtect",
> +          SdhcCtx->WriteProtectGpioPin.Bank,
> +          SdhcCtx->WriteProtectGpioPin.IoNumber);
> +    }
> +
> +    Status = gBS->InstallMultipleProtocolInterfaces(
> +                                                &SdhcCtx->SdhcProtocolHandle,
> +                                                &gEfiSdhcProtocolGuid,
> +                                                SdhcProtocol,
> +                                                NULL);
> +    if (EFI_ERROR(Status)) {
> +        LOG_ERROR("InstallMultipleProtocolInterfaces failed. %r", Status);
> +        goto Exit;
> +    }
> +
> +Exit:
> +    if (EFI_ERROR(Status)) {
> +        LOG_ERROR("Failed to register and initialize uSDHC%d", SdhcId);
> +
> +        if (SdhcProtocol != NULL && SdhcProtocol->PrivateContext != NULL) {
> +            FreePool(SdhcProtocol->PrivateContext);
> +            SdhcProtocol->PrivateContext = NULL;
> +        }
> +
> +        if (SdhcProtocol != NULL) {
> +            FreePool(SdhcProtocol);
> +            SdhcProtocol = NULL;
> +        }
> +    }
> +
> +    return Status;
> +}
> +
> +EFI_STATUS
> +SdhcInitialize(
> +    IN EFI_HANDLE ImageHandle,
> +    IN EFI_SYSTEM_TABLE *SystemTable
> +    )
> +{
> +    EFI_STATUS Status = EFI_SUCCESS;
> +    UINT32 uSdhcRegisteredCount = 0;
> +
> +    //
> +    // Register uSDHC1 through uSDHC4 if their base address is non-zero
> +    //

Can this hardwiring be moved out of the driver and into platform init
code using NonDiscoverableDeviceRegistrationLib, as per this
(unrelated) thread from earlier this year:
https://lists.01.org/pipermail/edk2-devel/2018-April/023922.html
?

> +
> +    //
> +    // uSDHC1
> +    //
> +    if (FixedPcdGet32(PcdSdhc1Enable)) {
> +        Status = uSdhcDeviceRegister(
> +            ImageHandle,
> +            1,
> +            (VOID*)FixedPcdGet32(PcdSdhc1Base),
> +            FixedPcdGet32(PcdSdhc1CardDetectSignal),
> +            FixedPcdGet32(PcdSdhc1WriteProtectSignal));
> +        if (!EFI_ERROR(Status)) {
> +            ++uSdhcRegisteredCount;
> +        }
> +    }
> +
> +    //
> +    // uSDHC2
> +    //
> +    if (FixedPcdGet32(PcdSdhc2Enable)) {
> +        Status = uSdhcDeviceRegister(
> +            ImageHandle,
> +            2,
> +            (VOID*)FixedPcdGet32(PcdSdhc2Base),
> +            FixedPcdGet32(PcdSdhc2CardDetectSignal),
> +            FixedPcdGet32(PcdSdhc2WriteProtectSignal));
> +        if (!EFI_ERROR(Status)) {
> +            ++uSdhcRegisteredCount;
> +        }
> +    }
> +
> +    //
> +    // uSDHC3
> +    //
> +    if (FixedPcdGet32(PcdSdhc3Enable)) {
> +        Status = uSdhcDeviceRegister(
> +            ImageHandle,
> +            3,
> +            (VOID*)FixedPcdGet32(PcdSdhc3Base),
> +            FixedPcdGet32(PcdSdhc3CardDetectSignal),
> +            FixedPcdGet32(PcdSdhc3WriteProtectSignal));
> +        if (!EFI_ERROR(Status)) {
> +            ++uSdhcRegisteredCount;
> +        }
> +    }
> +
> +    //
> +    // uSDHC4
> +    //
> +    if (FixedPcdGet32(PcdSdhc4Enable)) {
> +        Status = uSdhcDeviceRegister(
> +            ImageHandle,
> +            4,
> +            (VOID*)FixedPcdGet32(PcdSdhc4Base),
> +            FixedPcdGet32(PcdSdhc4CardDetectSignal),
> +            FixedPcdGet32(PcdSdhc4WriteProtectSignal));
> +        if (!EFI_ERROR(Status)) {
> +            ++uSdhcRegisteredCount;
> +        }
> +    }
> +
> +    //
> +    // Succeed driver loading if at least one enabled uSDHC got registered successfully
> +    //
> +    if ((Status != EFI_SUCCESS) && (uSdhcRegisteredCount > 0)) {
> +        Status = EFI_SUCCESS;
> +    }
> +
> +    return Status;
> +}
> diff --git a/Silicon/NXP/iMXPlatformPkg/Drivers/SdhcDxe/SdhcDxe.inf b/Silicon/NXP/iMXPlatformPkg/Drivers/SdhcDxe/SdhcDxe.inf
> new file mode 100644
> index 000000000000..5aee8b2ffbde
> --- /dev/null
> +++ b/Silicon/NXP/iMXPlatformPkg/Drivers/SdhcDxe/SdhcDxe.inf
> @@ -0,0 +1,67 @@
> +## @file
> +#
> +#  Copyright (c) Microsoft Corporation. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005

0x0001001a

> +  BASE_NAME                      = SdhcDxe
> +  FILE_GUID                      = A9945BAB-78C9-43C9-9175-F576CA189870
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = SdhcInitialize
> +
> +[Sources.common]
> +  SdhcDxe.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  Silicon/NXP/iMXPlatformPkg/iMXPlatformPkg.dec
> +  Platform/Microsoft/MsPkg.dec

Sorted alphabetically, please.

> +
> +[LibraryClasses]
> +  PcdLib
> +  UefiLib
> +  UefiDriverEntryPoint
> +  MemoryAllocationLib
> +  IoLib
> +  iMXIoMuxLib

Sorted alphabetically, please.

> +
> +[Guids]
> +
> +[Protocols]
> +  gEfiSdhcProtocolGuid
> +
> +[Pcd]
> +  giMXPlatformTokenSpaceGuid.PcdSdhc1Base
> +  giMXPlatformTokenSpaceGuid.PcdSdhc2Base
> +  giMXPlatformTokenSpaceGuid.PcdSdhc3Base
> +  giMXPlatformTokenSpaceGuid.PcdSdhc4Base
> +  giMXPlatformTokenSpaceGuid.PcdSdhc1Enable
> +  giMXPlatformTokenSpaceGuid.PcdSdhc2Enable
> +  giMXPlatformTokenSpaceGuid.PcdSdhc3Enable
> +  giMXPlatformTokenSpaceGuid.PcdSdhc4Enable
> +  giMXPlatformTokenSpaceGuid.PcdSdhc1CardDetectSignal
> +  giMXPlatformTokenSpaceGuid.PcdSdhc1WriteProtectSignal
> +  giMXPlatformTokenSpaceGuid.PcdSdhc2CardDetectSignal
> +  giMXPlatformTokenSpaceGuid.PcdSdhc2WriteProtectSignal
> +  giMXPlatformTokenSpaceGuid.PcdSdhc3CardDetectSignal
> +  giMXPlatformTokenSpaceGuid.PcdSdhc3WriteProtectSignal
> +  giMXPlatformTokenSpaceGuid.PcdSdhc4CardDetectSignal
> +  giMXPlatformTokenSpaceGuid.PcdSdhc4WriteProtectSignal
> +
> +[FixedPcd]
> +  giMXPlatformTokenSpaceGuid.PcdGpioBankMemoryRange
> +
> +[depex]
> +  TRUE
> diff --git a/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h b/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
> new file mode 100644
> index 000000000000..c75b543c8bb4
> --- /dev/null
> +++ b/Silicon/NXP/iMXPlatformPkg/Include/iMXGpio.h
> @@ -0,0 +1,101 @@
> +/** @file
> +*
> +*  Copyright (c) Microsoft Corporation. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD License
> +*  which accompanies this distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#ifndef _IMX_GPIO_H_
> +#define _IMX_GPIO_H_
> +
> +#include <Library/PcdLib.h>
> +
> +#ifndef C_ASSERT
> +#define C_ASSERT(e) typedef char __C_ASSERT__[(e)?1:-1]
> +#endif // C_ASSERT

What's wrong with normal ASSERT?

> +
> +typedef enum {
> +    IMX_GPIO_LOW = 0,
> +    IMX_GPIO_HIGH = 1
> +} IMX_GPIO_VALUE;
> +
> +typedef enum {
> +    IMX_GPIO_DIR_INPUT,
> +    IMX_GPIO_DIR_OUTPUT
> +} IMX_GPIO_DIR;
> +
> +typedef enum {
> +    IMX_GPIO_BANK1 = 1,
> +    IMX_GPIO_BANK2,
> +    IMX_GPIO_BANK3,
> +    IMX_GPIO_BANK4,
> +    IMX_GPIO_BANK5,
> +    IMX_GPIO_BANK6,
> +    IMX_GPIO_BANK7,
> +} IMX_GPIO_BANK;
> +
> +#pragma pack(push, 1)
> +
> +//
> +// GPIO reserved size is based on total size minus 8 DWORD of standard GPIO reg
> +//
> +C_ASSERT((FixedPcdGet32(PcdGpioBankMemoryRange) % 4) == 0);
> +
> +#define GPIO_RESERVED_SIZE \
> +    ((FixedPcdGet32(PcdGpioBankMemoryRange) / 4) - 8)
> +
> +typedef struct {
> +    UINT32 DR;                            // 0x00 GPIO data register (GPIO1_DR)
> +    UINT32 GDIR;                          // 0x04 GPIO direction register (GPIO1_GDIR)
> +    UINT32 PSR;                           // 0x08 GPIO pad status register (GPIO1_PSR)
> +    UINT32 ICR1;                          // 0x0C GPIO interrupt configuration register1 (GPIO1_ICR1)
> +    UINT32 ICR2;                          // 0x10 GPIO interrupt configuration register2 (GPIO1_ICR2)
> +    UINT32 IMR;                           // 0x14 GPIO interrupt mask register (GPIO1_IMR)
> +    UINT32 ISR;                           // 0x18 GPIO interrupt status register (GPIO1_ISR)
> +    UINT32 EDGE_SEL;                      // 0x1C GPIO edge select register (GPIO1_EDGE_SEL)
> +    UINT32 reserved[GPIO_RESERVED_SIZE];
> +} IMX_GPIO_BANK_REGISTERS;
> +
> +#pragma pack(pop)
> +
> +typedef struct {
> +    IMX_GPIO_BANK_REGISTERS Banks[7];
> +} IMX_GPIO_REGISTERS;
> +
> +/**
> +    Set the specified GPIO to the specified direction.
> +**/
> +VOID
> +ImxGpioDirection (
> +    IMX_GPIO_BANK Bank,
> +    UINT32 IoNumber,
> +    IMX_GPIO_DIR Direction
> +    );
> +
> +/**
> +    Write a value to a GPIO pin.
> +**/
> +VOID
> +ImxGpioWrite (
> +    IMX_GPIO_BANK Bank,
> +    UINT32 IoNumber,
> +    IMX_GPIO_VALUE Value
> +    );
> +
> +/**
> +    Read a GPIO pin input value.
> +**/
> +IMX_GPIO_VALUE
> +ImxGpioRead (
> +    IMX_GPIO_BANK Bank,
> +    UINT32 IoNumber
> +    );
> +
> +#endif
> diff --git a/Silicon/NXP/iMXPlatformPkg/Include/iMXuSdhc.h b/Silicon/NXP/iMXPlatformPkg/Include/iMXuSdhc.h
> new file mode 100644
> index 000000000000..acdbcd324631
> --- /dev/null
> +++ b/Silicon/NXP/iMXPlatformPkg/Include/iMXuSdhc.h
> @@ -0,0 +1,277 @@
> +/** @file
> +*
> +*  Copyright (c) Microsoft Corporation. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD License
> +*  which accompanies this distribution.  The full text of the license may be found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +*
> +**/
> +
> +#ifndef __IMX_USDHC_H__
> +#define __IMX_USDHC_H__
> +
> +//
> +// uSDHCx Registers Layout
> +//
> +typedef struct {
> +    UINT32 DS_ADDR;
> +    UINT32 BLK_ATT;
> +    UINT32 CMD_ARG;
> +    UINT32 CMD_XFR_TYP;
> +    UINT32 CMD_RSP0;
> +    UINT32 CMD_RSP1;
> +    UINT32 CMD_RSP2;
> +    UINT32 CMD_RSP3;
> +    UINT32 DATA_BUFF_ACC_PORT;
> +    UINT32 PRES_STATE;
> +    UINT32 PROT_CTRL;
> +    UINT32 SYS_CTRL;
> +    UINT32 INT_STATUS;
> +    UINT32 INT_STATUS_EN;
> +    UINT32 INT_SIGNAL_EN;
> +    UINT32 AUTOCMD12_ERR_STATUS;
> +    UINT32 HOST_CTRL_CAP;
> +    UINT32 WTMK_LVL;
> +    UINT32 MIX_CTRL;
> +    UINT32 _pad0;
> +    UINT32 FORCE_EVENT;
> +    UINT32 ADMA_ERR_STATUS;
> +    UINT32 ADMA_SYS_ADDR;
> +    UINT32 _pad1;
> +    UINT32 DLL_CTRL;
> +    UINT32 DLL_STATUS;
> +    UINT32 CLK_TUNE_CTRL_STATUS;
> +    UINT32 _pad2[21];
> +    UINT32 VEND_SPEC;
> +    UINT32 MMC_BOOT;
> +    UINT32 VEND_SPEC2;
> +} USDHC_REGISTERS;
> +
> +//
> +// Block Attributes uSDHCx_BLK_ATT fields
> +//
> +typedef union {
> +   UINT32 AsUint32;
> +   struct {
> +      UINT32 BLKSIZE    : 13; // 0:12
> +      UINT32 _reserved0 : 3;  // 13:15
> +      UINT32 BLKCNT     : 16; // 16:31
> +   } Fields;
> +} USDHC_BLK_ATT_REG;
> +
> +//
> +// Command Transfer Type uSDHCx_CMD_XFR_TYP fields
> +//
> +typedef union {
> +    UINT32 AsUint32;
> +    struct {
> +        UINT32 _reserved0   : 16; // 0:15
> +        UINT32 RSPTYP       : 2; // 16:17
> +        UINT32 _reserved1   : 1; // 18
> +        UINT32 CCCEN        : 1; // 19
> +        UINT32 CICEN        : 1; // 20
> +        UINT32 DPSEL        : 1; // 21
> +        UINT32 CMDTYP       : 2; // 22:23
> +        UINT32 CMDINX       : 6; // 24:29
> +        UINT32 _reserved2   : 2; // 30:31
> +    } Fields;
> +} USDHC_CMD_XFR_TYP_REG;
> +
> +#define USDHC_CMD_XFR_TYP_RSPTYP_NO_RSP          0
> +#define USDHC_CMD_XFR_TYP_RSPTYP_RSP_136         1
> +#define USDHC_CMD_XFR_TYP_RSPTYP_RSP_48          2
> +#define USDHC_CMD_XFR_TYP_RSPTYP_RSP_48_CHK_BSY  3
> +#define USDHC_CMD_XFR_TYP_CMDTYP_ABORT           3
> +
> +//
> +// System Control uSDHCx_SYS_CTRL fields
> +//
> +typedef union {
> +    UINT32 AsUint32;
> +    struct {
> +        UINT32 _reserved0   : 4; // 0:3
> +        UINT32 DVS          : 4; // 4:7
> +        UINT32 SDCLKFS      : 8; // 8:15
> +        UINT32 DTOCV        : 4; // 16:19
> +        UINT32 _reserved1   : 3; // 20:22
> +        UINT32 IPP_RST_N    : 1; // 23
> +        UINT32 RSTA         : 1; // 24
> +        UINT32 RSTC         : 1; // 25
> +        UINT32 RSTD         : 1; // 26
> +        UINT32 INITA        : 1; // 27
> +        UINT32 _reserved2   : 4; // 28-31
> +    } Fields;
> +} USDHC_SYS_CTRL_REG;
> +
> +//
> +// Host Controller Capabilities Register uSDHCx_HOST_CTRL_CAP fields
> +//
> +typedef union {
> +    UINT32 AsUint32;
> +    struct {
> +        UINT32 _reserved0       : 16; // 0:15
> +        UINT32 MBL              : 3; // 16:18
> +        UINT32 _reserved1       : 1; // 19
> +        UINT32 ADMAS            : 1; // 20
> +        UINT32 HSS              : 1; // 21
> +        UINT32 DMAS             : 1; // 22
> +        UINT32 SRS              : 1; // 23
> +        UINT32 VS33             : 1; // 24
> +        UINT32 VS30             : 1; // 25
> +        UINT32 VS18             : 1; // 26
> +        UINT32 _reserved2       : 5; // 27:31
> +    } Fields;
> +} USDHC_HOST_CTRL_CAP_REG;
> +
> +//
> +// Watermark Level uSDHCx_WTMK_LVL
> +//
> +typedef union {
> +    UINT32 AsUint32;
> +    struct {
> +        UINT32 RD_WML           : 8; // 0:7
> +        UINT32 RD_BRST_LEN      : 5; // 8:12
> +        UINT32 _reserved0       : 3; // 13:15
> +        UINT32 WR_WML           : 8; // 16:23
> +        UINT32 WR_BRST_LEN      : 5; // 24:28
> +        UINT32 _reserved1       : 3; // 29:31
> +    } Fields;
> +} USDHC_WTMK_LVL_REG;
> +
> +#define USDHC_WTMK_RD_WML_MAX_VAL   0x10
> +#define USDHC_WTMK_WR_WML_MAX_VAL   0x80
> +
> +//
> +// Mixer Control uSDHCx_MIX_CTRL fields
> +//
> +typedef union {
> +    UINT32 AsUint32;
> +    struct {
> +        UINT32 DMAEN          : 1; // 0
> +        UINT32 BCEN           : 1; // 1
> +        UINT32 AC12EN         : 1; // 2
> +        UINT32 DDR_EN         : 1; // 3
> +        UINT32 DTDSEL         : 1; // 4
> +        UINT32 MSBSEL         : 1; // 5
> +        UINT32 NIBBLE_POS     : 1; // 6
> +        UINT32 AC23EN         : 1; // 7
> +        UINT32 _reserved0     : 14; // 8:21
> +        UINT32 EXE_TUNE       : 1; // 22
> +        UINT32 SMP_CLK_SEL    : 1; // 23
> +        UINT32 AUTO_TUNE_EN   : 1; //24
> +        UINT32 FBCLK_SEL      : 1; // 25
> +        UINT32 _reserved1     : 6; // 26-31
> +    } Fields;
> +} USDHC_MIX_CTRL_REG;
> +
> +//
> +// Present State uSDHCx_PRES_STATE fields
> +//
> +typedef union {
> +    UINT32 AsUint32;
> +    struct {
> +        UINT32 CIHB         : 1; // 0
> +        UINT32 CDIHB        : 1; // 1
> +        UINT32 DLA          : 1; // 2
> +        UINT32 SDSTB        : 1; // 3
> +        UINT32 IPGOFF       : 1; // 4
> +        UINT32 HCKOFF       : 1; // 5
> +        UINT32 PEROFF       : 1; // 6
> +        UINT32 SDOFF        : 1; // 7
> +        UINT32 WTA          : 1; // 8
> +        UINT32 RTA          : 1; // 9
> +        UINT32 BWEN         : 1; // 10
> +        UINT32 BREN         : 1; // 11
> +        UINT32 PTR          : 1; // 12
> +        UINT32 _reserved0   : 3; // 13:15
> +        UINT32 CINST        : 1; // 16
> +        UINT32 _reserved1   : 1; // 17
> +        UINT32 CDPL         : 1; // 18
> +        UINT32 WPSPL        : 1; // 19
> +        UINT32 _reserved2   : 3; // 20:22
> +        UINT32 CLSL         : 1; // 23
> +        UINT32 DLSL         : 8; // 24:31
> +    } Fields;
> +} USDHC_PRES_STATE_REG;
> +
> +//
> +// Present State uSDHCx_PROT_CTRL fields
> +//
> +typedef union {
> +    UINT32 AsUint32;
> +    struct {
> +        UINT32 LCTL             : 1; // 0
> +        UINT32 DTW              : 2; // 1:2
> +        UINT32 D3CD             : 1; // 3
> +        UINT32 EMODE            : 2; // 4:5
> +        UINT32 CDTL             : 1; // 6
> +        UINT32 CDSS             : 1; // 7
> +        UINT32 DMASEL           : 2; // 8:9
> +        UINT32 _reserved0       : 6; // 10:15
> +        UINT32 SABGREQ          : 1; // 16
> +        UINT32 CREQ             : 1; // 17
> +        UINT32 RWCTL            : 1; // 18
> +        UINT32 IABG             : 1; // 19
> +        UINT32 RD_DONE_NO_8CLK  : 1; // 20
> +        UINT32 _reserved1       : 3; // 21:23
> +        UINT32 WECINT           : 1; // 24
> +        UINT32 WECINS           : 1; // 25
> +        UINT32 WECRM            : 1; // 26
> +        UINT32 BURST_LEN_EN     : 3; // 27:29
> +        UINT32 NON_EXACT_BLK_RD : 1; // 30
> +        UINT32 _reserved2       : 1; // 31
> +    } Fields;
> +} USDHC_PROT_CTRL_REG;
> +
> +#define USDHC_PROT_CTRL_DTW_1BIT             0x0
> +#define USDHC_PROT_CTRL_DTW_4BIT             0x1
> +#define USDHC_PROT_CTRL_DTW_8BIT             0x2
> +#define USDHC_PROT_CTRL_EMODE_LITTLE_ENDIAN  0x2
> +
> +//
> +// Interrupt Status uSDHCx_INT_STATUS fields
> +//
> +typedef union {
> +    UINT32 AsUint32;
> +    struct {
> +        UINT32 CC           : 1; // 0
> +        UINT32 TC           : 1; // 1
> +        UINT32 BGE          : 1; // 2
> +        UINT32 DINT         : 1; // 3
> +        UINT32 BWR          : 1; // 4
> +        UINT32 BRR          : 1; // 5
> +        UINT32 CINS         : 1; // 6
> +        UINT32 CRM          : 1; // 7
> +        UINT32 CINT         : 1; // 8
> +        UINT32 _reserved0   : 3; // 9:11
> +        UINT32 RTE          : 1; // 12
> +        UINT32 _reserved1   : 1; // 13
> +        UINT32 TP           : 1; // 14
> +        UINT32 _reserved2   : 1; // 15
> +        UINT32 CTOE         : 1; // 16
> +        UINT32 CCE          : 1; // 17
> +        UINT32 CEBE         : 1; // 18
> +        UINT32 CIE          : 1; // 19
> +        UINT32 DTOE         : 1; // 20
> +        UINT32 DCE          : 1; // 21
> +        UINT32 DEBE         : 1; // 22
> +        UINT32 _reserved3   : 1; // 23
> +        UINT32 AC12E        : 1; // 24
> +        UINT32 _reserved4   : 1; // 25
> +        UINT32 TNE          : 1; // 26
> +        UINT32 _reserved5   : 1; // 27
> +        UINT32 DMAE         : 1; // 28
> +        UINT32 _reserved6   : 3; // 29:31
> +    } Fields;
> +} USDHC_INT_STATUS_REG;
> +
> +#define USDHC_INT_STATUS_CMD_ERROR   (BIT16 | BIT17 | BIT18 | BIT19)
> +#define USDHC_INT_STATUS_DATA_ERROR  (BIT20 | BIT21 | BIT22)
> +#define USDHC_INT_STATUS_ERROR       (USDHC_INT_STATUS_CMD_ERROR | USDHC_INT_STATUS_DATA_ERROR)
> +
> +#endif // __IMX_USDHC_H__
> -- 
> 2.16.2.gvfs.1.33.gf5370f1
> 


  reply	other threads:[~2018-08-01 16:15 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-19  4:11 [PATCH edk2-platforms 0/7] Silicon/NXP: Import NXP i.MX platform package Chris Co
2018-07-19  4:11 ` [PATCH edk2-platforms 1/7] Silicon/NXP: Add support for iMX SDHC Chris Co
2018-08-01 16:15   ` Leif Lindholm [this message]
2018-08-01 23:59     ` Chris Co
2018-07-19  4:11 ` [PATCH edk2-platforms 2/7] Silicon/NXP: Add iMX display library support Chris Co
2018-07-19  4:11 ` [PATCH edk2-platforms 3/7] Silicon/NXP: Add I2C library support for iMX platforms Chris Co
2018-07-19  4:11 ` [PATCH edk2-platforms 4/7] Silicon/NXP: Add UART " Chris Co
2018-07-19  4:11 ` [PATCH edk2-platforms 5/7] Silicon/NXP: Add Virtual RTC support for IMX platform Chris Co
2018-07-19  4:11 ` [PATCH edk2-platforms 6/7] Silicon/NXP: Add iMXPlatformPkg dec Chris Co
2018-07-19  4:11 ` [PATCH edk2-platforms 7/7] Silicon/NXP: Add headers for other iMX packages to use Chris Co

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180801161508.uvfzsbbntzwtiv2d@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox