Re: [RFC/PATCH 03/13] net: wl12xx: remove some unnecessary prints

[Felipe Balbi <balbi@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2700 bytes --]

Hi,

On Mon, May 23, 2011 at 08:32:53AM +0300, Luciano Coelho wrote:
> > >> > in fact, i find these prints pretty useful.
> > >> > does changing wl1271_notice to wl1271_debug(DEBUG_MAC80211) will solve
> > >> > the "nosiness"?
> > >> > (i use DEBUG_MAC80211 rather than DEBUG_SDIO, as DEBUG_SDIO is really
> > >> > *very* noisy)
> > >> >
> > >> > (i'll send it as a new patch as the original patch was already applied)
> > >> >
> > >> > Eliad.
> > >> >
> > >>
> > >> err... s/nosiness/noisiness/
> > >
> > > I think these are pretty useless.  You can see whether the driver is
> > > loaded or not by lsmod'ing.  You can also use ftrace to get the same
> > > stuff, if you want to know whether the driver is loaded or not offline.
> > > Or what is the scenario where you think this is useful?
> > >
> > i was thinking about a simple offline log analysis, where it's pretty
> > useful to know when the wl12xx_sdio was insmod'ed/rmmod'ed.

It's quite weird statement IMHO. Drivers will load either if you
modprobe by hand of a device is created and matches a driver, in which
case udev will load it for you. In both cases, either you know driver is
loaded because you manually loaded it or, if you're connected to web, or
ifconfig -a shows something, or you can ping around etc...

Besides, if the driver doesn't load because e.g. it failed to allocate
memory, you should get a failure report on dmesg.

The best way to handle this is to be as silent as possible on the
working case and only bother people on failures ;-)

> > i haven't tried ftrace yet. i'll give it a look.
> > anyway, the whole wl12xx driver is full of similar logs that get
> > called multiple times, so i don't see the real advantage of removing 2
> > prints that get called only once.
> 
> Yeah, the driver is full of useless logs.  That's why I'm going to
> rework it.  My idea is to use more standard tracing (nobody needs to
> learn wl12xx debug bitmask and how to set it), using ftrace and friends.

you can also play around with dynamic printk ;-) You use standard
dev_dbg() macros but can enable disable those by line, file, module, or
some simple regexes.

> > > I'm reworking the whole way our traces are handled, so I don't think
> > > reintroducing them is a good thing.
> > 
> > ok. so i'll just wait for your rework :)
> 
> It may still take a bit of time, because it's not my highest priority
> right now and the changes are quite intrusive (in the sense that they
> will touch every file), but when it's ready, I think it's going to be
> cool, you'll be able to use trace-cmd, kernelshark etc.

that's neat. I should do the same for musb ;-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]