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
