[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 7 Jul 2012 09:58:04 -0400 (EDT)
From: Émeric Vigier
<emeric.vigier@...oirfairelinux.com>
To: Ben Hutchings <bhutchings@...arflare.com>
Cc: Steve Glendinning <steve@...well.net>,
steve glendinning <steve.glendinning@...c.com>,
netdev@...r.kernel.org, Nancy Lin <nancy.lin@...c.com>
Subject: Re: [PATCH] smsc95xx: support ethtool get_regs
----- Mail original -----
> On Fri, 2012-07-06 at 14:15 -0400, Émeric Vigier wrote:
> > From: Emeric Vigier <emeric.vigier@...oirfairelinux.com>
> >
> > Inspired by implementation in smsc911x.c and smsc9420.c
> > Tested on ARM/pandaboard rev A3
> >
> > Signed-off-by: Emeric Vigier <emeric.vigier@...oirfairelinux.com>
> > ---
> > drivers/net/usb/smsc95xx.c | 37
> > +++++++++++++++++++++++++++++++++++++
> > 1 files changed, 37 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/usb/smsc95xx.c
> > b/drivers/net/usb/smsc95xx.c
> > index b1112e7..bce14f6 100644
> > --- a/drivers/net/usb/smsc95xx.c
> > +++ b/drivers/net/usb/smsc95xx.c
> > @@ -578,6 +578,41 @@ static int smsc95xx_ethtool_set_eeprom(struct
> > net_device *netdev,
> > return smsc95xx_write_eeprom(dev, ee->offset, ee->len, data);
> > }
> >
> > +
> > +static int smsc95xx_ethtool_getregslen(struct net_device *dev)
> > +{
> > + /* all smsc95xx registers plus all phy registers */
> > + return COE_CR - ID_REV + 1 + 32 * sizeof(u32);
> > +}
> > +
> > +static void
> > +smsc95xx_ethtool_getregs(struct net_device *netdev, struct
> > ethtool_regs *regs,
> > + void *buf)
> > +{
> > + struct usbnet *dev = netdev_priv(netdev);
> > + unsigned int i, j = 0, retval;
> > + u32 *data = buf;
> > +
> > + netif_dbg(dev, hw, dev->net, "ethtool_getregs\n");
> > +
> > + retval = smsc95xx_read_reg(dev, ID_REV, ®s->version);
> > + if (retval < 0) {
> > + netdev_warn(dev->net, "REGS: cannot read ID_REV\n");
> > + return;
> > + }
> > +
> > + for (i = 0; i <= COE_CR; i += (sizeof(u32))) {
> > + retval = smsc95xx_read_reg(dev, i, &data[j++]);
> > + if (retval < 0) {
> > + netdev_warn(dev->net, "REGS: cannot read reg[%x]\n", i);
> > + return;
> > + }
> > + }
>
> Why does this start with i = 0 whereas the calculation of the length
> uses ID_REV as the starting point? Maybe ID_REV == 0, but you should
> be
> consistent in whether you use the name or literal number.
You are right. I will broadcast ID_REV usage.
>
> > + for (i = 0; i <= PHY_SPECIAL; i++)
> > + data[j++] = smsc95xx_mdio_read(netdev, dev->mii.phy_id, i);
> > +}
>
> Again, why use PHY_SPECIAL (+ 1) here as opposed to 32 in the
> calculation of the length?
32 was ok, but I hesitated between defining a SMSC95XX_PHY_END or using the last defined register.
Are 32 register-PHY generic to most devices? I mean could 32 be use widely?
>
> Ben.
>
> > static const struct ethtool_ops smsc95xx_ethtool_ops = {
> > .get_link = usbnet_get_link,
> > .nway_reset = usbnet_nway_reset,
> > @@ -589,6 +624,8 @@ static const struct ethtool_ops
> > smsc95xx_ethtool_ops = {
> > .get_eeprom_len = smsc95xx_ethtool_get_eeprom_len,
> > .get_eeprom = smsc95xx_ethtool_get_eeprom,
> > .set_eeprom = smsc95xx_ethtool_set_eeprom,
> > + .get_regs_len = smsc95xx_ethtool_getregslen,
> > + .get_regs = smsc95xx_ethtool_getregs,
> > };
> >
> > static int smsc95xx_ioctl(struct net_device *netdev, struct ifreq
> > *rq, int cmd)
>
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
>
--
Emeric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists