Re: [PATCH net 3/6 RESEND] net: dsa: b53: constrain SerDes interfaces to initialized PCSes

[Florian Fainelli <florian.fainelli@broadcom.com>]

On 10/26/26 16:47, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] Ports muxed to SGMII now fail phylink creation when SerDes
>    discovery fails or CONFIG_B53_SERDES=n.
> 
> --
> 
> net: dsa: b53: constrain SerDes interfaces to initialized PCSes
> 
> This patch stores a supported interface bitmap on each b53 PCS that is
> discovered, and uses that bitmap for capability reporting and PCS
> selection. On SRAB ports whose mux selects a SerDes, it clears the
> parallel interfaces inherited from the common code. It also stops
> advertising 2500BASE-X.
> 
>> diff --git a/drivers/net/dsa/b53/b53_srab.c b/drivers/net/dsa/b53/b53_srab.c
>> index 9818b3922a377..3fabae80dc1a8 100644
>> --- a/drivers/net/dsa/b53/b53_srab.c
>> +++ b/drivers/net/dsa/b53/b53_srab.c
>> @@ -451,9 +451,11 @@ static void b53_srab_phylink_get_caps(struct b53_device *dev, int port,
>>   
>>   	switch (p->mode) {
>>   	case PHY_INTERFACE_MODE_SGMII:
>> +		/* The mux connects this port to a SerDes, not a parallel bus. */
>> +		phy_interface_zero(config->supported_interfaces);
>>   #if IS_ENABLED(CONFIG_B53_SERDES)
>>   		/* If p->mode indicates SGMII mode, that essentially means we
>> -		 * are using a serdes. As the serdes for the capabilities.
>> +		 * are using a SerDes. Ask it for the supported interfaces.
>>   		 */
>>   		b53_serdes_phylink_get_caps(dev, port, config);
>>   #endif
> 
> [Severity: Medium]
> Could an SGMII-muxed port end up here with an empty supported_interfaces
> mask, so that phylink_create() fails for it?
> 
> b53_srab_mux_init() sets p->mode before it tries discovery. If discovery
> fails, the loop continues with p->mode still set to SGMII and
> pcs[lane].dev still NULL:
> 
>      case MUX_CONFIG_SGMII:
>          p->mode = PHY_INTERFACE_MODE_SGMII;
>          ret = b53_serdes_init(dev, port);
>          if (ret) {
>              dev_err(...);
>              continue;
>          }
> 
> In that state b53_serdes_phylink_get_caps() returns early on
> !dev->pcs[lane].dev, so nothing goes back into the mask.
> 
> With CONFIG_B53_SERDES=n, the b53_serdes_init() stub in b53_serdes.h
> always returns -ENODEV and the refill call is compiled out. Every
> SGMII-muxed port then ends up in this state.
> 
> phylink_create() then rejects the port:
> 
>      if (phy_interface_empty(config->supported_interfaces)) {
>          dev_err(config->dev,
>              "phylink: error: empty supported_interfaces\n");
>          return ERR_PTR(-EINVAL);
>      }
> 
> DSA then sets the user port up as unused. Before this patch, these ports
> kept GMII/MII/REVMII from b53_phylink_get_caps() and registered normally.
> 
> Ports that never select the PCS are hit as well. In
> bcm958625-meraki-alamo.dtsi, SRAB port@4 and port@5 (sw0/sw1) use a
> 1000 Mb/s fixed-link with no phy-mode, so DSA passes
> PHY_INTERFACE_MODE_NA. b53_serdes_phylink_mac_select_pcs() returns NULL
> for that mode both before and after this patch.
> 
> These two ports are the conduits for the cascaded QCA8337 switches
> (ethernet = <&sgmii0> / <&sgmii1>). Wouldn't those switches lose their
> upstream link too?
> 
> The commit message says:
> 
>      A failed discovery must not leave a usable serial interface
>      advertised without its PCS.
> 
> It does not mention that the port then fails to register at all, or that
> fixed-link/NA ports are affected. Is that the intended result?
> 
> Later patches in the series do not change b53_srab_phylink_get_caps() or
> b53_srab_mux_init(), so this still applies at the end of the series.
> 

Agreed, this was an unintended regression. The next revision preserves 
the legacy interface capabilities when SerDes discovery fails or 
CONFIG_B53_SERDES is disabled, allowing fixed links without phy-mode to 
continue registering. Only successful PCS discovery replaces those 
capabilities with the supported serial interfaces. The commit message 
has also been updated.

pw-bot: cr
-- 
Florian