On 11/26/23 19:16, Jonathan Cameron wrote:
On Sat, 25 Nov 2023 23:26:23 +0100
Marek Vasut <marex@xxxxxxx> wrote:
The ISL76682 is very basic ALS which only supports ALS or IR mode
in four ranges, 1k/4k/16k/64k LUX. There is no IRQ support or any
other fancy functionality.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Reviewed-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
Signed-off-by: Marek Vasut <marex@xxxxxxx>
Hi Marek,
One last question + a comment in general. Act on that if you like.
Thanks,
Jonathan
+static int integration_time_available[] = { 0, ISL76682_INT_TIME_US };
Why have an available attribute for a single value. Is it useful for anything?
To report it to userspace, iio-sensor-proxy uses that to control the ALS
poll interval .
+static int isl76682_probe(struct i2c_client *client)
+{
...
+ indio_dev->info = &isl76682_info;
+ indio_dev->channels = isl76682_channels;
+ indio_dev->num_channels = ARRAY_SIZE(isl76682_channels);
+ indio_dev->name = ISL76682_DRIVER_NAME;
Trivial but I'm not a fan of using defines in cases like this. It just makes
me go find the define when I could see the string directly here.
In cases where matching or similar strictly requires the naming to be the same
in various places a define is useful. In this case less so.
Anyhow, it's a very minor comment so never mind if you prefer to leave it
as it stands.
I added it to V6 .
I'll wait for the integration time reply above and then send V6 .
