Re: [PATCH net-next 11/11] net: dsa: ocelot: introduce driver for Seville VSC9953 switch

[Vladimir Oltean <olteanv@gmail.com>]

On Fri, 29 May 2020 at 00:56, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Extending the Felix driver to probe a PCI as well as a platform device
> > would have introduced unnecessary complexity. The 'meat' of both drivers
> > is in drivers/net/ethernet/mscc/ocelot*.c anyway, so let's just
> > duplicate the Felix driver, s/Felix/Seville/, and define the low-level
> > bits in seville_vsc9953.c.
>
> Hi Vladimir
>
> That has resulted in a lot of duplicated code.
>
> Is there an overall family name for these switch?
>
> Could you add foo_set_ageing_time() with both felix and saville share?
>
>       Andrew

Yes, it looks like I can. I can move Felix PCI probing to
felix_vsc9959.c, Seville platform device probing to seville_vsc9953.c,
and remove seville.c.
I would not be in a position to know whether there's any larger family
name which should be used here. According to
https://media.digikey.com/pdf/Data%20Sheets/Microsemi%20PDFs/Ocelot_Family_of_Ethernet_Switches_Dec2016.pdf,
"Ocelot is a low port count, small form factor Ethernet switch family
for the Industrial IoT market". Seville would not qualify as part of
the Ocelot family (high port count, no 1588) but that doesn't mean it
can't use the Ocelot driver. As confusing as it might be for the
people at Microchip, I would tend to call anything that probes as pure
switchdev "ocelot" and anything that probes as DSA "felix", since
these were the first 2 drivers that entered mainline. Under this
working model, Seville would reuse the struct dsa_switch_ops
felix_switch_ops, while having its own low-level seville_vsc9953.c
that deals with platform integration specific stuff (probing, internal
MDIO, register map, etc), and the felix_switch_ops would call into
ocelot for the common functionalities.
What do you think?

-Vladimir