diff mbox series

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

Message ID 20221227122227.460921-4-william.qiu@starfivetech.com
State Superseded
Headers show
Series StarFive's SDIO/eMMC driver support | expand

Commit Message

William Qiu Dec. 27, 2022, 12:22 p.m. UTC
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(+)

Comments

Ulf Hansson Jan. 2, 2023, 2:03 p.m. UTC | #1
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
William Qiu Jan. 4, 2023, 6:08 a.m. UTC | #2
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
William Qiu Jan. 6, 2023, 8:41 a.m. UTC | #3
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
diff mbox series

Patch

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";
+		};
 	};
 };