Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Wed, 29 Nov 2023 20:18:42 +0200
From:   Vladimir Oltean <olteanv@gmail.com>
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Gregory Clement <gregory.clement@bootlin.com>,
        Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Russell King <linux@armlinux.org.uk>,
        Florian Fainelli <f.fainelli@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Marek =?utf-8?B?QmVow7pu?= <kabel@kernel.org>,
        Christian Marangi <ansuelsmth@gmail.com>,
        linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
        Florian Fainelli <florian.fainelli@broadcom.com>
Subject: Re: [PATCH net-next v8 5/9] ARM64: dts: marvell: Fix some common
 switch mistakes
Message-ID: <20231129181842.74zihcd642lx5zgo@skbuf>
References: <20231114-marvell-88e6152-wan-led-v8-0-50688741691b@linaro.org>
 <20231114-marvell-88e6152-wan-led-v8-5-50688741691b@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231114-marvell-88e6152-wan-led-v8-5-50688741691b@linaro.org>
Precedence: bulk

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)?