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

[Marcin Wojtas <mw@semihalf.com>]

2017-10-26 15:55 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 26 October 2017 at 14:54, Marcin Wojtas <mw@semihalf.com> wrote:
>> 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?
>>
>
> Isn't that value read from device registers?

This field in generic Capability1 register is only 8-bit wide, hence
the 255MHz limitation. Xenon controller however is fed with 400MHz and
it clearly cannot be obtained from there.

Marcin