On Tue, Oct 05, 2004 at 06:02:47PM +0200, Steffen Klassert wrote:
> On Tue, Oct 05, 2004 at 10:17:17AM -0400 or thereabouts, Jeff Garzik wrote:
> > >+static u32 vortex_get_link(struct net_device *dev)
> > >+{
> > >+ struct vortex_private *vp = netdev_priv(dev);
> > >+ long ioaddr = dev->base_addr;
> > >+ EL3WINDOW(4);
> > >+ return mii_link_ok(&vp->mii);
> > >+}
> >
> > 3c59x should properly use netif_carrier_{on,off}, at which point you can
> > eliminate vortex_get_link in favor of generic ethtool_op_get_link
>
> I tried to use ethtool_op_get_link, but then ethtool reports
> Link detected: yes
> It does not matter wether the cable is connected or not.
Thus my comment "3c59x should properly use netif_carrier_{on,off}" :)
> > >+static int vortex_set_settings(struct net_device *dev, struct ethtool_cmd
> > >*cmd)
> > >+{
> > >+ struct vortex_private *vp = netdev_priv(dev);
> > >+ long ioaddr = dev->base_addr;
> > >+ EL3WINDOW(4);
> > >+ return mii_ethtool_sset(&vp->mii, cmd);
> > >+}
> >
> > I think that this and the other MII patch should do some amount of
> > locking, like the other drivers.
>
> Unlike the other drivers, the 3c59x locks with spin_lock_bh inside
> the mdio _{read,write} functions. Thats why I did not used any locks here.
Insufficient. 3c59x uses register windows, and you do not lock between
setting the window and using the window.
Jeff
|