Re: [PATCH v2 3/3] arm64: dts: imx8dxl: Add SolidRun SoM and HummingBoard

[Josua Mayer <josua@solid-run.com>]

Am 30.04.26 um 13:19 schrieb Vladimir Oltean:
> On Thu, Apr 30, 2026 at 11:17:36AM +0000, Josua Mayer wrote:
>> Am 09.04.26 um 14:46 schrieb Andrew Lunn:
>>>> +&eqos {
>>>> +	/* delays are added by connected ethernet-switch cpu port */
>>>> +	phy-mode = "rgmii";
>>>> +	pinctrl-0 = <&eqos_pins>;
>>>> +	pinctrl-names = "default";
>>>> +	status = "okay";
>>>> +
>>>> +	fixed-link {
>>>> +		full-duplex;
>>>> +		speed = <1000>;
>>>> +	};
>>>> +};
>>>> +	ethernet-switch@0 {
>>>> +		compatible = "nxp,sja1110a";
>>>> +		reg = <0>;
>>>> +		reset-gpios = <&lsio_gpio4 3 GPIO_ACTIVE_LOW>;
>>>> +		spi-max-frequency = <4000000>;
>>>> +
>>>> +		ethernet-ports {
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <0>;
>>>> +
>>>> +			/* 100Base-TX on connector J26 */
>>>> +			port@1 {
>>>> +				reg = <0x1>;
>>>> +				label = "lan1";
>>>> +				phy-handle = <&switch_port1_base_tx_phy>;
>>>> +				phy-mode = "internal";
>>>> +				status = "okay";
>> dropping unnecessary status okay for v3
>>>> +			};
>>>> +
>>>> +			/* CPU */
>>>> +			port@2 {
>>>> +				reg = <0x2>;
>>>> +				ethernet = <&eqos>;
>>>> +				label = "cpu";
> Please drop unused label = "cpu".
Okay.
> Also, can you use udev for user
> interface naming?
Possibly .... I am aware of systemd (udev) based default behaviour
for network interface aliases (eth0, eth1, ...).

However I have not seen it for dsa switch ports.

It has been common practice for switch ports to be named lan[0-9]+,
and I'd prefer to keep it that way here too.
Predictable names are important for users in particular when interfaces
are of different types and at different connectors.

Re. the T1 ports names I didn't know whether to use "lan" or "trx".
For those if "lan" is preferred, I can change them.

>
>>>> +				phy-mode = "rgmii-id";
>>>> +				rx-internal-delay-ps = <2000>;
>>>> +				tx-internal-delay-ps = <2000>;
>>>> +				status = "okay";
>> dropping unnecessary status okay for v3
>>>> +
>>>> +				fixed-link {
>>>> +					full-duplex;
>>>> +					speed = <1000>;
>>>> +				};
>>>> +			};
>>>> +
>>>> +			/* sgmii on addon board connector J21 */
>>>> +			port@3 {
>>>> +				reg = <0x3>;
>>>> +				label = "lan3";
>>>> +				status = "disabled";
>>>> +			};
>>>> +
>>>> +			/* sgmii on addon board connector J21 */
>>>> +			port@4 {
>>>> +				reg = <0x4>;
>>>> +				label = "lan4";
>>>> +				status = "disabled";
>>>> +			};
>>>> +
>>>> +			/* 100base-t1 on addon board connector J21 */
>>>> +			port@5 {
>>>> +				reg = <0x5>;
>>>> +				label = "trx1";
>>>> +				phy-handle = <&switch_port5_base_t1_phy>;
>>>> +				phy-mode = "internal";
>>>> +				status = "disabled";
>>>> +			};
>>>> +
>>>> +			/* 100base-t1 on addon board connector J21 */
>>>> +			port@6 {
>>>> +				reg = <0x6>;
>>>> +				label = "trx2";
>>>> +				phy-handle = <&switch_port6_base_t1_phy>;
>>>> +				phy-mode = "internal";
>>>> +				status = "disabled";
>>>> +			};
>>>> +
>>>> +			/* 100base-t1 on addon board connector J21 */
>>>> +			port@7 {
>>>> +				reg = <0x7>;
>>>> +				label = "trx3";
>>>> +				phy-handle = <&switch_port7_base_t1_phy>;
>>>> +				phy-mode = "internal";
>>>> +				status = "disabled";
>>>> +			};
>>>> +
>>>> +			/* 100base-t1 on addon board connector J21 */
>>>> +			port@8 {
>>>> +				reg = <0x8>;
>>>> +				label = "trx4";
>>>> +				phy-handle = <&switch_port8_base_t1_phy>;
>>>> +				phy-mode = "internal";
>>>> +				status = "disabled";
>>>> +			};
>>>> +
>>>> +			/* 100base-t1 on addon board connector J21 */
>>>> +			port@9 {
>>>> +				reg = <0x9>;
>>>> +				label = "trx5";
>>>> +				phy-handle = <&switch_port9_base_t1_phy>;
>>>> +				phy-mode = "internal";
>>>> +				status = "disabled";
>>>> +			};
>>>> +
>>>> +			/* 100Base-T1 on connector J26 */
>>>> +			port@a {
>>>> +				reg = <0xa>;
>>>> +				label = "trx6";
>>>> +				phy-handle = <&switch_port10_base_t1_phy>;
>>>> +				phy-mode = "internal";
>>>> +				status = "okay";
>> dropping unnecessary status okay for v3
>>>> +			};
>>>> +		};
>>>> +
>>>> +		mdios {
>>>> +			#address-cells = <1>;
>>>> +			#size-cells = <0>;
>>>> +
>>>> +			mdio@0 {
>>>> +				compatible = "nxp,sja1110-base-t1-mdio";
>>>> +				reg = <0>;
>>>> +				#address-cells = <1>;
>>>> +				#size-cells = <0>;
>>>> +
>>>> +				/* 100base-t1 on addon board connector J21 */
>>>> +				switch_port5_base_t1_phy: ethernet-phy@1 {
>>>> +					compatible = "ethernet-phy-ieee802.3-c45";
>>>> +					reg = <0x1>;
>>>> +					status = "disabled";
>>>> +				};
>>>> +
>>>> +				/* 100base-t1 on addon board connector J21 */
>>>> +				switch_port6_base_t1_phy: ethernet-phy@2 {
>>>> +					compatible = "ethernet-phy-ieee802.3-c45";
>>>> +					reg = <0x2>;
>>>> +					status = "disabled";
>>>> +				};
>>>> +
>>>> +				/* 100base-t1 on addon board connector J21 */
>>>> +				switch_port7_base_t1_phy: ethernet-phy@3 {
>>>> +					compatible = "ethernet-phy-ieee802.3-c45";
>>>> +					reg = <0x3>;
>>>> +					status = "disabled";
>>>> +				};
>>>> +
>>>> +				/* 100base-t1 on addon board connector J21 */
>>>> +				switch_port8_base_t1_phy: ethernet-phy@4 {
>>>> +					compatible = "ethernet-phy-ieee802.3-c45";
>>>> +					reg = <0x4>;
>>>> +					status = "disabled";
>>>> +				};
>>>> +
>>>> +				/* 100base-t1 on addon board connector J21 */
>>>> +				switch_port9_base_t1_phy: ethernet-phy@5 {
>>>> +					compatible = "ethernet-phy-ieee802.3-c45";
>>>> +					reg = <0x5>;
>>>> +					status = "disabled";
>>>> +				};
>>>> +
>>>> +				/* 100Base-T1 on connector J26 */
>>>> +				switch_port10_base_t1_phy: ethernet-phy@6 {
>>>> +					compatible = "ethernet-phy-ieee802.3-c45";
>>>> +					reg = <0x6>;
>>>> +				};
>>>> +			};
>>>> +
>>>> +			mdio@1 {
>>>> +				compatible = "nxp,sja1110-base-tx-mdio";
>>>> +				reg = <1>;
>>>> +				#address-cells = <1>;
>>>> +				#size-cells = <0>;
>>>> +
>>>> +				/* 100Base-TX on connector J26 */
>>>> +				switch_port1_base_tx_phy: ethernet-phy@1 {
>>>> +					reg = <0x1>;
>>>> +				};
>>>> +			};
>>> For these nodes only:
>>>
>>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>>>
>>>     Andrew
>> Thanks!
>>
>> I don't know how to keep this partial review for v3, so I will send it without.