Re: [PATCH v2 1/2] iio: light: ltr501: update header inclusions

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 20 Apr 2026 23:10:00 -0300
rafasales@usp.br wrote:

> From: "Rafael B. Sales" <rafasales@usp.br>
> 
> Update header inclusions to follow IWYU (Include What You Use)
> principle
> 
> Signed-off-by: Rafael B. Sales <rafasales@usp.br>
> Co-developed-by: Gustavo C. Arakaki <gustavo.arakaki@usp.br>
> Signed-off-by: Gustavo C. Arakaki <gustavo.arakaki@usp.br>
Hi,

One of the issues with the kernel view of IWYU is it's a bit fuzzy
(and not well documented yet).
That means there are some headers we assume will always include
certain others. So if you need the 'outer' header the inner one
is not normally included as well.

So the comments below are very much my opinion on what should
and should not be there.  Others may have different views and
it is always a little flexible for some of them.

> ---
>  drivers/iio/light/ltr501.c | 36 +++++++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/light/ltr501.c b/drivers/iio/light/ltr501.c
> index 4d99ae336f61..75a49fd9bce0 100644
> --- a/drivers/iio/light/ltr501.c
> +++ b/drivers/iio/light/ltr501.c
> @@ -9,20 +9,38 @@
>   * TODO: IR LED characteristics
>   */
>  
> -#include <linux/module.h>
> -#include <linux/mod_devicetable.h>
> -#include <linux/i2c.h>
> -#include <linux/err.h>
> +#include <asm/page.h>
asm headers if they are needed are always in a separate block
after all the linux/ ones

However, it is very rare to see asm/page.h in a driver. Why do
you need it here? 

I'm going to guess it is for PAGE_SIZE?

That bit of code needs a rework to use sysfs_emit_at()
instead. If you have time please could you make that change as well
as a precursor to this patch and then we won't need this one.

Alternative would be to convert to using read_avail().
That would be even better but is a more complex change.

> +#include <linux/array_size.h>
> +#include <linux/bitops.h>
I think it is always safe to assume bitops.h will include bits.h

> +#include <linux/bits.h>
> +#include <linux/byteorder/generic.h>
> +#include <linux/cleanup.h>

Is this used at this point?  I think this might want to be in the following patch.

>  #include <linux/delay.h>
> -#include <linux/regmap.h>
> -#include <linux/regulator/consumer.h>

> -
> -#include <linux/iio/iio.h>
> +#include <linux/dev_printk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>

It's a bit indirect but I think we can always assume err.h will include
errno.h

> +#include <linux/i2c.h>
> +#include <linux/iio/buffer.h>
>  #include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/trigger_consumer.h>
> -#include <linux/iio/buffer.h>
>  #include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/types.h>
Common practice (as this driver was previously doing) is to keep the
iio headers in a separate block at the end.  I don't tend to enforce this
but given the driver was already doing so, I think keeping it that way makes sense.

> +#include <linux/interrupt.h>
> +#include <linux/irqreturn.h>

interrupt.h will always include irqreturn.h so don't need
irqreturn.h

> +#include <linux/math.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/sprintf.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
>  
>  #define LTR501_ALS_CONTR 0x80 /* ALS operation mode, SW reset */
>  #define LTR501_PS_CONTR 0x81 /* PS operation mode */