[PATCH] mmc: meson-gx: change clock phase value on AGX SoCs

[Stefan Agner]

On 2020-12-15 06:19, Jaehoon Chung wrote:
> Hi,
> 
> On 12/15/20 3:58 AM, Neil Armstrong wrote:
>> Hi,
>>
>> On 07/12/2020 18:15, Stefan Agner wrote:
>>> Amlogic AGX SoCs seem to have issue communicating with some eMMC
>>> devices (in particular with a Micron 128GB eMMC 5.1). The device
>>> is detected with 1-bit bus width, and at higher temperature loading
>>> pretty much anything from the storage fails: (e.g. fs_devread read error
>>> - block).
>>>
>>> When phase is set to 270° it is detected with 8-bit bus width and is
>>> working fine accross all temperatures.
>>
>> This is new to G12, I only had such issues and reports for SM1 only until now.
>>
>>>
>>> Signed-off-by: Stefan Agner <stefan at agner.ch>
>>> ---
>>> Hi Neil,
>>>
>>> I debugged this issue today on an ODROID N2+ not booting reliably. I am
>>> not sure if we can safely switch to 270° for all SoCs with
>>> amlogic,meson-axg-mmc, but I guess we have to try and see what happens?
>>> I will do a bit broader testing in the comming days here.
>>
>> amlogic,meson-axg-mmc covers too much SoCs, I'll prefer if you introduce
>> an u-boot only amlogic,meson-g12b-mmc compatible like I did for SM1.
>>
>>>
>>> Btw, I do see that 180° is also set in Linux. Do you have a patch to
>>> address this in Linux?
>>
>> I never had such reports on Linux, I think because eMMCs are directly used
>> in HS200 mode and 180° is the right configuration for HS200...
> 
> I have checked mainline kernel with Odroid-C4.
> Linux Kernel is using PHASE_180 and enabled HS200 by default. That's
> why this issue doesn't report.
> It's working fine with PHASE_180. But if mode is downgrade as DDR50 or
> lower mode than HS200.
> It's also broken, like u-boot.

That make sense. It seems that HS200 is really required to have a stable
SD communication on this platform.

> 
> When i have changed from PHASE_180 to PHASE_270, it's working fine
> about all modes.
> 
> [    2.689222] mmc1: new HS200 MMC card at address 0001
> [    2.692070] xhci-hcd xhci-hcd.0.auto: irq 34, io mem 0xff500000
> [    2.697491] mmcblk1: mmc1:0001 BJTD4R 29.1 GiB
> 
> Current meson_gx_mmc doesn't support HS200 on u-boot side. (It needs
> to implement more things.)
> So I think that it's right way to set to PHASE_270 on U-boot side. 
> If it needs to fix this on kernel side, i will send the patch.

After more testing, we found that a 64GB eMMC stopped working when using
270°. We now switched back to 180° and lowered frequency to 24MHz. This
setting works on all eMMCs we had around to test, across temperature
ranges (booting when cold, as well as booting when heated up slightly).

Downstream U-Boot seems to use 40MHz clock, and switches phase depending
on frequency:
co-phase 0x3, tx-dly 0, clock 400000
co-phase 0x1, tx-dly 0, clock 40000000

I tried to implement this in a quick hack, but it did not work reliably.
However, as far as I can tell downstream U-Boot also does HS200 link
training etc... So it seems HS200 link training is required to make
things work properly.

--
Stefan


> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Neil
>>
>>>
>>> --
>>> Stefan
>>>
>>>
>>>  arch/arm/include/asm/arch-meson/sd_emmc.h | 1 +
>>>  drivers/mmc/meson_gx_mmc.c                | 9 +++++----
>>>  2 files changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/arch/arm/include/asm/arch-meson/sd_emmc.h
>>> index cb16f75fc6..db5e058098 100644
>>> --- a/arch/arm/include/asm/arch-meson/sd_emmc.h
>>> +++ b/arch/arm/include/asm/arch-meson/sd_emmc.h
>>> @@ -14,6 +14,7 @@
>>>
>>>  enum meson_gx_mmc_compatible {
>>>  	MMC_COMPATIBLE_GX,
>>> +	MMC_COMPATIBLE_AGX,
>>>  	MMC_COMPATIBLE_SM1,
>>>  };
>>>
>>> diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
>>> index 5facbfdd9a..2c27113c10 100644
>>> --- a/drivers/mmc/meson_gx_mmc.c
>>> +++ b/drivers/mmc/meson_gx_mmc.c
>>> @@ -64,14 +64,15 @@ static void meson_mmc_config_clock(struct mmc *mmc)
>>>
>>>  	/*
>>>  	 * SM1 SoCs doesn't work fine over 50MHz with CLK_CO_PHASE_180
>>> +	 * AGX SoCs don't work reliable with some eMMCs with CLK_CO_PHASE_180
>>>  	 * If CLK_CO_PHASE_270 is used, it's more stable than other.
>>>  	 * Other SoCs use CLK_CO_PHASE_180 by default.
>>>  	 * It needs to find what is a proper value about each SoCs.
>>>  	 */
>>> -	if (meson_gx_mmc_is_compatible(mmc->dev, MMC_COMPATIBLE_SM1))
>>> -		meson_mmc_clk |= CLK_CO_PHASE_270;
>>> -	else
>>> +	if (meson_gx_mmc_is_compatible(mmc->dev, MMC_COMPATIBLE_GX))
>>>  		meson_mmc_clk |= CLK_CO_PHASE_180;
>>> +	else
>>> +		meson_mmc_clk |= CLK_CO_PHASE_270;
>>>
>>>  	/* 180 phase tx clock */
>>>  	meson_mmc_clk |= CLK_TX_PHASE_000;
>>> @@ -327,7 +328,7 @@ int meson_mmc_bind(struct udevice *dev)
>>>
>>>  static const struct udevice_id meson_mmc_match[] = {
>>>  	{ .compatible = "amlogic,meson-gx-mmc", .data = MMC_COMPATIBLE_GX },
>>> -	{ .compatible = "amlogic,meson-axg-mmc", .data = MMC_COMPATIBLE_GX },
>>> +	{ .compatible = "amlogic,meson-axg-mmc", .data = MMC_COMPATIBLE_AGX },
>>>  	{ .compatible = "amlogic,meson-sm1-mmc", .data = MMC_COMPATIBLE_SM1 },
>>>  	{ /* sentinel */ }
>>>  };
>>>
>>
>>