diff mbox series

[v1,7/7] riscv: dts: starfive: visionfive-v2: Add phy delay_chain configuration

Message ID 20221201090242.2381-8-yanhong.wang@starfivetech.com (mailing list archive)
State Changes Requested
Headers show
Series Add Ethernet driver for StarFive JH7110 SoC | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

yanhong wang Dec. 1, 2022, 9:02 a.m. UTC
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(+)

Comments

Conor Dooley Dec. 1, 2022, 5:49 p.m. UTC | #1
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
>
Conor Dooley Dec. 1, 2022, 7:30 p.m. UTC | #2
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
> >
Andrew Lunn Dec. 1, 2022, 7:42 p.m. UTC | #3
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
yanhong wang Dec. 9, 2022, 1:19 a.m. UTC | #4
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
>>
yanhong wang Dec. 9, 2022, 1:28 a.m. UTC | #5
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 mbox series

Patch

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
index c8946cf3a268..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>;
+		};
+	};
+};