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()
Date: Fri, 2 Feb 2024 11:40:18 +0000 [thread overview]
Message-ID: <ZbzUotyQm/FyKK7G@shell.armlinux.org.uk> (raw)
In-Reply-To: <20240202-for-netnext-mt7530-improvements-2-v3-1-63d5adae99ca@arinc9.com>
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!
next prev parent reply other threads:[~2024-02-02 11:40 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-02 9:19 [PATCH net-next v3 0/7] MT7530 DSA Subdriver Improvements Act II Arınç ÜNAL via B4 Relay
2024-02-02 9:19 ` [PATCH net-next v3 1/7] net: dsa: mt7530: empty default case on mt7530_setup_port5() Arınç ÜNAL via B4 Relay
2024-02-02 11:40 ` Russell King (Oracle) [this message]
2024-02-02 17:44 ` Arınç ÜNAL
2024-02-02 18:05 ` Russell King (Oracle)
2024-02-02 20:25 ` Andrew Lunn
2024-02-05 20:29 ` Vladimir Oltean
2024-02-02 9:19 ` [PATCH net-next v3 2/7] net: dsa: mt7530: call port 6 setup from mt7530_mac_config() Arınç ÜNAL via B4 Relay
2024-02-02 11:45 ` Russell King (Oracle)
2024-02-02 9:19 ` [PATCH net-next v3 3/7] net: dsa: mt7530: remove pad_setup function pointer Arınç ÜNAL via B4 Relay
2024-02-02 11:46 ` Russell King (Oracle)
2024-02-02 9:19 ` [PATCH net-next v3 4/7] net: dsa: mt7530: move XTAL check to mt7530_setup() Arınç ÜNAL via B4 Relay
2024-02-02 11:48 ` Russell King (Oracle)
2024-02-02 18:16 ` Arınç ÜNAL
2024-02-02 18:39 ` Daniel Golle
2024-02-04 13:55 ` Arınç ÜNAL
2024-02-04 14:18 ` Russell King (Oracle)
2024-02-04 15:55 ` Arınç ÜNAL
2024-02-04 16:38 ` Russell King (Oracle)
2024-02-04 16:51 ` Arınç ÜNAL
2024-02-04 17:07 ` Russell King (Oracle)
2024-02-04 17:14 ` Arınç ÜNAL
2024-02-02 9:19 ` [PATCH net-next v3 5/7] net: dsa: mt7530: simplify mt7530_setup_port6() and change to void Arınç ÜNAL via B4 Relay
2024-02-02 11:51 ` Russell King (Oracle)
2024-02-02 9:19 ` [PATCH net-next v3 6/7] net: dsa: mt7530: correct port capabilities of MT7988 Arınç ÜNAL via B4 Relay
2024-02-02 9:19 ` [PATCH net-next v3 7/7] net: dsa: mt7530: do not clear config->supported_interfaces Arınç ÜNAL via B4 Relay
2024-02-02 11:52 ` Russell King (Oracle)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZbzUotyQm/FyKK7G@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=andrew@lunn.ch \
--cc=angelogioacchino.delregno@collabora.com \
--cc=arinc.unal@arinc9.com \
--cc=bartel.eerdekens@constell8.be \
--cc=daniel@makrotopia.org \
--cc=davem@davemloft.net \
--cc=dqfext@gmail.com \
--cc=edumazet@google.com \
--cc=erkin.bozoglu@xeront.com \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=mithat.guner@xeront.com \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=sean.wang@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).