Re: [PATCH v5 3/4] iio: chemical: Add Senseair Sunrise 006-0-007 driver

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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)

> +	 * 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.


...

> +	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] %.
> +			 */

-- 
With Best Regards,
Andy Shevchenko