[PATCH v4 0/3] mmc: mediatek: Use data tune for CMD line tune

[PATCH v4 0/3] mmc: mediatek: Use data tune for CMD line tune

[Yong Mao]

yong mao (3):
  mmc: dt-bindings: update Mediatek MMC bindings
  ARM64: dts: mediatek: configure some fixed mmc parameters
  mmc: mediatek: Use data tune for CMD line tune

 Documentation/devicetree/bindings/mmc/mtk-sd.txt |  12 ++
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts      |   3 +
 drivers/mmc/host/mtk-sd.c                        | 168 ++++++++++++++++++++---
 3 files changed, 167 insertions(+), 16 deletions(-)

-- 
1.8.1.1.dirty

[PATCH v4 1/3] mmc: dt-bindings: update Mediatek MMC bindings

[Yong Mao]

From: yong mao <yong.mao@mediatek.com>

Add description for mediatek,hs200-cmd-int-delay
Add description for mediatek,hs400-cmd-int-delay
Add description for mediatek,hs400-cmd-resp-sel-rising

Signed-off-by: Yong Mao <yong.mao@mediatek.com>
---
 Documentation/devicetree/bindings/mmc/mtk-sd.txt |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
index 0120c7f..4182ea3 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
@@ -21,6 +21,15 @@ Optional properties:
 - assigned-clocks: PLL of the source clock
 - assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
 - hs400-ds-delay: HS400 DS delay setting
+- mediatek,hs200-cmd-int-delay: HS200 command internal delay setting.
+				This field has total 32 stages.
+				The value is an integer from 0 to 31.
+- mediatek,hs400-cmd-int-delay: HS400 command internal delay setting
+				This field has total 32 stages.
+				The value is an integer from 0 to 31.
+- mediatek,hs400-cmd-resp-sel-rising:  HS400 command response sample selection
+				       If present,HS400 command responses are sampled on rising edges.
+				       If not present,HS400 command responses are sampled on falling edges.
 
 Examples:
 mmc0: mmc@11230000 {
@@ -38,4 +47,7 @@ mmc0: mmc@11230000 {
 	assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;
 	assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL_D2>;
 	hs400-ds-delay = <0x14015>;
+	mediatek,hs200-cmd-int-delay = <26>;
+	mediatek,hs400-cmd-int-delay = <14>;
+	mediatek,hs400-cmd-resp-sel-rising;
 };
-- 
1.7.9.5

[PATCH v4 2/3] ARM64: dts: mediatek: configure some fixed mmc parameters

[Yong Mao]

From: yong mao <yong.mao@mediatek.com>

configure some fixed mmc parameters

Signed-off-by: Yong Mao <yong.mao@mediatek.com>
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173-evb.dts |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
index 0ecaad4..1c3634f 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8173-evb.dts
@@ -134,6 +134,9 @@
 	bus-width = <8>;
 	max-frequency = <50000000>;
 	cap-mmc-highspeed;
+	mediatek,hs200-cmd-int-delay=<26>;
+	mediatek,hs400-cmd-int-delay=<14>;
+	mediatek,hs400-cmd-resp-sel-rising;
 	vmmc-supply = <&mt6397_vemc_3v3_reg>;
 	vqmmc-supply = <&mt6397_vio18_reg>;
 	non-removable;
-- 
1.7.9.5

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

[Yong Mao]

From: yong mao <yong.mao@mediatek.com>

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@mediatek.com>
Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com>
---
 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

Re: [PATCH v4 1/3] mmc: dt-bindings: update Mediatek MMC bindings

[Ulf Hansson]

+Rob, devicetree

On 21 January 2017 at 09:55, Yong Mao <yong.mao@mediatek.com> wrote:
> From: yong mao <yong.mao@mediatek.com>
>
> Add description for mediatek,hs200-cmd-int-delay
> Add description for mediatek,hs400-cmd-int-delay
> Add description for mediatek,hs400-cmd-resp-sel-rising
>
> Signed-off-by: Yong Mao <yong.mao@mediatek.com>

You need an ack from a DT maintainer for this. I have added Rob and
the devicetree list on cc.

Kind regards
Uffe

> ---
>  Documentation/devicetree/bindings/mmc/mtk-sd.txt |   12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> index 0120c7f..4182ea3 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> @@ -21,6 +21,15 @@ Optional properties:
>  - assigned-clocks: PLL of the source clock
>  - assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
>  - hs400-ds-delay: HS400 DS delay setting
> +- mediatek,hs200-cmd-int-delay: HS200 command internal delay setting.
> +                               This field has total 32 stages.
> +                               The value is an integer from 0 to 31.
> +- mediatek,hs400-cmd-int-delay: HS400 command internal delay setting
> +                               This field has total 32 stages.
> +                               The value is an integer from 0 to 31.
> +- mediatek,hs400-cmd-resp-sel-rising:  HS400 command response sample selection
> +                                      If present,HS400 command responses are sampled on rising edges.
> +                                      If not present,HS400 command responses are sampled on falling edges.
>
>  Examples:
>  mmc0: mmc@11230000 {
> @@ -38,4 +47,7 @@ mmc0: mmc@11230000 {
>         assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;
>         assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL_D2>;
>         hs400-ds-delay = <0x14015>;
> +       mediatek,hs200-cmd-int-delay = <26>;
> +       mediatek,hs400-cmd-int-delay = <14>;
> +       mediatek,hs400-cmd-resp-sel-rising;
>  };
> --
> 1.7.9.5
>

Re: [PATCH v4 1/3] mmc: dt-bindings: update Mediatek MMC bindings

[Yong Mao]

Dear Rob,

Could you please help to make comments for this patch?
Thanks.

On Tue, 2017-01-24 at 09:02 +0100, Ulf Hansson wrote:
> +Rob, devicetree
> 
> On 21 January 2017 at 09:55, Yong Mao <yong.mao@mediatek.com> wrote:
> > From: yong mao <yong.mao@mediatek.com>
> >
> > Add description for mediatek,hs200-cmd-int-delay
> > Add description for mediatek,hs400-cmd-int-delay
> > Add description for mediatek,hs400-cmd-resp-sel-rising
> >
> > Signed-off-by: Yong Mao <yong.mao@mediatek.com>
> 
> You need an ack from a DT maintainer for this. I have added Rob and
> the devicetree list on cc.
> 
> Kind regards
> Uffe
> 
> > ---
> >  Documentation/devicetree/bindings/mmc/mtk-sd.txt |   12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.txt b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > index 0120c7f..4182ea3 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.txt
> > @@ -21,6 +21,15 @@ Optional properties:
> >  - assigned-clocks: PLL of the source clock
> >  - assigned-clock-parents: parent of source clock, used for HS400 mode to get 400Mhz source clock
> >  - hs400-ds-delay: HS400 DS delay setting
> > +- mediatek,hs200-cmd-int-delay: HS200 command internal delay setting.
> > +                               This field has total 32 stages.
> > +                               The value is an integer from 0 to 31.
> > +- mediatek,hs400-cmd-int-delay: HS400 command internal delay setting
> > +                               This field has total 32 stages.
> > +                               The value is an integer from 0 to 31.
> > +- mediatek,hs400-cmd-resp-sel-rising:  HS400 command response sample selection
> > +                                      If present,HS400 command responses are sampled on rising edges.
> > +                                      If not present,HS400 command responses are sampled on falling edges.
> >
> >  Examples:
> >  mmc0: mmc@11230000 {
> > @@ -38,4 +47,7 @@ mmc0: mmc@11230000 {
> >         assigned-clocks = <&topckgen CLK_TOP_MSDC50_0_SEL>;
> >         assigned-clock-parents = <&topckgen CLK_TOP_MSDCPLL_D2>;
> >         hs400-ds-delay = <0x14015>;
> > +       mediatek,hs200-cmd-int-delay = <26>;
> > +       mediatek,hs400-cmd-int-delay = <14>;
> > +       mediatek,hs400-cmd-resp-sel-rising;
> >  };
> > --
> > 1.7.9.5
> >

Re: [PATCH v4 1/3] mmc: dt-bindings: update Mediatek MMC bindings

[Rob Herring]

On Fri, Feb 24, 2017 at 3:59 AM, Yong Mao <yong.mao@mediatek.com> wrote:
> Dear Rob,
>
> Could you please help to make comments for this patch?
> Thanks.

I already did comment. It's still wrong as Ulf commented. So fix and
send a new version. It has to go to the DT list if you want to be in
my queue.

Rob

Re: [PATCH v4 1/3] mmc: dt-bindings: update Mediatek MMC bindings

[Yong Mao]

On Fri, 2017-02-24 at 16:47 -0600, Rob Herring wrote:
> On Fri, Feb 24, 2017 at 3:59 AM, Yong Mao <yong.mao@mediatek.com> wrote:
> > Dear Rob,
> >
> > Could you please help to make comments for this patch?
> > Thanks.
> 
> I already did comment. It's still wrong as Ulf commented. So fix and
> send a new version. It has to go to the DT list if you want to be in
> my queue.
> 
> Rob

After reviewing the history, We guess your mentioned Ulf's comments is as below.
  
"> +- mtk-hs200-cmd-int-delay: HS200 command internal delay setting.
> + The value is an integer from 0 to 31

Please change to:

mediatek,hs200-cmd-delay

... and if there is a unit, like ns or us, please add that a suffix.

> +- mtk-hs400-cmd-int-delay: HS400 command internal delay setting
> + The value is an integer from 0 to 31

mediatek,hs400-cmd-delay and add unit if applicable.

> +- mtk-hs400-cmd-resp-sel: HS400 command response sample selection
> + The value is an integer from 0 to 1

mediatek,hs400-cmd-resp-sel

And make it a boolean value instead!"

==> We already fix this comment in v4. 
We use "mediatek,hs200-cmd-int-delay" to replace "mtk-hs200-cmd-int-delay", 
but not use "mediatek,hs200-cmd-delay". This is because "-int-" here means internal.
We should not drop it.

And this field does not have unit, it only have total 32 stages.
We also change the description in v4.

For comment about "mtk-hs400-cmd-resp-sel", we also make it a boolean value in v4.
And re-name it as "mediatek,hs400-cmd-resp-rising".

Please help to point out where we need to modify.
Thanks.

Re: [PATCH v4 1/3] mmc: dt-bindings: update Mediatek MMC bindings

[Yong Mao]

On Fri, 2017-03-03 at 11:49 +0800, Yong Mao wrote:
> On Fri, 2017-02-24 at 16:47 -0600, Rob Herring wrote:
> > On Fri, Feb 24, 2017 at 3:59 AM, Yong Mao <yong.mao@mediatek.com> wrote:
> > > Dear Rob,
> > >
> > > Could you please help to make comments for this patch?
> > > Thanks.
> > 
> > I already did comment. It's still wrong as Ulf commented. So fix and
> > send a new version. It has to go to the DT list if you want to be in
> > my queue.
> > 
> > Rob
> 
> After reviewing the history, We guess your mentioned Ulf's comments is as below.
>   
> "> +- mtk-hs200-cmd-int-delay: HS200 command internal delay setting.
> > + The value is an integer from 0 to 31
> 
> Please change to:
> 
> mediatek,hs200-cmd-delay
> 
> ... and if there is a unit, like ns or us, please add that a suffix.
> 
> > +- mtk-hs400-cmd-int-delay: HS400 command internal delay setting
> > + The value is an integer from 0 to 31
> 
> mediatek,hs400-cmd-delay and add unit if applicable.
> 
> > +- mtk-hs400-cmd-resp-sel: HS400 command response sample selection
> > + The value is an integer from 0 to 1
> 
> mediatek,hs400-cmd-resp-sel
> 
> And make it a boolean value instead!"
> 
> ==> We already fix this comment in v4. 
> We use "mediatek,hs200-cmd-int-delay" to replace "mtk-hs200-cmd-int-delay", 
> but not use "mediatek,hs200-cmd-delay". This is because "-int-" here means internal.
> We should not drop it.
> 
> And this field does not have unit, it only have total 32 stages.
> We also change the description in v4.
> 
> For comment about "mtk-hs400-cmd-resp-sel", we also make it a boolean value in v4.
> And re-name it as "mediatek,hs400-cmd-resp-rising".
> 
> Please help to point out where we need to modify.
> Thanks.
> 
> 

Please help to point out a bright way for this patch.
We are anxiously waiting for your response.

Best Regards,
Yong Mao.

Re: [PATCH v4 1/3] mmc: dt-bindings: update Mediatek MMC bindings

[Ulf Hansson]

[...]

>>
>
> Please help to point out a bright way for this patch.
> We are anxiously waiting for your response.

Yong, I suggest you make resend of the series. This time make sure you
send to DT maintainers as well.

Kind regards
Uffe