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

["Imre Kaloz" <kaloz@kernel.org>]

Hi,

On Sun, Nov 15, 2026, at 17:49, Elad Nachman wrote:
>> 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
>> 
> 
> Reviewed-by: Elad Nachman <enachman@marvell.com>

Thanks, Elad.

The Sashiko review found that a switch between 2500BASE-X and SGMII,
which share one MAC mode, skips the disable. v2 also compares the
speed and sets it for SGMII. Since that changes the logic you
reviewed, v2 will not carry your Reviewed-by; I would be glad if you
could look at v2 and add it there.


Thanks,
Imre