Re: [PATCH net-next v2] net: phy: sfp: detect presence via I2C when no MOD_DEF0 GPIO

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi Greg,

On 6/4/26 17:16, Greg Patrick wrote:
> An SFP cage (compatible "sff,sfp") whose MOD_DEF0 signal is not wired to a
> GPIO currently falls back to sff_gpio_get_state(), which unconditionally
> reports the module as present. An empty cage therefore fails its probe and
> is parked in SFP_MOD_ERROR forever; because SFP_F_PRESENT never deasserts
> there is no REMOVE event to recover the state machine, so a module inserted
> after boot is never detected, and empty cages spam -EIO at boot.
> 
> This affects boards that route none of the cage presence signal to a
> software-readable input. On the NicGiga S100-0800S-M (RTL9303, 8x SFP+) the
> cage I2C bus is the switch's SMBus master; TX_DISABLE is driven via a
> PCA9534 I/O expander, but no MOD_ABS/MOD_DEF0 line reaches a readable GPIO
> (the RTL9303 gpio0 lines read stuck-low, the single PCA9534 is fully
> consumed by TX_DISABLE, and there is no RTL8231).
> 
> For such an SFP cage, derive presence from a throttled single-byte I2C read
> of the module EEPROM instead: an ACK asserts SFP_F_PRESENT, two consecutive
> failures clear it (to ride out a transient error on a live module). The
> existing poll then emits SFP_E_INSERT / SFP_E_REMOVE normally, giving
> working hot-plug and silencing the boot-time -EIO spam on empty cages.
> 
> A soldered-down module (compatible "sff,sff") has no presence signal and is
> genuinely always present, so it continues to use sff_gpio_get_state(); the
> new path is gated on the cage type advertising SFP_F_PRESENT.
> 
> Signed-off-by: Greg Patrick <gregspatrick@hotmail.com>
> ---
> v2:
>  - Keep sff_gpio_get_state() and gate the new I2C-probe presence on the
>    cage advertising SFP_F_PRESENT (an "sff,sfp" cage), so a soldered
>    "sff,sff" module keeps its always-present behaviour. (Andrew Lunn)
>  - Reword the commit message to state precisely that no presence signal
>    reaches a readable input on the affected board, and that the cage bus
>    is the RTL9303 SMBus master. (Andrew Lunn)
>  - On using a 0-byte read like i2cdetect: not available on this board --
>    the cage master advertises only I2C_FUNC_SMBUS_BYTE_DATA (no
>    I2C_FUNC_I2C, no SMBus Quick Command), so sfp_i2c_configure() selects
>    sfp_smbus_byte_read() and a 1-byte read is the lightest presence
>    primitive; it also works through sfp->read on both raw-I2C and
>    SMBus-only adapters, whereas a zero-length transfer does not. (Andrew Lunn)
> 
> v1: https://lore.kernel.org/netdev/20260602235528.2795028-1-gregspatrick@hotmail.com/
> 
>  drivers/net/phy/sfp.c | 77 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 74 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index 376c705a9..056bbe6de 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -206,6 +206,11 @@ static const enum gpiod_flags gpio_flags[] = {
>  #define T_PROBE_RETRY_SLOW	msecs_to_jiffies(5000)
>  #define R_PROBE_RETRY_SLOW	12
>  
> +/* Interval at which sfp_i2c_get_state() re-probes the I2C bus for module
> + * presence on boards without a MOD_DEF0 GPIO (see sfp_i2c_get_state()).
> + */
> +#define T_PROBE_PRESENT		msecs_to_jiffies(2000)

I'm genuinly wondering if 2s for this isn't too slow. With a 4s total including the
retry, this definitely leaves time for a user to swap the module with another
different one, and we wouldn't see the hotswap from linux here, leading
to very interesting problems. I think we need a faster removal-detection time.

I'd like to give this patch a try, however I won't be able to do so
before at least saturday :(

Feel free to send a V3 though

Maxime