mbox series

[0/7] staging:iio: Header cleanup

Message ID 20210611152614.109361-1-jic23@kernel.org (mailing list archive)
Headers show
Series staging:iio: Header cleanup | expand

Message

Jonathan Cameron June 11, 2021, 3:26 p.m. UTC
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

0-day recently started giving some reports from the include-what-you-use
tool (needs LLVM builds).

I was curious so decided to give it a spin.  It produces a wealth of
information, but the suggestions require a high degree of interpretation
and some choices are down to personal taste.

This set covers resulting changes that I think make sense for remaining
IIO drivers in staging (which I just noticed are all Analog devices ones :)

Jonathan Cameron (7):
  staging:iio:adc: Cleanup includes
  staging:iio:addac:adt7316: Cleanup includes
  staging:iio:cdc:ad7746: Cleanup includes
  staging:iio:frequency: Cleanup includes
  staging:iio:impedance-analyzer: Cleanup includes
  staging:iio:meter:ade7854: Cleanup includes
  staging:iio:resolver:ad2s1210: Cleanup includes

 drivers/staging/iio/adc/ad7280a.c               | 2 ++
 drivers/staging/iio/adc/ad7816.c                | 3 +--
 drivers/staging/iio/addac/adt7316.c             | 5 -----
 drivers/staging/iio/addac/adt7316.h             | 1 +
 drivers/staging/iio/cdc/ad7746.c                | 6 +++---
 drivers/staging/iio/frequency/ad9832.c          | 3 ++-
 drivers/staging/iio/frequency/ad9834.c          | 6 ++----
 drivers/staging/iio/impedance-analyzer/ad5933.c | 5 ++++-
 drivers/staging/iio/meter/ade7854-i2c.c         | 3 ++-
 drivers/staging/iio/meter/ade7854-spi.c         | 3 ++-
 drivers/staging/iio/meter/ade7854.c             | 4 ----
 drivers/staging/iio/meter/ade7854.h             | 5 +++++
 drivers/staging/iio/resolver/ad2s1210.c         | 5 +++--
 13 files changed, 27 insertions(+), 24 deletions(-)

Comments

Andy Shevchenko June 11, 2021, 5:45 p.m. UTC | #1
On Fri, Jun 11, 2021 at 6:25 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> 0-day recently started giving some reports from the include-what-you-use
> tool (needs LLVM builds).
>
> I was curious so decided to give it a spin.  It produces a wealth of
> information, but the suggestions require a high degree of interpretation
> and some choices are down to personal taste.
>
> This set covers resulting changes that I think make sense for remaining
> IIO drivers in staging (which I just noticed are all Analog devices ones :)

In general it's a good idea, but the tool doesn't know the project specifics.
I believe that half of what you have done is simply wrong. That is, we
have a lot of drivers that include kernel.h which is in its turn a
rabbit hole of all possible headers and (circular) dependencies. So,
for this and the other series, please double check that removed
headers are not removed due to kernel.h (I believe this is the case
for almost all if not all entries of slab.h, for example).

The other half seems correct. But due to the above I can't give any
tag on these...
Jonathan Cameron June 11, 2021, 6:14 p.m. UTC | #2
On Fri, 11 Jun 2021 20:45:03 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Fri, Jun 11, 2021 at 6:25 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> >
> > 0-day recently started giving some reports from the include-what-you-use
> > tool (needs LLVM builds).
> >
> > I was curious so decided to give it a spin.  It produces a wealth of
> > information, but the suggestions require a high degree of interpretation
> > and some choices are down to personal taste.
> >
> > This set covers resulting changes that I think make sense for remaining
> > IIO drivers in staging (which I just noticed are all Analog devices ones :)  
> 
> In general it's a good idea, but the tool doesn't know the project specifics.
> I believe that half of what you have done is simply wrong. That is, we
> have a lot of drivers that include kernel.h which is in its turn a
> rabbit hole of all possible headers and (circular) dependencies. So,
> for this and the other series, please double check that removed
> headers are not removed due to kernel.h (I believe this is the case
> for almost all if not all entries of slab.h, for example).

The tool seems to go rather the other way and suggest including things
that are 'obviously' included via another header that we need.

The reason for kernel.h includes when being added is almost always ARRAY_SIZE
or container_of as you identified.

The drivers where I'm removing slab.h don't actually make any direct
allocate or free calls, they are all wrapped up in various IIO core function
(devm_iio_device_alloc etc.)

Jonathan


> 
> The other half seems correct. But due to the above I can't give any
> tag on these...
>
Andy Shevchenko June 11, 2021, 6:36 p.m. UTC | #3
On Fri, Jun 11, 2021 at 9:12 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Fri, 11 Jun 2021 20:45:03 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Jun 11, 2021 at 6:25 PM Jonathan Cameron <jic23@kernel.org> wrote:

...

> The reason for kernel.h includes when being added is almost always ARRAY_SIZE
> or container_of as you identified.

Give me some time. I will cook a couple of patches for ya to test
(splitting those from kernel.h).