netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink
@ 2024-12-04  8:41 Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 01/10] net: usb: lan78xx: Remove LAN8835 PHY fixup Oleksij Rempel
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, kernel, linux-kernel, netdev, UNGLinuxDriver,
	Phil Elwell

changes v2:
- split the patch set.

This patch set is part of the preparatory work for migrating the lan78xx
USB Ethernet driver to the PHYlink framework. During extensive testing,
I observed that resetting the USB adapter can lead to various read/write
errors. While the errors themselves are acceptable, they generate
excessive log messages, resulting in significant log spam. This set
improves error handling to reduce logging noise by addressing errors
directly and returning early when necessary.

Key highlights of this series include:
- Enhanced error handling to reduce log spam while preserving the
  original error values, avoiding unnecessary overwrites.
- Improved error reporting using the `%pe` specifier for better clarity
  in log messages.
- Removal of redundant and problematic PHY fixups for LAN8835 and
  KSZ9031, with detailed explanations in the respective patches.
- Cleanup of code structure, including unified `goto` labels for better
  readability and maintainability, even in simple editors.

Oleksij Rempel (10):
  net: usb: lan78xx: Remove LAN8835 PHY fixup
  net: usb: lan78xx: Remove KSZ9031 PHY fixup
  net: usb: lan78xx: move functions to avoid forward definitions
  net: usb: lan78xx: Improve error reporting with %pe specifier
  net: usb: lan78xx: Fix error handling in MII read/write functions
  net: usb: lan78xx: Improve error handling in EEPROM and OTP operations
  net: usb: lan78xx: Add error handling to lan78xx_init_ltm
  net: usb: lan78xx: Add error handling to set_rx_max_frame_length and
    set_mtu
  net: usb: lan78xx: Add error handling to lan78xx_irq_bus_sync_unlock
  net: usb: lan78xx: Improve error handling in dataport and multicast
    writes

 drivers/net/usb/lan78xx.c | 785 +++++++++++++++++++++-----------------
 1 file changed, 433 insertions(+), 352 deletions(-)

--
2.39.5


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 01/10] net: usb: lan78xx: Remove LAN8835 PHY fixup
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 02/10] net: usb: lan78xx: Remove KSZ9031 " Oleksij Rempel
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

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>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 35 -----------------------------------
 1 file changed, 35 deletions(-)

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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 02/10] net: usb: lan78xx: Remove KSZ9031 PHY fixup
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 01/10] net: usb: lan78xx: Remove LAN8835 PHY fixup Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 03/10] net: usb: lan78xx: move functions to avoid forward definitions Oleksij Rempel
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

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>
---
 drivers/net/usb/lan78xx.c | 38 +++++---------------------------------
 1 file changed, 5 insertions(+), 33 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 6e468e77d796..918b88bd9524 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -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);
@@ -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;
@@ -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;
 	}
@@ -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;
@@ -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)) {
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 03/10] net: usb: lan78xx: move functions to avoid forward definitions
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 01/10] net: usb: lan78xx: Remove LAN8835 PHY fixup Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 02/10] net: usb: lan78xx: Remove KSZ9031 " Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 04/10] net: usb: lan78xx: Improve error reporting with %pe specifier Oleksij Rempel
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

Move following functions to avoid forward declarations in the code:
- lan78xx_start_hw()
- lan78xx_stop_hw()
- lan78xx_flush_fifo()
- lan78xx_start_tx_path()
- lan78xx_stop_tx_path()
- lan78xx_flush_tx_fifo()
- lan78xx_start_rx_path()
- lan78xx_stop_rx_path()
- lan78xx_flush_rx_fifo()

These functions will be used in an upcoming PHYlink migration patch.

No modifications to the functionality of the code are made.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 300 +++++++++++++++++++-------------------
 1 file changed, 150 insertions(+), 150 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 918b88bd9524..dd9b5d3abcb3 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -808,6 +808,156 @@ static void lan78xx_update_stats(struct lan78xx_net *dev)
 	usb_autopm_put_interface(dev->intf);
 }
 
+static int lan78xx_start_hw(struct lan78xx_net *dev, u32 reg, u32 hw_enable)
+{
+	return lan78xx_update_reg(dev, reg, hw_enable, hw_enable);
+}
+
+static int lan78xx_stop_hw(struct lan78xx_net *dev, u32 reg, u32 hw_enabled,
+			   u32 hw_disabled)
+{
+	unsigned long timeout;
+	bool stopped = true;
+	int ret;
+	u32 buf;
+
+	/* Stop the h/w block (if not already stopped) */
+
+	ret = lan78xx_read_reg(dev, reg, &buf);
+	if (ret < 0)
+		return ret;
+
+	if (buf & hw_enabled) {
+		buf &= ~hw_enabled;
+
+		ret = lan78xx_write_reg(dev, reg, buf);
+		if (ret < 0)
+			return ret;
+
+		stopped = false;
+		timeout = jiffies + HW_DISABLE_TIMEOUT;
+		do  {
+			ret = lan78xx_read_reg(dev, reg, &buf);
+			if (ret < 0)
+				return ret;
+
+			if (buf & hw_disabled)
+				stopped = true;
+			else
+				msleep(HW_DISABLE_DELAY_MS);
+		} while (!stopped && !time_after(jiffies, timeout));
+	}
+
+	ret = stopped ? 0 : -ETIME;
+
+	return ret;
+}
+
+static int lan78xx_flush_fifo(struct lan78xx_net *dev, u32 reg, u32 fifo_flush)
+{
+	return lan78xx_update_reg(dev, reg, fifo_flush, fifo_flush);
+}
+
+static int lan78xx_start_tx_path(struct lan78xx_net *dev)
+{
+	int ret;
+
+	netif_dbg(dev, drv, dev->net, "start tx path");
+
+	/* Start the MAC transmitter */
+
+	ret = lan78xx_start_hw(dev, MAC_TX, MAC_TX_TXEN_);
+	if (ret < 0)
+		return ret;
+
+	/* Start the Tx FIFO */
+
+	ret = lan78xx_start_hw(dev, FCT_TX_CTL, FCT_TX_CTL_EN_);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int lan78xx_stop_tx_path(struct lan78xx_net *dev)
+{
+	int ret;
+
+	netif_dbg(dev, drv, dev->net, "stop tx path");
+
+	/* Stop the Tx FIFO */
+
+	ret = lan78xx_stop_hw(dev, FCT_TX_CTL, FCT_TX_CTL_EN_, FCT_TX_CTL_DIS_);
+	if (ret < 0)
+		return ret;
+
+	/* Stop the MAC transmitter */
+
+	ret = lan78xx_stop_hw(dev, MAC_TX, MAC_TX_TXEN_, MAC_TX_TXD_);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+/* The caller must ensure the Tx path is stopped before calling
+ * lan78xx_flush_tx_fifo().
+ */
+static int lan78xx_flush_tx_fifo(struct lan78xx_net *dev)
+{
+	return lan78xx_flush_fifo(dev, FCT_TX_CTL, FCT_TX_CTL_RST_);
+}
+
+static int lan78xx_start_rx_path(struct lan78xx_net *dev)
+{
+	int ret;
+
+	netif_dbg(dev, drv, dev->net, "start rx path");
+
+	/* Start the Rx FIFO */
+
+	ret = lan78xx_start_hw(dev, FCT_RX_CTL, FCT_RX_CTL_EN_);
+	if (ret < 0)
+		return ret;
+
+	/* Start the MAC receiver*/
+
+	ret = lan78xx_start_hw(dev, MAC_RX, MAC_RX_RXEN_);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int lan78xx_stop_rx_path(struct lan78xx_net *dev)
+{
+	int ret;
+
+	netif_dbg(dev, drv, dev->net, "stop rx path");
+
+	/* Stop the MAC receiver */
+
+	ret = lan78xx_stop_hw(dev, MAC_RX, MAC_RX_RXEN_, MAC_RX_RXD_);
+	if (ret < 0)
+		return ret;
+
+	/* Stop the Rx FIFO */
+
+	ret = lan78xx_stop_hw(dev, FCT_RX_CTL, FCT_RX_CTL_EN_, FCT_RX_CTL_DIS_);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+/* The caller must ensure the Rx path is stopped before calling
+ * lan78xx_flush_rx_fifo().
+ */
+static int lan78xx_flush_rx_fifo(struct lan78xx_net *dev)
+{
+	return lan78xx_flush_fifo(dev, FCT_RX_CTL, FCT_RX_CTL_RST_);
+}
+
 /* Loop until the read is completed with timeout called with phy_mutex held */
 static int lan78xx_phy_wait_not_busy(struct lan78xx_net *dev)
 {
@@ -2662,156 +2812,6 @@ static int lan78xx_urb_config_init(struct lan78xx_net *dev)
 	return result;
 }
 
-static int lan78xx_start_hw(struct lan78xx_net *dev, u32 reg, u32 hw_enable)
-{
-	return lan78xx_update_reg(dev, reg, hw_enable, hw_enable);
-}
-
-static int lan78xx_stop_hw(struct lan78xx_net *dev, u32 reg, u32 hw_enabled,
-			   u32 hw_disabled)
-{
-	unsigned long timeout;
-	bool stopped = true;
-	int ret;
-	u32 buf;
-
-	/* Stop the h/w block (if not already stopped) */
-
-	ret = lan78xx_read_reg(dev, reg, &buf);
-	if (ret < 0)
-		return ret;
-
-	if (buf & hw_enabled) {
-		buf &= ~hw_enabled;
-
-		ret = lan78xx_write_reg(dev, reg, buf);
-		if (ret < 0)
-			return ret;
-
-		stopped = false;
-		timeout = jiffies + HW_DISABLE_TIMEOUT;
-		do  {
-			ret = lan78xx_read_reg(dev, reg, &buf);
-			if (ret < 0)
-				return ret;
-
-			if (buf & hw_disabled)
-				stopped = true;
-			else
-				msleep(HW_DISABLE_DELAY_MS);
-		} while (!stopped && !time_after(jiffies, timeout));
-	}
-
-	ret = stopped ? 0 : -ETIME;
-
-	return ret;
-}
-
-static int lan78xx_flush_fifo(struct lan78xx_net *dev, u32 reg, u32 fifo_flush)
-{
-	return lan78xx_update_reg(dev, reg, fifo_flush, fifo_flush);
-}
-
-static int lan78xx_start_tx_path(struct lan78xx_net *dev)
-{
-	int ret;
-
-	netif_dbg(dev, drv, dev->net, "start tx path");
-
-	/* Start the MAC transmitter */
-
-	ret = lan78xx_start_hw(dev, MAC_TX, MAC_TX_TXEN_);
-	if (ret < 0)
-		return ret;
-
-	/* Start the Tx FIFO */
-
-	ret = lan78xx_start_hw(dev, FCT_TX_CTL, FCT_TX_CTL_EN_);
-	if (ret < 0)
-		return ret;
-
-	return 0;
-}
-
-static int lan78xx_stop_tx_path(struct lan78xx_net *dev)
-{
-	int ret;
-
-	netif_dbg(dev, drv, dev->net, "stop tx path");
-
-	/* Stop the Tx FIFO */
-
-	ret = lan78xx_stop_hw(dev, FCT_TX_CTL, FCT_TX_CTL_EN_, FCT_TX_CTL_DIS_);
-	if (ret < 0)
-		return ret;
-
-	/* Stop the MAC transmitter */
-
-	ret = lan78xx_stop_hw(dev, MAC_TX, MAC_TX_TXEN_, MAC_TX_TXD_);
-	if (ret < 0)
-		return ret;
-
-	return 0;
-}
-
-/* The caller must ensure the Tx path is stopped before calling
- * lan78xx_flush_tx_fifo().
- */
-static int lan78xx_flush_tx_fifo(struct lan78xx_net *dev)
-{
-	return lan78xx_flush_fifo(dev, FCT_TX_CTL, FCT_TX_CTL_RST_);
-}
-
-static int lan78xx_start_rx_path(struct lan78xx_net *dev)
-{
-	int ret;
-
-	netif_dbg(dev, drv, dev->net, "start rx path");
-
-	/* Start the Rx FIFO */
-
-	ret = lan78xx_start_hw(dev, FCT_RX_CTL, FCT_RX_CTL_EN_);
-	if (ret < 0)
-		return ret;
-
-	/* Start the MAC receiver*/
-
-	ret = lan78xx_start_hw(dev, MAC_RX, MAC_RX_RXEN_);
-	if (ret < 0)
-		return ret;
-
-	return 0;
-}
-
-static int lan78xx_stop_rx_path(struct lan78xx_net *dev)
-{
-	int ret;
-
-	netif_dbg(dev, drv, dev->net, "stop rx path");
-
-	/* Stop the MAC receiver */
-
-	ret = lan78xx_stop_hw(dev, MAC_RX, MAC_RX_RXEN_, MAC_RX_RXD_);
-	if (ret < 0)
-		return ret;
-
-	/* Stop the Rx FIFO */
-
-	ret = lan78xx_stop_hw(dev, FCT_RX_CTL, FCT_RX_CTL_EN_, FCT_RX_CTL_DIS_);
-	if (ret < 0)
-		return ret;
-
-	return 0;
-}
-
-/* The caller must ensure the Rx path is stopped before calling
- * lan78xx_flush_rx_fifo().
- */
-static int lan78xx_flush_rx_fifo(struct lan78xx_net *dev)
-{
-	return lan78xx_flush_fifo(dev, FCT_RX_CTL, FCT_RX_CTL_RST_);
-}
-
 static int lan78xx_reset(struct lan78xx_net *dev)
 {
 	struct lan78xx_priv *pdata = (struct lan78xx_priv *)(dev->data[0]);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 04/10] net: usb: lan78xx: Improve error reporting with %pe specifier
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (2 preceding siblings ...)
  2024-12-04  8:41 ` [PATCH net-next v2 03/10] net: usb: lan78xx: move functions to avoid forward definitions Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 05/10] net: usb: lan78xx: Fix error handling in MII read/write functions Oleksij Rempel
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

Replace integer error codes with the `%pe` format specifier in register
read and write error messages. This change provides human-readable error
strings, making logs more informative and debugging easier.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index dd9b5d3abcb3..94320deaaeea 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -621,8 +621,8 @@ static int lan78xx_read_reg(struct lan78xx_net *dev, u32 index, u32 *data)
 		*data = *buf;
 	} else if (net_ratelimit()) {
 		netdev_warn(dev->net,
-			    "Failed to read register index 0x%08x. ret = %d",
-			    index, ret);
+			    "Failed to read register index 0x%08x. ret = %pe",
+			    index, ERR_PTR(ret));
 	}
 
 	kfree(buf);
@@ -652,8 +652,8 @@ static int lan78xx_write_reg(struct lan78xx_net *dev, u32 index, u32 data)
 	if (unlikely(ret < 0) &&
 	    net_ratelimit()) {
 		netdev_warn(dev->net,
-			    "Failed to write register index 0x%08x. ret = %d",
-			    index, ret);
+			    "Failed to write register index 0x%08x. ret = %pe",
+			    index, ERR_PTR(ret));
 	}
 
 	kfree(buf);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 05/10] net: usb: lan78xx: Fix error handling in MII read/write functions
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (3 preceding siblings ...)
  2024-12-04  8:41 ` [PATCH net-next v2 04/10] net: usb: lan78xx: Improve error reporting with %pe specifier Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations Oleksij Rempel
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

Ensure proper error handling in `lan78xx_mdiobus_read` and
`lan78xx_mdiobus_write` by checking return values of register read/write
operations and returning errors to the caller.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 94320deaaeea..ee308be1e618 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2136,12 +2136,16 @@ static int lan78xx_mdiobus_read(struct mii_bus *bus, int phy_id, int idx)
 	/* set the address, index & direction (read from PHY) */
 	addr = mii_access(phy_id, idx, MII_READ);
 	ret = lan78xx_write_reg(dev, MII_ACC, addr);
+	if (ret < 0)
+		goto done;
 
 	ret = lan78xx_phy_wait_not_busy(dev);
 	if (ret < 0)
 		goto done;
 
 	ret = lan78xx_read_reg(dev, MII_DATA, &val);
+	if (ret < 0)
+		goto done;
 
 	ret = (int)(val & 0xFFFF);
 
@@ -2172,10 +2176,14 @@ static int lan78xx_mdiobus_write(struct mii_bus *bus, int phy_id, int idx,
 
 	val = (u32)regval;
 	ret = lan78xx_write_reg(dev, MII_DATA, val);
+	if (ret < 0)
+		goto done;
 
 	/* set the address, index & direction (write to PHY) */
 	addr = mii_access(phy_id, idx, MII_WRITE);
 	ret = lan78xx_write_reg(dev, MII_ACC, addr);
+	if (ret < 0)
+		goto done;
 
 	ret = lan78xx_phy_wait_not_busy(dev);
 	if (ret < 0)
@@ -2184,7 +2192,7 @@ static int lan78xx_mdiobus_write(struct mii_bus *bus, int phy_id, int idx,
 done:
 	mutex_unlock(&dev->phy_mutex);
 	usb_autopm_put_interface(dev->intf);
-	return 0;
+	return ret;
 }
 
 static int lan78xx_mdio_init(struct lan78xx_net *dev)
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (4 preceding siblings ...)
  2024-12-04  8:41 ` [PATCH net-next v2 05/10] net: usb: lan78xx: Fix error handling in MII read/write functions Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2025-03-03 18:02   ` Mark Brown
  2024-12-04  8:41 ` [PATCH net-next v2 07/10] net: usb: lan78xx: Add error handling to lan78xx_init_ltm Oleksij Rempel
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

Refine error handling in EEPROM and OTP read/write functions by:
- Return error values immediately upon detection.
- Avoid overwriting correct error codes with `-EIO`.
- Preserve initial error codes as they were appropriate for specific
  failures.
- Use `-ETIMEDOUT` for timeout conditions instead of `-EIO`.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 240 ++++++++++++++++++++++++--------------
 1 file changed, 152 insertions(+), 88 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index ee308be1e618..29f6e1a36e20 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1000,8 +1000,8 @@ static int lan78xx_wait_eeprom(struct lan78xx_net *dev)
 
 	do {
 		ret = lan78xx_read_reg(dev, E2P_CMD, &val);
-		if (unlikely(ret < 0))
-			return -EIO;
+		if (ret < 0)
+			return ret;
 
 		if (!(val & E2P_CMD_EPC_BUSY_) ||
 		    (val & E2P_CMD_EPC_TIMEOUT_))
@@ -1011,7 +1011,7 @@ static int lan78xx_wait_eeprom(struct lan78xx_net *dev)
 
 	if (val & (E2P_CMD_EPC_TIMEOUT_ | E2P_CMD_EPC_BUSY_)) {
 		netdev_warn(dev->net, "EEPROM read operation timeout");
-		return -EIO;
+		return -ETIMEDOUT;
 	}
 
 	return 0;
@@ -1025,8 +1025,8 @@ static int lan78xx_eeprom_confirm_not_busy(struct lan78xx_net *dev)
 
 	do {
 		ret = lan78xx_read_reg(dev, E2P_CMD, &val);
-		if (unlikely(ret < 0))
-			return -EIO;
+		if (ret < 0)
+			return ret;
 
 		if (!(val & E2P_CMD_EPC_BUSY_))
 			return 0;
@@ -1035,75 +1035,81 @@ static int lan78xx_eeprom_confirm_not_busy(struct lan78xx_net *dev)
 	} while (!time_after(jiffies, start_time + HZ));
 
 	netdev_warn(dev->net, "EEPROM is busy");
-	return -EIO;
+	return -ETIMEDOUT;
 }
 
 static int lan78xx_read_raw_eeprom(struct lan78xx_net *dev, u32 offset,
 				   u32 length, u8 *data)
 {
-	u32 val;
-	u32 saved;
+	u32 val, saved;
 	int i, ret;
-	int retval;
 
 	/* depends on chip, some EEPROM pins are muxed with LED function.
 	 * disable & restore LED function to access EEPROM.
 	 */
 	ret = lan78xx_read_reg(dev, HW_CFG, &val);
+	if (ret < 0)
+		return ret;
+
 	saved = val;
 	if (dev->chipid == ID_REV_CHIP_ID_7800_) {
 		val &= ~(HW_CFG_LED1_EN_ | HW_CFG_LED0_EN_);
 		ret = lan78xx_write_reg(dev, HW_CFG, val);
+		if (ret < 0)
+			return ret;
 	}
 
-	retval = lan78xx_eeprom_confirm_not_busy(dev);
-	if (retval)
-		return retval;
+	ret = lan78xx_eeprom_confirm_not_busy(dev);
+	if (ret == -ETIMEDOUT)
+		goto read_raw_eeprom_done;
+	/* If USB fails, there is nothing to do */
+	if (ret < 0)
+		return ret;
 
 	for (i = 0; i < length; i++) {
 		val = E2P_CMD_EPC_BUSY_ | E2P_CMD_EPC_CMD_READ_;
 		val |= (offset & E2P_CMD_EPC_ADDR_MASK_);
 		ret = lan78xx_write_reg(dev, E2P_CMD, val);
-		if (unlikely(ret < 0)) {
-			retval = -EIO;
-			goto exit;
-		}
+		if (ret < 0)
+			return ret;
 
-		retval = lan78xx_wait_eeprom(dev);
-		if (retval < 0)
-			goto exit;
+		ret = lan78xx_wait_eeprom(dev);
+		/* Looks like not USB specific error, try to recover */
+		if (ret == -ETIMEDOUT)
+			goto read_raw_eeprom_done;
+		/* If USB fails, there is nothing to do */
+		if (ret < 0)
+			return ret;
 
 		ret = lan78xx_read_reg(dev, E2P_DATA, &val);
-		if (unlikely(ret < 0)) {
-			retval = -EIO;
-			goto exit;
-		}
+		if (ret < 0)
+			return ret;
 
 		data[i] = val & 0xFF;
 		offset++;
 	}
 
-	retval = 0;
-exit:
+read_raw_eeprom_done:
 	if (dev->chipid == ID_REV_CHIP_ID_7800_)
-		ret = lan78xx_write_reg(dev, HW_CFG, saved);
+		return lan78xx_write_reg(dev, HW_CFG, saved);
 
-	return retval;
+	return 0;
 }
 
 static int lan78xx_read_eeprom(struct lan78xx_net *dev, u32 offset,
 			       u32 length, u8 *data)
 {
-	u8 sig;
 	int ret;
+	u8 sig;
 
 	ret = lan78xx_read_raw_eeprom(dev, 0, 1, &sig);
-	if ((ret == 0) && (sig == EEPROM_INDICATOR))
-		ret = lan78xx_read_raw_eeprom(dev, offset, length, data);
-	else
-		ret = -EINVAL;
+	if (ret < 0)
+		return ret;
 
-	return ret;
+	if (sig != EEPROM_INDICATOR)
+		return -ENODATA;
+
+	return lan78xx_read_raw_eeprom(dev, offset, length, data);
 }
 
 static int lan78xx_write_raw_eeprom(struct lan78xx_net *dev, u32 offset,
@@ -1112,113 +1118,144 @@ static int lan78xx_write_raw_eeprom(struct lan78xx_net *dev, u32 offset,
 	u32 val;
 	u32 saved;
 	int i, ret;
-	int retval;
 
 	/* depends on chip, some EEPROM pins are muxed with LED function.
 	 * disable & restore LED function to access EEPROM.
 	 */
 	ret = lan78xx_read_reg(dev, HW_CFG, &val);
+	if (ret < 0)
+		return ret;
+
 	saved = val;
 	if (dev->chipid == ID_REV_CHIP_ID_7800_) {
 		val &= ~(HW_CFG_LED1_EN_ | HW_CFG_LED0_EN_);
 		ret = lan78xx_write_reg(dev, HW_CFG, val);
+		if (ret < 0)
+			return ret;
 	}
 
-	retval = lan78xx_eeprom_confirm_not_busy(dev);
-	if (retval)
-		goto exit;
+	ret = lan78xx_eeprom_confirm_not_busy(dev);
+	/* Looks like not USB specific error, try to recover */
+	if (ret == -ETIMEDOUT)
+		goto write_raw_eeprom_done;
+	/* If USB fails, there is nothing to do */
+	if (ret < 0)
+		return ret;
 
 	/* Issue write/erase enable command */
 	val = E2P_CMD_EPC_BUSY_ | E2P_CMD_EPC_CMD_EWEN_;
 	ret = lan78xx_write_reg(dev, E2P_CMD, val);
-	if (unlikely(ret < 0)) {
-		retval = -EIO;
-		goto exit;
-	}
+	if (ret < 0)
+		return ret;
 
-	retval = lan78xx_wait_eeprom(dev);
-	if (retval < 0)
-		goto exit;
+	ret = lan78xx_wait_eeprom(dev);
+	/* Looks like not USB specific error, try to recover */
+	if (ret == -ETIMEDOUT)
+		goto write_raw_eeprom_done;
+	/* If USB fails, there is nothing to do */
+	if (ret < 0)
+		return ret;
 
 	for (i = 0; i < length; i++) {
 		/* Fill data register */
 		val = data[i];
 		ret = lan78xx_write_reg(dev, E2P_DATA, val);
-		if (ret < 0) {
-			retval = -EIO;
-			goto exit;
-		}
+		if (ret < 0)
+			return ret;
 
 		/* Send "write" command */
 		val = E2P_CMD_EPC_BUSY_ | E2P_CMD_EPC_CMD_WRITE_;
 		val |= (offset & E2P_CMD_EPC_ADDR_MASK_);
 		ret = lan78xx_write_reg(dev, E2P_CMD, val);
-		if (ret < 0) {
-			retval = -EIO;
-			goto exit;
-		}
+		if (ret < 0)
+			return ret;
 
-		retval = lan78xx_wait_eeprom(dev);
-		if (retval < 0)
-			goto exit;
+		ret = lan78xx_wait_eeprom(dev);
+		/* Looks like not USB specific error, try to recover */
+		if (ret == -ETIMEDOUT)
+			goto write_raw_eeprom_done;
+		/* If USB fails, there is nothing to do */
+		if (ret < 0)
+			return ret;
 
 		offset++;
 	}
 
-	retval = 0;
-exit:
+write_raw_eeprom_done:
 	if (dev->chipid == ID_REV_CHIP_ID_7800_)
-		ret = lan78xx_write_reg(dev, HW_CFG, saved);
+		return lan78xx_write_reg(dev, HW_CFG, saved);
 
-	return retval;
+	return 0;
 }
 
 static int lan78xx_read_raw_otp(struct lan78xx_net *dev, u32 offset,
 				u32 length, u8 *data)
 {
-	int i;
-	u32 buf;
 	unsigned long timeout;
+	int ret, i;
+	u32 buf;
 
-	lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+	ret = lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+	if (ret < 0)
+		return ret;
 
 	if (buf & OTP_PWR_DN_PWRDN_N_) {
 		/* clear it and wait to be cleared */
-		lan78xx_write_reg(dev, OTP_PWR_DN, 0);
+		ret = lan78xx_write_reg(dev, OTP_PWR_DN, 0);
+		if (ret < 0)
+			return ret;
 
 		timeout = jiffies + HZ;
 		do {
 			usleep_range(1, 10);
-			lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+			ret = lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+			if (ret < 0)
+				return ret;
+
 			if (time_after(jiffies, timeout)) {
 				netdev_warn(dev->net,
 					    "timeout on OTP_PWR_DN");
-				return -EIO;
+				return -ETIMEDOUT;
 			}
 		} while (buf & OTP_PWR_DN_PWRDN_N_);
 	}
 
 	for (i = 0; i < length; i++) {
-		lan78xx_write_reg(dev, OTP_ADDR1,
-				  ((offset + i) >> 8) & OTP_ADDR1_15_11);
-		lan78xx_write_reg(dev, OTP_ADDR2,
-				  ((offset + i) & OTP_ADDR2_10_3));
+		ret = lan78xx_write_reg(dev, OTP_ADDR1,
+					((offset + i) >> 8) & OTP_ADDR1_15_11);
+		if (ret < 0)
+			return ret;
+
+		ret = lan78xx_write_reg(dev, OTP_ADDR2,
+					((offset + i) & OTP_ADDR2_10_3));
+		if (ret < 0)
+			return ret;
+
+		ret = lan78xx_write_reg(dev, OTP_FUNC_CMD, OTP_FUNC_CMD_READ_);
+		if (ret < 0)
+			return ret;
 
-		lan78xx_write_reg(dev, OTP_FUNC_CMD, OTP_FUNC_CMD_READ_);
-		lan78xx_write_reg(dev, OTP_CMD_GO, OTP_CMD_GO_GO_);
+		ret = lan78xx_write_reg(dev, OTP_CMD_GO, OTP_CMD_GO_GO_);
+		if (ret < 0)
+			return ret;
 
 		timeout = jiffies + HZ;
 		do {
 			udelay(1);
-			lan78xx_read_reg(dev, OTP_STATUS, &buf);
+			ret = lan78xx_read_reg(dev, OTP_STATUS, &buf);
+			if (ret < 0)
+				return ret;
+
 			if (time_after(jiffies, timeout)) {
 				netdev_warn(dev->net,
 					    "timeout on OTP_STATUS");
-				return -EIO;
+				return -ETIMEDOUT;
 			}
 		} while (buf & OTP_STATUS_BUSY_);
 
-		lan78xx_read_reg(dev, OTP_RD_DATA, &buf);
+		ret = lan78xx_read_reg(dev, OTP_RD_DATA, &buf);
+		if (ret < 0)
+			return ret;
 
 		data[i] = (u8)(buf & 0xFF);
 	}
@@ -1232,45 +1269,72 @@ static int lan78xx_write_raw_otp(struct lan78xx_net *dev, u32 offset,
 	int i;
 	u32 buf;
 	unsigned long timeout;
+	int ret;
 
-	lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+	ret = lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+	if (ret < 0)
+		return ret;
 
 	if (buf & OTP_PWR_DN_PWRDN_N_) {
 		/* clear it and wait to be cleared */
-		lan78xx_write_reg(dev, OTP_PWR_DN, 0);
+		ret = lan78xx_write_reg(dev, OTP_PWR_DN, 0);
+		if (ret < 0)
+			return ret;
 
 		timeout = jiffies + HZ;
 		do {
 			udelay(1);
-			lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+			ret = lan78xx_read_reg(dev, OTP_PWR_DN, &buf);
+			if (ret < 0)
+				return ret;
+
 			if (time_after(jiffies, timeout)) {
 				netdev_warn(dev->net,
 					    "timeout on OTP_PWR_DN completion");
-				return -EIO;
+				return -ETIMEDOUT;
 			}
 		} while (buf & OTP_PWR_DN_PWRDN_N_);
 	}
 
 	/* set to BYTE program mode */
-	lan78xx_write_reg(dev, OTP_PRGM_MODE, OTP_PRGM_MODE_BYTE_);
+	ret = lan78xx_write_reg(dev, OTP_PRGM_MODE, OTP_PRGM_MODE_BYTE_);
+	if (ret < 0)
+		return ret;
 
 	for (i = 0; i < length; i++) {
-		lan78xx_write_reg(dev, OTP_ADDR1,
-				  ((offset + i) >> 8) & OTP_ADDR1_15_11);
-		lan78xx_write_reg(dev, OTP_ADDR2,
-				  ((offset + i) & OTP_ADDR2_10_3));
-		lan78xx_write_reg(dev, OTP_PRGM_DATA, data[i]);
-		lan78xx_write_reg(dev, OTP_TST_CMD, OTP_TST_CMD_PRGVRFY_);
-		lan78xx_write_reg(dev, OTP_CMD_GO, OTP_CMD_GO_GO_);
+		ret = lan78xx_write_reg(dev, OTP_ADDR1,
+					((offset + i) >> 8) & OTP_ADDR1_15_11);
+		if (ret < 0)
+			return ret;
+
+		ret = lan78xx_write_reg(dev, OTP_ADDR2,
+					((offset + i) & OTP_ADDR2_10_3));
+		if (ret < 0)
+			return ret;
+
+		ret = lan78xx_write_reg(dev, OTP_PRGM_DATA, data[i]);
+		if (ret < 0)
+			return ret;
+
+		ret = lan78xx_write_reg(dev, OTP_TST_CMD, OTP_TST_CMD_PRGVRFY_);
+		if (ret < 0)
+			return ret;
+
+		ret = lan78xx_write_reg(dev, OTP_CMD_GO, OTP_CMD_GO_GO_);
+		if (ret < 0)
+			return ret;
 
 		timeout = jiffies + HZ;
 		do {
 			udelay(1);
-			lan78xx_read_reg(dev, OTP_STATUS, &buf);
+			ret = lan78xx_read_reg(dev, OTP_STATUS, &buf);
+			if (ret < 0)
+				return ret;
+
 			if (time_after(jiffies, timeout)) {
 				netdev_warn(dev->net,
 					    "Timeout on OTP_STATUS completion");
-				return -EIO;
+				return -ETIMEDOUT;
 			}
 		} while (buf & OTP_STATUS_BUSY_);
 	}
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 07/10] net: usb: lan78xx: Add error handling to lan78xx_init_ltm
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (5 preceding siblings ...)
  2024-12-04  8:41 ` [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 08/10] net: usb: lan78xx: Add error handling to set_rx_max_frame_length and set_mtu Oleksij Rempel
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

Convert `lan78xx_init_ltm` to return error codes and handle errors
properly.  Previously, errors during the LTM initialization process were
not propagated, potentially leading to undetected issues. This patch
ensures:

- Errors in `lan78xx_read_reg` and `lan78xx_write_reg` are checked and
  handled.
- Errors are logged with detailed messages using `%pe` for clarity.
- The function exits immediately on error, returning the error code.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 50 ++++++++++++++++++++++++++++++---------
 1 file changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 29f6e1a36e20..33cda7f3dd12 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2807,13 +2807,16 @@ static int lan78xx_vlan_rx_kill_vid(struct net_device *netdev,
 	return 0;
 }
 
-static void lan78xx_init_ltm(struct lan78xx_net *dev)
+static int lan78xx_init_ltm(struct lan78xx_net *dev)
 {
+	u32 regs[6] = { 0 };
 	int ret;
 	u32 buf;
-	u32 regs[6] = { 0 };
 
 	ret = lan78xx_read_reg(dev, USB_CFG1, &buf);
+	if (ret < 0)
+		goto init_ltm_failed;
+
 	if (buf & USB_CFG1_LTM_ENABLE_) {
 		u8 temp[2];
 		/* Get values from EEPROM first */
@@ -2824,7 +2827,7 @@ static void lan78xx_init_ltm(struct lan78xx_net *dev)
 							      24,
 							      (u8 *)regs);
 				if (ret < 0)
-					return;
+					return ret;
 			}
 		} else if (lan78xx_read_otp(dev, 0x3F, 2, temp) == 0) {
 			if (temp[0] == 24) {
@@ -2833,17 +2836,40 @@ static void lan78xx_init_ltm(struct lan78xx_net *dev)
 							   24,
 							   (u8 *)regs);
 				if (ret < 0)
-					return;
+					return ret;
 			}
 		}
 	}
 
-	lan78xx_write_reg(dev, LTM_BELT_IDLE0, regs[0]);
-	lan78xx_write_reg(dev, LTM_BELT_IDLE1, regs[1]);
-	lan78xx_write_reg(dev, LTM_BELT_ACT0, regs[2]);
-	lan78xx_write_reg(dev, LTM_BELT_ACT1, regs[3]);
-	lan78xx_write_reg(dev, LTM_INACTIVE0, regs[4]);
-	lan78xx_write_reg(dev, LTM_INACTIVE1, regs[5]);
+	ret = lan78xx_write_reg(dev, LTM_BELT_IDLE0, regs[0]);
+	if (ret < 0)
+		goto init_ltm_failed;
+
+	ret = lan78xx_write_reg(dev, LTM_BELT_IDLE1, regs[1]);
+	if (ret < 0)
+		goto init_ltm_failed;
+
+	ret = lan78xx_write_reg(dev, LTM_BELT_ACT0, regs[2]);
+	if (ret < 0)
+		goto init_ltm_failed;
+
+	ret = lan78xx_write_reg(dev, LTM_BELT_ACT1, regs[3]);
+	if (ret < 0)
+		goto init_ltm_failed;
+
+	ret = lan78xx_write_reg(dev, LTM_INACTIVE0, regs[4]);
+	if (ret < 0)
+		goto init_ltm_failed;
+
+	ret = lan78xx_write_reg(dev, LTM_INACTIVE1, regs[5]);
+	if (ret < 0)
+		goto init_ltm_failed;
+
+	return 0;
+
+init_ltm_failed:
+	netdev_err(dev->net, "Failed to init LTM with error %pe\n", ERR_PTR(ret));
+	return ret;
 }
 
 static int lan78xx_urb_config_init(struct lan78xx_net *dev)
@@ -2939,7 +2965,9 @@ static int lan78xx_reset(struct lan78xx_net *dev)
 		return ret;
 
 	/* Init LTM */
-	lan78xx_init_ltm(dev);
+	ret = lan78xx_init_ltm(dev);
+	if (ret < 0)
+		return ret;
 
 	ret = lan78xx_write_reg(dev, BURST_CAP, dev->burst_cap);
 	if (ret < 0)
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 08/10] net: usb: lan78xx: Add error handling to set_rx_max_frame_length and set_mtu
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (6 preceding siblings ...)
  2024-12-04  8:41 ` [PATCH net-next v2 07/10] net: usb: lan78xx: Add error handling to lan78xx_init_ltm Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 09/10] net: usb: lan78xx: Add error handling to lan78xx_irq_bus_sync_unlock Oleksij Rempel
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

Improve error handling in `lan78xx_set_rx_max_frame_length` by:
- Checking return values from register read/write operations and
  propagating errors.
- Exiting immediately on failure to ensure proper error reporting.

In `lan78xx_change_mtu`, log errors when changing MTU fails, using `%pe`
for clear error representation.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 33cda7f3dd12..2d16c1fc850e 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2599,27 +2599,36 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
 
 static int lan78xx_set_rx_max_frame_length(struct lan78xx_net *dev, int size)
 {
-	u32 buf;
 	bool rxenabled;
+	u32 buf;
+	int ret;
 
-	lan78xx_read_reg(dev, MAC_RX, &buf);
+	ret = lan78xx_read_reg(dev, MAC_RX, &buf);
+	if (ret < 0)
+		return ret;
 
 	rxenabled = ((buf & MAC_RX_RXEN_) != 0);
 
 	if (rxenabled) {
 		buf &= ~MAC_RX_RXEN_;
-		lan78xx_write_reg(dev, MAC_RX, buf);
+		ret = lan78xx_write_reg(dev, MAC_RX, buf);
+		if (ret < 0)
+			return ret;
 	}
 
 	/* add 4 to size for FCS */
 	buf &= ~MAC_RX_MAX_SIZE_MASK_;
 	buf |= (((size + 4) << MAC_RX_MAX_SIZE_SHIFT_) & MAC_RX_MAX_SIZE_MASK_);
 
-	lan78xx_write_reg(dev, MAC_RX, buf);
+	ret = lan78xx_write_reg(dev, MAC_RX, buf);
+	if (ret < 0)
+		return ret;
 
 	if (rxenabled) {
 		buf |= MAC_RX_RXEN_;
-		lan78xx_write_reg(dev, MAC_RX, buf);
+		ret = lan78xx_write_reg(dev, MAC_RX, buf);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
@@ -2685,7 +2694,10 @@ static int lan78xx_change_mtu(struct net_device *netdev, int new_mtu)
 		return ret;
 
 	ret = lan78xx_set_rx_max_frame_length(dev, max_frame_len);
-	if (!ret)
+	if (ret < 0)
+		netdev_err(dev->net, "MTU changed to %d from %d failed with %pe\n",
+			   new_mtu, netdev->mtu, ERR_PTR(ret));
+	else
 		WRITE_ONCE(netdev->mtu, new_mtu);
 
 	usb_autopm_put_interface(dev->intf);
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 09/10] net: usb: lan78xx: Add error handling to lan78xx_irq_bus_sync_unlock
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (7 preceding siblings ...)
  2024-12-04  8:41 ` [PATCH net-next v2 08/10] net: usb: lan78xx: Add error handling to set_rx_max_frame_length and set_mtu Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-04  8:41 ` [PATCH net-next v2 10/10] net: usb: lan78xx: Improve error handling in dataport and multicast writes Oleksij Rempel
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

Update `lan78xx_irq_bus_sync_unlock` to handle errors in register
read/write operations. If an error occurs, log it and exit the function
appropriately.  This ensures proper handling of failures during IRQ
synchronization.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 2d16c1fc850e..2ae9565b5044 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -2382,13 +2382,22 @@ static void lan78xx_irq_bus_sync_unlock(struct irq_data *irqd)
 	struct lan78xx_net *dev =
 			container_of(data, struct lan78xx_net, domain_data);
 	u32 buf;
+	int ret;
 
 	/* call register access here because irq_bus_lock & irq_bus_sync_unlock
 	 * are only two callbacks executed in non-atomic contex.
 	 */
-	lan78xx_read_reg(dev, INT_EP_CTL, &buf);
+	ret = lan78xx_read_reg(dev, INT_EP_CTL, &buf);
+	if (ret < 0)
+		goto irq_bus_sync_unlock;
+
 	if (buf != data->irqenable)
-		lan78xx_write_reg(dev, INT_EP_CTL, data->irqenable);
+		ret = lan78xx_write_reg(dev, INT_EP_CTL, data->irqenable);
+
+irq_bus_sync_unlock:
+	if (ret < 0)
+		netdev_err(dev->net, "Failed to sync IRQ enable register: %pe\n",
+			   ERR_PTR(ret));
 
 	mutex_unlock(&data->irq_lock);
 }
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH net-next v2 10/10] net: usb: lan78xx: Improve error handling in dataport and multicast writes
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (8 preceding siblings ...)
  2024-12-04  8:41 ` [PATCH net-next v2 09/10] net: usb: lan78xx: Add error handling to lan78xx_irq_bus_sync_unlock Oleksij Rempel
@ 2024-12-04  8:41 ` Oleksij Rempel
  2024-12-07  1:55 ` [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Jakub Kicinski
  2024-12-07  2:00 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2024-12-04  8:41 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn
  Cc: Oleksij Rempel, Andrew Lunn, kernel, linux-kernel, netdev,
	UNGLinuxDriver, Phil Elwell

Update `lan78xx_dataport_write` and `lan78xx_deferred_multicast_write`
to:
- Handle errors during register read/write operations.
- Exit immediately on errors and log them using `%pe` for clarity.
- Avoid silent failures by propagating error codes properly.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 drivers/net/usb/lan78xx.c | 67 ++++++++++++++++++++++++++-------------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 2ae9565b5044..d5f6367d3714 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1371,7 +1371,7 @@ static int lan78xx_dataport_wait_not_busy(struct lan78xx_net *dev)
 
 		ret = lan78xx_read_reg(dev, DP_SEL, &dp_sel);
 		if (unlikely(ret < 0))
-			return -EIO;
+			return ret;
 
 		if (dp_sel & DP_SEL_DPRDY_)
 			return 0;
@@ -1381,44 +1381,51 @@ static int lan78xx_dataport_wait_not_busy(struct lan78xx_net *dev)
 
 	netdev_warn(dev->net, "%s timed out", __func__);
 
-	return -EIO;
+	return -ETIMEDOUT;
 }
 
 static int lan78xx_dataport_write(struct lan78xx_net *dev, u32 ram_select,
 				  u32 addr, u32 length, u32 *buf)
 {
 	struct lan78xx_priv *pdata = (struct lan78xx_priv *)(dev->data[0]);
-	u32 dp_sel;
 	int i, ret;
 
-	if (usb_autopm_get_interface(dev->intf) < 0)
-		return 0;
+	ret = usb_autopm_get_interface(dev->intf);
+	if (ret < 0)
+		return ret;
 
 	mutex_lock(&pdata->dataport_mutex);
 
 	ret = lan78xx_dataport_wait_not_busy(dev);
 	if (ret < 0)
-		goto done;
+		goto dataport_write;
 
-	ret = lan78xx_read_reg(dev, DP_SEL, &dp_sel);
-
-	dp_sel &= ~DP_SEL_RSEL_MASK_;
-	dp_sel |= ram_select;
-	ret = lan78xx_write_reg(dev, DP_SEL, dp_sel);
+	ret = lan78xx_update_reg(dev, DP_SEL, DP_SEL_RSEL_MASK_, ram_select);
+	if (ret < 0)
+		goto dataport_write;
 
 	for (i = 0; i < length; i++) {
 		ret = lan78xx_write_reg(dev, DP_ADDR, addr + i);
+		if (ret < 0)
+			goto dataport_write;
 
 		ret = lan78xx_write_reg(dev, DP_DATA, buf[i]);
+		if (ret < 0)
+			goto dataport_write;
 
 		ret = lan78xx_write_reg(dev, DP_CMD, DP_CMD_WRITE_);
+		if (ret < 0)
+			goto dataport_write;
 
 		ret = lan78xx_dataport_wait_not_busy(dev);
 		if (ret < 0)
-			goto done;
+			goto dataport_write;
 	}
 
-done:
+dataport_write:
+	if (ret < 0)
+		netdev_warn(dev->net, "dataport write failed %pe", ERR_PTR(ret));
+
 	mutex_unlock(&pdata->dataport_mutex);
 	usb_autopm_put_interface(dev->intf);
 
@@ -1454,23 +1461,39 @@ static void lan78xx_deferred_multicast_write(struct work_struct *param)
 	struct lan78xx_priv *pdata =
 			container_of(param, struct lan78xx_priv, set_multicast);
 	struct lan78xx_net *dev = pdata->dev;
-	int i;
+	int i, ret;
 
 	netif_dbg(dev, drv, dev->net, "deferred multicast write 0x%08x\n",
 		  pdata->rfe_ctl);
 
-	lan78xx_dataport_write(dev, DP_SEL_RSEL_VLAN_DA_, DP_SEL_VHF_VLAN_LEN,
-			       DP_SEL_VHF_HASH_LEN, pdata->mchash_table);
+	ret = lan78xx_dataport_write(dev, DP_SEL_RSEL_VLAN_DA_,
+				     DP_SEL_VHF_VLAN_LEN,
+				     DP_SEL_VHF_HASH_LEN, pdata->mchash_table);
+	if (ret < 0)
+		goto multicast_write_done;
 
 	for (i = 1; i < NUM_OF_MAF; i++) {
-		lan78xx_write_reg(dev, MAF_HI(i), 0);
-		lan78xx_write_reg(dev, MAF_LO(i),
-				  pdata->pfilter_table[i][1]);
-		lan78xx_write_reg(dev, MAF_HI(i),
-				  pdata->pfilter_table[i][0]);
+		ret = lan78xx_write_reg(dev, MAF_HI(i), 0);
+		if (ret < 0)
+			goto multicast_write_done;
+
+		ret = lan78xx_write_reg(dev, MAF_LO(i),
+					pdata->pfilter_table[i][1]);
+		if (ret < 0)
+			goto multicast_write_done;
+
+		ret = lan78xx_write_reg(dev, MAF_HI(i),
+					pdata->pfilter_table[i][0]);
+		if (ret < 0)
+			goto multicast_write_done;
 	}
 
-	lan78xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
+	ret = lan78xx_write_reg(dev, RFE_CTL, pdata->rfe_ctl);
+
+multicast_write_done:
+	if (ret < 0)
+		netdev_warn(dev->net, "multicast write failed %pe", ERR_PTR(ret));
+	return;
 }
 
 static void lan78xx_set_multicast(struct net_device *netdev)
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (9 preceding siblings ...)
  2024-12-04  8:41 ` [PATCH net-next v2 10/10] net: usb: lan78xx: Improve error handling in dataport and multicast writes Oleksij Rempel
@ 2024-12-07  1:55 ` Jakub Kicinski
  2024-12-07  2:00 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 17+ messages in thread
From: Jakub Kicinski @ 2024-12-07  1:55 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: David S. Miller, Eric Dumazet, Paolo Abeni, Woojung Huh,
	Andrew Lunn, kernel, linux-kernel, netdev, UNGLinuxDriver,
	Phil Elwell

On Wed,  4 Dec 2024 09:41:32 +0100 Oleksij Rempel wrote:
> Subject: [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink

Be careful when reusing cover letters, the 21 here makes patchwork think
the series will have 21 patches, even tho patches say /10.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink
  2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
                   ` (10 preceding siblings ...)
  2024-12-07  1:55 ` [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Jakub Kicinski
@ 2024-12-07  2:00 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 17+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-12-07  2:00 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: davem, edumazet, kuba, pabeni, woojung.huh, andrew+netdev, kernel,
	linux-kernel, netdev, UNGLinuxDriver, phil

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  4 Dec 2024 09:41:32 +0100 you wrote:
> changes v2:
> - split the patch set.
> 
> This patch set is part of the preparatory work for migrating the lan78xx
> USB Ethernet driver to the PHYlink framework. During extensive testing,
> I observed that resetting the USB adapter can lead to various read/write
> errors. While the errors themselves are acceptable, they generate
> excessive log messages, resulting in significant log spam. This set
> improves error handling to reduce logging noise by addressing errors
> directly and returning early when necessary.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,01/10] net: usb: lan78xx: Remove LAN8835 PHY fixup
    https://git.kernel.org/netdev/net-next/c/7b60c3bf93fa
  - [net-next,v2,02/10] net: usb: lan78xx: Remove KSZ9031 PHY fixup
    https://git.kernel.org/netdev/net-next/c/6782d06a47ad
  - [net-next,v2,03/10] net: usb: lan78xx: move functions to avoid forward definitions
    https://git.kernel.org/netdev/net-next/c/39aa1d620d10
  - [net-next,v2,04/10] net: usb: lan78xx: Improve error reporting with %pe specifier
    https://git.kernel.org/netdev/net-next/c/9bcdc610cfab
  - [net-next,v2,05/10] net: usb: lan78xx: Fix error handling in MII read/write functions
    https://git.kernel.org/netdev/net-next/c/32ee0dc76450
  - [net-next,v2,06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations
    https://git.kernel.org/netdev/net-next/c/8b1b2ca83b20
  - [net-next,v2,07/10] net: usb: lan78xx: Add error handling to lan78xx_init_ltm
    https://git.kernel.org/netdev/net-next/c/77586156b517
  - [net-next,v2,08/10] net: usb: lan78xx: Add error handling to set_rx_max_frame_length and set_mtu
    https://git.kernel.org/netdev/net-next/c/65520a70cb09
  - [net-next,v2,09/10] net: usb: lan78xx: Add error handling to lan78xx_irq_bus_sync_unlock
    https://git.kernel.org/netdev/net-next/c/0da202e6a56f
  - [net-next,v2,10/10] net: usb: lan78xx: Improve error handling in dataport and multicast writes
    https://git.kernel.org/netdev/net-next/c/48fb3d3c4be6

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] 17+ messages in thread

* Re: [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations
  2024-12-04  8:41 ` [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations Oleksij Rempel
@ 2025-03-03 18:02   ` Mark Brown
  2025-03-04  5:40     ` Oleksij Rempel
  2025-03-06  8:48     ` Oleksij Rempel
  0 siblings, 2 replies; 17+ messages in thread
From: Mark Brown @ 2025-03-03 18:02 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn, Andrew Lunn, kernel, linux-kernel,
	netdev, UNGLinuxDriver, Phil Elwell

[-- Attachment #1: Type: text/plain, Size: 4199 bytes --]

On Wed, Dec 04, 2024 at 09:41:38AM +0100, Oleksij Rempel wrote:
> Refine error handling in EEPROM and OTP read/write functions by:
> - Return error values immediately upon detection.
> - Avoid overwriting correct error codes with `-EIO`.
> - Preserve initial error codes as they were appropriate for specific
>   failures.
> - Use `-ETIMEDOUT` for timeout conditions instead of `-EIO`.

This patch (which is in Linus' tree) appears to break booting with a NFS
root filesystem on Raspberry Pi 3B+.  There appears to be at least no
incoming traffic seen on the device, I've not checked if there's
anything outgoing:

[   19.234086] usb 1-1.1.1: new high-speed USB device number 6 using dwc2
[   19.394134] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000): clkctl 0x50
[   19.710839] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: renamed from eth0
Device /sys/class/net/enxb827ebea22ac found
done.
Begin: Waiting up to 180 secs for any network device to become available ... done.
IP-Config: enxb827ebea22ac hardware address b8:27:eb:ea:22:ac mt[   20.663606] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: Link is Down
u 1500 DHCP
[   22.708103] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: Link is Up - 1Gbps/Full - flow control off
IP-Config: no response after 2 secs - giving up

The link did look like it was up on the switch.  Full log:

   https://lava.sirena.org.uk/scheduler/job/1158809#L965

A bisect points to this commit fairly cleanly:

git bisect start
# status: waiting for both good and bad commits
# bad: [7eb172143d5508b4da468ed59ee857c6e5e01da6] Linux 6.14-rc5
git bisect bad 7eb172143d5508b4da468ed59ee857c6e5e01da6
# status: waiting for good commit(s), bad commit known
# good: [ffd294d346d185b70e28b1a28abe367bbfe53c04] Linux 6.13
git bisect good ffd294d346d185b70e28b1a28abe367bbfe53c04
# bad: [f1c243fc78ca94fd72e2e6e8f0f49b7360fef475] Merge tag 'iommu-updates-v6.14' of git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux
git bisect bad f1c243fc78ca94fd72e2e6e8f0f49b7360fef475
# good: [5f537664e705b0bf8b7e329861f20128534f6a83] cachestat: fix page cache statistics permission checking
git bisect good 5f537664e705b0bf8b7e329861f20128534f6a83
# bad: [7b081a74c07d9e097f6829a1749f0aa441553c5e] Merge tag 'regulator-v6.14' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
git bisect bad 7b081a74c07d9e097f6829a1749f0aa441553c5e
# bad: [7b24f164cf005b9649138ef6de94aaac49c9f3d1] Merge tag 'ipsec-next-2025-01-09' of git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next
git bisect bad 7b24f164cf005b9649138ef6de94aaac49c9f3d1
# bad: [75e2c86c7b180fd1068ad271178c2820a199e7eb] net: netlink: catch attempts to send empty messages
git bisect bad 75e2c86c7b180fd1068ad271178c2820a199e7eb
# bad: [bf361b18d91e96dee50c5794097a80ff3594725c] net: usb: lan78xx: Fix return value handling in lan78xx_set_features
git bisect bad bf361b18d91e96dee50c5794097a80ff3594725c
# bad: [195c3d4631816f02071f0e01d2d2def51cf5067a] octeontx2-pf: map skb data as device writeable
git bisect bad 195c3d4631816f02071f0e01d2d2def51cf5067a
# good: [dcf3827cde8621d2317a7f98e069adbdc2112982] xdp, xsk: constify read-only arguments of some static inline helpers
git bisect good dcf3827cde8621d2317a7f98e069adbdc2112982
# good: [7a2716ac9a5b2a2dd0443b101766d3721f094ee1] Merge branch 'net-phylib-eee-cleanups'
git bisect good 7a2716ac9a5b2a2dd0443b101766d3721f094ee1
# bad: [18eabadd73ae60023ab05e376246bd725fb0c113] vrf: Make pcpu_dstats update functions available to other modules.
git bisect bad 18eabadd73ae60023ab05e376246bd725fb0c113
# bad: [8b1b2ca83b200fa46fdfb81e80ad5fe34537e6d4] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations
git bisect bad 8b1b2ca83b200fa46fdfb81e80ad5fe34537e6d4
# good: [39aa1d620d10cdd276f4728da50f136dbe939643] net: usb: lan78xx: move functions to avoid forward definitions
git bisect good 39aa1d620d10cdd276f4728da50f136dbe939643
# good: [32ee0dc764505278229078e496e7b56a6d65224b] net: usb: lan78xx: Fix error handling in MII read/write functions
git bisect good 32ee0dc764505278229078e496e7b56a6d65224b
# first bad commit: [8b1b2ca83b200fa46fdfb81e80ad5fe34537e6d4] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations
  2025-03-03 18:02   ` Mark Brown
@ 2025-03-04  5:40     ` Oleksij Rempel
  2025-03-06  8:48     ` Oleksij Rempel
  1 sibling, 0 replies; 17+ messages in thread
From: Oleksij Rempel @ 2025-03-04  5:40 UTC (permalink / raw)
  To: Mark Brown
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn, Andrew Lunn, kernel, linux-kernel,
	netdev, UNGLinuxDriver, Phil Elwell

On Mon, Mar 03, 2025 at 06:02:23PM +0000, Mark Brown wrote:
> On Wed, Dec 04, 2024 at 09:41:38AM +0100, Oleksij Rempel wrote:
> > Refine error handling in EEPROM and OTP read/write functions by:
> > - Return error values immediately upon detection.
> > - Avoid overwriting correct error codes with `-EIO`.
> > - Preserve initial error codes as they were appropriate for specific
> >   failures.
> > - Use `-ETIMEDOUT` for timeout conditions instead of `-EIO`.
> 
> This patch (which is in Linus' tree) appears to break booting with a NFS
> root filesystem on Raspberry Pi 3B+.  There appears to be at least no
> incoming traffic seen on the device, I've not checked if there's
> anything outgoing:
> 
> [   19.234086] usb 1-1.1.1: new high-speed USB device number 6 using dwc2
> [   19.394134] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000): clkctl 0x50
> [   19.710839] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: renamed from eth0
> Device /sys/class/net/enxb827ebea22ac found
> done.
> Begin: Waiting up to 180 secs for any network device to become available ... done.
> IP-Config: enxb827ebea22ac hardware address b8:27:eb:ea:22:ac mt[   20.663606] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: Link is Down
> u 1500 DHCP
> [   22.708103] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: Link is Up - 1Gbps/Full - flow control off
> IP-Config: no response after 2 secs - giving up
> 
> The link did look like it was up on the switch.  Full log:
> 
>    https://lava.sirena.org.uk/scheduler/job/1158809#L965

Thank you! I'm investigating it.
-- 
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] 17+ messages in thread

* Re: [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations
  2025-03-03 18:02   ` Mark Brown
  2025-03-04  5:40     ` Oleksij Rempel
@ 2025-03-06  8:48     ` Oleksij Rempel
  2025-03-06 13:17       ` Mark Brown
  1 sibling, 1 reply; 17+ messages in thread
From: Oleksij Rempel @ 2025-03-06  8:48 UTC (permalink / raw)
  To: Mark Brown
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn, Andrew Lunn, kernel, linux-kernel,
	netdev, UNGLinuxDriver, Phil Elwell

Hi Mark,

On Mon, Mar 03, 2025 at 06:02:23PM +0000, Mark Brown wrote:
> On Wed, Dec 04, 2024 at 09:41:38AM +0100, Oleksij Rempel wrote:
> > Refine error handling in EEPROM and OTP read/write functions by:
> > - Return error values immediately upon detection.
> > - Avoid overwriting correct error codes with `-EIO`.
> > - Preserve initial error codes as they were appropriate for specific
> >   failures.
> > - Use `-ETIMEDOUT` for timeout conditions instead of `-EIO`.
> 
> This patch (which is in Linus' tree) appears to break booting with a NFS
> root filesystem on Raspberry Pi 3B+.  There appears to be at least no
> incoming traffic seen on the device, I've not checked if there's
> anything outgoing:
> 
> [   19.234086] usb 1-1.1.1: new high-speed USB device number 6 using dwc2
> [   19.394134] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000): clkctl 0x50
> [   19.710839] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: renamed from eth0
> Device /sys/class/net/enxb827ebea22ac found
> done.
> Begin: Waiting up to 180 secs for any network device to become available ... done.
> IP-Config: enxb827ebea22ac hardware address b8:27:eb:ea:22:ac mt[   20.663606] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: Link is Down
> u 1500 DHCP
> [   22.708103] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: Link is Up - 1Gbps/Full - flow control off
> IP-Config: no response after 2 secs - giving up

I can't reproduce it without U-boot. Since netboot is used, this adapter
stays in some kind of preconfugured state. Where can I get the SD image
which is used on this system?

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] 17+ messages in thread

* Re: [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations
  2025-03-06  8:48     ` Oleksij Rempel
@ 2025-03-06 13:17       ` Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2025-03-06 13:17 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Woojung Huh, Andrew Lunn, Andrew Lunn, kernel, linux-kernel,
	netdev, UNGLinuxDriver, Phil Elwell

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Thu, Mar 06, 2025 at 09:48:04AM +0100, Oleksij Rempel wrote:
> On Mon, Mar 03, 2025 at 06:02:23PM +0000, Mark Brown wrote:

> > IP-Config: enxb827ebea22ac hardware address b8:27:eb:ea:22:ac mt[   20.663606] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: Link is Down
> > u 1500 DHCP
> > [   22.708103] lan78xx 1-1.1.1:1.0 enxb827ebea22ac: Link is Up - 1Gbps/Full - flow control off
> > IP-Config: no response after 2 secs - giving up

> I can't reproduce it without U-boot. Since netboot is used, this adapter
> stays in some kind of preconfugured state. Where can I get the SD image
> which is used on this system?

I'm using u-boot - this is a full netboot for CI so I'm using u-boot to
fetch the kernel image.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-03-06 13:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-04  8:41 [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 01/10] net: usb: lan78xx: Remove LAN8835 PHY fixup Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 02/10] net: usb: lan78xx: Remove KSZ9031 " Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 03/10] net: usb: lan78xx: move functions to avoid forward definitions Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 04/10] net: usb: lan78xx: Improve error reporting with %pe specifier Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 05/10] net: usb: lan78xx: Fix error handling in MII read/write functions Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 06/10] net: usb: lan78xx: Improve error handling in EEPROM and OTP operations Oleksij Rempel
2025-03-03 18:02   ` Mark Brown
2025-03-04  5:40     ` Oleksij Rempel
2025-03-06  8:48     ` Oleksij Rempel
2025-03-06 13:17       ` Mark Brown
2024-12-04  8:41 ` [PATCH net-next v2 07/10] net: usb: lan78xx: Add error handling to lan78xx_init_ltm Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 08/10] net: usb: lan78xx: Add error handling to set_rx_max_frame_length and set_mtu Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 09/10] net: usb: lan78xx: Add error handling to lan78xx_irq_bus_sync_unlock Oleksij Rempel
2024-12-04  8:41 ` [PATCH net-next v2 10/10] net: usb: lan78xx: Improve error handling in dataport and multicast writes Oleksij Rempel
2024-12-07  1:55 ` [PATCH net-next v2 00/21] lan78xx: Preparations for PHYlink Jakub Kicinski
2024-12-07  2:00 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).