| Message ID | 20201025085556.2861021-1-icenowy@aosc.io |
|---|---|
| State | New |
| Headers | show |
| Series | net: phy: realtek: omit setting PHY-side delay when "rgmii" specified | expand |
> >> 1. As the PHY chip has hardware configuration for configuring delays, > >> we should at least have a mode that respects what's set on the > >hardware. > > > >Yes, that is PHY_INTERFACE_MODE_NA. In DT, set the phy-mode to "". Or > >for most MAC drivers, don't list a phy-mode at all. > > However, we still need to tell the MAC it's RGMII mode that is in use, not > MII/RMII/*MII. So the phy-mode still needs to be something that > contains rgmii. So for this MAC driver, you are going to have to fix the device tree. And for the short turn, maybe implement the workaround discussed in the other thread. Andrew
> By referring to linux/phy.h, NA means not applicable. This surely > do not apply when RGMII is really in use. It means the PHY driver should not touch the mode, something else has set it up. That could be strapping, the bootloader, ACPI firmware, whatever. > I think no document declares RGMII must have all internal delays > of the PHY explicitly disabled. It just says RGMII. Please take a look at all the other PHY drivers. They should all disable delays when passed PHY_INTERFACE_MODE_RGMII. Andrew
Icenowy, On 10/26/20 7:12 AM, Andrew Lunn wrote: >> By referring to linux/phy.h, NA means not applicable. This surely >> do not apply when RGMII is really in use. > > It means the PHY driver should not touch the mode, something else has > set it up. That could be strapping, the bootloader, ACPI firmware, > whatever. > >> I think no document declares RGMII must have all internal delays >> of the PHY explicitly disabled. It just says RGMII. > > Please take a look at all the other PHY drivers. They should all > disable delays when passed PHY_INTERFACE_MODE_RGMII. Documentation/networking/phy.rst also makes this clear: 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 Regards, Samuel
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c index fb1db713b7fb..7d32db1c789f 100644 --- a/drivers/net/phy/realtek.c +++ b/drivers/net/phy/realtek.c @@ -189,11 +189,6 @@ static int rtl8211f_config_init(struct phy_device *phydev) phy_modify_paged_changed(phydev, 0xa43, RTL8211F_PHYCR1, val, val); switch (phydev->interface) { - case PHY_INTERFACE_MODE_RGMII: - val_txdly = 0; - val_rxdly = 0; - break; - case PHY_INTERFACE_MODE_RGMII_RXID: val_txdly = 0; val_rxdly = RTL8211F_RX_DELAY; @@ -253,9 +248,6 @@ static int rtl8211e_config_init(struct phy_device *phydev) /* enable TX/RX delay for rgmii-* modes, and disable them for rgmii. */ switch (phydev->interface) { - case PHY_INTERFACE_MODE_RGMII: - val = RTL8211E_CTRL_DELAY | 0; - break; case PHY_INTERFACE_MODE_RGMII_ID: val = RTL8211E_CTRL_DELAY | RTL8211E_TX_DELAY | RTL8211E_RX_DELAY; break;
Currently there are many boards that just set "rgmii" as phy-mode in the device tree, and leave the hardware [TR]XDLY pins to set PHY delay mode. In order to keep old device tree working, omit setting delay for just "RGMII" without any internal delay suffix, otherwise many devices are broken. The definition of "rgmii" in the DT binding document is "RX and TX delays are added by MAC when required", which at least literally do not forbid the PHY to add delays. Fixes: bbc4d71d6354 ("net: phy: realtek: fix rtl8211e rx/tx delay config") Signed-off-by: Icenowy Zheng <icenowy@aosc.io> --- drivers/net/phy/realtek.c | 8 -------- 1 file changed, 8 deletions(-)