On Aug 21 21:39, Francois Romieu wrote: > Corinna Vinschen <vinsc...@redhat.com> : > [...] > > diff --git a/drivers/net/ethernet/realtek/r8169.c > > b/drivers/net/ethernet/realtek/r8169.c > > index f790f61..f26a48d 100644 > > --- a/drivers/net/ethernet/realtek/r8169.c > > +++ b/drivers/net/ethernet/realtek/r8169.c > [...] > > @@ -2179,6 +2191,47 @@ static int rtl8169_get_sset_count(struct net_device > > *dev, int sset) > > } > > } > > > > +DECLARE_RTL_COND(rtl_reset_counters_cond) > > +{ > > + void __iomem *ioaddr = tp->mmio_addr; > > + > > + return RTL_R32(CounterAddrLow) & CounterReset; > > +} > > + > > +static void rtl8169_reset_counters(struct net_device *dev) > > +{ > > rtl8169_reset_counters duplicates most of rtl8169_update_counters. Please > factor out the dma_alloc + parametrized CounterAddrLow write + cleanup.
Ok, will do. > > + rtl8169_reset_counters(dev); > > + > > + rtl8169_update_counters(dev); > > > The code should propagate failure when both rtl8169_reset_counters and > rtl8169_update_counters fail. This one I don't understand. Neither failing to reset the counters nor failing to update the counters is fatal for the driver. So far the (unchanged) rtl8169_update_counters doesn't even print a log message, while a failing reset in rtl8169_reset_counters now does. Why is that not sufficent? Thanks, Corinna
pgpKL0AKrvb57.pgp
Description: PGP signature