On few SDHCI-MSM controllers, the host controller's clock tuning
circuit may go out of sync if controller clocks are gated which
eventually will result in data CRC, command CRC/timeout errors.
To overcome this h/w limitation, the DLL needs to be re-initialized
and restored with its old settings once clocks are ungated.
Signed-off-by: Veerabhadrarao Badiganti <[email protected]>
---
drivers/mmc/host/sdhci-msm.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 486462d..4a83850 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
#include <linux/of_device.h>
#include <linux/delay.h>
#include <linux/mmc/mmc.h>
+#include <linux/mmc/host.h>
#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/iopoll.h>
@@ -264,6 +265,8 @@ struct sdhci_msm_host {
u32 vmmc_level[2];
bool vqmmc_load;
u32 vqmmc_level[2];
+ bool restore_dll_cfg_needed;
+ bool restore_dll_cfg;
};
static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
@@ -1068,6 +1071,20 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
if (rc)
return rc;
+ /*
+ * If restore dll config flag is set, then just program the DLL with
+ * last saved tuning phase. If this operation fails, proceed with
+ * full-fledged tuning.
+ */
+ if (msm_host->restore_dll_cfg) {
+ rc = msm_config_cm_dll_phase(host,
+ msm_host->saved_tuning_phase);
+ msm_host->restore_dll_cfg = false;
+ if (!rc)
+ return rc;
+
+ }
+
phase = 0;
do {
/* Set the phase in delay line hw block */
@@ -1075,7 +1092,6 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
if (rc)
return rc;
- msm_host->saved_tuning_phase = phase;
rc = mmc_send_tuning(mmc, opcode, NULL);
if (!rc) {
/* Tuning is successful at this tuning point */
@@ -1100,6 +1116,7 @@ static int sdhci_msm_execute_tuning(struct mmc_host *mmc, u32 opcode)
rc = msm_config_cm_dll_phase(host, phase);
if (rc)
return rc;
+ msm_host->saved_tuning_phase = phase;
dev_dbg(mmc_dev(mmc), "%s: Setting the tuning phase to %d\n",
mmc_hostname(mmc), phase);
} else {
@@ -2049,6 +2066,9 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
+ if (device_property_read_bool(&pdev->dev, "qcom,restore-dll-config"))
+ msm_host->restore_dll_cfg_needed = true;
+
pm_runtime_get_noresume(&pdev->dev);
pm_runtime_set_active(&pdev->dev);
pm_runtime_enable(&pdev->dev);
@@ -2124,6 +2144,15 @@ static int sdhci_msm_runtime_resume(struct device *dev)
struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
+ /*
+ * Whenever core-clock is gated dynamically, it's needed to
+ * re-initialize the DLL when the clock is ungated.
+ */
+ if (msm_host->restore_dll_cfg_needed && msm_host->clk_rate) {
+ msm_host->restore_dll_cfg = true;
+ mmc_retune_needed(host->mmc);
+ }
+
return clk_bulk_prepare_enable(ARRAY_SIZE(msm_host->bulk_clks),
msm_host->bulk_clks);
}
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Hi,
On Tue, Sep 25, 2018 at 8:05 AM Veerabhadrarao Badiganti
<[email protected]> wrote:
> + /*
> + * Whenever core-clock is gated dynamically, it's needed to
> + * re-initialize the DLL when the clock is ungated.
> + */
> + if (msm_host->restore_dll_cfg_needed && msm_host->clk_rate) {
> + msm_host->restore_dll_cfg = true;
> + mmc_retune_needed(host->mmc);
Using the boolean "restore_dll_cfg" to communicate like this seems
really fragile. I have no basis in fact, but I worry that something
will happen in the meantime that really ought to invalidate the
"saved_tuning_phase" but the boolean will still be set.
Is there a reason you can't just call msm_config_cm_dll_phase()
directly from sdhci_msm_runtime_resume()? Perhaps after the
clk_bulk_prepare_enable() call below?
-Doug
Hi Doug,
On 9/26/2018 3:58 AM, Doug Anderson wrote:
> Hi,
>
> On Tue, Sep 25, 2018 at 8:05 AM Veerabhadrarao Badiganti
> <[email protected]> wrote:
>> + /*
>> + * Whenever core-clock is gated dynamically, it's needed to
>> + * re-initialize the DLL when the clock is ungated.
>> + */
>> + if (msm_host->restore_dll_cfg_needed && msm_host->clk_rate) {
>> + msm_host->restore_dll_cfg = true;
>> + mmc_retune_needed(host->mmc);
> Using the boolean "restore_dll_cfg" to communicate like this seems
> really fragile. I have no basis in fact, but I worry that something
> will happen in the meantime that really ought to invalidate the
> "saved_tuning_phase" but the boolean will still be set.
>
> Is there a reason you can't just call msm_config_cm_dll_phase()
> directly from sdhci_msm_runtime_resume()? Perhaps after the
> clk_bulk_prepare_enable() call below?
I can do that.
But we don't need to restore the clock phase for all speed modes since
every mode
doesn't use DLL. Only few speed modes (only SDR104 mode for SD card)
would need to restore DLL setting. So I would need to do some extra checks
for performing this operation for only required speed modes.
And we should re-initialize the DLL first before programming the phase.
I wanted to reuse the existing logic which does all these things. So did
this way.
>
> -Doug
Thanks
Veera
Hi,
On Thu, Sep 27, 2018 at 4:00 AM Veerabhadrarao Badiganti
<[email protected]> wrote:
>
> Hi Doug,
>
>
> On 9/26/2018 3:58 AM, Doug Anderson wrote:
> > Hi,
> >
> > On Tue, Sep 25, 2018 at 8:05 AM Veerabhadrarao Badiganti
> > <[email protected]> wrote:
> >> + /*
> >> + * Whenever core-clock is gated dynamically, it's needed to
> >> + * re-initialize the DLL when the clock is ungated.
> >> + */
> >> + if (msm_host->restore_dll_cfg_needed && msm_host->clk_rate) {
> >> + msm_host->restore_dll_cfg = true;
> >> + mmc_retune_needed(host->mmc);
> > Using the boolean "restore_dll_cfg" to communicate like this seems
> > really fragile. I have no basis in fact, but I worry that something
> > will happen in the meantime that really ought to invalidate the
> > "saved_tuning_phase" but the boolean will still be set.
> >
> > Is there a reason you can't just call msm_config_cm_dll_phase()
> > directly from sdhci_msm_runtime_resume()? Perhaps after the
> > clk_bulk_prepare_enable() call below?
>
> I can do that.
>
> But we don't need to restore the clock phase for all speed modes since
> every mode
> doesn't use DLL. Only few speed modes (only SDR104 mode for SD card)
> would need to restore DLL setting. So I would need to do some extra checks
> for performing this operation for only required speed modes.
> And we should re-initialize the DLL first before programming the phase.
>
> I wanted to reuse the existing logic which does all these things. So did
> this way.
Personally I'd rather see the extra checks or even restore the DLL
settings some cases when it wasn't truly needed.
In theory, though, I'd expect that there's a hardware register
somewhere that says whether the DLL settings matter or not. Can't you
just key off of that? ...or if that register is also lost when the
clock turns off then whenever you change that register to turn the DLL
off then clear your saved phase?
-Doug