| Message ID | 20230203081913.81968-4-william.qiu@starfivetech.com |
|---|---|
| State | New |
| Headers | show |
| Series | StarFive's SDIO/eMMC driver support | expand |
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 <william.qiu@starfivetech.com> > > >> + >> &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 >
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 <william.qiu@starfivetech.com> >>> >>> >>>> + >>>> &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 >
On Fri, 3 Feb 2023 at 09:21, William Qiu <william.qiu@starfivetech.com> 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 <william.qiu@starfivetech.com> > --- > .../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 > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2023/2/15 18:31, Emil Renner Berthing wrote: > On Fri, 3 Feb 2023 at 09:21, William Qiu <william.qiu@starfivetech.com> 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 <william.qiu@starfivetech.com> >> --- >> .../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 >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
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"; + }; }; };
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 <william.qiu@starfivetech.com> --- .../jh7110-starfive-visionfive-2.dtsi | 23 ++++++++++++ arch/riscv/boot/dts/starfive/jh7110.dtsi | 37 +++++++++++++++++++ 2 files changed, 60 insertions(+)