Re: [platforms: PATCH 09/10] Marvell/Drivers: XenonDxe: Fix base clock frequency

[Marcin Wojtas <mw@semihalf.com>]

2017-10-26 15:46 GMT+02:00 Leif Lindholm <leif.lindholm@linaro.org>:
> 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 <mw@semihalf.com>
>> ---
>>  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?

Marcin