* [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13
@ 2019-12-20 17:13 Albecki, Mateusz
2019-12-20 17:13 ` [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start Albecki, Mateusz
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Albecki, Mateusz @ 2019-12-20 17:13 UTC (permalink / raw)
To: devel; +Cc: Mateusz Albecki, Hao A Wu, Marcin Wojtas, Zhichao Gao, Liming Gao
The first patch refactors the SdMmcClockSupply function with a goal
of sending the EdkiiSdMmcSwitchClockFreq notification before we send the
CMD13 to check the switch status in eMMC init flow. This is required to
avoid sending the CMD13 on link that still has not been fixed by platform.
To avoid changing the driver behavior we avoid sending notifications
when the clock is setup for the first time or when we setup the clock
after the voltage switch procedure(adressed in second patch).
The second patch in the series optimizes the SD card detection routine
to stop it from going through the process of internal clock setup
after switching the voltage. According to SD HC specification there
is no need to setup internal clock all over again.
Tests performed:
- Booted eMMC in HS400 mode on platform which required post clock freq fixes
I wasn't able to test SD card yet due to the lack of setup with working SD.
The patch series is available on github here: https://github.com/malbecki/edk2/tree/sdmmc_post_freq_notify
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Marcin Wojtas <mw@semihalf.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Mateusz Albecki (2):
SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +--
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 25 +---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 ++++
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 136 +++++++++++----------
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 45 ++-----
5 files changed, 112 insertions(+), 138 deletions(-)
--
2.14.1.windows.1
--------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
2019-12-20 17:13 [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13 Albecki, Mateusz
@ 2019-12-20 17:13 ` Albecki, Mateusz
2019-12-24 2:52 ` Wu, Hao A
2019-12-20 17:13 ` [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock Albecki, Mateusz
2019-12-24 2:51 ` [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13 Wu, Hao A
2 siblings, 1 reply; 12+ messages in thread
From: Albecki, Mateusz @ 2019-12-20 17:13 UTC (permalink / raw)
To: devel; +Cc: Mateusz Albecki, Hao A Wu, Marcin Wojtas, Zhichao Gao, Liming Gao
For eMMC modules we used to notify the platform about frequency
change only after sending CMD13 which meant that platform
might not get a chance to apply required post frequency
change fixes to get the clock stable. To fix this
notification has been moved to SdMmcHcClockSupply function
just after we start the SD clock. During first time setup
the notification won't be sent to avoid changing old behavior.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Marcin Wojtas <mw@semihalf.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 28 ++----
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 +++++
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 107 +++++++++------------
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 44 ---------
5 files changed, 81 insertions(+), 142 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index 082904ccc5..776c0e796c 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -727,7 +727,7 @@ EmmcSwitchBusTiming (
//
// Convert the clock freq unit from MHz to KHz.
//
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
+ Status = SdMmcHcClockSupply (Private, Slot, BusTiming, FALSE, ClockFreq * 1000);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -745,24 +745,6 @@ EmmcSwitchBusTiming (
return EFI_DEVICE_ERROR;
}
- if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
- Status = mOverride->NotifyPhase (
- Private->ControllerHandle,
- Slot,
- EdkiiSdMmcSwitchClockFreqPost,
- &BusTiming
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
- __FUNCTION__,
- Status
- ));
- return Status;
- }
- }
-
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 336baade9e..d63dc54e8c 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1145,29 +1145,11 @@ SdCardSetBusMode (
return Status;
}
- Status = SdMmcHcClockSupply (PciIo, Slot, BusMode.ClockFreq * 1000, Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
+ Status = SdMmcHcClockSupply (Private, Slot, BusMode.BusTiming, FALSE, BusMode.ClockFreq * 1000);
if (EFI_ERROR (Status)) {
return Status;
}
- if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
- Status = mOverride->NotifyPhase (
- Private->ControllerHandle,
- Slot,
- EdkiiSdMmcSwitchClockFreqPost,
- &BusMode.BusTiming
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
- __FUNCTION__,
- Status
- ));
- return Status;
- }
- }
-
if ((BusMode.BusTiming == SdMmcUhsSdr104) || ((BusMode.BusTiming == SdMmcUhsSdr50) && (Capability->TuningSDR50 != 0))) {
Status = SdCardTuningClock (PciIo, PassThru, Slot);
if (EFI_ERROR (Status)) {
@@ -1345,7 +1327,13 @@ SdCardIdentification (
goto Error;
}
- SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
+ //
+ // Restart the clock with first time parameters.
+ // NOTE: it is not required to actually restart the clock
+ // and go through internal clock setup again. Some time
+ // could be saved if we simply started the SD clock.
+ //
+ SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
gBS->Stall (1000);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index c29e48767e..0304960132 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -796,6 +796,30 @@ SdCardIdentification (
IN UINT8 Slot
);
+/**
+ SD/MMC card clock supply.
+
+ Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
+
+ @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
+ @param[in] Slot The slot number of the SD card to send the command to.
+ @param[in] BusTiming BusTiming at which the frequency change is done.
+ @param[in] FirstTimeSetup Flag to indicate whether the clock is being setup for the first time.
+ @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
+
+ @retval EFI_SUCCESS The clock is supplied successfully.
+ @retval Others The clock isn't supplied successfully.
+
+**/
+EFI_STATUS
+SdMmcHcClockSupply (
+ IN SD_MMC_HC_PRIVATE_DATA *Private,
+ IN UINT8 Slot,
+ IN SD_MMC_BUS_MODE BusTiming,
+ IN BOOLEAN FirstTimeSetup,
+ IN UINT64 ClockFreq
+ );
+
/**
Software reset the specified SD/MMC host controller.
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index b9d04e0f17..f667264c5e 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -763,11 +763,11 @@ SdMmcHcStopClock (
Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
- @param[in] PciIo The PCI IO protocol instance.
- @param[in] Slot The slot number of the SD card to send the command to.
- @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
- @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
- @param[in] ControllerVer The version of host controller.
+ @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
+ @param[in] Slot The slot number of the SD card to send the command to.
+ @param[in] BusTiming BusTiming at which the frequency change is done.
+ @param[in] FirstTimeSetup Flag to indicate whether the clock is being setup for the first time.
+ @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
@retval EFI_SUCCESS The clock is supplied successfully.
@retval Others The clock isn't supplied successfully.
@@ -775,11 +775,11 @@ SdMmcHcStopClock (
**/
EFI_STATUS
SdMmcHcClockSupply (
- IN EFI_PCI_IO_PROTOCOL *PciIo,
- IN UINT8 Slot,
- IN UINT64 ClockFreq,
- IN UINT32 BaseClkFreq,
- IN UINT16 ControllerVer
+ IN SD_MMC_HC_PRIVATE_DATA *Private,
+ IN UINT8 Slot,
+ IN SD_MMC_BUS_MODE BusTiming,
+ IN BOOLEAN FirstTimeSetup,
+ IN UINT64 ClockFreq
)
{
EFI_STATUS Status;
@@ -787,13 +787,15 @@ SdMmcHcClockSupply (
UINT32 Divisor;
UINT32 Remainder;
UINT16 ClockCtrl;
+ UINT32 BaseClkFreq;
+ UINT16 ControllerVer;
+ EFI_PCI_IO_PROTOCOL *PciIo;
- //
- // Calculate a divisor for SD clock frequency
- //
- ASSERT (BaseClkFreq != 0);
+ PciIo = Private->PciIo;
+ BaseClkFreq = Private->BaseClkFreq[Slot];
+ ControllerVer = Private->ControllerVersion[Slot];
- if (ClockFreq == 0) {
+ if (BaseClkFreq == 0 || ClockFreq == 0) {
return EFI_INVALID_PARAMETER;
}
@@ -883,6 +885,29 @@ SdMmcHcClockSupply (
ClockCtrl = BIT2;
Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl);
+ //
+ // We don't notify the platform on first time setup to avoid changing
+ // legacy behavior. During first time setup we also don't know what type
+ // of the card slot it is and which enum value of BusTiming applies.
+ //
+ if (!FirstTimeSetup && mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcSwitchClockFreqPost,
+ &BusTiming
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+ }
+
return Status;
}
@@ -1038,49 +1063,6 @@ SdMmcHcInitV4Enhancements (
return EFI_SUCCESS;
}
-/**
- Supply SD/MMC card with lowest clock frequency at initialization.
-
- @param[in] PciIo The PCI IO protocol instance.
- @param[in] Slot The slot number of the SD card to send the command to.
- @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
- @param[in] ControllerVer The version of host controller.
-
- @retval EFI_SUCCESS The clock is supplied successfully.
- @retval Others The clock isn't supplied successfully.
-
-**/
-EFI_STATUS
-SdMmcHcInitClockFreq (
- IN EFI_PCI_IO_PROTOCOL *PciIo,
- IN UINT8 Slot,
- IN UINT32 BaseClkFreq,
- IN UINT16 ControllerVer
- )
-{
- EFI_STATUS Status;
- UINT32 InitFreq;
-
- //
- // According to SDHCI specification ver. 4.2, BaseClkFreq field value of
- // the Capability Register 1 can be zero, which means a need for obtaining
- // the clock frequency via another method. Fail in case it is not updated
- // by SW at this point.
- //
- if (BaseClkFreq == 0) {
- //
- // Don't support get Base Clock Frequency information via another method
- //
- return EFI_UNSUPPORTED;
- }
- //
- // Supply 400KHz clock frequency at initialization phase.
- //
- InitFreq = 400;
- Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq, ControllerVer);
- return Status;
-}
-
/**
Supply SD/MMC card with maximum voltage at initialization.
@@ -1216,7 +1198,14 @@ SdMmcHcInitHost (
return Status;
}
- Status = SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
+ //
+ // Perform first time clock setup with 400 KHz frequency.
+ // We send the 0 as the BusTiming value because at this time
+ // we still do not know the slot type and which enum value will apply.
+ // Since it is a first time setup SdMmcHcClockSupply won't notify
+ // the platofrm driver anyway so it doesn't matter.
+ //
+ Status = SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index 088c70451c..826e851b04 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -478,30 +478,6 @@ SdMmcHcStopClock (
IN UINT8 Slot
);
-/**
- SD/MMC card clock supply.
-
- Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
-
- @param[in] PciIo The PCI IO protocol instance.
- @param[in] Slot The slot number of the SD card to send the command to.
- @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
- @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
- @param[in] ControllerVer The version of host controller.
-
- @retval EFI_SUCCESS The clock is supplied successfully.
- @retval Others The clock isn't supplied successfully.
-
-**/
-EFI_STATUS
-SdMmcHcClockSupply (
- IN EFI_PCI_IO_PROTOCOL *PciIo,
- IN UINT8 Slot,
- IN UINT64 ClockFreq,
- IN UINT32 BaseClkFreq,
- IN UINT16 ControllerVer
- );
-
/**
SD/MMC bus power control.
@@ -542,26 +518,6 @@ SdMmcHcSetBusWidth (
IN UINT16 BusWidth
);
-/**
- Supply SD/MMC card with lowest clock frequency at initialization.
-
- @param[in] PciIo The PCI IO protocol instance.
- @param[in] Slot The slot number of the SD card to send the command to.
- @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
- @param[in] ControllerVer The version of host controller.
-
- @retval EFI_SUCCESS The clock is supplied successfully.
- @retval Others The clock isn't supplied successfully.
-
-**/
-EFI_STATUS
-SdMmcHcInitClockFreq (
- IN EFI_PCI_IO_PROTOCOL *PciIo,
- IN UINT8 Slot,
- IN UINT32 BaseClkFreq,
- IN UINT16 ControllerVer
- );
-
/**
Supply SD/MMC card with maximum voltage at initialization.
--
2.14.1.windows.1
--------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock
2019-12-20 17:13 [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13 Albecki, Mateusz
2019-12-20 17:13 ` [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start Albecki, Mateusz
@ 2019-12-20 17:13 ` Albecki, Mateusz
2019-12-24 2:52 ` Wu, Hao A
2019-12-24 2:51 ` [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13 Wu, Hao A
2 siblings, 1 reply; 12+ messages in thread
From: Albecki, Mateusz @ 2019-12-20 17:13 UTC (permalink / raw)
To: devel; +Cc: Mateusz Albecki, Hao A Wu, Marcin Wojtas, Zhichao Gao, Liming Gao
In SD card voltage switch flow we used to redo the
entire internal clock setup after voltage switch.
Since internal clock has already been setup this
is wasting time on polling the internal clock stable.
This commit changes it to only start the SD clock.
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Marcin Wojtas <mw@semihalf.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 11 +++-----
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 33 ++++++++++++++++++++----
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 15 +++++++++++
3 files changed, 47 insertions(+), 12 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index d63dc54e8c..b630daab76 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1327,13 +1327,10 @@ SdCardIdentification (
goto Error;
}
- //
- // Restart the clock with first time parameters.
- // NOTE: it is not required to actually restart the clock
- // and go through internal clock setup again. Some time
- // could be saved if we simply started the SD clock.
- //
- SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
+ Status = SdMmcHcStartSdClock (PciIo, Slot);
+ if (EFI_ERROR (Status)) {
+ goto Error;
+ }
gBS->Stall (1000);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index f667264c5e..e7f2fac69b 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -758,6 +758,30 @@ SdMmcHcStopClock (
return Status;
}
+/**
+ Start the SD clock.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] Slot The slot number.
+
+ @retval EFI_SUCCESS Succeeded to start the SD clock.
+ @rtval Others Failed to start the SD clock.
+**/
+EFI_STATUS
+SdMmcHcStartSdClock (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8 Slot
+ )
+{
+ UINT16 ClockCtrl;
+
+ //
+ // Set SD Clock Enable in the Clock Control register to 1
+ //
+ ClockCtrl = BIT2;
+ return SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl);
+}
+
/**
SD/MMC card clock supply.
@@ -879,11 +903,10 @@ SdMmcHcClockSupply (
return Status;
}
- //
- // Set SD Clock Enable in the Clock Control register to 1
- //
- ClockCtrl = BIT2;
- Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl);
+ Status = SdMmcHcStartSdClock (PciIo, Slot);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
//
// We don't notify the platform on first time setup to avoid changing
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index 826e851b04..4753bb6864 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -478,6 +478,21 @@ SdMmcHcStopClock (
IN UINT8 Slot
);
+/**
+ Start the SD clock.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] Slot The slot number.
+
+ @retval EFI_SUCCESS Succeeded to start the SD clock.
+ @rtval Others Failed to start the SD clock.
+**/
+EFI_STATUS
+SdMmcHcStartSdClock (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8 Slot
+ );
+
/**
SD/MMC bus power control.
--
2.14.1.windows.1
--------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13
2019-12-20 17:13 [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13 Albecki, Mateusz
2019-12-20 17:13 ` [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start Albecki, Mateusz
2019-12-20 17:13 ` [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock Albecki, Mateusz
@ 2019-12-24 2:51 ` Wu, Hao A
2020-01-03 11:04 ` Marcin Wojtas
2 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2019-12-24 2:51 UTC (permalink / raw)
To: Albecki, Mateusz, devel@edk2.groups.io
Cc: Marcin Wojtas, Gao, Zhichao, Gao, Liming, Ard Biesheuvel
> -----Original Message-----
> From: Albecki, Mateusz
> Sent: Saturday, December 21, 2019 1:13 AM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> Subject: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the
> EdkiiSdMmcSwitchClockFreq notification before sending CMD13
>
> The first patch refactors the SdMmcClockSupply function with a goal
> of sending the EdkiiSdMmcSwitchClockFreq notification before we send the
> CMD13 to check the switch status in eMMC init flow. This is required to
> avoid sending the CMD13 on link that still has not been fixed by platform.
>
> To avoid changing the driver behavior we avoid sending notifications
> when the clock is setup for the first time or when we setup the clock
> after the voltage switch procedure(adressed in second patch).
>
> The second patch in the series optimizes the SD card detection routine
> to stop it from going through the process of internal clock setup
> after switching the voltage. According to SD HC specification there
> is no need to setup internal clock all over again.
>
> Tests performed:
> - Booted eMMC in HS400 mode on platform which required post clock freq
> fixes
>
> I wasn't able to test SD card yet due to the lack of setup with working SD.
I performed a quick verification on the eMMC device and SD card on my side.
They work properly after the series.
So for the series,
Tested-by: Hao A Wu <hao.a.wu@intel.com>
>
> The patch series is available on github here:
> https://github.com/malbecki/edk2/tree/sdmmc_post_freq_notify
Add Ard to the loop to see if there is additional comment.
Best Regards,
Hao Wu
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Marcin Wojtas <mw@semihalf.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
>
> Mateusz Albecki (2):
> SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
> MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock
>
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +--
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 25 +---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 ++++
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 136
> +++++++++++----------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 45 ++-----
> 5 files changed, 112 insertions(+), 138 deletions(-)
>
> --
> 2.14.1.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
2019-12-20 17:13 ` [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start Albecki, Mateusz
@ 2019-12-24 2:52 ` Wu, Hao A
2019-12-24 9:54 ` Ard Biesheuvel
2019-12-31 14:49 ` Albecki, Mateusz
0 siblings, 2 replies; 12+ messages in thread
From: Wu, Hao A @ 2019-12-24 2:52 UTC (permalink / raw)
To: Albecki, Mateusz, devel@edk2.groups.io
Cc: Marcin Wojtas, Gao, Zhichao, Gao, Liming, Ard Biesheuvel
> -----Original Message-----
> From: Albecki, Mateusz
> Sent: Saturday, December 21, 2019 1:13 AM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> Subject: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after
> SD clock start
Hello Mateusz,
Just a minor format comment, how about changing the subject to:
MdeModulePkg/SdMmcPciHcDxe: Hook SwitchClockFreq after SD clock start
If there is no other major concern from other reviewers for the patch, I will
handle this when pushing the patch.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
>
> For eMMC modules we used to notify the platform about frequency
> change only after sending CMD13 which meant that platform
> might not get a chance to apply required post frequency
> change fixes to get the clock stable. To fix this
> notification has been moved to SdMmcHcClockSupply function
> just after we start the SD clock. During first time setup
> the notification won't be sent to avoid changing old behavior.
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Marcin Wojtas <mw@semihalf.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
>
> Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 28 ++----
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 +++++
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 107 +++++++++--
> ----------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 44 ---------
> 5 files changed, 81 insertions(+), 142 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> index 082904ccc5..776c0e796c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -727,7 +727,7 @@ EmmcSwitchBusTiming (
> //
> // Convert the clock freq unit from MHz to KHz.
> //
> - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> + Status = SdMmcHcClockSupply (Private, Slot, BusTiming, FALSE, ClockFreq *
> 1000);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> @@ -745,24 +745,6 @@ EmmcSwitchBusTiming (
> return EFI_DEVICE_ERROR;
> }
>
> - if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> - Status = mOverride->NotifyPhase (
> - Private->ControllerHandle,
> - Slot,
> - EdkiiSdMmcSwitchClockFreqPost,
> - &BusTiming
> - );
> - if (EFI_ERROR (Status)) {
> - DEBUG ((
> - DEBUG_ERROR,
> - "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> - __FUNCTION__,
> - Status
> - ));
> - return Status;
> - }
> - }
> -
> return Status;
> }
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 336baade9e..d63dc54e8c 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -1145,29 +1145,11 @@ SdCardSetBusMode (
> return Status;
> }
>
> - Status = SdMmcHcClockSupply (PciIo, Slot, BusMode.ClockFreq * 1000,
> Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> + Status = SdMmcHcClockSupply (Private, Slot, BusMode.BusTiming, FALSE,
> BusMode.ClockFreq * 1000);
> if (EFI_ERROR (Status)) {
> return Status;
> }
>
> - if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> - Status = mOverride->NotifyPhase (
> - Private->ControllerHandle,
> - Slot,
> - EdkiiSdMmcSwitchClockFreqPost,
> - &BusMode.BusTiming
> - );
> - if (EFI_ERROR (Status)) {
> - DEBUG ((
> - DEBUG_ERROR,
> - "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> - __FUNCTION__,
> - Status
> - ));
> - return Status;
> - }
> - }
> -
> if ((BusMode.BusTiming == SdMmcUhsSdr104) || ((BusMode.BusTiming ==
> SdMmcUhsSdr50) && (Capability->TuningSDR50 != 0))) {
> Status = SdCardTuningClock (PciIo, PassThru, Slot);
> if (EFI_ERROR (Status)) {
> @@ -1345,7 +1327,13 @@ SdCardIdentification (
> goto Error;
> }
>
> - SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], Private-
> >ControllerVersion[Slot]);
> + //
> + // Restart the clock with first time parameters.
> + // NOTE: it is not required to actually restart the clock
> + // and go through internal clock setup again. Some time
> + // could be saved if we simply started the SD clock.
> + //
> + SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
>
> gBS->Stall (1000);
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index c29e48767e..0304960132 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -796,6 +796,30 @@ SdCardIdentification (
> IN UINT8 Slot
> );
>
> +/**
> + SD/MMC card clock supply.
> +
> + Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> +
> + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
> instance.
> + @param[in] Slot The slot number of the SD card to send the command
> to.
> + @param[in] BusTiming BusTiming at which the frequency change is done.
> + @param[in] FirstTimeSetup Flag to indicate whether the clock is being setup
> for the first time.
> + @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> +
> + @retval EFI_SUCCESS The clock is supplied successfully.
> + @retval Others The clock isn't supplied successfully.
> +
> +**/
> +EFI_STATUS
> +SdMmcHcClockSupply (
> + IN SD_MMC_HC_PRIVATE_DATA *Private,
> + IN UINT8 Slot,
> + IN SD_MMC_BUS_MODE BusTiming,
> + IN BOOLEAN FirstTimeSetup,
> + IN UINT64 ClockFreq
> + );
> +
> /**
> Software reset the specified SD/MMC host controller.
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index b9d04e0f17..f667264c5e 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -763,11 +763,11 @@ SdMmcHcStopClock (
>
> Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
>
> - @param[in] PciIo The PCI IO protocol instance.
> - @param[in] Slot The slot number of the SD card to send the command
> to.
> - @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> - @param[in] BaseClkFreq The base clock frequency of host controller in
> MHz.
> - @param[in] ControllerVer The version of host controller.
> + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
> instance.
> + @param[in] Slot The slot number of the SD card to send the command
> to.
> + @param[in] BusTiming BusTiming at which the frequency change is done.
> + @param[in] FirstTimeSetup Flag to indicate whether the clock is being setup
> for the first time.
> + @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
>
> @retval EFI_SUCCESS The clock is supplied successfully.
> @retval Others The clock isn't supplied successfully.
> @@ -775,11 +775,11 @@ SdMmcHcStopClock (
> **/
> EFI_STATUS
> SdMmcHcClockSupply (
> - IN EFI_PCI_IO_PROTOCOL *PciIo,
> - IN UINT8 Slot,
> - IN UINT64 ClockFreq,
> - IN UINT32 BaseClkFreq,
> - IN UINT16 ControllerVer
> + IN SD_MMC_HC_PRIVATE_DATA *Private,
> + IN UINT8 Slot,
> + IN SD_MMC_BUS_MODE BusTiming,
> + IN BOOLEAN FirstTimeSetup,
> + IN UINT64 ClockFreq
> )
> {
> EFI_STATUS Status;
> @@ -787,13 +787,15 @@ SdMmcHcClockSupply (
> UINT32 Divisor;
> UINT32 Remainder;
> UINT16 ClockCtrl;
> + UINT32 BaseClkFreq;
> + UINT16 ControllerVer;
> + EFI_PCI_IO_PROTOCOL *PciIo;
>
> - //
> - // Calculate a divisor for SD clock frequency
> - //
> - ASSERT (BaseClkFreq != 0);
> + PciIo = Private->PciIo;
> + BaseClkFreq = Private->BaseClkFreq[Slot];
> + ControllerVer = Private->ControllerVersion[Slot];
>
> - if (ClockFreq == 0) {
> + if (BaseClkFreq == 0 || ClockFreq == 0) {
> return EFI_INVALID_PARAMETER;
> }
>
> @@ -883,6 +885,29 @@ SdMmcHcClockSupply (
> ClockCtrl = BIT2;
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> (ClockCtrl), &ClockCtrl);
>
> + //
> + // We don't notify the platform on first time setup to avoid changing
> + // legacy behavior. During first time setup we also don't know what type
> + // of the card slot it is and which enum value of BusTiming applies.
> + //
> + if (!FirstTimeSetup && mOverride != NULL && mOverride->NotifyPhase !=
> NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcSwitchClockFreqPost,
> + &BusTiming
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> + }
> +
> return Status;
> }
>
> @@ -1038,49 +1063,6 @@ SdMmcHcInitV4Enhancements (
> return EFI_SUCCESS;
> }
>
> -/**
> - Supply SD/MMC card with lowest clock frequency at initialization.
> -
> - @param[in] PciIo The PCI IO protocol instance.
> - @param[in] Slot The slot number of the SD card to send the command
> to.
> - @param[in] BaseClkFreq The base clock frequency of host controller in
> MHz.
> - @param[in] ControllerVer The version of host controller.
> -
> - @retval EFI_SUCCESS The clock is supplied successfully.
> - @retval Others The clock isn't supplied successfully.
> -
> -**/
> -EFI_STATUS
> -SdMmcHcInitClockFreq (
> - IN EFI_PCI_IO_PROTOCOL *PciIo,
> - IN UINT8 Slot,
> - IN UINT32 BaseClkFreq,
> - IN UINT16 ControllerVer
> - )
> -{
> - EFI_STATUS Status;
> - UINT32 InitFreq;
> -
> - //
> - // According to SDHCI specification ver. 4.2, BaseClkFreq field value of
> - // the Capability Register 1 can be zero, which means a need for obtaining
> - // the clock frequency via another method. Fail in case it is not updated
> - // by SW at this point.
> - //
> - if (BaseClkFreq == 0) {
> - //
> - // Don't support get Base Clock Frequency information via another method
> - //
> - return EFI_UNSUPPORTED;
> - }
> - //
> - // Supply 400KHz clock frequency at initialization phase.
> - //
> - InitFreq = 400;
> - Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq,
> ControllerVer);
> - return Status;
> -}
> -
> /**
> Supply SD/MMC card with maximum voltage at initialization.
>
> @@ -1216,7 +1198,14 @@ SdMmcHcInitHost (
> return Status;
> }
>
> - Status = SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot],
> Private->ControllerVersion[Slot]);
> + //
> + // Perform first time clock setup with 400 KHz frequency.
> + // We send the 0 as the BusTiming value because at this time
> + // we still do not know the slot type and which enum value will apply.
> + // Since it is a first time setup SdMmcHcClockSupply won't notify
> + // the platofrm driver anyway so it doesn't matter.
> + //
> + Status = SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index 088c70451c..826e851b04 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -478,30 +478,6 @@ SdMmcHcStopClock (
> IN UINT8 Slot
> );
>
> -/**
> - SD/MMC card clock supply.
> -
> - Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> -
> - @param[in] PciIo The PCI IO protocol instance.
> - @param[in] Slot The slot number of the SD card to send the command
> to.
> - @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> - @param[in] BaseClkFreq The base clock frequency of host controller in
> MHz.
> - @param[in] ControllerVer The version of host controller.
> -
> - @retval EFI_SUCCESS The clock is supplied successfully.
> - @retval Others The clock isn't supplied successfully.
> -
> -**/
> -EFI_STATUS
> -SdMmcHcClockSupply (
> - IN EFI_PCI_IO_PROTOCOL *PciIo,
> - IN UINT8 Slot,
> - IN UINT64 ClockFreq,
> - IN UINT32 BaseClkFreq,
> - IN UINT16 ControllerVer
> - );
> -
> /**
> SD/MMC bus power control.
>
> @@ -542,26 +518,6 @@ SdMmcHcSetBusWidth (
> IN UINT16 BusWidth
> );
>
> -/**
> - Supply SD/MMC card with lowest clock frequency at initialization.
> -
> - @param[in] PciIo The PCI IO protocol instance.
> - @param[in] Slot The slot number of the SD card to send the command
> to.
> - @param[in] BaseClkFreq The base clock frequency of host controller in
> MHz.
> - @param[in] ControllerVer The version of host controller.
> -
> - @retval EFI_SUCCESS The clock is supplied successfully.
> - @retval Others The clock isn't supplied successfully.
> -
> -**/
> -EFI_STATUS
> -SdMmcHcInitClockFreq (
> - IN EFI_PCI_IO_PROTOCOL *PciIo,
> - IN UINT8 Slot,
> - IN UINT32 BaseClkFreq,
> - IN UINT16 ControllerVer
> - );
> -
> /**
> Supply SD/MMC card with maximum voltage at initialization.
>
> --
> 2.14.1.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock
2019-12-20 17:13 ` [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock Albecki, Mateusz
@ 2019-12-24 2:52 ` Wu, Hao A
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2019-12-24 2:52 UTC (permalink / raw)
To: Albecki, Mateusz, devel@edk2.groups.io
Cc: Marcin Wojtas, Gao, Zhichao, Gao, Liming, Ard Biesheuvel
> -----Original Message-----
> From: Albecki, Mateusz
> Sent: Saturday, December 21, 2019 1:13 AM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> Subject: [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start
> SD clock
>
> In SD card voltage switch flow we used to redo the
> entire internal clock setup after voltage switch.
> Since internal clock has already been setup this
> is wasting time on polling the internal clock stable.
> This commit changes it to only start the SD clock.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Let us wait to see if other reviewers have additional comments.
Best Regards,
Hao Wu
>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Marcin Wojtas <mw@semihalf.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
>
> Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 11 +++-----
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 33
> ++++++++++++++++++++----
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 15 +++++++++++
> 3 files changed, 47 insertions(+), 12 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index d63dc54e8c..b630daab76 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -1327,13 +1327,10 @@ SdCardIdentification (
> goto Error;
> }
>
> - //
> - // Restart the clock with first time parameters.
> - // NOTE: it is not required to actually restart the clock
> - // and go through internal clock setup again. Some time
> - // could be saved if we simply started the SD clock.
> - //
> - SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> + Status = SdMmcHcStartSdClock (PciIo, Slot);
> + if (EFI_ERROR (Status)) {
> + goto Error;
> + }
>
> gBS->Stall (1000);
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index f667264c5e..e7f2fac69b 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -758,6 +758,30 @@ SdMmcHcStopClock (
> return Status;
> }
>
> +/**
> + Start the SD clock.
> +
> + @param[in] PciIo The PCI IO protocol instance.
> + @param[in] Slot The slot number.
> +
> + @retval EFI_SUCCESS Succeeded to start the SD clock.
> + @rtval Others Failed to start the SD clock.
> +**/
> +EFI_STATUS
> +SdMmcHcStartSdClock (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN UINT8 Slot
> + )
> +{
> + UINT16 ClockCtrl;
> +
> + //
> + // Set SD Clock Enable in the Clock Control register to 1
> + //
> + ClockCtrl = BIT2;
> + return SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> (ClockCtrl), &ClockCtrl);
> +}
> +
> /**
> SD/MMC card clock supply.
>
> @@ -879,11 +903,10 @@ SdMmcHcClockSupply (
> return Status;
> }
>
> - //
> - // Set SD Clock Enable in the Clock Control register to 1
> - //
> - ClockCtrl = BIT2;
> - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> (ClockCtrl), &ClockCtrl);
> + Status = SdMmcHcStartSdClock (PciIo, Slot);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
>
> //
> // We don't notify the platform on first time setup to avoid changing
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index 826e851b04..4753bb6864 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -478,6 +478,21 @@ SdMmcHcStopClock (
> IN UINT8 Slot
> );
>
> +/**
> + Start the SD clock.
> +
> + @param[in] PciIo The PCI IO protocol instance.
> + @param[in] Slot The slot number.
> +
> + @retval EFI_SUCCESS Succeeded to start the SD clock.
> + @rtval Others Failed to start the SD clock.
> +**/
> +EFI_STATUS
> +SdMmcHcStartSdClock (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN UINT8 Slot
> + );
> +
> /**
> SD/MMC bus power control.
>
> --
> 2.14.1.windows.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
2019-12-24 2:52 ` Wu, Hao A
@ 2019-12-24 9:54 ` Ard Biesheuvel
2019-12-30 8:44 ` Marcin Wojtas
2019-12-31 14:49 ` Albecki, Mateusz
1 sibling, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2019-12-24 9:54 UTC (permalink / raw)
To: Wu, Hao A, Marcin Wojtas
Cc: Albecki, Mateusz, devel@edk2.groups.io, Gao, Zhichao, Gao, Liming
On Tue, 24 Dec 2019 at 03:52, Wu, Hao A <hao.a.wu@intel.com> wrote:
>
> > -----Original Message-----
> > From: Albecki, Mateusz
> > Sent: Saturday, December 21, 2019 1:13 AM
> > To: devel@edk2.groups.io
> > Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> > Subject: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after
> > SD clock start
>
>
> Hello Mateusz,
>
> Just a minor format comment, how about changing the subject to:
> MdeModulePkg/SdMmcPciHcDxe: Hook SwitchClockFreq after SD clock start
>
> If there is no other major concern from other reviewers for the patch, I will
> handle this when pushing the patch.
>
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
>
> Best Regards,
> Hao Wu
>
I don't have any objections to this patch, but I'd like Marcin to
confirm, given that this override callback is used on the Marvell
platforms.
>
> >
> > For eMMC modules we used to notify the platform about frequency
> > change only after sending CMD13 which meant that platform
> > might not get a chance to apply required post frequency
> > change fixes to get the clock stable. To fix this
> > notification has been moved to SdMmcHcClockSupply function
> > just after we start the SD clock. During first time setup
> > the notification won't be sent to avoid changing old behavior.
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Marcin Wojtas <mw@semihalf.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> >
> > Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> > ---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 28 ++----
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 +++++
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 107 +++++++++--
> > ----------
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 44 ---------
> > 5 files changed, 81 insertions(+), 142 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > index 082904ccc5..776c0e796c 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > @@ -727,7 +727,7 @@ EmmcSwitchBusTiming (
> > //
> > // Convert the clock freq unit from MHz to KHz.
> > //
> > - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> > >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> > + Status = SdMmcHcClockSupply (Private, Slot, BusTiming, FALSE, ClockFreq *
> > 1000);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > @@ -745,24 +745,6 @@ EmmcSwitchBusTiming (
> > return EFI_DEVICE_ERROR;
> > }
> >
> > - if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > - Status = mOverride->NotifyPhase (
> > - Private->ControllerHandle,
> > - Slot,
> > - EdkiiSdMmcSwitchClockFreqPost,
> > - &BusTiming
> > - );
> > - if (EFI_ERROR (Status)) {
> > - DEBUG ((
> > - DEBUG_ERROR,
> > - "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > - __FUNCTION__,
> > - Status
> > - ));
> > - return Status;
> > - }
> > - }
> > -
> > return Status;
> > }
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > index 336baade9e..d63dc54e8c 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > @@ -1145,29 +1145,11 @@ SdCardSetBusMode (
> > return Status;
> > }
> >
> > - Status = SdMmcHcClockSupply (PciIo, Slot, BusMode.ClockFreq * 1000,
> > Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> > + Status = SdMmcHcClockSupply (Private, Slot, BusMode.BusTiming, FALSE,
> > BusMode.ClockFreq * 1000);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> >
> > - if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > - Status = mOverride->NotifyPhase (
> > - Private->ControllerHandle,
> > - Slot,
> > - EdkiiSdMmcSwitchClockFreqPost,
> > - &BusMode.BusTiming
> > - );
> > - if (EFI_ERROR (Status)) {
> > - DEBUG ((
> > - DEBUG_ERROR,
> > - "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > - __FUNCTION__,
> > - Status
> > - ));
> > - return Status;
> > - }
> > - }
> > -
> > if ((BusMode.BusTiming == SdMmcUhsSdr104) || ((BusMode.BusTiming ==
> > SdMmcUhsSdr50) && (Capability->TuningSDR50 != 0))) {
> > Status = SdCardTuningClock (PciIo, PassThru, Slot);
> > if (EFI_ERROR (Status)) {
> > @@ -1345,7 +1327,13 @@ SdCardIdentification (
> > goto Error;
> > }
> >
> > - SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], Private-
> > >ControllerVersion[Slot]);
> > + //
> > + // Restart the clock with first time parameters.
> > + // NOTE: it is not required to actually restart the clock
> > + // and go through internal clock setup again. Some time
> > + // could be saved if we simply started the SD clock.
> > + //
> > + SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> >
> > gBS->Stall (1000);
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > index c29e48767e..0304960132 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > @@ -796,6 +796,30 @@ SdCardIdentification (
> > IN UINT8 Slot
> > );
> >
> > +/**
> > + SD/MMC card clock supply.
> > +
> > + Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> > +
> > + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
> > instance.
> > + @param[in] Slot The slot number of the SD card to send the command
> > to.
> > + @param[in] BusTiming BusTiming at which the frequency change is done.
> > + @param[in] FirstTimeSetup Flag to indicate whether the clock is being setup
> > for the first time.
> > + @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> > +
> > + @retval EFI_SUCCESS The clock is supplied successfully.
> > + @retval Others The clock isn't supplied successfully.
> > +
> > +**/
> > +EFI_STATUS
> > +SdMmcHcClockSupply (
> > + IN SD_MMC_HC_PRIVATE_DATA *Private,
> > + IN UINT8 Slot,
> > + IN SD_MMC_BUS_MODE BusTiming,
> > + IN BOOLEAN FirstTimeSetup,
> > + IN UINT64 ClockFreq
> > + );
> > +
> > /**
> > Software reset the specified SD/MMC host controller.
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index b9d04e0f17..f667264c5e 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -763,11 +763,11 @@ SdMmcHcStopClock (
> >
> > Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> >
> > - @param[in] PciIo The PCI IO protocol instance.
> > - @param[in] Slot The slot number of the SD card to send the command
> > to.
> > - @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > MHz.
> > - @param[in] ControllerVer The version of host controller.
> > + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
> > instance.
> > + @param[in] Slot The slot number of the SD card to send the command
> > to.
> > + @param[in] BusTiming BusTiming at which the frequency change is done.
> > + @param[in] FirstTimeSetup Flag to indicate whether the clock is being setup
> > for the first time.
> > + @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> >
> > @retval EFI_SUCCESS The clock is supplied successfully.
> > @retval Others The clock isn't supplied successfully.
> > @@ -775,11 +775,11 @@ SdMmcHcStopClock (
> > **/
> > EFI_STATUS
> > SdMmcHcClockSupply (
> > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > - IN UINT8 Slot,
> > - IN UINT64 ClockFreq,
> > - IN UINT32 BaseClkFreq,
> > - IN UINT16 ControllerVer
> > + IN SD_MMC_HC_PRIVATE_DATA *Private,
> > + IN UINT8 Slot,
> > + IN SD_MMC_BUS_MODE BusTiming,
> > + IN BOOLEAN FirstTimeSetup,
> > + IN UINT64 ClockFreq
> > )
> > {
> > EFI_STATUS Status;
> > @@ -787,13 +787,15 @@ SdMmcHcClockSupply (
> > UINT32 Divisor;
> > UINT32 Remainder;
> > UINT16 ClockCtrl;
> > + UINT32 BaseClkFreq;
> > + UINT16 ControllerVer;
> > + EFI_PCI_IO_PROTOCOL *PciIo;
> >
> > - //
> > - // Calculate a divisor for SD clock frequency
> > - //
> > - ASSERT (BaseClkFreq != 0);
> > + PciIo = Private->PciIo;
> > + BaseClkFreq = Private->BaseClkFreq[Slot];
> > + ControllerVer = Private->ControllerVersion[Slot];
> >
> > - if (ClockFreq == 0) {
> > + if (BaseClkFreq == 0 || ClockFreq == 0) {
> > return EFI_INVALID_PARAMETER;
> > }
> >
> > @@ -883,6 +885,29 @@ SdMmcHcClockSupply (
> > ClockCtrl = BIT2;
> > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> > (ClockCtrl), &ClockCtrl);
> >
> > + //
> > + // We don't notify the platform on first time setup to avoid changing
> > + // legacy behavior. During first time setup we also don't know what type
> > + // of the card slot it is and which enum value of BusTiming applies.
> > + //
> > + if (!FirstTimeSetup && mOverride != NULL && mOverride->NotifyPhase !=
> > NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcSwitchClockFreqPost,
> > + &BusTiming
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > + }
> > +
> > return Status;
> > }
> >
> > @@ -1038,49 +1063,6 @@ SdMmcHcInitV4Enhancements (
> > return EFI_SUCCESS;
> > }
> >
> > -/**
> > - Supply SD/MMC card with lowest clock frequency at initialization.
> > -
> > - @param[in] PciIo The PCI IO protocol instance.
> > - @param[in] Slot The slot number of the SD card to send the command
> > to.
> > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > MHz.
> > - @param[in] ControllerVer The version of host controller.
> > -
> > - @retval EFI_SUCCESS The clock is supplied successfully.
> > - @retval Others The clock isn't supplied successfully.
> > -
> > -**/
> > -EFI_STATUS
> > -SdMmcHcInitClockFreq (
> > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > - IN UINT8 Slot,
> > - IN UINT32 BaseClkFreq,
> > - IN UINT16 ControllerVer
> > - )
> > -{
> > - EFI_STATUS Status;
> > - UINT32 InitFreq;
> > -
> > - //
> > - // According to SDHCI specification ver. 4.2, BaseClkFreq field value of
> > - // the Capability Register 1 can be zero, which means a need for obtaining
> > - // the clock frequency via another method. Fail in case it is not updated
> > - // by SW at this point.
> > - //
> > - if (BaseClkFreq == 0) {
> > - //
> > - // Don't support get Base Clock Frequency information via another method
> > - //
> > - return EFI_UNSUPPORTED;
> > - }
> > - //
> > - // Supply 400KHz clock frequency at initialization phase.
> > - //
> > - InitFreq = 400;
> > - Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq,
> > ControllerVer);
> > - return Status;
> > -}
> > -
> > /**
> > Supply SD/MMC card with maximum voltage at initialization.
> >
> > @@ -1216,7 +1198,14 @@ SdMmcHcInitHost (
> > return Status;
> > }
> >
> > - Status = SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot],
> > Private->ControllerVersion[Slot]);
> > + //
> > + // Perform first time clock setup with 400 KHz frequency.
> > + // We send the 0 as the BusTiming value because at this time
> > + // we still do not know the slot type and which enum value will apply.
> > + // Since it is a first time setup SdMmcHcClockSupply won't notify
> > + // the platofrm driver anyway so it doesn't matter.
> > + //
> > + Status = SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > index 088c70451c..826e851b04 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > @@ -478,30 +478,6 @@ SdMmcHcStopClock (
> > IN UINT8 Slot
> > );
> >
> > -/**
> > - SD/MMC card clock supply.
> > -
> > - Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> > -
> > - @param[in] PciIo The PCI IO protocol instance.
> > - @param[in] Slot The slot number of the SD card to send the command
> > to.
> > - @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > MHz.
> > - @param[in] ControllerVer The version of host controller.
> > -
> > - @retval EFI_SUCCESS The clock is supplied successfully.
> > - @retval Others The clock isn't supplied successfully.
> > -
> > -**/
> > -EFI_STATUS
> > -SdMmcHcClockSupply (
> > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > - IN UINT8 Slot,
> > - IN UINT64 ClockFreq,
> > - IN UINT32 BaseClkFreq,
> > - IN UINT16 ControllerVer
> > - );
> > -
> > /**
> > SD/MMC bus power control.
> >
> > @@ -542,26 +518,6 @@ SdMmcHcSetBusWidth (
> > IN UINT16 BusWidth
> > );
> >
> > -/**
> > - Supply SD/MMC card with lowest clock frequency at initialization.
> > -
> > - @param[in] PciIo The PCI IO protocol instance.
> > - @param[in] Slot The slot number of the SD card to send the command
> > to.
> > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > MHz.
> > - @param[in] ControllerVer The version of host controller.
> > -
> > - @retval EFI_SUCCESS The clock is supplied successfully.
> > - @retval Others The clock isn't supplied successfully.
> > -
> > -**/
> > -EFI_STATUS
> > -SdMmcHcInitClockFreq (
> > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > - IN UINT8 Slot,
> > - IN UINT32 BaseClkFreq,
> > - IN UINT16 ControllerVer
> > - );
> > -
> > /**
> > Supply SD/MMC card with maximum voltage at initialization.
> >
> > --
> > 2.14.1.windows.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
2019-12-24 9:54 ` Ard Biesheuvel
@ 2019-12-30 8:44 ` Marcin Wojtas
0 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2019-12-30 8:44 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Wu, Hao A, Albecki, Mateusz, devel@edk2.groups.io, Gao, Zhichao,
Gao, Liming
Hi,
wt., 24 gru 2019 o 10:54 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> On Tue, 24 Dec 2019 at 03:52, Wu, Hao A <hao.a.wu@intel.com> wrote:
> >
> > > -----Original Message-----
> > > From: Albecki, Mateusz
> > > Sent: Saturday, December 21, 2019 1:13 AM
> > > To: devel@edk2.groups.io
> > > Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> > > Subject: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after
> > > SD clock start
> >
> >
> > Hello Mateusz,
> >
> > Just a minor format comment, how about changing the subject to:
> > MdeModulePkg/SdMmcPciHcDxe: Hook SwitchClockFreq after SD clock start
> >
> > If there is no other major concern from other reviewers for the patch, I will
> > handle this when pushing the patch.
> >
> > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> >
> > Best Regards,
> > Hao Wu
> >
>
> I don't have any objections to this patch, but I'd like Marcin to
> confirm, given that this override callback is used on the Marvell
> platforms.
>
I'll check the branch today or tomorrow.
Best regards,
Marcin
> >
> > >
> > > For eMMC modules we used to notify the platform about frequency
> > > change only after sending CMD13 which meant that platform
> > > might not get a chance to apply required post frequency
> > > change fixes to get the clock stable. To fix this
> > > notification has been moved to SdMmcHcClockSupply function
> > > just after we start the SD clock. During first time setup
> > > the notification won't be sent to avoid changing old behavior.
> > >
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Marcin Wojtas <mw@semihalf.com>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > >
> > > Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> > > ---
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +---
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 28 ++----
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 +++++
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 107 +++++++++--
> > > ----------
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 44 ---------
> > > 5 files changed, 81 insertions(+), 142 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > > index 082904ccc5..776c0e796c 100644
> > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > > @@ -727,7 +727,7 @@ EmmcSwitchBusTiming (
> > > //
> > > // Convert the clock freq unit from MHz to KHz.
> > > //
> > > - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private-
> > > >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> > > + Status = SdMmcHcClockSupply (Private, Slot, BusTiming, FALSE, ClockFreq *
> > > 1000);
> > > if (EFI_ERROR (Status)) {
> > > return Status;
> > > }
> > > @@ -745,24 +745,6 @@ EmmcSwitchBusTiming (
> > > return EFI_DEVICE_ERROR;
> > > }
> > >
> > > - if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > > - Status = mOverride->NotifyPhase (
> > > - Private->ControllerHandle,
> > > - Slot,
> > > - EdkiiSdMmcSwitchClockFreqPost,
> > > - &BusTiming
> > > - );
> > > - if (EFI_ERROR (Status)) {
> > > - DEBUG ((
> > > - DEBUG_ERROR,
> > > - "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > > - __FUNCTION__,
> > > - Status
> > > - ));
> > > - return Status;
> > > - }
> > > - }
> > > -
> > > return Status;
> > > }
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > > index 336baade9e..d63dc54e8c 100644
> > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > > @@ -1145,29 +1145,11 @@ SdCardSetBusMode (
> > > return Status;
> > > }
> > >
> > > - Status = SdMmcHcClockSupply (PciIo, Slot, BusMode.ClockFreq * 1000,
> > > Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> > > + Status = SdMmcHcClockSupply (Private, Slot, BusMode.BusTiming, FALSE,
> > > BusMode.ClockFreq * 1000);
> > > if (EFI_ERROR (Status)) {
> > > return Status;
> > > }
> > >
> > > - if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > > - Status = mOverride->NotifyPhase (
> > > - Private->ControllerHandle,
> > > - Slot,
> > > - EdkiiSdMmcSwitchClockFreqPost,
> > > - &BusMode.BusTiming
> > > - );
> > > - if (EFI_ERROR (Status)) {
> > > - DEBUG ((
> > > - DEBUG_ERROR,
> > > - "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > > - __FUNCTION__,
> > > - Status
> > > - ));
> > > - return Status;
> > > - }
> > > - }
> > > -
> > > if ((BusMode.BusTiming == SdMmcUhsSdr104) || ((BusMode.BusTiming ==
> > > SdMmcUhsSdr50) && (Capability->TuningSDR50 != 0))) {
> > > Status = SdCardTuningClock (PciIo, PassThru, Slot);
> > > if (EFI_ERROR (Status)) {
> > > @@ -1345,7 +1327,13 @@ SdCardIdentification (
> > > goto Error;
> > > }
> > >
> > > - SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], Private-
> > > >ControllerVersion[Slot]);
> > > + //
> > > + // Restart the clock with first time parameters.
> > > + // NOTE: it is not required to actually restart the clock
> > > + // and go through internal clock setup again. Some time
> > > + // could be saved if we simply started the SD clock.
> > > + //
> > > + SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> > >
> > > gBS->Stall (1000);
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > > index c29e48767e..0304960132 100644
> > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > > @@ -796,6 +796,30 @@ SdCardIdentification (
> > > IN UINT8 Slot
> > > );
> > >
> > > +/**
> > > + SD/MMC card clock supply.
> > > +
> > > + Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> > > +
> > > + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
> > > instance.
> > > + @param[in] Slot The slot number of the SD card to send the command
> > > to.
> > > + @param[in] BusTiming BusTiming at which the frequency change is done.
> > > + @param[in] FirstTimeSetup Flag to indicate whether the clock is being setup
> > > for the first time.
> > > + @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> > > +
> > > + @retval EFI_SUCCESS The clock is supplied successfully.
> > > + @retval Others The clock isn't supplied successfully.
> > > +
> > > +**/
> > > +EFI_STATUS
> > > +SdMmcHcClockSupply (
> > > + IN SD_MMC_HC_PRIVATE_DATA *Private,
> > > + IN UINT8 Slot,
> > > + IN SD_MMC_BUS_MODE BusTiming,
> > > + IN BOOLEAN FirstTimeSetup,
> > > + IN UINT64 ClockFreq
> > > + );
> > > +
> > > /**
> > > Software reset the specified SD/MMC host controller.
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > > index b9d04e0f17..f667264c5e 100644
> > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > > @@ -763,11 +763,11 @@ SdMmcHcStopClock (
> > >
> > > Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> > >
> > > - @param[in] PciIo The PCI IO protocol instance.
> > > - @param[in] Slot The slot number of the SD card to send the command
> > > to.
> > > - @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> > > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > > MHz.
> > > - @param[in] ControllerVer The version of host controller.
> > > + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
> > > instance.
> > > + @param[in] Slot The slot number of the SD card to send the command
> > > to.
> > > + @param[in] BusTiming BusTiming at which the frequency change is done.
> > > + @param[in] FirstTimeSetup Flag to indicate whether the clock is being setup
> > > for the first time.
> > > + @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> > >
> > > @retval EFI_SUCCESS The clock is supplied successfully.
> > > @retval Others The clock isn't supplied successfully.
> > > @@ -775,11 +775,11 @@ SdMmcHcStopClock (
> > > **/
> > > EFI_STATUS
> > > SdMmcHcClockSupply (
> > > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > > - IN UINT8 Slot,
> > > - IN UINT64 ClockFreq,
> > > - IN UINT32 BaseClkFreq,
> > > - IN UINT16 ControllerVer
> > > + IN SD_MMC_HC_PRIVATE_DATA *Private,
> > > + IN UINT8 Slot,
> > > + IN SD_MMC_BUS_MODE BusTiming,
> > > + IN BOOLEAN FirstTimeSetup,
> > > + IN UINT64 ClockFreq
> > > )
> > > {
> > > EFI_STATUS Status;
> > > @@ -787,13 +787,15 @@ SdMmcHcClockSupply (
> > > UINT32 Divisor;
> > > UINT32 Remainder;
> > > UINT16 ClockCtrl;
> > > + UINT32 BaseClkFreq;
> > > + UINT16 ControllerVer;
> > > + EFI_PCI_IO_PROTOCOL *PciIo;
> > >
> > > - //
> > > - // Calculate a divisor for SD clock frequency
> > > - //
> > > - ASSERT (BaseClkFreq != 0);
> > > + PciIo = Private->PciIo;
> > > + BaseClkFreq = Private->BaseClkFreq[Slot];
> > > + ControllerVer = Private->ControllerVersion[Slot];
> > >
> > > - if (ClockFreq == 0) {
> > > + if (BaseClkFreq == 0 || ClockFreq == 0) {
> > > return EFI_INVALID_PARAMETER;
> > > }
> > >
> > > @@ -883,6 +885,29 @@ SdMmcHcClockSupply (
> > > ClockCtrl = BIT2;
> > > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> > > (ClockCtrl), &ClockCtrl);
> > >
> > > + //
> > > + // We don't notify the platform on first time setup to avoid changing
> > > + // legacy behavior. During first time setup we also don't know what type
> > > + // of the card slot it is and which enum value of BusTiming applies.
> > > + //
> > > + if (!FirstTimeSetup && mOverride != NULL && mOverride->NotifyPhase !=
> > > NULL) {
> > > + Status = mOverride->NotifyPhase (
> > > + Private->ControllerHandle,
> > > + Slot,
> > > + EdkiiSdMmcSwitchClockFreqPost,
> > > + &BusTiming
> > > + );
> > > + if (EFI_ERROR (Status)) {
> > > + DEBUG ((
> > > + DEBUG_ERROR,
> > > + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > > + __FUNCTION__,
> > > + Status
> > > + ));
> > > + return Status;
> > > + }
> > > + }
> > > +
> > > return Status;
> > > }
> > >
> > > @@ -1038,49 +1063,6 @@ SdMmcHcInitV4Enhancements (
> > > return EFI_SUCCESS;
> > > }
> > >
> > > -/**
> > > - Supply SD/MMC card with lowest clock frequency at initialization.
> > > -
> > > - @param[in] PciIo The PCI IO protocol instance.
> > > - @param[in] Slot The slot number of the SD card to send the command
> > > to.
> > > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > > MHz.
> > > - @param[in] ControllerVer The version of host controller.
> > > -
> > > - @retval EFI_SUCCESS The clock is supplied successfully.
> > > - @retval Others The clock isn't supplied successfully.
> > > -
> > > -**/
> > > -EFI_STATUS
> > > -SdMmcHcInitClockFreq (
> > > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > > - IN UINT8 Slot,
> > > - IN UINT32 BaseClkFreq,
> > > - IN UINT16 ControllerVer
> > > - )
> > > -{
> > > - EFI_STATUS Status;
> > > - UINT32 InitFreq;
> > > -
> > > - //
> > > - // According to SDHCI specification ver. 4.2, BaseClkFreq field value of
> > > - // the Capability Register 1 can be zero, which means a need for obtaining
> > > - // the clock frequency via another method. Fail in case it is not updated
> > > - // by SW at this point.
> > > - //
> > > - if (BaseClkFreq == 0) {
> > > - //
> > > - // Don't support get Base Clock Frequency information via another method
> > > - //
> > > - return EFI_UNSUPPORTED;
> > > - }
> > > - //
> > > - // Supply 400KHz clock frequency at initialization phase.
> > > - //
> > > - InitFreq = 400;
> > > - Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq,
> > > ControllerVer);
> > > - return Status;
> > > -}
> > > -
> > > /**
> > > Supply SD/MMC card with maximum voltage at initialization.
> > >
> > > @@ -1216,7 +1198,14 @@ SdMmcHcInitHost (
> > > return Status;
> > > }
> > >
> > > - Status = SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot],
> > > Private->ControllerVersion[Slot]);
> > > + //
> > > + // Perform first time clock setup with 400 KHz frequency.
> > > + // We send the 0 as the BusTiming value because at this time
> > > + // we still do not know the slot type and which enum value will apply.
> > > + // Since it is a first time setup SdMmcHcClockSupply won't notify
> > > + // the platofrm driver anyway so it doesn't matter.
> > > + //
> > > + Status = SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> > > if (EFI_ERROR (Status)) {
> > > return Status;
> > > }
> > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > > index 088c70451c..826e851b04 100644
> > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > > @@ -478,30 +478,6 @@ SdMmcHcStopClock (
> > > IN UINT8 Slot
> > > );
> > >
> > > -/**
> > > - SD/MMC card clock supply.
> > > -
> > > - Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> > > -
> > > - @param[in] PciIo The PCI IO protocol instance.
> > > - @param[in] Slot The slot number of the SD card to send the command
> > > to.
> > > - @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> > > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > > MHz.
> > > - @param[in] ControllerVer The version of host controller.
> > > -
> > > - @retval EFI_SUCCESS The clock is supplied successfully.
> > > - @retval Others The clock isn't supplied successfully.
> > > -
> > > -**/
> > > -EFI_STATUS
> > > -SdMmcHcClockSupply (
> > > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > > - IN UINT8 Slot,
> > > - IN UINT64 ClockFreq,
> > > - IN UINT32 BaseClkFreq,
> > > - IN UINT16 ControllerVer
> > > - );
> > > -
> > > /**
> > > SD/MMC bus power control.
> > >
> > > @@ -542,26 +518,6 @@ SdMmcHcSetBusWidth (
> > > IN UINT16 BusWidth
> > > );
> > >
> > > -/**
> > > - Supply SD/MMC card with lowest clock frequency at initialization.
> > > -
> > > - @param[in] PciIo The PCI IO protocol instance.
> > > - @param[in] Slot The slot number of the SD card to send the command
> > > to.
> > > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > > MHz.
> > > - @param[in] ControllerVer The version of host controller.
> > > -
> > > - @retval EFI_SUCCESS The clock is supplied successfully.
> > > - @retval Others The clock isn't supplied successfully.
> > > -
> > > -**/
> > > -EFI_STATUS
> > > -SdMmcHcInitClockFreq (
> > > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > > - IN UINT8 Slot,
> > > - IN UINT32 BaseClkFreq,
> > > - IN UINT16 ControllerVer
> > > - );
> > > -
> > > /**
> > > Supply SD/MMC card with maximum voltage at initialization.
> > >
> > > --
> > > 2.14.1.windows.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
2019-12-24 2:52 ` Wu, Hao A
2019-12-24 9:54 ` Ard Biesheuvel
@ 2019-12-31 14:49 ` Albecki, Mateusz
1 sibling, 0 replies; 12+ messages in thread
From: Albecki, Mateusz @ 2019-12-31 14:49 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io
Cc: Marcin Wojtas, Gao, Zhichao, Gao, Liming, Ard Biesheuvel
No problem with new subject from my side.
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Tuesday, December 24, 2019 3:52 AM
> To: Albecki, Mateusz <mateusz.albecki@intel.com>; devel@edk2.groups.io
> Cc: Marcin Wojtas <mw@semihalf.com>; Gao, Zhichao
> <zhichao.gao@intel.com>; Gao, Liming <liming.gao@intel.com>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: RE: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq
> after SD clock start
>
> > -----Original Message-----
> > From: Albecki, Mateusz
> > Sent: Saturday, December 21, 2019 1:13 AM
> > To: devel@edk2.groups.io
> > Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao,
> > Liming
> > Subject: [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq
> > after SD clock start
>
>
> Hello Mateusz,
>
> Just a minor format comment, how about changing the subject to:
> MdeModulePkg/SdMmcPciHcDxe: Hook SwitchClockFreq after SD clock start
>
> If there is no other major concern from other reviewers for the patch, I will
> handle this when pushing the patch.
>
> Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
>
> Best Regards,
> Hao Wu
>
>
> >
> > For eMMC modules we used to notify the platform about frequency
> change
> > only after sending CMD13 which meant that platform might not get a
> > chance to apply required post frequency change fixes to get the clock
> > stable. To fix this notification has been moved to SdMmcHcClockSupply
> > function just after we start the SD clock. During first time setup the
> > notification won't be sent to avoid changing old behavior.
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Marcin Wojtas <mw@semihalf.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> >
> > Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> > ---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 28 ++----
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 +++++
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 107
> +++++++++--
> > ----------
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 44 ---------
> > 5 files changed, 81 insertions(+), 142 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > index 082904ccc5..776c0e796c 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > @@ -727,7 +727,7 @@ EmmcSwitchBusTiming (
> > //
> > // Convert the clock freq unit from MHz to KHz.
> > //
> > - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000,
> > Private-
> > >BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> > + Status = SdMmcHcClockSupply (Private, Slot, BusTiming, FALSE,
> > + ClockFreq *
> > 1000);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > @@ -745,24 +745,6 @@ EmmcSwitchBusTiming (
> > return EFI_DEVICE_ERROR;
> > }
> >
> > - if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > - Status = mOverride->NotifyPhase (
> > - Private->ControllerHandle,
> > - Slot,
> > - EdkiiSdMmcSwitchClockFreqPost,
> > - &BusTiming
> > - );
> > - if (EFI_ERROR (Status)) {
> > - DEBUG ((
> > - DEBUG_ERROR,
> > - "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > - __FUNCTION__,
> > - Status
> > - ));
> > - return Status;
> > - }
> > - }
> > -
> > return Status;
> > }
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > index 336baade9e..d63dc54e8c 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > @@ -1145,29 +1145,11 @@ SdCardSetBusMode (
> > return Status;
> > }
> >
> > - Status = SdMmcHcClockSupply (PciIo, Slot, BusMode.ClockFreq * 1000,
> > Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]);
> > + Status = SdMmcHcClockSupply (Private, Slot, BusMode.BusTiming,
> > + FALSE,
> > BusMode.ClockFreq * 1000);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> >
> > - if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > - Status = mOverride->NotifyPhase (
> > - Private->ControllerHandle,
> > - Slot,
> > - EdkiiSdMmcSwitchClockFreqPost,
> > - &BusMode.BusTiming
> > - );
> > - if (EFI_ERROR (Status)) {
> > - DEBUG ((
> > - DEBUG_ERROR,
> > - "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > - __FUNCTION__,
> > - Status
> > - ));
> > - return Status;
> > - }
> > - }
> > -
> > if ((BusMode.BusTiming == SdMmcUhsSdr104) || ((BusMode.BusTiming
> ==
> > SdMmcUhsSdr50) && (Capability->TuningSDR50 != 0))) {
> > Status = SdCardTuningClock (PciIo, PassThru, Slot);
> > if (EFI_ERROR (Status)) {
> > @@ -1345,7 +1327,13 @@ SdCardIdentification (
> > goto Error;
> > }
> >
> > - SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot], Private-
> > >ControllerVersion[Slot]);
> > + //
> > + // Restart the clock with first time parameters.
> > + // NOTE: it is not required to actually restart the clock
> > + // and go through internal clock setup again. Some time
> > + // could be saved if we simply started the SD clock.
> > + //
> > + SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> >
> > gBS->Stall (1000);
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > index c29e48767e..0304960132 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> > @@ -796,6 +796,30 @@ SdCardIdentification (
> > IN UINT8 Slot
> > );
> >
> > +/**
> > + SD/MMC card clock supply.
> > +
> > + Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> > +
> > + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
> > instance.
> > + @param[in] Slot The slot number of the SD card to send the
> command
> > to.
> > + @param[in] BusTiming BusTiming at which the frequency change is
> done.
> > + @param[in] FirstTimeSetup Flag to indicate whether the clock is
> > + being setup
> > for the first time.
> > + @param[in] ClockFreq The max clock frequency to be set. The unit is
> KHz.
> > +
> > + @retval EFI_SUCCESS The clock is supplied successfully.
> > + @retval Others The clock isn't supplied successfully.
> > +
> > +**/
> > +EFI_STATUS
> > +SdMmcHcClockSupply (
> > + IN SD_MMC_HC_PRIVATE_DATA *Private,
> > + IN UINT8 Slot,
> > + IN SD_MMC_BUS_MODE BusTiming,
> > + IN BOOLEAN FirstTimeSetup,
> > + IN UINT64 ClockFreq
> > + );
> > +
> > /**
> > Software reset the specified SD/MMC host controller.
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index b9d04e0f17..f667264c5e 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -763,11 +763,11 @@ SdMmcHcStopClock (
> >
> > Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> >
> > - @param[in] PciIo The PCI IO protocol instance.
> > - @param[in] Slot The slot number of the SD card to send the
> command
> > to.
> > - @param[in] ClockFreq The max clock frequency to be set. The unit is
> KHz.
> > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > MHz.
> > - @param[in] ControllerVer The version of host controller.
> > + @param[in] Private A pointer to the SD_MMC_HC_PRIVATE_DATA
> > instance.
> > + @param[in] Slot The slot number of the SD card to send the
> command
> > to.
> > + @param[in] BusTiming BusTiming at which the frequency change is
> done.
> > + @param[in] FirstTimeSetup Flag to indicate whether the clock is
> > + being setup
> > for the first time.
> > + @param[in] ClockFreq The max clock frequency to be set. The unit is
> KHz.
> >
> > @retval EFI_SUCCESS The clock is supplied successfully.
> > @retval Others The clock isn't supplied successfully.
> > @@ -775,11 +775,11 @@ SdMmcHcStopClock ( **/ EFI_STATUS
> > SdMmcHcClockSupply (
> > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > - IN UINT8 Slot,
> > - IN UINT64 ClockFreq,
> > - IN UINT32 BaseClkFreq,
> > - IN UINT16 ControllerVer
> > + IN SD_MMC_HC_PRIVATE_DATA *Private,
> > + IN UINT8 Slot,
> > + IN SD_MMC_BUS_MODE BusTiming,
> > + IN BOOLEAN FirstTimeSetup,
> > + IN UINT64 ClockFreq
> > )
> > {
> > EFI_STATUS Status;
> > @@ -787,13 +787,15 @@ SdMmcHcClockSupply (
> > UINT32 Divisor;
> > UINT32 Remainder;
> > UINT16 ClockCtrl;
> > + UINT32 BaseClkFreq;
> > + UINT16 ControllerVer;
> > + EFI_PCI_IO_PROTOCOL *PciIo;
> >
> > - //
> > - // Calculate a divisor for SD clock frequency
> > - //
> > - ASSERT (BaseClkFreq != 0);
> > + PciIo = Private->PciIo;
> > + BaseClkFreq = Private->BaseClkFreq[Slot]; ControllerVer =
> > + Private->ControllerVersion[Slot];
> >
> > - if (ClockFreq == 0) {
> > + if (BaseClkFreq == 0 || ClockFreq == 0) {
> > return EFI_INVALID_PARAMETER;
> > }
> >
> > @@ -883,6 +885,29 @@ SdMmcHcClockSupply (
> > ClockCtrl = BIT2;
> > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> > (ClockCtrl), &ClockCtrl);
> >
> > + //
> > + // We don't notify the platform on first time setup to avoid
> > + changing // legacy behavior. During first time setup we also don't
> > + know what type // of the card slot it is and which enum value of
> BusTiming applies.
> > + //
> > + if (!FirstTimeSetup && mOverride != NULL && mOverride->NotifyPhase
> > + !=
> > NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcSwitchClockFreqPost,
> > + &BusTiming
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > + }
> > +
> > return Status;
> > }
> >
> > @@ -1038,49 +1063,6 @@ SdMmcHcInitV4Enhancements (
> > return EFI_SUCCESS;
> > }
> >
> > -/**
> > - Supply SD/MMC card with lowest clock frequency at initialization.
> > -
> > - @param[in] PciIo The PCI IO protocol instance.
> > - @param[in] Slot The slot number of the SD card to send the
> command
> > to.
> > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > MHz.
> > - @param[in] ControllerVer The version of host controller.
> > -
> > - @retval EFI_SUCCESS The clock is supplied successfully.
> > - @retval Others The clock isn't supplied successfully.
> > -
> > -**/
> > -EFI_STATUS
> > -SdMmcHcInitClockFreq (
> > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > - IN UINT8 Slot,
> > - IN UINT32 BaseClkFreq,
> > - IN UINT16 ControllerVer
> > - )
> > -{
> > - EFI_STATUS Status;
> > - UINT32 InitFreq;
> > -
> > - //
> > - // According to SDHCI specification ver. 4.2, BaseClkFreq field
> > value of
> > - // the Capability Register 1 can be zero, which means a need for
> > obtaining
> > - // the clock frequency via another method. Fail in case it is not
> > updated
> > - // by SW at this point.
> > - //
> > - if (BaseClkFreq == 0) {
> > - //
> > - // Don't support get Base Clock Frequency information via another
> method
> > - //
> > - return EFI_UNSUPPORTED;
> > - }
> > - //
> > - // Supply 400KHz clock frequency at initialization phase.
> > - //
> > - InitFreq = 400;
> > - Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq,
> > ControllerVer);
> > - return Status;
> > -}
> > -
> > /**
> > Supply SD/MMC card with maximum voltage at initialization.
> >
> > @@ -1216,7 +1198,14 @@ SdMmcHcInitHost (
> > return Status;
> > }
> >
> > - Status = SdMmcHcInitClockFreq (PciIo, Slot,
> > Private->BaseClkFreq[Slot],
> > Private->ControllerVersion[Slot]);
> > + //
> > + // Perform first time clock setup with 400 KHz frequency.
> > + // We send the 0 as the BusTiming value because at this time // we
> > + still do not know the slot type and which enum value will apply.
> > + // Since it is a first time setup SdMmcHcClockSupply won't notify
> > + // the platofrm driver anyway so it doesn't matter.
> > + //
> > + Status = SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > index 088c70451c..826e851b04 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > @@ -478,30 +478,6 @@ SdMmcHcStopClock (
> > IN UINT8 Slot
> > );
> >
> > -/**
> > - SD/MMC card clock supply.
> > -
> > - Refer to SD Host Controller Simplified spec 3.0 Section 3.2.1 for details.
> > -
> > - @param[in] PciIo The PCI IO protocol instance.
> > - @param[in] Slot The slot number of the SD card to send the
> command
> > to.
> > - @param[in] ClockFreq The max clock frequency to be set. The unit is
> KHz.
> > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > MHz.
> > - @param[in] ControllerVer The version of host controller.
> > -
> > - @retval EFI_SUCCESS The clock is supplied successfully.
> > - @retval Others The clock isn't supplied successfully.
> > -
> > -**/
> > -EFI_STATUS
> > -SdMmcHcClockSupply (
> > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > - IN UINT8 Slot,
> > - IN UINT64 ClockFreq,
> > - IN UINT32 BaseClkFreq,
> > - IN UINT16 ControllerVer
> > - );
> > -
> > /**
> > SD/MMC bus power control.
> >
> > @@ -542,26 +518,6 @@ SdMmcHcSetBusWidth (
> > IN UINT16 BusWidth
> > );
> >
> > -/**
> > - Supply SD/MMC card with lowest clock frequency at initialization.
> > -
> > - @param[in] PciIo The PCI IO protocol instance.
> > - @param[in] Slot The slot number of the SD card to send the
> command
> > to.
> > - @param[in] BaseClkFreq The base clock frequency of host controller in
> > MHz.
> > - @param[in] ControllerVer The version of host controller.
> > -
> > - @retval EFI_SUCCESS The clock is supplied successfully.
> > - @retval Others The clock isn't supplied successfully.
> > -
> > -**/
> > -EFI_STATUS
> > -SdMmcHcInitClockFreq (
> > - IN EFI_PCI_IO_PROTOCOL *PciIo,
> > - IN UINT8 Slot,
> > - IN UINT32 BaseClkFreq,
> > - IN UINT16 ControllerVer
> > - );
> > -
> > /**
> > Supply SD/MMC card with maximum voltage at initialization.
> >
> > --
> > 2.14.1.windows.1
>
--------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13
2019-12-24 2:51 ` [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13 Wu, Hao A
@ 2020-01-03 11:04 ` Marcin Wojtas
2020-01-06 5:18 ` Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Marcin Wojtas @ 2020-01-03 11:04 UTC (permalink / raw)
To: Wu, Hao A
Cc: Albecki, Mateusz, devel@edk2.groups.io, Gao, Zhichao, Gao, Liming,
Ard Biesheuvel
Hi,
wt., 24 gru 2019 o 03:52 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
>
> > -----Original Message-----
> > From: Albecki, Mateusz
> > Sent: Saturday, December 21, 2019 1:13 AM
> > To: devel@edk2.groups.io
> > Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> > Subject: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the
> > EdkiiSdMmcSwitchClockFreq notification before sending CMD13
> >
> > The first patch refactors the SdMmcClockSupply function with a goal
> > of sending the EdkiiSdMmcSwitchClockFreq notification before we send the
> > CMD13 to check the switch status in eMMC init flow. This is required to
> > avoid sending the CMD13 on link that still has not been fixed by platform.
> >
> > To avoid changing the driver behavior we avoid sending notifications
> > when the clock is setup for the first time or when we setup the clock
> > after the voltage switch procedure(adressed in second patch).
> >
> > The second patch in the series optimizes the SD card detection routine
> > to stop it from going through the process of internal clock setup
> > after switching the voltage. According to SD HC specification there
> > is no need to setup internal clock all over again.
> >
> > Tests performed:
> > - Booted eMMC in HS400 mode on platform which required post clock freq
> > fixes
> >
> > I wasn't able to test SD card yet due to the lack of setup with working SD.
>
>
> I performed a quick verification on the eMMC device and SD card on my side.
> They work properly after the series.
>
> So for the series,
> Tested-by: Hao A Wu <hao.a.wu@intel.com>
>
I verified eMMC HS200 / HS@50MHz and SD cards on 2 Armada platforms, so:
Tested-by: Marcin Wojtas <mw@semihalf.com>
Best regards,
Marcin
>
> >
> > The patch series is available on github here:
> > https://github.com/malbecki/edk2/tree/sdmmc_post_freq_notify
>
>
> Add Ard to the loop to see if there is additional comment.
>
> Best Regards,
> Hao Wu
>
>
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Marcin Wojtas <mw@semihalf.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> >
> > Mateusz Albecki (2):
> > SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
> > MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock
> >
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +--
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 25 +---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 ++++
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 136
> > +++++++++++----------
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 45 ++-----
> > 5 files changed, 112 insertions(+), 138 deletions(-)
> >
> > --
> > 2.14.1.windows.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13
2020-01-03 11:04 ` Marcin Wojtas
@ 2020-01-06 5:18 ` Wu, Hao A
2020-01-06 6:10 ` [edk2-devel] " Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2020-01-06 5:18 UTC (permalink / raw)
To: Marcin Wojtas
Cc: Albecki, Mateusz, devel@edk2.groups.io, Gao, Zhichao, Gao, Liming,
Ard Biesheuvel
> -----Original Message-----
> From: Marcin Wojtas [mailto:mw@semihalf.com]
> Sent: Friday, January 03, 2020 7:05 PM
> To: Wu, Hao A
> Cc: Albecki, Mateusz; devel@edk2.groups.io; Gao, Zhichao; Gao, Liming; Ard
> Biesheuvel
> Subject: Re: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the
> EdkiiSdMmcSwitchClockFreq notification before sending CMD13
>
> Hi,
>
> wt., 24 gru 2019 o 03:52 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
> >
> > > -----Original Message-----
> > > From: Albecki, Mateusz
> > > Sent: Saturday, December 21, 2019 1:13 AM
> > > To: devel@edk2.groups.io
> > > Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao,
> Liming
> > > Subject: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the
> > > EdkiiSdMmcSwitchClockFreq notification before sending CMD13
> > >
> > > The first patch refactors the SdMmcClockSupply function with a goal
> > > of sending the EdkiiSdMmcSwitchClockFreq notification before we send
> the
> > > CMD13 to check the switch status in eMMC init flow. This is required to
> > > avoid sending the CMD13 on link that still has not been fixed by platform.
> > >
> > > To avoid changing the driver behavior we avoid sending notifications
> > > when the clock is setup for the first time or when we setup the clock
> > > after the voltage switch procedure(adressed in second patch).
> > >
> > > The second patch in the series optimizes the SD card detection routine
> > > to stop it from going through the process of internal clock setup
> > > after switching the voltage. According to SD HC specification there
> > > is no need to setup internal clock all over again.
> > >
> > > Tests performed:
> > > - Booted eMMC in HS400 mode on platform which required post clock
> freq
> > > fixes
> > >
> > > I wasn't able to test SD card yet due to the lack of setup with working SD.
> >
> >
> > I performed a quick verification on the eMMC device and SD card on my
> side.
> > They work properly after the series.
> >
> > So for the series,
> > Tested-by: Hao A Wu <hao.a.wu@intel.com>
> >
>
> I verified eMMC HS200 / HS@50MHz and SD cards on 2 Armada platforms, so:
> Tested-by: Marcin Wojtas <mw@semihalf.com>
Thanks for the testing effort.
I will push the series soon.
Best Regards,
Hao Wu
>
> Best regards,
> Marcin
>
> >
> > >
> > > The patch series is available on github here:
> > > https://github.com/malbecki/edk2/tree/sdmmc_post_freq_notify
> >
> >
> > Add Ard to the loop to see if there is additional comment.
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Marcin Wojtas <mw@semihalf.com>
> > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > >
> > > Mateusz Albecki (2):
> > > SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start
> > > MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock
> > >
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +--
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 25 +---
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24 ++++
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 136
> > > +++++++++++----------
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 45 ++-----
> > > 5 files changed, 112 insertions(+), 138 deletions(-)
> > >
> > > --
> > > 2.14.1.windows.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13
2020-01-06 5:18 ` Wu, Hao A
@ 2020-01-06 6:10 ` Wu, Hao A
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Hao A @ 2020-01-06 6:10 UTC (permalink / raw)
To: devel@edk2.groups.io, Wu, Hao A
Cc: Albecki, Mateusz, Gao, Zhichao, Gao, Liming, Ard Biesheuvel,
Marcin Wojtas
Series has been pushed via commits b948a49615..f68cb23c14.
Best Regards,
Hao Wu
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Wu, Hao A
> Sent: Monday, January 06, 2020 1:19 PM
> To: Marcin Wojtas
> Cc: Albecki, Mateusz; devel@edk2.groups.io; Gao, Zhichao; Gao, Liming; Ard
> Biesheuvel
> Subject: Re: [edk2-devel] [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe:
> Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13
>
> > -----Original Message-----
> > From: Marcin Wojtas [mailto:mw@semihalf.com]
> > Sent: Friday, January 03, 2020 7:05 PM
> > To: Wu, Hao A
> > Cc: Albecki, Mateusz; devel@edk2.groups.io; Gao, Zhichao; Gao, Liming;
> Ard
> > Biesheuvel
> > Subject: Re: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the
> > EdkiiSdMmcSwitchClockFreq notification before sending CMD13
> >
> > Hi,
> >
> > wt., 24 gru 2019 o 03:52 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
> > >
> > > > -----Original Message-----
> > > > From: Albecki, Mateusz
> > > > Sent: Saturday, December 21, 2019 1:13 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao,
> > Liming
> > > > Subject: [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the
> > > > EdkiiSdMmcSwitchClockFreq notification before sending CMD13
> > > >
> > > > The first patch refactors the SdMmcClockSupply function with a goal
> > > > of sending the EdkiiSdMmcSwitchClockFreq notification before we send
> > the
> > > > CMD13 to check the switch status in eMMC init flow. This is required to
> > > > avoid sending the CMD13 on link that still has not been fixed by
> platform.
> > > >
> > > > To avoid changing the driver behavior we avoid sending notifications
> > > > when the clock is setup for the first time or when we setup the clock
> > > > after the voltage switch procedure(adressed in second patch).
> > > >
> > > > The second patch in the series optimizes the SD card detection routine
> > > > to stop it from going through the process of internal clock setup
> > > > after switching the voltage. According to SD HC specification there
> > > > is no need to setup internal clock all over again.
> > > >
> > > > Tests performed:
> > > > - Booted eMMC in HS400 mode on platform which required post clock
> > freq
> > > > fixes
> > > >
> > > > I wasn't able to test SD card yet due to the lack of setup with working
> SD.
> > >
> > >
> > > I performed a quick verification on the eMMC device and SD card on my
> > side.
> > > They work properly after the series.
> > >
> > > So for the series,
> > > Tested-by: Hao A Wu <hao.a.wu@intel.com>
> > >
> >
> > I verified eMMC HS200 / HS@50MHz and SD cards on 2 Armada platforms,
> so:
> > Tested-by: Marcin Wojtas <mw@semihalf.com>
>
>
> Thanks for the testing effort.
> I will push the series soon.
>
> Best Regards,
> Hao Wu
>
>
> >
> > Best regards,
> > Marcin
> >
> > >
> > > >
> > > > The patch series is available on github here:
> > > > https://github.com/malbecki/edk2/tree/sdmmc_post_freq_notify
> > >
> > >
> > > Add Ard to the loop to see if there is additional comment.
> > >
> > > Best Regards,
> > > Hao Wu
> > >
> > >
> > > >
> > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > Cc: Marcin Wojtas <mw@semihalf.com>
> > > > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > > > Cc: Liming Gao <liming.gao@intel.com>
> > > >
> > > > Mateusz Albecki (2):
> > > > SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock
> start
> > > > MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock
> > > >
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 20 +--
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 25 +---
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 24
> ++++
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 136
> > > > +++++++++++----------
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 45 ++----
> -
> > > > 5 files changed, 112 insertions(+), 138 deletions(-)
> > > >
> > > > --
> > > > 2.14.1.windows.1
> > >
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-01-06 6:10 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-20 17:13 [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13 Albecki, Mateusz
2019-12-20 17:13 ` [PATCH 1/2] SdMmcPciHcDxe: Send EdkiiSdMmcSwitchClockFreq after SD clock start Albecki, Mateusz
2019-12-24 2:52 ` Wu, Hao A
2019-12-24 9:54 ` Ard Biesheuvel
2019-12-30 8:44 ` Marcin Wojtas
2019-12-31 14:49 ` Albecki, Mateusz
2019-12-20 17:13 ` [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock Albecki, Mateusz
2019-12-24 2:52 ` Wu, Hao A
2019-12-24 2:51 ` [PATCH 0/2] MdeModulePkg/SdMmcPciHcDxe: Send the EdkiiSdMmcSwitchClockFreq notification before sending CMD13 Wu, Hao A
2020-01-03 11:04 ` Marcin Wojtas
2020-01-06 5:18 ` Wu, Hao A
2020-01-06 6:10 ` [edk2-devel] " Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox