On 02/08/13 20:31, Andy Shevchenko wrote: > On Fri, 2013-08-02 at 20:14 +0200, Benjamin Tissoires wrote: >> On 02/08/13 16:49, Andy Shevchenko wrote: >>> On Fri, 2013-08-02 at 16:30 +0200, Benjamin Tissoires wrote: > > [] > >>>> Moreover, the dev_dbg calls are compiled out if DEBUG symbol is not >>>> set. >>> >>> Yes, and what is the difference between previously used >>> if (debug) >>> dev_dbg(...) >>> >>> ? >> >> That's because the previous code was directly using >> dev_printk(KERN_DEBUG,...) and not dev_dbg() within the condition. >> >> This call does not use dynamic debugging or does not get compiled out if >> DEBUG is not set. > > Yeah, I've checked this just after I sent the message. > >> Also, to my defence, I can add that I developed this driver on an ARM >> board without dynamic debugging support enabled... :) > > > So, the problem is DYNAMIC_DEBUG is not default option in kernel (and I > could imagine why). > > I checked few distros for that: Debian/Ubuntu - off, > Fedora/OpenSuse/ArchLinux - on. > > Just to understand what kind of user might run this with debug? Is it > developer or person who knows the subject? If it's not a such person, it > would be difficult to him or her to enable dynamic debug in distros > like Ubuntu, which sucks. I'm afraid people reporting this kind of bugs are most of the time regular users, who just bought a brand new laptop, and who send a bug report through their distribution's bugzilla... :( I'm not expecting that much of developers or person who knows the subject to actually report the problem, because most of the time a normal user reports "my touchpad is not working". At least, that's what I have mostly seen in the input subsystem. > >>>> So, if we ever have to change this debug variable, I would prefer >>>> using the hid debug environment which would at least limit the number >>>> of debug outputs to the HID subsystem. >>> >>> Usually when I see such code I understood it was written in >>> pre-dynamic-debug epoch. So, my point is to switch to dynamic debug and >>> use it efficiently. >> >> Ok, so if you can guarantee me that adding the proper kernel parameter >> will allow me to retrieve all the i2c-hid logs from the boot, then I'd >> be happy to ack this. However, I have no way to test this right now, so >> I'll need to trust you (that's why I'm asking you to do proper testing). > > With only one condition if dynamic debug is enabled in kernel. > So, it would be nice to gather opinions, however, the decision is > totally depends on type of user who wants to debug the module. > Yep, I agree that Jiri's opinion would be helpful. Meanwhile, we could also wait a little for more i2c-hid to hit the market and to be widely tested, and then remove the debug flag. We should also remove the debug events in get_i2c_hid_get_input() which would pollute systems without dynamic debugging but with the DEBUG config still enabled. Cheers, Benjamin -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
