Ivan Vecera <ivecera@redhat.com> :
quoted text
> Some Realtek chips (RTL8169sb/8110sb in my case) are unable to retrieve
> ethtool statistics when the interface is down. The process stays in
> endless loop in rtl8169_get_ethtool_stats. This is because these chips
> need to have receiver enabled (CmdRxEnb bit in ChipCmd register) that is
> cleared when the interface is going down. It's better to update statistics
> only when the interface is up and otherwise return copy of statistics
> grabbed when the interface was up (in rtl8169_close).

It makes sense.

[...]
quoted text
> +static void rtl8169_update_counters(struct net_device *dev)
>  {
>  	struct rtl8169_private *tp = netdev_priv(dev);
>  	void __iomem *ioaddr = tp->mmio_addr;
>  	struct rtl8169_counters *counters;
>  	dma_addr_t paddr;
>  	u32 cmd;
> +	int wait = 1000;
>  
> -	ASSERT_RTNL();
> +	if (!netif_running(dev))
> +		return;


int dev_close(struct net_device *dev)
{
[...]
	clear_bit(__LINK_STATE_START, &dev->state);
        -> netif_running is not true anymore
[...]
	if (ops->ndo_stop)
		ops->ndo_stop(dev);

-> Calling rtl8169_update_counters in rtl8169_close looks like a no-op.

Aside from that, I do not understand from the description the need for
the "while (wait--)" loop.

Aside from the aside, the diff can probably be shorter if a local "counters"
variable is kept in the ethtool function (nit).

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html