From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.65; helo=mga03.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7966F220FB323 for ; Tue, 5 Dec 2017 19:37:45 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Dec 2017 19:42:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,366,1508828400"; d="scan'208";a="156427060" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.14]) ([10.239.9.14]) by orsmga004.jf.intel.com with ESMTP; 05 Dec 2017 19:42:15 -0800 To: "Zeng, Star" , Ard Biesheuvel , "edk2-devel@lists.01.org" Cc: "leif.lindholm@linaro.org" , "Kinney, Michael D" , "Tian, Feng" , "Wu, Hao A" References: <20171205180152.15758-1-ard.biesheuvel@linaro.org> <20171205180152.15758-2-ard.biesheuvel@linaro.org> <4db018d9-00a5-956f-9326-0184c5bcf424@Intel.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B9BFFF8@shsmsx102.ccr.corp.intel.com> From: "Ni, Ruiyu" Message-ID: Date: Wed, 6 Dec 2017 11:42:15 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: <0C09AFA07DD0434D9E2A0C6AEB0483103B9BFFF8@shsmsx102.ccr.corp.intel.com> Subject: Re: [PATCH v3 1/2] MdeModulePkg: introduce SD/MMC override protocol X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Dec 2017 03:37:46 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/6/2017 11:25 AM, Zeng, Star wrote: > It should be not needed for Capability, but may be needed for NotifyPhase. > Hao can explain more. When the SdMmcPassthru protocol is installed by gBS->InstallProtocol(), that's the indication of readiness of this protocol. Before that, it's not guaranteed that every interface of SdMmcPassthru is safe to call, e.g.: GetNextSlot or ResetDevice may not work because the device enumeration depends on the SdMmcOverride. Even for PassThru interface, is might be possible that some private data needed by PassThru to work hasn't been initialized, or future implementation might choose to defer some private data initialization to just before protocol installation. I think after Hao's evaluation, if PassThru is only needed and must-use interface for some usage case, we could just pass the PassThru function pointer to SdMmcOverride. > > > Thanks, > Star > -----Original Message----- > From: Ni, Ruiyu > Sent: Wednesday, December 6, 2017 11:22 AM > To: Ard Biesheuvel ; edk2-devel@lists.01.org > Cc: leif.lindholm@linaro.org; Kinney, Michael D ; Zeng, Star ; Tian, Feng ; Wu, Hao A > Subject: Re: [PATCH v3 1/2] MdeModulePkg: introduce SD/MMC override protocol > > On 12/6/2017 2:01 AM, Ard Biesheuvel wrote: >> Many ARM based SoCs have integrated SDHCI controllers, and often, >> these implementations deviate in subtle ways from the pertinent >> specifications. On the one hand, these deviations are quite easy to >> work around, but on the other hand, having a collection of SoC >> specific workarounds in the generic driver stack is undesirable. >> >> So let's introduce an optional SD/MMC override protocol that we can >> invoke at the appropriate moments in the device initialization. >> That way, the workaround itself remains platform specific, but we can >> still use the generic driver stack on such platforms. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >> MdeModulePkg/Include/Protocol/SdMmcOverride.h | 103 ++++++++++++++++++++ >> MdeModulePkg/MdeModulePkg.dec | 3 + >> 2 files changed, 106 insertions(+) >> >> diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h >> b/MdeModulePkg/Include/Protocol/SdMmcOverride.h >> new file mode 100644 >> index 000000000000..af57988f5625 >> --- /dev/null >> +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h >> @@ -0,0 +1,103 @@ >> +/** @file >> + Protocol to describe overrides required to support non-standard >> +SDHCI >> + implementations >> + >> + Copyright (c) 2017, Linaro, Ltd. 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 __SD_MMC_OVERRIDE_H__ >> +#define __SD_MMC_OVERRIDE_H__ >> + >> +#include >> + >> +#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_GUID \ >> + { 0xeaf9e3c1, 0xc9cd, 0x46db, { 0xa5, 0xe5, 0x5a, 0x12, 0x4c, 0x83, >> +0x23, 0x23 } } >> + >> +#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION 0x1 >> + >> +typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE; >> + >> +typedef enum { >> + EdkiiSdMmcResetPre, >> + EdkiiSdMmcResetPost, >> + EdkiiSdMmcInitHostPre, >> + EdkiiSdMmcInitHostPost, >> +} EDKII_SD_MMC_PHASE_TYPE; >> + >> +/** >> + >> + Override function for SDHCI capability bits >> + >> + @param[in] PassThru A pointer to the >> + EFI_SD_MMC_PASS_THRU_PROTOCOL instance. >> + @param[in] ControllerHandle The EFI_HANDLE of the controller. >> + @param[in] Slot The 0 based slot index. >> + @param[in,out] SdMmcHcSlotCapability The SDHCI capability structure. >> + >> + @retval EFI_SUCCESS The override function completed successfully. >> + @retval EFI_NOT_FOUND The specified controller or slot does not exist. >> + @retval EFI_INVALID_PARAMETER SdMmcHcSlotCapability is NULL >> + >> +**/ >> +typedef >> +EFI_STATUS >> +(EFIAPI * EDKII_SD_MMC_CAPABILITY) ( >> + IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, > By looking a bit deeper, I get confused about this parameter. > SdMmcOverride protocol is supposed to be used by SdMmcHostController driver to produce SdMmcPassthru protocol. > But how does SdMmcOverride uses the not-yet-produced SdMmcPassthru protocol? It's like a chicken-egg problem. > >> + IN EFI_HANDLE ControllerHandle, >> + IN UINT8 Slot, >> + IN OUT UINT64 *SdMmcHcSlotCapability >> + ); >> + >> +/** >> + >> + Override function for SDHCI controller operations >> + >> + @param[in] PassThru A pointer to the >> + EFI_SD_MMC_PASS_THRU_PROTOCOL instance. >> + @param[in] ControllerHandle The EFI_HANDLE of the controller. >> + @param[in] Slot The 0 based slot index. >> + @param[in,out] PhaseType The type of operation and whether the >> + hook is invoked right before (pre) or >> + right after (post) >> + >> + @retval EFI_SUCCESS The override function completed successfully. >> + @retval EFI_NOT_FOUND The specified controller or slot does not exist. >> + @retval EFI_INVALID_PARAMETER HookType is invalid >> + >> +**/ >> +typedef >> +EFI_STATUS >> +(EFIAPI * EDKII_SD_MMC_NOTIFY_PHASE) ( >> + IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, >> + IN EFI_HANDLE ControllerHandle, >> + IN UINT8 Slot, >> + IN EDKII_SD_MMC_PHASE_TYPE PhaseType >> + ); >> + >> +struct _EDKII_SD_MMC_OVERRIDE { >> + // >> + // Protocol version of this implementation >> + // >> + UINTN Version; >> + // >> + // Callback to override SD/MMC host controller capability bits >> + // >> + EDKII_SD_MMC_CAPABILITY Capability; >> + // >> + // Callback to invoke SD/MMC override hooks >> + // >> + EDKII_SD_MMC_NOTIFY_PHASE NotifyPhase; >> +}; >> + >> +extern EFI_GUID gEdkiiSdMmcOverrideProtocolGuid; >> + >> +#endif >> diff --git a/MdeModulePkg/MdeModulePkg.dec >> b/MdeModulePkg/MdeModulePkg.dec index 856d67aceb21..64ceea029f94 >> 100644 >> --- a/MdeModulePkg/MdeModulePkg.dec >> +++ b/MdeModulePkg/MdeModulePkg.dec >> @@ -562,6 +562,9 @@ [Protocols] >> ## Include/Protocol/SmmMemoryAttribute.h >> gEdkiiSmmMemoryAttributeProtocolGuid = { 0x69b792ea, 0x39ce, >> 0x402d, { 0xa2, 0xa6, 0xf7, 0x21, 0xde, 0x35, 0x1d, 0xfe } } >> >> + ## Include/Protocol/SdMmcOverride.h >> + gEdkiiSdMmcOverrideProtocolGuid = { 0xeaf9e3c1, 0xc9cd, 0x46db, { >> + 0xa5, 0xe5, 0x5a, 0x12, 0x4c, 0x83, 0x23, 0x23 } } >> + >> # >> # [Error.gEfiMdeModulePkgTokenSpaceGuid] >> # 0x80000001 | Invalid value provided. >> > > > -- > Thanks, > Ray > -- Thanks, Ray