linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 1/2] Driver for TI HDC20x0 humidity and temperature sensors
@ 2020-05-18 19:15 Eugene Zalkonnikov
  2020-05-18 19:22 ` [PATCH v8 2/2] Device tree bindings " Eugene Zalkonnikov
  2020-05-19 18:11 ` [PATCH v8 1/2] Driver " Jonathan Cameron
  0 siblings, 2 replies; 7+ messages in thread
From: Eugene Zalkonnikov @ 2020-05-18 19:15 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, development, linux-iio

HDC2010/2080 driver and sysfs documentation for its heater element.

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

diff -uprN -X linux-5.3.8/Documentation/dontdiff linux-5.3.8/Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010 linux-5.3.8_hdc2010/Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010
--- linux-5.3.8/Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010	1970-01-01 01:00:00.000000000 +0100
+++ linux-5.3.8_hdc2010/Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010	2019-12-02 11:09:25.803326999 +0100
@@ -0,0 +1,9 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_current_heater_raw
+What:		/sys/bus/iio/devices/iio:deviceX/out_current_heater_raw_available
+KernelVersion:	5.3.8
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Controls the heater device within the humidity sensor to get
+		rid of excess condensation.
+
+		Valid control values are 0 = OFF, and 1 = ON.
diff -uprN -X linux-5.3.8/Documentation/dontdiff linux-5.3.8/drivers/iio/humidity/hdc2010.c linux-5.3.8_hdc2010/drivers/iio/humidity/hdc2010.c
--- linux-5.3.8/drivers/iio/humidity/hdc2010.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-5.3.8_hdc2010/drivers/iio/humidity/hdc2010.c	2020-05-15 17:26:27.121390638 +0200
@@ -0,0 +1,348 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * hdc2010.c - Support for the TI HDC2010 and HDC2080
+ * temperature + relative humidity sensors
+ *
+ * Copyright (C) 2020 Norphonic AS
+ * Author: Eugene Zaikonnikov <eugene.zaikonnikov@norphonic.com>
+ *
+ * Datasheets:
+ * http://www.ti.com/product/HDC2010/datasheet
+ * http://www.ti.com/product/HDC2080/datasheet
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define HDC2010_REG_TEMP_LOW			0x00
+#define HDC2010_REG_TEMP_HIGH			0x01
+#define HDC2010_REG_HUMIDITY_LOW		0x02
+#define HDC2010_REG_HUMIDITY_HIGH		0x03
+#define HDC2010_REG_INTERRUPT_DRDY		0x04
+#define HDC2010_REG_TEMP_MAX			0x05
+#define HDC2010_REG_HUMIDITY_MAX		0x06
+#define HDC2010_REG_INTERRUPT_EN		0x07
+#define HDC2010_REG_TEMP_OFFSET_ADJ		0x08
+#define HDC2010_REG_HUMIDITY_OFFSET_ADJ		0x09
+#define HDC2010_REG_TEMP_THR_L			0x0a
+#define HDC2010_REG_TEMP_THR_H			0x0b
+#define HDC2010_REG_RH_THR_L			0x0c
+#define HDC2010_REG_RH_THR_H			0x0d
+#define HDC2010_REG_RESET_DRDY_INT_CONF		0x0e
+#define HDC2010_REG_MEASUREMENT_CONF		0x0f
+
+#define HDC2010_MEAS_CONF			GENMASK(2, 1)
+#define HDC2010_MEAS_TRIG			BIT(0)
+#define HDC2010_HEATER_EN			BIT(3)
+#define HDC2010_AMM				GENMASK(6, 4)
+
+struct hdc2010_data {
+	struct i2c_client *client;
+	struct mutex lock;
+	u8 measurement_config;
+	u8 interrupt_config;
+	u8 drdy_config;
+};
+
+enum hdc2010_addr_groups {
+	HDC2010_GROUP_TEMP = 0,
+	HDC2010_GROUP_HUMIDITY
+};
+
+struct hdc2010_reg_record {
+	unsigned long primary;
+	unsigned long peak;
+};
+
+static const struct hdc2010_reg_record hdc2010_reg_translation[] = {
+	[HDC2010_GROUP_TEMP] = {
+		.primary = HDC2010_REG_TEMP_LOW,
+		.peak = HDC2010_REG_TEMP_MAX,
+	},
+	[HDC2010_GROUP_HUMIDITY] = {
+		.primary = HDC2010_REG_HUMIDITY_LOW,
+		.peak = HDC2010_REG_HUMIDITY_MAX,
+	},
+};
+
+static IIO_CONST_ATTR(out_current_heater_raw_available,
+		"0 1");
+
+static struct attribute *hdc2010_attributes[] = {
+	&iio_const_attr_out_current_heater_raw_available.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group hdc2010_attribute_group = {
+	.attrs = hdc2010_attributes,
+};
+
+static const struct iio_chan_spec hdc2010_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.address = HDC2010_GROUP_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_PEAK) |
+			BIT(IIO_CHAN_INFO_OFFSET) |
+			BIT(IIO_CHAN_INFO_SCALE),
+	},
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.address = HDC2010_GROUP_HUMIDITY,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_PEAK) |
+			BIT(IIO_CHAN_INFO_SCALE),
+	},
+	{
+		.type = IIO_CURRENT,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+		.extend_name = "heater",
+		.output = 1,
+	},
+};
+
+static int hdc2010_update_drdy_config(struct hdc2010_data *data,
+					     char mask, char val)
+{
+	u8 tmp = (~mask & data->drdy_config) | val;
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(data->client,
+					HDC2010_REG_RESET_DRDY_INT_CONF, tmp);
+	if (!ret)
+		data->drdy_config = tmp;
+
+	return ret;
+}
+
+static int hdc2010_get_prim_measurement_word(struct hdc2010_data *data,
+					     struct iio_chan_spec const *chan)
+{
+	struct i2c_client *client = data->client;
+	s32 ret;
+
+	ret = i2c_smbus_read_word_data(client,
+			hdc2010_reg_translation[chan->address].primary);
+
+	if (ret < 0)
+		dev_err(&client->dev, "Could not read sensor measurement word\n");
+
+	return ret;
+}
+
+static int hdc2010_get_peak_measurement_byte(struct hdc2010_data *data,
+					     struct iio_chan_spec const *chan)
+{
+	struct i2c_client *client = data->client;
+	s32 ret;
+
+	ret = i2c_smbus_read_byte_data(client,
+			hdc2010_reg_translation[chan->address].peak);
+
+	if (ret < 0)
+		dev_err(&client->dev, "Could not read sensor measurement byte\n");
+
+	return ret;
+}
+
+static int hdc2010_get_heater_status(struct hdc2010_data *data)
+{
+	return !!(data->drdy_config & HDC2010_HEATER_EN);
+}
+
+static int hdc2010_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, int *val,
+			    int *val2, long mask)
+{
+	struct hdc2010_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW: {
+		int ret;
+
+		if (chan->type == IIO_CURRENT) {
+			*val = hdc2010_get_heater_status(data);
+			return IIO_VAL_INT;
+		}
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+		mutex_lock(&data->lock);
+		ret = hdc2010_get_prim_measurement_word(data, chan);
+		mutex_unlock(&data->lock);
+		iio_device_release_direct_mode(indio_dev);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_PEAK: {
+		int ret;
+
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+		mutex_lock(&data->lock);
+		ret = hdc2010_get_peak_measurement_byte(data, chan);
+		mutex_unlock(&data->lock);
+		iio_device_release_direct_mode(indio_dev);
+		if (ret < 0)
+			return ret;
+		/* Scaling up the value so we can use same offset as RAW */
+		*val = ret * 256;
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_SCALE:
+		*val2 = 65536;
+		if (chan->type == IIO_TEMP)
+			*val = 165000;
+		else
+			*val = 100000;
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = -15887;
+		*val2 = 515151;
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int hdc2010_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int val, int val2, long mask)
+{
+	struct hdc2010_data *data = iio_priv(indio_dev);
+	int new, ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (chan->type != IIO_CURRENT || val2 != 0)
+			return -EINVAL;
+
+		switch (val) {
+		case 1:
+			new = HDC2010_HEATER_EN;
+			break;
+		case 0:
+			new = 0;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+		mutex_lock(&data->lock);
+		ret = hdc2010_update_drdy_config(data, HDC2010_HEATER_EN, new);
+		mutex_unlock(&data->lock);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info hdc2010_info = {
+	.read_raw = hdc2010_read_raw,
+	.write_raw = hdc2010_write_raw,
+	.attrs = &hdc2010_attribute_group,
+};
+
+static int hdc2010_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct iio_dev *indio_dev;
+	struct hdc2010_data *data;
+	u8 tmp;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
+				     I2C_FUNC_SMBUS_BYTE | I2C_FUNC_I2C))
+		return -EOPNOTSUPP;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	i2c_set_clientdata(client, indio_dev);
+	data->client = client;
+	mutex_init(&data->lock);
+
+	indio_dev->dev.parent = &client->dev;
+	/*
+	 * As DEVICE ID register does not differentiate between
+	 * HDC2010 and HDC2080, we have the name hardcoded
+	 */
+	indio_dev->name = "hdc2010";
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &hdc2010_info;
+
+	indio_dev->channels = hdc2010_channels;
+	indio_dev->num_channels = ARRAY_SIZE(hdc2010_channels);
+
+	/* Enable Automatic Measurement Mode at 5Hz */
+	ret = hdc2010_update_drdy_config(data, HDC2010_AMM, HDC2010_AMM);
+	if (ret)
+		return ret;
+
+	/*
+	 * We enable both temp and humidity measurement.
+	 * However the measurement won't start even in AMM until triggered.
+	 */
+	tmp = (u8)(data->measurement_config & ~HDC2010_MEAS_CONF) |
+		HDC2010_MEAS_TRIG;
+
+	ret = i2c_smbus_write_byte_data(client,
+					HDC2010_REG_MEASUREMENT_CONF, tmp);
+	if (ret)
+		return ret;
+	data->measurement_config = tmp;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static int hdc2010_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+	struct hdc2010_data *data = iio_priv(indio_dev);
+
+	/* Disable Automatic Measurement Mode */
+	if (hdc2010_update_drdy_config(data, HDC2010_AMM, 0))
+		dev_warn(&client->dev, "Unable to restore default AMM\n");
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id hdc2010_id[] = {
+	{ "hdc2010" },
+	{ "hdc2080" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, hdc2010_id);
+
+static const struct of_device_id hdc2010_dt_ids[] = {
+	{ .compatible = "ti,hdc2010" },
+	{ .compatible = "ti,hdc2080" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, hdc2010_dt_ids);
+
+static struct i2c_driver hdc2010_driver = {
+	.driver = {
+		.name	= "hdc2010",
+		.of_match_table = hdc2010_dt_ids,
+	},
+	.probe = hdc2010_probe,
+	.remove = hdc2010_remove,
+	.id_table = hdc2010_id,
+};
+module_i2c_driver(hdc2010_driver);
+
+MODULE_AUTHOR("Eugene Zaikonnikov <eugene.zaikonnikov@norphonic.com>");
+MODULE_DESCRIPTION("TI HDC2010 humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");
diff -uprN -X linux-5.3.8/Documentation/dontdiff linux-5.3.8/drivers/iio/humidity/Kconfig linux-5.3.8_hdc2010/drivers/iio/humidity/Kconfig
--- linux-5.3.8/drivers/iio/humidity/Kconfig	2019-10-29 09:22:48.000000000 +0100
+++ linux-5.3.8_hdc2010/drivers/iio/humidity/Kconfig	2019-11-28 14:31:27.794242693 +0100
@@ -38,6 +38,16 @@ config HDC100X
 	  To compile this driver as a module, choose M here: the module
 	  will be called hdc100x.
 
+config HDC2010
+	tristate "TI HDC2010 relative humidity and temperature sensor"
+	depends on I2C
+	help
+	  Say yes here to build support for the Texas Instruments
+	  HDC2010 and HDC2080 relative humidity and temperature sensors.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called hdc2010.
+
 config HID_SENSOR_HUMIDITY
 	tristate "HID Environmental humidity sensor"
 	depends on HID_SENSOR_HUB
diff -uprN -X linux-5.3.8/Documentation/dontdiff linux-5.3.8/drivers/iio/humidity/Makefile linux-5.3.8_hdc2010/drivers/iio/humidity/Makefile
--- linux-5.3.8/drivers/iio/humidity/Makefile	2019-10-29 09:22:48.000000000 +0100
+++ linux-5.3.8_hdc2010/drivers/iio/humidity/Makefile	2019-11-28 14:30:40.609892061 +0100
@@ -6,6 +6,7 @@
 obj-$(CONFIG_AM2315) += am2315.o
 obj-$(CONFIG_DHT11) += dht11.o
 obj-$(CONFIG_HDC100X) += hdc100x.o
+obj-$(CONFIG_HDC2010) += hdc2010.o
 obj-$(CONFIG_HID_SENSOR_HUMIDITY) += hid-sensor-humidity.o
 
 hts221-y := hts221_core.o \


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v8 2/2] Device tree bindings for TI HDC20x0 humidity and temperature sensors
  2020-05-18 19:15 [PATCH v8 1/2] Driver for TI HDC20x0 humidity and temperature sensors Eugene Zalkonnikov
@ 2020-05-18 19:22 ` Eugene Zalkonnikov
  2020-05-26 18:35   ` Rob Herring
  2020-07-08  8:57   ` Eugene Zaikonnikov
  2020-05-19 18:11 ` [PATCH v8 1/2] Driver " Jonathan Cameron
  1 sibling, 2 replies; 7+ messages in thread
From: Eugene Zalkonnikov @ 2020-05-18 19:22 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Hartmut Knaack, Lars-Peter Clausen, development, linux-iio,
	devicetree, Mark Rutland

Device tree bindings for HDC2010/HDC2080 driver.


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

diff -uprN linux-5.3.8/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml linux-5.3.8_docs/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml
--- linux-5.3.8/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml	1970-01-01 01:00:00.000000000 +0100
+++ linux-5.3.8_docs/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml	2020-04-24 17:50:58.213007228 +0200
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/humidity/ti,hdc2010.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: HDC2010/HDC2080 humidity and temperature iio sensors
+
+maintainers:
+  - Eugene Zaikonnikov <eugene.zaikonnikov@norophonic.com>
+
+description: |
+  Relative humidity and tempereature sensors on I2C bus
+
+  Datasheets are available at:
+    http://www.ti.com/product/HDC2010/datasheet
+    http://www.ti.com/product/HDC2080/datasheet
+
+properties:
+  compatible:
+    enum:
+      - ti,hdc2010
+      - ti,hdc2080
+
+  vdd-supply:
+    maxItems: 1
+
+required:
+  - compatible
+
+examples:
+  - |
+    i2c0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      humidity@40 {
+          compatible = "ti,hdc2010";
+          reg = <0x40>;
+      };
+    };


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 1/2] Driver for TI HDC20x0 humidity and temperature sensors
  2020-05-18 19:15 [PATCH v8 1/2] Driver for TI HDC20x0 humidity and temperature sensors Eugene Zalkonnikov
  2020-05-18 19:22 ` [PATCH v8 2/2] Device tree bindings " Eugene Zalkonnikov
@ 2020-05-19 18:11 ` Jonathan Cameron
  2020-06-11 13:02   ` Eugene Zaikonnikov
  1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2020-05-19 18:11 UTC (permalink / raw)
  To: Eugene Zalkonnikov
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	development, linux-iio

On Mon, 18 May 2020 19:15:27 +0000
Eugene Zalkonnikov <ez@norphonic.com> wrote:

Hi Eugene

> HDC2010/2080 driver and sysfs documentation for its heater element.
> 

Would be nice to have a little more info on the device for anyone reading
the log after we commit this.

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

There is a potential double free in the remove path due to mix of
devm_iio_device_register and iio_device_unregister.

Also, the error paths in probe don't undo the various bits of setup
that it feels like they should.  This might not matter in reality
but the code will be more obviously correct if it is fine to reset
the amm bit.

DT patch looks fine to me but I'd like to leave a bit of time
for a possible DT rview. 

Otherwise all looks good to me.

Thanks,

Jonathan

> 
> diff -uprN -X linux-5.3.8/Documentation/dontdiff linux-5.3.8/Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010 linux-5.3.8_hdc2010/Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010
> --- linux-5.3.8/Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010	1970-01-01 01:00:00.000000000 +0100
> +++ linux-5.3.8_hdc2010/Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010	2019-12-02 11:09:25.803326999 +0100
> @@ -0,0 +1,9 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/out_current_heater_raw
> +What:		/sys/bus/iio/devices/iio:deviceX/out_current_heater_raw_available
> +KernelVersion:	5.3.8
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Controls the heater device within the humidity sensor to get
> +		rid of excess condensation.
> +
> +		Valid control values are 0 = OFF, and 1 = ON.
> diff -uprN -X linux-5.3.8/Documentation/dontdiff linux-5.3.8/drivers/iio/humidity/hdc2010.c linux-5.3.8_hdc2010/drivers/iio/humidity/hdc2010.c
> --- linux-5.3.8/drivers/iio/humidity/hdc2010.c	1970-01-01 01:00:00.000000000 +0100
> +++ linux-5.3.8_hdc2010/drivers/iio/humidity/hdc2010.c	2020-05-15 17:26:27.121390638 +0200
> @@ -0,0 +1,348 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * hdc2010.c - Support for the TI HDC2010 and HDC2080
> + * temperature + relative humidity sensors
> + *
> + * Copyright (C) 2020 Norphonic AS
> + * Author: Eugene Zaikonnikov <eugene.zaikonnikov@norphonic.com>
> + *
> + * Datasheets:
> + * http://www.ti.com/product/HDC2010/datasheet
> + * http://www.ti.com/product/HDC2080/datasheet
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define HDC2010_REG_TEMP_LOW			0x00
> +#define HDC2010_REG_TEMP_HIGH			0x01
> +#define HDC2010_REG_HUMIDITY_LOW		0x02
> +#define HDC2010_REG_HUMIDITY_HIGH		0x03
> +#define HDC2010_REG_INTERRUPT_DRDY		0x04
> +#define HDC2010_REG_TEMP_MAX			0x05
> +#define HDC2010_REG_HUMIDITY_MAX		0x06
> +#define HDC2010_REG_INTERRUPT_EN		0x07
> +#define HDC2010_REG_TEMP_OFFSET_ADJ		0x08
> +#define HDC2010_REG_HUMIDITY_OFFSET_ADJ		0x09
> +#define HDC2010_REG_TEMP_THR_L			0x0a
> +#define HDC2010_REG_TEMP_THR_H			0x0b
> +#define HDC2010_REG_RH_THR_L			0x0c
> +#define HDC2010_REG_RH_THR_H			0x0d
> +#define HDC2010_REG_RESET_DRDY_INT_CONF		0x0e
> +#define HDC2010_REG_MEASUREMENT_CONF		0x0f
> +
> +#define HDC2010_MEAS_CONF			GENMASK(2, 1)
> +#define HDC2010_MEAS_TRIG			BIT(0)
> +#define HDC2010_HEATER_EN			BIT(3)
> +#define HDC2010_AMM				GENMASK(6, 4)
> +
> +struct hdc2010_data {
> +	struct i2c_client *client;
> +	struct mutex lock;
> +	u8 measurement_config;
> +	u8 interrupt_config;
> +	u8 drdy_config;
> +};
> +
> +enum hdc2010_addr_groups {
> +	HDC2010_GROUP_TEMP = 0,
> +	HDC2010_GROUP_HUMIDITY
> +};
> +
> +struct hdc2010_reg_record {
> +	unsigned long primary;
> +	unsigned long peak;
> +};
> +
> +static const struct hdc2010_reg_record hdc2010_reg_translation[] = {
> +	[HDC2010_GROUP_TEMP] = {
> +		.primary = HDC2010_REG_TEMP_LOW,
> +		.peak = HDC2010_REG_TEMP_MAX,
> +	},
> +	[HDC2010_GROUP_HUMIDITY] = {
> +		.primary = HDC2010_REG_HUMIDITY_LOW,
> +		.peak = HDC2010_REG_HUMIDITY_MAX,
> +	},
> +};
> +
> +static IIO_CONST_ATTR(out_current_heater_raw_available,
> +		"0 1");

This will easily fit on one line.

> +
> +static struct attribute *hdc2010_attributes[] = {
> +	&iio_const_attr_out_current_heater_raw_available.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group hdc2010_attribute_group = {
> +	.attrs = hdc2010_attributes,
> +};
> +
> +static const struct iio_chan_spec hdc2010_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.address = HDC2010_GROUP_TEMP,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_PEAK) |
> +			BIT(IIO_CHAN_INFO_OFFSET) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +	{
> +		.type = IIO_HUMIDITYRELATIVE,
> +		.address = HDC2010_GROUP_HUMIDITY,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_PEAK) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +	{
> +		.type = IIO_CURRENT,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.extend_name = "heater",
> +		.output = 1,
> +	},
> +};
> +
> +static int hdc2010_update_drdy_config(struct hdc2010_data *data,
> +					     char mask, char val)
> +{
> +	u8 tmp = (~mask & data->drdy_config) | val;
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte_data(data->client,
> +					HDC2010_REG_RESET_DRDY_INT_CONF, tmp);
> +	if (!ret)
> +		data->drdy_config = tmp;
> +
> +	return ret;
> +}
> +
> +static int hdc2010_get_prim_measurement_word(struct hdc2010_data *data,
> +					     struct iio_chan_spec const *chan)
> +{
> +	struct i2c_client *client = data->client;
> +	s32 ret;
> +
> +	ret = i2c_smbus_read_word_data(client,
> +			hdc2010_reg_translation[chan->address].primary);
> +
> +	if (ret < 0)
> +		dev_err(&client->dev, "Could not read sensor measurement word\n");
> +
> +	return ret;
> +}
> +
> +static int hdc2010_get_peak_measurement_byte(struct hdc2010_data *data,
> +					     struct iio_chan_spec const *chan)
> +{
> +	struct i2c_client *client = data->client;
> +	s32 ret;
> +
> +	ret = i2c_smbus_read_byte_data(client,
> +			hdc2010_reg_translation[chan->address].peak);
> +
> +	if (ret < 0)
> +		dev_err(&client->dev, "Could not read sensor measurement byte\n");
> +
> +	return ret;
> +}
> +
> +static int hdc2010_get_heater_status(struct hdc2010_data *data)
> +{
> +	return !!(data->drdy_config & HDC2010_HEATER_EN);
> +}
> +
> +static int hdc2010_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan, int *val,
> +			    int *val2, long mask)
> +{
> +	struct hdc2010_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {
> +		int ret;
> +
> +		if (chan->type == IIO_CURRENT) {
> +			*val = hdc2010_get_heater_status(data);
> +			return IIO_VAL_INT;
> +		}
> +		ret = iio_device_claim_direct_mode(indio_dev);
> +		if (ret)
> +			return ret;
> +		mutex_lock(&data->lock);
> +		ret = hdc2010_get_prim_measurement_word(data, chan);
> +		mutex_unlock(&data->lock);
> +		iio_device_release_direct_mode(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +		*val = ret;
> +		return IIO_VAL_INT;
> +	}
> +	case IIO_CHAN_INFO_PEAK: {
> +		int ret;
> +
> +		ret = iio_device_claim_direct_mode(indio_dev);
> +		if (ret)
> +			return ret;
> +		mutex_lock(&data->lock);
> +		ret = hdc2010_get_peak_measurement_byte(data, chan);
> +		mutex_unlock(&data->lock);
> +		iio_device_release_direct_mode(indio_dev);
> +		if (ret < 0)
> +			return ret;
> +		/* Scaling up the value so we can use same offset as RAW */
> +		*val = ret * 256;
> +		return IIO_VAL_INT;
> +	}
> +	case IIO_CHAN_INFO_SCALE:
> +		*val2 = 65536;
> +		if (chan->type == IIO_TEMP)
> +			*val = 165000;
> +		else
> +			*val = 100000;
> +		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = -15887;
> +		*val2 = 515151;
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int hdc2010_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int val, int val2, long mask)
> +{
> +	struct hdc2010_data *data = iio_priv(indio_dev);
> +	int new, ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->type != IIO_CURRENT || val2 != 0)
> +			return -EINVAL;
> +
> +		switch (val) {
> +		case 1:
> +			new = HDC2010_HEATER_EN;
> +			break;
> +		case 0:
> +			new = 0;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +
> +		mutex_lock(&data->lock);
> +		ret = hdc2010_update_drdy_config(data, HDC2010_HEATER_EN, new);
> +		mutex_unlock(&data->lock);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info hdc2010_info = {
> +	.read_raw = hdc2010_read_raw,
> +	.write_raw = hdc2010_write_raw,
> +	.attrs = &hdc2010_attribute_group,
> +};
> +
> +static int hdc2010_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct iio_dev *indio_dev;
> +	struct hdc2010_data *data;
> +	u8 tmp;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
> +				     I2C_FUNC_SMBUS_BYTE | I2C_FUNC_I2C))
> +		return -EOPNOTSUPP;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->client = client;
> +	mutex_init(&data->lock);
> +
> +	indio_dev->dev.parent = &client->dev;
> +	/*
> +	 * As DEVICE ID register does not differentiate between
> +	 * HDC2010 and HDC2080, we have the name hardcoded
> +	 */
> +	indio_dev->name = "hdc2010";
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &hdc2010_info;
> +
> +	indio_dev->channels = hdc2010_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(hdc2010_channels);
> +
> +	/* Enable Automatic Measurement Mode at 5Hz */
> +	ret = hdc2010_update_drdy_config(data, HDC2010_AMM, HDC2010_AMM);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * We enable both temp and humidity measurement.
> +	 * However the measurement won't start even in AMM until triggered.
> +	 */
> +	tmp = (u8)(data->measurement_config & ~HDC2010_MEAS_CONF) |
> +		HDC2010_MEAS_TRIG;
> +
> +	ret = i2c_smbus_write_byte_data(client,
> +					HDC2010_REG_MEASUREMENT_CONF, tmp);
> +	if (ret)

if this fails you should be undoing the drdy config above.  
goto disable_amm;

will work if you make the change suggested below.

> +		return ret;
> +	data->measurement_config = tmp;
> +
> +	return devm_iio_device_register(&client->dev, indio_dev);
You shouldn't be calling the managed version here as you are (correctly)
explicitly unregistering it below.

	return iio_device_register(...);.

However, you should also be updating the drdy on error so it should probably be.

	ret = iio_device_register(indio_dev);
	if (ret)
		goto disable_amm;

	return 0;

disable_amm:
	hdc2010_update_drdy_config(data, HDC2010_AMM, 0);

	return ret;

> +}
> +
> +static int hdc2010_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +	struct hdc2010_data *data = iio_priv(indio_dev);
> +
> +	/* Disable Automatic Measurement Mode */
> +	if (hdc2010_update_drdy_config(data, HDC2010_AMM, 0))
> +		dev_warn(&client->dev, "Unable to restore default AMM\n");
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id hdc2010_id[] = {
> +	{ "hdc2010" },
> +	{ "hdc2080" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, hdc2010_id);
> +
> +static const struct of_device_id hdc2010_dt_ids[] = {
> +	{ .compatible = "ti,hdc2010" },
> +	{ .compatible = "ti,hdc2080" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, hdc2010_dt_ids);
> +
> +static struct i2c_driver hdc2010_driver = {
> +	.driver = {
> +		.name	= "hdc2010",
> +		.of_match_table = hdc2010_dt_ids,
> +	},
> +	.probe = hdc2010_probe,
> +	.remove = hdc2010_remove,
> +	.id_table = hdc2010_id,
> +};
> +module_i2c_driver(hdc2010_driver);
> +
> +MODULE_AUTHOR("Eugene Zaikonnikov <eugene.zaikonnikov@norphonic.com>");
> +MODULE_DESCRIPTION("TI HDC2010 humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
> diff -uprN -X linux-5.3.8/Documentation/dontdiff linux-5.3.8/drivers/iio/humidity/Kconfig linux-5.3.8_hdc2010/drivers/iio/humidity/Kconfig
> --- linux-5.3.8/drivers/iio/humidity/Kconfig	2019-10-29 09:22:48.000000000 +0100
> +++ linux-5.3.8_hdc2010/drivers/iio/humidity/Kconfig	2019-11-28 14:31:27.794242693 +0100
> @@ -38,6 +38,16 @@ config HDC100X
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called hdc100x.
>  
> +config HDC2010
> +	tristate "TI HDC2010 relative humidity and temperature sensor"
> +	depends on I2C
> +	help
> +	  Say yes here to build support for the Texas Instruments
> +	  HDC2010 and HDC2080 relative humidity and temperature sensors.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called hdc2010.
> +
>  config HID_SENSOR_HUMIDITY
>  	tristate "HID Environmental humidity sensor"
>  	depends on HID_SENSOR_HUB
> diff -uprN -X linux-5.3.8/Documentation/dontdiff linux-5.3.8/drivers/iio/humidity/Makefile linux-5.3.8_hdc2010/drivers/iio/humidity/Makefile
> --- linux-5.3.8/drivers/iio/humidity/Makefile	2019-10-29 09:22:48.000000000 +0100
> +++ linux-5.3.8_hdc2010/drivers/iio/humidity/Makefile	2019-11-28 14:30:40.609892061 +0100
> @@ -6,6 +6,7 @@
>  obj-$(CONFIG_AM2315) += am2315.o
>  obj-$(CONFIG_DHT11) += dht11.o
>  obj-$(CONFIG_HDC100X) += hdc100x.o
> +obj-$(CONFIG_HDC2010) += hdc2010.o
>  obj-$(CONFIG_HID_SENSOR_HUMIDITY) += hid-sensor-humidity.o
>  
>  hts221-y := hts221_core.o \
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 2/2] Device tree bindings for TI HDC20x0 humidity and temperature sensors
  2020-05-18 19:22 ` [PATCH v8 2/2] Device tree bindings " Eugene Zalkonnikov
@ 2020-05-26 18:35   ` Rob Herring
  2020-07-08  8:57   ` Eugene Zaikonnikov
  1 sibling, 0 replies; 7+ messages in thread
From: Rob Herring @ 2020-05-26 18:35 UTC (permalink / raw)
  To: Eugene Zalkonnikov
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	development, linux-iio, devicetree, Mark Rutland

On Mon, May 18, 2020 at 07:22:54PM +0000, Eugene Zalkonnikov wrote:
> Device tree bindings for HDC2010/HDC2080 driver.

Bindings are for h/w devices, not drivers.

Run 'git log --oneline <path>' and follow convention for $subject lines. 
(Hint: dt-bindings: iio: ...)

> 
> 
> Signed-off-by: Eugene Zaikonnikov <eugene.zaikonnikov@norphonic.com>
> 
> diff -uprN linux-5.3.8/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml linux-5.3.8_docs/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml
> --- linux-5.3.8/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml	1970-01-01 01:00:00.000000000 +0100
> +++ linux-5.3.8_docs/Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml	2020-04-24 17:50:58.213007228 +0200
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause)

GPL-2.0-only, not or-later please.

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/humidity/ti,hdc2010.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HDC2010/HDC2080 humidity and temperature iio sensors
> +
> +maintainers:
> +  - Eugene Zaikonnikov <eugene.zaikonnikov@norophonic.com>
> +
> +description: |
> +  Relative humidity and tempereature sensors on I2C bus
> +
> +  Datasheets are available at:
> +    http://www.ti.com/product/HDC2010/datasheet
> +    http://www.ti.com/product/HDC2080/datasheet
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,hdc2010
> +      - ti,hdc2080
> +
> +  vdd-supply:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    i2c0 {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      humidity@40 {
> +          compatible = "ti,hdc2010";
> +          reg = <0x40>;

Not documented.

> +      };
> +    };
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 1/2] Driver for TI HDC20x0 humidity and temperature sensors
  2020-05-19 18:11 ` [PATCH v8 1/2] Driver " Jonathan Cameron
@ 2020-06-11 13:02   ` Eugene Zaikonnikov
  2020-06-14 11:31     ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: Eugene Zaikonnikov @ 2020-06-11 13:02 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	development, linux-iio


Jonathan Cameron <Jonathan.Cameron@Huawei.com> writes:

> On Mon, 18 May 2020 19:15:27 +0000
> Eugene Zalkonnikov <ez@norphonic.com> wrote:
>
> Hi Eugene
>
>> HDC2010/2080 driver and sysfs documentation for its heater element.
>> 
>
> Would be nice to have a little more info on the device for anyone reading
> the log after we commit this.
>
Ok I'll expand the description.

> There is a potential double free in the remove path due to mix of
> devm_iio_device_register and iio_device_unregister.
>
Right, overlooked this. Will changine the unregister to the managed
version suffice?

> Also, the error paths in probe don't undo the various bits of setup
> that it feels like they should.  This might not matter in reality
> but the code will be more obviously correct if it is fine to reset
> the amm bit.
>
I'll add that.

-- 
Regards,

  Eugene Zaikonnikov

  Norphonic AS
  Tel: +47 98 23 97 73

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 1/2] Driver for TI HDC20x0 humidity and temperature sensors
  2020-06-11 13:02   ` Eugene Zaikonnikov
@ 2020-06-14 11:31     ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2020-06-14 11:31 UTC (permalink / raw)
  To: Eugene Zaikonnikov
  Cc: Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	development, linux-iio

On Thu, 11 Jun 2020 15:02:05 +0200
Eugene Zaikonnikov <ez@norphonic.com> wrote:

> Jonathan Cameron <Jonathan.Cameron@Huawei.com> writes:
> 
> > On Mon, 18 May 2020 19:15:27 +0000
> > Eugene Zalkonnikov <ez@norphonic.com> wrote:
> >
> > Hi Eugene
> >  
> >> HDC2010/2080 driver and sysfs documentation for its heater element.
> >>   
> >
> > Would be nice to have a little more info on the device for anyone reading
> > the log after we commit this.
> >  
> Ok I'll expand the description.
> 
> > There is a potential double free in the remove path due to mix of
> > devm_iio_device_register and iio_device_unregister.
> >  
> Right, overlooked this. Will changine the unregister to the managed
> version suffice?
Whilst that would work it would be a bad idea.

You need the unmanaged unregister before disabling auto measurement
(to remove the userspace interfaces before changing things so they won't
work).

So just use unmanaged iio_device_register in probe (and reorder to
put the unregister before the sensor stop).  I think that will also
mirror the ordering in probe which should always be the case unless
there is a very good reason to do otherwise.

Jonathan



> 
> > Also, the error paths in probe don't undo the various bits of setup
> > that it feels like they should.  This might not matter in reality
> > but the code will be more obviously correct if it is fine to reset
> > the amm bit.
> >  
> I'll add that.
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v8 2/2] Device tree bindings for TI HDC20x0 humidity and temperature sensors
  2020-05-18 19:22 ` [PATCH v8 2/2] Device tree bindings " Eugene Zalkonnikov
  2020-05-26 18:35   ` Rob Herring
@ 2020-07-08  8:57   ` Eugene Zaikonnikov
  1 sibling, 0 replies; 7+ messages in thread
From: Eugene Zaikonnikov @ 2020-07-08  8:57 UTC (permalink / raw)
  To: Rob Herring; +Cc: linux-iio, Jonathan Cameron, devicetree

> On Tue, 26 May 2020 at 12:35:08 -0600, Rob Herring wrote:

[...]

>
> Run 'git log --oneline <path>' and follow convention for $subject lines. 
> (Hint: dt-bindings: iio: ...)

The driver is outside source control in this tree, hope it's OK.

Fixing the rest.

-- 
Regards,

  Eugene Zaikonnikov

  Norphonic AS
  Tel: +47 98 23 97 73

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-07-08  8:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-18 19:15 [PATCH v8 1/2] Driver for TI HDC20x0 humidity and temperature sensors Eugene Zalkonnikov
2020-05-18 19:22 ` [PATCH v8 2/2] Device tree bindings " Eugene Zalkonnikov
2020-05-26 18:35   ` Rob Herring
2020-07-08  8:57   ` Eugene Zaikonnikov
2020-05-19 18:11 ` [PATCH v8 1/2] Driver " Jonathan Cameron
2020-06-11 13:02   ` Eugene Zaikonnikov
2020-06-14 11:31     ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).