Re: [net-next,v5] net: sfp: extend SMBus support
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-19 19:45:45
Also in:
lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: sfp: extend SMBus support This patch extends SMBus support in the SFP driver from single-byte accesses to also support SMBus I2C block and word access, enabling more SMBus controllers to work with SFP modules.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c index 84bef5099dda..8f0b34a93ae8 100644 --- a/drivers/net/phy/sfp.c +++ b/drivers/net/phy/sfp.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -759,31 +773,55 @@ static int sfp_smbus_byte_write(struct sfp *sfp, bool a2, u8 dev_addr, +static int sfp_smbus_write(struct sfp *sfp, bool a2, u8 dev_addr, void *buf, + size_t len) { union i2c_smbus_data smbus_data; u8 bus_addr = a2 ? 0x51 : 0x50; + size_t this_len, transferred; + u32 functionality; u8 *data = buf; int ret; + functionality = i2c_get_functionality(sfp->i2c); + while (len) { - smbus_data.byte = *data; - ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0, - I2C_SMBUS_WRITE, dev_addr, - I2C_SMBUS_BYTE_DATA, &smbus_data); - if (ret) + this_len = min(len, sfp->i2c_max_block_size); + + if (this_len > 2 && + functionality & I2C_FUNC_SMBUS_WRITE_I2C_BLOCK) { + smbus_data.block[0] = this_len; + memcpy(&smbus_data.block[1], data, this_len); + + ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0, + I2C_SMBUS_WRITE, dev_addr, + I2C_SMBUS_WORD_DATA, &smbus_data);
^^^^^^^^^^^^^^^^^
Should this be I2C_SMBUS_I2C_BLOCK_DATA instead of I2C_SMBUS_WORD_DATA?
The code checks for I2C_FUNC_SMBUS_WRITE_I2C_BLOCK support and sets up
smbus_data.block[] with the length byte and data for a block transfer,
but then calls i2c_smbus_xfer() with I2C_SMBUS_WORD_DATA protocol.
For comparison, the read function correctly uses I2C_SMBUS_I2C_BLOCK_DATA
when I2C_FUNC_SMBUS_READ_I2C_BLOCK is supported:
if (this_len > 2 &&
functionality & I2C_FUNC_SMBUS_READ_I2C_BLOCK) {
smbus_data.block[0] = this_len;
ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
I2C_SMBUS_READ, dev_addr,
I2C_SMBUS_I2C_BLOCK_DATA, &smbus_data);
With I2C_SMBUS_WORD_DATA, only 2 bytes from smbus_data.word would be
written instead of the this_len bytes set up in smbus_data.block[].
The pointer then advances by transferred (this_len) bytes, effectively
skipping data that was never written to the device.
+ transferred = this_len;
[ ... ] -- pw-bot: cr