On Mon, 16 Feb 2026 16:49:12 -0600 Tiernan Hubble wrote:

Commit 853a2944aaf3 ("net: atlantic: support reading SFP module info")
added support for reading SFP module info on AQC100-based cards. However,
it only supports reading directly from the controller's hardware
registers, and this does not seem to be supported on certain cards,
including my TRENDnet TEG-10GECSFP V3. "ethtool -m" times out when reading
certain registers, even when I increase the read poll timeout values.

The DPDK "atlantic" driver reads module info via firmware calls instead of
directly reading the hardware registers, provided that the NIC's firmware
version supports it.

This change adapts the DPDK firmware call code to the kernel driver. It
preserves the old hardware-based module read code as a fallback when the
firmware does not support it, to avoid breaking cards that are currently
working.

Tested on 2 different TRENDnet TEG-10GECSFP V3 cards, both with firmware
version 3.1.121 (current at the time of this patch). Both cards correctly
reported module info for a passive DAC cable and 2 different 10G optical
transceivers.

Fixes: 853a2944aaf3 ("net: atlantic: support reading SFP module info")
Signed-off-by: Tiernan Hubble <redacted>

Overall looks quite nice, but I don't think it qualifies as a fix.
Fix means something regressed from previous release, crashes, etc
For the adapters in question IIUC ethtool -m has never worked before.
Please drop the Fixes tag and repost next week for net-next.

Some drive by comments below..

maybe add a temp variable:

	const struct aq_fw_ops *ops = aq_nic->aq_fw_ops;

+	if (aq_nic->aq_fw_ops->read_module_eeprom) {
+		err = aq_nic->aq_fw_ops->read_module_eeprom(aq_nic->aq_hw,

to avoid the long lines and awkward wrapping ?

+			dev_addr, reg_start_addr, len, data);
+
+		/* If the only error is that the firmware version doesn't
+		 * support reading EEPROM, we can still attempt to read it
+		 * directly from the hardware if supported.
+		 */
+		if (err != -EOPNOTSUPP)
+			return err;
+	}
+
+	if (aq_nic->aq_hw_ops->hw_read_module_eeprom)
+		err = aq_nic->aq_hw_ops->hw_read_module_eeprom(aq_nic->aq_hw,

and here

+			dev_addr, reg_start_addr, len, data);
+
+	return err;
+}

+static int aq_fw2x_read_module_eeprom(struct aq_hw_s *self, u8 dev_addr,
+				      u8 reg_start_addr, int len, u8 *data)
+{
+	u32 low_status, orig_low_status, low_req = 0;
+	u32 res_bytes_remain_cnt = len % sizeof(u32);
+	u32 res_dword_cnt = len / sizeof(u32);
+	struct smbus_request request = { 0 };
+	u32 req_dword_cnt;
+	u32 result = 0;
+	u32 caps_lo;
+	u32 offset;
+	int err;
+
+	caps_lo = aq_fw2x_get_link_capabilities(self);
+	if (!(caps_lo & BIT(CAPS_LO_SMBUS_READ)))
+		return -EOPNOTSUPP;
+
+	request.msg_id = 0;
+	request.device_id = dev_addr;
+	request.address = reg_start_addr;
+	request.length = len;
+
+	/* Write SMBUS request to cfg memory */
+	req_dword_cnt = (sizeof(request) + sizeof(u32) - 1) / sizeof(u32);

DIV_ROUND_UP()

+	err = hw_atl_write_fwcfg_dwords(self, (void *)&request, req_dword_cnt);
+	if (err < 0)
+		return err;
+
+	/* Toggle 0x368.CAPS_LO_SMBUS_READ bit */
+	low_req = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL_ADDR);
+	orig_low_status = low_req & BIT(CAPS_LO_SMBUS_READ);

the mixing of BIT(CAPS_LO_SMBUS_READ)..

+	low_req ^= HW_ATL_FW2X_CAP_SMBUS_READ;

.. and HW_ATL_FW2X_CAP_SMBUS_READ which is defined to

+#define HW_ATL_FW2X_CAP_SMBUS_READ       BIT(CAPS_LO_SMBUS_READ)

seems unnecessary and confusing?
-- 
pw-bot: cr