public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Jens Emil Schulz Ostergaard
	<jensemil.schulzostergaard@microchip.com>,
	UNGLinuxDriver@microchip.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>,
	Simon Horman <horms@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Woojung Huh <woojung.huh@microchip.com>,
	Steen Hegelund <Steen.Hegelund@microchip.com>,
	Daniel Machon <daniel.machon@microchip.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH net-next 4/8] net: dsa: lan9645x: add basic dsa driver for LAN9645X
Date: Fri, 6 Mar 2026 14:22:14 +0000	[thread overview]
Message-ID: <aarjFiS6NX_7glkr@shell.armlinux.org.uk> (raw)
In-Reply-To: <052cf8c8-c60b-461a-86ce-aac8d7ebf4d9@lunn.ch>

On Thu, Mar 05, 2026 at 03:58:16PM +0100, Andrew Lunn wrote:
> > > > +     /* We configure delays on the MAC side. When the PHY is not responsible
> > > > +      * for delays, the MAC is, which is why RGMII_TXID results in
> > > > +      * rx_delay=true.
> > > > +      */
> > > > +     if (mode == PHY_INTERFACE_MODE_RGMII ||
> > > > +         mode == PHY_INTERFACE_MODE_RGMII_TXID)
> > > > +             rx_delay = true;
> > > > +
> > > > +     if (mode == PHY_INTERFACE_MODE_RGMII ||
> > > > +         mode == PHY_INTERFACE_MODE_RGMII_RXID)
> > > > +             tx_delay = true;
> > > 
> > > PHY_INTERFACE_MODE_RGMII means that the delays are provided by the
> > > board layout, so this seems wrong to me. Please review the phylib
> > > documentation concerning RGMII delays. I'd like Andrew to comment on
> > > this too.
> > > 
> > 
> > The idea with this was, if the PHY does not provide internal delay, then
> > we do it in the MAC.
> 
> That is not really what we want. The default in Linux is that the PHY
> adds the delays. In general, MAC drivers read the phy-mode, and pass
> it directly to the PHY. The MAC does not add delays. There are a small
> number of boards which don't respect this. In the past, there was been
> claims that an RDK used a PHY which was not capable of adding the
> needed delays, so the MAC had to. In practice, all the PHY drivers in
> linux which supporting RGMII support all four configurations of
> delays.

Unfortunately, I've discovered another case - dwmac-qcom-ethqos::
ethqos_rgmii_macro_init():

        /* Determine if the PHY adds a 2 ns TX delay or the MAC handles it */
        if (ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_ID ||
            ethqos->phy_mode == PHY_INTERFACE_MODE_RGMII_TXID)
                phase_shift = 0;
        else
                phase_shift = RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN;

This code was added by 164a9ebe9742 ("net: stmmac: dwmac-qcom-ethqos:
Respect phy-mode and TX delay") which doesn't have any acks from
phylib folk. However, before this commit, the driver always added the
transmit delay.

Sadly, too late to do anything about this now. :(

Should a regexp be added to MAINTAINERS phylib entry to detect patches
which contain PHY_INTERFACE_MODE_RGMII?

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

  parent reply	other threads:[~2026-03-06 14:22 UTC|newest]

Thread overview: 69+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-03 12:22 [PATCH net-next 0/8] net: dsa: add DSA support for the LAN9645x switch chip family Jens Emil Schulz Østergaard
2026-03-03 12:22 ` [PATCH net-next 1/8] net: dsa: add tag driver for LAN9645X Jens Emil Schulz Østergaard
2026-03-03 14:13   ` Andrew Lunn
2026-03-03 15:58     ` Jens Emil Schulz Ostergaard
2026-03-04 15:14       ` Andrew Lunn
2026-03-05 12:59         ` Jens Emil Schulz Ostergaard
2026-03-03 16:11   ` Vladimir Oltean
2026-03-05 13:53     ` Jens Emil Schulz Ostergaard
2026-03-04 15:23   ` Andrew Lunn
2026-03-05 13:01     ` Jens Emil Schulz Ostergaard
2026-03-03 12:22 ` [PATCH net-next 2/8] dt-bindings: net: lan9645x: add LAN9645X switch bindings Jens Emil Schulz Østergaard
2026-03-03 13:22   ` Vladimir Oltean
2026-03-03 16:00     ` Jens Emil Schulz Ostergaard
2026-03-03 14:18   ` Andrew Lunn
2026-03-03 19:04     ` Conor Dooley
2026-03-04 15:57       ` Jens Emil Schulz Ostergaard
2026-03-05 12:57       ` Jens Emil Schulz Ostergaard
2026-03-05 18:31         ` Conor Dooley
2026-03-06 15:08           ` Jens Emil Schulz Ostergaard
2026-03-06 15:20             ` Conor Dooley
2026-03-18 14:19               ` Jens Emil Schulz Ostergaard
2026-03-18 17:18                 ` Conor Dooley
2026-03-18 17:20                   ` Conor Dooley
2026-03-18 17:26                     ` Christian Marangi
2026-03-24 10:31                       ` Jens Emil Schulz Ostergaard
2026-03-04 15:55     ` Jens Emil Schulz Ostergaard
2026-03-03 18:49   ` Conor Dooley
2026-03-04 15:58     ` Jens Emil Schulz Ostergaard
2026-03-03 18:56   ` Conor Dooley
2026-03-04 16:10     ` Jens Emil Schulz Ostergaard
2026-03-04 16:14       ` Vladimir Oltean
2026-03-04 19:06         ` Conor Dooley
2026-03-05 13:08           ` Jens Emil Schulz Ostergaard
2026-03-03 12:22 ` [PATCH net-next 3/8] net: dsa: lan9645x: add autogenerated register macros Jens Emil Schulz Østergaard
2026-03-03 12:22 ` [PATCH net-next 4/8] net: dsa: lan9645x: add basic dsa driver for LAN9645X Jens Emil Schulz Østergaard
2026-03-03 14:15   ` Vladimir Oltean
2026-03-04 14:37     ` Jens Emil Schulz Ostergaard
2026-03-04 15:58   ` Russell King (Oracle)
2026-03-05 14:24     ` Jens Emil Schulz Ostergaard
2026-03-05 14:58       ` Andrew Lunn
2026-03-05 15:10         ` Vladimir Oltean
2026-03-05 16:54         ` Alexander Stein
2026-03-05 17:37           ` Andrew Lunn
2026-03-06 15:03             ` Jens Emil Schulz Ostergaard
2026-03-06 16:33               ` Andrew Lunn
2026-03-09 12:01                 ` Jens Emil Schulz Ostergaard
2026-03-06 14:22         ` Russell King (Oracle) [this message]
2026-03-06 21:03           ` Jakub Kicinski
2026-03-03 12:22 ` [PATCH net-next 5/8] net: dsa: lan9645x: add bridge support Jens Emil Schulz Østergaard
2026-03-03 14:20   ` Vladimir Oltean
2026-03-03 16:08     ` Jens Emil Schulz Ostergaard
2026-03-03 16:17       ` Vladimir Oltean
2026-03-05 13:14         ` Jens Emil Schulz Ostergaard
2026-03-03 14:51   ` Vladimir Oltean
2026-03-09 12:09     ` Jens Emil Schulz Ostergaard
2026-03-03 12:22 ` [PATCH net-next 6/8] net: dsa: lan9645x: add vlan support Jens Emil Schulz Østergaard
2026-03-03 14:59   ` Vladimir Oltean
2026-03-04 14:40     ` Jens Emil Schulz Ostergaard
2026-03-04 14:52       ` Vladimir Oltean
2026-03-03 12:22 ` [PATCH net-next 7/8] net: dsa: lan9645x: add mac table integration Jens Emil Schulz Østergaard
2026-03-03 15:27   ` Vladimir Oltean
2026-03-04 15:23     ` Jens Emil Schulz Ostergaard
2026-03-04 15:34       ` Andrew Lunn
2026-03-05 13:17         ` Jens Emil Schulz Ostergaard
2026-03-03 12:22 ` [PATCH net-next 8/8] net: dsa: lan9645x: add port statistics Jens Emil Schulz Østergaard
2026-03-03 16:01   ` Vladimir Oltean
2026-03-03 20:21     ` Andrew Lunn
2026-03-04 15:51       ` Jens Emil Schulz Ostergaard
2026-03-04 15:50     ` Jens Emil Schulz Ostergaard

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=aarjFiS6NX_7glkr@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Steen.Hegelund@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=conor+dt@kernel.org \
    --cc=daniel.machon@microchip.com \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jensemil.schulzostergaard@microchip.com \
    --cc=krzk+dt@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=robh@kernel.org \
    --cc=woojung.huh@microchip.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