Hi Andrew, On Sat, 15 Dec 2018 18:48:49 +0100, Andrew Lunn <and...@lunn.ch> wrote: > > > The first patch adds the base support for "dsa" interfaces. > > > > > > The second patch adds the boilerplate for the "mv88e6xxx" DSA driver, > > > all using 32 registers of 16 bits, the switch ID being available in > > > the port identification register 3. Support for other DSA drivers such > > > as "b53" or "ksz" can be added similarly later. Because the different > > > switches supported by mv88e6xxx have slightly different register layout, > > > we keep it simple and stupid by providing one dump function per switch. > > > > This looks good to me, the only "concern" is that mv88e6xxx set > > regs->version = 0, while we could probably put the switch model in there > > directly which would avoid other drivers to have to put the chip ID in > > regs[3] since that may, or may not be convenient. > > I was wondering about that. Having this all under 'dsa' seems too > granular. It would be better if we could have 'mv88e6xxx', 'b53', > 'ksz', etc. That might need a new DSA driver op to get the driver name > which we then use for the slave?
Indeed we could have DSA driver specific names, including or not a prefix such as "dsa-", as in "(dsa-)mv88e6xxx". However I choose not to go that way for the moment since casting the ethtool registers to u16 and checking regs[3] isn't destructive. I felt like changing the "dsa" driver name can be part of a future iteration, but I'd be glad to make the change now if you guys prefer. Thanks, Vivien
