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

[Jaehoon Chung]

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.

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.

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 */ }
>>  };
>>
> 
>