[EXT] Re: [PATCH] mmc: sdhci-esdhc-imx: inherit pins_100mhz from pins_200mhz when unconfigured

[Luke Wang]

> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum at pengutronix.de>
> Sent: Wednesday, May 21, 2025 7:45 PM
> To: Luke Wang <ziniu.wang_1 at nxp.com>; Bough Chen
> <haibo.chen at nxp.com>; adrian.hunter at intel.com; ulf.hansson at linaro.org;
> linux-mmc at vger.kernel.org
> Cc: imx at lists.linux.dev; dl-S32 <S32 at nxp.com>; shawnguo at kernel.org;
> s.hauer at pengutronix.de; linux-kernel at vger.kernel.org;
> kernel at pengutronix.de; festevam at gmail.com; linux-arm-
> kernel at lists.infradead.org
> Subject: [EXT] Re: [PATCH] mmc: sdhci-esdhc-imx: inherit pins_100mhz from
> pins_200mhz when unconfigured
>
> [You don't often get email from a.fatoum at pengutronix.de. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
> 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
>
>
> Hi Luke,
>
> Thanks for your patch.
>
> On 21.05.25 13:20, ziniu.wang_1 at nxp.com wrote:
> > From: Luke Wang <ziniu.wang_1 at nxp.com>
> >
> > On some new i.MX platforms, hardware guidelines recommend using
> identical
> > pin configurations for SDR50/DDR50 (100MHz) and SDR104/HS400
> (200MHz)
> > modes. But defining two identical pinctrl for 100MHz and 200MHz in dts
> > creates redundancy.
>
> I am not convinced this is an improvement. If 200mhz is missing, it's
> understood
> that it's not supported. Now if 100mhz is missing, it means something
> different
> depending on whether state_200mhz exists or not. This is more mental
> overhead
> than having:
>
>   pinctrl-names = "default", "state_100mhz", "state_200mhz";
>   pinctrl-0 = <&pinctrl_usdhc1>;
>   pinctrl-1 = <&pinctrl_usdhc1_uhs>;
>   pinctrl-2 = <&pinctrl_usdhc1_uhs>;
>

Hi Ahmad,

This way looks better. Will use pinctrl_usdhc1_uhs for both state_100mhz and state_200mhz if there are identical.

Thanks
Luke

> where it's directly evident that you share pinctrl states.
>
> > In this case, omit explicit 100MHz configuration,
> > driver will inherit 100MHz pinctrl from 200MHz.
> >
> > Preserves existing behavior if 100MHz is configured but 200MHz not (e.g,
> > imx8mp-navq.dts usdhc1 supports SDR50/DDR50 but SDR104/HS400 not).
>
> This conflicts with the binding, which doesn't allow for state_200mhz
> to exist without state_100mhz, so that would need adaptation.
>
> As noted before though, I don't think we really need to change anything
> here though.
>
> Thanks,
> Ahmad
>
> >
> > Signed-off-by: Luke Wang <ziniu.wang_1 at nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-
> esdhc-imx.c
> > index f206b562a6e3..dfd8be5000c8 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -1810,6 +1810,9 @@ sdhci_esdhc_imx_probe_dt(struct
> platform_device *pdev,
> >                                               ESDHC_PINCTRL_STATE_100MHZ);
> >               imx_data->pins_200mhz = pinctrl_lookup_state(imx_data->pinctrl,
> >                                               ESDHC_PINCTRL_STATE_200MHZ);
> > +
> > +             if (IS_ERR_OR_NULL(imx_data->pins_100mhz))
> > +                     imx_data->pins_100mhz = imx_data->pins_200mhz;
> >       }
> >
> >       /* call to generic mmc_of_parse to support additional capabilities */
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> http://www.p/
> engutronix.de%2F&data=05%7C02%7Cziniu.wang_1%40nxp.com%7Cd8145bf
> 22a114de4256f08dd985ced07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C638834247059505189%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0
> eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpb
> CIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=tXhmm%2Bd3rGbtfl2VorcnL
> OvfZ8NcVFc0EJ%2B4fTxYJns%3D&reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |