On 24/09/19 6:11 AM, Y.b. Lu wrote:
> Hi Adrian,
>
>> -----Original Message-----
>> From: linux-mmc-owner@xxxxxxxxxxxxxxx <linux-mmc-owner@xxxxxxxxxxxxxxx>
>> On Behalf Of Adrian Hunter
>> Sent: Monday, September 23, 2019 8:56 PM
>> To: Y.b. Lu <yangbo.lu@xxxxxxx>; linux-mmc@xxxxxxxxxxxxxxx; Ulf Hansson
>> <ulf.hansson@xxxxxxxxxx>
>> Cc: Yinbo Zhu <yinbo.zhu@xxxxxxx>
>> Subject: Re: [PATCH 3/3] mmc: sdhci-of-esdhc: fix up erratum A-008171
>> workaround
>>
>> On 17/09/19 7:46 AM, Yangbo Lu wrote:
>>> A previous patch implemented an incomplete workaround of erratum
>>> A-008171. The complete workaround is as below. This patch is to
>>> implement the complete workaround which uses SW tuning if HW tuning
>>> fails, and retries both HW/SW tuning once with reduced clock if
>>> workaround fails. This is suggested by hardware team, and the patch
>>> had been verified on LS1046A eSDHC + Phison 32G eMMC which could
>>> trigger the erratum.
>>>
>>> Workaround:
>>> /* For T1040, T2080, LS1021A, T1023 Rev 1: */ 1. Program
>>> TBPTR[TB_WNDW_END_PTR] = 3*DIV_RATIO.
>>> 2. Program TBPTR[TB_WNDW_START_PTR] = 5*DIV_RATIO.
>>> 3. Program the software tuning mode by setting TBCTL[TB_MODE] = 2'h3.
>>> 4. Set SYSCTL2[EXTN] and SYSCTL2[SAMPCLKSEL].
>>> 5. Issue SEND_TUNING_BLK Command (CMD19 for SD, CMD21 for MMC).
>>> 6. Wait for IRQSTAT[BRR], buffer read ready, to be set.
>>> 7. Clear IRQSTAT[BRR].
>>> 8. Check SYSCTL2[EXTN] to be cleared.
>>> 9. Check SYSCTL2[SAMPCLKSEL], Sampling Clock Select. It's set value
>>> indicate tuning procedure success, and clear indicate failure.
>>> In case of tuning failure, fixed sampling scheme could be used by
>>> clearing TBCTL[TB_EN].
>>> /* For LS1080A Rev 1, LS2088A Rev 1.0, LA1575A Rev 1.0: */ 1. Read the
>>> TBCTL[31:0] register. Write TBCTL[11:8]=4'h8 and wait for
>>> 1ms.
>>> 2. Read the TBCTL[31:0] register and rewrite again. Wait for 1ms second.
>>> 3. Read the TBSTAT[31:0] register twice.
>>> 3.1 Reset data lines by setting ESDHCCTL[RSTD] bit.
>>> 3.2 Check ESDHCCTL[RSTD] bit.
>>> 3.3 If ESDHCCTL[RSTD] is 0, go to step 3.4 else go to step 3.2.
>>> 3.4 Write 32'hFFFF_FFFF to IRQSTAT register.
>>> 4. if TBSTAT[15:8]-TBSTAT[7:0] > 4*DIV_RATIO or TBSTAT[7:0]-TBSTAT[15:8]
>>> > 4*DIV_RATIO , then program TBPTR[TB_WNDW_END_PTR] =
>> 4*DIV_RATIO and
>>> program TBPTR[TB_WNDW_START_PTR] = 8*DIV_RATIO.
>>> /* For LS1012A Rev1, LS1043A Rev 1.x, LS1046A 1.0: */ 1. Read the
>>> TBCTL[0:31] register. Write TBCTL[20:23]=4'h8 and wait for
>>> 1ms.
>>> 2. Read the TBCTL[0:31] register and rewrite again. Wait for 1ms second.
>>> 3. Read the TBSTAT[0:31] register twice.
>>> 3.1 Reset data lines by setting ESDHCCTL[RSTD] bit.
>>> 3.2 Check ESDHCCTL[RSTD] bit.
>>> 3.3 If ESDHCCTL[RSTD] is 0, go to step 3.4 else go to step 3.2.
>>> 3.4 Write 32'hFFFF_FFFF to IRQSTAT register.
>>> 4. if TBSTAT[16:23]-TBSTAT[24:31] > 4*DIV_RATIO or TBSTAT[24:31]-
>>> TBSTAT[16:23] > 4* DIV_RATIO , then program
>> TBPTR[TB_WNDW_END_PTR] =
>>> 4*DIV_RATIO and program TBPTR[TB_WNDW_START_PTR] =
>> 8*DIV_RATIO.
>>> /* For LS1080A Rev 1, LS2088A Rev 1.0, LA1575A Rev 1.0 LS1012A Rev1,
>>> * LS1043A Rev 1.x, LS1046A 1.0:
>>> */
>>> 5. else program TBPTR[TB_WNDW_END_PTR] = 3*DIV_RATIO and program
>>> TBPTR[TB_WNDW_START_PTR] = 5*DIV_RATIO.
>>> 6. Program the software tuning mode by setting TBCTL[TB_MODE] = 2'h3.
>>> 7. Set SYSCTL2[EXTN], wait 1us and SYSCTL2[SAMPCLKSEL].
>>> 8. Issue SEND_TUNING_BLK Command (CMD19 for SD, CMD21 for MMC).
>>> 9. Wait for IRQSTAT[BRR], buffer read ready, to be set.
>>> 10. Clear IRQSTAT[BRR].
>>> 11. Check SYSCTL2[EXTN] to be cleared.
>>> 12. Check SYSCTL2[SAMPCLKSEL], Sampling Clock Select. It's set value
>>> indicate tuning procedure success, and clear indicate failure.
>>> In case of tuning failure, fixed sampling scheme could be used by
>>> clearing TBCTL[TB_EN].
>>>
>>> Fixes: b1f378ab5334 ("mmc: sdhci-of-esdhc: add erratum A008171
>>> support")
>>> Signed-off-by: Yinbo Zhu <yinbo.zhu@xxxxxxx>
>>> Signed-off-by: Yangbo Lu <yangbo.lu@xxxxxxx>
>>> ---
>>> drivers/mmc/host/sdhci-esdhc.h | 9 ++
>>> drivers/mmc/host/sdhci-of-esdhc.c | 216
>>> ++++++++++++++++++++++++++++++++------
>>> 2 files changed, 192 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci-esdhc.h
>>> b/drivers/mmc/host/sdhci-esdhc.h index 57b582b..e88dee5 100644
>>> --- a/drivers/mmc/host/sdhci-esdhc.h
>>> +++ b/drivers/mmc/host/sdhci-esdhc.h
>>> @@ -59,7 +59,16 @@
>>> #define ESDHC_HS400_WNDW_ADJUST 0x00000040
>>> #define ESDHC_HS400_MODE 0x00000010
>>> #define ESDHC_TB_EN 0x00000004
>>> +#define ESDHC_TB_MODE_MASK 0x00000003
>>> +#define ESDHC_TB_MODE_SW 0x00000003
>>> +#define ESDHC_TB_MODE_3 0x00000002
>>> +
>>> +#define ESDHC_TBSTAT 0x124
>>> +
>>> #define ESDHC_TBPTR 0x128
>>> +#define ESDHC_WNDW_STRT_PTR_SHIFT 8
>>> +#define ESDHC_WNDW_STRT_PTR_MASK (0x7f << 8)
>>> +#define ESDHC_WNDW_END_PTR_MASK 0x7f
>>>
>>> /* SD Clock Control Register */
>>> #define ESDHC_SDCLKCTL 0x144
>>> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
>>> b/drivers/mmc/host/sdhci-of-esdhc.c
>>> index a01d3a5..ea8d35f 100644
>>> --- a/drivers/mmc/host/sdhci-of-esdhc.c
>>> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
>>> @@ -77,8 +77,10 @@ struct sdhci_esdhc {
>>> bool quirk_incorrect_hostver;
>>> bool quirk_limited_clk_division;
>>> bool quirk_unreliable_pulse_detection;
>>> - bool quirk_fixup_tuning;
>>> + bool quirk_tuning_erratum_type1;
>>> + bool quirk_tuning_erratum_type2;
>>> bool quirk_ignore_data_inhibit;
>>> + bool in_sw_tuning;
>>> unsigned int peripheral_clock;
>>> const struct esdhc_clk_fixup *clk_fixup;
>>> u32 div_ratio;
>>> @@ -806,16 +808,21 @@ static int esdhc_signal_voltage_switch(struct
>> mmc_host *mmc,
>>> }
>>> }
>>>
>>> -static struct soc_device_attribute soc_fixup_tuning[] = {
>>> +static struct soc_device_attribute soc_tuning_erratum_type1[] = {
>>> + { .family = "QorIQ T1023", .revision = "1.0", },
>>> { .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", },
>>> + { },
>>> +};
>>> +
>>> +static struct soc_device_attribute soc_tuning_erratum_type2[] = {
>>> { .family = "QorIQ LS1012A", .revision = "1.0", },
>>> { .family = "QorIQ LS1043A", .revision = "1.*", },
>>> { .family = "QorIQ LS1046A", .revision = "1.0", },
>>> + { .family = "QorIQ LS1080A", .revision = "1.0", },
>>> + { .family = "QorIQ LS2080A", .revision = "1.0", },
>>> + { .family = "QorIQ LA1575A", .revision = "1.0", },
>>> { },
>>> };
>>>
>>> @@ -856,15 +863,97 @@ static void esdhc_tuning_block_enable(struct
>> sdhci_host *host, bool enable)
>>> esdhc_clock_enable(host, true);
>>> }
>>>
>>> +static void esdhc_prepare_sw_tuning(struct sdhci_host *host, u8
>> *window_start,
>>> + u8 *window_end)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host);
>>> + u8 tbstat_15_8, tbstat_7_0;
>>> + u32 val;
>>> +
>>> + if (esdhc->quirk_tuning_erratum_type1) {
>>> + *window_start = 5 * esdhc->div_ratio;
>>> + *window_end = 3 * esdhc->div_ratio;
>>> + return;
>>> + }
>>> +
>>> + /* Write TBCTL[11:8]=4'h8 */
>>> + val = sdhci_readl(host, ESDHC_TBCTL);
>>> + val &= ~(0xf << 8);
>>> + val |= 8 << 8;
>>> + sdhci_writel(host, val, ESDHC_TBCTL);
>>> +
>>> + mdelay(1);
>>> +
>>> + /* Read TBCTL[31:0] register and rewrite again */
>>> + val = sdhci_readl(host, ESDHC_TBCTL);
>>> + sdhci_writel(host, val, ESDHC_TBCTL);
>>> +
>>> + mdelay(1);
>>> +
>>> + /* Read the TBSTAT[31:0] register twice */
>>> + val = sdhci_readl(host, ESDHC_TBSTAT);
>>> + val = sdhci_readl(host, ESDHC_TBSTAT);
>>> +
>>> + /* Reset data lines by setting ESDHCCTL[RSTD] */
>>> + sdhci_reset(host, SDHCI_RESET_DATA);
>>> + /* Write 32'hFFFF_FFFF to IRQSTAT register */
>>> + sdhci_writel(host, 0xFFFFFFFF, SDHCI_INT_STATUS);
>>> +
>>> + /* If TBSTAT[15:8]-TBSTAT[7:0] > 4 * div_ratio
>>> + * or TBSTAT[7:0]-TBSTAT[15:8] > 4 * div_ratio,
>>> + * then program TBPTR[TB_WNDW_END_PTR] = 4 * div_ratio
>>> + * and program TBPTR[TB_WNDW_START_PTR] = 8 * div_ratio.
>>> + */
>>> + tbstat_7_0 = val & 0xff;
>>> + tbstat_15_8 = (val >> 8) & 0xff;
>>> +
>>> + if (abs(tbstat_15_8 - tbstat_7_0) > (4 * esdhc->div_ratio)) {
>>> + *window_start = 8 * esdhc->div_ratio;
>>> + *window_end = 4 * esdhc->div_ratio;
>>> + } else {
>>> + *window_start = 5 * esdhc->div_ratio;
>>> + *window_end = 3 * esdhc->div_ratio;
>>> + }
>>> +}
>>> +
>>> +static int esdhc_execute_sw_tuning(struct mmc_host *mmc, u32 opcode,
>>> + u8 window_start, u8 window_end) {
>>> + 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;
>>> + int ret;
>>> +
>>> + /* Program TBPTR[TB_WNDW_END_PTR] and
>> TBPTR[TB_WNDW_START_PTR] */
>>> + val = ((u32)window_start << ESDHC_WNDW_STRT_PTR_SHIFT) &
>>> + ESDHC_WNDW_STRT_PTR_MASK;
>>> + val |= window_end & ESDHC_WNDW_END_PTR_MASK;
>>> + 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 &= ~ESDHC_TB_MODE_MASK;
>>> + val |= ESDHC_TB_MODE_SW;
>>> + sdhci_writel(host, val, ESDHC_TBCTL);
>>> +
>>> + esdhc->in_sw_tuning = true;
>>> + ret = sdhci_execute_tuning(mmc, opcode);
>>> + esdhc->in_sw_tuning = false;
>>> + return ret;
>>> +}
>>> +
>>> 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);
>>> + u8 window_start, window_end;
>>> + int ret, retries = 1;
>>> bool hs400_tuning;
>>> unsigned int clk;
>>> u32 val;
>>> - int ret;
>>>
>>> /* For tuning mode, the sd clock divisor value
>>> * must be larger than 3 according to reference manual.
>>> @@ -873,39 +962,73 @@ static int esdhc_execute_tuning(struct mmc_host
>> *mmc, u32 opcode)
>>> if (host->clock > clk)
>>> esdhc_of_set_clock(host, clk);
>>>
>>> - if (esdhc->quirk_limited_clk_division &&
>>> - host->flags & SDHCI_HS400_TUNING)
>>> - esdhc_of_set_clock(host, host->clock);
>>> -
>>> esdhc_tuning_block_enable(host, true);
>>>
>>> hs400_tuning = host->flags & SDHCI_HS400_TUNING;
>>> - ret = sdhci_execute_tuning(mmc, opcode);
>>>
>>> - if (hs400_tuning) {
>>> - val = sdhci_readl(host, ESDHC_SDTIMNGCTL);
>>> - val |= ESDHC_FLW_CTL_BG;
>>> - sdhci_writel(host, val, ESDHC_SDTIMNGCTL);
>>> - }
>>> + do {
>>> + if (esdhc->quirk_limited_clk_division &&
>>> + hs400_tuning)
>>> + esdhc_of_set_clock(host, host->clock);
>>>
>>> - if (host->tuning_err == -EAGAIN && esdhc->quirk_fixup_tuning) {
>>> + /* Do HW tuning */
>>> + val = sdhci_readl(host, ESDHC_TBCTL);
>>> + val &= ~ESDHC_TB_MODE_MASK;
>>> + val |= ESDHC_TB_MODE_3;
>>> + sdhci_writel(host, val, ESDHC_TBCTL);
>>>
>>> - /* 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);
>>> + ret = sdhci_execute_tuning(mmc, opcode);
>>> + if (ret)
>>> + break;
>>>
>>> - /* program the software tuning mode by setting
>>> - * TBCTL[TB_MODE]=2'h3
>>> + /* If HW tuning fails and triggers erratum,
>>> + * try workaround.
>>> */
>>> - val = sdhci_readl(host, ESDHC_TBCTL);
>>> - val |= 0x3;
>>> - sdhci_writel(host, val, ESDHC_TBCTL);
>>> - sdhci_execute_tuning(mmc, opcode);
>>> + ret = host->tuning_err;
>>> + if (ret == -EAGAIN &&
>>> + (esdhc->quirk_tuning_erratum_type1 ||
>>> + esdhc->quirk_tuning_erratum_type2)) {
>>> + /* Recover HS400 tuning flag */
>>> + if (hs400_tuning)
>>> + host->flags |= SDHCI_HS400_TUNING;
>>> + pr_info("%s: Hold on to use fixed sampling clock. Try SW
>> tuning!\n",
>>> + mmc_hostname(mmc));
>>> + /* Do SW tuning */
>>> + esdhc_prepare_sw_tuning(host, &window_start,
>>> + &window_end);
>>> + ret = esdhc_execute_sw_tuning(mmc, opcode,
>>> + window_start,
>>> + window_end);
>>> + if (ret)
>>> + break;
>>> +
>>> + /* Retry both HW/SW tuning with reduced clock. */
>>> + ret = host->tuning_err;
>>> + if (ret == -EAGAIN && retries) {
>>> + /* Recover HS400 tuning flag */
>>> + if (hs400_tuning)
>>> + host->flags |= SDHCI_HS400_TUNING;
>>> +
>>> + clk = host->max_clk / (esdhc->div_ratio + 1);
>>> + esdhc_of_set_clock(host, clk);
>>> + pr_info("%s: Hold on to use fixed sampling clock. Try
>> tuning with reduced clock!\n",
>>> + mmc_hostname(mmc));
>>> + } else {
>>> + break;
>>> + }
>>> + } else {
>>> + break;
>>> + }
>>> + } while (retries--);
>>> +
>>> + if (ret) {
>>> + esdhc_tuning_block_enable(host, false);
>>> + } else if (hs400_tuning) {
>>> + val = sdhci_readl(host, ESDHC_SDTIMNGCTL);
>>> + val |= ESDHC_FLW_CTL_BG;
>>> + sdhci_writel(host, val, ESDHC_SDTIMNGCTL);
>>> }
>>> +
>>> return ret;
>>> }
>>>
>>> @@ -937,6 +1060,26 @@ static u32 esdhc_irq(struct sdhci_host *host, u32
>> intmask)
>>> return intmask;
>>> }
>>>
>>> +void esdhc_start_tuning(struct sdhci_host *host) {
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_esdhc *esdhc = sdhci_pltfm_priv(pltfm_host);
>>> + u16 ctrl;
>>> +
>>> + ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> + ctrl |= SDHCI_CTRL_EXEC_TUNING;
>>> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>>> +
>>> + if (esdhc->in_sw_tuning) {
>>> + udelay(1);
>>> + ctrl |= SDHCI_CTRL_TUNED_CLK;
>>> + sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
>>
>> Why not do that in the ->write_w() callback for SDHCI_HOST_CONTROL2
>> instead of adding ->start_tuning()?
>
> [Y.b. Lu] The purpose is to set SDHCI_CTRL_TUNED_CLK bit just for starting tuning.
> I don’t think ->write_w() could check the condition of starting tuning, with esdhc->in_sw_tuning flag and SDHCI_CTRL_EXEC_TUNING bit set.
It seems to me that 0->1 transition of SDHCI_CTRL_EXEC_TUNING starts tuning
so it is equivalent to a ->start_tuning() callback.
> There is possibility SDHCI_HOST_CONTROL2 register will be written for several times in tuning in the future.
Can you expand on what you mean?
>
> Thanks.
>>
>>> + }
>>> +
>>> + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
>>> + sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE); }
>>> +
>>> #ifdef CONFIG_PM_SLEEP
>>> static u32 esdhc_proctl;
>>> static int esdhc_of_suspend(struct device *dev) @@ -985,6 +1128,7 @@
>>> static const struct sdhci_ops sdhci_esdhc_be_ops = {
>>> .reset = esdhc_reset,
>>> .set_uhs_signaling = esdhc_set_uhs_signaling,
>>> .irq = esdhc_irq,
>>> + .start_tuning = esdhc_start_tuning,
>>> };
>>>
>>> static const struct sdhci_ops sdhci_esdhc_le_ops = { @@ -1003,6
>>> +1147,7 @@ static const struct sdhci_ops sdhci_esdhc_le_ops = {
>>> .reset = esdhc_reset,
>>> .set_uhs_signaling = esdhc_set_uhs_signaling,
>>> .irq = esdhc_irq,
>>> + .start_tuning = esdhc_start_tuning,
>>> };
>>>
>>> static const struct sdhci_pltfm_data sdhci_esdhc_be_pdata = { @@
>>> -1140,10 +1285,15 @@ 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;
>>> + if (soc_device_match(soc_tuning_erratum_type1))
>>> + esdhc->quirk_tuning_erratum_type1 = true;
>>> + else
>>> + esdhc->quirk_tuning_erratum_type1 = false;
>>> +
>>> + if (soc_device_match(soc_tuning_erratum_type2))
>>> + esdhc->quirk_tuning_erratum_type2 = true;
>>> else
>>> - esdhc->quirk_fixup_tuning = false;
>>> + esdhc->quirk_tuning_erratum_type2 = false;
>>>
>>> if (esdhc->vendor_ver == VENDOR_V_22)
>>> host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;
>>>
>
