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

[Andy Lutomirski <luto@myrealbox.com>]

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