diff mbox

[13/23] mmc: sdhci-esdhc-imx: restore watermark level setting after resume

Message ID 1460741387-23815-14-git-send-email-aisheng.dong@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aisheng Dong April 15, 2016, 5:29 p.m. UTC
Currently, we config the watermark_level register only in probe.
This will cause the mmc write operation timeout issue after system
resume back in LPSR mode. Because in LPSR mode, after system resume
back, the watermark_level register(0x44) changes to 0x08000880, which
set the write watermark level as 0, and set the read watermark level
as 128. This value is incorrect.

This patch restores the setting of watermark level register after
system resume back.

Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Adrian Hunter May 10, 2016, 9:30 a.m. UTC | #1
On 15/04/16 20:29, Dong Aisheng wrote:
> Currently, we config the watermark_level register only in probe.
> This will cause the mmc write operation timeout issue after system
> resume back in LPSR mode. Because in LPSR mode, after system resume
> back, the watermark_level register(0x44) changes to 0x08000880, which
> set the write watermark level as 0, and set the read watermark level
> as 128. This value is incorrect.
> 
> This patch restores the setting of watermark level register after
> system resume back.

In sdhci_esdhc_imx_probe() the watermark setting is conditional on
esdhc_is_usdhc(imx_data), but in the resume it is not.  Is that intended?
If so, maybe explain that in the commit message.

Might be worth pulling out the constant 0x10401040 and #defining it since it
is used in more than one place too.

> 
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  drivers/mmc/host/sdhci-esdhc-imx.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> index 6fef6bc..4c28fbb 100644
> --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> @@ -1261,6 +1261,11 @@ static int sdhci_esdhc_suspend(struct device *dev)
>  
>  static int sdhci_esdhc_resume(struct device *dev)
>  {
> +	struct sdhci_host *host = dev_get_drvdata(dev);
> +
> +	/* restore watermark setting in case it's lost in low power mode */
> +	writel(0x10401040, host->ioaddr + ESDHC_WTMK_LVL);
> +
>  	return sdhci_pltfm_resume(dev);
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dong Aisheng May 31, 2016, 7:18 a.m. UTC | #2
On Tue, May 10, 2016 at 12:30:16PM +0300, Adrian Hunter wrote:
> On 15/04/16 20:29, Dong Aisheng wrote:
> > Currently, we config the watermark_level register only in probe.
> > This will cause the mmc write operation timeout issue after system
> > resume back in LPSR mode. Because in LPSR mode, after system resume
> > back, the watermark_level register(0x44) changes to 0x08000880, which
> > set the write watermark level as 0, and set the read watermark level
> > as 128. This value is incorrect.
> > 
> > This patch restores the setting of watermark level register after
> > system resume back.
> 
> In sdhci_esdhc_imx_probe() the watermark setting is conditional on
> esdhc_is_usdhc(imx_data), but in the resume it is not.  Is that intended?
> If so, maybe explain that in the commit message.
> 

Good catch!
I think we'd probably better keep that condition check.
Will update it in V2.


> Might be worth pulling out the constant 0x10401040 and #defining it since it
> is used in more than one place too.
> 

Good point!
Will do it.

Regards
Dong Aisheng

> > 
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> >  drivers/mmc/host/sdhci-esdhc-imx.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
> > index 6fef6bc..4c28fbb 100644
> > --- a/drivers/mmc/host/sdhci-esdhc-imx.c
> > +++ b/drivers/mmc/host/sdhci-esdhc-imx.c
> > @@ -1261,6 +1261,11 @@ static int sdhci_esdhc_suspend(struct device *dev)
> >  
> >  static int sdhci_esdhc_resume(struct device *dev)
> >  {
> > +	struct sdhci_host *host = dev_get_drvdata(dev);
> > +
> > +	/* restore watermark setting in case it's lost in low power mode */
> > +	writel(0x10401040, host->ioaddr + ESDHC_WTMK_LVL);
> > +
> >  	return sdhci_pltfm_resume(dev);
> >  }
> >  
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 6fef6bc..4c28fbb 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -1261,6 +1261,11 @@  static int sdhci_esdhc_suspend(struct device *dev)
 
 static int sdhci_esdhc_resume(struct device *dev)
 {
+	struct sdhci_host *host = dev_get_drvdata(dev);
+
+	/* restore watermark setting in case it's lost in low power mode */
+	writel(0x10401040, host->ioaddr + ESDHC_WTMK_LVL);
+
 	return sdhci_pltfm_resume(dev);
 }