| Message ID | 20221201090242.2381-1-yanhong.wang@starfivetech.com |
|---|---|
| Headers | show |
| Series | Add Ethernet driver for StarFive JH7110 SoC | expand |
On 2022/12/2 0:18, Krzysztof Kozlowski wrote: > On 01/12/2022 10:02, Yanhong Wang wrote: >> Add dwmac-5.20 version to snps.dwmac.yaml > > Drop full stop from subject and add it here instead. > Will update in the next version. >> >> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> > > Two people contributed this one single line? > Emil made this patch and I submitted it. > Best regards, > Krzysztof >
On 2022/12/2 0:21, Krzysztof Kozlowski wrote: > On 01/12/2022 10:02, Yanhong Wang wrote: >> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs. > > Subject: drop second, redundant "bindings". > I'll fix the title in the next version. >> >> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >> --- >> .../devicetree/bindings/net/snps,dwmac.yaml | 1 + >> .../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++++++++++ >> MAINTAINERS | 5 + >> 3 files changed, 112 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml >> >> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >> index d8779d3de3d6..13c5928d7170 100644 >> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml >> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >> @@ -33,6 +33,7 @@ select: >> - snps,dwmac-5.20 >> - snps,dwxgmac >> - snps,dwxgmac-2.10 >> + - starfive,dwmac >> >> # Deprecated >> - st,spear600-gmac >> diff --git a/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml >> new file mode 100644 >> index 000000000000..561cf2a713ab >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml > > Filename should base on compatible. > Will update file name. >> @@ -0,0 +1,106 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +# Copyright (C) 2022 StarFive Technology Co., Ltd. >> +%YAML 1.2 >> +--- >> +$id: "http://devicetree.org/schemas/net/dwmac-starfive.yaml#" > > Does not look like you tested the bindings. Please run `make > dt_binding_check` (see > Documentation/devicetree/bindings/writing-schema.rst for instructions). > >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#" > > Drop quotes from both lines. > Will remove quotes. >> + >> +title: StarFive DWMAC glue layer >> + >> +maintainers: >> + - Yanhong Wang <yanhong.wang@starfivetech.com> >> + >> +select: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - starfive,dwmac >> + required: >> + - compatible >> + >> +allOf: >> + - $ref: "snps,dwmac.yaml#" > > Drop quotes. > Will remove quotes. >> + >> +properties: >> + compatible: >> + oneOf: > > Drop oneOf. You do not have more cases here. > Will remove oneOf. >> + - items: >> + - enum: >> + - starfive,dwmac > > Wrong indentation.... kind of expected since you did not test the bindings. > Will fix. >> + - const: snps,dwmac-5.20 >> + >> + clocks: >> + items: >> + - description: GMAC main clock >> + - description: GMAC AHB clock >> + - description: PTP clock >> + - description: TX clock >> + - description: GTXC clock >> + - description: GTX clock >> + >> + clock-names: >> + contains: >> + enum: >> + - stmmaceth >> + - pclk >> + - ptp_ref >> + - tx >> + - gtxc >> + - gtx > > Names should be specific and with fixed order, just like clocks are. > Will fix. >> + >> +required: >> + - compatible >> + - clocks >> + - clock-names >> + - resets >> + - reset-names >> + >> +unevaluatedProperties: false >> + > > Best regards, > Krzysztof >
On 2022/12/2 1:35, Conor Dooley wrote: > On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote: >> On 01/12/2022 10:02, Yanhong Wang wrote: >> > Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs. >> >> Subject: drop second, redundant "bindings". >> >> > >> > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >> > --- >> > .../devicetree/bindings/net/snps,dwmac.yaml | 1 + >> > +properties: >> > + compatible: >> > + oneOf: >> >> Drop oneOf. You do not have more cases here. >> >> > + - items: >> > + - enum: >> > + - starfive,dwmac >> >> Wrong indentation.... kind of expected since you did not test the bindings. >> >> > + - const: snps,dwmac-5.20 > > Disclaimer: no familiarity with the version info with DW stuff > > Is it a bit foolish to call this binding "starfive,dwmac"? Could there > not be another StarFive SoC in the future that uses another DW mac IP > version & this would be better off as "starfive,jh7110-dwmac" or similar? > The StarFive JH8100 SoC in the future that uses the same mac IP version, so call this binding "starfive,dwmac". > Thanks, > Conor. >
On 02/12/2022 03:53, yanhong wang wrote: > > > On 2022/12/2 0:18, Krzysztof Kozlowski wrote: >> On 01/12/2022 10:02, Yanhong Wang wrote: >>> Add dwmac-5.20 version to snps.dwmac.yaml >> >> Drop full stop from subject and add it here instead. >> > > Will update in the next version. > >>> >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >> >> Two people contributed this one single line? >> > > Emil made this patch and I submitted it. If Emil made this patch, then your From field is incorrect. Best regards, Krzysztof
On 02/12/2022 08:16, yanhong wang wrote: > > > On 2022/12/2 1:35, Conor Dooley wrote: >> On Thu, Dec 01, 2022 at 05:21:04PM +0100, Krzysztof Kozlowski wrote: >>> On 01/12/2022 10:02, Yanhong Wang wrote: >>>> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs. >>> >>> Subject: drop second, redundant "bindings". >>> >>>> >>>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >>>> --- >>>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 + >>>> +properties: >>>> + compatible: >>>> + oneOf: >>> >>> Drop oneOf. You do not have more cases here. >>> >>>> + - items: >>>> + - enum: >>>> + - starfive,dwmac >>> >>> Wrong indentation.... kind of expected since you did not test the bindings. >>> >>>> + - const: snps,dwmac-5.20 >> >> Disclaimer: no familiarity with the version info with DW stuff >> >> Is it a bit foolish to call this binding "starfive,dwmac"? Could there >> not be another StarFive SoC in the future that uses another DW mac IP >> version & this would be better off as "starfive,jh7110-dwmac" or similar? >> > > The StarFive JH8100 SoC in the future that uses the same mac IP version, so call this binding "starfive,dwmac". It's not a reason to make compatible generic. Best regards, Krzysztof
On 01/12/2022 17:45, Conor Dooley wrote: > Hey Yanhong, > > On Thu, Dec 01, 2022 at 07:36:29AM -0600, Rob Herring wrote: >> >> On Thu, 01 Dec 2022 17:02:38 +0800, Yanhong Wang wrote: >>> Add bindings for the StarFive dwmac module on the StarFive RISC-V SoCs. >>> >>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >>> --- >>> .../devicetree/bindings/net/snps,dwmac.yaml | 1 + >>> .../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++++++++++ >>> MAINTAINERS | 5 + >>> 3 files changed, 112 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml >>> >> >> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >> on your patch (DT_CHECKER_FLAGS is new in v5.13): >> >> yamllint warnings/errors: >> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml:30:16: [warning] wrong indentation: expected 14 but found 15 (indentation) >> >> dtschema/dtc warnings/errors: >> ./Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml: $id: relative path/filename doesn't match actual path or filename >> expected: http://devicetree.org/schemas/net/starfive,dwmac-plat.yaml# >> Documentation/devicetree/bindings/net/starfive,dwmac-plat.example.dts:21:18: fatal error: dt-bindings/clock/starfive-jh7110.h: No such file or directory >> 21 | #include <dt-bindings/clock/starfive-jh7110.h> > > Perhaps, rather than putting a long list of "prerequisite-patch-id" in > your cover letters etc, you drop the need for headers from your bindings > entirely? Otherwise, you need to wait for the clock bindings to be applied > before any of your other peripherals etc can have drivers/bindings upstream. > > AFAIU, and Rob/Krzk please correct me, the example in a dt-binding > really is an *example* and there's no requirement for it to match the > jh7110 dts exactly. Because of that you can drop the header & just do > something like `clocks = <&clk 7>, <&clk 77>;` etc and the example is > still valid. Same goes for all of the other driver patchsets for new > StarFive stuff, like the pmu or crypto, that also have dt-bindings. > > The only person who has to worry then about dependencies is me when I > apply the .dts patches :) It's nice when example looks good and matching final DTS, but that's not a requirement. Pretty often clocks/resets/domains are mocked for the example. Best regards, Krzysztof
On 2022/12/1 23:40, Andrew Lunn wrote: >> +static const struct ytphy_reg_field ytphy_rxtxd_grp[] = { >> + { "rx_delay_sel", GENMASK(13, 10), 0x0 }, >> + { "tx_delay_sel_fe", GENMASK(7, 4), 0xf }, >> + { "tx_delay_sel", GENMASK(3, 0), 0x1 } >> +}; >> + >> +static const struct ytphy_reg_field ytphy_txinver_grp[] = { >> + { "tx_inverted_1000", BIT(14), 0x0 }, >> + { "tx_inverted_100", BIT(14), 0x0 }, >> + { "tx_inverted_10", BIT(14), 0x0 } >> +}; >> + >> +static const struct ytphy_reg_field ytphy_rxden_grp[] = { >> + { "rxc_dly_en", BIT(8), 0x1 } >> +}; >> + >> +static int ytphy_config_init(struct phy_device *phydev) >> +{ >> + struct device_node *of_node; >> + u32 val; >> + u32 mask; >> + u32 cfg; >> + int ret; >> + int i = 0; >> + >> + of_node = phydev->mdio.dev.of_node; >> + if (of_node) { >> + ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg); > > You need to document the device tree binding. > > Frank Sae always gets the locking wrong in this driver. Have you > tested your patch with lockdep enabled? > I will add document to describe details in the next version, such as "rx_delay_sel","tx_inverted_10" etc. > Andrew
On Fri, 2 Dec 2022 at 09:04, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 02/12/2022 03:53, yanhong wang wrote: > > > > > > On 2022/12/2 0:18, Krzysztof Kozlowski wrote: > >> On 01/12/2022 10:02, Yanhong Wang wrote: > >>> Add dwmac-5.20 version to snps.dwmac.yaml > >> > >> Drop full stop from subject and add it here instead. > >> > > > > Will update in the next version. > > > >>> > >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > >>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> > >> > >> Two people contributed this one single line? > >> > > > > Emil made this patch and I submitted it. > > If Emil made this patch, then your From field is incorrect. Yes, please don't change the author of the commits you cherry-picked from my tree. But now I'm curious. Did you check with your colleagues that the dwmac IP on the SoC is in fact version 5.20? This was just an educated guess from my side. /Emil > Best regards, > Krzysztof > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2022/12/7 21:56, Emil Renner Berthing wrote: > On Fri, 2 Dec 2022 at 09:04, Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 02/12/2022 03:53, yanhong wang wrote: >> > >> > >> > On 2022/12/2 0:18, Krzysztof Kozlowski wrote: >> >> On 01/12/2022 10:02, Yanhong Wang wrote: >> >>> Add dwmac-5.20 version to snps.dwmac.yaml >> >> >> >> Drop full stop from subject and add it here instead. >> >> >> > >> > Will update in the next version. >> > >> >>> >> >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >> >>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >> >> >> >> Two people contributed this one single line? >> >> >> > >> > Emil made this patch and I submitted it. >> >> If Emil made this patch, then your From field is incorrect. > > Yes, please don't change the author of the commits you cherry-picked > from my tree. > > But now I'm curious. Did you check with your colleagues that the dwmac > IP on the SoC is in fact version 5.20? I can confirm that the IP version is 5.20 on JH7110 SoC. > This was just an educated guess from my side. > > /Emil > >> Best regards, >> Krzysztof >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2022/12/2 3:30, Conor Dooley wrote: > On Thu, Dec 01, 2022 at 05:49:08PM +0000, Conor Dooley wrote: >> On Thu, Dec 01, 2022 at 05:02:42PM +0800, Yanhong Wang wrote: >> > riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration >> > >> > Add phy delay_chain configuration to support motorcomm phy driver for >> > StarFive VisionFive 2 board. > > nit: please re-word this commit next time around to actually say what > you're doing here. I didn't notice it initially, but this patch is doing > a lot more than adding `delay_chain` configuration. To my dwmac unaware > brain, there's nothing hits for that term outside of the changelog :( > I will re-word the commit message and add another dt-binding to describe the details that such as "rxc_dly_en","tx_inverted_10" etc. > Thanks, > Conor. > >> > >> > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >> > --- >> > .../jh7110-starfive-visionfive-v2.dts | 46 +++++++++++++++++++ >> > 1 file changed, 46 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..2868ef4c74ef 100644 >> > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts >> > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts >> > @@ -15,6 +15,8 @@ >> > >> > aliases { >> > serial0 = &uart0; >> > + ethernet0=&gmac0; >> > + ethernet1=&gmac1; >> >> Please match the whitespace usage of the existing entry. >> >> > }; >> > >> > chosen { >> > @@ -114,3 +116,47 @@ >> > pinctrl-0 = <&uart0_pins>; >> > status = "okay"; >> > }; >> > + >> > +&gmac0 { >> > + status = "okay"; >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + phy-handle = <&phy0>; >> > + status = "okay"; >> > + mdio0 { >> >> A line of whitespace before the child nodes too please :) >> >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + compatible = "snps,dwmac-mdio"; >> > + phy0: ethernet-phy@0 { >> > + reg = <0>; >> > + rxc_dly_en = <1>; >> > + tx_delay_sel_fe = <5>; >> > + tx_delay_sel = <0xa>; >> > + tx_inverted_10 = <0x1>; >> > + tx_inverted_100 = <0x1>; >> > + tx_inverted_1000 = <0x1>; >> > + }; >> > + }; >> > +}; >> > + >> > +&gmac1 { >> > + status = "okay"; >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + phy-handle = <&phy1>; >> > + status = "okay"; >> > + mdio1 { >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + compatible = "snps,dwmac-mdio"; >> > + phy1: ethernet-phy@1 { >> > + reg = <1>; >> > + tx_delay_sel_fe = <5>; >> > + tx_delay_sel = <0>; >> > + rxc_dly_en = <0>; >> > + tx_inverted_10 = <0x1>; >> > + tx_inverted_100 = <0x1>; >> > + tx_inverted_1000 = <0x0>; >> > + }; >> > + }; >> > +}; >> > -- >> > 2.17.1 >> >
This series adds ethernet support for the StarFive JH7110 RISC-V SoC. The series includes PHY and MAC drivers. The PHY model is YT8531 (from Motorcomm Inc), and the MAC version is dwmac-5.20 (from Synopsys DesignWare). For more information and support, you can visit RVspace wiki[1]. This patchset should be applied after the patchset [2], [3], [4]. [1] https://wiki.rvspace.org/ [2] https://lore.kernel.org/all/20221118010627.70576-1-hal.feng@starfivetech.com/ [3] https://lore.kernel.org/all/20221118011108.70715-1-hal.feng@starfivetech.com/ [4] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/ Yanhong Wang (7): dt-bindings: net: snps,dwmac: Add compatible string for dwmac-5.20 version. net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string dt-bindings: net: Add bindings for StarFive dwmac net: phy: motorcomm: Add YT8531 phy support net: stmmac: Add StarFive dwmac supoort riscv: dts: starfive: jh7110: Add ethernet device node riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration .../devicetree/bindings/net/snps,dwmac.yaml | 2 + .../bindings/net/starfive,dwmac-plat.yaml | 106 ++++++++++ MAINTAINERS | 6 + .../jh7110-starfive-visionfive-v2.dts | 46 +++++ arch/riscv/boot/dts/starfive/jh7110.dtsi | 80 ++++++++ drivers/net/ethernet/stmicro/stmmac/Kconfig | 11 ++ drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + .../stmicro/stmmac/dwmac-starfive-plat.c | 147 ++++++++++++++ .../ethernet/stmicro/stmmac/stmmac_platform.c | 3 +- drivers/net/phy/Kconfig | 3 +- drivers/net/phy/motorcomm.c | 185 ++++++++++++++++++ 11 files changed, 588 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/starfive,dwmac-plat.yaml create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa prerequisite-patch-id: 8ebfffa09b478904bf7c516f76e2d824ddb60140 prerequisite-patch-id: e8dd8258a4c4062eee2cf07c4607d52baea71f3a prerequisite-patch-id: d050d884d7b091ff30508a70f5ce5164bb3b72e5 prerequisite-patch-id: 0e41f8cfd4861fcbf6f2e6a2559ce28f0450299e prerequisite-patch-id: 6e1652501859b85f101ff3b15ced585d43c71c1b prerequisite-patch-id: 587628a67adad5c655e5f998bf6c4a368ec07d3c prerequisite-patch-id: 596490c0e397df6c0249c1306fbb1d5bf00b5b83 prerequisite-patch-id: dc873317826b50364344b25ac5cd74e811403f3d prerequisite-patch-id: a50150f41d8e874553023187e22eb24dffae8d16 prerequisite-patch-id: 735e62255c75801bdc4c0b4107850bce821ff7f5 prerequisite-patch-id: 9d2e83a2dd43e193f534283fab73e90b4f435043 prerequisite-patch-id: 7a43e0849a9afa3c6f83547fd16d9271b07619e5 prerequisite-patch-id: e7aa6fb05314bad6d94c465f3f59969871bf3d2e prerequisite-patch-id: 6276b2a23818c65ff2ad3d65b562615690cffee9 prerequisite-patch-id: d834ece14ffb525b8c3e661e78736692f33fca9b prerequisite-patch-id: 4c17a3ce4dae9b788795d915bf775630f5c43c53 prerequisite-patch-id: dabb913fd478e97593e45c23fee4be9fd807f851 prerequisite-patch-id: ba61df106fbe2ada21e8f22c3d2cfaf7809c84b6 prerequisite-patch-id: 287572fb64f83f5d931034f7c75674907584a087 prerequisite-patch-id: 536114f0732646095ef5302a165672b3290d4c75 prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9 prerequisite-patch-id: e09e995700a814a763aa304ad3881a7222acf556 prerequisite-patch-id: 841cd71b556b480d6a5a5e332eeca70d6a76ec3f prerequisite-patch-id: d074c7ffa2917a9f754d5801e3f67bc980f9de4c prerequisite-patch-id: 5f59bc7cbbf1230e5ff4761fa7c1116d4e6e5d71 prerequisite-patch-id: d5da3475c6a3588e11a1678feb565bdd459b548e