[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<DU2PR04MB85672AD76D82D5BB29FB5A47ED99A@DU2PR04MB8567.eurprd04.prod.outlook.com>
Date: Thu, 22 May 2025 04:47:39 +0000
From: Luke Wang <ziniu.wang_1@....com>
To: Ahmad Fatoum <a.fatoum@...gutronix.de>, Bough Chen <haibo.chen@....com>,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>, "ulf.hansson@...aro.org"
<ulf.hansson@...aro.org>, "linux-mmc@...r.kernel.org"
<linux-mmc@...r.kernel.org>
CC: "imx@...ts.linux.dev" <imx@...ts.linux.dev>, dl-S32 <S32@....com>,
"shawnguo@...nel.org" <shawnguo@...nel.org>, "s.hauer@...gutronix.de"
<s.hauer@...gutronix.de>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "kernel@...gutronix.de"
<kernel@...gutronix.de>, "festevam@...il.com" <festevam@...il.com>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [EXT] Re: [PATCH] mmc: sdhci-esdhc-imx: inherit pins_100mhz from
pins_200mhz when unconfigured
> -----Original Message-----
> From: Ahmad Fatoum <a.fatoum@...gutronix.de>
> Sent: Wednesday, May 21, 2025 7:45 PM
> To: Luke Wang <ziniu.wang_1@....com>; Bough Chen
> <haibo.chen@....com>; adrian.hunter@...el.com; ulf.hansson@...aro.org;
> linux-mmc@...r.kernel.org
> Cc: imx@...ts.linux.dev; dl-S32 <S32@....com>; shawnguo@...nel.org;
> s.hauer@...gutronix.de; linux-kernel@...r.kernel.org;
> kernel@...gutronix.de; festevam@...il.com; linux-arm-
> kernel@...ts.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@...gutronix.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@....com wrote:
> > From: Luke Wang <ziniu.wang_1@....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@....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 |
Powered by blists - more mailing lists