On 04/06/2016 08:04 AM, Peter Meerwald-Stadler wrote:
>> Add support for HopeRF pressure and temperature sensor.
>
> some minor comments
Thanks
[...]
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/pressure/hp03.txt
>> @@ -0,0 +1,17 @@
>> +HopeRF HP03 digital pressure/temperature sensors
>> +
>> +Required properties:
>> +- compatible: must be "hoperf,hp03"
>> +- xclr-gpio: must be device tree identifier of the XCLR pin .
>
> remove space before .
> indentation seems wrong
OK, fixed now.
>> + The XCLR pin is a reset of the ADC in the chip,
>> + it must be pulled HI before the conversion and
>> + readout of the value from the ADC registers and
>> + pulled LO afterward.
>> +
>> +Example:
[...]
>> +static int hp03_read_raw(struct iio_dev *indio_dev,
>> + struct iio_chan_spec const *chan,
>> + int *val, int *val2, long mask)
>> +{
>> + struct hp03_priv *priv = iio_priv(indio_dev);
>> + int ret;
>> +
>> + mutex_lock(&priv->lock);
>> + ret = hp03_update_temp_pressure(priv);
>> + mutex_unlock(&priv->lock);
>> +
>> + if (ret)
>> + return ret;
>> +
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + switch (chan->type) {
>> + case IIO_PRESSURE:
>> + *val = priv->pressure / 100;
>> + *val2 = (priv->pressure % 100) * 10000;
>> + ret = IIO_VAL_INT_PLUS_MICRO;
>
> maybe return directly, here and below
Good point. Looking at the code, I think I will also need to change the
code like the snippet below. The priv->pressure is in 1Pa steps, the
priv->temp is in 0.01C steps. Does it make sense or am I confused ?
switch (mask) {
case IIO_CHAN_INFO_RAW:
switch (chan->type) {
case IIO_PRESSURE:
*val = priv->pressure;
return IIO_VAL_INT;
case IIO_TEMP:
*val = priv->temp;
return IIO_VAL_INT;
default:
return -EINVAL;
}
break;
case IIO_CHAN_INFO_SCALE:
switch (chan->type) {
case IIO_PRESSURE:
*val = 1;
return IIO_VAL_INT;
case IIO_TEMP:
*val = 0;
*val2 = 10000;
return IIO_VAL_INT_PLUS_MICRO;
default:
return -EINVAL;
}
break;
default:
return -EINVAL;
}
>> + break;
>> + case IIO_TEMP:
>> + *val = priv->temp / 100;
>> + *val2 = (priv->temp % 100) * 10000;
>> + ret = IIO_VAL_INT_PLUS_MICRO;
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> + break;
>> + case IIO_CHAN_INFO_SCALE:
>> + switch (chan->type) {
>> + case IIO_PRESSURE:
>> + case IIO_TEMP:
>> + *val = 0;
>> + *val2 = 10000;
>> + ret = IIO_VAL_INT_PLUS_MICRO;
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> + break;
>> +
>> + default:
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> + return ret;
>> +}
[...]
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
