Jonathan,
Just got back to fixing most of the issues you pointed out, excepting these:
On 03.11.2019 13:19, Jonathan Cameron wrote:
> +
>> +static const struct iio_chan_spec hdc200x_channels[] = {
>> + {
>> + .type = IIO_TEMP,
>> + .extend_name = "Temperature",
> Any use of extend_name changes the ABI and should be done extremely
> carefully. It basically means that generic userspace code cannot
> use your driver.
>
> Here I can't see any advantage in doing so at all so drop it.
If I have two temperature channels and provide no extend_name to at least one of them, they fail to register.
>
>> + },
>> + },
>> + {
>> + .type = IIO_TEMP,
>> + .extend_name = "Temperature_MAX",
> Could we use the IIO_CHAN_INFO_PEAK element for this? Given
> the different scale, we'd need to do some work to make it
> have the same scale as temp.
Started looking into this.
Are there any examples or docs of using IIO_CHAN_INFO_PEAK? Couldn't find much in the IIO subtree.
Is it used together with INFO_RAW? And temp&max temp channels have different scales already.
>> +{
>> + char tmp = (~mask & data->drdy_config) | val;
>> + int ret;
>> +
>> + ret = i2c_smbus_write_byte_data(data->client,
>> + HDC200X_REG_RESET_DRDY_INT_CONF, tmp);
>> + if (!ret)
>> + data->drdy_config = tmp;
>> +
>> + return ret;
>> +}
>> +
>> +static int hdc200x_get_measurement_word(struct hdc200x_data *data,
>> + struct iio_chan_spec const *chan)
> These wrappers add relatively little. I'd prefer that you just call
> the i2c calls directly instead. Less code and ultimately a tiny
> bit easier to review.
Removed wrappers that are called once. Leaving the wrapper called in different places, makes intent more clear IMO.
--
Eugene
