Re: [PATCH RFC v3 2/8] mmc: meson_gx_mmc: add minimal non-DM driver

On Wed, Oct 08, 2025 at 10:00:38AM +0000, Ferass El Hafidi wrote:
> Add a minimal non-DM MMC driver for use in size-constrained
> environments.
> 
> Signed-off-by: Ferass El Hafidi <[email protected]>
> ---
>  arch/arm/include/asm/arch-meson/gx.h |  5 +++
>  drivers/mmc/meson_gx_mmc.c           | 75 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 79 insertions(+), 1 deletion(-)
> 
> --- a/drivers/mmc/meson_gx_mmc.c
> +++ b/drivers/mmc/meson_gx_mmc.c

...

> +#if !CONFIG_IS_ENABLED(DM_MMC)

This is a large conditional compilation block with nested #if's, do you
think commenting #else and #endif with the condition improves the
readibility? i.e.

	#if !CONFIG_IS_ENABLED(DM_MMC)

	...

	#else /* !CONFIG_IS_ENALBED(DM_MMC) */

	...

	#endif /* CONFIG_IS_ENALBED(DM_MMC) */

Best regards,
Yao Zi

> +struct meson_mmc_plat mmc_plat[2];
> +
> +static int meson_legacy_mmc_init(struct mmc *mmc)
> +{
> +	/* reset all status bits */
> +	meson_write(mmc, STATUS_MASK, MESON_SD_EMMC_STATUS);
> +
> +	/* disable interrupts */
> +	meson_write(mmc, 0, MESON_SD_EMMC_IRQ_EN);
> +
> +	return 0;
> +}
> +
> +static const struct mmc_ops meson_mmc_ops = {
> +	.send_cmd	= meson_legacy_mmc_send_cmd,
> +	.set_ios	= meson_legacy_mmc_set_ios,
> +	.init		= meson_legacy_mmc_init,
> +};
> +
> +struct mmc *meson_mmc_init(int mmc_no)
> +{
> +	struct meson_mmc_plat *pdata = &mmc_plat[mmc_no];
> +	struct mmc_config *cfg = &pdata->cfg;
> +
> +	cfg->voltages = MMC_VDD_33_34 | MMC_VDD_32_33 |
> +			MMC_VDD_31_32 | MMC_VDD_165_195;
> +	cfg->host_caps = MMC_MODE_8BIT | MMC_MODE_4BIT;
> +	cfg->f_min = DIV_ROUND_UP(SD_EMMC_CLKSRC_24M, CLK_MAX_DIV);
> +	cfg->f_max = 6000000; /* 6 MHz */
> +	cfg->b_max = 127; /* max 128 - 1 block */
> +	cfg->name = "Meson SD/eMMC";
> +	cfg->ops = &meson_mmc_ops;
> +
> +	if (mmc_no == 0) /* MMC1: SD card */
> +		pdata->regbase = (void *)0xd0072000;
> +	else if (mmc_no == 1) /* MMC2: eMMC */
> +		pdata->regbase = (void *)0xd0074000;
> +
> +#if CONFIG_IS_ENABLED(MMC_PWRSEQ)
> +	/* Enable power if needed */
> +	ret = mmc_pwrseq_get_power(dev, cfg);
> +	if (!ret) {
> +		ret = pwrseq_set_power(cfg->pwr_dev, true);
> +		if (ret)
> +			return ret;
> +	}
> +#endif
> +
> +	return mmc_create(cfg, pdata);
> +}
> +
> +#else
>  static const struct dm_mmc_ops meson_dm_mmc_ops = {
>  	.send_cmd = meson_dm_mmc_send_cmd,
>  	.set_ios = meson_dm_mmc_set_ios,
> @@ -282,6 +348,12 @@ static int meson_mmc_probe(struct udevice *dev)
>  	cfg->b_max = 511; /* max 512 - 1 blocks */
>  	cfg->name = dev->name;
>  
> +	if (IS_ENABLED(CONFIG_SPL_BUILD)) {
> +		cfg->host_caps &= ~(MMC_MODE_HS_52MHz | MMC_MODE_HS);
> +		cfg->f_max = 6000000; /* 6 MHz */
> +		cfg->b_max = 127; /* max 128 - 1 block */
> +	}
> +
>  	mmc->priv = pdata;
>  	upriv->mmc = mmc;
>  
> @@ -336,3 +408,4 @@ U_BOOT_DRIVER(meson_mmc) = {
>  	.of_to_plat = meson_mmc_of_to_plat,
>  	.plat_auto	= sizeof(struct meson_mmc_plat),
>  };
> +#endif
> 
> -- 
> 2.51.0
>