netdev - Re: [PATCH net-next 7/7] net: lan969x: add function for configuring RGMII port devices

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: <Zy32_Bs7gDAtay5V@shell.armlinux.org.uk>
Date: Fri, 8 Nov 2024 11:33:16 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Daniel Machon <daniel.machon@...rochip.com>
Cc: Andrew Lunn <andrew@...n.ch>, UNGLinuxDriver@...rochip.com,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Lars Povlsen <lars.povlsen@...rochip.com>,
	Steen Hegelund <Steen.Hegelund@...rochip.com>,
	Horatiu Vultur <horatiu.vultur@...rochip.com>,
	jacob.e.keller@...el.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH net-next 7/7] net: lan969x: add function for configuring
 RGMII port devices

On Fri, Nov 08, 2024 at 08:53:20AM +0000, Daniel Machon wrote:
> Hi Andrew,
> 
> > > +     if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
> > > +         conf->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
> > > +             rx_delay = true;
> > > +
> > > +     if (conf->phy_mode == PHY_INTERFACE_MODE_RGMII ||
> > > +         conf->phy_mode == PHY_INTERFACE_MODE_RGMII_RXID)
> > > +             tx_delay = true;
> > 
> > O.K, now warning bells are ringing in this reviews head.
> > 
> > What i don't see is the value you pass to the PHY? You obviously need
> > to mask out what the MAC is doing when talking to the PHY, otherwise
> > both ends will add delays.
> > 
> 
> What value should be passed to the PHY?
> 
> We (the MAC) add the delays based on the PHY modes - so does the PHY.
> 
> RGMII, we add both delays.
> RGMII_ID, the PHY adds both delays.
> RGMII_TXID, we add the rx delay, the PHY adds the tx delay.
> RGMII_RXID, we add the tx delay, the PHY adds the rx delay.
> 
> Am I missing something here? :-)

What if the board routing adds the necessary delays?

>From Documentation/networking/phy.rst:
"
* PHY_INTERFACE_MODE_RGMII: the PHY is not responsible for inserting any
  internal delay by itself, it assumes that either the Ethernet MAC (if capable)
  or the PCB traces insert the correct 1.5-2ns delay
...
For cases where the PHY is not capable of providing this delay, but the
Ethernet MAC driver is capable of doing so, the correct phy_interface_t value
should be PHY_INTERFACE_MODE_RGMII, and the Ethernet MAC driver should be
configured correctly in order to provide the required transmit and/or receive
side delay from the perspective of the PHY device. Conversely, if the Ethernet
MAC driver looks at the phy_interface_t value, for any other mode but
PHY_INTERFACE_MODE_RGMII, it should make sure that the MAC-level delays are
disabled."

The point here is that you have three entities that can deal with the
required delays - the PHY, the board, and the MAC.

PHY_INTERFACE_MODE_RGMII* passed to phylink/phylib tells the PHY how it
should program its delay capabilities.

We're then down to dealing with the MAC and board induced delays. Many
implementations use the rx-internal-delay-ps and tx-internal-delay-ps
properties defined in the ethernet-controller.yaml DT binding to
control the MAC delays.

However, there are a few which use PHY_INTERFACE_MODE_RGMII* on the MAC
end, but in this case, they always pass PHY_INTERFACE_MODE_RGMII to
phylib to stop the PHY adding any delays.

However, we don't have a way at present for DSA/phylink etc to handle a
MAC that wants to ddd its delays with the PHY set to
PHY_INTERFACE_MODE_RGMII.

Thanks.

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