From mboxrd@z Thu Jan 1 00:00:00 1970 From: Florian Fainelli Subject: Re: [PATCH 0/7] ethtool: add pretty dump for DSA mv88e6xxx drivers Date: Sun, 16 Dec 2018 10:02:27 -0800 Message-ID: References: <20181215025035.26977-1-vivien.didelot@gmail.com> <20181215174849.GE5922@lunn.ch> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Vivien Didelot , netdev@vger.kernel.org, Chris Healy , "John W . Linville" To: Andrew Lunn Return-path: Received: from mail-ot1-f68.google.com ([209.85.210.68]:41342 "EHLO mail-ot1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730449AbeLPSCb (ORCPT ); Sun, 16 Dec 2018 13:02:31 -0500 Received: by mail-ot1-f68.google.com with SMTP id u16so10115721otk.8 for ; Sun, 16 Dec 2018 10:02:30 -0800 (PST) In-Reply-To: <20181215174849.GE5922@lunn.ch> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: Le 12/15/18 à 9:48 AM, Andrew Lunn a écrit : > 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? We could indeed do that, the other option would be return something like: "dsa-mv88e6xxx" or "dsa-b53" in ethtool::get_drv_info() but this might be breaking ABI since "dsa" has been commonly established for the past 10 years or so. Changing things within ethtool is reasonably easy, but we probably need to decide now whether kernel changes are requited to assist with companion ethtool changes. -- Florian