Re: [PATCH v3] mmc: sdhci-msm: Add pm_runtime and system PM support

From: Ritesh Harjani
Date: Thu Sep 22 2016 - 10:33:10 EST


Hi Pramod,

On 9/15/2016 7:28 PM, Pramod Gurav wrote:
On 15 September 2016 at 15:49, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
On 15 September 2016 at 09:59, Pramod Gurav <pramod.gurav@xxxxxxxxxx> wrote:
On 9 September 2016 at 15:48, Georgi Djakov <georgi.djakov@xxxxxxxxxx> wrote:
On 09/08/2016 11:02 AM, Adrian Hunter wrote:

On 01/09/16 17:23, Pramod Gurav wrote:

Provides runtime PM callbacks to enable and disable clock resources
when idle. Also support system PM callbacks to be called during system
suspend and resume.

Signed-off-by: Pramod Gurav <pramod.gurav@xxxxxxxxxx>


Can we get some Tested/Reviewed/Acked-by from people using this driver?


Hi Pramod,
Thanks for the patch. Unfortunately, my db410c board fails to
boot when i apply it.


Thanks Georgi for testing the patch. Its my wrong I did not update my
kernel and continued fixing comments on old kernel.
After spending some time I came to know that below change is causing the issue:

Author: Dong Aisheng <aisheng.dong@xxxxxxx>
Date: Tue Jul 12 15:46:17 2016 +0800

mmc: sdhci: add standard hw auto retuning support

If HW supports SDHCI_TUNING_MODE_3 which is auto retuning, we won't
retune during runtime suspend and resume, instead we use Re-tuning
Request signaled via SDHCI_INT_RETUNE interrupt to do retuning and
hw auto retuning during data transfer to guarantee the signal sample
window correction.

This can avoid a mass of repeatedly retuning during small file system
data access and improve the performance.

Specially these lines that was added to suspend path:

+ if (host->tuning_mode != SDHCI_TUNING_MODE_3)
+ mmc_retune_needed(host->mmc);

During sdhci setup in msm driver, the host returns the values to set
sdhci auto tuning as supported.
Hence host->tuning_mode is set to SDHCI_TUNING_MODE_3 during setup.
But some how the auto tuning is not happening.
Just to verify my case, I removed the 'if' part in above code and got
the FS mounted.

Is there anything else needed in msm sdhci driver so that the auto
tuning is taken care of?

I am not familiar with any other than sdhci-esdhc-imx which supports
the SDHCI_TUNING_MODE_3. I may be wrong though.

In the sdhci-esdhc-imx case, enabling of auto tuning seems to be done
in esdhc_post_tuning(), where a vendor specific register
(ESDHC_MIX_CTRL) is being written to. Perhaps something similar in
your case?

Thanks Ulf for the comments. Will check this and see if there is
something of this sort we have to do to achieve auto tuning.
Adding Ritesh who has been posting some SDHCI MSM patches recently in
case he knows about this.

Internally, we don't use this Auto re-tuning and rely on explicit re-tune by host driver.

Question though -
1. why do we need to call sdhci_runtime_resume/suspend from sdhci_msm_runtime_suspend/resume?
From what I see is, sdhci_runtime_susend/resume will do reset and re-program of host->pwr and host->clk because of which a retune will be required for the next command after runtime resume.

We can *only* disable and enable the clocks in sdhci_msm_runtime_suspend/resume?
Thoughts? With this, I suppose you would not see any issue.


Though for this issue, since internally also auto retuning is never used, we can have this mode disabled. I can once again check with HW team to get more details about this mode for MSM controller.


Regards,
Pramod