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] [day] [month] [year] [list]

Message-ID: <20241112102617.lhb5ou7kxlqm7qhk@DEN-DL-M70577>
Date: Tue, 12 Nov 2024 10:26:17 +0000
From: Daniel Machon <daniel.machon@...rochip.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
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

Hi Russel,

> > 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
> ...

Ack. The case where the PCB traces add the delay is certainly not
handled with the current changes.

> 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.

Right, so using the {rx,tx}-internal-delay-ps allows me to configure the
MAC delays, or skip them entirely, in case the PCB adds them.

Thanks!

/Daniel