[PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Urs Fleisch]

Hi,

This is a driver for the new humidity and temperature sensors SHT21 and SHT25 from Sensirion.

Regards,
Urs


diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 95ccbe3..4a96429 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -688,6 +688,16 @@ config SENSORS_SHT15
 	  This driver can also be built as a module.  If so, the module
 	  will be called sht15.
 
+config SENSORS_SHT21
+	tristate "Sensiron humidity and temperature sensors. SHT21 and compat."
+	depends on I2C
+	help
+	  If you say yes here you get support for the Sensiron SHT21, SHT25
+	  humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called sht21.
+
 config SENSORS_S3C
 	tristate "S3C24XX/S3C64XX Inbuilt ADC"
 	depends on ARCH_S3C2410
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 33c2ee1..7439653 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
 obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
+obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
 obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
 obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
diff --git a/drivers/hwmon/sht21.c b/drivers/hwmon/sht21.c
new file mode 100644
index 0000000..7d3ec82
--- /dev/null
+++ b/drivers/hwmon/sht21.c
@@ -0,0 +1,392 @@
+/* Sensirion SHT21 humidity and temperature sensor driver
+ *
+ * Copyright (C) 2010 Urs Fleisch <urs.fleisch@sensirion.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * Data sheet available (5/2010) at
+ * http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+
+/* I2C commands bytes */
+enum sht21_command {
+	SHT21_TRIG_T_MEASUREMENT_HM    = 0xe3,
+	SHT21_TRIG_RH_MEASUREMENT_HM   = 0xe5,
+	SHT21_TRIG_T_MEASUREMENT_POLL  = 0xf3,
+	SHT21_TRIG_RH_MEASUREMENT_POLL = 0xf5,
+	SHT21_USER_REG_W               = 0xe6,
+	SHT21_USER_REG_R               = 0xe7,
+	SHT21_SOFT_RESET               = 0xfe
+};
+
+/* User register bit masks */
+enum sht21_userreg {
+	SHT21_RES_12_14BIT   = 0x00,
+	SHT21_RES_8_12BIT    = 0x01,
+	SHT21_RES_10_13BIT   = 0x80,
+	SHT21_RES_11_11BIT   = 0x81,
+	SHT21_RES_MASK       = 0x81,
+	SHT21_END_OF_BATTERY = 0x40,
+	SHT21_ENABLE_HEATER  = 0x04,
+	SHT21_WRITABLE_MASK  = 0x87
+};
+
+/* Types of measurements */
+enum sht21_measurement_index {
+	SHT21_TEMPERATURE,
+	SHT21_HUMIDITY,
+	SHT21_NUM_MEASUREMENTS
+};
+
+/**
+ * struct sht21 - SHT21 device specific data
+ * @hwmon_dev: device registered with hwmon
+ * @lock: mutex to ensure only one I2C transfer in progress at a time
+ * @userreg_orig: original value of user register
+ * @valid: !=0 if measurements are valid
+ * @last_update: time of last update (jiffies)
+ * @measurements: cached measurement values, temperature and humidity
+ */
+struct sht21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	u8 userreg_orig;
+	char valid;
+	unsigned long last_update;
+	int measurements[SHT21_NUM_MEASUREMENTS];
+};
+
+/* I2C command to be used for a specifc measurement */
+static u8 sht21_measurement_commands[SHT21_NUM_MEASUREMENTS] = {
+	SHT21_TRIG_T_MEASUREMENT_HM,
+	SHT21_TRIG_RH_MEASUREMENT_HM
+};
+
+/**
+ * sht21_temp_ticks_to_millicelsius() - convert raw temperature ticks to
+ * milli celsius
+ * @ticks: temperature ticks value received from sensor
+ */
+static int sht21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula T = -46.85 + 175.72 * ST / 2^16 from data sheet 6.2,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+/**
+ * sht21_rh_ticks_to_per_cent_mille() - convert raw humidity ticks to
+ * one-thousandths of a percent relative humidity
+ * @ticks: humidity ticks value received from sensor
+ */
+static int sht21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula RH = -6 + 125 * SRH / 2^16 from data sheet 6.1,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+typedef int (*convert_func)(int);
+
+/* Conversion function to be used for a specifc measurement */
+static convert_func sht21_convert_raw_value[SHT21_NUM_MEASUREMENTS] = {
+	sht21_temp_ticks_to_millicelsius,
+	sht21_rh_ticks_to_per_cent_mille
+};
+
+/**
+ * sht21_update_measurements() - get updated measurements from device
+ * @client: I2C client device
+ *
+ * Returns SHT21 client data containing measurements.
+ */
+static struct sht21 *sht21_update_measurements(struct i2c_client *client)
+{
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	mutex_lock(&sht21->lock);
+	/* Data sheet 2.4:
+	 * SHT2x should not be active for more than 10% of the time - e.g. maximum
+	 * two measurements per second at 12bit accuracy shall be made. */
+	if (time_after(jiffies, sht21->last_update + HZ / 2) || !sht21->valid) {
+		int meas_idx;
+		for (meas_idx = 0; meas_idx < SHT21_NUM_MEASUREMENTS; ++meas_idx) {
+			int result = i2c_smbus_read_word_data(client,
+			  sht21_measurement_commands[meas_idx]);
+			/* SMBus specifies low byte first, but the SHT21 returns MSB first,
+			 * so we have to swab16 the values */
+			if (result >= 0)
+				sht21->measurements[meas_idx] =
+				  sht21_convert_raw_value[meas_idx](swab16(result));
+		}
+		sht21->last_update = jiffies;
+		sht21->valid = 1;
+	}
+	mutex_unlock(&sht21->lock);
+
+	return sht21;
+}
+
+/**
+ * sht21_show_measurement() - show measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to a measurement sysfs attribute.
+ * Returns number of bytes written into buffer.
+ */
+static ssize_t sht21_show_measurement(struct device *dev,
+        struct device_attribute *attr,
+        char *buf)
+{
+	struct sensor_device_attribute *sda = to_sensor_dev_attr(attr);
+	struct sht21 *sht21 = sht21_update_measurements(to_i2c_client(dev));
+
+	return sprintf(buf, "%d\n", sht21->measurements[sda->index]);
+}
+
+/**
+ * sht21_show_userreg() - show user register value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to user_register sysfs attribute.
+ * Returns number of bytes written into buffer, -EIO on error.
+ */
+static ssize_t sht21_show_userreg(struct device *dev,
+        struct device_attribute *attr,
+        char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	int result;
+	mutex_lock(&sht21->lock);
+	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	mutex_unlock(&sht21->lock);
+
+	return result >= 0 ? sprintf(buf, "%d\n", result) : -EIO;
+}
+
+/**
+ * sht21_store_userreg() - store sysfs value in user register
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer containing value as string
+ * @count: number of characters in @buf
+ *
+ * Will be called on write access to user_register sysfs attribute.
+ * Returns @count, -EIO on error.
+ */
+static ssize_t sht21_store_userreg(struct device *dev, struct device_attribute *attr,
+        const char *buf, size_t count)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	int result;
+	char *endptr;
+	unsigned long value = simple_strtoul(buf, &endptr, 0);
+	if (endptr == buf || value > 0xff)
+		return -EINVAL;
+
+	mutex_lock(&sht21->lock);
+	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (result >= 0)
+		result = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
+		  (result & ~SHT21_WRITABLE_MASK) | (value & SHT21_WRITABLE_MASK));
+	mutex_unlock(&sht21->lock);
+
+	return result >= 0 ? count : -EIO;
+}
+
+/* sysfs attributes */
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, sht21_show_measurement,
+  NULL, SHT21_TEMPERATURE);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, sht21_show_measurement,
+  NULL, SHT21_HUMIDITY);
+static SENSOR_DEVICE_ATTR(user_register, S_IRUGO | S_IWUSR, sht21_show_userreg,
+  sht21_store_userreg, 0);
+
+static struct attribute *sht21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	&sensor_dev_attr_user_register.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group sht21_attr_group = {
+	.attrs = sht21_attributes,
+};
+
+/**
+ * sht21_probe() - probe device
+ * @client: I2C client device
+ * @id: device ID
+ *
+ * Called by the I2C core when an entry in the ID table matches a
+ * device's name.
+ * Returns 0 on success.
+ */
+static int __devinit sht21_probe(struct i2c_client *client,
+        const struct i2c_device_id *id)
+{
+	struct sht21 *sht21;
+	int status;
+	u8 userreg_changed;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
+		dev_err(&client->dev, "adapter does not support SMBus word transactions\n");
+		return -ENODEV;
+	}
+
+	sht21 = kzalloc(sizeof(*sht21), GFP_KERNEL);
+	if (!sht21) {
+		dev_dbg(&client->dev, "kzalloc failed\n");
+		return -ENOMEM;
+	}
+	i2c_set_clientdata(client, sht21);
+
+	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (status < 0) {
+		dev_err(&client->dev, "error reading user register\n");
+		goto fail_free;
+	}
+	sht21->userreg_orig = status;
+
+	userreg_changed = status ^ SHT21_RES_MASK;
+	status = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, userreg_changed);
+	if (status < 0) {
+		dev_err(&client->dev, "error writing user register\n");
+		goto fail_restore_config;
+	}
+	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (status < 0) {
+		dev_err(&client->dev, "error reading user register\n");
+		goto fail_restore_config;
+	}
+	if (status != userreg_changed) {
+		dev_err(&client->dev, "user register settings did not stick\n");
+		status = -ENODEV;
+		goto fail_restore_config;
+	}
+	status = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, sht21->userreg_orig);
+	if (status < 0) {
+		dev_err(&client->dev, "error restoring user register\n");
+		goto fail_restore_config;
+	}
+	sht21->last_update = jiffies - HZ;
+	mutex_init(&sht21->lock);
+
+	status = sysfs_create_group(&client->dev.kobj, &sht21_attr_group);
+	if (status) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		goto fail_restore_config;
+	}
+	sht21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(sht21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		status = PTR_ERR(sht21->hwmon_dev);
+		goto fail_remove_sysfs;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+fail_remove_sysfs:
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+fail_restore_config:
+	i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, sht21->userreg_orig);
+fail_free:
+	kfree(sht21);
+
+	return status;
+}
+
+/**
+ * sht21_remove() - remove device
+ * @client: I2C client device
+ */
+static int __devexit sht21_remove(struct i2c_client *client)
+{
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	int status;
+
+	hwmon_device_unregister(sht21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+
+	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (status >= 0 && status != sht21->userreg_orig) {
+		i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, sht21->userreg_orig);
+	}
+
+	kfree(sht21);
+
+	return 0;
+}
+
+/* Device ID table */
+static const struct i2c_device_id sht21_id[] = {
+	{ "sht21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sht21_id);
+
+static struct i2c_driver sht21_driver = {
+	.driver.name = "sht21",
+	.probe       = sht21_probe,
+	.remove      = __devexit_p(sht21_remove),
+	.id_table    = sht21_id,
+};
+
+/**
+ * sht21_init() - initialize driver
+ *
+ * Called when kernel is booted or module is inserted.
+ * Returns 0 on success.
+ */
+static int __init sht21_init(void)
+{
+	return i2c_add_driver(&sht21_driver);
+}
+module_init(sht21_init);
+
+/**
+ * sht21_init() - clean up driver
+ *
+ * Called when module is removed.
+ */
+static void __exit sht21_exit(void)
+{
+	i2c_del_driver(&sht21_driver);
+}
+module_exit(sht21_exit);
+
+MODULE_AUTHOR("Urs Fleisch <urs.fleisch@sensirion.com>");
+MODULE_DESCRIPTION("Sensirion SHT21 humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Jonathan Cameron]

On 12/29/10 12:45, Urs Fleisch wrote:
> Hi,
> 
> This is a driver for the new humidity and temperature sensors SHT21 and SHT25 from Sensirion.
Firstly, I've cc'd the relevant mailing list and maintainers for hwmon drivers.
Next, there are some bits in here that won't have passed through a run
of checkpatch.pl so I'm guessing you haven't followed all the steps
in Documentation/SubmittingPatches or the checklist.  All these steps
make for a cleaner path into the kernel.

Nice to see sensiron have finally adopted a standard bus type, much nicer
driver than the mess that was needed for the sht15 :)  I see from the
datasheet that you can also use that protocol to talk to this device
but people should definitely use this nice i2c driver if they can!

Excellent to see Sensiron producing drivers for their parts.

Other than formatting cleanups and requests for clarification my
only real issue is the magic numbers involved in that user register.
That must be broken out into easy to understand attributes.  For those
elements not covered by current hwmon standards, you should propose
an option on the mailing lists along with appropriate documentation
updates.  If you are unsure on what these should be, please consult
the maintainers cc'd to this email, they are both extremely helpful!

All in all, a nice simple driver that should be easy to fix up.

Thanks,

Jonathan
> 
> Regards,
> Urs
> 
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 95ccbe3..4a96429 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -688,6 +688,16 @@ config SENSORS_SHT15
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called sht15.
>  
> +config SENSORS_SHT21
> +	tristate "Sensiron humidity and temperature sensors. SHT21 and compat."
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Sensiron SHT21, SHT25
> +	  humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called sht21.
> +
>  config SENSORS_S3C
>  	tristate "S3C24XX/S3C64XX Inbuilt ADC"
>  	depends on ARCH_S3C2410
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 33c2ee1..7439653 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
>  obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
>  obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
>  obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
> +obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
>  obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
>  obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
>  obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
> diff --git a/drivers/hwmon/sht21.c b/drivers/hwmon/sht21.c
> new file mode 100644
> index 0000000..7d3ec82
> --- /dev/null
> +++ b/drivers/hwmon/sht21.c
> @@ -0,0 +1,392 @@
> +/* Sensirion SHT21 humidity and temperature sensor driver
> + *
> + * Copyright (C) 2010 Urs Fleisch <urs.fleisch@sensirion.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + * Data sheet available (5/2010) at
> + * http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/device.h>
> +
> +/* I2C commands bytes */
> +enum sht21_command {
These are certainly not a natural enum.  A set of '#define's
will be clearer.

> +	SHT21_TRIG_T_MEASUREMENT_HM    = 0xe3,
> +	SHT21_TRIG_RH_MEASUREMENT_HM   = 0xe5,
> +	SHT21_TRIG_T_MEASUREMENT_POLL  = 0xf3,
> +	SHT21_TRIG_RH_MEASUREMENT_POLL = 0xf5,
> +	SHT21_USER_REG_W               = 0xe6,
> +	SHT21_USER_REG_R               = 0xe7,
> +	SHT21_SOFT_RESET               = 0xfe
> +};
> +
> +/* User register bit masks */
> +enum sht21_userreg {
Same, here. This really isn't a sensible enum.
> +	SHT21_RES_12_14BIT   = 0x00,
> +	SHT21_RES_8_12BIT    = 0x01,
> +	SHT21_RES_10_13BIT   = 0x80,
> +	SHT21_RES_11_11BIT   = 0x81,
> +	SHT21_RES_MASK       = 0x81,
> +	SHT21_END_OF_BATTERY = 0x40,
> +	SHT21_ENABLE_HEATER  = 0x04,
> +	SHT21_WRITABLE_MASK  = 0x87
> +};
> +
> +/* Types of measurements */
> +enum sht21_measurement_index {
> +	SHT21_TEMPERATURE,
> +	SHT21_HUMIDITY,
> +	SHT21_NUM_MEASUREMENTS
> +};
> +
> +/**
> + * struct sht21 - SHT21 device specific data
> + * @hwmon_dev: device registered with hwmon
> + * @lock: mutex to ensure only one I2C transfer in progress at a time
Comment probably wants clarifying. I2C transfers can only happen one at a time
anyway. What you are actually protecting is the value of control registers
when you do a read - modify - write. It also protects various bits of this
structure.

> + * @userreg_orig: original value of user register
Why is this cached in here? Looks to be used as temporary cache inside probe
then set back on the driver being removed?  Is this appropriate? Typically
unless there is a strong reason why things like the default value of this
register are provided by plaform data for a given board.

> + * @valid: !=0 if measurements are valid
Perhaps ammend the comment to say they are only invalid before the first
measurement is taken?
> + * @last_update: time of last update (jiffies)
> + * @measurements: cached measurement values, temperature and humidity
> + */
> +struct sht21 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	u8 userreg_orig;
> +	char valid;
> +	unsigned long last_update;
> +	int measurements[SHT21_NUM_MEASUREMENTS];
> +};
> +
> +/* I2C command to be used for a specifc measurement */
const as well I think? Actually, given we only have one use of
this I'd loose this array and simply have a switch (or if / else)
inline.
> +static u8 sht21_measurement_commands[SHT21_NUM_MEASUREMENTS] = {
> +	SHT21_TRIG_T_MEASUREMENT_HM,
> +	SHT21_TRIG_RH_MEASUREMENT_HM
> +};
> +
> +/**
> + * sht21_temp_ticks_to_millicelsius() - convert raw temperature ticks to
> + * milli celsius
> + * @ticks: temperature ticks value received from sensor
> + */
> +static int sht21_temp_ticks_to_millicelsius(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/* Formula T = -46.85 + 175.72 * ST / 2^16 from data sheet 6.2,
> +	   optimized for integer fixed point (3 digits) arithmetic */
> +	return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +/**
> + * sht21_rh_ticks_to_per_cent_mille() - convert raw humidity ticks to
> + * one-thousandths of a percent relative humidity
> + * @ticks: humidity ticks value received from sensor
> + */
> +static int sht21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/* Formula RH = -6 + 125 * SRH / 2^16 from data sheet 6.1,
> +	   optimized for integer fixed point (3 digits) arithmetic */
> +	return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +typedef int (*convert_func)(int);
Don't do this. It just makes the code harder to read. Again, only
used once so I'd just use a switch or if /else inline and get
rid of this array.
> +
> +/* Conversion function to be used for a specifc measurement */
> +static convert_func sht21_convert_raw_value[SHT21_NUM_MEASUREMENTS] = {
> +	sht21_temp_ticks_to_millicelsius,
> +	sht21_rh_ticks_to_per_cent_mille
> +};
> +
> +/**
> + * sht21_update_measurements() - get updated measurements from device
> + * @client: I2C client device
> + *
> + * Returns SHT21 client data containing measurements.
> + */
> +static struct sht21 *sht21_update_measurements(struct i2c_client *client)
> +{
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&sht21->lock);
> +	/* Data sheet 2.4:
> +	 * SHT2x should not be active for more than 10% of the time - e.g. maximum
> +	 * two measurements per second at 12bit accuracy shall be made. */
> +	if (time_after(jiffies, sht21->last_update + HZ / 2) || !sht21->valid) {
> +		int meas_idx;
> +		for (meas_idx = 0; meas_idx < SHT21_NUM_MEASUREMENTS; ++meas_idx) {
> +			int result = i2c_smbus_read_word_data(client,
> +			  sht21_measurement_commands[meas_idx]);. 
> +			/* SMBus specifies low byte first, but the SHT21 returns MSB first,
> +			 * so we have to swab16 the values */
> +			if (result >= 0)
> +				sht21->measurements[meas_idx] =
> +				  sht21_convert_raw_value[meas_idx](swab16(result));
> +		}
> +		sht21->last_update = jiffies;
> +		sht21->valid = 1;
> +	}
> +	mutex_unlock(&sht21->lock);
> +
> +	return sht21;
> +}
> +
> +/**
> + * sht21_show_measurement() - show measurement value in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
> + *
> + * Will be called on read access to a measurement sysfs attribute.
> + * Returns number of bytes written into buffer.
> + */
> +static ssize_t sht21_show_measurement(struct device *dev,
> +        struct device_attribute *attr,
> +        char *buf)
> +{
> +	struct sensor_device_attribute *sda = to_sensor_dev_attr(attr);
> +	struct sht21 *sht21 = sht21_update_measurements(to_i2c_client(dev));
> +
> +	return sprintf(buf, "%d\n", sht21->measurements[sda->index]);
> +}
> +
> +/**
> + * sht21_show_userreg() - show user register value in sysfs
Sorry, but this isn't going to be acceptable.  Classic case of magic numbers.
This needs to be broken up into several different attributes.
We have:
* control over the measurement resolution (which is somewhat fiddly
on this device)
* Battery voltage threshold > 2.25V
* Enable on chip heater
* OTP stuff. I think this basically a 'one shot' mode where the device
flips back to default status after a single reading. Unless you have
a user of this, I'd be tempted to just ignore that bit.
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
> + *
> + * Will be called on read access to user_register sysfs attribute.
> + * Returns number of bytes written into buffer, -EIO on error.
> + */
> +static ssize_t sht21_show_userreg(struct device *dev,
> +        struct device_attribute *attr,
> +        char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +	int result;
> +	mutex_lock(&sht21->lock);
> +	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	mutex_unlock(&sht21->lock);
> +
> +	return result >= 0 ? sprintf(buf, "%d\n", result) : -EIO;
> +}
> +
> +/**
> + * sht21_store_userreg() - store sysfs value in user register
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer containing value as string
> + * @count: number of characters in @buf
> + *
> + * Will be called on write access to user_register sysfs attribute.
> + * Returns @count, -EIO on error.
> + */
> +static ssize_t sht21_store_userreg(struct device *dev, struct device_attribute *attr,
> +        const char *buf, size_t count)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +	int result;
> +	char *endptr;
> +	unsigned long value = simple_strtoul(buf, &endptr, 0);
> +	if (endptr == buf || value > 0xff)
> +		return -EINVAL;
> +
> +	mutex_lock(&sht21->lock);
> +	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	if (result >= 0)
> +		result = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
> +		  (result & ~SHT21_WRITABLE_MASK) | (value & SHT21_WRITABLE_MASK));
> +	mutex_unlock(&sht21->lock);
> +
> +	return result >= 0 ? count : -EIO;
> +}
> +
> +/* sysfs attributes */
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, sht21_show_measurement,
> +  NULL, SHT21_TEMPERATURE);
> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, sht21_show_measurement,
> +  NULL, SHT21_HUMIDITY);

So this one is the only one I have problem with. Other two attributes are
standard (well humidity is pretty unusual but no one ever complained about
the sht15 afaik!)
> +static SENSOR_DEVICE_ATTR(user_register, S_IRUGO | S_IWUSR, sht21_show_userreg,
> +  sht21_store_userreg, 0);
> +
> +static struct attribute *sht21_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	&sensor_dev_attr_user_register.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group sht21_attr_group = {
> +	.attrs = sht21_attributes,
> +};
> +
> +/**
> + * sht21_probe() - probe device
> + * @client: I2C client device
> + * @id: device ID
> + *
> + * Called by the I2C core when an entry in the ID table matches a
> + * device's name.
> + * Returns 0 on success.
> + */
> +static int __devinit sht21_probe(struct i2c_client *client,
> +        const struct i2c_device_id *id)
> +{
> +	struct sht21 *sht21;
> +	int status;
> +	u8 userreg_changed;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA)) {
> +		dev_err(&client->dev, "adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	sht21 = kzalloc(sizeof(*sht21), GFP_KERNEL);
> +	if (!sht21) {
> +		dev_dbg(&client->dev, "kzalloc failed\n");
> +		return -ENOMEM;
> +	}
> +	i2c_set_clientdata(client, sht21);
> +
> +	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	if (status < 0) {
> +		dev_err(&client->dev, "error reading user register\n");
> +		goto fail_free;
> +	}
> +	sht21->userreg_orig = status;
> +
> +	userreg_changed = status ^ SHT21_RES_MASK;
This needs a comment.  Why exclusive or with a mask?

> +	status = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, userreg_changed);
> +	if (status < 0) {
> +		dev_err(&client->dev, "error writing user register\n");
> +		goto fail_restore_config;
> +	}
> +	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	if (status < 0) {
> +		dev_err(&client->dev, "error reading user register\n");
> +		goto fail_restore_config;
> +	}
> +	if (status != userreg_changed) {
> +		dev_err(&client->dev, "user register settings did not stick\n");
This does seem a somewhat paranoid check. Is it really needed in
a production driver?  If this has been seen in the wild, then fair enough!
> +		status = -ENODEV;
> +		goto fail_restore_config;
> +	}
> +	status = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, sht21->userreg_orig);
> +	if (status < 0) {
> +		dev_err(&client->dev, "error restoring user register\n");
> +		goto fail_restore_config;
> +	}
comment to explain this please.  We don't have an update so why are we setting
last_update?  Obviously valid will ensure we do a new capture whatever this
value is set to.
> +	sht21->last_update = jiffies - HZ;
> +	mutex_init(&sht21->lock);
> +
> +	status = sysfs_create_group(&client->dev.kobj, &sht21_attr_group);
> +	if (status) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		goto fail_restore_config;
> +	}
> +	sht21->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(sht21->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		status = PTR_ERR(sht21->hwmon_dev);
> +		goto fail_remove_sysfs;
> +	}
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +fail_remove_sysfs:
> +	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
> +fail_restore_config:
> +	i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, sht21->userreg_orig);
> +fail_free:
> +	kfree(sht21);
> +
> +	return status;
> +}
> +
> +/**
> + * sht21_remove() - remove device
> + * @client: I2C client device
> + */
> +static int __devexit sht21_remove(struct i2c_client *client)
> +{
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +	int status;
> +
> +	hwmon_device_unregister(sht21->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
> +
> +	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	if (status >= 0 && status != sht21->userreg_orig) {
> +		i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, sht21->userreg_orig);
Superflous brackets.
> +	}
> +
> +	kfree(sht21);
> +
> +	return 0;
> +}
> +
> +/* Device ID table */
> +static const struct i2c_device_id sht21_id[] = {
> +	{ "sht21", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, sht21_id);
> +
> +static struct i2c_driver sht21_driver = {
> +	.driver.name = "sht21",
> +	.probe       = sht21_probe,
> +	.remove      = __devexit_p(sht21_remove),
> +	.id_table    = sht21_id,
> +};
> +
> +/**
> + * sht21_init() - initialize driver
> + *
> + * Called when kernel is booted or module is inserted.
> + * Returns 0 on success.
> + */
> +static int __init sht21_init(void)
> +{
> +	return i2c_add_driver(&sht21_driver);
> +}
> +module_init(sht21_init);
> +
> +/**
> + * sht21_init() - clean up driver
> + *
> + * Called when module is removed.
> + */
> +static void __exit sht21_exit(void)
> +{
> +	i2c_del_driver(&sht21_driver);
> +}
> +module_exit(sht21_exit);
> +
> +MODULE_AUTHOR("Urs Fleisch <urs.fleisch@sensirion.com>");
> +MODULE_DESCRIPTION("Sensirion SHT21 humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Urs Fleisch]

Hi Jonathan,

Thanks for reviewing the patch. I have fixed the issues you reported and the
style problems.

> Sorry, but this isn't going to be acceptable.  Classic case of magic numbers.
> This needs to be broken up into several different attributes.
> We have:
> * control over the measurement resolution (which is somewhat fiddly
> on this device)
> * Battery voltage threshold > 2.25V
> * Enable on chip heater

> So this one is the only one I have problem with. Other two attributes are
> standard (well humidity is pretty unusual but no one ever complained about
> the sht15 afaik!)

The user_register attribute is now replaced by
temp1_resolution_bits, humidity1_resolution_bits, in0_min_alarm, heater_enable
attributes. in0_min_alarm is used for the "end of battery" state, the other
attributes are non-standard. Unfortunately, the temperature and humidity
resolutions are not independent. Is this acceptable?

>> +	userreg_changed = status ^ SHT21_RES_MASK;
> This needs a comment.  Why exclusive or with a mask?
>> +	if (status != userreg_changed) {
>> +		dev_err(&client->dev, "user register settings did not stick\n");
> This does seem a somewhat paranoid check. Is it really needed in
> a production driver?  If this has been seen in the wild, then fair enough!

The resolution bits of the user register are toggled and read back to make
sure that this is really an SHT21 device.

Thanks,
Urs

Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>
---
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 95ccbe3..4a96429 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -688,6 +688,16 @@ config SENSORS_SHT15
 	  This driver can also be built as a module.  If so, the module
 	  will be called sht15.
 
+config SENSORS_SHT21
+	tristate "Sensiron humidity and temperature sensors. SHT21 and compat."
+	depends on I2C
+	help
+	  If you say yes here you get support for the Sensiron SHT21, SHT25
+	  humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called sht21.
+
 config SENSORS_S3C
 	tristate "S3C24XX/S3C64XX Inbuilt ADC"
 	depends on ARCH_S3C2410
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 33c2ee1..7439653 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
 obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
+obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
 obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
 obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
diff --git a/drivers/hwmon/sht21.c b/drivers/hwmon/sht21.c
new file mode 100644
index 0000000..7d3ec82
--- /dev/null
+++ b/drivers/hwmon/sht21.c
@@ -0,0 +1,607 @@
+/* Sensirion SHT21 humidity and temperature sensor driver
+ *
+ * Copyright (C) 2010 Urs Fleisch <urs.fleisch@sensirion.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * Data sheet available (5/2010) at
+ * http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+
+/* I2C commands bytes */
+#define SHT21_TRIG_T_MEASUREMENT_HM  0xe3
+#define SHT21_TRIG_RH_MEASUREMENT_HM 0xe5
+#define SHT21_USER_REG_W             0xe6
+#define SHT21_USER_REG_R             0xe7
+
+/* User register bit masks */
+#define SHT21_RES_12_14BIT   0x00
+#define SHT21_RES_8_12BIT    0x01
+#define SHT21_RES_10_13BIT   0x80
+#define SHT21_RES_11_11BIT   0x81
+#define SHT21_RES_MASK       0x81
+#define SHT21_END_OF_BATTERY 0x40
+#define SHT21_ENABLE_HEATER  0x04
+
+/**
+ * struct sht21 - SHT21 device specific data
+ * @hwmon_dev: device registered with hwmon
+ * @lock: mutex to protect measurement values and read-modify-write access to
+ * user register
+ * @valid: only 0 before first measurement is taken
+ * @last_update: time of last update (jiffies)
+ * @temperature: cached temperature measurement value
+ * @humidity: cached humidity measurement value
+ */
+struct sht21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	char valid;
+	unsigned long last_update;
+	int temperature;
+	int humidity;
+};
+
+/**
+ * sht21_temp_ticks_to_millicelsius() - convert raw temperature ticks to
+ * milli celsius
+ * @ticks: temperature ticks value received from sensor
+ */
+static inline int sht21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula T = -46.85 + 175.72 * ST / 2^16 from data sheet 6.2,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+/**
+ * sht21_rh_ticks_to_per_cent_mille() - convert raw humidity ticks to
+ * one-thousandths of a percent relative humidity
+ * @ticks: humidity ticks value received from sensor
+ */
+static inline int sht21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula RH = -6 + 125 * SRH / 2^16 from data sheet 6.1,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+/**
+ * sht21_update_measurements() - get updated measurements from device
+ * @client: I2C client device
+ *
+ * Returns SHT21 client data containing measurements.
+ */
+static struct sht21 *sht21_update_measurements(struct i2c_client *client)
+{
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	mutex_lock(&sht21->lock);
+	/* Data sheet 2.4:
+	 * SHT2x should not be active for more than 10% of the time - e.g.
+	 * maximum two measurements per second at 12bit accuracy shall be made.
+	 */
+	if (time_after(jiffies, sht21->last_update + HZ / 2) || !sht21->valid) {
+		int result = i2c_smbus_read_word_data(client,
+				SHT21_TRIG_T_MEASUREMENT_HM);
+		/* SMBus specifies low byte first, but the SHT21 returns MSB
+		 * first, so we have to swab16 the values */
+		if (result >= 0)
+			sht21->temperature =
+			  sht21_temp_ticks_to_millicelsius(swab16(result));
+		result = i2c_smbus_read_word_data(client,
+				SHT21_TRIG_RH_MEASUREMENT_HM);
+		if (result >= 0)
+			sht21->humidity =
+			  sht21_rh_ticks_to_per_cent_mille(swab16(result));
+		sht21->last_update = jiffies;
+		sht21->valid = 1;
+	}
+	mutex_unlock(&sht21->lock);
+
+	return sht21;
+}
+
+/**
+ * sht21_show_temperature() - show temperature measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to temp1_input sysfs attribute.
+ * Returns number of bytes written into buffer.
+ */
+static ssize_t sht21_show_temperature(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct sht21 *sht21 = sht21_update_measurements(to_i2c_client(dev));
+
+	return sprintf(buf, "%d\n", sht21->temperature);
+}
+
+/**
+ * sht21_show_humidity() - show humidity measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to humidity1_input sysfs attribute.
+ * Returns number of bytes written into buffer.
+ */
+static ssize_t sht21_show_humidity(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct sht21 *sht21 = sht21_update_measurements(to_i2c_client(dev));
+
+	return sprintf(buf, "%d\n", sht21->humidity);
+}
+
+/**
+ * sht21_read_user_register() - read user register
+ * @dev: device
+ *
+ * Returns byte value of user register, <0 on error.
+ */
+static s32 sht21_read_user_register(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	s32 result;
+	mutex_lock(&sht21->lock);
+	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	mutex_unlock(&sht21->lock);
+	return result;
+}
+
+/**
+ * sht21_modify_user_register() - modify user register
+ * @dev: device
+ * @mask: mask of bits to modify
+ * @value: bits to write into user register
+ *
+ * Returns <0 on error.
+ */
+static s32 sht21_modify_user_register(struct device *dev, u8 mask, u8 value)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	s32 result;
+
+	mutex_lock(&sht21->lock);
+	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (result >= 0)
+		result = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
+		  (result & ~mask) | (value & mask));
+	mutex_unlock(&sht21->lock);
+
+	return result;
+}
+
+/**
+ * sht21_show_temperature_resolution() - show temperature measurement resolution
+ * bits in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to temp1_resolution_bits sysfs attribute.
+ * Writes "11", "12", "13" or "14".
+ * Returns number of bytes written into buffer, -EIO on error.
+ */
+static ssize_t sht21_show_temperature_resolution(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	s32 userreg = sht21_read_user_register(dev);
+	if (userreg >= 0) {
+		int resolution;
+		switch (userreg & SHT21_RES_MASK) {
+		case SHT21_RES_8_12BIT:
+			resolution = 12;
+			break;
+		case SHT21_RES_10_13BIT:
+			resolution = 13;
+			break;
+		case SHT21_RES_11_11BIT:
+			resolution = 11;
+			break;
+		case SHT21_RES_12_14BIT:
+		default:
+			resolution = 14;
+		}
+		return sprintf(buf, "%d\n", resolution);
+	}
+	return -EIO;
+}
+
+/**
+ * sht21_store_temperature_resolution() - set temperature measurement resolution
+ * bits from sysfs value
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer containing value "11", "12", "13" or "14"
+ * @count: number of characters in @buf
+ *
+ * Will be called on write access to temp1_resolution_bits sysfs attribute.
+ * Returns @count, -EIO on error.
+ */
+static ssize_t sht21_store_temperature_resolution(struct device *dev,
+	struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	s32 result;
+	u8 res_bits;
+	unsigned long value;
+	if (strict_strtoul(buf, 10, &value))
+		return -EINVAL;
+
+	switch (value) {
+	case 11:
+		res_bits = SHT21_RES_11_11BIT;
+		break;
+	case 12:
+		res_bits = SHT21_RES_8_12BIT;
+		break;
+	case 13:
+		res_bits = SHT21_RES_10_13BIT;
+		break;
+	case 14:
+		res_bits = SHT21_RES_12_14BIT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	result = sht21_modify_user_register(dev, SHT21_RES_MASK, res_bits);
+
+	return result >= 0 ? count : -EIO;
+}
+
+/**
+ * sht21_show_humidity_resolution() - show humidity measurement resolution
+ * bits in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to humidity1_resolution_bits sysfs attribute.
+ * Writes "8", "10", "11" or "12".
+ * Returns number of bytes written into buffer, -EIO on error.
+ */
+static ssize_t sht21_show_humidity_resolution(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	s32 userreg = sht21_read_user_register(dev);
+	if (userreg >= 0) {
+		int resolution;
+		switch (userreg & SHT21_RES_MASK) {
+		case SHT21_RES_8_12BIT:
+			resolution = 8;
+			break;
+		case SHT21_RES_10_13BIT:
+			resolution = 10;
+			break;
+		case SHT21_RES_11_11BIT:
+			resolution = 11;
+			break;
+		case SHT21_RES_12_14BIT:
+		default:
+			resolution = 12;
+		}
+		return sprintf(buf, "%d\n", resolution);
+	}
+	return -EIO;
+}
+
+/**
+ * sht21_store_humidity_resolution() - set humidity measurement resolution
+ * bits from sysfs value
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer containing value "8", "10", "11" or "12"
+ * @count: number of characters in @buf
+ *
+ * Will be called on write access to humidity1_resolution_bits sysfs attribute.
+ * Returns @count, -EIO on error.
+ */
+static ssize_t sht21_store_humidity_resolution(struct device *dev,
+	struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	s32 result;
+	u8 res_bits;
+	unsigned long value;
+	if (strict_strtoul(buf, 10, &value))
+		return -EINVAL;
+
+	switch (value) {
+	case 8:
+		res_bits = SHT21_RES_8_12BIT;
+		break;
+	case 10:
+		res_bits = SHT21_RES_10_13BIT;
+		break;
+	case 11:
+		res_bits = SHT21_RES_11_11BIT;
+		break;
+	case 12:
+		res_bits = SHT21_RES_12_14BIT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	result = sht21_modify_user_register(dev, SHT21_RES_MASK, res_bits);
+
+	return result >= 0 ? count : -EIO;
+}
+
+/**
+ * sht21_show_end_of_battery() - show end of battery state in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to in0_min_alarm sysfs attribute.
+ * Writes "1" if VDD < 2.25V, else "0".
+ * Returns number of bytes written into buffer, -EIO on error.
+ */
+static ssize_t sht21_show_end_of_battery(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	s32 userreg = sht21_read_user_register(dev);
+	if (userreg >= 0)
+		return sprintf(buf, "%u\n", !!(userreg & SHT21_END_OF_BATTERY));
+	else
+		return -EIO;
+}
+
+/**
+ * sht21_show_heater_enable() - show on-chip heater state in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to heater_enable sysfs attribute.
+ * Writes "1" if heater is enabled, else "0".
+ * Returns number of bytes written into buffer, -EIO on error.
+ */
+static ssize_t sht21_show_heater_enable(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	s32 userreg = sht21_read_user_register(dev);
+	if (userreg >= 0)
+		return sprintf(buf, "%u\n", !!(userreg & SHT21_ENABLE_HEATER));
+	else
+		return -EIO;
+}
+
+/**
+ * sht21_store_heater_enable() - enable or disable heater from sysfs value
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer containing value "0" or "1"
+ * @count: number of characters in @buf
+ *
+ * Will be called on write access to heater_enable sysfs attribute.
+ * Returns @count, -EIO on error.
+ */
+static ssize_t sht21_store_heater_enable(struct device *dev,
+	struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	s32 result;
+	unsigned long value;
+	if (strict_strtoul(buf, 10, &value) || value > 1)
+		return -EINVAL;
+
+	result = sht21_modify_user_register(dev, SHT21_ENABLE_HEATER,
+					    value ? SHT21_ENABLE_HEATER : 0);
+
+	return result >= 0 ? count : -EIO;
+}
+
+
+/* sysfs attributes */
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, sht21_show_temperature,
+	NULL, 0);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, sht21_show_humidity,
+	NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_resolution_bits, S_IRUGO | S_IWUSR,
+	sht21_show_temperature_resolution, sht21_store_temperature_resolution,
+	0);
+static SENSOR_DEVICE_ATTR(humidity1_resolution_bits, S_IRUGO | S_IWUSR,
+	sht21_show_humidity_resolution, sht21_store_humidity_resolution, 0);
+static SENSOR_DEVICE_ATTR(in0_min_alarm, S_IRUGO, sht21_show_end_of_battery,
+	NULL, 0);
+static SENSOR_DEVICE_ATTR(heater_enable, S_IRUGO | S_IWUSR,
+	sht21_show_heater_enable, sht21_store_heater_enable, 0);
+
+static struct attribute *sht21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	&sensor_dev_attr_temp1_resolution_bits.dev_attr.attr,
+	&sensor_dev_attr_humidity1_resolution_bits.dev_attr.attr,
+	&sensor_dev_attr_in0_min_alarm.dev_attr.attr,
+	&sensor_dev_attr_heater_enable.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group sht21_attr_group = {
+	.attrs = sht21_attributes,
+};
+
+/**
+ * sht21_probe() - probe device
+ * @client: I2C client device
+ * @id: device ID
+ *
+ * Called by the I2C core when an entry in the ID table matches a
+ * device's name.
+ * Returns 0 on success.
+ */
+static int __devinit sht21_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct sht21 *sht21;
+	int status;
+	u8 userreg_orig, userreg_changed;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_WORD_DATA)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus word transactions\n");
+		return -ENODEV;
+	}
+
+	sht21 = kzalloc(sizeof(*sht21), GFP_KERNEL);
+	if (!sht21) {
+		dev_dbg(&client->dev, "kzalloc failed\n");
+		return -ENOMEM;
+	}
+	i2c_set_clientdata(client, sht21);
+
+	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (status < 0) {
+		dev_err(&client->dev, "error reading user register\n");
+		goto fail_free;
+	}
+	userreg_orig = status;
+
+	/* The user register resolution bits are toggled and read back to make
+	 * sure that we are really accessing an SHT21 device. */
+	userreg_changed = userreg_orig ^ SHT21_RES_MASK;
+	status = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
+					   userreg_changed);
+	if (status < 0) {
+		dev_err(&client->dev, "error writing user register\n");
+		goto fail_restore_config;
+	}
+	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (status < 0) {
+		dev_err(&client->dev, "error reading user register\n");
+		goto fail_restore_config;
+	}
+	if (status != userreg_changed) {
+		dev_err(&client->dev, "user register settings did not stick\n");
+		status = -ENODEV;
+		goto fail_restore_config;
+	}
+	status = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
+					   userreg_orig);
+	if (status < 0) {
+		dev_err(&client->dev, "error restoring user register\n");
+		goto fail_restore_config;
+	}
+	mutex_init(&sht21->lock);
+
+	status = sysfs_create_group(&client->dev.kobj, &sht21_attr_group);
+	if (status) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		goto fail_restore_config;
+	}
+	sht21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(sht21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		status = PTR_ERR(sht21->hwmon_dev);
+		goto fail_remove_sysfs;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+fail_remove_sysfs:
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+fail_restore_config:
+	i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, userreg_orig);
+fail_free:
+	kfree(sht21);
+
+	return status;
+}
+
+/**
+ * sht21_remove() - remove device
+ * @client: I2C client device
+ */
+static int __devexit sht21_remove(struct i2c_client *client)
+{
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(sht21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+	kfree(sht21);
+
+	return 0;
+}
+
+/* Device ID table */
+static const struct i2c_device_id sht21_id[] = {
+	{ "sht21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sht21_id);
+
+static struct i2c_driver sht21_driver = {
+	.driver.name = "sht21",
+	.probe       = sht21_probe,
+	.remove      = __devexit_p(sht21_remove),
+	.id_table    = sht21_id,
+};
+
+/**
+ * sht21_init() - initialize driver
+ *
+ * Called when kernel is booted or module is inserted.
+ * Returns 0 on success.
+ */
+static int __init sht21_init(void)
+{
+	return i2c_add_driver(&sht21_driver);
+}
+module_init(sht21_init);
+
+/**
+ * sht21_init() - clean up driver
+ *
+ * Called when module is removed.
+ */
+static void __exit sht21_exit(void)
+{
+	i2c_del_driver(&sht21_driver);
+}
+module_exit(sht21_exit);
+
+MODULE_AUTHOR("Urs Fleisch <urs.fleisch@sensirion.com>");
+MODULE_DESCRIPTION("Sensirion SHT21 humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Jonathan Cameron]

On 01/03/11 07:14, Urs Fleisch wrote:
> Hi Jonathan,
When posting an updated driver, use the form
[PATCH V2] hwmon: driver...

That way it will be obvious to people that it isn't just a repost of the
previous code.
> 
> Thanks for reviewing the patch. I have fixed the issues you reported and the
> style problems.
> 
>> Sorry, but this isn't going to be acceptable.  Classic case of magic numbers.
>> This needs to be broken up into several different attributes.
>> We have:
>> * control over the measurement resolution (which is somewhat fiddly
>> on this device)
>> * Battery voltage threshold > 2.25V
>> * Enable on chip heater
> 
>> So this one is the only one I have problem with. Other two attributes are
>> standard (well humidity is pretty unusual but no one ever complained about
>> the sht15 afaik!)
> 
> The user_register attribute is now replaced by
> temp1_resolution_bits, humidity1_resolution_bits, 
Those two aren't in the documentation for hwmon.
Guenter/Jean, how should this be done?  
Is there actualy a use case that means these can't both be set
by platform data?  Also, if these are acceptable, should they have
some means of indicating which combination of values are available?

> in0_min_alarm, heater_enable
> attributes. in0_min_alarm is used for the "end of battery" state, the other
> attributes are non-standard. Unfortunately, the temperature and humidity
> resolutions are not independent. Is this acceptable?
Up to maintainers.  Other than trying to work out some way of indicating valid
options for the two resolution attributes I'd be happy with these.
> 
>>> +	userreg_changed = status ^ SHT21_RES_MASK;
>> This needs a comment.  Why exclusive or with a mask?
>>> +	if (status != userreg_changed) {
>>> +		dev_err(&client->dev, "user register settings did not stick\n");
>> This does seem a somewhat paranoid check. Is it really needed in
>> a production driver?  If this has been seen in the wild, then fair enough!
> 
> The resolution bits of the user register are toggled and read back to make
> sure that this is really an SHT21 device.\
Rely on platform data to get this right rather than an adhoc test that
might well pass on some random devices.

Sorry to say I missed some error handling issues the first time around.
Please always provide userspace with the most detailed and correct error
possible. Normally this is the one comming up from the bus subsystem.

Nearly there as far as I am concerned!

Thanks,

Jonathan

> 
> Thanks,
> Urs
> 
> Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>
> ---
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 95ccbe3..4a96429 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -688,6 +688,16 @@ config SENSORS_SHT15
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called sht15.
>  
> +config SENSORS_SHT21
> +	tristate "Sensiron humidity and temperature sensors. SHT21 and compat."
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Sensiron SHT21, SHT25
> +	  humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called sht21.
> +
>  config SENSORS_S3C
>  	tristate "S3C24XX/S3C64XX Inbuilt ADC"
>  	depends on ARCH_S3C2410
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 33c2ee1..7439653 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
>  obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
>  obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
>  obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
> +obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
>  obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
>  obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
>  obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
> diff --git a/drivers/hwmon/sht21.c b/drivers/hwmon/sht21.c
> new file mode 100644
> index 0000000..7d3ec82
> --- /dev/null
> +++ b/drivers/hwmon/sht21.c
> @@ -0,0 +1,607 @@
> +/* Sensirion SHT21 humidity and temperature sensor driver
> + *
> + * Copyright (C) 2010 Urs Fleisch <urs.fleisch@sensirion.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + * Data sheet available (5/2010) at
> + * http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/device.h>
> +
> +/* I2C commands bytes */
> +#define SHT21_TRIG_T_MEASUREMENT_HM  0xe3
> +#define SHT21_TRIG_RH_MEASUREMENT_HM 0xe5
> +#define SHT21_USER_REG_W             0xe6
> +#define SHT21_USER_REG_R             0xe7
> +
> +/* User register bit masks */
> +#define SHT21_RES_12_14BIT   0x00
> +#define SHT21_RES_8_12BIT    0x01
> +#define SHT21_RES_10_13BIT   0x80
> +#define SHT21_RES_11_11BIT   0x81
> +#define SHT21_RES_MASK       0x81
> +#define SHT21_END_OF_BATTERY 0x40
> +#define SHT21_ENABLE_HEATER  0x04
> +
> +/**
> + * struct sht21 - SHT21 device specific data
> + * @hwmon_dev: device registered with hwmon
> + * @lock: mutex to protect measurement values and read-modify-write access to
> + * user register
> + * @valid: only 0 before first measurement is taken
> + * @last_update: time of last update (jiffies)
> + * @temperature: cached temperature measurement value
> + * @humidity: cached humidity measurement value
> + */
> +struct sht21 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	char valid;
> +	unsigned long last_update;
> +	int temperature;
> +	int humidity;
> +};
> +
> +/**
> + * sht21_temp_ticks_to_millicelsius() - convert raw temperature ticks to
> + * milli celsius
> + * @ticks: temperature ticks value received from sensor
> + */
> +static inline int sht21_temp_ticks_to_millicelsius(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/* Formula T = -46.85 + 175.72 * ST / 2^16 from data sheet 6.2,
> +	   optimized for integer fixed point (3 digits) arithmetic */
> +	return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +/**
> + * sht21_rh_ticks_to_per_cent_mille() - convert raw humidity ticks to
> + * one-thousandths of a percent relative humidity
> + * @ticks: humidity ticks value received from sensor
> + */
> +static inline int sht21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/* Formula RH = -6 + 125 * SRH / 2^16 from data sheet 6.1,
> +	   optimized for integer fixed point (3 digits) arithmetic */
> +	return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +/**
> + * sht21_update_measurements() - get updated measurements from device
> + * @client: I2C client device
> + *
> + * Returns SHT21 client data containing measurements.
> + */
> +static struct sht21 *sht21_update_measurements(struct i2c_client *client)
> +{
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&sht21->lock);
> +	/* Data sheet 2.4:
> +	 * SHT2x should not be active for more than 10% of the time - e.g.
> +	 * maximum two measurements per second at 12bit accuracy shall be made.
> +	 */
> +	if (time_after(jiffies, sht21->last_update + HZ / 2) || !sht21->valid) {
> +		int result = i2c_smbus_read_word_data(client,
> +				SHT21_TRIG_T_MEASUREMENT_HM);
> +		/* SMBus specifies low byte first, but the SHT21 returns MSB
> +		 * first, so we have to swab16 the values */
If it is less than 0, we have an error that ought to be correctly
handled. The correct option is to pass this all the way up to userspace.

> +		if (result >= 0)
> +			sht21->temperature =
> +			  sht21_temp_ticks_to_millicelsius(swab16(result));
> +		result = i2c_smbus_read_word_data(client,
> +				SHT21_TRIG_RH_MEASUREMENT_HM);
> +		if (result >= 0)
> +			sht21->humidity =
> +			  sht21_rh_ticks_to_per_cent_mille(swab16(result));
> +		sht21->last_update = jiffies;
> +		sht21->valid = 1;
> +	}
> +	mutex_unlock(&sht21->lock);
> +
> +	return sht21;
> +}
> +
> +/**
> + * sht21_show_temperature() - show temperature measurement value in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
> + *
> + * Will be called on read access to temp1_input sysfs attribute.
> + * Returns number of bytes written into buffer.
> + */
> +static ssize_t sht21_show_temperature(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct sht21 *sht21 = sht21_update_measurements(to_i2c_client(dev));
> +
> +	return sprintf(buf, "%d\n", sht21->temperature);
> +}
> +
> +/**
> + * sht21_show_humidity() - show humidity measurement value in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
> + *
> + * Will be called on read access to humidity1_input sysfs attribute.
> + * Returns number of bytes written into buffer.
> + */
> +static ssize_t sht21_show_humidity(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct sht21 *sht21 = sht21_update_measurements(to_i2c_client(dev));
> +
> +	return sprintf(buf, "%d\n", sht21->humidity);
> +}
> +
> +/**
> + * sht21_read_user_register() - read user register
> + * @dev: device
> + *
> + * Returns byte value of user register, <0 on error.
> + */
> +static s32 sht21_read_user_register(struct device *dev)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +	s32 result;
> +	mutex_lock(&sht21->lock);
> +	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	mutex_unlock(&sht21->lock);
> +	return result;
> +}
> +
> +/**
> + * sht21_modify_user_register() - modify user register
> + * @dev: device
> + * @mask: mask of bits to modify
> + * @value: bits to write into user register
> + *
> + * Returns <0 on error.
> + */
> +static s32 sht21_modify_user_register(struct device *dev, u8 mask, u8 value)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +	s32 result;
> +
> +	mutex_lock(&sht21->lock);
> +	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	if (result >= 0)
> +		result = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
> +		  (result & ~mask) | (value & mask));
> +	mutex_unlock(&sht21->lock);
> +
> +	return result;
> +}
> +
> +/**
> + * sht21_show_temperature_resolution() - show temperature measurement resolution
> + * bits in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
> + *
> + * Will be called on read access to temp1_resolution_bits sysfs attribute.
> + * Writes "11", "12", "13" or "14".
> + * Returns number of bytes written into buffer, -EIO on error.
> + */
> +static ssize_t sht21_show_temperature_resolution(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	s32 userreg = sht21_read_user_register(dev);
> +	if (userreg >= 0) {
> +		int resolution;
> +		switch (userreg & SHT21_RES_MASK) {
> +		case SHT21_RES_8_12BIT:
> +			resolution = 12;
> +			break;
> +		case SHT21_RES_10_13BIT:
> +			resolution = 13;
> +			break;
> +		case SHT21_RES_11_11BIT:
> +			resolution = 11;
> +			break;
> +		case SHT21_RES_12_14BIT:
> +		default:
> +			resolution = 14;
> +		}
> +		return sprintf(buf, "%d\n", resolution);
> +	}
Please return the actual error not this generic one (e.g. return userreg)
> +	return -EIO;
> +}
> +
> +/**
> + * sht21_store_temperature_resolution() - set temperature measurement resolution
> + * bits from sysfs value
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer containing value "11", "12", "13" or "14"
> + * @count: number of characters in @buf
> + *
> + * Will be called on write access to temp1_resolution_bits sysfs attribute.
> + * Returns @count, -EIO on error.
> + */
> +static ssize_t sht21_store_temperature_resolution(struct device *dev,
> +	struct device_attribute *attr,
> +	const char *buf, size_t count)
> +{
> +	s32 result;
> +	u8 res_bits;
> +	unsigned long value;
> +	if (strict_strtoul(buf, 10, &value))
> +		return -EINVAL;
> +
> +	switch (value) {
> +	case 11:
> +		res_bits = SHT21_RES_11_11BIT;
> +		break;
> +	case 12:
> +		res_bits = SHT21_RES_8_12BIT;
> +		break;
> +	case 13:
> +		res_bits = SHT21_RES_10_13BIT;
> +		break;
> +	case 14:
> +		res_bits = SHT21_RES_12_14BIT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	result = sht21_modify_user_register(dev, SHT21_RES_MASK, res_bits);
> +
> +	return result >= 0 ? count : -EIO;
Again, return the actual error rather than masking the extra information it
may have.
> +}
> +
> +/**
> + * sht21_show_humidity_resolution() - show humidity measurement resolution
> + * bits in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
> + *
> + * Will be called on read access to humidity1_resolution_bits sysfs attribute.
> + * Writes "8", "10", "11" or "12".
> + * Returns number of bytes written into buffer, -EIO on error.
> + */
> +static ssize_t sht21_show_humidity_resolution(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	s32 userreg = sht21_read_user_register(dev);
> +	if (userreg >= 0) {
> +		int resolution;
> +		switch (userreg & SHT21_RES_MASK) {
> +		case SHT21_RES_8_12BIT:
> +			resolution = 8;
> +			break;
> +		case SHT21_RES_10_13BIT:
> +			resolution = 10;
> +			break;
> +		case SHT21_RES_11_11BIT:
> +			resolution = 11;
> +			break;
> +		case SHT21_RES_12_14BIT:
> +		default:
> +			resolution = 12;
> +		}
> +		return sprintf(buf, "%d\n", resolution);
> +	}
> +	return -EIO;
ditto
> +}
> +
> +/**
> + * sht21_store_humidity_resolution() - set humidity measurement resolution
> + * bits from sysfs value
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer containing value "8", "10", "11" or "12"
> + * @count: number of characters in @buf
> + *
> + * Will be called on write access to humidity1_resolution_bits sysfs attribute.
> + * Returns @count, -EIO on error.
> + */
> +static ssize_t sht21_store_humidity_resolution(struct device *dev,
> +	struct device_attribute *attr,
> +	const char *buf, size_t count)
> +{
> +	s32 result;
> +	u8 res_bits;
> +	unsigned long value;
> +	if (strict_strtoul(buf, 10, &value))
> +		return -EINVAL;
> +
> +	switch (value) {
> +	case 8:
> +		res_bits = SHT21_RES_8_12BIT;
> +		break;
> +	case 10:
> +		res_bits = SHT21_RES_10_13BIT;
> +		break;
> +	case 11:
> +		res_bits = SHT21_RES_11_11BIT;
> +		break;
> +	case 12:
> +		res_bits = SHT21_RES_12_14BIT;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	result = sht21_modify_user_register(dev, SHT21_RES_MASK, res_bits);
> +
> +	return result >= 0 ? count : -EIO;
ditto
> +}
> +
> +/**
> + * sht21_show_end_of_battery() - show end of battery state in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
> + *
> + * Will be called on read access to in0_min_alarm sysfs attribute.
> + * Writes "1" if VDD < 2.25V, else "0".
> + * Returns number of bytes written into buffer, -EIO on error.
> + */
> +static ssize_t sht21_show_end_of_battery(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	s32 userreg = sht21_read_user_register(dev);
> +	if (userreg >= 0)
> +		return sprintf(buf, "%u\n", !!(userreg & SHT21_END_OF_BATTERY));
> +	else
> +		return -EIO;
ditto
> +}
> +
> +/**
> + * sht21_show_heater_enable() - show on-chip heater state in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
> + *
> + * Will be called on read access to heater_enable sysfs attribute.
> + * Writes "1" if heater is enabled, else "0".
> + * Returns number of bytes written into buffer, -EIO on error.
> + */
> +static ssize_t sht21_show_heater_enable(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	s32 userreg = sht21_read_user_register(dev);
> +	if (userreg >= 0)
> +		return sprintf(buf, "%u\n", !!(userreg & SHT21_ENABLE_HEATER));
> +	else
> +		return -EIO;
> +}
> +
> +/**
> + * sht21_store_heater_enable() - enable or disable heater from sysfs value
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer containing value "0" or "1"
> + * @count: number of characters in @buf
> + *
> + * Will be called on write access to heater_enable sysfs attribute.
> + * Returns @count, -EIO on error.
> + */
> +static ssize_t sht21_store_heater_enable(struct device *dev,
> +	struct device_attribute *attr,
> +	const char *buf, size_t count)
> +{
> +	s32 result;
> +	unsigned long value;
> +	if (strict_strtoul(buf, 10, &value) || value > 1)
> +		return -EINVAL;
> +
> +	result = sht21_modify_user_register(dev, SHT21_ENABLE_HEATER,
> +					    value ? SHT21_ENABLE_HEATER : 0);
> +
> +	return result >= 0 ? count : -EIO;
> +}
> +
> +
> +/* sysfs attributes */
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, sht21_show_temperature,
> +	NULL, 0);
> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, sht21_show_humidity,
> +	NULL, 0);
> +static SENSOR_DEVICE_ATTR(temp1_resolution_bits, S_IRUGO | S_IWUSR,
> +	sht21_show_temperature_resolution, sht21_store_temperature_resolution,
> +	0);
> +static SENSOR_DEVICE_ATTR(humidity1_resolution_bits, S_IRUGO | S_IWUSR,
> +	sht21_show_humidity_resolution, sht21_store_humidity_resolution, 0);
> +static SENSOR_DEVICE_ATTR(in0_min_alarm, S_IRUGO, sht21_show_end_of_battery,
> +	NULL, 0);
> +static SENSOR_DEVICE_ATTR(heater_enable, S_IRUGO | S_IWUSR,
> +	sht21_show_heater_enable, sht21_store_heater_enable, 0);
> +
> +static struct attribute *sht21_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	&sensor_dev_attr_temp1_resolution_bits.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_resolution_bits.dev_attr.attr,
> +	&sensor_dev_attr_in0_min_alarm.dev_attr.attr,
> +	&sensor_dev_attr_heater_enable.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group sht21_attr_group = {
> +	.attrs = sht21_attributes,
> +};
> +
> +/**
> + * sht21_probe() - probe device
> + * @client: I2C client device
> + * @id: device ID
> + *
> + * Called by the I2C core when an entry in the ID table matches a
> + * device's name.
> + * Returns 0 on success.
> + */
> +static int __devinit sht21_probe(struct i2c_client *client,
> +	const struct i2c_device_id *id)
> +{
> +	struct sht21 *sht21;
> +	int status;
> +	u8 userreg_orig, userreg_changed;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WORD_DATA)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	sht21 = kzalloc(sizeof(*sht21), GFP_KERNEL);
> +	if (!sht21) {
> +		dev_dbg(&client->dev, "kzalloc failed\n");
> +		return -ENOMEM;
> +	}
> +	i2c_set_clientdata(client, sht21);
> +
> +	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	if (status < 0) {
> +		dev_err(&client->dev, "error reading user register\n");
> +		goto fail_free;
> +	}
> +	userreg_orig = status;
> +
> +	/* The user register resolution bits are toggled and read back to make
> +	 * sure that we are really accessing an SHT21 device. */
> +	userreg_changed = userreg_orig ^ SHT21_RES_MASK;
> +	status = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
> +					   userreg_changed);
> +	if (status < 0) {
> +		dev_err(&client->dev, "error writing user register\n");
> +		goto fail_restore_config;
> +	}
> +	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
> +	if (status < 0) {
> +		dev_err(&client->dev, "error reading user register\n");
> +		goto fail_restore_config;
> +	}
> +	if (status != userreg_changed) {
> +		dev_err(&client->dev, "user register settings did not stick\n");
> +		status = -ENODEV;
> +		goto fail_restore_config;
> +	}
> +	status = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
> +					   userreg_orig);
> +	if (status < 0) {
> +		dev_err(&client->dev, "error restoring user register\n");
> +		goto fail_restore_config;
> +	}
> +	mutex_init(&sht21->lock);
> +
> +	status = sysfs_create_group(&client->dev.kobj, &sht21_attr_group);
> +	if (status) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		goto fail_restore_config;
> +	}
> +	sht21->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(sht21->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		status = PTR_ERR(sht21->hwmon_dev);
> +		goto fail_remove_sysfs;
> +	}
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +fail_remove_sysfs:
> +	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
> +fail_restore_config:
> +	i2c_smbus_write_byte_data(client, SHT21_USER_REG_W, userreg_orig);
> +fail_free:
> +	kfree(sht21);
> +
> +	return status;
> +}
> +
> +/**
> + * sht21_remove() - remove device
> + * @client: I2C client device
> + */
> +static int __devexit sht21_remove(struct i2c_client *client)
> +{
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(sht21->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
> +	kfree(sht21);
> +
> +	return 0;
> +}
> +
> +/* Device ID table */
> +static const struct i2c_device_id sht21_id[] = {
> +	{ "sht21", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, sht21_id);
> +
> +static struct i2c_driver sht21_driver = {
> +	.driver.name = "sht21",
> +	.probe       = sht21_probe,
> +	.remove      = __devexit_p(sht21_remove),
> +	.id_table    = sht21_id,
> +};
> +
> +/**
> + * sht21_init() - initialize driver
> + *
> + * Called when kernel is booted or module is inserted.
> + * Returns 0 on success.
> + */
> +static int __init sht21_init(void)
> +{
> +	return i2c_add_driver(&sht21_driver);
> +}
> +module_init(sht21_init);
> +
> +/**
> + * sht21_init() - clean up driver
> + *
> + * Called when module is removed.
> + */
> +static void __exit sht21_exit(void)
> +{
> +	i2c_del_driver(&sht21_driver);
> +}
> +module_exit(sht21_exit);
> +
> +MODULE_AUTHOR("Urs Fleisch <urs.fleisch@sensirion.com>");
> +MODULE_DESCRIPTION("Sensirion SHT21 humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
>

[PATCH V3] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Urs Fleisch]

Hi Jonathan,

> Is there actualy a use case that means these can't both be set
> by platform data?  Also, if these are acceptable, should they have
> some means of indicating which combination of values are available?

I can imagine a use case where an embedded device wants to operate in a
low-power mode while still monitoring humidity and/or temperature. Measurement
time and thus power consumption of the SHT21 are significantly smaller when
using a low resolution. Or you can use a low resolution while waiting for a
significant change in humidity and/or temperature and then switch to a higher
resolution to actually report the measurement values.

> Rely on platform data to get this right rather than an adhoc test that
> might well pass on some random devices.

I removed that probing stuff. As read and write addresses for the user
register are different, it would probably not pass on another device, but it
could disturb another device.

> Sorry to say I missed some error handling issues the first time around.
> Please always provide userspace with the most detailed and correct error
> possible. Normally this is the one comming up from the bus subsystem.

All error codes should now be passed up to userspace.

Thanks again,
Urs

Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>
---
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 95ccbe3..4a96429 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -688,6 +688,16 @@ config SENSORS_SHT15
 	  This driver can also be built as a module.  If so, the module
 	  will be called sht15.
 
+config SENSORS_SHT21
+	tristate "Sensiron humidity and temperature sensors. SHT21 and compat."
+	depends on I2C
+	help
+	  If you say yes here you get support for the Sensiron SHT21, SHT25
+	  humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called sht21.
+
 config SENSORS_S3C
 	tristate "S3C24XX/S3C64XX Inbuilt ADC"
 	depends on ARCH_S3C2410
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 33c2ee1..7439653 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
 obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
+obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
 obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
 obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
diff --git a/drivers/hwmon/sht21.c b/drivers/hwmon/sht21.c
new file mode 100644
index 0000000..7d3ec82
--- /dev/null
+++ b/drivers/hwmon/sht21.c
@@ -0,0 +1,592 @@
+/* Sensirion SHT21 humidity and temperature sensor driver
+ *
+ * Copyright (C) 2010 Urs Fleisch <urs.fleisch@sensirion.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * Data sheet available (5/2010) at
+ * http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+
+/* I2C commands bytes */
+#define SHT21_TRIG_T_MEASUREMENT_HM  0xe3
+#define SHT21_TRIG_RH_MEASUREMENT_HM 0xe5
+#define SHT21_USER_REG_W             0xe6
+#define SHT21_USER_REG_R             0xe7
+
+/* User register bit masks */
+#define SHT21_RES_12_14BIT   0x00
+#define SHT21_RES_8_12BIT    0x01
+#define SHT21_RES_10_13BIT   0x80
+#define SHT21_RES_11_11BIT   0x81
+#define SHT21_RES_MASK       0x81
+#define SHT21_END_OF_BATTERY 0x40
+#define SHT21_ENABLE_HEATER  0x04
+
+/**
+ * struct sht21 - SHT21 device specific data
+ * @hwmon_dev: device registered with hwmon
+ * @lock: mutex to protect measurement values and read-modify-write access to
+ * user register
+ * @valid: only 0 before first measurement is taken
+ * @last_update: time of last update (jiffies)
+ * @temperature: cached temperature measurement value
+ * @humidity: cached humidity measurement value
+ */
+struct sht21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	char valid;
+	unsigned long last_update;
+	int temperature;
+	int humidity;
+};
+
+/**
+ * sht21_temp_ticks_to_millicelsius() - convert raw temperature ticks to
+ * milli celsius
+ * @ticks: temperature ticks value received from sensor
+ */
+static inline int sht21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula T = -46.85 + 175.72 * ST / 2^16 from data sheet 6.2,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+/**
+ * sht21_rh_ticks_to_per_cent_mille() - convert raw humidity ticks to
+ * one-thousandths of a percent relative humidity
+ * @ticks: humidity ticks value received from sensor
+ */
+static inline int sht21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula RH = -6 + 125 * SRH / 2^16 from data sheet 6.1,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+/**
+ * sht21_update_measurements() - get updated measurements from device
+ * @client: I2C client device
+ *
+ * Returns 0 on success, else negative errno.
+ */
+static int sht21_update_measurements(struct i2c_client *client)
+{
+	int result = 0;
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	mutex_lock(&sht21->lock);
+	/* Data sheet 2.4:
+	 * SHT2x should not be active for more than 10% of the time - e.g.
+	 * maximum two measurements per second at 12bit accuracy shall be made.
+	 */
+	if (time_after(jiffies, sht21->last_update + HZ / 2) || !sht21->valid) {
+		result = i2c_smbus_read_word_data(client,
+				SHT21_TRIG_T_MEASUREMENT_HM);
+		/* SMBus specifies low byte first, but the SHT21 returns MSB
+		 * first, so we have to swab16 the values */
+		if (result >= 0) {
+			sht21->temperature = sht21_temp_ticks_to_millicelsius(
+				swab16(result));
+			result = i2c_smbus_read_word_data(client,
+					SHT21_TRIG_RH_MEASUREMENT_HM);
+			if (result >= 0) {
+				sht21->humidity =
+					sht21_rh_ticks_to_per_cent_mille(
+						swab16(result));
+				sht21->last_update = jiffies;
+				sht21->valid = 1;
+			}
+		}
+	}
+	mutex_unlock(&sht21->lock);
+
+	return result >= 0 ? 0 : result;
+}
+
+/**
+ * sht21_show_temperature() - show temperature measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to temp1_input sysfs attribute.
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_temperature(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	int result = sht21_update_measurements(client);
+	if (result >= 0) {
+		struct sht21 *sht21 = i2c_get_clientdata(client);
+
+		return sprintf(buf, "%d\n", sht21->temperature);
+	}
+	return result;
+}
+
+/**
+ * sht21_show_humidity() - show humidity measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to humidity1_input sysfs attribute.
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_humidity(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	int result = sht21_update_measurements(client);
+	if (result >= 0) {
+		struct sht21 *sht21 = i2c_get_clientdata(client);
+
+		return sprintf(buf, "%d\n", sht21->humidity);
+	}
+	return result;
+}
+
+/**
+ * sht21_read_user_register() - read user register
+ * @dev: device
+ *
+ * Returns byte value of user register, negative errno on error.
+ */
+static s32 sht21_read_user_register(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	s32 result;
+	mutex_lock(&sht21->lock);
+	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	mutex_unlock(&sht21->lock);
+	return result;
+}
+
+/**
+ * sht21_modify_user_register() - modify user register
+ * @dev: device
+ * @mask: mask of bits to modify
+ * @value: bits to write into user register
+ *
+ * Returns 0 on success, negative errno on error.
+ */
+static s32 sht21_modify_user_register(struct device *dev, u8 mask, u8 value)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	s32 result;
+
+	mutex_lock(&sht21->lock);
+	result = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (result >= 0)
+		result = i2c_smbus_write_byte_data(client, SHT21_USER_REG_W,
+		  (result & ~mask) | (value & mask));
+	mutex_unlock(&sht21->lock);
+
+	return result;
+}
+
+/**
+ * sht21_show_temperature_resolution() - show temperature measurement resolution
+ * bits in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to temp1_resolution_bits sysfs attribute.
+ * Writes "11", "12", "13" or "14".
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_temperature_resolution(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	s32 userreg = sht21_read_user_register(dev);
+	if (userreg >= 0) {
+		int resolution;
+		switch (userreg & SHT21_RES_MASK) {
+		case SHT21_RES_8_12BIT:
+			resolution = 12;
+			break;
+		case SHT21_RES_10_13BIT:
+			resolution = 13;
+			break;
+		case SHT21_RES_11_11BIT:
+			resolution = 11;
+			break;
+		case SHT21_RES_12_14BIT:
+		default:
+			resolution = 14;
+		}
+		return sprintf(buf, "%d\n", resolution);
+	}
+	return userreg;
+}
+
+/**
+ * sht21_store_temperature_resolution() - set temperature measurement resolution
+ * bits from sysfs value
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer containing value "11", "12", "13" or "14"
+ * @count: number of characters in @buf
+ *
+ * Will be called on write access to temp1_resolution_bits sysfs attribute.
+ * Returns @count, negative errno on error.
+ */
+static ssize_t sht21_store_temperature_resolution(struct device *dev,
+	struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	s32 result;
+	u8 res_bits;
+	unsigned long value;
+	if (strict_strtoul(buf, 10, &value))
+		return -EINVAL;
+
+	switch (value) {
+	case 11:
+		res_bits = SHT21_RES_11_11BIT;
+		break;
+	case 12:
+		res_bits = SHT21_RES_8_12BIT;
+		break;
+	case 13:
+		res_bits = SHT21_RES_10_13BIT;
+		break;
+	case 14:
+		res_bits = SHT21_RES_12_14BIT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	result = sht21_modify_user_register(dev, SHT21_RES_MASK, res_bits);
+
+	return result >= 0 ? count : result;
+}
+
+/**
+ * sht21_show_humidity_resolution() - show humidity measurement resolution
+ * bits in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to humidity1_resolution_bits sysfs attribute.
+ * Writes "8", "10", "11" or "12".
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_humidity_resolution(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	s32 userreg = sht21_read_user_register(dev);
+	if (userreg >= 0) {
+		int resolution;
+		switch (userreg & SHT21_RES_MASK) {
+		case SHT21_RES_8_12BIT:
+			resolution = 8;
+			break;
+		case SHT21_RES_10_13BIT:
+			resolution = 10;
+			break;
+		case SHT21_RES_11_11BIT:
+			resolution = 11;
+			break;
+		case SHT21_RES_12_14BIT:
+		default:
+			resolution = 12;
+		}
+		return sprintf(buf, "%d\n", resolution);
+	}
+	return userreg;
+}
+
+/**
+ * sht21_store_humidity_resolution() - set humidity measurement resolution
+ * bits from sysfs value
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer containing value "8", "10", "11" or "12"
+ * @count: number of characters in @buf
+ *
+ * Will be called on write access to humidity1_resolution_bits sysfs attribute.
+ * Returns @count, negative errno on error.
+ */
+static ssize_t sht21_store_humidity_resolution(struct device *dev,
+	struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	s32 result;
+	u8 res_bits;
+	unsigned long value;
+	if (strict_strtoul(buf, 10, &value))
+		return -EINVAL;
+
+	switch (value) {
+	case 8:
+		res_bits = SHT21_RES_8_12BIT;
+		break;
+	case 10:
+		res_bits = SHT21_RES_10_13BIT;
+		break;
+	case 11:
+		res_bits = SHT21_RES_11_11BIT;
+		break;
+	case 12:
+		res_bits = SHT21_RES_12_14BIT;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	result = sht21_modify_user_register(dev, SHT21_RES_MASK, res_bits);
+
+	return result >= 0 ? count : result;
+}
+
+/**
+ * sht21_show_end_of_battery() - show end of battery state in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to in0_min_alarm sysfs attribute.
+ * Writes "1" if VDD < 2.25V, else "0".
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_end_of_battery(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	s32 userreg = sht21_read_user_register(dev);
+	if (userreg >= 0)
+		return sprintf(buf, "%u\n", !!(userreg & SHT21_END_OF_BATTERY));
+	else
+		return userreg;
+}
+
+/**
+ * sht21_show_heater_enable() - show on-chip heater state in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where user register value is written to
+ *
+ * Will be called on read access to heater_enable sysfs attribute.
+ * Writes "1" if heater is enabled, else "0".
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_heater_enable(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	s32 userreg = sht21_read_user_register(dev);
+	if (userreg >= 0)
+		return sprintf(buf, "%u\n", !!(userreg & SHT21_ENABLE_HEATER));
+	else
+		return userreg;
+}
+
+/**
+ * sht21_store_heater_enable() - enable or disable heater from sysfs value
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer containing value "0" or "1"
+ * @count: number of characters in @buf
+ *
+ * Will be called on write access to heater_enable sysfs attribute.
+ * Returns @count, negative errno on error.
+ */
+static ssize_t sht21_store_heater_enable(struct device *dev,
+	struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	s32 result;
+	unsigned long value;
+	if (strict_strtoul(buf, 10, &value) || value > 1)
+		return -EINVAL;
+
+	result = sht21_modify_user_register(dev, SHT21_ENABLE_HEATER,
+					    value ? SHT21_ENABLE_HEATER : 0);
+
+	return result >= 0 ? count : result;
+}
+
+
+/* sysfs attributes */
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, sht21_show_temperature,
+	NULL, 0);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, sht21_show_humidity,
+	NULL, 0);
+static SENSOR_DEVICE_ATTR(temp1_resolution_bits, S_IRUGO | S_IWUSR,
+	sht21_show_temperature_resolution, sht21_store_temperature_resolution,
+	0);
+static SENSOR_DEVICE_ATTR(humidity1_resolution_bits, S_IRUGO | S_IWUSR,
+	sht21_show_humidity_resolution, sht21_store_humidity_resolution, 0);
+static SENSOR_DEVICE_ATTR(in0_min_alarm, S_IRUGO, sht21_show_end_of_battery,
+	NULL, 0);
+static SENSOR_DEVICE_ATTR(heater_enable, S_IRUGO | S_IWUSR,
+	sht21_show_heater_enable, sht21_store_heater_enable, 0);
+
+static struct attribute *sht21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	&sensor_dev_attr_temp1_resolution_bits.dev_attr.attr,
+	&sensor_dev_attr_humidity1_resolution_bits.dev_attr.attr,
+	&sensor_dev_attr_in0_min_alarm.dev_attr.attr,
+	&sensor_dev_attr_heater_enable.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group sht21_attr_group = {
+	.attrs = sht21_attributes,
+};
+
+/**
+ * sht21_probe() - probe device
+ * @client: I2C client device
+ * @id: device ID
+ *
+ * Called by the I2C core when an entry in the ID table matches a
+ * device's name.
+ * Returns 0 on success.
+ */
+static int __devinit sht21_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct sht21 *sht21;
+	int status;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_WORD_DATA)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus word transactions\n");
+		return -ENODEV;
+	}
+
+	sht21 = kzalloc(sizeof(*sht21), GFP_KERNEL);
+	if (!sht21) {
+		dev_dbg(&client->dev, "kzalloc failed\n");
+		return -ENOMEM;
+	}
+	i2c_set_clientdata(client, sht21);
+
+	status = i2c_smbus_read_byte_data(client, SHT21_USER_REG_R);
+	if (status < 0) {
+		dev_err(&client->dev, "error reading user register\n");
+		goto fail_free;
+	}
+
+	mutex_init(&sht21->lock);
+
+	status = sysfs_create_group(&client->dev.kobj, &sht21_attr_group);
+	if (status) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		goto fail_free;
+	}
+	sht21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(sht21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		status = PTR_ERR(sht21->hwmon_dev);
+		goto fail_remove_sysfs;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+fail_remove_sysfs:
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+fail_free:
+	kfree(sht21);
+
+	return status;
+}
+
+/**
+ * sht21_remove() - remove device
+ * @client: I2C client device
+ */
+static int __devexit sht21_remove(struct i2c_client *client)
+{
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(sht21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+	kfree(sht21);
+
+	return 0;
+}
+
+/* Device ID table */
+static const struct i2c_device_id sht21_id[] = {
+	{ "sht21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sht21_id);
+
+static struct i2c_driver sht21_driver = {
+	.driver.name = "sht21",
+	.probe       = sht21_probe,
+	.remove      = __devexit_p(sht21_remove),
+	.id_table    = sht21_id,
+};
+
+/**
+ * sht21_init() - initialize driver
+ *
+ * Called when kernel is booted or module is inserted.
+ * Returns 0 on success.
+ */
+static int __init sht21_init(void)
+{
+	return i2c_add_driver(&sht21_driver);
+}
+module_init(sht21_init);
+
+/**
+ * sht21_init() - clean up driver
+ *
+ * Called when module is removed.
+ */
+static void __exit sht21_exit(void)
+{
+	i2c_del_driver(&sht21_driver);
+}
+module_exit(sht21_exit);
+
+MODULE_AUTHOR("Urs Fleisch <urs.fleisch@sensirion.com>");
+MODULE_DESCRIPTION("Sensirion SHT21 humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Guenter Roeck]

On Mon, Jan 03, 2011 at 06:06:51AM -0500, Jonathan Cameron wrote:
> On 01/03/11 07:14, Urs Fleisch wrote:
> > Hi Jonathan,
> When posting an updated driver, use the form
> [PATCH V2] hwmon: driver...
> 
> That way it will be obvious to people that it isn't just a repost of the
> previous code.
> >
> > Thanks for reviewing the patch. I have fixed the issues you reported and the
> > style problems.
> >
> >> Sorry, but this isn't going to be acceptable.  Classic case of magic numbers.
> >> This needs to be broken up into several different attributes.
> >> We have:
> >> * control over the measurement resolution (which is somewhat fiddly
> >> on this device)
> >> * Battery voltage threshold > 2.25V
> >> * Enable on chip heater
> >
> >> So this one is the only one I have problem with. Other two attributes are
> >> standard (well humidity is pretty unusual but no one ever complained about
> >> the sht15 afaik!)
> >
Might make sense to document it in the ABI, though.

> > The user_register attribute is now replaced by
> > temp1_resolution_bits, humidity1_resolution_bits,
> Those two aren't in the documentation for hwmon.
> Guenter/Jean, how should this be done?
> Is there actualy a use case that means these can't both be set
> by platform data?  Also, if these are acceptable, should they have
> some means of indicating which combination of values are available?
> 
> > in0_min_alarm, heater_enable
> > attributes. in0_min_alarm is used for the "end of battery" state, the other
> > attributes are non-standard. Unfortunately, the temperature and humidity
> > resolutions are not independent. Is this acceptable?
> Up to maintainers.  Other than trying to work out some way of indicating valid
> options for the two resolution attributes I'd be happy with these.

I am opposed to non-ABI attributes. That doesn't mean that I am opposed to
extending the ABI; if the ABI needs to be extended, it should be extended
instead of providing non-standard attributes. But there should be a use case
for extending the ABI - meaning new ABI attributes should provide value not just
for one chip, but for others as well.

sysfs attributes should only be used for values expected to change during
runtime. "resolution" and "heater_enable" sounds like it might fit better into
platform data. System wide information can also be set using driver parameters.

> >
> >>> +   userreg_changed = status ^ SHT21_RES_MASK;
> >> This needs a comment.  Why exclusive or with a mask?
> >>> +   if (status != userreg_changed) {
> >>> +           dev_err(&client->dev, "user register settings did not stick\n");
> >> This does seem a somewhat paranoid check. Is it really needed in
> >> a production driver?  If this has been seen in the wild, then fair enough!
> >
> > The resolution bits of the user register are toggled and read back to make
> > sure that this is really an SHT21 device.\
> Rely on platform data to get this right rather than an adhoc test that
> might well pass on some random devices.
> 
Yes.

Guenter

Re: [PATCH V3] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Guenter Roeck]

On Mon, Jan 03, 2011 at 09:01:34AM -0500, Urs Fleisch wrote:
> Hi Jonathan,
> 
> > Is there actualy a use case that means these can't both be set
> > by platform data?  Also, if these are acceptable, should they have
> > some means of indicating which combination of values are available?
> 
> I can imagine a use case where an embedded device wants to operate in a
> low-power mode while still monitoring humidity and/or temperature. Measurement
> time and thus power consumption of the SHT21 are significantly smaller when
> using a low resolution. Or you can use a low resolution while waiting for a
> significant change in humidity and/or temperature and then switch to a higher
> resolution to actually report the measurement values.
> 
> > Rely on platform data to get this right rather than an adhoc test that
> > might well pass on some random devices.
> 
> I removed that probing stuff. As read and write addresses for the user
> register are different, it would probably not pass on another device, but it
> could disturb another device.
> 
> > Sorry to say I missed some error handling issues the first time around.
> > Please always provide userspace with the most detailed and correct error
> > possible. Normally this is the one comming up from the bus subsystem.
> 
> All error codes should now be passed up to userspace.
> 
> Thanks again,
> Urs
> 
> Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>

All the above shows up in the commit log if the patch is applied. Just adds
additional work for the maintainers.

> ---
[ ...] 
> +
> +/**
> + * sht21_show_temperature() - show temperature measurement value in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
> + *
> + * Will be called on read access to temp1_input sysfs attribute.
> + * Returns number of bytes written into buffer, negative errno on error.
> + */
> +static ssize_t sht21_show_temperature(struct device *dev,
> +       struct device_attribute *attr,
> +       char *buf)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +       int result = sht21_update_measurements(client);
> +       if (result >= 0) {
> +               struct sht21 *sht21 = i2c_get_clientdata(client);
> +
> +               return sprintf(buf, "%d\n", sht21->temperature);
> +       }
> +       return result;

Highly unusual way of detecting and returning errors. Common would be

	if (result < 0)
		return result;
	/* other code */

ie keep the code flow intact. Makes the code much easier to read.

Guenter

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Guenter Roeck]

On Mon, 2011-01-03 at 06:06 -0500, Jonathan Cameron wrote:
> On 01/03/11 07:14, Urs Fleisch wrote:
> > Hi Jonathan,
> When posting an updated driver, use the form
> [PATCH V2] hwmon: driver...
> 
> That way it will be obvious to people that it isn't just a repost of the
> previous code.
> >
> > Thanks for reviewing the patch. I have fixed the issues you reported and the
> > style problems.
> >
> >> Sorry, but this isn't going to be acceptable.  Classic case of magic numbers.
> >> This needs to be broken up into several different attributes.
> >> We have:
> >> * control over the measurement resolution (which is somewhat fiddly
> >> on this device)
> >> * Battery voltage threshold > 2.25V
> >> * Enable on chip heater
> >
> >> So this one is the only one I have problem with. Other two attributes are
> >> standard (well humidity is pretty unusual but no one ever complained about
> >> the sht15 afaik!)
> >
I have been thinking about this, and would like to get Jean's input.

Humidity doesn't really sound like "hardware monitoring"; more like
environmental monitoring. But on the other side, even though humidity
doesn't really monitor the HW, it does monitor operational conditions,
and its reported values may have impact on system operation (eg cause a
shutdown if humidity is too high, to prevent HW damage).

So question is if hwmon should include explicit environmental monitoring
attributes or not, and if hwmon and environmental monitoring can and
should be separated to start with (for example, what if any is the
difference between environment temperature and hw monitoring
temperature ?).

Thoughts, anyone ?

Guenter

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Jonathan Cameron]

On 01/03/11 18:09, Guenter Roeck wrote:
> On Mon, 2011-01-03 at 06:06 -0500, Jonathan Cameron wrote:
>> On 01/03/11 07:14, Urs Fleisch wrote:
>>> Hi Jonathan,
>> When posting an updated driver, use the form
>> [PATCH V2] hwmon: driver...
>>
>> That way it will be obvious to people that it isn't just a repost of the
>> previous code.
>>>
>>> Thanks for reviewing the patch. I have fixed the issues you reported and the
>>> style problems.
>>>
>>>> Sorry, but this isn't going to be acceptable.  Classic case of magic numbers.
>>>> This needs to be broken up into several different attributes.
>>>> We have:
>>>> * control over the measurement resolution (which is somewhat fiddly
>>>> on this device)
>>>> * Battery voltage threshold > 2.25V
>>>> * Enable on chip heater
>>>
>>>> So this one is the only one I have problem with. Other two attributes are
>>>> standard (well humidity is pretty unusual but no one ever complained about
>>>> the sht15 afaik!)
>>>
> I have been thinking about this, and would like to get Jean's input.
> 
> Humidity doesn't really sound like "hardware monitoring"; more like
> environmental monitoring. But on the other side, even though humidity
> doesn't really monitor the HW, it does monitor operational conditions,
> and its reported values may have impact on system operation (eg cause a
> shutdown if humidity is too high, to prevent HW damage).
That was my original argument when I proposed the sht15 driver. No idea
if anyone actually does this though!
> 
> So question is if hwmon should include explicit environmental monitoring
> attributes or not, and if hwmon and environmental monitoring can and
> should be separated to start with (for example, what if any is the
> difference between environment temperature and hw monitoring
> temperature ?).
> 
> Thoughts, anyone ?
I wondered exactly this when we added the sht15 driver.  Never really
came to a firm conclusion though.  If you and Jean decide these shouldn't
be in hwmon, I'm happy to take them both in IIO.  Given our scope is much
broader and we already have things like light sensors (also sometimes environment
monitoring devices), that would work for me.  There are a few sht15 users
out there I know of beyond the imote2 sensor boards (which is how I encountered
them).  The imote2 users should be fine as I maintain the platform and most of
the tools anyway, the others maybe have more issues with a move.
Uses I know of are indeed more general environment monitoring.
Not seen one of these in conventional hardware monitoring but could be wrong.
I guess, Urs will have a better idea of where these tend to be used?

Obviously there is an argument for an 'environment' monitoring subsystem, but
my guess in Linus won't pull based on exactly the same issue he had with ALS
when we proposed that.  Linus won't take small subsystems for sensors given
he wants them all in the same one.  Actually he mostly wanted them to be in input
but Dmitry quite rightly pushed back hard against that for exactly the reason
you are wondering whether they ought to be in hwmon, avoidance of scope spread.

> 
> Guenter
> 
> 
> 

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Guenter Roeck]

On Tue, Jan 04, 2011 at 07:13:04AM -0500, Jonathan Cameron wrote:
> On 01/03/11 18:09, Guenter Roeck wrote:
> > On Mon, 2011-01-03 at 06:06 -0500, Jonathan Cameron wrote:
> >> On 01/03/11 07:14, Urs Fleisch wrote:
> >>> Hi Jonathan,
> >> When posting an updated driver, use the form
> >> [PATCH V2] hwmon: driver...
> >>
> >> That way it will be obvious to people that it isn't just a repost of the
> >> previous code.
> >>>
> >>> Thanks for reviewing the patch. I have fixed the issues you reported and the
> >>> style problems.
> >>>
> >>>> Sorry, but this isn't going to be acceptable.  Classic case of magic numbers.
> >>>> This needs to be broken up into several different attributes.
> >>>> We have:
> >>>> * control over the measurement resolution (which is somewhat fiddly
> >>>> on this device)
> >>>> * Battery voltage threshold > 2.25V
> >>>> * Enable on chip heater
> >>>
> >>>> So this one is the only one I have problem with. Other two attributes are
> >>>> standard (well humidity is pretty unusual but no one ever complained about
> >>>> the sht15 afaik!)
> >>>
> > I have been thinking about this, and would like to get Jean's input.
> > 
> > Humidity doesn't really sound like "hardware monitoring"; more like
> > environmental monitoring. But on the other side, even though humidity
> > doesn't really monitor the HW, it does monitor operational conditions,
> > and its reported values may have impact on system operation (eg cause a
> > shutdown if humidity is too high, to prevent HW damage).
> That was my original argument when I proposed the sht15 driver. No idea
> if anyone actually does this though!
> > 
> > So question is if hwmon should include explicit environmental monitoring
> > attributes or not, and if hwmon and environmental monitoring can and
> > should be separated to start with (for example, what if any is the
> > difference between environment temperature and hw monitoring
> > temperature ?).
> > 
> > Thoughts, anyone ?
> I wondered exactly this when we added the sht15 driver.  Never really
> came to a firm conclusion though.  If you and Jean decide these shouldn't
> be in hwmon, I'm happy to take them both in IIO.  Given our scope is much
> broader and we already have things like light sensors (also sometimes environment
> monitoring devices), that would work for me.  There are a few sht15 users
> out there I know of beyond the imote2 sensor boards (which is how I encountered
> them).  The imote2 users should be fine as I maintain the platform and most of
> the tools anyway, the others maybe have more issues with a move.
> Uses I know of are indeed more general environment monitoring.
> Not seen one of these in conventional hardware monitoring but could be wrong.
> I guess, Urs will have a better idea of where these tend to be used?
> 
> Obviously there is an argument for an 'environment' monitoring subsystem, but
> my guess in Linus won't pull based on exactly the same issue he had with ALS
> when we proposed that.  Linus won't take small subsystems for sensors given
> he wants them all in the same one.  Actually he mostly wanted them to be in input
> but Dmitry quite rightly pushed back hard against that for exactly the reason
> you are wondering whether they ought to be in hwmon, avoidance of scope spread.
> 
If I take off my hwmon hat for a minute, I would argue that hwmon is a bad term
to start with, and that it should have been envmon to start with. That is how
it was handled at companies I worked for previously, and how it makes sense to me.
Reason is that the term tends to define the scope of a subsystem - if hwmon was named 
envmon, we would not even have this discussion.

Also, it would be desirable to have a linux kernel subsystem to map the scope of RFC 3433,
the Entity Sensor MIB. That MIB does include humidity sensors. The hwmon subsystem
seems to be the best fit for it.

So, in one sentence, for me it makes a lot of sense to explicitly include environmental
monitoring in the scope of the hwmon subsystem.
Now, that is my personal opinion. I'd definitely like to get input from Jean on this.

Guenter

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Urs Fleisch]

> Uses I know of are indeed more general environment monitoring.
> Not seen one of these in conventional hardware monitoring but could be wrong.
> I guess, Urs will have a better idea of where these tend to be used?

Mobile phones could be a promising market for humidity sensors in a Linux system. Fujitsu already sells a mobile phone with built-in temperature and humidity sensors. So we hope to see also Linux-based mobile phones (e.g. Android, MeGoo) with a humidity sensor. Such a sensor could be used to monitor environmental conditions, but there could be other applications too, some of them may not make sense and others can not be foreseen. The humidity sensor could also be used to monitor the hardware of the phone, in this situation, hwmon makes sense. If a sensor can be used for various application, there is probably not one true category. There may be other sensors in hwmon, which can be used for applications other than hardware monitoring. I chose the hwmon directory because the sht15 was already there.

> Highly unusual way of detecting and returning errors.

If this is a problem, I can alter the code. I could also drop all user-register related attributes if they cause problems with the ABI.

Regards,
Urs

Re: [PATCH] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Guenter Roeck]

On Tue, 2011-01-04 at 14:00 -0500, Urs Fleisch wrote:
> > Uses I know of are indeed more general environment monitoring.
> > Not seen one of these in conventional hardware monitoring but could be wrong.
> > I guess, Urs will have a better idea of where these tend to be used?
> 
> Mobile phones could be a promising market for humidity sensors in a Linux system. Fujitsu already sells a mobile phone with built-in temperature and humidity sensors. So we hope to see also Linux-based mobile phones (e.g. Android, MeGoo) with a humidity sensor. Such a sensor could be used to monitor environmental conditions, but there could be other applications too, some of them may not make sense and others can not be foreseen. The humidity sensor could also be used to monitor the hardware of the phone, in this situation, hwmon makes sense. If a sensor can be used for various application, there is probably not one true category. There may be other sensors in hwmon, which can be used for applications other than hardware monitoring. I chose the hwmon directory because the sht15 was already there.
> 
> > Highly unusual way of detecting and returning errors.
> 
> If this is a problem, I can alter the code. I could also drop all user-register related attributes if they cause problems with the ABI.
> 
Please.

Thanks,
Guenter

[PATCH V4] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Urs Fleisch]

Thanks for the review, Jonathan and Guenter.

Changes:
- removed user register related sysfs attributes, keeping only temp1_input and
  humidity1_input
- cleaned up code flow when returning errors for better readability

Regards,
Urs

Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>
---
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 95ccbe3..4a96429 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -688,6 +688,16 @@ config SENSORS_SHT15
 	  This driver can also be built as a module.  If so, the module
 	  will be called sht15.
 
+config SENSORS_SHT21
+	tristate "Sensiron humidity and temperature sensors. SHT21 and compat."
+	depends on I2C
+	help
+	  If you say yes here you get support for the Sensiron SHT21, SHT25
+	  humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called sht21.
+
 config SENSORS_S3C
 	tristate "S3C24XX/S3C64XX Inbuilt ADC"
 	depends on ARCH_S3C2410
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 33c2ee1..7439653 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
 obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
+obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
 obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
 obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
diff --git a/drivers/hwmon/sht21.c b/drivers/hwmon/sht21.c
new file mode 100644
index 0000000..7d3ec82
--- /dev/null
+++ b/drivers/hwmon/sht21.c
@@ -0,0 +1,300 @@
+/* Sensirion SHT21 humidity and temperature sensor driver
+ *
+ * Copyright (C) 2010 Urs Fleisch <urs.fleisch@sensirion.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * Data sheet available (5/2010) at
+ * http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+
+/* I2C command bytes */
+#define SHT21_TRIG_T_MEASUREMENT_HM  0xe3
+#define SHT21_TRIG_RH_MEASUREMENT_HM 0xe5
+
+/**
+ * struct sht21 - SHT21 device specific data
+ * @hwmon_dev: device registered with hwmon
+ * @lock: mutex to protect measurement values
+ * @valid: only 0 before first measurement is taken
+ * @last_update: time of last update (jiffies)
+ * @temperature: cached temperature measurement value
+ * @humidity: cached humidity measurement value
+ */
+struct sht21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	char valid;
+	unsigned long last_update;
+	int temperature;
+	int humidity;
+};
+
+/**
+ * sht21_temp_ticks_to_millicelsius() - convert raw temperature ticks to
+ * milli celsius
+ * @ticks: temperature ticks value received from sensor
+ */
+static inline int sht21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula T = -46.85 + 175.72 * ST / 2^16 from data sheet 6.2,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+/**
+ * sht21_rh_ticks_to_per_cent_mille() - convert raw humidity ticks to
+ * one-thousandths of a percent relative humidity
+ * @ticks: humidity ticks value received from sensor
+ */
+static inline int sht21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula RH = -6 + 125 * SRH / 2^16 from data sheet 6.1,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+/**
+ * sht21_read_word_data() - read word from register
+ * @client: I2C client device
+ * @reg: I2C command byte
+ *
+ * Returns value, negative errno on error.
+ */
+static inline int sht21_read_word_data(struct i2c_client *client, u8 reg)
+{
+	int ret = i2c_smbus_read_word_data(client, reg);
+	if (ret < 0)
+		return ret;
+	/* SMBus specifies low byte first, but the SHT21 returns MSB
+	 * first, so we have to swab16 the values */
+	return swab16(ret);
+}
+
+/**
+ * sht21_update_measurements() - get updated measurements from device
+ * @client: I2C client device
+ *
+ * Returns 0 on success, else negative errno.
+ */
+static int sht21_update_measurements(struct i2c_client *client)
+{
+	int ret = 0;
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	mutex_lock(&sht21->lock);
+	/* Data sheet 2.4:
+	 * SHT2x should not be active for more than 10% of the time - e.g.
+	 * maximum two measurements per second at 12bit accuracy shall be made.
+	 */
+	if (time_after(jiffies, sht21->last_update + HZ / 2) || !sht21->valid) {
+		ret = sht21_read_word_data(client, SHT21_TRIG_T_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		sht21->temperature = sht21_temp_ticks_to_millicelsius(ret);
+		ret = sht21_read_word_data(client,
+					SHT21_TRIG_RH_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		sht21->humidity = sht21_rh_ticks_to_per_cent_mille(ret);
+		sht21->last_update = jiffies;
+		sht21->valid = 1;
+	}
+out:
+	mutex_unlock(&sht21->lock);
+
+	return ret >= 0 ? 0 : ret;
+}
+
+/**
+ * sht21_show_temperature() - show temperature measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to temp1_input sysfs attribute.
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_temperature(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	int ret = sht21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", sht21->temperature);
+}
+
+/**
+ * sht21_show_humidity() - show humidity measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to humidity1_input sysfs attribute.
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_humidity(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	int ret = sht21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", sht21->humidity);
+}
+
+/* sysfs attributes */
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, sht21_show_temperature,
+	NULL, 0);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, sht21_show_humidity,
+	NULL, 0);
+
+static struct attribute *sht21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group sht21_attr_group = {
+	.attrs = sht21_attributes,
+};
+
+/**
+ * sht21_probe() - probe device
+ * @client: I2C client device
+ * @id: device ID
+ *
+ * Called by the I2C core when an entry in the ID table matches a
+ * device's name.
+ * Returns 0 on success.
+ */
+static int __devinit sht21_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct sht21 *sht21;
+	int err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_WORD_DATA)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus word transactions\n");
+		return -ENODEV;
+	}
+
+	sht21 = kzalloc(sizeof(*sht21), GFP_KERNEL);
+	if (!sht21) {
+		dev_dbg(&client->dev, "kzalloc failed\n");
+		return -ENOMEM;
+	}
+	i2c_set_clientdata(client, sht21);
+
+	mutex_init(&sht21->lock);
+
+	err = sysfs_create_group(&client->dev.kobj, &sht21_attr_group);
+	if (err) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		goto fail_free;
+	}
+	sht21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(sht21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		err = PTR_ERR(sht21->hwmon_dev);
+		goto fail_remove_sysfs;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+fail_remove_sysfs:
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+fail_free:
+	kfree(sht21);
+
+	return err;
+}
+
+/**
+ * sht21_remove() - remove device
+ * @client: I2C client device
+ */
+static int __devexit sht21_remove(struct i2c_client *client)
+{
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(sht21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+	kfree(sht21);
+
+	return 0;
+}
+
+/* Device ID table */
+static const struct i2c_device_id sht21_id[] = {
+	{ "sht21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sht21_id);
+
+static struct i2c_driver sht21_driver = {
+	.driver.name = "sht21",
+	.probe       = sht21_probe,
+	.remove      = __devexit_p(sht21_remove),
+	.id_table    = sht21_id,
+};
+
+/**
+ * sht21_init() - initialize driver
+ *
+ * Called when kernel is booted or module is inserted.
+ * Returns 0 on success.
+ */
+static int __init sht21_init(void)
+{
+	return i2c_add_driver(&sht21_driver);
+}
+module_init(sht21_init);
+
+/**
+ * sht21_init() - clean up driver
+ *
+ * Called when module is removed.
+ */
+static void __exit sht21_exit(void)
+{
+	i2c_del_driver(&sht21_driver);
+}
+module_exit(sht21_exit);
+
+MODULE_AUTHOR("Urs Fleisch <urs.fleisch@sensirion.com>");
+MODULE_DESCRIPTION("Sensirion SHT21 humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");

Re: [PATCH V4] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Jonathan Cameron]

On 01/06/11 07:43, Urs Fleisch wrote:
> Thanks for the review, Jonathan and Guenter.
> 
> Changes:
> - removed user register related sysfs attributes, keeping only temp1_input and
>   humidity1_input
> - cleaned up code flow when returning errors for better readability
Short and clean. I do wonder if you want some of those users register
bits available via platform data though.  Perhaps that can be added in
a later patch as and when someone needs them.
> 
> Regards,
> Urs
> 
> Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 95ccbe3..4a96429 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -688,6 +688,16 @@ config SENSORS_SHT15
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called sht15.
>  
> +config SENSORS_SHT21
> +	tristate "Sensiron humidity and temperature sensors. SHT21 and compat."
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Sensiron SHT21, SHT25
> +	  humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called sht21.
> +
>  config SENSORS_S3C
>  	tristate "S3C24XX/S3C64XX Inbuilt ADC"
>  	depends on ARCH_S3C2410
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 33c2ee1..7439653 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -80,6 +80,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
>  obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
>  obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
>  obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
> +obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
>  obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
>  obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
>  obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
> diff --git a/drivers/hwmon/sht21.c b/drivers/hwmon/sht21.c
> new file mode 100644
> index 0000000..7d3ec82
> --- /dev/null
> +++ b/drivers/hwmon/sht21.c
> @@ -0,0 +1,300 @@
> +/* Sensirion SHT21 humidity and temperature sensor driver
> + *
> + * Copyright (C) 2010 Urs Fleisch <urs.fleisch@sensirion.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + * Data sheet available (5/2010) at
> + * http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/device.h>
> +
> +/* I2C command bytes */
> +#define SHT21_TRIG_T_MEASUREMENT_HM  0xe3
> +#define SHT21_TRIG_RH_MEASUREMENT_HM 0xe5
> +
> +/**
> + * struct sht21 - SHT21 device specific data
> + * @hwmon_dev: device registered with hwmon
> + * @lock: mutex to protect measurement values
> + * @valid: only 0 before first measurement is taken
> + * @last_update: time of last update (jiffies)
> + * @temperature: cached temperature measurement value
> + * @humidity: cached humidity measurement value
> + */
> +struct sht21 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	char valid;
> +	unsigned long last_update;
> +	int temperature;
> +	int humidity;
> +};
> +
> +/**
> + * sht21_temp_ticks_to_millicelsius() - convert raw temperature ticks to
> + * milli celsius
> + * @ticks: temperature ticks value received from sensor
> + */
> +static inline int sht21_temp_ticks_to_millicelsius(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/* Formula T = -46.85 + 175.72 * ST / 2^16 from data sheet 6.2,
> +	   optimized for integer fixed point (3 digits) arithmetic */
> +	return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +/**
> + * sht21_rh_ticks_to_per_cent_mille() - convert raw humidity ticks to
> + * one-thousandths of a percent relative humidity
> + * @ticks: humidity ticks value received from sensor
> + */
> +static inline int sht21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/* Formula RH = -6 + 125 * SRH / 2^16 from data sheet 6.1,
> +	   optimized for integer fixed point (3 digits) arithmetic */
> +	return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +/**
> + * sht21_read_word_data() - read word from register
> + * @client: I2C client device
> + * @reg: I2C command byte
> + *
> + * Returns value, negative errno on error.
> + */
> +static inline int sht21_read_word_data(struct i2c_client *client, u8 reg)
> +{
> +	int ret = i2c_smbus_read_word_data(client, reg);
> +	if (ret < 0)
> +		return ret;
> +	/* SMBus specifies low byte first, but the SHT21 returns MSB
> +	 * first, so we have to swab16 the values */
> +	return swab16(ret);
> +}
> +
> +/**
> + * sht21_update_measurements() - get updated measurements from device
> + * @client: I2C client device
> + *
> + * Returns 0 on success, else negative errno.
> + */
> +static int sht21_update_measurements(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&sht21->lock);
> +	/* Data sheet 2.4:
> +	 * SHT2x should not be active for more than 10% of the time - e.g.
> +	 * maximum two measurements per second at 12bit accuracy shall be made.
> +	 */
> +	if (time_after(jiffies, sht21->last_update + HZ / 2) || !sht21->valid) {
> +		ret = sht21_read_word_data(client, SHT21_TRIG_T_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		sht21->temperature = sht21_temp_ticks_to_millicelsius(ret);
> +		ret = sht21_read_word_data(client,
> +					SHT21_TRIG_RH_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		sht21->humidity = sht21_rh_ticks_to_per_cent_mille(ret);
> +		sht21->last_update = jiffies;
> +		sht21->valid = 1;
> +	}
> +out:
> +	mutex_unlock(&sht21->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +/**
> + * sht21_show_temperature() - show temperature measurement value in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
> + *
> + * Will be called on read access to temp1_input sysfs attribute.
> + * Returns number of bytes written into buffer, negative errno on error.
> + */
> +static ssize_t sht21_show_temperature(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +	int ret = sht21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", sht21->temperature);
> +}
> +
> +/**
> + * sht21_show_humidity() - show humidity measurement value in sysfs
> + * @dev: device
> + * @attr: device attribute
> + * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
> + *
> + * Will be called on read access to humidity1_input sysfs attribute.
> + * Returns number of bytes written into buffer, negative errno on error.
> + */
> +static ssize_t sht21_show_humidity(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +	int ret = sht21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", sht21->humidity);
> +}
> +
> +/* sysfs attributes */
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, sht21_show_temperature,
> +	NULL, 0);
> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, sht21_show_humidity,
> +	NULL, 0);
> +
> +static struct attribute *sht21_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group sht21_attr_group = {
> +	.attrs = sht21_attributes,
> +};
> +
> +/**
> + * sht21_probe() - probe device
> + * @client: I2C client device
> + * @id: device ID
> + *
> + * Called by the I2C core when an entry in the ID table matches a
> + * device's name.
> + * Returns 0 on success.
> + */
> +static int __devinit sht21_probe(struct i2c_client *client,
> +	const struct i2c_device_id *id)
> +{
> +	struct sht21 *sht21;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WORD_DATA)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	sht21 = kzalloc(sizeof(*sht21), GFP_KERNEL);
> +	if (!sht21) {
> +		dev_dbg(&client->dev, "kzalloc failed\n");
> +		return -ENOMEM;
> +	}
> +	i2c_set_clientdata(client, sht21);
> +
> +	mutex_init(&sht21->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &sht21_attr_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		goto fail_free;
> +	}
> +	sht21->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(sht21->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(sht21->hwmon_dev);
> +		goto fail_remove_sysfs;
> +	}
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +fail_remove_sysfs:
> +	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
> +fail_free:
> +	kfree(sht21);
> +
> +	return err;
> +}
> +
> +/**
> + * sht21_remove() - remove device
> + * @client: I2C client device
> + */
> +static int __devexit sht21_remove(struct i2c_client *client)
> +{
> +	struct sht21 *sht21 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(sht21->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
> +	kfree(sht21);
> +
> +	return 0;
> +}
> +
> +/* Device ID table */
> +static const struct i2c_device_id sht21_id[] = {
> +	{ "sht21", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, sht21_id);
> +
> +static struct i2c_driver sht21_driver = {
> +	.driver.name = "sht21",
> +	.probe       = sht21_probe,
> +	.remove      = __devexit_p(sht21_remove),
> +	.id_table    = sht21_id,
> +};
> +
> +/**
> + * sht21_init() - initialize driver
> + *
> + * Called when kernel is booted or module is inserted.
> + * Returns 0 on success.
> + */
> +static int __init sht21_init(void)
> +{
> +	return i2c_add_driver(&sht21_driver);
> +}
> +module_init(sht21_init);
> +
> +/**
> + * sht21_init() - clean up driver
> + *
> + * Called when module is removed.
> + */
> +static void __exit sht21_exit(void)
> +{
> +	i2c_del_driver(&sht21_driver);
> +}
> +module_exit(sht21_exit);
> +
> +MODULE_AUTHOR("Urs Fleisch <urs.fleisch@sensirion.com>");
> +MODULE_DESCRIPTION("Sensirion SHT21 humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
> 

Re: [PATCH V4] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Guenter Roeck]

On Thu, Jan 06, 2011 at 05:58:23AM -0500, Jonathan Cameron wrote:
> On 01/06/11 07:43, Urs Fleisch wrote:
> > Thanks for the review, Jonathan and Guenter.
> > 
> > Changes:
> > - removed user register related sysfs attributes, keeping only temp1_input and
> >   humidity1_input
> > - cleaned up code flow when returning errors for better readability
> Short and clean. I do wonder if you want some of those users register
> bits available via platform data though.  Perhaps that can be added in
> a later patch as and when someone needs them.

Only thing missing is Documentation/hwmon/sht21.

Urs, can you add that ? 

Thanks,
Guenter

Re: [PATCH V4] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Guenter Roeck]

On Thu, Jan 06, 2011 at 02:43:38AM -0500, Urs Fleisch wrote:
> Thanks for the review, Jonathan and Guenter.
> 
> Changes:
> - removed user register related sysfs attributes, keeping only temp1_input and
>   humidity1_input
> - cleaned up code flow when returning errors for better readability
> 
> Regards,
> Urs
> 
> Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>

Urs,

what tree is your patch based on ? I am having trouble applying it.

Guenter

[PATCH V5] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Urs Fleisch]

> Only thing missing is Documentation/hwmon/sht21.
> 
> Urs, can you add that ? 
I have added a file Documentation/hwmon/sht21 to the patch.

> what tree is your patch based on ? I am having trouble applying it.
I used the 2.6.32 kernel from the rowboat project
(http://code.google.com/p/rowboat/) because my sensor is connected to an
OMAP3EVM board. To make the patch applicable for the latest kernel, I added
a line to the Makefile patch. I could apply the patch without problems to
the linux-next kernel, although patch reported fuzz with Kconfig. The patch
below is now based on linux-next, so there should be no longer problems.

Thanks,
Urs

Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>
---
diff --git a/Documentation/hwmon/sht21 b/Documentation/hwmon/sht21
new file mode 100644
index 0000000..db17fda
--- /dev/null
+++ b/Documentation/hwmon/sht21
@@ -0,0 +1,49 @@
+Kernel driver sht21
+===================
+
+Supported chips:
+  * Sensirion SHT21
+    Prefix: 'sht21'
+    Addresses scanned: none
+    Datasheet: Publicly available at the Sensirion website
+    http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
+
+  * Sensirion SHT25
+    Prefix: 'sht21'
+    Addresses scanned: none
+    Datasheet: Publicly available at the Sensirion website
+    http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT25.pdf
+
+Author:
+  Urs Fleisch <urs.fleisch@sensirion.com>
+
+Description
+-----------
+
+The SHT21 and SHT25 are humidity and temperature sensors in a DFN package of
+only 3 x 3 mm footprint and 1.1 mm height. The difference between the two
+devices is the higher level of precision of the SHT25 (1.8% relative humidity,
+0.2 degree Celsius) compared with the SHT21 (2.0% relative humidity,
+0.3 degree Celsius).
+
+The devices communicate with the I2C protocol. All sensors are set to the same
+I2C address 0x40, so an entry with I2C_BOARD_INFO("sht21", 0x40) can be used
+in the board setup code.
+
+sysfs-Interface
+---------------
+
+temp1_input - temperature input
+humidity1_input - humidity input
+
+Notes
+-----
+
+The driver uses the default resolution settings of 12 bit for humidity and 14
+bit for temperature, which results in typical measurement times of 22 ms for
+humidity and 66 ms for temperature. To keep self heating below 0.1 degree
+Celsius, the device should not be active for more than 10% of the time,
+e.g. maximum two measurements per second at the given resolution.
+
+Different resolutions, the on-chip heater, using the CRC checksum and reading
+the serial number are not supported yet.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 5fcdbb4..35f00da 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -744,6 +744,16 @@ config SENSORS_SHT15
 	  This driver can also be built as a module.  If so, the module
 	  will be called sht15.
 
+config SENSORS_SHT21
+	tristate "Sensiron humidity and temperature sensors. SHT21 and compat."
+	depends on I2C
+	help
+	  If you say yes here you get support for the Sensiron SHT21, SHT25
+	  humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called sht21.
+
 config SENSORS_S3C
 	tristate "Samsung built-in ADC"
 	depends on S3C_ADC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 42542d7..dde02d9 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
 obj-$(CONFIG_SENSORS_SHT15)	+= sht15.o
+obj-$(CONFIG_SENSORS_SHT21)	+= sht21.o
 obj-$(CONFIG_SENSORS_SIS5595)	+= sis5595.o
 obj-$(CONFIG_SENSORS_SMM665)	+= smm665.o
 obj-$(CONFIG_SENSORS_SMSC47B397)+= smsc47b397.o
diff --git a/drivers/hwmon/sht21.c b/drivers/hwmon/sht21.c
new file mode 100644
index 0000000..f14c262
--- /dev/null
+++ b/drivers/hwmon/sht21.c
@@ -0,0 +1,300 @@
+/* Sensirion SHT21 humidity and temperature sensor driver
+ *
+ * Copyright (C) 2010 Urs Fleisch <urs.fleisch@sensirion.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ * Data sheet available (5/2010) at
+ * http://www.sensirion.com/en/pdf/product_information/Datasheet-humidity-sensor-SHT21.pdf
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/err.h>
+#include <linux/mutex.h>
+#include <linux/device.h>
+
+/* I2C command bytes */
+#define SHT21_TRIG_T_MEASUREMENT_HM  0xe3
+#define SHT21_TRIG_RH_MEASUREMENT_HM 0xe5
+
+/**
+ * struct sht21 - SHT21 device specific data
+ * @hwmon_dev: device registered with hwmon
+ * @lock: mutex to protect measurement values
+ * @valid: only 0 before first measurement is taken
+ * @last_update: time of last update (jiffies)
+ * @temperature: cached temperature measurement value
+ * @humidity: cached humidity measurement value
+ */
+struct sht21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	char valid;
+	unsigned long last_update;
+	int temperature;
+	int humidity;
+};
+
+/**
+ * sht21_temp_ticks_to_millicelsius() - convert raw temperature ticks to
+ * milli celsius
+ * @ticks: temperature ticks value received from sensor
+ */
+static inline int sht21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula T = -46.85 + 175.72 * ST / 2^16 from data sheet 6.2,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+/**
+ * sht21_rh_ticks_to_per_cent_mille() - convert raw humidity ticks to
+ * one-thousandths of a percent relative humidity
+ * @ticks: humidity ticks value received from sensor
+ */
+static inline int sht21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/* Formula RH = -6 + 125 * SRH / 2^16 from data sheet 6.1,
+	   optimized for integer fixed point (3 digits) arithmetic */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+/**
+ * sht21_read_word_data() - read word from register
+ * @client: I2C client device
+ * @reg: I2C command byte
+ *
+ * Returns value, negative errno on error.
+ */
+static inline int sht21_read_word_data(struct i2c_client *client, u8 reg)
+{
+	int ret = i2c_smbus_read_word_data(client, reg);
+	if (ret < 0)
+		return ret;
+	/* SMBus specifies low byte first, but the SHT21 returns MSB
+	 * first, so we have to swab16 the values */
+	return swab16(ret);
+}
+
+/**
+ * sht21_update_measurements() - get updated measurements from device
+ * @client: I2C client device
+ *
+ * Returns 0 on success, else negative errno.
+ */
+static int sht21_update_measurements(struct i2c_client *client)
+{
+	int ret = 0;
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	mutex_lock(&sht21->lock);
+	/* Data sheet 2.4:
+	 * SHT2x should not be active for more than 10% of the time - e.g.
+	 * maximum two measurements per second at 12bit accuracy shall be made.
+	 */
+	if (time_after(jiffies, sht21->last_update + HZ / 2) || !sht21->valid) {
+		ret = sht21_read_word_data(client, SHT21_TRIG_T_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		sht21->temperature = sht21_temp_ticks_to_millicelsius(ret);
+		ret = sht21_read_word_data(client,
+					SHT21_TRIG_RH_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		sht21->humidity = sht21_rh_ticks_to_per_cent_mille(ret);
+		sht21->last_update = jiffies;
+		sht21->valid = 1;
+	}
+out:
+	mutex_unlock(&sht21->lock);
+
+	return ret >= 0 ? 0 : ret;
+}
+
+/**
+ * sht21_show_temperature() - show temperature measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to temp1_input sysfs attribute.
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_temperature(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	int ret = sht21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", sht21->temperature);
+}
+
+/**
+ * sht21_show_humidity() - show humidity measurement value in sysfs
+ * @dev: device
+ * @attr: device attribute
+ * @buf: sysfs buffer (PAGE_SIZE) where measurement values are written to
+ *
+ * Will be called on read access to humidity1_input sysfs attribute.
+ * Returns number of bytes written into buffer, negative errno on error.
+ */
+static ssize_t sht21_show_humidity(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+	int ret = sht21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", sht21->humidity);
+}
+
+/* sysfs attributes */
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, sht21_show_temperature,
+	NULL, 0);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, sht21_show_humidity,
+	NULL, 0);
+
+static struct attribute *sht21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group sht21_attr_group = {
+	.attrs = sht21_attributes,
+};
+
+/**
+ * sht21_probe() - probe device
+ * @client: I2C client device
+ * @id: device ID
+ *
+ * Called by the I2C core when an entry in the ID table matches a
+ * device's name.
+ * Returns 0 on success.
+ */
+static int __devinit sht21_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct sht21 *sht21;
+	int err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_WORD_DATA)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus word transactions\n");
+		return -ENODEV;
+	}
+
+	sht21 = kzalloc(sizeof(*sht21), GFP_KERNEL);
+	if (!sht21) {
+		dev_dbg(&client->dev, "kzalloc failed\n");
+		return -ENOMEM;
+	}
+	i2c_set_clientdata(client, sht21);
+
+	mutex_init(&sht21->lock);
+
+	err = sysfs_create_group(&client->dev.kobj, &sht21_attr_group);
+	if (err) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		goto fail_free;
+	}
+	sht21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(sht21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		err = PTR_ERR(sht21->hwmon_dev);
+		goto fail_remove_sysfs;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+fail_remove_sysfs:
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+fail_free:
+	kfree(sht21);
+
+	return err;
+}
+
+/**
+ * sht21_remove() - remove device
+ * @client: I2C client device
+ */
+static int __devexit sht21_remove(struct i2c_client *client)
+{
+	struct sht21 *sht21 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(sht21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &sht21_attr_group);
+	kfree(sht21);
+
+	return 0;
+}
+
+/* Device ID table */
+static const struct i2c_device_id sht21_id[] = {
+	{ "sht21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, sht21_id);
+
+static struct i2c_driver sht21_driver = {
+	.driver.name = "sht21",
+	.probe       = sht21_probe,
+	.remove      = __devexit_p(sht21_remove),
+	.id_table    = sht21_id,
+};
+
+/**
+ * sht21_init() - initialize driver
+ *
+ * Called when kernel is booted or module is inserted.
+ * Returns 0 on success.
+ */
+static int __init sht21_init(void)
+{
+	return i2c_add_driver(&sht21_driver);
+}
+module_init(sht21_init);
+
+/**
+ * sht21_init() - clean up driver
+ *
+ * Called when module is removed.
+ */
+static void __exit sht21_exit(void)
+{
+	i2c_del_driver(&sht21_driver);
+}
+module_exit(sht21_exit);
+
+MODULE_AUTHOR("Urs Fleisch <urs.fleisch@sensirion.com>");
+MODULE_DESCRIPTION("Sensirion SHT21 humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");

Re: [PATCH V5] hwmon: driver for Sensirion SHT21 humidity and temperature sensor

[Guenter Roeck]

On Fri, Jan 07, 2011 at 02:15:39AM -0500, Urs Fleisch wrote:
> > Only thing missing is Documentation/hwmon/sht21.
> >
> > Urs, can you add that ?
> I have added a file Documentation/hwmon/sht21 to the patch.
> 
> > what tree is your patch based on ? I am having trouble applying it.
> I used the 2.6.32 kernel from the rowboat project
> (http://code.google.com/p/rowboat/) because my sensor is connected to an
> OMAP3EVM board. To make the patch applicable for the latest kernel, I added
> a line to the Makefile patch. I could apply the patch without problems to
> the linux-next kernel, although patch reported fuzz with Kconfig. The patch
> below is now based on linux-next, so there should be no longer problems.
> 
> Thanks,
> Urs
> 
> Signed-off-by: Urs Fleisch <urs.fleisch@sensirion.com>

Applied to -next, with Jonathan's Acked-by and multi-line comments changed
to adhere to Documentation/CodingStyle.

Thanks,
Guenter