On Tue, Nov 21, 2023 at 04:10:40AM +0100, Marek Vasut 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.
...
> + for (i = 0; i < ARRAY_SIZE(isl76682_range_table); i++) {
> + if (chan->type == IIO_LIGHT) {
> + if (val2 != isl76682_range_table[i].als)
> + continue;
> + } else if (chan->type == IIO_INTENSITY) {
> + if (val2 != isl76682_range_table[i].ir)
> + continue;
> + }
Redundant 'else' and you can combine if:s.
if (chan->type == IIO_LIGHT && val2 != isl76682_range_table[i].als)
continue;
if (chan->type == IIO_INTENSITY && val2 != isl76682_range_table[i].ir)
continue;
But it's up to you and Jonathan.
> + scoped_guard(mutex, &chip->lock)
> + chip->range = isl76682_range_table[i].range;
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
...
> + case IIO_CHAN_INFO_RAW:
> + switch (chan->type) {
> + case IIO_LIGHT:
> + ret = isl76682_get(chip, false, val);
> + return (ret < 0) ? ret : IIO_VAL_INT;
> + case IIO_INTENSITY:
> + ret = isl76682_get(chip, true, val);
> + return (ret < 0) ? ret : IIO_VAL_INT;
> + default:
> + break;
> + }
> +
> + return -EINVAL;
default:
return -EINVAL;
...
> +static const struct of_device_id isl76682_of_match[] = {
> + { .compatible = "isil,isl76682", },
Inner comma is not needed.
> + { }
> +};
...
> +
> +module_i2c_driver(isl76682_driver);
> +MODULE_DESCRIPTION("ISL76682 Ambient Light Sensor driver");
...other way around:
};
module_i2c_driver(isl76682_driver);
MODULE_DESCRIPTION("ISL76682 Ambient Light Sensor driver");
...
Assuming you address all these,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
--
With Best Regards,
Andy Shevchenko
