On 11/10/20 5:05 PM, Neil Armstrong wrote: > 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.
Ok. I will rewrite with you patch. Is it ok that i send the patch on Tomorrow? > > The driver is functional for all the other SoCs, and I want to keep it stable > unless extensively tested. Agreed. If i can test all Amlogic SoCs, I want to do that..But it's impossible to me. In future, i hope that it will be fixed with generic approach. Best Regards, Jaehoon Chung > > Neil > >> >> Best Regards, >> Jaehoon Chung >> >>> >>> Neil >>> >> > >
