On Thu, 9 Sep 2021 16:01:48 +0300
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> On Thu, Sep 09, 2021 at 11:45:36AM +0200, Jacopo Mondi wrote:
> > Add support for the Senseair Sunrise 006-0-0007 driver through the
> > IIO subsystem.
>
> ...
>
> > + /*
> > + * Wake up sensor by sending sensor address: START, sensor address,
> > + * STOP. Sensor will not ACK this byte.
> > + *
> > + * The chip enters a low power state after 15msec without
>
> msec --> ms (everybody understands 'ms' unit)
yup. millisiemens :)
(couldn't resist)
Context is fine either way here.
>
> > + * communications or after a complete read/write sequence.
> > + */
>
> ...
>
> > + struct i2c_client *client = context;
> > + union i2c_smbus_data data;
> > +
> > + /* Discard reg address from values count. */
> > + if (count < 1)
> > + return -EINVAL;
> > + count--;
>
> Wouldn't be more natural to decrement and then check against 0?
>
> ...
>
> > + memcpy(&data.block[1], (u8 *)val_buf + 1, count);
>
> Not sure I understand why you need an explicit casting here.
C doesn't allow pointer arithmetic on void * (gcc has it as an extension
though so it's probably fine without the cast)
>
>
> ...
>
> > + mutex_lock(&sunrise->lock);
> > + ret = sunrise_read_word(sunrise, SUNRISE_ERROR_STATUS_REG, &value);
> > + if (ret) {
> > + mutex_unlock(&sunrise->lock);
>
> > + return -EINVAL;
>
> Why shadowing an actual error code?
>
> > + }
>
> ...
>
> > + /*
> > + * / 10^4 to comply with IIO scale for CO2 (percentage).
>
> "1 / 10^4"
>
> > + * The chip CO2 reading range is [400 - 5000] ppm
> > + * which corresponds to [0,004 - 0,5] %.
> > + */
>
