| Message ID | 20230215113249.47727-1-william.qiu@starfivetech.com |
|---|---|
| Headers | show |
| Series | StarFive's SDIO/eMMC driver support | expand |
Sorry, forgot what I said. I didn't see Krzysztof's reply on V3 serires. Best regards, Shengyu > Hello William, > > Are you sure changing driver is better than changing yaml bindings? All > > previous version sent was syscon and sysreg seems not consistent with > > other codes. > > Best regards, > > Shengyu > >> Add documentation to describe StarFive designware mobile storage >> host controller driver. >> >> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> --- >> .../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..51e1b04e799f >> --- /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 <william.qiu@starfivetech.com> >> + >> +properties: >> + compatible: >> + const: starfive,jh7110-mmc >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + items: >> + - description: biu clock >> + - description: ciu clock >> + >> + clock-names: >> + items: >> + - const: biu >> + - const: ciu >> + >> + interrupts: >> + maxItems: 1 >> + >> + starfive,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 = <&sys_syscon 0x14 0x1a 0x7c000000>; >> + };
On Wed, 15 Feb 2023 at 12:35, William Qiu <william.qiu@starfivetech.com> wrote: > > Add 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 | 47 +++++++++++++++++++ > 2 files changed, 70 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..17f7b3ee6ca3 100644 > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi > @@ -314,6 +314,11 @@ uart2: serial@10020000 { > status = "disabled"; > }; > > + stg_syscon: syscon@10240000 { > + compatible = "starfive,jh7110-stg-syscon", "syscon"; > + reg = <0x0 0x10240000 0x0 0x1000>; > + }; > + > uart3: serial@12000000 { > compatible = "snps,dw-apb-uart"; > reg = <0x0 0x12000000 0x0 0x10000>; > @@ -370,6 +375,11 @@ syscrg: clock-controller@13020000 { > #reset-cells = <1>; > }; > > + sys_syscon: syscon@13030000 { > + compatible = "starfive,jh7110-sys-syscon", "syscon"; > + reg = <0x0 0x13030000 0x0 0x1000>; > + }; > + > gpio: gpio@13040000 { > compatible = "starfive,jh7110-sys-pinctrl"; > reg = <0x0 0x13040000 0x0 0x10000>; > @@ -397,6 +407,11 @@ aoncrg: clock-controller@17000000 { > #reset-cells = <1>; > }; > > + aon_syscon: syscon@17010000 { > + compatible = "starfive,jh7110-aon-syscon", "syscon"; > + reg = <0x0 0x17010000 0x0 0x1000>; > + }; > + > gpioa: gpio@17020000 { > compatible = "starfive,jh7110-aon-pinctrl"; > reg = <0x0 0x17020000 0x0 0x10000>; > @@ -407,5 +422,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 = <&sys_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,sysreg = <&sys_syscon 0x9c 0x1 0x3e>; > + status = "disabled"; > + }; Hi William, These nodes still don't seem to be sorted by address, eg. by the number after the @ Also please move the dt-binding patch before this one, so dtb_check won't fail no matter where git bisect happens to land. /Emil > }; > }; > -- > 2.34.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2023/2/15 20:12, Emil Renner Berthing wrote: > On Wed, 15 Feb 2023 at 12:35, William Qiu <william.qiu@starfivetech.com> wrote: >> >> Add 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 | 47 +++++++++++++++++++ >> 2 files changed, 70 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..17f7b3ee6ca3 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi >> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> @@ -314,6 +314,11 @@ uart2: serial@10020000 { >> status = "disabled"; >> }; >> >> + stg_syscon: syscon@10240000 { >> + compatible = "starfive,jh7110-stg-syscon", "syscon"; >> + reg = <0x0 0x10240000 0x0 0x1000>; >> + }; >> + >> uart3: serial@12000000 { >> compatible = "snps,dw-apb-uart"; >> reg = <0x0 0x12000000 0x0 0x10000>; >> @@ -370,6 +375,11 @@ syscrg: clock-controller@13020000 { >> #reset-cells = <1>; >> }; >> >> + sys_syscon: syscon@13030000 { >> + compatible = "starfive,jh7110-sys-syscon", "syscon"; >> + reg = <0x0 0x13030000 0x0 0x1000>; >> + }; >> + >> gpio: gpio@13040000 { >> compatible = "starfive,jh7110-sys-pinctrl"; >> reg = <0x0 0x13040000 0x0 0x10000>; >> @@ -397,6 +407,11 @@ aoncrg: clock-controller@17000000 { >> #reset-cells = <1>; >> }; >> >> + aon_syscon: syscon@17010000 { >> + compatible = "starfive,jh7110-aon-syscon", "syscon"; >> + reg = <0x0 0x17010000 0x0 0x1000>; >> + }; >> + >> gpioa: gpio@17020000 { >> compatible = "starfive,jh7110-aon-pinctrl"; >> reg = <0x0 0x17020000 0x0 0x10000>; >> @@ -407,5 +422,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 = <&sys_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,sysreg = <&sys_syscon 0x9c 0x1 0x3e>; >> + status = "disabled"; >> + }; > > Hi William, > > These nodes still don't seem to be sorted by address, eg. by the > number after the @ > Also please move the dt-binding patch before this one, so dtb_check > won't fail no matter where git bisect happens to land. > > /Emil > Hi Emil, I'll update it in next version. Best Regards William >> }; >> }; >> -- >> 2.34.1 >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@starfivetech.com> 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/20221220011247.35560-1-hal.feng@starfivetech.com/ > > Changes v3->v4: > - Added documentation to describe StarFive System Controller Registers. > - Added aon_syscon and stg_syscon node. > - Fixed some checkpatch errors/warnings. > > Changes v2->v3: > - Wraped commit message according to Linux coding style. > - Rephrased the description of the patches. > - Changed the description of syscon regsiter. > - Dropped redundant properties. > > Changes v1->v2: > - 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. > > William Qiu (4): > dt-bindings: mmc: Add StarFive MMC module > mmc: starfive: Add sdio/emmc driver support > riscv: dts: starfive: Add mmc node > dt-bindings: syscon: Add StarFive syscon doc > > .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++ > .../bindings/soc/starfive/jh7110-syscon.yaml | 51 +++++ > MAINTAINERS | 11 ++ > .../jh7110-starfive-visionfive-2.dtsi | 23 +++ > arch/riscv/boot/dts/starfive/jh7110.dtsi | 47 +++++ > drivers/mmc/host/Kconfig | 10 + > drivers/mmc/host/Makefile | 1 + > drivers/mmc/host/dw_mmc-starfive.c | 186 ++++++++++++++++++ > 8 files changed, 406 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml > create mode 100644 Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml > create mode 100644 drivers/mmc/host/dw_mmc-starfive.c > I have dropped the v3 patches and applied patch1 and patch2 from the v4 series instead, for my next branch, thanks! Kind regards Uffe
On 2023/2/16 0:49, Shengyu Qu wrote: > Hello William, > > Thanks for your reply. So there's v5 series? Btw, please fix maintainer information: > > https://patchwork.kernel.org/project/linux-riscv/patch/20230215080203.27445-1-lukas.bulwahn@gmail.com/ > > Best regards, > > Shengyu > Hi Shengyu, Here is v4 series, and I fixed the maintainer information in this series which Uffe would merge in his next branch. Thanks for taking time to review this patch series. Best Regards William >> >> On 2023/2/15 19:59, Shengyu Qu wrote: >>> Hello William, >>> >>> Are you sure changing driver is better than changing yaml bindings? All >>> >>> previous version sent was syscon and sysreg seems not consistent with >>> >>> other codes. >>> >>> Best regards, >>> >>> Shengyu >>> >> Hi Shengyu, >> >> After discussing with colleagues, we decided to restore the lable name to >> sys_syscon, and sysreg was just a unique name for the functionality of MMC, >> which will be used in all future versions. >> >> Thanks for taking time reviewing this patch series. >> >> Best Regards >> William >> >>>> Add documentation to describe StarFive designware mobile storage >>>> host controller driver. >>>> >>>> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >>>> --- >>>> .../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..51e1b04e799f >>>> --- /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 <william.qiu@starfivetech.com> >>>> + >>>> +properties: >>>> + compatible: >>>> + const: starfive,jh7110-mmc >>>> + >>>> + reg: >>>> + maxItems: 1 >>>> + >>>> + clocks: >>>> + items: >>>> + - description: biu clock >>>> + - description: ciu clock >>>> + >>>> + clock-names: >>>> + items: >>>> + - const: biu >>>> + - const: ciu >>>> + >>>> + interrupts: >>>> + maxItems: 1 >>>> + >>>> + starfive,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 = <&sys_syscon 0x14 0x1a 0x7c000000>; >>>> + };
On 15/02/2023 12:59, Shengyu Qu wrote: > Hello William, > > Are you sure changing driver is better than changing yaml bindings? All What do you mean - changing driver? This is new driver, new code, isn't it? Best regards, Krzysztof
On 15/02/2023 12:32, William Qiu wrote: > Add documentation to describe StarFive System Controller Registers. > > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > --- Thank you for your patch. There is something to discuss/improve. > +properties: > + compatible: > + items: > + - enum: > + - starfive,jh7110-stg-syscon > + - starfive,jh7110-sys-syscon > + - starfive,jh7110-aon-syscon Maybe keep them ordered alphabetically? > + - const: syscon > + > + reg: > + maxItems: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + syscon@10240000 { > + compatible = "starfive,jh7110-stg-syscon", "syscon"; > + reg = <0x10240000 0x1000>; > + }; Keep only one example. All others are the same. Best regards, Krzysztof
On Thu, Feb 16, 2023 at 11:23:00AM +0100, Krzysztof Kozlowski wrote: > On 15/02/2023 12:32, William Qiu wrote: > > Add documentation to describe StarFive System Controller Registers. > > > > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > > --- > > Thank you for your patch. There is something to discuss/improve. > > > +properties: > > + compatible: > > + items: > > + - enum: > > + - starfive,jh7110-stg-syscon > > + - starfive,jh7110-sys-syscon > > + - starfive,jh7110-aon-syscon > > Maybe keep them ordered alphabetically? > > > + - const: syscon > > + > > + reg: > > + maxItems: 1 > > + > > +required: > > + - compatible > > + - reg > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + syscon@10240000 { > > + compatible = "starfive,jh7110-stg-syscon", "syscon"; > > + reg = <0x10240000 0x1000>; > > + }; > > Keep only one example. All others are the same. With these fixed: Reviewed-by: Conor Dooley <conor.dooley@microchip.com> @Krzysztof, I assume the location of the binding is okay with you since you didn't object to it & I suppose this one is up to me to apply if so. Cheers, Conor.
On 2023/2/16 18:23, Krzysztof Kozlowski wrote: > On 15/02/2023 12:32, William Qiu wrote: >> Add documentation to describe StarFive System Controller Registers. >> >> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >> --- > > Thank you for your patch. There is something to discuss/improve. > >> +properties: >> + compatible: >> + items: >> + - enum: >> + - starfive,jh7110-stg-syscon >> + - starfive,jh7110-sys-syscon >> + - starfive,jh7110-aon-syscon > > Maybe keep them ordered alphabetically? > I'm sorting by register address, or I can keep them ordered alphabetically，which is better? >> + - const: syscon >> + >> + reg: >> + maxItems: 1 >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + syscon@10240000 { >> + compatible = "starfive,jh7110-stg-syscon", "syscon"; >> + reg = <0x10240000 0x1000>; >> + }; > > Keep only one example. All others are the same. > Will update in next version. Thanks for taking times to review this patch series. Best regards William > > Best regards, > Krzysztof >
On Thu, Feb 16, 2023 at 11:21:16AM +0100, Krzysztof Kozlowski wrote: > On 15/02/2023 12:59, Shengyu Qu wrote: > > Hello William, > > > > Are you sure changing driver is better than changing yaml bindings? All > > What do you mean - changing driver? This is new driver, new code, isn't it? Changing w.r.t. the v3 that was applied I suppose. The v3 was dropped and patches 1 & 2 here have been applied instead, so this request from Shengyu is moot now anyway. Cheers, Conor.
On 16/02/2023 11:29, Conor Dooley wrote: > On Thu, Feb 16, 2023 at 11:23:00AM +0100, Krzysztof Kozlowski wrote: >> On 15/02/2023 12:32, William Qiu wrote: >>> Add documentation to describe StarFive System Controller Registers. >>> >>> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >>> --- >> >> Thank you for your patch. There is something to discuss/improve. >> >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - starfive,jh7110-stg-syscon >>> + - starfive,jh7110-sys-syscon >>> + - starfive,jh7110-aon-syscon >> >> Maybe keep them ordered alphabetically? >> >>> + - const: syscon >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + syscon@10240000 { >>> + compatible = "starfive,jh7110-stg-syscon", "syscon"; >>> + reg = <0x10240000 0x1000>; >>> + }; >> >> Keep only one example. All others are the same. > > With these fixed: > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > @Krzysztof, I assume the location of the binding is okay with you since > you didn't object to it & I suppose this one is up to me to apply if so. Yeah, generic sysreg devices go to soc. If their primary functions were different (e.g. clock controller which also is syscon), then they should go to respective directories, but it's not the case here, I think. Best regards, Krzysztof
On 16/02/2023 11:30, William Qiu wrote: > > > On 2023/2/16 18:23, Krzysztof Kozlowski wrote: >> On 15/02/2023 12:32, William Qiu wrote: >>> Add documentation to describe StarFive System Controller Registers. >>> >>> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >>> --- >> >> Thank you for your patch. There is something to discuss/improve. >> >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - starfive,jh7110-stg-syscon >>> + - starfive,jh7110-sys-syscon >>> + - starfive,jh7110-aon-syscon >> >> Maybe keep them ordered alphabetically? >> > > I'm sorting by register address, or I can keep them ordered > alphabetically，which is better? We don't know register address here, so I propose alphabetically. Best regards, Krzysztof
> On Thu, Feb 16, 2023 at 11:21:16AM +0100, Krzysztof Kozlowski wrote: >> On 15/02/2023 12:59, Shengyu Qu wrote: >>> Hello William, >>> >>> Are you sure changing driver is better than changing yaml bindings? All >> What do you mean - changing driver? This is new driver, new code, isn't it? > Changing w.r.t. the v3 that was applied I suppose. > The v3 was dropped and patches 1 & 2 here have been applied instead, so > this request from Shengyu is moot now anyway. > > Cheers, > Conor. That's my mistake. I misunderstood current situation :( Best regards, Shengyu
On 2023/2/15 20:37, Ulf Hansson wrote: > On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@starfivetech.com> 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/20221220011247.35560-1-hal.feng@starfivetech.com/ >> >> Changes v3->v4: >> - Added documentation to describe StarFive System Controller Registers. >> - Added aon_syscon and stg_syscon node. >> - Fixed some checkpatch errors/warnings. >> >> Changes v2->v3: >> - Wraped commit message according to Linux coding style. >> - Rephrased the description of the patches. >> - Changed the description of syscon regsiter. >> - Dropped redundant properties. >> >> Changes v1->v2: >> - 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. >> >> William Qiu (4): >> dt-bindings: mmc: Add StarFive MMC module >> mmc: starfive: Add sdio/emmc driver support >> riscv: dts: starfive: Add mmc node >> dt-bindings: syscon: Add StarFive syscon doc >> >> .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++ >> .../bindings/soc/starfive/jh7110-syscon.yaml | 51 +++++ >> MAINTAINERS | 11 ++ >> .../jh7110-starfive-visionfive-2.dtsi | 23 +++ >> arch/riscv/boot/dts/starfive/jh7110.dtsi | 47 +++++ >> drivers/mmc/host/Kconfig | 10 + >> drivers/mmc/host/Makefile | 1 + >> drivers/mmc/host/dw_mmc-starfive.c | 186 ++++++++++++++++++ >> 8 files changed, 406 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml >> create mode 100644 Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml >> create mode 100644 drivers/mmc/host/dw_mmc-starfive.c >> > > I have dropped the v3 patches and applied patch1 and patch2 from the > v4 series instead, for my next branch, thanks! > > Kind regards > Uffe Hi Uffe, Sorry to bother you.But I found a bug that in drivers/mmc/host/dw_mmc-starfive.c: 47 static int dw_mci_starfive_execute_tuning(struct dw_mci_slot *slot, 48 u32 opcode) 49 { 50 static const int grade = MAX_DELAY_CHAIN; 51 struct dw_mci *host = slot->host; 52 struct starfive_priv *priv = host->priv; 53 int rise_point = -1, fall_point = -1; 54 int err, prev_err; 55 int i; 56 bool found = 0; 57 u32 regval; 58 59 /* 60 * Use grade as the max delay chain, and use the rise_point and 61 * fall_point to ensure the best sampling point of a data input 62 * signals. 63 */ 64 for (i = 0; i < grade; i++) { 65 regval = i << priv->syscon_shift; 66 err = regmap_update_bits(priv->reg_syscon, priv->syscon_offset, 67 priv->syscon_mask, regval); 68 if (err) 69 return err; 70 mci_writel(host, RINTSTS, ALL_INT_CLR); 71 72 err = mmc_send_tuning(slot->mmc, opcode, NULL); 73 if (!err) 74 found = 1; 75 76 if (i > 0) { --> 77 if (err && !prev_err) prev_err was never initialized to zero. So I'm here to ask for your suggestion, should I send a new version to fix it or send you a patch with a fixes tag? Best regards William
On Mon, 27 Feb 2023 at 08:47, William Qiu <william.qiu@starfivetech.com> wrote: > > > > On 2023/2/15 20:37, Ulf Hansson wrote: > > On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@starfivetech.com> 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/20221220011247.35560-1-hal.feng@starfivetech.com/ > >> > >> Changes v3->v4: > >> - Added documentation to describe StarFive System Controller Registers. > >> - Added aon_syscon and stg_syscon node. > >> - Fixed some checkpatch errors/warnings. > >> > >> Changes v2->v3: > >> - Wraped commit message according to Linux coding style. > >> - Rephrased the description of the patches. > >> - Changed the description of syscon regsiter. > >> - Dropped redundant properties. > >> > >> Changes v1->v2: > >> - 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. > >> > >> William Qiu (4): > >> dt-bindings: mmc: Add StarFive MMC module > >> mmc: starfive: Add sdio/emmc driver support > >> riscv: dts: starfive: Add mmc node > >> dt-bindings: syscon: Add StarFive syscon doc > >> > >> .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++ > >> .../bindings/soc/starfive/jh7110-syscon.yaml | 51 +++++ > >> MAINTAINERS | 11 ++ > >> .../jh7110-starfive-visionfive-2.dtsi | 23 +++ > >> arch/riscv/boot/dts/starfive/jh7110.dtsi | 47 +++++ > >> drivers/mmc/host/Kconfig | 10 + > >> drivers/mmc/host/Makefile | 1 + > >> drivers/mmc/host/dw_mmc-starfive.c | 186 ++++++++++++++++++ > >> 8 files changed, 406 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml > >> create mode 100644 Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml > >> create mode 100644 drivers/mmc/host/dw_mmc-starfive.c > >> > > > > I have dropped the v3 patches and applied patch1 and patch2 from the > > v4 series instead, for my next branch, thanks! > > > > Kind regards > > Uffe > > Hi Uffe, > > Sorry to bother you.But I found a bug that in drivers/mmc/host/dw_mmc-starfive.c: > > 47 static int dw_mci_starfive_execute_tuning(struct dw_mci_slot *slot, > 48 u32 opcode) > 49 { > 50 static const int grade = MAX_DELAY_CHAIN; > 51 struct dw_mci *host = slot->host; > 52 struct starfive_priv *priv = host->priv; > 53 int rise_point = -1, fall_point = -1; > 54 int err, prev_err; > 55 int i; > 56 bool found = 0; > 57 u32 regval; > 58 > 59 /* > 60 * Use grade as the max delay chain, and use the rise_point and > 61 * fall_point to ensure the best sampling point of a data input > 62 * signals. > 63 */ > 64 for (i = 0; i < grade; i++) { > 65 regval = i << priv->syscon_shift; > 66 err = regmap_update_bits(priv->reg_syscon, priv->syscon_offset, > 67 priv->syscon_mask, regval); > 68 if (err) > 69 return err; > 70 mci_writel(host, RINTSTS, ALL_INT_CLR); > 71 > 72 err = mmc_send_tuning(slot->mmc, opcode, NULL); > 73 if (!err) > 74 found = 1; > 75 > 76 if (i > 0) { > --> 77 if (err && !prev_err) > > prev_err was never initialized to zero. > > So I'm here to ask for your suggestion, should I send a new version > to fix it or send you a patch with a fixes tag? Please send a new incremental patch on top. I will queue it up as a fix for v6.3-rc[n]. Kind regards Uffe
On 2023/2/27 22:53, Ulf Hansson wrote: > On Mon, 27 Feb 2023 at 08:47, William Qiu <william.qiu@starfivetech.com> wrote: >> >> >> >> On 2023/2/15 20:37, Ulf Hansson wrote: >> > On Wed, 15 Feb 2023 at 12:32, William Qiu <william.qiu@starfivetech.com> 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/20221220011247.35560-1-hal.feng@starfivetech.com/ >> >> >> >> Changes v3->v4: >> >> - Added documentation to describe StarFive System Controller Registers. >> >> - Added aon_syscon and stg_syscon node. >> >> - Fixed some checkpatch errors/warnings. >> >> >> >> Changes v2->v3: >> >> - Wraped commit message according to Linux coding style. >> >> - Rephrased the description of the patches. >> >> - Changed the description of syscon regsiter. >> >> - Dropped redundant properties. >> >> >> >> Changes v1->v2: >> >> - 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. >> >> >> >> William Qiu (4): >> >> dt-bindings: mmc: Add StarFive MMC module >> >> mmc: starfive: Add sdio/emmc driver support >> >> riscv: dts: starfive: Add mmc node >> >> dt-bindings: syscon: Add StarFive syscon doc >> >> >> >> .../bindings/mmc/starfive,jh7110-mmc.yaml | 77 ++++++++ >> >> .../bindings/soc/starfive/jh7110-syscon.yaml | 51 +++++ >> >> MAINTAINERS | 11 ++ >> >> .../jh7110-starfive-visionfive-2.dtsi | 23 +++ >> >> arch/riscv/boot/dts/starfive/jh7110.dtsi | 47 +++++ >> >> drivers/mmc/host/Kconfig | 10 + >> >> drivers/mmc/host/Makefile | 1 + >> >> drivers/mmc/host/dw_mmc-starfive.c | 186 ++++++++++++++++++ >> >> 8 files changed, 406 insertions(+) >> >> create mode 100644 Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml >> >> create mode 100644 Documentation/devicetree/bindings/soc/starfive/jh7110-syscon.yaml >> >> create mode 100644 drivers/mmc/host/dw_mmc-starfive.c >> >> >> > >> > I have dropped the v3 patches and applied patch1 and patch2 from the >> > v4 series instead, for my next branch, thanks! >> > >> > Kind regards >> > Uffe >> >> Hi Uffe, >> >> Sorry to bother you.But I found a bug that in drivers/mmc/host/dw_mmc-starfive.c: >> >> 47 static int dw_mci_starfive_execute_tuning(struct dw_mci_slot *slot, >> 48 u32 opcode) >> 49 { >> 50 static const int grade = MAX_DELAY_CHAIN; >> 51 struct dw_mci *host = slot->host; >> 52 struct starfive_priv *priv = host->priv; >> 53 int rise_point = -1, fall_point = -1; >> 54 int err, prev_err; >> 55 int i; >> 56 bool found = 0; >> 57 u32 regval; >> 58 >> 59 /* >> 60 * Use grade as the max delay chain, and use the rise_point and >> 61 * fall_point to ensure the best sampling point of a data input >> 62 * signals. >> 63 */ >> 64 for (i = 0; i < grade; i++) { >> 65 regval = i << priv->syscon_shift; >> 66 err = regmap_update_bits(priv->reg_syscon, priv->syscon_offset, >> 67 priv->syscon_mask, regval); >> 68 if (err) >> 69 return err; >> 70 mci_writel(host, RINTSTS, ALL_INT_CLR); >> 71 >> 72 err = mmc_send_tuning(slot->mmc, opcode, NULL); >> 73 if (!err) >> 74 found = 1; >> 75 >> 76 if (i > 0) { >> --> 77 if (err && !prev_err) >> >> prev_err was never initialized to zero. >> >> So I'm here to ask for your suggestion, should I send a new version >> to fix it or send you a patch with a fixes tag? > > Please send a new incremental patch on top. I will queue it up as a > fix for v6.3-rc[n]. > > Kind regards > Uffe Fine, I'll do it in my next version. Thanks for your apply. Best regards William
Hey William, On Thu, Feb 16, 2023 at 11:31:45AM +0100, Krzysztof Kozlowski wrote: > On 16/02/2023 11:29, Conor Dooley wrote: > > On Thu, Feb 16, 2023 at 11:23:00AM +0100, Krzysztof Kozlowski wrote: > >> On 15/02/2023 12:32, William Qiu wrote: > >>> Add documentation to describe StarFive System Controller Registers. > >>> > >>> Signed-off-by: William Qiu <william.qiu@starfivetech.com> > >>> --- > >> > >> Thank you for your patch. There is something to discuss/improve. Could you please submit a v5 of this, with the bits below fixed, whenever Hal sends their next version of the base dts series? There's no maintainers coverage for a soc/starfive subdirectory of dt-bindings yet, so please CC conor@kernel.org & linux-riscv@lists.infradead.com on the patch. Thanks, Conor. > >> > >>> +properties: > >>> + compatible: > >>> + items: > >>> + - enum: > >>> + - starfive,jh7110-stg-syscon > >>> + - starfive,jh7110-sys-syscon > >>> + - starfive,jh7110-aon-syscon > >> > >> Maybe keep them ordered alphabetically? > >> > >>> + - const: syscon > >>> + > >>> + reg: > >>> + maxItems: 1 > >>> + > >>> +required: > >>> + - compatible > >>> + - reg > >>> + > >>> +additionalProperties: false > >>> + > >>> +examples: > >>> + - | > >>> + syscon@10240000 { > >>> + compatible = "starfive,jh7110-stg-syscon", "syscon"; > >>> + reg = <0x10240000 0x1000>; > >>> + }; > >> > >> Keep only one example. All others are the same. > > > > With these fixed: > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> > > > > @Krzysztof, I assume the location of the binding is okay with you since > > you didn't object to it & I suppose this one is up to me to apply if so. > > Yeah, generic sysreg devices go to soc. If their primary functions were > different (e.g. clock controller which also is syscon), then they should > go to respective directories, but it's not the case here, I think. > > Best regards, > Krzysztof > >
On 2023/3/6 22:04, Conor Dooley wrote: > Hey William, > > On Thu, Feb 16, 2023 at 11:31:45AM +0100, Krzysztof Kozlowski wrote: >> On 16/02/2023 11:29, Conor Dooley wrote: >> > On Thu, Feb 16, 2023 at 11:23:00AM +0100, Krzysztof Kozlowski wrote: >> >> On 15/02/2023 12:32, William Qiu wrote: >> >>> Add documentation to describe StarFive System Controller Registers. >> >>> >> >>> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >> >>> --- >> >> >> >> Thank you for your patch. There is something to discuss/improve. > > Could you please submit a v5 of this, with the bits below fixed, > whenever Hal sends their next version of the base dts series? > There's no maintainers coverage for a soc/starfive subdirectory of > dt-bindings yet, so please CC conor@kernel.org & > linux-riscv@lists.infradead.com on the patch. > > Thanks, > Conor. > I'll do it today. Best regards William >> >> >> >>> +properties: >> >>> + compatible: >> >>> + items: >> >>> + - enum: >> >>> + - starfive,jh7110-stg-syscon >> >>> + - starfive,jh7110-sys-syscon >> >>> + - starfive,jh7110-aon-syscon >> >> >> >> Maybe keep them ordered alphabetically? >> >> >> >>> + - const: syscon >> >>> + >> >>> + reg: >> >>> + maxItems: 1 >> >>> + >> >>> +required: >> >>> + - compatible >> >>> + - reg >> >>> + >> >>> +additionalProperties: false >> >>> + >> >>> +examples: >> >>> + - | >> >>> + syscon@10240000 { >> >>> + compatible = "starfive,jh7110-stg-syscon", "syscon"; >> >>> + reg = <0x10240000 0x1000>; >> >>> + }; >> >> >> >> Keep only one example. All others are the same. >> > >> > With these fixed: >> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> >> > >> > @Krzysztof, I assume the location of the binding is okay with you since >> > you didn't object to it & I suppose this one is up to me to apply if so. >> >> Yeah, generic sysreg devices go to soc. If their primary functions were >> different (e.g. clock controller which also is syscon), then they should >> go to respective directories, but it's not the case here, I think. >> >> Best regards, >> Krzysztof >> >>
On Wed, 15 Feb 2023 at 13:26, William Qiu <william.qiu@starfivetech.com> wrote: > On 2023/2/15 20:22, Emil Renner Berthing wrote: > > On Wed, 15 Feb 2023 at 13:12, Emil Renner Berthing > > <emil.renner.berthing@canonical.com> wrote: > >> > >> On Wed, 15 Feb 2023 at 12:35, William Qiu <william.qiu@starfivetech.com> wrote: > >> > > >> > Add 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 | 47 +++++++++++++++++++ > >> > 2 files changed, 70 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"; > >> > +}; > > > > These nodes are also still oddly placed in the middle of the external > > clocks. Again please keep the external clocks at the top and then > > order the nodes alphabetically to have some sort of system. > > > > > Hi Emil, > > I'll update it in next version. Hi William, It seems the mmc nodes are still missing from the upstream device tree. The sysreg nodes have been added in Conors riscv-dt-for-next[1] branch, so I don't see any missing dependencies. Could you please update and send a new version of this? [1]: https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/log/?h=riscv-dt-for-next /Emil > Best Regards > William > > >> > &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..17f7b3ee6ca3 100644 > >> > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi > >> > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi > >> > @@ -314,6 +314,11 @@ uart2: serial@10020000 { > >> > status = "disabled"; > >> > }; > >> > > >> > + stg_syscon: syscon@10240000 { > >> > + compatible = "starfive,jh7110-stg-syscon", "syscon"; > >> > + reg = <0x0 0x10240000 0x0 0x1000>; > >> > + }; > >> > + > >> > uart3: serial@12000000 { > >> > compatible = "snps,dw-apb-uart"; > >> > reg = <0x0 0x12000000 0x0 0x10000>; > >> > @@ -370,6 +375,11 @@ syscrg: clock-controller@13020000 { > >> > #reset-cells = <1>; > >> > }; > >> > > >> > + sys_syscon: syscon@13030000 { > >> > + compatible = "starfive,jh7110-sys-syscon", "syscon"; > >> > + reg = <0x0 0x13030000 0x0 0x1000>; > >> > + }; > >> > + > >> > gpio: gpio@13040000 { > >> > compatible = "starfive,jh7110-sys-pinctrl"; > >> > reg = <0x0 0x13040000 0x0 0x10000>; > >> > @@ -397,6 +407,11 @@ aoncrg: clock-controller@17000000 { > >> > #reset-cells = <1>; > >> > }; > >> > > >> > + aon_syscon: syscon@17010000 { > >> > + compatible = "starfive,jh7110-aon-syscon", "syscon"; > >> > + reg = <0x0 0x17010000 0x0 0x1000>; > >> > + }; > >> > + > >> > gpioa: gpio@17020000 { > >> > compatible = "starfive,jh7110-aon-pinctrl"; > >> > reg = <0x0 0x17020000 0x0 0x10000>; > >> > @@ -407,5 +422,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 = <&sys_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,sysreg = <&sys_syscon 0x9c 0x1 0x3e>; > >> > + status = "disabled"; > >> > + }; > >> > >> Hi William, > >> > >> These nodes still don't seem to be sorted by address, eg. by the > >> number after the @ > >> Also please move the dt-binding patch before this one, so dtb_check > >> won't fail no matter where git bisect happens to land. > >> > >> /Emil > >> > >> > }; > >> > }; > >> > -- > >> > 2.34.1 > >> > > >> > > >> > _______________________________________________ > >> > linux-riscv mailing list > >> > linux-riscv@lists.infradead.org > >> > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2023/8/5 21:14, Emil Renner Berthing wrote: > On Wed, 15 Feb 2023 at 13:26, William Qiu <william.qiu@starfivetech.com> wrote: >> On 2023/2/15 20:22, Emil Renner Berthing wrote: >> > On Wed, 15 Feb 2023 at 13:12, Emil Renner Berthing >> > <emil.renner.berthing@canonical.com> wrote: >> >> >> >> On Wed, 15 Feb 2023 at 12:35, William Qiu <william.qiu@starfivetech.com> wrote: >> >> > >> >> > Add 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 | 47 +++++++++++++++++++ >> >> > 2 files changed, 70 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"; >> >> > +}; >> > >> > These nodes are also still oddly placed in the middle of the external >> > clocks. Again please keep the external clocks at the top and then >> > order the nodes alphabetically to have some sort of system. >> > >> >> >> Hi Emil, >> >> I'll update it in next version. > > Hi William, > > It seems the mmc nodes are still missing from the upstream device > tree. The sysreg nodes have been added in Conors riscv-dt-for-next[1] > branch, so I don't see any missing dependencies. Could you please > update and send a new version of this? > > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/log/?h=riscv-dt-for-next > > /Emil > Hi Emil, I will start to do the upstream work of this part from this week. Since the mmc driver has some modifications, I will send a separate patch series. Best Regards, William >> Best Regards >> William >> >> >> > &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..17f7b3ee6ca3 100644 >> >> > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi >> >> > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> >> > @@ -314,6 +314,11 @@ uart2: serial@10020000 { >> >> > status = "disabled"; >> >> > }; >> >> > >> >> > + stg_syscon: syscon@10240000 { >> >> > + compatible = "starfive,jh7110-stg-syscon", "syscon"; >> >> > + reg = <0x0 0x10240000 0x0 0x1000>; >> >> > + }; >> >> > + >> >> > uart3: serial@12000000 { >> >> > compatible = "snps,dw-apb-uart"; >> >> > reg = <0x0 0x12000000 0x0 0x10000>; >> >> > @@ -370,6 +375,11 @@ syscrg: clock-controller@13020000 { >> >> > #reset-cells = <1>; >> >> > }; >> >> > >> >> > + sys_syscon: syscon@13030000 { >> >> > + compatible = "starfive,jh7110-sys-syscon", "syscon"; >> >> > + reg = <0x0 0x13030000 0x0 0x1000>; >> >> > + }; >> >> > + >> >> > gpio: gpio@13040000 { >> >> > compatible = "starfive,jh7110-sys-pinctrl"; >> >> > reg = <0x0 0x13040000 0x0 0x10000>; >> >> > @@ -397,6 +407,11 @@ aoncrg: clock-controller@17000000 { >> >> > #reset-cells = <1>; >> >> > }; >> >> > >> >> > + aon_syscon: syscon@17010000 { >> >> > + compatible = "starfive,jh7110-aon-syscon", "syscon"; >> >> > + reg = <0x0 0x17010000 0x0 0x1000>; >> >> > + }; >> >> > + >> >> > gpioa: gpio@17020000 { >> >> > compatible = "starfive,jh7110-aon-pinctrl"; >> >> > reg = <0x0 0x17020000 0x0 0x10000>; >> >> > @@ -407,5 +422,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 = <&sys_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,sysreg = <&sys_syscon 0x9c 0x1 0x3e>; >> >> > + status = "disabled"; >> >> > + }; >> >> >> >> Hi William, >> >> >> >> These nodes still don't seem to be sorted by address, eg. by the >> >> number after the @ >> >> Also please move the dt-binding patch before this one, so dtb_check >> >> won't fail no matter where git bisect happens to land. >> >> >> >> /Emil >> >> >> >> > }; >> >> > }; >> >> > -- >> >> > 2.34.1 >> >> > >> >> > >> >> > _______________________________________________ >> >> > linux-riscv mailing list >> >> > linux-riscv@lists.infradead.org >> >> > http://lists.infradead.org/mailman/listinfo/linux-riscv