| Message ID | 20241203072154.2440034-2-o.rempel@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | lan78xx: Preparations for PHYlink | expand |
On Tue, Dec 03, 2024 at 08:21:34AM +0100, Oleksij Rempel wrote: > Remove the PHY fixup for the LAN8835 PHY in the lan78xx driver due to > the following reasons: > > - There is no publicly available information about the LAN8835 PHY. > However, it appears to be the integrated PHY used in the LAN7800 and > LAN7850 USB Ethernet controllers. These PHYs use the GMII interface, > not RGMII as configured by the fixup. > > - The correct driver for handling the LAN8835 PHY functionality is the > Microchip PHY driver (`drivers/net/phy/microchip.c`), which properly > supports these integrated PHYs. > > - The PHY ID `0x0007C130` is actually used by the LAN8742A PHY, which > only supports RMII. This interface is incompatible with the LAN78xx > MAC, as the LAN7801 (the only LAN78xx version without an integrated > PHY) supports only RGMII. > > - The mask applied for this fixup is overly broad, inadvertently > covering both Microchip LAN88xx PHYs and unrelated SMSC LAN8742A PHYs, > leading to potential conflicts with other devices. > > - Testing has shown that removing this fixup for LAN7800 and LAN7850 > does not result in any noticeable difference in functionality, as the > Microchip PHY driver (`drivers/net/phy/microchip.c`) handles all > necessary configurations for these integrated PHYs. > > - Registering this fixup globally (not limited to USB devices) risks > conflicts by unintentionally modifying other interfaces whenever a > LAN7801 adapter is connected to the system. > > Note that both LAN7800 and LAN7850 USB Ethernet controllers use an > integrated PHY with the ID `0x0007C132`. Additionally, the LAN7515, a > specialized part for Raspberry Pi, includes an integrated LAN7800 USB > Ethernet controller and USB hub in a multifunctional chip design, and it > also uses the same PHY ID (`0x0007C132`). I had a long frustrating discussion about adding yet more such fixups a while ago with somebody how did not understand the implications of adding another one. It is good to see this one being removed, with a good explanation why. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 531b1b6a37d1..6e468e77d796 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -473,7 +473,6 @@ struct lan78xx_net { }; /* define external phy id */ -#define PHY_LAN8835 (0x0007C130) #define PHY_KSZ9031RNX (0x00221620) /* use ethtool to change the level for any given device */ @@ -2234,29 +2233,6 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev) dev->domain_data.irqdomain = NULL; } -static int lan8835_fixup(struct phy_device *phydev) -{ - int buf; - struct lan78xx_net *dev = netdev_priv(phydev->attached_dev); - - /* LED2/PME_N/IRQ_N/RGMII_ID pin to IRQ_N mode */ - buf = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x8010); - buf &= ~0x1800; - buf |= 0x0800; - phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8010, buf); - - /* RGMII MAC TXC Delay Enable */ - lan78xx_write_reg(dev, MAC_RGMII_ID, - MAC_RGMII_ID_TXC_DELAY_EN_); - - /* RGMII TX DLL Tune Adjust */ - lan78xx_write_reg(dev, RGMII_TX_BYP_DLL, 0x3D00); - - dev->interface = PHY_INTERFACE_MODE_RGMII_TXID; - - return 1; -} - static int ksz9031rnx_fixup(struct phy_device *phydev) { struct lan78xx_net *dev = netdev_priv(phydev->attached_dev); @@ -2315,14 +2291,6 @@ static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev) netdev_err(dev->net, "Failed to register fixup for PHY_KSZ9031RNX\n"); return NULL; } - /* external PHY fixup for LAN8835 */ - ret = phy_register_fixup_for_uid(PHY_LAN8835, 0xfffffff0, - lan8835_fixup); - if (ret < 0) { - netdev_err(dev->net, "Failed to register fixup for PHY_LAN8835\n"); - return NULL; - } - /* add more external PHY fixup here if needed */ phydev->is_internal = false; } @@ -2384,8 +2352,6 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) } else { phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0); - phy_unregister_fixup_for_uid(PHY_LAN8835, - 0xfffffff0); } } return -EIO; @@ -4243,7 +4209,6 @@ static void lan78xx_disconnect(struct usb_interface *intf) phydev = net->phydev; phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0); - phy_unregister_fixup_for_uid(PHY_LAN8835, 0xfffffff0); phy_disconnect(net->phydev);
Remove the PHY fixup for the LAN8835 PHY in the lan78xx driver due to the following reasons: - There is no publicly available information about the LAN8835 PHY. However, it appears to be the integrated PHY used in the LAN7800 and LAN7850 USB Ethernet controllers. These PHYs use the GMII interface, not RGMII as configured by the fixup. - The correct driver for handling the LAN8835 PHY functionality is the Microchip PHY driver (`drivers/net/phy/microchip.c`), which properly supports these integrated PHYs. - The PHY ID `0x0007C130` is actually used by the LAN8742A PHY, which only supports RMII. This interface is incompatible with the LAN78xx MAC, as the LAN7801 (the only LAN78xx version without an integrated PHY) supports only RGMII. - The mask applied for this fixup is overly broad, inadvertently covering both Microchip LAN88xx PHYs and unrelated SMSC LAN8742A PHYs, leading to potential conflicts with other devices. - Testing has shown that removing this fixup for LAN7800 and LAN7850 does not result in any noticeable difference in functionality, as the Microchip PHY driver (`drivers/net/phy/microchip.c`) handles all necessary configurations for these integrated PHYs. - Registering this fixup globally (not limited to USB devices) risks conflicts by unintentionally modifying other interfaces whenever a LAN7801 adapter is connected to the system. Note that both LAN7800 and LAN7850 USB Ethernet controllers use an integrated PHY with the ID `0x0007C132`. Additionally, the LAN7515, a specialized part for Raspberry Pi, includes an integrated LAN7800 USB Ethernet controller and USB hub in a multifunctional chip design, and it also uses the same PHY ID (`0x0007C132`). Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/net/usb/lan78xx.c | 35 ----------------------------------- 1 file changed, 35 deletions(-)