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

[Jaehoon Chung]

Hi,

On 11/9/20 11:10 PM, 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.

Well, i didn't check Linux driver. but i can also check on Linux side.

> 
> 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.

Agreed. I can't mention that "it's working fine about all Amlogic SoCs".
In exynos's case, there are sdr and ddr timing about mmc/sd IP.
sdr/ddr timing are trying to get from dt's property, because it's possible that all Exynos SoCs have different values.

I think that Amlogic SoCs also needs to apply similar approach.

Best Regards,
Jaehoon Chung

>