On Tue, Jun 06, 2017 at 12:30:38PM +0300, Andy Shevchenko wrote: > On Tue, Jun 6, 2017 at 6:16 AM, Andy Lutomirski <luto@xxxxxxxxxx> wrote: > > Many laptops (and maybe servers?) have embedded WMI Binary MOF metadata. > > We do not yet have open-source tools for processing the data, although > > one is in the works thanks to Pali: > > > > https://github.com/pali/bmfdec > > > > There is currently no interface to get the data in the first place. By > > exposing it, we facilitate the development of new tools. > > My comments below. > Overall, FWIW, > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > > > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/init.h> > > +#include <linux/slab.h> > > +#include <linux/types.h> > > +#include <linux/input.h> > > +#include <linux/input/sparse-keymap.h> > > +#include <linux/acpi.h> > > +#include <linux/string.h> > > +#include <linux/dmi.h> > > +#include <linux/wmi.h> > > +#include <acpi/video.h> > > Alphabetical order? Up to you. OK, I failed to audit this... lots we don't need in here. The minimum to build is: #include <linux/wmi.h> So assuming this was copy/pasted from another file. Again, no guidance in coding-style.rst on includes. Seems to me we should include what we specifically require, regardless of whether or not another header also happens to include it. We need acpi for example, even though wmi also includes it. We should include modules, even though acpi includes it. We use several other things we aren't including for, like memcpy dev_kzalloc sysfs_create_bin_file So I suggest: #include <linux/acpi.h> #include <linux/device.h> #include <linux/fs.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/string.h> #include <linux/sysfs.h> #include <linux/types.h> #include <linux/wmi.h> Which removes: #include <acpi/video.h> #include <linux/dmi.h> #include <linux/init.h> #include <linux/input.h> #include <linux/input/sparse-keymap.h> #include <linux/slab.h> And adds: #include <linux/device.h> #include <linux/fs.h> #include <linux/sysfs.h> -- Darren Hart VMware Open Source Technology Center
