lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 6 Jul 2012 17:26:45 -0400 (EDT) From: Émeric Vigier <emeric.vigier@...oirfairelinux.com> To: Francois Romieu <romieu@...zoreil.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 ----- > De: "Francois Romieu" <romieu@...zoreil.com> > À: "Émeric Vigier" <emeric.vigier@...oirfairelinux.com> > Cc: "Steve Glendinning" <steve@...well.net>, "steve glendinning" <steve.glendinning@...c.com>, > netdev@...r.kernel.org, "Nancy Lin" <nancy.lin@...c.com> > Envoyé: Vendredi 6 Juillet 2012 16:01:46 > Objet: Re: [PATCH] smsc95xx: support ethtool get_regs > > Émeric Vigier <emeric.vigier@...oirfairelinux.com> : > [...] > > +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); > > I do not see where ID_REV is accounted for in the loops below. > > s/32 */PHY_SPECIAL */ or s/PHY_SPECIAL/32/ below. I will go for the second proposal. I love your sed syntax btw :-) > > I thought PHY registers were 16 bits wide. Moreover they are already > available through smsc95xx_ioctl(). Yes, there are 16 bits wide according to smsc95xx.h. But other smsc drivers define 32bit wide PHY regs. I made myself believe that smsc would use the same PHY for each ethernet chip. So would something like s/32 * sizeof(u32)/PHY_SPECIAL * sizeof(u16)/ solve the issue here? Concerning the ioctl, I found ethtool much easier to use. And I believe smsc9514 is a very popular chipset, so this could help others debugging it. > > > +} > > + > > +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; > > unsigned int i, j, retval; > > > + u32 *data = buf; > > + > > + netif_dbg(dev, hw, dev->net, "ethtool_getregs\n"); > > The tracing framework does provide almost the same information. Do you mean LTT? I am not familiar with it, I should have a look. I remove netif_dbg then. > > > + > > + retval = smsc95xx_read_reg(dev, ID_REV, ®s->version); > > + if (retval < 0) { > > + netdev_warn(dev->net, "REGS: cannot read ID_REV\n"); > > s/dev->net/netdev/ ? You are right, I also changed smsc95xx_ethtool_getregslen() definition to match this syntax. > > > > + return; > > + } > > + > > + for (i = 0; i <= COE_CR; i += (sizeof(u32))) { > > + retval = smsc95xx_read_reg(dev, i, &data[j++]); > > for (i = 0, j = 0; i <= COE_CR; i += sizeof(u32), j++) { > retval = smsc95xx_read_reg(dev, i, data + j); I should change that in previous "for" loop as well I suppose? > > -- > Ueimor > 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