[PATCH v3 0/2] mmc: mediatek: Add HS400 online tuning support

[PATCH v3 0/2] mmc: mediatek: Add HS400 online tuning support

[Wenbin Mei]

Change in v3:
- add detail descripthion for hs400 dly3

Change in v2:
- remove the check "mmc_can_ext_csd"
- change the hs400 tuning condition for "msdc_cmd_done" function and
"msdc_cmd_next" function
- use "-EIO" instead of "-ERANGE"

Wenbin Mei (2):
  dt-bindings: mmc: mtk-sd: add hs400 dly3 setting
  mmc: mediatek: Add HS400 online tuning support

 .../devicetree/bindings/mmc/mtk-sd.yaml       |  12 ++
 drivers/mmc/core/mmc.c                        |   8 ++
 drivers/mmc/host/mtk-sd.c                     | 118 +++++++++++++++++-
 include/linux/mmc/host.h                      |   3 +
 4 files changed, 139 insertions(+), 2 deletions(-)

--
2.25.1

[PATCH v3 1/2] dt-bindings: mmc: mtk-sd: add hs400 dly3 setting

[Wenbin Mei]

Add hs400 dly3 setting for mtk-sd yaml

Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 Documentation/devicetree/bindings/mmc/mtk-sd.yaml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
index e866e985549e..82768a807294 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
@@ -119,6 +119,18 @@ properties:
       If present, HS400 command responses are sampled on rising edges.
       If not present, HS400 command responses are sampled on falling edges.
 
+  mediatek,hs400-ds-dly3:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Gear of the third delay line for DS for input data latch in data
+      pad macro, there are 32 stages from 0 to 31.
+      For different corner IC, the time is different about one step, it is
+      about 100ps.
+      The value is confirmed by doing scan and calibration to find a best
+      value with corner IC and it is valid only for HS400 mode.
+    minimum: 0
+    maximum: 31
+
   mediatek,latch-ck:
     $ref: /schemas/types.yaml#/definitions/uint32
     description:
-- 
2.25.1

[PATCH v3 2/2] mmc: mediatek: Add HS400 online tuning support

[Wenbin Mei]

Due to the influence of the corner IC and vcore voltage, for the stability
of HS400 mode, we Add HS400 mode online tuning support for mediatek mmc
host.

Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
Reviewed-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 drivers/mmc/core/mmc.c    |   8 +++
 drivers/mmc/host/mtk-sd.c | 118 +++++++++++++++++++++++++++++++++++++-
 include/linux/mmc/host.h  |   3 +
 3 files changed, 127 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 838726b68ff3..0aa72acd8612 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1222,6 +1222,14 @@ static int mmc_select_hs400(struct mmc_card *card)
 	mmc_set_timing(host, MMC_TIMING_MMC_HS400);
 	mmc_set_bus_speed(card);
 
+	if (host->ops->execute_hs400_tuning) {
+		mmc_retune_disable(host);
+		err = host->ops->execute_hs400_tuning(host, card);
+		mmc_retune_enable(host);
+		if (err)
+			goto out_err;
+	}
+
 	if (host->ops->hs400_complete)
 		host->ops->hs400_complete(host);
 
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 4dfc246c5f95..484f5c38bfaf 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -258,6 +258,7 @@
 #define MSDC_PAD_TUNE_RD_SEL	  (0x1 << 13)   /* RW */
 #define MSDC_PAD_TUNE_CMD_SEL	  (0x1 << 21)   /* RW */
 
+#define PAD_DS_TUNE_DLY_SEL       (0x1 << 0)	/* 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 */
@@ -301,6 +302,11 @@
 #define PAD_CMD_RD_RXDLY_SEL    (0x1 << 11)     /* RW */
 #define PAD_CMD_TX_DLY          (0x1f << 12)    /* RW */
 
+/* EMMC50_PAD_DS_TUNE mask */
+#define PAD_DS_DLY_SEL		(0x1 << 16)	/* RW */
+#define PAD_DS_DLY1		(0x1f << 10)	/* RW */
+#define PAD_DS_DLY3		(0x1f << 0)	/* RW */
+
 #define REQ_CMD_EIO  (0x1 << 0)
 #define REQ_CMD_TMO  (0x1 << 1)
 #define REQ_DAT_ERR  (0x1 << 2)
@@ -448,11 +454,13 @@ struct msdc_host {
 	bool vqmmc_enabled;
 	u32 latch_ck;
 	u32 hs400_ds_delay;
+	u32 hs400_ds_dly3;
 	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 */
+	bool hs400_tuning;	/* hs400 mode online tuning */
 	bool internal_cd;	/* Use internal card-detect logic */
 	bool cqhci;		/* support eMMC hw cmdq */
 	struct msdc_save_para save_para; /* used when gate HCLK */
@@ -1190,7 +1198,8 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
 	if (!sbc_error && !(events & MSDC_INT_CMDRDY)) {
 		if (events & MSDC_INT_CMDTMO ||
 		    (cmd->opcode != MMC_SEND_TUNING_BLOCK &&
-		     cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200))
+		     cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200 &&
+		     !host->hs400_tuning))
 			/*
 			 * should not clear fifo/interrupt as the tune data
 			 * may have alreay come when cmd19/cmd21 gets response
@@ -1287,7 +1296,8 @@ static void msdc_cmd_next(struct msdc_host *host,
 	if ((cmd->error &&
 	    !(cmd->error == -EILSEQ &&
 	      (cmd->opcode == MMC_SEND_TUNING_BLOCK ||
-	       cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200))) ||
+	       cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200 ||
+	       host->hs400_tuning))) ||
 	    (mrq->sbc && mrq->sbc->error))
 		msdc_request_done(host, mrq);
 	else if (cmd == mrq->sbc)
@@ -2251,6 +2261,106 @@ static int msdc_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios)
 	return 0;
 }
 
+static int msdc_send_cxd_data(struct mmc_card *card, struct mmc_host *host)
+{
+	struct mmc_request mrq = {};
+	struct mmc_command cmd = {};
+	struct mmc_data data = {};
+	unsigned int len = 512;
+	struct scatterlist sg;
+	u8 *ext_csd;
+
+	ext_csd = kzalloc(len, GFP_KERNEL);
+	if (!ext_csd)
+		return -ENOMEM;
+
+	mrq.cmd = &cmd;
+	mrq.data = &data;
+
+	cmd.opcode = MMC_SEND_EXT_CSD;
+	cmd.arg = 0;
+	cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
+
+	data.blksz = len;
+	data.blocks = 1;
+	data.flags = MMC_DATA_READ;
+	data.sg = &sg;
+	data.sg_len = 1;
+
+	sg_init_one(&sg, ext_csd, len);
+	mmc_set_data_timeout(&data, card);
+	mmc_wait_for_req(host, &mrq);
+
+	kfree(ext_csd);
+
+	if (cmd.error)
+		return cmd.error;
+	if (data.error)
+		return data.error;
+
+	return 0;
+}
+
+static int msdc_execute_hs400_tuning(struct mmc_host *mmc, struct mmc_card *card)
+{
+	struct msdc_host *host = mmc_priv(mmc);
+	struct msdc_delay_phase dly1_delay;
+	u32 val, result_dly1 = 0;
+	int i, ret;
+
+	if (host->top_base) {
+		sdr_set_bits(host->top_base + EMMC50_PAD_DS_TUNE,
+			     PAD_DS_DLY_SEL);
+		if (host->hs400_ds_dly3)
+			sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE,
+				      PAD_DS_DLY3, host->hs400_ds_dly3);
+	} else {
+		sdr_set_bits(host->base + PAD_DS_TUNE, PAD_DS_TUNE_DLY_SEL);
+		if (host->hs400_ds_dly3)
+			sdr_set_field(host->base + PAD_DS_TUNE,
+				      PAD_DS_TUNE_DLY3, host->hs400_ds_dly3);
+	}
+
+	host->hs400_tuning = true;
+	for (i = 0; i < PAD_DELAY_MAX; i++) {
+		if (host->top_base)
+			sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE,
+				      PAD_DS_DLY1, i);
+		else
+			sdr_set_field(host->base + PAD_DS_TUNE,
+				      PAD_DS_TUNE_DLY1, i);
+		ret = msdc_send_cxd_data(card, mmc);
+		if (!ret)
+			result_dly1 |= (1 << i);
+	}
+	host->hs400_tuning = false;
+
+	dly1_delay = get_best_delay(host, result_dly1);
+	if (dly1_delay.maxlen == 0) {
+		dev_err(host->dev, "Failed to get DLY1 delay!\n");
+		goto fail;
+	}
+	if (host->top_base)
+		sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE,
+			      PAD_DS_DLY1, dly1_delay.final_phase);
+	else
+		sdr_set_field(host->base + PAD_DS_TUNE,
+			      PAD_DS_TUNE_DLY1, dly1_delay.final_phase);
+
+	if (host->top_base)
+		val = readl(host->top_base + EMMC50_PAD_DS_TUNE);
+	else
+		val = readl(host->base + PAD_DS_TUNE);
+
+	dev_info(host->dev, "Fianl PAD_DS_TUNE: 0x%x\n", val);
+
+	return 0;
+
+fail:
+	dev_err(host->dev, "Failed to tuning DS pin delay!\n");
+	return -EIO;
+}
+
 static void msdc_hw_reset(struct mmc_host *mmc)
 {
 	struct msdc_host *host = mmc_priv(mmc);
@@ -2377,6 +2487,7 @@ static const struct mmc_host_ops mt_msdc_ops = {
 	.card_busy = msdc_card_busy,
 	.execute_tuning = msdc_execute_tuning,
 	.prepare_hs400_tuning = msdc_prepare_hs400_tuning,
+	.execute_hs400_tuning = msdc_execute_hs400_tuning,
 	.hw_reset = msdc_hw_reset,
 };
 
@@ -2396,6 +2507,9 @@ static void msdc_of_property_parse(struct platform_device *pdev,
 	of_property_read_u32(pdev->dev.of_node, "hs400-ds-delay",
 			     &host->hs400_ds_delay);
 
+	of_property_read_u32(pdev->dev.of_node, "mediatek,hs400-ds-dly3",
+			     &host->hs400_ds_dly3);
+
 	of_property_read_u32(pdev->dev.of_node, "mediatek,hs200-cmd-int-delay",
 			     &host->hs200_cmd_int_delay);
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index ff1a251bb0bc..7552d5eb0074 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -162,6 +162,9 @@ struct mmc_host_ops {
 	/* Prepare HS400 target operating frequency depending host driver */
 	int	(*prepare_hs400_tuning)(struct mmc_host *host, struct mmc_ios *ios);
 
+	/* Execute HS400 tuning depending host driver */
+	int	(*execute_hs400_tuning)(struct mmc_host *host, struct mmc_card *card);
+
 	/* Prepare switch to DDR during the HS400 init sequence */
 	int	(*hs400_prepare_ddr)(struct mmc_host *host);
 
-- 
2.25.1

Re: [PATCH v3 2/2] mmc: mediatek: Add HS400 online tuning support

[Ulf Hansson]

On Wed, 8 Sept 2021 at 03:32, Wenbin Mei <wenbin.mei@mediatek.com> wrote:
>
> Due to the influence of the corner IC and vcore voltage, for the stability
> of HS400 mode, we Add HS400 mode online tuning support for mediatek mmc
> host.

My apologies, but I am not familiar with what 'HS400 online tuning'
is? Can you please elaborate on this?

Is it specific for a Mediatek eMMC controller - or is a common eMMC
feature that is described in the eMMC spec?

>
> Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
> Reviewed-by: Chaotian Jing <chaotian.jing@mediatek.com>
> ---
>  drivers/mmc/core/mmc.c    |   8 +++
>  drivers/mmc/host/mtk-sd.c | 118 +++++++++++++++++++++++++++++++++++++-
>  include/linux/mmc/host.h  |   3 +

Please split this patch into a core patch and a mtk-sd patch.

>  3 files changed, 127 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 838726b68ff3..0aa72acd8612 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1222,6 +1222,14 @@ static int mmc_select_hs400(struct mmc_card *card)
>         mmc_set_timing(host, MMC_TIMING_MMC_HS400);
>         mmc_set_bus_speed(card);
>
> +       if (host->ops->execute_hs400_tuning) {
> +               mmc_retune_disable(host);
> +               err = host->ops->execute_hs400_tuning(host, card);
> +               mmc_retune_enable(host);
> +               if (err)
> +                       goto out_err;
> +       }
> +
>         if (host->ops->hs400_complete)
>                 host->ops->hs400_complete(host);
>
> diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> index 4dfc246c5f95..484f5c38bfaf 100644
> --- a/drivers/mmc/host/mtk-sd.c
> +++ b/drivers/mmc/host/mtk-sd.c
> @@ -258,6 +258,7 @@
>  #define MSDC_PAD_TUNE_RD_SEL     (0x1 << 13)   /* RW */
>  #define MSDC_PAD_TUNE_CMD_SEL    (0x1 << 21)   /* RW */
>
> +#define PAD_DS_TUNE_DLY_SEL       (0x1 << 0)   /* 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 */
> @@ -301,6 +302,11 @@
>  #define PAD_CMD_RD_RXDLY_SEL    (0x1 << 11)     /* RW */
>  #define PAD_CMD_TX_DLY          (0x1f << 12)    /* RW */
>
> +/* EMMC50_PAD_DS_TUNE mask */
> +#define PAD_DS_DLY_SEL         (0x1 << 16)     /* RW */
> +#define PAD_DS_DLY1            (0x1f << 10)    /* RW */
> +#define PAD_DS_DLY3            (0x1f << 0)     /* RW */
> +
>  #define REQ_CMD_EIO  (0x1 << 0)
>  #define REQ_CMD_TMO  (0x1 << 1)
>  #define REQ_DAT_ERR  (0x1 << 2)
> @@ -448,11 +454,13 @@ struct msdc_host {
>         bool vqmmc_enabled;
>         u32 latch_ck;
>         u32 hs400_ds_delay;
> +       u32 hs400_ds_dly3;
>         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 */
> +       bool hs400_tuning;      /* hs400 mode online tuning */
>         bool internal_cd;       /* Use internal card-detect logic */
>         bool cqhci;             /* support eMMC hw cmdq */
>         struct msdc_save_para save_para; /* used when gate HCLK */
> @@ -1190,7 +1198,8 @@ static bool msdc_cmd_done(struct msdc_host *host, int events,
>         if (!sbc_error && !(events & MSDC_INT_CMDRDY)) {
>                 if (events & MSDC_INT_CMDTMO ||
>                     (cmd->opcode != MMC_SEND_TUNING_BLOCK &&
> -                    cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200))
> +                    cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200 &&
> +                    !host->hs400_tuning))
>                         /*
>                          * should not clear fifo/interrupt as the tune data
>                          * may have alreay come when cmd19/cmd21 gets response
> @@ -1287,7 +1296,8 @@ static void msdc_cmd_next(struct msdc_host *host,
>         if ((cmd->error &&
>             !(cmd->error == -EILSEQ &&
>               (cmd->opcode == MMC_SEND_TUNING_BLOCK ||
> -              cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200))) ||
> +              cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200 ||
> +              host->hs400_tuning))) ||
>             (mrq->sbc && mrq->sbc->error))
>                 msdc_request_done(host, mrq);
>         else if (cmd == mrq->sbc)
> @@ -2251,6 +2261,106 @@ static int msdc_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios)
>         return 0;
>  }
>
> +static int msdc_send_cxd_data(struct mmc_card *card, struct mmc_host *host)
> +{
> +       struct mmc_request mrq = {};
> +       struct mmc_command cmd = {};
> +       struct mmc_data data = {};
> +       unsigned int len = 512;
> +       struct scatterlist sg;
> +       u8 *ext_csd;
> +
> +       ext_csd = kzalloc(len, GFP_KERNEL);
> +       if (!ext_csd)
> +               return -ENOMEM;
> +
> +       mrq.cmd = &cmd;
> +       mrq.data = &data;
> +
> +       cmd.opcode = MMC_SEND_EXT_CSD;
> +       cmd.arg = 0;
> +       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> +
> +       data.blksz = len;
> +       data.blocks = 1;
> +       data.flags = MMC_DATA_READ;
> +       data.sg = &sg;
> +       data.sg_len = 1;
> +
> +       sg_init_one(&sg, ext_csd, len);
> +       mmc_set_data_timeout(&data, card);
> +       mmc_wait_for_req(host, &mrq);
> +
> +       kfree(ext_csd);
> +
> +       if (cmd.error)
> +               return cmd.error;
> +       if (data.error)
> +               return data.error;
> +
> +       return 0;

Why do we need to send a MMC_SEND_EXT_CSD command, exactly?

Why can't mmc_send_tuning() work here too? What does the eMMC spec
state about this?

> +}
> +
> +static int msdc_execute_hs400_tuning(struct mmc_host *mmc, struct mmc_card *card)
> +{
> +       struct msdc_host *host = mmc_priv(mmc);
> +       struct msdc_delay_phase dly1_delay;
> +       u32 val, result_dly1 = 0;
> +       int i, ret;
> +
> +       if (host->top_base) {
> +               sdr_set_bits(host->top_base + EMMC50_PAD_DS_TUNE,
> +                            PAD_DS_DLY_SEL);
> +               if (host->hs400_ds_dly3)
> +                       sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE,
> +                                     PAD_DS_DLY3, host->hs400_ds_dly3);
> +       } else {
> +               sdr_set_bits(host->base + PAD_DS_TUNE, PAD_DS_TUNE_DLY_SEL);
> +               if (host->hs400_ds_dly3)
> +                       sdr_set_field(host->base + PAD_DS_TUNE,
> +                                     PAD_DS_TUNE_DLY3, host->hs400_ds_dly3);
> +       }
> +
> +       host->hs400_tuning = true;
> +       for (i = 0; i < PAD_DELAY_MAX; i++) {
> +               if (host->top_base)
> +                       sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE,
> +                                     PAD_DS_DLY1, i);
> +               else
> +                       sdr_set_field(host->base + PAD_DS_TUNE,
> +                                     PAD_DS_TUNE_DLY1, i);
> +               ret = msdc_send_cxd_data(card, mmc);
> +               if (!ret)
> +                       result_dly1 |= (1 << i);
> +       }
> +       host->hs400_tuning = false;
> +
> +       dly1_delay = get_best_delay(host, result_dly1);
> +       if (dly1_delay.maxlen == 0) {
> +               dev_err(host->dev, "Failed to get DLY1 delay!\n");
> +               goto fail;
> +       }
> +       if (host->top_base)
> +               sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE,
> +                             PAD_DS_DLY1, dly1_delay.final_phase);
> +       else
> +               sdr_set_field(host->base + PAD_DS_TUNE,
> +                             PAD_DS_TUNE_DLY1, dly1_delay.final_phase);
> +
> +       if (host->top_base)
> +               val = readl(host->top_base + EMMC50_PAD_DS_TUNE);
> +       else
> +               val = readl(host->base + PAD_DS_TUNE);
> +
> +       dev_info(host->dev, "Fianl PAD_DS_TUNE: 0x%x\n", val);
> +
> +       return 0;
> +
> +fail:
> +       dev_err(host->dev, "Failed to tuning DS pin delay!\n");
> +       return -EIO;
> +}

[...]

Kind regards
Uffe

Re: [PATCH v3 2/2] mmc: mediatek: Add HS400 online tuning support

[Wenbin Mei]

On Tue, 2021-09-14 at 10:46 +0200, Ulf Hansson wrote:
> On Wed, 8 Sept 2021 at 03:32, Wenbin Mei <wenbin.mei@mediatek.com>
> wrote:
> > 
> > Due to the influence of the corner IC and vcore voltage, for the
> > stability
> > of HS400 mode, we Add HS400 mode online tuning support for mediatek
> > mmc
> > host.
> 
> My apologies, but I am not familiar with what 'HS400 online tuning'
> is? Can you please elaborate on this?
> 
> Is it specific for a Mediatek eMMC controller - or is a common eMMC
> feature that is described in the eMMC spec?
> 
According to JEDEC Spec, there is no need to do tuning under HS400 mode
since the Rx signal is aligned with the DS signal. However, MediaTek's
IC need set its "DS delay" internally to ensure it can latch Rx signal
correctly.
In previous version, We provide an "hs400-ds-delay" in device tree to
cover different chipset/PCB design, and it works fine in most cases.
But, with the development of process technology and the big VCore
voltage scale range(may have 0.7V/0.6V/0.55V), it is difficult to find
a suitable "hs400-ds-delay" to cover all of IC corner
cases(SSSS/TTTT/FFFF).
So that We must have the ability to do hs400 online tuning.
It is specific for the Mediatek eMMC controller which support HS400
mode.
> > 
> > Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
> > Reviewed-by: Chaotian Jing <chaotian.jing@mediatek.com>
> > ---
> >  drivers/mmc/core/mmc.c    |   8 +++
> >  drivers/mmc/host/mtk-sd.c | 118
> > +++++++++++++++++++++++++++++++++++++-
> >  include/linux/mmc/host.h  |   3 +
> 
> Please split this patch into a core patch and a mtk-sd patch.
> 
I will change it in the next version.
> >  3 files changed, 127 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> > index 838726b68ff3..0aa72acd8612 100644
> > --- a/drivers/mmc/core/mmc.c
> > +++ b/drivers/mmc/core/mmc.c
> > @@ -1222,6 +1222,14 @@ static int mmc_select_hs400(struct mmc_card
> > *card)
> >         mmc_set_timing(host, MMC_TIMING_MMC_HS400);
> >         mmc_set_bus_speed(card);
> > 
> > +       if (host->ops->execute_hs400_tuning) {
> > +               mmc_retune_disable(host);
> > +               err = host->ops->execute_hs400_tuning(host, card);
> > +               mmc_retune_enable(host);
> > +               if (err)
> > +                       goto out_err;
> > +       }
> > +
> >         if (host->ops->hs400_complete)
> >                 host->ops->hs400_complete(host);
> > 
> > diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
> > index 4dfc246c5f95..484f5c38bfaf 100644
> > --- a/drivers/mmc/host/mtk-sd.c
> > +++ b/drivers/mmc/host/mtk-sd.c
> > @@ -258,6 +258,7 @@
> >  #define MSDC_PAD_TUNE_RD_SEL     (0x1 << 13)   /* RW */
> >  #define MSDC_PAD_TUNE_CMD_SEL    (0x1 << 21)   /* RW */
> > 
> > +#define PAD_DS_TUNE_DLY_SEL       (0x1 << 0)   /* 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 */
> > @@ -301,6 +302,11 @@
> >  #define PAD_CMD_RD_RXDLY_SEL    (0x1 << 11)     /* RW */
> >  #define PAD_CMD_TX_DLY          (0x1f << 12)    /* RW */
> > 
> > +/* EMMC50_PAD_DS_TUNE mask */
> > +#define PAD_DS_DLY_SEL         (0x1 << 16)     /* RW */
> > +#define PAD_DS_DLY1            (0x1f << 10)    /* RW */
> > +#define PAD_DS_DLY3            (0x1f << 0)     /* RW */
> > +
> >  #define REQ_CMD_EIO  (0x1 << 0)
> >  #define REQ_CMD_TMO  (0x1 << 1)
> >  #define REQ_DAT_ERR  (0x1 << 2)
> > @@ -448,11 +454,13 @@ struct msdc_host {
> >         bool vqmmc_enabled;
> >         u32 latch_ck;
> >         u32 hs400_ds_delay;
> > +       u32 hs400_ds_dly3;
> >         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 */
> > +       bool hs400_tuning;      /* hs400 mode online tuning */
> >         bool internal_cd;       /* Use internal card-detect logic
> > */
> >         bool cqhci;             /* support eMMC hw cmdq */
> >         struct msdc_save_para save_para; /* used when gate HCLK */
> > @@ -1190,7 +1198,8 @@ static bool msdc_cmd_done(struct msdc_host
> > *host, int events,
> >         if (!sbc_error && !(events & MSDC_INT_CMDRDY)) {
> >                 if (events & MSDC_INT_CMDTMO ||
> >                     (cmd->opcode != MMC_SEND_TUNING_BLOCK &&
> > -                    cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200))
> > +                    cmd->opcode != MMC_SEND_TUNING_BLOCK_HS200 &&
> > +                    !host->hs400_tuning))
> >                         /*
> >                          * should not clear fifo/interrupt as the
> > tune data
> >                          * may have alreay come when cmd19/cmd21
> > gets response
> > @@ -1287,7 +1296,8 @@ static void msdc_cmd_next(struct msdc_host
> > *host,
> >         if ((cmd->error &&
> >             !(cmd->error == -EILSEQ &&
> >               (cmd->opcode == MMC_SEND_TUNING_BLOCK ||
> > -              cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200))) ||
> > +              cmd->opcode == MMC_SEND_TUNING_BLOCK_HS200 ||
> > +              host->hs400_tuning))) ||
> >             (mrq->sbc && mrq->sbc->error))
> >                 msdc_request_done(host, mrq);
> >         else if (cmd == mrq->sbc)
> > @@ -2251,6 +2261,106 @@ static int msdc_prepare_hs400_tuning(struct
> > mmc_host *mmc, struct mmc_ios *ios)
> >         return 0;
> >  }
> > 
> > +static int msdc_send_cxd_data(struct mmc_card *card, struct
> > mmc_host *host)
> > +{
> > +       struct mmc_request mrq = {};
> > +       struct mmc_command cmd = {};
> > +       struct mmc_data data = {};
> > +       unsigned int len = 512;
> > +       struct scatterlist sg;
> > +       u8 *ext_csd;
> > +
> > +       ext_csd = kzalloc(len, GFP_KERNEL);
> > +       if (!ext_csd)
> > +               return -ENOMEM;
> > +
> > +       mrq.cmd = &cmd;
> > +       mrq.data = &data;
> > +
> > +       cmd.opcode = MMC_SEND_EXT_CSD;
> > +       cmd.arg = 0;
> > +       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> > +
> > +       data.blksz = len;
> > +       data.blocks = 1;
> > +       data.flags = MMC_DATA_READ;
> > +       data.sg = &sg;
> > +       data.sg_len = 1;
> > +
> > +       sg_init_one(&sg, ext_csd, len);
> > +       mmc_set_data_timeout(&data, card);
> > +       mmc_wait_for_req(host, &mrq);
> > +
> > +       kfree(ext_csd);
> > +
> > +       if (cmd.error)
> > +               return cmd.error;
> > +       if (data.error)
> > +               return data.error;
> > +
> > +       return 0;
> 
> Why do we need to send a MMC_SEND_EXT_CSD command, exactly?
> 
> Why can't mmc_send_tuning() work here too? What does the eMMC spec
> state about this?
> 
The CMD21 is illegal under hs400 mode so that cannot use the
mmc_send_tuning(). The CMD8 is suitable because it will receive 1 block
of non-zero data.
> > +}
> > +
> > +static int msdc_execute_hs400_tuning(struct mmc_host *mmc, struct
> > mmc_card *card)
> > +{
> > +       struct msdc_host *host = mmc_priv(mmc);
> > +       struct msdc_delay_phase dly1_delay;
> > +       u32 val, result_dly1 = 0;
> > +       int i, ret;
> > +
> > +       if (host->top_base) {
> > +               sdr_set_bits(host->top_base + EMMC50_PAD_DS_TUNE,
> > +                            PAD_DS_DLY_SEL);
> > +               if (host->hs400_ds_dly3)
> > +                       sdr_set_field(host->top_base +
> > EMMC50_PAD_DS_TUNE,
> > +                                     PAD_DS_DLY3, host-
> > >hs400_ds_dly3);
> > +       } else {
> > +               sdr_set_bits(host->base + PAD_DS_TUNE,
> > PAD_DS_TUNE_DLY_SEL);
> > +               if (host->hs400_ds_dly3)
> > +                       sdr_set_field(host->base + PAD_DS_TUNE,
> > +                                     PAD_DS_TUNE_DLY3, host-
> > >hs400_ds_dly3);
> > +       }
> > +
> > +       host->hs400_tuning = true;
> > +       for (i = 0; i < PAD_DELAY_MAX; i++) {
> > +               if (host->top_base)
> > +                       sdr_set_field(host->top_base +
> > EMMC50_PAD_DS_TUNE,
> > +                                     PAD_DS_DLY1, i);
> > +               else
> > +                       sdr_set_field(host->base + PAD_DS_TUNE,
> > +                                     PAD_DS_TUNE_DLY1, i);
> > +               ret = msdc_send_cxd_data(card, mmc);
> > +               if (!ret)
> > +                       result_dly1 |= (1 << i);
> > +       }
> > +       host->hs400_tuning = false;
> > +
> > +       dly1_delay = get_best_delay(host, result_dly1);
> > +       if (dly1_delay.maxlen == 0) {
> > +               dev_err(host->dev, "Failed to get DLY1 delay!\n");
> > +               goto fail;
> > +       }
> > +       if (host->top_base)
> > +               sdr_set_field(host->top_base + EMMC50_PAD_DS_TUNE,
> > +                             PAD_DS_DLY1, dly1_delay.final_phase);
> > +       else
> > +               sdr_set_field(host->base + PAD_DS_TUNE,
> > +                             PAD_DS_TUNE_DLY1,
> > dly1_delay.final_phase);
> > +
> > +       if (host->top_base)
> > +               val = readl(host->top_base + EMMC50_PAD_DS_TUNE);
> > +       else
> > +               val = readl(host->base + PAD_DS_TUNE);
> > +
> > +       dev_info(host->dev, "Fianl PAD_DS_TUNE: 0x%x\n", val);
> > +
> > +       return 0;
> > +
> > +fail:
> > +       dev_err(host->dev, "Failed to tuning DS pin delay!\n");
> > +       return -EIO;
> > +}
> 
> [...]
> 
> Kind regards
> Uffe

Re: [PATCH v3 1/2] dt-bindings: mmc: mtk-sd: add hs400 dly3 setting

[Linus Walleij]

On Wed, Sep 8, 2021 at 3:32 AM Wenbin Mei <wenbin.mei@mediatek.com> wrote:

> Add hs400 dly3 setting for mtk-sd yaml
>
> Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
> Acked-by: Rob Herring <robh@kernel.org>

Excellent doc!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v3 2/2] mmc: mediatek: Add HS400 online tuning support

[Ulf Hansson]

On Thu, 16 Sept 2021 at 11:47, Wenbin Mei <wenbin.mei@mediatek.com> wrote:
>
> On Tue, 2021-09-14 at 10:46 +0200, Ulf Hansson wrote:
> > On Wed, 8 Sept 2021 at 03:32, Wenbin Mei <wenbin.mei@mediatek.com>
> > wrote:
> > >
> > > Due to the influence of the corner IC and vcore voltage, for the
> > > stability
> > > of HS400 mode, we Add HS400 mode online tuning support for mediatek
> > > mmc
> > > host.
> >
> > My apologies, but I am not familiar with what 'HS400 online tuning'
> > is? Can you please elaborate on this?
> >
> > Is it specific for a Mediatek eMMC controller - or is a common eMMC
> > feature that is described in the eMMC spec?
> >
> According to JEDEC Spec, there is no need to do tuning under HS400 mode
> since the Rx signal is aligned with the DS signal. However, MediaTek's
> IC need set its "DS delay" internally to ensure it can latch Rx signal
> correctly.
> In previous version, We provide an "hs400-ds-delay" in device tree to
> cover different chipset/PCB design, and it works fine in most cases.
> But, with the development of process technology and the big VCore
> voltage scale range(may have 0.7V/0.6V/0.55V), it is difficult to find
> a suitable "hs400-ds-delay" to cover all of IC corner
> cases(SSSS/TTTT/FFFF).
> So that We must have the ability to do hs400 online tuning.
> It is specific for the Mediatek eMMC controller which support HS400
> mode.

I see, thanks for clarifying. Please put some of this information in
the commit message for the next version, it certainly helps to
understand.

[...]

> > > +static int msdc_send_cxd_data(struct mmc_card *card, struct
> > > mmc_host *host)
> > > +{
> > > +       struct mmc_request mrq = {};
> > > +       struct mmc_command cmd = {};
> > > +       struct mmc_data data = {};
> > > +       unsigned int len = 512;
> > > +       struct scatterlist sg;
> > > +       u8 *ext_csd;
> > > +
> > > +       ext_csd = kzalloc(len, GFP_KERNEL);
> > > +       if (!ext_csd)
> > > +               return -ENOMEM;
> > > +
> > > +       mrq.cmd = &cmd;
> > > +       mrq.data = &data;
> > > +
> > > +       cmd.opcode = MMC_SEND_EXT_CSD;
> > > +       cmd.arg = 0;
> > > +       cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_ADTC;
> > > +
> > > +       data.blksz = len;
> > > +       data.blocks = 1;
> > > +       data.flags = MMC_DATA_READ;
> > > +       data.sg = &sg;
> > > +       data.sg_len = 1;
> > > +
> > > +       sg_init_one(&sg, ext_csd, len);
> > > +       mmc_set_data_timeout(&data, card);
> > > +       mmc_wait_for_req(host, &mrq);
> > > +
> > > +       kfree(ext_csd);
> > > +
> > > +       if (cmd.error)
> > > +               return cmd.error;
> > > +       if (data.error)
> > > +               return data.error;
> > > +
> > > +       return 0;
> >
> > Why do we need to send a MMC_SEND_EXT_CSD command, exactly?
> >
> > Why can't mmc_send_tuning() work here too? What does the eMMC spec
> > state about this?
> >
> The CMD21 is illegal under hs400 mode so that cannot use the
> mmc_send_tuning(). The CMD8 is suitable because it will receive 1 block
> of non-zero data.

I see.

In that case it seems better to use mmc_get_ext_csd(), from the core,
rather than open coding the above. To do that, you also need to move
the declaration of mmc_get_ext_csd() to include/linux/mmc/host.h.

[...]

Kind regards
Uffe