From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: [PATCH 0/7] ethtool: add pretty dump for DSA mv88e6xxx drivers Date: Sat, 15 Dec 2018 18:48:49 +0100 Message-ID: <20181215174849.GE5922@lunn.ch> References: <20181215025035.26977-1-vivien.didelot@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: Vivien Didelot , netdev@vger.kernel.org, Chris Healy , "John W . Linville" To: Florian Fainelli Return-path: Received: from vps0.lunn.ch ([185.16.172.187]:48853 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726641AbeLORsx (ORCPT ); Sat, 15 Dec 2018 12:48:53 -0500 Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Sat, Dec 15, 2018 at 09:28:41AM -0800, Florian Fainelli wrote: > Hi Vivien, > > Le 12/14/18 à 6:50 PM, Vivien Didelot a écrit : > > This patch series adds support to pretty dump the registers of user > > ports created by the kernel "dsa" subsystem. > > > > 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. Hi Florian, Vivien 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? Andrew