| Message ID | 1305321990-22041-4-git-send-email-balbi@ti.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Sat, 2011-05-14 at 00:26 +0300, Felipe Balbi wrote: > Those have little value. Remove those to make > the driver less noisy. > > Signed-off-by: Felipe Balbi <balbi@ti.com> > --- Applied also this one. Thanks!
On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote: > Those have little value. Remove those to make > the driver less noisy. > > Signed-off-by: Felipe Balbi <balbi@ti.com> > --- [...] > @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func, > /* Tell PM core that we don't need the card to be powered now */ > pm_runtime_put_noidle(&func->dev); > > - wl1271_notice("initialized"); > - > return 0; > > static void __exit wl1271_exit(void) > { > sdio_unregister_driver(&wl1271_sdio_driver); > - > - wl1271_notice("unloaded"); > } > 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. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, May 22, 2011 at 3:34 PM, Eliad Peller <eliad@wizery.com> wrote: > On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote: >> Those have little value. Remove those to make >> the driver less noisy. >> >> Signed-off-by: Felipe Balbi <balbi@ti.com> >> --- > [...] >> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func, >> /* Tell PM core that we don't need the card to be powered now */ >> pm_runtime_put_noidle(&func->dev); >> >> - wl1271_notice("initialized"); >> - >> return 0; >> > >> static void __exit wl1271_exit(void) >> { >> sdio_unregister_driver(&wl1271_sdio_driver); >> - >> - wl1271_notice("unloaded"); >> } >> > > 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/ -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 2011-05-22 at 15:37 +0300, Eliad Peller wrote: > On Sun, May 22, 2011 at 3:34 PM, Eliad Peller <eliad@wizery.com> wrote: > > On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote: > >> Those have little value. Remove those to make > >> the driver less noisy. > >> > >> Signed-off-by: Felipe Balbi <balbi@ti.com> > >> --- > > [...] > >> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func, > >> /* Tell PM core that we don't need the card to be powered now */ > >> pm_runtime_put_noidle(&func->dev); > >> > >> - wl1271_notice("initialized"); > >> - > >> return 0; > >> > > > >> static void __exit wl1271_exit(void) > >> { > >> sdio_unregister_driver(&wl1271_sdio_driver); > >> - > >> - wl1271_notice("unloaded"); > >> } > >> > > > > 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'm reworking the whole way our traces are handled, so I don't think reintroducing them is a good thing.
On Sun, May 22, 2011 at 8:30 PM, Luciano Coelho <coelho@ti.com> wrote: > On Sun, 2011-05-22 at 15:37 +0300, Eliad Peller wrote: >> On Sun, May 22, 2011 at 3:34 PM, Eliad Peller <eliad@wizery.com> wrote: >> > On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote: >> >> Those have little value. Remove those to make >> >> the driver less noisy. >> >> >> >> Signed-off-by: Felipe Balbi <balbi@ti.com> >> >> --- >> > [...] >> >> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func, >> >> /* Tell PM core that we don't need the card to be powered now */ >> >> pm_runtime_put_noidle(&func->dev); >> >> >> >> - wl1271_notice("initialized"); >> >> - >> >> return 0; >> >> >> > >> >> static void __exit wl1271_exit(void) >> >> { >> >> sdio_unregister_driver(&wl1271_sdio_driver); >> >> - >> >> - wl1271_notice("unloaded"); >> >> } >> >> >> > >> > 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. 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. > 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 :) thanks, Eliad. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-05-23 at 00:57 +0300, Eliad Peller wrote: > On Sun, May 22, 2011 at 8:30 PM, Luciano Coelho <coelho@ti.com> wrote: > > On Sun, 2011-05-22 at 15:37 +0300, Eliad Peller wrote: > >> On Sun, May 22, 2011 at 3:34 PM, Eliad Peller <eliad@wizery.com> wrote: > >> > On Sat, May 14, 2011 at 12:26 AM, Felipe Balbi <balbi@ti.com> wrote: > >> >> Those have little value. Remove those to make > >> >> the driver less noisy. > >> >> > >> >> Signed-off-by: Felipe Balbi <balbi@ti.com> > >> >> --- > >> > [...] > >> >> @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func, > >> >> /* Tell PM core that we don't need the card to be powered now */ > >> >> pm_runtime_put_noidle(&func->dev); > >> >> > >> >> - wl1271_notice("initialized"); > >> >> - > >> >> return 0; > >> >> > >> > > >> >> static void __exit wl1271_exit(void) > >> >> { > >> >> sdio_unregister_driver(&wl1271_sdio_driver); > >> >> - > >> >> - wl1271_notice("unloaded"); > >> >> } > >> >> > >> > > >> > 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. Have you really had to know when the modules are insmodded or rmmoded? > 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. These two were removed now because Felipe has reworked the SDIO/SPI modules and, at the same time, cleaned it up a little. I think it's good to clean up things little by little, especially on the parts that are being touched anyway. > > 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.
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 ;-)
diff --git a/drivers/net/wireless/wl12xx/sdio.c b/drivers/net/wireless/wl12xx/sdio.c index fe775d3..0832b80 100644 --- a/drivers/net/wireless/wl12xx/sdio.c +++ b/drivers/net/wireless/wl12xx/sdio.c @@ -287,8 +287,6 @@ static int __devinit wl1271_probe(struct sdio_func *func, /* Tell PM core that we don't need the card to be powered now */ pm_runtime_put_noidle(&func->dev); - wl1271_notice("initialized"); - return 0; err3: @@ -347,23 +345,12 @@ static struct sdio_driver wl1271_sdio_driver = { static int __init wl1271_init(void) { - int ret; - - ret = sdio_register_driver(&wl1271_sdio_driver); - if (ret < 0) { - wl1271_error("failed to register sdio driver: %d", ret); - goto out; - } - -out: - return ret; + return sdio_register_driver(&wl1271_sdio_driver); } static void __exit wl1271_exit(void) { sdio_unregister_driver(&wl1271_sdio_driver); - - wl1271_notice("unloaded"); } module_init(wl1271_init);
Those have little value. Remove those to make the driver less noisy. Signed-off-by: Felipe Balbi <balbi@ti.com> --- drivers/net/wireless/wl12xx/sdio.c | 15 +-------------- 1 files changed, 1 insertions(+), 14 deletions(-)