[PATCH 0/3] MAX6675 IIO temperature driver

[PATCH 0/3] MAX6675 IIO temperature driver

[Matt Porter]

This series adds a driver for the MAX6675 SPI thermocouple converter.
The device supports temperature measurements via type-K thermocouples
and implements cold-junction compensation within the part. The datasheet
can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf

Matt Porter (3):
  iio: temperature: add max6675 dt binding
  iio: temperature: add max6675 thermocouple converter driver
  MAINTAINERS: add max6675 driver

 .../bindings/iio/temperature/max6675.txt           |  19 +++
 MAINTAINERS                                        |   7 +
 drivers/iio/temperature/Kconfig                    |  11 ++
 drivers/iio/temperature/Makefile                   |   1 +
 drivers/iio/temperature/max6675.c                  | 155 +++++++++++++++++++++
 5 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/max6675.txt
 create mode 100644 drivers/iio/temperature/max6675.c

-- 
2.1.4

[PATCH 0/3] MAX6675 IIO temperature driver

[Matt Porter]

This series adds a driver for the MAX6675 SPI thermocouple converter.
The device supports temperature measurements via type-K thermocouples
and implements cold-junction compensation within the part. The datasheet
can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf

Matt Porter (3):
  iio: temperature: add max6675 dt binding
  iio: temperature: add max6675 thermocouple converter driver
  MAINTAINERS: add max6675 driver

 .../bindings/iio/temperature/max6675.txt           |  19 +++
 MAINTAINERS                                        |   7 +
 drivers/iio/temperature/Kconfig                    |  11 ++
 drivers/iio/temperature/Makefile                   |   1 +
 drivers/iio/temperature/max6675.c                  | 155 +++++++++++++++++++++
 5 files changed, 193 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/max6675.txt
 create mode 100644 drivers/iio/temperature/max6675.c

-- 
2.1.4

[PATCH 1/3] iio: temperature: add max6675 dt binding

[Matt Porter]

Add a DT binding for the MAX6675 SPI thermocouple converter.
The datasheet for this device may be found at
http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf

Signed-off-by: Matt Porter <mporter@konsulko.com>
---
 .../devicetree/bindings/iio/temperature/max6675.txt   | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/max6675.txt

diff --git a/Documentation/devicetree/bindings/iio/temperature/max6675.txt b/Documentation/devicetree/bindings/iio/temperature/max6675.txt
new file mode 100644
index 0000000..4e643e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/max6675.txt
@@ -0,0 +1,19 @@
+Maxim MAX6675 thermocouple converter
+
+Required properties:
+
+	- compatible: must be "maxim,max6675"
+	- reg: SPI chip select number for the device
+	- spi-max-frequency: must be 4300000
+	- spi-cpha: SPI Mode 1.
+
+	Refer to spi/spi-bus.txt for generic SPI slave bindings
+
+Example:
+
+	max6675@0 {
+		compatible = "maxim,max6675";
+		reg = <0>;
+		spi-max-frequency = <4300000>;
+		spi-cpha;
+	};
-- 
2.1.4

[PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

Add a driver for the MAX6675 thermocouple converter. This
device interfaces with K-type thermocouples and provides
cold-junction compensated temperature readings via a
SPI interface.

Signed-off-by: Matt Porter <mporter@konsulko.com>
---
 drivers/iio/temperature/Kconfig   |  11 +++
 drivers/iio/temperature/Makefile  |   1 +
 drivers/iio/temperature/max6675.c | 155 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 167 insertions(+)
 create mode 100644 drivers/iio/temperature/max6675.c

diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 21feaa4..b73fbf4 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -3,6 +3,17 @@
 #
 menu "Temperature sensors"
 
+config MAX6675
+	tristate "MAX6675 thermocouple converter"
+	depends on SPI
+	help
+	  If you say yes here you get support for the Maxim
+	  MAX6675 thermocouple converter connected with SPI.
+
+	  This driver can also be built as a module. If so, the module will
+	  be called max6675.
+
+
 config MLX90614
 	tristate "MLX90614 contact-less infrared sensor"
 	depends on I2C
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 40710a8..d67ef40 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -2,5 +2,6 @@
 # Makefile for industrial I/O temperature drivers
 #
 
+obj-$(CONFIG_MAX6675) += max6675.o
 obj-$(CONFIG_MLX90614) += mlx90614.o
 obj-$(CONFIG_TMP006) += tmp006.o
diff --git a/drivers/iio/temperature/max6675.c b/drivers/iio/temperature/max6675.c
new file mode 100644
index 0000000..2d1fda2
--- /dev/null
+++ b/drivers/iio/temperature/max6675.c
@@ -0,0 +1,155 @@
+/*
+ * max6675.c - MAX6675 thermocouple converter driver
+ *
+ * Copyright (C) 2015 Konsulko Group, Matt Porter <mporter@konsulko.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/acpi.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/iio/iio.h>
+#include <linux/spi/spi.h>
+
+struct max6675_state {
+	struct spi_device *spi;
+};
+
+static const struct iio_chan_spec max6675_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate =
+			BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static int max6675_read(struct max6675_state *st, int *val)
+{
+	int ret;
+
+	ret = spi_read(st->spi, val, 2);
+	if (ret < 0)
+		return ret;
+
+	/* Temperature is bits 14..3 */
+	*val = (*val >> 3) & 0xfff;
+
+	return ret;
+}
+
+static int max6675_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val,
+			    int *val2,
+			    long m)
+{
+	struct max6675_state *st = iio_priv(indio_dev);
+	int ret;
+
+	if (m == IIO_CHAN_INFO_RAW) {
+		*val2 = 0;
+		ret = max6675_read(st, val);
+		if (ret)
+			return ret;
+	} else if (m == IIO_CHAN_INFO_SCALE) {
+		*val = 250;
+		*val2 = 0;
+	} else
+		return -EINVAL;
+
+	return IIO_VAL_INT;
+}
+
+static const struct iio_info max6675_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &max6675_read_raw,
+};
+
+static int max6675_probe(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev;
+	struct max6675_state *st;
+	int ret = 0;
+
+	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+
+	spi->mode = SPI_MODE_1;
+	spi->bits_per_word = 16;
+
+	spi_set_drvdata(spi, indio_dev);
+
+	indio_dev->dev.parent = &spi->dev;
+	indio_dev->name = spi_get_device_id(spi)->name;
+	indio_dev->channels = max6675_channels;
+	indio_dev->num_channels = ARRAY_SIZE(max6675_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &max6675_info;
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		dev_err(&spi->dev, "unable to register device\n");
+
+	return ret;
+}
+
+static int max6675_remove(struct spi_device *spi)
+{
+	struct iio_dev *indio_dev = spi_get_drvdata(spi);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct acpi_device_id max6675_acpi_ids[] = {
+	{ "MXIM6675", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(acpi, max6675_acpi_ids);
+
+static const struct of_device_id max6675_dt_ids[] = {
+	{ .compatible = "maxim,max6675" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max6675_dt_ids);
+
+static const struct spi_device_id max6675_spi_ids[] = {
+	{"max6675", 0},
+	{},
+};
+MODULE_DEVICE_TABLE(spi, max6675_spi_ids);
+
+static struct spi_driver max6675_driver = {
+	.driver = {
+		.name	= "max6675",
+		.owner	= THIS_MODULE,
+		.acpi_match_table = ACPI_PTR(max6675_acpi_ids),
+		.of_match_table = of_match_ptr(max6675_dt_ids),
+	},
+	.probe		= max6675_probe,
+	.remove		= max6675_remove,
+	.id_table	= max6675_spi_ids,
+};
+module_spi_driver(max6675_driver);
+
+MODULE_AUTHOR("Matt Porter <mporter@konsulko.com>");
+MODULE_DESCRIPTION("MAX6675 thermocouple converter driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("spi:max6675");
-- 
2.1.4

[PATCH 3/3] MAINTAINERS: add max6675 driver

[Matt Porter]

Add myself as the MAX6675 driver maintainer.

Signed-off-by: Matt Porter <mporter@konsulko.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a226416..355cc09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6543,6 +6543,13 @@ S:	Maintained
 F:	Documentation/hwmon/max6650
 F:	drivers/hwmon/max6650.c
 
+MAX6675 THERMOCOUPLE CONVERTER DRIVER
+M:	Matt Porter <mporter@konsulko.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/temperature/max6675.txt
+F:	drivers/iio/temperature/max6675.c
+
 MAX6697 HARDWARE MONITOR DRIVER
 M:	Guenter Roeck <linux@roeck-us.net>
 L:	lm-sensors@lm-sensors.org
-- 
2.1.4

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Peter Meerwald]

On Mon, 3 Aug 2015, Matt Porter wrote:

> Add a driver for the MAX6675 thermocouple converter. This
> device interfaces with K-type thermocouples and provides
> cold-junction compensated temperature readings via a
> SPI interface.

comments below
 
> Signed-off-by: Matt Porter <mporter@konsulko.com>
> ---
>  drivers/iio/temperature/Kconfig   |  11 +++
>  drivers/iio/temperature/Makefile  |   1 +
>  drivers/iio/temperature/max6675.c | 155 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 167 insertions(+)
>  create mode 100644 drivers/iio/temperature/max6675.c
> 
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 21feaa4..b73fbf4 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -3,6 +3,17 @@
>  #
>  menu "Temperature sensors"
>  
> +config MAX6675
> +	tristate "MAX6675 thermocouple converter"
> +	depends on SPI
> +	help
> +	  If you say yes here you get support for the Maxim
> +	  MAX6675 thermocouple converter connected with SPI.
> +
> +	  This driver can also be built as a module. If so, the module will
> +	  be called max6675.
> +

extra newline

> +
>  config MLX90614
>  	tristate "MLX90614 contact-less infrared sensor"
>  	depends on I2C
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 40710a8..d67ef40 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -2,5 +2,6 @@
>  # Makefile for industrial I/O temperature drivers
>  #
>  
> +obj-$(CONFIG_MAX6675) += max6675.o
>  obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
> diff --git a/drivers/iio/temperature/max6675.c b/drivers/iio/temperature/max6675.c
> new file mode 100644
> index 0000000..2d1fda2
> --- /dev/null
> +++ b/drivers/iio/temperature/max6675.c
> @@ -0,0 +1,155 @@
> +/*
> + * max6675.c - MAX6675 thermocouple converter driver
> + *
> + * Copyright (C) 2015 Konsulko Group, Matt Porter <mporter@konsulko.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/acpi.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/iio/iio.h>
> +#include <linux/spi/spi.h>
> +
> +struct max6675_state {
> +	struct spi_device *spi;
> +};
> +
> +static const struct iio_chan_spec max6675_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate =
> +			BIT(IIO_CHAN_INFO_RAW) |
> +			BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static int max6675_read(struct max6675_state *st, int *val)
> +{
> +	int ret;
> +
> +	ret = spi_read(st->spi, val, 2);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Temperature is bits 14..3 */
> +	*val = (*val >> 3) & 0xfff;

what about endianness conversion?
use be16_to_cpu()

> +
> +	return ret;
> +}
> +
> +static int max6675_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val,
> +			    int *val2,
> +			    long m)
> +{
> +	struct max6675_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (m == IIO_CHAN_INFO_RAW) {
> +		*val2 = 0;

no need to set *val2 to 0 when returning VAL_INT

> +		ret = max6675_read(st, val);
> +		if (ret)
> +			return ret;
> +	} else if (m == IIO_CHAN_INFO_SCALE) {
> +		*val = 250;
> +		*val2 = 0;
> +	} else
> +		return -EINVAL;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info max6675_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = &max6675_read_raw,
> +};
> +
> +static int max6675_probe(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev;
> +	struct max6675_state *st;
> +	int ret = 0;

no need to initialize ret

> +
> +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->spi = spi;
> +
> +	spi->mode = SPI_MODE_1;
> +	spi->bits_per_word = 16;
> +
> +	spi_set_drvdata(spi, indio_dev);
> +
> +	indio_dev->dev.parent = &spi->dev;
> +	indio_dev->name = spi_get_device_id(spi)->name;
> +	indio_dev->channels = max6675_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(max6675_channels);
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &max6675_info;
> +
> +	ret = iio_device_register(indio_dev);

devm_ and drop _remove()

> +	if (ret < 0)
> +		dev_err(&spi->dev, "unable to register device\n");
> +
> +	return ret;
> +}
> +
> +static int max6675_remove(struct spi_device *spi)
> +{
> +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +	iio_device_unregister(indio_dev);
> +
> +	return 0;
> +}
> +
> +static const struct acpi_device_id max6675_acpi_ids[] = {
> +	{ "MXIM6675", 0 },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(acpi, max6675_acpi_ids);
> +
> +static const struct of_device_id max6675_dt_ids[] = {
> +	{ .compatible = "maxim,max6675" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, max6675_dt_ids);
> +
> +static const struct spi_device_id max6675_spi_ids[] = {
> +	{"max6675", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, max6675_spi_ids);
> +
> +static struct spi_driver max6675_driver = {
> +	.driver = {
> +		.name	= "max6675",
> +		.owner	= THIS_MODULE,
> +		.acpi_match_table = ACPI_PTR(max6675_acpi_ids),
> +		.of_match_table = of_match_ptr(max6675_dt_ids),
> +	},
> +	.probe		= max6675_probe,
> +	.remove		= max6675_remove,
> +	.id_table	= max6675_spi_ids,
> +};
> +module_spi_driver(max6675_driver);
> +
> +MODULE_AUTHOR("Matt Porter <mporter@konsulko.com>");
> +MODULE_DESCRIPTION("MAX6675 thermocouple converter driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("spi:max6675");
> 

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Ranostay]

On Mon, Aug 3, 2015 at 1:56 PM, Matt Porter <mporter@konsulko.com> wrote:
> Add a driver for the MAX6675 thermocouple converter. This
> device interfaces with K-type thermocouples and provides
> cold-junction compensated temperature readings via a
> SPI interface.
>
> Signed-off-by: Matt Porter <mporter@konsulko.com>
> ---
>  drivers/iio/temperature/Kconfig   |  11 +++
>  drivers/iio/temperature/Makefile  |   1 +
>  drivers/iio/temperature/max6675.c | 155 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 167 insertions(+)
>  create mode 100644 drivers/iio/temperature/max6675.c
>
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 21feaa4..b73fbf4 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -3,6 +3,17 @@
>  #
>  menu "Temperature sensors"
>
> +config MAX6675
> +       tristate "MAX6675 thermocouple converter"
> +       depends on SPI
> +       help
> +         If you say yes here you get support for the Maxim
> +         MAX6675 thermocouple converter connected with SPI.
> +
> +         This driver can also be built as a module. If so, the module will
> +         be called max6675.
> +
> +
>  config MLX90614
>         tristate "MLX90614 contact-less infrared sensor"
>         depends on I2C
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 40710a8..d67ef40 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -2,5 +2,6 @@
>  # Makefile for industrial I/O temperature drivers
>  #
>
> +obj-$(CONFIG_MAX6675) += max6675.o
>  obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
> diff --git a/drivers/iio/temperature/max6675.c b/drivers/iio/temperature/max6675.c
> new file mode 100644
> index 0000000..2d1fda2
> --- /dev/null
> +++ b/drivers/iio/temperature/max6675.c
> @@ -0,0 +1,155 @@
> +/*
> + * max6675.c - MAX6675 thermocouple converter driver
> + *
> + * Copyright (C) 2015 Konsulko Group, Matt Porter <mporter@konsulko.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/acpi.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/iio/iio.h>
> +#include <linux/spi/spi.h>
> +
> +struct max6675_state {
> +       struct spi_device *spi;
> +};
> +
> +static const struct iio_chan_spec max6675_channels[] = {
> +       {
> +               .type = IIO_TEMP,
> +               .info_mask_separate =
> +                       BIT(IIO_CHAN_INFO_RAW) |
> +                       BIT(IIO_CHAN_INFO_SCALE),
> +       },
> +};
> +
> +static int max6675_read(struct max6675_state *st, int *val)
> +{
> +       int ret;
> +
> +       ret = spi_read(st->spi, val, 2);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* Temperature is bits 14..3 */
> +       *val = (*val >> 3) & 0xfff;
> +
> +       return ret;
> +}
> +
> +static int max6675_read_raw(struct iio_dev *indio_dev,
> +                           struct iio_chan_spec const *chan,
> +                           int *val,
> +                           int *val2,
> +                           long m)
> +{
> +       struct max6675_state *st = iio_priv(indio_dev);
> +       int ret;
> +
> +       if (m == IIO_CHAN_INFO_RAW) {
> +               *val2 = 0;
> +               ret = max6675_read(st, val);
> +               if (ret)
> +                       return ret;
> +       } else if (m == IIO_CHAN_INFO_SCALE) {
> +               *val = 250;
> +               *val2 = 0;
> +       } else
> +               return -EINVAL;
> +
> +       return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info max6675_info = {
> +       .driver_module = THIS_MODULE,
> +       .read_raw = &max6675_read_raw,
> +};
> +
> +static int max6675_probe(struct spi_device *spi)
> +{
> +       struct iio_dev *indio_dev;
> +       struct max6675_state *st;
> +       int ret = 0;
> +
> +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       st = iio_priv(indio_dev);
> +       st->spi = spi;
> +
> +       spi->mode = SPI_MODE_1;
> +       spi->bits_per_word = 16;

Have this error or display a warning when it doesn't match the passed
DT binding settings. Otherwise it may get confusing why other SPI
modes and word sizes don't work.

> +
> +       spi_set_drvdata(spi, indio_dev);
> +
> +       indio_dev->dev.parent = &spi->dev;
> +       indio_dev->name = spi_get_device_id(spi)->name;
> +       indio_dev->channels = max6675_channels;
> +       indio_dev->num_channels = ARRAY_SIZE(max6675_channels);
> +       indio_dev->modes = INDIO_DIRECT_MODE;
> +       indio_dev->info = &max6675_info;
> +
> +       ret = iio_device_register(indio_dev);
> +       if (ret < 0)
> +               dev_err(&spi->dev, "unable to register device\n");
> +
> +       return ret;
> +}
> +
> +static int max6675_remove(struct spi_device *spi)
> +{
> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +       iio_device_unregister(indio_dev);
> +
> +       return 0;
> +}
> +
> +static const struct acpi_device_id max6675_acpi_ids[] = {
> +       { "MXIM6675", 0 },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(acpi, max6675_acpi_ids);
> +
> +static const struct of_device_id max6675_dt_ids[] = {
> +       { .compatible = "maxim,max6675" },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, max6675_dt_ids);
> +
> +static const struct spi_device_id max6675_spi_ids[] = {
> +       {"max6675", 0},
> +       {},
> +};
> +MODULE_DEVICE_TABLE(spi, max6675_spi_ids);
> +
> +static struct spi_driver max6675_driver = {
> +       .driver = {
> +               .name   = "max6675",
> +               .owner  = THIS_MODULE,
> +               .acpi_match_table = ACPI_PTR(max6675_acpi_ids),
> +               .of_match_table = of_match_ptr(max6675_dt_ids),
> +       },
> +       .probe          = max6675_probe,
> +       .remove         = max6675_remove,
> +       .id_table       = max6675_spi_ids,
> +};
> +module_spi_driver(max6675_driver);
> +
> +MODULE_AUTHOR("Matt Porter <mporter@konsulko.com>");
> +MODULE_DESCRIPTION("MAX6675 thermocouple converter driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("spi:max6675");
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Ranostay]

On Mon, Aug 3, 2015 at 1:56 PM, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> Add a driver for the MAX6675 thermocouple converter. This
> device interfaces with K-type thermocouples and provides
> cold-junction compensated temperature readings via a
> SPI interface.
>
> Signed-off-by: Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/iio/temperature/Kconfig   |  11 +++
>  drivers/iio/temperature/Makefile  |   1 +
>  drivers/iio/temperature/max6675.c | 155 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 167 insertions(+)
>  create mode 100644 drivers/iio/temperature/max6675.c
>
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 21feaa4..b73fbf4 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -3,6 +3,17 @@
>  #
>  menu "Temperature sensors"
>
> +config MAX6675
> +       tristate "MAX6675 thermocouple converter"
> +       depends on SPI
> +       help
> +         If you say yes here you get support for the Maxim
> +         MAX6675 thermocouple converter connected with SPI.
> +
> +         This driver can also be built as a module. If so, the module will
> +         be called max6675.
> +
> +
>  config MLX90614
>         tristate "MLX90614 contact-less infrared sensor"
>         depends on I2C
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 40710a8..d67ef40 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -2,5 +2,6 @@
>  # Makefile for industrial I/O temperature drivers
>  #
>
> +obj-$(CONFIG_MAX6675) += max6675.o
>  obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
> diff --git a/drivers/iio/temperature/max6675.c b/drivers/iio/temperature/max6675.c
> new file mode 100644
> index 0000000..2d1fda2
> --- /dev/null
> +++ b/drivers/iio/temperature/max6675.c
> @@ -0,0 +1,155 @@
> +/*
> + * max6675.c - MAX6675 thermocouple converter driver
> + *
> + * Copyright (C) 2015 Konsulko Group, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
> + *
> + * 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/acpi.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/iio/iio.h>
> +#include <linux/spi/spi.h>
> +
> +struct max6675_state {
> +       struct spi_device *spi;
> +};
> +
> +static const struct iio_chan_spec max6675_channels[] = {
> +       {
> +               .type = IIO_TEMP,
> +               .info_mask_separate =
> +                       BIT(IIO_CHAN_INFO_RAW) |
> +                       BIT(IIO_CHAN_INFO_SCALE),
> +       },
> +};
> +
> +static int max6675_read(struct max6675_state *st, int *val)
> +{
> +       int ret;
> +
> +       ret = spi_read(st->spi, val, 2);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* Temperature is bits 14..3 */
> +       *val = (*val >> 3) & 0xfff;
> +
> +       return ret;
> +}
> +
> +static int max6675_read_raw(struct iio_dev *indio_dev,
> +                           struct iio_chan_spec const *chan,
> +                           int *val,
> +                           int *val2,
> +                           long m)
> +{
> +       struct max6675_state *st = iio_priv(indio_dev);
> +       int ret;
> +
> +       if (m == IIO_CHAN_INFO_RAW) {
> +               *val2 = 0;
> +               ret = max6675_read(st, val);
> +               if (ret)
> +                       return ret;
> +       } else if (m == IIO_CHAN_INFO_SCALE) {
> +               *val = 250;
> +               *val2 = 0;
> +       } else
> +               return -EINVAL;
> +
> +       return IIO_VAL_INT;
> +}
> +
> +static const struct iio_info max6675_info = {
> +       .driver_module = THIS_MODULE,
> +       .read_raw = &max6675_read_raw,
> +};
> +
> +static int max6675_probe(struct spi_device *spi)
> +{
> +       struct iio_dev *indio_dev;
> +       struct max6675_state *st;
> +       int ret = 0;
> +
> +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       st = iio_priv(indio_dev);
> +       st->spi = spi;
> +
> +       spi->mode = SPI_MODE_1;
> +       spi->bits_per_word = 16;

Have this error or display a warning when it doesn't match the passed
DT binding settings. Otherwise it may get confusing why other SPI
modes and word sizes don't work.

> +
> +       spi_set_drvdata(spi, indio_dev);
> +
> +       indio_dev->dev.parent = &spi->dev;
> +       indio_dev->name = spi_get_device_id(spi)->name;
> +       indio_dev->channels = max6675_channels;
> +       indio_dev->num_channels = ARRAY_SIZE(max6675_channels);
> +       indio_dev->modes = INDIO_DIRECT_MODE;
> +       indio_dev->info = &max6675_info;
> +
> +       ret = iio_device_register(indio_dev);
> +       if (ret < 0)
> +               dev_err(&spi->dev, "unable to register device\n");
> +
> +       return ret;
> +}
> +
> +static int max6675_remove(struct spi_device *spi)
> +{
> +       struct iio_dev *indio_dev = spi_get_drvdata(spi);
> +
> +       iio_device_unregister(indio_dev);
> +
> +       return 0;
> +}
> +
> +static const struct acpi_device_id max6675_acpi_ids[] = {
> +       { "MXIM6675", 0 },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(acpi, max6675_acpi_ids);
> +
> +static const struct of_device_id max6675_dt_ids[] = {
> +       { .compatible = "maxim,max6675" },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, max6675_dt_ids);
> +
> +static const struct spi_device_id max6675_spi_ids[] = {
> +       {"max6675", 0},
> +       {},
> +};
> +MODULE_DEVICE_TABLE(spi, max6675_spi_ids);
> +
> +static struct spi_driver max6675_driver = {
> +       .driver = {
> +               .name   = "max6675",
> +               .owner  = THIS_MODULE,
> +               .acpi_match_table = ACPI_PTR(max6675_acpi_ids),
> +               .of_match_table = of_match_ptr(max6675_dt_ids),
> +       },
> +       .probe          = max6675_probe,
> +       .remove         = max6675_remove,
> +       .id_table       = max6675_spi_ids,
> +};
> +module_spi_driver(max6675_driver);
> +
> +MODULE_AUTHOR("Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>");
> +MODULE_DESCRIPTION("MAX6675 thermocouple converter driver");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("spi:max6675");
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

On Mon, Aug 03, 2015 at 03:39:00PM -0700, Matt Ranostay Matt Ranostay wrote:
> On Mon, Aug 3, 2015 at 1:56 PM, Matt Porter <mporter@konsulko.com> wrote:

...

> > +static int max6675_probe(struct spi_device *spi)
> > +{
> > +       struct iio_dev *indio_dev;
> > +       struct max6675_state *st;
> > +       int ret = 0;
> > +
> > +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +       if (!indio_dev)
> > +               return -ENOMEM;
> > +
> > +       st = iio_priv(indio_dev);
> > +       st->spi = spi;
> > +
> > +       spi->mode = SPI_MODE_1;
> > +       spi->bits_per_word = 16;
> 
> Have this error or display a warning when it doesn't match the passed
> DT binding settings. Otherwise it may get confusing why other SPI
> modes and word sizes don't work.

Ok, good point. The only thing here is that I've specified that spi-cpha
is required in the binding, indicating that Mode 1 will be used. I need
this driver to be instantiated via three methods: ACPI, DT, and "board
file" so for the latter I'm hardcoding in the driver the mode. The
device only works in Mode 1 so this seems sane, it's not configurable.
I don't parse that mode from either DT or ACPI data since it's not
needed. About the only thing I could do is pedantically check for
spi-cpha and if not present complain..but I think it's fine to simply
not parse at all given that we've hardcoded this for the allowed mode.

There's no property for bits_per_word, it's configured on a per-transfer
basis. In this case, this configures the default to do a 16-bit transfer
as required by the device. There is no way to modify this by a client of
this driver.

-Matt

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

On Mon, Aug 03, 2015 at 03:39:00PM -0700, Matt Ranostay Matt Ranostay wrote:
> On Mon, Aug 3, 2015 at 1:56 PM, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:

...

> > +static int max6675_probe(struct spi_device *spi)
> > +{
> > +       struct iio_dev *indio_dev;
> > +       struct max6675_state *st;
> > +       int ret = 0;
> > +
> > +       indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +       if (!indio_dev)
> > +               return -ENOMEM;
> > +
> > +       st = iio_priv(indio_dev);
> > +       st->spi = spi;
> > +
> > +       spi->mode = SPI_MODE_1;
> > +       spi->bits_per_word = 16;
> 
> Have this error or display a warning when it doesn't match the passed
> DT binding settings. Otherwise it may get confusing why other SPI
> modes and word sizes don't work.

Ok, good point. The only thing here is that I've specified that spi-cpha
is required in the binding, indicating that Mode 1 will be used. I need
this driver to be instantiated via three methods: ACPI, DT, and "board
file" so for the latter I'm hardcoding in the driver the mode. The
device only works in Mode 1 so this seems sane, it's not configurable.
I don't parse that mode from either DT or ACPI data since it's not
needed. About the only thing I could do is pedantically check for
spi-cpha and if not present complain..but I think it's fine to simply
not parse at all given that we've hardcoded this for the allowed mode.

There's no property for bits_per_word, it's configured on a per-transfer
basis. In this case, this configures the default to do a 16-bit transfer
as required by the device. There is no way to modify this by a client of
this driver.

-Matt

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

On Mon, Aug 03, 2015 at 11:26:12PM +0200, Peter Meerwald wrote:
> On Mon, 3 Aug 2015, Matt Porter wrote:
> 
> > Add a driver for the MAX6675 thermocouple converter. This
> > device interfaces with K-type thermocouples and provides
> > cold-junction compensated temperature readings via a
> > SPI interface.
> 
> comments below

Thanks for the fast turnaround on the review! Will incorporate
the comments into v2 as noted below.

> > Signed-off-by: Matt Porter <mporter@konsulko.com>
> > ---
> >  drivers/iio/temperature/Kconfig   |  11 +++
> >  drivers/iio/temperature/Makefile  |   1 +
> >  drivers/iio/temperature/max6675.c | 155 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 167 insertions(+)
> >  create mode 100644 drivers/iio/temperature/max6675.c
> > 
> > diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> > index 21feaa4..b73fbf4 100644
> > --- a/drivers/iio/temperature/Kconfig
> > +++ b/drivers/iio/temperature/Kconfig
> > @@ -3,6 +3,17 @@
> >  #
> >  menu "Temperature sensors"
> >  
> > +config MAX6675
> > +	tristate "MAX6675 thermocouple converter"
> > +	depends on SPI
> > +	help
> > +	  If you say yes here you get support for the Maxim
> > +	  MAX6675 thermocouple converter connected with SPI.
> > +
> > +	  This driver can also be built as a module. If so, the module will
> > +	  be called max6675.
> > +
> 
> extra newline

Ok

> 
> > +
> >  config MLX90614
> >  	tristate "MLX90614 contact-less infrared sensor"
> >  	depends on I2C
> > diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> > index 40710a8..d67ef40 100644
> > --- a/drivers/iio/temperature/Makefile
> > +++ b/drivers/iio/temperature/Makefile
> > @@ -2,5 +2,6 @@
> >  # Makefile for industrial I/O temperature drivers
> >  #
> >  
> > +obj-$(CONFIG_MAX6675) += max6675.o
> >  obj-$(CONFIG_MLX90614) += mlx90614.o
> >  obj-$(CONFIG_TMP006) += tmp006.o
> > diff --git a/drivers/iio/temperature/max6675.c b/drivers/iio/temperature/max6675.c
> > new file mode 100644
> > index 0000000..2d1fda2
> > --- /dev/null
> > +++ b/drivers/iio/temperature/max6675.c
> > @@ -0,0 +1,155 @@
> > +/*
> > + * max6675.c - MAX6675 thermocouple converter driver
> > + *
> > + * Copyright (C) 2015 Konsulko Group, Matt Porter <mporter@konsulko.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/acpi.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/spi/spi.h>
> > +
> > +struct max6675_state {
> > +	struct spi_device *spi;
> > +};
> > +
> > +static const struct iio_chan_spec max6675_channels[] = {
> > +	{
> > +		.type = IIO_TEMP,
> > +		.info_mask_separate =
> > +			BIT(IIO_CHAN_INFO_RAW) |
> > +			BIT(IIO_CHAN_INFO_SCALE),
> > +	},
> > +};
> > +
> > +static int max6675_read(struct max6675_state *st, int *val)
> > +{
> > +	int ret;
> > +
> > +	ret = spi_read(st->spi, val, 2);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Temperature is bits 14..3 */
> > +	*val = (*val >> 3) & 0xfff;
> 
> what about endianness conversion?
> use be16_to_cpu()

Ok, will fix.

> > +
> > +	return ret;
> > +}
> > +
> > +static int max6675_read_raw(struct iio_dev *indio_dev,
> > +			    struct iio_chan_spec const *chan,
> > +			    int *val,
> > +			    int *val2,
> > +			    long m)
> > +{
> > +	struct max6675_state *st = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	if (m == IIO_CHAN_INFO_RAW) {
> > +		*val2 = 0;
> 
> no need to set *val2 to 0 when returning VAL_INT

Ok

> 
> > +		ret = max6675_read(st, val);
> > +		if (ret)
> > +			return ret;
> > +	} else if (m == IIO_CHAN_INFO_SCALE) {
> > +		*val = 250;
> > +		*val2 = 0;
> > +	} else
> > +		return -EINVAL;
> > +
> > +	return IIO_VAL_INT;
> > +}
> > +
> > +static const struct iio_info max6675_info = {
> > +	.driver_module = THIS_MODULE,
> > +	.read_raw = &max6675_read_raw,
> > +};
> > +
> > +static int max6675_probe(struct spi_device *spi)
> > +{
> > +	struct iio_dev *indio_dev;
> > +	struct max6675_state *st;
> > +	int ret = 0;
> 
> no need to initialize ret

Ok

> 
> > +
> > +	indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	st = iio_priv(indio_dev);
> > +	st->spi = spi;
> > +
> > +	spi->mode = SPI_MODE_1;
> > +	spi->bits_per_word = 16;
> > +
> > +	spi_set_drvdata(spi, indio_dev);
> > +
> > +	indio_dev->dev.parent = &spi->dev;
> > +	indio_dev->name = spi_get_device_id(spi)->name;
> > +	indio_dev->channels = max6675_channels;
> > +	indio_dev->num_channels = ARRAY_SIZE(max6675_channels);
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +	indio_dev->info = &max6675_info;
> > +
> > +	ret = iio_device_register(indio_dev);
> 
> devm_ and drop _remove()

Ok

> 
> > +	if (ret < 0)
> > +		dev_err(&spi->dev, "unable to register device\n");
> > +
> > +	return ret;
> > +}
> > +
> > +static int max6675_remove(struct spi_device *spi)
> > +{
> > +	struct iio_dev *indio_dev = spi_get_drvdata(spi);
> > +
> > +	iio_device_unregister(indio_dev);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct acpi_device_id max6675_acpi_ids[] = {
> > +	{ "MXIM6675", 0 },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(acpi, max6675_acpi_ids);
> > +
> > +static const struct of_device_id max6675_dt_ids[] = {
> > +	{ .compatible = "maxim,max6675" },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, max6675_dt_ids);
> > +
> > +static const struct spi_device_id max6675_spi_ids[] = {
> > +	{"max6675", 0},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(spi, max6675_spi_ids);
> > +
> > +static struct spi_driver max6675_driver = {
> > +	.driver = {
> > +		.name	= "max6675",
> > +		.owner	= THIS_MODULE,
> > +		.acpi_match_table = ACPI_PTR(max6675_acpi_ids),
> > +		.of_match_table = of_match_ptr(max6675_dt_ids),
> > +	},
> > +	.probe		= max6675_probe,
> > +	.remove		= max6675_remove,
> > +	.id_table	= max6675_spi_ids,
> > +};
> > +module_spi_driver(max6675_driver);
> > +
> > +MODULE_AUTHOR("Matt Porter <mporter@konsulko.com>");
> > +MODULE_DESCRIPTION("MAX6675 thermocouple converter driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_ALIAS("spi:max6675");
> > 
> 
> -- 
> 
> Peter Meerwald
> +43-664-2444418 (mobile)

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Daniel Baluta]

On Mon, Aug 3, 2015 at 11:56 PM, Matt Porter <mporter@konsulko.com> wrote:
> Add a driver for the MAX6675 thermocouple converter. This
> device interfaces with K-type thermocouples and provides
> cold-junction compensated temperature readings via a
> SPI interface.

./scripts/checkpatch.pl --strict says:

CHECK: braces {} should be used on all arms of this statement
#118: FILE: drivers/iio/temperature/max6675.c:61:

thanks,
Daniel.

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Daniel Baluta]

On Mon, Aug 3, 2015 at 11:56 PM, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> Add a driver for the MAX6675 thermocouple converter. This
> device interfaces with K-type thermocouples and provides
> cold-junction compensated temperature readings via a
> SPI interface.

./scripts/checkpatch.pl --strict says:

CHECK: braces {} should be used on all arms of this statement
#118: FILE: drivers/iio/temperature/max6675.c:61:

thanks,
Daniel.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Paul Bolle]

On ma, 2015-08-03 at 16:56 -0400, Matt Porter wrote:
> --- /dev/null
> +++ b/drivers/iio/temperature/max6675.c
> +static const struct spi_device_id max6675_spi_ids[] = {
> +	{"max6675", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, max6675_spi_ids);

> +MODULE_ALIAS("spi:max6675");

For the "spi" alias this is "belt and suspenders":
    modinfo ./max6675.ko | grep alias
    alias:          spi:max6675
    alias:          acpi*:MXIM6675:*
    alias:          of:N*T*Cmaxim,max6675*
    alias:          spi:max6675

I'd drop the MODULE_ALIAS().

(Mark Brown made it quite clear I shouldn't nag people about the origin
of the various strings used in these module aliases. So I won't. But if
you'd volunteer to explain me where "max6675" might come from for the
spi alias that would, at least, satisfy my curiosity.)

Thanks,


Paul Bolle

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Paul Bolle]

On ma, 2015-08-03 at 16:56 -0400, Matt Porter wrote:
> --- /dev/null
> +++ b/drivers/iio/temperature/max6675.c
> +static const struct spi_device_id max6675_spi_ids[] = {
> +	{"max6675", 0},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, max6675_spi_ids);

> +MODULE_ALIAS("spi:max6675");

For the "spi" alias this is "belt and suspenders":
    modinfo ./max6675.ko | grep alias
    alias:          spi:max6675
    alias:          acpi*:MXIM6675:*
    alias:          of:N*T*Cmaxim,max6675*
    alias:          spi:max6675

I'd drop the MODULE_ALIAS().

(Mark Brown made it quite clear I shouldn't nag people about the origin
of the various strings used in these module aliases. So I won't. But if
you'd volunteer to explain me where "max6675" might come from for the
spi alias that would, at least, satisfy my curiosity.)

Thanks,


Paul Bolle
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

On Tue, Aug 04, 2015 at 10:50:32AM +0300, Daniel Baluta wrote:
> On Mon, Aug 3, 2015 at 11:56 PM, Matt Porter <mporter@konsulko.com> wrote:
> > Add a driver for the MAX6675 thermocouple converter. This
> > device interfaces with K-type thermocouples and provides
> > cold-junction compensated temperature readings via a
> > SPI interface.
> 
> ./scripts/checkpatch.pl --strict says:
> 
> CHECK: braces {} should be used on all arms of this statement
> #118: FILE: drivers/iio/temperature/max6675.c:61:

Will address in v2, thanks.

-Matt

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

On Tue, Aug 04, 2015 at 11:30:36AM +0200, Paul Bolle wrote:
> On ma, 2015-08-03 at 16:56 -0400, Matt Porter wrote:
> > --- /dev/null
> > +++ b/drivers/iio/temperature/max6675.c
> > +static const struct spi_device_id max6675_spi_ids[] = {
> > +	{"max6675", 0},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(spi, max6675_spi_ids);
> 
> > +MODULE_ALIAS("spi:max6675");
> 
> For the "spi" alias this is "belt and suspenders":
>     modinfo ./max6675.ko | grep alias
>     alias:          spi:max6675
>     alias:          acpi*:MXIM6675:*
>     alias:          of:N*T*Cmaxim,max6675*
>     alias:          spi:max6675
> 
> I'd drop the MODULE_ALIAS().

Ok, given that it generates a redundant alias I'll drop it.

> (Mark Brown made it quite clear I shouldn't nag people about the origin
> of the various strings used in these module aliases. So I won't. But if
> you'd volunteer to explain me where "max6675" might come from for the
> spi alias that would, at least, satisfy my curiosity.)

Sure, one might hotplug in an entire SPI master and this SPI slave device
via any discoverable bus and need the alias to match the module. It's
also typically used in a board file on embedded x86 stuff where overriding
the DSDT is not desirable.

-Matt

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

On Tue, Aug 04, 2015 at 11:30:36AM +0200, Paul Bolle wrote:
> On ma, 2015-08-03 at 16:56 -0400, Matt Porter wrote:
> > --- /dev/null
> > +++ b/drivers/iio/temperature/max6675.c
> > +static const struct spi_device_id max6675_spi_ids[] = {
> > +	{"max6675", 0},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(spi, max6675_spi_ids);
> 
> > +MODULE_ALIAS("spi:max6675");
> 
> For the "spi" alias this is "belt and suspenders":
>     modinfo ./max6675.ko | grep alias
>     alias:          spi:max6675
>     alias:          acpi*:MXIM6675:*
>     alias:          of:N*T*Cmaxim,max6675*
>     alias:          spi:max6675
> 
> I'd drop the MODULE_ALIAS().

Ok, given that it generates a redundant alias I'll drop it.

> (Mark Brown made it quite clear I shouldn't nag people about the origin
> of the various strings used in these module aliases. So I won't. But if
> you'd volunteer to explain me where "max6675" might come from for the
> spi alias that would, at least, satisfy my curiosity.)

Sure, one might hotplug in an entire SPI master and this SPI slave device
via any discoverable bus and need the alias to match the module. It's
also typically used in a board file on embedded x86 stuff where overriding
the DSDT is not desirable.

-Matt
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Jonathan Cameron]

On 3 August 2015 21:56:47 BST, Matt Porter <mporter@konsulko.com> wrote:
>This series adds a driver for the MAX6675 SPI thermocouple converter.
>The device supports temperature measurements via type-K thermocouples
>and implements cold-junction compensation within the part. The
>datasheet
>can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
I want to see an argument for why IIO makes more sense in the cover letter + cc
 at least the hwmon MAINTAINERS if not the list...
>
>Matt Porter (3):
>  iio: temperature: add max6675 dt binding
>  iio: temperature: add max6675 thermocouple converter driver
>  MAINTAINERS: add max6675 driver
>
> .../bindings/iio/temperature/max6675.txt           |  19 +++
> MAINTAINERS                                        |   7 +
> drivers/iio/temperature/Kconfig                    |  11 ++
> drivers/iio/temperature/Makefile                   |   1 +
>drivers/iio/temperature/max6675.c                  | 155
>+++++++++++++++++++++
> 5 files changed, 193 insertions(+)
>create mode 100644
>Documentation/devicetree/bindings/iio/temperature/max6675.txt
> create mode 100644 drivers/iio/temperature/max6675.c

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Jonathan Cameron]

On 3 August 2015 21:56:47 BST, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>This series adds a driver for the MAX6675 SPI thermocouple converter.
>The device supports temperature measurements via type-K thermocouples
>and implements cold-junction compensation within the part. The
>datasheet
>can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
I want to see an argument for why IIO makes more sense in the cover letter + cc
 at least the hwmon MAINTAINERS if not the list...
>
>Matt Porter (3):
>  iio: temperature: add max6675 dt binding
>  iio: temperature: add max6675 thermocouple converter driver
>  MAINTAINERS: add max6675 driver
>
> .../bindings/iio/temperature/max6675.txt           |  19 +++
> MAINTAINERS                                        |   7 +
> drivers/iio/temperature/Kconfig                    |  11 ++
> drivers/iio/temperature/Makefile                   |   1 +
>drivers/iio/temperature/max6675.c                  | 155
>+++++++++++++++++++++
> 5 files changed, 193 insertions(+)
>create mode 100644
>Documentation/devicetree/bindings/iio/temperature/max6675.txt
> create mode 100644 drivers/iio/temperature/max6675.c

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Matt Porter]

On Tue, Aug 04, 2015 at 05:52:17PM +0100, Jonathan Cameron wrote:
> 
> 
> On 3 August 2015 21:56:47 BST, Matt Porter <mporter@konsulko.com> wrote:
> >This series adds a driver for the MAX6675 SPI thermocouple converter.
> >The device supports temperature measurements via type-K thermocouples
> >and implements cold-junction compensation within the part. The
> >datasheet
> >can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
> For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
> I want to see an argument for why IIO makes more sense in the cover letter + cc
>  at least the hwmon MAINTAINERS if not the list...

Will do. Just to summarize here, the typical use case for this type of
thermocouple converter involves sample rates that are relatively high
compared to what the hwmon interface can support. The upcoming hrtimer
trigger will match up well with this to support fine-grained
periodic samples. I'll add this in the v2 cover latter.

-Matt

> >Matt Porter (3):
> >  iio: temperature: add max6675 dt binding
> >  iio: temperature: add max6675 thermocouple converter driver
> >  MAINTAINERS: add max6675 driver
> >
> > .../bindings/iio/temperature/max6675.txt           |  19 +++
> > MAINTAINERS                                        |   7 +
> > drivers/iio/temperature/Kconfig                    |  11 ++
> > drivers/iio/temperature/Makefile                   |   1 +
> >drivers/iio/temperature/max6675.c                  | 155
> >+++++++++++++++++++++
> > 5 files changed, 193 insertions(+)
> >create mode 100644
> >Documentation/devicetree/bindings/iio/temperature/max6675.txt
> > create mode 100644 drivers/iio/temperature/max6675.c
> 
> -- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Matt Porter]

On Tue, Aug 04, 2015 at 05:52:17PM +0100, Jonathan Cameron wrote:
> 
> 
> On 3 August 2015 21:56:47 BST, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> >This series adds a driver for the MAX6675 SPI thermocouple converter.
> >The device supports temperature measurements via type-K thermocouples
> >and implements cold-junction compensation within the part. The
> >datasheet
> >can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
> For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
> I want to see an argument for why IIO makes more sense in the cover letter + cc
>  at least the hwmon MAINTAINERS if not the list...

Will do. Just to summarize here, the typical use case for this type of
thermocouple converter involves sample rates that are relatively high
compared to what the hwmon interface can support. The upcoming hrtimer
trigger will match up well with this to support fine-grained
periodic samples. I'll add this in the v2 cover latter.

-Matt

> >Matt Porter (3):
> >  iio: temperature: add max6675 dt binding
> >  iio: temperature: add max6675 thermocouple converter driver
> >  MAINTAINERS: add max6675 driver
> >
> > .../bindings/iio/temperature/max6675.txt           |  19 +++
> > MAINTAINERS                                        |   7 +
> > drivers/iio/temperature/Kconfig                    |  11 ++
> > drivers/iio/temperature/Makefile                   |   1 +
> >drivers/iio/temperature/max6675.c                  | 155
> >+++++++++++++++++++++
> > 5 files changed, 193 insertions(+)
> >create mode 100644
> >Documentation/devicetree/bindings/iio/temperature/max6675.txt
> > create mode 100644 drivers/iio/temperature/max6675.c
> 
> -- 
> Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Daniel Baluta]

On Tue, Aug 4, 2015 at 8:34 PM, Matt Porter <mporter@konsulko.com> wrote:
> On Tue, Aug 04, 2015 at 05:52:17PM +0100, Jonathan Cameron wrote:
>>
>>
>> On 3 August 2015 21:56:47 BST, Matt Porter <mporter@konsulko.com> wrote:
>> >This series adds a driver for the MAX6675 SPI thermocouple converter.
>> >The device supports temperature measurements via type-K thermocouples
>> >and implements cold-junction compensation within the part. The
>> >datasheet
>> >can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>> For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
>> I want to see an argument for why IIO makes more sense in the cover letter + cc
>>  at least the hwmon MAINTAINERS if not the list...
>
> Will do. Just to summarize here, the typical use case for this type of
> thermocouple converter involves sample rates that are relatively high
> compared to what the hwmon interface can support. The upcoming hrtimer
> trigger will match up well with this to support fine-grained
> periodic samples. I'll add this in the v2 cover latter.

Are we talking about this hrtimer trigger patches:

http://marc.info/?l=linux-iio&m=143109196107382&w=2

Right? I've been busy with some other things, but I hope to have a
final version by Sunday :).

thanks,
Daniel.

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Daniel Baluta]

On Tue, Aug 4, 2015 at 8:34 PM, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> On Tue, Aug 04, 2015 at 05:52:17PM +0100, Jonathan Cameron wrote:
>>
>>
>> On 3 August 2015 21:56:47 BST, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>> >This series adds a driver for the MAX6675 SPI thermocouple converter.
>> >The device supports temperature measurements via type-K thermocouples
>> >and implements cold-junction compensation within the part. The
>> >datasheet
>> >can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>> For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
>> I want to see an argument for why IIO makes more sense in the cover letter + cc
>>  at least the hwmon MAINTAINERS if not the list...
>
> Will do. Just to summarize here, the typical use case for this type of
> thermocouple converter involves sample rates that are relatively high
> compared to what the hwmon interface can support. The upcoming hrtimer
> trigger will match up well with this to support fine-grained
> periodic samples. I'll add this in the v2 cover latter.

Are we talking about this hrtimer trigger patches:

http://marc.info/?l=linux-iio&m=143109196107382&w=2

Right? I've been busy with some other things, but I hope to have a
final version by Sunday :).

thanks,
Daniel.

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Matt Porter]

On Wed, Aug 05, 2015 at 11:33:25AM +0300, Daniel Baluta wrote:
> On Tue, Aug 4, 2015 at 8:34 PM, Matt Porter <mporter@konsulko.com> wrote:
> > On Tue, Aug 04, 2015 at 05:52:17PM +0100, Jonathan Cameron wrote:
> >>
> >>
> >> On 3 August 2015 21:56:47 BST, Matt Porter <mporter@konsulko.com> wrote:
> >> >This series adds a driver for the MAX6675 SPI thermocouple converter.
> >> >The device supports temperature measurements via type-K thermocouples
> >> >and implements cold-junction compensation within the part. The
> >> >datasheet
> >> >can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
> >> For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
> >> I want to see an argument for why IIO makes more sense in the cover letter + cc
> >>  at least the hwmon MAINTAINERS if not the list...
> >
> > Will do. Just to summarize here, the typical use case for this type of
> > thermocouple converter involves sample rates that are relatively high
> > compared to what the hwmon interface can support. The upcoming hrtimer
> > trigger will match up well with this to support fine-grained
> > periodic samples. I'll add this in the v2 cover latter.
> 
> Are we talking about this hrtimer trigger patches:
> 
> http://marc.info/?l=linux-iio&m=143109196107382&w=2
> 
> Right? I've been busy with some other things, but I hope to have a
> final version by Sunday :).

That's it! Thanks. :)

-Matt

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

On Mon, Aug 03, 2015 at 11:26:12PM +0200, Peter Meerwald wrote:
> On Mon, 3 Aug 2015, Matt Porter wrote:

...

> > +static int max6675_read(struct max6675_state *st, int *val)
> > +{
> > +	int ret;
> > +
> > +	ret = spi_read(st->spi, val, 2);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* Temperature is bits 14..3 */
> > +	*val = (*val >> 3) & 0xfff;
> 
> what about endianness conversion?
> use be16_to_cpu()

Apologies, I spoke before engaging the brain on my first reply to this
As specified by the SPI subsystem docs, SPI buffers are always stored
in native endian order. There is no need for endianness conversion here.

-Matt

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Jonathan Cameron]

On 04/08/15 18:34, Matt Porter wrote:
> On Tue, Aug 04, 2015 at 05:52:17PM +0100, Jonathan Cameron wrote:
>>
>>
>> On 3 August 2015 21:56:47 BST, Matt Porter <mporter@konsulko.com> wrote:
>>> This series adds a driver for the MAX6675 SPI thermocouple converter.
>>> The device supports temperature measurements via type-K thermocouples
>>> and implements cold-junction compensation within the part. The
>>> datasheet
>>> can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>> For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
>> I want to see an argument for why IIO makes more sense in the cover letter + cc
>>  at least the hwmon MAINTAINERS if not the list...
> 
> Will do. Just to summarize here, the typical use case for this type of
> thermocouple converter involves sample rates that are relatively high
> compared to what the hwmon interface can support. The upcoming hrtimer
> trigger will match up well with this to support fine-grained
> periodic samples. I'll add this in the v2 cover latter.
Fair enough - will want a least an OK from Jean / Guenter that they accept
this reasoning before I take the driver though.  Best to avoid stepping
one peoples toes!
> 
> -Matt
> 
>>> Matt Porter (3):
>>>  iio: temperature: add max6675 dt binding
>>>  iio: temperature: add max6675 thermocouple converter driver
>>>  MAINTAINERS: add max6675 driver
>>>
>>> .../bindings/iio/temperature/max6675.txt           |  19 +++
>>> MAINTAINERS                                        |   7 +
>>> drivers/iio/temperature/Kconfig                    |  11 ++
>>> drivers/iio/temperature/Makefile                   |   1 +
>>> drivers/iio/temperature/max6675.c                  | 155
>>> +++++++++++++++++++++
>>> 5 files changed, 193 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/iio/temperature/max6675.txt
>>> create mode 100644 drivers/iio/temperature/max6675.c
>>
>> -- 
>> Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH 0/3] MAX6675 IIO temperature driver

[Jonathan Cameron]

On 04/08/15 18:34, Matt Porter wrote:
> On Tue, Aug 04, 2015 at 05:52:17PM +0100, Jonathan Cameron wrote:
>>
>>
>> On 3 August 2015 21:56:47 BST, Matt Porter <mporter-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>>> This series adds a driver for the MAX6675 SPI thermocouple converter.
>>> The device supports temperature measurements via type-K thermocouples
>>> and implements cold-junction compensation within the part. The
>>> datasheet
>>> can be found at http://datasheets.maximintegrated.com/en/ds/MAX6675.pdf
>> For a device like this where perhaps it's position wrt iio/hwmon boundary is unclear
>> I want to see an argument for why IIO makes more sense in the cover letter + cc
>>  at least the hwmon MAINTAINERS if not the list...
> 
> Will do. Just to summarize here, the typical use case for this type of
> thermocouple converter involves sample rates that are relatively high
> compared to what the hwmon interface can support. The upcoming hrtimer
> trigger will match up well with this to support fine-grained
> periodic samples. I'll add this in the v2 cover latter.
Fair enough - will want a least an OK from Jean / Guenter that they accept
this reasoning before I take the driver though.  Best to avoid stepping
one peoples toes!
> 
> -Matt
> 
>>> Matt Porter (3):
>>>  iio: temperature: add max6675 dt binding
>>>  iio: temperature: add max6675 thermocouple converter driver
>>>  MAINTAINERS: add max6675 driver
>>>
>>> .../bindings/iio/temperature/max6675.txt           |  19 +++
>>> MAINTAINERS                                        |   7 +
>>> drivers/iio/temperature/Kconfig                    |  11 ++
>>> drivers/iio/temperature/Makefile                   |   1 +
>>> drivers/iio/temperature/max6675.c                  | 155
>>> +++++++++++++++++++++
>>> 5 files changed, 193 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/iio/temperature/max6675.txt
>>> create mode 100644 drivers/iio/temperature/max6675.c
>>
>> -- 
>> Sent from my Android device with K-9 Mail. Please excuse my brevity.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Jonathan Cameron]

On 06/08/15 18:38, Matt Porter wrote:
> On Mon, Aug 03, 2015 at 11:26:12PM +0200, Peter Meerwald wrote:
>> On Mon, 3 Aug 2015, Matt Porter wrote:
> 
> ...
> 
>>> +static int max6675_read(struct max6675_state *st, int *val)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = spi_read(st->spi, val, 2);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	/* Temperature is bits 14..3 */
>>> +	*val = (*val >> 3) & 0xfff;
>>
>> what about endianness conversion?
>> use be16_to_cpu()
> 
> Apologies, I spoke before engaging the brain on my first reply to this
> As specified by the SPI subsystem docs, SPI buffers are always stored
> in native endian order. There is no need for endianness conversion here.
First of all, which doc say this?
Secondly how does SPI know the endianness of the sensor which is what
actually matters here?  I2C can in theory make these guarantees as there
is an expected byte order on the wire (even if quite a few drivers don't
conform to the spec anyway). No such guarantee can exist for SPI.

Jonathan


> 
> -Matt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Matt Porter]

On Sat, Aug 08, 2015 at 12:39:40PM +0100, Jonathan Cameron wrote:
> On 06/08/15 18:38, Matt Porter wrote:
> > On Mon, Aug 03, 2015 at 11:26:12PM +0200, Peter Meerwald wrote:
> >> On Mon, 3 Aug 2015, Matt Porter wrote:
> > 
> > ...
> > 
> >>> +static int max6675_read(struct max6675_state *st, int *val)
> >>> +{
> >>> +	int ret;
> >>> +
> >>> +	ret = spi_read(st->spi, val, 2);
> >>> +	if (ret < 0)
> >>> +		return ret;
> >>> +
> >>> +	/* Temperature is bits 14..3 */
> >>> +	*val = (*val >> 3) & 0xfff;
> >>
> >> what about endianness conversion?
> >> use be16_to_cpu()
> > 
> > Apologies, I spoke before engaging the brain on my first reply to this
> > As specified by the SPI subsystem docs, SPI buffers are always stored
> > in native endian order. There is no need for endianness conversion here.
> First of all, which doc say this?
> Secondly how does SPI know the endianness of the sensor which is what
> actually matters here?  I2C can in theory make these guarantees as there
> is an expected byte order on the wire (even if quite a few drivers don't
> conform to the spec anyway). No such guarantee can exist for SPI.

include/linux/spi/spi.h:

 * In-memory data values are always in native CPU byte order, translated
 * from the wire byte order (big-endian except with SPI_LSB_FIRST).  So
 * for example when bits_per_word is sixteen, buffers are 2N bytes long
 * (@len = 2N) and hold N sixteen bit words in CPU byte order.

So, as you mention, there's no standardized byte order but it's
controlled with the per transfer flag and big endian by default.

-Matt

Re: [PATCH 2/3] iio: temperature: add max6675 thermocouple converter driver

[Jonathan Cameron]

On 20/08/15 01:23, Matt Porter wrote:
> On Sat, Aug 08, 2015 at 12:39:40PM +0100, Jonathan Cameron wrote:
>> On 06/08/15 18:38, Matt Porter wrote:
>>> On Mon, Aug 03, 2015 at 11:26:12PM +0200, Peter Meerwald wrote:
>>>> On Mon, 3 Aug 2015, Matt Porter wrote:
>>>
>>> ...
>>>
>>>>> +static int max6675_read(struct max6675_state *st, int *val)
>>>>> +{
>>>>> +	int ret;
>>>>> +
>>>>> +	ret = spi_read(st->spi, val, 2);
>>>>> +	if (ret < 0)
>>>>> +		return ret;
>>>>> +
>>>>> +	/* Temperature is bits 14..3 */
>>>>> +	*val = (*val >> 3) & 0xfff;
>>>>
>>>> what about endianness conversion?
>>>> use be16_to_cpu()
>>>
>>> Apologies, I spoke before engaging the brain on my first reply to this
>>> As specified by the SPI subsystem docs, SPI buffers are always stored
>>> in native endian order. There is no need for endianness conversion here.
>> First of all, which doc say this?
>> Secondly how does SPI know the endianness of the sensor which is what
>> actually matters here?  I2C can in theory make these guarantees as there
>> is an expected byte order on the wire (even if quite a few drivers don't
>> conform to the spec anyway). No such guarantee can exist for SPI.
> 
> include/linux/spi/spi.h:
> 
>  * In-memory data values are always in native CPU byte order, translated
>  * from the wire byte order (big-endian except with SPI_LSB_FIRST).  So
>  * for example when bits_per_word is sixteen, buffers are 2N bytes long
>  * (@len = 2N) and hold N sixteen bit words in CPU byte order.
> 
> So, as you mention, there's no standardized byte order but it's
> controlled with the per transfer flag and big endian by default.
> 
Thanks, I'd never picked up on that before!

Jonathan
>