[PATCH] mmc: meson_gx_mmc: change a clock phase to stable value

[Jaehoon Chung]

On 11/9/20 11:23 PM, Neil Armstrong wrote:
> On 09/11/2020 15:10, Mark Kettenis wrote:
>>> From: Neil Armstrong <narmstrong at baylibre.com>
>>> Date: Mon, 9 Nov 2020 14:37:09 +0100
>>>
>>> Hi,
>>>
>>> On 09/11/2020 04:12, Jaehoon Chung wrote:
>>>> Core clock phase value is changed from 180' to 270'.
>>>> It's more stable than before.
>>>> - Odroidn-N2/C4 : Working fine with 52MHz
>>>> - VIM3 : Working fine with 52MHz
>>>>
>>>> Before this patch, Odroid-C4 doesn't work fine with 52MHz.
>>>>
>>>> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
>>>> ---
>>>>  drivers/mmc/meson_gx_mmc.c | 14 ++++++++++----
>>>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
>>>> index 719dd1e5e570..7c60e0566560 100644
>>>> --- a/drivers/mmc/meson_gx_mmc.c
>>>> +++ b/drivers/mmc/meson_gx_mmc.c
>>>> @@ -52,10 +52,16 @@ static void meson_mmc_config_clock(struct mmc *mmc)
>>>>  	}
>>>>  	clk_div = DIV_ROUND_UP(clk, mmc->clock);
>>>>  
>>>> -	/* 180 phase core clock */
>>>> -	meson_mmc_clk |= CLK_CO_PHASE_180;
>>>> -
>>>> -	/* 180 phase tx clock */
>>>> +	/*
>>>> +	 * Clock Phase needs to set a proper value.
>>>> +	 * It can be changed to other value.
>>>> +	 * Because CORE : 270' Phase and TX : 0' Phase are stable,
>>>> +	 * set to them by default.
>>>> +	 */
>>>> +	/* Core Clock Phase */
>>>> +	meson_mmc_clk |= CLK_CO_PHASE_270;
>>>> +
>>>> +	/* TX Clock Phase */
>>>>  	meson_mmc_clk |= CLK_TX_PHASE_000;
>>>>  
>>>>  	/* clock settings */
>>>>
>>>
>>> The previous values were aligned on the Linux driver, which is functional.
>>
>> Actually the Linux driver isn't really functional; the 52 MHz
>> high-speed mode doesn't work.  But since HS200 does work in Linux,
>> probably nobody noticed this.
>>
>> That said, I'm not confident a single clock phase setting will work
>> across all Amlogic SoCs and across different boards.  Maybe we need
>> something in the device tree such that we can control the values on a
>> per-board level.
>>
> 
> So this is specific to SM1 SoCs then, because others families doesn't have such issues
> at 52MHz.

I don't have much knowledge of SM1 SoCs. But how did its phase value select them on Linux driver?

> 
> So the Linux must be fixes, including the bindings to introduce a new compatible, then
> ported to U-Boot.
> 
> So in the meantime, it's right to limit to 26MHz on SM1 in U-boot until we have all this
> clarified.
If you want to limit to 26MHz, I don't have any objection about your opinion.
But I wonder how to clarify all. And I also wonder that values what is used on Linux kernel are really right.

Best Regards,
Jaehoon Chung

> 
> Neil
>