| Message ID | 20221201090242.2381-8-yanhong.wang@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Conor Dooley |
| Headers | show |
| Series | Add Ethernet driver for StarFive JH7110 SoC | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Guessing tree name failed |
On Thu, Dec 01, 2022 at 05:02:42PM +0800, Yanhong Wang wrote: > Add phy delay_chain configuration to support motorcomm phy driver for > StarFive VisionFive 2 board. > > 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 >
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 :( 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 > >
On Thu, Dec 01, 2022 at 07:30:16PM +0000, 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 :( Hi Conor I suspect once we see the documentation of the binding, it will get rejected and implemented differently. So i would not worry too much about this at the moment. Andrew
On 2022/12/2 1:49, Conor Dooley wrote: > On Thu, Dec 01, 2022 at 05:02:42PM +0800, Yanhong Wang wrote: >> Add phy delay_chain configuration to support motorcomm phy driver for >> StarFive VisionFive 2 board. >> >> 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. > Will fix in the next version. >> }; >> >> 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 :) > Will fix. >> + #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 >>
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 >> >
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; }; 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 { + #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>; + }; + }; +};
Add phy delay_chain configuration to support motorcomm phy driver for StarFive VisionFive 2 board. Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> --- .../jh7110-starfive-visionfive-v2.dts | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+)