diff mbox series

mmc: sdhci-esdhc-imx: restore pin state when resume back

Message ID 20181227114256.711-1-haibo.chen@nxp.com (mailing list archive)
State New, archived
Headers show
Series mmc: sdhci-esdhc-imx: restore pin state when resume back | expand

Commit Message

Bough Chen Dec. 27, 2018, 11:35 a.m. UTC
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@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Adrian Hunter Dec. 28, 2018, 1:58 p.m. UTC | #1
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@nxp.com>
> ---
>  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?

> +
> +	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);
>  
>
Bough Chen Jan. 2, 2019, 10:40 a.m. UTC | #2
> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
> Sent: 2018年12月28日 21:59
> To: BOUGH CHEN <haibo.chen@nxp.com>; ulf.hansson@linaro.org
> Cc: linux-mmc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> 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@nxp.com>
> > ---
> >  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'.

> > +
> > +	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);
> >
> >
Adrian Hunter Jan. 10, 2019, 1:26 p.m. UTC | #3
On 2/01/19 12:40 PM, BOUGH CHEN wrote:
>> -----Original Message-----
>> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
>> Sent: 2018年12月28日 21:59
>> To: BOUGH CHEN <haibo.chen@nxp.com>; ulf.hansson@linaro.org
>> Cc: linux-mmc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
>> 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@nxp.com>
>>> ---
>>>  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);
>>>
>>>
>
diff mbox series

Patch

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);
+
+	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);