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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+h21hpqf720YO84QJ6vBbF7chZkgnv_ow2-mRmP9OaOC_Ho1g@mail.gmail.com>
Date:   Fri, 29 May 2020 11:30:43 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        "David S. Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Russell King - ARM Linux admin <linux@...linux.org.uk>,
        Antoine Tenart <antoine.tenart@...tlin.com>,
        Horatiu Vultur <horatiu.vultur@...rochip.com>,
        "Allan W. Nielsen" <allan.nielsen@...rochip.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Alexandru Marginean <alexandru.marginean@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        "Madalin Bucur (OSS)" <madalin.bucur@....nxp.com>,
        radu-andrei.bulie@....com, fido_max@...ox.ru
Subject: Re: [PATCH net-next 11/11] net: dsa: ocelot: introduce driver for
 Seville VSC9953 switch

Hi Alexandre,

On Fri, 29 May 2020 at 11:14, Alexandre Belloni
<alexandre.belloni@...tlin.com> wrote:
>
> On 29/05/2020 01:09:16+0300, Vladimir Oltean wrote:
> > On Fri, 29 May 2020 at 00:56, Andrew Lunn <andrew@...n.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