Re: [PATCH v5 3/8] mmc: sdhci-esdhc-imx: fix esdhc_change_pinstate() to allow default state restore

[Frank Li <Frank.li@oss.nxp.com>]

On Tue, Jul 07, 2026 at 07:02:58PM +0800, ziniu.wang_1@oss.nxp.com wrote:
> From: Luke Wang <ziniu.wang_1@nxp.com>
>
> esdhc_change_pinstate() checks for pins_100mhz and pins_200mhz at the
> top of the function and returns -EINVAL if either is not defined. This
> prevents the default case from ever being reached, which means devices
> with a sleep pinctrl state but without high-speed pin states (100mhz/
> 200mhz) can never restore their default pin configuration.
>
> Move the IS_ERR checks for pins_100mhz and pins_200mhz into their
> respective switch cases.
>
> Signed-off-by: Luke Wang <ziniu.wang_1@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 7230d70e02ae..ead4685d621a 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1326,19 +1326,21 @@ static int esdhc_change_pinstate(struct sdhci_host *host,
>
>  	dev_dbg(mmc_dev(host->mmc), "change pinctrl state for uhs %d\n", uhs);
>
> -	if (IS_ERR(imx_data->pinctrl) ||
> -		IS_ERR(imx_data->pins_100mhz) ||
> -		IS_ERR(imx_data->pins_200mhz))
> +	if (IS_ERR(imx_data->pinctrl))
>  		return -EINVAL;

Maybe off topic,  I suggest set pins_100mhz and pins_200mhz to
default pinctrl if not exist.

Currently driver use pinctrl_200mhz/100mhz judge if support SDR50 and HS200.
Actually it is hidden ABI.

You can save max support speed to variable after check pinctrl_200mhz
and pinctl_100mhz. then make these two variable to fallback to low frequency
one.

So you can reduce these check in code.

Is it simpler?

Frank

>
>  	switch (uhs) {
>  	case MMC_TIMING_UHS_SDR50:
>  	case MMC_TIMING_UHS_DDR50:
> +		if (IS_ERR(imx_data->pins_100mhz))
> +			return -EINVAL;
>  		pinctrl = imx_data->pins_100mhz;
>  		break;
>  	case MMC_TIMING_UHS_SDR104:
>  	case MMC_TIMING_MMC_HS200:
>  	case MMC_TIMING_MMC_HS400:
> +		if (IS_ERR(imx_data->pins_200mhz))
> +			return -EINVAL;
>  		pinctrl = imx_data->pins_200mhz;
>  		break;
>  	default:
> --
> 2.34.1
>
>