Re: [PATCH v2 03/11] iio: temperature: ltc2983: make headers conform to IWYU

[Jonathan Cameron]

> Remove the catch-all kernel.h header and add the missing headers to
> enforce IWYU.

Sashiko correctly observed that this wasn't really IWYU.  So, to avoid
confusion we should be referring to something "approximate IWYU taking into
account some headers that always include others".

> 
> Signed-off-by: Joshua Crofts <joshua.crofts1@xxxxxxxxx>
>
> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
> index fe9eec3058d4..b8deb9aa9d48 100644
> --- a/drivers/iio/temperature/ltc2983.c
> +++ b/drivers/iio/temperature/ltc2983.c
> @@ -5,19 +5,26 @@
>   *
>   * Copyright 2019 Analog Devices Inc.
>   */
> +
> +#include <linux/array_size.h>
>  #include <linux/bitfield.h>
> +#include <linux/bitops.h>
>  #include <linux/completion.h>
> +#include <linux/container_of.h>
> +#include <linux/dev_printk.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
> -#include <linux/errno.h>
> -#include <linux/kernel.h>
>  #include <linux/interrupt.h>
>  #include <linux/list.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/slab.h>
>  #include <linux/spi/spi.h>
> +#include <linux/types.h>
>  #include <linux/unaligned.h>

There were a few other sashiko comments... 

  [Severity: Medium]
  To fully conform to IWYU, are there other missing headers that should be
  included here?

  For example, __ltc2983_custom_sensor_new() calls strcmp():

 	if ((index % 2) != 0 && !strcmp(propname, "adi,custom-leak-detector"))
 		temp = temp * 1000000 + 273150000;

  Does this require including <linux/string.h>?

Seems reasonable to me.  Why did you not include it?

  Similarly, ltc2983_probe() calls devm_gpiod_get_optional() and usleep_range():

 	gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(gpio))
		return PTR_ERR(gpio);

 	usleep_range(1000, 1200);

  Would including <linux/gpio/consumer.h> and <linux/delay.h> be necessary
  here?

Both seem reasonable to me.


  Also, the ltc2983_pm_ops struct is defined using the DEFINE_SIMPLE_DEV_PM_OPS
  macro:

  static DEFINE_SIMPLE_DEV_PM_OPS(ltc2983_pm_ops, ltc2983_suspend,
  				ltc2983_resume);

  Does this macro require including <linux/pm.h>?


Also seems sensible.

-- 
Jonathan Cameron <jonathan.cameron@xxxxxxxxxxxxxxxx>