RE: [PATCH v4] mmc: sdhci-esdhc-imx: consolidate imx25/35 data

[Luke Wang]

> -----Original Message-----
> From: Adrián García Casado <adriangarciacasado42@xxxxxxxxx>
> Sent: Wednesday, March 18, 2026 3:33 AM
> To: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Cc: Bough Chen <haibo.chen@xxxxxxx>; Frank Li <frank.li@xxxxxxx>;
> Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Miguel Ojeda <miguel.ojeda.sandonis@xxxxxxxxx>;
> Adrián García Casado <adriangarciacasado42@xxxxxxxxx>
> Subject: [EXT] [PATCH v4] mmc: sdhci-esdhc-imx: consolidate imx25/35 data
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Consolidate esdhc_imx25 and esdhc_imx35 soc data into a single shared
> struct since they share the same flags. This reduces redundancy and
> simplifies data management for i.MX25 and i.MX35 SoCs.
> 
> Signed-off-by: Adrián García Casado <adriangarciacasado42@xxxxxxxxx>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> esdhc-imx.c
> index a7a5df673..9cfa26722 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -256,11 +256,7 @@ struct esdhc_soc_data {
>         u32 quirks;
>  };
> 
> -static const struct esdhc_soc_data esdhc_imx25_data = {
> -       .flags = ESDHC_FLAG_ERR004536,
> -};
> -
> -static const struct esdhc_soc_data esdhc_imx35_data = {
> +static const struct esdhc_soc_data esdhc_imx25_35_data = {
>         .flags = ESDHC_FLAG_ERR004536,
>  };
> 
> @@ -391,8 +387,8 @@ struct pltfm_imx_data {
>  };
> 
>  static const struct of_device_id imx_esdhc_dt_ids[] = {
> -       { .compatible = "fsl,imx25-esdhc", .data = &esdhc_imx25_data, },
> -       { .compatible = "fsl,imx35-esdhc", .data = &esdhc_imx35_data, },
> +       { .compatible = "fsl,imx25-esdhc", .data = &esdhc_imx25_35_data, },
> +       { .compatible = "fsl,imx35-esdhc", .data = &esdhc_imx25_35_data, },
>         { .compatible = "fsl,imx51-esdhc", .data = &esdhc_imx51_data, },
>         { .compatible = "fsl,imx53-esdhc", .data = &esdhc_imx53_data, },
>         { .compatible = "fsl,imx6sx-usdhc", .data = &usdhc_imx6sx_data, },
> @@ -414,7 +410,7 @@ MODULE_DEVICE_TABLE(of, imx_esdhc_dt_ids);
> 
>  static inline int is_imx25_esdhc(struct pltfm_imx_data *data)
>  {
> -       return data->socdata == &esdhc_imx25_data;
> +       return data->socdata == &esdhc_imx25_35_data;
>  }

I took a deeper look at the history related to mx25.

The original use of is_imx25_esdhc() to avoid setting DMA SELECT bits was because mx25 doesn't support ADMA, introduced in commit 9a0985b78d70.

However, before that commit, imx25 was already using SDHCI_QUIRK_BROKEN_ADMA to disable ADMA (commit 97e4ba6a5ea9), which theoretically should prevent the DMA SELECT bits from being set.

The actual reason DMA SELECT was being set incorrectly was due to a bug: u32 new_val was used without being initialized, causing DMA SELECT to potentially be set to random values.

The new_val initialization issue was fixed in commit 81a0a8bc380d, but that fix was made for a different reason - it was not recognized at that time that the DMA SELECT issue was also caused by the uninitialized new_val. As a result, the is_imx25_esdhc() check was preserved.

After the new_val initialization fix, is_imx25_esdhc() should no longer be necessary. So merging mx25 and mx35 is indeed valid. If you'd like, you can go further and remove the is_imx25_esdhc() related code entirely.

Regards,
Luke

> 
>  static inline int is_imx53_esdhc(struct pltfm_imx_data *data)
> --
> 2.47.3
>