Re: [PATCH v3 2/2] iio: light: isl76682: Add ISL76682 driver

[Marek Vasut <marex@denx.de>]

On 11/20/23 13:02, Andy Shevchenko wrote:

[...]

>> +static int isl76682_clear_configure_reg(struct isl76682_chip *chip)
>> +{
>> +	struct device *dev = regmap_get_device(chip->regmap);
>> +	int ret;
>> +
>> +	ret = regmap_write(chip->regmap, ISL76682_REG_COMMAND, 0x0);
>> +	if (ret < 0)
>> +		dev_err(dev, "Error %d clearing the CONFIGURE register\n", ret);
> 
>> +	chip->command = 0;
> 
> Even in an error case? Is it a problem?

I added a comment in V4, if the I2C communication fails, hopefully the 
next time user reads data the command register will be rewritten again 
and the communication would have succeeded by then (assuming this was 
some weird glitch on the I2C bus). So this is best effort attempt to 
recover from that.

>> +static const struct of_device_id isl76682_of_match[] = {
>> +	{ .compatible = "isil,isl76682", },
>> +	{ },
> 
> Remove comma from the terminator.
> 
>> +};
> 
> ...
> 
>> +
> 
> Redundant blank line.
> 
>> +module_i2c_driver(isl76682_driver);

That ^ newline is above the module_i2c_driver or below it ?

I removed the one below .