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

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

[Greg Patrick]

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)
+
 /* SFP modules appear to always have their PHY configured for bus address
  * 0x56 (which with mdio-i2c, translates to a PHY address of 22).
  * RollBall SFPs access phy via SFP Enhanced Digital Diagnostic Interface
@@ -249,6 +254,13 @@ struct sfp {
 
 	bool need_poll;
 
+	/* I2C-probed presence, for boards without a MOD_DEF0 GPIO.
+	 * Access rules: st_mutex held (updated from the poll/state machine).
+	 */
+	bool i2c_present;
+	u8 i2c_present_nak;
+	unsigned long i2c_present_next;
+
 	/* Access rules:
 	 * state_hw_drive: st_mutex held
 	 * state_hw_mask: st_mutex held
@@ -863,6 +875,44 @@ static int sfp_read(struct sfp *sfp, bool a2, u8 addr, void *buf, size_t len)
 	return sfp->read(sfp, a2, addr, buf, len);
 }
 
+/* Probe whether a module is physically present by attempting a single-byte
+ * I2C read of the EEPROM identifier (an empty cage NAKs). Used as the presence
+ * source on boards that do not wire MOD_DEF0 to a GPIO.
+ */
+static bool sfp_module_present_i2c(struct sfp *sfp)
+{
+	u8 id;
+
+	return sfp_read(sfp, false, SFP_PHYS_ID, &id, sizeof(id)) == sizeof(id);
+}
+
+/* get_state variant for boards without a MOD_DEF0 GPIO. Instead of assuming
+ * the module is always present, derive SFP_F_PRESENT from a throttled I2C
+ * probe so that hot-insertion and removal are detected. A single ACK asserts
+ * presence; two consecutive failures clear it, to ride out a transient I2C
+ * error on a live module.
+ */
+static unsigned int sfp_i2c_get_state(struct sfp *sfp)
+{
+	unsigned int state = sfp_gpio_get_state(sfp);
+
+	if (time_after_eq(jiffies, sfp->i2c_present_next)) {
+		if (sfp_module_present_i2c(sfp)) {
+			sfp->i2c_present = true;
+			sfp->i2c_present_nak = 0;
+		} else if (sfp->i2c_present && ++sfp->i2c_present_nak >= 2) {
+			sfp->i2c_present = false;
+			sfp->i2c_present_nak = 0;
+		}
+		sfp->i2c_present_next = jiffies + T_PROBE_PRESENT;
+	}
+
+	if (sfp->i2c_present)
+		state |= SFP_F_PRESENT;
+
+	return state;
+}
+
 static int sfp_write(struct sfp *sfp, bool a2, u8 addr, void *buf, size_t len)
 {
 	return sfp->write(sfp, a2, addr, buf, len);
@@ -3168,9 +3218,30 @@ static int sfp_probe(struct platform_device *pdev)
 	sfp->get_state = sfp_gpio_get_state;
 	sfp->set_state = sfp_gpio_set_state;
 
-	/* Modules that have no detect signal are always present */
-	if (!(sfp->gpio[GPIO_MODDEF0]))
-		sfp->get_state = sff_gpio_get_state;
+	/* An SFP cage with no MOD_DEF0 GPIO has no hardware presence signal.
+	 * Assuming the module is always present traps an empty cage in
+	 * MOD_ERROR and never detects hot-insertion, so derive presence from a
+	 * throttled I2C probe and poll for changes instead. sfp_i2c_configure()
+	 * has already set i2c_max_block_size; seed i2c_block_size so the
+	 * presence read does not issue a zero-length transfer before the first
+	 * EEPROM read. Seed i2c_present_next to jiffies so the first probe
+	 * happens immediately (a zero value would be in the past relative to
+	 * the negative INITIAL_JIFFIES at boot and delay detection).
+	 *
+	 * A soldered-down module (sff,sff) has no presence signal and is
+	 * genuinely always present, so it keeps the always-present behaviour;
+	 * the I2C probe is gated on the cage type advertising SFP_F_PRESENT.
+	 */
+	if (!sfp->gpio[GPIO_MODDEF0]) {
+		if (sff->gpios & SFP_F_PRESENT) {
+			sfp->get_state = sfp_i2c_get_state;
+			sfp->i2c_block_size = sfp->i2c_max_block_size;
+			sfp->i2c_present_next = jiffies;
+			sfp->need_poll = true;
+		} else {
+			sfp->get_state = sff_gpio_get_state;
+		}
+	}
 
 	device_property_read_u32(&pdev->dev, "maximum-power-milliwatt",
 				 &sfp->max_power_mW);
-- 
2.53.0

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

[Jakub Kicinski]

On Thu,  4 Jun 2026 15:16:14 +0000 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>

Hi Maxime!

I think both Russell and Andrew are (semi-) AFK.
Would you be able to review this patch?

> 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)
> +
>  /* SFP modules appear to always have their PHY configured for bus address
>   * 0x56 (which with mdio-i2c, translates to a PHY address of 22).
>   * RollBall SFPs access phy via SFP Enhanced Digital Diagnostic Interface
> @@ -249,6 +254,13 @@ struct sfp {
>  
>  	bool need_poll;
>  
> +	/* I2C-probed presence, for boards without a MOD_DEF0 GPIO.
> +	 * Access rules: st_mutex held (updated from the poll/state machine).
> +	 */
> +	bool i2c_present;
> +	u8 i2c_present_nak;
> +	unsigned long i2c_present_next;
> +
>  	/* Access rules:
>  	 * state_hw_drive: st_mutex held
>  	 * state_hw_mask: st_mutex held
> @@ -863,6 +875,44 @@ static int sfp_read(struct sfp *sfp, bool a2, u8 addr, void *buf, size_t len)
>  	return sfp->read(sfp, a2, addr, buf, len);
>  }
>  
> +/* Probe whether a module is physically present by attempting a single-byte
> + * I2C read of the EEPROM identifier (an empty cage NAKs). Used as the presence
> + * source on boards that do not wire MOD_DEF0 to a GPIO.
> + */
> +static bool sfp_module_present_i2c(struct sfp *sfp)
> +{
> +	u8 id;
> +
> +	return sfp_read(sfp, false, SFP_PHYS_ID, &id, sizeof(id)) == sizeof(id);
> +}
> +
> +/* get_state variant for boards without a MOD_DEF0 GPIO. Instead of assuming
> + * the module is always present, derive SFP_F_PRESENT from a throttled I2C
> + * probe so that hot-insertion and removal are detected. A single ACK asserts
> + * presence; two consecutive failures clear it, to ride out a transient I2C
> + * error on a live module.
> + */
> +static unsigned int sfp_i2c_get_state(struct sfp *sfp)
> +{
> +	unsigned int state = sfp_gpio_get_state(sfp);
> +
> +	if (time_after_eq(jiffies, sfp->i2c_present_next)) {
> +		if (sfp_module_present_i2c(sfp)) {
> +			sfp->i2c_present = true;
> +			sfp->i2c_present_nak = 0;
> +		} else if (sfp->i2c_present && ++sfp->i2c_present_nak >= 2) {
> +			sfp->i2c_present = false;
> +			sfp->i2c_present_nak = 0;
> +		}
> +		sfp->i2c_present_next = jiffies + T_PROBE_PRESENT;
> +	}
> +
> +	if (sfp->i2c_present)
> +		state |= SFP_F_PRESENT;
> +
> +	return state;
> +}
> +
>  static int sfp_write(struct sfp *sfp, bool a2, u8 addr, void *buf, size_t len)
>  {
>  	return sfp->write(sfp, a2, addr, buf, len);
> @@ -3168,9 +3218,30 @@ static int sfp_probe(struct platform_device *pdev)
>  	sfp->get_state = sfp_gpio_get_state;
>  	sfp->set_state = sfp_gpio_set_state;
>  
> -	/* Modules that have no detect signal are always present */
> -	if (!(sfp->gpio[GPIO_MODDEF0]))
> -		sfp->get_state = sff_gpio_get_state;
> +	/* An SFP cage with no MOD_DEF0 GPIO has no hardware presence signal.
> +	 * Assuming the module is always present traps an empty cage in
> +	 * MOD_ERROR and never detects hot-insertion, so derive presence from a
> +	 * throttled I2C probe and poll for changes instead. sfp_i2c_configure()
> +	 * has already set i2c_max_block_size; seed i2c_block_size so the
> +	 * presence read does not issue a zero-length transfer before the first
> +	 * EEPROM read. Seed i2c_present_next to jiffies so the first probe
> +	 * happens immediately (a zero value would be in the past relative to
> +	 * the negative INITIAL_JIFFIES at boot and delay detection).
> +	 *
> +	 * A soldered-down module (sff,sff) has no presence signal and is
> +	 * genuinely always present, so it keeps the always-present behaviour;
> +	 * the I2C probe is gated on the cage type advertising SFP_F_PRESENT.
> +	 */
> +	if (!sfp->gpio[GPIO_MODDEF0]) {
> +		if (sff->gpios & SFP_F_PRESENT) {
> +			sfp->get_state = sfp_i2c_get_state;
> +			sfp->i2c_block_size = sfp->i2c_max_block_size;
> +			sfp->i2c_present_next = jiffies;
> +			sfp->need_poll = true;
> +		} else {
> +			sfp->get_state = sff_gpio_get_state;
> +		}
> +	}
>  
>  	device_property_read_u32(&pdev->dev, "maximum-power-milliwatt",
>  				 &sfp->max_power_mW);

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

[Maxime Chevallier]

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