netdev - Re: [PATCH net-next v8 5/9] ARM64: dts: marvell: Fix some common switch mistakes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231129181842.74zihcd642lx5zgo@skbuf>
Date: Wed, 29 Nov 2023 20:18:42 +0200
From: Vladimir Oltean <olteanv@...il.com>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Andrew Lunn <andrew@...n.ch>,
	Gregory Clement <gregory.clement@...tlin.com>,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
	Rob Herring <robh+dt@...nel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Russell King <linux@...linux.org.uk>,
	Florian Fainelli <f.fainelli@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Marek Behún <kabel@...nel.org>,
	Christian Marangi <ansuelsmth@...il.com>,
	linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	Florian Fainelli <florian.fainelli@...adcom.com>
Subject: Re: [PATCH net-next v8 5/9] ARM64: dts: marvell: Fix some common
 switch mistakes

On Tue, Nov 14, 2023 at 12:36:00AM +0100, Linus Walleij wrote:
> diff --git a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> index 9eab2bb22134..66cd98b67744 100644
> --- a/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> +++ b/arch/arm64/boot/dts/marvell/armada-3720-turris-mox.dts
> @@ -304,7 +304,13 @@ phy1: ethernet-phy@1 {
>  		reg = <1>;
>  	};
>  
> -	/* switch nodes are enabled by U-Boot if modules are present */
> +	/*
> +	 * NOTE: switch nodes are enabled by U-Boot if modules are present
> +	 * DO NOT change this node name (switch0@10) even if it is not following
> +	 * conventions! Deployed U-Boot binaries are explicitly looking for
> +	 * this node in order to augment the device tree!
> +	 * Also do not touch the "ports" or "port@n" nodes. These are also ABI.
> +	 */
>  	switch0@10 {
>  		compatible = "marvell,mv88e6190";
>  		reg = <0x10>;
> @@ -430,6 +436,7 @@ port-sfp@a {
>  		};
>  	};
>  
> +	/* NOTE: this node name is ABI, don't change it! */
>  	switch0@2 {
>  		compatible = "marvell,mv88e6085";
>  		reg = <0x2>;
> @@ -497,6 +504,7 @@ port@5 {
>  		};
>  	};
>  
> +	/* NOTE: this node name is ABI, don't change it! */
>  	switch1@11 {
>  		compatible = "marvell,mv88e6190";
>  		reg = <0x11>;
> @@ -622,6 +630,7 @@ port-sfp@a {
>  		};
>  	};
>  
> +	/* NOTE: this node name is ABI, don't change it! */
>  	switch1@2 {
>  		compatible = "marvell,mv88e6085";
>  		reg = <0x2>;
> @@ -689,6 +698,7 @@ port@5 {
>  		};
>  	};
>  
> +	/* NOTE: this node name is ABI, don't change it! */
>  	switch2@12 {
>  		compatible = "marvell,mv88e6190";
>  		reg = <0x12>;
> @@ -805,6 +815,7 @@ port-sfp@a {
>  		};
>  	};
>  
> +	/* NOTE: this node name is ABI, don't change it! */
>  	switch2@2 {
>  		compatible = "marvell,mv88e6085";
>  		reg = <0x2>;

I wouldn't spam the device tree with all these comments; doing so gives
a false sense of completeness. Code inspection shows that the "port-sfp@a"
node name is also established ABI, but there isn't any explicit comment
to point that out. I think a single comment that uses plural to refer to
all nodes should be enough.

Also, doesn't the comment go better along with the patch that changes
the switch compatible strings, rather than with the patch that actually
fixes what can be fixed (ethernet-phy node names)?