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

[Anand Moon]

Hi Neil,

On Mon, 9 Nov 2020 at 19:56, Neil Armstrong <narmstrong at baylibre.com> wrote:
>
> On 09/11/2020 15:10, 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.
> >
> > 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.
>
> 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.
>
> Neil

Earlier I had a similar approach for this FIX but somehow it did not get merged.

Please add my
Tested-by: Anand Moon <linux.amoon at gmail.com>

Best Regards
-Anand