Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v4 0/2] net: phy: sfp/mdio-i2c: defer RollBall probe + fix mii_bus leak
@ 2026-06-24  8:48 Petr Wozniak
  2026-06-24  8:48 ` [PATCH net v4 1/2] net: phy: sfp: free mii_bus in sfp_i2c_mdiobus_destroy Petr Wozniak
  2026-06-24  8:48 ` [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery Petr Wozniak
  0 siblings, 2 replies; 6+ messages in thread
From: Petr Wozniak @ 2026-06-24  8:48 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Heiner Kallweit
  Cc: Jakub Kicinski, David S . Miller, Eric Dumazet, Paolo Abeni,
	netdev, linux-kernel, linux-phy, Maxime Chevallier, Bjorn Mork,
	Aleksander Bajkowski, Marek Behun, Petr Wozniak

This series resends the RollBall bridge probe deferral (a fix for the
regression in commit 8fe125892f40) and adds a related mii_bus leak fix.

Patch 1 fixes a pre-existing mii_bus leak in sfp_i2c_mdiobus_destroy()
that has been present since the helper was introduced in 2022. Patch 2
introduces a new -ENODEV path that destroys the MDIO bus via
sfp_i2c_mdiobus_destroy(), so patch 1 is a prerequisite to avoid leaking
the bus on that path.

v4:
 - Retargeted net-next -> net: both patches carry Fixes: tags and
   8fe125892f40 is now in mainline.
 - Patch 1: added Reviewed-by: Maxime Chevallier.
 - Patch 2: reworked post-probe error handling to drop an over-80-column
   line and moved two block comment terminators to their own line
   (checkpatch); set err = 0 after switching to MDIO_I2C_NONE so the SFP
   state machine does not schedule a redundant 1 s retry. No functional
   change to the probe logic.
v3:
 - Resend: v2 defer patch was corrupted in transit and failed to apply
   (netdev/apply); regenerated against current net-next.
 - Fixed block comment style flagged by checkpatch. No functional change.
 - Added patch 1/2 (sfp: free mii_bus in sfp_i2c_mdiobus_destroy).
v2 (defer):
 - Generalized scope: regression affects boot-inserted and hotplugged
   modules where bridge init exceeds 200 ms; Aleksander Bajkowski
   confirmed FLYPRO SFP-10GT-CS-30M / AQR113C broken when hotplugged.
 - Corrected state machine description (probe runs in SFP_S_INIT after
   SFP_S_WAIT) - Jan Hoffmann.
 - No code changes from v1.
v1: initial submission.

Petr Wozniak (2):
  net: phy: sfp: free mii_bus in sfp_i2c_mdiobus_destroy
  net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery

 drivers/net/mdio/mdio-i2c.c   | 15 +++++++++------
 drivers/net/phy/sfp.c         | 23 +++++++++++++++--------
 include/linux/mdio/mdio-i2c.h |  1 +
 3 files changed, 25 insertions(+), 14 deletions(-)

-- 
2.51.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net v4 1/2] net: phy: sfp: free mii_bus in sfp_i2c_mdiobus_destroy
  2026-06-24  8:48 [PATCH net v4 0/2] net: phy: sfp/mdio-i2c: defer RollBall probe + fix mii_bus leak Petr Wozniak
@ 2026-06-24  8:48 ` Petr Wozniak
  2026-06-24  8:48 ` [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery Petr Wozniak
  1 sibling, 0 replies; 6+ messages in thread
From: Petr Wozniak @ 2026-06-24  8:48 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Heiner Kallweit
  Cc: Jakub Kicinski, David S . Miller, Eric Dumazet, Paolo Abeni,
	netdev, linux-kernel, linux-phy, Maxime Chevallier, Bjorn Mork,
	Aleksander Bajkowski, Marek Behun, Petr Wozniak

sfp_i2c_mdiobus_create() allocates the I2C MDIO bus with mdio_i2c_alloc(),
a plain (non-devm) allocation, and registers it. sfp_i2c_mdiobus_destroy()
only unregisters the bus and clears sfp->i2c_mii without calling
mdiobus_free(). As the only reference to the bus is then cleared, the
struct mii_bus is leaked.

This is hit whenever a copper/RollBall SFP module that instantiated an MDIO
bus is removed: sfp_sm_main() takes the global teardown path and calls
sfp_i2c_mdiobus_destroy(). sfp_cleanup(), on driver unbind, frees
sfp->i2c_mii directly, which is why the leak only triggered on module
hot-removal and not on unbind.

Free the bus in sfp_i2c_mdiobus_destroy() to match the allocation done in
sfp_i2c_mdiobus_create().

Fixes: e85b1347ace6 ("net: sfp: create/destroy I2C mdiobus before PHY probe/after PHY release")
Signed-off-by: Petr Wozniak <petr.wozniak@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 drivers/net/phy/sfp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 03bfd8640db9..c4d274ab651e 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -963,6 +963,7 @@ static int sfp_i2c_mdiobus_create(struct sfp *sfp)
 static void sfp_i2c_mdiobus_destroy(struct sfp *sfp)
 {
 	mdiobus_unregister(sfp->i2c_mii);
+	mdiobus_free(sfp->i2c_mii);
 	sfp->i2c_mii = NULL;
 }
 
-- 
2.51.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
  2026-06-24  8:48 [PATCH net v4 0/2] net: phy: sfp/mdio-i2c: defer RollBall probe + fix mii_bus leak Petr Wozniak
  2026-06-24  8:48 ` [PATCH net v4 1/2] net: phy: sfp: free mii_bus in sfp_i2c_mdiobus_destroy Petr Wozniak
@ 2026-06-24  8:48 ` Petr Wozniak
  2026-06-24 21:44   ` Aleksander Jan Bajkowski
  2026-06-25  8:48   ` sashiko-bot
  1 sibling, 2 replies; 6+ messages in thread
From: Petr Wozniak @ 2026-06-24  8:48 UTC (permalink / raw)
  To: Russell King, Andrew Lunn, Heiner Kallweit
  Cc: Jakub Kicinski, David S . Miller, Eric Dumazet, Paolo Abeni,
	netdev, linux-kernel, linux-phy, Maxime Chevallier, Bjorn Mork,
	Aleksander Bajkowski, Marek Behun, Petr Wozniak

commit 8fe125892f40 ("net: phy: sfp: probe for RollBall I2C-to-MDIO
bridge in mdio-i2c") introduced a regression: the RollBall I2C-to-MDIO
bridge is not yet ready to respond to CMD_READ/CMD_DONE cycles when
sfp_sm_add_mdio_bus() runs in SFP_S_INIT.  The 200 ms probe times out,
i2c_mii_probe_rollball() returns -ENODEV, and sfp_sm_add_mdio_bus()
sets mdio_protocol = MDIO_I2C_NONE.  By the time sfp_sm_probe_for_phy()
runs (up to ~17 s later on affected hardware), the bridge is fully
initialized but PHY probing is skipped because the protocol has already
been changed to NONE.

This affects both modules inserted before boot and hotplugged modules on
hardware where bridge initialization exceeds the 200 ms probe window
(confirmed: FLYPRO SFP-10GT-CS-30M with Aquantia AQR113C, hotplugged).

Move the probe from i2c_mii_init_rollball(), called at bus-creation time,
to sfp_sm_probe_for_phy() in sfp.c, where it runs after the SFP state
machine module initialization delays.  Export the probe function as
mdio_i2c_probe_rollball() so sfp.c can call it.

For RTL8261BE-based modules the probe correctly returns -ENODEV at PHY
discovery time, causing sfp_sm_probe_for_phy() to destroy the MDIO bus
and set MDIO_I2C_NONE, eliminating the 5+ minute PHY probe retry loop.

For genuine RollBall modules (e.g. FLYPRO SFP-10GT-CS-30M with Aquantia
AQR113C) the probe now runs after initialization is complete and
correctly returns 0, so PHY detection proceeds normally.

Reported-by: Aleksander Bajkowski <olek2@wp.pl>
Fixes: 8fe125892f40 ("net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in mdio-i2c")
Signed-off-by: Petr Wozniak <petr.wozniak@gmail.com>
---
 drivers/net/mdio/mdio-i2c.c   | 15 +++++++++------
 drivers/net/phy/sfp.c         | 22 ++++++++++++++--------
 include/linux/mdio/mdio-i2c.h |  1 +
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mdio/mdio-i2c.c b/drivers/net/mdio/mdio-i2c.c
index b88f63234b4e..2a3a418c1369 100644
--- a/drivers/net/mdio/mdio-i2c.c
+++ b/drivers/net/mdio/mdio-i2c.c
@@ -419,7 +419,7 @@ static int i2c_mii_write_rollball(struct mii_bus *bus, int phy_id, int devad,
 	return 0;
 }
 
-static int i2c_mii_probe_rollball(struct i2c_adapter *i2c)
+int mdio_i2c_probe_rollball(struct i2c_adapter *i2c)
 {
 	u8 data_buf[] = { ROLLBALL_DATA_ADDR, 0x01, 0x00, 0x00 };
 	u8 cmd_buf[]  = { ROLLBALL_CMD_ADDR, ROLLBALL_CMD_READ };
@@ -462,9 +462,13 @@ static int i2c_mii_probe_rollball(struct i2c_adapter *i2c)
 
 	return -ENODEV;
 }
+EXPORT_SYMBOL_GPL(mdio_i2c_probe_rollball);
 
 static int i2c_mii_init_rollball(struct i2c_adapter *i2c)
 {
+	/* Send the RollBall unlock password; bridge presence is verified
+	 * later, in sfp_sm_probe_for_phy(), after module initialization.
+	 */
 	struct i2c_msg msg;
 	u8 pw[5];
 	int ret;
@@ -486,7 +490,7 @@ static int i2c_mii_init_rollball(struct i2c_adapter *i2c)
 	if (ret != 1)
 		return -EIO;
 
-	return i2c_mii_probe_rollball(i2c);
+	return 0;
 }
 
 static bool mdio_i2c_check_functionality(struct i2c_adapter *i2c,
@@ -531,10 +535,9 @@ struct mii_bus *mdio_i2c_alloc(struct device *parent, struct i2c_adapter *i2c,
 	case MDIO_I2C_ROLLBALL:
 		ret = i2c_mii_init_rollball(i2c);
 		if (ret < 0) {
-			if (ret != -ENODEV)
-				dev_err(parent,
-					"Cannot initialize RollBall MDIO I2C protocol: %d\n",
-					ret);
+			dev_err(parent,
+				"Cannot initialize RollBall MDIO I2C protocol: %d\n",
+				ret);
 			mdiobus_free(mii);
 			return ERR_PTR(ret);
 		}
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index c4d274ab651e..01b941a38eed 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -2174,17 +2174,10 @@ static void sfp_sm_fault(struct sfp *sfp, unsigned int next_state, bool warn)
 
 static int sfp_sm_add_mdio_bus(struct sfp *sfp)
 {
-	int ret;
-
 	if (sfp->mdio_protocol == MDIO_I2C_NONE)
 		return 0;
 
-	ret = sfp_i2c_mdiobus_create(sfp);
-	if (ret == -ENODEV) {
-		sfp->mdio_protocol = MDIO_I2C_NONE;
-		return 0;
-	}
-	return ret;
+	return sfp_i2c_mdiobus_create(sfp);
 }
 
 /* Probe a SFP for a PHY device if the module supports copper - the PHY
@@ -2215,6 +2208,19 @@ static int sfp_sm_probe_for_phy(struct sfp *sfp)
 		break;
 
 	case MDIO_I2C_ROLLBALL:
+		/* Probe here, after module initialization delays, so that
+		 * genuine RollBall bridges have had time to start up.
+		 * Modules without a bridge (e.g. RTL8261BE) return -ENODEV.
+		 */
+		err = mdio_i2c_probe_rollball(sfp->i2c);
+		if (err == -ENODEV) {
+			sfp_i2c_mdiobus_destroy(sfp);
+			sfp->mdio_protocol = MDIO_I2C_NONE;
+			err = 0;
+			break;
+		}
+		if (err)
+			break;
 		err = sfp_sm_probe_phy(sfp, SFP_PHY_ADDR_ROLLBALL, true);
 		break;
 	}
diff --git a/include/linux/mdio/mdio-i2c.h b/include/linux/mdio/mdio-i2c.h
index 65b550a6fc32..5cf14f45c94b 100644
--- a/include/linux/mdio/mdio-i2c.h
+++ b/include/linux/mdio/mdio-i2c.h
@@ -20,5 +20,6 @@ enum mdio_i2c_proto {
 
 struct mii_bus *mdio_i2c_alloc(struct device *parent, struct i2c_adapter *i2c,
 			       enum mdio_i2c_proto protocol);
+int mdio_i2c_probe_rollball(struct i2c_adapter *i2c);
 
 #endif
-- 
2.51.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
  2026-06-24  8:48 ` [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery Petr Wozniak
@ 2026-06-24 21:44   ` Aleksander Jan Bajkowski
  2026-06-25 15:23     ` Jakub Kicinski
  2026-06-25  8:48   ` sashiko-bot
  1 sibling, 1 reply; 6+ messages in thread
From: Aleksander Jan Bajkowski @ 2026-06-24 21:44 UTC (permalink / raw)
  To: Petr Wozniak, Russell King, Andrew Lunn, Heiner Kallweit
  Cc: Jakub Kicinski, David S . Miller, Eric Dumazet, Paolo Abeni,
	netdev, linux-kernel, linux-phy, Maxime Chevallier, Bjorn Mork,
	Marek Behun

Hi Petr,

W dniu 24.06.2026 o 10:48, Petr Wozniak pisze:
> commit 8fe125892f40 ("net: phy: sfp: probe for RollBall I2C-to-MDIO
> bridge in mdio-i2c") introduced a regression: the RollBall I2C-to-MDIO
> bridge is not yet ready to respond to CMD_READ/CMD_DONE cycles when
> sfp_sm_add_mdio_bus() runs in SFP_S_INIT.  The 200 ms probe times out,
> i2c_mii_probe_rollball() returns -ENODEV, and sfp_sm_add_mdio_bus()
> sets mdio_protocol = MDIO_I2C_NONE.  By the time sfp_sm_probe_for_phy()
> runs (up to ~17 s later on affected hardware), the bridge is fully
> initialized but PHY probing is skipped because the protocol has already
> been changed to NONE.
>
> This affects both modules inserted before boot and hotplugged modules on
> hardware where bridge initialization exceeds the 200 ms probe window
> (confirmed: FLYPRO SFP-10GT-CS-30M with Aquantia AQR113C, hotplugged).
>
> Move the probe from i2c_mii_init_rollball(), called at bus-creation time,
> to sfp_sm_probe_for_phy() in sfp.c, where it runs after the SFP state
> machine module initialization delays.  Export the probe function as
> mdio_i2c_probe_rollball() so sfp.c can call it.
>
> For RTL8261BE-based modules the probe correctly returns -ENODEV at PHY
> discovery time, causing sfp_sm_probe_for_phy() to destroy the MDIO bus
> and set MDIO_I2C_NONE, eliminating the 5+ minute PHY probe retry loop.
>
> For genuine RollBall modules (e.g. FLYPRO SFP-10GT-CS-30M with Aquantia
> AQR113C) the probe now runs after initialization is complete and
> correctly returns 0, so PHY detection proceeds normally.
The FLPRO SFP module still fails to detect the PHY. It is necessary to
increase `module_t_wait` to 20 seconds. Most likely, during this time
the module loads the PHY firmware from SPI memory or from the
microcontroller (rollball bridge) via MDIO. Same probably applies to
most SFP modules with a PHY that load firmware at start-up (AQR113,
RTL8261C etc.).
>
> Reported-by: Aleksander Bajkowski <olek2@wp.pl>
> Fixes: 8fe125892f40 ("net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in mdio-i2c")
> Signed-off-by: Petr Wozniak <petr.wozniak@gmail.com>
> ---
>   drivers/net/mdio/mdio-i2c.c   | 15 +++++++++------
>   drivers/net/phy/sfp.c         | 22 ++++++++++++++--------
>   include/linux/mdio/mdio-i2c.h |  1 +
>   3 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/mdio/mdio-i2c.c b/drivers/net/mdio/mdio-i2c.c
> index b88f63234b4e..2a3a418c1369 100644
> --- a/drivers/net/mdio/mdio-i2c.c
> +++ b/drivers/net/mdio/mdio-i2c.c
> @@ -419,7 +419,7 @@ static int i2c_mii_write_rollball(struct mii_bus *bus, int phy_id, int devad,
>   	return 0;
>   }
>   
> -static int i2c_mii_probe_rollball(struct i2c_adapter *i2c)
> +int mdio_i2c_probe_rollball(struct i2c_adapter *i2c)
>   {
>   	u8 data_buf[] = { ROLLBALL_DATA_ADDR, 0x01, 0x00, 0x00 };
>   	u8 cmd_buf[]  = { ROLLBALL_CMD_ADDR, ROLLBALL_CMD_READ };
> @@ -462,9 +462,13 @@ static int i2c_mii_probe_rollball(struct i2c_adapter *i2c)
>   
>   	return -ENODEV;
>   }
> +EXPORT_SYMBOL_GPL(mdio_i2c_probe_rollball);
>   
>   static int i2c_mii_init_rollball(struct i2c_adapter *i2c)
>   {
> +	/* Send the RollBall unlock password; bridge presence is verified
> +	 * later, in sfp_sm_probe_for_phy(), after module initialization.
> +	 */
>   	struct i2c_msg msg;
>   	u8 pw[5];
>   	int ret;
> @@ -486,7 +490,7 @@ static int i2c_mii_init_rollball(struct i2c_adapter *i2c)
>   	if (ret != 1)
>   		return -EIO;
>   
> -	return i2c_mii_probe_rollball(i2c);
> +	return 0;
>   }
>   
>   static bool mdio_i2c_check_functionality(struct i2c_adapter *i2c,
> @@ -531,10 +535,9 @@ struct mii_bus *mdio_i2c_alloc(struct device *parent, struct i2c_adapter *i2c,
>   	case MDIO_I2C_ROLLBALL:
>   		ret = i2c_mii_init_rollball(i2c);
>   		if (ret < 0) {
> -			if (ret != -ENODEV)
> -				dev_err(parent,
> -					"Cannot initialize RollBall MDIO I2C protocol: %d\n",
> -					ret);
> +			dev_err(parent,
> +				"Cannot initialize RollBall MDIO I2C protocol: %d\n",
> +				ret);
>   			mdiobus_free(mii);
>   			return ERR_PTR(ret);
>   		}
> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index c4d274ab651e..01b941a38eed 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -2174,17 +2174,10 @@ static void sfp_sm_fault(struct sfp *sfp, unsigned int next_state, bool warn)
>   
>   static int sfp_sm_add_mdio_bus(struct sfp *sfp)
>   {
> -	int ret;
> -
>   	if (sfp->mdio_protocol == MDIO_I2C_NONE)
>   		return 0;
>   
> -	ret = sfp_i2c_mdiobus_create(sfp);
> -	if (ret == -ENODEV) {
> -		sfp->mdio_protocol = MDIO_I2C_NONE;
> -		return 0;
> -	}
> -	return ret;
> +	return sfp_i2c_mdiobus_create(sfp);
>   }
>   
>   /* Probe a SFP for a PHY device if the module supports copper - the PHY
> @@ -2215,6 +2208,19 @@ static int sfp_sm_probe_for_phy(struct sfp *sfp)
>   		break;
>   
>   	case MDIO_I2C_ROLLBALL:
> +		/* Probe here, after module initialization delays, so that
> +		 * genuine RollBall bridges have had time to start up.
> +		 * Modules without a bridge (e.g. RTL8261BE) return -ENODEV.
> +		 */
> +		err = mdio_i2c_probe_rollball(sfp->i2c);
> +		if (err == -ENODEV) {
> +			sfp_i2c_mdiobus_destroy(sfp);
> +			sfp->mdio_protocol = MDIO_I2C_NONE;
> +			err = 0;
> +			break;
> +		}
> +		if (err)
> +			break;
>   		err = sfp_sm_probe_phy(sfp, SFP_PHY_ADDR_ROLLBALL, true);
>   		break;
>   	}
> diff --git a/include/linux/mdio/mdio-i2c.h b/include/linux/mdio/mdio-i2c.h
> index 65b550a6fc32..5cf14f45c94b 100644
> --- a/include/linux/mdio/mdio-i2c.h
> +++ b/include/linux/mdio/mdio-i2c.h
> @@ -20,5 +20,6 @@ enum mdio_i2c_proto {
>   
>   struct mii_bus *mdio_i2c_alloc(struct device *parent, struct i2c_adapter *i2c,
>   			       enum mdio_i2c_proto protocol);
> +int mdio_i2c_probe_rollball(struct i2c_adapter *i2c);
>   
>   #endif

Best regards,
Aleksander


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
  2026-06-24  8:48 ` [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery Petr Wozniak
  2026-06-24 21:44   ` Aleksander Jan Bajkowski
@ 2026-06-25  8:48   ` sashiko-bot
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-06-25  8:48 UTC (permalink / raw)
  To: Petr Wozniak; +Cc: olteanv, vkoul, neil.armstrong, linux-phy

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Moving `mdio_i2c_probe_rollball()` into `sfp_sm_probe_for_phy()` aborts the PHY discovery retry loop on the first NAK or timeout, permanently bypassing PHY detection for genuine RollBall modules.
- [High] Removal of the `-ENODEV` fallback in `sfp_sm_add_mdio_bus()` causes fatal SFP module initialization failure instead of gracefully downgrading to non-MDIO mode.
--

commit 21ce5e35892ea02b56c9ead087f775ee5de05519
Author: Petr Wozniak <petr.wozniak@gmail.com>

net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery

This patch moves the RollBall I2C-to-MDIO bridge probe from bus-creation
time to PHY discovery time to allow slow-booting modules time to
initialize. It also removes the -ENODEV fallback from
sfp_sm_add_mdio_bus().

> By the time sfp_sm_probe_for_phy() runs (up to ~17 s later on affected
> hardware), the bridge is fully initialized but PHY probing is skipped
> because the protocol has already been changed to NONE.

Looking at sfp_sm_event(), sfp_sm_probe_for_phy() runs immediately after
sfp_sm_add_mdio_bus(). The 17-second delay on affected hardware appears to
be the duration of the PHY probe retry loop itself, rather than a delay
before sfp_sm_probe_for_phy() is called.

> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> index c4d274ab651e3..01b941a38eed2 100644
> --- a/drivers/net/phy/sfp.c
> +++ b/drivers/net/phy/sfp.c
> @@ -2174,17 +2174,10 @@ static void sfp_sm_fault(struct sfp *sfp, unsigned int next_state, bool warn)
>  
>  static int sfp_sm_add_mdio_bus(struct sfp *sfp)
>  {
> -	int ret;
> -
>  	if (sfp->mdio_protocol == MDIO_I2C_NONE)
>  		return 0;
>  
> -	ret = sfp_i2c_mdiobus_create(sfp);
> -	if (ret == -ENODEV) {
> -		sfp->mdio_protocol = MDIO_I2C_NONE;
> -		return 0;
> -	}
> -	return ret;
> +	return sfp_i2c_mdiobus_create(sfp);
>  }

[Severity: High]
If the initial password write in i2c_mii_init_rollball() returns -ENODEV
(due to a slow-booting module or transient I2C errors), will this change
cause the SFP state machine to fail entirely?

In sfp_sm_event(), sfp_sm_add_mdio_bus() is called, and if it returns an
error, the module is forced into SFP_S_FAIL:

sfp_sm_event() {
...
        ret = sfp_sm_add_mdio_bus(sfp);
        if (ret < 0) {
                sfp_sm_next(sfp, SFP_S_FAIL, 0);
                break;
        }
...
}

Without the -ENODEV fallback to MDIO_I2C_NONE, it seems this will assert
TX_DISABLE and disable the network link instead of gracefully downgrading.

[ ... ]

> @@ -2215,6 +2208,19 @@ static int sfp_sm_probe_for_phy(struct sfp *sfp)
>  		break;
>  
>  	case MDIO_I2C_ROLLBALL:
> +		/* Probe here, after module initialization delays, so that
> +		 * genuine RollBall bridges have had time to start up.
> +		 * Modules without a bridge (e.g. RTL8261BE) return -ENODEV.
> +		 */
> +		err = mdio_i2c_probe_rollball(sfp->i2c);
> +		if (err == -ENODEV) {
> +			sfp_i2c_mdiobus_destroy(sfp);
> +			sfp->mdio_protocol = MDIO_I2C_NONE;
> +			err = 0;
> +			break;
> +		}

[Severity: High]
Does this bypass the PHY retry loop entirely for slow-booting genuine
RollBall modules?

The commit message notes that bridge initialization can take ~17s, but
looking at sfp_sm_event(), sfp_sm_probe_for_phy() is called immediately
after sfp_sm_add_mdio_bus() with no delay:

sfp_sm_event() {
...
        init_done:
                ret = sfp_sm_add_mdio_bus(sfp);
...
                goto phy_probe;
...
        phy_probe:
                ret = sfp_sm_probe_for_phy(sfp);
...
}

Since the probe happens immediately, a slow module that hasn't finished
initializing will return -ENODEV. This new code intercepts -ENODEV, sets
err = 0, and breaks out.

Because it returns 0, won't this tell the SFP state machine to skip
the PHY retry loop and proceed without PHY detection?

> +		if (err)
> +			break;
>  		err = sfp_sm_probe_phy(sfp, SFP_PHY_ADDR_ROLLBALL, true);
>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624084814.20972-1-petr.wozniak@gmail.com?part=2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
  2026-06-24 21:44   ` Aleksander Jan Bajkowski
@ 2026-06-25 15:23     ` Jakub Kicinski
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-06-25 15:23 UTC (permalink / raw)
  To: Aleksander Jan Bajkowski
  Cc: Petr Wozniak, Russell King, Andrew Lunn, Heiner Kallweit,
	David S . Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel,
	linux-phy, Maxime Chevallier, Bjorn Mork, Marek Behun

On Wed, 24 Jun 2026 23:44:19 +0200 Aleksander Jan Bajkowski wrote:
> > For genuine RollBall modules (e.g. FLYPRO SFP-10GT-CS-30M with Aquantia
> > AQR113C) the probe now runs after initialization is complete and
> > correctly returns 0, so PHY detection proceeds normally.  
> The FLPRO SFP module still fails to detect the PHY. It is necessary to
> increase `module_t_wait` to 20 seconds. Most likely, during this time
> the module loads the PHY firmware from SPI memory or from the
> microcontroller (rollball bridge) via MDIO. Same probably applies to
> most SFP modules with a PHY that load firmware at start-up (AQR113,
> RTL8261C etc.).

Just to clarify is FLPRO a typo or a knock off ?
Do you want something to be changed here or you're just flagging that
more follow ups are needed if we want to cover more modules?

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-25 15:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-24  8:48 [PATCH net v4 0/2] net: phy: sfp/mdio-i2c: defer RollBall probe + fix mii_bus leak Petr Wozniak
2026-06-24  8:48 ` [PATCH net v4 1/2] net: phy: sfp: free mii_bus in sfp_i2c_mdiobus_destroy Petr Wozniak
2026-06-24  8:48 ` [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery Petr Wozniak
2026-06-24 21:44   ` Aleksander Jan Bajkowski
2026-06-25 15:23     ` Jakub Kicinski
2026-06-25  8:48   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox