[PATCH 0/2] Add mtk-sd support for MT8196

[PATCH 0/2] Add mtk-sd support for MT8196

[Andy-ld Lu]

There are some new features for Mediatek SoC MT8196, which include new
command/data transmitting and receiving path (abbreviated as tx/rx), and
a new bus design that prevents the CPU from accessing the IP registers
unless the source clock control gate is enabled. Furthermore, some register
settings are also modified.

The driver code has to be adapted to implement the above changes, and the
compatible 'mediatek,msdc-v2' is added to driver and devicetree bindings,
for MT8196 and future compatible SoCs that support new tx/rx.

Andy-ld Lu (2):
  mmc: mtk-sd: Add support for MT8196
  dt-bindings: mmc: mtk-sd: Add support for MT8196

 .../devicetree/bindings/mmc/mtk-sd.yaml       |  26 +++
 drivers/mmc/host/mtk-sd.c                     | 179 +++++++++++++++---
 2 files changed, 182 insertions(+), 23 deletions(-)

-- 
2.46.0

[PATCH 1/2] mmc: mtk-sd: Add support for MT8196

[Andy-ld Lu]

Mediatek SoC MT8196 features a new design for tx/rx path. The new tx
path incorporates register settings that are closely associated with
bus timing. And the difference between new rx path and older versions
is the usage of distinct register bits when setting the data sampling
edge as part of the tuning process. Add compatible 'mediatek,msdc-v2'
for future SoCs that support new tx/rx.

Compared to the previous SoC, MT8196 showcases a new bus design, which
prevents the CPU from accessing the IP registers when the source CG of
our IP is disabled. To address this, add a property named 'mediatek,
prohibit-gate-cg' in the device tree to ensure source CG is not gated
before writing to MSDC_CFG register.

Besides, there are modified register settings for STOP_DLY_SEL and
POP_EN_CNT, with two new properties added to the device tree to reflect
the modifications.

Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
---
 drivers/mmc/host/mtk-sd.c | 179 +++++++++++++++++++++++++++++++++-----
 1 file changed, 156 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index 89018b6c97b9..c265e56eec09 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -65,6 +65,7 @@
 #define SDC_RESP3        0x4c
 #define SDC_BLK_NUM      0x50
 #define SDC_ADV_CFG0     0x64
+#define MSDC_NEW_RX_CFG  0x68
 #define EMMC_IOCON       0x7c
 #define SDC_ACMD_RESP    0x80
 #define DMA_SA_H4BIT     0x8c
@@ -91,6 +92,7 @@
 #define EMMC_TOP_CONTROL	0x00
 #define EMMC_TOP_CMD		0x04
 #define EMMC50_PAD_DS_TUNE	0x0c
+#define LOOP_TEST_CONTROL	0x30
 
 /*--------------------------------------------------------------------------*/
 /* Register Mask                                                            */
@@ -202,9 +204,13 @@
 #define SDC_STS_CMDBUSY         BIT(1)	/* RW */
 #define SDC_STS_SWR_COMPL       BIT(31)	/* RW */
 
-#define SDC_DAT1_IRQ_TRIGGER	BIT(19)	/* RW */
 /* SDC_ADV_CFG0 mask */
+#define SDC_DAT1_IRQ_TRIGGER	BIT(19)	/* RW */
 #define SDC_RX_ENHANCE_EN	BIT(20)	/* RW */
+#define SDC_NEW_TX_EN		BIT(31)	/* RW */
+
+/* MSDC_NEW_RX_CFG mask */
+#define MSDC_NEW_RX_PATH_SEL	BIT(0)	/* RW */
 
 /* DMA_SA_H4BIT mask */
 #define DMA_ADDR_HIGH_4BIT      GENMASK(3, 0)	/* RW */
@@ -226,6 +232,7 @@
 
 /* MSDC_PATCH_BIT mask */
 #define MSDC_PATCH_BIT_ODDSUPP    BIT(1)	/* RW */
+#define MSDC_PATCH_BIT_RD_DAT_SEL BIT(3)	/* RW */
 #define MSDC_INT_DAT_LATCH_CK_SEL GENMASK(9, 7)
 #define MSDC_CKGEN_MSDC_DLY_SEL   GENMASK(14, 10)
 #define MSDC_PATCH_BIT_IODSSEL    BIT(16)	/* RW */
@@ -247,6 +254,8 @@
 #define MSDC_PB2_SUPPORT_64G      BIT(1)    /* RW */
 #define MSDC_PB2_RESPWAIT         GENMASK(3, 2)   /* RW */
 #define MSDC_PB2_RESPSTSENSEL     GENMASK(18, 16) /* RW */
+#define MSDC_PB2_POP_EN_CNT       GENMASK(23, 20) /* RW */
+#define MSDC_PB2_CFGCRCSTSEDGE    BIT(25)   /* RW */
 #define MSDC_PB2_CRCSTSENSEL      GENMASK(31, 29) /* RW */
 
 #define MSDC_PAD_TUNE_DATWRDLY	  GENMASK(4, 0)		/* RW */
@@ -311,6 +320,12 @@
 #define PAD_DS_DLY1		GENMASK(14, 10)	/* RW */
 #define PAD_DS_DLY3		GENMASK(4, 0)	/* RW */
 
+/* LOOP_TEST_CONTROL mask */
+#define TEST_LOOP_DSCLK_MUX_SEL        BIT(0)	/* RW */
+#define TEST_LOOP_LATCH_MUX_SEL        BIT(1)	/* RW */
+#define LOOP_EN_SEL_CLK                BIT(20)	/* RW */
+#define TEST_HS400_CMD_LOOP_MUX_SEL    BIT(31)	/* RW */
+
 #define REQ_CMD_EIO  BIT(0)
 #define REQ_CMD_TMO  BIT(1)
 #define REQ_DAT_ERR  BIT(2)
@@ -391,6 +406,7 @@ struct msdc_save_para {
 	u32 emmc_top_control;
 	u32 emmc_top_cmd;
 	u32 emmc50_pad_ds_tune;
+	u32 loop_test_control;
 };
 
 struct mtk_mmc_compatible {
@@ -405,6 +421,8 @@ struct mtk_mmc_compatible {
 	bool enhance_rx;
 	bool support_64g;
 	bool use_internal_cd;
+	bool support_new_tx;
+	bool support_new_rx;
 };
 
 struct msdc_tune_para {
@@ -462,6 +480,8 @@ struct msdc_host {
 	unsigned char timing;
 	bool vqmmc_enabled;
 	u32 latch_ck;
+	u32 stop_dly_sel;
+	u32 pop_en_cnt;
 	u32 hs400_ds_delay;
 	u32 hs400_ds_dly3;
 	u32 hs200_cmd_int_delay; /* cmd internal delay for HS200/SDR104 */
@@ -473,6 +493,7 @@ struct msdc_host {
 	bool hs400_tuning;	/* hs400 mode online tuning */
 	bool internal_cd;	/* Use internal card-detect logic */
 	bool cqhci;		/* support eMMC hw cmdq */
+	bool prohibit_gate_cg;	/* prohibit to gate source clock cg before access register */
 	struct msdc_save_para save_para; /* used when gate HCLK */
 	struct msdc_tune_para def_tune_para; /* default tune setting */
 	struct msdc_tune_para saved_tune_para; /* tune result of CMD21/CMD19 */
@@ -621,6 +642,21 @@ static const struct mtk_mmc_compatible mt8516_compat = {
 	.stop_clk_fix = true,
 };
 
+static const struct mtk_mmc_compatible msdc_v2_compat = {
+	.clk_div_bits = 12,
+	.recheck_sdio_irq = false,
+	.hs400_tune = false,
+	.pad_tune_reg = MSDC_PAD_TUNE0,
+	.async_fifo = true,
+	.data_tune = true,
+	.busy_check = true,
+	.stop_clk_fix = true,
+	.enhance_rx = true,
+	.support_64g = true,
+	.support_new_tx = true,
+	.support_new_rx = true,
+};
+
 static const struct of_device_id msdc_of_ids[] = {
 	{ .compatible = "mediatek,mt2701-mmc", .data = &mt2701_compat},
 	{ .compatible = "mediatek,mt2712-mmc", .data = &mt2712_compat},
@@ -633,6 +669,7 @@ static const struct of_device_id msdc_of_ids[] = {
 	{ .compatible = "mediatek,mt8173-mmc", .data = &mt8173_compat},
 	{ .compatible = "mediatek,mt8183-mmc", .data = &mt8183_compat},
 	{ .compatible = "mediatek,mt8516-mmc", .data = &mt8516_compat},
+	{ .compatible = "mediatek,msdc-v2", .data = &msdc_v2_compat},
 
 	{}
 };
@@ -872,6 +909,42 @@ static int msdc_ungate_clock(struct msdc_host *host)
 				  (val & MSDC_CFG_CKSTB), 1, 20000);
 }
 
+static void msdc_new_tx_setting(struct msdc_host *host)
+{
+	if (host->top_base) {
+		sdr_set_bits(host->top_base + LOOP_TEST_CONTROL,
+			     TEST_LOOP_DSCLK_MUX_SEL);
+		sdr_set_bits(host->top_base + LOOP_TEST_CONTROL,
+			     TEST_LOOP_LATCH_MUX_SEL);
+		sdr_clr_bits(host->top_base + LOOP_TEST_CONTROL,
+			     TEST_HS400_CMD_LOOP_MUX_SEL);
+	}
+
+	switch (host->timing) {
+	case MMC_TIMING_LEGACY:
+	case MMC_TIMING_MMC_HS:
+	case MMC_TIMING_SD_HS:
+	case MMC_TIMING_UHS_SDR12:
+	case MMC_TIMING_UHS_SDR25:
+	case MMC_TIMING_UHS_DDR50:
+	case MMC_TIMING_MMC_DDR52:
+		if (host->top_base)
+			sdr_clr_bits(host->top_base + LOOP_TEST_CONTROL,
+				     LOOP_EN_SEL_CLK);
+		break;
+	case MMC_TIMING_UHS_SDR50:
+	case MMC_TIMING_UHS_SDR104:
+	case MMC_TIMING_MMC_HS200:
+	case MMC_TIMING_MMC_HS400:
+		if (host->top_base)
+			sdr_set_bits(host->top_base + LOOP_TEST_CONTROL,
+				     LOOP_EN_SEL_CLK);
+		break;
+	default:
+		break;
+	}
+}
+
 static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 {
 	struct mmc_host *mmc = mmc_from_priv(host);
@@ -881,6 +954,7 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 	u32 sclk;
 	u32 tune_reg = host->dev_comp->pad_tune_reg;
 	u32 val;
+	bool timing_changed = false;
 
 	if (!hz) {
 		dev_dbg(host->dev, "set mclk to 0\n");
@@ -890,6 +964,9 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 		return;
 	}
 
+	if (host->timing != timing)
+		timing_changed = true;
+
 	flags = readl(host->base + MSDC_INTEN);
 	sdr_clr_bits(host->base + MSDC_INTEN, flags);
 	if (host->dev_comp->clk_div_bits == 8)
@@ -941,7 +1018,8 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 	}
 	sdr_clr_bits(host->base + MSDC_CFG, MSDC_CFG_CKPDN);
 
-	clk_disable_unprepare(host->src_clk_cg);
+	if (!host->prohibit_gate_cg)
+		clk_disable_unprepare(host->src_clk_cg);
 	if (host->dev_comp->clk_div_bits == 8)
 		sdr_set_field(host->base + MSDC_CFG,
 			      MSDC_CFG_CKMOD | MSDC_CFG_CKDIV,
@@ -951,7 +1029,8 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 			      MSDC_CFG_CKMOD_EXTRA | MSDC_CFG_CKDIV_EXTRA,
 			      (mode << 12) | div);
 
-	clk_prepare_enable(host->src_clk_cg);
+	if (!host->prohibit_gate_cg)
+		clk_prepare_enable(host->src_clk_cg);
 	readl_poll_timeout(host->base + MSDC_CFG, val, (val & MSDC_CFG_CKSTB), 0, 0);
 	sdr_set_bits(host->base + MSDC_CFG, MSDC_CFG_CKPDN);
 	mmc->actual_clock = sclk;
@@ -996,6 +1075,9 @@ static void msdc_set_mclk(struct msdc_host *host, unsigned char timing, u32 hz)
 		sdr_set_field(host->base + tune_reg,
 			      MSDC_PAD_TUNE_CMDRRDLY,
 			      host->hs400_cmd_int_delay);
+	if (timing_changed && host->dev_comp->support_new_tx)
+		msdc_new_tx_setting(host);
+
 	dev_dbg(host->dev, "sclk: %d, timing: %d\n", mmc->actual_clock,
 		timing);
 }
@@ -1704,6 +1786,17 @@ static void msdc_init_hw(struct msdc_host *host)
 		reset_control_deassert(host->reset);
 	}
 
+	/* New tx/rx enable bit need to be 0->1 for hardware check */
+	if (host->dev_comp->support_new_tx) {
+		sdr_clr_bits(host->base + SDC_ADV_CFG0, SDC_NEW_TX_EN);
+		sdr_set_bits(host->base + SDC_ADV_CFG0, SDC_NEW_TX_EN);
+		msdc_new_tx_setting(host);
+	}
+	if (host->dev_comp->support_new_rx) {
+		sdr_clr_bits(host->base + MSDC_NEW_RX_CFG, MSDC_NEW_RX_PATH_SEL);
+		sdr_set_bits(host->base + MSDC_NEW_RX_CFG, MSDC_NEW_RX_PATH_SEL);
+	}
+
 	/* Configure to MMC/SD mode, clock free running */
 	sdr_set_bits(host->base + MSDC_CFG, MSDC_CFG_MODE | MSDC_CFG_CKPDN);
 
@@ -1742,8 +1835,19 @@ static void msdc_init_hw(struct msdc_host *host)
 	sdr_set_bits(host->base + EMMC50_CFG0, EMMC50_CFG_CFCSTS_SEL);
 
 	if (host->dev_comp->stop_clk_fix) {
-		sdr_set_field(host->base + MSDC_PATCH_BIT1,
-			      MSDC_PATCH_BIT1_STOP_DLY, 3);
+		if (host->stop_dly_sel)
+			sdr_set_field(host->base + MSDC_PATCH_BIT1,
+				      MSDC_PATCH_BIT1_STOP_DLY,
+				      host->stop_dly_sel & 0xf);
+		else
+			sdr_set_field(host->base + MSDC_PATCH_BIT1,
+				      MSDC_PATCH_BIT1_STOP_DLY, 3);
+
+		if (host->pop_en_cnt)
+			sdr_set_field(host->base + MSDC_PATCH_BIT2,
+				      MSDC_PB2_POP_EN_CNT,
+				      host->pop_en_cnt & 0xf);
+
 		sdr_clr_bits(host->base + SDC_FIFO_CFG,
 			     SDC_FIFO_CFG_WRVALIDSEL);
 		sdr_clr_bits(host->base + SDC_FIFO_CFG,
@@ -2055,6 +2159,19 @@ static inline void msdc_set_data_delay(struct msdc_host *host, u32 value)
 	}
 }
 
+static inline void msdc_set_data_sample_edge(struct msdc_host *host, bool rising)
+{
+	u32 value = rising ? 0 : 1;
+
+	if (host->dev_comp->support_new_rx) {
+		sdr_set_field(host->base + MSDC_PATCH_BIT, MSDC_PATCH_BIT_RD_DAT_SEL, value);
+		sdr_set_field(host->base + MSDC_PATCH_BIT2, MSDC_PB2_CFGCRCSTSEDGE, value);
+	} else {
+		sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_DSPL, value);
+		sdr_set_field(host->base + MSDC_IOCON, MSDC_IOCON_W_DSPL, value);
+	}
+}
+
 static int msdc_tune_response(struct mmc_host *mmc, u32 opcode)
 {
 	struct msdc_host *host = mmc_priv(mmc);
@@ -2210,8 +2327,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
 
 	sdr_set_field(host->base + MSDC_PATCH_BIT, MSDC_INT_DAT_LATCH_CK_SEL,
 		      host->latch_ck);
-	sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_DSPL);
-	sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_W_DSPL);
+	msdc_set_data_sample_edge(host, true);
 	for (i = 0; i < host->tuning_step; i++) {
 		msdc_set_data_delay(host, i);
 		ret = mmc_send_tuning(mmc, opcode, NULL);
@@ -2224,8 +2340,7 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
 	    (final_rise_delay.start == 0 && final_rise_delay.maxlen >= 4))
 		goto skip_fall;
 
-	sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_DSPL);
-	sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_W_DSPL);
+	msdc_set_data_sample_edge(host, false);
 	for (i = 0; i < host->tuning_step; i++) {
 		msdc_set_data_delay(host, i);
 		ret = mmc_send_tuning(mmc, opcode, NULL);
@@ -2237,12 +2352,10 @@ static int msdc_tune_data(struct mmc_host *mmc, u32 opcode)
 skip_fall:
 	final_maxlen = max(final_rise_delay.maxlen, final_fall_delay.maxlen);
 	if (final_maxlen == final_rise_delay.maxlen) {
-		sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_DSPL);
-		sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_W_DSPL);
+		msdc_set_data_sample_edge(host, true);
 		final_delay = final_rise_delay.final_phase;
 	} else {
-		sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_DSPL);
-		sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_W_DSPL);
+		msdc_set_data_sample_edge(host, false);
 		final_delay = final_fall_delay.final_phase;
 	}
 	msdc_set_data_delay(host, final_delay);
@@ -2267,8 +2380,7 @@ static int msdc_tune_together(struct mmc_host *mmc, u32 opcode)
 		      host->latch_ck);
 
 	sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
-	sdr_clr_bits(host->base + MSDC_IOCON,
-		     MSDC_IOCON_DSPL | MSDC_IOCON_W_DSPL);
+	msdc_set_data_sample_edge(host, true);
 	for (i = 0; i < host->tuning_step; i++) {
 		msdc_set_cmd_delay(host, i);
 		msdc_set_data_delay(host, i);
@@ -2283,8 +2395,7 @@ static int msdc_tune_together(struct mmc_host *mmc, u32 opcode)
 		goto skip_fall;
 
 	sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
-	sdr_set_bits(host->base + MSDC_IOCON,
-		     MSDC_IOCON_DSPL | MSDC_IOCON_W_DSPL);
+	msdc_set_data_sample_edge(host, false);
 	for (i = 0; i < host->tuning_step; i++) {
 		msdc_set_cmd_delay(host, i);
 		msdc_set_data_delay(host, i);
@@ -2298,13 +2409,11 @@ static int msdc_tune_together(struct mmc_host *mmc, u32 opcode)
 	final_maxlen = max(final_rise_delay.maxlen, final_fall_delay.maxlen);
 	if (final_maxlen == final_rise_delay.maxlen) {
 		sdr_clr_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
-		sdr_clr_bits(host->base + MSDC_IOCON,
-			     MSDC_IOCON_DSPL | MSDC_IOCON_W_DSPL);
+		msdc_set_data_sample_edge(host, true);
 		final_delay = final_rise_delay.final_phase;
 	} else {
 		sdr_set_bits(host->base + MSDC_IOCON, MSDC_IOCON_RSPL);
-		sdr_set_bits(host->base + MSDC_IOCON,
-			     MSDC_IOCON_DSPL | MSDC_IOCON_W_DSPL);
+		msdc_set_data_sample_edge(host, false);
 		final_delay = final_fall_delay.final_phase;
 	}
 
@@ -2324,8 +2433,7 @@ static int msdc_execute_tuning(struct mmc_host *mmc, u32 opcode)
 	if (host->dev_comp->data_tune && host->dev_comp->async_fifo) {
 		ret = msdc_tune_together(mmc, opcode);
 		if (host->hs400_mode) {
-			sdr_clr_bits(host->base + MSDC_IOCON,
-				     MSDC_IOCON_DSPL | MSDC_IOCON_W_DSPL);
+			msdc_set_data_sample_edge(host, true);
 			msdc_set_data_delay(host, 0);
 		}
 		goto tune_done;
@@ -2649,12 +2757,24 @@ static void msdc_of_property_parse(struct platform_device *pdev,
 	of_property_read_u32(pdev->dev.of_node, "mediatek,hs400-cmd-int-delay",
 			     &host->hs400_cmd_int_delay);
 
+	of_property_read_u32(pdev->dev.of_node, "mediatek,stop-dly-sel",
+			     &host->stop_dly_sel);
+
+	of_property_read_u32(pdev->dev.of_node, "mediatek,pop-en-cnt",
+			     &host->pop_en_cnt);
+
 	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;
 
+	if (of_property_read_bool(pdev->dev.of_node,
+				  "mediatek,prohibit-gate-cg"))
+		host->prohibit_gate_cg = true;
+	else
+		host->prohibit_gate_cg = false;
+
 	if (of_property_read_u32(pdev->dev.of_node, "mediatek,tuning-step",
 				 &host->tuning_step)) {
 		if (mmc->caps2 & MMC_CAP2_NO_MMC)
@@ -2995,6 +3115,8 @@ static void msdc_save_reg(struct msdc_host *host)
 			readl(host->top_base + EMMC_TOP_CMD);
 		host->save_para.emmc50_pad_ds_tune =
 			readl(host->top_base + EMMC50_PAD_DS_TUNE);
+		host->save_para.loop_test_control =
+			readl(host->top_base + LOOP_TEST_CONTROL);
 	} else {
 		host->save_para.pad_tune = readl(host->base + tune_reg);
 	}
@@ -3005,6 +3127,15 @@ static void msdc_restore_reg(struct msdc_host *host)
 	struct mmc_host *mmc = mmc_from_priv(host);
 	u32 tune_reg = host->dev_comp->pad_tune_reg;
 
+	if (host->dev_comp->support_new_tx) {
+		sdr_clr_bits(host->base + SDC_ADV_CFG0, SDC_NEW_TX_EN);
+		sdr_set_bits(host->base + SDC_ADV_CFG0, SDC_NEW_TX_EN);
+	}
+	if (host->dev_comp->support_new_rx) {
+		sdr_clr_bits(host->base + MSDC_NEW_RX_CFG, MSDC_NEW_RX_PATH_SEL);
+		sdr_set_bits(host->base + MSDC_NEW_RX_CFG, MSDC_NEW_RX_PATH_SEL);
+	}
+
 	writel(host->save_para.msdc_cfg, host->base + MSDC_CFG);
 	writel(host->save_para.iocon, host->base + MSDC_IOCON);
 	writel(host->save_para.sdc_cfg, host->base + SDC_CFG);
@@ -3023,6 +3154,8 @@ static void msdc_restore_reg(struct msdc_host *host)
 		       host->top_base + EMMC_TOP_CMD);
 		writel(host->save_para.emmc50_pad_ds_tune,
 		       host->top_base + EMMC50_PAD_DS_TUNE);
+		writel(host->save_para.loop_test_control,
+		       host->top_base + LOOP_TEST_CONTROL);
 	} else {
 		writel(host->save_para.pad_tune, host->base + tune_reg);
 	}
-- 
2.46.0

[PATCH 2/2] dt-bindings: mmc: mtk-sd: Add support for MT8196

[Andy-ld Lu]

Extend the devicetree bindings to include the MT8196 mmc controller
by adding the compatible string 'mediatek,msdc-v2', which could be
also used for future compatible SoCs that support new tx/rx.

Add three properties for MT8196 settings:
- 'mediatek,prohibit-gate-cg', indicate if the source clock CG could
  be disabled when CPU access IP registers.

- 'mediatek,stop-dly-sel', configure read data clock stops at block gap.

- 'mediatek,pop-en-cnt', configure the margins of write and read
  pointers while begin to pop data transfer.

Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
---
 .../devicetree/bindings/mmc/mtk-sd.yaml       | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
index c532ec92d2d9..82d1a9fac67c 100644
--- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
+++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
@@ -25,6 +25,7 @@ properties:
           - mediatek,mt8173-mmc
           - mediatek,mt8183-mmc
           - mediatek,mt8516-mmc
+          - mediatek,msdc-v2
       - items:
           - const: mediatek,mt7623-mmc
           - const: mediatek,mt2701-mmc
@@ -154,6 +155,30 @@ properties:
     enum: [32, 64]
     default: 32
 
+  mediatek,stop-dly-sel:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Some SoCs need to set appropriate stop-dly-sel to configure read data clock
+      stops at block gap. The valid range is from 0 to 0xf.
+    minimum: 0
+    maximum: 0xf
+
+  mediatek,pop-en-cnt:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Some SoCs need to set appropriate pop-en-cnt to configure the margins of write
+      and read pointers while begin to pop data transfer. The valid range is from 0
+      to 0xf.
+    minimum: 0
+    maximum: 0xf
+
+  mediatek,prohibit-gate-cg:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      Decide if source clock CG could be disabled when CPU access IP registers.
+      If present, source clock CG could not be disabled.
+      If not present, source clock CG could be disabled.
+
   resets:
     maxItems: 1
 
@@ -191,6 +216,7 @@ allOf:
             - mediatek,mt8188-mmc
             - mediatek,mt8195-mmc
             - mediatek,mt8516-mmc
+            - mediatek,msdc-v2
     then:
       properties:
         clocks:
-- 
2.46.0

Re: [PATCH 2/2] dt-bindings: mmc: mtk-sd: Add support for MT8196

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3017 bytes --]

On Thu, Sep 26, 2024 at 03:03:18PM +0800, Andy-ld Lu wrote:
> Extend the devicetree bindings to include the MT8196 mmc controller
> by adding the compatible string 'mediatek,msdc-v2', which could be
> also used for future compatible SoCs that support new tx/rx.
> 
> Add three properties for MT8196 settings:
> - 'mediatek,prohibit-gate-cg', indicate if the source clock CG could
>   be disabled when CPU access IP registers.
> 
> - 'mediatek,stop-dly-sel', configure read data clock stops at block gap.
> 
> - 'mediatek,pop-en-cnt', configure the margins of write and read
>   pointers while begin to pop data transfer.
> 
> Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
> ---
>  .../devicetree/bindings/mmc/mtk-sd.yaml       | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> index c532ec92d2d9..82d1a9fac67c 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> @@ -25,6 +25,7 @@ properties:
>            - mediatek,mt8173-mmc
>            - mediatek,mt8183-mmc
>            - mediatek,mt8516-mmc
> +          - mediatek,msdc-v2
>        - items:
>            - const: mediatek,mt7623-mmc
>            - const: mediatek,mt2701-mmc
> @@ -154,6 +155,30 @@ properties:
>      enum: [32, 64]
>      default: 32
>  
> +  mediatek,stop-dly-sel:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Some SoCs need to set appropriate stop-dly-sel to configure read data clock
> +      stops at block gap. The valid range is from 0 to 0xf.

If you used a specific compatible for these devices, which you should,
you would not require either this or "pop-en-cnt".

> +    minimum: 0
> +    maximum: 0xf
> +
> +  mediatek,pop-en-cnt:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Some SoCs need to set appropriate pop-en-cnt to configure the margins of write
> +      and read pointers while begin to pop data transfer. The valid range is from 0
> +      to 0xf.
> +    minimum: 0
> +    maximum: 0xf
> +
> +  mediatek,prohibit-gate-cg:

This is also probably detectable from a specific compatible, if you had
one?
TL;DR specific compatibles please, non of this "msdc-v2" stuff, sorry.

Thanks,
Conor.

> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Decide if source clock CG could be disabled when CPU access IP registers.
> +      If present, source clock CG could not be disabled.
> +      If not present, source clock CG could be disabled.
> +
>    resets:
>      maxItems: 1
>  
> @@ -191,6 +216,7 @@ allOf:
>              - mediatek,mt8188-mmc
>              - mediatek,mt8195-mmc
>              - mediatek,mt8516-mmc
> +            - mediatek,msdc-v2
>      then:
>        properties:
>          clocks:
> -- 
> 2.46.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] dt-bindings: mmc: mtk-sd: Add support for MT8196

[Andy-ld Lu (卢东)]

On Thu, 2024-09-26 at 17:13 +0100, Conor Dooley wrote:
> On Thu, Sep 26, 2024 at 03:03:18PM +0800, Andy-ld Lu wrote:
> > Extend the devicetree bindings to include the MT8196 mmc controller
> > by adding the compatible string 'mediatek,msdc-v2', which could be
> > also used for future compatible SoCs that support new tx/rx.
> > 
> > Add three properties for MT8196 settings:
> > - 'mediatek,prohibit-gate-cg', indicate if the source clock CG
> > could
> >   be disabled when CPU access IP registers.
> > 
> > - 'mediatek,stop-dly-sel', configure read data clock stops at block
> > gap.
> > 
> > - 'mediatek,pop-en-cnt', configure the margins of write and read
> >   pointers while begin to pop data transfer.
> > 
> > Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
> > ---
> >  .../devicetree/bindings/mmc/mtk-sd.yaml       | 26
> > +++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > index c532ec92d2d9..82d1a9fac67c 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > @@ -25,6 +25,7 @@ properties:
> >            - mediatek,mt8173-mmc
> >            - mediatek,mt8183-mmc
> >            - mediatek,mt8516-mmc
> > +          - mediatek,msdc-v2
> >        - items:
> >            - const: mediatek,mt7623-mmc
> >            - const: mediatek,mt2701-mmc
> > @@ -154,6 +155,30 @@ properties:
> >      enum: [32, 64]
> >      default: 32
> >  
> > +  mediatek,stop-dly-sel:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Some SoCs need to set appropriate stop-dly-sel to configure
> > read data clock
> > +      stops at block gap. The valid range is from 0 to 0xf.
> 
> If you used a specific compatible for these devices, which you
> should,
> you would not require either this or "pop-en-cnt".

Thanks for your review.

I will follow your comment in next change.

> 
> > +    minimum: 0
> > +    maximum: 0xf
> > +
> > +  mediatek,pop-en-cnt:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Some SoCs need to set appropriate pop-en-cnt to configure
> > the margins of write
> > +      and read pointers while begin to pop data transfer. The
> > valid range is from 0
> > +      to 0xf.
> > +    minimum: 0
> > +    maximum: 0xf
> > +
> > +  mediatek,prohibit-gate-cg:
> 
> This is also probably detectable from a specific compatible, if you
> had
> one?
> TL;DR specific compatibles please, non of this "msdc-v2" stuff,
> sorry.
> 
> Thanks,
> Conor.

I will follow your comment in next change.

> 
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description:
> > +      Decide if source clock CG could be disabled when CPU access
> > IP registers.
> > +      If present, source clock CG could not be disabled.
> > +      If not present, source clock CG could be disabled.
> > +
> >    resets:
> >      maxItems: 1
> >  
> > @@ -191,6 +216,7 @@ allOf:
> >              - mediatek,mt8188-mmc
> >              - mediatek,mt8195-mmc
> >              - mediatek,mt8516-mmc
> > +            - mediatek,msdc-v2
> >      then:
> >        properties:
> >          clocks:
> > -- 
> > 2.46.0
> > 
> > 

Re: [PATCH 2/2] dt-bindings: mmc: mtk-sd: Add support for MT8196

[Rob Herring]

On Thu, Sep 26, 2024 at 03:03:18PM +0800, Andy-ld Lu wrote:
> Extend the devicetree bindings to include the MT8196 mmc controller
> by adding the compatible string 'mediatek,msdc-v2', which could be
> also used for future compatible SoCs that support new tx/rx.

Generally, every SoC ends up changing at least slightly. So we don't do 
version numbers except when there's a well defined versioning scheme of 
the h/w (e.g. FPGA IP blocks). So, use SoC for compatible string.

> 
> Add three properties for MT8196 settings:
> - 'mediatek,prohibit-gate-cg', indicate if the source clock CG could
>   be disabled when CPU access IP registers.
> 
> - 'mediatek,stop-dly-sel', configure read data clock stops at block gap.
> 
> - 'mediatek,pop-en-cnt', configure the margins of write and read
>   pointers while begin to pop data transfer.
> 
> Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
> ---
>  .../devicetree/bindings/mmc/mtk-sd.yaml       | 26 +++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> index c532ec92d2d9..82d1a9fac67c 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> @@ -25,6 +25,7 @@ properties:
>            - mediatek,mt8173-mmc
>            - mediatek,mt8183-mmc
>            - mediatek,mt8516-mmc
> +          - mediatek,msdc-v2
>        - items:
>            - const: mediatek,mt7623-mmc
>            - const: mediatek,mt2701-mmc
> @@ -154,6 +155,30 @@ properties:
>      enum: [32, 64]
>      default: 32
>  
> +  mediatek,stop-dly-sel:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Some SoCs need to set appropriate stop-dly-sel to configure read data clock
> +      stops at block gap. The valid range is from 0 to 0xf.

SoC dependent or board dependent? Imply from the compatible for the 
former. A property is fine for the latter case.

> +    minimum: 0
> +    maximum: 0xf
> +
> +  mediatek,pop-en-cnt:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Some SoCs need to set appropriate pop-en-cnt to configure the margins of write
> +      and read pointers while begin to pop data transfer. The valid range is from 0
> +      to 0xf.
> +    minimum: 0
> +    maximum: 0xf

Same question.

> +
> +  mediatek,prohibit-gate-cg:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      Decide if source clock CG could be disabled when CPU access IP registers.
> +      If present, source clock CG could not be disabled.
> +      If not present, source clock CG could be disabled.


Sounds like you need to describe the clock in "clocks".

> +
>    resets:
>      maxItems: 1
>  
> @@ -191,6 +216,7 @@ allOf:
>              - mediatek,mt8188-mmc
>              - mediatek,mt8195-mmc
>              - mediatek,mt8516-mmc
> +            - mediatek,msdc-v2
>      then:
>        properties:
>          clocks:
> -- 
> 2.46.0
> 

Re: [PATCH 2/2] dt-bindings: mmc: mtk-sd: Add support for MT8196

[Andy-ld Lu (卢东)]

On Thu, 2024-09-26 at 12:54 -0500, Rob Herring wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Thu, Sep 26, 2024 at 03:03:18PM +0800, Andy-ld Lu wrote:
> > Extend the devicetree bindings to include the MT8196 mmc controller
> > by adding the compatible string 'mediatek,msdc-v2', which could be
> > also used for future compatible SoCs that support new tx/rx.
> 
> Generally, every SoC ends up changing at least slightly. So we don't
> do 
> version numbers except when there's a well defined versioning scheme
> of 
> the h/w (e.g. FPGA IP blocks). So, use SoC for compatible string.
Thanks for your review.

The new tx/rx represents a significant update for mmc controller of
mediatek from the MT8196 chipset, and the dependent settings remain
consistent for MT8196 and subsequent SoCs. Therefore, It is proposed to
use a unified compatible string for these SoCs that support new tx/rx,
and name it as 'IP(msdc) version two'.
> 
> > 
> > Add three properties for MT8196 settings:
> > - 'mediatek,prohibit-gate-cg', indicate if the source clock CG
> could
> >   be disabled when CPU access IP registers.
> > 
> > - 'mediatek,stop-dly-sel', configure read data clock stops at block
> gap.
> > 
> > - 'mediatek,pop-en-cnt', configure the margins of write and read
> >   pointers while begin to pop data transfer.
> > 
> > Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
> > ---
> >  .../devicetree/bindings/mmc/mtk-sd.yaml       | 26
> +++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > index c532ec92d2d9..82d1a9fac67c 100644
> > --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> > @@ -25,6 +25,7 @@ properties:
> >            - mediatek,mt8173-mmc
> >            - mediatek,mt8183-mmc
> >            - mediatek,mt8516-mmc
> > +          - mediatek,msdc-v2
> >        - items:
> >            - const: mediatek,mt7623-mmc
> >            - const: mediatek,mt2701-mmc
> > @@ -154,6 +155,30 @@ properties:
> >      enum: [32, 64]
> >      default: 32
> >  
> > +  mediatek,stop-dly-sel:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Some SoCs need to set appropriate stop-dly-sel to configure
> read data clock
> > +      stops at block gap. The valid range is from 0 to 0xf.
> 
> SoC dependent or board dependent? Imply from the compatible for the 
> former. A property is fine for the latter case.
Yes, it is SoC dependent in principle, but we would like to use a
unified compatible string for MT8196 and subsequent SoCs as mentioned
above, and this register setting(not relavant with new tx/rx) may be
variant. Therefore, want to use the property of device tree to set
individually.
> 
> > +    minimum: 0
> > +    maximum: 0xf
> > +
> > +  mediatek,pop-en-cnt:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Some SoCs need to set appropriate pop-en-cnt to configure
> the margins of write
> > +      and read pointers while begin to pop data transfer. The
> valid range is from 0
> > +      to 0xf.
> > +    minimum: 0
> > +    maximum: 0xf
> 
> Same question.
Same as the reply above.
> 
> > +
> > +  mediatek,prohibit-gate-cg:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description:
> > +      Decide if source clock CG could be disabled when CPU access
> IP registers.
> > +      If present, source clock CG could not be disabled.
> > +      If not present, source clock CG could be disabled.
> 
> 
> Sounds like you need to describe the clock in "clocks".
It is not with clock itself, but rather with the bus design that would
check the 'source_cg' status bit to decide whether to prevent the CPU
from accessing the IP registers. And there have been no changes to the
'source_cg'(already been descirbed before) for MT8196.
> 
> > +
> >    resets:
> >      maxItems: 1
> >  
> > @@ -191,6 +216,7 @@ allOf:
> >              - mediatek,mt8188-mmc
> >              - mediatek,mt8195-mmc
> >              - mediatek,mt8516-mmc
> > +            - mediatek,msdc-v2
> >      then:
> >        properties:
> >          clocks:
> > -- 
> > 2.46.0
> > 

Re: [PATCH 2/2] dt-bindings: mmc: mtk-sd: Add support for MT8196

[Krzysztof Kozlowski]

On 28/09/2024 15:14, Andy-ld Lu (卢东) wrote:
> On Thu, 2024-09-26 at 12:54 -0500, Rob Herring wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  On Thu, Sep 26, 2024 at 03:03:18PM +0800, Andy-ld Lu wrote:
>>> Extend the devicetree bindings to include the MT8196 mmc controller
>>> by adding the compatible string 'mediatek,msdc-v2', which could be
>>> also used for future compatible SoCs that support new tx/rx.
>>
>> Generally, every SoC ends up changing at least slightly. So we don't
>> do 
>> version numbers except when there's a well defined versioning scheme
>> of 
>> the h/w (e.g. FPGA IP blocks). So, use SoC for compatible string.
> Thanks for your review.
> 
> The new tx/rx represents a significant update for mmc controller of
> mediatek from the MT8196 chipset, and the dependent settings remain
> consistent for MT8196 and subsequent SoCs. Therefore, It is proposed to
> use a unified compatible string for these SoCs that support new tx/rx,
> and name it as 'IP(msdc) version two'.

Sorry, nope. That's not valid reasons.

>>
>>>
>>> Add three properties for MT8196 settings:
>>> - 'mediatek,prohibit-gate-cg', indicate if the source clock CG
>> could
>>>   be disabled when CPU access IP registers.
>>>
>>> - 'mediatek,stop-dly-sel', configure read data clock stops at block
>> gap.
>>>
>>> - 'mediatek,pop-en-cnt', configure the margins of write and read
>>>   pointers while begin to pop data transfer.
>>>
>>> Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
>>> ---
>>>  .../devicetree/bindings/mmc/mtk-sd.yaml       | 26
>> +++++++++++++++++++
>>>  1 file changed, 26 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>> b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>>> index c532ec92d2d9..82d1a9fac67c 100644
>>> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
>>> @@ -25,6 +25,7 @@ properties:
>>>            - mediatek,mt8173-mmc
>>>            - mediatek,mt8183-mmc
>>>            - mediatek,mt8516-mmc
>>> +          - mediatek,msdc-v2
>>>        - items:
>>>            - const: mediatek,mt7623-mmc
>>>            - const: mediatek,mt2701-mmc
>>> @@ -154,6 +155,30 @@ properties:
>>>      enum: [32, 64]
>>>      default: 32
>>>  
>>> +  mediatek,stop-dly-sel:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description:
>>> +      Some SoCs need to set appropriate stop-dly-sel to configure
>> read data clock
>>> +      stops at block gap. The valid range is from 0 to 0xf.
>>
>> SoC dependent or board dependent? Imply from the compatible for the 
>> former. A property is fine for the latter case.
> Yes, it is SoC dependent in principle, but we would like to use a
> unified compatible string for MT8196 and subsequent SoCs as mentioned

Also no :(. Use SoC specific compatible.

> above, and this register setting(not relavant with new tx/rx) may be
> variant. Therefore, want to use the property of device tree to set
> individually.
>>
>>> +    minimum: 0
>>> +    maximum: 0xf
>>> +
>>> +  mediatek,pop-en-cnt:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    description:
>>> +      Some SoCs need to set appropriate pop-en-cnt to configure
>> the margins of write
>>> +      and read pointers while begin to pop data transfer. The
>> valid range is from 0
>>> +      to 0xf.
>>> +    minimum: 0
>>> +    maximum: 0xf
>>
>> Same question.
> Same as the reply above.
>>
>>> +
>>> +  mediatek,prohibit-gate-cg:
>>> +    $ref: /schemas/types.yaml#/definitions/flag
>>> +    description:
>>> +      Decide if source clock CG could be disabled when CPU access
>> IP registers.
>>> +      If present, source clock CG could not be disabled.
>>> +      If not present, source clock CG could be disabled.
>>
>>
>> Sounds like you need to describe the clock in "clocks".
> It is not with clock itself, but rather with the bus design that would
> check the 'source_cg' status bit to decide whether to prevent the CPU
> from accessing the IP registers. And there have been no changes to the
> 'source_cg'(already been descirbed before) for MT8196.

I don't understand why do you need this property if it was already
described. Please do not describe things twice.


Best regards,
Krzysztof

Re: [PATCH 2/2] dt-bindings: mmc: mtk-sd: Add support for MT8196

[Andy-ld Lu (卢东)]

On Sat, 2024-09-28 at 15:39 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 28/09/2024 15:14, Andy-ld Lu (卢东) wrote:
> > On Thu, 2024-09-26 at 12:54 -0500, Rob Herring wrote:
> >>   
> >> External email : Please do not click links or open attachments
> until
> >> you have verified the sender or the content.
> >>  On Thu, Sep 26, 2024 at 03:03:18PM +0800, Andy-ld Lu wrote:
> >>> Extend the devicetree bindings to include the MT8196 mmc
> controller
> >>> by adding the compatible string 'mediatek,msdc-v2', which could
> be
> >>> also used for future compatible SoCs that support new tx/rx.
> >>
> >> Generally, every SoC ends up changing at least slightly. So we
> don't
> >> do 
> >> version numbers except when there's a well defined versioning
> scheme
> >> of 
> >> the h/w (e.g. FPGA IP blocks). So, use SoC for compatible string.
> > Thanks for your review.
> > 
> > The new tx/rx represents a significant update for mmc controller of
> > mediatek from the MT8196 chipset, and the dependent settings remain
> > consistent for MT8196 and subsequent SoCs. Therefore, It is
> proposed to
> > use a unified compatible string for these SoCs that support new
> tx/rx,
> > and name it as 'IP(msdc) version two'.
> 
> Sorry, nope. That's not valid reasons.

Thanks for your review.

I will follow your comment in next change.

> 
> >>
> >>>
> >>> Add three properties for MT8196 settings:
> >>> - 'mediatek,prohibit-gate-cg', indicate if the source clock CG
> >> could
> >>>   be disabled when CPU access IP registers.
> >>>
> >>> - 'mediatek,stop-dly-sel', configure read data clock stops at
> block
> >> gap.
> >>>
> >>> - 'mediatek,pop-en-cnt', configure the margins of write and read
> >>>   pointers while begin to pop data transfer.
> >>>
> >>> Signed-off-by: Andy-ld Lu <andy-ld.lu@mediatek.com>
> >>> ---
> >>>  .../devicetree/bindings/mmc/mtk-sd.yaml       | 26
> >> +++++++++++++++++++
> >>>  1 file changed, 26 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> >> b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> >>> index c532ec92d2d9..82d1a9fac67c 100644
> >>> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> >>> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> >>> @@ -25,6 +25,7 @@ properties:
> >>>            - mediatek,mt8173-mmc
> >>>            - mediatek,mt8183-mmc
> >>>            - mediatek,mt8516-mmc
> >>> +          - mediatek,msdc-v2
> >>>        - items:
> >>>            - const: mediatek,mt7623-mmc
> >>>            - const: mediatek,mt2701-mmc
> >>> @@ -154,6 +155,30 @@ properties:
> >>>      enum: [32, 64]
> >>>      default: 32
> >>>  
> >>> +  mediatek,stop-dly-sel:
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    description:
> >>> +      Some SoCs need to set appropriate stop-dly-sel to
> configure
> >> read data clock
> >>> +      stops at block gap. The valid range is from 0 to 0xf.
> >>
> >> SoC dependent or board dependent? Imply from the compatible for
> the 
> >> former. A property is fine for the latter case.
> > Yes, it is SoC dependent in principle, but we would like to use a
> > unified compatible string for MT8196 and subsequent SoCs as
> mentioned
> 
> Also no :(. Use SoC specific compatible.

I will follow your comment in next change.

> 
> > above, and this register setting(not relavant with new tx/rx) may
> be
> > variant. Therefore, want to use the property of device tree to set
> > individually.
> >>
> >>> +    minimum: 0
> >>> +    maximum: 0xf
> >>> +
> >>> +  mediatek,pop-en-cnt:
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    description:
> >>> +      Some SoCs need to set appropriate pop-en-cnt to configure
> >> the margins of write
> >>> +      and read pointers while begin to pop data transfer. The
> >> valid range is from 0
> >>> +      to 0xf.
> >>> +    minimum: 0
> >>> +    maximum: 0xf
> >>
> >> Same question.
> > Same as the reply above.
> >>
> >>> +
> >>> +  mediatek,prohibit-gate-cg:
> >>> +    $ref: /schemas/types.yaml#/definitions/flag
> >>> +    description:
> >>> +      Decide if source clock CG could be disabled when CPU
> access
> >> IP registers.
> >>> +      If present, source clock CG could not be disabled.
> >>> +      If not present, source clock CG could be disabled.
> >>
> >>
> >> Sounds like you need to describe the clock in "clocks".
> > It is not with clock itself, but rather with the bus design that
> would
> > check the 'source_cg' status bit to decide whether to prevent the
> CPU
> > from accessing the IP registers. And there have been no changes to
> the
> > 'source_cg'(already been descirbed before) for MT8196.
> 
> I don't understand why do you need this property if it was already
> described. Please do not describe things twice.

I will follow your comment in next change.

> 
> 
> Best regards,
> Krzysztof
>