On Sep 7 23:52, Francois Romieu wrote: > Corinna Vinschen <vinsc...@redhat.com> : > [...] > > I have a bit of a problem with this patch. It crashes when loading the > > driver manually w/ modprobe. For some reason dev_get_stats is called > > during rtl_init_one and at that time the counters pointer is NULL, so > > the kernel gets a SEGV. > > > > After I worked around that I got a SEGV in > > rtl_remove_one, which I still have to find out why. I didn't test with > > the latest kernel, though, so I still have to check if that's the reason > > for the crashes. That takes a bit longer, I just wanted to let you > > know. > > Call me stupid: I forgot that it's perfectly fine to request the stats > of a down interface. :o/ > > Updated patch is on the way. > > > There's also a problem with rtl8169_cmd_counters: It always calls > > rtl_udelay_loop_wait_low w/ rtl_reset_counters_cond, even when called > > from rtl8169_update_counters, where it should call > > rtl_udelay_loop_wait_low w/ rtl_counters_cond to check for the > > CounterDump flag, rather than for the CounterReset flag. > > > > For now I applied the below patch, but I think it's a bit ugly to > > conditionalize the rtl_cond struct by the incoming flag value. > > <captain obvious> > > Let's check both at once: > > return RTL_R32(CounterAddrLow) & (CounterDump | CounterReset);
If you're sure that's valid, then why not? It's certainly cleaner code. Corinna
pgpNZQ0zHUR2P.pgp
Description: PGP signature
