2022-12-27 12:38:36

by William Qiu

[permalink] [raw]
Subject: [PATCH v2 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 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


2022-12-27 12:57:30

by William Qiu

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

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

2023-01-02 14:39:36

by Ulf Hansson

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

On Tue, 27 Dec 2022 at 13:22, William Qiu <[email protected]> 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 <[email protected]>
> ---
> .../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

2023-01-04 06:24:52

by William Qiu

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



On 2023/1/2 22:03, Ulf Hansson wrote:
> On Tue, 27 Dec 2022 at 13:22, William Qiu <[email protected]> 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 <[email protected]>
>> ---
>> .../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

2023-01-04 16:11:31

by Ulf Hansson

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

On Wed, 4 Jan 2023 at 07:08, William Qiu <[email protected]> wrote:
>
>
>
> On 2023/1/2 22:03, Ulf Hansson wrote:
> > On Tue, 27 Dec 2022 at 13:22, William Qiu <[email protected]> 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 <[email protected]>
> >> ---
> >> .../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

2023-01-06 09:02:15

by William Qiu

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



On 2023/1/5 0:05, Ulf Hansson wrote:
> On Wed, 4 Jan 2023 at 07:08, William Qiu <[email protected]> wrote:
>>
>>
>>
>> On 2023/1/2 22:03, Ulf Hansson wrote:
>> > On Tue, 27 Dec 2022 at 13:22, William Qiu <[email protected]> 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 <[email protected]>
>> >> ---
>> >> .../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

2023-01-06 09:34:30

by William Qiu

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



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

2023-01-20 06:22:19

by Conor Dooley

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

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

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

Thanks,
Conor.


Attachments:
(No filename) (646.00 B)
signature.asc (235.00 B)
Download all attachments

2023-01-31 08:02:17

by William Qiu

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



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