Message-ID: <20251209001610.611575-24-sashal@kernel.org>
Date: Mon,  8 Dec 2025 19:15:16 -0500
From: Sasha Levin <sashal@...nel.org>
To: patches@...ts.linux.dev,
	stable@...r.kernel.org
Cc: Alok Tiwari <alok.a.tiwari@...cle.com>,
	Simon Horman <horms@...nel.org>,
	Shyam Sundar S K <Shyam-sundar.S-k@....com>,
	Jakub Kicinski <kuba@...nel.org>,
	Sasha Levin <sashal@...nel.org>,
	linux@...linux.org.uk,
	netdev@...r.kernel.org
Subject: [PATCH AUTOSEL 6.18-5.10] net: amd-xgbe: use EOPNOTSUPP instead of ENOTSUPP in xgbe_phy_mii_read_c45

From: Alok Tiwari <alok.a.tiwari@...cle.com>

[ Upstream commit bd853a59a87ecf9fc060dd54b10a3a94e9786182 ]

The MDIO read callback xgbe_phy_mii_read_c45() can propagate its return
value up through phylink_mii_ioctl() to user space via netdev ioctls such
as SIOCGMIIREG. Returning ENOTSUPP results in user space seeing
"Unknown error", since ENOTSUPP is not a standard errno value.

Replace ENOTSUPP with EOPNOTSUPP to align with the MDIO core’s
usage and ensure user space receives a proper "Operation not supported"
error instead of an unknown code.

Signed-off-by: Alok Tiwari <alok.a.tiwari@...cle.com>
Reviewed-by: Simon Horman <horms@...nel.org>
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@....com>
Link: https://patch.msgid.link/20251015025751.1532149-1-alok.a.tiwari@oracle.com
Signed-off-by: Jakub Kicinski <kuba@...nel.org>
Signed-off-by: Sasha Levin <sashal@...nel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information I need for a complete analysis. Let me
compile my findings:

## Complete Analysis

### 1. COMMIT MESSAGE ANALYSIS

**Subject:** `net: amd-xgbe: use EOPNOTSUPP instead of ENOTSUPP in
xgbe_phy_mii_read_c45`

**Key points from the commit message:**
- The MDIO read callback `xgbe_phy_mii_read_c45()` can propagate its
  return value up through `phylink_mii_ioctl()` to **user space** via
  netdev ioctls (SIOCGMIIREG)
- Returning ENOTSUPP (error code 524) results in **user space seeing
  "Unknown error"** since ENOTSUPP is not a standard errno value
- The fix replaces ENOTSUPP (524, kernel-internal) with EOPNOTSUPP (95,
  standard POSIX errno)

**Tags present:**
- `Signed-off-by: Alok Tiwari <alok.a.tiwari@...cle.com>`
- `Reviewed-by: Simon Horman <horms@...nel.org>` - Important! This is a
  well-known networking reviewer
- `Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@....com>` - AMD
  maintainer acknowledgment
- `Signed-off-by: Jakub Kicinski <kuba@...nel.org>` - Networking
  maintainer

**Missing tags:**
- No `Cc: stable@...r.kernel.org`
- No `Fixes:` tag (though one should have been added: `Fixes:
  070f6186a2f1d ("amd-xgbe: Separate C22 and C45 transactions")`)

### 2. CODE CHANGE ANALYSIS

**The diff (single line change):**
```c
- ret = -ENOTSUPP;
+               ret = -EOPNOTSUPP;
```

**Location:** `drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c`, function
`xgbe_phy_mii_read_c45()`

**Context Analysis:**
The function handles MII (Media Independent Interface) Clause 45 read
operations:

```c
static int xgbe_phy_mii_read_c45(struct mii_bus *mii, int addr, int
devad,
                                 int reg)
{
        ...
        if (phy_data->conn_type == XGBE_CONN_TYPE_SFP)
                ret = -EOPNOTSUPP;   // Already correct
        else if (phy_data->conn_type & XGBE_CONN_TYPE_MDIO)
                ret = xgbe_phy_mdio_mii_read_c45(pdata, addr, devad,
reg);
        else
                ret = -ENOTSUPP;     // BUG: should be -EOPNOTSUPP
```

**Root cause of the bug:**
- Commit `070f6186a2f1d` ("amd-xgbe: Separate C22 and C45 transactions")
  introduced this function in January 2023
- Andrew Lunn correctly used `EOPNOTSUPP` for the SFP case
- But **inconsistently/accidentally** used `ENOTSUPP` for the final else
  branch
- This is clearly an oversight/typo during the refactoring

**Technical explanation of ENOTSUPP vs EOPNOTSUPP:**
- `ENOTSUPP` (524): Defined in `include/linux/errno.h` as a **kernel-
  internal error code** originally for NFSv3 protocol
- `EOPNOTSUPP` (95): Defined in `include/uapi/asm-generic/errno.h` as a
  **standard POSIX errno** "Operation not supported"
- When kernel code returns errors through syscalls/ioctls, it must use
  standard POSIX errno values
- User-space `strerror()` doesn't know what errno 524 means → "Unknown
  error 524"

### 3. CLASSIFICATION

- **Type:** Bug fix (incorrect error code returned to userspace)
- **NOT a feature addition**
- **NOT a device ID, quirk, or DT update**
- This is fixing **incorrect API behavior** - returning a non-standard
  errno to userspace

### 4. SCOPE AND RISK ASSESSMENT

**Size:**
- 1 line changed
- 1 file touched
- **Minimal scope**

**Risk:**
- **EXTREMELY LOW** - This is a pure error code change
- Cannot cause crashes, data corruption, or regressions
- The error path itself is unchanged; only the error code returned
  differs
- Changes from an unknown error (524) to a proper descriptive error (95)

**Similar precedents:**
I found multiple similar fixes already merged:
- `f82acf6fb4211` - ixgbe: use EOPNOTSUPP instead of ENOTSUPP
- `21d08d1c4c29f` - igc: use EOPNOTSUPP instead of ENOTSUPP
- `bc73c5885c606` - igb: use EOPNOTSUPP instead of ENOTSUPP
- `ab15aabac028a` - mtd: rawnand: qcom: Use EOPNOTSUPP instead of
  ENOTSUPP

All these have `Fixes:` tags and address the same class of bug.

### 5. USER IMPACT

**Who is affected:**
- Users of AMD XGBE network devices (10Gb Ethernet)
- Any tool or application using SIOCGMIIREG/SIOCGMIIPHY ioctls
- Diagnostic tools like `mii-tool`, `ethtool`, or custom MDIO access
  utilities

**Severity:**
- When the hardware connection type is neither SFP nor MDIO, the user
  sees:
  - **Before fix:** "Unknown error 524" - confusing, hard to debug
  - **After fix:** "Operation not supported" - clear, standard message

**Real-world impact:**
- This is a **user-visible bug** affecting error reporting
- While not a crash or data corruption, it degrades user experience
- Makes debugging and troubleshooting harder

### 6. STABILITY INDICATORS

- `Reviewed-by: Simon Horman` - Very experienced networking reviewer
- `Acked-by: Shyam Sundar S K` - AMD driver maintainer
- `Signed-off-by: Jakub Kicinski` - Net maintainer
- The change is trivially correct - single character difference in error
  code

### 7. DEPENDENCY CHECK

**Dependencies:** None
- The change is self-contained
- No other commits are required
- The affected code exists in all stable kernels from 6.3 onwards (where
  `070f6186a2f1d` was merged)

**Affected stable trees:**
- 6.6.y: **Yes** - Contains the buggy commit, needs this fix
- 6.10.y through 6.18.y: **Yes** - All affected
- 6.1.y and earlier: **No** - The buggy code doesn't exist (function was
  introduced in 6.3)

### FINAL ASSESSMENT

**Pros for backporting:**
1. ✅ Fixes a **real user-visible bug** - users see "Unknown error"
   messages
2. ✅ **Extremely small and surgical** - single 1-line change
3. ✅ **Obviously correct** - changing from kernel-internal errno to
   standard POSIX errno
4. ✅ **Zero risk** - cannot introduce regressions
5. ✅ **Well reviewed** - by networking and AMD maintainers
6. ✅ **Precedent exists** - similar fixes have been applied to stable
   trees
7. ✅ **No dependencies** - applies cleanly to all affected stable trees

**Cons:**
1. ⚠️ No `Cc: stable@...r.kernel.org` tag - author didn't explicitly
   request stable backport
2. ⚠️ No `Fixes:` tag - though it clearly should have had one pointing
   to `070f6186a2f1d`
3. ⚠️ Minor severity - not a crash or security issue

**Conclusion:**

This commit fixes a clear bug where an incorrect kernel-internal error
code (ENOTSUPP, 524) is returned to userspace instead of the proper
POSIX errno (EOPNOTSUPP, 95). This causes user-space tools to display
"Unknown error 524" instead of "Operation not supported", degrading user
experience and making debugging harder.

The fix is:
- Trivially small (1 line)
- Obviously correct
- Zero risk of regression
- Well-reviewed by subsystem experts
- Has precedent with identical fixes in other drivers

While the commit lacks explicit `Cc: stable` and `Fixes:` tags, it meets
all the criteria from stable-kernel-rules.rst: it fixes "a real bug that
bothers people", is small, obviously correct, and tested (reviewed by
multiple maintainers).

**YES**

 drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
index a56efc1bee339..35a381a83647d 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
@@ -668,7 +668,7 @@ static int xgbe_phy_mii_read_c45(struct mii_bus *mii, int addr, int devad,
 	else if (phy_data->conn_type & XGBE_CONN_TYPE_MDIO)
 		ret = xgbe_phy_mdio_mii_read_c45(pdata, addr, devad, reg);
 	else
-		ret = -ENOTSUPP;
+		ret = -EOPNOTSUPP;
 
 	xgbe_phy_put_comm_ownership(pdata);
 
-- 
2.51.0

Powered by blists - more mailing lists