[PATCHv6 2/5] mmc: meson-gx: Use proper compatible string as per the dts

[Anand Moon]

Hi Neil

Thanks for your review comments.

On Sun, 9 Feb 2020 at 18:31, Neil Armstrong <narmstrong at baylibre.com> wrote:
>
> Hi,
>
> Le 09/02/2020 à 12:05, Anand Moon a écrit :
> > Use proper compatible string as per the dts so that mmc driver
> > could be tuned properly. SoC family S905, S905X have common clk
> > tuning parameters setting, while AGX and G12 have common clk tuning
> > parameters setting for mmc driver.
> >
> > Suggested-by: Neil Armstrong <narmstrong at baylibre.com>
> > Signed-off-by: Anand Moon <linux.amoon at gmail.com>
> > ---
> > No changes.
> > ---
> >  arch/arm/include/asm/arch-meson/sd_emmc.h |  7 ++++
> >  drivers/mmc/meson_gx_mmc.c                | 46 +++++++++++++++++------
> >  2 files changed, 41 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-meson/sd_emmc.h b/arch/arm/include/asm/arch-meson/sd_emmc.h
> > index f4299485dc..83142d5d3f 100644
> > --- a/arch/arm/include/asm/arch-meson/sd_emmc.h
> > +++ b/arch/arm/include/asm/arch-meson/sd_emmc.h
> > @@ -13,6 +13,12 @@
> >  #define SDIO_PORT_B                  1
> >  #define SDIO_PORT_C                  2
> >
> > +enum mmc_compatible {
> > +        MMC_COMPATIBLE_GXBB,
> > +        MMC_COMPATIBLE_GX,
> > +        MMC_COMPATIBLE_AXG,
> > +};
> > +
> >  #define SD_EMMC_CLKSRC_24M           24000000        /* 24 MHz */
> >  #define SD_EMMC_CLKSRC_DIV2          1000000000      /* 1 GHz */
> >
> > @@ -87,6 +93,7 @@
> >  struct meson_mmc_platdata {
> >       struct mmc_config cfg;
> >       struct mmc mmc;
> > +     int compat;
> >       void *regbase;
> >       void *w_buf;
> >  };
> > diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
> > index b013c7c5fb..1aefe360c4 100644
> > --- a/drivers/mmc/meson_gx_mmc.c
> > +++ b/drivers/mmc/meson_gx_mmc.c
> > @@ -37,7 +37,8 @@ static inline void meson_write(struct mmc *mmc, uint32_t val, int offset)
> >       writel(val, get_regbase(mmc) + offset);
> >  }
> >
> > -static void meson_mmc_config_clock(struct mmc *mmc)
> > +static void meson_mmc_config_clock(struct mmc *mmc,
> > +                                struct meson_mmc_platdata *pdata)
> >  {
> >       uint32_t meson_mmc_clk = 0;
> >       unsigned int clk, clk_src, clk_div;
> > @@ -66,14 +67,20 @@ static void meson_mmc_config_clock(struct mmc *mmc)
> >       /* RX clock phase 0:180 */
> >       meson_mmc_clk |= FIELD_PREP(CLK_RX_PHASE_MASK, CLK_PHASE_0);
> >
> > -#ifdef CONFIG_MESON_GX
> > -     /* clk always on */
> > -     meson_mmc_clk |= CLK_V2_ALWAYS_ON;
> > -#endif
> > -#if (defined(CONFIG_MESON_AXG) || defined(CONFIG_MESON_G12A))
> > -     /* clk always on */
> > -     meson_mmc_clk |= CLK_V3_ALWAYS_ON;
> > -#endif
> > +     switch (pdata->compat) {
> > +     case MMC_COMPATIBLE_GXBB:
> > +     case MMC_COMPATIBLE_GX:
> > +             /* clk always on */
> > +             meson_mmc_clk |= CLK_V2_ALWAYS_ON;
> > +             break;
> > +     case MMC_COMPATIBLE_AXG:
> > +             /* clk always on */
> > +             meson_mmc_clk |= CLK_V3_ALWAYS_ON;
> > +             break;
> > +     default:
> > +             debug("no compatible supported");
> > +             break;
> > +     }
> >
> >       /* clock settings */
> >       meson_mmc_clk |= clk_src;
> > @@ -85,9 +92,11 @@ static void meson_mmc_config_clock(struct mmc *mmc)
> >  static int meson_dm_mmc_set_ios(struct udevice *dev)
> >  {
> >       struct mmc *mmc = mmc_get_mmc_dev(dev);
> > +     struct meson_mmc_platdata *pdata =
> > +             (struct meson_mmc_platdata *)dev_get_driver_data(dev);
> >       uint32_t meson_mmc_cfg;
> >
> > -     meson_mmc_config_clock(mmc);
> > +     meson_mmc_config_clock(mmc, pdata);
> >
> >       meson_mmc_cfg = meson_read(mmc, MESON_SD_EMMC_CFG);
> >
> > @@ -324,9 +333,22 @@ int meson_mmc_bind(struct udevice *dev)
> >       return mmc_bind(dev, &pdata->mmc, &pdata->cfg);
> >  }
> >
> > +static const struct meson_mmc_platdata gxbb_data = {
> > +        .compat = MMC_COMPATIBLE_GXBB,
> > +};
> > +
> > +static const struct meson_mmc_platdata gx_data = {
> > +        .compat = MMC_COMPATIBLE_GX,
> > +};
> > +
> > +static const struct meson_mmc_platdata axg_data = {
> > +        .compat = MMC_COMPATIBLE_AXG,
> > +};
> > +
> >  static const struct udevice_id meson_mmc_match[] = {
> > -     { .compatible = "amlogic,meson-gx-mmc" },
> > -     { .compatible = "amlogic,meson-axg-mmc" },
> > +     { .compatible = "amlogic,meson-gxbb-mmc", .data = (ulong)&gxbb_data },
> > +     { .compatible = "amlogic,meson-gx-mmc", .data = (ulong)&gx_data },
> > +     { .compatible = "amlogic,meson-axg-mmc", .data = (ulong)&axg_data },
> >       { /* sentinel */ }
> >  };
> >
> >
>
>
> It's fine but you should do that before patch 1, and introduce the clk setup directly with the MMC_COMPATIBLE_*.
>

Only GXBB and GLX have  CLK_ALWAYS_ON(24) bit set
and AXG and G12X  have CLK_ALWAYS_ON(28) bit set
for clk enable rest of the configuration is all most common for all the SoC.
let keep this simple as of now.


> If you move it before, then:
>
> Reviewed-by: Neil Armstrong <narmstrong at baylibre.com>
>
> Neil

-Anand