Skip to content

Commit

Permalink
net: usb: lan78xx: Remove KSZ9031 PHY fixup
Browse files Browse the repository at this point in the history
Remove the KSZ9031RNX PHY fixup from the lan78xx driver. The fixup applied
specific RGMII pad skew configurations globally, but these settings violate the
RGMII specification and cause more harm than benefit.

Key issues with the fixup:
1. **Non-Compliant Timing**: The fixup's delay settings fall outside the RGMII
   specification requirements of 1.5 ns to 2.0 ns:
   - RX Path: Total delay of **2.16 ns** (PHY internal delay of 1.2 ns + 0.96
     ns skew).
   - TX Path: Total delay of **0.96 ns**, significantly below the RGMII minimum
     of 1.5 ns.

2. **Redundant or Incorrect Configurations**:
   - The RGMII skew registers written by the fixup do not meaningfully alter
     the PHY's default behavior and fail to account for its internal delays.
   - The TX_DATA pad skew was not configured, relying on power-on defaults
     that are insufficient for RGMII compliance.

3. **Micrel Driver Support**: By setting `PHY_INTERFACE_MODE_RGMII_ID`, the
   Micrel driver can calculate and assign appropriate skew values for the
   KSZ9031 PHY.  This ensures better timing configurations without relying on
   external fixups.

4. **System Interference**: The fixup applied globally, reconfiguring all
   KSZ9031 PHYs in the system, even those unrelated to the LAN78xx adapter.
   This could lead to unintended and harmful behavior on unrelated interfaces.

While the fixup is removed, a better mechanism is still needed to dynamically
determine the optimal combination of PHY and MAC delays to fully meet RGMII
requirements without relying on Device Tree or global fixups. This would allow
for robust operation across different hardware configurations.

The Micrel driver is capable of using the interface mode value to calculate and
apply better skew values, providing a configuration much closer to the RGMII
specification than the fixup. Removing the fixup ensures better default
behavior and prevents harm to other system interfaces.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Link: https://patch.msgid.link/20241204084142.1152696-3-o.rempel@pengutronix.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Oleksij Rempel authored and Jakub Kicinski committed Dec 7, 2024
1 parent 7b60c3b commit 6782d06
Showing 1 changed file with 5 additions and 33 deletions.
38 changes: 5 additions & 33 deletions drivers/net/usb/lan78xx.c
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,6 @@ struct lan78xx_net {
struct irq_domain_data domain_data;
};

/* define external phy id */
#define PHY_KSZ9031RNX (0x00221620)

/* use ethtool to change the level for any given device */
static int msg_level = -1;
module_param(msg_level, int, 0);
Expand Down Expand Up @@ -2233,23 +2230,6 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev)
dev->domain_data.irqdomain = NULL;
}

static int ksz9031rnx_fixup(struct phy_device *phydev)
{
struct lan78xx_net *dev = netdev_priv(phydev->attached_dev);

/* Micrel9301RNX PHY configuration */
/* RGMII Control Signal Pad Skew */
phy_write_mmd(phydev, MDIO_MMD_WIS, 4, 0x0077);
/* RGMII RX Data Pad Skew */
phy_write_mmd(phydev, MDIO_MMD_WIS, 5, 0x7777);
/* RGMII RX Clock Pad Skew */
phy_write_mmd(phydev, MDIO_MMD_WIS, 8, 0x1FF);

dev->interface = PHY_INTERFACE_MODE_RGMII_RXID;

return 1;
}

static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev)
{
u32 buf;
Expand Down Expand Up @@ -2283,14 +2263,11 @@ static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev)
netdev_err(dev->net, "no PHY driver found\n");
return NULL;
}
dev->interface = PHY_INTERFACE_MODE_RGMII;
/* external PHY fixup for KSZ9031RNX */
ret = phy_register_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0,
ksz9031rnx_fixup);
if (ret < 0) {
netdev_err(dev->net, "Failed to register fixup for PHY_KSZ9031RNX\n");
return NULL;
}
dev->interface = PHY_INTERFACE_MODE_RGMII_ID;
/* The PHY driver is responsible to configure proper RGMII
* interface delays. Disable RGMII delays on MAC side.
*/
lan78xx_write_reg(dev, MAC_RGMII_ID, 0);

phydev->is_internal = false;
}
Expand Down Expand Up @@ -2349,9 +2326,6 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
if (phy_is_pseudo_fixed_link(phydev)) {
fixed_phy_unregister(phydev);
phy_device_free(phydev);
} else {
phy_unregister_fixup_for_uid(PHY_KSZ9031RNX,
0xfffffff0);
}
}
return -EIO;
Expand Down Expand Up @@ -4208,8 +4182,6 @@ static void lan78xx_disconnect(struct usb_interface *intf)

phydev = net->phydev;

phy_unregister_fixup_for_uid(PHY_KSZ9031RNX, 0xfffffff0);

phy_disconnect(net->phydev);

if (phy_is_pseudo_fixed_link(phydev)) {
Expand Down

0 comments on commit 6782d06

Please sign in to comment.