* [PATCH net-next v8 1/7] net: usb: lan78xx: Improve error handling in PHY initialization
2025-05-05 8:43 [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion Oleksij Rempel
@ 2025-05-05 8:43 ` Oleksij Rempel
2025-05-07 3:56 ` Thangaraj.S
2025-05-05 8:43 ` [PATCH net-next v8 2/7] net: usb: lan78xx: remove explicit check for missing PHY driver Oleksij Rempel
` (6 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Oleksij Rempel @ 2025-05-05 8:43 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Woojung Huh, Andrew Lunn, Russell King, Thangaraj Samynathan,
Rengarajan Sundararajan
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
Phil Elwell, Maxime Chevallier, Simon Horman
Ensure that return values from `lan78xx_write_reg()`,
`lan78xx_read_reg()`, and `phy_find_first()` are properly checked and
propagated. Use `ERR_PTR(ret)` for error reporting in
`lan7801_phy_init()` and replace `-EIO` with `-ENODEV` where appropriate
to provide more accurate error codes.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v6:
- confirmed with maintainers that fixed_phy_register() leak is acceptable
as soon as it is fixed within the patch set
changes v5:
- make sure lan7801_phy_init() caller is testing against IS_ERR
instead of NULL.
changes v4:
- split the patch and move part of it before PHYlink migration
---
drivers/net/usb/lan78xx.c | 47 ++++++++++++++++++++++++++-------------
1 file changed, 31 insertions(+), 16 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index e4f1663b6204..19db18cf0504 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2510,14 +2510,13 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev)
static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev)
{
- u32 buf;
- int ret;
struct fixed_phy_status fphy_status = {
.link = 1,
.speed = SPEED_1000,
.duplex = DUPLEX_FULL,
};
struct phy_device *phydev;
+ int ret;
phydev = phy_find_first(dev->mdiobus);
if (!phydev) {
@@ -2525,30 +2524,40 @@ static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev)
phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
if (IS_ERR(phydev)) {
netdev_err(dev->net, "No PHY/fixed_PHY found\n");
- return NULL;
+ return ERR_PTR(-ENODEV);
}
netdev_dbg(dev->net, "Registered FIXED PHY\n");
dev->interface = PHY_INTERFACE_MODE_RGMII;
ret = lan78xx_write_reg(dev, MAC_RGMII_ID,
MAC_RGMII_ID_TXC_DELAY_EN_);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
ret = lan78xx_write_reg(dev, RGMII_TX_BYP_DLL, 0x3D00);
- ret = lan78xx_read_reg(dev, HW_CFG, &buf);
- buf |= HW_CFG_CLK125_EN_;
- buf |= HW_CFG_REFCLK25_EN_;
- ret = lan78xx_write_reg(dev, HW_CFG, buf);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ ret = lan78xx_update_reg(dev, HW_CFG, HW_CFG_CLK125_EN_ |
+ HW_CFG_REFCLK25_EN_,
+ HW_CFG_CLK125_EN_ | HW_CFG_REFCLK25_EN_);
+ if (ret < 0)
+ return ERR_PTR(ret);
} else {
if (!phydev->drv) {
netdev_err(dev->net, "no PHY driver found\n");
- return NULL;
+ return ERR_PTR(-EINVAL);
}
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);
+ ret = lan78xx_write_reg(dev, MAC_RGMII_ID, 0);
+ if (ret < 0)
+ return ERR_PTR(ret);
phydev->is_internal = false;
}
+
return phydev;
}
@@ -2562,9 +2571,10 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
switch (dev->chipid) {
case ID_REV_CHIP_ID_7801_:
phydev = lan7801_phy_init(dev);
- if (!phydev) {
- netdev_err(dev->net, "lan7801: PHY Init Failed");
- return -EIO;
+ if (IS_ERR(phydev)) {
+ netdev_err(dev->net, "lan7801: failed to init PHY: %pe\n",
+ phydev);
+ return PTR_ERR(phydev);
}
break;
@@ -2573,7 +2583,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
phydev = phy_find_first(dev->mdiobus);
if (!phydev) {
netdev_err(dev->net, "no PHY found\n");
- return -EIO;
+ return -ENODEV;
}
phydev->is_internal = true;
dev->interface = PHY_INTERFACE_MODE_GMII;
@@ -2581,7 +2591,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
default:
netdev_err(dev->net, "Unknown CHIP ID found\n");
- return -EIO;
+ return -ENODEV;
}
/* if phyirq is not set, use polling mode in phylib */
@@ -2633,7 +2643,10 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
sizeof(u32));
if (len >= 0) {
/* Ensure the appropriate LEDs are enabled */
- lan78xx_read_reg(dev, HW_CFG, ®);
+ ret = lan78xx_read_reg(dev, HW_CFG, ®);
+ if (ret < 0)
+ return ret;
+
reg &= ~(HW_CFG_LED0_EN_ |
HW_CFG_LED1_EN_ |
HW_CFG_LED2_EN_ |
@@ -2642,7 +2655,9 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
(len > 1) * HW_CFG_LED1_EN_ |
(len > 2) * HW_CFG_LED2_EN_ |
(len > 3) * HW_CFG_LED3_EN_;
- lan78xx_write_reg(dev, HW_CFG, reg);
+ ret = lan78xx_write_reg(dev, HW_CFG, reg);
+ if (ret < 0)
+ return ret;
}
}
--
2.39.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v8 1/7] net: usb: lan78xx: Improve error handling in PHY initialization
2025-05-05 8:43 ` [PATCH net-next v8 1/7] net: usb: lan78xx: Improve error handling in PHY initialization Oleksij Rempel
@ 2025-05-07 3:56 ` Thangaraj.S
0 siblings, 0 replies; 16+ messages in thread
From: Thangaraj.S @ 2025-05-07 3:56 UTC (permalink / raw)
To: andrew+netdev, rmk+kernel, davem, Rengarajan.S, Woojung.Huh,
pabeni, o.rempel, edumazet, kuba
Cc: phil, kernel, horms, linux-kernel, netdev, UNGLinuxDriver,
maxime.chevallier
On Mon, 2025-05-05 at 10:43 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Ensure that return values from `lan78xx_write_reg()`,
> `lan78xx_read_reg()`, and `phy_find_first()` are properly checked and
> propagated. Use `ERR_PTR(ret)` for error reporting in
> `lan7801_phy_init()` and replace `-EIO` with `-ENODEV` where
> appropriate
> to provide more accurate error codes.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
Reviewed-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v8 2/7] net: usb: lan78xx: remove explicit check for missing PHY driver
2025-05-05 8:43 [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion Oleksij Rempel
2025-05-05 8:43 ` [PATCH net-next v8 1/7] net: usb: lan78xx: Improve error handling in PHY initialization Oleksij Rempel
@ 2025-05-05 8:43 ` Oleksij Rempel
2025-05-07 3:57 ` Thangaraj.S
2025-05-05 8:43 ` [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration Oleksij Rempel
` (5 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Oleksij Rempel @ 2025-05-05 8:43 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Woojung Huh, Andrew Lunn, Russell King, Thangaraj Samynathan,
Rengarajan Sundararajan
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
Phil Elwell, Maxime Chevallier, Simon Horman
RGMII timing correctness relies on the PHY providing internal delays.
This is typically ensured via PHY driver, strap pins, or PCB layout.
Explicitly checking for a PHY driver here is unnecessary and non-standard.
This logic applies to all MACs, not just LAN78xx, and should be left to
phylib, phylink, or platform configuration.
Drop the check and rely on standard subsystem behavior.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v6:
- this patch is added in v6
---
drivers/net/usb/lan78xx.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 19db18cf0504..9c0658227bde 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2543,10 +2543,6 @@ static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev)
if (ret < 0)
return ERR_PTR(ret);
} else {
- if (!phydev->drv) {
- netdev_err(dev->net, "no PHY driver found\n");
- return ERR_PTR(-EINVAL);
- }
dev->interface = PHY_INTERFACE_MODE_RGMII_ID;
/* The PHY driver is responsible to configure proper RGMII
* interface delays. Disable RGMII delays on MAC side.
--
2.39.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v8 2/7] net: usb: lan78xx: remove explicit check for missing PHY driver
2025-05-05 8:43 ` [PATCH net-next v8 2/7] net: usb: lan78xx: remove explicit check for missing PHY driver Oleksij Rempel
@ 2025-05-07 3:57 ` Thangaraj.S
0 siblings, 0 replies; 16+ messages in thread
From: Thangaraj.S @ 2025-05-07 3:57 UTC (permalink / raw)
To: andrew+netdev, rmk+kernel, davem, Rengarajan.S, Woojung.Huh,
pabeni, o.rempel, edumazet, kuba
Cc: phil, kernel, horms, linux-kernel, netdev, UNGLinuxDriver,
maxime.chevallier
On Mon, 2025-05-05 at 10:43 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> RGMII timing correctness relies on the PHY providing internal delays.
> This is typically ensured via PHY driver, strap pins, or PCB layout.
>
> Explicitly checking for a PHY driver here is unnecessary and non-
> standard.
> This logic applies to all MACs, not just LAN78xx, and should be left
> to
> phylib, phylink, or platform configuration.
>
> Drop the check and rely on standard subsystem behavior.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
Reviewed-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration
2025-05-05 8:43 [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion Oleksij Rempel
2025-05-05 8:43 ` [PATCH net-next v8 1/7] net: usb: lan78xx: Improve error handling in PHY initialization Oleksij Rempel
2025-05-05 8:43 ` [PATCH net-next v8 2/7] net: usb: lan78xx: remove explicit check for missing PHY driver Oleksij Rempel
@ 2025-05-05 8:43 ` Oleksij Rempel
2025-05-06 5:31 ` Thangaraj.S
2025-05-07 3:58 ` Thangaraj.S
2025-05-05 8:43 ` [PATCH net-next v8 4/7] net: usb: lan78xx: move LED DT configuration to helper Oleksij Rempel
` (4 subsequent siblings)
7 siblings, 2 replies; 16+ messages in thread
From: Oleksij Rempel @ 2025-05-05 8:43 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Woojung Huh, Andrew Lunn, Russell King, Thangaraj Samynathan,
Rengarajan Sundararajan
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
Phil Elwell, Maxime Chevallier, Simon Horman
Split out PHY detection into lan78xx_get_phy() and MAC-side setup into
lan78xx_mac_prepare_for_phy(), making the main lan78xx_phy_init() cleaner
and easier to follow.
This improves separation of concerns and prepares the code for a future
transition to phylink. Fixed PHY registration and interface selection
are now handled in lan78xx_get_phy(), while MAC-side delay configuration
is done in lan78xx_mac_prepare_for_phy().
The fixed PHY fallback is preserved for setups like EVB-KSZ9897-1,
where LAN7801 connects directly to a KSZ switch without a standard PHY
or device tree support.
No functional changes intended.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v6:
- this patch is added in v6
---
drivers/net/usb/lan78xx.c | 174 ++++++++++++++++++++++++++++----------
1 file changed, 128 insertions(+), 46 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 9c0658227bde..7f1ecc415d53 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2508,53 +2508,145 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev)
dev->domain_data.irqdomain = NULL;
}
-static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev)
+/**
+ * lan78xx_register_fixed_phy() - Register a fallback fixed PHY
+ * @dev: LAN78xx device
+ *
+ * Registers a fixed PHY with 1 Gbps full duplex. This is used in special cases
+ * like EVB-KSZ9897-1, where LAN7801 acts as a USB-to-Ethernet interface to a
+ * switch without a visible PHY.
+ *
+ * Return: pointer to the registered fixed PHY, or ERR_PTR() on error.
+ */
+static struct phy_device *lan78xx_register_fixed_phy(struct lan78xx_net *dev)
{
struct fixed_phy_status fphy_status = {
.link = 1,
.speed = SPEED_1000,
.duplex = DUPLEX_FULL,
};
+
+ netdev_info(dev->net,
+ "No PHY found on LAN7801 – registering fixed PHY (e.g. EVB-KSZ9897-1)\n");
+
+ return fixed_phy_register(PHY_POLL, &fphy_status, NULL);
+}
+
+/**
+ * lan78xx_get_phy() - Probe or register PHY device and set interface mode
+ * @dev: LAN78xx device structure
+ *
+ * This function attempts to find a PHY on the MDIO bus. If no PHY is found
+ * and the chip is LAN7801, it registers a fixed PHY as fallback. It also
+ * sets dev->interface based on chip ID and detected PHY type.
+ *
+ * Return: a valid PHY device pointer, or ERR_PTR() on failure.
+ */
+static struct phy_device *lan78xx_get_phy(struct lan78xx_net *dev)
+{
struct phy_device *phydev;
- int ret;
+ /* Attempt to locate a PHY on the MDIO bus */
phydev = phy_find_first(dev->mdiobus);
- if (!phydev) {
- netdev_dbg(dev->net, "PHY Not Found!! Registering Fixed PHY\n");
- phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL);
- if (IS_ERR(phydev)) {
- netdev_err(dev->net, "No PHY/fixed_PHY found\n");
- return ERR_PTR(-ENODEV);
+
+ switch (dev->chipid) {
+ case ID_REV_CHIP_ID_7801_:
+ if (phydev) {
+ /* External RGMII PHY detected */
+ dev->interface = PHY_INTERFACE_MODE_RGMII_ID;
+ phydev->is_internal = false;
+
+ if (!phydev->drv)
+ netdev_warn(dev->net,
+ "PHY driver not found – assuming RGMII delays are on PCB or strapped for the PHY\n");
+
+ return phydev;
}
- netdev_dbg(dev->net, "Registered FIXED PHY\n");
+
dev->interface = PHY_INTERFACE_MODE_RGMII;
+ /* No PHY found – fallback to fixed PHY (e.g. KSZ switch board) */
+ return lan78xx_register_fixed_phy(dev);
+
+ case ID_REV_CHIP_ID_7800_:
+ case ID_REV_CHIP_ID_7850_:
+ if (!phydev)
+ return ERR_PTR(-ENODEV);
+
+ /* These use internal GMII-connected PHY */
+ dev->interface = PHY_INTERFACE_MODE_GMII;
+ phydev->is_internal = true;
+ return phydev;
+
+ default:
+ netdev_err(dev->net, "Unknown CHIP ID: 0x%08x\n", dev->chipid);
+ return ERR_PTR(-ENODEV);
+ }
+}
+
+/**
+ * lan78xx_mac_prepare_for_phy() - Preconfigure MAC-side interface settings
+ * @dev: LAN78xx device
+ *
+ * Configure MAC-side registers according to dev->interface, which should be
+ * set by lan78xx_get_phy().
+ *
+ * - For PHY_INTERFACE_MODE_RGMII:
+ * Enable MAC-side TXC delay. This mode seems to be used in a special setup
+ * without a real PHY, likely on EVB-KSZ9897-1. In that design, LAN7801 is
+ * connected to the KSZ9897 switch, and the link timing is expected to be
+ * hardwired (e.g. via strapping or board layout). No devicetree support is
+ * assumed here.
+ *
+ * - For PHY_INTERFACE_MODE_RGMII_ID:
+ * Disable MAC-side delay and rely on the PHY driver to provide delay.
+ *
+ * - For GMII, no MAC-specific config is needed.
+ *
+ * Return: 0 on success or a negative error code.
+ */
+static int lan78xx_mac_prepare_for_phy(struct lan78xx_net *dev)
+{
+ int ret;
+
+ switch (dev->interface) {
+ case PHY_INTERFACE_MODE_RGMII:
+ /* Enable MAC-side TX clock delay */
ret = lan78xx_write_reg(dev, MAC_RGMII_ID,
MAC_RGMII_ID_TXC_DELAY_EN_);
if (ret < 0)
- return ERR_PTR(ret);
+ return ret;
ret = lan78xx_write_reg(dev, RGMII_TX_BYP_DLL, 0x3D00);
if (ret < 0)
- return ERR_PTR(ret);
+ return ret;
- ret = lan78xx_update_reg(dev, HW_CFG, HW_CFG_CLK125_EN_ |
- HW_CFG_REFCLK25_EN_,
+ ret = lan78xx_update_reg(dev, HW_CFG,
+ HW_CFG_CLK125_EN_ | HW_CFG_REFCLK25_EN_,
HW_CFG_CLK125_EN_ | HW_CFG_REFCLK25_EN_);
if (ret < 0)
- return ERR_PTR(ret);
- } else {
- dev->interface = PHY_INTERFACE_MODE_RGMII_ID;
- /* The PHY driver is responsible to configure proper RGMII
- * interface delays. Disable RGMII delays on MAC side.
- */
+ return ret;
+
+ break;
+
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ /* Disable MAC-side TXC delay, PHY provides it */
ret = lan78xx_write_reg(dev, MAC_RGMII_ID, 0);
if (ret < 0)
- return ERR_PTR(ret);
+ return ret;
- phydev->is_internal = false;
+ break;
+
+ case PHY_INTERFACE_MODE_GMII:
+ /* No MAC-specific configuration required */
+ break;
+
+ default:
+ netdev_warn(dev->net, "Unsupported interface mode: %d\n",
+ dev->interface);
+ break;
}
- return phydev;
+ return 0;
}
static int lan78xx_phy_init(struct lan78xx_net *dev)
@@ -2564,31 +2656,13 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
u32 mii_adv;
struct phy_device *phydev;
- switch (dev->chipid) {
- case ID_REV_CHIP_ID_7801_:
- phydev = lan7801_phy_init(dev);
- if (IS_ERR(phydev)) {
- netdev_err(dev->net, "lan7801: failed to init PHY: %pe\n",
- phydev);
- return PTR_ERR(phydev);
- }
- break;
-
- case ID_REV_CHIP_ID_7800_:
- case ID_REV_CHIP_ID_7850_:
- phydev = phy_find_first(dev->mdiobus);
- if (!phydev) {
- netdev_err(dev->net, "no PHY found\n");
- return -ENODEV;
- }
- phydev->is_internal = true;
- dev->interface = PHY_INTERFACE_MODE_GMII;
- break;
+ phydev = lan78xx_get_phy(dev);
+ if (IS_ERR(phydev))
+ return PTR_ERR(phydev);
- default:
- netdev_err(dev->net, "Unknown CHIP ID found\n");
- return -ENODEV;
- }
+ ret = lan78xx_mac_prepare_for_phy(dev);
+ if (ret < 0)
+ goto free_phy;
/* if phyirq is not set, use polling mode in phylib */
if (dev->domain_data.phyirq > 0)
@@ -2662,6 +2736,14 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
dev->fc_autoneg = phydev->autoneg;
return 0;
+
+free_phy:
+ if (phy_is_pseudo_fixed_link(phydev)) {
+ fixed_phy_unregister(phydev);
+ phy_device_free(phydev);
+ }
+
+ return ret;
}
static int lan78xx_set_rx_max_frame_length(struct lan78xx_net *dev, int size)
--
2.39.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration
2025-05-05 8:43 ` [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration Oleksij Rempel
@ 2025-05-06 5:31 ` Thangaraj.S
2025-05-06 10:13 ` Oleksij Rempel
2025-05-07 3:58 ` Thangaraj.S
1 sibling, 1 reply; 16+ messages in thread
From: Thangaraj.S @ 2025-05-06 5:31 UTC (permalink / raw)
To: andrew+netdev, rmk+kernel, davem, Rengarajan.S, Woojung.Huh,
pabeni, o.rempel, edumazet, kuba
Cc: phil, kernel, horms, linux-kernel, netdev, UNGLinuxDriver,
maxime.chevallier
Hi Oleksj.
Thanks for the patch.
On Mon, 2025-05-05 at 10:43 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Split out PHY detection into lan78xx_get_phy() and MAC-side setup
> into
> lan78xx_mac_prepare_for_phy(), making the main lan78xx_phy_init()
> cleaner
> and easier to follow.
>
> This improves separation of concerns and prepares the code for a
> future
> transition to phylink. Fixed PHY registration and interface selection
> are now handled in lan78xx_get_phy(), while MAC-side delay
> configuration
> is done in lan78xx_mac_prepare_for_phy().
>
> The fixed PHY fallback is preserved for setups like EVB-KSZ9897-1,
> where LAN7801 connects directly to a KSZ switch without a standard
> PHY
> or device tree support.
>
> No functional changes intended.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v6:
> - this patch is added in v6
> ---
> drivers/net/usb/lan78xx.c | 174 ++++++++++++++++++++++++++++------
> ----
> 1 file changed, 128 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 9c0658227bde..7f1ecc415d53 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -2508,53 +2508,145 @@ static void lan78xx_remove_irq_domain(struct
> lan78xx_net *dev)
> dev->domain_data.irqdomain = NULL;
> }
>
>
> static int lan78xx_phy_init(struct lan78xx_net *dev)
> @@ -2564,31 +2656,13 @@ static int lan78xx_phy_init(struct
> lan78xx_net *dev)
> u32 mii_adv;
> struct phy_device *phydev;
>
> - switch (dev->chipid) {
> - case ID_REV_CHIP_ID_7801_:
> - phydev = lan7801_phy_init(dev);
> - if (IS_ERR(phydev)) {
> - netdev_err(dev->net, "lan7801: failed to init
> PHY: %pe\n",
> - phydev);
> - return PTR_ERR(phydev);
> - }
> - break;
> -
> - case ID_REV_CHIP_ID_7800_:
> - case ID_REV_CHIP_ID_7850_:
> - phydev = phy_find_first(dev->mdiobus);
> - if (!phydev) {
> - netdev_err(dev->net, "no PHY found\n");
> - return -ENODEV;
> - }
> - phydev->is_internal = true;
> - dev->interface = PHY_INTERFACE_MODE_GMII;
> - break;
> + phydev = lan78xx_get_phy(dev);
> + if (IS_ERR(phydev))
> + return PTR_ERR(phydev);
>
> - default:
> - netdev_err(dev->net, "Unknown CHIP ID found\n");
> - return -ENODEV;
> - }
> + ret = lan78xx_mac_prepare_for_phy(dev);
> + if (ret < 0)
> + goto free_phy;
>
> /* if phyirq is not set, use polling mode in phylib */
> if (dev->domain_data.phyirq > 0)
> @@ -2662,6 +2736,14 @@ static int lan78xx_phy_init(struct lan78xx_net
> *dev)
> dev->fc_autoneg = phydev->autoneg;
>
> return 0;
> +
> +free_phy:
> + if (phy_is_pseudo_fixed_link(phydev)) {
> + fixed_phy_unregister(phydev);
> + phy_device_free(phydev);
> + }
> +
> + return ret;
> }
Could see as per implementation, this case might hit on normal phy
other than fixed-phy too. Should we not add any cleanup for phydev
here?
Thanks,
Thangaraj Samynathan
>
> static int lan78xx_set_rx_max_frame_length(struct lan78xx_net *dev,
> int size)
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration
2025-05-06 5:31 ` Thangaraj.S
@ 2025-05-06 10:13 ` Oleksij Rempel
0 siblings, 0 replies; 16+ messages in thread
From: Oleksij Rempel @ 2025-05-06 10:13 UTC (permalink / raw)
To: Thangaraj.S
Cc: andrew+netdev, rmk+kernel, davem, Rengarajan.S, Woojung.Huh,
pabeni, edumazet, kuba, phil, kernel, horms, linux-kernel,
netdev, UNGLinuxDriver, maxime.chevallier
Hi Thangaraj,
Thanks for the review!
On Tue, May 06, 2025 at 05:31:30AM +0000, Thangaraj.S@microchip.com wrote:
> Hi Oleksj.
> Thanks for the patch.
>
> On Mon, 2025-05-05 at 10:43 +0200, Oleksij Rempel wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > Split out PHY detection into lan78xx_get_phy() and MAC-side setup
> > into
> > lan78xx_mac_prepare_for_phy(), making the main lan78xx_phy_init()
> > cleaner
> > and easier to follow.
> >
> > This improves separation of concerns and prepares the code for a
> > future
> > transition to phylink. Fixed PHY registration and interface selection
> > are now handled in lan78xx_get_phy(), while MAC-side delay
> > configuration
> > is done in lan78xx_mac_prepare_for_phy().
> >
> > The fixed PHY fallback is preserved for setups like EVB-KSZ9897-1,
> > where LAN7801 connects directly to a KSZ switch without a standard
> > PHY
> > or device tree support.
> >
> > No functional changes intended.
> >
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> > changes v6:
> > - this patch is added in v6
> > ---
> > drivers/net/usb/lan78xx.c | 174 ++++++++++++++++++++++++++++------
> > ----
> > 1 file changed, 128 insertions(+), 46 deletions(-)
> >
> > diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> > index 9c0658227bde..7f1ecc415d53 100644
> > --- a/drivers/net/usb/lan78xx.c
> > +++ b/drivers/net/usb/lan78xx.c
> > @@ -2508,53 +2508,145 @@ static void lan78xx_remove_irq_domain(struct
> > lan78xx_net *dev)
> > dev->domain_data.irqdomain = NULL;
> > }
> >
> >
> > static int lan78xx_phy_init(struct lan78xx_net *dev)
> > @@ -2564,31 +2656,13 @@ static int lan78xx_phy_init(struct
> > lan78xx_net *dev)
> > u32 mii_adv;
> > struct phy_device *phydev;
> >
> > - switch (dev->chipid) {
> > - case ID_REV_CHIP_ID_7801_:
> > - phydev = lan7801_phy_init(dev);
> > - if (IS_ERR(phydev)) {
> > - netdev_err(dev->net, "lan7801: failed to init
> > PHY: %pe\n",
> > - phydev);
> > - return PTR_ERR(phydev);
> > - }
> > - break;
> > -
> > - case ID_REV_CHIP_ID_7800_:
> > - case ID_REV_CHIP_ID_7850_:
> > - phydev = phy_find_first(dev->mdiobus);
> > - if (!phydev) {
> > - netdev_err(dev->net, "no PHY found\n");
> > - return -ENODEV;
> > - }
> > - phydev->is_internal = true;
> > - dev->interface = PHY_INTERFACE_MODE_GMII;
> > - break;
> > + phydev = lan78xx_get_phy(dev);
> > + if (IS_ERR(phydev))
> > + return PTR_ERR(phydev);
> >
> > - default:
> > - netdev_err(dev->net, "Unknown CHIP ID found\n");
> > - return -ENODEV;
> > - }
> > + ret = lan78xx_mac_prepare_for_phy(dev);
> > + if (ret < 0)
> > + goto free_phy;
> >
> > /* if phyirq is not set, use polling mode in phylib */
> > if (dev->domain_data.phyirq > 0)
> > @@ -2662,6 +2736,14 @@ static int lan78xx_phy_init(struct lan78xx_net
> > *dev)
> > dev->fc_autoneg = phydev->autoneg;
> >
> > return 0;
> > +
> > +free_phy:
> > + if (phy_is_pseudo_fixed_link(phydev)) {
> > + fixed_phy_unregister(phydev);
> > + phy_device_free(phydev);
> > + }
> > +
> > + return ret;
> > }
>
> Could see as per implementation, this case might hit on normal phy
> other than fixed-phy too. Should we not add any cleanup for phydev
> here?
You're right to ask — but in this case, we don't need to clean up
non-fixed PHYs, since we only probe them using phy_find_first(), and do
not allocate, register, or attach them in lan78xx_get_phy(). So no extra
cleanup is needed in the error path for those cases.
If we ever call phy_connect_direct() earlier in the flow, we would need
to add a corresponding phy_disconnect(), but that's not the case here
yet.
Best regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration
2025-05-05 8:43 ` [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration Oleksij Rempel
2025-05-06 5:31 ` Thangaraj.S
@ 2025-05-07 3:58 ` Thangaraj.S
1 sibling, 0 replies; 16+ messages in thread
From: Thangaraj.S @ 2025-05-07 3:58 UTC (permalink / raw)
To: andrew+netdev, rmk+kernel, davem, Rengarajan.S, Woojung.Huh,
pabeni, o.rempel, edumazet, kuba
Cc: phil, kernel, horms, linux-kernel, netdev, UNGLinuxDriver,
maxime.chevallier
On Mon, 2025-05-05 at 10:43 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Split out PHY detection into lan78xx_get_phy() and MAC-side setup
> into
> lan78xx_mac_prepare_for_phy(), making the main lan78xx_phy_init()
> cleaner
> and easier to follow.
>
> This improves separation of concerns and prepares the code for a
> future
> transition to phylink. Fixed PHY registration and interface selection
> are now handled in lan78xx_get_phy(), while MAC-side delay
> configuration
> is done in lan78xx_mac_prepare_for_phy().
>
> The fixed PHY fallback is preserved for setups like EVB-KSZ9897-1,
> where LAN7801 connects directly to a KSZ switch without a standard
> PHY
> or device tree support.
>
> No functional changes intended.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
Reviewed-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v8 4/7] net: usb: lan78xx: move LED DT configuration to helper
2025-05-05 8:43 [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion Oleksij Rempel
` (2 preceding siblings ...)
2025-05-05 8:43 ` [PATCH net-next v8 3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration Oleksij Rempel
@ 2025-05-05 8:43 ` Oleksij Rempel
2025-05-07 3:59 ` Thangaraj.S
2025-05-05 8:43 ` [PATCH net-next v8 5/7] net: usb: lan78xx: Extract PHY interrupt acknowledgment " Oleksij Rempel
` (3 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Oleksij Rempel @ 2025-05-05 8:43 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Woojung Huh, Andrew Lunn, Russell King, Thangaraj Samynathan,
Rengarajan Sundararajan
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
Phil Elwell, Maxime Chevallier, Simon Horman
Extract the LED enable logic based on the "microchip,led-modes"
property into a new helper function lan78xx_configure_leds_from_dt().
This simplifies lan78xx_phy_init() and improves modularity.
No functional changes intended.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v6:
- this patch is added in v6
---
drivers/net/usb/lan78xx.c | 72 +++++++++++++++++++++++++--------------
1 file changed, 46 insertions(+), 26 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 7f1ecc415d53..07530eef82cb 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2649,6 +2649,49 @@ static int lan78xx_mac_prepare_for_phy(struct lan78xx_net *dev)
return 0;
}
+/**
+ * lan78xx_configure_leds_from_dt() - Configure LED enables based on DT
+ * @dev: LAN78xx device
+ * @phydev: PHY device (must be valid)
+ *
+ * Reads "microchip,led-modes" property from the PHY's DT node and enables
+ * the corresponding number of LEDs by writing to HW_CFG.
+ *
+ * This helper preserves the original logic, enabling up to 4 LEDs.
+ * If the property is not present, this function does nothing.
+ *
+ * Return: 0 on success or a negative error code.
+ */
+static int lan78xx_configure_leds_from_dt(struct lan78xx_net *dev,
+ struct phy_device *phydev)
+{
+ struct device_node *np = phydev->mdio.dev.of_node;
+ u32 reg;
+ int len, ret;
+
+ if (!np)
+ return 0;
+
+ len = of_property_count_elems_of_size(np, "microchip,led-modes",
+ sizeof(u32));
+ if (len < 0)
+ return 0;
+
+ ret = lan78xx_read_reg(dev, HW_CFG, ®);
+ if (ret < 0)
+ return ret;
+
+ reg &= ~(HW_CFG_LED0_EN_ | HW_CFG_LED1_EN_ |
+ HW_CFG_LED2_EN_ | HW_CFG_LED3_EN_);
+
+ reg |= (len > 0) * HW_CFG_LED0_EN_ |
+ (len > 1) * HW_CFG_LED1_EN_ |
+ (len > 2) * HW_CFG_LED2_EN_ |
+ (len > 3) * HW_CFG_LED3_EN_;
+
+ return lan78xx_write_reg(dev, HW_CFG, reg);
+}
+
static int lan78xx_phy_init(struct lan78xx_net *dev)
{
__ETHTOOL_DECLARE_LINK_MODE_MASK(fc) = { 0, };
@@ -2704,32 +2747,9 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
phy_support_eee(phydev);
- if (phydev->mdio.dev.of_node) {
- u32 reg;
- int len;
-
- len = of_property_count_elems_of_size(phydev->mdio.dev.of_node,
- "microchip,led-modes",
- sizeof(u32));
- if (len >= 0) {
- /* Ensure the appropriate LEDs are enabled */
- ret = lan78xx_read_reg(dev, HW_CFG, ®);
- if (ret < 0)
- return ret;
-
- reg &= ~(HW_CFG_LED0_EN_ |
- HW_CFG_LED1_EN_ |
- HW_CFG_LED2_EN_ |
- HW_CFG_LED3_EN_);
- reg |= (len > 0) * HW_CFG_LED0_EN_ |
- (len > 1) * HW_CFG_LED1_EN_ |
- (len > 2) * HW_CFG_LED2_EN_ |
- (len > 3) * HW_CFG_LED3_EN_;
- ret = lan78xx_write_reg(dev, HW_CFG, reg);
- if (ret < 0)
- return ret;
- }
- }
+ ret = lan78xx_configure_leds_from_dt(dev, phydev);
+ if (ret)
+ goto free_phy;
genphy_config_aneg(phydev);
--
2.39.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v8 4/7] net: usb: lan78xx: move LED DT configuration to helper
2025-05-05 8:43 ` [PATCH net-next v8 4/7] net: usb: lan78xx: move LED DT configuration to helper Oleksij Rempel
@ 2025-05-07 3:59 ` Thangaraj.S
0 siblings, 0 replies; 16+ messages in thread
From: Thangaraj.S @ 2025-05-07 3:59 UTC (permalink / raw)
To: andrew+netdev, rmk+kernel, davem, Rengarajan.S, Woojung.Huh,
pabeni, o.rempel, edumazet, kuba
Cc: phil, kernel, horms, linux-kernel, netdev, UNGLinuxDriver,
maxime.chevallier
On Mon, 2025-05-05 at 10:43 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Extract the LED enable logic based on the "microchip,led-modes"
> property into a new helper function lan78xx_configure_leds_from_dt().
>
> This simplifies lan78xx_phy_init() and improves modularity.
> No functional changes intended.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
Reviewed-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v8 5/7] net: usb: lan78xx: Extract PHY interrupt acknowledgment to helper
2025-05-05 8:43 [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion Oleksij Rempel
` (3 preceding siblings ...)
2025-05-05 8:43 ` [PATCH net-next v8 4/7] net: usb: lan78xx: move LED DT configuration to helper Oleksij Rempel
@ 2025-05-05 8:43 ` Oleksij Rempel
2025-05-07 3:59 ` Thangaraj.S
2025-05-05 8:43 ` [PATCH net-next v8 6/7] net: usb: lan78xx: Refactor USB link power configuration into helper Oleksij Rempel
` (2 subsequent siblings)
7 siblings, 1 reply; 16+ messages in thread
From: Oleksij Rempel @ 2025-05-05 8:43 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Woojung Huh, Andrew Lunn, Russell King, Thangaraj Samynathan,
Rengarajan Sundararajan
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
Phil Elwell, Maxime Chevallier, Simon Horman
Move the PHY interrupt acknowledgment logic from lan78xx_link_reset()
to a new helper function lan78xx_phy_int_ack(). This simplifies the
code and prepares for reusing the acknowledgment logic independently
from the full link reset process, such as when using phylink.
No functional change intended.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v6:
- this patch is added in v6
---
drivers/net/usb/lan78xx.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 07530eef82cb..de2b429e906e 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1636,6 +1636,20 @@ static int lan78xx_mac_reset(struct lan78xx_net *dev)
return ret;
}
+/**
+ * lan78xx_phy_int_ack - Acknowledge PHY interrupt
+ * @dev: pointer to the LAN78xx device structure
+ *
+ * This function acknowledges the PHY interrupt by setting the
+ * INT_STS_PHY_INT_ bit in the interrupt status register (INT_STS).
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+static int lan78xx_phy_int_ack(struct lan78xx_net *dev)
+{
+ return lan78xx_write_reg(dev, INT_STS, INT_STS_PHY_INT_);
+}
+
static int lan78xx_link_reset(struct lan78xx_net *dev)
{
struct phy_device *phydev = dev->net->phydev;
@@ -1644,7 +1658,7 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
u32 buf;
/* clear LAN78xx interrupt status */
- ret = lan78xx_write_reg(dev, INT_STS, INT_STS_PHY_INT_);
+ ret = lan78xx_phy_int_ack(dev);
if (unlikely(ret < 0))
return ret;
--
2.39.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v8 5/7] net: usb: lan78xx: Extract PHY interrupt acknowledgment to helper
2025-05-05 8:43 ` [PATCH net-next v8 5/7] net: usb: lan78xx: Extract PHY interrupt acknowledgment " Oleksij Rempel
@ 2025-05-07 3:59 ` Thangaraj.S
0 siblings, 0 replies; 16+ messages in thread
From: Thangaraj.S @ 2025-05-07 3:59 UTC (permalink / raw)
To: andrew+netdev, rmk+kernel, davem, Rengarajan.S, Woojung.Huh,
pabeni, o.rempel, edumazet, kuba
Cc: phil, kernel, horms, linux-kernel, netdev, UNGLinuxDriver,
maxime.chevallier
On Mon, 2025-05-05 at 10:43 +0200, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> Move the PHY interrupt acknowledgment logic from lan78xx_link_reset()
> to a new helper function lan78xx_phy_int_ack(). This simplifies the
> code and prepares for reusing the acknowledgment logic independently
> from the full link reset process, such as when using phylink.
>
> No functional change intended.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
>
Reviewed-by: Thangaraj Samynathan <thangaraj.s@microchip.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH net-next v8 6/7] net: usb: lan78xx: Refactor USB link power configuration into helper
2025-05-05 8:43 [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion Oleksij Rempel
` (4 preceding siblings ...)
2025-05-05 8:43 ` [PATCH net-next v8 5/7] net: usb: lan78xx: Extract PHY interrupt acknowledgment " Oleksij Rempel
@ 2025-05-05 8:43 ` Oleksij Rempel
2025-05-05 8:43 ` [PATCH net-next v8 7/7] net: usb: lan78xx: Extract flow control configuration to helper Oleksij Rempel
2025-05-07 12:00 ` [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion patchwork-bot+netdevbpf
7 siblings, 0 replies; 16+ messages in thread
From: Oleksij Rempel @ 2025-05-05 8:43 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Woojung Huh, Andrew Lunn, Russell King, Thangaraj Samynathan,
Rengarajan Sundararajan
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
Phil Elwell, Maxime Chevallier, Simon Horman
Move the USB link power configuration logic from lan78xx_link_reset()
to a new helper function lan78xx_configure_usb(). This simplifies the
main link reset path and isolates USB-specific logic.
The new function handles U1/U2 enablement based on Ethernet link speed,
but only for SuperSpeed-capable devices (LAN7800 and LAN7801). LAN7850,
a High-Speed-only device, is explicitly excluded. A warning is logged
if SuperSpeed is reported unexpectedly for LAN7850.
Add a forward declaration for lan78xx_configure_usb() as preparation for
the upcoming phylink conversion, where it will also be used from the
mac_link_up() callback.
Open questions remain:
- Why is the 1000 Mbps configuration split into two steps (U2 disable,
then U1 enable), unlike the single-step config used for 10/100 Mbps?
- U1/U2 behavior appears to depend on proper EEPROM configuration.
There are known devices in the field without EEPROM. Should the driver
enforce safe defaults in such cases?
Due to lack of USB subsystem expertise, no changes were made to this logic
beyond structural refactoring.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v6:
- this patch is added in v6
---
drivers/net/usb/lan78xx.c | 90 +++++++++++++++++++++++++--------------
1 file changed, 59 insertions(+), 31 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index de2b429e906e..bff53324c70a 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1650,12 +1650,13 @@ static int lan78xx_phy_int_ack(struct lan78xx_net *dev)
return lan78xx_write_reg(dev, INT_STS, INT_STS_PHY_INT_);
}
+static int lan78xx_configure_usb(struct lan78xx_net *dev, int speed);
+
static int lan78xx_link_reset(struct lan78xx_net *dev)
{
struct phy_device *phydev = dev->net->phydev;
struct ethtool_link_ksettings ecmd;
int ladv, radv, ret, link;
- u32 buf;
/* clear LAN78xx interrupt status */
ret = lan78xx_phy_int_ack(dev);
@@ -1681,36 +1682,9 @@ static int lan78xx_link_reset(struct lan78xx_net *dev)
phy_ethtool_ksettings_get(phydev, &ecmd);
- if (dev->udev->speed == USB_SPEED_SUPER) {
- if (ecmd.base.speed == 1000) {
- /* disable U2 */
- ret = lan78xx_read_reg(dev, USB_CFG1, &buf);
- if (ret < 0)
- return ret;
- buf &= ~USB_CFG1_DEV_U2_INIT_EN_;
- ret = lan78xx_write_reg(dev, USB_CFG1, buf);
- if (ret < 0)
- return ret;
- /* enable U1 */
- ret = lan78xx_read_reg(dev, USB_CFG1, &buf);
- if (ret < 0)
- return ret;
- buf |= USB_CFG1_DEV_U1_INIT_EN_;
- ret = lan78xx_write_reg(dev, USB_CFG1, buf);
- if (ret < 0)
- return ret;
- } else {
- /* enable U1 & U2 */
- ret = lan78xx_read_reg(dev, USB_CFG1, &buf);
- if (ret < 0)
- return ret;
- buf |= USB_CFG1_DEV_U2_INIT_EN_;
- buf |= USB_CFG1_DEV_U1_INIT_EN_;
- ret = lan78xx_write_reg(dev, USB_CFG1, buf);
- if (ret < 0)
- return ret;
- }
- }
+ ret = lan78xx_configure_usb(dev, ecmd.base.speed);
+ if (ret < 0)
+ return ret;
ladv = phy_read(phydev, MII_ADVERTISE);
if (ladv < 0)
@@ -2522,6 +2496,60 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev)
dev->domain_data.irqdomain = NULL;
}
+/**
+ * lan78xx_configure_usb - Configure USB link power settings
+ * @dev: pointer to the LAN78xx device structure
+ * @speed: negotiated Ethernet link speed (in Mbps)
+ *
+ * This function configures U1/U2 link power management for SuperSpeed
+ * USB devices based on the current Ethernet link speed. It uses the
+ * USB_CFG1 register to enable or disable U1 and U2 low-power states.
+ *
+ * Note: Only LAN7800 and LAN7801 support SuperSpeed (USB 3.x).
+ * LAN7850 is a High-Speed-only (USB 2.0) device and is skipped.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+static int lan78xx_configure_usb(struct lan78xx_net *dev, int speed)
+{
+ u32 mask, val;
+ int ret;
+
+ /* Only configure USB settings for SuperSpeed devices */
+ if (dev->udev->speed != USB_SPEED_SUPER)
+ return 0;
+
+ /* LAN7850 does not support USB 3.x */
+ if (dev->chipid == ID_REV_CHIP_ID_7850_) {
+ netdev_warn_once(dev->net, "Unexpected SuperSpeed for LAN7850 (USB 2.0 only)\n");
+ return 0;
+ }
+
+ switch (speed) {
+ case SPEED_1000:
+ /* Disable U2, enable U1 */
+ ret = lan78xx_update_reg(dev, USB_CFG1,
+ USB_CFG1_DEV_U2_INIT_EN_, 0);
+ if (ret < 0)
+ return ret;
+
+ return lan78xx_update_reg(dev, USB_CFG1,
+ USB_CFG1_DEV_U1_INIT_EN_,
+ USB_CFG1_DEV_U1_INIT_EN_);
+
+ case SPEED_100:
+ case SPEED_10:
+ /* Enable both U1 and U2 */
+ mask = USB_CFG1_DEV_U1_INIT_EN_ | USB_CFG1_DEV_U2_INIT_EN_;
+ val = mask;
+ return lan78xx_update_reg(dev, USB_CFG1, mask, val);
+
+ default:
+ netdev_warn(dev->net, "Unsupported link speed: %d\n", speed);
+ return -EINVAL;
+ }
+}
+
/**
* lan78xx_register_fixed_phy() - Register a fallback fixed PHY
* @dev: LAN78xx device
--
2.39.5
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH net-next v8 7/7] net: usb: lan78xx: Extract flow control configuration to helper
2025-05-05 8:43 [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion Oleksij Rempel
` (5 preceding siblings ...)
2025-05-05 8:43 ` [PATCH net-next v8 6/7] net: usb: lan78xx: Refactor USB link power configuration into helper Oleksij Rempel
@ 2025-05-05 8:43 ` Oleksij Rempel
2025-05-07 12:00 ` [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion patchwork-bot+netdevbpf
7 siblings, 0 replies; 16+ messages in thread
From: Oleksij Rempel @ 2025-05-05 8:43 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Woojung Huh, Andrew Lunn, Russell King, Thangaraj Samynathan,
Rengarajan Sundararajan
Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
Phil Elwell, Maxime Chevallier, Simon Horman
Move flow control register configuration from
lan78xx_update_flowcontrol() into a new helper function
lan78xx_configure_flowcontrol(). This separates hardware-specific
programming from policy logic and simplifies the upcoming phylink
integration.
The values used in this initial version of
lan78xx_configure_flowcontrol() are taken over as-is from the original
implementation to avoid functional changes. While they may not be
optimal for all USB and link speed combinations, they are known to work
reliably. Optimization of pause time and thresholds based on runtime
conditions can be done in a separate follow-up patch.
The forward declaration of lan78xx_configure_flowcontrol() will also be
removed later during the phylink conversion.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v6:
- this patch is added in v6
---
drivers/net/usb/lan78xx.c | 105 +++++++++++++++++++++++++++++++-------
1 file changed, 87 insertions(+), 18 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index bff53324c70a..58e3589e3b89 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1554,10 +1554,12 @@ static void lan78xx_set_multicast(struct net_device *netdev)
schedule_work(&pdata->set_multicast);
}
+static int lan78xx_configure_flowcontrol(struct lan78xx_net *dev,
+ bool tx_pause, bool rx_pause);
+
static int lan78xx_update_flowcontrol(struct lan78xx_net *dev, u8 duplex,
u16 lcladv, u16 rmtadv)
{
- u32 flow = 0, fct_flow = 0;
u8 cap;
if (dev->fc_autoneg)
@@ -1565,27 +1567,13 @@ static int lan78xx_update_flowcontrol(struct lan78xx_net *dev, u8 duplex,
else
cap = dev->fc_request_control;
- if (cap & FLOW_CTRL_TX)
- flow |= (FLOW_CR_TX_FCEN_ | 0xFFFF);
-
- if (cap & FLOW_CTRL_RX)
- flow |= FLOW_CR_RX_FCEN_;
-
- if (dev->udev->speed == USB_SPEED_SUPER)
- fct_flow = FLOW_CTRL_THRESHOLD(FLOW_ON_SS, FLOW_OFF_SS);
- else if (dev->udev->speed == USB_SPEED_HIGH)
- fct_flow = FLOW_CTRL_THRESHOLD(FLOW_ON_HS, FLOW_OFF_HS);
-
netif_dbg(dev, link, dev->net, "rx pause %s, tx pause %s",
(cap & FLOW_CTRL_RX ? "enabled" : "disabled"),
(cap & FLOW_CTRL_TX ? "enabled" : "disabled"));
- lan78xx_write_reg(dev, FCT_FLOW, fct_flow);
-
- /* threshold value should be set before enabling flow */
- lan78xx_write_reg(dev, FLOW, flow);
-
- return 0;
+ return lan78xx_configure_flowcontrol(dev,
+ cap & FLOW_CTRL_TX,
+ cap & FLOW_CTRL_RX);
}
static void lan78xx_rx_urb_submit_all(struct lan78xx_net *dev);
@@ -2550,6 +2538,87 @@ static int lan78xx_configure_usb(struct lan78xx_net *dev, int speed)
}
}
+/**
+ * lan78xx_configure_flowcontrol - Set MAC and FIFO flow control configuration
+ * @dev: pointer to the LAN78xx device structure
+ * @tx_pause: enable transmission of pause frames
+ * @rx_pause: enable reception of pause frames
+ *
+ * This function configures the LAN78xx flow control settings by writing
+ * to the FLOW and FCT_FLOW registers. The pause time is set to the
+ * maximum allowed value (65535 quanta). FIFO thresholds are selected
+ * based on USB speed.
+ *
+ * The Pause Time field is measured in units of 512-bit times (quanta):
+ * - At 1 Gbps: 1 quanta = 512 ns → max ~33.6 ms pause
+ * - At 100 Mbps: 1 quanta = 5.12 µs → max ~335 ms pause
+ * - At 10 Mbps: 1 quanta = 51.2 µs → max ~3.3 s pause
+ *
+ * Flow control thresholds (FCT_FLOW) are used to trigger pause/resume:
+ * - RXUSED is the number of bytes used in the RX FIFO
+ * - Flow is turned ON when RXUSED ≥ FLOW_ON threshold
+ * - Flow is turned OFF when RXUSED ≤ FLOW_OFF threshold
+ * - Both thresholds are encoded in units of 512 bytes (rounded up)
+ *
+ * Thresholds differ by USB speed because available USB bandwidth
+ * affects how fast packets can be drained from the RX FIFO:
+ * - USB 3.x (SuperSpeed):
+ * FLOW_ON = 9216 bytes → 18 units
+ * FLOW_OFF = 4096 bytes → 8 units
+ * - USB 2.0 (High-Speed):
+ * FLOW_ON = 8704 bytes → 17 units
+ * FLOW_OFF = 1024 bytes → 2 units
+ *
+ * Note: The FCT_FLOW register must be configured before enabling TX pause
+ * (i.e., before setting FLOW_CR_TX_FCEN_), as required by the hardware.
+ *
+ * Return: 0 on success or a negative error code on failure.
+ */
+static int lan78xx_configure_flowcontrol(struct lan78xx_net *dev,
+ bool tx_pause, bool rx_pause)
+{
+ /* Use maximum pause time: 65535 quanta (512-bit times) */
+ const u32 pause_time_quanta = 65535;
+ u32 fct_flow = 0;
+ u32 flow = 0;
+ int ret;
+
+ /* Prepare MAC flow control bits */
+ if (tx_pause)
+ flow |= FLOW_CR_TX_FCEN_ | pause_time_quanta;
+
+ if (rx_pause)
+ flow |= FLOW_CR_RX_FCEN_;
+
+ /* Select RX FIFO thresholds based on USB speed
+ *
+ * FCT_FLOW layout:
+ * bits [6:0] FLOW_ON threshold (RXUSED ≥ ON → assert pause)
+ * bits [14:8] FLOW_OFF threshold (RXUSED ≤ OFF → deassert pause)
+ * thresholds are expressed in units of 512 bytes
+ */
+ switch (dev->udev->speed) {
+ case USB_SPEED_SUPER:
+ fct_flow = FLOW_CTRL_THRESHOLD(FLOW_ON_SS, FLOW_OFF_SS);
+ break;
+ case USB_SPEED_HIGH:
+ fct_flow = FLOW_CTRL_THRESHOLD(FLOW_ON_HS, FLOW_OFF_HS);
+ break;
+ default:
+ netdev_warn(dev->net, "Unsupported USB speed: %d\n",
+ dev->udev->speed);
+ return -EINVAL;
+ }
+
+ /* Step 1: Write FIFO thresholds before enabling pause frames */
+ ret = lan78xx_write_reg(dev, FCT_FLOW, fct_flow);
+ if (ret < 0)
+ return ret;
+
+ /* Step 2: Enable MAC pause functionality */
+ return lan78xx_write_reg(dev, FLOW, flow);
+}
+
/**
* lan78xx_register_fixed_phy() - Register a fallback fixed PHY
* @dev: LAN78xx device
--
2.39.5
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion
2025-05-05 8:43 [PATCH net-next v8 0/7] lan78xx: preparation for PHYLINK conversion Oleksij Rempel
` (6 preceding siblings ...)
2025-05-05 8:43 ` [PATCH net-next v8 7/7] net: usb: lan78xx: Extract flow control configuration to helper Oleksij Rempel
@ 2025-05-07 12:00 ` patchwork-bot+netdevbpf
7 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-05-07 12:00 UTC (permalink / raw)
To: Oleksij Rempel
Cc: davem, edumazet, kuba, pabeni, woojung.huh, andrew+netdev,
rmk+kernel, Thangaraj.S, Rengarajan.S, kernel, linux-kernel,
netdev, UNGLinuxDriver, phil, maxime.chevallier, horms
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:
On Mon, 5 May 2025 10:43:34 +0200 you wrote:
> This patch series contains the first part of the LAN78xx driver
> refactoring in preparation for converting the driver to use the PHYLINK
> framework.
>
> The goal of this initial part is to reduce the size and complexity of
> the final PHYLINK conversion by introducing incremental cleanups and
> logical separation of concerns, such as:
>
> [...]
Here is the summary with links:
- [net-next,v8,1/7] net: usb: lan78xx: Improve error handling in PHY initialization
https://git.kernel.org/netdev/net-next/c/232aa459aa40
- [net-next,v8,2/7] net: usb: lan78xx: remove explicit check for missing PHY driver
https://git.kernel.org/netdev/net-next/c/3da0ae52705d
- [net-next,v8,3/7] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration
https://git.kernel.org/netdev/net-next/c/d39f339d2603
- [net-next,v8,4/7] net: usb: lan78xx: move LED DT configuration to helper
https://git.kernel.org/netdev/net-next/c/8ba1f33c55d2
- [net-next,v8,5/7] net: usb: lan78xx: Extract PHY interrupt acknowledgment to helper
https://git.kernel.org/netdev/net-next/c/f485849a381f
- [net-next,v8,6/7] net: usb: lan78xx: Refactor USB link power configuration into helper
https://git.kernel.org/netdev/net-next/c/d746e0740b28
- [net-next,v8,7/7] net: usb: lan78xx: Extract flow control configuration to helper
https://git.kernel.org/netdev/net-next/c/ef6a29e86785
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 16+ messages in thread