2023-02-03 08:21:32

by William Qiu

[permalink] [raw]
Subject: [PATCH v3 0/3] StarFive's SDIO/eMMC driver support

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/[email protected]/

Changes since v2:
- Wraped commit message according to Linux coding style.
- Rephrased the description of the patches.
- Changed the description of syscon regsiter.
- Dropped redundant properties.

The patch series is based on v6.1.

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

.../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++
MAINTAINERS | 6 +
.../jh7110-starfive-visionfive-2.dtsi | 23 +++
arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 ++++
drivers/mmc/host/Kconfig | 10 +
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/dw_mmc-starfive.c | 185 ++++++++++++++++++
7 files changed, 339 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



2023-02-03 08:21:34

by William Qiu

[permalink] [raw]
Subject: [PATCH v3 1/3] dt-bindings: mmc: Add StarFive MMC module

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

Signed-off-by: William Qiu <[email protected]>
---
.../bindings/mmc/starfive,jh7110-mmc.yaml | 77 +++++++++++++++++++
1 file changed, 77 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..deacf8e9cfb2
--- /dev/null
+++ b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
@@ -0,0 +1,77 @@
+# 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 <[email protected]>
+
+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,sysreg:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ items:
+ - items:
+ - description: phandle to System Register Controller syscon node
+ - description: offset of SYS_SYSCONSAIF__SYSCFG register for MMC controller
+ - description: shift of SYS_SYSCONSAIF__SYSCFG register for MMC controller
+ - description: mask of SYS_SYSCONSAIF__SYSCFG register for MMC controller
+ description:
+ Should be four parameters, the phandle to System Register Controller
+ syscon node and the offset/shift/mask of SYS_SYSCONSAIF__SYSCFG register
+ for MMC controller.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - interrupts
+ - starfive,sysreg
+
+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,sysreg = <&sysreg 0x14 0x1a 0x7c000000>;
+ };
--
2.34.1


2023-02-03 08:21:36

by William Qiu

[permalink] [raw]
Subject: [PATCH v3 3/3] riscv: dts: starfive: Add mmc node

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

Signed-off-by: William Qiu <[email protected]>
---
.../jh7110-starfive-visionfive-2.dtsi | 23 ++++++++++++
arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 +++++++++++++++++++
2 files changed, 60 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
index c60280b89c73..e1a0248e907f 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
@@ -42,6 +42,29 @@ &rtc_osc {
clock-frequency = <32768>;
};

+&mmc0 {
+ max-frequency = <100000000>;
+ 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>;
+ 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 64d260ea1f29..ae1a664e7af5 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -370,6 +370,11 @@ syscrg: clock-controller@13020000 {
#reset-cells = <1>;
};

+ sysreg: syscon@13030000 {
+ compatible = "starfive,sysreg", "syscon";
+ reg = <0x0 0x13030000 0x0 0x1000>;
+ };
+
gpio: gpio@13040000 {
compatible = "starfive,jh7110-sys-pinctrl";
reg = <0x0 0x13040000 0x0 0x10000>;
@@ -407,5 +412,37 @@ gpioa: gpio@17020000 {
gpio-controller;
#gpio-cells = <2>;
};
+
+ 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,sysreg = <&sysreg 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,sysreg = <&sysreg 0x9c 0x1 0x3e>;
+ status = "disabled";
+ };
};
};
--
2.34.1


2023-02-03 08:21:39

by William Qiu

[permalink] [raw]
Subject: [PATCH v3 2/3] mmc: starfive: Add sdio/emmc driver support

Add sdio/emmc driver support for StarFive JH7110 soc.

Tested-by: Conor Dooley <[email protected]>
Signed-off-by: William Qiu <[email protected]>
---
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 85e8f83161d7..0ff348da6463 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19659,6 +19659,12 @@ F: Documentation/devicetree/bindings/reset/starfive,jh7100-reset.yaml
F: drivers/reset/starfive/reset-starfive-jh71*
F: include/dt-bindings/reset/starfive?jh71*.h

+STARFIVE JH7110 MMC/SD/SDIO DRIVER
+M: William Qiu <[email protected]>
+S: Maintained
+F: Documentation/devicetree/bindings/mmc/starfive*
+F: drivers/mmc/dw_mmc-starfive.c
+
STATIC BRANCH/CALL
M: Peter Zijlstra <[email protected]>
M: Josh Poimboeuf <[email protected]>
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


2023-02-03 09:00:35

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] dt-bindings: mmc: Add StarFive MMC module

On 03/02/2023 09:19, William Qiu wrote:
> Add documentation to describe StarFive designware mobile storage
> host controller driver.
>
> Signed-off-by: William Qiu <[email protected]>
> ---
> .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 +++++++++++++++++++
> 1 file changed, 77 insertions(+)


Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof


2023-02-03 09:02:44

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] riscv: dts: starfive: Add mmc node

On 03/02/2023 09:19, William Qiu wrote:
> This adds the mmc node for the StarFive JH7110 SoC.

Do not use "This xxx". Use imperative mode.
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> Set mmco node to emmc and set mmc1 node to sd.
>
> Signed-off-by: William Qiu <[email protected]>


> +
> &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 64d260ea1f29..ae1a664e7af5 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -370,6 +370,11 @@ syscrg: clock-controller@13020000 {
> #reset-cells = <1>;
> };
>
> + sysreg: syscon@13030000 {
> + compatible = "starfive,sysreg", "syscon";

No:
1. Undocumented.
2. A bit too generic. You should have here SoC specific compatible as
well (either as second or third compatible, if all your SoCs share
register layout).

> + reg = <0x0 0x13030000 0x0 0x1000>;
> + };
> +

Best regards,
Krzysztof


2023-02-03 09:24:26

by William Qiu

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] riscv: dts: starfive: Add mmc node



On 2023/2/3 17:02, Krzysztof Kozlowski wrote:
> On 03/02/2023 09:19, William Qiu wrote:
>> This adds the mmc node for the StarFive JH7110 SoC.
>
> Do not use "This xxx". Use imperative mode.
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
>> Set mmco node to emmc and set mmc1 node to sd.
>>
>> Signed-off-by: William Qiu <[email protected]>
>
>
>> +
>> &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 64d260ea1f29..ae1a664e7af5 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -370,6 +370,11 @@ syscrg: clock-controller@13020000 {
>> #reset-cells = <1>;
>> };
>>
>> + sysreg: syscon@13030000 {
>> + compatible = "starfive,sysreg", "syscon";
>
> No:
> 1. Undocumented.
> 2. A bit too generic. You should have here SoC specific compatible as
> well (either as second or third compatible, if all your SoCs share
> register layout).
>

Hi Krzysztof,

As for the compatible, I will change it to "starfive,jh7110-sysreg"
in next version,but for undocumented, I don't get it, can you
clarify that.
Thank you anyway.

Best regards,
William Qiu
>> + reg = <0x0 0x13030000 0x0 0x1000>;
>> + };
>> +
>
> Best regards,
> Krzysztof
>

2023-02-03 09:31:17

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] riscv: dts: starfive: Add mmc node

On 03/02/2023 10:23, William Qiu wrote:
>
>
> On 2023/2/3 17:02, Krzysztof Kozlowski wrote:
>> On 03/02/2023 09:19, William Qiu wrote:
>>> This adds the mmc node for the StarFive JH7110 SoC.
>>
>> Do not use "This xxx". Use imperative mode.
>> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>>
>>> Set mmco node to emmc and set mmc1 node to sd.
>>>
>>> Signed-off-by: William Qiu <[email protected]>
>>
>>
>>> +
>>> &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 64d260ea1f29..ae1a664e7af5 100644
>>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>> @@ -370,6 +370,11 @@ syscrg: clock-controller@13020000 {
>>> #reset-cells = <1>;
>>> };
>>>
>>> + sysreg: syscon@13030000 {
>>> + compatible = "starfive,sysreg", "syscon";
>>
>> No:
>> 1. Undocumented.
>> 2. A bit too generic. You should have here SoC specific compatible as
>> well (either as second or third compatible, if all your SoCs share
>> register layout).
>>
>
> Hi Krzysztof,
>
> As for the compatible, I will change it to "starfive,jh7110-sysreg"
> in next version,but for undocumented, I don't get it, can you
> clarify that.
> Thank you anyway.

You need bindings for it. I don't see it in linux-next, cover letter
dependencies nor here.

Best regards,
Krzysztof


2023-02-03 09:35:54

by William Qiu

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] riscv: dts: starfive: Add mmc node



On 2023/2/3 17:30, Krzysztof Kozlowski wrote:
> On 03/02/2023 10:23, William Qiu wrote:
>>
>>
>> On 2023/2/3 17:02, Krzysztof Kozlowski wrote:
>>> On 03/02/2023 09:19, William Qiu wrote:
>>>> This adds the mmc node for the StarFive JH7110 SoC.
>>>
>>> Do not use "This xxx". Use imperative mode.
>>> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>>>
>>>> Set mmco node to emmc and set mmc1 node to sd.
>>>>
>>>> Signed-off-by: William Qiu <[email protected]>
>>>
>>>
>>>> +
>>>> &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 64d260ea1f29..ae1a664e7af5 100644
>>>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>>>> @@ -370,6 +370,11 @@ syscrg: clock-controller@13020000 {
>>>> #reset-cells = <1>;
>>>> };
>>>>
>>>> + sysreg: syscon@13030000 {
>>>> + compatible = "starfive,sysreg", "syscon";
>>>
>>> No:
>>> 1. Undocumented.
>>> 2. A bit too generic. You should have here SoC specific compatible as
>>> well (either as second or third compatible, if all your SoCs share
>>> register layout).
>>>
>>
>> Hi Krzysztof,
>>
>> As for the compatible, I will change it to "starfive,jh7110-sysreg"
>> in next version,but for undocumented, I don't get it, can you
>> clarify that.
>> Thank you anyway.
>
> You need bindings for it. I don't see it in linux-next, cover letter
> dependencies nor here.
>

I see, I will update it in next version.
Thank you for taking time to review and provide helpful comments for this patch.

Best regards,
William Qiu
> Best regards,
> Krzysztof
>

2023-02-14 12:19:01

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] StarFive's SDIO/eMMC driver support

On Fri, 3 Feb 2023 at 09:19, William Qiu <[email protected]> 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/[email protected]/
>
> Changes since v2:
> - Wraped commit message according to Linux coding style.
> - Rephrased the description of the patches.
> - Changed the description of syscon regsiter.
> - Dropped redundant properties.
>
> The patch series is based on v6.1.
>
> William Qiu (3):
> dt-bindings: mmc: Add StarFive MMC module
> mmc: starfive: Add sdio/emmc driver support
> riscv: dts: starfive: Add mmc node
>
> .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++
> MAINTAINERS | 6 +
> .../jh7110-starfive-visionfive-2.dtsi | 23 +++
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 ++++
> drivers/mmc/host/Kconfig | 10 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/dw_mmc-starfive.c | 185 ++++++++++++++++++
> 7 files changed, 339 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
> create mode 100644 drivers/mmc/host/dw_mmc-starfive.c
>

Patch 1 and patch 2 applied for next, thanks!

Note that I fixed some checkpatch errors/warnings. Next time, please
run ./scripts/checkpatch.pl before you submit your patches.

Kind regards
Uffe

2023-02-15 01:56:54

by William Qiu

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] StarFive's SDIO/eMMC driver support



On 2023/2/14 20:18, Ulf Hansson wrote:
> On Fri, 3 Feb 2023 at 09:19, William Qiu <[email protected]> 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/[email protected]/
>>
>> Changes since v2:
>> - Wraped commit message according to Linux coding style.
>> - Rephrased the description of the patches.
>> - Changed the description of syscon regsiter.
>> - Dropped redundant properties.
>>
>> The patch series is based on v6.1.
>>
>> William Qiu (3):
>> dt-bindings: mmc: Add StarFive MMC module
>> mmc: starfive: Add sdio/emmc driver support
>> riscv: dts: starfive: Add mmc node
>>
>> .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++
>> MAINTAINERS | 6 +
>> .../jh7110-starfive-visionfive-2.dtsi | 23 +++
>> arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 ++++
>> drivers/mmc/host/Kconfig | 10 +
>> drivers/mmc/host/Makefile | 1 +
>> drivers/mmc/host/dw_mmc-starfive.c | 185 ++++++++++++++++++
>> 7 files changed, 339 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
>> create mode 100644 drivers/mmc/host/dw_mmc-starfive.c
>>
>
> Patch 1 and patch 2 applied for next, thanks!
>
> Note that I fixed some checkpatch errors/warnings. Next time, please
> run ./scripts/checkpatch.pl before you submit your patches.
>
> Kind regards
> Uffe
z
Hi Uffe,

Sorry about that, I'll check the patches before submit next time.
Thanks for taking time reviewing this patch series.

Best Regards
William

2023-02-15 10:01:49

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] StarFive's SDIO/eMMC driver support

On Tue, 14 Feb 2023 at 13:21, Ulf Hansson <[email protected]> wrote:
>
> On Fri, 3 Feb 2023 at 09:19, William Qiu <[email protected]> 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/[email protected]/
> >
> > Changes since v2:
> > - Wraped commit message according to Linux coding style.
> > - Rephrased the description of the patches.
> > - Changed the description of syscon regsiter.
> > - Dropped redundant properties.
> >
> > The patch series is based on v6.1.
> >
> > William Qiu (3):
> > dt-bindings: mmc: Add StarFive MMC module
> > mmc: starfive: Add sdio/emmc driver support
> > riscv: dts: starfive: Add mmc node
> >
> > .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++
> > MAINTAINERS | 6 +
> > .../jh7110-starfive-visionfive-2.dtsi | 23 +++
> > arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 ++++
> > drivers/mmc/host/Kconfig | 10 +
> > drivers/mmc/host/Makefile | 1 +
> > drivers/mmc/host/dw_mmc-starfive.c | 185 ++++++++++++++++++
> > 7 files changed, 339 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
> > create mode 100644 drivers/mmc/host/dw_mmc-starfive.c
> >
>
> Patch 1 and patch 2 applied for next, thanks!
>
> Note that I fixed some checkpatch errors/warnings. Next time, please
> run ./scripts/checkpatch.pl before you submit your patches.

Hi,

Something here is not right. The bindings document "starfive,sysreg",
but the code is looking for "starfive,syscon".
William, did you test this and run dtbs_check before sending this series?

/Emil

> Kind regards
> Uffe
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-15 10:16:44

by William Qiu

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] StarFive's SDIO/eMMC driver support



On 2023/2/15 18:00, Emil Renner Berthing wrote:
> On Tue, 14 Feb 2023 at 13:21, Ulf Hansson <[email protected]> wrote:
>>
>> On Fri, 3 Feb 2023 at 09:19, William Qiu <[email protected]> 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/[email protected]/
>> >
>> > Changes since v2:
>> > - Wraped commit message according to Linux coding style.
>> > - Rephrased the description of the patches.
>> > - Changed the description of syscon regsiter.
>> > - Dropped redundant properties.
>> >
>> > The patch series is based on v6.1.
>> >
>> > William Qiu (3):
>> > dt-bindings: mmc: Add StarFive MMC module
>> > mmc: starfive: Add sdio/emmc driver support
>> > riscv: dts: starfive: Add mmc node
>> >
>> > .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++
>> > MAINTAINERS | 6 +
>> > .../jh7110-starfive-visionfive-2.dtsi | 23 +++
>> > arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 ++++
>> > drivers/mmc/host/Kconfig | 10 +
>> > drivers/mmc/host/Makefile | 1 +
>> > drivers/mmc/host/dw_mmc-starfive.c | 185 ++++++++++++++++++
>> > 7 files changed, 339 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
>> > create mode 100644 drivers/mmc/host/dw_mmc-starfive.c
>> >
>>
>> Patch 1 and patch 2 applied for next, thanks!
>>
>> Note that I fixed some checkpatch errors/warnings. Next time, please
>> run ./scripts/checkpatch.pl before you submit your patches.
>
> Hi,
>
> Something here is not right. The bindings document "starfive,sysreg",
> but the code is looking for "starfive,syscon".
> William, did you test this and run dtbs_check before sending this series?
>
> /Emil

Hi Emil,

Sorry about that, it's my negligence that I did dtbs_check without
checking the configuration of CONFIG_SOC_STARFIVE. I made a modification
in the next version and add binding for all the syscon at the same time. Next
time I'll make sure the check is correct.I'm sorry again for this oversight.
I'll send v4 patch series today.

Best Regards
William
>
>> Kind regards
>> Uffe
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-15 10:22:42

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] StarFive's SDIO/eMMC driver support

On Wed, Feb 15, 2023 at 06:16:35PM +0800, William Qiu wrote:
>
>
> On 2023/2/15 18:00, Emil Renner Berthing wrote:
> > On Tue, 14 Feb 2023 at 13:21, Ulf Hansson <[email protected]> wrote:
> >>
> >> On Fri, 3 Feb 2023 at 09:19, William Qiu <[email protected]> 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/[email protected]/
> >> >
> >> > Changes since v2:
> >> > - Wraped commit message according to Linux coding style.
> >> > - Rephrased the description of the patches.
> >> > - Changed the description of syscon regsiter.
> >> > - Dropped redundant properties.
> >> >
> >> > The patch series is based on v6.1.
> >> >
> >> > William Qiu (3):
> >> > dt-bindings: mmc: Add StarFive MMC module
> >> > mmc: starfive: Add sdio/emmc driver support
> >> > riscv: dts: starfive: Add mmc node
> >> >
> >> > .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++
> >> > MAINTAINERS | 6 +
> >> > .../jh7110-starfive-visionfive-2.dtsi | 23 +++
> >> > arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 ++++
> >> > drivers/mmc/host/Kconfig | 10 +
> >> > drivers/mmc/host/Makefile | 1 +
> >> > drivers/mmc/host/dw_mmc-starfive.c | 185 ++++++++++++++++++
> >> > 7 files changed, 339 insertions(+)
> >> > create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
> >> > create mode 100644 drivers/mmc/host/dw_mmc-starfive.c
> >> >
> >>
> >> Patch 1 and patch 2 applied for next, thanks!
> >>
> >> Note that I fixed some checkpatch errors/warnings. Next time, please
> >> run ./scripts/checkpatch.pl before you submit your patches.
> >
> > Hi,
> >
> > Something here is not right. The bindings document "starfive,sysreg",
> > but the code is looking for "starfive,syscon".
> > William, did you test this and run dtbs_check before sending this series?
> >
> > /Emil
>
> Hi Emil,
>
> Sorry about that, it's my negligence that I did dtbs_check without

dtbs_check would be okay though, no? The binding and dts (from checking
with eyeballs!) are consistent & it is just the driver that is looking
for the wrong property name, no?

> checking the configuration of CONFIG_SOC_STARFIVE. I made a modification
> in the next version and add binding for all the syscon at the same time. Next
> time I'll make sure the check is correct.I'm sorry again for this oversight.
> I'll send v4 patch series today.

Ulf's subsystem & his rules, but I would just send a follow-on patch
fixing the driver to use the property that is in the dt-binding.

Sorta unrelated, but please remind me about the dts patch once the base
dts is merged. It's still in the RISC-V patchwork so *should* be fine,
but in case the base dts takes a while it may get automatically
archived.

Thanks,
Conor.


Attachments:
(No filename) (3.17 kB)
signature.asc (228.00 B)
Download all attachments

2023-02-15 10:31:39

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] riscv: dts: starfive: Add mmc node

On Fri, 3 Feb 2023 at 09:21, William Qiu <[email protected]> wrote:
>
> This adds the mmc node for the StarFive JH7110 SoC.
> Set mmco node to emmc and set mmc1 node to sd.
>
> Signed-off-by: William Qiu <[email protected]>
> ---
> .../jh7110-starfive-visionfive-2.dtsi | 23 ++++++++++++
> arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 +++++++++++++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> index c60280b89c73..e1a0248e907f 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
> @@ -42,6 +42,29 @@ &rtc_osc {
> clock-frequency = <32768>;
> };
>
> +&mmc0 {
> + max-frequency = <100000000>;
> + 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>;
> + bus-width = <4>;
> + no-sdio;ru
> + no-mmc;
> + broken-cd;
> + cap-sd-highspeed;
> + post-power-on-delay-ms = <200>;
> + status = "okay";
> +};

Please add these so they're sorted alphabetically (but keep the clocks
at the top), so there's at least some sort of system.

> &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 64d260ea1f29..ae1a664e7af5 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -370,6 +370,11 @@ syscrg: clock-controller@13020000 {
> #reset-cells = <1>;
> };
>
> + sysreg: syscon@13030000 {
> + compatible = "starfive,sysreg", "syscon";
> + reg = <0x0 0x13030000 0x0 0x1000>;
> + };
> +
> gpio: gpio@13040000 {
> compatible = "starfive,jh7110-sys-pinctrl";
> reg = <0x0 0x13040000 0x0 0x10000>;
> @@ -407,5 +412,37 @@ gpioa: gpio@17020000 {
> gpio-controller;
> #gpio-cells = <2>;
> };
> +
> + 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,sysreg = <&sysreg 0x14 0x1a 0x7c000000>;

This may need updating depending on whether you fix the driver or bindings.

> + 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,sysreg = <&sysreg 0x9c 0x1 0x3e>;
> + status = "disabled";
> + };
> };
> };
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-15 10:32:41

by William Qiu

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] riscv: dts: starfive: Add mmc node



On 2023/2/15 18:31, Emil Renner Berthing wrote:
> On Fri, 3 Feb 2023 at 09:21, William Qiu <[email protected]> wrote:
>>
>> This adds the mmc node for the StarFive JH7110 SoC.
>> Set mmco node to emmc and set mmc1 node to sd.
>>
>> Signed-off-by: William Qiu <[email protected]>
>> ---
>> .../jh7110-starfive-visionfive-2.dtsi | 23 ++++++++++++
>> arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 +++++++++++++++++++
>> 2 files changed, 60 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> index c60280b89c73..e1a0248e907f 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi
>> @@ -42,6 +42,29 @@ &rtc_osc {
>> clock-frequency = <32768>;
>> };
>>
>> +&mmc0 {
>> + max-frequency = <100000000>;
>> + 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>;
>> + bus-width = <4>;
>> + no-sdio;ru
>> + no-mmc;
>> + broken-cd;
>> + cap-sd-highspeed;
>> + post-power-on-delay-ms = <200>;
>> + status = "okay";
>> +};
>
> Please add these so they're sorted alphabetically (but keep the clocks
> at the top), so there's at least some sort of system.
>
>> &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 64d260ea1f29..ae1a664e7af5 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -370,6 +370,11 @@ syscrg: clock-controller@13020000 {
>> #reset-cells = <1>;
>> };
>>
>> + sysreg: syscon@13030000 {
>> + compatible = "starfive,sysreg", "syscon";
>> + reg = <0x0 0x13030000 0x0 0x1000>;
>> + };
>> +
>> gpio: gpio@13040000 {
>> compatible = "starfive,jh7110-sys-pinctrl";
>> reg = <0x0 0x13040000 0x0 0x10000>;
>> @@ -407,5 +412,37 @@ gpioa: gpio@17020000 {
>> gpio-controller;
>> #gpio-cells = <2>;
>> };
>> +
>> + 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,sysreg = <&sysreg 0x14 0x1a 0x7c000000>;
>
> This may need updating depending on whether you fix the driver or bindings.
>
I'll do it then
>> + 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,sysreg = <&sysreg 0x9c 0x1 0x3e>;
>> + status = "disabled";
>> + };
>> };
>> };
>> --
>> 2.34.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

2023-02-15 10:36:34

by William Qiu

[permalink] [raw]
Subject: Re: [PATCH v3 0/3] StarFive's SDIO/eMMC driver support



On 2023/2/15 18:22, Conor Dooley wrote:
> On Wed, Feb 15, 2023 at 06:16:35PM +0800, William Qiu wrote:
>>
>>
>> On 2023/2/15 18:00, Emil Renner Berthing wrote:
>> > On Tue, 14 Feb 2023 at 13:21, Ulf Hansson <[email protected]> wrote:
>> >>
>> >> On Fri, 3 Feb 2023 at 09:19, William Qiu <[email protected]> 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/[email protected]/
>> >> >
>> >> > Changes since v2:
>> >> > - Wraped commit message according to Linux coding style.
>> >> > - Rephrased the description of the patches.
>> >> > - Changed the description of syscon regsiter.
>> >> > - Dropped redundant properties.
>> >> >
>> >> > The patch series is based on v6.1.
>> >> >
>> >> > William Qiu (3):
>> >> > dt-bindings: mmc: Add StarFive MMC module
>> >> > mmc: starfive: Add sdio/emmc driver support
>> >> > riscv: dts: starfive: Add mmc node
>> >> >
>> >> > .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++
>> >> > MAINTAINERS | 6 +
>> >> > .../jh7110-starfive-visionfive-2.dtsi | 23 +++
>> >> > arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 ++++
>> >> > drivers/mmc/host/Kconfig | 10 +
>> >> > drivers/mmc/host/Makefile | 1 +
>> >> > drivers/mmc/host/dw_mmc-starfive.c | 185 ++++++++++++++++++
>> >> > 7 files changed, 339 insertions(+)
>> >> > create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml
>> >> > create mode 100644 drivers/mmc/host/dw_mmc-starfive.c
>> >> >
>> >>
>> >> Patch 1 and patch 2 applied for next, thanks!
>> >>
>> >> Note that I fixed some checkpatch errors/warnings. Next time, please
>> >> run ./scripts/checkpatch.pl before you submit your patches.
>> >
>> > Hi,
>> >
>> > Something here is not right. The bindings document "starfive,sysreg",
>> > but the code is looking for "starfive,syscon".
>> > William, did you test this and run dtbs_check before sending this series?
>> >
>> > /Emil
>>
>> Hi Emil,
>>
>> Sorry about that, it's my negligence that I did dtbs_check without
>
> dtbs_check would be okay though, no? The binding and dts (from checking
> with eyeballs!) are consistent & it is just the driver that is looking
> for the wrong property name, no?
>


It was my negligence that I didn't test the function

>> checking the configuration of CONFIG_SOC_STARFIVE. I made a modification
>> in the next version and add binding for all the syscon at the same time. Next
>> time I'll make sure the check is correct.I'm sorry again for this oversight.
>> I'll send v4 patch series today.
>
> Ulf's subsystem & his rules, but I would just send a follow-on patch
> fixing the driver to use the property that is in the dt-binding.
>
> Sorta unrelated, but please remind me about the dts patch once the base
> dts is merged. It's still in the RISC-V patchwork so *should* be fine,
> but in case the base dts takes a while it may get automatically
> archived.
>
> Thanks,
> Conor.
>

I'll do it then.
Thanks,
William