From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-x233.google.com (mail-io0-x233.google.com [IPv6:2607:f8b0:4001:c06::233]) (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 821DE81C92 for ; Tue, 22 Nov 2016 00:58:23 -0800 (PST) Received: by mail-io0-x233.google.com with SMTP id j65so62221449iof.0 for ; Tue, 22 Nov 2016 00:58:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=EU7nJxtpFnA9Pu0vEmMzOzIIgjqBh2hE1zBaQ+2Nbcg=; b=SpfK6yGdnmltJ01HTs3/wSYOczUkvm35MYzQHfFw5sfjAbdODCUx4wudTv+4fD8N9C lD3Bz8kTyTjtvZ7OIYnbkRqtdPOvk8Rf/s/eoOAsEsfFZ4dMn2FmYtP5SNNcpyyJwqkl ZgSKvq3mzSg99YVif7mXIoU8wGmMsOW2asQZQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=EU7nJxtpFnA9Pu0vEmMzOzIIgjqBh2hE1zBaQ+2Nbcg=; b=jytvLbTdT8Mjo2nQkuDv26G+YSgvwgBZXvk+MGVHbdBW8SsdF/FiGiVdDhidkLQK9f bA9EBd4WH5N5+DBE229Ju9WS5dRtkQfk7In+vwrnLQ/N4+Ja65VAvJtEJkYvkUyxj6JN IdN6DDvMHuji5sXkKghPqQro9GW/yLISSX07DlE7vZYDdxjZlKjzktmQvueEZM0tdGv1 eM/WoCXoW2uKbZNS/RklB2SlR19JQc0SnbHa5zlsaKsAV7tk+BKxerO8nOKElCl5fjhw HRpMWnTKXkpf/cuUPfLCVJpGlz5mZlz6pVo8e8DUq0JqlCSTRg+iK/AuvRMe2DbtAohM yFcw== X-Gm-Message-State: AKaTC00t9mSFRD5Do5M89a5tERxP06tyO1cfisvYrweOwn4D3lNkfetEsdtibJdQ58Gy+oJ3XKne6jKj33xg84Kb X-Received: by 10.107.25.204 with SMTP id 195mr14674554ioz.138.1479805102692; Tue, 22 Nov 2016 00:58:22 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.59.147 with HTTP; Tue, 22 Nov 2016 00:58:22 -0800 (PST) In-Reply-To: <1479801028-3587-2-git-send-email-haojian.zhuang@linaro.org> References: <1479801028-3587-1-git-send-email-haojian.zhuang@linaro.org> <1479801028-3587-2-git-send-email-haojian.zhuang@linaro.org> From: Ard Biesheuvel Date: Tue, 22 Nov 2016 08:58:22 +0000 Message-ID: To: Haojian Zhuang Cc: Ryan Harkin , edk2-devel-01 , Leif Lindholm Subject: Re: [PATCH v6 1/4] MmcDxe: add interface to change io width and speed X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 Nov 2016 08:58:23 -0000 Content-Type: text/plain; charset=UTF-8 Hello Haojian, On 22 November 2016 at 07:50, Haojian Zhuang wrote: > By default, MMC is initialized with 1-bit mode and less than 400KHz bus > clock. It causes MMC working inefficiently. > > Add the interface to change the bus width and speed. > > Set io bus width on both MMC controller and EXTCSD. Otherwise, it may > cause unmatched failure case. And support more timing mode, high speed, > HS200 & HS400 mode. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Haojian Zhuang > Tested-by: Ryan Harkin > --- > EmbeddedPkg/Include/Protocol/MmcHost.h | 20 +++ > EmbeddedPkg/Universal/MmcDxe/Mmc.h | 5 + > EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 185 ++++++++++++++++++++++- > 3 files changed, 207 insertions(+), 3 deletions(-) > > diff --git a/EmbeddedPkg/Include/Protocol/MmcHost.h b/EmbeddedPkg/Include/Protocol/MmcHost.h > index 89f2e80..01c0bf4 100644 > --- a/EmbeddedPkg/Include/Protocol/MmcHost.h > +++ b/EmbeddedPkg/Include/Protocol/MmcHost.h > @@ -49,6 +49,7 @@ typedef UINT32 MMC_CMD; > #define MMC_CMD2 (MMC_INDX(2) | MMC_CMD_WAIT_RESPONSE | MMC_CMD_LONG_RESPONSE) > #define MMC_CMD3 (MMC_INDX(3) | MMC_CMD_WAIT_RESPONSE) > #define MMC_CMD5 (MMC_INDX(5) | MMC_CMD_WAIT_RESPONSE | MMC_CMD_NO_CRC_RESPONSE) > +#define MMC_CMD6 (MMC_INDX(6) | MMC_CMD_WAIT_RESPONSE) > #define MMC_CMD7 (MMC_INDX(7) | MMC_CMD_WAIT_RESPONSE) > #define MMC_CMD8 (MMC_INDX(8) | MMC_CMD_WAIT_RESPONSE) > #define MMC_CMD9 (MMC_INDX(9) | MMC_CMD_WAIT_RESPONSE | MMC_CMD_LONG_RESPONSE) > @@ -82,6 +83,16 @@ typedef enum _MMC_STATE { > MmcDisconnectState, > } MMC_STATE; > > +#define EMMCBACKWARD (0) > +#define EMMCHS26 (1 << 0) // High-Speed @26MHz at rated device voltages > +#define EMMCHS52 (1 << 1) // High-Speed @52MHz at rated device voltages > +#define EMMCHS52DDR1V8 (1 << 2) // High-Speed Dual Data Rate @52MHz 1.8V or 3V I/O > +#define EMMCHS52DDR1V2 (1 << 3) // High-Speed Dual Data Rate @52MHz 1.2V I/O > +#define EMMCHS200SDR1V8 (1 << 4) // HS200 Single Data Rate @200MHz 1.8V I/O > +#define EMMCHS200SDR1V2 (1 << 5) // HS200 Single Data Rate @200MHz 1.2V I/O > +#define EMMCHS400DDR1V8 (1 << 6) // HS400 Dual Data Rate @400MHz 1.8V I/O > +#define EMMCHS400DDR1V2 (1 << 7) // HS400 Dual Data Rate @400MHz 1.2V I/O > + > /// > /// Forward declaration for EFI_MMC_HOST_PROTOCOL > /// > @@ -131,6 +142,13 @@ typedef EFI_STATUS (EFIAPI *MMC_WRITEBLOCKDATA) ( > IN UINT32 *Buffer > ); > > +typedef EFI_STATUS (EFIAPI *MMC_SETIOS) ( > + IN EFI_MMC_HOST_PROTOCOL *This, > + IN UINT32 BusClockFreq, > + IN UINT32 BusWidth, > + IN UINT32 TimingMode > + ); > + > > struct _EFI_MMC_HOST_PROTOCOL { > > @@ -147,6 +165,8 @@ struct _EFI_MMC_HOST_PROTOCOL { > MMC_READBLOCKDATA ReadBlockData; > MMC_WRITEBLOCKDATA WriteBlockData; > > + MMC_SETIOS SetIos; > + > }; > > #define MMC_HOST_PROTOCOL_REVISION 0x00010001 // 1.1 If you add new methods to the protocol, you should increase this version number, and only call the new methods if the version number is high enough. You could add some macros to test for that if you like, e.g., #define MMC_HOST_HAS_SETIOS(Host) (Host->Revision >= MMC_HOST_PROTOCOL_REVISION && Host->SetIos != NULL) and use the macro instead of testing the method pointer for NULL. The reason is that new fields are not guaranteed to be NULL for drivers that do not implement the functionality, due to the binary ABI between modules in UEFI. It is perfectly legal to combine a binary build of the PL180 module with a source build of the MMC host driver, and if the binary PL180 is built against an older version, the SetIos field points into an adjacent allocation. Apologies for only pointing this out now, but it is really quite important that we do this correctly. > diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h b/EmbeddedPkg/Universal/MmcDxe/Mmc.h > index 67f449c..fd1fccf 100644 > --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h > +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h > @@ -55,6 +55,11 @@ > #define MMC_R0_STATE_TRAN 4 > #define MMC_R0_STATE_DATA 5 > > +#define EMMC_CMD6_ARG_ACCESS(x) (((x) & 0x3) << 24) > +#define EMMC_CMD6_ARG_INDEX(x) (((x) & 0xFF) << 16) > +#define EMMC_CMD6_ARG_VALUE(x) (((x) & 0xFF) << 8) > +#define EMMC_CMD6_ARG_CMD_SET(x) (((x) & 0x7) << 0) > + > typedef enum { > UNKNOWN_CARD, > MMC_CARD, //MMC card > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > index 378e438..90aed33 100644 > --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > @@ -25,11 +25,111 @@ typedef union { > #define EMMC_CARD_SIZE 512 > #define EMMC_ECSD_SIZE_OFFSET 53 > > +#define EXTCSD_BUS_WIDTH 183 > +#define EXTCSD_HS_TIMING 185 > + > +#define EMMC_TIMING_BACKWARD 0 > +#define EMMC_TIMING_HS 1 > +#define EMMC_TIMING_HS200 2 > +#define EMMC_TIMING_HS400 3 > + > +#define EMMC_BUS_WIDTH_1BIT 0 > +#define EMMC_BUS_WIDTH_4BIT 1 > +#define EMMC_BUS_WIDTH_8BIT 2 > +#define EMMC_BUS_WIDTH_DDR_4BIT 5 > +#define EMMC_BUS_WIDTH_DDR_8BIT 6 > + > +#define EMMC_SWITCH_ERROR (1 << 7) > + > +#define DEVICE_STATE(x) (((x) >> 9) & 0xf) > +typedef enum _EMMC_DEVICE_STATE { > + EMMC_IDLE_STATE = 0, > + EMMC_READY_STATE, > + EMMC_IDENT_STATE, > + EMMC_STBY_STATE, > + EMMC_TRAN_STATE, > + EMMC_DATA_STATE, > + EMMC_RCV_STATE, > + EMMC_PRG_STATE, > + EMMC_DIS_STATE, > + EMMC_BTST_STATE, > + EMMC_SLP_STATE > +} EMMC_DEVICE_STATE; > + > UINT32 mEmmcRcaCount = 0; > > STATIC > EFI_STATUS > EFIAPI > +EmmcGetDeviceState ( > + IN MMC_HOST_INSTANCE *MmcHostInstance, > + OUT EMMC_DEVICE_STATE *State > + ) > +{ > + EFI_MMC_HOST_PROTOCOL *Host; > + EFI_STATUS Status; > + UINT32 Data, RCA; > + > + if (State == NULL) { > + return EFI_INVALID_PARAMETER; > + } > + > + Host = MmcHostInstance->MmcHost; > + RCA = MmcHostInstance->CardInfo.RCA << RCA_SHIFT_OFFSET; > + Status = Host->SendCommand (Host, MMC_CMD13, RCA); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to get card status, Status=%r.\n", Status)); > + return Status; > + } > + Status = Host->ReceiveResponse (Host, MMC_RESPONSE_TYPE_R1, &Data); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to get response of CMD13, Status=%r.\n", Status)); > + return Status; > + } > + if (Data & EMMC_SWITCH_ERROR) { > + DEBUG ((EFI_D_ERROR, "EmmcGetDeviceState(): Failed to switch expected mode, Status=%r.\n", Status)); > + return EFI_DEVICE_ERROR; > + } > + *State = DEVICE_STATE(Data); > + return EFI_SUCCESS; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +EmmcSetEXTCSD ( > + IN MMC_HOST_INSTANCE *MmcHostInstance, > + UINT32 ExtCmdIndex, > + UINT32 Value > + ) > +{ > + EFI_MMC_HOST_PROTOCOL *Host; > + EMMC_DEVICE_STATE State; > + EFI_STATUS Status; > + UINT32 Argument; > + > + Host = MmcHostInstance->MmcHost; > + Argument = EMMC_CMD6_ARG_ACCESS(3) | EMMC_CMD6_ARG_INDEX(ExtCmdIndex) | > + EMMC_CMD6_ARG_VALUE(Value) | EMMC_CMD6_ARG_CMD_SET(1); > + Status = Host->SendCommand (Host, MMC_CMD6, Argument); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "EmmcSetEXTCSD(): Failed to send CMD6, Status=%r.\n", Status)); > + return Status; > + } > + // Make sure device exiting prog mode > + do { > + Status = EmmcGetDeviceState (MmcHostInstance, &State); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "EmmcSetEXTCSD(): Failed to get device state, Status=%r.\n", Status)); > + return Status; > + } > + } while (State == EMMC_PRG_STATE); > + return EFI_SUCCESS; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > EmmcIdentificationMode ( > IN MMC_HOST_INSTANCE *MmcHostInstance, > IN OCR_RESPONSE Response > @@ -38,6 +138,7 @@ EmmcIdentificationMode ( > EFI_MMC_HOST_PROTOCOL *Host; > EFI_BLOCK_IO_MEDIA *Media; > EFI_STATUS Status; > + EMMC_DEVICE_STATE State; > UINT32 RCA; > > Host = MmcHostInstance->MmcHost; > @@ -84,6 +185,22 @@ EmmcIdentificationMode ( > DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): Card selection error, Status=%r.\n", Status)); > } > > + if (Host->SetIos) { > + // Set 1-bit bus width > + Status = Host->SetIos (Host, 0, 1, EMMCBACKWARD); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): Set 1-bit bus width error, Status=%r.\n", Status)); > + return Status; > + } > + > + // Set 1-bit bus width for EXTCSD > + Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, EMMC_BUS_WIDTH_1BIT); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): Set extcsd bus width error, Status=%r.\n", Status)); > + return Status; > + } > + } > + > // Fetch ECSD > Status = Host->SendCommand (Host, MMC_CMD8, RCA); > if (EFI_ERROR (Status)) { > @@ -96,6 +213,15 @@ EmmcIdentificationMode ( > return Status; > } > > + // Make sure device exiting data mode > + do { > + Status = EmmcGetDeviceState (MmcHostInstance, &State); > + if (EFI_ERROR (Status)) { > + DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): Failed to get device state, Status=%r.\n", Status)); > + return Status; > + } > + } while (State == EMMC_DATA_STATE); > + > // Set up media > Media->BlockSize = EMMC_CARD_SIZE; // 512-byte support is mandatory for eMMC cards > Media->MediaId = MmcHostInstance->CardInfo.CIDData.PSN; > @@ -113,6 +239,57 @@ EmmcIdentificationMode ( > > STATIC > EFI_STATUS > +InitializeEmmcDevice ( > + IN MMC_HOST_INSTANCE *MmcHostInstance > + ) > +{ > + EFI_MMC_HOST_PROTOCOL *Host; > + EFI_STATUS Status = EFI_SUCCESS; > + ECSD *ECSDData; > + UINT32 BusClockFreq, Idx; > + UINT32 TimingMode[4] = {EMMCHS52DDR1V2, EMMCHS52DDR1V8, EMMCHS52, EMMCHS26}; > + > + Host = MmcHostInstance->MmcHost; > + ECSDData = &MmcHostInstance->CardInfo.ECSDData; > + if (ECSDData->DEVICE_TYPE == EMMCBACKWARD) > + return EFI_SUCCESS; > + > + if (!Host->SetIos) { > + return EFI_SUCCESS; > + } > + Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_HS_TIMING, EMMC_TIMING_HS); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "InitializeEmmcDevice(): Failed to switch high speed mode, Status:%r.\n", Status)); > + return Status; > + } > + > + for (Idx = 0; Idx < 4; Idx++) { > + switch (TimingMode[Idx]) { > + case EMMCHS52DDR1V2: > + case EMMCHS52DDR1V8: > + case EMMCHS52: > + BusClockFreq = 52000000; > + break; > + case EMMCHS26: > + BusClockFreq = 26000000; > + break; > + default: > + return EFI_UNSUPPORTED; > + } > + Status = Host->SetIos (Host, BusClockFreq, 8, TimingMode[Idx]); > + if (!EFI_ERROR (Status)) { > + Status = EmmcSetEXTCSD (MmcHostInstance, EXTCSD_BUS_WIDTH, EMMC_BUS_WIDTH_DDR_8BIT); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "InitializeEmmcDevice(): Failed to set EXTCSD bus width, Status:%r\n", Status)); > + } > + return Status; > + } > + } > + return Status; > +} > + > +STATIC > +EFI_STATUS > InitializeSdMmcDevice ( > IN MMC_HOST_INSTANCE *MmcHostInstance > ) > @@ -431,9 +608,11 @@ InitializeMmcDevice ( > > if (MmcHostInstance->CardInfo.CardType != EMMC_CARD) { > Status = InitializeSdMmcDevice (MmcHostInstance); > - if (EFI_ERROR (Status)) { > - return Status; > - } > + } else { > + Status = InitializeEmmcDevice (MmcHostInstance); > + } > + if (EFI_ERROR (Status)) { > + return Status; > } > > // Set Block Length > -- > 2.7.4 >