diff mbox series

[net-next,v1,01/21] net: usb: lan78xx: Remove LAN8835 PHY fixup

Message ID 20241203072154.2440034-2-o.rempel@pengutronix.de (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series lan78xx: Preparations for PHYlink | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 1 maintainers not CCed: linux-usb@vger.kernel.org
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 306 this patch: 306
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 65 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Oleksij Rempel Dec. 3, 2024, 7:21 a.m. UTC
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(-)

Comments

Andrew Lunn Dec. 3, 2024, 8:26 p.m. UTC | #1
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 mbox series

Patch

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);