[PATCH 0/3] mmc: add hi3798mv200 specific extensions of DWMMC

[PATCH 0/3] mmc: add hi3798mv200 specific extensions of DWMMC

[Yang Xiwen via B4 Relay]

it's modified from hi3798cv200 driver, but quite a lot of code gets
rewritten because of the hardware differences. Actually cv200 DWMMC core
is called HIMCIV200 while mv200 DWMMC core is called HIMCIV300 in
downstream.

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

 .../bindings/mmc/hi3798cv200-dw-mshc.txt           |  40 ----
 .../devicetree/bindings/mmc/histb-dw-mshc.yaml     | 130 +++++++++++
 drivers/mmc/host/Kconfig                           |   9 +
 drivers/mmc/host/Makefile                          |   1 +
 drivers/mmc/host/dw_mmc-hi3798mv200.c              | 238 +++++++++++++++++++++
 5 files changed, 378 insertions(+), 40 deletions(-)
---
base-commit: 8d3dea210042f54b952b481838c1e7dfc4ec751d
change-id: 20240121-b4-mmc-hi3798mv200-a5730edf122c

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

[PATCH 1/3] mmc: dw_mmc: 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/Kconfig              |   9 ++
 drivers/mmc/host/Makefile             |   1 +
 drivers/mmc/host/dw_mmc-hi3798mv200.c | 238 ++++++++++++++++++++++++++++++++++
 3 files changed, 248 insertions(+)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 81f2c4e05287..aebc587f77a7 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -798,6 +798,15 @@ config MMC_DW_HI3798CV200
 	  Synopsys DesignWare Memory Card Interface driver. Select this option
 	  for platforms based on HiSilicon Hi3798CV200 SoC.
 
+config MMC_DW_HI3798MV200
+	tristate "Hi3798MV200 specific extensions for Synopsys DW Memory Card Interface"
+	depends on MMC_DW
+	select MMC_DW_PLTFM
+	help
+	  This selects support for HiSilicon Hi3798MV200 SoC specific extensions to the
+	  Synopsys DesignWare Memory Card Interface driver. Select this option
+	  for platforms based on HiSilicon Hi3798MV200 SoC.
+
 config MMC_DW_K3
 	tristate "K3 specific extensions for Synopsys DW Memory Card Interface"
 	depends on MMC_DW
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index d0be4465f3ec..f53f86d200ac 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -51,6 +51,7 @@ 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_HI3798MV200) += dw_mmc-hi3798mv200.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-hi3798mv200.c b/drivers/mmc/host/dw_mmc-hi3798mv200.c
new file mode 100644
index 000000000000..ebbeccecf5b7
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-hi3798mv200.c
@@ -0,0 +1,238 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Modified from dw_mmc-hi3798cv200.c
+ *
+ * Copyright (c) 2024 Yang Xiwen <forbidden405@outlook.com>
+ * Copyright (c) 2018 HiSilicon Technologies Co., Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mmc/host.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#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
+
+/* DLL ctrl reg */
+#define SAP_DLL_CTRL_DLLMODE	BIT(16)
+
+struct dw_mci_hi3798mv200_priv {
+	struct clk *sample_clk;
+	struct clk *drive_clk;
+	struct regmap *sap_dll_reg;
+	struct mmc_clk_phase_map phase_map;
+};
+
+static void dw_mci_hi3798mv200_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+	struct dw_mci_hi3798mv200_priv *priv = host->priv;
+	struct mmc_clk_phase phase = priv->phase_map.phase[ios->timing];
+	u32 val;
+
+	val = mci_readl(host, ENABLE_SHIFT);
+	if (ios->timing == MMC_TIMING_MMC_DDR52
+	    || ios->timing == MMC_TIMING_UHS_DDR50)
+		val |= SDMMC_ENABLE_PHASE;
+	else
+		val &= ~SDMMC_ENABLE_PHASE;
+	mci_writel(host, ENABLE_SHIFT, val);
+
+	val = mci_readl(host, DDR_REG);
+	if (ios->timing == MMC_TIMING_MMC_HS400)
+		val |= SDMMC_DDR_HS400;
+	else
+		val &= ~SDMMC_DDR_HS400;
+	mci_writel(host, DDR_REG, val);
+
+	if (clk_set_rate(host->ciu_clk, ios->clock))
+		dev_warn(host->dev, "Failed to set rate to %u\n", ios->clock);
+	else
+		// CLK_MUX_ROUND_NEAREST is enabled for this clock
+		// The actual clock rate is not what we setted, but a rounded value
+		// so we should get the rate once again
+		host->bus_hz = clk_get_rate(host->ciu_clk);
+
+	if (phase.valid) {
+		clk_set_phase(priv->drive_clk, phase.out_deg);
+		clk_set_phase(priv->sample_clk, phase.in_deg);
+	} else {
+		dev_warn(host->dev,
+			 "The phase entry for timing mode %d is missing in device tree.\n",
+			 ios->timing);
+	}
+}
+
+static inline int dw_mci_hi3798mv200_enable_tuning(struct dw_mci_slot *slot)
+{
+	struct dw_mci_hi3798mv200_priv *priv = slot->host->priv;
+	struct regmap *dll_reg = priv->sap_dll_reg;
+
+	return regmap_clear_bits(dll_reg, 0, SAP_DLL_CTRL_DLLMODE);
+}
+
+static inline int dw_mci_hi3798mv200_disable_tuning(struct dw_mci_slot *slot)
+{
+	struct dw_mci_hi3798mv200_priv *priv = slot->host->priv;
+	struct regmap *dll_reg = priv->sap_dll_reg;
+
+	return regmap_set_bits(dll_reg, 0, SAP_DLL_CTRL_DLLMODE);
+}
+
+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_hi3798mv200_priv *priv = host->priv;
+	int raise_point = -1, fall_point = -1;
+	int err, prev_err = -1;
+	int found = 0;
+	int regval;
+	int i;
+	int ret;
+
+	// enable tuning
+	ret = dw_mci_hi3798mv200_enable_tuning(slot);
+	if (ret < 0)
+		return ret;
+	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) {
+			regval = mci_readl(host, TUNING_CTRL);
+			if (regval & SDMMC_TUNING_FIND_EDGE)
+				err = 1;
+			else
+				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:
+	ret = dw_mci_hi3798mv200_disable_tuning(slot);
+	if (ret < 0)
+		return ret;
+	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;
+		}
+
+		// use the same phase table for both HS200 and HS400
+		priv->phase_map.phase[MMC_TIMING_MMC_HS200].in_deg = degrees[i];
+		priv->phase_map.phase[MMC_TIMING_MMC_HS400].in_deg = degrees[i];
+
+		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_hi3798mv200_init(struct dw_mci *host)
+{
+	struct dw_mci_hi3798mv200_priv *priv;
+	struct device_node *np = host->dev->of_node;
+
+	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
+
+	priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
+	if (IS_ERR(priv->sample_clk)) {
+		dev_err(host->dev, "failed to get enabled ciu-sample clock\n");
+		return PTR_ERR(priv->sample_clk);
+	}
+
+	priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
+	if (IS_ERR(priv->drive_clk)) {
+		dev_err(host->dev, "failed to get enabled ciu-drive clock\n");
+		return PTR_ERR(priv->drive_clk);
+	}
+
+	priv->sap_dll_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
+	if (IS_ERR(priv->sap_dll_reg)) {
+		dev_err(host->dev, "failed to get sap-dll-reg\n");
+		return PTR_ERR(priv->sap_dll_reg);
+	}
+
+	host->priv = priv;
+	return 0;
+}
+
+static const struct dw_mci_drv_data hi3798mv200_data = {
+	.common_caps = MMC_CAP_CMD23,
+	.init = dw_mci_hi3798mv200_init,
+	.set_ios = dw_mci_hi3798mv200_set_ios,
+	.execute_tuning = dw_mci_hi3798mv200_execute_tuning_mix_mode,
+};
+
+static const struct of_device_id dw_mci_hi3798mv200_match[] = {
+	{ .compatible = "hisilicon,hi3798mv200-dw-mshc" },
+	{},
+};
+
+static int dw_mci_hi3798mv200_probe(struct platform_device *pdev)
+{
+	return dw_mci_pltfm_register(pdev, &hi3798mv200_data);
+}
+
+static void dw_mci_hi3798mv200_remove(struct platform_device *pdev)
+{
+	dw_mci_pltfm_remove(pdev);
+}
+
+MODULE_DEVICE_TABLE(of, dw_mci_hi3798mv200_match);
+static struct platform_driver dw_mci_hi3798mv200_driver = {
+	.probe = dw_mci_hi3798mv200_probe,
+	.remove_new = dw_mci_hi3798mv200_remove,
+	.driver = {
+		.name = "dwmmc_hi3798mv200",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.of_match_table = dw_mci_hi3798mv200_match,
+	},
+};
+module_platform_driver(dw_mci_hi3798mv200_driver);
+
+MODULE_DESCRIPTION("HiSilicon Hi3798MV200 Specific DW-MSHC Driver Extension");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:dwmmc_hi3798mv200");

-- 
2.43.0

Re: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200

[Krzysztof Kozlowski]

On 15/02/2024 18:46, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
> 
> Add support for Hi3798MV200 specific extension.
> 
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>


> +
> +static int dw_mci_hi3798mv200_init(struct dw_mci *host)
> +{
> +	struct dw_mci_hi3798mv200_priv *priv;
> +	struct device_node *np = host->dev->of_node;
> +
> +	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
> +
> +	priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
> +	if (IS_ERR(priv->sample_clk)) {
> +		dev_err(host->dev, "failed to get enabled ciu-sample clock\n");

syntax is: return dev_err_probe()

> +		return PTR_ERR(priv->sample_clk);
> +	}
> +
> +	priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
> +	if (IS_ERR(priv->drive_clk)) {
> +		dev_err(host->dev, "failed to get enabled ciu-drive clock\n");

syntax is: return dev_err_probe()

> +		return PTR_ERR(priv->drive_clk);
> +	}
> +
> +	priv->sap_dll_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
> +	if (IS_ERR(priv->sap_dll_reg)) {
> +		dev_err(host->dev, "failed to get sap-dll-reg\n");

syntax is: return dev_err_probe()

> +		return PTR_ERR(priv->sap_dll_reg);
> +	}
> +
> +	host->priv = priv;
> +	return 0;
> +}
> +

....

> +
> +MODULE_DEVICE_TABLE(of, dw_mci_hi3798mv200_match);
> +static struct platform_driver dw_mci_hi3798mv200_driver = {
> +	.probe = dw_mci_hi3798mv200_probe,
> +	.remove_new = dw_mci_hi3798mv200_remove,
> +	.driver = {
> +		.name = "dwmmc_hi3798mv200",
> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
> +		.of_match_table = dw_mci_hi3798mv200_match,
> +	},
> +};
> +module_platform_driver(dw_mci_hi3798mv200_driver);
> +
> +MODULE_DESCRIPTION("HiSilicon Hi3798MV200 Specific DW-MSHC Driver Extension");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:dwmmc_hi3798mv200");

You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong (e.g. misses either
entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
for incomplete ID table.

> 

Best regards,
Krzysztof

Re: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200

[Yang Xiwen]

On 2/16/2024 3:15 AM, Krzysztof Kozlowski wrote:
> On 15/02/2024 18:46, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
>>
>> Add support for Hi3798MV200 specific extension.
>>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>
>> +
>> +static int dw_mci_hi3798mv200_init(struct dw_mci *host)
>> +{
>> +	struct dw_mci_hi3798mv200_priv *priv;
>> +	struct device_node *np = host->dev->of_node;
>> +
>> +	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
>> +
>> +	priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
>> +	if (IS_ERR(priv->sample_clk)) {
>> +		dev_err(host->dev, "failed to get enabled ciu-sample clock\n");
> syntax is: return dev_err_probe()
Will fix in next version.
>
>> +		return PTR_ERR(priv->sample_clk);
>> +	}
>> +
>> +	priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
>> +	if (IS_ERR(priv->drive_clk)) {
>> +		dev_err(host->dev, "failed to get enabled ciu-drive clock\n");
> syntax is: return dev_err_probe()
Will fix in next version.
>
>> +		return PTR_ERR(priv->drive_clk);
>> +	}
>> +
>> +	priv->sap_dll_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
>> +	if (IS_ERR(priv->sap_dll_reg)) {
>> +		dev_err(host->dev, "failed to get sap-dll-reg\n");
> syntax is: return dev_err_probe()
Will fix in next version.
>
>> +		return PTR_ERR(priv->sap_dll_reg);
>> +	}
>> +
>> +	host->priv = priv;
>> +	return 0;
>> +}
>> +
> ....
>
>> +
>> +MODULE_DEVICE_TABLE(of, dw_mci_hi3798mv200_match);
>> +static struct platform_driver dw_mci_hi3798mv200_driver = {
>> +	.probe = dw_mci_hi3798mv200_probe,
>> +	.remove_new = dw_mci_hi3798mv200_remove,
>> +	.driver = {
>> +		.name = "dwmmc_hi3798mv200",
>> +		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>> +		.of_match_table = dw_mci_hi3798mv200_match,
>> +	},
>> +};
>> +module_platform_driver(dw_mci_hi3798mv200_driver);
>> +
>> +MODULE_DESCRIPTION("HiSilicon Hi3798MV200 Specific DW-MSHC Driver Extension");
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("platform:dwmmc_hi3798mv200");
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong (e.g. misses either
> entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute
> for incomplete ID table.
Will fix it in v2. I simply copied this from dw_mmc-hi3798cv200.c and 
s/cv200/mv200/. I'll remove it for dw_mmc-hi3798cv200.c in next version 
too because it seems not useful in that driver too.
> Best regards,
> Krzysztof
>

-- 
Regards,
Yang Xiwen

Re: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200

[kernel test robot]

Hi Yang,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8d3dea210042f54b952b481838c1e7dfc4ec751d]

url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/mmc-dw_mmc-add-support-for-hi3798mv200/20240216-014744
base:   8d3dea210042f54b952b481838c1e7dfc4ec751d
patch link:    https://lore.kernel.org/r/20240216-b4-mmc-hi3798mv200-v1-1-7d46db845ae6%40outlook.com
patch subject: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200
config: parisc-allyesconfig (https://download.01.org/0day-ci/archive/20240218/202402181540.H4Ose96P-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240218/202402181540.H4Ose96P-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402181540.H4Ose96P-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/mmc/host/dw_mmc-hi3798mv200.c: In function 'dw_mci_hi3798mv200_init':
>> drivers/mmc/host/dw_mmc-hi3798mv200.c:178:36: error: passing argument 1 of 'mmc_of_parse_clk_phase' from incompatible pointer type [-Werror=incompatible-pointer-types]
     178 |         mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
         |                                ~~~~^~~~~
         |                                    |
         |                                    struct device *
   In file included from drivers/mmc/host/dw_mmc-hi3798mv200.c:11:
   include/linux/mmc/host.h:542:46: note: expected 'struct mmc_host *' but argument is of type 'struct device *'
     542 | void mmc_of_parse_clk_phase(struct mmc_host *host,
         |                             ~~~~~~~~~~~~~~~~~^~~~
   cc1: some warnings being treated as errors


vim +/mmc_of_parse_clk_phase +178 drivers/mmc/host/dw_mmc-hi3798mv200.c

   168	
   169	static int dw_mci_hi3798mv200_init(struct dw_mci *host)
   170	{
   171		struct dw_mci_hi3798mv200_priv *priv;
   172		struct device_node *np = host->dev->of_node;
   173	
   174		priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
   175		if (!priv)
   176			return -ENOMEM;
   177	
 > 178		mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
   179	
   180		priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
   181		if (IS_ERR(priv->sample_clk)) {
   182			dev_err(host->dev, "failed to get enabled ciu-sample clock\n");
   183			return PTR_ERR(priv->sample_clk);
   184		}
   185	
   186		priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
   187		if (IS_ERR(priv->drive_clk)) {
   188			dev_err(host->dev, "failed to get enabled ciu-drive clock\n");
   189			return PTR_ERR(priv->drive_clk);
   190		}
   191	
   192		priv->sap_dll_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
   193		if (IS_ERR(priv->sap_dll_reg)) {
   194			dev_err(host->dev, "failed to get sap-dll-reg\n");
   195			return PTR_ERR(priv->sap_dll_reg);
   196		}
   197	
   198		host->priv = priv;
   199		return 0;
   200	}
   201	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200

[Yang Xiwen]

On 2/18/2024 4:03 PM, kernel test robot wrote:
> Hi Yang,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 8d3dea210042f54b952b481838c1e7dfc4ec751d]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/mmc-dw_mmc-add-support-for-hi3798mv200/20240216-014744
> base:   8d3dea210042f54b952b481838c1e7dfc4ec751d
> patch link:    https://lore.kernel.org/r/20240216-b4-mmc-hi3798mv200-v1-1-7d46db845ae6%40outlook.com
> patch subject: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200
> config: parisc-allyesconfig (https://download.01.org/0day-ci/archive/20240218/202402181540.H4Ose96P-lkp@intel.com/config)
> compiler: hppa-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240218/202402181540.H4Ose96P-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202402181540.H4Ose96P-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>     drivers/mmc/host/dw_mmc-hi3798mv200.c: In function 'dw_mci_hi3798mv200_init':
>>> drivers/mmc/host/dw_mmc-hi3798mv200.c:178:36: error: passing argument 1 of 'mmc_of_parse_clk_phase' from incompatible pointer type [-Werror=incompatible-pointer-types]
>       178 |         mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
>           |                                ~~~~^~~~~
>           |                                    |
>           |                                    struct device *
>     In file included from drivers/mmc/host/dw_mmc-hi3798mv200.c:11:
>     include/linux/mmc/host.h:542:46: note: expected 'struct mmc_host *' but argument is of type 'struct device *'
>       542 | void mmc_of_parse_clk_phase(struct mmc_host *host,
>           |                             ~~~~~~~~~~~~~~~~~^~~~
>     cc1: some warnings being treated as errors
>
>
> vim +/mmc_of_parse_clk_phase +178 drivers/mmc/host/dw_mmc-hi3798mv200.c
>
>     168	
>     169	static int dw_mci_hi3798mv200_init(struct dw_mci *host)
>     170	{
>     171		struct dw_mci_hi3798mv200_priv *priv;
>     172		struct device_node *np = host->dev->of_node;
>     173	
>     174		priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>     175		if (!priv)
>     176			return -ENOMEM;
>     177	
>   > 178		mmc_of_parse_clk_phase(host->dev, &priv->phase_map);

Please note that this patch is depending on another patch[1].

[1]: 
https://lore.kernel.org/linux-mmc/ae876e697ba16ba2925ec217c6b4e3d8ffea4ab3.camel@codeconstruct.com.au/T/#t


Ulf, what do think about picking that patch in this patch set? So that 
the dependencies can be resolved.

>     179	
>     180		priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
>     181		if (IS_ERR(priv->sample_clk)) {
>     182			dev_err(host->dev, "failed to get enabled ciu-sample clock\n");
>     183			return PTR_ERR(priv->sample_clk);
>     184		}
>     185	
>     186		priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
>     187		if (IS_ERR(priv->drive_clk)) {
>     188			dev_err(host->dev, "failed to get enabled ciu-drive clock\n");
>     189			return PTR_ERR(priv->drive_clk);
>     190		}
>     191	
>     192		priv->sap_dll_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
>     193		if (IS_ERR(priv->sap_dll_reg)) {
>     194			dev_err(host->dev, "failed to get sap-dll-reg\n");
>     195			return PTR_ERR(priv->sap_dll_reg);
>     196		}
>     197	
>     198		host->priv = priv;
>     199		return 0;
>     200	}
>     201	
>

-- 
Regards,
Yang Xiwen

Re: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200

[Krzysztof Kozlowski]

On 18/02/2024 09:21, Yang Xiwen wrote:
> On 2/18/2024 4:03 PM, kernel test robot wrote:
>> Hi Yang,
>>
>> kernel test robot noticed the following build errors:
>>
>> [auto build test ERROR on 8d3dea210042f54b952b481838c1e7dfc4ec751d]
>>
>> url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/mmc-dw_mmc-add-support-for-hi3798mv200/20240216-014744
>> base:   8d3dea210042f54b952b481838c1e7dfc4ec751d
>> patch link:    https://lore.kernel.org/r/20240216-b4-mmc-hi3798mv200-v1-1-7d46db845ae6%40outlook.com
>> patch subject: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200
>> config: parisc-allyesconfig (https://download.01.org/0day-ci/archive/20240218/202402181540.H4Ose96P-lkp@intel.com/config)
>> compiler: hppa-linux-gcc (GCC) 13.2.0
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240218/202402181540.H4Ose96P-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202402181540.H4Ose96P-lkp@intel.com/
>>
>> All errors (new ones prefixed by >>):
>>
>>     drivers/mmc/host/dw_mmc-hi3798mv200.c: In function 'dw_mci_hi3798mv200_init':
>>>> drivers/mmc/host/dw_mmc-hi3798mv200.c:178:36: error: passing argument 1 of 'mmc_of_parse_clk_phase' from incompatible pointer type [-Werror=incompatible-pointer-types]
>>       178 |         mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
>>           |                                ~~~~^~~~~
>>           |                                    |
>>           |                                    struct device *
>>     In file included from drivers/mmc/host/dw_mmc-hi3798mv200.c:11:
>>     include/linux/mmc/host.h:542:46: note: expected 'struct mmc_host *' but argument is of type 'struct device *'
>>       542 | void mmc_of_parse_clk_phase(struct mmc_host *host,
>>           |                             ~~~~~~~~~~~~~~~~~^~~~
>>     cc1: some warnings being treated as errors
>>
>>
>> vim +/mmc_of_parse_clk_phase +178 drivers/mmc/host/dw_mmc-hi3798mv200.c
>>
>>     168	
>>     169	static int dw_mci_hi3798mv200_init(struct dw_mci *host)
>>     170	{
>>     171		struct dw_mci_hi3798mv200_priv *priv;
>>     172		struct device_node *np = host->dev->of_node;
>>     173	
>>     174		priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
>>     175		if (!priv)
>>     176			return -ENOMEM;
>>     177	
>>   > 178		mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
> 
> Please note that this patch is depending on another patch[1].
> 
> [1]: 
> https://lore.kernel.org/linux-mmc/ae876e697ba16ba2925ec217c6b4e3d8ffea4ab3.camel@codeconstruct.com.au/T/#t
> 

You did not provide any links to dependencies in cover letter nor in
patch changelog. You included on purpose, which is nice and correct, the
base commit:

	base-commit: 8d3dea210042f54b952b481838c1e7dfc4ec751d

which does not include above commit apparently.

How anyone can guess it and test your patches?

Please send with proper dependencies marked.

Best regards,
Krzysztof

Re: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200

[kernel test robot]

Hi Yang,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8d3dea210042f54b952b481838c1e7dfc4ec751d]

url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Xiwen-via-B4-Relay/mmc-dw_mmc-add-support-for-hi3798mv200/20240216-014744
base:   8d3dea210042f54b952b481838c1e7dfc4ec751d
patch link:    https://lore.kernel.org/r/20240216-b4-mmc-hi3798mv200-v1-1-7d46db845ae6%40outlook.com
patch subject: [PATCH 1/3] mmc: dw_mmc: add support for hi3798mv200
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20240219/202402190531.qUVUPNDD-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 36adfec155de366d722f2bac8ff9162289dcf06c)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240219/202402190531.qUVUPNDD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402190531.qUVUPNDD-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/mmc/host/dw_mmc-hi3798mv200.c:178:25: error: incompatible pointer types passing 'struct device *' to parameter of type 'struct mmc_host *' [-Werror,-Wincompatible-pointer-types]
     178 |         mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
         |                                ^~~~~~~~~
   include/linux/mmc/host.h:542:46: note: passing argument to parameter 'host' here
     542 | void mmc_of_parse_clk_phase(struct mmc_host *host,
         |                                              ^
   1 error generated.


vim +178 drivers/mmc/host/dw_mmc-hi3798mv200.c

   168	
   169	static int dw_mci_hi3798mv200_init(struct dw_mci *host)
   170	{
   171		struct dw_mci_hi3798mv200_priv *priv;
   172		struct device_node *np = host->dev->of_node;
   173	
   174		priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
   175		if (!priv)
   176			return -ENOMEM;
   177	
 > 178		mmc_of_parse_clk_phase(host->dev, &priv->phase_map);
   179	
   180		priv->sample_clk = devm_clk_get_enabled(host->dev, "ciu-sample");
   181		if (IS_ERR(priv->sample_clk)) {
   182			dev_err(host->dev, "failed to get enabled ciu-sample clock\n");
   183			return PTR_ERR(priv->sample_clk);
   184		}
   185	
   186		priv->drive_clk = devm_clk_get_enabled(host->dev, "ciu-drive");
   187		if (IS_ERR(priv->drive_clk)) {
   188			dev_err(host->dev, "failed to get enabled ciu-drive clock\n");
   189			return PTR_ERR(priv->drive_clk);
   190		}
   191	
   192		priv->sap_dll_reg = syscon_regmap_lookup_by_phandle(np, "hisilicon,sap-dll-reg");
   193		if (IS_ERR(priv->sap_dll_reg)) {
   194			dev_err(host->dev, "failed to get sap-dll-reg\n");
   195			return PTR_ERR(priv->sap_dll_reg);
   196		}
   197	
   198		host->priv = priv;
   199		return 0;
   200	}
   201	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH 2/3] dt-bindings: mmc: dw-mshc-hi3798cv200: convert to YAML

[Yang Xiwen via B4 Relay]

From: Yang Xiwen <forbidden405@outlook.com>

convert the legacy txt binding to modern YAML. No semantic change.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
 .../bindings/mmc/hi3798cv200-dw-mshc.txt           | 40 ----------
 .../bindings/mmc/hi3798cv200-dw-mshc.yaml          | 86 ++++++++++++++++++++++
 2 files changed, 86 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 a0693b7145f2..000000000000
--- 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/hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
new file mode 100644
index 000000000000..5db99cd94b90
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title:
+  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC 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#
+
+properties:
+  compatible:
+    enum:
+      - hisilicon,hi3798cv200-dw-mshc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 4
+    maxItems: 4
+    description: A list of phandles for the clocks listed in clock-names
+
+  clock-names:
+    items:
+      - const: ciu
+      - const: biu
+      - const: ciu-sample
+      - const: ciu-drive
+    description:
+      Apart from the clock-names "biu" and "ciu" two more clocks
+      "ciu-drive" and "ciu-sample" are added. They are used to
+      control the clock phases, "ciu-sample" is required for tuning
+      high speed modes.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/histb-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    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";
+      resets = <&crg 0xa0 4>;
+      reset-names = "reset";
+      pinctrl-names = "default";
+      pinctrl-0 = <&emmc_pins_1 &emmc_pins_2
+                   &emmc_pins_3 &emmc_pins_4>;
+      fifo-depth = <256>;
+      clock-frequency = <200000000>;
+      cap-mmc-highspeed;
+      mmc-ddr-1_8v;
+      mmc-hs200-1_8v;
+      non-removable;
+      bus-width = <8>;
+      status = "okay";
+    };

-- 
2.43.0

Re: [PATCH 2/3] dt-bindings: mmc: dw-mshc-hi3798cv200: convert to YAML

[Krzysztof Kozlowski]

On 15/02/2024 18:46, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
> 
> convert the legacy txt binding to modern YAML. No semantic change.
> 
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> ---


> +++ b/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml

Filename like compatible.

> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title:
> +  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller

One line please.

> +
> +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#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - hisilicon,hi3798cv200-dw-mshc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 4

Drop minItems

> +    maxItems: 4
> +    description: A list of phandles for the clocks listed in clock-names

Drop description

> +
> +  clock-names:
> +    items:
> +      - const: ciu
> +      - const: biu
> +      - const: ciu-sample
> +      - const: ciu-drive
> +    description:
> +      Apart from the clock-names "biu" and "ciu" two more clocks
> +      "ciu-drive" and "ciu-sample" are added. They are used to
> +      control the clock phases, "ciu-sample" is required for tuning
> +      high speed modes.

Description should go to clocks: to individual items.


> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/histb-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    emmc: mmc@9830000 {

Drop label

> +      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";
> +      resets = <&crg 0xa0 4>;
> +      reset-names = "reset";
> +      pinctrl-names = "default";
> +      pinctrl-0 = <&emmc_pins_1 &emmc_pins_2
> +                   &emmc_pins_3 &emmc_pins_4>;
> +      fifo-depth = <256>;
> +      clock-frequency = <200000000>;
> +      cap-mmc-highspeed;
> +      mmc-ddr-1_8v;
> +      mmc-hs200-1_8v;
> +      non-removable;
> +      bus-width = <8>;
> +      status = "okay";

Drop

> +    };
> 

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: mmc: dw-mshc-hi3798cv200: convert to YAML

[Yang Xiwen]

On 2/16/2024 4:19 PM, Krzysztof Kozlowski wrote:
> On 15/02/2024 18:46, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
>>
>> convert the legacy txt binding to modern YAML. No semantic change.
>>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>> ---
>
>> +++ b/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
> Filename like compatible.
>
>> @@ -0,0 +1,86 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title:
>> +  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
> One line please.
>
>> +
>> +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#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - hisilicon,hi3798cv200-dw-mshc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    minItems: 4
> Drop minItems
>
>> +    maxItems: 4
>> +    description: A list of phandles for the clocks listed in clock-names
> Drop description
>
>> +
>> +  clock-names:
>> +    items:
>> +      - const: ciu
>> +      - const: biu
>> +      - const: ciu-sample
>> +      - const: ciu-drive
>> +    description:
>> +      Apart from the clock-names "biu" and "ciu" two more clocks
>> +      "ciu-drive" and "ciu-sample" are added. They are used to
>> +      control the clock phases, "ciu-sample" is required for tuning
>> +      high speed modes.
> Description should go to clocks: to individual items.
Actually copied directly from rockchip-dw-mshc.yaml. Will fix in v2.
>
>
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/histb-clock.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    emmc: mmc@9830000 {
> Drop label
>
>> +      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";
>> +      resets = <&crg 0xa0 4>;
>> +      reset-names = "reset";
>> +      pinctrl-names = "default";
>> +      pinctrl-0 = <&emmc_pins_1 &emmc_pins_2
>> +                   &emmc_pins_3 &emmc_pins_4>;
>> +      fifo-depth = <256>;
>> +      clock-frequency = <200000000>;
>> +      cap-mmc-highspeed;
>> +      mmc-ddr-1_8v;
>> +      mmc-hs200-1_8v;
>> +      non-removable;
>> +      bus-width = <8>;
>> +      status = "okay";
> Drop
Drop `status` property? Will fix in v2.
>
>> +    };
>>
> Best regards,
> Krzysztof
>

-- 
Regards,
Yang Xiwen

[PATCH 3/3] dt-bindings: mmc: dw-mshc-hi3798cv200: rename to dw-mshc-histb

[Yang Xiwen via B4 Relay]

From: Yang Xiwen <forbidden405@outlook.com>

Add binding for Hi3798MV200 DWMMC specific extension.

Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
---
 ...hi3798cv200-dw-mshc.yaml => histb-dw-mshc.yaml} | 60 +++++++++++++++++++---
 1 file changed, 52 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
similarity index 57%
rename from Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
rename to Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
index 5db99cd94b90..d2f5b7bb7a58 100644
--- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
+++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
@@ -1,11 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
+$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title:
-  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
+  Hisilicon HiSTB SoCs specific extensions to the Synopsys DWMMC controller
 
 maintainers:
   - Yang Xiwen <forbidden405@outlook.com>
@@ -14,16 +14,14 @@ 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#
+  by synopsys-dw-mshc.txt and the properties used by the Hisilicon HiSTB specific
+  extensions to the Synopsys Designware Mobile Storage Host Controller.
 
 properties:
   compatible:
     enum:
       - hisilicon,hi3798cv200-dw-mshc
+      - hisilicon,hi3798mv200-dw-mshc
 
   reg:
     maxItems: 1
@@ -48,6 +46,12 @@ properties:
       control the clock phases, "ciu-sample" is required for tuning
       high speed modes.
 
+  hisilicon,sap-dll-reg:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      A phandle points to the sample delay-locked-loop(DLL)
+      syscon node, used for tuning.
+
 required:
   - compatible
   - reg
@@ -55,13 +59,25 @@ required:
   - clocks
   - clock-names
 
+allOf:
+  - $ref: synopsys-dw-mshc-common.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: hisilicon,hi3798mv200-dw-mshc
+    then:
+      required:
+        - hisilicon,sap-dll-reg
+
 unevaluatedProperties: false
 
 examples:
   - |
     #include <dt-bindings/clock/histb-clock.h>
     #include <dt-bindings/interrupt-controller/arm-gic.h>
-    emmc: mmc@9830000 {
+    mmc@9830000 {
       compatible = "hisilicon,hi3798cv200-dw-mshc";
       reg = <0x9830000 0x10000>;
       interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
@@ -84,3 +100,31 @@ examples:
       bus-width = <8>;
       status = "okay";
     };
+  - |
+    #include <dt-bindings/clock/histb-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    mmc@9830000 {
+      compatible = "hisilicon,hi3798mv200-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";
+      resets = <&crg 0xa0 4>;
+      reset-names = "reset";
+      pinctrl-names = "default";
+      pinctrl-0 = <&emmc_pins>;
+      fifo-depth = <256>;
+      clock-frequency = <50000000>;
+      max-frequency = <150000000>;
+      cap-mmc-highspeed;
+      mmc-ddr-1_8v;
+      mmc-hs200-1_8v;
+      mmc-hs400-1_8v;
+      non-removable;
+      bus-width = <8>;
+      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
+      status = "okay";
+    };

-- 
2.43.0

Re: [PATCH 3/3] dt-bindings: mmc: dw-mshc-hi3798cv200: rename to dw-mshc-histb

[Krzysztof Kozlowski]

On 15/02/2024 18:46, Yang Xiwen via B4 Relay wrote:
> From: Yang Xiwen <forbidden405@outlook.com>
> 
> Add binding for Hi3798MV200 DWMMC specific extension.
> 
> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
> ---
>  ...hi3798cv200-dw-mshc.yaml => histb-dw-mshc.yaml} | 60 +++++++++++++++++++---
>  1 file changed, 52 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
> similarity index 57%
> rename from Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
> rename to Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
> index 5db99cd94b90..d2f5b7bb7a58 100644
> --- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
> @@ -1,11 +1,11 @@
>  # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>  %YAML 1.2
>  ---
> -$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
> +$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#

Really, one wrong filename into another...

>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
>  title:
> -  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
> +  Hisilicon HiSTB SoCs specific extensions to the Synopsys DWMMC controller
>  
>  maintainers:
>    - Yang Xiwen <forbidden405@outlook.com>
> @@ -14,16 +14,14 @@ 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.

Just drop this sentence in previous/conversion patch. It's useless.

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

Put it in correct place in the first time. Don't needlessly shuffle the
code right after previous patch.


> +  by synopsys-dw-mshc.txt and the properties used by the Hisilicon HiSTB specific
> +  extensions to the Synopsys Designware Mobile Storage Host Controller.
>  
>  properties:
>    compatible:
>      enum:
>        - hisilicon,hi3798cv200-dw-mshc
> +      - hisilicon,hi3798mv200-dw-mshc
>  
>    reg:
>      maxItems: 1
> @@ -48,6 +46,12 @@ properties:
>        control the clock phases, "ciu-sample" is required for tuning
>        high speed modes.
>  
> +  hisilicon,sap-dll-reg:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      A phandle points to the sample delay-locked-loop(DLL)
> +      syscon node, used for tuning.

Does hi3798cv200 have it?

> +
>  required:
>    - compatible
>    - reg
> @@ -55,13 +59,25 @@ required:
>    - clocks
>    - clock-names
>  
> +allOf:
> +  - $ref: synopsys-dw-mshc-common.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: hisilicon,hi3798mv200-dw-mshc
> +    then:
> +      required:
> +        - hisilicon,sap-dll-reg
> +
>  unevaluatedProperties: false
>  
>  examples:
>    - |
>      #include <dt-bindings/clock/histb-clock.h>
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
> -    emmc: mmc@9830000 {
> +    mmc@9830000 {

???

>        compatible = "hisilicon,hi3798cv200-dw-mshc";
>        reg = <0x9830000 0x10000>;
>        interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
> @@ -84,3 +100,31 @@ examples:
>        bus-width = <8>;
>        status = "okay";
>      };
> +  - |
> +    #include <dt-bindings/clock/histb-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    mmc@9830000 {
> +      compatible = "hisilicon,hi3798mv200-dw-mshc";

No need for new example.

> +      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";
> +      resets = <&crg 0xa0 4>;
> +      reset-names = "reset";
> +      pinctrl-names = "default";
> +      pinctrl-0 = <&emmc_pins>;
> +      fifo-depth = <256>;
> +      clock-frequency = <50000000>;
> +      max-frequency = <150000000>;
> +      cap-mmc-highspeed;
> +      mmc-ddr-1_8v;
> +      mmc-hs200-1_8v;
> +      mmc-hs400-1_8v;
> +      non-removable;
> +      bus-width = <8>;
> +      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
> +      status = "okay";

No, really...

> +    };
> 

Best regards,
Krzysztof

Re: [PATCH 3/3] dt-bindings: mmc: dw-mshc-hi3798cv200: rename to dw-mshc-histb

[Yang Xiwen]

On 2/16/2024 4:21 PM, Krzysztof Kozlowski wrote:
> On 15/02/2024 18:46, Yang Xiwen via B4 Relay wrote:
>> From: Yang Xiwen <forbidden405@outlook.com>
>>
>> Add binding for Hi3798MV200 DWMMC specific extension.
>>
>> Signed-off-by: Yang Xiwen <forbidden405@outlook.com>
>> ---
>>   ...hi3798cv200-dw-mshc.yaml => histb-dw-mshc.yaml} | 60 +++++++++++++++++++---
>>   1 file changed, 52 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
>> similarity index 57%
>> rename from Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
>> rename to Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
>> index 5db99cd94b90..d2f5b7bb7a58 100644
>> --- a/Documentation/devicetree/bindings/mmc/hi3798cv200-dw-mshc.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/histb-dw-mshc.yaml
>> @@ -1,11 +1,11 @@
>>   # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>   %YAML 1.2
>>   ---
>> -$id: http://devicetree.org/schemas/mmc/hi3798cv200-dw-mshc.yaml#
>> +$id: http://devicetree.org/schemas/mmc/histb-dw-mshc.yaml#
> Really, one wrong filename into another...
How about "hisilicon,dw-mshc.yaml"? I found rockchip using a similar 
naming: "rockchip-dw-mshc.yaml"
>
>>   $schema: http://devicetree.org/meta-schemas/core.yaml#
>>   
>>   title:
>> -  Hisilicon Hi3798CV200 SoC specific extensions to the Synopsys DWMMC controller
>> +  Hisilicon HiSTB SoCs specific extensions to the Synopsys DWMMC controller
>>   
>>   maintainers:
>>     - Yang Xiwen <forbidden405@outlook.com>
>> @@ -14,16 +14,14 @@ 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.
> Just drop this sentence in previous/conversion patch. It's useless.
Will do in v2.
>
>> -
>> -allOf:
>> -  - $ref: synopsys-dw-mshc-common.yaml#
> Put it in correct place in the first time. Don't needlessly shuffle the
> code right after previous patch.
Will fix in v2.
>
>
>> +  by synopsys-dw-mshc.txt and the properties used by the Hisilicon HiSTB specific
>> +  extensions to the Synopsys Designware Mobile Storage Host Controller.
>>   
>>   properties:
>>     compatible:
>>       enum:
>>         - hisilicon,hi3798cv200-dw-mshc
>> +      - hisilicon,hi3798mv200-dw-mshc
>>   
>>     reg:
>>       maxItems: 1
>> @@ -48,6 +46,12 @@ properties:
>>         control the clock phases, "ciu-sample" is required for tuning
>>         high speed modes.
>>   
>> +  hisilicon,sap-dll-reg:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      A phandle points to the sample delay-locked-loop(DLL)
>> +      syscon node, used for tuning.
> Does hi3798cv200 have it?
No it does not. Currently only hi3798mv200 has it (it's called himci 
v300 in downstream, while cv200 is using himci v200).
>
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -55,13 +59,25 @@ required:
>>     - clocks
>>     - clock-names
>>   
>> +allOf:
>> +  - $ref: synopsys-dw-mshc-common.yaml#
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: hisilicon,hi3798mv200-dw-mshc
>> +    then:
>> +      required:
>> +        - hisilicon,sap-dll-reg
>> +
>>   unevaluatedProperties: false
>>   
>>   examples:
>>     - |
>>       #include <dt-bindings/clock/histb-clock.h>
>>       #include <dt-bindings/interrupt-controller/arm-gic.h>
>> -    emmc: mmc@9830000 {
>> +    mmc@9830000 {
> ???
It's complaining about duplicated label when i added emmc label to both 
nodes. I'll remove it in previous patch in v2.
>>         compatible = "hisilicon,hi3798cv200-dw-mshc";
>>         reg = <0x9830000 0x10000>;
>>         interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>> @@ -84,3 +100,31 @@ examples:
>>         bus-width = <8>;
>>         status = "okay";
>>       };
>> +  - |
>> +    #include <dt-bindings/clock/histb-clock.h>
>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +    mmc@9830000 {
>> +      compatible = "hisilicon,hi3798mv200-dw-mshc";
> No need for new example.
>
>> +      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";
>> +      resets = <&crg 0xa0 4>;
>> +      reset-names = "reset";
>> +      pinctrl-names = "default";
>> +      pinctrl-0 = <&emmc_pins>;
>> +      fifo-depth = <256>;
>> +      clock-frequency = <50000000>;
>> +      max-frequency = <150000000>;
>> +      cap-mmc-highspeed;
>> +      mmc-ddr-1_8v;
>> +      mmc-hs200-1_8v;
>> +      mmc-hs400-1_8v;
>> +      non-removable;
>> +      bus-width = <8>;
>> +      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
>> +      status = "okay";
> No, really...
The property "hisilicon,sap-dll-reg" is introduced in this patch, i want 
to add an example for it here since the common dtsi will use this 
binding and will be submitted when it gets ready.
>
>> +    };
>>
> Best regards,
> Krzysztof
>

-- 
Regards,
Yang Xiwen

Re: [PATCH 3/3] dt-bindings: mmc: dw-mshc-hi3798cv200: rename to dw-mshc-histb

[Krzysztof Kozlowski]

On 16/02/2024 09:29, Yang Xiwen wrote:
>>>     reg:
>>>       maxItems: 1
>>> @@ -48,6 +46,12 @@ properties:
>>>         control the clock phases, "ciu-sample" is required for tuning
>>>         high speed modes.
>>>   
>>> +  hisilicon,sap-dll-reg:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle
>>> +    description:
>>> +      A phandle points to the sample delay-locked-loop(DLL)
>>> +      syscon node, used for tuning.
>> Does hi3798cv200 have it?
> No it does not. Currently only hi3798mv200 has it (it's called himci 
> v300 in downstream, while cv200 is using himci v200).

then in your if:
else:
  properties:
    hisilicon,sap-dll-reg: false

>>
>>> +
>>>   required:
>>>     - compatible
>>>     - reg
>>> @@ -55,13 +59,25 @@ required:
>>>     - clocks
>>>     - clock-names
>>>   
>>> +allOf:
>>> +  - $ref: synopsys-dw-mshc-common.yaml#
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            const: hisilicon,hi3798mv200-dw-mshc
>>> +    then:
>>> +      required:
>>> +        - hisilicon,sap-dll-reg
>>> +
>>>   unevaluatedProperties: false
>>>   
>>>   examples:
>>>     - |
>>>       #include <dt-bindings/clock/histb-clock.h>
>>>       #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> -    emmc: mmc@9830000 {
>>> +    mmc@9830000 {
>> ???
> It's complaining about duplicated label when i added emmc label to both 
> nodes. I'll remove it in previous patch in v2.
>>>         compatible = "hisilicon,hi3798cv200-dw-mshc";
>>>         reg = <0x9830000 0x10000>;
>>>         interrupts = <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>;
>>> @@ -84,3 +100,31 @@ examples:
>>>         bus-width = <8>;
>>>         status = "okay";
>>>       };
>>> +  - |
>>> +    #include <dt-bindings/clock/histb-clock.h>
>>> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>>> +    mmc@9830000 {
>>> +      compatible = "hisilicon,hi3798mv200-dw-mshc";
>> No need for new example.
>>
>>> +      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";
>>> +      resets = <&crg 0xa0 4>;
>>> +      reset-names = "reset";
>>> +      pinctrl-names = "default";
>>> +      pinctrl-0 = <&emmc_pins>;
>>> +      fifo-depth = <256>;
>>> +      clock-frequency = <50000000>;
>>> +      max-frequency = <150000000>;
>>> +      cap-mmc-highspeed;
>>> +      mmc-ddr-1_8v;
>>> +      mmc-hs200-1_8v;
>>> +      mmc-hs400-1_8v;
>>> +      non-removable;
>>> +      bus-width = <8>;
>>> +      hisilicon,sap-dll-reg = <&emmc_sap_dll_reg>;
>>> +      status = "okay";
>> No, really...
> The property "hisilicon,sap-dll-reg" is introduced in this patch, i want 
> to add an example for it here since the common dtsi will use this 
> binding and will be submitted when it gets ready.

One new property does not justify new example.

Best regards,
Krzysztof