Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753225AbdHOQ2a convert rfc822-to-8bit (ORCPT ); Tue, 15 Aug 2017 12:28:30 -0400 Received: from esa6.microchip.iphmx.com ([216.71.154.253]:36924 "EHLO esa6.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752421AbdHOQ22 (ORCPT ); Tue, 15 Aug 2017 12:28:28 -0400 X-IronPort-AV: E=Sophos;i="5.41,378,1498546800"; d="scan'208";a="3698787" From: To: , CC: , , , , , , , , , , , , , , , , Subject: RE: [PATCH net-next 06/11] net: dsa: debugfs: add port registers Thread-Topic: [PATCH net-next 06/11] net: dsa: debugfs: add port registers Thread-Index: AQHTFUx642NFW6Unz0KCvnzx4PhBcqKFmXYw Date: Tue, 15 Aug 2017 16:28:12 +0000 Message-ID: <9235D6609DB808459E95D78E17F2E43D40AFD219@CHN-SV-EXMX02.mchp-main.com> References: <20170814222242.10643-1-vivien.didelot@savoirfairelinux.com> <20170814222242.10643-7-vivien.didelot@savoirfairelinux.com> In-Reply-To: <20170814222242.10643-7-vivien.didelot@savoirfairelinux.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.10.76.4] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1580 Lines: 48 Vivien, > Subject: [PATCH net-next 06/11] net: dsa: debugfs: add port registers > > Add a debug filesystem "regs" entry to query a port's hardware registers > through the .get_regs_len and .get_regs_len switch operations. > > This is very convenient because it allows one to dump the registers of > DSA links, which are not exposed to userspace. This series will be very useful to get various debug information. Do you have any plan to expand 32bit register access and switch-level registers In addition to port-level registers? > +static void dsa_debugfs_regs_read_count(struct dsa_switch *ds, int id, > + struct seq_file *seq, int count) > +{ > + u16 data[count * ETH_GSTRING_LEN]; I think this should be u16 data[count] because it is value of registers. > + struct ethtool_regs regs; > + int i; > + > + ds->ops->get_regs(ds, id, ®s, data); > + > + for (i = 0; i < count / 2; i++) > + seq_printf(seq, "%2d: %04x\n", i, data[i]); > +} > + > +static int dsa_debugfs_regs_read(struct dsa_switch *ds, int id, > + struct seq_file *seq) > +{ > + int count; > + > + if (!ds->ops->get_regs_len || !ds->ops->get_regs) > + return -EOPNOTSUPP; > + > + count = ds->ops->get_regs_len(ds, id); > + if (count < 0) > + return count; Because get_regs_len returns length than count per mv88e6xxx/chip.c, it requires some math before passing to dsa_debugfs_regs_read_count(). It does "count/2" in dsa-debugfs_regs_read_count(), however, As commented above, "count" is used at "u16 data[...]". So, it would be nice to match with name. Ie, count or size/length. Thanks. Woojung