linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] IIO: add si7020 driver
@ 2014-09-18 20:01 David Barksdale
  2014-09-21 12:27 ` Hartmut Knaack
  0 siblings, 1 reply; 6+ messages in thread
From: David Barksdale @ 2014-09-18 20:01 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: David Barksdale, Peter Meerwald, linux-kernel, linux-iio

This patch adds support to the Industrial IO subsystem
for the Silicon Labs Si7013/20/21 Relative Humidity and
Temperature Sensors.

Website: http://www.silabs.com/products/sensors/humidity-sensors/Pages/si7013-20-21.aspx

These are i2c devices which measure relative humidity
and temperature and all use the same protocol. The
Si7013 has an additional input with programmable
linearization which is not supported because that's
complicated and I didn't need it.

Signed-off-by: David Barksdale <dbarksdale@uplogix.com>

--
Changes since v1:
* Renamed to si7020 and replaced Si701x/2x with Si7013/20/21.
* Removed unneeded mutex.
* Pre-computed floating-point constant expressions.
* Removed address_list and I2C_CLASS_HWMON.

---
 drivers/iio/humidity/Kconfig  |  10 +++
 drivers/iio/humidity/Makefile |   1 +
 drivers/iio/humidity/si7020.c | 142 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+)

diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
index e116bd8..4813b79 100644
--- a/drivers/iio/humidity/Kconfig
+++ b/drivers/iio/humidity/Kconfig
@@ -22,4 +22,14 @@ config SI7005
 	  To compile this driver as a module, choose M here: the module
 	  will be called si7005.
 
+config SI7020
+	tristate "Si7013/20/21 Relative Humidity and Temperature Sensors"
+	depends on I2C
+	help
+	  Say yes here to build support for the Silicon Labs Si7013/20/21
+	  Relative Humidity and Temperature Sensors.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called si7020.
+
 endmenu
diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
index e3f3d94..86e2d26 100644
--- a/drivers/iio/humidity/Makefile
+++ b/drivers/iio/humidity/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_DHT11) += dht11.o
 obj-$(CONFIG_SI7005) += si7005.o
+obj-$(CONFIG_SI7020) += si7020.o
diff --git a/drivers/iio/humidity/si7020.c b/drivers/iio/humidity/si7020.c
new file mode 100644
index 0000000..cabdb7d
--- /dev/null
+++ b/drivers/iio/humidity/si7020.c
@@ -0,0 +1,142 @@
+/*
+ * si7020.c - Silicon Labs Si7013/20/21 Relative Humidity and Temp Sensors
+ * Copyright (c) 2013,2014  Uplogix, Inc.
+ * David Barksdale <dbarksdale@uplogix.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * 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.
+ */
+
+/*
+ * The Silicon Labs Si7013/20/21 Relative Humidity and Temperature Sensors
+ * are i2c devices which have an identical programming interface for
+ * measuring relative humidity and temperature. The Si7013 has an additional
+ * temperature input which this driver does not support.
+ *
+ * Data Sheets:
+ *   Si7013: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7013.pdf
+ *   Si7020: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7020.pdf
+ *   Si7021: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7021.pdf
+ */
+
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+enum {
+	/* Measure Relative Humidity, Hold Master Mode */
+	SI7020CMD_RH_HOLD	= 0xE5,
+	/* Measure Temperature, Hold Master Mode */
+	SI7020CMD_TEMP_HOLD	= 0xE3,
+};
+
+static int si7020_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int *val,
+			   int *val2, long mask)
+{
+	struct i2c_client **client = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = i2c_smbus_read_word_data(*client,
+					       chan->type == IIO_TEMP ?
+					       SI7020CMD_TEMP_HOLD :
+					       SI7020CMD_RH_HOLD);
+		if (ret < 0)
+			return ret;
+		*val = ret;
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_TEMP)
+			*val = 175720; /* = 175.72 * 1000 */
+		else
+			*val = 125 * 1000;
+		*val2 = 65536;
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_OFFSET:
+		if (chan->type == IIO_TEMP)
+			*val = -17473; /* = -46.85 * 65536 / 175.72 */
+		else
+			*val = -3146; /* = -6 * 65536 / 125 */
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_chan_spec si7020_channels[] = {
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+	},
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
+	}
+};
+
+static const struct iio_info si7020_info = {
+	.read_raw = si7020_read_raw,
+	.driver_module = THIS_MODULE,
+};
+
+static int si7020_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct iio_dev *dev;
+	struct i2c_client **data;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
+		return -ENODEV;
+
+	dev = devm_iio_device_alloc(&client->dev, sizeof(*client));
+	if (!dev)
+		return -ENOMEM;
+
+	data = iio_priv(dev);
+	*data = client;
+	i2c_set_clientdata(client, dev);
+
+	dev->dev.parent = &client->dev;
+	dev->name = dev_name(&client->dev);
+	dev->modes = INDIO_DIRECT_MODE;
+	dev->info = &si7020_info;
+	dev->channels = si7020_channels;
+	dev->num_channels = ARRAY_SIZE(si7020_channels);
+
+	return devm_iio_device_register(&client->dev, dev);
+}
+
+static const struct i2c_device_id si7020_id[] = {
+	{ "si7020", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, si7020_id);
+
+static struct i2c_driver si7020_driver = {
+	.driver.name	= "si7020",
+	.probe		= si7020_probe,
+	.id_table	= si7020_id,
+};
+
+module_i2c_driver(si7020_driver);
+MODULE_DESCRIPTION("Silicon Labs Si7013/20/21 "
+		   "Relative Humidity and Temperature Sensors");
+MODULE_AUTHOR("David Barksdale <dbarksdale@uplogix.com>");
+MODULE_LICENSE("GPL");
+
-- 
tg: (59753a8..) upstream/si7020 (depends on: upstream/master)

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

* Re: [PATCH v2] IIO: add si7020 driver
  2014-09-18 20:01 [PATCH v2] IIO: add si7020 driver David Barksdale
@ 2014-09-21 12:27 ` Hartmut Knaack
  2014-09-22 13:09   ` David Barksdale
  2014-09-22 15:40   ` David Barksdale
  0 siblings, 2 replies; 6+ messages in thread
From: Hartmut Knaack @ 2014-09-21 12:27 UTC (permalink / raw)
  To: David Barksdale, Jonathan Cameron; +Cc: Peter Meerwald, linux-kernel, linux-iio

David Barksdale schrieb, Am 18.09.2014 22:01:
> This patch adds support to the Industrial IO subsystem
> for the Silicon Labs Si7013/20/21 Relative Humidity and
> Temperature Sensors.
> 
> Website: http://www.silabs.com/products/sensors/humidity-sensors/Pages/si7013-20-21.aspx
> 
> These are i2c devices which measure relative humidity
> and temperature and all use the same protocol. The
> Si7013 has an additional input with programmable
> linearization which is not supported because that's
> complicated and I didn't need it.
> 
> Signed-off-by: David Barksdale <dbarksdale@uplogix.com>
> 
Hi,
I'm not really convinced of the concept of treating all measurements as 16 bit data, since the datasheet states a maximum resolution of 14 bits for temperature and 12 bits for humidity. It also states, that the 2 LSBs contain an identifier rather than measured data. So, the least to do would be to mask out everything, which is not measured data (which should also include the 2 MSBs of humidity channel). Even better would be to also apply a shift and treat the data with the real resolution (especially for scale, as it would represent the sensitivity of the sensor a bit better).
Besides that, I think it would be a good idea to initialize the sensor during probe (for example set resolution to maximum), since you can never be sure, in which state the sensor has been before.
Also, find some small style issues inline.
> --
> Changes since v1:
> * Renamed to si7020 and replaced Si701x/2x with Si7013/20/21.
> * Removed unneeded mutex.
> * Pre-computed floating-point constant expressions.
> * Removed address_list and I2C_CLASS_HWMON.
> 
> ---
>  drivers/iio/humidity/Kconfig  |  10 +++
>  drivers/iio/humidity/Makefile |   1 +
>  drivers/iio/humidity/si7020.c | 142 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 153 insertions(+)
> 
> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
> index e116bd8..4813b79 100644
> --- a/drivers/iio/humidity/Kconfig
> +++ b/drivers/iio/humidity/Kconfig
> @@ -22,4 +22,14 @@ config SI7005
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called si7005.
>  
> +config SI7020
> +	tristate "Si7013/20/21 Relative Humidity and Temperature Sensors"
> +	depends on I2C
> +	help
> +	  Say yes here to build support for the Silicon Labs Si7013/20/21
> +	  Relative Humidity and Temperature Sensors.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called si7020.
> +
>  endmenu
> diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
> index e3f3d94..86e2d26 100644
> --- a/drivers/iio/humidity/Makefile
> +++ b/drivers/iio/humidity/Makefile
> @@ -4,3 +4,4 @@
>  
>  obj-$(CONFIG_DHT11) += dht11.o
>  obj-$(CONFIG_SI7005) += si7005.o
> +obj-$(CONFIG_SI7020) += si7020.o
> diff --git a/drivers/iio/humidity/si7020.c b/drivers/iio/humidity/si7020.c
> new file mode 100644
> index 0000000..cabdb7d
> --- /dev/null
> +++ b/drivers/iio/humidity/si7020.c
> @@ -0,0 +1,142 @@
> +/*
> + * si7020.c - Silicon Labs Si7013/20/21 Relative Humidity and Temp Sensors
> + * Copyright (c) 2013,2014  Uplogix, Inc.
> + * David Barksdale <dbarksdale@uplogix.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * 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.
> + */
> +
> +/*
> + * The Silicon Labs Si7013/20/21 Relative Humidity and Temperature Sensors
> + * are i2c devices which have an identical programming interface for
> + * measuring relative humidity and temperature. The Si7013 has an additional
> + * temperature input which this driver does not support.
> + *
> + * Data Sheets:
> + *   Si7013: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7013.pdf
> + *   Si7020: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7020.pdf
> + *   Si7021: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7021.pdf
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/sysfs.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +enum {
> +	/* Measure Relative Humidity, Hold Master Mode */
> +	SI7020CMD_RH_HOLD	= 0xE5,
> +	/* Measure Temperature, Hold Master Mode */
> +	SI7020CMD_TEMP_HOLD	= 0xE3,
> +};
> +
> +static int si7020_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan, int *val,
> +			   int *val2, long mask)
> +{
> +	struct i2c_client **client = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = i2c_smbus_read_word_data(*client,
> +					       chan->type == IIO_TEMP ?
> +					       SI7020CMD_TEMP_HOLD :
> +					       SI7020CMD_RH_HOLD);
> +		if (ret < 0)
> +			return ret;
> +		*val = ret;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_TEMP)
> +			*val = 175720; /* = 175.72 * 1000 */
> +		else
> +			*val = 125 * 1000;
> +		*val2 = 65536;
> +		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_OFFSET:
> +		if (chan->type == IIO_TEMP)
> +			*val = -17473; /* = -46.85 * 65536 / 175.72 */
> +		else
> +			*val = -3146; /* = -6 * 65536 / 125 */
> +		return IIO_VAL_INT;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_chan_spec si7020_channels[] = {
> +	{
> +		.type = IIO_HUMIDITYRELATIVE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
> +	},
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
> +	}
> +};
> +
> +static const struct iio_info si7020_info = {
> +	.read_raw = si7020_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static int si7020_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct iio_dev *dev;
Better name it indio_dev.
> +	struct i2c_client **data;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
> +		return -ENODEV;
> +
> +	dev = devm_iio_device_alloc(&client->dev, sizeof(*client));
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(dev);
> +	*data = client;
> +	i2c_set_clientdata(client, dev);
> +
> +	dev->dev.parent = &client->dev;
> +	dev->name = dev_name(&client->dev);
> +	dev->modes = INDIO_DIRECT_MODE;
> +	dev->info = &si7020_info;
> +	dev->channels = si7020_channels;
> +	dev->num_channels = ARRAY_SIZE(si7020_channels);
> +
> +	return devm_iio_device_register(&client->dev, dev);
> +}
> +
> +static const struct i2c_device_id si7020_id[] = {
> +	{ "si7020", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, si7020_id);
> +
> +static struct i2c_driver si7020_driver = {
> +	.driver.name	= "si7020",
> +	.probe		= si7020_probe,
> +	.id_table	= si7020_id,
> +};
> +
> +module_i2c_driver(si7020_driver);
> +MODULE_DESCRIPTION("Silicon Labs Si7013/20/21 "
Trailing whitespace in string.
> +		   "Relative Humidity and Temperature Sensors");
> +MODULE_AUTHOR("David Barksdale <dbarksdale@uplogix.com>");
> +MODULE_LICENSE("GPL");
> +
> 


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

* Re: [PATCH v2] IIO: add si7020 driver
  2014-09-21 12:27 ` Hartmut Knaack
@ 2014-09-22 13:09   ` David Barksdale
  2014-09-22 19:51     ` Hartmut Knaack
  2014-09-22 15:40   ` David Barksdale
  1 sibling, 1 reply; 6+ messages in thread
From: David Barksdale @ 2014-09-22 13:09 UTC (permalink / raw)
  To: Hartmut Knaack; +Cc: Jonathan Cameron, linux-kernel, linux-iio



On 09/21/2014 07:27 AM, Hartmut Knaack wrote:
> David Barksdale schrieb, Am 18.09.2014 22:01:
>> This patch adds support to the Industrial IO subsystem
>> for the Silicon Labs Si7013/20/21 Relative Humidity and
>> Temperature Sensors.
>>
>> Website: http://www.silabs.com/products/sensors/humidity-sensors/Pages/si7013-20-21.aspx
>>
>> These are i2c devices which measure relative humidity
>> and temperature and all use the same protocol. The
>> Si7013 has an additional input with programmable
>> linearization which is not supported because that's
>> complicated and I didn't need it.
>>
>> Signed-off-by: David Barksdale <dbarksdale@uplogix.com>
>>
> Hi,
> I'm not really convinced of the concept of treating all measurements as 16 bit data, since the datasheet states a maximum resolution of 14 bits for temperature and 12 bits for humidity. It also states, that the 2 LSBs contain an identifier rather than measured data. So, the least to do would be to mask out everything, which is not measured data (which should also include the 2 MSBs of humidity channel). Even better would be to also apply a shift and treat the data with the real resolution (especially for scale, as it would represent the sensitivity of the sensor a bit better).
> Besides that, I think it would be a good idea to initialize the sensor during probe (for example set resolution to maximum), since you can never be sure, in which state the sensor has been before.

Good idea, Thanks.

> Also, find some small style issues inline.
>> --
>> Changes since v1:
>> * Renamed to si7020 and replaced Si701x/2x with Si7013/20/21.
>> * Removed unneeded mutex.
>> * Pre-computed floating-point constant expressions.
>> * Removed address_list and I2C_CLASS_HWMON.
>>
>> ---
>>   drivers/iio/humidity/Kconfig  |  10 +++
>>   drivers/iio/humidity/Makefile |   1 +
>>   drivers/iio/humidity/si7020.c | 142 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 153 insertions(+)
>>
>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>> index e116bd8..4813b79 100644
>> --- a/drivers/iio/humidity/Kconfig
>> +++ b/drivers/iio/humidity/Kconfig
>> @@ -22,4 +22,14 @@ config SI7005
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called si7005.
>>
>> +config SI7020
>> +	tristate "Si7013/20/21 Relative Humidity and Temperature Sensors"
>> +	depends on I2C
>> +	help
>> +	  Say yes here to build support for the Silicon Labs Si7013/20/21
>> +	  Relative Humidity and Temperature Sensors.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called si7020.
>> +
>>   endmenu
>> diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
>> index e3f3d94..86e2d26 100644
>> --- a/drivers/iio/humidity/Makefile
>> +++ b/drivers/iio/humidity/Makefile
>> @@ -4,3 +4,4 @@
>>
>>   obj-$(CONFIG_DHT11) += dht11.o
>>   obj-$(CONFIG_SI7005) += si7005.o
>> +obj-$(CONFIG_SI7020) += si7020.o
>> diff --git a/drivers/iio/humidity/si7020.c b/drivers/iio/humidity/si7020.c
>> new file mode 100644
>> index 0000000..cabdb7d
>> --- /dev/null
>> +++ b/drivers/iio/humidity/si7020.c
>> @@ -0,0 +1,142 @@
>> +/*
>> + * si7020.c - Silicon Labs Si7013/20/21 Relative Humidity and Temp Sensors
>> + * Copyright (c) 2013,2014  Uplogix, Inc.
>> + * David Barksdale <dbarksdale@uplogix.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * 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.
>> + */
>> +
>> +/*
>> + * The Silicon Labs Si7013/20/21 Relative Humidity and Temperature Sensors
>> + * are i2c devices which have an identical programming interface for
>> + * measuring relative humidity and temperature. The Si7013 has an additional
>> + * temperature input which this driver does not support.
>> + *
>> + * Data Sheets:
>> + *   Si7013: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7013.pdf
>> + *   Si7020: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7020.pdf
>> + *   Si7021: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7021.pdf
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysfs.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +enum {
>> +	/* Measure Relative Humidity, Hold Master Mode */
>> +	SI7020CMD_RH_HOLD	= 0xE5,
>> +	/* Measure Temperature, Hold Master Mode */
>> +	SI7020CMD_TEMP_HOLD	= 0xE3,
>> +};
>> +
>> +static int si7020_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *chan, int *val,
>> +			   int *val2, long mask)
>> +{
>> +	struct i2c_client **client = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		ret = i2c_smbus_read_word_data(*client,
>> +					       chan->type == IIO_TEMP ?
>> +					       SI7020CMD_TEMP_HOLD :
>> +					       SI7020CMD_RH_HOLD);
>> +		if (ret < 0)
>> +			return ret;
>> +		*val = ret;
>> +		return IIO_VAL_INT;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		if (chan->type == IIO_TEMP)
>> +			*val = 175720; /* = 175.72 * 1000 */
>> +		else
>> +			*val = 125 * 1000;
>> +		*val2 = 65536;
>> +		return IIO_VAL_FRACTIONAL;
>> +	case IIO_CHAN_INFO_OFFSET:
>> +		if (chan->type == IIO_TEMP)
>> +			*val = -17473; /* = -46.85 * 65536 / 175.72 */
>> +		else
>> +			*val = -3146; /* = -6 * 65536 / 125 */
>> +		return IIO_VAL_INT;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static const struct iio_chan_spec si7020_channels[] = {
>> +	{
>> +		.type = IIO_HUMIDITYRELATIVE,
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
>> +	},
>> +	{
>> +		.type = IIO_TEMP,
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
>> +	}
>> +};
>> +
>> +static const struct iio_info si7020_info = {
>> +	.read_raw = si7020_read_raw,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +static int si7020_probe(struct i2c_client *client,
>> +			const struct i2c_device_id *id)
>> +{
>> +	struct iio_dev *dev;
> Better name it indio_dev.

Seems to be the popular choice.

>> +	struct i2c_client **data;
>> +
>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
>> +		return -ENODEV;
>> +
>> +	dev = devm_iio_device_alloc(&client->dev, sizeof(*client));
>> +	if (!dev)
>> +		return -ENOMEM;
>> +
>> +	data = iio_priv(dev);
>> +	*data = client;
>> +	i2c_set_clientdata(client, dev);
>> +
>> +	dev->dev.parent = &client->dev;
>> +	dev->name = dev_name(&client->dev);
>> +	dev->modes = INDIO_DIRECT_MODE;
>> +	dev->info = &si7020_info;
>> +	dev->channels = si7020_channels;
>> +	dev->num_channels = ARRAY_SIZE(si7020_channels);
>> +
>> +	return devm_iio_device_register(&client->dev, dev);
>> +}
>> +
>> +static const struct i2c_device_id si7020_id[] = {
>> +	{ "si7020", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, si7020_id);
>> +
>> +static struct i2c_driver si7020_driver = {
>> +	.driver.name	= "si7020",
>> +	.probe		= si7020_probe,
>> +	.id_table	= si7020_id,
>> +};
>> +
>> +module_i2c_driver(si7020_driver);
>> +MODULE_DESCRIPTION("Silicon Labs Si7013/20/21 "
> Trailing whitespace in string.

The end of the string is on the line below.

>> +		   "Relative Humidity and Temperature Sensors");
>> +MODULE_AUTHOR("David Barksdale <dbarksdale@uplogix.com>");
>> +MODULE_LICENSE("GPL");
>> +
>>
>

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

* Re: [PATCH v2] IIO: add si7020 driver
  2014-09-21 12:27 ` Hartmut Knaack
  2014-09-22 13:09   ` David Barksdale
@ 2014-09-22 15:40   ` David Barksdale
  2014-09-22 20:16     ` Hartmut Knaack
  1 sibling, 1 reply; 6+ messages in thread
From: David Barksdale @ 2014-09-22 15:40 UTC (permalink / raw)
  To: Hartmut Knaack, Jonathan Cameron; +Cc: Peter Meerwald, linux-kernel, linux-iio


On 09/21/2014 07:27 AM, Hartmut Knaack wrote:
> David Barksdale schrieb, Am 18.09.2014 22:01:
>> This patch adds support to the Industrial IO subsystem
>> for the Silicon Labs Si7013/20/21 Relative Humidity and
>> Temperature Sensors.
>>
>> Website: http://www.silabs.com/products/sensors/humidity-sensors/Pages/si7013-20-21.aspx
>>
>> These are i2c devices which measure relative humidity
>> and temperature and all use the same protocol. The
>> Si7013 has an additional input with programmable
>> linearization which is not supported because that's
>> complicated and I didn't need it.
>>
>> Signed-off-by: David Barksdale <dbarksdale@uplogix.com>
>>
> Hi,
> I'm not really convinced of the concept of treating all measurements as 16 bit data, since the datasheet states a maximum resolution of 14 bits for temperature and 12 bits for humidity. It also states, that the 2 LSBs contain an identifier rather than measured data. So, the least to do would be to mask out everything, which is not measured data (which should also include the 2 MSBs of humidity channel). Even better would be to also apply a shift and treat the data with the real resolution (especially for scale, as it would represent the sensitivity of the sensor a bit better).
> Besides that, I think it would be a good idea to initialize the sensor during probe (for example set resolution to maximum), since you can never be sure, in which state the sensor has been before.
> Also, find some small style issues inline.

The math done by iio_convert_raw_to_processed_unlocked loses accuracy in 
my case because I need the offset to be fractional. It assumes offset is 
an integer so I have to round the true offset to the nearest integer.
If I provide IIO_CHAN_INFO_PROCESSED then I can do the math correctly.
Would it be kosher to provide only IIO_CHAN_INFO_RAW and 
IIO_CHAN_INFO_PROCESSED?

>> --
>> Changes since v1:
>> * Renamed to si7020 and replaced Si701x/2x with Si7013/20/21.
>> * Removed unneeded mutex.
>> * Pre-computed floating-point constant expressions.
>> * Removed address_list and I2C_CLASS_HWMON.
>>
>> ---
>>   drivers/iio/humidity/Kconfig  |  10 +++
>>   drivers/iio/humidity/Makefile |   1 +
>>   drivers/iio/humidity/si7020.c | 142 ++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 153 insertions(+)
>>
>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>> index e116bd8..4813b79 100644
>> --- a/drivers/iio/humidity/Kconfig
>> +++ b/drivers/iio/humidity/Kconfig
>> @@ -22,4 +22,14 @@ config SI7005
>>   	  To compile this driver as a module, choose M here: the module
>>   	  will be called si7005.
>>
>> +config SI7020
>> +	tristate "Si7013/20/21 Relative Humidity and Temperature Sensors"
>> +	depends on I2C
>> +	help
>> +	  Say yes here to build support for the Silicon Labs Si7013/20/21
>> +	  Relative Humidity and Temperature Sensors.
>> +
>> +	  To compile this driver as a module, choose M here: the module
>> +	  will be called si7020.
>> +
>>   endmenu
>> diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
>> index e3f3d94..86e2d26 100644
>> --- a/drivers/iio/humidity/Makefile
>> +++ b/drivers/iio/humidity/Makefile
>> @@ -4,3 +4,4 @@
>>
>>   obj-$(CONFIG_DHT11) += dht11.o
>>   obj-$(CONFIG_SI7005) += si7005.o
>> +obj-$(CONFIG_SI7020) += si7020.o
>> diff --git a/drivers/iio/humidity/si7020.c b/drivers/iio/humidity/si7020.c
>> new file mode 100644
>> index 0000000..cabdb7d
>> --- /dev/null
>> +++ b/drivers/iio/humidity/si7020.c
>> @@ -0,0 +1,142 @@
>> +/*
>> + * si7020.c - Silicon Labs Si7013/20/21 Relative Humidity and Temp Sensors
>> + * Copyright (c) 2013,2014  Uplogix, Inc.
>> + * David Barksdale <dbarksdale@uplogix.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * 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.
>> + */
>> +
>> +/*
>> + * The Silicon Labs Si7013/20/21 Relative Humidity and Temperature Sensors
>> + * are i2c devices which have an identical programming interface for
>> + * measuring relative humidity and temperature. The Si7013 has an additional
>> + * temperature input which this driver does not support.
>> + *
>> + * Data Sheets:
>> + *   Si7013: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7013.pdf
>> + *   Si7020: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7020.pdf
>> + *   Si7021: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7021.pdf
>> + */
>> +
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/sysfs.h>
>> +
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/sysfs.h>
>> +
>> +enum {
>> +	/* Measure Relative Humidity, Hold Master Mode */
>> +	SI7020CMD_RH_HOLD	= 0xE5,
>> +	/* Measure Temperature, Hold Master Mode */
>> +	SI7020CMD_TEMP_HOLD	= 0xE3,
>> +};
>> +
>> +static int si7020_read_raw(struct iio_dev *indio_dev,
>> +			   struct iio_chan_spec const *chan, int *val,
>> +			   int *val2, long mask)
>> +{
>> +	struct i2c_client **client = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		ret = i2c_smbus_read_word_data(*client,
>> +					       chan->type == IIO_TEMP ?
>> +					       SI7020CMD_TEMP_HOLD :
>> +					       SI7020CMD_RH_HOLD);
>> +		if (ret < 0)
>> +			return ret;
>> +		*val = ret;
>> +		return IIO_VAL_INT;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		if (chan->type == IIO_TEMP)
>> +			*val = 175720; /* = 175.72 * 1000 */
>> +		else
>> +			*val = 125 * 1000;
>> +		*val2 = 65536;
>> +		return IIO_VAL_FRACTIONAL;
>> +	case IIO_CHAN_INFO_OFFSET:
>> +		if (chan->type == IIO_TEMP)
>> +			*val = -17473; /* = -46.85 * 65536 / 175.72 */
>> +		else
>> +			*val = -3146; /* = -6 * 65536 / 125 */
>> +		return IIO_VAL_INT;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static const struct iio_chan_spec si7020_channels[] = {
>> +	{
>> +		.type = IIO_HUMIDITYRELATIVE,
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
>> +	},
>> +	{
>> +		.type = IIO_TEMP,
>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
>> +	}
>> +};
>> +
>> +static const struct iio_info si7020_info = {
>> +	.read_raw = si7020_read_raw,
>> +	.driver_module = THIS_MODULE,
>> +};
>> +
>> +static int si7020_probe(struct i2c_client *client,
>> +			const struct i2c_device_id *id)
>> +{
>> +	struct iio_dev *dev;
> Better name it indio_dev.
>> +	struct i2c_client **data;
>> +
>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
>> +		return -ENODEV;
>> +
>> +	dev = devm_iio_device_alloc(&client->dev, sizeof(*client));
>> +	if (!dev)
>> +		return -ENOMEM;
>> +
>> +	data = iio_priv(dev);
>> +	*data = client;
>> +	i2c_set_clientdata(client, dev);
>> +
>> +	dev->dev.parent = &client->dev;
>> +	dev->name = dev_name(&client->dev);
>> +	dev->modes = INDIO_DIRECT_MODE;
>> +	dev->info = &si7020_info;
>> +	dev->channels = si7020_channels;
>> +	dev->num_channels = ARRAY_SIZE(si7020_channels);
>> +
>> +	return devm_iio_device_register(&client->dev, dev);
>> +}
>> +
>> +static const struct i2c_device_id si7020_id[] = {
>> +	{ "si7020", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(i2c, si7020_id);
>> +
>> +static struct i2c_driver si7020_driver = {
>> +	.driver.name	= "si7020",
>> +	.probe		= si7020_probe,
>> +	.id_table	= si7020_id,
>> +};
>> +
>> +module_i2c_driver(si7020_driver);
>> +MODULE_DESCRIPTION("Silicon Labs Si7013/20/21 "
> Trailing whitespace in string.
>> +		   "Relative Humidity and Temperature Sensors");
>> +MODULE_AUTHOR("David Barksdale <dbarksdale@uplogix.com>");
>> +MODULE_LICENSE("GPL");
>> +
>>
>

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

* Re: [PATCH v2] IIO: add si7020 driver
  2014-09-22 13:09   ` David Barksdale
@ 2014-09-22 19:51     ` Hartmut Knaack
  0 siblings, 0 replies; 6+ messages in thread
From: Hartmut Knaack @ 2014-09-22 19:51 UTC (permalink / raw)
  To: David Barksdale; +Cc: Jonathan Cameron, linux-kernel, linux-iio

David Barksdale schrieb, Am 22.09.2014 15:09:
> 
> 
> On 09/21/2014 07:27 AM, Hartmut Knaack wrote:
>> David Barksdale schrieb, Am 18.09.2014 22:01:
>>> This patch adds support to the Industrial IO subsystem
>>> for the Silicon Labs Si7013/20/21 Relative Humidity and
>>> Temperature Sensors.
>>>
>>> Website: http://www.silabs.com/products/sensors/humidity-sensors/Pages/si7013-20-21.aspx
>>>
>>> These are i2c devices which measure relative humidity
>>> and temperature and all use the same protocol. The
>>> Si7013 has an additional input with programmable
>>> linearization which is not supported because that's
>>> complicated and I didn't need it.
>>>
>>> Signed-off-by: David Barksdale <dbarksdale@uplogix.com>
>>>
>> Hi,
>> I'm not really convinced of the concept of treating all measurements as 16 bit data, since the datasheet states a maximum resolution of 14 bits for temperature and 12 bits for humidity. It also states, that the 2 LSBs contain an identifier rather than measured data. So, the least to do would be to mask out everything, which is not measured data (which should also include the 2 MSBs of humidity channel). Even better would be to also apply a shift and treat the data with the real resolution (especially for scale, as it would represent the sensitivity of the sensor a bit better).
>> Besides that, I think it would be a good idea to initialize the sensor during probe (for example set resolution to maximum), since you can never be sure, in which state the sensor has been before.
> 
> Good idea, Thanks.
> 
>> Also, find some small style issues inline.
>>> --
>>> Changes since v1:
>>> * Renamed to si7020 and replaced Si701x/2x with Si7013/20/21.
>>> * Removed unneeded mutex.
>>> * Pre-computed floating-point constant expressions.
>>> * Removed address_list and I2C_CLASS_HWMON.
>>>
>>> ---
>>>   drivers/iio/humidity/Kconfig  |  10 +++
>>>   drivers/iio/humidity/Makefile |   1 +
>>>   drivers/iio/humidity/si7020.c | 142 ++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 153 insertions(+)
>>>
>>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>>> index e116bd8..4813b79 100644
>>> --- a/drivers/iio/humidity/Kconfig
>>> +++ b/drivers/iio/humidity/Kconfig
>>> @@ -22,4 +22,14 @@ config SI7005
>>>   	  To compile this driver as a module, choose M here: the module
>>>   	  will be called si7005.
>>>
>>> +config SI7020
>>> +	tristate "Si7013/20/21 Relative Humidity and Temperature Sensors"
>>> +	depends on I2C
>>> +	help
>>> +	  Say yes here to build support for the Silicon Labs Si7013/20/21
>>> +	  Relative Humidity and Temperature Sensors.
>>> +
>>> +	  To compile this driver as a module, choose M here: the module
>>> +	  will be called si7020.
>>> +
>>>   endmenu
>>> diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
>>> index e3f3d94..86e2d26 100644
>>> --- a/drivers/iio/humidity/Makefile
>>> +++ b/drivers/iio/humidity/Makefile
>>> @@ -4,3 +4,4 @@
>>>
>>>   obj-$(CONFIG_DHT11) += dht11.o
>>>   obj-$(CONFIG_SI7005) += si7005.o
>>> +obj-$(CONFIG_SI7020) += si7020.o
>>> diff --git a/drivers/iio/humidity/si7020.c b/drivers/iio/humidity/si7020.c
>>> new file mode 100644
>>> index 0000000..cabdb7d
>>> --- /dev/null
>>> +++ b/drivers/iio/humidity/si7020.c
>>> @@ -0,0 +1,142 @@
>>> +/*
>>> + * si7020.c - Silicon Labs Si7013/20/21 Relative Humidity and Temp Sensors
>>> + * Copyright (c) 2013,2014  Uplogix, Inc.
>>> + * David Barksdale <dbarksdale@uplogix.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +/*
>>> + * The Silicon Labs Si7013/20/21 Relative Humidity and Temperature Sensors
>>> + * are i2c devices which have an identical programming interface for
>>> + * measuring relative humidity and temperature. The Si7013 has an additional
>>> + * temperature input which this driver does not support.
>>> + *
>>> + * Data Sheets:
>>> + *   Si7013: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7013.pdf
>>> + *   Si7020: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7020.pdf
>>> + *   Si7021: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7021.pdf
>>> + */
>>> +
>>> +#include <linux/i2c.h>
>>> +#include <linux/module.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/sysfs.h>
>>> +
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +
>>> +enum {
>>> +	/* Measure Relative Humidity, Hold Master Mode */
>>> +	SI7020CMD_RH_HOLD	= 0xE5,
>>> +	/* Measure Temperature, Hold Master Mode */
>>> +	SI7020CMD_TEMP_HOLD	= 0xE3,
>>> +};
>>> +
>>> +static int si7020_read_raw(struct iio_dev *indio_dev,
>>> +			   struct iio_chan_spec const *chan, int *val,
>>> +			   int *val2, long mask)
>>> +{
>>> +	struct i2c_client **client = iio_priv(indio_dev);
>>> +	int ret;
>>> +
>>> +	switch (mask) {
>>> +	case IIO_CHAN_INFO_RAW:
>>> +		ret = i2c_smbus_read_word_data(*client,
>>> +					       chan->type == IIO_TEMP ?
>>> +					       SI7020CMD_TEMP_HOLD :
>>> +					       SI7020CMD_RH_HOLD);
>>> +		if (ret < 0)
>>> +			return ret;
>>> +		*val = ret;
>>> +		return IIO_VAL_INT;
>>> +	case IIO_CHAN_INFO_SCALE:
>>> +		if (chan->type == IIO_TEMP)
>>> +			*val = 175720; /* = 175.72 * 1000 */
>>> +		else
>>> +			*val = 125 * 1000;
>>> +		*val2 = 65536;
>>> +		return IIO_VAL_FRACTIONAL;
>>> +	case IIO_CHAN_INFO_OFFSET:
>>> +		if (chan->type == IIO_TEMP)
>>> +			*val = -17473; /* = -46.85 * 65536 / 175.72 */
>>> +		else
>>> +			*val = -3146; /* = -6 * 65536 / 125 */
>>> +		return IIO_VAL_INT;
>>> +	default:
>>> +		break;
>>> +	}
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +
>>> +static const struct iio_chan_spec si7020_channels[] = {
>>> +	{
>>> +		.type = IIO_HUMIDITYRELATIVE,
>>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
>>> +	},
>>> +	{
>>> +		.type = IIO_TEMP,
>>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
>>> +	}
>>> +};
>>> +
>>> +static const struct iio_info si7020_info = {
>>> +	.read_raw = si7020_read_raw,
>>> +	.driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static int si7020_probe(struct i2c_client *client,
>>> +			const struct i2c_device_id *id)
>>> +{
>>> +	struct iio_dev *dev;
>> Better name it indio_dev.
> 
> Seems to be the popular choice.
> 
>>> +	struct i2c_client **data;
>>> +
>>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
>>> +		return -ENODEV;
>>> +
>>> +	dev = devm_iio_device_alloc(&client->dev, sizeof(*client));
>>> +	if (!dev)
>>> +		return -ENOMEM;
>>> +
>>> +	data = iio_priv(dev);
>>> +	*data = client;
>>> +	i2c_set_clientdata(client, dev);
>>> +
>>> +	dev->dev.parent = &client->dev;
>>> +	dev->name = dev_name(&client->dev);
>>> +	dev->modes = INDIO_DIRECT_MODE;
>>> +	dev->info = &si7020_info;
>>> +	dev->channels = si7020_channels;
>>> +	dev->num_channels = ARRAY_SIZE(si7020_channels);
>>> +
>>> +	return devm_iio_device_register(&client->dev, dev);
>>> +}
>>> +
>>> +static const struct i2c_device_id si7020_id[] = {
>>> +	{ "si7020", 0 },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, si7020_id);
>>> +
>>> +static struct i2c_driver si7020_driver = {
>>> +	.driver.name	= "si7020",
>>> +	.probe		= si7020_probe,
>>> +	.id_table	= si7020_id,
>>> +};
>>> +
>>> +module_i2c_driver(si7020_driver);
>>> +MODULE_DESCRIPTION("Silicon Labs Si7013/20/21 "
>> Trailing whitespace in string.
> 
> The end of the string is on the line below.
Good point, but I guess keeping it in one line would be the better solution. As the coding style guideline states: "However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them."
> 
>>> +		   "Relative Humidity and Temperature Sensors");
>>> +MODULE_AUTHOR("David Barksdale <dbarksdale@uplogix.com>");
>>> +MODULE_LICENSE("GPL");
>>> +
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

* Re: [PATCH v2] IIO: add si7020 driver
  2014-09-22 15:40   ` David Barksdale
@ 2014-09-22 20:16     ` Hartmut Knaack
  0 siblings, 0 replies; 6+ messages in thread
From: Hartmut Knaack @ 2014-09-22 20:16 UTC (permalink / raw)
  To: David Barksdale, Jonathan Cameron; +Cc: Peter Meerwald, linux-kernel, linux-iio

David Barksdale schrieb, Am 22.09.2014 17:40:
> 
> On 09/21/2014 07:27 AM, Hartmut Knaack wrote:
>> David Barksdale schrieb, Am 18.09.2014 22:01:
>>> This patch adds support to the Industrial IO subsystem
>>> for the Silicon Labs Si7013/20/21 Relative Humidity and
>>> Temperature Sensors.
>>>
>>> Website: http://www.silabs.com/products/sensors/humidity-sensors/Pages/si7013-20-21.aspx
>>>
>>> These are i2c devices which measure relative humidity
>>> and temperature and all use the same protocol. The
>>> Si7013 has an additional input with programmable
>>> linearization which is not supported because that's
>>> complicated and I didn't need it.
>>>
>>> Signed-off-by: David Barksdale <dbarksdale@uplogix.com>
>>>
>> Hi,
>> I'm not really convinced of the concept of treating all measurements as 16 bit data, since the datasheet states a maximum resolution of 14 bits for temperature and 12 bits for humidity. It also states, that the 2 LSBs contain an identifier rather than measured data. So, the least to do would be to mask out everything, which is not measured data (which should also include the 2 MSBs of humidity channel). Even better would be to also apply a shift and treat the data with the real resolution (especially for scale, as it would represent the sensitivity of the sensor a bit better).
>> Besides that, I think it would be a good idea to initialize the sensor during probe (for example set resolution to maximum), since you can never be sure, in which state the sensor has been before.
>> Also, find some small style issues inline.
> 
> The math done by iio_convert_raw_to_processed_unlocked loses accuracy in 
> my case because I need the offset to be fractional. It assumes offset is 
> an integer so I have to round the true offset to the nearest integer.
> If I provide IIO_CHAN_INFO_PROCESSED then I can do the math correctly.
> Would it be kosher to provide only IIO_CHAN_INFO_RAW and 
> IIO_CHAN_INFO_PROCESSED?
It would be a bit more accurate using a fractional offset, but this error is just in the range of 10 m°C or 4 m%RH. Compared to the tolerances of the sensor for temperature (at least 500 m°C) or humidity (at least 3000 m%RH), this doesn't seem relevant.
> 
>>> --
>>> Changes since v1:
>>> * Renamed to si7020 and replaced Si701x/2x with Si7013/20/21.
>>> * Removed unneeded mutex.
>>> * Pre-computed floating-point constant expressions.
>>> * Removed address_list and I2C_CLASS_HWMON.
>>>
>>> ---
>>>   drivers/iio/humidity/Kconfig  |  10 +++
>>>   drivers/iio/humidity/Makefile |   1 +
>>>   drivers/iio/humidity/si7020.c | 142 ++++++++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 153 insertions(+)
>>>
>>> diff --git a/drivers/iio/humidity/Kconfig b/drivers/iio/humidity/Kconfig
>>> index e116bd8..4813b79 100644
>>> --- a/drivers/iio/humidity/Kconfig
>>> +++ b/drivers/iio/humidity/Kconfig
>>> @@ -22,4 +22,14 @@ config SI7005
>>>   	  To compile this driver as a module, choose M here: the module
>>>   	  will be called si7005.
>>>
>>> +config SI7020
>>> +	tristate "Si7013/20/21 Relative Humidity and Temperature Sensors"
>>> +	depends on I2C
>>> +	help
>>> +	  Say yes here to build support for the Silicon Labs Si7013/20/21
>>> +	  Relative Humidity and Temperature Sensors.
>>> +
>>> +	  To compile this driver as a module, choose M here: the module
>>> +	  will be called si7020.
>>> +
>>>   endmenu
>>> diff --git a/drivers/iio/humidity/Makefile b/drivers/iio/humidity/Makefile
>>> index e3f3d94..86e2d26 100644
>>> --- a/drivers/iio/humidity/Makefile
>>> +++ b/drivers/iio/humidity/Makefile
>>> @@ -4,3 +4,4 @@
>>>
>>>   obj-$(CONFIG_DHT11) += dht11.o
>>>   obj-$(CONFIG_SI7005) += si7005.o
>>> +obj-$(CONFIG_SI7020) += si7020.o
>>> diff --git a/drivers/iio/humidity/si7020.c b/drivers/iio/humidity/si7020.c
>>> new file mode 100644
>>> index 0000000..cabdb7d
>>> --- /dev/null
>>> +++ b/drivers/iio/humidity/si7020.c
>>> @@ -0,0 +1,142 @@
>>> +/*
>>> + * si7020.c - Silicon Labs Si7013/20/21 Relative Humidity and Temp Sensors
>>> + * Copyright (c) 2013,2014  Uplogix, Inc.
>>> + * David Barksdale <dbarksdale@uplogix.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +/*
>>> + * The Silicon Labs Si7013/20/21 Relative Humidity and Temperature Sensors
>>> + * are i2c devices which have an identical programming interface for
>>> + * measuring relative humidity and temperature. The Si7013 has an additional
>>> + * temperature input which this driver does not support.
>>> + *
>>> + * Data Sheets:
>>> + *   Si7013: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7013.pdf
>>> + *   Si7020: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7020.pdf
>>> + *   Si7021: http://www.silabs.com/Support%20Documents/TechnicalDocs/Si7021.pdf
>>> + */
>>> +
>>> +#include <linux/i2c.h>
>>> +#include <linux/module.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/sysfs.h>
>>> +
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +
>>> +enum {
>>> +	/* Measure Relative Humidity, Hold Master Mode */
>>> +	SI7020CMD_RH_HOLD	= 0xE5,
>>> +	/* Measure Temperature, Hold Master Mode */
>>> +	SI7020CMD_TEMP_HOLD	= 0xE3,
>>> +};
>>> +
>>> +static int si7020_read_raw(struct iio_dev *indio_dev,
>>> +			   struct iio_chan_spec const *chan, int *val,
>>> +			   int *val2, long mask)
>>> +{
>>> +	struct i2c_client **client = iio_priv(indio_dev);
>>> +	int ret;
>>> +
>>> +	switch (mask) {
>>> +	case IIO_CHAN_INFO_RAW:
>>> +		ret = i2c_smbus_read_word_data(*client,
>>> +					       chan->type == IIO_TEMP ?
>>> +					       SI7020CMD_TEMP_HOLD :
>>> +					       SI7020CMD_RH_HOLD);
>>> +		if (ret < 0)
>>> +			return ret;
>>> +		*val = ret;
>>> +		return IIO_VAL_INT;
>>> +	case IIO_CHAN_INFO_SCALE:
>>> +		if (chan->type == IIO_TEMP)
>>> +			*val = 175720; /* = 175.72 * 1000 */
>>> +		else
>>> +			*val = 125 * 1000;
>>> +		*val2 = 65536;
>>> +		return IIO_VAL_FRACTIONAL;
>>> +	case IIO_CHAN_INFO_OFFSET:
>>> +		if (chan->type == IIO_TEMP)
>>> +			*val = -17473; /* = -46.85 * 65536 / 175.72 */
>>> +		else
>>> +			*val = -3146; /* = -6 * 65536 / 125 */
>>> +		return IIO_VAL_INT;
>>> +	default:
>>> +		break;
>>> +	}
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +
>>> +static const struct iio_chan_spec si7020_channels[] = {
>>> +	{
>>> +		.type = IIO_HUMIDITYRELATIVE,
>>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
>>> +	},
>>> +	{
>>> +		.type = IIO_TEMP,
>>> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>>> +			BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_OFFSET),
>>> +	}
>>> +};
>>> +
>>> +static const struct iio_info si7020_info = {
>>> +	.read_raw = si7020_read_raw,
>>> +	.driver_module = THIS_MODULE,
>>> +};
>>> +
>>> +static int si7020_probe(struct i2c_client *client,
>>> +			const struct i2c_device_id *id)
>>> +{
>>> +	struct iio_dev *dev;
>> Better name it indio_dev.
>>> +	struct i2c_client **data;
>>> +
>>> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA))
>>> +		return -ENODEV;
>>> +
>>> +	dev = devm_iio_device_alloc(&client->dev, sizeof(*client));
>>> +	if (!dev)
>>> +		return -ENOMEM;
>>> +
>>> +	data = iio_priv(dev);
>>> +	*data = client;
>>> +	i2c_set_clientdata(client, dev);
>>> +
>>> +	dev->dev.parent = &client->dev;
>>> +	dev->name = dev_name(&client->dev);
>>> +	dev->modes = INDIO_DIRECT_MODE;
>>> +	dev->info = &si7020_info;
>>> +	dev->channels = si7020_channels;
>>> +	dev->num_channels = ARRAY_SIZE(si7020_channels);
>>> +
>>> +	return devm_iio_device_register(&client->dev, dev);
>>> +}
>>> +
>>> +static const struct i2c_device_id si7020_id[] = {
>>> +	{ "si7020", 0 },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(i2c, si7020_id);
>>> +
>>> +static struct i2c_driver si7020_driver = {
>>> +	.driver.name	= "si7020",
>>> +	.probe		= si7020_probe,
>>> +	.id_table	= si7020_id,
>>> +};
>>> +
>>> +module_i2c_driver(si7020_driver);
>>> +MODULE_DESCRIPTION("Silicon Labs Si7013/20/21 "
>> Trailing whitespace in string.
>>> +		   "Relative Humidity and Temperature Sensors");
>>> +MODULE_AUTHOR("David Barksdale <dbarksdale@uplogix.com>");
>>> +MODULE_LICENSE("GPL");
>>> +
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

end of thread, other threads:[~2014-09-22 20:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-18 20:01 [PATCH v2] IIO: add si7020 driver David Barksdale
2014-09-21 12:27 ` Hartmut Knaack
2014-09-22 13:09   ` David Barksdale
2014-09-22 19:51     ` Hartmut Knaack
2014-09-22 15:40   ` David Barksdale
2014-09-22 20:16     ` Hartmut Knaack

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