2022-10-17 10:09:11

by Biao Huang (黄彪)

Subject: [PATCH] arm64: dts: mt8195: Add Ethernet controller

Add Ethernet controller node for mt8195.

Signed-off-by: Biao Huang <[email protected]>
---
arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 88 ++++++++++++++++++++
arch/arm64/boot/dts/mediatek/mt8195.dtsi | 87 +++++++++++++++++++
2 files changed, 175 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
index 4fbd99eb496a..02e04f82a4ae 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
@@ -258,6 +258,72 @@ &mt6359_vsram_others_ldo_reg {
};

&pio {
+ eth_default: eth_default {
+ txd_pins {
+ pinmux = <PINMUX_GPIO77__FUNC_GBE_TXD3>,
+ <PINMUX_GPIO78__FUNC_GBE_TXD2>,
+ <PINMUX_GPIO79__FUNC_GBE_TXD1>,
+ <PINMUX_GPIO80__FUNC_GBE_TXD0>;
+ drive-strength = <MTK_DRIVE_8mA>;
+ };
+ cc_pins {
+ pinmux = <PINMUX_GPIO85__FUNC_GBE_TXC>,
+ <PINMUX_GPIO88__FUNC_GBE_TXEN>,
+ <PINMUX_GPIO87__FUNC_GBE_RXDV>,
+ <PINMUX_GPIO86__FUNC_GBE_RXC>;
+ drive-strength = <MTK_DRIVE_8mA>;
+ };
+ rxd_pins {
+ pinmux = <PINMUX_GPIO81__FUNC_GBE_RXD3>,
+ <PINMUX_GPIO82__FUNC_GBE_RXD2>,
+ <PINMUX_GPIO83__FUNC_GBE_RXD1>,
+ <PINMUX_GPIO84__FUNC_GBE_RXD0>;
+ };
+ mdio_pins {
+ pinmux = <PINMUX_GPIO89__FUNC_GBE_MDC>,
+ <PINMUX_GPIO90__FUNC_GBE_MDIO>;
+ input-enable;
+ };
+ power_pins {
+ pinmux = <PINMUX_GPIO91__FUNC_GPIO91>,
+ <PINMUX_GPIO92__FUNC_GPIO92>;
+ output-high;
+ };
+ };
+
+ eth_sleep: eth_sleep {
+ txd_pins {
+ pinmux = <PINMUX_GPIO77__FUNC_GPIO77>,
+ <PINMUX_GPIO78__FUNC_GPIO78>,
+ <PINMUX_GPIO79__FUNC_GPIO79>,
+ <PINMUX_GPIO80__FUNC_GPIO80>;
+ };
+ cc_pins {
+ pinmux = <PINMUX_GPIO85__FUNC_GPIO85>,
+ <PINMUX_GPIO88__FUNC_GPIO88>,
+ <PINMUX_GPIO87__FUNC_GPIO87>,
+ <PINMUX_GPIO86__FUNC_GPIO86>;
+ };
+ rxd_pins {
+ pinmux = <PINMUX_GPIO81__FUNC_GPIO81>,
+ <PINMUX_GPIO82__FUNC_GPIO82>,
+ <PINMUX_GPIO83__FUNC_GPIO83>,
+ <PINMUX_GPIO84__FUNC_GPIO84>;
+ };
+ mdio_pins {
+ pinmux = <PINMUX_GPIO89__FUNC_GPIO89>,
+ <PINMUX_GPIO90__FUNC_GPIO90>;
+ input-disable;
+ bias-disable;
+ };
+ power_pins {
+ pinmux = <PINMUX_GPIO91__FUNC_GPIO91>,
+ <PINMUX_GPIO92__FUNC_GPIO92>;
+ input-disable;
+ bias-disable;
+ };
+ };
+
gpio_keys_pins: gpio-keys-pins {
pins {
pinmux = <PINMUX_GPIO106__FUNC_GPIO106>;
@@ -434,6 +500,28 @@ &xhci0 {
status = "okay";
};

+&eth {
+ phy-mode ="rgmii-rxid";
+ phy-handle = <&eth_phy0>;
+ snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>;
+ snps,reset-delays-us = <0 10000 10000>;
+ mediatek,tx-delay-ps = <2030>;
+ pinctrl-names = "default", "sleep";
+ pinctrl-0 = <&eth_default>;
+ pinctrl-1 = <&eth_sleep>;
+ status = "okay";
+
+ mdio {
+ compatible = "snps,dwmac-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ eth_phy0: eth_phy0@1 {
+ compatible = "ethernet-phy-id001c.c916";
+ reg = <0x1>;
+ };
+ };
+};
+
&xhci1 {
vusb33-supply = <&mt6359_vusb_ldo_reg>;
status = "okay";
diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
index 905d1a90b406..aa1fcc3b9cb6 100644
--- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
@@ -1042,6 +1042,93 @@ spis1: spi@1101e000 {
status = "disabled";
};

+ stmmac_axi_setup: stmmac-axi-config {
+ snps,wr_osr_lmt = <0x7>;
+ snps,rd_osr_lmt = <0x7>;
+ snps,blen = <0 0 0 0 16 8 4>;
+ };
+
+ mtl_rx_setup: rx-queues-config {
+ snps,rx-queues-to-use = <4>;
+ snps,rx-sched-sp;
+ queue0 {
+ snps,dcb-algorithm;
+ snps,map-to-dma-channel = <0x0>;
+ };
+ queue1 {
+ snps,dcb-algorithm;
+ snps,map-to-dma-channel = <0x0>;
+ };
+ queue2 {
+ snps,dcb-algorithm;
+ snps,map-to-dma-channel = <0x0>;
+ };
+ queue3 {
+ snps,dcb-algorithm;
+ snps,map-to-dma-channel = <0x0>;
+ };
+ };
+
+ mtl_tx_setup: tx-queues-config {
+ snps,tx-queues-to-use = <4>;
+ snps,tx-sched-wrr;
+ queue0 {
+ snps,weight = <0x10>;
+ snps,dcb-algorithm;
+ snps,priority = <0x0>;
+ };
+ queue1 {
+ snps,weight = <0x11>;
+ snps,dcb-algorithm;
+ snps,priority = <0x1>;
+ };
+ queue2 {
+ snps,weight = <0x12>;
+ snps,dcb-algorithm;
+ snps,priority = <0x2>;
+ };
+ queue3 {
+ snps,weight = <0x13>;
+ snps,dcb-algorithm;
+ snps,priority = <0x3>;
+ };
+ };
+
+ eth: ethernet@11021000 {
+ compatible = "mediatek,mt8195-gmac", "snps,dwmac-5.10a";
+ reg = <0 0x11021000 0 0x4000>;
+ interrupts = <GIC_SPI 716 IRQ_TYPE_LEVEL_HIGH 0>;
+ interrupt-names = "macirq";
+ mac-address = [00 55 7b b5 7d f7];
+ clock-names = "axi",
+ "apb",
+ "mac_cg",
+ "mac_main",
+ "ptp_ref",
+ "rmii_internal";
+ clocks = <&pericfg_ao CLK_PERI_AO_ETHERNET>,
+ <&pericfg_ao CLK_PERI_AO_ETHERNET_BUS>,
+ <&pericfg_ao CLK_PERI_AO_ETHERNET_MAC>,
+ <&topckgen CLK_TOP_SNPS_ETH_250M>,
+ <&topckgen CLK_TOP_SNPS_ETH_62P4M_PTP>,
+ <&topckgen CLK_TOP_SNPS_ETH_50M_RMII>;
+ assigned-clocks = <&topckgen CLK_TOP_SNPS_ETH_250M>,
+ <&topckgen CLK_TOP_SNPS_ETH_62P4M_PTP>,
+ <&topckgen CLK_TOP_SNPS_ETH_50M_RMII>;
+ assigned-clock-parents = <&topckgen CLK_TOP_ETHPLL_D2>,
+ <&topckgen CLK_TOP_ETHPLL_D8>,
+ <&topckgen CLK_TOP_ETHPLL_D10>;
+ power-domains = <&spm MT8195_POWER_DOMAIN_ETHER>;
+ mediatek,pericfg = <&infracfg_ao>;
+ snps,axi-config = <&stmmac_axi_setup>;
+ snps,mtl-rx-config = <&mtl_rx_setup>;
+ snps,mtl-tx-config = <&mtl_tx_setup>;
+ snps,txpbl = <16>;
+ snps,rxpbl = <16>;
+ snps,clk-csr = <0>;
+ status = "disabled";
+ };
+
xhci0: usb@11200000 {
compatible = "mediatek,mt8195-xhci",
"mediatek,mtk-xhci";
--
2.25.1

2022-10-18 02:52:06

by Krzysztof Kozlowski

Subject: Re: [PATCH] arm64: dts: mt8195: Add Ethernet controller

On 17/10/2022 05:58, Biao Huang wrote:
> Add Ethernet controller node for mt8195.
>
> Signed-off-by: Biao Huang <[email protected]>
> ---
> arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 88 ++++++++++++++++++++
> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 87 +++++++++++++++++++
> 2 files changed, 175 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> index 4fbd99eb496a..02e04f82a4ae 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> @@ -258,6 +258,72 @@ &mt6359_vsram_others_ldo_reg {
> };
>
> &pio {
> + eth_default: eth_default {

No underscores in node names. Please also be sure your patch does not
bring new warnings with `dtbs_check` (lack of suffix above could mean it
brings...)

> + txd_pins {

No underscores

> + pinmux = <PINMUX_GPIO77__FUNC_GBE_TXD3>,
> + <PINMUX_GPIO78__FUNC_GBE_TXD2>,
> + <PINMUX_GPIO79__FUNC_GBE_TXD1>,
> + <PINMUX_GPIO80__FUNC_GBE_TXD0>;
> + drive-strength = <MTK_DRIVE_8mA>;
> + };
> + cc_pins {

Ditto... and so on.

> + pinmux = <PINMUX_GPIO85__FUNC_GBE_TXC>,
> + <PINMUX_GPIO88__FUNC_GBE_TXEN>,
> + <PINMUX_GPIO87__FUNC_GBE_RXDV>,
> + <PINMUX_GPIO86__FUNC_GBE_RXC>;
> + drive-strength = <MTK_DRIVE_8mA>;
> + };
> + rxd_pins {
> + pinmux = <PINMUX_GPIO81__FUNC_GBE_RXD3>,
> + <PINMUX_GPIO82__FUNC_GBE_RXD2>,
> + <PINMUX_GPIO83__FUNC_GBE_RXD1>,
> + <PINMUX_GPIO84__FUNC_GBE_RXD0>;
> + };
> + mdio_pins {
> + pinmux = <PINMUX_GPIO89__FUNC_GBE_MDC>,
> + <PINMUX_GPIO90__FUNC_GBE_MDIO>;
> + input-enable;
> + };
> + power_pins {
> + pinmux = <PINMUX_GPIO91__FUNC_GPIO91>,
> + <PINMUX_GPIO92__FUNC_GPIO92>;
> + output-high;
> + };
> + };
> +
> + eth_sleep: eth_sleep {
> + txd_pins {
> + pinmux = <PINMUX_GPIO77__FUNC_GPIO77>,
> + <PINMUX_GPIO78__FUNC_GPIO78>,
> + <PINMUX_GPIO79__FUNC_GPIO79>,
> + <PINMUX_GPIO80__FUNC_GPIO80>;
> + };
> + cc_pins {
> + pinmux = <PINMUX_GPIO85__FUNC_GPIO85>,
> + <PINMUX_GPIO88__FUNC_GPIO88>,
> + <PINMUX_GPIO87__FUNC_GPIO87>,
> + <PINMUX_GPIO86__FUNC_GPIO86>;
> + };
> + rxd_pins {
> + pinmux = <PINMUX_GPIO81__FUNC_GPIO81>,
> + <PINMUX_GPIO82__FUNC_GPIO82>,
> + <PINMUX_GPIO83__FUNC_GPIO83>,
> + <PINMUX_GPIO84__FUNC_GPIO84>;
> + };
> + mdio_pins {
> + pinmux = <PINMUX_GPIO89__FUNC_GPIO89>,
> + <PINMUX_GPIO90__FUNC_GPIO90>;
> + input-disable;
> + bias-disable;
> + };
> + power_pins {
> + pinmux = <PINMUX_GPIO91__FUNC_GPIO91>,
> + <PINMUX_GPIO92__FUNC_GPIO92>;
> + input-disable;
> + bias-disable;
> + };
> + };
> +
> gpio_keys_pins: gpio-keys-pins {
> pins {
> pinmux = <PINMUX_GPIO106__FUNC_GPIO106>;
> @@ -434,6 +500,28 @@ &xhci0 {
> status = "okay";
> };
>
> +&eth {
> + phy-mode ="rgmii-rxid";
> + phy-handle = <&eth_phy0>;
> + snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>;
> + snps,reset-delays-us = <0 10000 10000>;
> + mediatek,tx-delay-ps = <2030>;
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&eth_default>;
> + pinctrl-1 = <&eth_sleep>;
> + status = "okay";
> +
> + mdio {
> + compatible = "snps,dwmac-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + eth_phy0: eth_phy0@1 {

ethernet-phy@1

> + compatible = "ethernet-phy-id001c.c916";
> + reg = <0x1>;
> + };
> + };
> +};
> +
> &xhci1 {
> vusb33-supply = <&mt6359_vusb_ldo_reg>;
> status = "okay";
> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> index 905d1a90b406..aa1fcc3b9cb6 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> @@ -1042,6 +1042,93 @@ spis1: spi@1101e000 {
> status = "disabled";
> };
>
> + stmmac_axi_setup: stmmac-axi-config {
> + snps,wr_osr_lmt = <0x7>;
> + snps,rd_osr_lmt = <0x7>;
> + snps,blen = <0 0 0 0 16 8 4>;
> + };
> +
> + mtl_rx_setup: rx-queues-config {
> + snps,rx-queues-to-use = <4>;
> + snps,rx-sched-sp;
> + queue0 {
> + snps,dcb-algorithm;
> + snps,map-to-dma-channel = <0x0>;
> + };
> + queue1 {
> + snps,dcb-algorithm;
> + snps,map-to-dma-channel = <0x0>;
> + };
> + queue2 {
> + snps,dcb-algorithm;
> + snps,map-to-dma-channel = <0x0>;
> + };
> + queue3 {
> + snps,dcb-algorithm;
> + snps,map-to-dma-channel = <0x0>;
> + };
> + };
> +
> + mtl_tx_setup: tx-queues-config {
> + snps,tx-queues-to-use = <4>;
> + snps,tx-sched-wrr;
> + queue0 {
> + snps,weight = <0x10>;
> + snps,dcb-algorithm;
> + snps,priority = <0x0>;
> + };
> + queue1 {
> + snps,weight = <0x11>;
> + snps,dcb-algorithm;
> + snps,priority = <0x1>;
> + };
> + queue2 {
> + snps,weight = <0x12>;
> + snps,dcb-algorithm;
> + snps,priority = <0x2>;
> + };
> + queue3 {
> + snps,weight = <0x13>;
> + snps,dcb-algorithm;
> + snps,priority = <0x3>;
> + };
> + };
> +
> + eth: ethernet@11021000 {
> + compatible = "mediatek,mt8195-gmac", "snps,dwmac-5.10a";
> + reg = <0 0x11021000 0 0x4000>;
> + interrupts = <GIC_SPI 716 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-names = "macirq";
> + mac-address = [00 55 7b b5 7d f7];

How is this property of a SoC? Are you saying now that all MT8195 SoCs
have the same MAC address?

Best regards,
Krzysztof

2022-10-18 06:47:25

by Biao Huang (黄彪)

Subject: Re: [PATCH] arm64: dts: mt8195: Add Ethernet controller

Dear Krzysztof,
Thanks for your comments!

On Mon, 2022-10-17 at 22:01 -0400, Krzysztof Kozlowski wrote:
> On 17/10/2022 05:58, Biao Huang wrote:
> > Add Ethernet controller node for mt8195.
> >
> > Signed-off-by: Biao Huang <[email protected]>
> > ---
> > arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 88
> > ++++++++++++++++++++
> > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 87
> > +++++++++++++++++++
> > 2 files changed, 175 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > index 4fbd99eb496a..02e04f82a4ae 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > @@ -258,6 +258,72 @@ &mt6359_vsram_others_ldo_reg {
> > };
> >
> > &pio {
> > + eth_default: eth_default {
>
> No underscores in node names. Please also be sure your patch does not
> bring new warnings with `dtbs_check` (lack of suffix above could mean
> it
> brings...)
OK, I'll fix the underscores issue in next send.
As to "lack of suffix" issue, do you mean I should write it like:
eth-default: eth-default@0 {
...
}
If yes, other nodes in current file don't have such suffix.
e.g.
gpio_keys_pins: gpio-keys-pins

Should I keep unified style with other nodes?
>
> > + txd_pins {
>
> No underscores
OK, will fix in next send.
>
> > + pinmux = <PINMUX_GPIO77__FUNC_GBE_TXD3>,
> > + <PINMUX_GPIO78__FUNC_GBE_TXD2>,
> > + <PINMUX_GPIO79__FUNC_GBE_TXD1>,
> > + <PINMUX_GPIO80__FUNC_GBE_TXD0>;
> > + drive-strength = <MTK_DRIVE_8mA>;
> > + };
> > + cc_pins {
>
> Ditto... and so on.
OK, will fix in next send.
>
> > + pinmux = <PINMUX_GPIO85__FUNC_GBE_TXC>,
> > + <PINMUX_GPIO88__FUNC_GBE_TXEN>,
> > + <PINMUX_GPIO87__FUNC_GBE_RXDV>,
> > + <PINMUX_GPIO86__FUNC_GBE_RXC>;
> > + drive-strength = <MTK_DRIVE_8mA>;
> > + };
> > + rxd_pins {
> > + pinmux = <PINMUX_GPIO81__FUNC_GBE_RXD3>,
> > + <PINMUX_GPIO82__FUNC_GBE_RXD2>,
> > + <PINMUX_GPIO83__FUNC_GBE_RXD1>,
> > + <PINMUX_GPIO84__FUNC_GBE_RXD0>;
> > + };
> > + mdio_pins {
> > + pinmux = <PINMUX_GPIO89__FUNC_GBE_MDC>,
> > + <PINMUX_GPIO90__FUNC_GBE_MDIO>;
> > + input-enable;
> > + };
> > + power_pins {
> > + pinmux = <PINMUX_GPIO91__FUNC_GPIO91>,
> > + <PINMUX_GPIO92__FUNC_GPIO92>;
> > + output-high;
> > + };
> > + };
> > +
> > + eth_sleep: eth_sleep {
> > + txd_pins {
> > + pinmux = <PINMUX_GPIO77__FUNC_GPIO77>,
> > + <PINMUX_GPIO78__FUNC_GPIO78>,
> > + <PINMUX_GPIO79__FUNC_GPIO79>,
> > + <PINMUX_GPIO80__FUNC_GPIO80>;
> > + };
> > + cc_pins {
> > + pinmux = <PINMUX_GPIO85__FUNC_GPIO85>,
> > + <PINMUX_GPIO88__FUNC_GPIO88>,
> > + <PINMUX_GPIO87__FUNC_GPIO87>,
> > + <PINMUX_GPIO86__FUNC_GPIO86>;
> > + };
> > + rxd_pins {
> > + pinmux = <PINMUX_GPIO81__FUNC_GPIO81>,
> > + <PINMUX_GPIO82__FUNC_GPIO82>,
> > + <PINMUX_GPIO83__FUNC_GPIO83>,
> > + <PINMUX_GPIO84__FUNC_GPIO84>;
> > + };
> > + mdio_pins {
> > + pinmux = <PINMUX_GPIO89__FUNC_GPIO89>,
> > + <PINMUX_GPIO90__FUNC_GPIO90>;
> > + input-disable;
> > + bias-disable;
> > + };
> > + power_pins {
> > + pinmux = <PINMUX_GPIO91__FUNC_GPIO91>,
> > + <PINMUX_GPIO92__FUNC_GPIO92>;
> > + input-disable;
> > + bias-disable;
> > + };
> > + };
> > +
> > gpio_keys_pins: gpio-keys-pins {
> > pins {
> > pinmux = <PINMUX_GPIO106__FUNC_GPIO106>;
> > @@ -434,6 +500,28 @@ &xhci0 {
> > status = "okay";
> > };
> >
> > +&eth {
> > + phy-mode ="rgmii-rxid";
> > + phy-handle = <&eth_phy0>;
> > + snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>;
> > + snps,reset-delays-us = <0 10000 10000>;
> > + mediatek,tx-delay-ps = <2030>;
> > + pinctrl-names = "default", "sleep";
> > + pinctrl-0 = <&eth_default>;
> > + pinctrl-1 = <&eth_sleep>;
> > + status = "okay";
> > +
> > + mdio {
> > + compatible = "snps,dwmac-mdio";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + eth_phy0: eth_phy0@1 {
>
> ethernet-phy@1
OK, will modify in next send.
>
> > + compatible = "ethernet-phy-id001c.c916";
> > + reg = <0x1>;
> > + };
> > + };
> > +};
> > +
> > &xhci1 {
> > vusb33-supply = <&mt6359_vusb_ldo_reg>;
> > status = "okay";
> > diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > index 905d1a90b406..aa1fcc3b9cb6 100644
> > --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi
> > @@ -1042,6 +1042,93 @@ spis1: spi@1101e000 {
> > status = "disabled";
> > };
> >
> > + stmmac_axi_setup: stmmac-axi-config {
> > + snps,wr_osr_lmt = <0x7>;
> > + snps,rd_osr_lmt = <0x7>;
> > + snps,blen = <0 0 0 0 16 8 4>;
> > + };
> > +
> > + mtl_rx_setup: rx-queues-config {
> > + snps,rx-queues-to-use = <4>;
> > + snps,rx-sched-sp;
> > + queue0 {
> > + snps,dcb-algorithm;
> > + snps,map-to-dma-channel = <0x0>;
> > + };
> > + queue1 {
> > + snps,dcb-algorithm;
> > + snps,map-to-dma-channel = <0x0>;
> > + };
> > + queue2 {
> > + snps,dcb-algorithm;
> > + snps,map-to-dma-channel = <0x0>;
> > + };
> > + queue3 {
> > + snps,dcb-algorithm;
> > + snps,map-to-dma-channel = <0x0>;
> > + };
> > + };
> > +
> > + mtl_tx_setup: tx-queues-config {
> > + snps,tx-queues-to-use = <4>;
> > + snps,tx-sched-wrr;
> > + queue0 {
> > + snps,weight = <0x10>;
> > + snps,dcb-algorithm;
> > + snps,priority = <0x0>;
> > + };
> > + queue1 {
> > + snps,weight = <0x11>;
> > + snps,dcb-algorithm;
> > + snps,priority = <0x1>;
> > + };
> > + queue2 {
> > + snps,weight = <0x12>;
> > + snps,dcb-algorithm;
> > + snps,priority = <0x2>;
> > + };
> > + queue3 {
> > + snps,weight = <0x13>;
> > + snps,dcb-algorithm;
> > + snps,priority = <0x3>;
> > + };
> > + };
> > +
> > + eth: ethernet@11021000 {
> > + compatible = "mediatek,mt8195-gmac",
> > "snps,dwmac-5.10a";
> > + reg = <0 0x11021000 0 0x4000>;
> > + interrupts = <GIC_SPI 716 IRQ_TYPE_LEVEL_HIGH
> > 0>;
> > + interrupt-names = "macirq";
> > + mac-address = [00 55 7b b5 7d f7];
>
> How is this property of a SoC? Are you saying now that all MT8195
> SoCs
> have the same MAC address?
The mac-address here is taken as a default mac address in eth driver
rather than a randome one.
Actually, there will be a tool to customize eth mac address (e.g
through "ifconfig eth0 hw ether xx:xx:xx:xx:xx:xx"), so every
MT8195 SoCs will get their specified mac address in real product.
>
> Best regards,
> Krzysztof
>
Best Regards!
Biao

2022-10-18 13:26:36

by Krzysztof Kozlowski

Subject: Re: [PATCH] arm64: dts: mt8195: Add Ethernet controller

On 18/10/2022 02:37, Biao Huang wrote:
> Dear Krzysztof,
> Thanks for your comments!
>
> On Mon, 2022-10-17 at 22:01 -0400, Krzysztof Kozlowski wrote:
>> On 17/10/2022 05:58, Biao Huang wrote:
>>> Add Ethernet controller node for mt8195.
>>>
>>> Signed-off-by: Biao Huang <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 88
>>> ++++++++++++++++++++
>>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 87
>>> +++++++++++++++++++
>>> 2 files changed, 175 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> index 4fbd99eb496a..02e04f82a4ae 100644
>>> --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
>>> @@ -258,6 +258,72 @@ &mt6359_vsram_others_ldo_reg {
>>> };
>>>
>>> &pio {
>>> + eth_default: eth_default {
>>
>> No underscores in node names. Please also be sure your patch does not
>> bring new warnings with `dtbs_check` (lack of suffix above could mean
>> it
>> brings...)
> OK, I'll fix the underscores issue in next send.
> As to "lack of suffix" issue, do you mean I should write it like:
> eth-default: eth-default@0 {

I don't know whether you should have here suffix or not - please check
your bindings. Several pinctrl bindings require suffixes (or prefixes),
thus I asked.

BTW, In the label you must use underscore.

> ...
> }
> If yes, other nodes in current file don't have such suffix.
> e.g.
> gpio_keys_pins: gpio-keys-pins
>
> Should I keep unified style with other nodes?

Check what bindings are requiring.

>>
>>> + txd_pins {
>>

(...)

>>> +
>>> + eth: ethernet@11021000 {
>>> + compatible = "mediatek,mt8195-gmac",
>>> "snps,dwmac-5.10a";
>>> + reg = <0 0x11021000 0 0x4000>;
>>> + interrupts = <GIC_SPI 716 IRQ_TYPE_LEVEL_HIGH
>>> 0>;
>>> + interrupt-names = "macirq";
>>> + mac-address = [00 55 7b b5 7d f7];
>>
>> How is this property of a SoC? Are you saying now that all MT8195
>> SoCs
>> have the same MAC address?
> The mac-address here is taken as a default mac address in eth driver
> rather than a randome one.
> Actually, there will be a tool to customize eth mac address (e.g
> through "ifconfig eth0 hw ether xx:xx:xx:xx:xx:xx"), so every
> MT8195 SoCs will get their specified mac address in real product.

So this means this is not one MAC address for all SoCs, so this does not
belong to DTSI. Actually it doesn't belong to DTS either. Look how
others are doing...


Best regards,
Krzysztof

2022-10-19 09:31:51

by Biao Huang (黄彪)

Subject: Re: [PATCH] arm64: dts: mt8195: Add Ethernet controller

Dear Krzysztof,
Thanks for your comments~

On Tue, 2022-10-18 at 08:51 -0400, Krzysztof Kozlowski wrote:
> On 18/10/2022 02:37, Biao Huang wrote:
> > Dear Krzysztof,
> > Thanks for your comments!
> >
> > On Mon, 2022-10-17 at 22:01 -0400, Krzysztof Kozlowski wrote:
> > > On 17/10/2022 05:58, Biao Huang wrote:
> > > > Add Ethernet controller node for mt8195.
> > > >
> > > > Signed-off-by: Biao Huang <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/mediatek/mt8195-demo.dts | 88
> > > > ++++++++++++++++++++
> > > > arch/arm64/boot/dts/mediatek/mt8195.dtsi | 87
> > > > +++++++++++++++++++
> > > > 2 files changed, 175 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > > > b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > > > index 4fbd99eb496a..02e04f82a4ae 100644
> > > > --- a/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > > > +++ b/arch/arm64/boot/dts/mediatek/mt8195-demo.dts
> > > > @@ -258,6 +258,72 @@ &mt6359_vsram_others_ldo_reg {
> > > > };
> > > >
> > > > &pio {
> > > > + eth_default: eth_default {
> > >
> > > No underscores in node names. Please also be sure your patch does
> > > not
> > > bring new warnings with `dtbs_check` (lack of suffix above could
> > > mean
> > > it
> > > brings...)
> >
> > OK, I'll fix the underscores issue in next send.
> > As to "lack of suffix" issue, do you mean I should write it like:
> > eth-default: eth-default@0 {
>
> I don't know whether you should have here suffix or not - please
> check
> your bindings. Several pinctrl bindings require suffixes (or
> prefixes),
> thus I asked.
>
> BTW, In the label you must use underscore.
OK, I'll check the pinctrl-mt8195.yaml, and modify the eth related
node.
>
> > ...
> > }
> > If yes, other nodes in current file don't have such suffix.
> > e.g.
> > gpio_keys_pins: gpio-keys-pins
> >
> > Should I keep unified style with other nodes?
>
> Check what bindings are requiring.
OK.
>
> > >
> > > > + txd_pins {
>
> (...)
>
> > > > +
> > > > + eth: ethernet@11021000 {
> > > > + compatible = "mediatek,mt8195-gmac",
> > > > "snps,dwmac-5.10a";
> > > > + reg = <0 0x11021000 0 0x4000>;
> > > > + interrupts = <GIC_SPI 716
> > > > IRQ_TYPE_LEVEL_HIGH
> > > > 0>;
> > > > + interrupt-names = "macirq";
> > > > + mac-address = [00 55 7b b5 7d f7];
> > >
> > > How is this property of a SoC? Are you saying now that all MT8195
> > > SoCs
> > > have the same MAC address?
> >
> > The mac-address here is taken as a default mac address in eth
> > driver
> > rather than a randome one.
> > Actually, there will be a tool to customize eth mac address (e.g
> > through "ifconfig eth0 hw ether xx:xx:xx:xx:xx:xx"), so every
> > MT8195 SoCs will get their specified mac address in real product.
>
> So this means this is not one MAC address for all SoCs, so this does
> not
> belong to DTSI. Actually it doesn't belong to DTS either. Look how
> others are doing...
OK, will remove it in next send.
>
>
> Best regards,
> Krzysztof
>