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

Groups > linux.kernel > #1659049

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

Show all headers | View raw

On Tue, Jun 6, 2017 at 7:54 PM, Darren Hart <dvhart@infradead.org> wrote:
> 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:

>> > +#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.

Usually it's a sane choice.
Regarding to order the rationale I see there is easiest way to detect
(on the glance) what headers are already there and there is no
duplication. I saw in the past few patches to remove header
duplication since the original list wasn't in order in the first
place.

Of course there might be exceptions.

> 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>

Works for me!

-- 
With Best Regards,
Andy Shevchenko

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