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

["Russell King (Oracle)" <linux@armlinux.org.uk>]

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!