From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: None (no SPF record) identity=mailfrom; client-ip=2607:f8b0:4864:20::d43; helo=mail-io1-xd43.google.com; envelope-from=mw@semihalf.com; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd43.google.com (mail-io1-xd43.google.com [IPv6:2607:f8b0:4864:20::d43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4C53A21BADAB6 for ; Mon, 8 Oct 2018 05:59:22 -0700 (PDT) Received: by mail-io1-xd43.google.com with SMTP id t7-v6so15726070ioj.13 for ; Mon, 08 Oct 2018 05:59:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=D08lc7r5NlosWz1ZYdcHF6IjbcyIY5OgjhGEatnNdto=; b=TetM2p6I3gtcficpzug5Y8U3pXxKhsLC+zGKgJNidT+S0vw2HMlz6IIxDinRM/degt kyaI7aTk9QA5/Nw6ssKz3lf0YnhWVi9BsM9U3jFskiJuER9PwEUDjzdpDS5zjq1krVO8 hezPc6hzEDJsIDJOkAQ0szE9sPu66GW038VK/u8EqiRbfG6kjs8pjvU6RNI1CcumQqhe x3aKGYaX2JWmjBHWXZAF5vr6sbXYMxCq8LvHhWHXBE4JFsoKmN7tB9IkOyqDok5wsRAb 8kDaaOV0v+K7feHbrxt8Sq0bmqNvKABRpQNyvaYuJecMgntjYd5oRAx7g31qgv1cZmZa I2Gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=D08lc7r5NlosWz1ZYdcHF6IjbcyIY5OgjhGEatnNdto=; b=PQ6/1NiX3j7In0/0l2hcoRiAoynl6fmO3Va/swSp93F64MpQoiiX2yIY/ZxvSveKSm TwJn0S3XduJBqz6bklWFvRSYc0z5REMWzktTfW3GjsRgYAl0jIUCWgV7VfjuElWnHfJ/ 2KgIdpmiJo4RI5+9OtHi3zje6ADh2Iw0MTV1ae35Al5wXhg3xRMZ43yjVqRZkqjl0zk0 GWbmDC2Bog0c73a29ivcGImW7jvl+CNNuTljnRcuKjtAxjn7S2TERmOtOii1+SGh3FGY ZTUzO86Cqt1+VqF/FVrWbPdoxsVhHS+RH4v9mQO4hpsz1gXpbMPvI8Ej0iWs4xpBsT1W mb2A== X-Gm-Message-State: ABuFfogLSTfwn8JWRW4+NHKwUb77nhluS4UqmS2nwBu6coaTRchI+IqQ ft6FlB6xZUr5ZlsMWq2/IHegW6QcbWFan333MWGbFQ== X-Google-Smtp-Source: ACcGV60hEnvIL5mJJARxEXBOrct4nZ4okVqLi73DrWJzHyBsbHCOmFT/suxbzS0wNdXVIucxonISOgDQsms1sS8DkTY= X-Received: by 2002:a6b:b74d:: with SMTP id h74-v6mr15669979iof.248.1539003561224; Mon, 08 Oct 2018 05:59:21 -0700 (PDT) MIME-Version: 1.0 References: <1538745911-22484-1-git-send-email-mw@semihalf.com> <1538745911-22484-3-git-send-email-mw@semihalf.com> In-Reply-To: From: Marcin Wojtas Date: Mon, 8 Oct 2018 14:59:06 +0200 Message-ID: To: Ard Biesheuvel Cc: "Zeng, Star" , eric.dong@intel.com, "Ni, Ruiyu" , edk2-devel-01 , "Tian, Feng" , "Kinney, Michael D" , "Gao, Liming" , Leif Lindholm , hao.a.wu@intel.com, nadavh@marvell.com, "jsd@semihalf.com" , Tomasz Michalec Subject: Re: [PATCH v2 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Oct 2018 12:59:22 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ard, pon., 8 pa=C5=BA 2018 o 14:41 Ard Biesheuvel na= pisa=C5=82(a): > > (add MdeModulePkg maintainers) > > On 5 October 2018 at 15:25, Marcin Wojtas wrote: > > From: Tomasz Michalec > > > > Some SD Host Controlers use different values in Host Control 2 Register > > to select UHS Mode. This patch adds a new UhsSignaling type routine to > > the NotifyPhase of the SdMmcOverride protocol. > > > > UHS signaling configuration is moved to a common, default routine > > (SdMmcHcUhsSignaling), which is called when SdMmcOverride does not > > cover this functionality. > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Marcin Wojtas > > --- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 50 +++++++ > > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 2 + > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 153 ++++++++++++---= ----- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 37 +++-- > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 69 +++++++++ > > 5 files changed, 243 insertions(+), 68 deletions(-) > > > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModu= lePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > > index e389d52..a03160d 100644 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > > @@ -63,6 +63,39 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, E= ITHER EXPRESS OR IMPLIED. > > #define SD_MMC_HC_CTRL_VER 0xFE > > > > // > > +// SD Host Controler bits to HOST_CTRL2 register > > +// > > +#define SD_MMC_HC_CTRL_UHS_MASK 0x0007 > > +#define SD_MMC_HC_CTRL_UHS_SDR12 0x0000 > > +#define SD_MMC_HC_CTRL_UHS_SDR25 0x0001 > > +#define SD_MMC_HC_CTRL_UHS_SDR50 0x0002 > > +#define SD_MMC_HC_CTRL_UHS_SDR104 0x0003 > > +#define SD_MMC_HC_CTRL_UHS_DDR50 0x0004 > > +#define SD_MMC_HC_CTRL_MMC_DDR52 0x0004 > > +#define SD_MMC_HC_CTRL_MMC_SDR50 0x0002 > > +#define SD_MMC_HC_CTRL_MMC_SDR25 0x0001 > > +#define SD_MMC_HC_CTRL_MMC_SDR12 0x0000 > > +#define SD_MMC_HC_CTRL_HS200 0x0003 > > +#define SD_MMC_HC_CTRL_HS400 0x0005 > > + > > +// > > +// Timing modes for uhs > > +// > > +typedef enum { > > + SdMmcUhsSdr12, > > + SdMmcUhsSdr25, > > + SdMmcUhsSdr50, > > + SdMmcUhsSdr104, > > + SdMmcUhsDdr50, > > + SdMmcMmcDdr52, > > + SdMmcMmcSdr50, > > + SdMmcMmcSdr25, > > + SdMmcMmcSdr12, > > + SdMmcMmcHs200, > > + SdMmcMmcHs400, > > +} SD_MMC_UHS_TIMING; > > + > > Here, we end up with two sets of symbolic constants for the same > thing, and I suppose this enum will be duplicated in your > SdMmcOverride implementation? > Why duplicated? Macros are for generic UHS_MODE_SEL field values for SD and MMC in HostControl2Register. SD_MMC_UHS_TIMING is just a timing mode indicator, it can be used not only in UhsSignaling routine (actually the next patch, with SwitchClockFreqPost, use it...). In my SdMmcOverride implementation this enum is not duplicated, because this file (SdMmcPciHci.h) is included via Protocol/SdMmcOverride.h. > > > > +// > > // The transfer modes supported by SD Host Controller > > // Simplified Spec 3.0 Table 1-2 > > // > > @@ -508,4 +541,21 @@ SdMmcHcInitTimeoutCtrl ( > > IN UINT8 Slot > > ); > > > > +/** > > + Set SD Host Controler control 2 registry according to selected speed= . > > + > > + @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] Timing The timing to select. > > + > > + @retval EFI_SUCCESS The timing is set successfully. > > + @retval Others The timing isn't set successfully. > > +**/ > > +EFI_STATUS > > +SdMmcHcUhsSignaling ( > > + IN EFI_PCI_IO_PROTOCOL *PciIo, > > + IN UINT8 Slot, > > + IN SD_MMC_UHS_TIMING Timing > > + ); > > + > > #endif > > diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModuleP= kg/Include/Protocol/SdMmcOverride.h > > index 178945f..25db98a 100644 > > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > @@ -17,6 +17,7 @@ > > #ifndef __SD_MMC_OVERRIDE_H__ > > #define __SD_MMC_OVERRIDE_H__ > > > > +#include > > #include > > > > #define EDKII_SD_MMC_OVERRIDE_PROTOCOL_GUID \ > > @@ -31,6 +32,7 @@ typedef enum { > > EdkiiSdMmcResetPost, > > EdkiiSdMmcInitHostPre, > > EdkiiSdMmcInitHostPost, > > + EdkiiSdMmcUhsSignaling, > > } EDKII_SD_MMC_PHASE_TYPE; > > > > /** > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModul= ePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > index c5fd214..05bd4a0 100755 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > @@ -740,10 +740,13 @@ EmmcSwitchToHighSpeed ( > > IN UINT8 BusWidth > > ) > > { > > - EFI_STATUS Status; > > - UINT8 HsTiming; > > - UINT8 HostCtrl1; > > - UINT8 HostCtrl2; > > + EFI_STATUS Status; > > + UINT8 HsTiming; > > + UINT8 HostCtrl1; > > + SD_MMC_UHS_TIMING Timing; > > + SD_MMC_HC_PRIVATE_DATA *Private; > > + > > + Private =3D SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > > > > Status =3D EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, Bu= sWidth); > > if (EFI_ERROR (Status)) { > > @@ -758,27 +761,37 @@ EmmcSwitchToHighSpeed ( > > return Status; > > } > > > > - // > > - // Clean UHS Mode Select field of Host Control 2 reigster before upd= ate > > - // > > - HostCtrl2 =3D (UINT8)~0x7; > > - Status =3D SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof= (HostCtrl2), &HostCtrl2); > > - if (EFI_ERROR (Status)) { > > - return Status; > > - } > > - // > > - // Set UHS Mode Select field of Host Control 2 reigster to SDR12/25/= 50 > > - // > > if (IsDdr) { > > - HostCtrl2 =3D BIT2; > > + Timing =3D SdMmcMmcDdr52; > > } else if (ClockFreq =3D=3D 52) { > > - HostCtrl2 =3D BIT0; > > + Timing =3D SdMmcMmcSdr50; > > + } else if (ClockFreq =3D=3D 26) { > > + Timing =3D SdMmcMmcSdr25; > > } else { > > - HostCtrl2 =3D 0; > > + Timing =3D SdMmcMmcSdr12; > > } > > - Status =3D SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof = (HostCtrl2), &HostCtrl2); > > - if (EFI_ERROR (Status)) { > > - return Status; > > + > > + if (mOverride !=3D NULL && mOverride->NotifyPhase !=3D NULL) { > > + Status =3D mOverride->NotifyPhase ( > > + Private->ControllerHandle, > > + Slot, > > + EdkiiSdMmcUhsSignaling, > > + &Timing > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG (( > > + DEBUG_ERROR, > > + "%a: SD/MMC uhs signaling notifier callback failed - %r\n", > > + __FUNCTION__, > > + Status > > + )); > > + return Status; > > + } > > + } else { > > + Status =3D SdMmcHcUhsSignaling (PciIo, Slot, Timing); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > } > > > > HsTiming =3D 1; > > @@ -814,10 +827,13 @@ EmmcSwitchToHS200 ( > > IN UINT8 BusWidth > > ) > > { > > - EFI_STATUS Status; > > - UINT8 HsTiming; > > - UINT8 HostCtrl2; > > - UINT16 ClockCtrl; > > + EFI_STATUS Status; > > + UINT8 HsTiming; > > + UINT16 ClockCtrl; > > + SD_MMC_UHS_TIMING Timing; > > + SD_MMC_HC_PRIVATE_DATA *Private; > > + > > + Private =3D SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > > > > if ((BusWidth !=3D 4) && (BusWidth !=3D 8)) { > > return EFI_INVALID_PARAMETER; > > @@ -837,21 +853,30 @@ EmmcSwitchToHS200 ( > > if (EFI_ERROR (Status)) { > > return Status; > > } > > - // > > - // Clean UHS Mode Select field of Host Control 2 reigster before upd= ate > > - // > > - HostCtrl2 =3D (UINT8)~0x7; > > - Status =3D SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof= (HostCtrl2), &HostCtrl2); > > - if (EFI_ERROR (Status)) { > > - return Status; > > - } > > - // > > - // Set UHS Mode Select field of Host Control 2 reigster to SDR104 > > - // > > - HostCtrl2 =3D BIT0 | BIT1; > > - Status =3D SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof = (HostCtrl2), &HostCtrl2); > > - if (EFI_ERROR (Status)) { > > - return Status; > > + > > + Timing =3D SdMmcMmcHs200; > > + > > + if (mOverride !=3D NULL && mOverride->NotifyPhase !=3D NULL) { > > + Status =3D mOverride->NotifyPhase ( > > + Private->ControllerHandle, > > + Slot, > > + EdkiiSdMmcUhsSignaling, > > + &Timing > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG (( > > + DEBUG_ERROR, > > + "%a: SD/MMC uhs signaling notifier callback failed - %r\n", > > + __FUNCTION__, > > + Status > > + )); > > + return Status; > > + } > > + } else { > > + Status =3D SdMmcHcUhsSignaling (PciIo, Slot, Timing); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > } > > // > > // Wait Internal Clock Stable in the Clock Control register to be 1 = before set SD Clock Enable bit > > @@ -910,9 +935,12 @@ EmmcSwitchToHS400 ( > > IN UINT32 ClockFreq > > ) > > { > > - EFI_STATUS Status; > > - UINT8 HsTiming; > > - UINT8 HostCtrl2; > > + EFI_STATUS Status; > > + UINT8 HsTiming; > > + SD_MMC_UHS_TIMING Timing; > > + SD_MMC_HC_PRIVATE_DATA *Private; > > + > > + Private =3D SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > > > > Status =3D EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq,= 8); > > if (EFI_ERROR (Status)) { > > @@ -933,21 +961,30 @@ EmmcSwitchToHS400 ( > > if (EFI_ERROR (Status)) { > > return Status; > > } > > - // > > - // Clean UHS Mode Select field of Host Control 2 reigster before upd= ate > > - // > > - HostCtrl2 =3D (UINT8)~0x7; > > - Status =3D SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof= (HostCtrl2), &HostCtrl2); > > - if (EFI_ERROR (Status)) { > > - return Status; > > - } > > - // > > - // Set UHS Mode Select field of Host Control 2 reigster to HS400 > > - // > > - HostCtrl2 =3D BIT0 | BIT2; > > - Status =3D SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof = (HostCtrl2), &HostCtrl2); > > - if (EFI_ERROR (Status)) { > > - return Status; > > + > > + Timing =3D SdMmcMmcHs400; > > + > > + if (mOverride !=3D NULL && mOverride->NotifyPhase !=3D NULL) { > > + Status =3D mOverride->NotifyPhase ( > > + Private->ControllerHandle, > > + Slot, > > + EdkiiSdMmcUhsSignaling, > > + &Timing > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG (( > > + DEBUG_ERROR, > > + "%a: SD/MMC uhs signaling notifier callback failed - %r\n", > > + __FUNCTION__, > > + Status > > + )); > > + return Status; > > + } > > + } else { > > + Status =3D SdMmcHcUhsSignaling (PciIo, Slot, Timing); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > } > > > > HsTiming =3D 3; > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModuleP= kg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > index 8c93933..5645a71 100644 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > @@ -784,8 +784,8 @@ SdCardSetBusMode ( > > UINT8 BusWidth; > > UINT8 AccessMode; > > UINT8 HostCtrl1; > > - UINT8 HostCtrl2; > > UINT8 SwitchResp[64]; > > + SD_MMC_UHS_TIMING Timing; > > SD_MMC_HC_PRIVATE_DATA *Private; > > > > Private =3D SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > > @@ -817,18 +817,23 @@ SdCardSetBusMode ( > > if (S18A && (Capability->Sdr104 !=3D 0) && ((SwitchResp[13] & BIT3) = !=3D 0)) { > > ClockFreq =3D 208; > > AccessMode =3D 3; > > + Timing =3D SdMmcUhsSdr104; > > } else if (S18A && (Capability->Sdr50 !=3D 0) && ((SwitchResp[13] & = BIT2) !=3D 0)) { > > ClockFreq =3D 100; > > AccessMode =3D 2; > > + Timing =3D SdMmcUhsSdr50; > > } else if (S18A && (Capability->Ddr50 !=3D 0) && ((SwitchResp[13] & = BIT4) !=3D 0)) { > > ClockFreq =3D 50; > > AccessMode =3D 4; > > + Timing =3D SdMmcUhsDdr50; > > } else if ((SwitchResp[13] & BIT1) !=3D 0) { > > ClockFreq =3D 50; > > AccessMode =3D 1; > > + Timing =3D SdMmcUhsSdr25; > > } else { > > ClockFreq =3D 25; > > AccessMode =3D 0; > > + Timing =3D SdMmcUhsSdr12; > > } > > > > Status =3D SdCardSwitch (PassThru, Slot, AccessMode, 0xF, 0xF, 0xF, = TRUE, SwitchResp); > > @@ -854,15 +859,27 @@ SdCardSetBusMode ( > > } > > } > > > > - HostCtrl2 =3D (UINT8)~0x7; > > - Status =3D SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof= (HostCtrl2), &HostCtrl2); > > - if (EFI_ERROR (Status)) { > > - return Status; > > - } > > - HostCtrl2 =3D AccessMode; > > - Status =3D SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof = (HostCtrl2), &HostCtrl2); > > - if (EFI_ERROR (Status)) { > > - return Status; > > + if (mOverride !=3D NULL && mOverride->NotifyPhase !=3D NULL) { > > + Status =3D mOverride->NotifyPhase ( > > + Private->ControllerHandle, > > + Slot, > > + EdkiiSdMmcUhsSignaling, > > + &Timing > > + ); > > + if (EFI_ERROR (Status)) { > > + DEBUG (( > > + DEBUG_ERROR, > > + "%a: SD/MMC uhs signaling notifier callback failed - %r\n", > > + __FUNCTION__, > > + Status > > + )); > > + return Status; > > + } > > + } else { > > + Status =3D SdMmcHcUhsSignaling (PciIo, Slot, Timing); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > } > > > > Status =3D SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capab= ility); > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModu= lePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > > index 02eb4ad..38d6202 100644 > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > > @@ -1137,6 +1137,75 @@ SdMmcHcInitHost ( > > } > > > > /** > > + Set SD Host Controler control 2 registry according to selected speed= . > > + > > + @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] Timing The timing to select. > > + > > + @retval EFI_SUCCESS The timing is set successfully. > > + @retval Others The timing isn't set successfully. > > +**/ > > +EFI_STATUS > > +SdMmcHcUhsSignaling ( > > + IN EFI_PCI_IO_PROTOCOL *PciIo, > > + IN UINT8 Slot, > > + IN SD_MMC_UHS_TIMING Timing > > + ) > > +{ > > + EFI_STATUS Status; > > + UINT8 HostCtrl2; > > + > > + HostCtrl2 =3D (UINT8)~SD_MMC_HC_CTRL_UHS_MASK; > > + Status =3D SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof= (HostCtrl2), &HostCtrl2); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + > > + switch (Timing) { > > + case SdMmcUhsSdr12: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_UHS_SDR12; > > + break; > > + case SdMmcUhsSdr25: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_UHS_SDR25; > > + break; > > + case SdMmcUhsSdr50: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_UHS_SDR50; > > + break; > > + case SdMmcUhsSdr104: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_UHS_SDR104; > > + break; > > + case SdMmcUhsDdr50: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_UHS_DDR50; > > + break; > > + case SdMmcMmcDdr52: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_MMC_DDR52; > > + break; > > + case SdMmcMmcSdr50: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_MMC_SDR50; > > + break; > > + case SdMmcMmcSdr25: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_MMC_SDR25; > > + break; > > + case SdMmcMmcSdr12: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_MMC_SDR12; > > + break; > > + case SdMmcMmcHs200: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_HS200; > > + break; > > + case SdMmcMmcHs400: > > + HostCtrl2 =3D SD_MMC_HC_CTRL_HS400; > > + break; > > + default: > > + HostCtrl2 =3D 0; > > + break; > > + } > > + Status =3D SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof = (HostCtrl2), &HostCtrl2); > > + > > + return Status; > > +} > > + > > This function looks identical to the override that your are proposing > in your platform code. Why is that? Are the values of those defines > different? > This is exactly the reason. For Xenon, MMC_HS400 and MMC_HS200 values of UHS_MODE_SEL field in HostControl2Register are custom. Actually, non-generic UHS_MODE_SEL values are pretty common, from what I can see in the Linux code. The new callback allows overriding it. Looking forward to your feedback. Best regards, Marcin