On 09/11/2020 23:19, Jaehoon Chung wrote: > On 11/9/20 11:23 PM, Neil Armstrong wrote: >> On 09/11/2020 15:10, Mark Kettenis wrote: >>>> From: Neil Armstrong <narmstr...@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.ch...@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.
OK, I'm not a MMC & SDCard expert on Amlogic, all the work was done in Linux. I just want to keep a working and stable SDCard & eMMC support for the 7 SoCs Families. Could you rewrite my "[PATCH 2/3] mmc: meson-gx: limit max frequency on SM1 SoCs" by instead changing the clock phase only for SM1 ? I'll then apply it with the 2 other patches and push then for the current development cycle. The driver is functional for all the other SoCs, and I want to keep it stable unless extensively tested. Neil > > Best Regards, > Jaehoon Chung > >> >> Neil >> >
