Re: [PATCH V3] iio: pressure: hp03: Add Hope RF HP03 sensor support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 18.04.2016 14:26, Marek Vasut wrote:
On 04/16/2016 09:40 PM, Jonathan Cameron wrote:
On 10/04/16 21:52, Marek Vasut wrote:
Add support for HopeRF pressure and temperature sensor.

This device uses two fixed I2C addresses, one for storing
calibration coefficients and another for accessing the ADC.

Signed-off-by: Marek Vasut <marex@xxxxxxx>
Cc: Matt Ranostay <mranostay@xxxxxxxxx>
Cc: Jonathan Cameron <jic23@xxxxxxxxxx>
Sorry I didn't get to this earlier in the week.

Unfortunately the resulting scales don't match the standard ABI for these
two channel types.

Ah, sorry for the inconvenience.

Otherwise, looks good. I've cc'd the devicetree list and maintainers.
The binding is trivial I think, but always good to give people a
opportunity to comment.

Jonathan
---
V2: - Expand the binding document with more details on the XCLR pin
    - Switch from IIO_CHAN_INFO_PROCESSED to RAW + SCALE
    - Add failpath into hp03_update_temp_pressure() for the case
      when ADC readout fails. This correctly sets the XCLR pin back
      to LO now.
    - Add comment explaining the need for allocation of child device
      in hp03_probe().
V3: - Fix indent in the DT binding documentation
    - Report raw pressure and temperature unmodified
Good
    - Report pressure scale to be 1 , since pressure is in Pa
Standard units for pressure (see Documentation/ABI/testing/sysfs-bus-iio
are KPa so it wants to report 0.001)

OK, got it.

- Report temperature scale to be 0.01 , since temp is in 0.01C steps
Unfortunately the documented base unit for temp (originally from hwmon before we started going for SI units every time) are milli Celcius. Thus the value reported * scale should end up in milli degrees Celcius. Hence if it is in 0.01
steps the scale should be 0.1

Shouldn't this be 10 ? The value is in 0.01C steps , so the value has to
be multiplied by 10 to convert it into mC units.
err. yes I'm clearly wrong :)


---
 .../devicetree/bindings/iio/pressure/hp03.txt      |  17 ++
 drivers/iio/pressure/Kconfig                       |  11 +
 drivers/iio/pressure/Makefile                      |   1 +
drivers/iio/pressure/hp03.c | 307 +++++++++++++++++++++
 4 files changed, 336 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/pressure/hp03.txt
 create mode 100644 drivers/iio/pressure/hp03.c


[...]

+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;
+			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;
*val = 0, *val2 = 1000;
return IIO_VAL_INT_PLUS_MICRO;
+		case IIO_TEMP:
+			*val = 0;
+			*val2 = 10000;
Val2 should I think be 100000 giving scale * lsb of 0.001 degrees.

I think this should be:

val = 10;
return IIO_VAL_INT;

Since the temp value is in 10mC steps.

+			return IIO_VAL_INT_PLUS_MICRO;
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return -EINVAL;
+}

[...]

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux