Re: [PATCH net-next v9 1/3] net: sfp: initialize i2c_block_size at adapter configure time

[Jonas Jelonek <jelonek.jonas@gmail.com>]

Hi Jakub,

On 04.06.26 03:06, Jakub Kicinski wrote:
> On Thu, 28 May 2026 20:52:40 +0000 Jonas Jelonek wrote:
>> sfp->i2c_block_size is only assigned in sfp_sm_mod_probe(), which runs
>> from the state machine timer after SFP_F_PRESENT has been set. Between
>> those two points, sfp_module_eeprom() (the ethtool -m callback) gates
>> only on SFP_F_PRESENT and can be entered with i2c_block_size still at
>> its kzalloc'd value of 0.
>>
>> On a pure-I2C adapter, sfp_i2c_read() then issues an i2c_transfer()
>> with msgs[1].len = 0 inside a loop that subtracts this_len from len
>> each iteration; on adapters that succeed a zero-length read the loop
>> never advances, spinning while holding rtnl_lock.
>>
>> This was previously addressed by initializing i2c_block_size in
>> sfp_alloc() (commit 813c2dd78618), but the initialization was dropped
>> when i2c_block_size was split from i2c_max_block_size.
>>
>> Initialize sfp->i2c_block_size from sfp->i2c_max_block_size in
>> sfp_i2c_configure(), so the field is valid as soon as the adapter is
>> known. sfp_sm_mod_probe() still reassigns it on each module insertion
>> to recover from a per-module clamp to 1 (sfp_id_needs_byte_io).
>>
>> Fixes: 7662abf4db94 ("net: phy: sfp: Add support for SMBus module access")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Jonas Jelonek <jelonek.jonas@gmail.com>
> Thanks for splitting this out.
> This is a fix it needs to take the net/Linus route rather than the
> net-next route. I'll apply just patch 1 and you'll have to repost
> patches 2 and 3 on Friday.

Sorry, missed that and was wondering why they stay deferred ^^.
Will send another version soon.

> In the meantime - AI seems to also be saying something the cap being
> potentially off by 1 in patch 2? We add 1 to the len? Maybe I'm
> misunderstanding..
>
> https://sashiko.dev/#/patchset/20260528205242.971410-2-jelonek.jonas@gmail.com

Yes it probably is. This should only affect the I2C path because it adds
another byte there, SMBus doesn't. In my opinion, this needs a fix in the
I2C write path, alongside properly honoring the cap there.

Should I include that here or leave for another series?

Best,
Jonas