Re: [PATCH v5 3/3] mmc: mediatek: Use data tune for CMD line tune

[Ulf Hansson]

On 15 March 2017 at 08:26, Yong Mao <yong.mao@xxxxxxxxxxxx> wrote:
> From: yong mao <yong.mao@xxxxxxxxxxxx>
>
> If we don't select a set of better parameters for our emmc host,
> It may easily occur CMD response CRC error. And also it may cause
> cannot boot up issue.
>
> Fot getting a set of better parameters, our emmc host supports
> data tune mechanism.Therefore, our emmc driver also should change
> to use data tune for CMD line.
>
> Because our emmc host use the different clock source to sample the
> CMD signal between HS200 and HS400 mode, the parameters are also
> different between these two modes.
> Separate cmd internal delay for HS200/HS400 mode.
>
> This change can fix "System can not boot up" issue.
>
> Signed-off-by: Yong Mao <yong.mao@xxxxxxxxxxxx>
> Signed-off-by: Chaotian Jing <chaotian.jing@xxxxxxxxxxxx>


Thanks, applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/mtk-sd.c |  168 ++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 152 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 80ba034..07f3236 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -75,6 +75,7 @@
>  #define MSDC_PATCH_BIT1  0xb4
>  #define MSDC_PAD_TUNE    0xec
>  #define PAD_DS_TUNE      0x188
> +#define PAD_CMD_TUNE     0x18c
>  #define EMMC50_CFG0      0x208
>
>  /*--------------------------------------------------------------------------*/
> @@ -210,13 +211,18 @@
>  #define MSDC_PATCH_BIT_SPCPUSH    (0x1 << 29)  /* RW */
>  #define MSDC_PATCH_BIT_DECRCTMO   (0x1 << 30)  /* RW */
>
> +#define MSDC_PAD_TUNE_DATWRDLY   (0x1f <<  0)  /* RW */
>  #define MSDC_PAD_TUNE_DATRRDLY   (0x1f <<  8)  /* RW */
>  #define MSDC_PAD_TUNE_CMDRDLY    (0x1f << 16)  /* RW */
> +#define MSDC_PAD_TUNE_CMDRRDLY   (0x1f << 22)  /* RW */
> +#define MSDC_PAD_TUNE_CLKTDLY    (0x1f << 27)  /* RW */
>
>  #define PAD_DS_TUNE_DLY1         (0x1f << 2)   /* RW */
>  #define PAD_DS_TUNE_DLY2         (0x1f << 7)   /* RW */
>  #define PAD_DS_TUNE_DLY3         (0x1f << 12)  /* RW */
>
> +#define PAD_CMD_TUNE_RX_DLY3     (0x1f << 1)  /* RW */
> +
>  #define EMMC50_CFG_PADCMD_LATCHCK (0x1 << 0)   /* RW */
>  #define EMMC50_CFG_CRCSTS_EDGE    (0x1 << 3)   /* RW */
>  #define EMMC50_CFG_CFCSTS_SEL     (0x1 << 4)   /* RW */
> @@ -284,12 +290,14 @@ struct msdc_save_para {
>         u32 patch_bit0;
>         u32 patch_bit1;
>         u32 pad_ds_tune;
> +       u32 pad_cmd_tune;
>         u32 emmc50_cfg0;
>  };
>
>  struct msdc_tune_para {
>         u32 iocon;
>         u32 pad_tune;
> +       u32 pad_cmd_tune;
>  };
>
>  struct msdc_delay_phase {
> @@ -331,6 +339,10 @@ struct msdc_host {
>         unsigned char timing;
>         bool vqmmc_enabled;
>         u32 hs400_ds_delay;
> +       u32 hs200_cmd_int_delay; /* cmd internal delay for HS200/SDR104 */
> +       u32 hs400_cmd_int_delay; /* cmd internal delay for HS400 */
> +       bool hs400_cmd_resp_sel_rising;
> +                                /* cmd response sample selection for HS400 */
>         bool hs400_mode;        /* current eMMC will run at hs400 mode */
>         struct msdc_save_para save_para; /* used when gate HCLK */
>         struct msdc_tune_para def_tune_para; /* default tune setting */
> @@ -600,8 +612,14 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
>         } else {
>                 writel(host->saved_tune_para.iocon, host->base + MSDC_IOCON);
>                 writel(host->saved_tune_para.pad_tune, host->base + MSDC_PAD_TUNE);
> +               writel(host->saved_tune_para.pad_cmd_tune,
> +                      host->base + PAD_CMD_TUNE);
>         }
>
> +       if (timing == MMC_TIMING_MMC_HS400)
> +               sdr_set_field(host->base + PAD_CMD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRRDLY,
> +                             host->hs400_cmd_int_delay);
>         dev_dbg(host->dev, "sclk: %d, timing: %d\n", host->sclk, timing);
>  }
>
> @@ -1302,7 +1320,7 @@ static struct msdc_delay_phase get_best_delay(struct msdc_host *host, u32 delay)
>                         len_final = len;
>                 }
>                 start += len ? len : 1;
> -               if (len >= 8 && start_final < 4)
> +               if (len >= 12 && start_final < 4)
>                         break;
>         }
>
> @@ -1325,36 +1343,67 @@ static int msdc_tune_response(struct mmc_host *mmc, u32 opcode)
>         struct msdc_host *host = mmc_priv(mmc);
>         u32 rise_delay = 0, fall_delay = 0;
>         struct msdc_delay_phase final_rise_delay, final_fall_delay = { 0,};
> +       struct msdc_delay_phase internal_delay_phase;
>         u8 final_delay, final_maxlen;
> +       u32 internal_delay = 0;
>         int cmd_err;
> -       int i;
> +       int i, j;
> +
> +       if (mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
> +           mmc->ios.timing == MMC_TIMING_UHS_SDR104)
> +               sdr_set_field(host->base + MSDC_PAD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRRDLY,
> +                             host->hs200_cmd_int_delay);
>
>         sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
>         for (i = 0 ; i < PAD_DELAY_MAX; i++) {
>                 sdr_set_field(host->base + MSDC_PAD_TUNE,
>                               MSDC_PAD_TUNE_CMDRDLY, i);
> -               mmc_send_tuning(mmc, opcode, &cmd_err);
> -               if (!cmd_err)
> -                       rise_delay |= (1 << i);
> +               /*
> +                * Using the same parameters, it may sometimes pass the test,
> +                * but sometimes it may fail. To make sure the parameters are
> +                * more stable, we test each set of parameters 3 times.
> +                */
> +               for (j = 0; j < 3; j++) {
> +                       mmc_send_tuning(mmc, opcode, &cmd_err);
> +                       if (!cmd_err) {
> +                               rise_delay |= (1 << i);
> +                       } else {
> +                               rise_delay &= ~(1 << i);
> +                               break;
> +                       }
> +               }
>         }
>         final_rise_delay = get_best_delay(host, rise_delay);
>         /* if rising edge has enough margin, then do not scan falling edge */
> -       if (final_rise_delay.maxlen >= 10 ||
> -           (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
> +       if (final_rise_delay.maxlen >= 12 && final_rise_delay.start < 4)
>                 goto skip_fall;
>
>         sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
>         for (i = 0; i < PAD_DELAY_MAX; i++) {
>                 sdr_set_field(host->base + MSDC_PAD_TUNE,
>                               MSDC_PAD_TUNE_CMDRDLY, i);
> -               mmc_send_tuning(mmc, opcode, &cmd_err);
> -               if (!cmd_err)
> -                       fall_delay |= (1 << i);
> +               /*
> +                * Using the same parameters, it may sometimes pass the test,
> +                * but sometimes it may fail. To make sure the parameters are
> +                * more stable, we test each set of parameters 3 times.
> +                */
> +               for (j = 0; j < 3; j++) {
> +                       mmc_send_tuning(mmc, opcode, &cmd_err);
> +                       if (!cmd_err) {
> +                               fall_delay |= (1 << i);
> +                       } else {
> +                               fall_delay &= ~(1 << i);
> +                               break;
> +                       }
> +               }
>         }
>         final_fall_delay = get_best_delay(host, fall_delay);
>
>  skip_fall:
>         final_maxlen = max(final_rise_delay.maxlen, final_fall_delay.maxlen);
> +       if (final_fall_delay.maxlen >= 12 && final_fall_delay.start < 4)
> +               final_maxlen = final_fall_delay.maxlen;
>         if (final_maxlen == final_rise_delay.maxlen) {
>                 sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
>                 sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRDLY,
> @@ -1366,7 +1415,71 @@ static int msdc_tune_response(struct mmc_host *mmc, u32 opcode)
>                               final_fall_delay.final_phase);
>                 final_delay = final_fall_delay.final_phase;
>         }
> +       if (host->hs200_cmd_int_delay)
> +               goto skip_internal;
> +
> +       for (i = 0; i < PAD_DELAY_MAX; i++) {
> +               sdr_set_field(host->base + MSDC_PAD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRRDLY, i);
> +               mmc_send_tuning(mmc, opcode, &cmd_err);
> +               if (!cmd_err)
> +                       internal_delay |= (1 << i);
> +       }
> +       dev_dbg(host->dev, "Final internal delay: 0x%x\n", internal_delay);
> +       internal_delay_phase = get_best_delay(host, internal_delay);
> +       sdr_set_field(host->base + MSDC_PAD_TUNE, MSDC_PAD_TUNE_CMDRRDLY,
> +                     internal_delay_phase.final_phase);
> +skip_internal:
> +       dev_dbg(host->dev, "Final cmd pad delay: %x\n", final_delay);
> +       return final_delay == 0xff ? -EIO : 0;
> +}
> +
> +static int hs400_tune_response(struct mmc_host *mmc, u32 opcode)
> +{
> +       struct msdc_host *host = mmc_priv(mmc);
> +       u32 cmd_delay = 0;
> +       struct msdc_delay_phase final_cmd_delay = { 0,};
> +       u8 final_delay;
> +       int cmd_err;
> +       int i, j;
> +
> +       /* select EMMC50 PAD CMD tune */
> +       sdr_set_bits(host->base + PAD_CMD_TUNE, BIT(0));
> +
> +       if (mmc->ios.timing == MMC_TIMING_MMC_HS200 ||
> +           mmc->ios.timing == MMC_TIMING_UHS_SDR104)
> +               sdr_set_field(host->base + MSDC_PAD_TUNE,
> +                             MSDC_PAD_TUNE_CMDRRDLY,
> +                             host->hs200_cmd_int_delay);
> +
> +       if (host->hs400_cmd_resp_sel_rising)
> +               sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> +       else
> +               sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
> +       for (i = 0 ; i < PAD_DELAY_MAX; i++) {
> +               sdr_set_field(host->base + PAD_CMD_TUNE,
> +                             PAD_CMD_TUNE_RX_DLY3, i);
> +               /*
> +                * Using the same parameters, it may sometimes pass the test,
> +                * but sometimes it may fail. To make sure the parameters are
> +                * more stable, we test each set of parameters 3 times.
> +                */
> +               for (j = 0; j < 3; j++) {
> +                       mmc_send_tuning(mmc, opcode, &cmd_err);
> +                       if (!cmd_err) {
> +                               cmd_delay |= (1 << i);
> +                       } else {
> +                               cmd_delay &= ~(1 << i);
> +                               break;
> +                       }
> +               }
> +       }
> +       final_cmd_delay = get_best_delay(host, cmd_delay);
> +       sdr_set_field(host->base + PAD_CMD_TUNE, PAD_CMD_TUNE_RX_DLY3,
> +                     final_cmd_delay.final_phase);
> +       final_delay = final_cmd_delay.final_phase;
>
> +       dev_dbg(host->dev, "Final cmd pad delay: %x\n", final_delay);
>         return final_delay == 0xff ? -EIO : 0;
>  }
>
> @@ -1389,7 +1502,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
>         }
>         final_rise_delay = get_best_delay(host, rise_delay);
>         /* if rising edge has enough margin, then do not scan falling edge */
> -       if (final_rise_delay.maxlen >= 10 ||
> +       if (final_rise_delay.maxlen >= 12 ||
>             (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
>                 goto skip_fall;
>
> @@ -1422,6 +1535,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
>                 final_delay = final_fall_delay.final_phase;
>         }
>
> +       dev_dbg(host->dev, "Final data pad delay: %x\n", final_delay);
>         return final_delay == 0xff ? -EIO : 0;
>  }
>
> @@ -1430,7 +1544,10 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
>         struct msdc_host *host = mmc_priv(mmc);
>         int ret;
>
> -       ret = msdc_tune_response(mmc, opcode);
> +       if (host->hs400_mode)
> +               ret = hs400_tune_response(mmc, opcode);
> +       else
> +               ret = msdc_tune_response(mmc, opcode);
>         if (ret == -EIO) {
>                 dev_err(host->dev, "Tune response fail!\n");
>                 return ret;
> @@ -1443,6 +1560,7 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
>
>         host->saved_tune_para.iocon = readl(host->base + MSDC_IOCON);
>         host->saved_tune_para.pad_tune = readl(host->base + MSDC_PAD_TUNE);
> +       host->saved_tune_para.pad_cmd_tune = readl(host->base + PAD_CMD_TUNE);
>         return ret;
>  }
>
> @@ -1477,6 +1595,25 @@ static void msdc_hw_reset(struct mmc_host *mmc)
>         .hw_reset = msdc_hw_reset,
>  };
>
> +static void msdc_of_property_parse(struct platform_device *pdev,
> +                                  struct msdc_host *host)
> +{
> +       of_property_read_u32(pdev->dev.of_node, "hs400-ds-delay",
> +                            &host->hs400_ds_delay);
> +
> +       of_property_read_u32(pdev->dev.of_node, "mediatek,hs200-cmd-int-delay",
> +                            &host->hs200_cmd_int_delay);
> +
> +       of_property_read_u32(pdev->dev.of_node, "mediatek,hs400-cmd-int-delay",
> +                            &host->hs400_cmd_int_delay);
> +
> +       if (of_property_read_bool(pdev->dev.of_node,
> +                                 "mediatek,hs400-cmd-resp-sel-rising"))
> +               host->hs400_cmd_resp_sel_rising = true;
> +       else
> +               host->hs400_cmd_resp_sel_rising = false;
> +}
> +
>  static int msdc_drv_probe(struct platform_device *pdev)
>  {
>         struct mmc_host *mmc;
> @@ -1548,10 +1685,7 @@ static int msdc_drv_probe(struct platform_device *pdev)
>                 goto host_free;
>         }
>
> -       if (!of_property_read_u32(pdev->dev.of_node, "hs400-ds-delay",
> -                                 &host->hs400_ds_delay))
> -               dev_dbg(&pdev->dev, "hs400-ds-delay: %x\n",
> -                       host->hs400_ds_delay);
> +       msdc_of_property_parse(pdev, host);
>
>         host->dev = &pdev->dev;
>         host->mmc = mmc;
> @@ -1663,6 +1797,7 @@ static void msdc_save_reg(struct msdc_host *host)
>         host->save_para.patch_bit0 = readl(host->base + MSDC_PATCH_BIT);
>         host->save_para.patch_bit1 = readl(host->base + MSDC_PATCH_BIT1);
>         host->save_para.pad_ds_tune = readl(host->base + PAD_DS_TUNE);
> +       host->save_para.pad_cmd_tune = readl(host->base + PAD_CMD_TUNE);
>         host->save_para.emmc50_cfg0 = readl(host->base + EMMC50_CFG0);
>  }
>
> @@ -1675,6 +1810,7 @@ static void msdc_restore_reg(struct msdc_host *host)
>         writel(host->save_para.patch_bit0, host->base + MSDC_PATCH_BIT);
>         writel(host->save_para.patch_bit1, host->base + MSDC_PATCH_BIT1);
>         writel(host->save_para.pad_ds_tune, host->base + PAD_DS_TUNE);
> +       writel(host->save_para.pad_cmd_tune, host->base + PAD_CMD_TUNE);
>         writel(host->save_para.emmc50_cfg0, host->base + EMMC50_CFG0);
>  }
>
> --
> 1.7.9.5
>