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

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

[Imre Kaloz]

prestera_pcs_config() writes a new mode, speed or inband setting to an
enabled port. The firmware can then report link down until the port is
disabled and enabled again.

Disable the port before such a write. 2500BASE-X and SGMII share one MAC
mode, so SGMII sets its own speed and the speed is compared as well.
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>
---
Changes in v2:
- Also disable the port on a speed change and set the SGMII speed, so
  that a switch between 2500BASE-X and SGMII, which share one MAC mode,
  is covered.
- Elad's Reviewed-by from v1 is not carried over, given the changes to the logic.
- Link to v1: https://lore.kernel.org/netdev/20261111101521.9878-1-kaloz@kernel.org/

 .../ethernet/marvell/prestera/prestera_main.c   | 17 ++++++++++++++++-
 1 file changed, 16 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..3173145573600 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;
@@ -329,6 +330,7 @@ static int prestera_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 		cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
 		break;
 	case PHY_INTERFACE_MODE_SGMII:
+		cfg_mac.speed = SPEED_1000;
 		cfg_mac.mode = PRESTERA_MAC_MODE_SGMII;
 		break;
 	case PHY_INTERFACE_MODE_1000BASEX:
@@ -339,6 +341,19 @@ static int prestera_pcs_config(struct phylink_pcs *pcs, unsigned int neg_mode,
 		break;
 	}
 
+	/* Firmware may not report link up after the mode, speed 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.speed != cfg_mac.speed ||
+			  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 v2] 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.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

 - 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.

*IMPORTANT*: If you do repost (for other reasons) - please include
the relevant info in the cover letter or commit messages.
-- 
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.