Re: [PATCH r8169] ethtool support and sane speed selection/detection

[Andy Lutomirski <luto@xxxxxxxxxxxxx>]

Felipe W Damasio wrote:

> > +static int rtl8169_get_settings(struct net_device *dev,
> > +                   struct ethtool_cmd *cmd)
> > +{
> > +    struct rtl8169_private *tp = dev->priv;
> > +    void *ioaddr = tp->mmio_addr;
> > +    u8 status = RTL_R8(PHYstatus);
>
>
>     IMHO you should hold the device lock here (tp->lock).

Why?  Isn't reading the PHYstatus atomic w.r.t. writing it?  I think the 
lock just needs to be held for writes.

Oh -- there is a race if set_settings is called between the PHYstatus 
read and the rest of the code.  I can't imagine that happening, but it 
wouldn't hurt to take the lock.

Francois, do you want a new patch, or should I wait until you finish 
whatever you're doing?

--Andy