All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
@ 2013-08-28  6:49 ` Markezana, William
  0 siblings, 0 replies; 15+ messages in thread
From: Markezana, William @ 2013-08-28  6:49 UTC (permalink / raw)
  To: khali, linux, lm-sensors; +Cc: linux-kernel

>From 55fc390d0079841405d5345b55a6e158ca5f3749 Mon Sep 17 00:00:00 2001
From: William Markezana <william.markezana@meas-spec.com>
Date: Wed, 28 Aug 2013 08:33:49 +0200
Subject: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support

---
 Documentation/hwmon/htu21 |   43 ++++++++++
 drivers/hwmon/Kconfig     |   10 +++
 drivers/hwmon/Makefile    |    1 +
 drivers/hwmon/htu21.c     |  201 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 255 insertions(+)
 create mode 100644 Documentation/hwmon/htu21
 create mode 100644 drivers/hwmon/htu21.c

diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
new file mode 100644
index 0000000..ab756bc
--- /dev/null
+++ b/Documentation/hwmon/htu21
@@ -0,0 +1,43 @@
+Kernel driver htu21
+===================
+
+Supported chips:
+  * Measurement Specialties HTU21D
+    Prefix: 'htu21'
+    Addresses scanned: none
+    Datasheet: Publicly available at the Measurement Specialties website
+    http://www.meas-spec.com/downloads/HTU21D.pdf
+
+
+Author:
+  William Markezana <william.markezana@meas-spec.com>
+
+Description
+-----------
+
+HTU21D(F), the new digital humidity sensor with temperature output of MEAS is about 
+to set new standards in terms of size and intelligence: Embedded in a reflow 
+solderable Dual Flat No leads (DFN) package of 3 x 3 mm foot print and 0.9 mm height 
+it provides calibrated, linearized signals in digital, I²C format.
+
+The devices communicate with the I2C protocol. All sensors are set to the same
+I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 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 11 ms for
+humidity and 44 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 e989f7f..eb46eb8 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -511,6 +511,16 @@ config SENSORS_HIH6130
 	  This driver can also be built as a module.  If so, the module
 	  will be called hih6130.
 
+config SENSORS_HTU21
+	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Measurement Specialties
+	  HTU21D humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called htu21.
+
 config SENSORS_CORETEMP
 	tristate "Intel Core/Core2/Atom temperature sensor"
 	depends on X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4f0fb52..ec7cde0 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
 obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
 obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
 obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
+obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
 obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
 obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
 obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
new file mode 100644
index 0000000..8b8d132
--- /dev/null
+++ b/drivers/hwmon/htu21.c
@@ -0,0 +1,201 @@
+/* Measurement Specialties HTU21D humidity and temperature sensor driver
+ *
+ * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Data sheet available (7/2013) at
+ * http://www.meas-spec.com/downloads/HTU21D.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>
+#include <linux/jiffies.h>
+
+/* HTU21 Commands */
+#define HTU21_T_MEASUREMENT_HM  0xE3
+#define HTU21_RH_MEASUREMENT_HM 0xE5
+
+struct htu21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	char valid;
+	unsigned long last_update;
+	int temperature;
+	int humidity;
+};
+
+static inline int htu21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/*
+	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
+	 * optimized for integer fixed point (3 digits) arithmetic
+	 */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/*
+	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
+	 * optimized for integer fixed point (3 digits) arithmetic
+	 */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+static int htu21_update_measurements(struct i2c_client *client)
+{
+	int ret = 0;
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+
+	mutex_lock(&htu21->lock);
+
+	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
+		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
+		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
+		htu21->last_update = jiffies;
+		htu21->valid = 1;
+	}
+out:
+	mutex_unlock(&htu21->lock);
+
+	return ret >= 0 ? 0 : ret;
+}
+
+static ssize_t htu21_show_temperature(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+	int ret = htu21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", htu21->temperature);
+}
+
+static ssize_t htu21_show_humidity(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+	int ret = htu21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", htu21->humidity);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, htu21_show_temperature,	NULL, 0);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, htu21_show_humidity, NULL, 0);
+
+static struct attribute *htu21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group htu21_group = {
+	.attrs = htu21_attributes,
+};
+
+static int htu21_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct htu21 *htu21;
+	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;
+	}
+
+	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
+	if (!htu21)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, htu21);
+
+	mutex_init(&htu21->lock);
+
+	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
+	if (err) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		return err;
+	}
+	htu21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(htu21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		err = PTR_ERR(htu21->hwmon_dev);
+		goto error;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+error:
+	sysfs_remove_group(&client->dev.kobj, &htu21_group);
+	return err;
+}
+
+static int htu21_remove(struct i2c_client *client)
+{
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(htu21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &htu21_group);
+
+	return 0;
+}
+
+static const struct i2c_device_id htu21_id[] = {
+	{ "htu21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, htu21_id);
+
+static struct i2c_driver htu21_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name	= "htu21",
+	},
+	.probe       = htu21_probe,
+	.remove      = htu21_remove,
+	.id_table    = htu21_id,
+};
+
+module_i2c_driver(htu21_driver);
+
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_DESCRIPTION("Measurement Specialties HTU21D humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");
-- 
1.7.10.4


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

* [lm-sensors] [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
@ 2013-08-28  6:49 ` Markezana, William
  0 siblings, 0 replies; 15+ messages in thread
From: Markezana, William @ 2013-08-28  6:49 UTC (permalink / raw)
  To: khali, linux, lm-sensors; +Cc: linux-kernel

From 55fc390d0079841405d5345b55a6e158ca5f3749 Mon Sep 17 00:00:00 2001
From: William Markezana <william.markezana@meas-spec.com>
Date: Wed, 28 Aug 2013 08:33:49 +0200
Subject: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support

---
 Documentation/hwmon/htu21 |   43 ++++++++++
 drivers/hwmon/Kconfig     |   10 +++
 drivers/hwmon/Makefile    |    1 +
 drivers/hwmon/htu21.c     |  201 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 255 insertions(+)
 create mode 100644 Documentation/hwmon/htu21
 create mode 100644 drivers/hwmon/htu21.c

diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
new file mode 100644
index 0000000..ab756bc
--- /dev/null
+++ b/Documentation/hwmon/htu21
@@ -0,0 +1,43 @@
+Kernel driver htu21
+===================
+
+Supported chips:
+  * Measurement Specialties HTU21D
+    Prefix: 'htu21'
+    Addresses scanned: none
+    Datasheet: Publicly available at the Measurement Specialties website
+    http://www.meas-spec.com/downloads/HTU21D.pdf
+
+
+Author:
+  William Markezana <william.markezana@meas-spec.com>
+
+Description
+-----------
+
+HTU21D(F), the new digital humidity sensor with temperature output of MEAS is about 
+to set new standards in terms of size and intelligence: Embedded in a reflow 
+solderable Dual Flat No leads (DFN) package of 3 x 3 mm foot print and 0.9 mm height 
+it provides calibrated, linearized signals in digital, I²C format.
+
+The devices communicate with the I2C protocol. All sensors are set to the same
+I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 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 11 ms for
+humidity and 44 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 e989f7f..eb46eb8 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -511,6 +511,16 @@ config SENSORS_HIH6130
 	  This driver can also be built as a module.  If so, the module
 	  will be called hih6130.
 
+config SENSORS_HTU21
+	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Measurement Specialties
+	  HTU21D humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called htu21.
+
 config SENSORS_CORETEMP
 	tristate "Intel Core/Core2/Atom temperature sensor"
 	depends on X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4f0fb52..ec7cde0 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
 obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
 obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
 obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
+obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
 obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
 obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
 obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
new file mode 100644
index 0000000..8b8d132
--- /dev/null
+++ b/drivers/hwmon/htu21.c
@@ -0,0 +1,201 @@
+/* Measurement Specialties HTU21D humidity and temperature sensor driver
+ *
+ * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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., 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ * Data sheet available (7/2013) at
+ * http://www.meas-spec.com/downloads/HTU21D.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>
+#include <linux/jiffies.h>
+
+/* HTU21 Commands */
+#define HTU21_T_MEASUREMENT_HM  0xE3
+#define HTU21_RH_MEASUREMENT_HM 0xE5
+
+struct htu21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	char valid;
+	unsigned long last_update;
+	int temperature;
+	int humidity;
+};
+
+static inline int htu21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/*
+	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
+	 * optimized for integer fixed point (3 digits) arithmetic
+	 */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/*
+	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
+	 * optimized for integer fixed point (3 digits) arithmetic
+	 */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+static int htu21_update_measurements(struct i2c_client *client)
+{
+	int ret = 0;
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+
+	mutex_lock(&htu21->lock);
+
+	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
+		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
+		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
+		htu21->last_update = jiffies;
+		htu21->valid = 1;
+	}
+out:
+	mutex_unlock(&htu21->lock);
+
+	return ret >= 0 ? 0 : ret;
+}
+
+static ssize_t htu21_show_temperature(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+	int ret = htu21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", htu21->temperature);
+}
+
+static ssize_t htu21_show_humidity(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+	int ret = htu21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", htu21->humidity);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, htu21_show_temperature,	NULL, 0);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, htu21_show_humidity, NULL, 0);
+
+static struct attribute *htu21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group htu21_group = {
+	.attrs = htu21_attributes,
+};
+
+static int htu21_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct htu21 *htu21;
+	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;
+	}
+
+	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
+	if (!htu21)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, htu21);
+
+	mutex_init(&htu21->lock);
+
+	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
+	if (err) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		return err;
+	}
+	htu21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(htu21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		err = PTR_ERR(htu21->hwmon_dev);
+		goto error;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+error:
+	sysfs_remove_group(&client->dev.kobj, &htu21_group);
+	return err;
+}
+
+static int htu21_remove(struct i2c_client *client)
+{
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(htu21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &htu21_group);
+
+	return 0;
+}
+
+static const struct i2c_device_id htu21_id[] = {
+	{ "htu21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, htu21_id);
+
+static struct i2c_driver htu21_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name	= "htu21",
+	},
+	.probe       = htu21_probe,
+	.remove      = htu21_remove,
+	.id_table    = htu21_id,
+};
+
+module_i2c_driver(htu21_driver);
+
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_DESCRIPTION("Measurement Specialties HTU21D humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");
-- 
1.7.10.4


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* Re: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
  2013-08-28  6:49 ` [lm-sensors] " Markezana, William
@ 2013-08-28 16:18   ` Guenter Roeck
  -1 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2013-08-28 16:18 UTC (permalink / raw)
  To: Markezana, William; +Cc: khali, lm-sensors, linux-kernel

On Wed, Aug 28, 2013 at 08:49:36AM +0200, Markezana, William wrote:
> From 55fc390d0079841405d5345b55a6e158ca5f3749 Mon Sep 17 00:00:00 2001
> From: William Markezana <william.markezana@meas-spec.com>
> Date: Wed, 28 Aug 2013 08:33:49 +0200
> Subject: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
> 

Your patch got corrupted and produces lots of checkpatch errors as result
(and can not be aplied). YOu might want to use a diffeent mailer for the next
version.

In addition to that, there are several other checkpatch errors and warnings.

ERROR: trailing statements should be on next line
WARNING: line over 80 characters
ERROR: Missing Signed-off-by: line(s)

Some more comments below.

Thanks,
Guenter

> ---
>  Documentation/hwmon/htu21 |   43 ++++++++++
>  drivers/hwmon/Kconfig     |   10 +++
>  drivers/hwmon/Makefile    |    1 +
>  drivers/hwmon/htu21.c     |  201 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 255 insertions(+)
>  create mode 100644 Documentation/hwmon/htu21
>  create mode 100644 drivers/hwmon/htu21.c
> 
> diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> new file mode 100644
> index 0000000..ab756bc
> --- /dev/null
> +++ b/Documentation/hwmon/htu21
> @@ -0,0 +1,43 @@
> +Kernel driver htu21
> +===================
> +
> +Supported chips:
> +  * Measurement Specialties HTU21D
> +    Prefix: 'htu21'
> +    Addresses scanned: none
> +    Datasheet: Publicly available at the Measurement Specialties website
> +    http://www.meas-spec.com/downloads/HTU21D.pdf
> +
> +
> +Author:
> +  William Markezana <william.markezana@meas-spec.com>
> +
> +Description
> +-----------
> +
> +HTU21D(F), the new digital humidity sensor with temperature output of MEAS is about 
> +to set new standards in terms of size and intelligence: Embedded in a reflow 
> +solderable Dual Flat No leads (DFN) package of 3 x 3 mm foot print and 0.9 mm height 
> +it provides calibrated, linearized signals in digital, I²C format.
> +
Please drop the marketing pitch. Just describe what the chip is doing.

> +The devices communicate with the I2C protocol. All sensors are set to the same
> +I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can be used
> +in the board setup code.
> +

I would suggest to add something like

"This driver does not auto-detect devices. You will have to instantiate the
devices explicitly. Please see Documentation/i2c/instantiating-devices for
details."

to the notes instead. After all, there are other means to instantiate the
driver.

> +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 11 ms for
> +humidity and 44 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.
> +
You might want to add that the driver is doing this, or phrase it differently,
such as

"To keep self heating below 0.1 degree Celsius, the device should not be
active for more than 10% of the time. For this reason, the driver performs no
more than two measurements per second and reports cached information if polled
more frequently."

> +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 e989f7f..eb46eb8 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -511,6 +511,16 @@ config SENSORS_HIH6130
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called hih6130.
>  
> +config SENSORS_HTU21
> +	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  HTU21D humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called htu21.
> +
>  config SENSORS_CORETEMP
>  	tristate "Intel Core/Core2/Atom temperature sensor"
>  	depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4f0fb52..ec7cde0 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
>  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
>  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
>  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
> +obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
>  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
>  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
> diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> new file mode 100644
> index 0000000..8b8d132
> --- /dev/null
> +++ b/drivers/hwmon/htu21.c
> @@ -0,0 +1,201 @@
> +/* Measurement Specialties HTU21D humidity and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + *

Please drop the "You should have..." note and the FSF address. The license
is in the "COPYING" file, and the address may change.

> + * Data sheet available (7/2013) at
> + * http://www.meas-spec.com/downloads/HTU21D.pdf

The datasheet location is already mentioned in the Documentation. Please drop
it from here, so we don't have to update it at two places is the link changes.

> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +
> +/* HTU21 Commands */
> +#define HTU21_T_MEASUREMENT_HM  0xE3
> +#define HTU21_RH_MEASUREMENT_HM 0xE5
> +
Please use tabs before the 0x

> +struct htu21 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	char valid;

The above should be bool (and use '= true' for assignments).

> +	unsigned long last_update;
> +	int temperature;
> +	int humidity;
> +};
> +
> +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +static int htu21_update_measurements(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&htu21->lock);
> +
> +	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> +		htu21->last_update = jiffies;
> +		htu21->valid = 1;
> +	}
> +out:
> +	mutex_unlock(&htu21->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t htu21_show_temperature(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->temperature);
> +}
> +
> +static ssize_t htu21_show_humidity(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->humidity);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, htu21_show_temperature,	NULL, 0);

space instead of tab before the NULL

> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, htu21_show_humidity, NULL, 0);
> +
> +static struct attribute *htu21_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group htu21_group = {
> +	.attrs = htu21_attributes,
> +};
> +
> +static int htu21_probe(struct i2c_client *client,
> +	const struct i2c_device_id *id)
> +{
> +	struct htu21 *htu21;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WORD_DATA)) {

Strictly speaking, you only need I2C_FUNC_SMBUS_READ_WORD_DATA.

> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> +	if (!htu21)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, htu21);
> +
> +	mutex_init(&htu21->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	htu21->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(htu21->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(htu21->hwmon_dev);
> +		goto error;
> +	}
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +	return err;
> +}
> +
> +static int htu21_remove(struct i2c_client *client)
> +{
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(htu21->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id htu21_id[] = {
> +	{ "htu21", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
> +
> +static struct i2c_driver htu21_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "htu21",
> +	},
> +	.probe       = htu21_probe,
> +	.remove      = htu21_remove,
> +	.id_table    = htu21_id,
> +};
> +
> +module_i2c_driver(htu21_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("Measurement Specialties HTU21D humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
> -- 
> 1.7.10.4
> 
> 

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

* Re: [lm-sensors] [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
@ 2013-08-28 16:18   ` Guenter Roeck
  0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2013-08-28 16:18 UTC (permalink / raw)
  To: Markezana, William; +Cc: khali, lm-sensors, linux-kernel

On Wed, Aug 28, 2013 at 08:49:36AM +0200, Markezana, William wrote:
> From 55fc390d0079841405d5345b55a6e158ca5f3749 Mon Sep 17 00:00:00 2001
> From: William Markezana <william.markezana@meas-spec.com>
> Date: Wed, 28 Aug 2013 08:33:49 +0200
> Subject: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
> 

Your patch got corrupted and produces lots of checkpatch errors as result
(and can not be aplied). YOu might want to use a diffeent mailer for the next
version.

In addition to that, there are several other checkpatch errors and warnings.

ERROR: trailing statements should be on next line
WARNING: line over 80 characters
ERROR: Missing Signed-off-by: line(s)

Some more comments below.

Thanks,
Guenter

> ---
>  Documentation/hwmon/htu21 |   43 ++++++++++
>  drivers/hwmon/Kconfig     |   10 +++
>  drivers/hwmon/Makefile    |    1 +
>  drivers/hwmon/htu21.c     |  201 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 255 insertions(+)
>  create mode 100644 Documentation/hwmon/htu21
>  create mode 100644 drivers/hwmon/htu21.c
> 
> diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> new file mode 100644
> index 0000000..ab756bc
> --- /dev/null
> +++ b/Documentation/hwmon/htu21
> @@ -0,0 +1,43 @@
> +Kernel driver htu21
> +===================
> +
> +Supported chips:
> +  * Measurement Specialties HTU21D
> +    Prefix: 'htu21'
> +    Addresses scanned: none
> +    Datasheet: Publicly available at the Measurement Specialties website
> +    http://www.meas-spec.com/downloads/HTU21D.pdf
> +
> +
> +Author:
> +  William Markezana <william.markezana@meas-spec.com>
> +
> +Description
> +-----------
> +
> +HTU21D(F), the new digital humidity sensor with temperature output of MEAS is about 
> +to set new standards in terms of size and intelligence: Embedded in a reflow 
> +solderable Dual Flat No leads (DFN) package of 3 x 3 mm foot print and 0.9 mm height 
> +it provides calibrated, linearized signals in digital, I²C format.
> +
Please drop the marketing pitch. Just describe what the chip is doing.

> +The devices communicate with the I2C protocol. All sensors are set to the same
> +I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can be used
> +in the board setup code.
> +

I would suggest to add something like

"This driver does not auto-detect devices. You will have to instantiate the
devices explicitly. Please see Documentation/i2c/instantiating-devices for
details."

to the notes instead. After all, there are other means to instantiate the
driver.

> +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 11 ms for
> +humidity and 44 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.
> +
You might want to add that the driver is doing this, or phrase it differently,
such as

"To keep self heating below 0.1 degree Celsius, the device should not be
active for more than 10% of the time. For this reason, the driver performs no
more than two measurements per second and reports cached information if polled
more frequently."

> +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 e989f7f..eb46eb8 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -511,6 +511,16 @@ config SENSORS_HIH6130
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called hih6130.
>  
> +config SENSORS_HTU21
> +	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  HTU21D humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called htu21.
> +
>  config SENSORS_CORETEMP
>  	tristate "Intel Core/Core2/Atom temperature sensor"
>  	depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4f0fb52..ec7cde0 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
>  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
>  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
>  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
> +obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
>  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
>  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
> diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> new file mode 100644
> index 0000000..8b8d132
> --- /dev/null
> +++ b/drivers/hwmon/htu21.c
> @@ -0,0 +1,201 @@
> +/* Measurement Specialties HTU21D humidity and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + *

Please drop the "You should have..." note and the FSF address. The license
is in the "COPYING" file, and the address may change.

> + * Data sheet available (7/2013) at
> + * http://www.meas-spec.com/downloads/HTU21D.pdf

The datasheet location is already mentioned in the Documentation. Please drop
it from here, so we don't have to update it at two places is the link changes.

> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +
> +/* HTU21 Commands */
> +#define HTU21_T_MEASUREMENT_HM  0xE3
> +#define HTU21_RH_MEASUREMENT_HM 0xE5
> +
Please use tabs before the 0x

> +struct htu21 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	char valid;

The above should be bool (and use '= true' for assignments).

> +	unsigned long last_update;
> +	int temperature;
> +	int humidity;
> +};
> +
> +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +static int htu21_update_measurements(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&htu21->lock);
> +
> +	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> +		htu21->last_update = jiffies;
> +		htu21->valid = 1;
> +	}
> +out:
> +	mutex_unlock(&htu21->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t htu21_show_temperature(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->temperature);
> +}
> +
> +static ssize_t htu21_show_humidity(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->humidity);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, htu21_show_temperature,	NULL, 0);

space instead of tab before the NULL

> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, htu21_show_humidity, NULL, 0);
> +
> +static struct attribute *htu21_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group htu21_group = {
> +	.attrs = htu21_attributes,
> +};
> +
> +static int htu21_probe(struct i2c_client *client,
> +	const struct i2c_device_id *id)
> +{
> +	struct htu21 *htu21;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WORD_DATA)) {

Strictly speaking, you only need I2C_FUNC_SMBUS_READ_WORD_DATA.

> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> +	if (!htu21)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, htu21);
> +
> +	mutex_init(&htu21->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	htu21->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(htu21->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(htu21->hwmon_dev);
> +		goto error;
> +	}
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +	return err;
> +}
> +
> +static int htu21_remove(struct i2c_client *client)
> +{
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(htu21->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id htu21_id[] = {
> +	{ "htu21", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
> +
> +static struct i2c_driver htu21_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "htu21",
> +	},
> +	.probe       = htu21_probe,
> +	.remove      = htu21_remove,
> +	.id_table    = htu21_id,
> +};
> +
> +module_i2c_driver(htu21_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("Measurement Specialties HTU21D humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
> -- 
> 1.7.10.4
> 
> 

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* [lm-sensors] RE : [PATCH] hwmon: (htu2 =?iso-8859-1?q?1=29_Add_Measurem
@ 2013-08-29  8:06   ` Markezana, William
  2013-08-29  9:40       ` [lm-sensors] RE : [PATCH] hwmon: (htu Guenter Roeck
  0 siblings, 1 reply; 15+ messages in thread
From: Markezana, William @ 2013-08-29  8:06 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

--===============6590873921361201084==
Content-class: urn:content-classes:message
Content-Type: multipart/alternative;
	boundary="----_=_NextPart_001_01CEA48E.E1AB64DA"

This is a multi-part message in MIME format.

[-- Attachment #2: Type: text/plain, Size: 20940 bytes --]

diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
new file mode 100644
index 0000000..ddcef55
--- /dev/null
+++ b/Documentation/hwmon/htu21
@@ -0,0 +1,46 @@
+Kernel driver htu21
+===================
+
+Supported chips:
+  * Measurement Specialties HTU21D
+    Prefix: 'htu21'
+    Addresses scanned: none
+    Datasheet: Publicly available at the Measurement Specialties website
+    http://www.meas-spec.com/downloads/HTU21D.pdf
+
+
+Author:
+  William Markezana <william.markezana@meas-spec.com>
+
+Description
+-----------
+
+The HTU21D is a humidity and temperature sensor in a DFN package of
+only 3 x 3 mm footprint and 0.9 mm height.
+
+The devices communicate with the I2C protocol. All sensors are set to the same
+I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can be used
+in the board setup code.
+
+This driver does not auto-detect devices. You will have to instantiate the 
+devices explicitly. Please see Documentation/i2c/instantiating-devices 
+for details."
+
+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 11 ms for
+humidity and 44 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. For 
+this reason, the driver performs no more than two measurements per second and 
+reports cached information if polled more frequently.
+
+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 e989f7f..eb46eb8 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -511,6 +511,16 @@ config SENSORS_HIH6130
 	  This driver can also be built as a module.  If so, the module
 	  will be called hih6130.
 
+config SENSORS_HTU21
+	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Measurement Specialties
+	  HTU21D humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called htu21.
+
 config SENSORS_CORETEMP
 	tristate "Intel Core/Core2/Atom temperature sensor"
 	depends on X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4f0fb52..ec7cde0 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
 obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
 obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
 obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
+obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
 obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
 obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
 obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
new file mode 100644
index 0000000..d9470fb
--- /dev/null
+++ b/drivers/hwmon/htu21.c
@@ -0,0 +1,198 @@
+/*
+ * Measurement Specialties HTU21D humidity and temperature sensor driver
+ *
+ * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
+ *
+ */
+
+#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>
+#include <linux/jiffies.h>
+
+/* HTU21 Commands */
+#define HTU21_T_MEASUREMENT_HM	0xE3
+#define HTU21_RH_MEASUREMENT_HM	0xE5
+
+struct htu21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	bool valid;
+	unsigned long last_update;
+	int temperature;
+	int humidity;
+};
+
+static inline int htu21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/*
+	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
+	 * optimized for integer fixed point (3 digits) arithmetic
+	 */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/*
+	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
+	 * optimized for integer fixed point (3 digits) arithmetic
+	 */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+static int htu21_update_measurements(struct i2c_client *client)
+{
+	int ret = 0;
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+
+	mutex_lock(&htu21->lock);
+
+	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
+		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
+		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
+		htu21->last_update = jiffies;
+		htu21->valid = true;
+	}
+out:
+	mutex_unlock(&htu21->lock);
+
+	return ret >= 0 ? 0 : ret;
+}
+
+static ssize_t htu21_show_temperature(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+	int ret = htu21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", htu21->temperature);
+}
+
+static ssize_t htu21_show_humidity(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+	int ret = htu21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", htu21->humidity);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, 
+	htu21_show_temperature, NULL, 0);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, 
+	htu21_show_humidity, NULL, 0);
+
+static struct attribute *htu21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group htu21_group = {
+	.attrs = htu21_attributes,
+};
+
+static int htu21_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct htu21 *htu21;
+	int err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_WORD_DATA)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus word transactions\n");
+		return -ENODEV;
+	}
+
+	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
+	if (!htu21)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, htu21);
+
+	mutex_init(&htu21->lock);
+
+	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
+	if (err) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		return err;
+	}
+	htu21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(htu21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		err = PTR_ERR(htu21->hwmon_dev);
+		goto error;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+error:
+	sysfs_remove_group(&client->dev.kobj, &htu21_group);
+	return err;
+}
+
+static int htu21_remove(struct i2c_client *client)
+{
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(htu21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &htu21_group);
+
+	return 0;
+}
+
+static const struct i2c_device_id htu21_id[] = {
+	{ "htu21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, htu21_id);
+
+static struct i2c_driver htu21_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name	= "htu21",
+	},
+	.probe       = htu21_probe,
+	.remove      = htu21_remove,
+	.id_table    = htu21_id,
+};
+
+module_i2c_driver(htu21_driver);
+
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_DESCRIPTION("MEAS HTU21D humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");



-------- Message d'origine--------
De: Guenter Roeck de la part de Guenter Roeck
Date: mer. 28/08/2013 18:18
À: Markezana, William
Cc: khali@linux-fr.org; lm-sensors@lm-sensors.org; linux-kernel@vger.kernel.org
Objet : Re: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
 
On Wed, Aug 28, 2013 at 08:49:36AM +0200, Markezana, William wrote:
> From 55fc390d0079841405d5345b55a6e158ca5f3749 Mon Sep 17 00:00:00 2001
> From: William Markezana <william.markezana@meas-spec.com>
> Date: Wed, 28 Aug 2013 08:33:49 +0200
> Subject: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
> 

Your patch got corrupted and produces lots of checkpatch errors as result
(and can not be aplied). YOu might want to use a diffeent mailer for the next
version.

In addition to that, there are several other checkpatch errors and warnings.

ERROR: trailing statements should be on next line
WARNING: line over 80 characters
ERROR: Missing Signed-off-by: line(s)

Some more comments below.

Thanks,
Guenter

> ---
>  Documentation/hwmon/htu21 |   43 ++++++++++
>  drivers/hwmon/Kconfig     |   10 +++
>  drivers/hwmon/Makefile    |    1 +
>  drivers/hwmon/htu21.c     |  201 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 255 insertions(+)
>  create mode 100644 Documentation/hwmon/htu21
>  create mode 100644 drivers/hwmon/htu21.c
> 
> diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> new file mode 100644
> index 0000000..ab756bc
> --- /dev/null
> +++ b/Documentation/hwmon/htu21
> @@ -0,0 +1,43 @@
> +Kernel driver htu21
> +===================
> +
> +Supported chips:
> +  * Measurement Specialties HTU21D
> +    Prefix: 'htu21'
> +    Addresses scanned: none
> +    Datasheet: Publicly available at the Measurement Specialties website
> +    http://www.meas-spec.com/downloads/HTU21D.pdf
> +
> +
> +Author:
> +  William Markezana <william.markezana@meas-spec.com>
> +
> +Description
> +-----------
> +
> +HTU21D(F), the new digital humidity sensor with temperature output of MEAS is about 
> +to set new standards in terms of size and intelligence: Embedded in a reflow 
> +solderable Dual Flat No leads (DFN) package of 3 x 3 mm foot print and 0.9 mm height 
> +it provides calibrated, linearized signals in digital, I²C format.
> +
Please drop the marketing pitch. Just describe what the chip is doing.

> +The devices communicate with the I2C protocol. All sensors are set to the same
> +I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can be used
> +in the board setup code.
> +

I would suggest to add something like

"This driver does not auto-detect devices. You will have to instantiate the
devices explicitly. Please see Documentation/i2c/instantiating-devices for
details."

to the notes instead. After all, there are other means to instantiate the
driver.

> +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 11 ms for
> +humidity and 44 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.
> +
You might want to add that the driver is doing this, or phrase it differently,
such as

"To keep self heating below 0.1 degree Celsius, the device should not be
active for more than 10% of the time. For this reason, the driver performs no
more than two measurements per second and reports cached information if polled
more frequently."

> +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 e989f7f..eb46eb8 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -511,6 +511,16 @@ config SENSORS_HIH6130
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called hih6130.
>  
> +config SENSORS_HTU21
> +	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  HTU21D humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called htu21.
> +
>  config SENSORS_CORETEMP
>  	tristate "Intel Core/Core2/Atom temperature sensor"
>  	depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4f0fb52..ec7cde0 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
>  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
>  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
>  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
> +obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
>  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
>  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
> diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> new file mode 100644
> index 0000000..8b8d132
> --- /dev/null
> +++ b/drivers/hwmon/htu21.c
> @@ -0,0 +1,201 @@
> +/* Measurement Specialties HTU21D humidity and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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., 675 Mass Ave, Cambridge, MA 02139, USA.
> + *

Please drop the "You should have..." note and the FSF address. The license
is in the "COPYING" file, and the address may change.

> + * Data sheet available (7/2013) at
> + * http://www.meas-spec.com/downloads/HTU21D.pdf

The datasheet location is already mentioned in the Documentation. Please drop
it from here, so we don't have to update it at two places is the link changes.

> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +
> +/* HTU21 Commands */
> +#define HTU21_T_MEASUREMENT_HM  0xE3
> +#define HTU21_RH_MEASUREMENT_HM 0xE5
> +
Please use tabs before the 0x

> +struct htu21 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	char valid;

The above should be bool (and use '= true' for assignments).

> +	unsigned long last_update;
> +	int temperature;
> +	int humidity;
> +};
> +
> +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +static int htu21_update_measurements(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&htu21->lock);
> +
> +	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> +		htu21->last_update = jiffies;
> +		htu21->valid = 1;
> +	}
> +out:
> +	mutex_unlock(&htu21->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t htu21_show_temperature(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->temperature);
> +}
> +
> +static ssize_t htu21_show_humidity(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->humidity);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, htu21_show_temperature,	NULL, 0);

space instead of tab before the NULL

> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, htu21_show_humidity, NULL, 0);
> +
> +static struct attribute *htu21_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group htu21_group = {
> +	.attrs = htu21_attributes,
> +};
> +
> +static int htu21_probe(struct i2c_client *client,
> +	const struct i2c_device_id *id)
> +{
> +	struct htu21 *htu21;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WORD_DATA)) {

Strictly speaking, you only need I2C_FUNC_SMBUS_READ_WORD_DATA.

> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> +	if (!htu21)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, htu21);
> +
> +	mutex_init(&htu21->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	htu21->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(htu21->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(htu21->hwmon_dev);
> +		goto error;
> +	}
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +	return err;
> +}
> +
> +static int htu21_remove(struct i2c_client *client)
> +{
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(htu21->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id htu21_id[] = {
> +	{ "htu21", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
> +
> +static struct i2c_driver htu21_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "htu21",
> +	},
> +	.probe       = htu21_probe,
> +	.remove      = htu21_remove,
> +	.id_table    = htu21_id,
> +};
> +
> +module_i2c_driver(htu21_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("Measurement Specialties HTU21D humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
> -- 
> 1.7.10.4
> 
> 


[-- Attachment #3: Type: text/html, Size: 35952 bytes --]

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

* Re: RE : [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
  2013-08-29  8:06   ` [lm-sensors] RE : [PATCH] hwmon: (htu2 =?iso-8859-1?q?1=29_Add_Measurem Markezana, William
@ 2013-08-29  9:40       ` Guenter Roeck
  0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2013-08-29  9:40 UTC (permalink / raw)
  To: Markezana, William; +Cc: khali, lm-sensors, linux-kernel

On Thu, Aug 29, 2013 at 10:06:00AM +0200, Markezana, William wrote:

Please see Documentation/SubmittingPatches, section 15, "The canonical patch
format". Also, please run your patch through scripts/checkpatch.pl; we won't
accept it with checkpatch warnings or errors unless you provide a good reason
why the problem should be acceptable. It is also customary to include a list
of changes made compared to the previous version, as explained in section 15
of SubmittingPatches.

Thanks,
Guenter

> diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> new file mode 100644
> index 0000000..ddcef55
> --- /dev/null
> +++ b/Documentation/hwmon/htu21
> @@ -0,0 +1,46 @@
> +Kernel driver htu21
> +===================
> +
> +Supported chips:
> +  * Measurement Specialties HTU21D
> +    Prefix: 'htu21'
> +    Addresses scanned: none
> +    Datasheet: Publicly available at the Measurement Specialties website
> +    http://www.meas-spec.com/downloads/HTU21D.pdf
> +
> +
> +Author:
> +  William Markezana <william.markezana@meas-spec.com>
> +
> +Description
> +-----------
> +
> +The HTU21D is a humidity and temperature sensor in a DFN package of
> +only 3 x 3 mm footprint and 0.9 mm height.
> +
> +The devices communicate with the I2C protocol. All sensors are set to the same
> +I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can be used
> +in the board setup code.
> +
> +This driver does not auto-detect devices. You will have to instantiate the 
> +devices explicitly. Please see Documentation/i2c/instantiating-devices 
> +for details."
> +
> +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 11 ms for
> +humidity and 44 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. For 
> +this reason, the driver performs no more than two measurements per second and 
> +reports cached information if polled more frequently.
> +
> +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 e989f7f..eb46eb8 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -511,6 +511,16 @@ config SENSORS_HIH6130
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called hih6130.
>  
> +config SENSORS_HTU21
> +	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  HTU21D humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called htu21.
> +
>  config SENSORS_CORETEMP
>  	tristate "Intel Core/Core2/Atom temperature sensor"
>  	depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4f0fb52..ec7cde0 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
>  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
>  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
>  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
> +obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
>  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
>  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
> diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> new file mode 100644
> index 0000000..d9470fb
> --- /dev/null
> +++ b/drivers/hwmon/htu21.c
> @@ -0,0 +1,198 @@
> +/*
> + * Measurement Specialties HTU21D humidity and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
> + *
> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +
> +/* HTU21 Commands */
> +#define HTU21_T_MEASUREMENT_HM	0xE3
> +#define HTU21_RH_MEASUREMENT_HM	0xE5
> +
> +struct htu21 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	bool valid;
> +	unsigned long last_update;
> +	int temperature;
> +	int humidity;
> +};
> +
> +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +static int htu21_update_measurements(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&htu21->lock);
> +
> +	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> +		htu21->last_update = jiffies;
> +		htu21->valid = true;
> +	}
> +out:
> +	mutex_unlock(&htu21->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t htu21_show_temperature(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->temperature);
> +}
> +
> +static ssize_t htu21_show_humidity(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->humidity);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, 
> +	htu21_show_temperature, NULL, 0);
> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, 
> +	htu21_show_humidity, NULL, 0);
> +
> +static struct attribute *htu21_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group htu21_group = {
> +	.attrs = htu21_attributes,
> +};
> +
> +static int htu21_probe(struct i2c_client *client,
> +	const struct i2c_device_id *id)
> +{
> +	struct htu21 *htu21;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> +	if (!htu21)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, htu21);
> +
> +	mutex_init(&htu21->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	htu21->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(htu21->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(htu21->hwmon_dev);
> +		goto error;
> +	}
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +	return err;
> +}
> +
> +static int htu21_remove(struct i2c_client *client)
> +{
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(htu21->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id htu21_id[] = {
> +	{ "htu21", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
> +
> +static struct i2c_driver htu21_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "htu21",
> +	},
> +	.probe       = htu21_probe,
> +	.remove      = htu21_remove,
> +	.id_table    = htu21_id,
> +};
> +
> +module_i2c_driver(htu21_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("MEAS HTU21D humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
> 
> 
> 
> -------- Message d'origine--------
> De: Guenter Roeck de la part de Guenter Roeck
> Date: mer. 28/08/2013 18:18
> À: Markezana, William
> Cc: khali@linux-fr.org; lm-sensors@lm-sensors.org; linux-kernel@vger.kernel.org
> Objet : Re: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
>  
> On Wed, Aug 28, 2013 at 08:49:36AM +0200, Markezana, William wrote:
> > From 55fc390d0079841405d5345b55a6e158ca5f3749 Mon Sep 17 00:00:00 2001
> > From: William Markezana <william.markezana@meas-spec.com>
> > Date: Wed, 28 Aug 2013 08:33:49 +0200
> > Subject: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
> > 
> 
> Your patch got corrupted and produces lots of checkpatch errors as result
> (and can not be aplied). YOu might want to use a diffeent mailer for the next
> version.
> 
> In addition to that, there are several other checkpatch errors and warnings.
> 
> ERROR: trailing statements should be on next line
> WARNING: line over 80 characters
> ERROR: Missing Signed-off-by: line(s)
> 
> Some more comments below.
> 
> Thanks,
> Guenter
> 
> > ---
> >  Documentation/hwmon/htu21 |   43 ++++++++++
> >  drivers/hwmon/Kconfig     |   10 +++
> >  drivers/hwmon/Makefile    |    1 +
> >  drivers/hwmon/htu21.c     |  201 +++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 255 insertions(+)
> >  create mode 100644 Documentation/hwmon/htu21
> >  create mode 100644 drivers/hwmon/htu21.c
> > 
> > diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> > new file mode 100644
> > index 0000000..ab756bc
> > --- /dev/null
> > +++ b/Documentation/hwmon/htu21
> > @@ -0,0 +1,43 @@
> > +Kernel driver htu21
> > +===================
> > +
> > +Supported chips:
> > +  * Measurement Specialties HTU21D
> > +    Prefix: 'htu21'
> > +    Addresses scanned: none
> > +    Datasheet: Publicly available at the Measurement Specialties website
> > +    http://www.meas-spec.com/downloads/HTU21D.pdf
> > +
> > +
> > +Author:
> > +  William Markezana <william.markezana@meas-spec.com>
> > +
> > +Description
> > +-----------
> > +
> > +HTU21D(F), the new digital humidity sensor with temperature output of MEAS is about 
> > +to set new standards in terms of size and intelligence: Embedded in a reflow 
> > +solderable Dual Flat No leads (DFN) package of 3 x 3 mm foot print and 0.9 mm height 
> > +it provides calibrated, linearized signals in digital, I²C format.
> > +
> Please drop the marketing pitch. Just describe what the chip is doing.
> 
> > +The devices communicate with the I2C protocol. All sensors are set to the same
> > +I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can be used
> > +in the board setup code.
> > +
> 
> I would suggest to add something like
> 
> "This driver does not auto-detect devices. You will have to instantiate the
> devices explicitly. Please see Documentation/i2c/instantiating-devices for
> details."
> 
> to the notes instead. After all, there are other means to instantiate the
> driver.
> 
> > +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 11 ms for
> > +humidity and 44 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.
> > +
> You might want to add that the driver is doing this, or phrase it differently,
> such as
> 
> "To keep self heating below 0.1 degree Celsius, the device should not be
> active for more than 10% of the time. For this reason, the driver performs no
> more than two measurements per second and reports cached information if polled
> more frequently."
> 
> > +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 e989f7f..eb46eb8 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -511,6 +511,16 @@ config SENSORS_HIH6130
> >  	  This driver can also be built as a module.  If so, the module
> >  	  will be called hih6130.
> >  
> > +config SENSORS_HTU21
> > +	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
> > +	depends on I2C
> > +	help
> > +	  If you say yes here you get support for the Measurement Specialties
> > +	  HTU21D humidity and temperature sensors.
> > +
> > +	  This driver can also be built as a module.  If so, the module
> > +	  will be called htu21.
> > +
> >  config SENSORS_CORETEMP
> >  	tristate "Intel Core/Core2/Atom temperature sensor"
> >  	depends on X86
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index 4f0fb52..ec7cde0 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
> >  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
> >  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
> >  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
> > +obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
> >  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
> >  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
> >  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
> > diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> > new file mode 100644
> > index 0000000..8b8d132
> > --- /dev/null
> > +++ b/drivers/hwmon/htu21.c
> > @@ -0,0 +1,201 @@
> > +/* Measurement Specialties HTU21D humidity and temperature sensor driver
> > + *
> > + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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., 675 Mass Ave, Cambridge, MA 02139, USA.
> > + *
> 
> Please drop the "You should have..." note and the FSF address. The license
> is in the "COPYING" file, and the address may change.
> 
> > + * Data sheet available (7/2013) at
> > + * http://www.meas-spec.com/downloads/HTU21D.pdf
> 
> The datasheet location is already mentioned in the Documentation. Please drop
> it from here, so we don't have to update it at two places is the link changes.
> 
> > + */
> > +
> > +#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>
> > +#include <linux/jiffies.h>
> > +
> > +/* HTU21 Commands */
> > +#define HTU21_T_MEASUREMENT_HM  0xE3
> > +#define HTU21_RH_MEASUREMENT_HM 0xE5
> > +
> Please use tabs before the 0x
> 
> > +struct htu21 {
> > +	struct device *hwmon_dev;
> > +	struct mutex lock;
> > +	char valid;
> 
> The above should be bool (and use '= true' for assignments).
> 
> > +	unsigned long last_update;
> > +	int temperature;
> > +	int humidity;
> > +};
> > +
> > +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> > +{
> > +	ticks &= ~0x0003; /* clear status bits */
> > +	/*
> > +	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> > +	 * optimized for integer fixed point (3 digits) arithmetic
> > +	 */
> > +	return ((21965 * ticks) >> 13) - 46850;
> > +}
> > +
> > +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> > +{
> > +	ticks &= ~0x0003; /* clear status bits */
> > +	/*
> > +	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> > +	 * optimized for integer fixed point (3 digits) arithmetic
> > +	 */
> > +	return ((15625 * ticks) >> 13) - 6000;
> > +}
> > +
> > +static int htu21_update_measurements(struct i2c_client *client)
> > +{
> > +	int ret = 0;
> > +	struct htu21 *htu21 = i2c_get_clientdata(client);
> > +
> > +	mutex_lock(&htu21->lock);
> > +
> > +	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
> > +		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
> > +		if (ret < 0)
> > +			goto out;
> > +		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> > +		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
> > +		if (ret < 0)
> > +			goto out;
> > +		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> > +		htu21->last_update = jiffies;
> > +		htu21->valid = 1;
> > +	}
> > +out:
> > +	mutex_unlock(&htu21->lock);
> > +
> > +	return ret >= 0 ? 0 : ret;
> > +}
> > +
> > +static ssize_t htu21_show_temperature(struct device *dev,
> > +	struct device_attribute *attr,
> > +	char *buf)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct htu21 *htu21 = i2c_get_clientdata(client);
> > +	int ret = htu21_update_measurements(client);
> > +	if (ret < 0)
> > +		return ret;
> > +	return sprintf(buf, "%d\n", htu21->temperature);
> > +}
> > +
> > +static ssize_t htu21_show_humidity(struct device *dev,
> > +	struct device_attribute *attr,
> > +	char *buf)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct htu21 *htu21 = i2c_get_clientdata(client);
> > +	int ret = htu21_update_measurements(client);
> > +	if (ret < 0)
> > +		return ret;
> > +	return sprintf(buf, "%d\n", htu21->humidity);
> > +}
> > +
> > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, htu21_show_temperature,	NULL, 0);
> 
> space instead of tab before the NULL
> 
> > +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, htu21_show_humidity, NULL, 0);
> > +
> > +static struct attribute *htu21_attributes[] = {
> > +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> > +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group htu21_group = {
> > +	.attrs = htu21_attributes,
> > +};
> > +
> > +static int htu21_probe(struct i2c_client *client,
> > +	const struct i2c_device_id *id)
> > +{
> > +	struct htu21 *htu21;
> > +	int err;
> > +
> > +	if (!i2c_check_functionality(client->adapter,
> > +				     I2C_FUNC_SMBUS_WORD_DATA)) {
> 
> Strictly speaking, you only need I2C_FUNC_SMBUS_READ_WORD_DATA.
> 
> > +		dev_err(&client->dev,
> > +			"adapter does not support SMBus word transactions\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> > +	if (!htu21)
> > +		return -ENOMEM;
> > +
> > +	i2c_set_clientdata(client, htu21);
> > +
> > +	mutex_init(&htu21->lock);
> > +
> > +	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> > +	if (err) {
> > +		dev_dbg(&client->dev, "could not create sysfs files\n");
> > +		return err;
> > +	}
> > +	htu21->hwmon_dev = hwmon_device_register(&client->dev);
> > +	if (IS_ERR(htu21->hwmon_dev)) {
> > +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> > +		err = PTR_ERR(htu21->hwmon_dev);
> > +		goto error;
> > +	}
> > +
> > +	dev_info(&client->dev, "initialized\n");
> > +
> > +	return 0;
> > +
> > +error:
> > +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> > +	return err;
> > +}
> > +
> > +static int htu21_remove(struct i2c_client *client)
> > +{
> > +	struct htu21 *htu21 = i2c_get_clientdata(client);
> > +
> > +	hwmon_device_unregister(htu21->hwmon_dev);
> > +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct i2c_device_id htu21_id[] = {
> > +	{ "htu21", 0 },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, htu21_id);
> > +
> > +static struct i2c_driver htu21_driver = {
> > +	.class = I2C_CLASS_HWMON,
> > +	.driver = {
> > +		.name	= "htu21",
> > +	},
> > +	.probe       = htu21_probe,
> > +	.remove      = htu21_remove,
> > +	.id_table    = htu21_id,
> > +};
> > +
> > +module_i2c_driver(htu21_driver);
> > +
> > +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> > +MODULE_DESCRIPTION("Measurement Specialties HTU21D humidity and temperature sensor driver");
> > +MODULE_LICENSE("GPL");
> > -- 
> > 1.7.10.4
> > 
> > 
> 

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

* Re: [lm-sensors] RE : [PATCH] hwmon:  (htu
@ 2013-08-29  9:40       ` Guenter Roeck
  0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2013-08-29  9:40 UTC (permalink / raw)
  To: Markezana, William; +Cc: khali, lm-sensors, linux-kernel

On Thu, Aug 29, 2013 at 10:06:00AM +0200, Markezana, William wrote:

Please see Documentation/SubmittingPatches, section 15, "The canonical patch
format". Also, please run your patch through scripts/checkpatch.pl; we won't
accept it with checkpatch warnings or errors unless you provide a good reason
why the problem should be acceptable. It is also customary to include a list
of changes made compared to the previous version, as explained in section 15
of SubmittingPatches.

Thanks,
Guenter

> diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> new file mode 100644
> index 0000000..ddcef55
> --- /dev/null
> +++ b/Documentation/hwmon/htu21
> @@ -0,0 +1,46 @@
> +Kernel driver htu21
> +===================
> +
> +Supported chips:
> +  * Measurement Specialties HTU21D
> +    Prefix: 'htu21'
> +    Addresses scanned: none
> +    Datasheet: Publicly available at the Measurement Specialties website
> +    http://www.meas-spec.com/downloads/HTU21D.pdf
> +
> +
> +Author:
> +  William Markezana <william.markezana@meas-spec.com>
> +
> +Description
> +-----------
> +
> +The HTU21D is a humidity and temperature sensor in a DFN package of
> +only 3 x 3 mm footprint and 0.9 mm height.
> +
> +The devices communicate with the I2C protocol. All sensors are set to the same
> +I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can be used
> +in the board setup code.
> +
> +This driver does not auto-detect devices. You will have to instantiate the 
> +devices explicitly. Please see Documentation/i2c/instantiating-devices 
> +for details."
> +
> +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 11 ms for
> +humidity and 44 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. For 
> +this reason, the driver performs no more than two measurements per second and 
> +reports cached information if polled more frequently.
> +
> +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 e989f7f..eb46eb8 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -511,6 +511,16 @@ config SENSORS_HIH6130
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called hih6130.
>  
> +config SENSORS_HTU21
> +	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  HTU21D humidity and temperature sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called htu21.
> +
>  config SENSORS_CORETEMP
>  	tristate "Intel Core/Core2/Atom temperature sensor"
>  	depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4f0fb52..ec7cde0 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
>  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
>  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
>  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
> +obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
>  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
>  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
>  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
> diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> new file mode 100644
> index 0000000..d9470fb
> --- /dev/null
> +++ b/drivers/hwmon/htu21.c
> @@ -0,0 +1,198 @@
> +/*
> + * Measurement Specialties HTU21D humidity and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
> + *
> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +
> +/* HTU21 Commands */
> +#define HTU21_T_MEASUREMENT_HM	0xE3
> +#define HTU21_RH_MEASUREMENT_HM	0xE5
> +
> +struct htu21 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	bool valid;
> +	unsigned long last_update;
> +	int temperature;
> +	int humidity;
> +};
> +
> +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +	ticks &= ~0x0003; /* clear status bits */
> +	/*
> +	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> +	 * optimized for integer fixed point (3 digits) arithmetic
> +	 */
> +	return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +static int htu21_update_measurements(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&htu21->lock);
> +
> +	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> +		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
> +		if (ret < 0)
> +			goto out;
> +		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> +		htu21->last_update = jiffies;
> +		htu21->valid = true;
> +	}
> +out:
> +	mutex_unlock(&htu21->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t htu21_show_temperature(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->temperature);
> +}
> +
> +static ssize_t htu21_show_humidity(struct device *dev,
> +	struct device_attribute *attr,
> +	char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +	int ret = htu21_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", htu21->humidity);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, 
> +	htu21_show_temperature, NULL, 0);
> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, 
> +	htu21_show_humidity, NULL, 0);
> +
> +static struct attribute *htu21_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group htu21_group = {
> +	.attrs = htu21_attributes,
> +};
> +
> +static int htu21_probe(struct i2c_client *client,
> +	const struct i2c_device_id *id)
> +{
> +	struct htu21 *htu21;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> +	if (!htu21)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, htu21);
> +
> +	mutex_init(&htu21->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	htu21->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(htu21->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(htu21->hwmon_dev);
> +		goto error;
> +	}
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +	return err;
> +}
> +
> +static int htu21_remove(struct i2c_client *client)
> +{
> +	struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(htu21->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id htu21_id[] = {
> +	{ "htu21", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
> +
> +static struct i2c_driver htu21_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "htu21",
> +	},
> +	.probe       = htu21_probe,
> +	.remove      = htu21_remove,
> +	.id_table    = htu21_id,
> +};
> +
> +module_i2c_driver(htu21_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("MEAS HTU21D humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
> 
> 
> 
> -------- Message d'origine--------
> De: Guenter Roeck de la part de Guenter Roeck
> Date: mer. 28/08/2013 18:18
> À: Markezana, William
> Cc: khali@linux-fr.org; lm-sensors@lm-sensors.org; linux-kernel@vger.kernel.org
> Objet : Re: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
>  
> On Wed, Aug 28, 2013 at 08:49:36AM +0200, Markezana, William wrote:
> > From 55fc390d0079841405d5345b55a6e158ca5f3749 Mon Sep 17 00:00:00 2001
> > From: William Markezana <william.markezana@meas-spec.com>
> > Date: Wed, 28 Aug 2013 08:33:49 +0200
> > Subject: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
> > 
> 
> Your patch got corrupted and produces lots of checkpatch errors as result
> (and can not be aplied). YOu might want to use a diffeent mailer for the next
> version.
> 
> In addition to that, there are several other checkpatch errors and warnings.
> 
> ERROR: trailing statements should be on next line
> WARNING: line over 80 characters
> ERROR: Missing Signed-off-by: line(s)
> 
> Some more comments below.
> 
> Thanks,
> Guenter
> 
> > ---
> >  Documentation/hwmon/htu21 |   43 ++++++++++
> >  drivers/hwmon/Kconfig     |   10 +++
> >  drivers/hwmon/Makefile    |    1 +
> >  drivers/hwmon/htu21.c     |  201 +++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 255 insertions(+)
> >  create mode 100644 Documentation/hwmon/htu21
> >  create mode 100644 drivers/hwmon/htu21.c
> > 
> > diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> > new file mode 100644
> > index 0000000..ab756bc
> > --- /dev/null
> > +++ b/Documentation/hwmon/htu21
> > @@ -0,0 +1,43 @@
> > +Kernel driver htu21
> > +===================
> > +
> > +Supported chips:
> > +  * Measurement Specialties HTU21D
> > +    Prefix: 'htu21'
> > +    Addresses scanned: none
> > +    Datasheet: Publicly available at the Measurement Specialties website
> > +    http://www.meas-spec.com/downloads/HTU21D.pdf
> > +
> > +
> > +Author:
> > +  William Markezana <william.markezana@meas-spec.com>
> > +
> > +Description
> > +-----------
> > +
> > +HTU21D(F), the new digital humidity sensor with temperature output of MEAS is about 
> > +to set new standards in terms of size and intelligence: Embedded in a reflow 
> > +solderable Dual Flat No leads (DFN) package of 3 x 3 mm foot print and 0.9 mm height 
> > +it provides calibrated, linearized signals in digital, I²C format.
> > +
> Please drop the marketing pitch. Just describe what the chip is doing.
> 
> > +The devices communicate with the I2C protocol. All sensors are set to the same
> > +I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can be used
> > +in the board setup code.
> > +
> 
> I would suggest to add something like
> 
> "This driver does not auto-detect devices. You will have to instantiate the
> devices explicitly. Please see Documentation/i2c/instantiating-devices for
> details."
> 
> to the notes instead. After all, there are other means to instantiate the
> driver.
> 
> > +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 11 ms for
> > +humidity and 44 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.
> > +
> You might want to add that the driver is doing this, or phrase it differently,
> such as
> 
> "To keep self heating below 0.1 degree Celsius, the device should not be
> active for more than 10% of the time. For this reason, the driver performs no
> more than two measurements per second and reports cached information if polled
> more frequently."
> 
> > +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 e989f7f..eb46eb8 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -511,6 +511,16 @@ config SENSORS_HIH6130
> >  	  This driver can also be built as a module.  If so, the module
> >  	  will be called hih6130.
> >  
> > +config SENSORS_HTU21
> > +	tristate "Measurement Specialties HTU21D humidity and temperature sensors"
> > +	depends on I2C
> > +	help
> > +	  If you say yes here you get support for the Measurement Specialties
> > +	  HTU21D humidity and temperature sensors.
> > +
> > +	  This driver can also be built as a module.  If so, the module
> > +	  will be called htu21.
> > +
> >  config SENSORS_CORETEMP
> >  	tristate "Intel Core/Core2/Atom temperature sensor"
> >  	depends on X86
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index 4f0fb52..ec7cde0 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
> >  obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
> >  obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
> >  obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
> > +obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
> >  obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
> >  obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
> >  obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
> > diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> > new file mode 100644
> > index 0000000..8b8d132
> > --- /dev/null
> > +++ b/drivers/hwmon/htu21.c
> > @@ -0,0 +1,201 @@
> > +/* Measurement Specialties HTU21D humidity and temperature sensor driver
> > + *
> > + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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., 675 Mass Ave, Cambridge, MA 02139, USA.
> > + *
> 
> Please drop the "You should have..." note and the FSF address. The license
> is in the "COPYING" file, and the address may change.
> 
> > + * Data sheet available (7/2013) at
> > + * http://www.meas-spec.com/downloads/HTU21D.pdf
> 
> The datasheet location is already mentioned in the Documentation. Please drop
> it from here, so we don't have to update it at two places is the link changes.
> 
> > + */
> > +
> > +#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>
> > +#include <linux/jiffies.h>
> > +
> > +/* HTU21 Commands */
> > +#define HTU21_T_MEASUREMENT_HM  0xE3
> > +#define HTU21_RH_MEASUREMENT_HM 0xE5
> > +
> Please use tabs before the 0x
> 
> > +struct htu21 {
> > +	struct device *hwmon_dev;
> > +	struct mutex lock;
> > +	char valid;
> 
> The above should be bool (and use '= true' for assignments).
> 
> > +	unsigned long last_update;
> > +	int temperature;
> > +	int humidity;
> > +};
> > +
> > +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> > +{
> > +	ticks &= ~0x0003; /* clear status bits */
> > +	/*
> > +	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> > +	 * optimized for integer fixed point (3 digits) arithmetic
> > +	 */
> > +	return ((21965 * ticks) >> 13) - 46850;
> > +}
> > +
> > +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> > +{
> > +	ticks &= ~0x0003; /* clear status bits */
> > +	/*
> > +	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> > +	 * optimized for integer fixed point (3 digits) arithmetic
> > +	 */
> > +	return ((15625 * ticks) >> 13) - 6000;
> > +}
> > +
> > +static int htu21_update_measurements(struct i2c_client *client)
> > +{
> > +	int ret = 0;
> > +	struct htu21 *htu21 = i2c_get_clientdata(client);
> > +
> > +	mutex_lock(&htu21->lock);
> > +
> > +	if (time_after(jiffies, htu21->last_update + HZ / 2) || !htu21->valid) {
> > +		ret = i2c_smbus_read_word_swapped(client, HTU21_T_MEASUREMENT_HM);
> > +		if (ret < 0)
> > +			goto out;
> > +		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> > +		ret = i2c_smbus_read_word_swapped(client, HTU21_RH_MEASUREMENT_HM);
> > +		if (ret < 0)
> > +			goto out;
> > +		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> > +		htu21->last_update = jiffies;
> > +		htu21->valid = 1;
> > +	}
> > +out:
> > +	mutex_unlock(&htu21->lock);
> > +
> > +	return ret >= 0 ? 0 : ret;
> > +}
> > +
> > +static ssize_t htu21_show_temperature(struct device *dev,
> > +	struct device_attribute *attr,
> > +	char *buf)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct htu21 *htu21 = i2c_get_clientdata(client);
> > +	int ret = htu21_update_measurements(client);
> > +	if (ret < 0)
> > +		return ret;
> > +	return sprintf(buf, "%d\n", htu21->temperature);
> > +}
> > +
> > +static ssize_t htu21_show_humidity(struct device *dev,
> > +	struct device_attribute *attr,
> > +	char *buf)
> > +{
> > +	struct i2c_client *client = to_i2c_client(dev);
> > +	struct htu21 *htu21 = i2c_get_clientdata(client);
> > +	int ret = htu21_update_measurements(client);
> > +	if (ret < 0)
> > +		return ret;
> > +	return sprintf(buf, "%d\n", htu21->humidity);
> > +}
> > +
> > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, htu21_show_temperature,	NULL, 0);
> 
> space instead of tab before the NULL
> 
> > +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO, htu21_show_humidity, NULL, 0);
> > +
> > +static struct attribute *htu21_attributes[] = {
> > +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> > +	&sensor_dev_attr_humidity1_input.dev_attr.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group htu21_group = {
> > +	.attrs = htu21_attributes,
> > +};
> > +
> > +static int htu21_probe(struct i2c_client *client,
> > +	const struct i2c_device_id *id)
> > +{
> > +	struct htu21 *htu21;
> > +	int err;
> > +
> > +	if (!i2c_check_functionality(client->adapter,
> > +				     I2C_FUNC_SMBUS_WORD_DATA)) {
> 
> Strictly speaking, you only need I2C_FUNC_SMBUS_READ_WORD_DATA.
> 
> > +		dev_err(&client->dev,
> > +			"adapter does not support SMBus word transactions\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> > +	if (!htu21)
> > +		return -ENOMEM;
> > +
> > +	i2c_set_clientdata(client, htu21);
> > +
> > +	mutex_init(&htu21->lock);
> > +
> > +	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> > +	if (err) {
> > +		dev_dbg(&client->dev, "could not create sysfs files\n");
> > +		return err;
> > +	}
> > +	htu21->hwmon_dev = hwmon_device_register(&client->dev);
> > +	if (IS_ERR(htu21->hwmon_dev)) {
> > +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> > +		err = PTR_ERR(htu21->hwmon_dev);
> > +		goto error;
> > +	}
> > +
> > +	dev_info(&client->dev, "initialized\n");
> > +
> > +	return 0;
> > +
> > +error:
> > +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> > +	return err;
> > +}
> > +
> > +static int htu21_remove(struct i2c_client *client)
> > +{
> > +	struct htu21 *htu21 = i2c_get_clientdata(client);
> > +
> > +	hwmon_device_unregister(htu21->hwmon_dev);
> > +	sysfs_remove_group(&client->dev.kobj, &htu21_group);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct i2c_device_id htu21_id[] = {
> > +	{ "htu21", 0 },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(i2c, htu21_id);
> > +
> > +static struct i2c_driver htu21_driver = {
> > +	.class = I2C_CLASS_HWMON,
> > +	.driver = {
> > +		.name	= "htu21",
> > +	},
> > +	.probe       = htu21_probe,
> > +	.remove      = htu21_remove,
> > +	.id_table    = htu21_id,
> > +};
> > +
> > +module_i2c_driver(htu21_driver);
> > +
> > +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> > +MODULE_DESCRIPTION("Measurement Specialties HTU21D humidity and temperature sensor driver");
> > +MODULE_LICENSE("GPL");
> > -- 
> > 1.7.10.4
> > 
> > 
> 

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* [lm-sensors] [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
  2013-08-28  6:49 ` [lm-sensors] " Markezana, William
  (?)
  (?)
@ 2013-08-29 11:51 ` Markezana, William
  2013-08-30  3:35     ` [lm-sensors] " Guenter Roeck
  2013-09-10 15:09   ` [lm-sensors] [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support Markezana, William
  -1 siblings, 2 replies; 15+ messages in thread
From: Markezana, William @ 2013-08-29 11:51 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

--===============4845491198832804118==
Content-class: urn:content-classes:message
Content-Type: multipart/alternative;
	boundary="----_=_NextPart_001_01CEA4AE.21175D65"

This is a multi-part message in MIME format.

[-- Attachment #2: Type: text/plain, Size: 8920 bytes --]

From: William Markezana <william.markezana@meas-spec.com>

hwmon: (htu21) Add Measurement Specialties HTU21D support
Signed-off-by: William Markezana <william.markezana@meas-spec.com>
---
diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
new file mode 100644
index 0000000..8082879
--- /dev/null
+++ b/Documentation/hwmon/htu21
@@ -0,0 +1,46 @@
+Kernel driver htu21
+===================
+
+Supported chips:
+  * Measurement Specialties HTU21D
+    Prefix: 'htu21'
+    Addresses scanned: none
+    Datasheet: Publicly available at the Measurement Specialties website
+    http://www.meas-spec.com/downloads/HTU21D.pdf
+
+
+Author:
+  William Markezana <william.markezana@meas-spec.com>
+
+Description
+-----------
+
+The HTU21D is a humidity and temperature sensor in a DFN package of
+only 3 x 3 mm footprint and 0.9 mm height.
+
+The devices communicate with the I2C protocol. All sensors are set to the
+same I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can
+be used in the board setup code.
+
+This driver does not auto-detect devices. You will have to instantiate the
+devices explicitly. Please see Documentation/i2c/instantiating-devices
+for details."
+
+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 11 ms for
+humidity and 44 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. For
+this reason, the driver performs no more than two measurements per second and
+reports cached information if polled more frequently.
+
+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 e989f7f..55973cd 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -511,6 +511,16 @@ config SENSORS_HIH6130
 	  This driver can also be built as a module.  If so, the module
 	  will be called hih6130.
 
+config SENSORS_HTU21
+	tristate "Measurement Specialties HTU21D humidity/temperature sensors"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Measurement Specialties
+	  HTU21D humidity and temperature sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called htu21.
+
 config SENSORS_CORETEMP
 	tristate "Intel Core/Core2/Atom temperature sensor"
 	depends on X86
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 4f0fb52..ec7cde0 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM)	+= gl518sm.o
 obj-$(CONFIG_SENSORS_GL520SM)	+= gl520sm.o
 obj-$(CONFIG_SENSORS_GPIO_FAN)	+= gpio-fan.o
 obj-$(CONFIG_SENSORS_HIH6130)	+= hih6130.o
+obj-$(CONFIG_SENSORS_HTU21)	+= htu21.o
 obj-$(CONFIG_SENSORS_ULTRA45)	+= ultra45_env.o
 obj-$(CONFIG_SENSORS_I5K_AMB)	+= i5k_amb.o
 obj-$(CONFIG_SENSORS_IBMAEM)	+= ibmaem.o
diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
new file mode 100644
index 0000000..743bf32
--- /dev/null
+++ b/drivers/hwmon/htu21.c
@@ -0,0 +1,201 @@
+/*
+ * Measurement Specialties HTU21D humidity and temperature sensor driver
+ *
+ * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
+ *
+ */
+
+#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>
+#include <linux/jiffies.h>
+
+/* HTU21 Commands */
+#define HTU21_T_MEASUREMENT_HM	0xE3
+#define HTU21_RH_MEASUREMENT_HM	0xE5
+
+struct htu21 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	bool valid;
+	unsigned long last_update;
+	int temperature;
+	int humidity;
+};
+
+static inline int htu21_temp_ticks_to_millicelsius(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/*
+	 * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
+	 * optimized for integer fixed point (3 digits) arithmetic
+	 */
+	return ((21965 * ticks) >> 13) - 46850;
+}
+
+static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
+{
+	ticks &= ~0x0003; /* clear status bits */
+	/*
+	 * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
+	 * optimized for integer fixed point (3 digits) arithmetic
+	 */
+	return ((15625 * ticks) >> 13) - 6000;
+}
+
+static int htu21_update_measurements(struct i2c_client *client)
+{
+	int ret = 0;
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+
+	mutex_lock(&htu21->lock);
+
+	if (time_after(jiffies, htu21->last_update + HZ / 2) ||
+	 !htu21->valid) {
+		ret = i2c_smbus_read_word_swapped(client,
+			HTU21_T_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
+		ret = i2c_smbus_read_word_swapped(client,
+			HTU21_RH_MEASUREMENT_HM);
+		if (ret < 0)
+			goto out;
+		htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
+		htu21->last_update = jiffies;
+		htu21->valid = true;
+	}
+out:
+	mutex_unlock(&htu21->lock);
+
+	return ret >= 0 ? 0 : ret;
+}
+
+static ssize_t htu21_show_temperature(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+	int ret = htu21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", htu21->temperature);
+}
+
+static ssize_t htu21_show_humidity(struct device *dev,
+	struct device_attribute *attr,
+	char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+	int ret = htu21_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", htu21->humidity);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
+	htu21_show_temperature, NULL, 0);
+static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO,
+	htu21_show_humidity, NULL, 0);
+
+static struct attribute *htu21_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_humidity1_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group htu21_group = {
+	.attrs = htu21_attributes,
+};
+
+static int htu21_probe(struct i2c_client *client,
+	const struct i2c_device_id *id)
+{
+	struct htu21 *htu21;
+	int err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_WORD_DATA)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus word transactions\n");
+		return -ENODEV;
+	}
+
+	htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
+	if (!htu21)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, htu21);
+
+	mutex_init(&htu21->lock);
+
+	err = sysfs_create_group(&client->dev.kobj, &htu21_group);
+	if (err) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		return err;
+	}
+	htu21->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(htu21->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		err = PTR_ERR(htu21->hwmon_dev);
+		goto error;
+	}
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+error:
+	sysfs_remove_group(&client->dev.kobj, &htu21_group);
+	return err;
+}
+
+static int htu21_remove(struct i2c_client *client)
+{
+	struct htu21 *htu21 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(htu21->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &htu21_group);
+
+	return 0;
+}
+
+static const struct i2c_device_id htu21_id[] = {
+	{ "htu21", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, htu21_id);
+
+static struct i2c_driver htu21_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name	= "htu21",
+	},
+	.probe       = htu21_probe,
+	.remove      = htu21_remove,
+	.id_table    = htu21_id,
+};
+
+module_i2c_driver(htu21_driver);
+
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_DESCRIPTION("MEAS HTU21D humidity and temperature sensor driver");
+MODULE_LICENSE("GPL");

[-- Attachment #3: Type: text/html, Size: 16803 bytes --]

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

* Re: [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
  2013-08-29 11:51 ` [lm-sensors] [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support Markezana, William
@ 2013-08-30  3:35     ` Guenter Roeck
  2013-09-10 15:09   ` [lm-sensors] [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support Markezana, William
  1 sibling, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2013-08-30  3:35 UTC (permalink / raw)
  To: Markezana, William; +Cc: khali, lm-sensors, linux-kernel

On 08/29/2013 04:51 AM, Markezana, William wrote:
> From: William Markezana <william.markezana@meas-spec.com>
>
> hwmon: (htu21) Add Measurement Specialties HTU21D support
> Signed-off-by: William Markezana <william.markezana@meas-spec.com>
> ---

Applied [ with minor formatting changes ] to -next.

Thanks,
Guenter

> diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> new file mode 100644
> index 0000000..8082879
> --- /dev/null
> +++ b/Documentation/hwmon/htu21
> @@ -0,0 +1,46 @@
> +Kernel driver htu21
> +===================
> +
> +Supported chips:
> +  * Measurement Specialties HTU21D
> +    Prefix: 'htu21'
> +    Addresses scanned: none
> +    Datasheet: Publicly available at the Measurement Specialties website
> + http://www.meas-spec.com/downloads/HTU21D.pdf
> +
> +
> +Author:
> +  William Markezana <william.markezana@meas-spec.com>
> +
> +Description
> +-----------
> +
> +The HTU21D is a humidity and temperature sensor in a DFN package of
> +only 3 x 3 mm footprint and 0.9 mm height.
> +
> +The devices communicate with the I2C protocol. All sensors are set to the
> +same I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can
> +be used in the board setup code.
> +
> +This driver does not auto-detect devices. You will have to instantiate the
> +devices explicitly. Please see Documentation/i2c/instantiating-devices
> +for details."
> +
> +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 11 ms for
> +humidity and 44 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. For
> +this reason, the driver performs no more than two measurements per second and
> +reports cached information if polled more frequently.
> +
> +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 e989f7f..55973cd 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -511,6 +511,16 @@ config SENSORS_HIH6130
>            This driver can also be built as a module.  If so, the module
>            will be called hih6130.
>
> +config SENSORS_HTU21
> +       tristate "Measurement Specialties HTU21D humidity/temperature sensors"
> +       depends on I2C
> +       help
> +         If you say yes here you get support for the Measurement Specialties
> +         HTU21D humidity and temperature sensors.
> +
> +         This driver can also be built as a module.  If so, the module
> +         will be called htu21.
> +
>   config SENSORS_CORETEMP
>          tristate "Intel Core/Core2/Atom temperature sensor"
>          depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4f0fb52..ec7cde0 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
>   obj-$(CONFIG_SENSORS_GL520SM)  += gl520sm.o
>   obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
>   obj-$(CONFIG_SENSORS_HIH6130)  += hih6130.o
> +obj-$(CONFIG_SENSORS_HTU21)    += htu21.o
>   obj-$(CONFIG_SENSORS_ULTRA45)  += ultra45_env.o
>   obj-$(CONFIG_SENSORS_I5K_AMB)  += i5k_amb.o
>   obj-$(CONFIG_SENSORS_IBMAEM)   += ibmaem.o
> diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> new file mode 100644
> index 0000000..743bf32
> --- /dev/null
> +++ b/drivers/hwmon/htu21.c
> @@ -0,0 +1,201 @@
> +/*
> + * Measurement Specialties HTU21D humidity and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
> + *
> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +
> +/* HTU21 Commands */
> +#define HTU21_T_MEASUREMENT_HM 0xE3
> +#define HTU21_RH_MEASUREMENT_HM        0xE5
> +
> +struct htu21 {
> +       struct device *hwmon_dev;
> +       struct mutex lock;
> +       bool valid;
> +       unsigned long last_update;
> +       int temperature;
> +       int humidity;
> +};
> +
> +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> +{
> +       ticks &= ~0x0003; /* clear status bits */
> +       /*
> +        * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> +        * optimized for integer fixed point (3 digits) arithmetic
> +        */
> +       return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +       ticks &= ~0x0003; /* clear status bits */
> +       /*
> +        * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> +        * optimized for integer fixed point (3 digits) arithmetic
> +        */
> +       return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +static int htu21_update_measurements(struct i2c_client *client)
> +{
> +       int ret = 0;
> +       struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +       mutex_lock(&htu21->lock);
> +
> +       if (time_after(jiffies, htu21->last_update + HZ / 2) ||
> +        !htu21->valid) {
> +               ret = i2c_smbus_read_word_swapped(client,
> +                       HTU21_T_MEASUREMENT_HM);
> +               if (ret < 0)
> +                       goto out;
> +               htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> +               ret = i2c_smbus_read_word_swapped(client,
> +                       HTU21_RH_MEASUREMENT_HM);
> +               if (ret < 0)
> +                       goto out;
> +               htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> +               htu21->last_update = jiffies;
> +               htu21->valid = true;
> +       }
> +out:
> +       mutex_unlock(&htu21->lock);
> +
> +       return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t htu21_show_temperature(struct device *dev,
> +       struct device_attribute *attr,
> +       char *buf)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +       struct htu21 *htu21 = i2c_get_clientdata(client);
> +       int ret = htu21_update_measurements(client);
> +       if (ret < 0)
> +               return ret;
> +       return sprintf(buf, "%d\n", htu21->temperature);
> +}
> +
> +static ssize_t htu21_show_humidity(struct device *dev,
> +       struct device_attribute *attr,
> +       char *buf)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +       struct htu21 *htu21 = i2c_get_clientdata(client);
> +       int ret = htu21_update_measurements(client);
> +       if (ret < 0)
> +               return ret;
> +       return sprintf(buf, "%d\n", htu21->humidity);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
> +       htu21_show_temperature, NULL, 0);
> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO,
> +       htu21_show_humidity, NULL, 0);
> +
> +static struct attribute *htu21_attributes[] = {
> +       &sensor_dev_attr_temp1_input.dev_attr.attr,
> +       &sensor_dev_attr_humidity1_input.dev_attr.attr,
> +       NULL
> +};
> +
> +static const struct attribute_group htu21_group = {
> +       .attrs = htu21_attributes,
> +};
> +
> +static int htu21_probe(struct i2c_client *client,
> +       const struct i2c_device_id *id)
> +{
> +       struct htu21 *htu21;
> +       int err;
> +
> +       if (!i2c_check_functionality(client->adapter,
> +                                    I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +               dev_err(&client->dev,
> +                       "adapter does not support SMBus word transactions\n");
> +               return -ENODEV;
> +       }
> +
> +       htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> +       if (!htu21)
> +               return -ENOMEM;
> +
> +       i2c_set_clientdata(client, htu21);
> +
> +       mutex_init(&htu21->lock);
> +
> +       err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> +       if (err) {
> +               dev_dbg(&client->dev, "could not create sysfs files\n");
> +               return err;
> +       }
> +       htu21->hwmon_dev = hwmon_device_register(&client->dev);
> +       if (IS_ERR(htu21->hwmon_dev)) {
> +               dev_dbg(&client->dev, "unable to register hwmon device\n");
> +               err = PTR_ERR(htu21->hwmon_dev);
> +               goto error;
> +       }
> +
> +       dev_info(&client->dev, "initialized\n");
> +
> +       return 0;
> +
> +error:
> +       sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +       return err;
> +}
> +
> +static int htu21_remove(struct i2c_client *client)
> +{
> +       struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +       hwmon_device_unregister(htu21->hwmon_dev);
> +       sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +
> +       return 0;
> +}
> +
> +static const struct i2c_device_id htu21_id[] = {
> +       { "htu21", 0 },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
> +
> +static struct i2c_driver htu21_driver = {
> +       .class = I2C_CLASS_HWMON,
> +       .driver = {
> +               .name   = "htu21",
> +       },
> +       .probe       = htu21_probe,
> +       .remove      = htu21_remove,
> +       .id_table    = htu21_id,
> +};
> +
> +module_i2c_driver(htu21_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("MEAS HTU21D humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
>


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

* Re: [lm-sensors] [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support
@ 2013-08-30  3:35     ` Guenter Roeck
  0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2013-08-30  3:35 UTC (permalink / raw)
  To: Markezana, William; +Cc: khali, lm-sensors, linux-kernel

On 08/29/2013 04:51 AM, Markezana, William wrote:
> From: William Markezana <william.markezana@meas-spec.com>
>
> hwmon: (htu21) Add Measurement Specialties HTU21D support
> Signed-off-by: William Markezana <william.markezana@meas-spec.com>
> ---

Applied [ with minor formatting changes ] to -next.

Thanks,
Guenter

> diff --git a/Documentation/hwmon/htu21 b/Documentation/hwmon/htu21
> new file mode 100644
> index 0000000..8082879
> --- /dev/null
> +++ b/Documentation/hwmon/htu21
> @@ -0,0 +1,46 @@
> +Kernel driver htu21
> +=========> +
> +Supported chips:
> +  * Measurement Specialties HTU21D
> +    Prefix: 'htu21'
> +    Addresses scanned: none
> +    Datasheet: Publicly available at the Measurement Specialties website
> + http://www.meas-spec.com/downloads/HTU21D.pdf
> +
> +
> +Author:
> +  William Markezana <william.markezana@meas-spec.com>
> +
> +Description
> +-----------
> +
> +The HTU21D is a humidity and temperature sensor in a DFN package of
> +only 3 x 3 mm footprint and 0.9 mm height.
> +
> +The devices communicate with the I2C protocol. All sensors are set to the
> +same I2C address 0x40, so an entry with I2C_BOARD_INFO("htu21", 0x40) can
> +be used in the board setup code.
> +
> +This driver does not auto-detect devices. You will have to instantiate the
> +devices explicitly. Please see Documentation/i2c/instantiating-devices
> +for details."
> +
> +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 11 ms for
> +humidity and 44 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. For
> +this reason, the driver performs no more than two measurements per second and
> +reports cached information if polled more frequently.
> +
> +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 e989f7f..55973cd 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -511,6 +511,16 @@ config SENSORS_HIH6130
>            This driver can also be built as a module.  If so, the module
>            will be called hih6130.
>
> +config SENSORS_HTU21
> +       tristate "Measurement Specialties HTU21D humidity/temperature sensors"
> +       depends on I2C
> +       help
> +         If you say yes here you get support for the Measurement Specialties
> +         HTU21D humidity and temperature sensors.
> +
> +         This driver can also be built as a module.  If so, the module
> +         will be called htu21.
> +
>   config SENSORS_CORETEMP
>          tristate "Intel Core/Core2/Atom temperature sensor"
>          depends on X86
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 4f0fb52..ec7cde0 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -65,6 +65,7 @@ obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
>   obj-$(CONFIG_SENSORS_GL520SM)  += gl520sm.o
>   obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
>   obj-$(CONFIG_SENSORS_HIH6130)  += hih6130.o
> +obj-$(CONFIG_SENSORS_HTU21)    += htu21.o
>   obj-$(CONFIG_SENSORS_ULTRA45)  += ultra45_env.o
>   obj-$(CONFIG_SENSORS_I5K_AMB)  += i5k_amb.o
>   obj-$(CONFIG_SENSORS_IBMAEM)   += ibmaem.o
> diff --git a/drivers/hwmon/htu21.c b/drivers/hwmon/htu21.c
> new file mode 100644
> index 0000000..743bf32
> --- /dev/null
> +++ b/drivers/hwmon/htu21.c
> @@ -0,0 +1,201 @@
> +/*
> + * Measurement Specialties HTU21D humidity and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
> + *
> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +
> +/* HTU21 Commands */
> +#define HTU21_T_MEASUREMENT_HM 0xE3
> +#define HTU21_RH_MEASUREMENT_HM        0xE5
> +
> +struct htu21 {
> +       struct device *hwmon_dev;
> +       struct mutex lock;
> +       bool valid;
> +       unsigned long last_update;
> +       int temperature;
> +       int humidity;
> +};
> +
> +static inline int htu21_temp_ticks_to_millicelsius(int ticks)
> +{
> +       ticks &= ~0x0003; /* clear status bits */
> +       /*
> +        * Formula T = -46.85 + 175.72 * ST / 2^16 from datasheet p14,
> +        * optimized for integer fixed point (3 digits) arithmetic
> +        */
> +       return ((21965 * ticks) >> 13) - 46850;
> +}
> +
> +static inline int htu21_rh_ticks_to_per_cent_mille(int ticks)
> +{
> +       ticks &= ~0x0003; /* clear status bits */
> +       /*
> +        * Formula RH = -6 + 125 * SRH / 2^16 from datasheet p14,
> +        * optimized for integer fixed point (3 digits) arithmetic
> +        */
> +       return ((15625 * ticks) >> 13) - 6000;
> +}
> +
> +static int htu21_update_measurements(struct i2c_client *client)
> +{
> +       int ret = 0;
> +       struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +       mutex_lock(&htu21->lock);
> +
> +       if (time_after(jiffies, htu21->last_update + HZ / 2) ||
> +        !htu21->valid) {
> +               ret = i2c_smbus_read_word_swapped(client,
> +                       HTU21_T_MEASUREMENT_HM);
> +               if (ret < 0)
> +                       goto out;
> +               htu21->temperature = htu21_temp_ticks_to_millicelsius(ret);
> +               ret = i2c_smbus_read_word_swapped(client,
> +                       HTU21_RH_MEASUREMENT_HM);
> +               if (ret < 0)
> +                       goto out;
> +               htu21->humidity = htu21_rh_ticks_to_per_cent_mille(ret);
> +               htu21->last_update = jiffies;
> +               htu21->valid = true;
> +       }
> +out:
> +       mutex_unlock(&htu21->lock);
> +
> +       return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t htu21_show_temperature(struct device *dev,
> +       struct device_attribute *attr,
> +       char *buf)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +       struct htu21 *htu21 = i2c_get_clientdata(client);
> +       int ret = htu21_update_measurements(client);
> +       if (ret < 0)
> +               return ret;
> +       return sprintf(buf, "%d\n", htu21->temperature);
> +}
> +
> +static ssize_t htu21_show_humidity(struct device *dev,
> +       struct device_attribute *attr,
> +       char *buf)
> +{
> +       struct i2c_client *client = to_i2c_client(dev);
> +       struct htu21 *htu21 = i2c_get_clientdata(client);
> +       int ret = htu21_update_measurements(client);
> +       if (ret < 0)
> +               return ret;
> +       return sprintf(buf, "%d\n", htu21->humidity);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
> +       htu21_show_temperature, NULL, 0);
> +static SENSOR_DEVICE_ATTR(humidity1_input, S_IRUGO,
> +       htu21_show_humidity, NULL, 0);
> +
> +static struct attribute *htu21_attributes[] = {
> +       &sensor_dev_attr_temp1_input.dev_attr.attr,
> +       &sensor_dev_attr_humidity1_input.dev_attr.attr,
> +       NULL
> +};
> +
> +static const struct attribute_group htu21_group = {
> +       .attrs = htu21_attributes,
> +};
> +
> +static int htu21_probe(struct i2c_client *client,
> +       const struct i2c_device_id *id)
> +{
> +       struct htu21 *htu21;
> +       int err;
> +
> +       if (!i2c_check_functionality(client->adapter,
> +                                    I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +               dev_err(&client->dev,
> +                       "adapter does not support SMBus word transactions\n");
> +               return -ENODEV;
> +       }
> +
> +       htu21 = devm_kzalloc(&client->dev, sizeof(*htu21), GFP_KERNEL);
> +       if (!htu21)
> +               return -ENOMEM;
> +
> +       i2c_set_clientdata(client, htu21);
> +
> +       mutex_init(&htu21->lock);
> +
> +       err = sysfs_create_group(&client->dev.kobj, &htu21_group);
> +       if (err) {
> +               dev_dbg(&client->dev, "could not create sysfs files\n");
> +               return err;
> +       }
> +       htu21->hwmon_dev = hwmon_device_register(&client->dev);
> +       if (IS_ERR(htu21->hwmon_dev)) {
> +               dev_dbg(&client->dev, "unable to register hwmon device\n");
> +               err = PTR_ERR(htu21->hwmon_dev);
> +               goto error;
> +       }
> +
> +       dev_info(&client->dev, "initialized\n");
> +
> +       return 0;
> +
> +error:
> +       sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +       return err;
> +}
> +
> +static int htu21_remove(struct i2c_client *client)
> +{
> +       struct htu21 *htu21 = i2c_get_clientdata(client);
> +
> +       hwmon_device_unregister(htu21->hwmon_dev);
> +       sysfs_remove_group(&client->dev.kobj, &htu21_group);
> +
> +       return 0;
> +}
> +
> +static const struct i2c_device_id htu21_id[] = {
> +       { "htu21", 0 },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(i2c, htu21_id);
> +
> +static struct i2c_driver htu21_driver = {
> +       .class = I2C_CLASS_HWMON,
> +       .driver = {
> +               .name   = "htu21",
> +       },
> +       .probe       = htu21_probe,
> +       .remove      = htu21_remove,
> +       .id_table    = htu21_id,
> +};
> +
> +module_i2c_driver(htu21_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("MEAS HTU21D humidity and temperature sensor driver");
> +MODULE_LICENSE("GPL");
>


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* [lm-sensors] [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support
@ 2013-09-10 15:09   ` Markezana, William
  2013-09-10 15:20       ` [lm-sensors] " Guenter Roeck
  0 siblings, 1 reply; 15+ messages in thread
From: Markezana, William @ 2013-09-10 15:09 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 215 bytes --]

--===============1342085540040239079==
Content-class: urn:content-classes:message
Content-Type: multipart/alternative;
	boundary="----_=_NextPart_001_01CEAE37.D0171FEE"

This is a multi-part message in MIME format.

[-- Attachment #2: Type: text/plain, Size: 9831 bytes --]

From: William Markezana <william.markezana@meas-spec.com>

hwmon: (ms5637) Add Measurement Specialties MS5637support
Signed-off-by: William Markezana <william.markezana@meas-spec.com>
---
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 55973cd..c4f1c8e 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -954,6 +954,16 @@ config SENSORS_MCP3021
 	  This driver can also be built as a module.  If so, the module
 	  will be called mcp3021.
 
+config SENSORS_MS5637
+	tristate "Measurement Specialties MS5637 pressure sensors"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Measurement Specialties
+	  MS5637 pressure sensors.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called ms5637.
+
 config SENSORS_NCT6775
 	tristate "Nuvoton NCT6775F and compatibles"
 	depends on !PPC
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index ec7cde0..5d8f699 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -110,6 +110,7 @@ obj-$(CONFIG_SENSORS_MAX6650)	+= max6650.o
 obj-$(CONFIG_SENSORS_MAX6697)	+= max6697.o
 obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
 obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
+obj-$(CONFIG_SENSORS_MS5637)	+= ms5637.o
 obj-$(CONFIG_SENSORS_NCT6775)	+= nct6775.o
 obj-$(CONFIG_SENSORS_NTC_THERMISTOR)	+= ntc_thermistor.o
 obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
diff --git a/drivers/hwmon/ms5637.c b/drivers/hwmon/ms5637.c
new file mode 100644
index 0000000..fe2c2df
--- /dev/null
+++ b/drivers/hwmon/ms5637.c
@@ -0,0 +1,302 @@
+/*
+ * Measurement Specialties MS5637 pressure and temperature sensor driver
+ *
+ * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
+ *
+ */
+
+#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>
+#include <linux/jiffies.h>
+#include <linux/delay.h>
+
+/* MS5637 Commands */
+#define MS5637_CONVERT_D1_OSR_4096	(0x48)
+#define MS5637_CONVERT_D2_OSR_4096	(0x58)
+#define MS5637_ADC_READ				(0x00)
+#define MS5637_PROM_READ			(0xA0)
+
+struct ms5637 {
+	struct device *hwmon_dev;
+	struct mutex lock;
+	bool valid;
+	unsigned long last_update;
+	int temperature;
+	int pressure;
+	unsigned short calibration_words[6];
+	bool got_calibration_words;
+};
+
+static int ms5637_get_calibration_word(struct i2c_client *client,
+	unsigned char address, unsigned short *word)
+{
+	int ret = 0;
+	ret = i2c_smbus_read_word_swapped(client,
+		MS5637_PROM_READ + (address << 1));
+	if (ret < 0)
+		return ret;
+	*word = (unsigned short)ret & 0xFFFF;
+	return 0;
+}
+
+static int ms5637_fill_calibration_words(struct i2c_client *client)
+{
+	int i, ret = 0;
+	struct ms5637 *ms5637 = i2c_get_clientdata(client);
+
+	for (i = 0; i < 6; i++) {
+		ret = ms5637_get_calibration_word(client, i+1,
+			&ms5637->calibration_words[i]);
+		if (ret < 0) {
+			dev_err(&client->dev,
+				"unable to get calibration word at address %d\n",
+				i+1);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+static int ms5637_get_adc_value(struct i2c_client *client,
+	unsigned int *adc_value)
+{
+	int ret = 0;
+	unsigned char buf[3];
+	ret = i2c_smbus_read_i2c_block_data(client, MS5637_ADC_READ, 3, buf);
+	if (ret < 0)
+		return ret;
+	*adc_value = (buf[0] << 16) + (buf[1] << 8) + buf[2];
+	return 0;
+}
+
+static int ms5637_get_conversion_result(struct i2c_client *client,
+	unsigned char command, unsigned int *adc_value)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte(client, command);
+	if (ret < 0)
+		return ret;
+	msleep(9);
+	ret = ms5637_get_adc_value(client, adc_value);
+	if (ret < 0)
+		return ret;
+	return 0;
+}
+
+static int ms5637_update_measurements(struct i2c_client *client)
+{
+	int ret = 0;
+	unsigned int d1, d2;
+	int dt, temp, p;
+	long long int off, sens;
+	struct ms5637 *ms5637 = i2c_get_clientdata(client);
+
+	mutex_lock(&ms5637->lock);
+
+	if (time_after(jiffies, ms5637->last_update + HZ / 2) ||
+	    !ms5637->valid) {
+
+		if (!ms5637->got_calibration_words)	{
+			ret = ms5637_fill_calibration_words(client);
+			if (ret < 0) {
+				dev_err(&client->dev,
+					"unable to get calibration words\n");
+				goto out;
+			}
+
+			ms5637->got_calibration_words = true;
+		}
+
+		ret = ms5637_get_conversion_result(client,
+			MS5637_CONVERT_D1_OSR_4096, &d1);
+		if (ret < 0) {
+			dev_err(&client->dev,
+				"unable to get adc conversion of D1_OSR_4096\n");
+			goto out;
+		}
+
+		ret = ms5637_get_conversion_result(client,
+			MS5637_CONVERT_D2_OSR_4096, &d2);
+		if (ret < 0) {
+			dev_err(&client->dev,
+				"unable to get adc conversion of D2_OSR_4096\n");
+			goto out;
+		}
+
+		dt = d2 - (int)ms5637->calibration_words[4] * 256;
+		temp = 20000 + div_s64((long long int)dt *
+			(long long int)ms5637->calibration_words[5], 838861);
+
+		off = (long long int)ms5637->calibration_words[1] * 131072 +
+			div_s64((long long int)ms5637->calibration_words[3] *
+			(long long int)dt, 64);
+		sens = (long long int)ms5637->calibration_words[0] * 65536 +
+			div_s64((long long int)ms5637->calibration_words[2] *
+			(long long int)dt, 128);
+		p = (int)div_s64((div_s64((long long int)d1 * sens, 2097152) -
+			off), 3277);
+
+		ms5637->temperature = temp;
+		ms5637->pressure = p;
+		ms5637->last_update = jiffies;
+		ms5637->valid = true;
+	}
+out:
+	mutex_unlock(&ms5637->lock);
+
+	return ret >= 0 ? 0 : ret;
+}
+
+static ssize_t ms5637_show_temperature(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ms5637 *ms5637 = i2c_get_clientdata(client);
+	int ret = ms5637_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", ms5637->temperature);
+}
+
+static ssize_t ms5637_show_pressure(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct ms5637 *ms5637 = i2c_get_clientdata(client);
+	int ret = ms5637_update_measurements(client);
+	if (ret < 0)
+		return ret;
+	return sprintf(buf, "%d\n", ms5637->pressure);
+}
+
+static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
+			  ms5637_show_temperature, NULL, 0);
+static SENSOR_DEVICE_ATTR(pressure1_input, S_IRUGO,
+			  ms5637_show_pressure, NULL, 0);
+
+static struct attribute *ms5637_attributes[] = {
+	&sensor_dev_attr_temp1_input.dev_attr.attr,
+	&sensor_dev_attr_pressure1_input.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group ms5637_group = {
+	.attrs = ms5637_attributes,
+};
+
+static int ms5637_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
+{
+	struct ms5637 *ms5637;
+	int err;
+
+	if (!i2c_check_functionality(client->adapter,
+					I2C_FUNC_SMBUS_READ_WORD_DATA)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus word transactions\n");
+		return -ENODEV;
+	}
+
+	if (!i2c_check_functionality(client->adapter,
+				    I2C_FUNC_SMBUS_WRITE_BYTE)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus byte transactions\n");
+		return -ENODEV;
+	}
+
+	if (!i2c_check_functionality(client->adapter,
+				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
+		dev_err(&client->dev,
+			"adapter does not support SMBus block transactions\n");
+		return -ENODEV;
+	}
+
+	ms5637 = devm_kzalloc(&client->dev, sizeof(*ms5637), GFP_KERNEL);
+	if (!ms5637)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, ms5637);
+
+	mutex_init(&ms5637->lock);
+
+	err = sysfs_create_group(&client->dev.kobj, &ms5637_group);
+	if (err) {
+		dev_dbg(&client->dev, "could not create sysfs files\n");
+		return err;
+	}
+	ms5637->hwmon_dev = hwmon_device_register(&client->dev);
+	if (IS_ERR(ms5637->hwmon_dev)) {
+		dev_dbg(&client->dev, "unable to register hwmon device\n");
+		err = PTR_ERR(ms5637->hwmon_dev);
+		goto error;
+	}
+
+	mutex_lock(&ms5637->lock);
+	err = ms5637_fill_calibration_words(client);
+	if (err >= 0)
+		ms5637->got_calibration_words = true;
+	mutex_unlock(&ms5637->lock);
+
+	dev_info(&client->dev, "initialized\n");
+
+	return 0;
+
+error:
+	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
+	return err;
+}
+
+static int ms5637_remove(struct i2c_client *client)
+{
+	struct ms5637 *ms5637 = i2c_get_clientdata(client);
+
+	hwmon_device_unregister(ms5637->hwmon_dev);
+	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
+
+	return 0;
+}
+
+static const struct i2c_device_id ms5637_id[] = {
+	{ "ms5637", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, ms5637_id);
+
+static struct i2c_driver ms5637_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name	= "ms5637",
+	},
+	.probe       = ms5637_probe,
+	.remove      = ms5637_remove,
+	.id_table    = ms5637_id,
+};
+
+module_i2c_driver(ms5637_driver);
+
+MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
+MODULE_DESCRIPTION("MEAS MS5637 pressure and temperature sensor driver");
+MODULE_LICENSE("GPL");

[-- Attachment #3: Type: text/html, Size: 25009 bytes --]

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

* Re: [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support
  2013-09-10 15:09   ` [lm-sensors] [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support Markezana, William
@ 2013-09-10 15:20       ` Guenter Roeck
  0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2013-09-10 15:20 UTC (permalink / raw)
  To: Markezana, William; +Cc: khali, lm-sensors, linux-kernel

On Tue, Sep 10, 2013 at 05:09:57PM +0200, Markezana, William wrote:
> From: William Markezana <william.markezana@meas-spec.com>
> 
> hwmon: (ms5637) Add Measurement Specialties MS5637support
> Signed-off-by: William Markezana <william.markezana@meas-spec.com>

Hi William,

"pressure" is hardly a hardware monitoring attribute. It might make more sense
to add your driver to the iio subsystem, which already supports at least one
other pressure sensor.

Thanks,
Guenter

> ---
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 55973cd..c4f1c8e 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -954,6 +954,16 @@ config SENSORS_MCP3021
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called mcp3021.
>  
> +config SENSORS_MS5637
> +	tristate "Measurement Specialties MS5637 pressure sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  MS5637 pressure sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ms5637.
> +
>  config SENSORS_NCT6775
>  	tristate "Nuvoton NCT6775F and compatibles"
>  	depends on !PPC
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index ec7cde0..5d8f699 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -110,6 +110,7 @@ obj-$(CONFIG_SENSORS_MAX6650)	+= max6650.o
>  obj-$(CONFIG_SENSORS_MAX6697)	+= max6697.o
>  obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
>  obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
> +obj-$(CONFIG_SENSORS_MS5637)	+= ms5637.o
>  obj-$(CONFIG_SENSORS_NCT6775)	+= nct6775.o
>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)	+= ntc_thermistor.o
>  obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
> diff --git a/drivers/hwmon/ms5637.c b/drivers/hwmon/ms5637.c
> new file mode 100644
> index 0000000..fe2c2df
> --- /dev/null
> +++ b/drivers/hwmon/ms5637.c
> @@ -0,0 +1,302 @@
> +/*
> + * Measurement Specialties MS5637 pressure and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
> + *
> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +#include <linux/delay.h>
> +
> +/* MS5637 Commands */
> +#define MS5637_CONVERT_D1_OSR_4096	(0x48)
> +#define MS5637_CONVERT_D2_OSR_4096	(0x58)
> +#define MS5637_ADC_READ				(0x00)
> +#define MS5637_PROM_READ			(0xA0)
> +
> +struct ms5637 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	bool valid;
> +	unsigned long last_update;
> +	int temperature;
> +	int pressure;
> +	unsigned short calibration_words[6];
> +	bool got_calibration_words;
> +};
> +
> +static int ms5637_get_calibration_word(struct i2c_client *client,
> +	unsigned char address, unsigned short *word)
> +{
> +	int ret = 0;
> +	ret = i2c_smbus_read_word_swapped(client,
> +		MS5637_PROM_READ + (address << 1));
> +	if (ret < 0)
> +		return ret;
> +	*word = (unsigned short)ret & 0xFFFF;
> +	return 0;
> +}
> +
> +static int ms5637_fill_calibration_words(struct i2c_client *client)
> +{
> +	int i, ret = 0;
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	for (i = 0; i < 6; i++) {
> +		ret = ms5637_get_calibration_word(client, i+1,
> +			&ms5637->calibration_words[i]);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get calibration word at address %d\n",
> +				i+1);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int ms5637_get_adc_value(struct i2c_client *client,
> +	unsigned int *adc_value)
> +{
> +	int ret = 0;
> +	unsigned char buf[3];
> +	ret = i2c_smbus_read_i2c_block_data(client, MS5637_ADC_READ, 3, buf);
> +	if (ret < 0)
> +		return ret;
> +	*adc_value = (buf[0] << 16) + (buf[1] << 8) + buf[2];
> +	return 0;
> +}
> +
> +static int ms5637_get_conversion_result(struct i2c_client *client,
> +	unsigned char command, unsigned int *adc_value)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte(client, command);
> +	if (ret < 0)
> +		return ret;
> +	msleep(9);
> +	ret = ms5637_get_adc_value(client, adc_value);
> +	if (ret < 0)
> +		return ret;
> +	return 0;
> +}
> +
> +static int ms5637_update_measurements(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	unsigned int d1, d2;
> +	int dt, temp, p;
> +	long long int off, sens;
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&ms5637->lock);
> +
> +	if (time_after(jiffies, ms5637->last_update + HZ / 2) ||
> +	    !ms5637->valid) {
> +
> +		if (!ms5637->got_calibration_words)	{
> +			ret = ms5637_fill_calibration_words(client);
> +			if (ret < 0) {
> +				dev_err(&client->dev,
> +					"unable to get calibration words\n");
> +				goto out;
> +			}
> +
> +			ms5637->got_calibration_words = true;
> +		}
> +
> +		ret = ms5637_get_conversion_result(client,
> +			MS5637_CONVERT_D1_OSR_4096, &d1);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get adc conversion of D1_OSR_4096\n");
> +			goto out;
> +		}
> +
> +		ret = ms5637_get_conversion_result(client,
> +			MS5637_CONVERT_D2_OSR_4096, &d2);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get adc conversion of D2_OSR_4096\n");
> +			goto out;
> +		}
> +
> +		dt = d2 - (int)ms5637->calibration_words[4] * 256;
> +		temp = 20000 + div_s64((long long int)dt *
> +			(long long int)ms5637->calibration_words[5], 838861);
> +
> +		off = (long long int)ms5637->calibration_words[1] * 131072 +
> +			div_s64((long long int)ms5637->calibration_words[3] *
> +			(long long int)dt, 64);
> +		sens = (long long int)ms5637->calibration_words[0] * 65536 +
> +			div_s64((long long int)ms5637->calibration_words[2] *
> +			(long long int)dt, 128);
> +		p = (int)div_s64((div_s64((long long int)d1 * sens, 2097152) -
> +			off), 3277);
> +
> +		ms5637->temperature = temp;
> +		ms5637->pressure = p;
> +		ms5637->last_update = jiffies;
> +		ms5637->valid = true;
> +	}
> +out:
> +	mutex_unlock(&ms5637->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t ms5637_show_temperature(struct device *dev,
> +				      struct device_attribute *attr, char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +	int ret = ms5637_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", ms5637->temperature);
> +}
> +
> +static ssize_t ms5637_show_pressure(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +	int ret = ms5637_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", ms5637->pressure);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
> +			  ms5637_show_temperature, NULL, 0);
> +static SENSOR_DEVICE_ATTR(pressure1_input, S_IRUGO,
> +			  ms5637_show_pressure, NULL, 0);
> +
> +static struct attribute *ms5637_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_pressure1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group ms5637_group = {
> +	.attrs = ms5637_attributes,
> +};
> +
> +static int ms5637_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	struct ms5637 *ms5637;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +					I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_WRITE_BYTE)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus byte transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus block transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	ms5637 = devm_kzalloc(&client->dev, sizeof(*ms5637), GFP_KERNEL);
> +	if (!ms5637)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, ms5637);
> +
> +	mutex_init(&ms5637->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &ms5637_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	ms5637->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(ms5637->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(ms5637->hwmon_dev);
> +		goto error;
> +	}
> +
> +	mutex_lock(&ms5637->lock);
> +	err = ms5637_fill_calibration_words(client);
> +	if (err >= 0)
> +		ms5637->got_calibration_words = true;
> +	mutex_unlock(&ms5637->lock);
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
> +	return err;
> +}
> +
> +static int ms5637_remove(struct i2c_client *client)
> +{
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(ms5637->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id ms5637_id[] = {
> +	{ "ms5637", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ms5637_id);
> +
> +static struct i2c_driver ms5637_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "ms5637",
> +	},
> +	.probe       = ms5637_probe,
> +	.remove      = ms5637_remove,
> +	.id_table    = ms5637_id,
> +};
> +
> +module_i2c_driver(ms5637_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("MEAS MS5637 pressure and temperature sensor driver");
> +MODULE_LICENSE("GPL");

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

* Re: [lm-sensors] [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support
@ 2013-09-10 15:20       ` Guenter Roeck
  0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2013-09-10 15:20 UTC (permalink / raw)
  To: Markezana, William; +Cc: khali, lm-sensors, linux-kernel

On Tue, Sep 10, 2013 at 05:09:57PM +0200, Markezana, William wrote:
> From: William Markezana <william.markezana@meas-spec.com>
> 
> hwmon: (ms5637) Add Measurement Specialties MS5637support
> Signed-off-by: William Markezana <william.markezana@meas-spec.com>

Hi William,

"pressure" is hardly a hardware monitoring attribute. It might make more sense
to add your driver to the iio subsystem, which already supports at least one
other pressure sensor.

Thanks,
Guenter

> ---
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 55973cd..c4f1c8e 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -954,6 +954,16 @@ config SENSORS_MCP3021
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called mcp3021.
>  
> +config SENSORS_MS5637
> +	tristate "Measurement Specialties MS5637 pressure sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  MS5637 pressure sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ms5637.
> +
>  config SENSORS_NCT6775
>  	tristate "Nuvoton NCT6775F and compatibles"
>  	depends on !PPC
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index ec7cde0..5d8f699 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -110,6 +110,7 @@ obj-$(CONFIG_SENSORS_MAX6650)	+= max6650.o
>  obj-$(CONFIG_SENSORS_MAX6697)	+= max6697.o
>  obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
>  obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
> +obj-$(CONFIG_SENSORS_MS5637)	+= ms5637.o
>  obj-$(CONFIG_SENSORS_NCT6775)	+= nct6775.o
>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)	+= ntc_thermistor.o
>  obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
> diff --git a/drivers/hwmon/ms5637.c b/drivers/hwmon/ms5637.c
> new file mode 100644
> index 0000000..fe2c2df
> --- /dev/null
> +++ b/drivers/hwmon/ms5637.c
> @@ -0,0 +1,302 @@
> +/*
> + * Measurement Specialties MS5637 pressure and temperature sensor driver
> + *
> + * Copyright (C) 2013 William Markezana <william.markezana@meas-spec.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.
> + *
> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +#include <linux/delay.h>
> +
> +/* MS5637 Commands */
> +#define MS5637_CONVERT_D1_OSR_4096	(0x48)
> +#define MS5637_CONVERT_D2_OSR_4096	(0x58)
> +#define MS5637_ADC_READ				(0x00)
> +#define MS5637_PROM_READ			(0xA0)
> +
> +struct ms5637 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	bool valid;
> +	unsigned long last_update;
> +	int temperature;
> +	int pressure;
> +	unsigned short calibration_words[6];
> +	bool got_calibration_words;
> +};
> +
> +static int ms5637_get_calibration_word(struct i2c_client *client,
> +	unsigned char address, unsigned short *word)
> +{
> +	int ret = 0;
> +	ret = i2c_smbus_read_word_swapped(client,
> +		MS5637_PROM_READ + (address << 1));
> +	if (ret < 0)
> +		return ret;
> +	*word = (unsigned short)ret & 0xFFFF;
> +	return 0;
> +}
> +
> +static int ms5637_fill_calibration_words(struct i2c_client *client)
> +{
> +	int i, ret = 0;
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	for (i = 0; i < 6; i++) {
> +		ret = ms5637_get_calibration_word(client, i+1,
> +			&ms5637->calibration_words[i]);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get calibration word at address %d\n",
> +				i+1);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int ms5637_get_adc_value(struct i2c_client *client,
> +	unsigned int *adc_value)
> +{
> +	int ret = 0;
> +	unsigned char buf[3];
> +	ret = i2c_smbus_read_i2c_block_data(client, MS5637_ADC_READ, 3, buf);
> +	if (ret < 0)
> +		return ret;
> +	*adc_value = (buf[0] << 16) + (buf[1] << 8) + buf[2];
> +	return 0;
> +}
> +
> +static int ms5637_get_conversion_result(struct i2c_client *client,
> +	unsigned char command, unsigned int *adc_value)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte(client, command);
> +	if (ret < 0)
> +		return ret;
> +	msleep(9);
> +	ret = ms5637_get_adc_value(client, adc_value);
> +	if (ret < 0)
> +		return ret;
> +	return 0;
> +}
> +
> +static int ms5637_update_measurements(struct i2c_client *client)
> +{
> +	int ret = 0;
> +	unsigned int d1, d2;
> +	int dt, temp, p;
> +	long long int off, sens;
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&ms5637->lock);
> +
> +	if (time_after(jiffies, ms5637->last_update + HZ / 2) ||
> +	    !ms5637->valid) {
> +
> +		if (!ms5637->got_calibration_words)	{
> +			ret = ms5637_fill_calibration_words(client);
> +			if (ret < 0) {
> +				dev_err(&client->dev,
> +					"unable to get calibration words\n");
> +				goto out;
> +			}
> +
> +			ms5637->got_calibration_words = true;
> +		}
> +
> +		ret = ms5637_get_conversion_result(client,
> +			MS5637_CONVERT_D1_OSR_4096, &d1);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get adc conversion of D1_OSR_4096\n");
> +			goto out;
> +		}
> +
> +		ret = ms5637_get_conversion_result(client,
> +			MS5637_CONVERT_D2_OSR_4096, &d2);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get adc conversion of D2_OSR_4096\n");
> +			goto out;
> +		}
> +
> +		dt = d2 - (int)ms5637->calibration_words[4] * 256;
> +		temp = 20000 + div_s64((long long int)dt *
> +			(long long int)ms5637->calibration_words[5], 838861);
> +
> +		off = (long long int)ms5637->calibration_words[1] * 131072 +
> +			div_s64((long long int)ms5637->calibration_words[3] *
> +			(long long int)dt, 64);
> +		sens = (long long int)ms5637->calibration_words[0] * 65536 +
> +			div_s64((long long int)ms5637->calibration_words[2] *
> +			(long long int)dt, 128);
> +		p = (int)div_s64((div_s64((long long int)d1 * sens, 2097152) -
> +			off), 3277);
> +
> +		ms5637->temperature = temp;
> +		ms5637->pressure = p;
> +		ms5637->last_update = jiffies;
> +		ms5637->valid = true;
> +	}
> +out:
> +	mutex_unlock(&ms5637->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t ms5637_show_temperature(struct device *dev,
> +				      struct device_attribute *attr, char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +	int ret = ms5637_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", ms5637->temperature);
> +}
> +
> +static ssize_t ms5637_show_pressure(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +	int ret = ms5637_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", ms5637->pressure);
> +}
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
> +			  ms5637_show_temperature, NULL, 0);
> +static SENSOR_DEVICE_ATTR(pressure1_input, S_IRUGO,
> +			  ms5637_show_pressure, NULL, 0);
> +
> +static struct attribute *ms5637_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_pressure1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group ms5637_group = {
> +	.attrs = ms5637_attributes,
> +};
> +
> +static int ms5637_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	struct ms5637 *ms5637;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +					I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_WRITE_BYTE)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus byte transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus block transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	ms5637 = devm_kzalloc(&client->dev, sizeof(*ms5637), GFP_KERNEL);
> +	if (!ms5637)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, ms5637);
> +
> +	mutex_init(&ms5637->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &ms5637_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	ms5637->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(ms5637->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(ms5637->hwmon_dev);
> +		goto error;
> +	}
> +
> +	mutex_lock(&ms5637->lock);
> +	err = ms5637_fill_calibration_words(client);
> +	if (err >= 0)
> +		ms5637->got_calibration_words = true;
> +	mutex_unlock(&ms5637->lock);
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
> +	return err;
> +}
> +
> +static int ms5637_remove(struct i2c_client *client)
> +{
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(ms5637->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id ms5637_id[] = {
> +	{ "ms5637", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ms5637_id);
> +
> +static struct i2c_driver ms5637_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "ms5637",
> +	},
> +	.probe       = ms5637_probe,
> +	.remove      = ms5637_remove,
> +	.id_table    = ms5637_id,
> +};
> +
> +module_i2c_driver(ms5637_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("MEAS MS5637 pressure and temperature sensor driver");
> +MODULE_LICENSE("GPL");

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

* RE: [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support
  2013-09-10 15:20       ` [lm-sensors] " Guenter Roeck
@ 2013-09-11  5:48         ` Markezana, William
  -1 siblings, 0 replies; 15+ messages in thread
From: Markezana, William @ 2013-09-11  5:48 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: khali, lm-sensors, linux-kernel

Hi Guenter,

Thank you for your feedback, I will rewrite the drivers for IIO then.

Best regards,

William MARKEZANA
Direct : + 33 (0) 582 082 286
http://www.meas-spec.com


-----Message d'origine-----
De : Guenter Roeck [mailto:groeck7@gmail.com] De la part de Guenter Roeck
Envoyé : mardi 10 septembre 2013 17:21
À : Markezana, William
Cc : khali@linux-fr.org; lm-sensors@lm-sensors.org; linux-kernel@vger.kernel.org
Objet : Re: [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support

On Tue, Sep 10, 2013 at 05:09:57PM +0200, Markezana, William wrote:
> From: William Markezana <william.markezana@meas-spec.com>
> 
> hwmon: (ms5637) Add Measurement Specialties MS5637support
> Signed-off-by: William Markezana <william.markezana@meas-spec.com>

Hi William,

"pressure" is hardly a hardware monitoring attribute. It might make more sense to add your driver to the iio subsystem, which already supports at least one other pressure sensor.

Thanks,
Guenter

> ---
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 
> 55973cd..c4f1c8e 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -954,6 +954,16 @@ config SENSORS_MCP3021
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called mcp3021.
>  
> +config SENSORS_MS5637
> +	tristate "Measurement Specialties MS5637 pressure sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  MS5637 pressure sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ms5637.
> +
>  config SENSORS_NCT6775
>  	tristate "Nuvoton NCT6775F and compatibles"
>  	depends on !PPC
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 
> ec7cde0..5d8f699 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -110,6 +110,7 @@ obj-$(CONFIG_SENSORS_MAX6650)	+= max6650.o
>  obj-$(CONFIG_SENSORS_MAX6697)	+= max6697.o
>  obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
>  obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
> +obj-$(CONFIG_SENSORS_MS5637)	+= ms5637.o
>  obj-$(CONFIG_SENSORS_NCT6775)	+= nct6775.o
>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)	+= ntc_thermistor.o
>  obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
> diff --git a/drivers/hwmon/ms5637.c b/drivers/hwmon/ms5637.c new file 
> mode 100644 index 0000000..fe2c2df
> --- /dev/null
> +++ b/drivers/hwmon/ms5637.c
> @@ -0,0 +1,302 @@
> +/*
> + * Measurement Specialties MS5637 pressure and temperature sensor 
> +driver
> + *
> + * Copyright (C) 2013 William Markezana 
> +<william.markezana@meas-spec.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.
> + *
> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +#include <linux/delay.h>
> +
> +/* MS5637 Commands */
> +#define MS5637_CONVERT_D1_OSR_4096	(0x48)
> +#define MS5637_CONVERT_D2_OSR_4096	(0x58)
> +#define MS5637_ADC_READ				(0x00)
> +#define MS5637_PROM_READ			(0xA0)
> +
> +struct ms5637 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	bool valid;
> +	unsigned long last_update;
> +	int temperature;
> +	int pressure;
> +	unsigned short calibration_words[6];
> +	bool got_calibration_words;
> +};
> +
> +static int ms5637_get_calibration_word(struct i2c_client *client,
> +	unsigned char address, unsigned short *word) {
> +	int ret = 0;
> +	ret = i2c_smbus_read_word_swapped(client,
> +		MS5637_PROM_READ + (address << 1));
> +	if (ret < 0)
> +		return ret;
> +	*word = (unsigned short)ret & 0xFFFF;
> +	return 0;
> +}
> +
> +static int ms5637_fill_calibration_words(struct i2c_client *client) {
> +	int i, ret = 0;
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	for (i = 0; i < 6; i++) {
> +		ret = ms5637_get_calibration_word(client, i+1,
> +			&ms5637->calibration_words[i]);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get calibration word at address %d\n",
> +				i+1);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int ms5637_get_adc_value(struct i2c_client *client,
> +	unsigned int *adc_value)
> +{
> +	int ret = 0;
> +	unsigned char buf[3];
> +	ret = i2c_smbus_read_i2c_block_data(client, MS5637_ADC_READ, 3, buf);
> +	if (ret < 0)
> +		return ret;
> +	*adc_value = (buf[0] << 16) + (buf[1] << 8) + buf[2];
> +	return 0;
> +}
> +
> +static int ms5637_get_conversion_result(struct i2c_client *client,
> +	unsigned char command, unsigned int *adc_value) {
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte(client, command);
> +	if (ret < 0)
> +		return ret;
> +	msleep(9);
> +	ret = ms5637_get_adc_value(client, adc_value);
> +	if (ret < 0)
> +		return ret;
> +	return 0;
> +}
> +
> +static int ms5637_update_measurements(struct i2c_client *client) {
> +	int ret = 0;
> +	unsigned int d1, d2;
> +	int dt, temp, p;
> +	long long int off, sens;
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&ms5637->lock);
> +
> +	if (time_after(jiffies, ms5637->last_update + HZ / 2) ||
> +	    !ms5637->valid) {
> +
> +		if (!ms5637->got_calibration_words)	{
> +			ret = ms5637_fill_calibration_words(client);
> +			if (ret < 0) {
> +				dev_err(&client->dev,
> +					"unable to get calibration words\n");
> +				goto out;
> +			}
> +
> +			ms5637->got_calibration_words = true;
> +		}
> +
> +		ret = ms5637_get_conversion_result(client,
> +			MS5637_CONVERT_D1_OSR_4096, &d1);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get adc conversion of D1_OSR_4096\n");
> +			goto out;
> +		}
> +
> +		ret = ms5637_get_conversion_result(client,
> +			MS5637_CONVERT_D2_OSR_4096, &d2);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get adc conversion of D2_OSR_4096\n");
> +			goto out;
> +		}
> +
> +		dt = d2 - (int)ms5637->calibration_words[4] * 256;
> +		temp = 20000 + div_s64((long long int)dt *
> +			(long long int)ms5637->calibration_words[5], 838861);
> +
> +		off = (long long int)ms5637->calibration_words[1] * 131072 +
> +			div_s64((long long int)ms5637->calibration_words[3] *
> +			(long long int)dt, 64);
> +		sens = (long long int)ms5637->calibration_words[0] * 65536 +
> +			div_s64((long long int)ms5637->calibration_words[2] *
> +			(long long int)dt, 128);
> +		p = (int)div_s64((div_s64((long long int)d1 * sens, 2097152) -
> +			off), 3277);
> +
> +		ms5637->temperature = temp;
> +		ms5637->pressure = p;
> +		ms5637->last_update = jiffies;
> +		ms5637->valid = true;
> +	}
> +out:
> +	mutex_unlock(&ms5637->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t ms5637_show_temperature(struct device *dev,
> +				      struct device_attribute *attr, char *buf) {
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +	int ret = ms5637_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", ms5637->temperature); }
> +
> +static ssize_t ms5637_show_pressure(struct device *dev,
> +				   struct device_attribute *attr, char *buf) {
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +	int ret = ms5637_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", ms5637->pressure); }
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
> +			  ms5637_show_temperature, NULL, 0); static 
> +SENSOR_DEVICE_ATTR(pressure1_input, S_IRUGO,
> +			  ms5637_show_pressure, NULL, 0);
> +
> +static struct attribute *ms5637_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_pressure1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group ms5637_group = {
> +	.attrs = ms5637_attributes,
> +};
> +
> +static int ms5637_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id) {
> +	struct ms5637 *ms5637;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +					I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_WRITE_BYTE)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus byte transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus block transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	ms5637 = devm_kzalloc(&client->dev, sizeof(*ms5637), GFP_KERNEL);
> +	if (!ms5637)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, ms5637);
> +
> +	mutex_init(&ms5637->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &ms5637_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	ms5637->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(ms5637->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(ms5637->hwmon_dev);
> +		goto error;
> +	}
> +
> +	mutex_lock(&ms5637->lock);
> +	err = ms5637_fill_calibration_words(client);
> +	if (err >= 0)
> +		ms5637->got_calibration_words = true;
> +	mutex_unlock(&ms5637->lock);
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
> +	return err;
> +}
> +
> +static int ms5637_remove(struct i2c_client *client) {
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(ms5637->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id ms5637_id[] = {
> +	{ "ms5637", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ms5637_id);
> +
> +static struct i2c_driver ms5637_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "ms5637",
> +	},
> +	.probe       = ms5637_probe,
> +	.remove      = ms5637_remove,
> +	.id_table    = ms5637_id,
> +};
> +
> +module_i2c_driver(ms5637_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("MEAS MS5637 pressure and temperature sensor 
> +driver"); MODULE_LICENSE("GPL");

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

* Re: [lm-sensors] [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support
@ 2013-09-11  5:48         ` Markezana, William
  0 siblings, 0 replies; 15+ messages in thread
From: Markezana, William @ 2013-09-11  5:48 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: khali, lm-sensors, linux-kernel

Hi Guenter,

Thank you for your feedback, I will rewrite the drivers for IIO then.

Best regards,

William MARKEZANA
Direct : + 33 (0) 582 082 286
http://www.meas-spec.com


-----Message d'origine-----
De : Guenter Roeck [mailto:groeck7@gmail.com] De la part de Guenter Roeck
Envoyé : mardi 10 septembre 2013 17:21
À : Markezana, William
Cc : khali@linux-fr.org; lm-sensors@lm-sensors.org; linux-kernel@vger.kernel.org
Objet : Re: [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support

On Tue, Sep 10, 2013 at 05:09:57PM +0200, Markezana, William wrote:
> From: William Markezana <william.markezana@meas-spec.com>
> 
> hwmon: (ms5637) Add Measurement Specialties MS5637support
> Signed-off-by: William Markezana <william.markezana@meas-spec.com>

Hi William,

"pressure" is hardly a hardware monitoring attribute. It might make more sense to add your driver to the iio subsystem, which already supports at least one other pressure sensor.

Thanks,
Guenter

> ---
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 
> 55973cd..c4f1c8e 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -954,6 +954,16 @@ config SENSORS_MCP3021
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called mcp3021.
>  
> +config SENSORS_MS5637
> +	tristate "Measurement Specialties MS5637 pressure sensors"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Measurement Specialties
> +	  MS5637 pressure sensors.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called ms5637.
> +
>  config SENSORS_NCT6775
>  	tristate "Nuvoton NCT6775F and compatibles"
>  	depends on !PPC
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 
> ec7cde0..5d8f699 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -110,6 +110,7 @@ obj-$(CONFIG_SENSORS_MAX6650)	+= max6650.o
>  obj-$(CONFIG_SENSORS_MAX6697)	+= max6697.o
>  obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o
>  obj-$(CONFIG_SENSORS_MCP3021)	+= mcp3021.o
> +obj-$(CONFIG_SENSORS_MS5637)	+= ms5637.o
>  obj-$(CONFIG_SENSORS_NCT6775)	+= nct6775.o
>  obj-$(CONFIG_SENSORS_NTC_THERMISTOR)	+= ntc_thermistor.o
>  obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
> diff --git a/drivers/hwmon/ms5637.c b/drivers/hwmon/ms5637.c new file 
> mode 100644 index 0000000..fe2c2df
> --- /dev/null
> +++ b/drivers/hwmon/ms5637.c
> @@ -0,0 +1,302 @@
> +/*
> + * Measurement Specialties MS5637 pressure and temperature sensor 
> +driver
> + *
> + * Copyright (C) 2013 William Markezana 
> +<william.markezana@meas-spec.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.
> + *
> + */
> +
> +#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>
> +#include <linux/jiffies.h>
> +#include <linux/delay.h>
> +
> +/* MS5637 Commands */
> +#define MS5637_CONVERT_D1_OSR_4096	(0x48)
> +#define MS5637_CONVERT_D2_OSR_4096	(0x58)
> +#define MS5637_ADC_READ				(0x00)
> +#define MS5637_PROM_READ			(0xA0)
> +
> +struct ms5637 {
> +	struct device *hwmon_dev;
> +	struct mutex lock;
> +	bool valid;
> +	unsigned long last_update;
> +	int temperature;
> +	int pressure;
> +	unsigned short calibration_words[6];
> +	bool got_calibration_words;
> +};
> +
> +static int ms5637_get_calibration_word(struct i2c_client *client,
> +	unsigned char address, unsigned short *word) {
> +	int ret = 0;
> +	ret = i2c_smbus_read_word_swapped(client,
> +		MS5637_PROM_READ + (address << 1));
> +	if (ret < 0)
> +		return ret;
> +	*word = (unsigned short)ret & 0xFFFF;
> +	return 0;
> +}
> +
> +static int ms5637_fill_calibration_words(struct i2c_client *client) {
> +	int i, ret = 0;
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	for (i = 0; i < 6; i++) {
> +		ret = ms5637_get_calibration_word(client, i+1,
> +			&ms5637->calibration_words[i]);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get calibration word at address %d\n",
> +				i+1);
> +			return ret;
> +		}
> +	}
> +	return 0;
> +}
> +
> +static int ms5637_get_adc_value(struct i2c_client *client,
> +	unsigned int *adc_value)
> +{
> +	int ret = 0;
> +	unsigned char buf[3];
> +	ret = i2c_smbus_read_i2c_block_data(client, MS5637_ADC_READ, 3, buf);
> +	if (ret < 0)
> +		return ret;
> +	*adc_value = (buf[0] << 16) + (buf[1] << 8) + buf[2];
> +	return 0;
> +}
> +
> +static int ms5637_get_conversion_result(struct i2c_client *client,
> +	unsigned char command, unsigned int *adc_value) {
> +	int ret;
> +
> +	ret = i2c_smbus_write_byte(client, command);
> +	if (ret < 0)
> +		return ret;
> +	msleep(9);
> +	ret = ms5637_get_adc_value(client, adc_value);
> +	if (ret < 0)
> +		return ret;
> +	return 0;
> +}
> +
> +static int ms5637_update_measurements(struct i2c_client *client) {
> +	int ret = 0;
> +	unsigned int d1, d2;
> +	int dt, temp, p;
> +	long long int off, sens;
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	mutex_lock(&ms5637->lock);
> +
> +	if (time_after(jiffies, ms5637->last_update + HZ / 2) ||
> +	    !ms5637->valid) {
> +
> +		if (!ms5637->got_calibration_words)	{
> +			ret = ms5637_fill_calibration_words(client);
> +			if (ret < 0) {
> +				dev_err(&client->dev,
> +					"unable to get calibration words\n");
> +				goto out;
> +			}
> +
> +			ms5637->got_calibration_words = true;
> +		}
> +
> +		ret = ms5637_get_conversion_result(client,
> +			MS5637_CONVERT_D1_OSR_4096, &d1);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get adc conversion of D1_OSR_4096\n");
> +			goto out;
> +		}
> +
> +		ret = ms5637_get_conversion_result(client,
> +			MS5637_CONVERT_D2_OSR_4096, &d2);
> +		if (ret < 0) {
> +			dev_err(&client->dev,
> +				"unable to get adc conversion of D2_OSR_4096\n");
> +			goto out;
> +		}
> +
> +		dt = d2 - (int)ms5637->calibration_words[4] * 256;
> +		temp = 20000 + div_s64((long long int)dt *
> +			(long long int)ms5637->calibration_words[5], 838861);
> +
> +		off = (long long int)ms5637->calibration_words[1] * 131072 +
> +			div_s64((long long int)ms5637->calibration_words[3] *
> +			(long long int)dt, 64);
> +		sens = (long long int)ms5637->calibration_words[0] * 65536 +
> +			div_s64((long long int)ms5637->calibration_words[2] *
> +			(long long int)dt, 128);
> +		p = (int)div_s64((div_s64((long long int)d1 * sens, 2097152) -
> +			off), 3277);
> +
> +		ms5637->temperature = temp;
> +		ms5637->pressure = p;
> +		ms5637->last_update = jiffies;
> +		ms5637->valid = true;
> +	}
> +out:
> +	mutex_unlock(&ms5637->lock);
> +
> +	return ret >= 0 ? 0 : ret;
> +}
> +
> +static ssize_t ms5637_show_temperature(struct device *dev,
> +				      struct device_attribute *attr, char *buf) {
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +	int ret = ms5637_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", ms5637->temperature); }
> +
> +static ssize_t ms5637_show_pressure(struct device *dev,
> +				   struct device_attribute *attr, char *buf) {
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +	int ret = ms5637_update_measurements(client);
> +	if (ret < 0)
> +		return ret;
> +	return sprintf(buf, "%d\n", ms5637->pressure); }
> +
> +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
> +			  ms5637_show_temperature, NULL, 0); static 
> +SENSOR_DEVICE_ATTR(pressure1_input, S_IRUGO,
> +			  ms5637_show_pressure, NULL, 0);
> +
> +static struct attribute *ms5637_attributes[] = {
> +	&sensor_dev_attr_temp1_input.dev_attr.attr,
> +	&sensor_dev_attr_pressure1_input.dev_attr.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group ms5637_group = {
> +	.attrs = ms5637_attributes,
> +};
> +
> +static int ms5637_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id) {
> +	struct ms5637 *ms5637;
> +	int err;
> +
> +	if (!i2c_check_functionality(client->adapter,
> +					I2C_FUNC_SMBUS_READ_WORD_DATA)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus word transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_WRITE_BYTE)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus byte transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	if (!i2c_check_functionality(client->adapter,
> +				    I2C_FUNC_SMBUS_READ_I2C_BLOCK)) {
> +		dev_err(&client->dev,
> +			"adapter does not support SMBus block transactions\n");
> +		return -ENODEV;
> +	}
> +
> +	ms5637 = devm_kzalloc(&client->dev, sizeof(*ms5637), GFP_KERNEL);
> +	if (!ms5637)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, ms5637);
> +
> +	mutex_init(&ms5637->lock);
> +
> +	err = sysfs_create_group(&client->dev.kobj, &ms5637_group);
> +	if (err) {
> +		dev_dbg(&client->dev, "could not create sysfs files\n");
> +		return err;
> +	}
> +	ms5637->hwmon_dev = hwmon_device_register(&client->dev);
> +	if (IS_ERR(ms5637->hwmon_dev)) {
> +		dev_dbg(&client->dev, "unable to register hwmon device\n");
> +		err = PTR_ERR(ms5637->hwmon_dev);
> +		goto error;
> +	}
> +
> +	mutex_lock(&ms5637->lock);
> +	err = ms5637_fill_calibration_words(client);
> +	if (err >= 0)
> +		ms5637->got_calibration_words = true;
> +	mutex_unlock(&ms5637->lock);
> +
> +	dev_info(&client->dev, "initialized\n");
> +
> +	return 0;
> +
> +error:
> +	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
> +	return err;
> +}
> +
> +static int ms5637_remove(struct i2c_client *client) {
> +	struct ms5637 *ms5637 = i2c_get_clientdata(client);
> +
> +	hwmon_device_unregister(ms5637->hwmon_dev);
> +	sysfs_remove_group(&client->dev.kobj, &ms5637_group);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id ms5637_id[] = {
> +	{ "ms5637", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, ms5637_id);
> +
> +static struct i2c_driver ms5637_driver = {
> +	.class = I2C_CLASS_HWMON,
> +	.driver = {
> +		.name	= "ms5637",
> +	},
> +	.probe       = ms5637_probe,
> +	.remove      = ms5637_remove,
> +	.id_table    = ms5637_id,
> +};
> +
> +module_i2c_driver(ms5637_driver);
> +
> +MODULE_AUTHOR("William Markezana <william.markezana@meas-spec.com>");
> +MODULE_DESCRIPTION("MEAS MS5637 pressure and temperature sensor 
> +driver"); MODULE_LICENSE("GPL");

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

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

end of thread, other threads:[~2013-09-11  5:48 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-28  6:49 [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support Markezana, William
2013-08-28  6:49 ` [lm-sensors] " Markezana, William
2013-08-28 16:18 ` Guenter Roeck
2013-08-28 16:18   ` [lm-sensors] " Guenter Roeck
2013-08-29  8:06   ` [lm-sensors] RE : [PATCH] hwmon: (htu2 =?iso-8859-1?q?1=29_Add_Measurem Markezana, William
2013-08-29  9:40     ` RE : [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support Guenter Roeck
2013-08-29  9:40       ` [lm-sensors] RE : [PATCH] hwmon: (htu Guenter Roeck
2013-08-29 11:51 ` [lm-sensors] [PATCH] hwmon: (htu21) Add Measurement Specialties HTU21D support Markezana, William
2013-08-30  3:35   ` Guenter Roeck
2013-08-30  3:35     ` [lm-sensors] " Guenter Roeck
2013-09-10 15:09   ` [lm-sensors] [PATCH] hwmon: (ms5637) Add Measurement Specialties MS5637 support Markezana, William
2013-09-10 15:20     ` Guenter Roeck
2013-09-10 15:20       ` [lm-sensors] " Guenter Roeck
2013-09-11  5:48       ` Markezana, William
2013-09-11  5:48         ` [lm-sensors] " Markezana, William

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.