2017-10-26 16:02 GMT+02:00 Leif Lindholm <[email protected]>: > On Thu, Oct 26, 2017 at 03:54:41PM +0200, Marcin Wojtas wrote: >> 2017-10-26 15:46 GMT+02:00 Leif Lindholm <[email protected]>: >> > On Thu, Oct 26, 2017 at 03:19:36AM +0200, Marcin Wojtas wrote: >> >> Incorrectly the clock divisor was calculated relatively >> >> to 255MHz instead of actual 400MHz. >> > >> > This describes the specific symptom, not the problem with the existing >> > code. >> > >> >> Fix this. >> >> >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> >> Signed-off-by: Marcin Wojtas <[email protected]> >> >> --- >> >> Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c | 4 ++-- >> >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c >> >> b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c >> >> index ccbf355..0b9328b 100644 >> >> --- a/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c >> >> +++ b/Platform/Marvell/Drivers/SdMmc/XenonDxe/SdMmcPciHci.c >> >> @@ -16,6 +16,7 @@ >> >> **/ >> >> >> >> #include "SdMmcPciHcDxe.h" >> >> +#include "XenonSdhci.h" >> >> >> >> /** >> >> Dump the content of SD/MMC host controller's Capability Register. >> >> @@ -703,9 +704,8 @@ SdMmcHcClockSupply ( >> >> // >> >> // Calculate a divisor for SD clock frequency >> >> // >> >> - ASSERT (Capability.BaseClkFreq != 0); >> >> >> >> - BaseClkFreq = Capability.BaseClkFreq; >> > >> > Why is Capability.BaseClkFreq the wrong frequency to use? >> >> The Capability.BaseClkFreq is UINT8 and can hold up to 0xff -> 255MHz. >> An alternative would be change this generic type to UINT16 and update >> field properly during initialization - do you prefer that? > > No, I'm still dreaming we might be able to reintegrate this into the > MdeModulePkg driver in some glorious future.
Yes, that would be great. I imagine some SDMMC_HOST_PROTOCOL exposing callbacks to set UHS, custom clock handling, etc (like it's done in the Linux). > > So what you are basically saying is that this controller is running at > a higher frequency than is permitted (or even describable) by the > specification? If so, _that_ needs to be in the commit message (and > really, a comment by the code as well). Yes, this clock value is Xenon controller's quirk. I will mention it in commit log/comment, but please let know if you wish me to: a. extend BaseClkFreq to UINT16 and configure it properly during init b. leave as is with better description only I lean towards a., it's a very low-cost quirk to be applied. Best regards, Marcin _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
