From: Seungwon Jeon <tgih.jun@samsung.com>
To: "'Doug Anderson'" <dianders@chromium.org>, "'Chris Ball'" <cjb@laptop.org>
Cc: "'Olof Johansson'" <olof@lixom.net>,
        "'Jaehoon Chung'" <jh80.chung@samsung.com>,
        "'James Hogan'" <james.hogan@imgtec.com>,
        "'Grant Grundler'" <grundler@chromium.org>,
        "'Alim Akhtar'" <alim.akhtar@samsung.com>,
        "'Abhilash Kesavan'" <a.kesavan@samsung.com>,
        "'Tomasz Figa'" <tomasz.figa@gmail.com>, linux-mmc@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <1373470926-19314-1-git-send-email-dianders@chromium.org>
 <1375825071-20922-1-git-send-email-dianders@chromium.org>
 <1375825071-20922-4-git-send-email-dianders@chromium.org>
In-reply-to: <1375825071-20922-4-git-send-email-dianders@chromium.org>
Subject: RE: [PATCH v4 3/4] mmc: dw_mmc: Always setup the bus after
 suspend/resume
Date: Fri, 09 Aug 2013 22:35:45 +0900
Message-id: <001a01ce9505$59c28b20$0d47a160$%jun@samsung.com>
MIME-version: 1.0
Content-type: text/plain; charset=ks_c_5601-1987
Content-transfer-encoding: 7bit
Thread-index: Ac6S7VFYXAi7gls0T8+n2OoLvfQf2wB+K0Nw
Content-language: ko
DLP-Filter: Pass
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2708
Lines: 75

On Wed, August 07, 2013, Doug Anderson wrote:
> After suspend/resume all of the dw_mmc registers are reset to
> defaults.  We restore most of them, but specifically don't setup the
> clock registers after resume unless we've got a powered card.  Things
> still work because the core will eventually call set_ios() and we'll
> set things up.

Hmm, I didn't get the need of this call during resume.
I think set_ios is only valid where core layer calls.
Besides, important things is ios's parameters.
If suspend has finished successfully, last call of set_ios() is from mmc_power_off().
On seeing fields of 'mmc->ios' stored last, these values aren't proper in resume phase.
Please check mmc_power_off() function.
In case MMC_PM_KEEP_POWER it could be kept.

Thanks,
Seungwon Jeon
> 
> There doesn't seem to be any reason that I can see _not_ to set things
> up after resume.  Restoring this state makes the code easier to reason
> about and should help prevent bugs.  It also allows us to do a
> register dump before and after suspend/resume to confirm that we've
> set things up OK.
> 
> I examined the state of the dw_mmc instance before and after suspend
> after this patch.  I had no card inserted in an SD card slot.
> 
> Before this patch, differences were:
> * CLKDIV (0x08)
> * CLKENA (0x10)
> * TMOUT (0x14)
> * CMD (0x2C) - difference is not important
> * CLKSEL (0x9C - exynos specific)
> 
> After this patch, only TMOUT was different.  I have a separate patch
> for that.
> 
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> ---
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/mmc/host/dw_mmc.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 13a363c..0fa3135 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2522,9 +2522,7 @@ int dw_mci_resume(struct dw_mci *host)
>  		struct dw_mci_slot *slot = host->slot[i];
>  		if (!slot)
>  			continue;
> -		if (slot->mmc->pm_flags & MMC_PM_KEEP_POWER) {
> -			dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
> -		}
> +		dw_mci_set_ios(slot->mmc, &slot->mmc->ios);
> 
>  		ret = mmc_resume_host(host->slot[i]->mmc);
>  		if (ret < 0)
> --
> 1.8.3
> 
> --
> 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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/