Skip to contentSkip to conversation

iio: humidity: Add TI HDC20x0 support

PATCH Superseded Eugene Zaikonnikov 4 participants 12 messages Jul 10, 2020 – Aug 7, 2020

Subsystems (2)
Device tree bindings IIO

v9 of 10 Superseded Latest is v10 →

lore
Share & tools
lore

Eugene Zaikonnikov

author
#0 lore raw

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

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.

Signed-off-by Eugene Zaikonnikov <[email protected]>
Documentation/ABI/testing/sysfs-bus-iio-humidity-hdc2010 +9 -0
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
@@ -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:	[email protected]
+Description:
+		Controls the heater device within the humidity sensor to get
+		rid of excess condensation.
+
+		Valid control values are 0 = OFF, and 1 = ON.
drivers/iio/humidity/hdc2010.c +354 -0

354 lines not on this page · raw message

drivers/iio/humidity/Kconfig +10 -0
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
@@ -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
drivers/iio/humidity/Makefile +1 -0
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
@@ -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 \
author
#1 lore raw

PATCH v9 2/2 dt-bindings: iio: humidity: Add TI HDC20x0 support

Add device tree bindings for HDC2010/HDC2080 family of humidity and temperature sensors.

Changes from v8:
- document the use of reg
- change the license terms to GPL-2.0-only

Signed-off-by Eugene Zaikonnikov <[email protected]>
Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml +45 -0
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
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only 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 <[email protected]>
+
+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
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      humidity@40 {
+          compatible = "ti,hdc2010";
+          reg = <0x40>;
+      };
+    };
maintainer
#2 lore raw

Re: [PATCH v9 2/2] dt-bindings: iio: humidity: Add TI HDC20x0 support

Quoted 3 lines · On Fri, 10 Jul 2020 14:20:58 +0200 Eugene Zaikonnikov <[email protected]> wrote:

Add device tree bindings for HDC2010/HDC2080 family of humidity and temperature sensors.

As Andy mentioned for patch 1 please avoid attachments.

Quoted 3 lines

Changes from v8:
- document the use of reg
- change the license terms to GPL-2.0-only

The change log should be after the --- (which isn't here because
of the attachments issue). This is so we don't track a bunch of change logs in the eventual git log. They tend not to be of much interest once a patch has merged. If there is stuff in there of interest, it should usually be added to the patch description itself (e.g. some design decision or similar).

The license terms seem to be dual with BSD, which is good, but not what the change log says!

I've pasted in the actual patch content below, so others can easily review this version.

Thanks,
Jonathan

Quoted 53 lines
Signed-off-by Eugene Zaikonnikov <[email protected]>

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

Documentation/devicetree/bindings/iio/humidity/ti,hdc2010.yaml +45 -0
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only 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 <[email protected]>
+
+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
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c0 {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      humidity@40 {
+          compatible = "ti,hdc2010";
+          reg = <0x40>;
+      };
+    };
maintainer Reviewed-by
#3 lore raw

Re: [PATCH v9 2/2] dt-bindings: iio: humidity: Add TI HDC20x0 support

Quoted 19 lines · Jonathan Cameron
Quoted 3 lines · On Fri, 10 Jul 2020 14:20:58 +0200 Eugene Zaikonnikov <[email protected]> wrote:

Add device tree bindings for HDC2010/HDC2080 family of humidity and temperature sensors.

As Andy mentioned for patch 1 please avoid attachments.

Quoted 3 lines

Changes from v8:
- document the use of reg
- change the license terms to GPL-2.0-only

The change log should be after the --- (which isn't here because
of the attachments issue). This is so we don't track a bunch of change logs in the eventual git log. They tend not to be of much interest once a patch has merged. If there is stuff in there of interest, it should usually be added to the patch description itself (e.g. some design decision or similar).

Quoted 5 lines

The license terms seem to be dual with BSD, which is good, but not what the change log says!

I've pasted in the actual patch content below, so others can easily review this version.

Other than those issues,

Reviewed-by Rob Herring <[email protected]>
#4 lore raw
In reply to Eugene Zaikonnikov
Quoted 10 lines · Eugene Zaikonnikov

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.

Signed-off-by Eugene Zaikonnikov <[email protected]>

You have to use Git tools, such as git format-patch ... followed by git send-email ...
Attachments are very hard to review.

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#5 lore raw
In reply to Eugene Zaikonnikov
Quoted 8 lines · Eugene Zaikonnikov

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: ...

Quoted 1 line
Signed-off-by Eugene Zaikonnikov <[email protected]>

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!

Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#6 lore raw
Quoted 18 lines · On Sat, 11 Jul 2020 18:27:09 +0300 Andy Shevchenko <[email protected]>…
Quoted 8 lines · Eugene Zaikonnikov

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: ...

Quoted 7 lines
Quoted 1 line
Signed-off-by Eugene Zaikonnikov <[email protected]>

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?

For that one, we could indeed use the read_avail callback here for the out_current_heater_raw_available. I've not yet started insisting on this because of the huge number of drivers that predate introduction of that stuff to the core and as a result a lack of good examples. Eugene, if you are happy to change this one over to that and hence act as an example it would be great!

Quoted 19 lines

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;

Quoted 9 lines

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!

Thanks,

Jonathan

author
#7 lore raw

Hello,

Quoted 3 lines · Jonathan Cameron

On Sat, 11 Jul 2020 18:27:09 +0300
Andy Shevchenko <[email protected]> wrote:

[...]

Quoted 10 lines
Quoted 2 lines

4. IIO_CONST_ATTR can be one line, but hey don't we have IIO core to take care of it?

For that one, we could indeed use the read_avail callback here for the out_current_heater_raw_available. I've not yet started insisting on this because of the huge number of drivers that predate introduction of that stuff to the core and as a result a lack of good examples. Eugene, if you are happy to change this one over to that and hence act as an example it would be great!

Just looked into that.

We use this driver here with trees as early as 4.9.11, and it does not appear to have this callback in iio_info yet. Hope it's OK as it is in this iteration.

--
Regards,

Eugene Zaikonnikov

Norphonic AS
Tel: +47 98 23 97 73

maintainer
#8 lore raw
Quoted 20 lines · On Tue, 04 Aug 2020 12:23:56 +0200 Eugene Zaikonnikov <[email protected]> wrote:

Hello,

Quoted 3 lines · Jonathan Cameron

On Sat, 11 Jul 2020 18:27:09 +0300
Andy Shevchenko <[email protected]> wrote:

[...]

Quoted 10 lines
Quoted 2 lines

4. IIO_CONST_ATTR can be one line, but hey don't we have IIO core to take care of it?

For that one, we could indeed use the read_avail callback here for the out_current_heater_raw_available. I've not yet started insisting on this because of the huge number of drivers that predate introduction of that stuff to the core and as a result a lack of good examples. Eugene, if you are happy to change this one over to that and hence act as an example it would be great!

Quoted 5 lines

Just looked into that.

We use this driver here with trees as early as 4.9.11, and it does not appear to have this callback in iio_info yet. Hope it's OK as it is in this iteration.

OK. It is more than possible that someone will post patches tidying
that up though and I'm not going to reject them to make life easier for your older trees. For now, put a note in the commit log though so I don't forget it.

Thanks,

Jonathan

Quoted 8 lines

--
Regards,

Eugene Zaikonnikov

Norphonic AS
Tel: +47 98 23 97 73

author
#9 lore raw

Hi Jonathan,

Quoted 2 lines · Jonathan Cameron

On Tue, 04 Aug 2020 12:23:56 +0200
Eugene Zaikonnikov <[email protected]> wrote:

[...]

Quoted 8 lines
Quoted 3 lines

We use this driver here with trees as early as 4.9.11, and it does not appear to have this callback in iio_info yet. Hope it's OK as it is in this iteration.

OK. It is more than possible that someone will post patches tidying
that up though and I'm not going to reject them to make life easier for your older trees. For now, put a note in the commit log though so I don't forget it.

That's reasonable. We didn't plan to sit on 4.9.x for so long and we'll eventually move to newer trees on all products here too. I may as well tidy this part up myself later in the year, along with fuller support for device features.

--
Regards,

Eugene Zaikonnikov

Norphonic AS
Tel: +47 98 23 97 73

author
#10 lore raw
In reply to Andy Shevchenko

Andy,

Thanks for the comments, going to fix the style/indentation as suggested.

Sorry for the attachments, it was an accident.

Quoted 1 line · Eugene Zaikonnikov

[...]

Quoted 7 lines

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");

Must be misunderstanding something, as neither of these fits 80 column fill if made into one line. Found also couple other places I need in fact to wrap down to 80.

--
Regards,

Eugene Zaikonnikov

Norphonic AS
Tel: +47 98 23 97 73

#11 lore raw
Quoted 1 line · Eugene Zaikonnikov

Sorry for the attachments, it was an accident.

I hope the next version will be sent properly.

Quoted 8 lines
Quoted 1 line · Eugene Zaikonnikov

[...]

Quoted 4 lines

8. It's one line
ret = i2c_smbus_write_byte_data(client,

    HDC2010_REG_MEASUREMENT_CONF, tmp);
Quoted 6 lines
Quoted 3 lines

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

Must be misunderstanding something, as neither of these fits 80 column fill if made into one line. Found also couple other places I need in fact to wrap down to 80.

For how many? And since it's a new driver you may use 100. Besides that there is no limitation for the messages for a long time (see dev_warn() example above).

Signature · 2 lines
With Best Regards,
Andy Shevchenko