Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1658945

Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata

Show all headers | View raw

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@kernel.org> 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@gmail.com>
> 
> 
> > +#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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Andy Lutomirski <luto@kernel.org> - 2017-06-06 05:20 +0200
  Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-06-06 11:40 +0200
    Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Darren Hart <dvhart@infradead.org> - 2017-06-06 18:40 +0200
    Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Darren Hart <dvhart@infradead.org> - 2017-06-06 19:00 +0200
      Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-06-06 20:50 +0200
  Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Pali Rohár <pali.rohar@gmail.com> - 2017-06-06 12:10 +0200
    Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Darren Hart <dvhart@infradead.org> - 2017-06-06 19:10 +0200
      Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded Binary WMI MOF metadata Pali Rohár <pali.rohar@gmail.com> - 2017-06-06 23:00 +0200
  Re: [PATCH v2] platform/x86: wmi-bmof: New driver to expose embedded  Binary WMI MOF metadata Andy Lutomirski <luto@amacapital.net> - 2017-06-07 00:40 +0200

csiph-web