On 2/01/19 12:40 PM, BOUGH CHEN wrote: >> -----Original Message----- >> From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx] >> Sent: 2018年12月28日 21:59 >> To: BOUGH CHEN <haibo.chen@xxxxxxx>; ulf.hansson@xxxxxxxxxx >> Cc: linux-mmc@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx> >> Subject: Re: [PATCH] mmc: sdhci-esdhc-imx: restore pin state when resume >> back >> >> On 27/12/18 1:35 PM, BOUGH CHEN wrote: >>> In some low power mode, SoC will lose the pin state, so need to >>> restore the pin state when resume back. >>> >>> Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx> >>> --- >>> drivers/mmc/host/sdhci-esdhc-imx.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c >>> b/drivers/mmc/host/sdhci-esdhc-imx.c >>> index 984cc1a788cb..7cfcc8618e45 100644 >>> --- a/drivers/mmc/host/sdhci-esdhc-imx.c >>> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c >>> @@ -1392,17 +1392,23 @@ static int sdhci_esdhc_imx_remove(struct >>> platform_device *pdev) static int sdhci_esdhc_suspend(struct device >>> *dev) { >>> struct sdhci_host *host = dev_get_drvdata(dev); >>> + int ret; >>> >>> if (host->tuning_mode != SDHCI_TUNING_MODE_3) >>> mmc_retune_needed(host->mmc); >>> >>> - return sdhci_suspend_host(host); >>> + ret = sdhci_suspend_host(host); >>> + pinctrl_pm_select_sleep_state(dev); >> >> Why select a sleep state even if the suspend fails? >> > Hi Adrian, > > This is because sdhci_suspend_host() always return 0, So I think no need to check the return value 'ret'. It is safer not to assume sdhci_suspend_host() won't get changed sometime. > >>> + >>> + return ret; >>> } >>> >>> static int sdhci_esdhc_resume(struct device *dev) { >>> struct sdhci_host *host = dev_get_drvdata(dev); >>> >>> + pinctrl_pm_select_default_state(dev); >>> + >>> /* re-initialize hw state in case it's lost in low power mode */ >>> sdhci_esdhc_imx_hwinit(host); >>> >>> >