[PATCH 0/2] Add support for Aosong AM2315 ambient temperature & humidity sensor

[PATCH 0/2] Add support for Aosong AM2315 ambient temperature & humidity sensor

[Tiberiu Breana]

This patch set adds basic support for for Aosong AM2315 ambient
temperature & humidity sensor.
Patch 1 adds basic support, readings and ACPI detection.
Patch 2 adds triggered buffer support.

Tiberiu Breana (2):
  iio: temperature: Add support for AM2315
  iio: temperature: Add triggered buffer support for AM2315

 drivers/iio/temperature/Kconfig  |  10 ++
 drivers/iio/temperature/Makefile |   1 +
 drivers/iio/temperature/am2315.c | 293 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 304 insertions(+)
 create mode 100644 drivers/iio/temperature/am2315.c

-- 
1.9.1

[PATCH 1/2] iio: temperature: Add support for AM2315

[Tiberiu Breana]

Add basic support for the Aosong AM2315 ambient temperature
and humidity sensor.
Includes support for raw readings and ACPI detection.

Datasheet:
http://www.aosong.com/asp_bin/Products/en/AM2315.pdf

Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
---
 drivers/iio/temperature/Kconfig  |  10 ++
 drivers/iio/temperature/Makefile |   1 +
 drivers/iio/temperature/am2315.c | 216 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 227 insertions(+)
 create mode 100644 drivers/iio/temperature/am2315.c

diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index c4664e5..79e4409 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -3,6 +3,16 @@
 #
 menu "Temperature sensors"
 
+config AM2315
+	tristate "Aosong AM2315 temperature and humidity sensor"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Aosong AM2315
+	  ambient temperature and humidity sensor.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called am2315.
+
 config MLX90614
 	tristate "MLX90614 contact-less infrared sensor"
 	depends on I2C
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 02bc79d..2e7521b 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -2,6 +2,7 @@
 # Makefile for industrial I/O temperature drivers
 #
 
+obj-$(CONFIG_AM2315) += am2315.o
 obj-$(CONFIG_MLX90614) += mlx90614.o
 obj-$(CONFIG_TMP006) += tmp006.o
 obj-$(CONFIG_TSYS01) += tsys01.o
diff --git a/drivers/iio/temperature/am2315.c b/drivers/iio/temperature/am2315.c
new file mode 100644
index 0000000..2642bae
--- /dev/null
+++ b/drivers/iio/temperature/am2315.c
@@ -0,0 +1,216 @@
+/**
+ * Aosong AM2315 ambient temperature & humidity sensor driver
+ *
+ * Copyright (c) 2016, Intel Corporation.
+ *
+ * This file is subject to the terms and conditions of version 2 of
+ * the GNU General Public License. See the file COPYING in the main
+ * directory of this archive for more details.
+ *
+ * 7-bit I2C address: 0x5C.
+ */
+
+#include <linux/acpi.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#define AM2315_REG_HUM_MSB			0x00
+#define AM2315_REG_HUM_LSB			0x01
+#define AM2315_REG_TEMP_MSB			0x02
+#define AM2315_REG_TEMP_LSB			0x03
+
+#define AM2315_FUNCTION_READ			0x03
+#define AM2315_HUM_OFFSET			0
+#define AM2315_TEMP_OFFSET			2
+
+#define AM2315_DRIVER_NAME			"am2315"
+
+struct am2315_data {
+	struct i2c_client *client;
+};
+
+static const struct iio_chan_spec am2315_channels[] = {
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE)
+	},
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE)
+	},
+};
+
+/* CRC calculation algorithm, as specified in the datasheet (page 13). */
+static u16 am2315_crc(u8 *data, u8 nr_bytes)
+{
+	int i;
+	u16 crc = 0xffff;
+
+	while (nr_bytes--) {
+		crc ^= *data++;
+		for (i = 0; i < 8; i++) {
+			if (crc & 0x01) {
+				crc >>= 1;
+				crc ^= 0xA001;
+			} else {
+				crc >>= 1;
+			}
+		}
+	}
+
+	return crc;
+}
+
+/* Simple function that sends a few bytes to the device to wake it up. */
+static inline void am2315_ping(struct i2c_client *client)
+{
+	i2c_smbus_read_byte_data(client, AM2315_REG_HUM_MSB);
+}
+
+static int am2315_read_data(struct i2c_client *client, u8 *rx_buf)
+{
+	int ret;
+	/* tx_buf format: <function code> <start addr> <nr of regs to read> */
+	u8 tx_buf[3] = { AM2315_FUNCTION_READ, AM2315_REG_HUM_MSB, 4 };
+	u16 crc;
+
+	/* First wake up the device. */
+	am2315_ping(client);
+
+	ret = i2c_master_send(client, tx_buf, sizeof(tx_buf));
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to send read request\n");
+		return ret;
+	}
+	/* Wait 2-3 ms, then read back the data sent by the device. */
+	usleep_range(2000, 3000);
+
+	ret = i2c_master_recv(client, rx_buf, sizeof(rx_buf));
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to read sensor data\n");
+		return ret;
+	}
+	/*
+	 * Do a CRC check on the data and compare it to the value
+	 * calculated by the device.
+	 */
+	crc = am2315_crc(rx_buf, sizeof(rx_buf) - 2);
+	if ((crc & 0xff) != rx_buf[6] || (crc >> 8) != rx_buf[7]) {
+		dev_err(&client->dev, "failed to verify sensor data\n");
+		return -1;
+	}
+
+	return ret;
+}
+
+static int am2315_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	int ret;
+	s16 value;
+	u8 rx_buf[8];
+	u8 offset;
+	struct am2315_data *data = iio_priv(indio_dev);
+	struct i2c_client *client = data->client;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		offset = (chan->type == IIO_HUMIDITYRELATIVE) ?
+				AM2315_HUM_OFFSET : AM2315_TEMP_OFFSET;
+		/* Do a bulk data read, then pick out what we need. */
+		ret = am2315_read_data(client, rx_buf);
+		if (ret < 0)
+			return ret;
+		/*
+		 * rx_buf format:
+		 * <function code> <number of registers read>
+		 * <humidity MSB> <humidity LSB> <temp MSB> <temp LSB>
+		 * <CRC LSB> <CRC MSB>
+		 */
+		value = (rx_buf[offset + 2] << 8) | rx_buf[offset + 3];
+		*val = value / 10;
+		*val2 = (value % 10) * 100000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = 100000;
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+
+	return -EINVAL;
+}
+
+static const struct iio_info am2315_info = {
+	.driver_module		= THIS_MODULE,
+	.read_raw		= am2315_read_raw,
+};
+
+static int am2315_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct iio_dev *indio_dev;
+	struct am2315_data *data;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	if (!indio_dev) {
+		dev_err(&client->dev, "iio allocation failed!\n");
+		return -ENOMEM;
+	}
+
+	data = iio_priv(indio_dev);
+	data->client = client;
+	i2c_set_clientdata(client, indio_dev);
+
+	indio_dev->dev.parent = &client->dev;
+	indio_dev->info = &am2315_info;
+	indio_dev->name = AM2315_DRIVER_NAME;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = am2315_channels;
+	indio_dev->num_channels = ARRAY_SIZE(am2315_channels);
+
+	return iio_device_register(indio_dev);
+}
+
+static int am2315_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct i2c_device_id am2315_i2c_id[] = {
+	{"am2315", 0},
+	{}
+};
+
+static const struct acpi_device_id am2315_acpi_id[] = {
+	{"AOS2315", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(acpi, am2315_acpi_id);
+
+static struct i2c_driver am2315_driver = {
+	.driver = {
+		.name = "am2315",
+		.acpi_match_table = ACPI_PTR(am2315_acpi_id),
+	},
+	.probe =            am2315_probe,
+	.remove =	    am2315_remove,
+	.id_table =         am2315_i2c_id,
+};
+
+module_i2c_driver(am2315_driver);
+
+MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
+MODULE_DESCRIPTION("Aosong AM2315 ambient temperature & humidity sensor driver");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

Re: [PATCH 1/2] iio: temperature: Add support for AM2315

[Jonathan Cameron]

On 15/04/16 16:36, Tiberiu Breana wrote:
> Add basic support for the Aosong AM2315 ambient temperature
> and humidity sensor.
> Includes support for raw readings and ACPI detection.
> 
> Datasheet:
> http://www.aosong.com/asp_bin/Products/en/AM2315.pdf
> 
> Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
I think I'd prefer this in drivers/iio/humidty/.  If people just want a temperature
sensor they'll not use the much more complex device needed to measure humidity,
whereas to measure humidity you have to know what the temperature is.  Hence I'd
argue that here the humidity side of the device in the primary one.
'Interestingly' temperature and humidity seems to sound better to me in English than
humidity and temperature... hohum.

I suppose we could roll these two directories together if people think that is sensible?

Various little bits inline..

Jonathan
> ---
>  drivers/iio/temperature/Kconfig  |  10 ++
>  drivers/iio/temperature/Makefile |   1 +
>  drivers/iio/temperature/am2315.c | 216 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 227 insertions(+)
>  create mode 100644 drivers/iio/temperature/am2315.c
> 
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index c4664e5..79e4409 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -3,6 +3,16 @@
>  #
>  menu "Temperature sensors"
>  
> +config AM2315
> +	tristate "Aosong AM2315 temperature and humidity sensor"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for the Aosong AM2315
> +	  ambient temperature and humidity sensor.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called am2315.
> +
>  config MLX90614
>  	tristate "MLX90614 contact-less infrared sensor"
>  	depends on I2C
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 02bc79d..2e7521b 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -2,6 +2,7 @@
>  # Makefile for industrial I/O temperature drivers
>  #
>  
> +obj-$(CONFIG_AM2315) += am2315.o
>  obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
>  obj-$(CONFIG_TSYS01) += tsys01.o
> diff --git a/drivers/iio/temperature/am2315.c b/drivers/iio/temperature/am2315.c
> new file mode 100644
> index 0000000..2642bae
> --- /dev/null
> +++ b/drivers/iio/temperature/am2315.c
> @@ -0,0 +1,216 @@
> +/**
> + * Aosong AM2315 ambient temperature & humidity sensor driver
> + *
> + * Copyright (c) 2016, Intel Corporation.
> + *
> + * This file is subject to the terms and conditions of version 2 of
> + * the GNU General Public License. See the file COPYING in the main
> + * directory of this archive for more details.
> + *
> + * 7-bit I2C address: 0x5C.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#define AM2315_REG_HUM_MSB			0x00
> +#define AM2315_REG_HUM_LSB			0x01
> +#define AM2315_REG_TEMP_MSB			0x02
> +#define AM2315_REG_TEMP_LSB			0x03
> +
> +#define AM2315_FUNCTION_READ			0x03
> +#define AM2315_HUM_OFFSET			0
> +#define AM2315_TEMP_OFFSET			2
> +
> +#define AM2315_DRIVER_NAME			"am2315"
> +
> +struct am2315_data {
> +	struct i2c_client *client;
> +};
> +
> +static const struct iio_chan_spec am2315_channels[] = {
> +	{
> +		.type = IIO_HUMIDITYRELATIVE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE)
> +	},
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE)
> +	},
> +};
> +
> +/* CRC calculation algorithm, as specified in the datasheet (page 13). */
> +static u16 am2315_crc(u8 *data, u8 nr_bytes)

I think this 'might' be reverse CRC16-IBM according to wikipedia...
Which is obscure enough that it probably doesn't have a generic implementation
already in the kernel...

There is one other user of this polynomial that I can fine, not enough
to justify a common implementation at this point I guess.

Note that 16bit polynomials tend to get implemented as lookup tables
if generalized.  Speed isn't that important here though I guess.

> +{
> +	int i;
> +	u16 crc = 0xffff;
> +
> +	while (nr_bytes--) {
> +		crc ^= *data++;
> +		for (i = 0; i < 8; i++) {
> +			if (crc & 0x01) {
> +				crc >>= 1;
> +				crc ^= 0xA001;
> +			} else {
> +				crc >>= 1;
> +			}
> +		}
> +	}
> +
> +	return crc;
> +}
> +
> +/* Simple function that sends a few bytes to the device to wake it up. */
We always like novel hardware corners <groans>
> +static inline void am2315_ping(struct i2c_client *client)
> +{
> +	i2c_smbus_read_byte_data(client, AM2315_REG_HUM_MSB);
> +}
> +
> +static int am2315_read_data(struct i2c_client *client, u8 *rx_buf)
> +{
> +	int ret;
> +	/* tx_buf format: <function code> <start addr> <nr of regs to read> */
> +	u8 tx_buf[3] = { AM2315_FUNCTION_READ, AM2315_REG_HUM_MSB, 4 };
> +	u16 crc;
> +
> +	/* First wake up the device. */
> +	am2315_ping(client);
> +
> +	ret = i2c_master_send(client, tx_buf, sizeof(tx_buf));
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to send read request\n");
> +		return ret;
> +	}
> +	/* Wait 2-3 ms, then read back the data sent by the device. */
> +	usleep_range(2000, 3000);
> +
> +	ret = i2c_master_recv(client, rx_buf, sizeof(rx_buf));
> +	if (ret < 0) {
> +		dev_err(&client->dev, "failed to read sensor data\n");
> +		return ret;
> +	}
> +	/*
> +	 * Do a CRC check on the data and compare it to the value
> +	 * calculated by the device.
> +	 */
> +	crc = am2315_crc(rx_buf, sizeof(rx_buf) - 2);
> +	if ((crc & 0xff) != rx_buf[6] || (crc >> 8) != rx_buf[7]) {
> +		dev_err(&client->dev, "failed to verify sensor data\n");
> +		return -1;
Provide a more informative error code... -EIO maybe...
> +	}
> +
> +	return ret;
> +}
> +
> +static int am2315_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	int ret;
> +	s16 value;
> +	u8 rx_buf[8];
> +	u8 offset;
> +	struct am2315_data *data = iio_priv(indio_dev);
> +	struct i2c_client *client = data->client;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		offset = (chan->type == IIO_HUMIDITYRELATIVE) ?
> +				AM2315_HUM_OFFSET : AM2315_TEMP_OFFSET;
> +		/* Do a bulk data read, then pick out what we need. */
> +		ret = am2315_read_data(client, rx_buf);
> +		if (ret < 0)
> +			return ret;
> +		/*
> +		 * rx_buf format:
> +		 * <function code> <number of registers read>
> +		 * <humidity MSB> <humidity LSB> <temp MSB> <temp LSB>
> +		 * <CRC LSB> <CRC MSB>
> +		 */
> +		value = (rx_buf[offset + 2] << 8) | rx_buf[offset + 3];
> +		*val = value / 10;
> +		*val2 = (value % 10) * 100000;

So for temperature this is outputing a value in degrees?  Give you are
outputting as raw, don't divide by 10 and just return IIO_VAL_INT;  Leave the maths
to userspace.

> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = 100000;
For temp to be presented in standard base units (milli degrees C) this should be
*val2 = 1000 I think...

Same for humidity - obviously this will change if you don't do the divide by
10 in the raw read out.


> +		return IIO_VAL_INT_PLUS_MICRO;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info am2315_info = {
> +	.driver_module		= THIS_MODULE,
> +	.read_raw		= am2315_read_raw,
> +};
> +
> +static int am2315_probe(struct i2c_client *client,
> +			const struct i2c_device_id *id)
> +{
> +	struct iio_dev *indio_dev;
> +	struct am2315_data *data;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	if (!indio_dev) {
> +		dev_err(&client->dev, "iio allocation failed!\n");
> +		return -ENOMEM;
> +	}
> +
> +	data = iio_priv(indio_dev);
> +	data->client = client;
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	indio_dev->dev.parent = &client->dev;
> +	indio_dev->info = &am2315_info;
> +	indio_dev->name = AM2315_DRIVER_NAME;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = am2315_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(am2315_channels);
> +
> +	return iio_device_register(indio_dev);
> +}
> +
> +static int am2315_remove(struct i2c_client *client)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +

If all you have in remove is the iio_device_unregister, you are
safe to use devm_iio_device_unregister in probe and drop the remove
entirely. (It's relatively rare that there is nothing to do in remove,
but it is to simiplify that rare case that we have the devm_iio_device_register
call)
> +	iio_device_unregister(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct i2c_device_id am2315_i2c_id[] = {
> +	{"am2315", 0},
> +	{}
> +};
> +
> +static const struct acpi_device_id am2315_acpi_id[] = {
> +	{"AOS2315", 0},
> +	{}
> +};
> +
> +MODULE_DEVICE_TABLE(acpi, am2315_acpi_id);
> +
> +static struct i2c_driver am2315_driver = {
> +	.driver = {
> +		.name = "am2315",
> +		.acpi_match_table = ACPI_PTR(am2315_acpi_id),
> +	},
> +	.probe =            am2315_probe,
> +	.remove =	    am2315_remove,
> +	.id_table =         am2315_i2c_id,
> +};
> +
> +module_i2c_driver(am2315_driver);
> +
> +MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
> +MODULE_DESCRIPTION("Aosong AM2315 ambient temperature & humidity sensor driver");
> +MODULE_LICENSE("GPL v2");
> 

RE: [PATCH 1/2] iio: temperature: Add support for AM2315

[Breana, Tiberiu A]

> -----Original Message-----
> From: Jonathan Cameron [mailto:jic23@kernel.org]
> Sent: Saturday, April 16, 2016 1:53 PM
> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
> iio@vger.kernel.org
> Subject: Re: [PATCH 1/2] iio: temperature: Add support for AM2315
> 
> On 15/04/16 16:36, Tiberiu Breana wrote:
> > Add basic support for the Aosong AM2315 ambient temperature and
> > humidity sensor.
> > Includes support for raw readings and ACPI detection.
> >
> > Datasheet:
> > http://www.aosong.com/asp_bin/Products/en/AM2315.pdf
> >
> > Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
> I think I'd prefer this in drivers/iio/humidty/.  If people just want a
> temperature sensor they'll not use the much more complex device needed
> to measure humidity, whereas to measure humidity you have to know what
> the temperature is.  Hence I'd argue that here the humidity side of the
> device in the primary one.
> 'Interestingly' temperature and humidity seems to sound better to me in
> English than humidity and temperature... hohum.
> 
> I suppose we could roll these two directories together if people think that is
> sensible?

Sounds logical. I'll move it to iio/humidity.

> 
> Various little bits inline..
> 
> Jonathan
> > ---
> >  drivers/iio/temperature/Kconfig  |  10 ++
> >  drivers/iio/temperature/Makefile |   1 +
> >  drivers/iio/temperature/am2315.c | 216
> > +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 227 insertions(+)
> >  create mode 100644 drivers/iio/temperature/am2315.c
> >
> > diff --git a/drivers/iio/temperature/Kconfig
> > b/drivers/iio/temperature/Kconfig index c4664e5..79e4409 100644
> > --- a/drivers/iio/temperature/Kconfig
> > +++ b/drivers/iio/temperature/Kconfig
> > @@ -3,6 +3,16 @@
> >  #
> >  menu "Temperature sensors"
> >
> > +config AM2315
> > +	tristate "Aosong AM2315 temperature and humidity sensor"
> > +	depends on I2C
> > +	help
> > +	  If you say yes here you get support for the Aosong AM2315
> > +	  ambient temperature and humidity sensor.
> > +
> > +	  This driver can also be built as a module. If so, the module will
> > +	  be called am2315.
> > +
> >  config MLX90614
> >  	tristate "MLX90614 contact-less infrared sensor"
> >  	depends on I2C
> > diff --git a/drivers/iio/temperature/Makefile
> > b/drivers/iio/temperature/Makefile
> > index 02bc79d..2e7521b 100644
> > --- a/drivers/iio/temperature/Makefile
> > +++ b/drivers/iio/temperature/Makefile
> > @@ -2,6 +2,7 @@
> >  # Makefile for industrial I/O temperature drivers  #
> >
> > +obj-$(CONFIG_AM2315) += am2315.o
> >  obj-$(CONFIG_MLX90614) += mlx90614.o
> >  obj-$(CONFIG_TMP006) += tmp006.o
> >  obj-$(CONFIG_TSYS01) += tsys01.o
> > diff --git a/drivers/iio/temperature/am2315.c
> > b/drivers/iio/temperature/am2315.c
> > new file mode 100644
> > index 0000000..2642bae
> > --- /dev/null
> > +++ b/drivers/iio/temperature/am2315.c
> > @@ -0,0 +1,216 @@
> > +/**
> > + * Aosong AM2315 ambient temperature & humidity sensor driver
> > + *
> > + * Copyright (c) 2016, Intel Corporation.
> > + *
> > + * This file is subject to the terms and conditions of version 2 of
> > + * the GNU General Public License. See the file COPYING in the main
> > + * directory of this archive for more details.
> > + *
> > + * 7-bit I2C address: 0x5C.
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/delay.h>
> > +#include <linux/i2c.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +
> > +#define AM2315_REG_HUM_MSB			0x00
> > +#define AM2315_REG_HUM_LSB			0x01
> > +#define AM2315_REG_TEMP_MSB			0x02
> > +#define AM2315_REG_TEMP_LSB			0x03
> > +
> > +#define AM2315_FUNCTION_READ			0x03
> > +#define AM2315_HUM_OFFSET			0
> > +#define AM2315_TEMP_OFFSET			2
> > +
> > +#define AM2315_DRIVER_NAME			"am2315"
> > +
> > +struct am2315_data {
> > +	struct i2c_client *client;
> > +};
> > +
> > +static const struct iio_chan_spec am2315_channels[] = {
> > +	{
> > +		.type = IIO_HUMIDITYRELATIVE,
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +				      BIT(IIO_CHAN_INFO_SCALE)
> > +	},
> > +	{
> > +		.type = IIO_TEMP,
> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > +				      BIT(IIO_CHAN_INFO_SCALE)
> > +	},
> > +};
> > +
> > +/* CRC calculation algorithm, as specified in the datasheet (page
> > +13). */ static u16 am2315_crc(u8 *data, u8 nr_bytes)
> 
> I think this 'might' be reverse CRC16-IBM according to wikipedia...
> Which is obscure enough that it probably doesn't have a generic
> implementation already in the kernel...
> 
> There is one other user of this polynomial that I can fine, not enough to
> justify a common implementation at this point I guess.
> 
> Note that 16bit polynomials tend to get implemented as lookup tables if
> generalized.  Speed isn't that important here though I guess.

Given the already slow nature of the device - see sleep time before data
retrieval, recommended 2s wait time before measurements, indeed,
speed is not very important here.

> 
> > +{
> > +	int i;
> > +	u16 crc = 0xffff;
> > +
> > +	while (nr_bytes--) {
> > +		crc ^= *data++;
> > +		for (i = 0; i < 8; i++) {
> > +			if (crc & 0x01) {
> > +				crc >>= 1;
> > +				crc ^= 0xA001;
> > +			} else {
> > +				crc >>= 1;
> > +			}
> > +		}
> > +	}
> > +
> > +	return crc;
> > +}
> > +
> > +/* Simple function that sends a few bytes to the device to wake it
> > +up. */
> We always like novel hardware corners <groans>
> > +static inline void am2315_ping(struct i2c_client *client) {
> > +	i2c_smbus_read_byte_data(client, AM2315_REG_HUM_MSB); }
> > +
> > +static int am2315_read_data(struct i2c_client *client, u8 *rx_buf) {
> > +	int ret;
> > +	/* tx_buf format: <function code> <start addr> <nr of regs to read>
> */
> > +	u8 tx_buf[3] = { AM2315_FUNCTION_READ,
> AM2315_REG_HUM_MSB, 4 };
> > +	u16 crc;
> > +
> > +	/* First wake up the device. */
> > +	am2315_ping(client);
> > +
> > +	ret = i2c_master_send(client, tx_buf, sizeof(tx_buf));
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "failed to send read request\n");
> > +		return ret;
> > +	}
> > +	/* Wait 2-3 ms, then read back the data sent by the device. */
> > +	usleep_range(2000, 3000);
> > +
> > +	ret = i2c_master_recv(client, rx_buf, sizeof(rx_buf));
> > +	if (ret < 0) {
> > +		dev_err(&client->dev, "failed to read sensor data\n");
> > +		return ret;
> > +	}
> > +	/*
> > +	 * Do a CRC check on the data and compare it to the value
> > +	 * calculated by the device.
> > +	 */
> > +	crc = am2315_crc(rx_buf, sizeof(rx_buf) - 2);
> > +	if ((crc & 0xff) != rx_buf[6] || (crc >> 8) != rx_buf[7]) {
> > +		dev_err(&client->dev, "failed to verify sensor data\n");
> > +		return -1;
> Provide a more informative error code... -EIO maybe...

Will do this in v2.

> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int am2315_read_raw(struct iio_dev *indio_dev,
> > +			   struct iio_chan_spec const *chan,
> > +			   int *val, int *val2, long mask) {
> > +	int ret;
> > +	s16 value;
> > +	u8 rx_buf[8];
> > +	u8 offset;
> > +	struct am2315_data *data = iio_priv(indio_dev);
> > +	struct i2c_client *client = data->client;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		offset = (chan->type == IIO_HUMIDITYRELATIVE) ?
> > +				AM2315_HUM_OFFSET :
> AM2315_TEMP_OFFSET;
> > +		/* Do a bulk data read, then pick out what we need. */
> > +		ret = am2315_read_data(client, rx_buf);
> > +		if (ret < 0)
> > +			return ret;
> > +		/*
> > +		 * rx_buf format:
> > +		 * <function code> <number of registers read>
> > +		 * <humidity MSB> <humidity LSB> <temp MSB> <temp LSB>
> > +		 * <CRC LSB> <CRC MSB>
> > +		 */
> > +		value = (rx_buf[offset + 2] << 8) | rx_buf[offset + 3];
> > +		*val = value / 10;
> > +		*val2 = (value % 10) * 100000;
> 
> So for temperature this is outputing a value in degrees?  Give you are
> outputting as raw, don't divide by 10 and just return IIO_VAL_INT;  Leave the
> maths to userspace.

Ok, will output raw values in v2.

> 
> > +		return IIO_VAL_INT_PLUS_MICRO;
> > +	case IIO_CHAN_INFO_SCALE:
> > +		*val = 0;
> > +		*val2 = 100000;
> For temp to be presented in standard base units (milli degrees C) this should
> be
> *val2 = 1000 I think...
> 
> Same for humidity - obviously this will change if you don't do the divide by
> 10 in the raw read out.

Actually if we're aiming for milli units, the scale should be 100.

> 
> 
> > +		return IIO_VAL_INT_PLUS_MICRO;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static const struct iio_info am2315_info = {
> > +	.driver_module		= THIS_MODULE,
> > +	.read_raw		= am2315_read_raw,
> > +};
> > +
> > +static int am2315_probe(struct i2c_client *client,
> > +			const struct i2c_device_id *id)
> > +{
> > +	struct iio_dev *indio_dev;
> > +	struct am2315_data *data;
> > +
> > +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> > +	if (!indio_dev) {
> > +		dev_err(&client->dev, "iio allocation failed!\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	data = iio_priv(indio_dev);
> > +	data->client = client;
> > +	i2c_set_clientdata(client, indio_dev);
> > +
> > +	indio_dev->dev.parent = &client->dev;
> > +	indio_dev->info = &am2315_info;
> > +	indio_dev->name = AM2315_DRIVER_NAME;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->channels = am2315_channels;
> > +	indio_dev->num_channels = ARRAY_SIZE(am2315_channels);
> > +
> > +	return iio_device_register(indio_dev); }
> > +
> > +static int am2315_remove(struct i2c_client *client) {
> > +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > +
> 
> If all you have in remove is the iio_device_unregister, you are safe to use
> devm_iio_device_unregister in probe and drop the remove entirely. (It's
> relatively rare that there is nothing to do in remove, but it is to simiplify that
> rare case that we have the devm_iio_device_register
> call)

I initially used the devm variant, but to reduce patch 2 diff, I've decided to add
the .remove function here and in v2 just add a iio_triggered_buffer_cleanup call.
If you don't agree with this, I can change it.

> > +	iio_device_unregister(indio_dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct i2c_device_id am2315_i2c_id[] = {
> > +	{"am2315", 0},
> > +	{}
> > +};
> > +
> > +static const struct acpi_device_id am2315_acpi_id[] = {
> > +	{"AOS2315", 0},
> > +	{}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(acpi, am2315_acpi_id);
> > +
> > +static struct i2c_driver am2315_driver = {
> > +	.driver = {
> > +		.name = "am2315",
> > +		.acpi_match_table = ACPI_PTR(am2315_acpi_id),
> > +	},
> > +	.probe =            am2315_probe,
> > +	.remove =	    am2315_remove,
> > +	.id_table =         am2315_i2c_id,
> > +};
> > +
> > +module_i2c_driver(am2315_driver);
> > +
> > +MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
> > +MODULE_DESCRIPTION("Aosong AM2315 ambient temperature &
> humidity
> > +sensor driver"); MODULE_LICENSE("GPL v2");
> >

RE: [PATCH 1/2] iio: temperature: Add support for AM2315

[jic23]

On 18.04.2016 10:40, Breana, Tiberiu A wrote:
>> -----Original Message-----
>> From: Jonathan Cameron [mailto:jic23@kernel.org]
>> Sent: Saturday, April 16, 2016 1:53 PM
>> To: Breana, Tiberiu A <tiberiu.a.breana@intel.com>; linux-
>> iio@vger.kernel.org
>> Subject: Re: [PATCH 1/2] iio: temperature: Add support for AM2315
>> 
>> On 15/04/16 16:36, Tiberiu Breana wrote:
>> > Add basic support for the Aosong AM2315 ambient temperature and
>> > humidity sensor.
>> > Includes support for raw readings and ACPI detection.
>> >
>> > Datasheet:
>> > http://www.aosong.com/asp_bin/Products/en/AM2315.pdf
>> >
>> > Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
>> I think I'd prefer this in drivers/iio/humidty/.  If people just want 
>> a
>> temperature sensor they'll not use the much more complex device needed
>> to measure humidity, whereas to measure humidity you have to know what
>> the temperature is.  Hence I'd argue that here the humidity side of 
>> the
>> device in the primary one.
>> 'Interestingly' temperature and humidity seems to sound better to me 
>> in
>> English than humidity and temperature... hohum.
>> 
>> I suppose we could roll these two directories together if people think 
>> that is
>> sensible?
> 
> Sounds logical. I'll move it to iio/humidity.
> 
>> 
>> Various little bits inline..
>> 
>> Jonathan
>> > ---
>> >  drivers/iio/temperature/Kconfig  |  10 ++
>> >  drivers/iio/temperature/Makefile |   1 +
>> >  drivers/iio/temperature/am2315.c | 216
>> > +++++++++++++++++++++++++++++++++++++++
>> >  3 files changed, 227 insertions(+)
>> >  create mode 100644 drivers/iio/temperature/am2315.c
>> >
>> > diff --git a/drivers/iio/temperature/Kconfig
>> > b/drivers/iio/temperature/Kconfig index c4664e5..79e4409 100644
>> > --- a/drivers/iio/temperature/Kconfig
>> > +++ b/drivers/iio/temperature/Kconfig
>> > @@ -3,6 +3,16 @@
>> >  #
>> >  menu "Temperature sensors"
>> >
>> > +config AM2315
>> > +	tristate "Aosong AM2315 temperature and humidity sensor"
>> > +	depends on I2C
>> > +	help
>> > +	  If you say yes here you get support for the Aosong AM2315
>> > +	  ambient temperature and humidity sensor.
>> > +
>> > +	  This driver can also be built as a module. If so, the module will
>> > +	  be called am2315.
>> > +
>> >  config MLX90614
>> >  	tristate "MLX90614 contact-less infrared sensor"
>> >  	depends on I2C
>> > diff --git a/drivers/iio/temperature/Makefile
>> > b/drivers/iio/temperature/Makefile
>> > index 02bc79d..2e7521b 100644
>> > --- a/drivers/iio/temperature/Makefile
>> > +++ b/drivers/iio/temperature/Makefile
>> > @@ -2,6 +2,7 @@
>> >  # Makefile for industrial I/O temperature drivers  #
>> >
>> > +obj-$(CONFIG_AM2315) += am2315.o
>> >  obj-$(CONFIG_MLX90614) += mlx90614.o
>> >  obj-$(CONFIG_TMP006) += tmp006.o
>> >  obj-$(CONFIG_TSYS01) += tsys01.o
>> > diff --git a/drivers/iio/temperature/am2315.c
>> > b/drivers/iio/temperature/am2315.c
>> > new file mode 100644
>> > index 0000000..2642bae
>> > --- /dev/null
>> > +++ b/drivers/iio/temperature/am2315.c
>> > @@ -0,0 +1,216 @@
>> > +/**
>> > + * Aosong AM2315 ambient temperature & humidity sensor driver
>> > + *
>> > + * Copyright (c) 2016, Intel Corporation.
>> > + *
>> > + * This file is subject to the terms and conditions of version 2 of
>> > + * the GNU General Public License. See the file COPYING in the main
>> > + * directory of this archive for more details.
>> > + *
>> > + * 7-bit I2C address: 0x5C.
>> > + */
>> > +
>> > +#include <linux/acpi.h>
>> > +#include <linux/delay.h>
>> > +#include <linux/i2c.h>
>> > +#include <linux/kernel.h>
>> > +#include <linux/module.h>
>> > +#include <linux/iio/iio.h>
>> > +#include <linux/iio/sysfs.h>
>> > +
>> > +#define AM2315_REG_HUM_MSB			0x00
>> > +#define AM2315_REG_HUM_LSB			0x01
>> > +#define AM2315_REG_TEMP_MSB			0x02
>> > +#define AM2315_REG_TEMP_LSB			0x03
>> > +
>> > +#define AM2315_FUNCTION_READ			0x03
>> > +#define AM2315_HUM_OFFSET			0
>> > +#define AM2315_TEMP_OFFSET			2
>> > +
>> > +#define AM2315_DRIVER_NAME			"am2315"
>> > +
>> > +struct am2315_data {
>> > +	struct i2c_client *client;
>> > +};
>> > +
>> > +static const struct iio_chan_spec am2315_channels[] = {
>> > +	{
>> > +		.type = IIO_HUMIDITYRELATIVE,
>> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> > +				      BIT(IIO_CHAN_INFO_SCALE)
>> > +	},
>> > +	{
>> > +		.type = IIO_TEMP,
>> > +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>> > +				      BIT(IIO_CHAN_INFO_SCALE)
>> > +	},
>> > +};
>> > +
>> > +/* CRC calculation algorithm, as specified in the datasheet (page
>> > +13). */ static u16 am2315_crc(u8 *data, u8 nr_bytes)
>> 
>> I think this 'might' be reverse CRC16-IBM according to wikipedia...
>> Which is obscure enough that it probably doesn't have a generic
>> implementation already in the kernel...
>> 
>> There is one other user of this polynomial that I can fine, not enough 
>> to
>> justify a common implementation at this point I guess.
>> 
>> Note that 16bit polynomials tend to get implemented as lookup tables 
>> if
>> generalized.  Speed isn't that important here though I guess.
> 
> Given the already slow nature of the device - see sleep time before 
> data
> retrieval, recommended 2s wait time before measurements, indeed,
> speed is not very important here.
> 
>> 
>> > +{
>> > +	int i;
>> > +	u16 crc = 0xffff;
>> > +
>> > +	while (nr_bytes--) {
>> > +		crc ^= *data++;
>> > +		for (i = 0; i < 8; i++) {
>> > +			if (crc & 0x01) {
>> > +				crc >>= 1;
>> > +				crc ^= 0xA001;
>> > +			} else {
>> > +				crc >>= 1;
>> > +			}
>> > +		}
>> > +	}
>> > +
>> > +	return crc;
>> > +}
>> > +
>> > +/* Simple function that sends a few bytes to the device to wake it
>> > +up. */
>> We always like novel hardware corners <groans>
>> > +static inline void am2315_ping(struct i2c_client *client) {
>> > +	i2c_smbus_read_byte_data(client, AM2315_REG_HUM_MSB); }
>> > +
>> > +static int am2315_read_data(struct i2c_client *client, u8 *rx_buf) {
>> > +	int ret;
>> > +	/* tx_buf format: <function code> <start addr> <nr of regs to read>
>> */
>> > +	u8 tx_buf[3] = { AM2315_FUNCTION_READ,
>> AM2315_REG_HUM_MSB, 4 };
>> > +	u16 crc;
>> > +
>> > +	/* First wake up the device. */
>> > +	am2315_ping(client);
>> > +
>> > +	ret = i2c_master_send(client, tx_buf, sizeof(tx_buf));
>> > +	if (ret < 0) {
>> > +		dev_err(&client->dev, "failed to send read request\n");
>> > +		return ret;
>> > +	}
>> > +	/* Wait 2-3 ms, then read back the data sent by the device. */
>> > +	usleep_range(2000, 3000);
>> > +
>> > +	ret = i2c_master_recv(client, rx_buf, sizeof(rx_buf));
>> > +	if (ret < 0) {
>> > +		dev_err(&client->dev, "failed to read sensor data\n");
>> > +		return ret;
>> > +	}
>> > +	/*
>> > +	 * Do a CRC check on the data and compare it to the value
>> > +	 * calculated by the device.
>> > +	 */
>> > +	crc = am2315_crc(rx_buf, sizeof(rx_buf) - 2);
>> > +	if ((crc & 0xff) != rx_buf[6] || (crc >> 8) != rx_buf[7]) {
>> > +		dev_err(&client->dev, "failed to verify sensor data\n");
>> > +		return -1;
>> Provide a more informative error code... -EIO maybe...
> 
> Will do this in v2.
> 
>> > +	}
>> > +
>> > +	return ret;
>> > +}
>> > +
>> > +static int am2315_read_raw(struct iio_dev *indio_dev,
>> > +			   struct iio_chan_spec const *chan,
>> > +			   int *val, int *val2, long mask) {
>> > +	int ret;
>> > +	s16 value;
>> > +	u8 rx_buf[8];
>> > +	u8 offset;
>> > +	struct am2315_data *data = iio_priv(indio_dev);
>> > +	struct i2c_client *client = data->client;
>> > +
>> > +	switch (mask) {
>> > +	case IIO_CHAN_INFO_RAW:
>> > +		offset = (chan->type == IIO_HUMIDITYRELATIVE) ?
>> > +				AM2315_HUM_OFFSET :
>> AM2315_TEMP_OFFSET;
>> > +		/* Do a bulk data read, then pick out what we need. */
>> > +		ret = am2315_read_data(client, rx_buf);
>> > +		if (ret < 0)
>> > +			return ret;
>> > +		/*
>> > +		 * rx_buf format:
>> > +		 * <function code> <number of registers read>
>> > +		 * <humidity MSB> <humidity LSB> <temp MSB> <temp LSB>
>> > +		 * <CRC LSB> <CRC MSB>
>> > +		 */
>> > +		value = (rx_buf[offset + 2] << 8) | rx_buf[offset + 3];
>> > +		*val = value / 10;
>> > +		*val2 = (value % 10) * 100000;
>> 
>> So for temperature this is outputing a value in degrees?  Give you are
>> outputting as raw, don't divide by 10 and just return IIO_VAL_INT;  
>> Leave the
>> maths to userspace.
> 
> Ok, will output raw values in v2.
> 
>> 
>> > +		return IIO_VAL_INT_PLUS_MICRO;
>> > +	case IIO_CHAN_INFO_SCALE:
>> > +		*val = 0;
>> > +		*val2 = 100000;
>> For temp to be presented in standard base units (milli degrees C) this 
>> should
>> be
>> *val2 = 1000 I think...
>> 
>> Same for humidity - obviously this will change if you don't do the 
>> divide by
>> 10 in the raw read out.
> 
> Actually if we're aiming for milli units, the scale should be 100.
Yeah, I was getting of these wrong yesterday for some reason!

Jonathan
> 
>> 
>> 
>> > +		return IIO_VAL_INT_PLUS_MICRO;
>> > +	}
>> > +
>> > +	return -EINVAL;
>> > +}
>> > +
>> > +static const struct iio_info am2315_info = {
>> > +	.driver_module		= THIS_MODULE,
>> > +	.read_raw		= am2315_read_raw,
>> > +};
>> > +
>> > +static int am2315_probe(struct i2c_client *client,
>> > +			const struct i2c_device_id *id)
>> > +{
>> > +	struct iio_dev *indio_dev;
>> > +	struct am2315_data *data;
>> > +
>> > +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>> > +	if (!indio_dev) {
>> > +		dev_err(&client->dev, "iio allocation failed!\n");
>> > +		return -ENOMEM;
>> > +	}
>> > +
>> > +	data = iio_priv(indio_dev);
>> > +	data->client = client;
>> > +	i2c_set_clientdata(client, indio_dev);
>> > +
>> > +	indio_dev->dev.parent = &client->dev;
>> > +	indio_dev->info = &am2315_info;
>> > +	indio_dev->name = AM2315_DRIVER_NAME;
>> > +	indio_dev->modes = INDIO_DIRECT_MODE;
>> > +	indio_dev->channels = am2315_channels;
>> > +	indio_dev->num_channels = ARRAY_SIZE(am2315_channels);
>> > +
>> > +	return iio_device_register(indio_dev); }
>> > +
>> > +static int am2315_remove(struct i2c_client *client) {
>> > +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>> > +
>> 
>> If all you have in remove is the iio_device_unregister, you are safe 
>> to use
>> devm_iio_device_unregister in probe and drop the remove entirely. 
>> (It's
>> relatively rare that there is nothing to do in remove, but it is to 
>> simiplify that
>> rare case that we have the devm_iio_device_register
>> call)
> 
> I initially used the devm variant, but to reduce patch 2 diff, I've
> decided to add
> the .remove function here and in v2 just add a
> iio_triggered_buffer_cleanup call.
> If you don't agree with this, I can change it.
That's fine as is.
> 
>> > +	iio_device_unregister(indio_dev);
>> > +
>> > +	return 0;
>> > +}
>> > +
>> > +static const struct i2c_device_id am2315_i2c_id[] = {
>> > +	{"am2315", 0},
>> > +	{}
>> > +};
>> > +
>> > +static const struct acpi_device_id am2315_acpi_id[] = {
>> > +	{"AOS2315", 0},
>> > +	{}
>> > +};
>> > +
>> > +MODULE_DEVICE_TABLE(acpi, am2315_acpi_id);
>> > +
>> > +static struct i2c_driver am2315_driver = {
>> > +	.driver = {
>> > +		.name = "am2315",
>> > +		.acpi_match_table = ACPI_PTR(am2315_acpi_id),
>> > +	},
>> > +	.probe =            am2315_probe,
>> > +	.remove =	    am2315_remove,
>> > +	.id_table =         am2315_i2c_id,
>> > +};
>> > +
>> > +module_i2c_driver(am2315_driver);
>> > +
>> > +MODULE_AUTHOR("Tiberiu Breana <tiberiu.a.breana@intel.com>");
>> > +MODULE_DESCRIPTION("Aosong AM2315 ambient temperature &
>> humidity
>> > +sensor driver"); MODULE_LICENSE("GPL v2");
>> >
> 
> --
> 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

[PATCH 2/2] iio: temperature: Add triggered buffer support for AM2315

[Tiberiu Breana]

Signed-off-by: Tiberiu Breana <tiberiu.a.breana@intel.com>
---
 drivers/iio/temperature/am2315.c | 83 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 80 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/temperature/am2315.c b/drivers/iio/temperature/am2315.c
index 2642bae..c65653c 100644
--- a/drivers/iio/temperature/am2315.c
+++ b/drivers/iio/temperature/am2315.c
@@ -15,8 +15,11 @@
 #include <linux/i2c.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 #define AM2315_REG_HUM_MSB			0x00
 #define AM2315_REG_HUM_LSB			0x01
@@ -26,24 +29,43 @@
 #define AM2315_FUNCTION_READ			0x03
 #define AM2315_HUM_OFFSET			0
 #define AM2315_TEMP_OFFSET			2
+#define AM2315_ALL_CHANNEL_MASK			GENMASK(1, 0)
 
 #define AM2315_DRIVER_NAME			"am2315"
 
 struct am2315_data {
 	struct i2c_client *client;
+	struct mutex lock;
+	s16 buffer[8]; /* 2x16-bit channels + 2x16 padding + 4x16 timestamp */
 };
 
 static const struct iio_chan_spec am2315_channels[] = {
 	{
 		.type = IIO_HUMIDITYRELATIVE,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-				      BIT(IIO_CHAN_INFO_SCALE)
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_CPU,
+		},
 	},
 	{
 		.type = IIO_TEMP,
 		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
-				      BIT(IIO_CHAN_INFO_SCALE)
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 16,
+			.storagebits = 16,
+			.endianness = IIO_CPU,
+		},
+
 	},
+		IIO_CHAN_SOFT_TIMESTAMP(2),
 };
 
 /* CRC calculation algorithm, as specified in the datasheet (page 13). */
@@ -109,6 +131,43 @@ static int am2315_read_data(struct i2c_client *client, u8 *rx_buf)
 	return ret;
 }
 
+static irqreturn_t am2315_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct am2315_data *data = iio_priv(indio_dev);
+	int ret;
+	int bit;
+	int i;
+	u8 rx_buf[8];
+
+	mutex_lock(&data->lock);
+	ret = am2315_read_data(data->client, rx_buf);
+	if (ret < 0) {
+		mutex_unlock(&data->lock);
+		goto err;
+	}
+	if (*(indio_dev->active_scan_mask) == AM2315_ALL_CHANNEL_MASK) {
+		data->buffer[0] = (rx_buf[2] << 8) | rx_buf[3];
+		data->buffer[1] = (rx_buf[4] << 8) | rx_buf[5];
+	} else {
+		i = 0;
+		for_each_set_bit(bit, indio_dev->active_scan_mask,
+				 indio_dev->masklength) {
+			data->buffer[i] = (rx_buf[(bit * 2) + 2] << 8) |
+					   rx_buf[(bit * 2) + 3];
+			i++;
+		}
+	}
+	mutex_unlock(&data->lock);
+
+	iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
+					   pf->timestamp);
+err:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
 static int am2315_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long mask)
@@ -155,6 +214,7 @@ static const struct iio_info am2315_info = {
 static int am2315_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
+	int ret;
 	struct iio_dev *indio_dev;
 	struct am2315_data *data;
 
@@ -167,6 +227,7 @@ static int am2315_probe(struct i2c_client *client,
 	data = iio_priv(indio_dev);
 	data->client = client;
 	i2c_set_clientdata(client, indio_dev);
+	mutex_init(&data->lock);
 
 	indio_dev->dev.parent = &client->dev;
 	indio_dev->info = &am2315_info;
@@ -175,7 +236,22 @@ static int am2315_probe(struct i2c_client *client,
 	indio_dev->channels = am2315_channels;
 	indio_dev->num_channels = ARRAY_SIZE(am2315_channels);
 
-	return iio_device_register(indio_dev);
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 am2315_trigger_handler, NULL);
+	if (ret < 0) {
+		dev_err(&client->dev, "iio triggered buffer setup failed\n");
+		return ret;
+	}
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		goto err_buffer_cleanup;
+
+	return 0;
+
+err_buffer_cleanup:
+	iio_triggered_buffer_cleanup(indio_dev);
+	return ret;
 }
 
 static int am2315_remove(struct i2c_client *client)
@@ -183,6 +259,7 @@ static int am2315_remove(struct i2c_client *client)
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 
 	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
 
 	return 0;
 }
-- 
1.9.1