From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vivien Didelot Subject: Re: [PATCH 0/7] ethtool: add pretty dump for DSA mv88e6xxx drivers Date: Sun, 16 Dec 2018 12:57:40 -0500 Message-ID: <20181216125740.GB2063@slim.localdomain> References: <20181215025035.26977-1-vivien.didelot@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: netdev@vger.kernel.org, Chris Healy , "John W . Linville" , andrew@lunn.ch To: Florian Fainelli Return-path: Received: from mail-qt1-f195.google.com ([209.85.160.195]:45612 "EHLO mail-qt1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730518AbeLPR5o (ORCPT ); Sun, 16 Dec 2018 12:57:44 -0500 Received: by mail-qt1-f195.google.com with SMTP id e5so11703718qtr.12 for ; Sun, 16 Dec 2018 09:57:43 -0800 (PST) In-Reply-To: Content-Disposition: inline Sender: netdev-owner@vger.kernel.org List-ID: On Sat, 15 Dec 2018 09:28:41 -0800, Florian Fainelli 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. Note that the driver doesn't put the chip ID in regs[3], that is just how Marvell registers are organized, chip ID are in port register 3 of all models. > For instance the B53 devices are organized by page (0-255), and then by > offsets (0-255 again). Within a page, there are registers that are > really per-port and some that are global. Also, registers can be > 8/16/32/48/64 bits (which is going to be interesting to support). Thanks, Vivien