On 09/02/2020 18:23, Anand Moon wrote: > Hi Neil > > Thanks for your review comments. > > On Sun, 9 Feb 2020 at 18:31, Neil Armstrong <[email protected]> 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 <[email protected]> >>> Signed-off-by: Anand Moon <[email protected]> >>> --- >>> 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, >>> +}; >>> +a in >>> 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.
It's not the point, just add the "struct meson_mmc_platdata" and .data in meson_mmc_match _before_ patch 1, to avoid adding hideous #ifdef/#elif/#endif in the meantime. Neil > > >> If you move it before, then: >> >> Reviewed-by: Neil Armstrong <[email protected]> >> >> Neil > > -Anand >
