Re: [PATCH v9 1/2] iio: humidity: Add TI HDC20x0 support

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Fri, Jul 10, 2020 at 2:54 PM Eugene Zaikonnikov <ez@norphonic.com> wrote:
>
> Add support for HDC2010/2080 driver and sysfs documentation for its
> heater element.
>
> HDC2010 is an integrated high-accuracy humidity and temperature sensor
> with very low power consumption. The device includes a resistive heating
> element. The temperature range is -40C to 125C with 0.2C
> accuracy. Humidity measurement is 0 to 100% with 2% RH accuracy.

Now, some almost context-less comments to the contribution.

Please, use Datasheet tag(s) here with URL(s) to the datasheet(s).
Also, be sure you are using https (note S) everywhere.

Datasheet: https://...
Datasheet: ...

> Signed-off-by: Eugene Zaikonnikov <ez@norphonic.com>

1. No need to put file name into the file
2. Missed at least bits.h inclusion
3. Keep comma in HDC2010_GROUP_HUMIDITY
4. IIO_CONST_ATTR can be one line, but hey don't we have IIO core to
take care of it?
5. Use traditional pattern

if (ret)
  return ret;

data->drdy_config = tmp;

return 0;

6. Indent better

if (!i2c_check_functionality(client->adapter,
    I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_I2C))

(or split last line above by operand per line or alike)

7. Drop unneeded casting
tmp = (data->measurement_config & ~HDC2010_MEAS_CONF) | HDC2010_MEAS_TRIG;

8. It's one line
ret = i2c_smbus_write_byte_data(client,
    HDC2010_REG_MEASUREMENT_CONF, tmp);

Ditto:
dev_warn(&client->dev, "Unable to restore default AMM\n");

In general it doesn't look bad!

-- 
With Best Regards,
Andy Shevchenko