| Message ID | 20180720102512.17091-1-yinbo.zhu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 20 July 2018 at 12:25, Yinbo Zhu <yinbo.zhu@nxp.com> wrote: > In tuning mode of operation, when TBCTL[TB_EN] is set, eSDHC may report > one of the following errors : > 1)Tuning error while running tuning operation where SYSCTL2[SAMPCLKSEL] > will not get set even when SYSCTL2[EXTN] is reset. OR > 2)Data transaction error (e.g. IRQSTAT[DCE], IRQSTAT[DEBE]) during data > transaction errors. > This issue occurs when the data window sampled within eSDHC is in full > cycle. So, in that case, eSDHC is not able to find out the start and > end points of the data window and sets the sampling pointer at default > location (which is middle of the internal SD clock). If this sampling > point coincides with the data eye boundary, then it can result in the > above mentioned errors. Impact: Tuning mode of operation for SDR50, > SDR104 or HS200 speed modes may not work properly > Workaround: In case eSDHC reports tuning error or data errors in tuning > mode of operation, by add the erratum A008171 support to fix the issue. > > Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com> > --- > Change in v2: > Clear the tuning flag after execute erratum and add a SoC variable > > drivers/mmc/host/sdhci-esdhc.h | 1 + > drivers/mmc/host/sdhci-of-esdhc.c | 47 ++++++++++++++++++++++++++++++++++++- > drivers/mmc/host/sdhci.c | 1 + > drivers/mmc/host/sdhci.h | 1 + > 4 files changed, 49 insertions(+), 1 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h > index dfa58f8..3f16d9c 100644 > --- a/drivers/mmc/host/sdhci-esdhc.h > +++ b/drivers/mmc/host/sdhci-esdhc.h > @@ -60,6 +60,7 @@ > /* Tuning Block Control Register */ > #define ESDHC_TBCTL 0x120 > #define ESDHC_TB_EN 0x00000004 > +#define ESDHC_TBPTR 0x128 > > /* Control Register for DMA transfer */ > #define ESDHC_DMA_SYSCTL 0x40c > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c > index 4ffa6b1..497a570 100644 > --- a/drivers/mmc/host/sdhci-of-esdhc.c > +++ b/drivers/mmc/host/sdhci-of-esdhc.c > @@ -26,6 +26,8 @@ > #include "sdhci-pltfm.h" > #include "sdhci-esdhc.h" > > +#define DRIVER_NAME "sdhci-esdhc" > + Leftover from debugging? > #define VENDOR_V_22 0x12 > #define VENDOR_V_23 0x13 > > @@ -33,7 +35,9 @@ struct sdhci_esdhc { > u8 vendor_ver; > u8 spec_ver; > bool quirk_incorrect_hostver; > + bool quirk_fixup_tuning; > unsigned int peripheral_clock; > + u32 div_ratio; > }; > > /** > @@ -541,6 +545,7 @@ static void esdhc_of_set_clock(struct sdhci_host *host, unsigned int clock) > dev_dbg(mmc_dev(host->mmc), "desired SD clock: %d, actual: %d\n", > clock, host->max_clk / pre_div / div); > host->mmc->actual_clock = host->max_clk / pre_div / div; > + esdhc->div_ratio = pre_div * div; > pre_div >>= 1; > div--; > > @@ -673,9 +678,24 @@ static int esdhc_signal_voltage_switch(struct mmc_host *mmc, > } > } > > +static struct soc_device_attribute soc_fixup_tuning[] = { > + { .family = "QorIQ T1040", .revision = "1.0", }, > + { .family = "QorIQ T2080", .revision = "1.0", }, > + { .family = "QorIQ T1023", .revision = "1.0", }, > + { .family = "QorIQ LS1021A", .revision = "1.0", }, > + { .family = "QorIQ LS1080A", .revision = "1.0", }, > + { .family = "QorIQ LS2080A", .revision = "1.0", }, > + { .family = "QorIQ LS1012A", .revision = "1.0", }, > + { .family = "QorIQ LS1043A", .revision = "1.*", }, > + { .family = "QorIQ LS1046A", .revision = "1.0", }, > + { }, > +}; > + > static int esdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) > { > struct sdhci_host *host = mmc_priv(mmc); > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host); > u32 val; > > /* Use tuning block for tuning procedure */ > @@ -689,7 +709,27 @@ static int esdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) > sdhci_writel(host, val, ESDHC_TBCTL); > esdhc_clock_enable(host, true); > > - return sdhci_execute_tuning(mmc, opcode); > + sdhci_execute_tuning(mmc, opcode); > + if (host->tuning_flag && esdhc->quirk_fixup_tuning) { Well, to me, this still doesn't seems right. Should you really ignore the errors returned from sdhci_execute_tuning()? Perhaps another way to solve the problem, would be to let __sdhci_execute_tuning() to change from being "void" to return an error code. Then, let sdhci_execute_tuning() propagate its error code. In that way, the callers of sdhci_execute_tuning() get more information about what happens and can act accordingly. Would that work? > + > + /* program TBPTR[TB_WNDW_END_PTR] = 3*DIV_RATIO and > + * program TBPTR[TB_WNDW_START_PTR] = 5*DIV_RATIO > + */ > + val = sdhci_readl(host, ESDHC_TBPTR); > + val = (val & ~((0x7f << 8) | 0x7f)) | > + (3 * esdhc->div_ratio) | ((5 * esdhc->div_ratio) << 8); > + sdhci_writel(host, val, ESDHC_TBPTR); > + > + /* program the software tuning mode by setting > + * TBCTL[TB_MODE]=2'h3 > + */ > + val = sdhci_readl(host, ESDHC_TBCTL); > + val |= 0x3; > + sdhci_writel(host, val, ESDHC_TBCTL); > + sdhci_execute_tuning(mmc, opcode); > + } > + host->tuning_flag = 0; > + return 0; > } > > #ifdef CONFIG_PM_SLEEP > @@ -860,6 +900,11 @@ static int sdhci_esdhc_probe(struct platform_device *pdev) > > pltfm_host = sdhci_priv(host); > esdhc = sdhci_pltfm_priv(pltfm_host); > + if (soc_device_match(soc_fixup_tuning)) > + esdhc->quirk_fixup_tuning = true; > + else > + esdhc->quirk_fixup_tuning = false; > + > if (esdhc->vendor_ver == VENDOR_V_22) > host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23; > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index d35deb7..36a2b3e 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -2161,6 +2161,7 @@ static void __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode) > if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) { > if (ctrl & SDHCI_CTRL_TUNED_CLK) > return; /* Success! */ > + host->tuning_flag = 1; > break; > } > > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 1d7d61e..6490267 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -540,6 +540,7 @@ struct sdhci_host { > > unsigned int tuning_count; /* Timer count for re-tuning */ > unsigned int tuning_mode; /* Re-tuning mode supported by host */ > + unsigned int tuning_flag; /* Re-tuning flag */ > #define SDHCI_TUNING_MODE_1 0 > #define SDHCI_TUNING_MODE_2 1 > #define SDHCI_TUNING_MODE_3 2 > -- Kind regards Uffe -- 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 --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h index dfa58f8..3f16d9c 100644 --- a/drivers/mmc/host/sdhci-esdhc.h +++ b/drivers/mmc/host/sdhci-esdhc.h @@ -60,6 +60,7 @@ /* Tuning Block Control Register */ #define ESDHC_TBCTL 0x120 #define ESDHC_TB_EN 0x00000004 +#define ESDHC_TBPTR 0x128 /* Control Register for DMA transfer */ #define ESDHC_DMA_SYSCTL 0x40c diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c index 4ffa6b1..497a570 100644 --- a/drivers/mmc/host/sdhci-of-esdhc.c +++ b/drivers/mmc/host/sdhci-of-esdhc.c @@ -26,6 +26,8 @@ #include "sdhci-pltfm.h" #include "sdhci-esdhc.h" +#define DRIVER_NAME "sdhci-esdhc" + #define VENDOR_V_22 0x12 #define VENDOR_V_23 0x13 @@ -33,7 +35,9 @@ struct sdhci_esdhc { u8 vendor_ver; u8 spec_ver; bool quirk_incorrect_hostver; + bool quirk_fixup_tuning; unsigned int peripheral_clock; + u32 div_ratio; }; /** @@ -541,6 +545,7 @@ static void esdhc_of_set_clock(struct sdhci_host *host, unsigned int clock) dev_dbg(mmc_dev(host->mmc), "desired SD clock: %d, actual: %d\n", clock, host->max_clk / pre_div / div); host->mmc->actual_clock = host->max_clk / pre_div / div; + esdhc->div_ratio = pre_div * div; pre_div >>= 1; div--; @@ -673,9 +678,24 @@ static int esdhc_signal_voltage_switch(struct mmc_host *mmc, } } +static struct soc_device_attribute soc_fixup_tuning[] = { + { .family = "QorIQ T1040", .revision = "1.0", }, + { .family = "QorIQ T2080", .revision = "1.0", }, + { .family = "QorIQ T1023", .revision = "1.0", }, + { .family = "QorIQ LS1021A", .revision = "1.0", }, + { .family = "QorIQ LS1080A", .revision = "1.0", }, + { .family = "QorIQ LS2080A", .revision = "1.0", }, + { .family = "QorIQ LS1012A", .revision = "1.0", }, + { .family = "QorIQ LS1043A", .revision = "1.*", }, + { .family = "QorIQ LS1046A", .revision = "1.0", }, + { }, +}; + static int esdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) { struct sdhci_host *host = mmc_priv(mmc); + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); + struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host); u32 val; /* Use tuning block for tuning procedure */ @@ -689,7 +709,27 @@ static int esdhc_execute_tuning(struct mmc_host *mmc, u32 opcode) sdhci_writel(host, val, ESDHC_TBCTL); esdhc_clock_enable(host, true); - return sdhci_execute_tuning(mmc, opcode); + sdhci_execute_tuning(mmc, opcode); + if (host->tuning_flag && esdhc->quirk_fixup_tuning) { + + /* program TBPTR[TB_WNDW_END_PTR] = 3*DIV_RATIO and + * program TBPTR[TB_WNDW_START_PTR] = 5*DIV_RATIO + */ + val = sdhci_readl(host, ESDHC_TBPTR); + val = (val & ~((0x7f << 8) | 0x7f)) | + (3 * esdhc->div_ratio) | ((5 * esdhc->div_ratio) << 8); + sdhci_writel(host, val, ESDHC_TBPTR); + + /* program the software tuning mode by setting + * TBCTL[TB_MODE]=2'h3 + */ + val = sdhci_readl(host, ESDHC_TBCTL); + val |= 0x3; + sdhci_writel(host, val, ESDHC_TBCTL); + sdhci_execute_tuning(mmc, opcode); + } + host->tuning_flag = 0; + return 0; } #ifdef CONFIG_PM_SLEEP @@ -860,6 +900,11 @@ static int sdhci_esdhc_probe(struct platform_device *pdev) pltfm_host = sdhci_priv(host); esdhc = sdhci_pltfm_priv(pltfm_host); + if (soc_device_match(soc_fixup_tuning)) + esdhc->quirk_fixup_tuning = true; + else + esdhc->quirk_fixup_tuning = false; + if (esdhc->vendor_ver == VENDOR_V_22) host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23; diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index d35deb7..36a2b3e 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -2161,6 +2161,7 @@ static void __sdhci_execute_tuning(struct sdhci_host *host, u32 opcode) if (!(ctrl & SDHCI_CTRL_EXEC_TUNING)) { if (ctrl & SDHCI_CTRL_TUNED_CLK) return; /* Success! */ + host->tuning_flag = 1; break; } diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index 1d7d61e..6490267 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -540,6 +540,7 @@ struct sdhci_host { unsigned int tuning_count; /* Timer count for re-tuning */ unsigned int tuning_mode; /* Re-tuning mode supported by host */ + unsigned int tuning_flag; /* Re-tuning flag */ #define SDHCI_TUNING_MODE_1 0 #define SDHCI_TUNING_MODE_2 1 #define SDHCI_TUNING_MODE_3 2
In tuning mode of operation, when TBCTL[TB_EN] is set, eSDHC may report one of the following errors : 1)Tuning error while running tuning operation where SYSCTL2[SAMPCLKSEL] will not get set even when SYSCTL2[EXTN] is reset. OR 2)Data transaction error (e.g. IRQSTAT[DCE], IRQSTAT[DEBE]) during data transaction errors. This issue occurs when the data window sampled within eSDHC is in full cycle. So, in that case, eSDHC is not able to find out the start and end points of the data window and sets the sampling pointer at default location (which is middle of the internal SD clock). If this sampling point coincides with the data eye boundary, then it can result in the above mentioned errors. Impact: Tuning mode of operation for SDR50, SDR104 or HS200 speed modes may not work properly Workaround: In case eSDHC reports tuning error or data errors in tuning mode of operation, by add the erratum A008171 support to fix the issue. Signed-off-by: Yinbo Zhu <yinbo.zhu@nxp.com> --- Change in v2: Clear the tuning flag after execute erratum and add a SoC variable drivers/mmc/host/sdhci-esdhc.h | 1 + drivers/mmc/host/sdhci-of-esdhc.c | 47 ++++++++++++++++++++++++++++++++++++- drivers/mmc/host/sdhci.c | 1 + drivers/mmc/host/sdhci.h | 1 + 4 files changed, 49 insertions(+), 1 deletions(-)