[PATCH v2 0/3] StarFive's SDIO/eMMC driver support

[PATCH v2 0/3] StarFive's SDIO/eMMC driver support

[William Qiu]

Hi,

This patchset adds initial rudimentary support for the StarFive
designware mobile storage host controller driver. And this driver will
be used in StarFive's VisionFive 2 board. The main purpose of adding
this driver is to accommodate the ultra-high speed mode of eMMC.

The last patch should be applied after the patchset [1]:
[1] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/

Changes since v1:
- Renamed the dt-binding 'starfive,jh7110-sdio.yaml' to 'starfive,jh7110-mmc.yaml'.
- Changed the type of 'starfive,syscon' and modify its description.
- Deleted unused head files like '#include <linux/gpio.h>'.
- Added comment for the 'rise_point' and 'fall_point'.
- Changed the API 'num_caps' to 'common_caps'.
- Changed the node name 'sys_syscon' to 'syscon'.
- Changed the node name 'sdio' to 'mmc'.

The patch series is based on v6.1-rc5.

William Qiu (3):
  dt-bindings: mmc: Add bindings for StarFive
  mmc: starfive: Add sdio/emmc driver support
  riscv: dts: starfive: Add mmc node

 .../bindings/mmc/starfive,jh7110-mmc.yaml     |  72 +++++++
 MAINTAINERS                                   |   6 +
 .../jh7110-starfive-visionfive-v2.dts         |  25 +++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  38 ++++
 drivers/mmc/host/Kconfig                      |  10 +
 drivers/mmc/host/Makefile                     |   1 +
 drivers/mmc/host/dw_mmc-starfive.c            | 185 ++++++++++++++++++
 7 files changed, 337 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
 create mode 100644 drivers/mmc/host/dw_mmc-starfive.c

--
2.34.1

[PATCH v2 1/3] dt-bindings: mmc: Add bindings for StarFive

[William Qiu]

Add documentation to describe StarFive
designware mobile storage host controller driver.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../bindings/mmc/starfive,jh7110-mmc.yaml     | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml

diff --git a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
new file mode 100644
index 000000000000..430dd5f24933
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mmc/starfive,jh7110-mmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive Designware Mobile Storage Host Controller
+
+description:
+  StarFive uses the Synopsys designware mobile storage host controller
+  to interface a SoC with storage medium such as eMMC or SD/MMC cards.
+
+allOf:
+  - $ref: synopsys-dw-mshc-common.yaml#
+
+maintainers:
+  - William Qiu <william.qiu@starfivetech.com>
+
+properties:
+  compatible:
+    const: starfive,jh7110-mmc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: biu clock
+      - description: ciu clock
+
+  clock-names:
+    items:
+      - const: biu
+      - const: ciu
+
+  interrupts:
+    maxItems: 1
+
+  starfive,syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      arg0:arg0 is syscon.
+      arg1:arg1 is syscon register offset, used to enable MMC function.
+      arg2:arg2 is used to enable the register shift of the MMC function.
+      arg3:arg3 is used to enable the register mask of the MMC function.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - starfive,syscon
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    mmc@16010000 {
+        compatible = "starfive,jh7110-mmc";
+        reg = <0x16010000 0x10000>;
+        clocks = <&syscrg 91>,
+                 <&syscrg 93>;
+        clock-names = "biu","ciu";
+        resets = <&syscrg 64>;
+        reset-names = "reset";
+        interrupts = <74>;
+        fifo-depth = <32>;
+        fifo-watermark-aligned;
+        data-addr = <0>;
+        starfive,syscon = <&syscon 0x14 0x1a 0x7c000000>;
+    };
-- 
2.34.1

Re: [PATCH v2 1/3] dt-bindings: mmc: Add bindings for StarFive

[Krzysztof Kozlowski]

On 27/12/2022 13:22, William Qiu wrote:
> Add documentation to describe StarFive
> designware mobile storage host controller driver.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586

The subject is a bit redundant and not precise. No need to mention
"bindings" twice but "StarFive" is really too generic. Do you add now
all possible devices from StarFive? I see only one. Rephrase (use git
log to find examples).

> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../bindings/mmc/starfive,jh7110-mmc.yaml     | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
> new file mode 100644
> index 000000000000..430dd5f24933
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mmc/starfive,jh7110-mmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive Designware Mobile Storage Host Controller
> +
> +description:
> +  StarFive uses the Synopsys designware mobile storage host controller
> +  to interface a SoC with storage medium such as eMMC or SD/MMC cards.
> +
> +allOf:
> +  - $ref: synopsys-dw-mshc-common.yaml#
> +
> +maintainers:
> +  - William Qiu <william.qiu@starfivetech.com>
> +
> +properties:
> +  compatible:
> +    const: starfive,jh7110-mmc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: biu clock
> +      - description: ciu clock
> +
> +  clock-names:
> +    items:
> +      - const: biu
> +      - const: ciu
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  starfive,syscon:

Name is not specific enough. What is syscon? Any syscon? This needs to
be specific.

> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      arg0:arg0 is syscon.

What is syscon?

> +      arg1:arg1 is syscon register offset, used to enable MMC function.

Describe the argument, not what it is used for, e.g. "offset of XXX YYY ZZZ"

> +      arg2:arg2 is used to enable the register shift of the MMC function.
> +      arg3:arg3 is used to enable the register mask of the MMC function.

That's not how the phandle array is described. Instead:

https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42



Best regards,
Krzysztof

Re: [PATCH v2 1/3] dt-bindings: mmc: Add bindings for StarFive

[William Qiu]

On 2022/12/27 21:05, Krzysztof Kozlowski wrote:
> On 27/12/2022 13:22, William Qiu wrote:
>> Add documentation to describe StarFive
>> designware mobile storage host controller driver.
> 
> Please wrap commit message according to Linux coding style / submission
> process (neither too early nor over the limit):
> https://elixir.bootlin.com/linux/v5.18-rc4/source/Documentation/process/submitting-patches.rst#L586
> 
> The subject is a bit redundant and not precise. No need to mention
> "bindings" twice but "StarFive" is really too generic. Do you add now
> all possible devices from StarFive? I see only one. Rephrase (use git
> log to find examples).
> 

Will update

>> 
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>>  .../bindings/mmc/starfive,jh7110-mmc.yaml     | 72 +++++++++++++++++++
>>  1 file changed, 72 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
>> new file mode 100644
>> index 000000000000..430dd5f24933
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
>> @@ -0,0 +1,72 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/mmc/starfive,jh7110-mmc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive Designware Mobile Storage Host Controller
>> +
>> +description:
>> +  StarFive uses the Synopsys designware mobile storage host controller
>> +  to interface a SoC with storage medium such as eMMC or SD/MMC cards.
>> +
>> +allOf:
>> +  - $ref: synopsys-dw-mshc-common.yaml#
>> +
>> +maintainers:
>> +  - William Qiu <william.qiu@starfivetech.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-mmc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: biu clock
>> +      - description: ciu clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: biu
>> +      - const: ciu
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  starfive,syscon:
> 
> Name is not specific enough. What is syscon? Any syscon? This needs to
> be specific.
> 

Will update.

>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description:
>> +      arg0:arg0 is syscon.
> 
> What is syscon?
> 

Will update.

>> +      arg1:arg1 is syscon register offset, used to enable MMC function.
> 
> Describe the argument, not what it is used for, e.g. "offset of XXX YYY ZZZ"
> 
>> +      arg2:arg2 is used to enable the register shift of the MMC function.
>> +      arg3:arg3 is used to enable the register mask of the MMC function.
> 
> That's not how the phandle array is described. Instead:
> 
> https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42
> 

Hi Krzysztof,

Thank you for taking time to review and provide helpful comments for this patch.
I will fix all the above issues in the next version.

Best Regards,
William Qiu

> 
> 
> Best regards,
> Krzysztof
> 

[PATCH v2 2/3] mmc: starfive: Add sdio/emmc driver support

[William Qiu]

Add sdio/emmc driver support for StarFive JH7110 soc.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 MAINTAINERS                        |   6 +
 drivers/mmc/host/Kconfig           |  10 ++
 drivers/mmc/host/Makefile          |   1 +
 drivers/mmc/host/dw_mmc-starfive.c | 185 +++++++++++++++++++++++++++++
 4 files changed, 202 insertions(+)
 create mode 100644 drivers/mmc/host/dw_mmc-starfive.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a70c1d0f303e..2b46ef07f5dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19623,6 +19623,12 @@ F:	Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
 F:	drivers/reset/starfive/
 F:	include/dt-bindings/reset/starfive*
 
+STARFIVE JH7110 MMC/SD/SDIO DRIVER
+M:	William Qiu <william.qiu@starfivetech.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mmc/starfive*
+F:	drivers/mmc/dw_mmc-starfive.c
+
 STATIC BRANCH/CALL
 M:	Peter Zijlstra <peterz@infradead.org>
 M:	Josh Poimboeuf <jpoimboe@kernel.org>
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index fb1062a6394c..b87262503403 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -871,6 +871,16 @@ config MMC_DW_ROCKCHIP
 	  Synopsys DesignWare Memory Card Interface driver. Select this option
 	  for platforms based on RK3066, RK3188 and RK3288 SoC's.
 
+config MMC_DW_STARFIVE
+	tristate "StarFive specific extensions for Synopsys DW Memory Card Interface"
+	depends on SOC_STARFIVE
+	depends on MMC_DW
+	select MMC_DW_PLTFM
+	help
+	  This selects support for StarFive JH7110 SoC specific extensions to the
+	  Synopsys DesignWare Memory Card Interface driver. Select this option
+	  for platforms based on StarFive JH7110 SoC.
+
 config MMC_SH_MMCIF
 	tristate "SuperH Internal MMCIF support"
 	depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 4e4ceb32c4b4..32c0e5564b9a 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_MMC_DW_HI3798CV200) += dw_mmc-hi3798cv200.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
+obj-$(CONFIG_MMC_DW_STARFIVE)	+= dw_mmc-starfive.o
 obj-$(CONFIG_MMC_SH_MMCIF)	+= sh_mmcif.o
 obj-$(CONFIG_MMC_JZ4740)	+= jz4740_mmc.o
 obj-$(CONFIG_MMC_VUB300)	+= vub300.o
diff --git a/drivers/mmc/host/dw_mmc-starfive.c b/drivers/mmc/host/dw_mmc-starfive.c
new file mode 100644
index 000000000000..e4d0bdb40d12
--- /dev/null
+++ b/drivers/mmc/host/dw_mmc-starfive.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * StarFive Designware Mobile Storage Host Controller Driver
+ *
+ * Copyright (c) 2022 StarFive Technology Co., Ltd.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.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 ALL_INT_CLR		0x1ffff
+#define MAX_DELAY_CHAIN		32
+
+struct starfive_priv {
+	struct device *dev;
+	struct regmap *reg_syscon;
+	u32 syscon_offset;
+	u32 syscon_shift;
+	u32 syscon_mask;
+};
+
+static void dw_mci_starfive_set_ios(struct dw_mci *host, struct mmc_ios *ios)
+{
+	int ret;
+	unsigned int clock;
+
+	if (ios->timing == MMC_TIMING_MMC_DDR52 || ios->timing == MMC_TIMING_UHS_DDR50) {
+		clock = (ios->clock > 50000000 && ios->clock <= 52000000) ? 100000000 : ios->clock;
+		ret = clk_set_rate(host->ciu_clk, clock);
+		if (ret)
+			dev_dbg(host->dev, "Use an external frequency divider %uHz\n", ios->clock);
+		host->bus_hz = clk_get_rate(host->ciu_clk);
+	} else {
+		dev_dbg(host->dev, "Using the internal divider\n");
+	}
+}
+
+static int dw_mci_starfive_execute_tuning(struct dw_mci_slot *slot,
+					     u32 opcode)
+{
+	static const int grade  = MAX_DELAY_CHAIN;
+	struct dw_mci *host = slot->host;
+	struct starfive_priv *priv = host->priv;
+	int rise_point = -1, fall_point = -1;
+	int err, prev_err;
+	int i;
+	bool found = 0;
+	u32 regval;
+
+	/* Use grade as the max delay chain, and use the rise_point and 
+	 * fall_point to ensure the best sampling point of a data input
+	 * signals.
+	 */
+	for (i = 0; i < grade; i++) {
+		regval = i << priv->syscon_shift;
+		err = regmap_update_bits(priv->reg_syscon, priv->syscon_offset,
+						priv->syscon_mask, regval);
+		if (err)
+			return err;
+		mci_writel(host, RINTSTS, ALL_INT_CLR);
+
+		err = mmc_send_tuning(slot->mmc, opcode, NULL);
+		if (!err)
+			found = 1;
+
+		if (i > 0) {
+			if (err && !prev_err)
+				fall_point = i - 1;
+			if (!err && prev_err)
+				rise_point = i;
+		}
+
+		if (rise_point != -1 && fall_point != -1)
+			goto tuning_out;
+
+		prev_err = err;
+		err = 0;
+	}
+
+tuning_out:
+	if (found) {
+		if (rise_point == -1)
+			rise_point = 0;
+		if (fall_point == -1)
+			fall_point = grade - 1;
+		if (fall_point < rise_point) {
+			if ((rise_point + fall_point) >
+			    (grade - 1))
+				i = fall_point / 2;
+			else
+				i = (rise_point + grade - 1) / 2;
+		} else {
+			i = (rise_point + fall_point) / 2;
+		}
+
+		regval = i << priv->syscon_shift;
+		err = regmap_update_bits(priv->reg_syscon, priv->syscon_offset,
+						priv->syscon_mask, regval);
+		if (err)
+			return err;
+		mci_writel(host, RINTSTS, ALL_INT_CLR);
+
+		dev_info(host->dev, "Found valid delay chain! use it [delay=%d]\n", i);
+	} else {
+		dev_err(host->dev, "No valid delay chain! use default\n");
+		err = -EINVAL;
+	}
+
+	mci_writel(host, RINTSTS, ALL_INT_CLR);
+	return err;
+}
+
+static int dw_mci_starfive_parse_dt(struct dw_mci *host)
+{
+	struct of_phandle_args args;
+	struct starfive_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(host->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	ret = of_parse_phandle_with_fixed_args(host->dev->of_node,
+						"starfive,syscon", 3, 0, &args);
+	if (ret) {
+		dev_err(host->dev, "Failed to parse starfive,syscon\n");
+		return -EINVAL;
+	}
+
+	priv->reg_syscon = syscon_node_to_regmap(args.np);
+	of_node_put(args.np);
+	if (IS_ERR(priv->reg_syscon))
+		return PTR_ERR(priv->reg_syscon);
+
+	priv->syscon_offset = args.args[0];
+	priv->syscon_shift  = args.args[1];
+	priv->syscon_mask   = args.args[2];
+
+	host->priv = priv;
+
+	return 0;
+}
+
+static const struct dw_mci_drv_data starfive_data = {
+	.common_caps 		= MMC_CAP_CMD23,
+	.set_ios 		= dw_mci_starfive_set_ios,
+	.parse_dt 		= dw_mci_starfive_parse_dt,
+	.execute_tuning 	= dw_mci_starfive_execute_tuning,
+};
+
+static const struct of_device_id dw_mci_starfive_match[] = {
+	{ .compatible = "starfive,jh7110-mmc",
+		.data = &starfive_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, dw_mci_starfive_match);
+
+static int dw_mci_starfive_probe(struct platform_device *pdev)
+{	
+	return dw_mci_pltfm_register(pdev, &starfive_data);;
+}
+
+static struct platform_driver dw_mci_starfive_driver = {
+	.probe = dw_mci_starfive_probe,
+	.remove = dw_mci_pltfm_remove,
+	.driver = {
+		.name = "dwmmc_starfive",
+		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
+		.of_match_table = dw_mci_starfive_match,
+	},
+};
+module_platform_driver(dw_mci_starfive_driver);
+
+MODULE_DESCRIPTION("StarFive JH7110 Specific DW-MSHC Driver Extension");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:dwmmc_starfive");
-- 
2.34.1

Re: [PATCH v2 2/3] mmc: starfive: Add sdio/emmc driver support

[Conor Dooley]

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

On Tue, Dec 27, 2022 at 08:22:26PM +0800, William Qiu wrote:
> Add sdio/emmc driver support for StarFive JH7110 soc.
> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>

Gave the patch a go w/ a chroot running on the sdcard. Seemed fine :)
Tested-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.


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

[PATCH v2 3/3] riscv: dts: starfive: Add mmc node

[William Qiu]

This adds the mmc node for the StarFive JH7110 SoC.
Set sdioo node to emmc and set sdio1 node to sd.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../jh7110-starfive-visionfive-v2.dts         | 25 ++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 38 +++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
index c8946cf3a268..d8244fd1f5a0 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
@@ -47,6 +47,31 @@ &clk_rtc {
 	clock-frequency = <32768>;
 };
 
+&mmc0 {
+	max-frequency = <100000000>;
+	card-detect-delay = <300>;
+	bus-width = <8>;
+	cap-mmc-highspeed;
+	mmc-ddr-1_8v;
+	mmc-hs200-1_8v;
+	non-removable;
+	cap-mmc-hw-reset;
+	post-power-on-delay-ms = <200>;
+	status = "okay";
+};
+
+&mmc1 {
+	max-frequency = <100000000>;
+	card-detect-delay = <300>;
+	bus-width = <4>;
+	no-sdio;
+	no-mmc;
+	broken-cd;
+	cap-sd-highspeed;
+	post-power-on-delay-ms = <200>;
+	status = "okay";
+};
+
 &gmac0_rmii_refin {
 	clock-frequency = <50000000>;
 };
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index c22e8f1d2640..08a780d2c0f4 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -331,6 +331,11 @@ aoncrg: clock-controller@17000000 {
 			#reset-cells = <1>;
 		};
 
+		syscon: syscon@13030000 {
+			compatible = "starfive,syscon", "syscon";
+			reg = <0x0 0x13030000 0x0 0x1000>;
+		};
+
 		gpio: gpio@13040000 {
 			compatible = "starfive,jh7110-sys-pinctrl";
 			reg = <0x0 0x13040000 0x0 0x10000>;
@@ -433,5 +438,38 @@ uart5: serial@12020000 {
 			reg-shift = <2>;
 			status = "disabled";
 		};
+
+		/* unremovable emmc as mmcblk0 */
+		mmc0: mmc@16010000 {
+			compatible = "starfive,jh7110-mmc";
+			reg = <0x0 0x16010000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SDIO0_AHB>,
+				 <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>;
+			clock-names = "biu","ciu";
+			resets = <&syscrg JH7110_SYSRST_SDIO0_AHB>;
+			reset-names = "reset";
+			interrupts = <74>;
+			fifo-depth = <32>;
+			fifo-watermark-aligned;
+			data-addr = <0>;
+			starfive,syscon = <&syscon 0x14 0x1a 0x7c000000>;
+			status = "disabled";
+		};
+
+		mmc1: mmc@16020000 {
+			compatible = "starfive,jh7110-mmc";
+			reg = <0x0 0x16020000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SDIO1_AHB>,
+				 <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>;
+			clock-names = "biu","ciu";
+			resets = <&syscrg JH7110_SYSRST_SDIO1_AHB>;
+			reset-names = "reset";
+			interrupts = <75>;
+			fifo-depth = <32>;
+			fifo-watermark-aligned;
+			data-addr = <0>;
+			starfive,syscon = <&syscon 0x9c 0x1 0x3e>;
+			status = "disabled";
+		};
 	};
 };
-- 
2.34.1

Re: [PATCH v2 3/3] riscv: dts: starfive: Add mmc node

[Ulf Hansson]

On Tue, 27 Dec 2022 at 13:22, William Qiu <william.qiu@starfivetech.com> wrote:
>
> This adds the mmc node for the StarFive JH7110 SoC.
> Set sdioo node to emmc and set sdio1 node to sd.
>
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../jh7110-starfive-visionfive-v2.dts         | 25 ++++++++++++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 38 +++++++++++++++++++
>  2 files changed, 63 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> index c8946cf3a268..d8244fd1f5a0 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> @@ -47,6 +47,31 @@ &clk_rtc {
>         clock-frequency = <32768>;
>  };
>
> +&mmc0 {
> +       max-frequency = <100000000>;
> +       card-detect-delay = <300>;

Nitpick:  This seems redundant for a non-removable card!?

> +       bus-width = <8>;
> +       cap-mmc-highspeed;
> +       mmc-ddr-1_8v;
> +       mmc-hs200-1_8v;
> +       non-removable;
> +       cap-mmc-hw-reset;
> +       post-power-on-delay-ms = <200>;
> +       status = "okay";
> +};
> +
> +&mmc1 {
> +       max-frequency = <100000000>;
> +       card-detect-delay = <300>;

Nitpick: This looks redundant for polling based card detection
(broken-cd is set a few lines below).

> +       bus-width = <4>;
> +       no-sdio;
> +       no-mmc;
> +       broken-cd;
> +       cap-sd-highspeed;
> +       post-power-on-delay-ms = <200>;
> +       status = "okay";
> +};
> +
>  &gmac0_rmii_refin {
>         clock-frequency = <50000000>;
>  };
> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> index c22e8f1d2640..08a780d2c0f4 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -331,6 +331,11 @@ aoncrg: clock-controller@17000000 {
>                         #reset-cells = <1>;
>                 };
>
> +               syscon: syscon@13030000 {
> +                       compatible = "starfive,syscon", "syscon";
> +                       reg = <0x0 0x13030000 0x0 0x1000>;
> +               };
> +
>                 gpio: gpio@13040000 {
>                         compatible = "starfive,jh7110-sys-pinctrl";
>                         reg = <0x0 0x13040000 0x0 0x10000>;
> @@ -433,5 +438,38 @@ uart5: serial@12020000 {
>                         reg-shift = <2>;
>                         status = "disabled";
>                 };
> +
> +               /* unremovable emmc as mmcblk0 */

Don't confuse the mmc0 node name with mmcblk0. There is no guarantee
that this is true, unless you also specify an alias.

> +               mmc0: mmc@16010000 {
> +                       compatible = "starfive,jh7110-mmc";
> +                       reg = <0x0 0x16010000 0x0 0x10000>;
> +                       clocks = <&syscrg JH7110_SYSCLK_SDIO0_AHB>,
> +                                <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>;
> +                       clock-names = "biu","ciu";
> +                       resets = <&syscrg JH7110_SYSRST_SDIO0_AHB>;
> +                       reset-names = "reset";
> +                       interrupts = <74>;
> +                       fifo-depth = <32>;
> +                       fifo-watermark-aligned;
> +                       data-addr = <0>;
> +                       starfive,syscon = <&syscon 0x14 0x1a 0x7c000000>;
> +                       status = "disabled";
> +               };
> +
> +               mmc1: mmc@16020000 {
> +                       compatible = "starfive,jh7110-mmc";
> +                       reg = <0x0 0x16020000 0x0 0x10000>;
> +                       clocks = <&syscrg JH7110_SYSCLK_SDIO1_AHB>,
> +                                <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>;
> +                       clock-names = "biu","ciu";
> +                       resets = <&syscrg JH7110_SYSRST_SDIO1_AHB>;
> +                       reset-names = "reset";
> +                       interrupts = <75>;
> +                       fifo-depth = <32>;
> +                       fifo-watermark-aligned;
> +                       data-addr = <0>;
> +                       starfive,syscon = <&syscon 0x9c 0x1 0x3e>;
> +                       status = "disabled";
> +               };
>         };
>  };

Kind regards
Uffe

Re: [PATCH v2 3/3] riscv: dts: starfive: Add mmc node

[William Qiu]

On 2023/1/2 22:03, Ulf Hansson wrote:
> On Tue, 27 Dec 2022 at 13:22, William Qiu <william.qiu@starfivetech.com> wrote:
>>
>> This adds the mmc node for the StarFive JH7110 SoC.
>> Set sdioo node to emmc and set sdio1 node to sd.
>>
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>>  .../jh7110-starfive-visionfive-v2.dts         | 25 ++++++++++++
>>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 38 +++++++++++++++++++
>>  2 files changed, 63 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> index c8946cf3a268..d8244fd1f5a0 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> @@ -47,6 +47,31 @@ &clk_rtc {
>>         clock-frequency = <32768>;
>>  };
>>
>> +&mmc0 {
>> +       max-frequency = <100000000>;
>> +       card-detect-delay = <300>;
> 
> Nitpick:  This seems redundant for a non-removable card!?
> 

Will drop

>> +       bus-width = <8>;
>> +       cap-mmc-highspeed;
>> +       mmc-ddr-1_8v;
>> +       mmc-hs200-1_8v;
>> +       non-removable;
>> +       cap-mmc-hw-reset;
>> +       post-power-on-delay-ms = <200>;
>> +       status = "okay";
>> +};
>> +
>> +&mmc1 {
>> +       max-frequency = <100000000>;
>> +       card-detect-delay = <300>;
> 
> Nitpick: This looks redundant for polling based card detection
> (broken-cd is set a few lines below).
> 

Will drop

>> +       bus-width = <4>;
>> +       no-sdio;
>> +       no-mmc;
>> +       broken-cd;
>> +       cap-sd-highspeed;
>> +       post-power-on-delay-ms = <200>;
>> +       status = "okay";
>> +};
>> +
>>  &gmac0_rmii_refin {
>>         clock-frequency = <50000000>;
>>  };
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index c22e8f1d2640..08a780d2c0f4 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -331,6 +331,11 @@ aoncrg: clock-controller@17000000 {
>>                         #reset-cells = <1>;
>>                 };
>>
>> +               syscon: syscon@13030000 {
>> +                       compatible = "starfive,syscon", "syscon";
>> +                       reg = <0x0 0x13030000 0x0 0x1000>;
>> +               };
>> +
>>                 gpio: gpio@13040000 {
>>                         compatible = "starfive,jh7110-sys-pinctrl";
>>                         reg = <0x0 0x13040000 0x0 0x10000>;
>> @@ -433,5 +438,38 @@ uart5: serial@12020000 {
>>                         reg-shift = <2>;
>>                         status = "disabled";
>>                 };
>> +
>> +               /* unremovable emmc as mmcblk0 */
> 
> Don't confuse the mmc0 node name with mmcblk0. There is no guarantee
> that this is true, unless you also specify an alias.
> 

Hi Ulf,

Thank you for taking time to review and provide helpful comments for this patch.
Actually we define mmc0 as eMMC, which is mmcblk0 in the kernel, and define mmc1 as SDIO,
which is mmcblk1 in the kernel, so it's not confuse.

Best Regards
William Qiu
>> +               mmc0: mmc@16010000 {
>> +                       compatible = "starfive,jh7110-mmc";
>> +                       reg = <0x0 0x16010000 0x0 0x10000>;
>> +                       clocks = <&syscrg JH7110_SYSCLK_SDIO0_AHB>,
>> +                                <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>;
>> +                       clock-names = "biu","ciu";
>> +                       resets = <&syscrg JH7110_SYSRST_SDIO0_AHB>;
>> +                       reset-names = "reset";
>> +                       interrupts = <74>;
>> +                       fifo-depth = <32>;
>> +                       fifo-watermark-aligned;
>> +                       data-addr = <0>;
>> +                       starfive,syscon = <&syscon 0x14 0x1a 0x7c000000>;
>> +                       status = "disabled";
>> +               };
>> +
>> +               mmc1: mmc@16020000 {
>> +                       compatible = "starfive,jh7110-mmc";
>> +                       reg = <0x0 0x16020000 0x0 0x10000>;
>> +                       clocks = <&syscrg JH7110_SYSCLK_SDIO1_AHB>,
>> +                                <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>;
>> +                       clock-names = "biu","ciu";
>> +                       resets = <&syscrg JH7110_SYSRST_SDIO1_AHB>;
>> +                       reset-names = "reset";
>> +                       interrupts = <75>;
>> +                       fifo-depth = <32>;
>> +                       fifo-watermark-aligned;
>> +                       data-addr = <0>;
>> +                       starfive,syscon = <&syscon 0x9c 0x1 0x3e>;
>> +                       status = "disabled";
>> +               };
>>         };
>>  };
> 
> Kind regards
> Uffe

Re: [PATCH v2 3/3] riscv: dts: starfive: Add mmc node

[Ulf Hansson]

On Wed, 4 Jan 2023 at 07:08, William Qiu <william.qiu@starfivetech.com> wrote:
>
>
>
> On 2023/1/2 22:03, Ulf Hansson wrote:
> > On Tue, 27 Dec 2022 at 13:22, William Qiu <william.qiu@starfivetech.com> wrote:
> >>
> >> This adds the mmc node for the StarFive JH7110 SoC.
> >> Set sdioo node to emmc and set sdio1 node to sd.
> >>
> >> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> >> ---
> >>  .../jh7110-starfive-visionfive-v2.dts         | 25 ++++++++++++
> >>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 38 +++++++++++++++++++
> >>  2 files changed, 63 insertions(+)
> >>
> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> >> index c8946cf3a268..d8244fd1f5a0 100644
> >> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
> >> @@ -47,6 +47,31 @@ &clk_rtc {
> >>         clock-frequency = <32768>;
> >>  };
> >>
> >> +&mmc0 {
> >> +       max-frequency = <100000000>;
> >> +       card-detect-delay = <300>;
> >
> > Nitpick:  This seems redundant for a non-removable card!?
> >
>
> Will drop
>
> >> +       bus-width = <8>;
> >> +       cap-mmc-highspeed;
> >> +       mmc-ddr-1_8v;
> >> +       mmc-hs200-1_8v;
> >> +       non-removable;
> >> +       cap-mmc-hw-reset;
> >> +       post-power-on-delay-ms = <200>;
> >> +       status = "okay";
> >> +};
> >> +
> >> +&mmc1 {
> >> +       max-frequency = <100000000>;
> >> +       card-detect-delay = <300>;
> >
> > Nitpick: This looks redundant for polling based card detection
> > (broken-cd is set a few lines below).
> >
>
> Will drop
>
> >> +       bus-width = <4>;
> >> +       no-sdio;
> >> +       no-mmc;
> >> +       broken-cd;
> >> +       cap-sd-highspeed;
> >> +       post-power-on-delay-ms = <200>;
> >> +       status = "okay";
> >> +};
> >> +
> >>  &gmac0_rmii_refin {
> >>         clock-frequency = <50000000>;
> >>  };
> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >> index c22e8f1d2640..08a780d2c0f4 100644
> >> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> >> @@ -331,6 +331,11 @@ aoncrg: clock-controller@17000000 {
> >>                         #reset-cells = <1>;
> >>                 };
> >>
> >> +               syscon: syscon@13030000 {
> >> +                       compatible = "starfive,syscon", "syscon";
> >> +                       reg = <0x0 0x13030000 0x0 0x1000>;
> >> +               };
> >> +
> >>                 gpio: gpio@13040000 {
> >>                         compatible = "starfive,jh7110-sys-pinctrl";
> >>                         reg = <0x0 0x13040000 0x0 0x10000>;
> >> @@ -433,5 +438,38 @@ uart5: serial@12020000 {
> >>                         reg-shift = <2>;
> >>                         status = "disabled";
> >>                 };
> >> +
> >> +               /* unremovable emmc as mmcblk0 */
> >
> > Don't confuse the mmc0 node name with mmcblk0. There is no guarantee
> > that this is true, unless you also specify an alias.
> >
>
> Hi Ulf,
>
> Thank you for taking time to review and provide helpful comments for this patch.
> Actually we define mmc0 as eMMC, which is mmcblk0 in the kernel, and define mmc1 as SDIO,
> which is mmcblk1 in the kernel, so it's not confuse.
>

My point is, mmc0 from DT node perspective doesn't necessarily need to
map to mmc0, as that depends on the "probe" order of the devices. At
least for the Linux kernel, mmc0 from DT point of view, could end up
being mmc1.

To avoid confusion, please drop the "mmcblk*" here. It's anyway a
Linux specific thing. Don't get me wrong, feel free to keep the
information about eMMC and SDIO for the corresponding mmc controller
node.

Moreover, if you can't use PARTID/UUID to find the rootfs device -
then you may use an aliases node, to let mmc0 to be enumerated as
mmc0, for example. See below.

aliases {
     mmc0 = &mmc0;
}

Kind regards
Uffe

Re: [PATCH v2 3/3] riscv: dts: starfive: Add mmc node

[William Qiu]

On 2023/1/5 0:05, Ulf Hansson wrote:
> On Wed, 4 Jan 2023 at 07:08, William Qiu <william.qiu@starfivetech.com> wrote:
>>
>>
>>
>> On 2023/1/2 22:03, Ulf Hansson wrote:
>> > On Tue, 27 Dec 2022 at 13:22, William Qiu <william.qiu@starfivetech.com> wrote:
>> >>
>> >> This adds the mmc node for the StarFive JH7110 SoC.
>> >> Set sdioo node to emmc and set sdio1 node to sd.
>> >>
>> >> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> >> ---
>> >>  .../jh7110-starfive-visionfive-v2.dts         | 25 ++++++++++++
>> >>  arch/riscv/boot/dts/starfive/jh7110.dtsi      | 38 +++++++++++++++++++
>> >>  2 files changed, 63 insertions(+)
>> >>
>> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> >> index c8946cf3a268..d8244fd1f5a0 100644
>> >> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> >> @@ -47,6 +47,31 @@ &clk_rtc {
>> >>         clock-frequency = <32768>;
>> >>  };
>> >>
>> >> +&mmc0 {
>> >> +       max-frequency = <100000000>;
>> >> +       card-detect-delay = <300>;
>> >
>> > Nitpick:  This seems redundant for a non-removable card!?
>> >
>>
>> Will drop
>>
>> >> +       bus-width = <8>;
>> >> +       cap-mmc-highspeed;
>> >> +       mmc-ddr-1_8v;
>> >> +       mmc-hs200-1_8v;
>> >> +       non-removable;
>> >> +       cap-mmc-hw-reset;
>> >> +       post-power-on-delay-ms = <200>;
>> >> +       status = "okay";
>> >> +};
>> >> +
>> >> +&mmc1 {
>> >> +       max-frequency = <100000000>;
>> >> +       card-detect-delay = <300>;
>> >
>> > Nitpick: This looks redundant for polling based card detection
>> > (broken-cd is set a few lines below).
>> >
>>
>> Will drop
>>
>> >> +       bus-width = <4>;
>> >> +       no-sdio;
>> >> +       no-mmc;
>> >> +       broken-cd;
>> >> +       cap-sd-highspeed;
>> >> +       post-power-on-delay-ms = <200>;
>> >> +       status = "okay";
>> >> +};
>> >> +
>> >>  &gmac0_rmii_refin {
>> >>         clock-frequency = <50000000>;
>> >>  };
>> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> >> index c22e8f1d2640..08a780d2c0f4 100644
>> >> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> >> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> >> @@ -331,6 +331,11 @@ aoncrg: clock-controller@17000000 {
>> >>                         #reset-cells = <1>;
>> >>                 };
>> >>
>> >> +               syscon: syscon@13030000 {
>> >> +                       compatible = "starfive,syscon", "syscon";
>> >> +                       reg = <0x0 0x13030000 0x0 0x1000>;
>> >> +               };
>> >> +
>> >>                 gpio: gpio@13040000 {
>> >>                         compatible = "starfive,jh7110-sys-pinctrl";
>> >>                         reg = <0x0 0x13040000 0x0 0x10000>;
>> >> @@ -433,5 +438,38 @@ uart5: serial@12020000 {
>> >>                         reg-shift = <2>;
>> >>                         status = "disabled";
>> >>                 };
>> >> +
>> >> +               /* unremovable emmc as mmcblk0 */
>> >
>> > Don't confuse the mmc0 node name with mmcblk0. There is no guarantee
>> > that this is true, unless you also specify an alias.
>> >
>>
>> Hi Ulf,
>>
>> Thank you for taking time to review and provide helpful comments for this patch.
>> Actually we define mmc0 as eMMC, which is mmcblk0 in the kernel, and define mmc1 as SDIO,
>> which is mmcblk1 in the kernel, so it's not confuse.
>>
> 
> My point is, mmc0 from DT node perspective doesn't necessarily need to
> map to mmc0, as that depends on the "probe" order of the devices. At
> least for the Linux kernel, mmc0 from DT point of view, could end up
> being mmc1.
> 
> To avoid confusion, please drop the "mmcblk*" here. It's anyway a
> Linux specific thing. Don't get me wrong, feel free to keep the
> information about eMMC and SDIO for the corresponding mmc controller
> node.
> 
> Moreover, if you can't use PARTID/UUID to find the rootfs device -
> then you may use an aliases node, to let mmc0 to be enumerated as
> mmc0, for example. See below.
> 
> aliases {
>      mmc0 = &mmc0;
> }
> 
> Kind regards
> Uffe

Hi Uffe,

Thank you for taking time to review.
I'll take your suggestion into consideration and drop the "mmcblk*".

Best Regards
William Qiu

Re: [PATCH v2 3/3] riscv: dts: starfive: Add mmc node

[Conor Dooley]

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

Hey William,

On Tue, Dec 27, 2022 at 08:22:27PM +0800, William Qiu wrote:
> This adds the mmc node for the StarFive JH7110 SoC.
> Set sdioo node to emmc and set sdio1 node to sd.
> 
> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
> ---
>  .../jh7110-starfive-visionfive-v2.dts         | 25 ++++++++++++

FYI, this file does not exist in the v3 Devicetree patchset sent by Hal
Feng:
https://lore.kernel.org/linux-riscv/20221220011247.35560-1-hal.feng@starfivetech.com

Would you make sure that future revisions take into account that there
is now a jh7110-starfive-visionfive-2.dtsi file instead?

Thanks,
Conor.


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

Re: [PATCH v2 3/3] riscv: dts: starfive: Add mmc node

[William Qiu]

On 2023/1/20 2:43, Conor Dooley wrote:
> Hey William,
> 
> On Tue, Dec 27, 2022 at 08:22:27PM +0800, William Qiu wrote:
>> This adds the mmc node for the StarFive JH7110 SoC.
>> Set sdioo node to emmc and set sdio1 node to sd.
>> 
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>>  .../jh7110-starfive-visionfive-v2.dts         | 25 ++++++++++++
> 
> FYI, this file does not exist in the v3 Devicetree patchset sent by Hal
> Feng:
> https://lore.kernel.org/linux-riscv/20221220011247.35560-1-hal.feng@starfivetech.com
> 
> Would you make sure that future revisions take into account that there
> is now a jh7110-starfive-visionfive-2.dtsi file instead?
> 
> Thanks,
> Conor.
> 
Hi Conor,

I did it based on v2 and will rebase to v3 in the next version.

Best Regards
William Qiu

Re: [PATCH v2 0/3] StarFive's SDIO/eMMC driver support

[William Qiu]

On 2022/12/27 20:22, William Qiu wrote:
> Hi,
> 
> This patchset adds initial rudimentary support for the StarFive
> designware mobile storage host controller driver. And this driver will
> be used in StarFive's VisionFive 2 board. The main purpose of adding
> this driver is to accommodate the ultra-high speed mode of eMMC.
> 
> The last patch should be applied after the patchset [1]:
> [1] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/
> 
> Changes since v1:
> - Renamed the dt-binding 'starfive,jh7110-sdio.yaml' to 'starfive,jh7110-mmc.yaml'.
> - Changed the type of 'starfive,syscon' and modify its description.
> - Deleted unused head files like '#include <linux/gpio.h>'.
> - Added comment for the 'rise_point' and 'fall_point'.
> - Changed the API 'num_caps' to 'common_caps'.
> - Changed the node name 'sys_syscon' to 'syscon'.
> - Changed the node name 'sdio' to 'mmc'.
> 
> The patch series is based on v6.1-rc5.
> 
> William Qiu (3):
>   dt-bindings: mmc: Add bindings for StarFive
>   mmc: starfive: Add sdio/emmc driver support
>   riscv: dts: starfive: Add mmc node
> 
>  .../bindings/mmc/starfive,jh7110-mmc.yaml     |  72 +++++++
>  MAINTAINERS                                   |   6 +
>  .../jh7110-starfive-visionfive-v2.dts         |  25 +++
>  arch/riscv/boot/dts/starfive/jh7110.dtsi      |  38 ++++
>  drivers/mmc/host/Kconfig                      |  10 +
>  drivers/mmc/host/Makefile                     |   1 +
>  drivers/mmc/host/dw_mmc-starfive.c            | 185 ++++++++++++++++++
>  7 files changed, 337 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
>  create mode 100644 drivers/mmc/host/dw_mmc-starfive.c
> 
> --
> 2.34.1
> 

Hi Rob/Jaehoon,

Could you please help to review and provide comments on this patch series?
Thank you in advance.

Best regards,
William Qiu

[PATCH v2 0/3] StarFive's SDIO/eMMC driver support

[William Qiu]

Hi,

This patchset adds initial rudimentary support for the StarFive
designware mobile storage host controller driver. And this driver will
be used in StarFive's VisionFive 2 board. The main purpose of adding
this driver is to accommodate the ultra-high speed mode of eMMC.

The last patch should be applied after the patchset [1]:
[1] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/

Changes since v1:
- Renamed the dt-binding 'starfive,jh7110-sdio.yaml' to 'starfive,jh7110-mmc.yaml'.
- Changed the type of 'starfive,syscon' and modify its description.
- Deleted unused head files like '#include <linux/gpio.h>'.
- Added comment for the 'rise_point' and 'fall_point'.
- Changed the API 'num_caps' to 'common_caps'.
- Changed the node name 'sys_syscon' to 'syscon'.
- Changed the node name 'sdio' to 'mmc'.

The patch series is based on v6.1-rc5.

William Qiu (3):
  dt-bindings: mmc: Add bindings for StarFive
  mmc: starfive: Add sdio/emmc driver support
  riscv: dts: starfive: Add mmc node

 .../bindings/mmc/starfive,jh7110-mmc.yaml     |  72 +++++++
 MAINTAINERS                                   |   6 +
 .../jh7110-starfive-visionfive-v2.dts         |  25 +++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  38 ++++
 drivers/mmc/host/Kconfig                      |  10 +
 drivers/mmc/host/Makefile                     |   1 +
 drivers/mmc/host/dw_mmc-starfive.c            | 185 ++++++++++++++++++
 7 files changed, 337 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
 create mode 100644 drivers/mmc/host/dw_mmc-starfive.c

--
2.34.1

[PATCH v2 3/3] riscv: dts: starfive: Add mmc node

[William Qiu]

This adds the mmc node for the StarFive JH7110 SoC.
Set sdioo node to emmc and set sdio1 node to sd.

Signed-off-by: William Qiu <william.qiu@starfivetech.com>
---
 .../jh7110-starfive-visionfive-v2.dts         | 25 ++++++++++++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      | 38 +++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
index c8946cf3a268..d8244fd1f5a0 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
@@ -47,6 +47,31 @@ &clk_rtc {
 	clock-frequency = <32768>;
 };
 
+&mmc0 {
+	max-frequency = <100000000>;
+	card-detect-delay = <300>;
+	bus-width = <8>;
+	cap-mmc-highspeed;
+	mmc-ddr-1_8v;
+	mmc-hs200-1_8v;
+	non-removable;
+	cap-mmc-hw-reset;
+	post-power-on-delay-ms = <200>;
+	status = "okay";
+};
+
+&mmc1 {
+	max-frequency = <100000000>;
+	card-detect-delay = <300>;
+	bus-width = <4>;
+	no-sdio;
+	no-mmc;
+	broken-cd;
+	cap-sd-highspeed;
+	post-power-on-delay-ms = <200>;
+	status = "okay";
+};
+
 &gmac0_rmii_refin {
 	clock-frequency = <50000000>;
 };
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index c22e8f1d2640..08a780d2c0f4 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -331,6 +331,11 @@ aoncrg: clock-controller@17000000 {
 			#reset-cells = <1>;
 		};
 
+		syscon: syscon@13030000 {
+			compatible = "starfive,syscon", "syscon";
+			reg = <0x0 0x13030000 0x0 0x1000>;
+		};
+
 		gpio: gpio@13040000 {
 			compatible = "starfive,jh7110-sys-pinctrl";
 			reg = <0x0 0x13040000 0x0 0x10000>;
@@ -433,5 +438,38 @@ uart5: serial@12020000 {
 			reg-shift = <2>;
 			status = "disabled";
 		};
+
+		/* unremovable emmc as mmcblk0 */
+		mmc0: mmc@16010000 {
+			compatible = "starfive,jh7110-mmc";
+			reg = <0x0 0x16010000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SDIO0_AHB>,
+				 <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>;
+			clock-names = "biu","ciu";
+			resets = <&syscrg JH7110_SYSRST_SDIO0_AHB>;
+			reset-names = "reset";
+			interrupts = <74>;
+			fifo-depth = <32>;
+			fifo-watermark-aligned;
+			data-addr = <0>;
+			starfive,syscon = <&syscon 0x14 0x1a 0x7c000000>;
+			status = "disabled";
+		};
+
+		mmc1: mmc@16020000 {
+			compatible = "starfive,jh7110-mmc";
+			reg = <0x0 0x16020000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_SDIO1_AHB>,
+				 <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>;
+			clock-names = "biu","ciu";
+			resets = <&syscrg JH7110_SYSRST_SDIO1_AHB>;
+			reset-names = "reset";
+			interrupts = <75>;
+			fifo-depth = <32>;
+			fifo-watermark-aligned;
+			data-addr = <0>;
+			starfive,syscon = <&syscon 0x9c 0x1 0x3e>;
+			status = "disabled";
+		};
 	};
 };
-- 
2.34.1