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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Nov 21, 2023 at 04:14:54AM +0100, Marek Vasut wrote:
> 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.

OK.

...

> > > +
> > 
> > Redundant blank line.
> > 
> > > +module_i2c_driver(isl76682_driver);
> 
> That ^ newline is above the module_i2c_driver or below it ?
> 
> I removed the one below .

Hmm... Comment was clearly about above one (as you see a single + there).

-- 
With Best Regards,
Andy Shevchenko






[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux