* Re: [PATCH net-next v7 2/2] net: sfp: extend SMBus support
2026-05-07 9:33 ` [PATCH net-next v7 2/2] net: sfp: extend SMBus support Jonas Jelonek
@ 2026-05-07 20:45 ` Maxime Chevallier
2026-05-10 16:47 ` Simon Horman
2026-05-15 7:33 ` Maxime Chevallier
2 siblings, 0 replies; 9+ messages in thread
From: Maxime Chevallier @ 2026-05-07 20:45 UTC (permalink / raw)
To: Jonas Jelonek, Russell King, Andrew Lunn, Heiner Kallweit,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Bjørn Mork
On 07/05/2026 11:33, Jonas Jelonek wrote:
> Commit 7662abf4db94 ("net: phy: sfp: Add support for SMBus module access")
> added SMBus access for SFP modules, but limited it to single-byte
> transfers. As a side effect, hwmon is disabled (16-bit reads cannot be
> guaranteed atomic) and a warning is printed.
>
> Many SMBus-only I2C controllers in the wild support more than just
> byte access, and SFP cages are often wired to such controllers
> rather than to a full-featured I2C controller -- e.g. the SMBus
> controllers in the Realtek longan and mango SoCs, which advertise
> word access and I2C block reads. Today, they cannot drive an SFP at
> all without falling back to the byte-only path.
>
> Extend sfp_smbus_read()/sfp_smbus_write() so that, in addition to
> the existing byte access, they also use SMBus word access and SMBus
> I2C block access whenever the adapter advertises them. Both
> directions are handled in a single read and a single write helper
> that pick the largest supported transfer per chunk and fall back as
> needed.
>
> I2C-block is preferred unconditionally when available: the protocol
> carries any length 1..32, so it can serve every chunk -- including
> the 1- and 2-byte tails -- without help from word or byte access.
> Note that this requires I2C_FUNC_SMBUS_I2C_BLOCK, which reads a
> caller-specified number of bytes. This deviates from the official
> SMBus Block Read (length is supplied by the slave) but is widely
> supported by Linux I2C controllers/drivers.
>
> Capability matrix this implementation supports:
>
> - BYTE only: works (unchanged behaviour); 1-byte
> xfers, hwmon disabled.
> - BYTE + WORD: word for >=2-byte chunks, byte for
> trailing odd byte.
> - I2C_BLOCK present (with or
> without BYTE/WORD): block as the universal transport for
> every chunk.
> - WORD only (no BYTE/BLOCK): accepted with WARN_ONCE. Even-length
> transfers work; odd-length transfers
> (e.g. the 3-byte cotsworks fixup
> write) hit the BYTE branch which the
> adapter does not implement, so the
> xfer returns an error and the
> operation is aborted. No mainline
> I2C driver was found to advertise
> WORD without BYTE; the warning lets
> us learn about it if it ever shows
> up.
>
> Adapters with asymmetric R/W capabilities (e.g. only READ_I2C_BLOCK
> but not WRITE_I2C_BLOCK) remain functionally correct -- the
> per-iteration fallback uses the direction-specific bits -- but the
> shared i2c_max_block_size is sized by the all-bits-set check, so a
> transfer in the better-supported direction is not upgraded. None of
> the mainline I2C bus drivers surveyed during review advertise such
> asymmetry; promoting i2c_max_block_size to per-direction sizes can
> be revisited if needed.
>
> Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Thank you !
Maxime
> ---
> drivers/net/phy/sfp.c | 134 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 107 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index e58e29a1e8d2..16d41d7ee632 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -14,6 +14,7 @@
> #include <linux/platform_device.h>
> #include <linux/rtnetlink.h>
> #include <linux/slab.h>
> +#include <linux/unaligned.h>
> #include <linux/workqueue.h>
>
> #include "sfp.h"
> @@ -756,50 +757,110 @@ static int sfp_i2c_write(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
> return ret == ARRAY_SIZE(msgs) ? len : 0;
> }
>
> -static int sfp_smbus_byte_read(struct sfp *sfp, bool a2, u8 dev_addr,
> - void *buf, size_t len)
> +static int sfp_smbus_read(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;
>
> - while (len) {
> - ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
> - I2C_SMBUS_READ, dev_addr,
> - I2C_SMBUS_BYTE_DATA, &smbus_data);
> - if (ret < 0)
> - return ret;
> + functionality = i2c_get_functionality(sfp->i2c);
>
> - *data = smbus_data.byte;
> + while (len) {
> + this_len = min(len, sfp->i2c_block_size);
> +
> + if (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);
> + if (ret < 0)
> + return ret;
> +
> + memcpy(data, &smbus_data.block[1], this_len);
> + transferred = this_len;
> + } else if (this_len >= 2 &&
> + (functionality & I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> + ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
> + I2C_SMBUS_READ, dev_addr,
> + I2C_SMBUS_WORD_DATA, &smbus_data);
> + if (ret < 0)
> + return ret;
> +
> + put_unaligned_le16(smbus_data.word, data);
> + transferred = 2;
> + } else {
> + ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
> + I2C_SMBUS_READ, dev_addr,
> + I2C_SMBUS_BYTE_DATA, &smbus_data);
> + if (ret < 0)
> + return ret;
> +
> + *data = smbus_data.byte;
> + transferred = 1;
> + }
>
> - len--;
> - data++;
> - dev_addr++;
> + data += transferred;
> + len -= transferred;
> + dev_addr += transferred;
> }
>
> return data - (u8 *)buf;
> }
>
> -static int sfp_smbus_byte_write(struct sfp *sfp, bool a2, u8 dev_addr,
> - void *buf, size_t len)
> +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)
> - return ret;
> + this_len = min(len, sfp->i2c_block_size);
> +
> + if (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_I2C_BLOCK_DATA, &smbus_data);
> + if (ret < 0)
> + return ret;
> +
> + transferred = this_len;
> + } else if (this_len >= 2 &&
> + (functionality & I2C_FUNC_SMBUS_WRITE_WORD_DATA)) {
> + smbus_data.word = get_unaligned_le16(data);
> + ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
> + I2C_SMBUS_WRITE, dev_addr,
> + I2C_SMBUS_WORD_DATA, &smbus_data);
> + if (ret < 0)
> + return ret;
> +
> + transferred = 2;
> + } else {
> + 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 < 0)
> + return ret;
> +
> + transferred = 1;
> + }
>
> - len--;
> - data++;
> - dev_addr++;
> + data += transferred;
> + len -= transferred;
> + dev_addr += transferred;
> }
>
> return data - (u8 *)buf;
> @@ -815,10 +876,29 @@ static int sfp_i2c_configure(struct sfp *sfp, struct i2c_adapter *i2c)
> sfp->read = sfp_i2c_read;
> sfp->write = sfp_i2c_write;
> max_block_size = SFP_EEPROM_BLOCK_SIZE;
> - } else if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_BYTE_DATA)) {
> - sfp->read = sfp_smbus_byte_read;
> - sfp->write = sfp_smbus_byte_write;
> - max_block_size = 1;
> + } else if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_BYTE_DATA) ||
> + i2c_check_functionality(i2c, I2C_FUNC_SMBUS_I2C_BLOCK)) {
> + /* Either protocol alone covers any length: I2C-block carries
> + * 1..32 bytes per xfer, byte iterates one byte at a time.
> + */
> + sfp->read = sfp_smbus_read;
> + sfp->write = sfp_smbus_write;
> +
> + if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_I2C_BLOCK))
> + max_block_size = SFP_EEPROM_BLOCK_SIZE;
> + else if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_WORD_DATA))
> + max_block_size = 2;
> + else
> + max_block_size = 1;
> + } else if (WARN_ONCE(i2c_check_functionality(i2c, I2C_FUNC_SMBUS_WORD_DATA),
> + "SMBus word-only adapter; odd-length transfers will fail\n")) {
> + /* Word-only: even-length xfers work; odd-length xfers fall
> + * to BYTE, which the adapter does not advertise and will
> + * likely fail.
> + */
> + sfp->read = sfp_smbus_read;
> + sfp->write = sfp_smbus_write;
> + max_block_size = 2;
> } else {
> sfp->i2c = NULL;
> return -EINVAL;
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next v7 2/2] net: sfp: extend SMBus support
2026-05-07 9:33 ` [PATCH net-next v7 2/2] net: sfp: extend SMBus support Jonas Jelonek
2026-05-07 20:45 ` Maxime Chevallier
@ 2026-05-10 16:47 ` Simon Horman
2026-05-15 7:33 ` Maxime Chevallier
2 siblings, 0 replies; 9+ messages in thread
From: Simon Horman @ 2026-05-10 16:47 UTC (permalink / raw)
To: jelonek.jonas
Cc: 'Simon Horman', linux, andrew, hkallweit1, davem,
edumazet, kuba, pabeni, maxime.chevallier, netdev, linux-kernel,
bjorn
From: 'Simon Horman' <horms@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net: sfp: extend SMBus support
This commit extends SFP module SMBus support by adding word and I2C
block access capabilities. It implements single read and write helpers
that pick the largest supported transfer per chunk and fall back to smaller
chunks as needed.
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index e58e29a1e8d27..16d41d7ee6328 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
[ ... ]
> @@ -756,50 +757,110 @@ static int sfp_i2c_write(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
[ ... ]
> +static int sfp_smbus_read(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) {
> + this_len = min(len, sfp->i2c_block_size);
> +
> + if (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);
> + if (ret < 0)
> + return ret;
> +
> + memcpy(data, &smbus_data.block[1], this_len);
Can this sequence leak uninitialized kernel stack memory to the output buffer?
When performing an I2C_SMBUS_I2C_BLOCK_DATA transfer, i2c_smbus_xfer()
updates smbus_data.block[0] with the actual number of bytes read from the
bus upon success.
If the adapter or device returns fewer bytes than requested (a short read),
smbus_data.block[0] will be less than this_len. Because memcpy()
unconditionally copies this_len bytes, it appears it would overrun the valid
data and copy uninitialized memory from the smbus_data stack variable.
Since the SFP EEPROM data is accessible to userspace via ethtool, could this
expose uninitialized kernel stack memory to userspace?
Should the code verify that smbus_data.block[0] == this_len, or use
smbus_data.block[0] as the actual transferred length?
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next v7 2/2] net: sfp: extend SMBus support
2026-05-07 9:33 ` [PATCH net-next v7 2/2] net: sfp: extend SMBus support Jonas Jelonek
2026-05-07 20:45 ` Maxime Chevallier
2026-05-10 16:47 ` Simon Horman
@ 2026-05-15 7:33 ` Maxime Chevallier
2026-05-15 8:00 ` Jonas Jelonek
2 siblings, 1 reply; 9+ messages in thread
From: Maxime Chevallier @ 2026-05-15 7:33 UTC (permalink / raw)
To: Jonas Jelonek, Russell King, Andrew Lunn, Heiner Kallweit,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Bjørn Mork
Hi Jonas,
On 5/7/26 11:33, Jonas Jelonek wrote:
> Commit 7662abf4db94 ("net: phy: sfp: Add support for SMBus module access")
> added SMBus access for SFP modules, but limited it to single-byte
> transfers. As a side effect, hwmon is disabled (16-bit reads cannot be
> guaranteed atomic) and a warning is printed.
>
> Many SMBus-only I2C controllers in the wild support more than just
> byte access, and SFP cages are often wired to such controllers
> rather than to a full-featured I2C controller -- e.g. the SMBus
> controllers in the Realtek longan and mango SoCs, which advertise
> word access and I2C block reads. Today, they cannot drive an SFP at
> all without falling back to the byte-only path.
>
> Extend sfp_smbus_read()/sfp_smbus_write() so that, in addition to
> the existing byte access, they also use SMBus word access and SMBus
> I2C block access whenever the adapter advertises them. Both
> directions are handled in a single read and a single write helper
> that pick the largest supported transfer per chunk and fall back as
> needed.
>
> I2C-block is preferred unconditionally when available: the protocol
> carries any length 1..32, so it can serve every chunk -- including
> the 1- and 2-byte tails -- without help from word or byte access.
> Note that this requires I2C_FUNC_SMBUS_I2C_BLOCK, which reads a
> caller-specified number of bytes. This deviates from the official
> SMBus Block Read (length is supplied by the slave) but is widely
> supported by Linux I2C controllers/drivers.
>
> Capability matrix this implementation supports:
>
> - BYTE only: works (unchanged behaviour); 1-byte
> xfers, hwmon disabled.
> - BYTE + WORD: word for >=2-byte chunks, byte for
> trailing odd byte.
> - I2C_BLOCK present (with or
> without BYTE/WORD): block as the universal transport for
> every chunk.
> - WORD only (no BYTE/BLOCK): accepted with WARN_ONCE. Even-length
> transfers work; odd-length transfers
> (e.g. the 3-byte cotsworks fixup
> write) hit the BYTE branch which the
> adapter does not implement, so the
> xfer returns an error and the
> operation is aborted. No mainline
> I2C driver was found to advertise
> WORD without BYTE; the warning lets
> us learn about it if it ever shows
> up.
>
> Adapters with asymmetric R/W capabilities (e.g. only READ_I2C_BLOCK
> but not WRITE_I2C_BLOCK) remain functionally correct -- the
> per-iteration fallback uses the direction-specific bits -- but the
> shared i2c_max_block_size is sized by the all-bits-set check, so a
> transfer in the better-supported direction is not upgraded. None of
> the mainline I2C bus drivers surveyed during review advertise such
> asymmetry; promoting i2c_max_block_size to per-direction sizes can
> be revisited if needed.
>
> Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
I gave this a test and it seems to work fine with all modules I have :)
I tested on both boards that have single-byte smbus, and ones with a
real i2c controller. I had to hack around in the i2c controller to
pretend it only supported smbus, that did the trick.
I'm unsure if you're going to followup with the sashiko review on this
particular patch, it does seem to raise a valid point. I'll followup
with tested-by/reviewed-by on the next iteration :)
Thanks for this work :)
Maxime
> ---
> drivers/net/phy/sfp.c | 134 +++++++++++++++++++++++++++++++++---------
> 1 file changed, 107 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index e58e29a1e8d2..16d41d7ee632 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -14,6 +14,7 @@
> #include <linux/platform_device.h>
> #include <linux/rtnetlink.h>
> #include <linux/slab.h>
> +#include <linux/unaligned.h>
> #include <linux/workqueue.h>
>
> #include "sfp.h"
> @@ -756,50 +757,110 @@ static int sfp_i2c_write(struct sfp *sfp, bool a2, u8 dev_addr, void *buf,
> return ret == ARRAY_SIZE(msgs) ? len : 0;
> }
>
> -static int sfp_smbus_byte_read(struct sfp *sfp, bool a2, u8 dev_addr,
> - void *buf, size_t len)
> +static int sfp_smbus_read(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;
>
> - while (len) {
> - ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
> - I2C_SMBUS_READ, dev_addr,
> - I2C_SMBUS_BYTE_DATA, &smbus_data);
> - if (ret < 0)
> - return ret;
> + functionality = i2c_get_functionality(sfp->i2c);
>
> - *data = smbus_data.byte;
> + while (len) {
> + this_len = min(len, sfp->i2c_block_size);
> +
> + if (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);
> + if (ret < 0)
> + return ret;
> +
> + memcpy(data, &smbus_data.block[1], this_len);
> + transferred = this_len;
> + } else if (this_len >= 2 &&
> + (functionality & I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> + ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
> + I2C_SMBUS_READ, dev_addr,
> + I2C_SMBUS_WORD_DATA, &smbus_data);
> + if (ret < 0)
> + return ret;
> +
> + put_unaligned_le16(smbus_data.word, data);
> + transferred = 2;
> + } else {
> + ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
> + I2C_SMBUS_READ, dev_addr,
> + I2C_SMBUS_BYTE_DATA, &smbus_data);
> + if (ret < 0)
> + return ret;
> +
> + *data = smbus_data.byte;
> + transferred = 1;
> + }
>
> - len--;
> - data++;
> - dev_addr++;
> + data += transferred;
> + len -= transferred;
> + dev_addr += transferred;
> }
>
> return data - (u8 *)buf;
> }
>
> -static int sfp_smbus_byte_write(struct sfp *sfp, bool a2, u8 dev_addr,
> - void *buf, size_t len)
> +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)
> - return ret;
> + this_len = min(len, sfp->i2c_block_size);
> +
> + if (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_I2C_BLOCK_DATA, &smbus_data);
> + if (ret < 0)
> + return ret;
> +
> + transferred = this_len;
> + } else if (this_len >= 2 &&
> + (functionality & I2C_FUNC_SMBUS_WRITE_WORD_DATA)) {
> + smbus_data.word = get_unaligned_le16(data);
> + ret = i2c_smbus_xfer(sfp->i2c, bus_addr, 0,
> + I2C_SMBUS_WRITE, dev_addr,
> + I2C_SMBUS_WORD_DATA, &smbus_data);
> + if (ret < 0)
> + return ret;
> +
> + transferred = 2;
> + } else {
> + 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 < 0)
> + return ret;
> +
> + transferred = 1;
> + }
>
> - len--;
> - data++;
> - dev_addr++;
> + data += transferred;
> + len -= transferred;
> + dev_addr += transferred;
> }
>
> return data - (u8 *)buf;
> @@ -815,10 +876,29 @@ static int sfp_i2c_configure(struct sfp *sfp, struct i2c_adapter *i2c)
> sfp->read = sfp_i2c_read;
> sfp->write = sfp_i2c_write;
> max_block_size = SFP_EEPROM_BLOCK_SIZE;
> - } else if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_BYTE_DATA)) {
> - sfp->read = sfp_smbus_byte_read;
> - sfp->write = sfp_smbus_byte_write;
> - max_block_size = 1;
> + } else if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_BYTE_DATA) ||
> + i2c_check_functionality(i2c, I2C_FUNC_SMBUS_I2C_BLOCK)) {
> + /* Either protocol alone covers any length: I2C-block carries
> + * 1..32 bytes per xfer, byte iterates one byte at a time.
> + */
> + sfp->read = sfp_smbus_read;
> + sfp->write = sfp_smbus_write;
> +
> + if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_I2C_BLOCK))
> + max_block_size = SFP_EEPROM_BLOCK_SIZE;
> + else if (i2c_check_functionality(i2c, I2C_FUNC_SMBUS_WORD_DATA))
> + max_block_size = 2;
> + else
> + max_block_size = 1;
> + } else if (WARN_ONCE(i2c_check_functionality(i2c, I2C_FUNC_SMBUS_WORD_DATA),
> + "SMBus word-only adapter; odd-length transfers will fail\n")) {
> + /* Word-only: even-length xfers work; odd-length xfers fall
> + * to BYTE, which the adapter does not advertise and will
> + * likely fail.
> + */
> + sfp->read = sfp_smbus_read;
> + sfp->write = sfp_smbus_write;
> + max_block_size = 2;
> } else {
> sfp->i2c = NULL;
> return -EINVAL;
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH net-next v7 2/2] net: sfp: extend SMBus support
2026-05-15 7:33 ` Maxime Chevallier
@ 2026-05-15 8:00 ` Jonas Jelonek
0 siblings, 0 replies; 9+ messages in thread
From: Jonas Jelonek @ 2026-05-15 8:00 UTC (permalink / raw)
To: Maxime Chevallier, Russell King, Andrew Lunn, Heiner Kallweit,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Bjørn Mork
Hi Maxime,
On 15.05.26 09:33, Maxime Chevallier wrote:
> Hi Jonas,
>
> [...]
> I gave this a test and it seems to work fine with all modules I have :)
>
> I tested on both boards that have single-byte smbus, and ones with a real i2c controller. I had to hack around in the i2c controller to pretend it only supported smbus, that did the trick.
Happy to hear that, thanks for the testing effort :)
>
> I'm unsure if you're going to followup with the sashiko review on this particular patch, it does seem to raise a valid point. I'll followup with tested-by/reviewed-by on the next iteration :)
>
Yes, that's a valid point. I'll provide a follow up where this is fixed.
> Thanks for this work :)
>
> Maxime
>
> [...]
Regards,
Jonas Jelonek
^ permalink raw reply [flat|nested] 9+ messages in thread