diff mbox series

[net-next,v8,3/6] arm64: dts: mt2712: update ethernet device node

Message ID 20211210013129.811-4-biao.huang@mediatek.com (mailing list archive)
State New, archived
Headers show
Series MediaTek Ethernet Patches on MT8195 | expand

Commit Message

Biao Huang (黄彪) Dec. 10, 2021, 1:31 a.m. UTC
Since there are some changes in ethernet driver,
update ethernet device node in dts to accommodate to it.

Signed-off-by: Biao Huang <biao.huang@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt2712-evb.dts |  1 +
 arch/arm64/boot/dts/mediatek/mt2712e.dtsi   | 14 +++++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

Comments

Matthias Brugger Dec. 15, 2021, 7:20 p.m. UTC | #1
On 10/12/2021 02:31, Biao Huang wrote:
> Since there are some changes in ethernet driver,
> update ethernet device node in dts to accommodate to it.
> 

I have a hard time to understand how the changes in 1/6 and 2/6 are related to 
that.

Please explain better what has changed. Also beware that we shouldn't introduce 
any non-backward compatible DTS changes. That means, the device should work with 
the new driver but an older device tree.

Regards,
Matthias

> Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt2712-evb.dts |  1 +
>   arch/arm64/boot/dts/mediatek/mt2712e.dtsi   | 14 +++++++++-----
>   2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> index 7d369fdd3117..11aa135aa0f3 100644
> --- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> @@ -110,6 +110,7 @@ &eth {
>   	phy-handle = <&ethernet_phy0>;
>   	mediatek,tx-delay-ps = <1530>;
>   	snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
> +	snps,reset-delays-us = <0 10000 10000>;
>   	pinctrl-names = "default", "sleep";
>   	pinctrl-0 = <&eth_default>;
>   	pinctrl-1 = <&eth_sleep>;
> diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> index a9cca9c146fd..9e850e04fffb 100644
> --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> @@ -726,7 +726,7 @@ queue2 {
>   	};
>   
>   	eth: ethernet@1101c000 {
> -		compatible = "mediatek,mt2712-gmac";
> +		compatible = "mediatek,mt2712-gmac", "snps,dwmac-4.20a";
>   		reg = <0 0x1101c000 0 0x1300>;
>   		interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
>   		interrupt-names = "macirq";
> @@ -734,15 +734,19 @@ eth: ethernet@1101c000 {
>   		clock-names = "axi",
>   			      "apb",
>   			      "mac_main",
> -			      "ptp_ref";
> +			      "ptp_ref",
> +			      "rmii_internal";
>   		clocks = <&pericfg CLK_PERI_GMAC>,
>   			 <&pericfg CLK_PERI_GMAC_PCLK>,
>   			 <&topckgen CLK_TOP_ETHER_125M_SEL>,
> -			 <&topckgen CLK_TOP_ETHER_50M_SEL>;
> +			 <&topckgen CLK_TOP_ETHER_50M_SEL>,
> +			 <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
>   		assigned-clocks = <&topckgen CLK_TOP_ETHER_125M_SEL>,
> -				  <&topckgen CLK_TOP_ETHER_50M_SEL>;
> +				  <&topckgen CLK_TOP_ETHER_50M_SEL>,
> +				  <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
>   		assigned-clock-parents = <&topckgen CLK_TOP_ETHERPLL_125M>,
> -					 <&topckgen CLK_TOP_APLL1_D3>;
> +					 <&topckgen CLK_TOP_APLL1_D3>,
> +					 <&topckgen CLK_TOP_ETHERPLL_50M>;
>   		power-domains = <&scpsys MT2712_POWER_DOMAIN_AUDIO>;
>   		mediatek,pericfg = <&pericfg>;
>   		snps,axi-config = <&stmmac_axi_setup>;
>
Matthias Brugger Dec. 15, 2021, 7:22 p.m. UTC | #2
On 10/12/2021 02:31, Biao Huang wrote:
> Since there are some changes in ethernet driver,
> update ethernet device node in dts to accommodate to it.
> 

I have a hard time to understand how the first two patches are related to this 
one. Please be more specific in your commit message.

Also please beware that we should make sure that a newer driver version should 
still work properly with an older device tree, which does not have your changes.

Regards,
Matthias

> Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> ---
>   arch/arm64/boot/dts/mediatek/mt2712-evb.dts |  1 +
>   arch/arm64/boot/dts/mediatek/mt2712e.dtsi   | 14 +++++++++-----
>   2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> index 7d369fdd3117..11aa135aa0f3 100644
> --- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> @@ -110,6 +110,7 @@ &eth {
>   	phy-handle = <&ethernet_phy0>;
>   	mediatek,tx-delay-ps = <1530>;
>   	snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
> +	snps,reset-delays-us = <0 10000 10000>;
>   	pinctrl-names = "default", "sleep";
>   	pinctrl-0 = <&eth_default>;
>   	pinctrl-1 = <&eth_sleep>;
> diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> index a9cca9c146fd..9e850e04fffb 100644
> --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> @@ -726,7 +726,7 @@ queue2 {
>   	};
>   
>   	eth: ethernet@1101c000 {
> -		compatible = "mediatek,mt2712-gmac";
> +		compatible = "mediatek,mt2712-gmac", "snps,dwmac-4.20a";
>   		reg = <0 0x1101c000 0 0x1300>;
>   		interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
>   		interrupt-names = "macirq";
> @@ -734,15 +734,19 @@ eth: ethernet@1101c000 {
>   		clock-names = "axi",
>   			      "apb",
>   			      "mac_main",
> -			      "ptp_ref";
> +			      "ptp_ref",
> +			      "rmii_internal";
>   		clocks = <&pericfg CLK_PERI_GMAC>,
>   			 <&pericfg CLK_PERI_GMAC_PCLK>,
>   			 <&topckgen CLK_TOP_ETHER_125M_SEL>,
> -			 <&topckgen CLK_TOP_ETHER_50M_SEL>;
> +			 <&topckgen CLK_TOP_ETHER_50M_SEL>,
> +			 <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
>   		assigned-clocks = <&topckgen CLK_TOP_ETHER_125M_SEL>,
> -				  <&topckgen CLK_TOP_ETHER_50M_SEL>;
> +				  <&topckgen CLK_TOP_ETHER_50M_SEL>,
> +				  <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
>   		assigned-clock-parents = <&topckgen CLK_TOP_ETHERPLL_125M>,
> -					 <&topckgen CLK_TOP_APLL1_D3>;
> +					 <&topckgen CLK_TOP_APLL1_D3>,
> +					 <&topckgen CLK_TOP_ETHERPLL_50M>;
>   		power-domains = <&scpsys MT2712_POWER_DOMAIN_AUDIO>;
>   		mediatek,pericfg = <&pericfg>;
>   		snps,axi-config = <&stmmac_axi_setup>;
>
Biao Huang (黄彪) Dec. 16, 2021, 1:28 a.m. UTC | #3
Dear Matthias,
	Thanks for your comments~
On Wed, 2021-12-15 at 20:22 +0100, Matthias Brugger wrote:
> 
> On 10/12/2021 02:31, Biao Huang wrote:
> > Since there are some changes in ethernet driver,
> > update ethernet device node in dts to accommodate to it.
> > 
> 
> I have a hard time to understand how the first two patches are
> related to this 
> one. Please be more specific in your commit message.
This dts patch is not related to previous two patches in this series.

Actually, this patch should be sent with commit
"71a55a2315b047352b3d65e2d24724207be85ae2", which added extra RMII
support in driver. But unfortunately, we missed it out.

Is there any proper way to relate this patch to commit
"71a55a2315b047352b3d65e2d24724207be85ae2"? (Fixed tag seems not a good
choice, or just add more details in commit message?)
>  
> Also please beware that we should make sure that a newer driver
> version should 
> still work properly with an older device tree, which does not have
> your changes.
> 
> Regards,
> Matthias
> 
> > Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt2712-evb.dts |  1 +
> >   arch/arm64/boot/dts/mediatek/mt2712e.dtsi   | 14 +++++++++-----
> >   2 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> > b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> > index 7d369fdd3117..11aa135aa0f3 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> > @@ -110,6 +110,7 @@ &eth {
> >   	phy-handle = <&ethernet_phy0>;
> >   	mediatek,tx-delay-ps = <1530>;
> >   	snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
> > +	snps,reset-delays-us = <0 10000 10000>;
> >   	pinctrl-names = "default", "sleep";
> >   	pinctrl-0 = <&eth_default>;
> >   	pinctrl-1 = <&eth_sleep>;
> > diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> > index a9cca9c146fd..9e850e04fffb 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> > @@ -726,7 +726,7 @@ queue2 {
> >   	};
> >   
> >   	eth: ethernet@1101c000 {
> > -		compatible = "mediatek,mt2712-gmac";
> > +		compatible = "mediatek,mt2712-gmac", "snps,dwmac-
> > 4.20a";
> >   		reg = <0 0x1101c000 0 0x1300>;
> >   		interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
> >   		interrupt-names = "macirq";
> > @@ -734,15 +734,19 @@ eth: ethernet@1101c000 {
> >   		clock-names = "axi",
> >   			      "apb",
> >   			      "mac_main",
> > -			      "ptp_ref";
> > +			      "ptp_ref",
> > +			      "rmii_internal";
> >   		clocks = <&pericfg CLK_PERI_GMAC>,
> >   			 <&pericfg CLK_PERI_GMAC_PCLK>,
> >   			 <&topckgen CLK_TOP_ETHER_125M_SEL>,
> > -			 <&topckgen CLK_TOP_ETHER_50M_SEL>;
> > +			 <&topckgen CLK_TOP_ETHER_50M_SEL>,
> > +			 <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
> >   		assigned-clocks = <&topckgen CLK_TOP_ETHER_125M_SEL>,
> > -				  <&topckgen CLK_TOP_ETHER_50M_SEL>;
> > +				  <&topckgen CLK_TOP_ETHER_50M_SEL>,
> > +				  <&topckgen
> > CLK_TOP_ETHER_50M_RMII_SEL>;
> >   		assigned-clock-parents = <&topckgen
> > CLK_TOP_ETHERPLL_125M>,
> > -					 <&topckgen CLK_TOP_APLL1_D3>;
> > +					 <&topckgen CLK_TOP_APLL1_D3>,
> > +					 <&topckgen
> > CLK_TOP_ETHERPLL_50M>;
> >   		power-domains = <&scpsys MT2712_POWER_DOMAIN_AUDIO>;
> >   		mediatek,pericfg = <&pericfg>;
> >   		snps,axi-config = <&stmmac_axi_setup>;
> >
Biao Huang (黄彪) Dec. 16, 2021, 9:19 a.m. UTC | #4
Dear Matthias,
	Thanks for your comments~

On Wed, 2021-12-15 at 20:20 +0100, Matthias Brugger wrote:
> 
> On 10/12/2021 02:31, Biao Huang wrote:
> > Since there are some changes in ethernet driver,
> > update ethernet device node in dts to accommodate to it.
> > 
> 
> I have a hard time to understand how the changes in 1/6 and 2/6 are
> related to 
> that.
> 
> Please explain better what has changed. Also beware that we shouldn't
> introduce 
> any non-backward compatible DTS changes. That means, the device
> should work with 
> the new driver but an older device tree.
> 
> Regards,
> Matthias
> 

http://lists.infradead.org/pipermail/linux-mediatek/2021-December/032812.html
I've add more details to commit message in v10 send,
please kindly review it.

Regards!
Biao

> > Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> > ---
> >   arch/arm64/boot/dts/mediatek/mt2712-evb.dts |  1 +
> >   arch/arm64/boot/dts/mediatek/mt2712e.dtsi   | 14 +++++++++-----
> >   2 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> > b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> > index 7d369fdd3117..11aa135aa0f3 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
> > @@ -110,6 +110,7 @@ &eth {
> >   	phy-handle = <&ethernet_phy0>;
> >   	mediatek,tx-delay-ps = <1530>;
> >   	snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
> > +	snps,reset-delays-us = <0 10000 10000>;
> >   	pinctrl-names = "default", "sleep";
> >   	pinctrl-0 = <&eth_default>;
> >   	pinctrl-1 = <&eth_sleep>;
> > diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> > index a9cca9c146fd..9e850e04fffb 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
> > @@ -726,7 +726,7 @@ queue2 {
> >   	};
> >   
> >   	eth: ethernet@1101c000 {
> > -		compatible = "mediatek,mt2712-gmac";
> > +		compatible = "mediatek,mt2712-gmac", "snps,dwmac-
> > 4.20a";
> >   		reg = <0 0x1101c000 0 0x1300>;
> >   		interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
> >   		interrupt-names = "macirq";
> > @@ -734,15 +734,19 @@ eth: ethernet@1101c000 {
> >   		clock-names = "axi",
> >   			      "apb",
> >   			      "mac_main",
> > -			      "ptp_ref";
> > +			      "ptp_ref",
> > +			      "rmii_internal";
> >   		clocks = <&pericfg CLK_PERI_GMAC>,
> >   			 <&pericfg CLK_PERI_GMAC_PCLK>,
> >   			 <&topckgen CLK_TOP_ETHER_125M_SEL>,
> > -			 <&topckgen CLK_TOP_ETHER_50M_SEL>;
> > +			 <&topckgen CLK_TOP_ETHER_50M_SEL>,
> > +			 <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
> >   		assigned-clocks = <&topckgen CLK_TOP_ETHER_125M_SEL>,
> > -				  <&topckgen CLK_TOP_ETHER_50M_SEL>;
> > +				  <&topckgen CLK_TOP_ETHER_50M_SEL>,
> > +				  <&topckgen
> > CLK_TOP_ETHER_50M_RMII_SEL>;
> >   		assigned-clock-parents = <&topckgen
> > CLK_TOP_ETHERPLL_125M>,
> > -					 <&topckgen CLK_TOP_APLL1_D3>;
> > +					 <&topckgen CLK_TOP_APLL1_D3>,
> > +					 <&topckgen
> > CLK_TOP_ETHERPLL_50M>;
> >   		power-domains = <&scpsys MT2712_POWER_DOMAIN_AUDIO>;
> >   		mediatek,pericfg = <&pericfg>;
> >   		snps,axi-config = <&stmmac_axi_setup>;
> >
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
index 7d369fdd3117..11aa135aa0f3 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt2712-evb.dts
@@ -110,6 +110,7 @@  &eth {
 	phy-handle = <&ethernet_phy0>;
 	mediatek,tx-delay-ps = <1530>;
 	snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
+	snps,reset-delays-us = <0 10000 10000>;
 	pinctrl-names = "default", "sleep";
 	pinctrl-0 = <&eth_default>;
 	pinctrl-1 = <&eth_sleep>;
diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
index a9cca9c146fd..9e850e04fffb 100644
--- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi
@@ -726,7 +726,7 @@  queue2 {
 	};
 
 	eth: ethernet@1101c000 {
-		compatible = "mediatek,mt2712-gmac";
+		compatible = "mediatek,mt2712-gmac", "snps,dwmac-4.20a";
 		reg = <0 0x1101c000 0 0x1300>;
 		interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
 		interrupt-names = "macirq";
@@ -734,15 +734,19 @@  eth: ethernet@1101c000 {
 		clock-names = "axi",
 			      "apb",
 			      "mac_main",
-			      "ptp_ref";
+			      "ptp_ref",
+			      "rmii_internal";
 		clocks = <&pericfg CLK_PERI_GMAC>,
 			 <&pericfg CLK_PERI_GMAC_PCLK>,
 			 <&topckgen CLK_TOP_ETHER_125M_SEL>,
-			 <&topckgen CLK_TOP_ETHER_50M_SEL>;
+			 <&topckgen CLK_TOP_ETHER_50M_SEL>,
+			 <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
 		assigned-clocks = <&topckgen CLK_TOP_ETHER_125M_SEL>,
-				  <&topckgen CLK_TOP_ETHER_50M_SEL>;
+				  <&topckgen CLK_TOP_ETHER_50M_SEL>,
+				  <&topckgen CLK_TOP_ETHER_50M_RMII_SEL>;
 		assigned-clock-parents = <&topckgen CLK_TOP_ETHERPLL_125M>,
-					 <&topckgen CLK_TOP_APLL1_D3>;
+					 <&topckgen CLK_TOP_APLL1_D3>,
+					 <&topckgen CLK_TOP_ETHERPLL_50M>;
 		power-domains = <&scpsys MT2712_POWER_DOMAIN_AUDIO>;
 		mediatek,pericfg = <&pericfg>;
 		snps,axi-config = <&stmmac_axi_setup>;