Re: [PATCH 00/12] iio:accel: Header Cleanups.

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: linux-iio <linux-iio@vger.kernel.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Aleksei Mamlin <mamlinav@gmail.com>,
	Dan Robertson <dan@dlrobertson.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Harinath Nampally <harinath922@gmail.com>,
	Jelle van der Waa <jelle@vdwaa.nl>,
	Jonathan Bakker <xc-racer2@live.ca>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Linus Walleij <linus.walleij@linaro.org>,
	Michael Hennerich <Michael.Hennerich@analog.com>,
	Mike Looijmans <mike.looijmans@topic.nl>,
	Sean Nyekjaer <sean@geanix.com>,
	Tomas Melin <tomas.melin@vaisala.com>
Subject: Re: [PATCH 00/12] iio:accel: Header Cleanups.
Date: Fri, 11 Jun 2021 22:36:52 +0300	[thread overview]
Message-ID: <CAHp75Vfs0GdijtzU=Akw6Am1Cp2fO1=46kt0awBOWVeCAAK=nA@mail.gmail.com> (raw)
In-Reply-To: <20210611201903.751cdaf0@jic23-huawei>

On Fri, Jun 11, 2021 at 10:17 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 11 Jun 2021 21:35:06 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Jun 11, 2021 at 9:21 PM Jonathan Cameron <jic23@kernel.org> wrote:
> > > On Fri, 11 Jun 2021 20:48:08 +0300
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

...

> > But I understand you and not insisting that you have to go with it. My
> > point is that...
> >
> > > I 'could' just drop the kernel.h where added on the basis we were clearly
> > > getting it indirectly. I've not included a whole bunch of other suggestions
> > > on that basis.
> >
> > (Which is probably not a good idea, because explicit in this case is
> > better than implicit, i.e. kernel.h is not guaranteed to be included
> > by other headers and I have long standing work to actually make sure
> > that most of the headers won't require kernel.h!)
>
> I think I was unclear, what I was proposing was not to touch includes of
> kernel.h at all. So not make anything worse, but also not make it any better.
> Where added in this set, kernel.h was for things directly in kernel.h,
> not the files it includes.

Ah, that's good! That was my main point of worry.

> > > Note I didn't include a whole bunch of other headers on the basis
> > > they were a bit more esoteric.
> > >
> > > To give an idea of how noisy this is here's the output another file...
> >
> > ...the tool simply doesn't know anything about kernel and header
> > guarantees. That's why it tries play dumb.
> >
> > If you would like to continue with this, please drop the removal of
> > the headers that are not guaranteed to be included by others
> > (excluding kernel.h from the equation).
>
> This is where the confusion lies... I haven't done that (subject to bugs
> of course)
>
> > Otherwise it will become
> > someone else's problem to _reinstantiate_ all those headers, and since
> > I already had a headache with panic.h, I won't repeat it. Still no tag
> > from me, although no explicit NAK (actually opposite, no explicit ACK
> > because of the dependencies), you just really need to spend more time
> > on this.
>
> I've not removed any headers on the basis they were guaranteed to be
> included by others. The tool assumes the opposite model - everything
> should be explicitly included directly in the file where it's used.
> For some files it lists 50+ headers.
> These patches are very conservative on that front.
>
> What I haven't done is included everything under the sun that wasn't
> already included.  e.g. I've not included
> linux/device/driver.h on the basis it is definitely included by
> linux/device.h and that seems very unlikely to change.
>
> There are some corner cases that are more interesting - such as whether
> we can rely on interrupt.h always including irqreturn.h.  Plenty of IIO
> drivers don't call anything in interrupt.h because of various wrappers but
> do use the return values. So in this case we could switch many of them
> to the more specific header, but I haven't done that yet.

I guess you may consider a guarantee there.

Actually what kernel header mess misses right now is the list of those
guarantees.

But you see there cases like

using dev_err() and struct device * in the same C file, what do you include?

Temptation is to go with device.h, but I would go with

#include <linux/dev_printk.h>

struct device;

Not sure if that tool can handle this kind of use.

> So, in short, the headers for which includes are removed in this series are
> not used at all in the files in question (unless I messed up of course!).

Cool! So, let bots and other people have more time on eyeballing this.
Will see how it goes.

-- 
With Best Regards,
Andy Shevchenko