Re: [PATCH net-next 1/4] dt-bindings: net: ethernet-controller: update descriptions of RGMII modes

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

On Wed, Apr 16, 2025 at 09:41:57AM +0200, Matthias Schiffer wrote:
> Also note that (as I understand it) I'm not changing anything, I'm updating the
> documentation to reflect what has been the intended behavior already. Please see
> the previous discussion with Andrew that I linked, where he convinced me that
> this is the correct approach.

I think you are as I stated in my email yesterday. The use of "MAC or
PHY" in your new descriptions opens avenues for confusion such as the
scenarios that I described in yesterday's email.

> Andrew specifically asked to leave it open in the DT bindings whether MAC
> or PHY add the delay, and it might differ between drivers (and different
> operating systems using the same Device Tree).

I'm hoping that Andrew will read my email form yesterday and reconsider
because to me this is a backwards step - it doesn't solve the problem
with unclear documentation. I believe it makes the problem worse, and
will lead to more bugs and misunderstandings in this area.

> Whether the MAC should add a required delay in cases where it's configurable
> is an interesting question - not one of the Device Tree bindings, but of
> driver implementation.

Where Andrew gets this from are MAC drivers that detect the rgmii-*id
modes, apply the delay at the MAC, and then convert the value passed to
phylib to PHY_INTERFACE_MODE_RGMII. This is a load of additional special
handling in the MAC driver, and I'd say it's "advanced" usage and takes
more time to review. It's open to mistakes without review by those who
know this "trick", and the chances of phylib maintainers being Cc'd on
MAC drivers is pretty low.

So, I don't think it's something we want to be generally encouraging,
but instead the more normal "phy-mode describes the phy_interface_mode_t
that is passed to phylib" and only allow the "advanced" case in
exceptional cases.

> On Linux, there currently isn't a way for the MAC driver to query from the PHY
> whether it could include the delays itself. My assumption is that most PHYs
> either don't have internal delays, or the delays are configurable.

motorcomm, dp83tg720, icplus, marvell, dp 838678, adin, micrel, tja11xx,
vitesse, dp83822, mscc, at803x, microchip_t1, broadcom, dp83869,
intel-xway, realtek all do handle internal delays. I haven't checked
whether there are PHYs that don't - that's harder because we don't know
whether PHYs that don't mention RGMII in the driver actually support
RGMII or not.

> If this is
> the case, having the MAC add them in internal-delay modes and not adding them on
> the PHY side would be the best default (also for PHY-less/fixed-link setups,
> which should be handled like a PHY without internal delay capabilities.)

See my "advanced" use case above. We do have drivers doing that.

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