Hi Alexandre, On Fri, 29 May 2020 at 11:14, Alexandre Belloni <alexandre.bell...@bootlin.com> wrote: > > On 29/05/2020 01:09:16+0300, Vladimir Oltean wrote: > > On Fri, 29 May 2020 at 00:56, Andrew Lunn <and...@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 > > As ocelot can be used in a DSA configuration (even if it is not > implemented yet), I don't think this would be correct. From my point of > view, felix and seville are part of the ocelot family. >
In this case, there would be a third driver in drivers/net/dsa/ocelot/ocelot_vsc7511.c which uses the intermediate felix_switch_ops from felix.c to access the ocelot core implementation. Unless you have better naming suggestions? > > 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 > > -- > Alexandre Belloni, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com Thanks, -Vladimir
