Invoke the newly introduced SD/MMC override protocol to override
the capabilities register after reading it from the device registers,
and to call the pre/post host init and reset hooks at the appropriate
times.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 134 +++++++++++++++++++-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 1 +
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf | 2 +
3 files changed, 133 insertions(+), 4 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index 0be8828abfcc..f1ea78de809e 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -17,6 +17,8 @@
#include "SdMmcPciHcDxe.h"
+STATIC EDKII_SD_MMC_OVERRIDE *mOverride;
+
//
// Driver Global Variables
//
@@ -214,6 +216,104 @@ Done:
}
/**
+ Execute a SD/MMC host controller reset
+
+ @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
+ @param[in] Slot The slot number of the host controller to reset.
+**/
+STATIC
+EFI_STATUS
+SdMmcPciHcResetHost (
+ IN SD_MMC_HC_PRIVATE_DATA *Private,
+ IN UINT8 Slot
+ )
+{
+ EFI_STATUS Status;
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ &Private->PassThru,
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcResetPre);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN,
+ "%a: SD/MMC pre reset notifier callback failed - %r\n",
+ __FUNCTION__, Status));
+ return Status;
+ }
+ }
+
+ Status = SdMmcHcReset (Private->PciIo, Slot);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ &Private->PassThru,
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcResetPost);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN,
+ "%a: SD/MMC post reset notifier callback failed - %r\n",
+ __FUNCTION__, Status));
+ }
+ }
+ return Status;
+}
+
+/**
+ Initialize a SD/MMC host controller
+
+ @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
+ @param[in] Slot The slot number of the host controller to initialize.
+**/
+STATIC
+EFI_STATUS
+SdMmcPciHcInitHost (
+ IN SD_MMC_HC_PRIVATE_DATA *Private,
+ IN UINT8 Slot
+ )
+{
+ EFI_STATUS Status;
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ &Private->PassThru,
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcInitHostPre);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN,
+ "%a: SD/MMC pre init notifier callback failed - %r\n",
+ __FUNCTION__, Status));
+ return Status;
+ }
+ }
+
+ Status = SdMmcHcInitHost (Private->PciIo, Slot, Private->Capability[Slot]);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ &Private->PassThru,
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcInitHostPost);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN,
+ "%a: SD/MMC post init notifier callback failed - %r\n",
+ __FUNCTION__, Status));
+ }
+ }
+ return Status;
+}
+
+/**
Sd removable device enumeration callback function when the timer event is signaled.
@param[in] Event The Event this notify function registered to.
@@ -281,14 +381,14 @@ SdMmcPciHcEnumerateDevice (
//
// Reset the specified slot of the SD/MMC Pci Host Controller
//
- Status = SdMmcHcReset (Private->PciIo, Slot);
+ Status = SdMmcPciHcResetHost (Private, Slot);
if (EFI_ERROR (Status)) {
continue;
}
//
// Reinitialize slot and restart identification process for the new attached device
//
- Status = SdMmcHcInitHost (Private->PciIo, Slot, Private->Capability[Slot]);
+ Status = SdMmcPciHcInitHost (Private, Slot);
if (EFI_ERROR (Status)) {
continue;
}
@@ -601,6 +701,20 @@ SdMmcPciHcDriverBindingStart (
goto Done;
}
+ //
+ // Attempt to locate the singleton instance of the SD/MMC override protocol,
+ // which implements platform specific workarounds for non-standard SDHCI
+ // implementations.
+ //
+ if (mOverride == NULL) {
+ Status = gBS->HandleProtocol (Controller, &gEdkiiSdMmcOverrideProtocolGuid,
+ (VOID **)&mOverride);
+ if (!EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a: found SD/MMC override protocol\n",
+ __FUNCTION__));
+ }
+ }
+
Support64BitDma = TRUE;
for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
Private->Slot[Slot].Enable = TRUE;
@@ -609,6 +723,18 @@ SdMmcPciHcDriverBindingStart (
if (EFI_ERROR (Status)) {
continue;
}
+ if (mOverride != NULL && mOverride->Capability != NULL) {
+ Status = mOverride->Capability (
+ &Private->PassThru,
+ Controller,
+ Slot,
+ (VOID *)&Private->Capability[Slot]);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n",
+ __FUNCTION__, Status));
+ continue;
+ }
+ }
DumpCapabilityReg (Slot, &Private->Capability[Slot]);
Support64BitDma &= Private->Capability[Slot].SysBus64;
@@ -627,7 +753,7 @@ SdMmcPciHcDriverBindingStart (
//
// Reset the specified slot of the SD/MMC Pci Host Controller
//
- Status = SdMmcHcReset (PciIo, Slot);
+ Status = SdMmcPciHcResetHost (Private, Slot);
if (EFI_ERROR (Status)) {
continue;
}
@@ -642,7 +768,7 @@ SdMmcPciHcDriverBindingStart (
continue;
}
- Status = SdMmcHcInitHost (PciIo, Slot, Private->Capability[Slot]);
+ Status = SdMmcPciHcInitHost (Private, Slot);
if (EFI_ERROR (Status)) {
continue;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 6a2a27969936..8830cd437edd 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -35,6 +35,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Protocol/DriverBinding.h>
#include <Protocol/ComponentName.h>
#include <Protocol/ComponentName2.h>
+#include <Protocol/SdMmcOverride.h>
#include <Protocol/SdMmcPassThru.h>
#include "SdMmcPciHci.h"
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
index e26e6a098c17..154ce45d8223 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
@@ -48,6 +48,7 @@ [Sources]
[Packages]
MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
[LibraryClasses]
DevicePathLib
@@ -61,6 +62,7 @@ [LibraryClasses]
DebugLib
[Protocols]
+ gEdkiiSdMmcOverrideProtocolGuid ## SOMETIMES_CONSUMES
gEfiDevicePathProtocolGuid ## TO_START
gEfiPciIoProtocolGuid ## TO_START
gEfiSdMmcPassThruProtocolGuid ## BY_START
--
2.11.0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard,
I should have provided some of them in the last version.
Sorry about that.
We just found an internal/private SdMmcPciHc implementation
developed by other teams. We are evaluating whether your
proposed SdMmcOverride can be used to retire that private
implementation.
On 12/6/2017 2:01 AM, Ard Biesheuvel wrote:
> Invoke the newly introduced SD/MMC override protocol to override
> the capabilities register after reading it from the device registers,
> and to call the pre/post host init and reset hooks at the appropriate
> times.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 134 +++++++++++++++++++-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 1 +
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf | 2 +
> 3 files changed, 133 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index 0be8828abfcc..f1ea78de809e 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -17,6 +17,8 @@
>
> #include "SdMmcPciHcDxe.h"
>
> +STATIC EDKII_SD_MMC_OVERRIDE *mOverride;
> +
> //
> // Driver Global Variables
> //
> @@ -214,6 +216,104 @@ Done:
> }
>
> /**
> + Execute a SD/MMC host controller reset
> +
> + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
> + @param[in] Slot The slot number of the host controller to reset.
> +**/
> +STATIC
> +EFI_STATUS
> +SdMmcPciHcResetHost (
> + IN SD_MMC_HC_PRIVATE_DATA *Private,
> + IN UINT8 Slot
> + )
I checked the implementation of SdMmcHcReset(). It's quite simple.
So how about we do not introduce this new function SdMmcPciHcResetHost,
but just put the two NotifyPhase call inside SdMmcHcReset().
Because the names of the two functions (SdMmcPciHcResetHost and
SdMmcHcReset) are quite similar. After not a long period, maintainer
may get confused about which is which.
I agree parameters of SdMmcHcReset need to change.
> +{
> + EFI_STATUS Status;
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + &Private->PassThru,
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcResetPre);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN,
> + "%a: SD/MMC pre reset notifier callback failed - %r\n",
> + __FUNCTION__, Status));
> + return Status;
> + }
> + }
> +
> + Status = SdMmcHcReset (Private->PciIo, Slot);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + &Private->PassThru,
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcResetPost);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN,
> + "%a: SD/MMC post reset notifier callback failed - %r\n",
> + __FUNCTION__, Status));
> + }
> + }
> + return Status;
> +}
> +
> +/**
> + Initialize a SD/MMC host controller
> +
> + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
> + @param[in] Slot The slot number of the host controller to initialize.
> +**/
> +STATIC
> +EFI_STATUS
> +SdMmcPciHcInitHost (
> + IN SD_MMC_HC_PRIVATE_DATA *Private,
> + IN UINT8 Slot
> + )
> +{
> + EFI_STATUS Status;
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + &Private->PassThru,
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcInitHostPre);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN,
> + "%a: SD/MMC pre init notifier callback failed - %r\n",
> + __FUNCTION__, Status));
> + return Status;
> + }
> + }
> +
> + Status = SdMmcHcInitHost (Private->PciIo, Slot, Private->Capability[Slot]);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + &Private->PassThru,
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcInitHostPost);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN,
> + "%a: SD/MMC post init notifier callback failed - %r\n",
> + __FUNCTION__, Status));
> + }
> + }
> + return Status;
> +}
> +
> +/**
> Sd removable device enumeration callback function when the timer event is signaled.
>
> @param[in] Event The Event this notify function registered to.
> @@ -281,14 +381,14 @@ SdMmcPciHcEnumerateDevice (
> //
> // Reset the specified slot of the SD/MMC Pci Host Controller
> //
> - Status = SdMmcHcReset (Private->PciIo, Slot);
> + Status = SdMmcPciHcResetHost (Private, Slot);
> if (EFI_ERROR (Status)) {
> continue;
> }
> //
> // Reinitialize slot and restart identification process for the new attached device
> //
> - Status = SdMmcHcInitHost (Private->PciIo, Slot, Private->Capability[Slot]);
> + Status = SdMmcPciHcInitHost (Private, Slot);
> if (EFI_ERROR (Status)) {
> continue;
> }
> @@ -601,6 +701,20 @@ SdMmcPciHcDriverBindingStart (
> goto Done;
> }
>
> + //
> + // Attempt to locate the singleton instance of the SD/MMC override protocol,
> + // which implements platform specific workarounds for non-standard SDHCI
> + // implementations.
> + //
> + if (mOverride == NULL) {
> + Status = gBS->HandleProtocol (Controller, &gEdkiiSdMmcOverrideProtocolGuid,
> + (VOID **)&mOverride);
HandleProtocol() on the Controller cannot work. HandleProtocol() looks
for the SdMmcOverride instance on the specific handle.
You should use LocateProtocol() to look for the SdMmcOverride instance
regardless the handle.
> + if (!EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_INFO, "%a: found SD/MMC override protocol\n",
> + __FUNCTION__));
> + }
> + }
> +
> Support64BitDma = TRUE;
> for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
> Private->Slot[Slot].Enable = TRUE;
> @@ -609,6 +723,18 @@ SdMmcPciHcDriverBindingStart (
> if (EFI_ERROR (Status)) {
> continue;
> }
> + if (mOverride != NULL && mOverride->Capability != NULL) {
> + Status = mOverride->Capability (
> + &Private->PassThru,
> + Controller,
> + Slot,
> + (VOID *)&Private->Capability[Slot]);
(VOID *) can be removed?
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n",
> + __FUNCTION__, Status));
> + continue;
> + }
> + }
> DumpCapabilityReg (Slot, &Private->Capability[Slot]);
>
> Support64BitDma &= Private->Capability[Slot].SysBus64;
> @@ -627,7 +753,7 @@ SdMmcPciHcDriverBindingStart (
> //
> // Reset the specified slot of the SD/MMC Pci Host Controller
> //
> - Status = SdMmcHcReset (PciIo, Slot);
> + Status = SdMmcPciHcResetHost (Private, Slot);
> if (EFI_ERROR (Status)) {
> continue;
> }
> @@ -642,7 +768,7 @@ SdMmcPciHcDriverBindingStart (
> continue;
> }
>
> - Status = SdMmcHcInitHost (PciIo, Slot, Private->Capability[Slot]);
> + Status = SdMmcPciHcInitHost (Private, Slot);
> if (EFI_ERROR (Status)) {
> continue;
> }
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 6a2a27969936..8830cd437edd 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -35,6 +35,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include <Protocol/DriverBinding.h>
> #include <Protocol/ComponentName.h>
> #include <Protocol/ComponentName2.h>
> +#include <Protocol/SdMmcOverride.h>
> #include <Protocol/SdMmcPassThru.h>
>
> #include "SdMmcPciHci.h"
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
> index e26e6a098c17..154ce45d8223 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
> @@ -48,6 +48,7 @@ [Sources]
>
> [Packages]
> MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
>
> [LibraryClasses]
> DevicePathLib
> @@ -61,6 +62,7 @@ [LibraryClasses]
> DebugLib
>
> [Protocols]
> + gEdkiiSdMmcOverrideProtocolGuid ## SOMETIMES_CONSUMES
> gEfiDevicePathProtocolGuid ## TO_START
> gEfiPciIoProtocolGuid ## TO_START
> gEfiSdMmcPassThruProtocolGuid ## BY_START
>
--
Thanks,
Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 6 December 2017 at 03:33, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> Ard,
> I should have provided some of them in the last version.
> Sorry about that.
>
> We just found an internal/private SdMmcPciHc implementation
> developed by other teams. We are evaluating whether your
> proposed SdMmcOverride can be used to retire that private
> implementation.
>
OK, good to know.
>
>
> On 12/6/2017 2:01 AM, Ard Biesheuvel wrote:
>>
>> Invoke the newly introduced SD/MMC override protocol to override
>> the capabilities register after reading it from the device registers,
>> and to call the pre/post host init and reset hooks at the appropriate
>> times.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 134
>> +++++++++++++++++++-
>> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 1 +
>> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf | 2 +
>> 3 files changed, 133 insertions(+), 4 deletions(-)
>>
>> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
>> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
>> index 0be8828abfcc..f1ea78de809e 100644
>> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
>> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
>> @@ -17,6 +17,8 @@
>> #include "SdMmcPciHcDxe.h"
>> +STATIC EDKII_SD_MMC_OVERRIDE *mOverride;
>> +
>> //
>> // Driver Global Variables
>> //
>> @@ -214,6 +216,104 @@ Done:
>> }
>> /**
>> + Execute a SD/MMC host controller reset
>> +
>> + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
>> instance.
>> + @param[in] Slot The slot number of the host controller to
>> reset.
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +SdMmcPciHcResetHost (
>> + IN SD_MMC_HC_PRIVATE_DATA *Private,
>> + IN UINT8 Slot
>> + )
>
> I checked the implementation of SdMmcHcReset(). It's quite simple.
> So how about we do not introduce this new function SdMmcPciHcResetHost,
> but just put the two NotifyPhase call inside SdMmcHcReset().
>
> Because the names of the two functions (SdMmcPciHcResetHost and
> SdMmcHcReset) are quite similar. After not a long period, maintainer
> may get confused about which is which.
>
> I agree parameters of SdMmcHcReset need to change.
>
OK, that works for me.
>
>> +{
>> + EFI_STATUS Status;
>> +
>> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
>> + Status = mOverride->NotifyPhase (
>> + &Private->PassThru,
>> + Private->ControllerHandle,
>> + Slot,
>> + EdkiiSdMmcResetPre);
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_WARN,
>> + "%a: SD/MMC pre reset notifier callback failed - %r\n",
>> + __FUNCTION__, Status));
>> + return Status;
>> + }
>> + }
>> +
>> + Status = SdMmcHcReset (Private->PciIo, Slot);
>> + if (EFI_ERROR (Status)) {
>> + return Status;
>> + }
>> +
>> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
>> + Status = mOverride->NotifyPhase (
>> + &Private->PassThru,
>> + Private->ControllerHandle,
>> + Slot,
>> + EdkiiSdMmcResetPost);
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_WARN,
>> + "%a: SD/MMC post reset notifier callback failed - %r\n",
>> + __FUNCTION__, Status));
>> + }
>> + }
>> + return Status;
>> +}
>> +
>> +/**
>> + Initialize a SD/MMC host controller
>> +
>> + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
>> instance.
>> + @param[in] Slot The slot number of the host controller to
>> initialize.
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +SdMmcPciHcInitHost (
>> + IN SD_MMC_HC_PRIVATE_DATA *Private,
>> + IN UINT8 Slot
>> + )
>> +{
>> + EFI_STATUS Status;
>> +
>> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
>> + Status = mOverride->NotifyPhase (
>> + &Private->PassThru,
>> + Private->ControllerHandle,
>> + Slot,
>> + EdkiiSdMmcInitHostPre);
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_WARN,
>> + "%a: SD/MMC pre init notifier callback failed - %r\n",
>> + __FUNCTION__, Status));
>> + return Status;
>> + }
>> + }
>> +
>> + Status = SdMmcHcInitHost (Private->PciIo, Slot,
>> Private->Capability[Slot]);
>> + if (EFI_ERROR (Status)) {
>> + return Status;
>> + }
>> +
>> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
>> + Status = mOverride->NotifyPhase (
>> + &Private->PassThru,
>> + Private->ControllerHandle,
>> + Slot,
>> + EdkiiSdMmcInitHostPost);
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_WARN,
>> + "%a: SD/MMC post init notifier callback failed - %r\n",
>> + __FUNCTION__, Status));
>> + }
>> + }
>> + return Status;
>> +}
>> +
>> +/**
>> Sd removable device enumeration callback function when the timer event
>> is signaled.
>> @param[in] Event The Event this notify function registered to.
>> @@ -281,14 +381,14 @@ SdMmcPciHcEnumerateDevice (
>> //
>> // Reset the specified slot of the SD/MMC Pci Host Controller
>> //
>> - Status = SdMmcHcReset (Private->PciIo, Slot);
>> + Status = SdMmcPciHcResetHost (Private, Slot);
>> if (EFI_ERROR (Status)) {
>> continue;
>> }
>> //
>> // Reinitialize slot and restart identification process for the
>> new attached device
>> //
>> - Status = SdMmcHcInitHost (Private->PciIo, Slot,
>> Private->Capability[Slot]);
>> + Status = SdMmcPciHcInitHost (Private, Slot);
>> if (EFI_ERROR (Status)) {
>> continue;
>> }
>> @@ -601,6 +701,20 @@ SdMmcPciHcDriverBindingStart (
>> goto Done;
>> }
>> + //
>> + // Attempt to locate the singleton instance of the SD/MMC override
>> protocol,
>> + // which implements platform specific workarounds for non-standard
>> SDHCI
>> + // implementations.
>> + //
>> + if (mOverride == NULL) {
>> + Status = gBS->HandleProtocol (Controller,
>> &gEdkiiSdMmcOverrideProtocolGuid,
>> + (VOID **)&mOverride);
>
>
> HandleProtocol() on the Controller cannot work. HandleProtocol() looks
> for the SdMmcOverride instance on the specific handle.
>
> You should use LocateProtocol() to look for the SdMmcOverride instance
> regardless the handle.
>
Oops. Yes, I missed that when updating the code. Will fix.
>> + if (!EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_INFO, "%a: found SD/MMC override protocol\n",
>> + __FUNCTION__));
>> + }
>> + }
>> +
>> Support64BitDma = TRUE;
>> for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
>> Private->Slot[Slot].Enable = TRUE;
>> @@ -609,6 +723,18 @@ SdMmcPciHcDriverBindingStart (
>> if (EFI_ERROR (Status)) {
>> continue;
>> }
>> + if (mOverride != NULL && mOverride->Capability != NULL) {
>> + Status = mOverride->Capability (
>> + &Private->PassThru,
>> + Controller,
>> + Slot,
>> + (VOID *)&Private->Capability[Slot]);
>
> (VOID *) can be removed?
>
No, Capability () takes a UINT64 pointer, because the capability
struct is exactly 64 bits. So the cast is needed.
But now that I think of it, the struct may not be 64-bit aligned, so
casting it is incorrect. I will revert that change.
>
>> + if (EFI_ERROR (Status)) {
>> + DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n",
>> + __FUNCTION__, Status));
>> + continue;
>> + }
>> + }
>> DumpCapabilityReg (Slot, &Private->Capability[Slot]);
>> Support64BitDma &= Private->Capability[Slot].SysBus64;
>> @@ -627,7 +753,7 @@ SdMmcPciHcDriverBindingStart (
>> //
>> // Reset the specified slot of the SD/MMC Pci Host Controller
>> //
>> - Status = SdMmcHcReset (PciIo, Slot);
>> + Status = SdMmcPciHcResetHost (Private, Slot);
>> if (EFI_ERROR (Status)) {
>> continue;
>> }
>> @@ -642,7 +768,7 @@ SdMmcPciHcDriverBindingStart (
>> continue;
>> }
>> - Status = SdMmcHcInitHost (PciIo, Slot, Private->Capability[Slot]);
>> + Status = SdMmcPciHcInitHost (Private, Slot);
>> if (EFI_ERROR (Status)) {
>> continue;
>> }
>> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
>> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
>> index 6a2a27969936..8830cd437edd 100644
>> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
>> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
>> @@ -35,6 +35,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
>> EITHER EXPRESS OR IMPLIED.
>> #include <Protocol/DriverBinding.h>
>> #include <Protocol/ComponentName.h>
>> #include <Protocol/ComponentName2.h>
>> +#include <Protocol/SdMmcOverride.h>
>> #include <Protocol/SdMmcPassThru.h>
>> #include "SdMmcPciHci.h"
>> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
>> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
>> index e26e6a098c17..154ce45d8223 100644
>> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
>> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
>> @@ -48,6 +48,7 @@ [Sources]
>> [Packages]
>> MdePkg/MdePkg.dec
>> + MdeModulePkg/MdeModulePkg.dec
>> [LibraryClasses]
>> DevicePathLib
>> @@ -61,6 +62,7 @@ [LibraryClasses]
>> DebugLib
>> [Protocols]
>> + gEdkiiSdMmcOverrideProtocolGuid ## SOMETIMES_CONSUMES
>> gEfiDevicePathProtocolGuid ## TO_START
>> gEfiPciIoProtocolGuid ## TO_START
>> gEfiSdMmcPassThruProtocolGuid ## BY_START
>>
>
>
> --
> Thanks,
> Ray
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.