From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.1153.1609821076732821627 for ; Mon, 04 Jan 2021 20:31:17 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: jeremy.linton@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7595A101E; Mon, 4 Jan 2021 20:31:15 -0800 (PST) Received: from [192.168.122.166] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 31E4A3F70D; Mon, 4 Jan 2021 20:31:15 -0800 (PST) Subject: Re: [edk2-devel] [PATCH v3 3/7] Platform/RaspberryPi: Split MMC register defintions To: devel@edk2.groups.io, philmd@redhat.com Cc: ard.biesheuvel@arm.com, leif@nuviainc.com, pete@akeo.ie, samer.el-haj-mahmoud@arm.com, Andrei Warkentin References: <20210104173731.1413044-1-jeremy.linton@arm.com> <20210104173731.1413044-4-jeremy.linton@arm.com> From: "Jeremy Linton" Message-ID: Date: Mon, 4 Jan 2021 22:31:14 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi, First, thanks for taking a look at this: On 1/4/21 12:41 PM, Philippe Mathieu-Daud=C3=A9 via groups.io wrote: > Hi Jeremy, >=20 > On 1/4/21 6:37 PM, Jeremy Linton wrote: >> The current MMC (really SDHCI) defintions are tied to the >=20 > Typo "definitions". >=20 >> arasan controller. As we intend to reuse the definitions lets >=20 > "Arasan" >=20 >> make the base address configurable when the driver loads. >> >> This assumes we won't ever want to run both the emmc2 >=20 > "eMMC2" >=20 >> and arasan sdhci controller at the same time. >=20 > "Arasan SDHCI" Sure.. >=20 >> >> Signed-off-by: Jeremy Linton >> Reviewed-by: Andrei Warkentin >> --- >> .../Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c | 9 ++++- >> .../Bcm283x/Include/IndustryStandard/Bcm2836Sdio.h | 42 ++++++++++++-= --------- >> 2 files changed, 32 insertions(+), 19 deletions(-) >> >> diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHos= tDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c >> index 88e9126e35..0cb7e85b38 100644 >> --- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c >> +++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c >> @@ -16,6 +16,7 @@ STATIC CARD_DETECT_STATE mCardDetectState =3D CardDet= ectRequired; >> UINT32 LastExecutedCommand =3D (UINT32) -1; >> =20 >> STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol; >> +STATIC UINTN MMCHS_BASE; >=20 > When reading "MMCHS_BASE" I expect a definition, shouldn't > this be a variable name? Yes, that is clearer, given I ended up renaming the variable anyway to=20 drop the "1". >=20 >> =20 >> /** >> These SD commands are optional, according to the SD Spec >> @@ -763,7 +764,13 @@ MMCInitialize ( >> =20 >> DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCInitialize()\n")); >> =20 >> - if (!PcdGet32 (PcdSdIsArasan)) { >> + if (PcdGet32 (PcdSdIsArasan)) { >> + DEBUG ((DEBUG_INFO, "SD is routed to Arasan\n")); >> + MMCHS_BASE =3D MMCHS1_BASE; >> + } else if (RPI_MODEL =3D=3D 4) { >> + DEBUG ((DEBUG_INFO, "SD is routed to emmc2\n")); >> + MMCHS_BASE =3D MMCHS2_BASE; >> + } else { >> DEBUG ((DEBUG_INFO, "SD is not routed to Arasan\n")); >> return EFI_REQUEST_UNLOAD_IMAGE; >> } >> diff --git a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836S= dio.h b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Sdio.h >> index fd07b47170..e6892d36cf 100644 >> --- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Sdio.h >> +++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Sdio.h >> @@ -13,15 +13,18 @@ >> =20 >> // MMC/SD/SDIO1 register definitions. >> #define MMCHS1_OFFSET 0x00300000 >> +#define MMCHS2_OFFSET 0x00340000 >> #define MMCHS1_BASE (BCM2836_SOC_REGISTERS + MMCHS1_OFFSET) >> +#define MMCHS2_BASE (BCM2836_SOC_REGISTERS + MMCHS2_OFFSET) >> #define MMCHS1_LENGTH 0x00000100 >> +#define MMCHS2_LENGTH 0x00000100 >> =20 >> -#define MMCHS_BLK (MMCHS1_BASE + 0x4) >> +#define MMCHS_BLK (MMCHS_BASE + 0x4) >> #define BLEN_512BYTES (0x200UL << 0) >> =20 >> -#define MMCHS_ARG (MMCHS1_BASE + 0x8) >> +#define MMCHS_ARG (MMCHS_BASE + 0x8) >> =20 >> -#define MMCHS_CMD (MMCHS1_BASE + 0xC) >> +#define MMCHS_CMD (MMCHS_BASE + 0xC) >> #define BCE_ENABLE BIT1 >> #define DDIR_READ BIT4 >> #define DDIR_WRITE (0x0UL << 4) >> @@ -43,13 +46,13 @@ >> #define INDX(CMD_INDX) (TYPE(CMD_TYPE_NORMAL) | _INDX(CMD_INDX)= ) >> #define INDX_ABORT(CMD_INDX) (TYPE(CMD_TYPE_ABORT) | _INDX(CMD_INDX)) >> =20 >> -#define MMCHS_RSP10 (MMCHS1_BASE + 0x10) >> -#define MMCHS_RSP32 (MMCHS1_BASE + 0x14) >> -#define MMCHS_RSP54 (MMCHS1_BASE + 0x18) >> -#define MMCHS_RSP76 (MMCHS1_BASE + 0x1C) >> -#define MMCHS_DATA (MMCHS1_BASE + 0x20) >> +#define MMCHS_RSP10 (MMCHS_BASE + 0x10) >> +#define MMCHS_RSP32 (MMCHS_BASE + 0x14) >> +#define MMCHS_RSP54 (MMCHS_BASE + 0x18) >> +#define MMCHS_RSP76 (MMCHS_BASE + 0x1C) >> +#define MMCHS_DATA (MMCHS_BASE + 0x20) >> =20 >> -#define MMCHS_PRES_STATE (MMCHS1_BASE + 0x24) >> +#define MMCHS_PRES_STATE (MMCHS_BASE + 0x24) >> #define CMDI_MASK BIT0 >> #define CMDI_ALLOWED (0x0UL << 0) >> #define CMDI_NOT_ALLOWED BIT0 >> @@ -58,17 +61,19 @@ >> #define DATI_NOT_ALLOWED BIT1 >> #define WRITE_PROTECT_OFF BIT19 >> =20 >> -#define MMCHS_HCTL (MMCHS1_BASE + 0x28) >> +#define MMCHS_HCTL (MMCHS_BASE + 0x28) >> #define DTW_1_BIT (0x0UL << 1) >> #define DTW_4_BIT BIT1 >> #define SDBP_MASK BIT8 >> #define SDBP_OFF (0x0UL << 8) >> #define SDBP_ON BIT8 >> +#define SDVS_MASK (0x7UL << 9) >> #define SDVS_1_8_V (0x5UL << 9) >> #define SDVS_3_0_V (0x6UL << 9) >> +#define SDVS_3_3_V (0x7UL << 9) >> #define IWE BIT24 >> =20 >> -#define MMCHS_SYSCTL (MMCHS1_BASE + 0x2C) >> +#define MMCHS_SYSCTL (MMCHS_BASE + 0x2C) >> #define ICE BIT0 >> #define ICS_MASK BIT1 >> #define ICS BIT1 >> @@ -84,7 +89,7 @@ >> #define SRC BIT25 >> #define SRD BIT26 >> =20 >> -#define MMCHS_INT_STAT (MMCHS1_BASE + 0x30) >> +#define MMCHS_INT_STAT (MMCHS_BASE + 0x30) >> #define CC BIT0 >> #define TC BIT1 >> #define BWR BIT4 >> @@ -96,7 +101,7 @@ >> #define DCRC BIT21 >> #define DEB BIT22 >> =20 >> -#define MMCHS_IE (MMCHS1_BASE + 0x34) >> +#define MMCHS_IE (MMCHS_BASE + 0x34) >> #define CC_EN BIT0 >> #define TC_EN BIT1 >> #define BWR_EN BIT4 >> @@ -112,7 +117,7 @@ >> #define BADA_EN BIT29 >> #define ALL_EN 0xFFFFFFFF >> =20 >> -#define MMCHS_ISE (MMCHS1_BASE + 0x38) >> +#define MMCHS_ISE (MMCHS_BASE + 0x38) >> #define CC_SIGEN BIT0 >> #define TC_SIGEN BIT1 >> #define BWR_SIGEN BIT4 >> @@ -127,14 +132,15 @@ >> #define CERR_SIGEN BIT28 >> #define BADA_SIGEN BIT29 >> =20 >> -#define MMCHS_AC12 (MMCHS1_BASE + 0x3C) >> +#define MMCHS_AC12 (MMCHS_BASE + 0x3C) >> +#define MMCHS_HC2R (MMCHS_BASE + 0x3E) >> =20 >> -#define MMCHS_CAPA (MMCHS1_BASE + 0x40) >> +#define MMCHS_CAPA (MMCHS_BASE + 0x40) >> #define VS30 BIT25 >> #define VS18 BIT26 >> =20 >> -#define MMCHS_CUR_CAPA (MMCHS1_BASE + 0x48) >> -#define MMCHS_REV (MMCHS1_BASE + 0xFC) >> +#define MMCHS_CUR_CAPA (MMCHS_BASE + 0x48) >> +#define MMCHS_REV (MMCHS_BASE + 0xFC) >> =20 >> #define BLOCK_COUNT_SHIFT 16 >> #define RCA_SHIFT 16 >> >=20 >=20 >=20 >=20 >=20 >=20