Received-SPF: pass (google.com: domain of linux-kernel+bounces-49780-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Fri, 2 Feb 2024 11:40:18 +0000
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: arinc.unal@arinc9.com
Cc: Daniel Golle <daniel@makrotopia.org>, DENG Qingfang <dqfext@gmail.com>,
	Sean Wang <sean.wang@mediatek.com>, Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>,
	mithat.guner@xeront.com, erkin.bozoglu@xeront.com,
	Bartel Eerdekens <bartel.eerdekens@constell8.be>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH net-next v3 1/7] net: dsa: mt7530: empty default case on
 mt7530_setup_port5()
Message-ID: <ZbzUotyQm/FyKK7G@shell.armlinux.org.uk>
References: <20240202-for-netnext-mt7530-improvements-2-v3-0-63d5adae99ca@arinc9.com>
 <20240202-for-netnext-mt7530-improvements-2-v3-1-63d5adae99ca@arinc9.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240202-for-netnext-mt7530-improvements-2-v3-1-63d5adae99ca@arinc9.com>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>

On Fri, Feb 02, 2024 at 12:19:07PM +0300, Arınç ÜNAL via B4 Relay wrote:
> From: Arınç ÜNAL <arinc.unal@arinc9.com>
> 
> There're two code paths for setting up port 5:
> 
> mt7530_setup()
> -> mt7530_setup_port5()
> 
> mt753x_phylink_mac_config()
> -> mt753x_mac_config()
>    -> mt7530_mac_config()
>       -> mt7530_setup_port5()
> 
> On the first code path, priv->p5_intf_sel is either set to
> P5_INTF_SEL_PHY_P0 or P5_INTF_SEL_PHY_P4 when mt7530_setup_port5() is run.
> 
> On the second code path, priv->p5_intf_sel is set to P5_INTF_SEL_GMAC5 when
> mt7530_setup_port5() is run.
> 
> Empty the default case which will never run but is needed nonetheless to
> handle all the remaining enumeration values.
> 
> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com>
> Reviewed-by: Vladimir Oltean <olteanv@gmail.com>

Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>

Thanks!

While reviewing this change, but not related to it, I notice that this
function sets the TX delay based on the RGMII interface mode. This isn't
correct. I've explained why this is this many times in the past, but
essentially it comes down to the model:


phy-mode in NIC node	Network driver	PCB		PHY
rgmii			no delays	delays		no delays
rgmii-id		no delays	no delays	tx/rx delays
rgmii-txid		no delays	no delays	tx delays
rgmii-rxid		no delays	no delays	rx delays

Then we have rx-internal-delay-ps and tx-internal-delay-ps in the NIC
node which define the RGMII delays at the local end and similar
properties for the PHY node.


So, if we take the view that, when a switch is connected to a NIC in
RGMII mode, then the phy-mode specified delays still should not impact
the local NIC.

Now, for the switch, we specify the phy-mode in the port node as well.
Consider the case of a switch port connected to a RGMII PHY. This has
to operate in exactly the same way as a normal NIC - that is, the
RGMII delays at the port should be ignored as it's the responsibility
of a PHY.

The final scenario to examine is the case of a RGMII switch port
connected to a NIC. The NIC's phy-mode has no way to be communicated
to DSA or vice versa, so neither phy-mode can impact the other side
of the RGMII link, but should only place the link into RGMII mode.
Given everything I've said above, the only way to configure RGMII
delays is via the rx-internal-delay-ps and tx-internal-delay-ps
properties. So, DSA drivers should _not_ be configuring their ports
with RGMII delays based on the RGMII phy interface mode.

The above is my purely logically reasoned point of view on this
subject. Others may have other (to me completely illogical)
interpretations that can only lead to interoperability issues.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!