Re: [RFC v8 net-next 15/16] net: dsa: felix: add phylink_get_caps capability

Vladimir Oltean
2022-05-09T17:30:41Z

On Mon, May 09, 2022 at 05:23:32PM -0700, Colin Foster wrote:
> > > @@ -982,15 +982,23 @@ static void felix_phylink_get_caps(struct dsa_switch *ds, int port,
> > > struct phylink_config *config)
> > > {
> > > struct ocelot *ocelot = ds->priv;
> > > + struct felix *felix;
> > >
> > > - /* This driver does not make use of the speed, duplex, pause or the
> > > - * advertisement in its mac_config, so it is safe to mark this driver
> > > - * as non-legacy.
> > > - */
> > > - config->legacy_pre_march2020 = false;
> > > + felix = ocelot_to_felix(ocelot);
> > > +
> > > + if (felix->info->phylink_get_caps) {
> > > + felix->info->phylink_get_caps(ocelot, port, config);
> > > + } else {
> > >
> > > - __set_bit(ocelot->ports[port]->phy_mode,
> > > - config->supported_interfaces);
> > > + /* This driver does not make use of the speed, duplex, pause or
> > > + * the advertisement in its mac_config, so it is safe to mark
> > > + * this driver as non-legacy.
> > > + */
> > > + config->legacy_pre_march2020 = false;
> >
> > I don't think you mean to set legacy_pre_march2020 to true only
> > felix->info->phylink_get_caps is absent, do you?
> >
> > Also, I'm thinking maybe we could provide an implementation of this
> > function for all switches, not just for vsc7512.
>
> I had assumed these last two patches might spark more discussion, which
> is why I kept them separate (specifically the last patch).
>
> With this, are you simply suggesting to take everything that is
> currently in felix_phylink_get_caps and doing it in the felix / seville
> implementations? This is because the default condition is no longer the
> "only" condition. Sounds easy enough.

No, not everything, just the way in which config->supported_interfaces
is populated. We have different PCS implementations, so it's likely that
the procedures to retrieve the valid SERDES protocols (when changing
them will be supported) are different.

But in fact I seriously doubt that the current way in which supported_interfaces
gets populated is limiting you from doing anything right now, precisely
because you don't have any code that supports changing the phy-mode.

Also, it's unlikely (I'd say impossible) for one driver to be
unconverted to the post-March-2020 requirements and the other not to be.
The simple reason is that they share the same mac_config implementation.
So it's perfectly ok to keep "config->legacy_pre_march2020 = false"
right where it is.

So I'd say it's up to you, but I'd drop this patch right now.