On 9/12/24 08:41, Krzysztof Olędzki wrote:
Due to HW/FW limitation, page A2h (I2C 0x51) may not be available.
Do not mask the problem so the userspace can properly handle it.
When returning the error to the userspace, use -EIO instead of
"err" because it holds MAD_STATUS.
Fixes: f5826c8c9d57 ("net/mlx4_en: Fix wrong return value on ioctl EEPROM query failure")
Fixes: 32a173c7f9e9 ("net/mlx4_core: Introduce mlx4_get_module_info for cable module info reading")
Signed-off-by: Krzysztof Piotr Oledzki <ole@xxxxxx>
---
drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/port.c | 9 +--------
2 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
index 4c985d62af12..677917168bd5 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c
@@ -2094,7 +2094,7 @@ static int mlx4_en_get_module_eeprom(struct net_device *dev,
en_err(priv,
"mlx4_get_module_info i(%d) offset(%d) bytes_to_read(%d) - FAILED (0x%x)\n",
i, offset, ee->len - i, ret);
- return ret;
+ return -EIO;
here you are masking also all other explicit error paths of
mlx4_get_module_info(), what is not good in general, I would instead
mask below (see next comment)
}
i += ret;
diff --git a/drivers/net/ethernet/mellanox/mlx4/port.c b/drivers/net/ethernet/mellanox/mlx4/port.c
index 1ebd459d1d21..8c2a384404f9 100644
--- a/drivers/net/ethernet/mellanox/mlx4/port.c
+++ b/drivers/net/ethernet/mellanox/mlx4/port.c
@@ -2198,14 +2198,7 @@ int mlx4_get_module_info(struct mlx4_dev *dev, u8 port,
MLX4_ATTR_CABLE_INFO, port, i2c_addr, offset, size,
ret, cable_info_mad_err_str(ret));
- if (i2c_addr == I2C_ADDR_HIGH &&
- MAD_STATUS_2_CABLE_ERR(ret) == CABLE_INF_I2C_ADDR)
- /* Some SFP cables do not support i2c slave
- * address 0x51 (high page), abort silently.
- */
- ret = 0;
- else
- ret = -ret;
+ ret = -ret;
this is the only place that mlx4_get_module_info() returns non standard
error code so, I believe, it's here where we want to overwrite with -EIO
then you could limit to just a single Fixes: tag (the second one)
