[PATCH net] net: marvell: prestera: disable port before changing its MAC mode

[PATCH net] net: marvell: prestera: disable port before changing its MAC mode

[Imre Kaloz]

prestera_pcs_config() writes a new mode or inband setting to a port
that is already enabled, as when phylink switches an SFP port to
inband SGMII. The firmware can then report link down and no link up
until the port is disabled and enabled again.

Disable the port before such a write. Exercising this needs a link
partner, so no in-tree test covers it.

Fixes: 52323ef75414 ("net: marvell: prestera: add phylink support")
Signed-off-by: Imre Kaloz <kaloz@kernel.org>
---
 .../net/ethernet/marvell/prestera/prestera_main.c  | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/prestera/prestera_main.c b/drivers/net/ethernet/marvell/prestera/prestera_main.c
index a82e7a8029851..39ebd1944be55 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_main.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_main.c
@@ -307,13 +307,14 @@ static int prestera_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 			       bool permit_pause_to_mac)
 {
 	struct prestera_port *port = prestera_pcs_to_port(pcs);
-	struct prestera_port_mac_config cfg_mac;
+	struct prestera_port_mac_config cfg_mac, old;
 	int err;
 
 	err = prestera_port_cfg_mac_read(port, &cfg_mac);
 	if (err)
 		return err;
 
+	old = cfg_mac;
 	cfg_mac.admin = true;
 	cfg_mac.fec = PRESTERA_PORT_FEC_OFF;
 	cfg_mac.inband = neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED;
@@ -339,6 +340,17 @@ static int prestera_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 		break;
 	}
 
+	/* Firmware may not report link up after the mode or inband setting of
+	 * an enabled port changes until the port is disabled and re-enabled.
+	 */
+	if (old.admin && (old.mode != cfg_mac.mode ||
+			  old.inband != cfg_mac.inband)) {
+		old.admin = false;
+		err = prestera_port_cfg_mac_write(port, &old);
+		if (err)
+			return err;
+	}
+
 	err = prestera_port_cfg_mac_write(port, &cfg_mac);
 	if (err)
 		return err;
-- 
2.47.3

Re: [PATCH net] net: marvell: prestera: disable port before changing its MAC mode

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [EXTERNAL] [PATCH net] net: marvell: prestera: disable port before changing its MAC mode

[Imre Kaloz]

On Wed, Nov 11, 2026, at 15:39, Elad Nachman wrote:

<snip>

>> +     /* Firmware may not report link up after the mode or inband setting of
>> +      * an enabled port changes until the port is disabled and re-enabled.
>> +      */
>> +     if (old.admin && (old.mode != cfg_mac.mode ||
>> +                       old.inband != cfg_mac.inband)) {
>> +             old.admin = false;
>> +             err = prestera_port_cfg_mac_write(port, &old);
>> +             if (err)
>> +                     return err;
>> +     }
> 
> From my experience, if you take the port down, it takes time for the firmware
> state machine to act accordingly, so you need some significant delay here
> (1-3 seconds) before you write the new configuration.

Thanks Elad. I found this bringing up the SFP+ ports on a Delta TN48M
(98DX3265) with firmware 4.1.0. With an S-RJ01 module in sfp-52,
the port came up in 30 of 30 boots with this patch, about 2.1 s after
link up, and in 21 of 30 without it. The disable write waits for the
firmware’s acknowledgement, and I saw no failures without a delay.

Is the delay needed on particular firmware versions or devices? This
path runs under rtnl when an SFP module changes the mode, so I would
rather not sleep for seconds there. If the firmware reports when the
port is really down, I can wait for that instead, with a bounded
timeout.


Thanks,
Imre

Re: [EXTERNAL] [PATCH net] net: marvell: prestera: disable port before changing its MAC mode

[Imre Kaloz]

Hi Elad,

On Thu, Nov 12, 2026, at 09:22, Elad Nachman wrote:

>> > From my experience, if you take the port down, it takes time for the firmware
>> > state machine to act accordingly, so you need some significant delay here
>> > (1-3 seconds) before you write the new configuration.
>> 
>> Thanks Elad. I found this bringing up the SFP+ ports on a Delta TN48M
>> (98DX3265) with firmware 4.1.0. With an S-RJ01 module in sfp-52,
>> the port came up in 30 of 30 boots with this patch, about 2.1 s after
>> link up, and in 21 of 30 without it. The disable write waits for the
>> firmware’s acknowledgement, and I saw no failures without a delay.
>> 
>> Is the delay needed on particular firmware versions or devices? This
>> path runs under rtnl when an SFP module changes the mode, so I would
>> rather not sleep for seconds there. If the firmware reports when the
>> port is really down, I can wait for that instead, with a bounded
>> timeout.
> 
> Your best shot is monitoring what *prestera_hw_port_phy_mode_get* 
> <https://elixir.bootlin.com/linux/v7.3.1/C/ident/prestera_hw_port_phy_mode_get>() 
> returns as mdix status.

Thanks. I looked at prestera_hw_port_phy_mode_get(): it only maps the
firmware's mdix byte to ETH_TP_MDI/MDI_X/AUTO, anything else becoming
ETH_TP_MDI_INVALID, and the driver never calls it for SFP ports (the
sfp-52 port goes through phylink and the MAC mode attribute only).

Two things the code cannot tell me: which mdix value does the firmware
report while the port is admin down (INVALID?), and does that attribute
carry anything for an SFP port in SGMII mode with a copper module, where
the PHY sits in the module?


Thanks,
Imre