On Fri, 27 Feb 2009, Pavel Machek wrote: > > +module_param(debug_ec, bool, S_IRUGO); > > +MODULE_PARM_DESC(debug_ec, > > + "Present EC debugging interface in procfs. WARNING: writing to the " > > + "EC can hang your system and possibly damage your hardware."); > > Sounds dangerous and clearly does not belong to /proc. Please drop it. ThinkPad ACPI also has one. OTOH, we are fairly sure one cannot damage the standard ThinkPad hardware through the "IBM thinkpad-style" EC firmware, the EC sanity-checks things. I suppose you could cause the box to go away for lunch and don't come back until a complete hardware reset, or hit one of the crash-the-EC-firmware bugs by accident, though. It has been invaluable for debugging. I can well guess why the thinkpad-sl driver would want to add it as well. What I am heavily considering is to taint the kernel when it is used (plus it is not enabled by default, requires a module parameter to enable, etc), plus output a big warning to syslog. That is probably enough to keep the users from abusing it. > > +/************************************************************************* > > + bluetooth sysfs - copied nearly verbatim from thinkpad_acpi.c > > + *************************************************************************/ > > That's quite a lot of code for verbatim copy; create shared helper? Yeah, I am game. But let's do it later on and not delay anything because of it, please. I will start breaking thinkpad-acpi into multiple source files, and after that is done (should take a while, thinkpad-acpi is a big module with a LOT of subdriver interdependencies) we can look for the best way to reduce code duplication. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
