[PATCH RFC 0/3] mmc: add support for the dw-mmc controller on Hi3798MV200

[PATCH RFC 0/3] mmc: add support for the dw-mmc controller on Hi3798MV200

[Yang Xiwen via B4 Relay]

The dw-mmc controller found on Hi3798MV200 is like the one found on
Hi3798CV200, but has some tweaks.
Also refreshed the dt binding and converted it to YAML.
Unfortunately, DDR52 is not supported yet.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
Yang Xiwen (3):
      mmc: dw_mmc-hi3798cv200: rename to dw_mmc-histb
      mmc: dw_mmc-histb: add support for hi3798mv200
      binding: mmc: hi3798cv200-dw-mshc: convert to YAML and rename to histb-dw-mshc, add compatible of hi3798mv200

 .../bindings/mmc/hi3798cv200-dw-mshc.txt           |  40 ---
 .../devicetree/bindings/mmc/histb-dw-mshc.yaml     |  90 ++++++
 drivers/mmc/host/Kconfig                           |   8 +-
 drivers/mmc/host/Makefile                          |   2 +-
 drivers/mmc/host/dw_mmc-hi3798cv200.c              | 206 -------------
 drivers/mmc/host/dw_mmc-histb.c                    | 336 +++++++++++++++++++++
 6 files changed, 431 insertions(+), 251 deletions(-)
---
base-commit: 76f598ba7d8e2bfb4855b5298caedd5af0c374a8
change-id: 20230415-mmc-hi3798mv200-ce15e9b96866

Best regards,
-- 
Yang Xiwen <forbidden405@outlook.com>

[PATCH RFC 1/3] mmc: dw_mmc-hi3798cv200: rename to dw_mmc-histb

[Yang Xiwen via B4 Relay]

From: Yang Xiwen <forbidden405@outlook.com>

Rename to dw_mmc-histb and introduce a mechanism similar to
dw-mmc_exynos to support more devices in a single driver. It is a
preparation for introducing extension for Hi3798MV200.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
 drivers/mmc/host/Kconfig                           |  8 +--
 drivers/mmc/host/Makefile                          |  2 +-
 .../host/{dw_mmc-hi3798cv200.c => dw_mmc-histb.c}  | 79 ++++++++++++++--------
 3 files changed, 57 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 4745fe217ade3..0aef4d845b743 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -779,14 +779,14 @@ config MMC_DW_EXYNOS
 	  Synopsys DesignWare Memory Card Interface driver. Select this option
 	  for platforms based on Exynos4 and Exynos5 SoC's.
 
-config MMC_DW_HI3798CV200
-	tristate "Hi3798CV200 specific extensions for Synopsys DW Memory Card Interface"
+config MMC_DW_HISTB
+	tristate "HiSTB specific extensions for Synopsys DW Memory Card Interface"
 	depends on MMC_DW
 	select MMC_DW_PLTFM
 	help
-	  This selects support for HiSilicon Hi3798CV200 SoC specific extensions to the
+	  This selects support for HiSilicon HiSTB SoC specific extensions to the
 	  Synopsys DesignWare Memory Card Interface driver. Select this option
-	  for platforms based on HiSilicon Hi3798CV200 SoC.
+	  for platforms based on HiSilicon HiSTB SoC.
 
 config MMC_DW_K3
 	tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index a693fa3d3f1cc..0373741afebf1 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -50,7 +50,7 @@ obj-$(CONFIG_MMC_DW)		+= dw_mmc.o
 obj-$(CONFIG_MMC_DW_PLTFM)	+= dw_mmc-pltfm.o
 obj-$(CONFIG_MMC_DW_BLUEFIELD)	+= dw_mmc-bluefield.o
 obj-$(CONFIG_MMC_DW_EXYNOS)	+= dw_mmc-exynos.o
-obj-$(CONFIG_MMC_DW_HI3798CV200) += dw_mmc-hi3798cv200.o
+obj-$(CONFIG_MMC_DW_HISTB) += dw_mmc-histb.o
 obj-$(CONFIG_MMC_DW_K3)		+= dw_mmc-k3.o
 obj-$(CONFIG_MMC_DW_PCI)	+= dw_mmc-pci.o
 obj-$(CONFIG_MMC_DW_ROCKCHIP)	+= dw_mmc-rockchip.o
diff --git a/drivers/mmc/host/dw_mmc-hi3798cv200.c b/drivers/mmc/host/dw_mmc-histb.c
similarity index 69%
rename from drivers/mmc/host/dw_mmc-hi3798cv200.c
rename to drivers/mmc/host/dw_mmc-histb.c
index 6f22fe0540879..106e586bcff4b 100644
--- a/drivers/mmc/host/dw_mmc-hi3798cv200.c
+++ b/drivers/mmc/host/dw_mmc-histb.c
@@ -18,14 +18,29 @@
 
 #define ALL_INT_CLR		0x1ffff
 
-struct hi3798cv200_priv {
+enum dw_mci_histb_type {
+	DW_MCI_TYPE_HI3798CV200,
+};
+
+static struct dw_mci_histb_compat {
+	const char * const compatible;
+	enum dw_mci_histb_type ctrl_type;
+} histb_compat[] = {
+	{
+		.compatible = "hisilicon,hi3798cv200-dw-mshc",
+		.ctrl_type = DW_MCI_TYPE_HI3798CV200,
+	},
+};
+
+struct dw_mci_histb_priv {
+	enum dw_mci_histb_type ctrl_type;
 	struct clk *sample_clk;
 	struct clk *drive_clk;
 };
 
-static void dw_mci_hi3798cv200_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+static void dw_mci_histb_set_ios(struct dw_mci *host, struct mmc_ios *ios)
 {
-	struct hi3798cv200_priv *priv = host->priv;
+	struct dw_mci_histb_priv *priv = host->priv;
 	u32 val;
 
 	val = mci_readl(host, UHS_REG);
@@ -62,7 +77,7 @@ static int dw_mci_hi3798cv200_execute_tuning(struct dw_mci_slot *slot,
 {
 	static const int degrees[] = { 0, 45, 90, 135, 180, 225, 270, 315 };
 	struct dw_mci *host = slot->host;
-	struct hi3798cv200_priv *priv = host->priv;
+	struct dw_mci_histb_priv *priv = host->priv;
 	int raise_point = -1, fall_point = -1;
 	int err, prev_err = -1;
 	int found = 0;
@@ -118,15 +133,21 @@ static int dw_mci_hi3798cv200_execute_tuning(struct dw_mci_slot *slot,
 	return err;
 }
 
-static int dw_mci_hi3798cv200_init(struct dw_mci *host)
+static int dw_mci_histb_init(struct dw_mci *host)
 {
-	struct hi3798cv200_priv *priv;
-	int ret;
+	struct dw_mci_histb_priv *priv;
+	struct device_node *np = host->dev->of_node;
+	int ret, idx;
 
 	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
 
+	for (idx = 0; idx < ARRAY_SIZE(histb_compat); idx++) {
+		if (of_device_is_compatible(np, histb_compat[idx].compatible))
+			priv->ctrl_type = histb_compat[idx].ctrl_type;
+	}
+
 	priv->sample_clk = devm_clk_get(host->dev, "ciu-sample");
 	if (IS_ERR(priv->sample_clk)) {
 		dev_err(host->dev, "failed to get ciu-sample clock\n");
@@ -161,20 +182,29 @@ static int dw_mci_hi3798cv200_init(struct dw_mci *host)
 
 static const struct dw_mci_drv_data hi3798cv200_data = {
 	.common_caps = MMC_CAP_CMD23,
-	.init = dw_mci_hi3798cv200_init,
-	.set_ios = dw_mci_hi3798cv200_set_ios,
+	.init = dw_mci_histb_init,
+	.set_ios = dw_mci_histb_set_ios,
 	.execute_tuning = dw_mci_hi3798cv200_execute_tuning,
 };
 
-static int dw_mci_hi3798cv200_probe(struct platform_device *pdev)
+static const struct of_device_id dw_mci_histb_match[] = {
+	{ .compatible = "hisilicon,hi3798cv200-dw-mshc", .data = &hi3798cv200_data },
+	{},
+};
+
+static int dw_mci_histb_probe(struct platform_device *pdev)
 {
-	return dw_mci_pltfm_register(pdev, &hi3798cv200_data);
+	const struct of_device_id *match;
+
+	match = of_match_node(dw_mci_histb_match, pdev->dev.of_node);
+
+	return dw_mci_pltfm_register(pdev, match->data);
 }
 
-static int dw_mci_hi3798cv200_remove(struct platform_device *pdev)
+static int dw_mci_histb_remove(struct platform_device *pdev)
 {
 	struct dw_mci *host = platform_get_drvdata(pdev);
-	struct hi3798cv200_priv *priv = host->priv;
+	struct dw_mci_histb_priv *priv = host->priv;
 
 	clk_disable_unprepare(priv->drive_clk);
 	clk_disable_unprepare(priv->sample_clk);
@@ -184,23 +214,18 @@ static int dw_mci_hi3798cv200_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id dw_mci_hi3798cv200_match[] = {
-	{ .compatible = "hisilicon,hi3798cv200-dw-mshc", },
-	{},
-};
-
-MODULE_DEVICE_TABLE(of, dw_mci_hi3798cv200_match);
-static struct platform_driver dw_mci_hi3798cv200_driver = {
-	.probe = dw_mci_hi3798cv200_probe,
-	.remove = dw_mci_hi3798cv200_remove,
+MODULE_DEVICE_TABLE(of, dw_mci_histb_match);
+static struct platform_driver dw_mci_histb_driver = {
+	.probe = dw_mci_histb_probe,
+	.remove = dw_mci_histb_remove,
 	.driver = {
-		.name = "dwmmc_hi3798cv200",
+		.name = "dwmmc_histb",
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
-		.of_match_table = dw_mci_hi3798cv200_match,
+		.of_match_table = dw_mci_histb_match,
 	},
 };
-module_platform_driver(dw_mci_hi3798cv200_driver);
+module_platform_driver(dw_mci_histb_driver);
 
-MODULE_DESCRIPTION("HiSilicon Hi3798CV200 Specific DW-MSHC Driver Extension");
+MODULE_DESCRIPTION("HiSilicon HiSTB Specific DW-MSHC Driver Extension");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:dwmmc_hi3798cv200");
+MODULE_ALIAS("platform:dwmmc_histb");

-- 
2.39.2

[PATCH RFC 2/3] mmc: dw_mmc-histb: add support for hi3798mv200

[Yang Xiwen via B4 Relay]

From: Yang Xiwen <forbidden405@outlook.com>

Add support for Hi3798MV200 specific extension.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
 drivers/mmc/host/dw_mmc-histb.c | 105 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc-histb.c b/drivers/mmc/host/dw_mmc-histb.c
index 106e586bcff4b..9c6fd78009668 100644
--- a/drivers/mmc/host/dw_mmc-histb.c
+++ b/drivers/mmc/host/dw_mmc-histb.c
@@ -16,10 +16,14 @@
 #include "dw_mmc.h"
 #include "dw_mmc-pltfm.h"
 
+#define SDMMC_TUNING_CTRL	0x118
+#define SDMMC_TUNING_FIND_EDGE	BIT(5)
+
 #define ALL_INT_CLR		0x1ffff
 
 enum dw_mci_histb_type {
 	DW_MCI_TYPE_HI3798CV200,
+	DW_MCI_TYPE_HI3798MV200,
 };
 
 static struct dw_mci_histb_compat {
@@ -29,6 +33,9 @@ static struct dw_mci_histb_compat {
 	{
 		.compatible = "hisilicon,hi3798cv200-dw-mshc",
 		.ctrl_type = DW_MCI_TYPE_HI3798CV200,
+	}, {
+		.compatible = "hisilicon,hi3798mv200-dw-mshc",
+		.ctrl_type = DW_MCI_TYPE_HI3798MV200,
 	},
 };
 
@@ -36,6 +43,7 @@ struct dw_mci_histb_priv {
 	enum dw_mci_histb_type ctrl_type;
 	struct clk *sample_clk;
 	struct clk *drive_clk;
+	struct clk *sap_dll_mode_clk;
 };
 
 static void dw_mci_histb_set_ios(struct dw_mci *host, struct mmc_ios *ios)
@@ -133,6 +141,75 @@ static int dw_mci_hi3798cv200_execute_tuning(struct dw_mci_slot *slot,
 	return err;
 }
 
+static int dw_mci_hi3798mv200_execute_tuning_mix_mode(struct dw_mci_slot *slot,
+					     u32 opcode)
+{
+	static const int degrees[] = { 0, 45, 90, 135, 180, 225, 270, 315 };
+	struct dw_mci *host = slot->host;
+	struct dw_mci_histb_priv *priv = host->priv;
+	int raise_point = -1, fall_point = -1;
+	int err, prev_err = -1;
+	int found = 0;
+	int regval;
+	int i;
+
+	clk_disable(priv->sap_dll_mode_clk);
+	for (i = 0; i < ARRAY_SIZE(degrees); i++) {
+		clk_set_phase(priv->sample_clk, degrees[i]);
+		mci_writel(host, RINTSTS, ALL_INT_CLR);
+
+		err = mmc_send_tuning(slot->mmc, opcode, NULL);
+		if (err)
+			found = 1;
+		else {
+			regval = mci_readl(host, TUNING_CTRL);
+			if (regval & SDMMC_TUNING_FIND_EDGE)
+				found = 1;
+		};
+
+		if (i > 0) {
+			if (err && !prev_err)
+				fall_point = i - 1;
+			if (!err && prev_err)
+				raise_point = i;
+		}
+
+		if (raise_point != -1 && fall_point != -1)
+			goto tuning_out;
+
+		prev_err = err;
+		err = 0;
+	}
+
+tuning_out:
+	clk_enable(priv->sap_dll_mode_clk);
+	if (found) {
+		if (raise_point == -1)
+			raise_point = 0;
+		if (fall_point == -1)
+			fall_point = ARRAY_SIZE(degrees) - 1;
+		if (fall_point < raise_point) {
+			if ((raise_point + fall_point) >
+			    (ARRAY_SIZE(degrees) - 1))
+				i = fall_point / 2;
+			else
+				i = (raise_point + ARRAY_SIZE(degrees) - 1) / 2;
+		} else {
+			i = (raise_point + fall_point) / 2;
+		}
+
+		clk_set_phase(priv->sample_clk, degrees[i]);
+		dev_dbg(host->dev, "Tuning clk_sample[%d, %d], set[%d]\n",
+			raise_point, fall_point, degrees[i]);
+	} else {
+		dev_err(host->dev, "No valid clk_sample shift! use default\n");
+		err = -EINVAL;
+	}
+
+	mci_writel(host, RINTSTS, ALL_INT_CLR);
+	return err;
+}
+
 static int dw_mci_histb_init(struct dw_mci *host)
 {
 	struct dw_mci_histb_priv *priv;
@@ -160,6 +237,14 @@ static int dw_mci_histb_init(struct dw_mci *host)
 		return PTR_ERR(priv->drive_clk);
 	}
 
+	if (priv->ctrl_type == DW_MCI_TYPE_HI3798MV200) {
+		priv->sap_dll_mode_clk = devm_clk_get(host->dev, "sap-dll-mode");
+		if (IS_ERR(priv->sap_dll_mode_clk)) {
+			dev_err(host->dev, "failed to get sap-dll-mode clock\n");
+			return PTR_ERR(priv->sap_dll_mode_clk);
+		}
+	}
+
 	ret = clk_prepare_enable(priv->sample_clk);
 	if (ret) {
 		dev_err(host->dev, "failed to enable ciu-sample clock\n");
@@ -172,9 +257,19 @@ static int dw_mci_histb_init(struct dw_mci *host)
 		goto disable_sample_clk;
 	}
 
+	if (priv->ctrl_type == DW_MCI_TYPE_HI3798MV200) {
+		ret = clk_prepare_enable(priv->sap_dll_mode_clk);
+		if (ret) {
+			dev_err(host->dev, "failed to disable tuning mode");
+			goto disable_drive_clk;
+		}
+	}
+
 	host->priv = priv;
 	return 0;
 
+disable_drive_clk:
+	clk_disable_unprepare(priv->drive_clk);
 disable_sample_clk:
 	clk_disable_unprepare(priv->sample_clk);
 	return ret;
@@ -187,8 +282,16 @@ static const struct dw_mci_drv_data hi3798cv200_data = {
 	.execute_tuning = dw_mci_hi3798cv200_execute_tuning,
 };
 
+static const struct dw_mci_drv_data hi3798mv200_data = {
+	.common_caps = MMC_CAP_CMD23,
+	.init = dw_mci_histb_init,
+	.set_ios = dw_mci_histb_set_ios,
+	.execute_tuning = dw_mci_hi3798mv200_execute_tuning_mix_mode,
+};
+
 static const struct of_device_id dw_mci_histb_match[] = {
 	{ .compatible = "hisilicon,hi3798cv200-dw-mshc", .data = &hi3798cv200_data },
+	{ .compatible = "hisilicon,hi3798mv200-dw-mshc", .data = &hi3798mv200_data },
 	{},
 };
 
@@ -208,6 +311,8 @@ static int dw_mci_histb_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(priv->drive_clk);
 	clk_disable_unprepare(priv->sample_clk);
+	if (priv->ctrl_type == DW_MCI_TYPE_HI3798MV200)
+		clk_disable_unprepare(priv->sap_dll_mode_clk);
 
 	dw_mci_pltfm_remove(pdev);
 

-- 
2.39.2

[PATCH RFC 3/3] binding: mmc: hi3798cv200-dw-mshc: convert to YAML and rename to histb-dw-mshc, add compatible of hi3798mv200

[Yang Xiwen via B4 Relay]

From: Yang Xiwen <forbidden405@outlook.com>

Hi3798MV200 has an extra clock, also document it here.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
 .../bindings/mmc/hi3798cv200-dw-mshc.txt           | 40 ----------
 .../devicetree/bindings/mmc/histb-dw-mshc.yaml     | 90 ++++++++++++++++++++++
 2 files changed, 90 insertions(+), 40 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt
deleted file mode 100644
index a0693b7145f2a..0000000000000
--- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-* Hisilicon Hi3798CV200 specific extensions to the Synopsys Designware Mobile
-  Storage Host Controller
-
-Read synopsys-dw-mshc.txt for more details
-
-The Synopsys designware mobile storage host controller is used to interface
-a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
-differences between the core Synopsys dw mshc controller properties described
-by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
-specific extensions to the Synopsys Designware Mobile Storage Host Controller.
-
-Required Properties:
-- compatible: Should contain "hisilicon,hi3798cv200-dw-mshc".
-- clocks: A list of phandle + clock-specifier pairs for the clocks listed
-  in clock-names.
-- clock-names: Should contain the following:
-	"ciu" - The ciu clock described in synopsys-dw-mshc.txt.
-	"biu" - The biu clock described in synopsys-dw-mshc.txt.
-	"ciu-sample" - Hi3798CV200 extended phase clock for ciu sampling.
-	"ciu-drive"  - Hi3798CV200 extended phase clock for ciu driving.
-
-Example:
-
-	emmc: mmc@9830000 {
-		compatible = "hisilicon,hi3798cv200-dw-mshc";
-		reg = <0x9830000 0x10000>;
-		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&crg HISTB_MMC_CIU_CLK>,
-			 <&crg HISTB_MMC_BIU_CLK>,
-			 <&crg HISTB_MMC_SAMPLE_CLK>,
-			 <&crg HISTB_MMC_DRV_CLK>;
-		clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
-		fifo-depth = <256>;
-		clock-frequency = <200000000>;
-		cap-mmc-highspeed;
-		mmc-ddr-1_8v;
-		mmc-hs200-1_8v;
-		non-removable;
-		bus-width = <8>;
-	};
diff --git a/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
new file mode 100644
index 0000000000000..05a435185e9da
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title:
+  Hisilicon Hi3798CV200 specific extensions to the Synopsys Designware Mobile
+  Storage Host Controller
+
+maintainers:
+  - Yang Xiwen <forbidden405@outlook.com>
+
+description:
+  The Synopsys designware mobile storage host controller is used to interface
+  a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
+  differences between the core Synopsys dw mshc controller properties described
+  by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
+  specific extensions to the Synopsys Designware Mobile Storage Host Controller.
+
+allOf:
+  - $ref: "synopsys-dw-mshc-common.yaml#"
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: hisilicon,hi3798mv200-dw-mshc
+    then:
+      properties:
+        clocks:
+          minItems: 5
+
+        clock-names:
+          minItems: 5
+
+properties:
+  compatible:
+    enum:
+      - hisilicon,hi3798cv200-dw-mshc
+      - hisilicon,hi3798mv200-dw-mshc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 4
+    maxItems: 5
+    description: A list of phandles for the clocks listed in clock-names
+
+  clock-names:
+    minItems: 4
+    items:
+      - const: ciu
+      - const: biu
+      - const: ciu-sample
+      - const: ciu-drive
+      - const: sap-dll-mode
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    emmc: mmc@9830000 {
+            compatible = "hisilicon,hi3798cv200-dw-mshc";
+            reg = <0x9830000 0x10000>;
+            interrupts = <35>;
+            clocks = <&crg 1>,
+                     <&crg 2>,
+                     <&crg 3>,
+                     <&crg 4>;
+            clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
+            fifo-depth = <256>;
+            clock-frequency = <200000000>;
+            cap-mmc-highspeed;
+            mmc-ddr-1_8v;
+            mmc-hs200-1_8v;
+            non-removable;
+            bus-width = <8>;
+    };
+

-- 
2.39.2

Re: [PATCH RFC 3/3] binding: mmc: hi3798cv200-dw-mshc: convert to YAML and rename to histb-dw-mshc, add compatible of hi3798mv200

[Krzysztof Kozlowski]

On 15/04/2023 16:18, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
> 
> Hi3798MV200 has an extra clock, also document it here.

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).


Subject: it is way too long. It should be just conversion. Don't
introduce new compatibles.

> 
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> ---
>  .../bindings/mmc/hi3798cv200-dw-mshc.txt           | 40 ----------
>  .../devicetree/bindings/mmc/histb-dw-mshc.yaml     | 90 ++++++++++++++++++++++
>  2 files changed, 90 insertions(+), 40 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt
> deleted file mode 100644
> index a0693b7145f2a..0000000000000
> --- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -* Hisilicon Hi3798CV200 specific extensions to the Synopsys Designware Mobile
> -  Storage Host Controller
> -
> -Read synopsys-dw-mshc.txt for more details
> -
> -The Synopsys designware mobile storage host controller is used to interface
> -a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
> -differences between the core Synopsys dw mshc controller properties described
> -by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
> -specific extensions to the Synopsys Designware Mobile Storage Host Controller.
> -
> -Required Properties:
> -- compatible: Should contain "hisilicon,hi3798cv200-dw-mshc".
> -- clocks: A list of phandle + clock-specifier pairs for the clocks listed
> -  in clock-names.
> -- clock-names: Should contain the following:
> -	"ciu" - The ciu clock described in synopsys-dw-mshc.txt.
> -	"biu" - The biu clock described in synopsys-dw-mshc.txt.
> -	"ciu-sample" - Hi3798CV200 extended phase clock for ciu sampling.
> -	"ciu-drive"  - Hi3798CV200 extended phase clock for ciu driving.
> -
> -Example:
> -
> -	emmc: mmc@9830000 {
> -		compatible = "hisilicon,hi3798cv200-dw-mshc";
> -		reg = <0x9830000 0x10000>;
> -		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> -		clocks = <&crg HISTB_MMC_CIU_CLK>,
> -			 <&crg HISTB_MMC_BIU_CLK>,
> -			 <&crg HISTB_MMC_SAMPLE_CLK>,
> -			 <&crg HISTB_MMC_DRV_CLK>;
> -		clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
> -		fifo-depth = <256>;
> -		clock-frequency = <200000000>;
> -		cap-mmc-highspeed;
> -		mmc-ddr-1_8v;
> -		mmc-hs200-1_8v;
> -		non-removable;
> -		bus-width = <8>;
> -	};
> diff --git a/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
> new file mode 100644
> index 0000000000000..05a435185e9da
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title:
> +  Hisilicon Hi3798CV200 specific extensions to the Synopsys Designware Mobile
> +  Storage Host Controller
> +
> +maintainers:
> +  - Yang Xiwen <forbidden405@outlook.com>
> +
> +description:
> +  The Synopsys designware mobile storage host controller is used to interface
> +  a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
> +  differences between the core Synopsys dw mshc controller properties described
> +  by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
> +  specific extensions to the Synopsys Designware Mobile Storage Host Controller.

Drop the last sentence.

> +
> +allOf:
> +  - $ref: "synopsys-dw-mshc-common.yaml#"

Drop quotes.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: hisilicon,hi3798mv200-dw-mshc
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 5
> +
> +        clock-names:
> +          minItems: 5

Entire allOf goes after required:.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - hisilicon,hi3798cv200-dw-mshc
> +      - hisilicon,hi3798mv200-dw-mshc

New compatibles are not related to conversion.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 4
> +    maxItems: 5
> +    description: A list of phandles for the clocks listed in clock-names

Drop description.

> +
> +  clock-names:
> +    minItems: 4
> +    items:
> +      - const: ciu
> +      - const: biu
> +      - const: ciu-sample
> +      - const: ciu-drive
> +      - const: sap-dll-mode

Only four clocks were in original document. Don't mix different changes
in one patch.

> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  - |
> +    emmc: mmc@9830000 {
> +            compatible = "hisilicon,hi3798cv200-dw-mshc";
> +            reg = <0x9830000 0x10000>;

Wrong indentation. Use 4 spaces for example indentation.

> +            interrupts = <35>;
> +            clocks = <&crg 1>,

> +
> 

Best regards,
Krzysztof

Re: [PATCH RFC 3/3] binding: mmc: hi3798cv200-dw-mshc: convert to YAML and rename to histb-dw-mshc, add compatible of hi3798mv200

[forbidden405]

On 4/16/2023 3:26 PM, Krzysztof Kozlowski wrote:
> On 15/04/2023 16:18, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
>>
>> Hi3798MV200 has an extra clock, also document it here.
> 
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
> 
> 
> Subject: it is way too long. It should be just conversion. Don't
> introduce new compatibles.
> 
>>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>> ---
>>  .../bindings/mmc/hi3798cv200-dw-mshc.txt           | 40 ----------
>>  .../devicetree/bindings/mmc/histb-dw-mshc.yaml     | 90 ++++++++++++++++++++++
>>  2 files changed, 90 insertions(+), 40 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt
>> deleted file mode 100644
>> index a0693b7145f2a..0000000000000
>> --- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.txt
>> +++ /dev/null
>> @@ -1,40 +0,0 @@
>> -* Hisilicon Hi3798CV200 specific extensions to the Synopsys Designware Mobile
>> -  Storage Host Controller
>> -
>> -Read synopsys-dw-mshc.txt for more details
>> -
>> -The Synopsys designware mobile storage host controller is used to interface
>> -a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
>> -differences between the core Synopsys dw mshc controller properties described
>> -by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
>> -specific extensions to the Synopsys Designware Mobile Storage Host Controller.
>> -
>> -Required Properties:
>> -- compatible: Should contain "hisilicon,hi3798cv200-dw-mshc".
>> -- clocks: A list of phandle + clock-specifier pairs for the clocks listed
>> -  in clock-names.
>> -- clock-names: Should contain the following:
>> -	"ciu" - The ciu clock described in synopsys-dw-mshc.txt.
>> -	"biu" - The biu clock described in synopsys-dw-mshc.txt.
>> -	"ciu-sample" - Hi3798CV200 extended phase clock for ciu sampling.
>> -	"ciu-drive"  - Hi3798CV200 extended phase clock for ciu driving.
>> -
>> -Example:
>> -
>> -	emmc: mmc@9830000 {
>> -		compatible = "hisilicon,hi3798cv200-dw-mshc";
>> -		reg = <0x9830000 0x10000>;
>> -		interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>> -		clocks = <&crg HISTB_MMC_CIU_CLK>,
>> -			 <&crg HISTB_MMC_BIU_CLK>,
>> -			 <&crg HISTB_MMC_SAMPLE_CLK>,
>> -			 <&crg HISTB_MMC_DRV_CLK>;
>> -		clock-names = "ciu", "biu", "ciu-sample", "ciu-drive";
>> -		fifo-depth = <256>;
>> -		clock-frequency = <200000000>;
>> -		cap-mmc-highspeed;
>> -		mmc-ddr-1_8v;
>> -		mmc-hs200-1_8v;
>> -		non-removable;
>> -		bus-width = <8>;
>> -	};
>> diff --git a/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
>> new file mode 100644
>> index 0000000000000..05a435185e9da
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
>> @@ -0,0 +1,90 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title:
>> +  Hisilicon Hi3798CV200 specific extensions to the Synopsys Designware Mobile
>> +  Storage Host Controller
>> +
>> +maintainers:
>> +  - Yang Xiwen <forbidden405@outlook.com>
>> +
>> +description:
>> +  The Synopsys designware mobile storage host controller is used to interface
>> +  a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
>> +  differences between the core Synopsys dw mshc controller properties described
>> +  by synopsys-dw-mshc.txt and the properties used by the Hisilicon Hi3798CV200
>> +  specific extensions to the Synopsys Designware Mobile Storage Host Controller.
> 
> Drop the last sentence.
> 
>> +
>> +allOf:
>> +  - $ref: "synopsys-dw-mshc-common.yaml#"
> 
> Drop quotes.
> 
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: hisilicon,hi3798mv200-dw-mshc
>> +    then:
>> +      properties:
>> +        clocks:
>> +          minItems: 5
>> +
>> +        clock-names:
>> +          minItems: 5
> 
> Entire allOf goes after required:.
> 
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - hisilicon,hi3798cv200-dw-mshc
>> +      - hisilicon,hi3798mv200-dw-mshc
> 
> New compatibles are not related to conversion.
> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    minItems: 4
>> +    maxItems: 5
>> +    description: A list of phandles for the clocks listed in clock-names
> 
> Drop description.
> 
>> +
>> +  clock-names:
>> +    minItems: 4
>> +    items:
>> +      - const: ciu
>> +      - const: biu
>> +      - const: ciu-sample
>> +      - const: ciu-drive
>> +      - const: sap-dll-mode
> 
> Only four clocks were in original document. Don't mix different changes
> in one patch.
> 
>> +
>> +unevaluatedProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +
>> +examples:
>> +  - |
>> +    emmc: mmc@9830000 {
>> +            compatible = "hisilicon,hi3798cv200-dw-mshc";
>> +            reg = <0x9830000 0x10000>;
> 
> Wrong indentation. Use 4 spaces for example indentation.
> 
>> +            interrupts = <35>;
>> +            clocks = <&crg 1>,
> 
>> +
>>
> 
> Best regards,
> Krzysztof
> 
Thanks, will fix in v3.
By the way, this is v1, v2 fixed some mistakes and the changelog is in
the cover-letter.
-- 
Best regards,
Yang Xiwen