Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-05 Thread Ard Biesheuvel 
On 5 December 2017 at 13:09, Wu, Hao A <hao.a...@intel.com> wrote:
>> -Original Message-
>> From: Ni, Ruiyu
>> Sent: Tuesday, December 05, 2017 6:35 PM
>> To: Ard Biesheuvel; Wu, Hao A
>> Cc: edk2-devel@lists.01.org; Leif Lindholm; Tian, Feng; Zeng, Star
>> Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override
>> protocol
>>
>> On 12/5/2017 6:24 PM, Ard Biesheuvel wrote:
>> > On 5 December 2017 at 10:12, Ni, Ruiyu <ruiyu...@intel.com> wrote:
>> >> Some comments re the detailed interfaces embedded in below.
>> >>
>> >> On 11/30/2017 6:11 PM, 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 <ard.biesheu...@linaro.org>
>> >>> ---
>> >>>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 ..5a5c393896f4
>> >>> --- /dev/null
>> >>> +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
>> >>> @@ -0,0 +1,103 @@
>> >>> +/** @file
>> >>> +  Protocol to describe overrides required to support non-standard SDHCI
>> >>> +  implementations
>>  >>> ...
>> >>> +  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
>> >>> +  IN  EFI_HANDLE  ControllerHandle,
>> >>> +  IN  UINT8   Slot,
>> >>> +  IN  OUT UINT64  *SdMmcHcSlotCapability
>> >>> +  );
>> >>
>> >>
>> >> Is this API too specific?
>> >> Besides SlotCapability, is there any other attributes that may need
>> >> override as well?
>> >>
>> >
>> > The capability structure is the root data structure that describes the
>> > SD/MMC host controller. Which other data structures did you have in
>> > mind?
>> >
>>
>> I do not know either.
>> Hao, any comments?
>
> The service is overriding the 'Capability' field of the private data
> structure within the HC driver. It's the value read from the 'CAP'
> register of the SD/MMC HC.
>
> After a glance of the other fields in 'SD_MMC_HC_PRIVATE_DATA', maybe the
> 'MaxCurrent' field is another candidate can be overriden. However, I am
> not sure about this.
>

In my experience, trying to cover every imaginable use case upfront
usually fails. That is why I used enum based hooks, which are easily
expanded later if we need to. If we need to override maxcurrent later
on, we can just add it.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-05 Thread Wu, Hao A 
> -Original Message-
> From: Ni, Ruiyu
> Sent: Tuesday, December 05, 2017 6:35 PM
> To: Ard Biesheuvel; Wu, Hao A
> Cc: edk2-devel@lists.01.org; Leif Lindholm; Tian, Feng; Zeng, Star
> Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override
> protocol
> 
> On 12/5/2017 6:24 PM, Ard Biesheuvel wrote:
> > On 5 December 2017 at 10:12, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> >> Some comments re the detailed interfaces embedded in below.
> >>
> >> On 11/30/2017 6:11 PM, 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 <ard.biesheu...@linaro.org>
> >>> ---
> >>>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 ..5a5c393896f4
> >>> --- /dev/null
> >>> +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> >>> @@ -0,0 +1,103 @@
> >>> +/** @file
> >>> +  Protocol to describe overrides required to support non-standard SDHCI
> >>> +  implementations
>  >>> ...
> >>> +  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
> >>> +  IN  EFI_HANDLE  ControllerHandle,
> >>> +  IN  UINT8   Slot,
> >>> +  IN  OUT UINT64  *SdMmcHcSlotCapability
> >>> +  );
> >>
> >>
> >> Is this API too specific?
> >> Besides SlotCapability, is there any other attributes that may need
> >> override as well?
> >>
> >
> > The capability structure is the root data structure that describes the
> > SD/MMC host controller. Which other data structures did you have in
> > mind?
> >
> 
> I do not know either.
> Hao, any comments?

The service is overriding the 'Capability' field of the private data
structure within the HC driver. It's the value read from the 'CAP'
register of the SD/MMC HC.

After a glance of the other fields in 'SD_MMC_HC_PRIVATE_DATA', maybe the
'MaxCurrent' field is another candidate can be overriden. However, I am
not sure about this.

Best Regards,
Hao Wu

> 
> --
> Thanks,
> Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-05 Thread Ni, Ruiyu 

On 12/5/2017 6:24 PM, Ard Biesheuvel wrote:

On 5 December 2017 at 10:12, Ni, Ruiyu  wrote:

Some comments re the detailed interfaces embedded in below.

On 11/30/2017 6:11 PM, 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 ..5a5c393896f4
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -0,0 +1,103 @@
+/** @file
+  Protocol to describe overrides required to support non-standard SDHCI
+  implementations

>>> ...

+  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
+  IN  EFI_HANDLE  ControllerHandle,
+  IN  UINT8   Slot,
+  IN  OUT UINT64  *SdMmcHcSlotCapability
+  );



Is this API too specific?
Besides SlotCapability, is there any other attributes that may need
override as well?



The capability structure is the root data structure that describes the
SD/MMC host controller. Which other data structures did you have in
mind?



I do not know either.
Hao, any comments?

--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-05 Thread Zeng, Star 
BTW, I prefer all the typedef and structure/field names have EDKII_ prefix, for 
example MdeModulePkg\Include\Protocol\IoMmu.h and 
MdeModulePkg\Include\Ppi\SdMmcHostController.h.

Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Tuesday, December 5, 2017 6:25 PM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: Wu, Hao A <hao.a...@intel.com>; edk2-devel@lists.01.org; Leif Lindholm 
<leif.lindh...@linaro.org>; Tian, Feng <feng.t...@intel.com>; Zeng, Star 
<star.z...@intel.com>
Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override 
protocol

On 5 December 2017 at 10:12, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> Some comments re the detailed interfaces embedded in below.
>
> On 11/30/2017 6:11 PM, 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 <ard.biesheu...@linaro.org>
>> ---
>>   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 ..5a5c393896f4
>> --- /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_VERSION0x1
>> +
>> +typedef struct _SD_MMC_OVERRIDE SD_MMC_OVERRIDE;
>> +
>> +typedef enum {
>> +  SD_MMC_OVERRIDE_RESET_PRE_HOOK,
>> +  SD_MMC_OVERRIDE_RESET_POST_HOOK,
>> +  SD_MMC_OVERRIDE_INIT_HOST_PRE_HOOK,
>> +  SD_MMC_OVERRIDE_INIT_HOST_POST_HOOK,
>
>
> How about using name like "SdMmcResetPre"?

Sure

> Do "ResetPre"/"ResetPost" / "InitHostPre" / "InitHostPost" cover all 
> potential check points?
>

Perhaps not, but it is hard to predict the future :-) This is why I added the 
version field. (I should also update the second patch to reject the protocol if 
its version is higher than
EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION)

>> +} SD_MMC_OVERRIDE_HOOK;
>
> How about use "SD_MMC_PHASE_TYPE"?
>

Sure.

>
>> +
>> +/**
>> +
>> +  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.
>>

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-05 Thread Ni, Ruiyu 

Some comments re the detailed interfaces embedded in below.

On 11/30/2017 6:11 PM, 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 ..5a5c393896f4
--- /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_VERSION0x1
+
+typedef struct _SD_MMC_OVERRIDE SD_MMC_OVERRIDE;
+
+typedef enum {
+  SD_MMC_OVERRIDE_RESET_PRE_HOOK,
+  SD_MMC_OVERRIDE_RESET_POST_HOOK,
+  SD_MMC_OVERRIDE_INIT_HOST_PRE_HOOK,
+  SD_MMC_OVERRIDE_INIT_HOST_POST_HOOK,


How about using name like "SdMmcResetPre"?
Do "ResetPre"/"ResetPost" / "InitHostPre" / "InitHostPost" cover
all potential check points?


+} SD_MMC_OVERRIDE_HOOK;

How about use "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 * SD_MMC_OVERRIDE_CAPABILITY) (


How about "SD_MMC_CAPABILITY"?


+  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
+  IN  EFI_HANDLE  ControllerHandle,
+  IN  UINT8   Slot,
+  IN  OUT UINT64  *SdMmcHcSlotCapability
+  );


Is this API too specific?
Besides SlotCapability, is there any other attributes that may need
override as well?


+
+/**
+
+  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]  HookType  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 * SD_MMC_OVERRIDE_INVOKE_HOOK) (


How about "SD_MMC_NOTIFY_PHASE"?


+  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
+  IN  EFI_HANDLE  ControllerHandle,
+  IN  UINT8   Slot,
+  IN  SD_MMC_OVERRIDE_HOOKHookType
+  );
+
+struct _SD_MMC_OVERRIDE {
+  //
+  // Protocol version of this implementation
+  //
+  UINTN   Version;
+  //
+  // Callback to override SD/MMC host controller capability bits
+  //
+  SD_MMC_OVERRIDE_CAPABILITY  OverrideCapability;How about

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-05 Thread Ard Biesheuvel 
On 5 December 2017 at 08:50, Zeng, Star  wrote:
> Regardless the protocol name SdMmcOverride/SdMmcPlatform, so you mean the 
> protocol should be produced by a DXE driver, but not a UEFI driver, right? I 
> saw the example at 
> https://lists.01.org/pipermail/edk2-devel/2017-November/017672.html shared by 
> Ard installs the protocol in PlatformDxe that is a DXE driver.
>
> And you mean the SD/MMC host controller driver code should use 
> LocateProtocol, but not HandleProtocol to find out the protocol instance, 
> right?
>

I think this makes sense, yes. I will respin the second patch accordingly.

-- 
Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-05 Thread Zeng, Star 
Regardless the protocol name SdMmcOverride/SdMmcPlatform, so you mean the 
protocol should be produced by a DXE driver, but not a UEFI driver, right? I 
saw the example at 
https://lists.01.org/pipermail/edk2-devel/2017-November/017672.html shared by 
Ard installs the protocol in PlatformDxe that is a DXE driver.

And you mean the SD/MMC host controller driver code should use LocateProtocol, 
but not HandleProtocol to find out the protocol instance, right?


Thanks,
Star
-Original Message-
From: Ni, Ruiyu 
Sent: Tuesday, December 5, 2017 4:37 PM
To: Zeng, Star <star.z...@intel.com>; Ard Biesheuvel 
<ard.biesheu...@linaro.org>; edk2-devel@lists.01.org
Cc: Wu, Hao A <hao.a...@intel.com>; Kinney, Michael D 
<michael.d.kin...@intel.com>; Tian, Feng <feng.t...@intel.com>; 
leif.lindh...@linaro.org
Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override 
protocol

On 12/5/2017 3:20 PM, Zeng, Star wrote:
> If making this protocol a platform level singleton instance, is it so hard to 
> define the interfaces and parameters since different controllers may need 
> different hook points and parameters?
> 
> Thanks,
> Star
> -Original Message-
> From: Ni, Ruiyu
> Sent: Tuesday, December 5, 2017 3:09 PM
> To: Ard Biesheuvel <ard.biesheu...@linaro.org>; 
> edk2-devel@lists.01.org
> Cc: "hao.a...@intel.com; Kinney.d.michael"@intel.com; Tian, Feng 
> <feng.t...@intel.com>; Zeng, Star <star.z...@intel.com>; 
> leif.lindh...@linaro.org
> Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC 
> override protocol
> 
> On 11/30/2017 6:11 PM, 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 <ard.biesheu...@linaro.org>
>> ---
>>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 ..5a5c393896f4
>> --- /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_VERSION0x1
>> +
>> +typedef struct _SD_MMC_OVERRIDE SD_MMC_OVERRIDE;
>> +
>> +typedef enum {
>> +  SD_MMC_OVERRIDE_RESET_PRE_HOOK,
>> +  SD_MMC_OVERRIDE_RESET_POST_HOOK,
>> +  SD_MMC_OVERRIDE_INIT_HOST_PRE_HOOK,
>> +  SD_MMC_OVERRIDE_INIT_HOST_POST_HOOK,
>> +} SD_MMC_OVERRIDE_HOOK;
>> +
>> +/**
>> +
>> +  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.
>> +  @para

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-05 Thread Ni, Ruiyu 

On 12/5/2017 3:20 PM, Zeng, Star wrote:

If making this protocol a platform level singleton instance, is it so hard to 
define the interfaces and parameters since different controllers may need 
different hook points and parameters?

Thanks,
Star
-Original Message-
From: Ni, Ruiyu
Sent: Tuesday, December 5, 2017 3:09 PM
To: Ard Biesheuvel <ard.biesheu...@linaro.org>; edk2-devel@lists.01.org
Cc: "hao.a...@intel.com; Kinney.d.michael"@intel.com; Tian, Feng 
<feng.t...@intel.com>; Zeng, Star <star.z...@intel.com>; leif.lindh...@linaro.org
Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override 
protocol

On 11/30/2017 6:11 PM, 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 <ard.biesheu...@linaro.org>
---
   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 ..5a5c393896f4
--- /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_VERSION0x1
+
+typedef struct _SD_MMC_OVERRIDE SD_MMC_OVERRIDE;
+
+typedef enum {
+  SD_MMC_OVERRIDE_RESET_PRE_HOOK,
+  SD_MMC_OVERRIDE_RESET_POST_HOOK,
+  SD_MMC_OVERRIDE_INIT_HOST_PRE_HOOK,
+  SD_MMC_OVERRIDE_INIT_HOST_POST_HOOK,
+} SD_MMC_OVERRIDE_HOOK;
+
+/**
+
+  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 * SD_MMC_OVERRIDE_CAPABILITY) (
+  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
+  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]  HookType  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 * SD_MMC_OVERRIDE_INVOKE_HOOK) (
+  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
+  IN  EFI_HANDLE  ControllerHandle,
+  IN  UINT8   Slot,
+  IN  SD_MMC_OVERRIDE_HOOKHookType
+  );
+
+struct _SD

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-04 Thread Zeng, Star 
If making this protocol a platform level singleton instance, is it so hard to 
define the interfaces and parameters since different controllers may need 
different hook points and parameters?

Thanks,
Star
-Original Message-
From: Ni, Ruiyu 
Sent: Tuesday, December 5, 2017 3:09 PM
To: Ard Biesheuvel <ard.biesheu...@linaro.org>; edk2-devel@lists.01.org
Cc: "hao.a...@intel.com; Kinney.d.michael"@intel.com; Tian, Feng 
<feng.t...@intel.com>; Zeng, Star <star.z...@intel.com>; 
leif.lindh...@linaro.org
Subject: Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override 
protocol

On 11/30/2017 6:11 PM, 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 <ard.biesheu...@linaro.org>
> ---
>   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 ..5a5c393896f4
> --- /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_VERSION0x1
> +
> +typedef struct _SD_MMC_OVERRIDE SD_MMC_OVERRIDE;
> +
> +typedef enum {
> +  SD_MMC_OVERRIDE_RESET_PRE_HOOK,
> +  SD_MMC_OVERRIDE_RESET_POST_HOOK,
> +  SD_MMC_OVERRIDE_INIT_HOST_PRE_HOOK,
> +  SD_MMC_OVERRIDE_INIT_HOST_POST_HOOK,
> +} SD_MMC_OVERRIDE_HOOK;
> +
> +/**
> +
> +  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 * SD_MMC_OVERRIDE_CAPABILITY) (
> +  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
> +  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]  HookType  The type of operation and whether the
> +hook is invoked right before (pre) or
> +right after (post)
> +
> +  @retval EFI_SUCCESS

Re: [edk2] [PATCH v2 1/2] MdeModulePkg: introduce SD/MMC override protocol

2017-12-04 Thread Ni, Ruiyu 

On 11/30/2017 6:11 PM, 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 ..5a5c393896f4
--- /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_VERSION0x1
+
+typedef struct _SD_MMC_OVERRIDE SD_MMC_OVERRIDE;
+
+typedef enum {
+  SD_MMC_OVERRIDE_RESET_PRE_HOOK,
+  SD_MMC_OVERRIDE_RESET_POST_HOOK,
+  SD_MMC_OVERRIDE_INIT_HOST_PRE_HOOK,
+  SD_MMC_OVERRIDE_INIT_HOST_POST_HOOK,
+} SD_MMC_OVERRIDE_HOOK;
+
+/**
+
+  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 * SD_MMC_OVERRIDE_CAPABILITY) (
+  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
+  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]  HookType  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 * SD_MMC_OVERRIDE_INVOKE_HOOK) (
+  IN  EFI_SD_MMC_PASS_THRU_PROTOCOL   *PassThru,
+  IN  EFI_HANDLE  ControllerHandle,
+  IN  UINT8   Slot,
+  IN  SD_MMC_OVERRIDE_HOOKHookType
+  );
+
+struct _SD_MMC_OVERRIDE {
+  //
+  // Protocol version of this implementation
+  //
+  UINTN   Version;
+  //
+  // Callback to override SD/MMC host controller capability bits
+  //
+  SD_MMC_OVERRIDE_CAPABILITY  OverrideCapability;
+  //
+  // Callback to invoke SD/MMC override hooks
+  //
+  SD_MMC_OVERRIDE_INVOKE_HOOK InvokeHook;
+};
+
+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