All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: temperature: add support for Maxim thermocouple chips
@ 2016-05-27  1:31 Matt Ranostay
  2016-05-27  1:58 ` Marek Vasut
  2016-05-27 12:46 ` Daniel Baluta
  0 siblings, 2 replies; 4+ messages in thread
From: Matt Ranostay @ 2016-05-27  1:31 UTC (permalink / raw)
  To: jic23; +Cc: linux-iio, Matt Ranostay, Marek Vasut, Matt Porter

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>
---
 .../iio/temperature/maxim_thermocouple.txt         |  21 ++
 drivers/iio/temperature/Kconfig                    |  14 ++
 drivers/iio/temperature/Makefile                   |   1 +
 drivers/iio/temperature/maxim_thermocouple.c       | 278 +++++++++++++++++++++
 4 files changed, 314 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..7ab4aa1
--- /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..c9c9be9
--- /dev/null
+++ b/drivers/iio/temperature/maxim_thermocouple.c
@@ -0,0 +1,278 @@
+/*
+ * 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,
+};
+
+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),
+};
+
+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;
+};
+
+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;
+	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, (void *) &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;
+
+	mutex_lock(&indio_dev->mlock);
+
+	if (iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) {
+		ret = -EBUSY;
+		goto error_busy;
+	}
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = maxim_thermocouple_read(data, chan, val);
+		if (!ret)
+			ret = IIO_VAL_INT;
+		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;
+	}
+
+error_busy:
+	mutex_unlock(&indio_dev->mlock);
+
+	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;
+	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

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

* Re: [PATCH] iio: temperature: add support for Maxim thermocouple chips
  2016-05-27  1:31 [PATCH] iio: temperature: add support for Maxim thermocouple chips Matt Ranostay
@ 2016-05-27  1:58 ` Marek Vasut
  2016-05-27  2:19   ` Matt Ranostay
  2016-05-27 12:46 ` Daniel Baluta
  1 sibling, 1 reply; 4+ messages in thread
From: Marek Vasut @ 2016-05-27  1:58 UTC (permalink / raw)
  To: Matt Ranostay, jic23; +Cc: linux-iio, Matt Porter

On 05/27/2016 03:31 AM, Matt Ranostay wrote:
> 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>
> ---
>  .../iio/temperature/maxim_thermocouple.txt         |  21 ++
>  drivers/iio/temperature/Kconfig                    |  14 ++
>  drivers/iio/temperature/Makefile                   |   1 +
>  drivers/iio/temperature/maxim_thermocouple.c       | 278 +++++++++++++++++++++
>  4 files changed, 314 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..7ab4aa1
> --- /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

Fullstop is missing here at the end of the sentence ;-)

> +
> +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..c9c9be9
> --- /dev/null
> +++ b/drivers/iio/temperature/maxim_thermocouple.c
> @@ -0,0 +1,278 @@
> +/*
> + * 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,
> +};
> +
> +struct iio_chan_spec max6675_channels[] = {

static const ?

> +	{	/* 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),
> +};
> +
> +struct iio_chan_spec max31885_channels[] = {

Also static const ?

> +	{	/* 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;

I believe all of the members can be const and the structure below can
also be static 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;
> +	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;

if (storage_bytes == 2)
 *val ...
else if (storage_bytes == 4)
 *val ...

is half the length of the code :-)

> +	}
> +
> +	/* 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, (void *) &data->buffer, data->chip->read_size);

I suspect you this should be

ret = spi_read(data->spi, data->buffer, data->chip->read_size);

since data->buffer is already (u8 *). The compiler didn't generate an
error because you typecast the second argument.

> +	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;
> +
> +	mutex_lock(&indio_dev->mlock);
> +
> +	if (iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) {
> +		ret = -EBUSY;
> +		goto error_busy;
> +	}
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = maxim_thermocouple_read(data, chan, val);
> +		if (!ret)
> +			ret = IIO_VAL_INT;
> +		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;
> +	}
> +
> +error_busy:
> +	mutex_unlock(&indio_dev->mlock);
> +
> +	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;
> +	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);

devm_iio_device_register() ?

> +	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");
> 


-- 
Best regards,
Marek Vasut

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

* Re: [PATCH] iio: temperature: add support for Maxim thermocouple chips
  2016-05-27  1:58 ` Marek Vasut
@ 2016-05-27  2:19   ` Matt Ranostay
  0 siblings, 0 replies; 4+ messages in thread
From: Matt Ranostay @ 2016-05-27  2:19 UTC (permalink / raw)
  To: Marek Vasut; +Cc: Jonathan Cameron, linux-iio@vger.kernel.org, Matt Porter

On Thu, May 26, 2016 at 6:58 PM, Marek Vasut <marex@denx.de> wrote:
> On 05/27/2016 03:31 AM, Matt Ranostay wrote:
>> 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>
>> ---
>>  .../iio/temperature/maxim_thermocouple.txt         |  21 ++
>>  drivers/iio/temperature/Kconfig                    |  14 ++
>>  drivers/iio/temperature/Makefile                   |   1 +
>>  drivers/iio/temperature/maxim_thermocouple.c       | 278 +++++++++++++++++++++
>>  4 files changed, 314 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..7ab4aa1
>> --- /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
>
> Fullstop is missing here at the end of the sentence ;-)
>
>> +
>> +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..c9c9be9
>> --- /dev/null
>> +++ b/drivers/iio/temperature/maxim_thermocouple.c
>> @@ -0,0 +1,278 @@
>> +/*
>> + * 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,
>> +};
>> +
>> +struct iio_chan_spec max6675_channels[] = {
>
> static const ?
>
>> +     {       /* 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),
>> +};
>> +
>> +struct iio_chan_spec max31885_channels[] = {
>
> Also static const ?
>
>> +     {       /* 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;
>
> I believe all of the members can be const and the structure below can
> also be static const.

Yeah missed that. Will fix in v2

>
>> +};
>> +
>> +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;
>> +     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;
>
> if (storage_bytes == 2)
>  *val ...
> else if (storage_bytes == 4)
>  *val ...
>
> is half the length of the code :-)
>
>> +     }
>> +
>> +     /* 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, (void *) &data->buffer, data->chip->read_size);
>
> I suspect you this should be
>
> ret = spi_read(data->spi, data->buffer, data->chip->read_size);
>
> since data->buffer is already (u8 *). The compiler didn't generate an
> error because you typecast the second argument.

Ah of course. also gets rid of the only warning from checkpatch.pl as well...

>
>> +     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;
>> +
>> +     mutex_lock(&indio_dev->mlock);
>> +
>> +     if (iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) {
>> +             ret = -EBUSY;
>> +             goto error_busy;
>> +     }
>> +
>> +     switch (mask) {
>> +     case IIO_CHAN_INFO_RAW:
>> +             ret = maxim_thermocouple_read(data, chan, val);
>> +             if (!ret)
>> +                     ret = IIO_VAL_INT;
>> +             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;
>> +     }
>> +
>> +error_busy:
>> +     mutex_unlock(&indio_dev->mlock);
>> +
>> +     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;
>> +     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);
>
> devm_iio_device_register() ?

Can't because of the iio_triggered_buffer_cleanup that is required
since we are using software triggers.
>
>> +     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");
>>
>
>
> --
> Best regards,
> Marek Vasut

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

* Re: [PATCH] iio: temperature: add support for Maxim thermocouple chips
  2016-05-27  1:31 [PATCH] iio: temperature: add support for Maxim thermocouple chips Matt Ranostay
  2016-05-27  1:58 ` Marek Vasut
@ 2016-05-27 12:46 ` Daniel Baluta
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel Baluta @ 2016-05-27 12:46 UTC (permalink / raw)
  To: Matt Ranostay
  Cc: Jonathan Cameron, linux-iio@vger.kernel.org, Marek Vasut,
	Matt Porter

> +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;
> +
> +       mutex_lock(&indio_dev->mlock);
> +

Lets try to use here iio_device_claim_direct_mode wrappers.


> +       if (iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) {
> +               ret = -EBUSY;
> +               goto error_busy;
> +       }

thanks,
Daniel.

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

end of thread, other threads:[~2016-05-27 12:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-27  1:31 [PATCH] iio: temperature: add support for Maxim thermocouple chips Matt Ranostay
2016-05-27  1:58 ` Marek Vasut
2016-05-27  2:19   ` Matt Ranostay
2016-05-27 12:46 ` Daniel Baluta

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