Thread (5 messages) 5 messages, 3 authors, 2025-10-09

Re: [PATCH] net: usb: lan78xx: Fix lost EEPROM write timeout error(-ETIMEDOUT) in lan78xx_write_raw_eeprom

From: Bhanu Seshu Kumar Valluri <hidden>
Date: 2025-10-07 06:12:20
Also in: linux-kernel-mentees, linux-usb, lkml

On 07/10/25 00:30, Khalid Aziz wrote:
On 10/3/25 10:07 PM, Bhanu Seshu Kumar Valluri wrote:
quoted
The function lan78xx_write_raw_eeprom failed to properly propagate EEPROM
write timeout errors (-ETIMEDOUT). In the timeout  fallthrough path, it first
attempted to restore the pin configuration for LED outputs and then
returned only the status of that restore operation, discarding the
original timeout error saved in ret.

As a result, callers could mistakenly treat EEPROM write operation as
successful even though the EEPROM write had actually timed out with no
or partial data write.

To fix this, handle errors in restoring the LED pin configuration separately.
If the restore succeeds, return any prior EEPROM write timeout error saved
in ret to the caller.

Suggested-by: Oleksij Rempel <o.rempel@pengutronix.de>
Fixes: 8b1b2ca83b20 ("net: usb: lan78xx: Improve error handling in EEPROM and OTP operations")
Signed-off-by: Bhanu Seshu Kumar Valluri <redacted>
---
  Note:
  The patch is compiled and tested.
  The patch was suggested by Oleksij Rempel while reviewing a fix to a bug
  found by syzbot earlier.
  The review mail chain where this fix was suggested is given below.
  https://lore.kernel.org/all/aNzojoXK-m1Tn6Lc@pengutronix.de/ (local)

  drivers/net/usb/lan78xx.c | 11 +++++++----
  1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index d75502ebbc0d..5ccbe6ae2ebe 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -1174,10 +1174,13 @@ static int lan78xx_write_raw_eeprom(struct lan78xx_net *dev, u32 offset,
      }
    write_raw_eeprom_done:
-    if (dev->chipid == ID_REV_CHIP_ID_7800_)
-        return lan78xx_write_reg(dev, HW_CFG, saved);
-
-    return 0;
+    if (dev->chipid == ID_REV_CHIP_ID_7800_) {
+        int rc = lan78xx_write_reg(dev, HW_CFG, saved);
+        /* If USB fails, there is nothing to do */
+        if (rc < 0)
+            return rc;
+    }
+    return ret;
  }
    static int lan78xx_read_raw_otp(struct lan78xx_net *dev, u32 offset,
You were able to test the change to read eeprom code by forcing a timeout while doing probe on EVB-LAN7800LC. Were you able to test this code change the same way just to make sure callers of the write function handle the new ETIMEDOUT return value correctly?

Thanks,
Hi Khalid,

This function is only invoked from user's ethtool operations.  The ethtool handles errors returned from the driver callback functions. 
I tested it with ethtool -E option by forcing a ETIMEDOUT error early in the lan78xx_write_raw_eeprom temporarily. The ethtool 
reported error with "Cannot set EEPROM data" message. The ethtool version used is 5.16.

Regards,
Bhanu Seshu Kumar Valluri
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help