netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: "Arınç ÜNAL" <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 18:05:36 +0000	[thread overview]
Message-ID: <Zb0u8NY0q6ay17j5@shell.armlinux.org.uk> (raw)
In-Reply-To: <e3b4add6-425c-46ca-9da5-8713055fc422@arinc9.com>

On Fri, Feb 02, 2024 at 08:44:39PM +0300, Arınç ÜNAL wrote:
> On 2.02.2024 14:40, Russell King (Oracle) wrote:
> > 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.
> 
> I will address this with the next patch series. Thank you for explaining it
> in detail.

This is a good time to point out not to rush with the next patch
series, as my email will _likely_ provoke some additional discussion
from Andrew and/or Vladimir. So please give it a few days (maybe
around the middle of next week) to give them time to consider my
email and respond.

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

  reply	other threads:[~2024-02-02 18:06 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)
2024-02-02 17:44     ` Arınç ÜNAL
2024-02-02 18:05       ` Russell King (Oracle) [this message]
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=Zb0u8NY0q6ay17j5@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).