Re: [PATCH net-next 3/3] net: mvpp2: update mvpp2 validate() implementation

[Antoine Tenart <antoine.tenart@bootlin.com>]

On Fri, Dec 13, 2019 at 04:11:44PM +0000, Russell King - ARM Linux admin wrote:
> On Fri, Dec 13, 2019 at 05:04:20PM +0100, Antoine Tenart wrote:
> > On Thu, Dec 12, 2019 at 05:43:46PM +0000, Russell King wrote:
> > > 
> > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > index 111b3b8239e1..fddd856338b4 100644
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -4786,6 +4786,8 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
> > >  			phylink_set(mask, 10000baseER_Full);
> > >  			phylink_set(mask, 10000baseKR_Full);
> > >  		}
> > > +		if (state->interface != PHY_INTERFACE_MODE_NA)
> > > +			break;
> > 
> > >  		/* Fall-through */
> > >  	case PHY_INTERFACE_MODE_RGMII:
> > >  	case PHY_INTERFACE_MODE_RGMII_ID:
> > > @@ -4796,13 +4798,21 @@ static void mvpp2_phylink_validate(struct phylink_config *config,
> > >  		phylink_set(mask, 10baseT_Full);
> > >  		phylink_set(mask, 100baseT_Half);
> > >  		phylink_set(mask, 100baseT_Full);
> > > +		if (state->interface != PHY_INTERFACE_MODE_NA)
> > > +			break;
> > 
> > The two checks above will break the 10G/1G interfaces on the mcbin
> > (eth0/eth1) as they can support both 10gbase-kr and 10/100/1000baseT
> > modes depending on what's connected. With this patch only the modes
> > related to the one defined in the device tree would be valid, breaking
> > run-time reconfiguration of the link.
> 
> Exactly which scenario are you talking about?  The mcbin doubleshot
> setup, or the singleshot setup?

I was thinking about the doubleshot.

> This patch (when combined with the others) has no effect on the
> doubleshot, and should have no effect on the SFP cages on the single
> shot.

You're right, I just tested the series and it the two 10G/1G ports were
able to be reconfigured at runtime, my bad.

However it seems cp1_eth1 is coming up at 100Mbps only. (I haven't
looked into it more than a quick test so far).

  # ethtool eth2
  Settings for eth2:
          Supported ports: [ TP MII FIBRE ]
          Supported link modes:   10baseT/Half 10baseT/Full
                                  100baseT/Half 100baseT/Full
          Supported pause frame use: Symmetric Receive-only
          Supports auto-negotiation: Yes
          Supported FEC modes: Not reported
          Advertised link modes:  10baseT/Half 10baseT/Full
                                  100baseT/Half 100baseT/Full
          Advertised pause frame use: Symmetric Receive-only
          Advertised auto-negotiation: Yes
          Advertised FEC modes: Not reported
          Link partner advertised link modes:  10baseT/Half 10baseT/Full
                                               100baseT/Half 100baseT/Full
          Link partner advertised pause frame use: Symmetric Receive-only
          Link partner advertised auto-negotiation: Yes
          Link partner advertised FEC modes: Not reported
          Speed: 100Mb/s
          Duplex: Full
          Port: MII
          PHYAD: 0
          Transceiver: internal
          Auto-negotiation: on
          Link detected: yes

The link partner however advertises:

          Advertised link modes:  10baseT/Half 10baseT/Full
                                  100baseT/Half 100baseT/Full
                                  1000baseT/Full

Thanks,
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com