[PATCH v2] iio: temperature: add support for Maxim thermocouple chips

[PATCH v2] iio: temperature: add support for Maxim thermocouple chips

[Matt Ranostay]

Add initial driver support for MAX6675, and MAX31885 thermocouple chips.

Cc: Marek Vasut <marex@denx.de>
Cc: Matt Porter <mporter@konsulko.com>
Signed-off-by: Matt Ranostay <mranostay@gmail.com>
---
Changes from v1:
* switch to iio_device_*_direct_mode wrappers
* add const to structs
* removed useless cast

.../iio/temperature/maxim_thermocouple.txt         |  21 ++
 drivers/iio/temperature/Kconfig                    |  14 ++
 drivers/iio/temperature/Makefile                   |   1 +
 drivers/iio/temperature/maxim_thermocouple.c       | 273 +++++++++++++++++++++
 4 files changed, 309 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
 create mode 100644 drivers/iio/temperature/maxim_thermocouple.c

diff --git a/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
new file mode 100644
index 0000000..44fb029
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
@@ -0,0 +1,21 @@
+Maxim thermocouple support
+
+* https://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
+* https://datasheets.maximintegrated.com/en/ds/MAX31855.pdf
+
+Required properties:
+
+	- compatible: must be "maxim,max31885" or "maxim,max6675"
+	- reg: SPI chip select number for the device
+	- spi-max-frequency: must be 4300000
+	- spi-cpha: must be defined for max6675 to enable SPI mode 1
+
+	Refer to spi/spi-bus.txt for generic SPI slave bindings.
+
+Example:
+
+	max31885@0 {
+		compatible = "maxim,max31885";
+		reg = <0>;
+		spi-max-frequency = <4300000>;
+	};
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index c4664e5..c9f5342 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -3,6 +3,20 @@
 #
 menu "Temperature sensors"
 
+config MAXIM_THERMOCOUPLE
+	tristate "Maxim thermocouple sensors"
+	depends on SPI
+	help
+	  If you say yes here you get support for the Maxim series of
+	  thermocouple sensors connected via SPI.
+
+	  Supported sensors:
+	   * MAX6675
+	   * MAX31885
+
+	  This driver can also be built as a module. If so, the module will
+	  be called maxim_thermocouple.
+
 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..78c3de0 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -2,6 +2,7 @@
 # Makefile for industrial I/O temperature drivers
 #
 
+obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
 obj-$(CONFIG_MLX90614) += mlx90614.o
 obj-$(CONFIG_TMP006) += tmp006.o
 obj-$(CONFIG_TSYS01) += tsys01.o
diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
new file mode 100644
index 0000000..dffe3c2
--- /dev/null
+++ b/drivers/iio/temperature/maxim_thermocouple.c
@@ -0,0 +1,273 @@
+/*
+ * maxim_thermocouple.c  - Support for Maxim thermocouple chips
+ *
+ * Copyright (C) 2016 Matt Ranostay <mranostay@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/mutex.h>
+#include <linux/err.h>
+#include <linux/spi/spi.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+
+#define MAXIM_THERMOCOUPLE_DRV_NAME	"maxim_thermocouple"
+
+enum {
+	MAX6675,
+	MAX31885,
+};
+
+const struct iio_chan_spec max6675_channels[] = {
+	{	/* thermocouple temperature */
+		.type = IIO_TEMP,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 13,
+			.storagebits = 16,
+			.shift = 3,
+			.endianness = IIO_BE,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(1),
+};
+
+const struct iio_chan_spec max31885_channels[] = {
+	{	/* thermocouple temperature */
+		.type = IIO_TEMP,
+		.address = 2,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 0,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 14,
+			.storagebits = 16,
+			.shift = 2,
+			.endianness = IIO_BE,
+		},
+	},
+	{	/* cold junction temperature */
+		.type = IIO_TEMP,
+		.channel2 = IIO_MOD_TEMP_AMBIENT,
+		.modified = 1,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+		.scan_index = 1,
+		.scan_type = {
+			.sign = 's',
+			.realbits = 12,
+			.storagebits = 16,
+			.shift = 4,
+			.endianness = IIO_BE,
+		},
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(2),
+};
+
+struct maxim_thermocouple_chip {
+	const struct iio_chan_spec *channels;
+	int num_channels;
+	int read_size;
+
+	/* bit-check for valid input */
+	int status_bit;
+};
+
+const struct maxim_thermocouple_chip maxim_thermocouple_chips[] = {
+	[MAX6675] = {
+			.channels = max6675_channels,
+			.num_channels = ARRAY_SIZE(max6675_channels),
+			.read_size = 2,
+			.status_bit = BIT(2),
+		},
+	[MAX31885] = {
+			.channels = max31885_channels,
+			.num_channels = ARRAY_SIZE(max31885_channels),
+			.read_size = 4,
+			.status_bit = BIT(16),
+		},
+};
+
+struct maxim_thermocouple_data {
+	struct spi_device *spi;
+	const struct maxim_thermocouple_chip *chip;
+
+	u8 buffer[16]; /* 4 byte data + 4 byte padding + 8 byte timestamp */
+};
+
+static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
+				   struct iio_chan_spec const *chan, int *val)
+{
+	unsigned int storage_bytes = data->chip->read_size;
+	unsigned int shift = chan->scan_type.shift + (chan->address * 8);
+	unsigned int buf;
+	int ret;
+
+	ret = spi_read(data->spi, (void *) &buf, storage_bytes);
+	if (ret)
+		return ret;
+
+	switch (storage_bytes) {
+	case 2:
+		*val = be16_to_cpu(buf);
+		break;
+	case 4:
+		*val = be32_to_cpu(buf);
+		break;
+	}
+
+	/* check to be sure this is a valid reading */
+	if (*val & data->chip->status_bit)
+		return -EINVAL;
+
+	*val = sign_extend32(*val >> shift, chan->scan_type.realbits - 1);
+
+	return 0;
+}
+
+static irqreturn_t maxim_thermocouple_trigger_handler(int irq, void *private)
+{
+	struct iio_poll_func *pf = private;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct maxim_thermocouple_data *data = iio_priv(indio_dev);
+	int ret;
+
+	/* userspace HAL must do validation of data pushed to buffers */
+	ret = spi_read(data->spi, &data->buffer, data->chip->read_size);
+	if (!ret)
+		iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
+						   iio_get_time_ns());
+
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int maxim_thermocouple_read_raw(struct iio_dev *indio_dev,
+				       struct iio_chan_spec const *chan,
+				       int *val, int *val2, long mask)
+{
+	struct maxim_thermocouple_data *data = iio_priv(indio_dev);
+	int ret = -EINVAL;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (iio_device_claim_direct_mode(indio_dev))
+			return -EBUSY;
+
+		ret = maxim_thermocouple_read(data, chan, val);
+		if (!ret)
+			ret = IIO_VAL_INT;
+
+		iio_device_release_direct_mode(indio_dev);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->channel2) {
+		case IIO_MOD_TEMP_AMBIENT:
+			*val = 62;
+			*val2 = 500000; /* 1000 * 0.0625 */
+			ret = IIO_VAL_INT_PLUS_MICRO;
+			break;
+		default:
+			*val = 250; /* 1000 * 0.25 */
+			ret = IIO_VAL_INT;
+		};
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info maxim_thermocouple_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = maxim_thermocouple_read_raw,
+};
+
+static int maxim_thermocouple_probe(struct spi_device *spi)
+{
+	const struct spi_device_id *id = spi_get_device_id(spi);
+	struct iio_dev *indio_dev;
+	struct maxim_thermocouple_data *data;
+	const struct maxim_thermocouple_chip *chip =
+			&maxim_thermocouple_chips[id->driver_data];
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	indio_dev->info = &maxim_thermocouple_info;
+	indio_dev->name = MAXIM_THERMOCOUPLE_DRV_NAME;
+	indio_dev->channels = chip->channels;
+	indio_dev->num_channels = chip->num_channels;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	data = iio_priv(indio_dev);
+	data->spi = spi;
+	data->chip = chip;
+
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+				maxim_thermocouple_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto error_unreg_buffer;
+
+	return 0;
+
+error_unreg_buffer:
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	return ret;
+}
+
+static int maxim_thermocouple_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+	iio_triggered_buffer_cleanup(indio_dev);
+
+	return 0;
+}
+
+static const struct spi_device_id maxim_thermocouple_id[] = {
+	{"max6675", MAX6675},
+	{"max31885", MAX31885},
+	{},
+};
+MODULE_DEVICE_TABLE(spi, maxim_thermocouple_id);
+
+static struct spi_driver maxim_thermocouple_driver = {
+	.driver = {
+		.name	= MAXIM_THERMOCOUPLE_DRV_NAME,
+	},
+	.probe		= maxim_thermocouple_probe,
+	.remove		= maxim_thermocouple_remove,
+	.id_table	= maxim_thermocouple_id,
+};
+module_spi_driver(maxim_thermocouple_driver);
+
+MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
+MODULE_DESCRIPTION("Maxim thermocouple sensors");
+MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH v2] iio: temperature: add support for Maxim thermocouple chips

[Peter Meerwald-Stadler]

> Add initial driver support for MAX6675, and MAX31885 thermocouple chips.

comments below
 
> Cc: Marek Vasut <marex@denx.de>
> Cc: Matt Porter <mporter@konsulko.com>
> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
> ---
> Changes from v1:
> * switch to iio_device_*_direct_mode wrappers
> * add const to structs
> * removed useless cast
> 
> .../iio/temperature/maxim_thermocouple.txt         |  21 ++
>  drivers/iio/temperature/Kconfig                    |  14 ++
>  drivers/iio/temperature/Makefile                   |   1 +
>  drivers/iio/temperature/maxim_thermocouple.c       | 273 +++++++++++++++++++++
>  4 files changed, 309 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>  create mode 100644 drivers/iio/temperature/maxim_thermocouple.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
> new file mode 100644
> index 0000000..44fb029
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
> @@ -0,0 +1,21 @@
> +Maxim thermocouple support
> +
> +* https://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
> +* https://datasheets.maximintegrated.com/en/ds/MAX31855.pdf
> +
> +Required properties:
> +
> +	- compatible: must be "maxim,max31885" or "maxim,max6675"
> +	- reg: SPI chip select number for the device
> +	- spi-max-frequency: must be 4300000
> +	- spi-cpha: must be defined for max6675 to enable SPI mode 1
> +
> +	Refer to spi/spi-bus.txt for generic SPI slave bindings.
> +
> +Example:
> +
> +	max31885@0 {
> +		compatible = "maxim,max31885";
> +		reg = <0>;
> +		spi-max-frequency = <4300000>;
> +	};
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index c4664e5..c9f5342 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -3,6 +3,20 @@
>  #
>  menu "Temperature sensors"
>  
> +config MAXIM_THERMOCOUPLE
> +	tristate "Maxim thermocouple sensors"
> +	depends on SPI
> +	help
> +	  If you say yes here you get support for the Maxim series of
> +	  thermocouple sensors connected via SPI.
> +
> +	  Supported sensors:
> +	   * MAX6675
> +	   * MAX31885
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called maxim_thermocouple.
> +
>  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..78c3de0 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -2,6 +2,7 @@
>  # Makefile for industrial I/O temperature drivers
>  #
>  
> +obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
>  obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
>  obj-$(CONFIG_TSYS01) += tsys01.o
> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
> new file mode 100644
> index 0000000..dffe3c2
> --- /dev/null
> +++ b/drivers/iio/temperature/maxim_thermocouple.c
> @@ -0,0 +1,273 @@
> +/*
> + * maxim_thermocouple.c  - Support for Maxim thermocouple chips
> + *
> + * Copyright (C) 2016 Matt Ranostay <mranostay@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/mutex.h>
> +#include <linux/err.h>
> +#include <linux/spi/spi.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +
> +#define MAXIM_THERMOCOUPLE_DRV_NAME	"maxim_thermocouple"
> +
> +enum {
> +	MAX6675,
> +	MAX31885,
> +};
> +
> +const struct iio_chan_spec max6675_channels[] = {
> +	{	/* thermocouple temperature */
> +		.type = IIO_TEMP,

.address seems to be used; maybe make it explicit by setting it 0

> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 13,
> +			.storagebits = 16,
> +			.shift = 3,
> +			.endianness = IIO_BE,
> +		},
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(1),
> +};
> +
> +const struct iio_chan_spec max31885_channels[] = {
> +	{	/* thermocouple temperature */
> +		.type = IIO_TEMP,
> +		.address = 2,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 0,
> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 14,
> +			.storagebits = 16,
> +			.shift = 2,
> +			.endianness = IIO_BE,
> +		},
> +	},
> +	{	/* cold junction temperature */
> +		.type = IIO_TEMP,
> +		.channel2 = IIO_MOD_TEMP_AMBIENT,
> +		.modified = 1,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
> +		.scan_index = 1,
> +		.scan_type = {
> +			.sign = 's',
> +			.realbits = 12,
> +			.storagebits = 16,
> +			.shift = 4,
> +			.endianness = IIO_BE,
> +		},
> +	},
> +	IIO_CHAN_SOFT_TIMESTAMP(2),
> +};
> +
> +struct maxim_thermocouple_chip {
> +	const struct iio_chan_spec *channels;
> +	int num_channels;
> +	int read_size;
> +
> +	/* bit-check for valid input */
> +	int status_bit;
> +};
> +
> +const struct maxim_thermocouple_chip maxim_thermocouple_chips[] = {
> +	[MAX6675] = {
> +			.channels = max6675_channels,
> +			.num_channels = ARRAY_SIZE(max6675_channels),
> +			.read_size = 2,
> +			.status_bit = BIT(2),
> +		},
> +	[MAX31885] = {
> +			.channels = max31885_channels,
> +			.num_channels = ARRAY_SIZE(max31885_channels),
> +			.read_size = 4,
> +			.status_bit = BIT(16),
> +		},
> +};
> +
> +struct maxim_thermocouple_data {
> +	struct spi_device *spi;
> +	const struct maxim_thermocouple_chip *chip;
> +
> +	u8 buffer[16]; /* 4 byte data + 4 byte padding + 8 byte timestamp */
> +};
> +
> +static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
> +				   struct iio_chan_spec const *chan, int *val)
> +{
> +	unsigned int storage_bytes = data->chip->read_size;
> +	unsigned int shift = chan->scan_type.shift + (chan->address * 8);
> +	unsigned int buf;
> +	int ret;
> +
> +	ret = spi_read(data->spi, (void *) &buf, storage_bytes);
> +	if (ret)
> +		return ret;
> +
> +	switch (storage_bytes) {
> +	case 2:
> +		*val = be16_to_cpu(buf);
> +		break;
> +	case 4:
> +		*val = be32_to_cpu(buf);
> +		break;
> +	}
> +
> +	/* check to be sure this is a valid reading */
> +	if (*val & data->chip->status_bit)
> +		return -EINVAL;
> +
> +	*val = sign_extend32(*val >> shift, chan->scan_type.realbits - 1);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t maxim_thermocouple_trigger_handler(int irq, void *private)
> +{
> +	struct iio_poll_func *pf = private;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct maxim_thermocouple_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	/* userspace HAL must do validation of data pushed to buffers */
> +	ret = spi_read(data->spi, &data->buffer, data->chip->read_size);

read_size can be 4 (bytes) but storagebits can be 16?!

> +	if (!ret)
> +		iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
> +						   iio_get_time_ns());
> +
> +	iio_trigger_notify_done(indio_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int maxim_thermocouple_read_raw(struct iio_dev *indio_dev,
> +				       struct iio_chan_spec const *chan,
> +				       int *val, int *val2, long mask)
> +{
> +	struct maxim_thermocouple_data *data = iio_priv(indio_dev);
> +	int ret = -EINVAL;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (iio_device_claim_direct_mode(indio_dev))
> +			return -EBUSY;
> +
> +		ret = maxim_thermocouple_read(data, chan, val);
> +		if (!ret)
> +			ret = IIO_VAL_INT;
> +
> +		iio_device_release_direct_mode(indio_dev);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->channel2) {
> +		case IIO_MOD_TEMP_AMBIENT:
> +			*val = 62;
> +			*val2 = 500000; /* 1000 * 0.0625 */
> +			ret = IIO_VAL_INT_PLUS_MICRO;
> +			break;
> +		default:
> +			*val = 250; /* 1000 * 0.25 */
> +			ret = IIO_VAL_INT;
> +		};
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info maxim_thermocouple_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = maxim_thermocouple_read_raw,
> +};
> +
> +static int maxim_thermocouple_probe(struct spi_device *spi)
> +{
> +	const struct spi_device_id *id = spi_get_device_id(spi);
> +	struct iio_dev *indio_dev;
> +	struct maxim_thermocouple_data *data;
> +	const struct maxim_thermocouple_chip *chip =
> +			&maxim_thermocouple_chips[id->driver_data];
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	indio_dev->info = &maxim_thermocouple_info;
> +	indio_dev->name = MAXIM_THERMOCOUPLE_DRV_NAME;
> +	indio_dev->channels = chip->channels;
> +	indio_dev->num_channels = chip->num_channels;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	data = iio_priv(indio_dev);
> +	data->spi = spi;
> +	data->chip = chip;
> +
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +				maxim_thermocouple_trigger_handler, NULL);
> +	if (ret)
> +		return ret;
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto error_unreg_buffer;
> +
> +	return 0;
> +
> +error_unreg_buffer:
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
> +	return ret;
> +}
> +
> +static int maxim_thermocouple_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	iio_device_unregister(indio_dev);
> +	iio_triggered_buffer_cleanup(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct spi_device_id maxim_thermocouple_id[] = {
> +	{"max6675", MAX6675},
> +	{"max31885", MAX31885},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, maxim_thermocouple_id);
> +
> +static struct spi_driver maxim_thermocouple_driver = {
> +	.driver = {
> +		.name	= MAXIM_THERMOCOUPLE_DRV_NAME,
> +	},
> +	.probe		= maxim_thermocouple_probe,
> +	.remove		= maxim_thermocouple_remove,
> +	.id_table	= maxim_thermocouple_id,
> +};
> +module_spi_driver(maxim_thermocouple_driver);
> +
> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
> +MODULE_DESCRIPTION("Maxim thermocouple sensors");
> +MODULE_LICENSE("GPL");
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)

Re: [PATCH v2] iio: temperature: add support for Maxim thermocouple chips

[Jonathan Cameron]

On 29/05/16 09:16, Peter Meerwald-Stadler wrote:
> 
>> Add initial driver support for MAX6675, and MAX31885 thermocouple chips.
> 
> comments below
Not much to add other than a question inline...

Jonathan
>  
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Matt Porter <mporter@konsulko.com>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>> Changes from v1:
>> * switch to iio_device_*_direct_mode wrappers
>> * add const to structs
>> * removed useless cast
>>
>> .../iio/temperature/maxim_thermocouple.txt         |  21 ++
>>  drivers/iio/temperature/Kconfig                    |  14 ++
>>  drivers/iio/temperature/Makefile                   |   1 +
>>  drivers/iio/temperature/maxim_thermocouple.c       | 273 +++++++++++++++++++++
>>  4 files changed, 309 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>  create mode 100644 drivers/iio/temperature/maxim_thermocouple.c
>>
>> diff --git a/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>> new file mode 100644
>> index 0000000..44fb029
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>> @@ -0,0 +1,21 @@
>> +Maxim thermocouple support
>> +
>> +* https://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>> +* https://datasheets.maximintegrated.com/en/ds/MAX31855.pdf
>> +
>> +Required properties:
>> +
>> +	- compatible: must be "maxim,max31885" or "maxim,max6675"
>> +	- reg: SPI chip select number for the device
>> +	- spi-max-frequency: must be 4300000
>> +	- spi-cpha: must be defined for max6675 to enable SPI mode 1
>> +
>> +	Refer to spi/spi-bus.txt for generic SPI slave bindings.
>> +
>> +Example:
>> +
>> +	max31885@0 {
>> +		compatible = "maxim,max31885";
>> +		reg = <0>;
>> +		spi-max-frequency = <4300000>;
>> +	};
>> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
>> index c4664e5..c9f5342 100644
>> --- a/drivers/iio/temperature/Kconfig
>> +++ b/drivers/iio/temperature/Kconfig
>> @@ -3,6 +3,20 @@
>>  #
>>  menu "Temperature sensors"
>>  
>> +config MAXIM_THERMOCOUPLE
>> +	tristate "Maxim thermocouple sensors"
>> +	depends on SPI
>> +	help
>> +	  If you say yes here you get support for the Maxim series of
>> +	  thermocouple sensors connected via SPI.
>> +
>> +	  Supported sensors:
>> +	   * MAX6675
>> +	   * MAX31885
>> +
>> +	  This driver can also be built as a module. If so, the module will
>> +	  be called maxim_thermocouple.
>> +
>>  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..78c3de0 100644
>> --- a/drivers/iio/temperature/Makefile
>> +++ b/drivers/iio/temperature/Makefile
>> @@ -2,6 +2,7 @@
>>  # Makefile for industrial I/O temperature drivers
>>  #
>>  
>> +obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
>>  obj-$(CONFIG_MLX90614) += mlx90614.o
>>  obj-$(CONFIG_TMP006) += tmp006.o
>>  obj-$(CONFIG_TSYS01) += tsys01.o
>> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
>> new file mode 100644
>> index 0000000..dffe3c2
>> --- /dev/null
>> +++ b/drivers/iio/temperature/maxim_thermocouple.c
>> @@ -0,0 +1,273 @@
>> +/*
>> + * maxim_thermocouple.c  - Support for Maxim thermocouple chips
>> + *
>> + * Copyright (C) 2016 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/mutex.h>
>> +#include <linux/err.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +
>> +#define MAXIM_THERMOCOUPLE_DRV_NAME	"maxim_thermocouple"
>> +
>> +enum {
>> +	MAX6675,
>> +	MAX31885,
>> +};
>> +
>> +const struct iio_chan_spec max6675_channels[] = {
>> +	{	/* thermocouple temperature */
>> +		.type = IIO_TEMP,
> 
> .address seems to be used; maybe make it explicit by setting it 0
> 
>> +		.info_mask_separate =
>> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +		.scan_index = 0,
>> +		.scan_type = {
>> +			.sign = 's',
>> +			.realbits = 13,
>> +			.storagebits = 16,
>> +			.shift = 3,
>> +			.endianness = IIO_BE,
>> +		},
>> +	},
>> +	IIO_CHAN_SOFT_TIMESTAMP(1),
>> +};
>> +
>> +const struct iio_chan_spec max31885_channels[] = {
>> +	{	/* thermocouple temperature */
>> +		.type = IIO_TEMP,
>> +		.address = 2,
>> +		.info_mask_separate =
>> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +		.scan_index = 0,
>> +		.scan_type = {
>> +			.sign = 's',
>> +			.realbits = 14,
>> +			.storagebits = 16,
>> +			.shift = 2,
>> +			.endianness = IIO_BE,
>> +		},
>> +	},
>> +	{	/* cold junction temperature */
>> +		.type = IIO_TEMP,
>> +		.channel2 = IIO_MOD_TEMP_AMBIENT,
>> +		.modified = 1,
>> +		.info_mask_separate =
>> +			BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +		.scan_index = 1,
>> +		.scan_type = {
>> +			.sign = 's',
>> +			.realbits = 12,
>> +			.storagebits = 16,
>> +			.shift = 4,
>> +			.endianness = IIO_BE,
>> +		},
>> +	},
>> +	IIO_CHAN_SOFT_TIMESTAMP(2),
>> +};
>> +
>> +struct maxim_thermocouple_chip {
>> +	const struct iio_chan_spec *channels;
>> +	int num_channels;
>> +	int read_size;
>> +
>> +	/* bit-check for valid input */
>> +	int status_bit;
>> +};
>> +
>> +const struct maxim_thermocouple_chip maxim_thermocouple_chips[] = {
>> +	[MAX6675] = {
>> +			.channels = max6675_channels,
>> +			.num_channels = ARRAY_SIZE(max6675_channels),
>> +			.read_size = 2,
>> +			.status_bit = BIT(2),
>> +		},
>> +	[MAX31885] = {
>> +			.channels = max31885_channels,
>> +			.num_channels = ARRAY_SIZE(max31885_channels),
>> +			.read_size = 4,
>> +			.status_bit = BIT(16),
>> +		},
>> +};
>> +
>> +struct maxim_thermocouple_data {
>> +	struct spi_device *spi;
>> +	const struct maxim_thermocouple_chip *chip;
>> +
>> +	u8 buffer[16]; /* 4 byte data + 4 byte padding + 8 byte timestamp */
>> +};
>> +
>> +static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
>> +				   struct iio_chan_spec const *chan, int *val)
>> +{
>> +	unsigned int storage_bytes = data->chip->read_size;
>> +	unsigned int shift = chan->scan_type.shift + (chan->address * 8);
>> +	unsigned int buf;
>> +	int ret;
>> +
>> +	ret = spi_read(data->spi, (void *) &buf, storage_bytes);
>> +	if (ret)
>> +		return ret;
>> +
>> +	switch (storage_bytes) {
>> +	case 2:
>> +		*val = be16_to_cpu(buf);
>> +		break;
>> +	case 4:
>> +		*val = be32_to_cpu(buf);
>> +		break;
>> +	}
>> +
>> +	/* check to be sure this is a valid reading */
>> +	if (*val & data->chip->status_bit)
>> +		return -EINVAL;
>> +
>> +	*val = sign_extend32(*val >> shift, chan->scan_type.realbits - 1);
>> +
>> +	return 0;
>> +}
>> +
>> +static irqreturn_t maxim_thermocouple_trigger_handler(int irq, void *private)
>> +{
>> +	struct iio_poll_func *pf = private;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>> +	struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>> +	int ret;
>> +
>> +	/* userspace HAL must do validation of data pushed to buffers */
Silly question, but what do you mean by this comment?
>> +	ret = spi_read(data->spi, &data->buffer, data->chip->read_size);
> 
> read_size can be 4 (bytes) but storagebits can be 16?!
> 
>> +	if (!ret)
>> +		iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>> +						   iio_get_time_ns());
>> +
>> +	iio_trigger_notify_done(indio_dev->trig);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int maxim_thermocouple_read_raw(struct iio_dev *indio_dev,
>> +				       struct iio_chan_spec const *chan,
>> +				       int *val, int *val2, long mask)
>> +{
>> +	struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>> +	int ret = -EINVAL;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		if (iio_device_claim_direct_mode(indio_dev))
>> +			return -EBUSY;
>> +
>> +		ret = maxim_thermocouple_read(data, chan, val);
>> +		if (!ret)
>> +			ret = IIO_VAL_INT;
>> +
>> +		iio_device_release_direct_mode(indio_dev);
>> +		break;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		switch (chan->channel2) {
>> +		case IIO_MOD_TEMP_AMBIENT:
>> +			*val = 62;
>> +			*val2 = 500000; /* 1000 * 0.0625 */
>> +			ret = IIO_VAL_INT_PLUS_MICRO;
>> +			break;
>> +		default:
>> +			*val = 250; /* 1000 * 0.25 */
>> +			ret = IIO_VAL_INT;
>> +		};
>> +		break;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct iio_info maxim_thermocouple_info = {
>> +	.driver_module = THIS_MODULE,
>> +	.read_raw = maxim_thermocouple_read_raw,
>> +};
>> +
>> +static int maxim_thermocouple_probe(struct spi_device *spi)
>> +{
>> +	const struct spi_device_id *id = spi_get_device_id(spi);
>> +	struct iio_dev *indio_dev;
>> +	struct maxim_thermocouple_data *data;
>> +	const struct maxim_thermocouple_chip *chip =
>> +			&maxim_thermocouple_chips[id->driver_data];
>> +	int ret;
>> +
>> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	indio_dev->info = &maxim_thermocouple_info;
>> +	indio_dev->name = MAXIM_THERMOCOUPLE_DRV_NAME;
>> +	indio_dev->channels = chip->channels;
>> +	indio_dev->num_channels = chip->num_channels;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +	data = iio_priv(indio_dev);
>> +	data->spi = spi;
>> +	data->chip = chip;
>> +
>> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> +				maxim_thermocouple_trigger_handler, NULL);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = iio_device_register(indio_dev);
>> +	if (ret)
>> +		goto error_unreg_buffer;
>> +
>> +	return 0;
>> +
>> +error_unreg_buffer:
>> +	iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +	return ret;
>> +}
>> +
>> +static int maxim_thermocouple_remove(struct spi_device *spi)
>> +{
>> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +
>> +	iio_device_unregister(indio_dev);
>> +	iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct spi_device_id maxim_thermocouple_id[] = {
>> +	{"max6675", MAX6675},
>> +	{"max31885", MAX31885},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(spi, maxim_thermocouple_id);
>> +
>> +static struct spi_driver maxim_thermocouple_driver = {
>> +	.driver = {
>> +		.name	= MAXIM_THERMOCOUPLE_DRV_NAME,
>> +	},
>> +	.probe		= maxim_thermocouple_probe,
>> +	.remove		= maxim_thermocouple_remove,
>> +	.id_table	= maxim_thermocouple_id,
>> +};
>> +module_spi_driver(maxim_thermocouple_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("Maxim thermocouple sensors");
>> +MODULE_LICENSE("GPL");
>>
> 

Re: [PATCH v2] iio: temperature: add support for Maxim thermocouple chips

[Matt Ranostay]

On Sun, May 29, 2016 at 1:16 AM, Peter Meerwald-Stadler
<pmeerw@pmeerw.net> wrote:
>
>> Add initial driver support for MAX6675, and MAX31885 thermocouple chips.
>
> comments below
>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Matt Porter <mporter@konsulko.com>
>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>> ---
>> Changes from v1:
>> * switch to iio_device_*_direct_mode wrappers
>> * add const to structs
>> * removed useless cast
>>
>> .../iio/temperature/maxim_thermocouple.txt         |  21 ++
>>  drivers/iio/temperature/Kconfig                    |  14 ++
>>  drivers/iio/temperature/Makefile                   |   1 +
>>  drivers/iio/temperature/maxim_thermocouple.c       | 273 +++++++++++++++++++++
>>  4 files changed, 309 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>  create mode 100644 drivers/iio/temperature/maxim_thermocouple.c
>>
>> diff --git a/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>> new file mode 100644
>> index 0000000..44fb029
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>> @@ -0,0 +1,21 @@
>> +Maxim thermocouple support
>> +
>> +* https://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>> +* https://datasheets.maximintegrated.com/en/ds/MAX31855.pdf
>> +
>> +Required properties:
>> +
>> +     - compatible: must be "maxim,max31885" or "maxim,max6675"
>> +     - reg: SPI chip select number for the device
>> +     - spi-max-frequency: must be 4300000
>> +     - spi-cpha: must be defined for max6675 to enable SPI mode 1
>> +
>> +     Refer to spi/spi-bus.txt for generic SPI slave bindings.
>> +
>> +Example:
>> +
>> +     max31885@0 {
>> +             compatible = "maxim,max31885";
>> +             reg = <0>;
>> +             spi-max-frequency = <4300000>;
>> +     };
>> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
>> index c4664e5..c9f5342 100644
>> --- a/drivers/iio/temperature/Kconfig
>> +++ b/drivers/iio/temperature/Kconfig
>> @@ -3,6 +3,20 @@
>>  #
>>  menu "Temperature sensors"
>>
>> +config MAXIM_THERMOCOUPLE
>> +     tristate "Maxim thermocouple sensors"
>> +     depends on SPI
>> +     help
>> +       If you say yes here you get support for the Maxim series of
>> +       thermocouple sensors connected via SPI.
>> +
>> +       Supported sensors:
>> +        * MAX6675
>> +        * MAX31885
>> +
>> +       This driver can also be built as a module. If so, the module will
>> +       be called maxim_thermocouple.
>> +
>>  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..78c3de0 100644
>> --- a/drivers/iio/temperature/Makefile
>> +++ b/drivers/iio/temperature/Makefile
>> @@ -2,6 +2,7 @@
>>  # Makefile for industrial I/O temperature drivers
>>  #
>>
>> +obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
>>  obj-$(CONFIG_MLX90614) += mlx90614.o
>>  obj-$(CONFIG_TMP006) += tmp006.o
>>  obj-$(CONFIG_TSYS01) += tsys01.o
>> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
>> new file mode 100644
>> index 0000000..dffe3c2
>> --- /dev/null
>> +++ b/drivers/iio/temperature/maxim_thermocouple.c
>> @@ -0,0 +1,273 @@
>> +/*
>> + * maxim_thermocouple.c  - Support for Maxim thermocouple chips
>> + *
>> + * Copyright (C) 2016 Matt Ranostay <mranostay@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/mutex.h>
>> +#include <linux/err.h>
>> +#include <linux/spi/spi.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +
>> +#define MAXIM_THERMOCOUPLE_DRV_NAME  "maxim_thermocouple"
>> +
>> +enum {
>> +     MAX6675,
>> +     MAX31885,
>> +};
>> +
>> +const struct iio_chan_spec max6675_channels[] = {
>> +     {       /* thermocouple temperature */
>> +             .type = IIO_TEMP,
>
> .address seems to be used; maybe make it explicit by setting it 0

Ok.. default to zero by aid of the compiler. But explicit can't hurt.

>
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +             .scan_index = 0,
>> +             .scan_type = {
>> +                     .sign = 's',
>> +                     .realbits = 13,
>> +                     .storagebits = 16,
>> +                     .shift = 3,
>> +                     .endianness = IIO_BE,
>> +             },
>> +     },
>> +     IIO_CHAN_SOFT_TIMESTAMP(1),
>> +};
>> +
>> +const struct iio_chan_spec max31885_channels[] = {
>> +     {       /* thermocouple temperature */
>> +             .type = IIO_TEMP,
>> +             .address = 2,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +             .scan_index = 0,
>> +             .scan_type = {
>> +                     .sign = 's',
>> +                     .realbits = 14,
>> +                     .storagebits = 16,
>> +                     .shift = 2,
>> +                     .endianness = IIO_BE,
>> +             },
>> +     },
>> +     {       /* cold junction temperature */
>> +             .type = IIO_TEMP,
>> +             .channel2 = IIO_MOD_TEMP_AMBIENT,
>> +             .modified = 1,
>> +             .info_mask_separate =
>> +                     BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>> +             .scan_index = 1,
>> +             .scan_type = {
>> +                     .sign = 's',
>> +                     .realbits = 12,
>> +                     .storagebits = 16,
>> +                     .shift = 4,
>> +                     .endianness = IIO_BE,
>> +             },
>> +     },
>> +     IIO_CHAN_SOFT_TIMESTAMP(2),
>> +};
>> +
>> +struct maxim_thermocouple_chip {
>> +     const struct iio_chan_spec *channels;
>> +     int num_channels;
>> +     int read_size;
>> +
>> +     /* bit-check for valid input */
>> +     int status_bit;
>> +};
>> +
>> +const struct maxim_thermocouple_chip maxim_thermocouple_chips[] = {
>> +     [MAX6675] = {
>> +                     .channels = max6675_channels,
>> +                     .num_channels = ARRAY_SIZE(max6675_channels),
>> +                     .read_size = 2,
>> +                     .status_bit = BIT(2),
>> +             },
>> +     [MAX31885] = {
>> +                     .channels = max31885_channels,
>> +                     .num_channels = ARRAY_SIZE(max31885_channels),
>> +                     .read_size = 4,
>> +                     .status_bit = BIT(16),
>> +             },
>> +};
>> +
>> +struct maxim_thermocouple_data {
>> +     struct spi_device *spi;
>> +     const struct maxim_thermocouple_chip *chip;
>> +
>> +     u8 buffer[16]; /* 4 byte data + 4 byte padding + 8 byte timestamp */
>> +};
>> +
>> +static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
>> +                                struct iio_chan_spec const *chan, int *val)
>> +{
>> +     unsigned int storage_bytes = data->chip->read_size;
>> +     unsigned int shift = chan->scan_type.shift + (chan->address * 8);
>> +     unsigned int buf;
>> +     int ret;
>> +
>> +     ret = spi_read(data->spi, (void *) &buf, storage_bytes);
>> +     if (ret)
>> +             return ret;
>> +
>> +     switch (storage_bytes) {
>> +     case 2:
>> +             *val = be16_to_cpu(buf);
>> +             break;
>> +     case 4:
>> +             *val = be32_to_cpu(buf);
>> +             break;
>> +     }
>> +
>> +     /* check to be sure this is a valid reading */
>> +     if (*val & data->chip->status_bit)
>> +             return -EINVAL;
>> +
>> +     *val = sign_extend32(*val >> shift, chan->scan_type.realbits - 1);
>> +
>> +     return 0;
>> +}
>> +
>> +static irqreturn_t maxim_thermocouple_trigger_handler(int irq, void *private)
>> +{
>> +     struct iio_poll_func *pf = private;
>> +     struct iio_dev *indio_dev = pf->indio_dev;
>> +     struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>> +     int ret;
>> +
>> +     /* userspace HAL must do validation of data pushed to buffers */
>> +     ret = spi_read(data->spi, &data->buffer, data->chip->read_size);
>
> read_size can be 4 (bytes) but storagebits can be 16?!

Ok good catch... need to setup the available masks.. since it always
reads both channels.

>
>> +     if (!ret)
>> +             iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>> +                                                iio_get_time_ns());
>> +
>> +     iio_trigger_notify_done(indio_dev->trig);
>> +
>> +     return IRQ_HANDLED;
>> +}
>> +
>> +static int maxim_thermocouple_read_raw(struct iio_dev *indio_dev,
>> +                                    struct iio_chan_spec const *chan,
>> +                                    int *val, int *val2, long mask)
>> +{
>> +     struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>> +     int ret = -EINVAL;
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>> +             if (iio_device_claim_direct_mode(indio_dev))
>> +                     return -EBUSY;
>> +
>> +             ret = maxim_thermocouple_read(data, chan, val);
>> +             if (!ret)
>> +                     ret = IIO_VAL_INT;
>> +
>> +             iio_device_release_direct_mode(indio_dev);
>> +             break;
>> +     case IIO_CHAN_INFO_SCALE:
>> +             switch (chan->channel2) {
>> +             case IIO_MOD_TEMP_AMBIENT:
>> +                     *val = 62;
>> +                     *val2 = 500000; /* 1000 * 0.0625 */
>> +                     ret = IIO_VAL_INT_PLUS_MICRO;
>> +                     break;
>> +             default:
>> +                     *val = 250; /* 1000 * 0.25 */
>> +                     ret = IIO_VAL_INT;
>> +             };
>> +             break;
>> +     }
>> +
>> +     return ret;
>> +}
>> +
>> +static const struct iio_info maxim_thermocouple_info = {
>> +     .driver_module = THIS_MODULE,
>> +     .read_raw = maxim_thermocouple_read_raw,
>> +};
>> +
>> +static int maxim_thermocouple_probe(struct spi_device *spi)
>> +{
>> +     const struct spi_device_id *id = spi_get_device_id(spi);
>> +     struct iio_dev *indio_dev;
>> +     struct maxim_thermocouple_data *data;
>> +     const struct maxim_thermocouple_chip *chip =
>> +                     &maxim_thermocouple_chips[id->driver_data];
>> +     int ret;
>> +
>> +     indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
>> +     if (!indio_dev)
>> +             return -ENOMEM;
>> +
>> +     indio_dev->info = &maxim_thermocouple_info;
>> +     indio_dev->name = MAXIM_THERMOCOUPLE_DRV_NAME;
>> +     indio_dev->channels = chip->channels;
>> +     indio_dev->num_channels = chip->num_channels;
>> +     indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +     data = iio_priv(indio_dev);
>> +     data->spi = spi;
>> +     data->chip = chip;
>> +
>> +     ret = iio_triggered_buffer_setup(indio_dev, NULL,
>> +                             maxim_thermocouple_trigger_handler, NULL);
>> +     if (ret)
>> +             return ret;
>> +
>> +     ret = iio_device_register(indio_dev);
>> +     if (ret)
>> +             goto error_unreg_buffer;
>> +
>> +     return 0;
>> +
>> +error_unreg_buffer:
>> +     iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +     return ret;
>> +}
>> +
>> +static int maxim_thermocouple_remove(struct spi_device *spi)
>> +{
>> +     struct iio_dev *indio_dev = spi_get_drvdata(spi);
>> +
>> +     iio_device_unregister(indio_dev);
>> +     iio_triggered_buffer_cleanup(indio_dev);
>> +
>> +     return 0;
>> +}
>> +
>> +static const struct spi_device_id maxim_thermocouple_id[] = {
>> +     {"max6675", MAX6675},
>> +     {"max31885", MAX31885},
>> +     {},
>> +};
>> +MODULE_DEVICE_TABLE(spi, maxim_thermocouple_id);
>> +
>> +static struct spi_driver maxim_thermocouple_driver = {
>> +     .driver = {
>> +             .name   = MAXIM_THERMOCOUPLE_DRV_NAME,
>> +     },
>> +     .probe          = maxim_thermocouple_probe,
>> +     .remove         = maxim_thermocouple_remove,
>> +     .id_table       = maxim_thermocouple_id,
>> +};
>> +module_spi_driver(maxim_thermocouple_driver);
>> +
>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>> +MODULE_DESCRIPTION("Maxim thermocouple sensors");
>> +MODULE_LICENSE("GPL");
>>
>
> --
>
> Peter Meerwald-Stadler
> +43-664-2444418 (mobile)

Re: [PATCH v2] iio: temperature: add support for Maxim thermocouple chips

[Matt Ranostay]

On Sun, May 29, 2016 at 11:44 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 29/05/16 09:16, Peter Meerwald-Stadler wrote:
>>
>>> Add initial driver support for MAX6675, and MAX31885 thermocouple chips.
>>
>> comments below
> Not much to add other than a question inline...
>
> Jonathan
>>
>>> Cc: Marek Vasut <marex@denx.de>
>>> Cc: Matt Porter <mporter@konsulko.com>
>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>>> ---
>>> Changes from v1:
>>> * switch to iio_device_*_direct_mode wrappers
>>> * add const to structs
>>> * removed useless cast
>>>
>>> .../iio/temperature/maxim_thermocouple.txt         |  21 ++
>>>  drivers/iio/temperature/Kconfig                    |  14 ++
>>>  drivers/iio/temperature/Makefile                   |   1 +
>>>  drivers/iio/temperature/maxim_thermocouple.c       | 273 +++++++++++++++++++++
>>>  4 files changed, 309 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>>  create mode 100644 drivers/iio/temperature/maxim_thermocouple.c
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>> new file mode 100644
>>> index 0000000..44fb029
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>> @@ -0,0 +1,21 @@
>>> +Maxim thermocouple support
>>> +
>>> +* https://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>>> +* https://datasheets.maximintegrated.com/en/ds/MAX31855.pdf
>>> +
>>> +Required properties:
>>> +
>>> +    - compatible: must be "maxim,max31885" or "maxim,max6675"
>>> +    - reg: SPI chip select number for the device
>>> +    - spi-max-frequency: must be 4300000
>>> +    - spi-cpha: must be defined for max6675 to enable SPI mode 1
>>> +
>>> +    Refer to spi/spi-bus.txt for generic SPI slave bindings.
>>> +
>>> +Example:
>>> +
>>> +    max31885@0 {
>>> +            compatible = "maxim,max31885";
>>> +            reg = <0>;
>>> +            spi-max-frequency = <4300000>;
>>> +    };
>>> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
>>> index c4664e5..c9f5342 100644
>>> --- a/drivers/iio/temperature/Kconfig
>>> +++ b/drivers/iio/temperature/Kconfig
>>> @@ -3,6 +3,20 @@
>>>  #
>>>  menu "Temperature sensors"
>>>
>>> +config MAXIM_THERMOCOUPLE
>>> +    tristate "Maxim thermocouple sensors"
>>> +    depends on SPI
>>> +    help
>>> +      If you say yes here you get support for the Maxim series of
>>> +      thermocouple sensors connected via SPI.
>>> +
>>> +      Supported sensors:
>>> +       * MAX6675
>>> +       * MAX31885
>>> +
>>> +      This driver can also be built as a module. If so, the module will
>>> +      be called maxim_thermocouple.
>>> +
>>>  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..78c3de0 100644
>>> --- a/drivers/iio/temperature/Makefile
>>> +++ b/drivers/iio/temperature/Makefile
>>> @@ -2,6 +2,7 @@
>>>  # Makefile for industrial I/O temperature drivers
>>>  #
>>>
>>> +obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
>>>  obj-$(CONFIG_MLX90614) += mlx90614.o
>>>  obj-$(CONFIG_TMP006) += tmp006.o
>>>  obj-$(CONFIG_TSYS01) += tsys01.o
>>> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
>>> new file mode 100644
>>> index 0000000..dffe3c2
>>> --- /dev/null
>>> +++ b/drivers/iio/temperature/maxim_thermocouple.c
>>> @@ -0,0 +1,273 @@
>>> +/*
>>> + * maxim_thermocouple.c  - Support for Maxim thermocouple chips
>>> + *
>>> + * Copyright (C) 2016 Matt Ranostay <mranostay@gmail.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/init.h>
>>> +#include <linux/mutex.h>
>>> +#include <linux/err.h>
>>> +#include <linux/spi/spi.h>
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/buffer.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +
>>> +#define MAXIM_THERMOCOUPLE_DRV_NAME "maxim_thermocouple"
>>> +
>>> +enum {
>>> +    MAX6675,
>>> +    MAX31885,
>>> +};
>>> +
>>> +const struct iio_chan_spec max6675_channels[] = {
>>> +    {       /* thermocouple temperature */
>>> +            .type = IIO_TEMP,
>>
>> .address seems to be used; maybe make it explicit by setting it 0
>>
>>> +            .info_mask_separate =
>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>> +            .scan_index = 0,
>>> +            .scan_type = {
>>> +                    .sign = 's',
>>> +                    .realbits = 13,
>>> +                    .storagebits = 16,
>>> +                    .shift = 3,
>>> +                    .endianness = IIO_BE,
>>> +            },
>>> +    },
>>> +    IIO_CHAN_SOFT_TIMESTAMP(1),
>>> +};
>>> +
>>> +const struct iio_chan_spec max31885_channels[] = {
>>> +    {       /* thermocouple temperature */
>>> +            .type = IIO_TEMP,
>>> +            .address = 2,
>>> +            .info_mask_separate =
>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>> +            .scan_index = 0,
>>> +            .scan_type = {
>>> +                    .sign = 's',
>>> +                    .realbits = 14,
>>> +                    .storagebits = 16,
>>> +                    .shift = 2,
>>> +                    .endianness = IIO_BE,
>>> +            },
>>> +    },
>>> +    {       /* cold junction temperature */
>>> +            .type = IIO_TEMP,
>>> +            .channel2 = IIO_MOD_TEMP_AMBIENT,
>>> +            .modified = 1,
>>> +            .info_mask_separate =
>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>> +            .scan_index = 1,
>>> +            .scan_type = {
>>> +                    .sign = 's',
>>> +                    .realbits = 12,
>>> +                    .storagebits = 16,
>>> +                    .shift = 4,
>>> +                    .endianness = IIO_BE,
>>> +            },
>>> +    },
>>> +    IIO_CHAN_SOFT_TIMESTAMP(2),
>>> +};
>>> +
>>> +struct maxim_thermocouple_chip {
>>> +    const struct iio_chan_spec *channels;
>>> +    int num_channels;
>>> +    int read_size;
>>> +
>>> +    /* bit-check for valid input */
>>> +    int status_bit;
>>> +};
>>> +
>>> +const struct maxim_thermocouple_chip maxim_thermocouple_chips[] = {
>>> +    [MAX6675] = {
>>> +                    .channels = max6675_channels,
>>> +                    .num_channels = ARRAY_SIZE(max6675_channels),
>>> +                    .read_size = 2,
>>> +                    .status_bit = BIT(2),
>>> +            },
>>> +    [MAX31885] = {
>>> +                    .channels = max31885_channels,
>>> +                    .num_channels = ARRAY_SIZE(max31885_channels),
>>> +                    .read_size = 4,
>>> +                    .status_bit = BIT(16),
>>> +            },
>>> +};
>>> +
>>> +struct maxim_thermocouple_data {
>>> +    struct spi_device *spi;
>>> +    const struct maxim_thermocouple_chip *chip;
>>> +
>>> +    u8 buffer[16]; /* 4 byte data + 4 byte padding + 8 byte timestamp */
>>> +};
>>> +
>>> +static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
>>> +                               struct iio_chan_spec const *chan, int *val)
>>> +{
>>> +    unsigned int storage_bytes = data->chip->read_size;
>>> +    unsigned int shift = chan->scan_type.shift + (chan->address * 8);
>>> +    unsigned int buf;
>>> +    int ret;
>>> +
>>> +    ret = spi_read(data->spi, (void *) &buf, storage_bytes);
>>> +    if (ret)
>>> +            return ret;
>>> +
>>> +    switch (storage_bytes) {
>>> +    case 2:
>>> +            *val = be16_to_cpu(buf);
>>> +            break;
>>> +    case 4:
>>> +            *val = be32_to_cpu(buf);
>>> +            break;
>>> +    }
>>> +
>>> +    /* check to be sure this is a valid reading */
>>> +    if (*val & data->chip->status_bit)
>>> +            return -EINVAL;
>>> +
>>> +    *val = sign_extend32(*val >> shift, chan->scan_type.realbits - 1);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static irqreturn_t maxim_thermocouple_trigger_handler(int irq, void *private)
>>> +{
>>> +    struct iio_poll_func *pf = private;
>>> +    struct iio_dev *indio_dev = pf->indio_dev;
>>> +    struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>>> +    int ret;
>>> +
>>> +    /* userspace HAL must do validation of data pushed to buffers */
> Silly question, but what do you mean by this comment?

Mainly that userspace will have to check the status bit itself to be
sure a readings is valid.

>>> +    ret = spi_read(data->spi, &data->buffer, data->chip->read_size);
>>
>> read_size can be 4 (bytes) but storagebits can be 16?!
>>
>>> +    if (!ret)
>>> +            iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>>> +                                               iio_get_time_ns());
>>> +
>>> +    iio_trigger_notify_done(indio_dev->trig);
>>> +
>>> +    return IRQ_HANDLED;
>>> +}
>>> +
>>> +static int maxim_thermocouple_read_raw(struct iio_dev *indio_dev,
>>> +                                   struct iio_chan_spec const *chan,
>>> +                                   int *val, int *val2, long mask)
>>> +{
>>> +    struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>>> +    int ret = -EINVAL;
>>> +
>>> +    switch (mask) {
>>> +    case IIO_CHAN_INFO_RAW:
>>> +            if (iio_device_claim_direct_mode(indio_dev))
>>> +                    return -EBUSY;
>>> +
>>> +            ret = maxim_thermocouple_read(data, chan, val);
>>> +            if (!ret)
>>> +                    ret = IIO_VAL_INT;
>>> +
>>> +            iio_device_release_direct_mode(indio_dev);
>>> +            break;
>>> +    case IIO_CHAN_INFO_SCALE:
>>> +            switch (chan->channel2) {
>>> +            case IIO_MOD_TEMP_AMBIENT:
>>> +                    *val = 62;
>>> +                    *val2 = 500000; /* 1000 * 0.0625 */
>>> +                    ret = IIO_VAL_INT_PLUS_MICRO;
>>> +                    break;
>>> +            default:
>>> +                    *val = 250; /* 1000 * 0.25 */
>>> +                    ret = IIO_VAL_INT;
>>> +            };
>>> +            break;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static const struct iio_info maxim_thermocouple_info = {
>>> +    .driver_module = THIS_MODULE,
>>> +    .read_raw = maxim_thermocouple_read_raw,
>>> +};
>>> +
>>> +static int maxim_thermocouple_probe(struct spi_device *spi)
>>> +{
>>> +    const struct spi_device_id *id = spi_get_device_id(spi);
>>> +    struct iio_dev *indio_dev;
>>> +    struct maxim_thermocouple_data *data;
>>> +    const struct maxim_thermocouple_chip *chip =
>>> +                    &maxim_thermocouple_chips[id->driver_data];
>>> +    int ret;
>>> +
>>> +    indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
>>> +    if (!indio_dev)
>>> +            return -ENOMEM;
>>> +
>>> +    indio_dev->info = &maxim_thermocouple_info;
>>> +    indio_dev->name = MAXIM_THERMOCOUPLE_DRV_NAME;
>>> +    indio_dev->channels = chip->channels;
>>> +    indio_dev->num_channels = chip->num_channels;
>>> +    indio_dev->modes = INDIO_DIRECT_MODE;
>>> +
>>> +    data = iio_priv(indio_dev);
>>> +    data->spi = spi;
>>> +    data->chip = chip;
>>> +
>>> +    ret = iio_triggered_buffer_setup(indio_dev, NULL,
>>> +                            maxim_thermocouple_trigger_handler, NULL);
>>> +    if (ret)
>>> +            return ret;
>>> +
>>> +    ret = iio_device_register(indio_dev);
>>> +    if (ret)
>>> +            goto error_unreg_buffer;
>>> +
>>> +    return 0;
>>> +
>>> +error_unreg_buffer:
>>> +    iio_triggered_buffer_cleanup(indio_dev);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static int maxim_thermocouple_remove(struct spi_device *spi)
>>> +{
>>> +    struct iio_dev *indio_dev = spi_get_drvdata(spi);
>>> +
>>> +    iio_device_unregister(indio_dev);
>>> +    iio_triggered_buffer_cleanup(indio_dev);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static const struct spi_device_id maxim_thermocouple_id[] = {
>>> +    {"max6675", MAX6675},
>>> +    {"max31885", MAX31885},
>>> +    {},
>>> +};
>>> +MODULE_DEVICE_TABLE(spi, maxim_thermocouple_id);
>>> +
>>> +static struct spi_driver maxim_thermocouple_driver = {
>>> +    .driver = {
>>> +            .name   = MAXIM_THERMOCOUPLE_DRV_NAME,
>>> +    },
>>> +    .probe          = maxim_thermocouple_probe,
>>> +    .remove         = maxim_thermocouple_remove,
>>> +    .id_table       = maxim_thermocouple_id,
>>> +};
>>> +module_spi_driver(maxim_thermocouple_driver);
>>> +
>>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>>> +MODULE_DESCRIPTION("Maxim thermocouple sensors");
>>> +MODULE_LICENSE("GPL");
>>>
>>
>

Re: [PATCH v2] iio: temperature: add support for Maxim thermocouple chips

[Jonathan Cameron]

On 29/05/16 19:47, Matt Ranostay wrote:
> On Sun, May 29, 2016 at 11:44 AM, Jonathan Cameron <jic23@kernel.org> wrote:
>> On 29/05/16 09:16, Peter Meerwald-Stadler wrote:
>>>
>>>> Add initial driver support for MAX6675, and MAX31885 thermocouple chips.
>>>
>>> comments below
>> Not much to add other than a question inline...
>>
>> Jonathan
>>>
>>>> Cc: Marek Vasut <marex@denx.de>
>>>> Cc: Matt Porter <mporter@konsulko.com>
>>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>>>> ---
>>>> Changes from v1:
>>>> * switch to iio_device_*_direct_mode wrappers
>>>> * add const to structs
>>>> * removed useless cast
>>>>
>>>> .../iio/temperature/maxim_thermocouple.txt         |  21 ++
>>>>  drivers/iio/temperature/Kconfig                    |  14 ++
>>>>  drivers/iio/temperature/Makefile                   |   1 +
>>>>  drivers/iio/temperature/maxim_thermocouple.c       | 273 +++++++++++++++++++++
>>>>  4 files changed, 309 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>>>  create mode 100644 drivers/iio/temperature/maxim_thermocouple.c
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>>> new file mode 100644
>>>> index 0000000..44fb029
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>>> @@ -0,0 +1,21 @@
>>>> +Maxim thermocouple support
>>>> +
>>>> +* https://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>>>> +* https://datasheets.maximintegrated.com/en/ds/MAX31855.pdf
>>>> +
>>>> +Required properties:
>>>> +
>>>> +    - compatible: must be "maxim,max31885" or "maxim,max6675"
>>>> +    - reg: SPI chip select number for the device
>>>> +    - spi-max-frequency: must be 4300000
>>>> +    - spi-cpha: must be defined for max6675 to enable SPI mode 1
>>>> +
>>>> +    Refer to spi/spi-bus.txt for generic SPI slave bindings.
>>>> +
>>>> +Example:
>>>> +
>>>> +    max31885@0 {
>>>> +            compatible = "maxim,max31885";
>>>> +            reg = <0>;
>>>> +            spi-max-frequency = <4300000>;
>>>> +    };
>>>> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
>>>> index c4664e5..c9f5342 100644
>>>> --- a/drivers/iio/temperature/Kconfig
>>>> +++ b/drivers/iio/temperature/Kconfig
>>>> @@ -3,6 +3,20 @@
>>>>  #
>>>>  menu "Temperature sensors"
>>>>
>>>> +config MAXIM_THERMOCOUPLE
>>>> +    tristate "Maxim thermocouple sensors"
>>>> +    depends on SPI
>>>> +    help
>>>> +      If you say yes here you get support for the Maxim series of
>>>> +      thermocouple sensors connected via SPI.
>>>> +
>>>> +      Supported sensors:
>>>> +       * MAX6675
>>>> +       * MAX31885
>>>> +
>>>> +      This driver can also be built as a module. If so, the module will
>>>> +      be called maxim_thermocouple.
>>>> +
>>>>  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..78c3de0 100644
>>>> --- a/drivers/iio/temperature/Makefile
>>>> +++ b/drivers/iio/temperature/Makefile
>>>> @@ -2,6 +2,7 @@
>>>>  # Makefile for industrial I/O temperature drivers
>>>>  #
>>>>
>>>> +obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
>>>>  obj-$(CONFIG_MLX90614) += mlx90614.o
>>>>  obj-$(CONFIG_TMP006) += tmp006.o
>>>>  obj-$(CONFIG_TSYS01) += tsys01.o
>>>> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
>>>> new file mode 100644
>>>> index 0000000..dffe3c2
>>>> --- /dev/null
>>>> +++ b/drivers/iio/temperature/maxim_thermocouple.c
>>>> @@ -0,0 +1,273 @@
>>>> +/*
>>>> + * maxim_thermocouple.c  - Support for Maxim thermocouple chips
>>>> + *
>>>> + * Copyright (C) 2016 Matt Ranostay <mranostay@gmail.com>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License as published by
>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>> + * (at your option) any later version.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> + * GNU General Public License for more details.
>>>> + */
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/init.h>
>>>> +#include <linux/mutex.h>
>>>> +#include <linux/err.h>
>>>> +#include <linux/spi/spi.h>
>>>> +#include <linux/iio/iio.h>
>>>> +#include <linux/iio/trigger.h>
>>>> +#include <linux/iio/buffer.h>
>>>> +#include <linux/iio/triggered_buffer.h>
>>>> +#include <linux/iio/trigger_consumer.h>
>>>> +
>>>> +#define MAXIM_THERMOCOUPLE_DRV_NAME "maxim_thermocouple"
>>>> +
>>>> +enum {
>>>> +    MAX6675,
>>>> +    MAX31885,
>>>> +};
>>>> +
>>>> +const struct iio_chan_spec max6675_channels[] = {
>>>> +    {       /* thermocouple temperature */
>>>> +            .type = IIO_TEMP,
>>>
>>> .address seems to be used; maybe make it explicit by setting it 0
>>>
>>>> +            .info_mask_separate =
>>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>>> +            .scan_index = 0,
>>>> +            .scan_type = {
>>>> +                    .sign = 's',
>>>> +                    .realbits = 13,
>>>> +                    .storagebits = 16,
>>>> +                    .shift = 3,
>>>> +                    .endianness = IIO_BE,
>>>> +            },
>>>> +    },
>>>> +    IIO_CHAN_SOFT_TIMESTAMP(1),
>>>> +};
>>>> +
>>>> +const struct iio_chan_spec max31885_channels[] = {
>>>> +    {       /* thermocouple temperature */
>>>> +            .type = IIO_TEMP,
>>>> +            .address = 2,
>>>> +            .info_mask_separate =
>>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>>> +            .scan_index = 0,
>>>> +            .scan_type = {
>>>> +                    .sign = 's',
>>>> +                    .realbits = 14,
>>>> +                    .storagebits = 16,
>>>> +                    .shift = 2,
>>>> +                    .endianness = IIO_BE,
>>>> +            },
>>>> +    },
>>>> +    {       /* cold junction temperature */
>>>> +            .type = IIO_TEMP,
>>>> +            .channel2 = IIO_MOD_TEMP_AMBIENT,
>>>> +            .modified = 1,
>>>> +            .info_mask_separate =
>>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>>> +            .scan_index = 1,
>>>> +            .scan_type = {
>>>> +                    .sign = 's',
>>>> +                    .realbits = 12,
>>>> +                    .storagebits = 16,
>>>> +                    .shift = 4,
>>>> +                    .endianness = IIO_BE,
>>>> +            },
>>>> +    },
>>>> +    IIO_CHAN_SOFT_TIMESTAMP(2),
>>>> +};
>>>> +
>>>> +struct maxim_thermocouple_chip {
>>>> +    const struct iio_chan_spec *channels;
>>>> +    int num_channels;
>>>> +    int read_size;
>>>> +
>>>> +    /* bit-check for valid input */
>>>> +    int status_bit;
>>>> +};
>>>> +
>>>> +const struct maxim_thermocouple_chip maxim_thermocouple_chips[] = {
>>>> +    [MAX6675] = {
>>>> +                    .channels = max6675_channels,
>>>> +                    .num_channels = ARRAY_SIZE(max6675_channels),
>>>> +                    .read_size = 2,
>>>> +                    .status_bit = BIT(2),
>>>> +            },
>>>> +    [MAX31885] = {
>>>> +                    .channels = max31885_channels,
>>>> +                    .num_channels = ARRAY_SIZE(max31885_channels),
>>>> +                    .read_size = 4,
>>>> +                    .status_bit = BIT(16),
>>>> +            },
>>>> +};
>>>> +
>>>> +struct maxim_thermocouple_data {
>>>> +    struct spi_device *spi;
>>>> +    const struct maxim_thermocouple_chip *chip;
>>>> +
>>>> +    u8 buffer[16]; /* 4 byte data + 4 byte padding + 8 byte timestamp */
>>>> +};
>>>> +
>>>> +static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
>>>> +                               struct iio_chan_spec const *chan, int *val)
>>>> +{
>>>> +    unsigned int storage_bytes = data->chip->read_size;
>>>> +    unsigned int shift = chan->scan_type.shift + (chan->address * 8);
>>>> +    unsigned int buf;
>>>> +    int ret;
>>>> +
>>>> +    ret = spi_read(data->spi, (void *) &buf, storage_bytes);
>>>> +    if (ret)
>>>> +            return ret;
>>>> +
>>>> +    switch (storage_bytes) {
>>>> +    case 2:
>>>> +            *val = be16_to_cpu(buf);
>>>> +            break;
>>>> +    case 4:
>>>> +            *val = be32_to_cpu(buf);
>>>> +            break;
>>>> +    }
>>>> +
>>>> +    /* check to be sure this is a valid reading */
>>>> +    if (*val & data->chip->status_bit)
>>>> +            return -EINVAL;
>>>> +
>>>> +    *val = sign_extend32(*val >> shift, chan->scan_type.realbits - 1);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static irqreturn_t maxim_thermocouple_trigger_handler(int irq, void *private)
>>>> +{
>>>> +    struct iio_poll_func *pf = private;
>>>> +    struct iio_dev *indio_dev = pf->indio_dev;
>>>> +    struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>>>> +    int ret;
>>>> +
>>>> +    /* userspace HAL must do validation of data pushed to buffers */
>> Silly question, but what do you mean by this comment?
> 
> Mainly that userspace will have to check the status bit itself to be
> sure a readings is valid.
That's ugly.  hmm. Might be worth scrubbing the whole result in the buffer
to make that clear?  Not filling the buffer at all will lead to issues so
not sure how best to handle this.  

Long ago we used to put -1 in timestamp fields when we had no way of knowing when
the sample occurred - perhaps something similar here...
> 
>>>> +    ret = spi_read(data->spi, &data->buffer, data->chip->read_size);
>>>
>>> read_size can be 4 (bytes) but storagebits can be 16?!
>>>
>>>> +    if (!ret)
>>>> +            iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>>>> +                                               iio_get_time_ns());
>>>> +
>>>> +    iio_trigger_notify_done(indio_dev->trig);
>>>> +
>>>> +    return IRQ_HANDLED;
>>>> +}
>>>> +
>>>> +static int maxim_thermocouple_read_raw(struct iio_dev *indio_dev,
>>>> +                                   struct iio_chan_spec const *chan,
>>>> +                                   int *val, int *val2, long mask)
>>>> +{
>>>> +    struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>>>> +    int ret = -EINVAL;
>>>> +
>>>> +    switch (mask) {
>>>> +    case IIO_CHAN_INFO_RAW:
>>>> +            if (iio_device_claim_direct_mode(indio_dev))
>>>> +                    return -EBUSY;
>>>> +
>>>> +            ret = maxim_thermocouple_read(data, chan, val);
>>>> +            if (!ret)
>>>> +                    ret = IIO_VAL_INT;
>>>> +
>>>> +            iio_device_release_direct_mode(indio_dev);
>>>> +            break;
>>>> +    case IIO_CHAN_INFO_SCALE:
>>>> +            switch (chan->channel2) {
>>>> +            case IIO_MOD_TEMP_AMBIENT:
>>>> +                    *val = 62;
>>>> +                    *val2 = 500000; /* 1000 * 0.0625 */
>>>> +                    ret = IIO_VAL_INT_PLUS_MICRO;
>>>> +                    break;
>>>> +            default:
>>>> +                    *val = 250; /* 1000 * 0.25 */
>>>> +                    ret = IIO_VAL_INT;
>>>> +            };
>>>> +            break;
>>>> +    }
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static const struct iio_info maxim_thermocouple_info = {
>>>> +    .driver_module = THIS_MODULE,
>>>> +    .read_raw = maxim_thermocouple_read_raw,
>>>> +};
>>>> +
>>>> +static int maxim_thermocouple_probe(struct spi_device *spi)
>>>> +{
>>>> +    const struct spi_device_id *id = spi_get_device_id(spi);
>>>> +    struct iio_dev *indio_dev;
>>>> +    struct maxim_thermocouple_data *data;
>>>> +    const struct maxim_thermocouple_chip *chip =
>>>> +                    &maxim_thermocouple_chips[id->driver_data];
>>>> +    int ret;
>>>> +
>>>> +    indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
>>>> +    if (!indio_dev)
>>>> +            return -ENOMEM;
>>>> +
>>>> +    indio_dev->info = &maxim_thermocouple_info;
>>>> +    indio_dev->name = MAXIM_THERMOCOUPLE_DRV_NAME;
>>>> +    indio_dev->channels = chip->channels;
>>>> +    indio_dev->num_channels = chip->num_channels;
>>>> +    indio_dev->modes = INDIO_DIRECT_MODE;
>>>> +
>>>> +    data = iio_priv(indio_dev);
>>>> +    data->spi = spi;
>>>> +    data->chip = chip;
>>>> +
>>>> +    ret = iio_triggered_buffer_setup(indio_dev, NULL,
>>>> +                            maxim_thermocouple_trigger_handler, NULL);
>>>> +    if (ret)
>>>> +            return ret;
>>>> +
>>>> +    ret = iio_device_register(indio_dev);
>>>> +    if (ret)
>>>> +            goto error_unreg_buffer;
>>>> +
>>>> +    return 0;
>>>> +
>>>> +error_unreg_buffer:
>>>> +    iio_triggered_buffer_cleanup(indio_dev);
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static int maxim_thermocouple_remove(struct spi_device *spi)
>>>> +{
>>>> +    struct iio_dev *indio_dev = spi_get_drvdata(spi);
>>>> +
>>>> +    iio_device_unregister(indio_dev);
>>>> +    iio_triggered_buffer_cleanup(indio_dev);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static const struct spi_device_id maxim_thermocouple_id[] = {
>>>> +    {"max6675", MAX6675},
>>>> +    {"max31885", MAX31885},
>>>> +    {},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(spi, maxim_thermocouple_id);
>>>> +
>>>> +static struct spi_driver maxim_thermocouple_driver = {
>>>> +    .driver = {
>>>> +            .name   = MAXIM_THERMOCOUPLE_DRV_NAME,
>>>> +    },
>>>> +    .probe          = maxim_thermocouple_probe,
>>>> +    .remove         = maxim_thermocouple_remove,
>>>> +    .id_table       = maxim_thermocouple_id,
>>>> +};
>>>> +module_spi_driver(maxim_thermocouple_driver);
>>>> +
>>>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>>>> +MODULE_DESCRIPTION("Maxim thermocouple sensors");
>>>> +MODULE_LICENSE("GPL");
>>>>
>>>
>>

Re: [PATCH v2] iio: temperature: add support for Maxim thermocouple chips

[Matt Ranostay]

On Sun, May 29, 2016 at 12:00 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 29/05/16 19:47, Matt Ranostay wrote:
>> On Sun, May 29, 2016 at 11:44 AM, Jonathan Cameron <jic23@kernel.org> wrote:
>>> On 29/05/16 09:16, Peter Meerwald-Stadler wrote:
>>>>
>>>>> Add initial driver support for MAX6675, and MAX31885 thermocouple chips.
>>>>
>>>> comments below
>>> Not much to add other than a question inline...
>>>
>>> Jonathan
>>>>
>>>>> Cc: Marek Vasut <marex@denx.de>
>>>>> Cc: Matt Porter <mporter@konsulko.com>
>>>>> Signed-off-by: Matt Ranostay <mranostay@gmail.com>
>>>>> ---
>>>>> Changes from v1:
>>>>> * switch to iio_device_*_direct_mode wrappers
>>>>> * add const to structs
>>>>> * removed useless cast
>>>>>
>>>>> .../iio/temperature/maxim_thermocouple.txt         |  21 ++
>>>>>  drivers/iio/temperature/Kconfig                    |  14 ++
>>>>>  drivers/iio/temperature/Makefile                   |   1 +
>>>>>  drivers/iio/temperature/maxim_thermocouple.c       | 273 +++++++++++++++++++++
>>>>>  4 files changed, 309 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>>>>  create mode 100644 drivers/iio/temperature/maxim_thermocouple.c
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>>>> new file mode 100644
>>>>> index 0000000..44fb029
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/iio/temperature/maxim_thermocouple.txt
>>>>> @@ -0,0 +1,21 @@
>>>>> +Maxim thermocouple support
>>>>> +
>>>>> +* https://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>>>>> +* https://datasheets.maximintegrated.com/en/ds/MAX31855.pdf
>>>>> +
>>>>> +Required properties:
>>>>> +
>>>>> +    - compatible: must be "maxim,max31885" or "maxim,max6675"
>>>>> +    - reg: SPI chip select number for the device
>>>>> +    - spi-max-frequency: must be 4300000
>>>>> +    - spi-cpha: must be defined for max6675 to enable SPI mode 1
>>>>> +
>>>>> +    Refer to spi/spi-bus.txt for generic SPI slave bindings.
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +    max31885@0 {
>>>>> +            compatible = "maxim,max31885";
>>>>> +            reg = <0>;
>>>>> +            spi-max-frequency = <4300000>;
>>>>> +    };
>>>>> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
>>>>> index c4664e5..c9f5342 100644
>>>>> --- a/drivers/iio/temperature/Kconfig
>>>>> +++ b/drivers/iio/temperature/Kconfig
>>>>> @@ -3,6 +3,20 @@
>>>>>  #
>>>>>  menu "Temperature sensors"
>>>>>
>>>>> +config MAXIM_THERMOCOUPLE
>>>>> +    tristate "Maxim thermocouple sensors"
>>>>> +    depends on SPI
>>>>> +    help
>>>>> +      If you say yes here you get support for the Maxim series of
>>>>> +      thermocouple sensors connected via SPI.
>>>>> +
>>>>> +      Supported sensors:
>>>>> +       * MAX6675
>>>>> +       * MAX31885
>>>>> +
>>>>> +      This driver can also be built as a module. If so, the module will
>>>>> +      be called maxim_thermocouple.
>>>>> +
>>>>>  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..78c3de0 100644
>>>>> --- a/drivers/iio/temperature/Makefile
>>>>> +++ b/drivers/iio/temperature/Makefile
>>>>> @@ -2,6 +2,7 @@
>>>>>  # Makefile for industrial I/O temperature drivers
>>>>>  #
>>>>>
>>>>> +obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
>>>>>  obj-$(CONFIG_MLX90614) += mlx90614.o
>>>>>  obj-$(CONFIG_TMP006) += tmp006.o
>>>>>  obj-$(CONFIG_TSYS01) += tsys01.o
>>>>> diff --git a/drivers/iio/temperature/maxim_thermocouple.c b/drivers/iio/temperature/maxim_thermocouple.c
>>>>> new file mode 100644
>>>>> index 0000000..dffe3c2
>>>>> --- /dev/null
>>>>> +++ b/drivers/iio/temperature/maxim_thermocouple.c
>>>>> @@ -0,0 +1,273 @@
>>>>> +/*
>>>>> + * maxim_thermocouple.c  - Support for Maxim thermocouple chips
>>>>> + *
>>>>> + * Copyright (C) 2016 Matt Ranostay <mranostay@gmail.com>
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or modify
>>>>> + * it under the terms of the GNU General Public License as published by
>>>>> + * the Free Software Foundation; either version 2 of the License, or
>>>>> + * (at your option) any later version.
>>>>> + *
>>>>> + * This program is distributed in the hope that it will be useful,
>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>>> + * GNU General Public License for more details.
>>>>> + */
>>>>> +
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/init.h>
>>>>> +#include <linux/mutex.h>
>>>>> +#include <linux/err.h>
>>>>> +#include <linux/spi/spi.h>
>>>>> +#include <linux/iio/iio.h>
>>>>> +#include <linux/iio/trigger.h>
>>>>> +#include <linux/iio/buffer.h>
>>>>> +#include <linux/iio/triggered_buffer.h>
>>>>> +#include <linux/iio/trigger_consumer.h>
>>>>> +
>>>>> +#define MAXIM_THERMOCOUPLE_DRV_NAME "maxim_thermocouple"
>>>>> +
>>>>> +enum {
>>>>> +    MAX6675,
>>>>> +    MAX31885,
>>>>> +};
>>>>> +
>>>>> +const struct iio_chan_spec max6675_channels[] = {
>>>>> +    {       /* thermocouple temperature */
>>>>> +            .type = IIO_TEMP,
>>>>
>>>> .address seems to be used; maybe make it explicit by setting it 0
>>>>
>>>>> +            .info_mask_separate =
>>>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>>>> +            .scan_index = 0,
>>>>> +            .scan_type = {
>>>>> +                    .sign = 's',
>>>>> +                    .realbits = 13,
>>>>> +                    .storagebits = 16,
>>>>> +                    .shift = 3,
>>>>> +                    .endianness = IIO_BE,
>>>>> +            },
>>>>> +    },
>>>>> +    IIO_CHAN_SOFT_TIMESTAMP(1),
>>>>> +};
>>>>> +
>>>>> +const struct iio_chan_spec max31885_channels[] = {
>>>>> +    {       /* thermocouple temperature */
>>>>> +            .type = IIO_TEMP,
>>>>> +            .address = 2,
>>>>> +            .info_mask_separate =
>>>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>>>> +            .scan_index = 0,
>>>>> +            .scan_type = {
>>>>> +                    .sign = 's',
>>>>> +                    .realbits = 14,
>>>>> +                    .storagebits = 16,
>>>>> +                    .shift = 2,
>>>>> +                    .endianness = IIO_BE,
>>>>> +            },
>>>>> +    },
>>>>> +    {       /* cold junction temperature */
>>>>> +            .type = IIO_TEMP,
>>>>> +            .channel2 = IIO_MOD_TEMP_AMBIENT,
>>>>> +            .modified = 1,
>>>>> +            .info_mask_separate =
>>>>> +                    BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
>>>>> +            .scan_index = 1,
>>>>> +            .scan_type = {
>>>>> +                    .sign = 's',
>>>>> +                    .realbits = 12,
>>>>> +                    .storagebits = 16,
>>>>> +                    .shift = 4,
>>>>> +                    .endianness = IIO_BE,
>>>>> +            },
>>>>> +    },
>>>>> +    IIO_CHAN_SOFT_TIMESTAMP(2),
>>>>> +};
>>>>> +
>>>>> +struct maxim_thermocouple_chip {
>>>>> +    const struct iio_chan_spec *channels;
>>>>> +    int num_channels;
>>>>> +    int read_size;
>>>>> +
>>>>> +    /* bit-check for valid input */
>>>>> +    int status_bit;
>>>>> +};
>>>>> +
>>>>> +const struct maxim_thermocouple_chip maxim_thermocouple_chips[] = {
>>>>> +    [MAX6675] = {
>>>>> +                    .channels = max6675_channels,
>>>>> +                    .num_channels = ARRAY_SIZE(max6675_channels),
>>>>> +                    .read_size = 2,
>>>>> +                    .status_bit = BIT(2),
>>>>> +            },
>>>>> +    [MAX31885] = {
>>>>> +                    .channels = max31885_channels,
>>>>> +                    .num_channels = ARRAY_SIZE(max31885_channels),
>>>>> +                    .read_size = 4,
>>>>> +                    .status_bit = BIT(16),
>>>>> +            },
>>>>> +};
>>>>> +
>>>>> +struct maxim_thermocouple_data {
>>>>> +    struct spi_device *spi;
>>>>> +    const struct maxim_thermocouple_chip *chip;
>>>>> +
>>>>> +    u8 buffer[16]; /* 4 byte data + 4 byte padding + 8 byte timestamp */
>>>>> +};
>>>>> +
>>>>> +static int maxim_thermocouple_read(struct maxim_thermocouple_data *data,
>>>>> +                               struct iio_chan_spec const *chan, int *val)
>>>>> +{
>>>>> +    unsigned int storage_bytes = data->chip->read_size;
>>>>> +    unsigned int shift = chan->scan_type.shift + (chan->address * 8);
>>>>> +    unsigned int buf;
>>>>> +    int ret;
>>>>> +
>>>>> +    ret = spi_read(data->spi, (void *) &buf, storage_bytes);
>>>>> +    if (ret)
>>>>> +            return ret;
>>>>> +
>>>>> +    switch (storage_bytes) {
>>>>> +    case 2:
>>>>> +            *val = be16_to_cpu(buf);
>>>>> +            break;
>>>>> +    case 4:
>>>>> +            *val = be32_to_cpu(buf);
>>>>> +            break;
>>>>> +    }
>>>>> +
>>>>> +    /* check to be sure this is a valid reading */
>>>>> +    if (*val & data->chip->status_bit)
>>>>> +            return -EINVAL;
>>>>> +
>>>>> +    *val = sign_extend32(*val >> shift, chan->scan_type.realbits - 1);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static irqreturn_t maxim_thermocouple_trigger_handler(int irq, void *private)
>>>>> +{
>>>>> +    struct iio_poll_func *pf = private;
>>>>> +    struct iio_dev *indio_dev = pf->indio_dev;
>>>>> +    struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>>>>> +    int ret;
>>>>> +
>>>>> +    /* userspace HAL must do validation of data pushed to buffers */
>>> Silly question, but what do you mean by this comment?
>>
>> Mainly that userspace will have to check the status bit itself to be
>> sure a readings is valid.
> That's ugly.  hmm. Might be worth scrubbing the whole result in the buffer
> to make that clear?  Not filling the buffer at all will lead to issues so
> not sure how best to handle this.
>
> Long ago we used to put -1 in timestamp fields when we had no way of knowing when
> the sample occurred - perhaps something similar here...

Works for me. will fix in v3


>>
>>>>> +    ret = spi_read(data->spi, &data->buffer, data->chip->read_size);
>>>>
>>>> read_size can be 4 (bytes) but storagebits can be 16?!
>>>>
>>>>> +    if (!ret)
>>>>> +            iio_push_to_buffers_with_timestamp(indio_dev, data->buffer,
>>>>> +                                               iio_get_time_ns());
>>>>> +
>>>>> +    iio_trigger_notify_done(indio_dev->trig);
>>>>> +
>>>>> +    return IRQ_HANDLED;
>>>>> +}
>>>>> +
>>>>> +static int maxim_thermocouple_read_raw(struct iio_dev *indio_dev,
>>>>> +                                   struct iio_chan_spec const *chan,
>>>>> +                                   int *val, int *val2, long mask)
>>>>> +{
>>>>> +    struct maxim_thermocouple_data *data = iio_priv(indio_dev);
>>>>> +    int ret = -EINVAL;
>>>>> +
>>>>> +    switch (mask) {
>>>>> +    case IIO_CHAN_INFO_RAW:
>>>>> +            if (iio_device_claim_direct_mode(indio_dev))
>>>>> +                    return -EBUSY;
>>>>> +
>>>>> +            ret = maxim_thermocouple_read(data, chan, val);
>>>>> +            if (!ret)
>>>>> +                    ret = IIO_VAL_INT;
>>>>> +
>>>>> +            iio_device_release_direct_mode(indio_dev);
>>>>> +            break;
>>>>> +    case IIO_CHAN_INFO_SCALE:
>>>>> +            switch (chan->channel2) {
>>>>> +            case IIO_MOD_TEMP_AMBIENT:
>>>>> +                    *val = 62;
>>>>> +                    *val2 = 500000; /* 1000 * 0.0625 */
>>>>> +                    ret = IIO_VAL_INT_PLUS_MICRO;
>>>>> +                    break;
>>>>> +            default:
>>>>> +                    *val = 250; /* 1000 * 0.25 */
>>>>> +                    ret = IIO_VAL_INT;
>>>>> +            };
>>>>> +            break;
>>>>> +    }
>>>>> +
>>>>> +    return ret;
>>>>> +}
>>>>> +
>>>>> +static const struct iio_info maxim_thermocouple_info = {
>>>>> +    .driver_module = THIS_MODULE,
>>>>> +    .read_raw = maxim_thermocouple_read_raw,
>>>>> +};
>>>>> +
>>>>> +static int maxim_thermocouple_probe(struct spi_device *spi)
>>>>> +{
>>>>> +    const struct spi_device_id *id = spi_get_device_id(spi);
>>>>> +    struct iio_dev *indio_dev;
>>>>> +    struct maxim_thermocouple_data *data;
>>>>> +    const struct maxim_thermocouple_chip *chip =
>>>>> +                    &maxim_thermocouple_chips[id->driver_data];
>>>>> +    int ret;
>>>>> +
>>>>> +    indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*data));
>>>>> +    if (!indio_dev)
>>>>> +            return -ENOMEM;
>>>>> +
>>>>> +    indio_dev->info = &maxim_thermocouple_info;
>>>>> +    indio_dev->name = MAXIM_THERMOCOUPLE_DRV_NAME;
>>>>> +    indio_dev->channels = chip->channels;
>>>>> +    indio_dev->num_channels = chip->num_channels;
>>>>> +    indio_dev->modes = INDIO_DIRECT_MODE;
>>>>> +
>>>>> +    data = iio_priv(indio_dev);
>>>>> +    data->spi = spi;
>>>>> +    data->chip = chip;
>>>>> +
>>>>> +    ret = iio_triggered_buffer_setup(indio_dev, NULL,
>>>>> +                            maxim_thermocouple_trigger_handler, NULL);
>>>>> +    if (ret)
>>>>> +            return ret;
>>>>> +
>>>>> +    ret = iio_device_register(indio_dev);
>>>>> +    if (ret)
>>>>> +            goto error_unreg_buffer;
>>>>> +
>>>>> +    return 0;
>>>>> +
>>>>> +error_unreg_buffer:
>>>>> +    iio_triggered_buffer_cleanup(indio_dev);
>>>>> +
>>>>> +    return ret;
>>>>> +}
>>>>> +
>>>>> +static int maxim_thermocouple_remove(struct spi_device *spi)
>>>>> +{
>>>>> +    struct iio_dev *indio_dev = spi_get_drvdata(spi);
>>>>> +
>>>>> +    iio_device_unregister(indio_dev);
>>>>> +    iio_triggered_buffer_cleanup(indio_dev);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static const struct spi_device_id maxim_thermocouple_id[] = {
>>>>> +    {"max6675", MAX6675},
>>>>> +    {"max31885", MAX31885},
>>>>> +    {},
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(spi, maxim_thermocouple_id);
>>>>> +
>>>>> +static struct spi_driver maxim_thermocouple_driver = {
>>>>> +    .driver = {
>>>>> +            .name   = MAXIM_THERMOCOUPLE_DRV_NAME,
>>>>> +    },
>>>>> +    .probe          = maxim_thermocouple_probe,
>>>>> +    .remove         = maxim_thermocouple_remove,
>>>>> +    .id_table       = maxim_thermocouple_id,
>>>>> +};
>>>>> +module_spi_driver(maxim_thermocouple_driver);
>>>>> +
>>>>> +MODULE_AUTHOR("Matt Ranostay <mranostay@gmail.com>");
>>>>> +MODULE_DESCRIPTION("Maxim thermocouple sensors");
>>>>> +MODULE_LICENSE("GPL");
>>>>>
>>>>
>>>
>