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@xxxxxxxxxxxxx> > > 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@xxxxxxxxxxxxxxx> Intel Finland Oy
