Re: [PATCH v5 2/4] iio: light: add support for veml6031x00 ALS series

From: Javier Carrasco

Date: Mon Aug 10 2026 - 19:09:43 EST


On Mon Aug 10, 2026 at 3:38 PM CEST, Andy Shevchenko wrote:
> On Fri, Aug 07, 2026 at 03:51:53PM +0200, Javier Carrasco wrote:
>> These sensors provide two light channels (ALS and IR), I2C communication
>> and a multiplexed interrupt line to signal data ready and configurable
>> threshold alarms.
>>
>> This first implementation provides basic functionality (measurement
>> configuration, raw reads and ID validation) and defines the different
>> register regions in preparation for extended features in the subsequent
>> patches of the series.
>
> Since it's going to be a new version, my comments below.
>

Hi Andy, thank you for your review. I agree with most of your comments
and I will apply your suggestions for V6.

> ...
>
> + array_size.h // ARRAY_SIZE()
>
>> +#include <linux/bitfield.h>
>
> Is this in use?
>

This one is in use for FIELD_PREP()

> + bits.h // BIT()
>
>> +#include <linux/cleanup.h>
>> +#include <linux/delay.h>
>
>> +#include <linux/device.h>
>
> Oh, the whole headers hell is loaded just due to dev_get_drvdata() it seems...
>

I will rely on <linux/i2c.h> instead, which is also what I am going to do
to get rid of <linux/mod_devicetable.h> as Uwe suggested.


>> +#include <linux/err.h>
>> +#include <linux/i2c.h>
>> +#include <linux/limits.h>
>
> I missed probably it, but is it used?
>

This one is used for U16_MAX.

>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/mutex.h>
>
>> +#include <linux/pm.h>
>

The rest of your comments need no reply from my side.

Best regards,
Javier