lkml.org 
[lkml]   [2018]   [Feb]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v1 1/2] platform/x86: intel-vbtn: Remove redundant inclusions
From
Date
On Wed, 2018-01-31 at 11:04 -0800, Darren Hart wrote:
> On Wed, Jan 31, 2018 at 08:59:20PM +0200, Andy Shevchenko wrote:
> > On Wed, Jan 31, 2018 at 8:50 PM, Darren Hart <dvhart@infradead.org>
> > wrote:
> > > On Wed, Jan 31, 2018 at 07:54:06PM +0200, Andy Shevchenko wrote:
> > > > Some headers are not needed since the driver can be built as
> > > > module.
> > > > Remove them.
> > >
> > > Removing init because it's included by module.h, and removing
> > > acpi_bus.h
> > > because it's included by acpi.h - but not because it can be built
> > > as a
> > > module - right? Just checking, the wording in the commit msg
> > > seemed odd
> > > to me.
> >
> > Correct. I'll rephrase this in place.


My gosh, I forgot to do this and can't rebase anymore. Sorry.

> > > These removals seem appropriate to me, but so we have it recorded
> > > here -
> > > in general, including headers that you explicitly make use of is
> > > good
> > > practice, and not depending on others to include them. But in this
> > > case,
> > > the implicit includes are reasonable expectations as they are
> > > tightly
> > > coupled with the parent include.
> >
> > There are two classes of headers (at least?):
> > - let say "user-visible" ones, which drivers usually include like
> > you
> > pointed above
> > - low level ones, which in most cases are not supposed to be
> > included directly
> >
> > So, for first class I agree with you, and acpi_bus.h in this case
> > can
> > be considered as an example of second class.
>
> Agreed, acpi_bus.h is tightly coupled with acpi.h. The practice I've
> seen from others and want to discourage / avoid is including acpi.h
> and
> then deleting ... say... spinlock.h because somewhere somehow acpi.h
> also pulls it in. They are not tightly coupled conceptually, so
> spinlock.h should remain in the include list if the file uses
> spinlocks
> directly. I think we're in violent agreement here :-)

It's a problem of header organization I think. AFAIK Plan 9 has an idea
that each header is independent, and each C module has to include
headers in appropriate order. (Always trade off between flexibility and
strict hierarchy).

--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

\
 
 \ /
  Last update: 2018-02-05 17:24    [W:0.088 / U:1.648 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site