* [PATCH v3 2/2] iio: temperature: Add MCP9600 thermocouple EMF converter driver
2023-03-04 0:41 [PATCH v3 1/2] dt-bindings: iio: Add MCP9600 thermocouple EMF converter bindings Andrew Hepp
@ 2023-03-04 0:41 ` Andrew Hepp
2023-03-04 11:20 ` [PATCH v3 1/2] dt-bindings: iio: Add MCP9600 thermocouple EMF converter bindings Krzysztof Kozlowski
2023-03-04 17:09 ` Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Andrew Hepp @ 2023-03-04 0:41 UTC (permalink / raw)
To: devicetree, linux-iio
Cc: Andrew Hepp, Rob Herring, Krzysztof Kozlowski, Jonathan Cameron
Add support for the MCP9600 thermocouple EMF converter.
Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/MCP960X-Data-Sheet-20005426.pdf
Signed-off-by: Andrew Hepp <andrew.hepp@ahepp.dev>
---
Changes for v3:
- none
Changes for v2:
- remove unused sysfs include
- remove unused scan fields from channel
- warn rather than fail when probing unknown device
- register device through devm
- clean up style and prints
---
drivers/iio/temperature/Kconfig | 10 ++
drivers/iio/temperature/Makefile | 1 +
drivers/iio/temperature/mcp9600.c | 146 ++++++++++++++++++++++++++++++
3 files changed, 157 insertions(+)
create mode 100644 drivers/iio/temperature/mcp9600.c
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index ed384f33e0c7..ea2ce364b2e9 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -158,4 +158,14 @@ config MAX31865
This driver can also be build as a module. If so, the module
will be called max31865.
+config MCP9600
+ tristate "MCP9600 thermocouple EMF converter"
+ depends on I2C
+ help
+ If you say yes here you get support for MCP9600
+ thermocouple EMF converter connected via I2C.
+
+ This driver can also be built as a module. If so, the module
+ will be called mcp9600.
+
endmenu
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index dfec8c6d3019..9330d4a39598 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
obj-$(CONFIG_MAX30208) += max30208.o
obj-$(CONFIG_MAX31856) += max31856.o
obj-$(CONFIG_MAX31865) += max31865.o
+obj-$(CONFIG_MCP9600) += mcp9600.o
obj-$(CONFIG_MLX90614) += mlx90614.o
obj-$(CONFIG_MLX90632) += mlx90632.o
obj-$(CONFIG_TMP006) += tmp006.o
diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
new file mode 100644
index 000000000000..d938e09632cf
--- /dev/null
+++ b/drivers/iio/temperature/mcp9600.c
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * mcp9600.c - Support for Microchip MCP9600 thermocouple EMF converter
+ *
+ * Copyright (c) 2022 Andrew Hepp
+ * Author: <andrew.hepp@ahepp.dev>
+ */
+
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+
+#include <linux/iio/iio.h>
+
+/* MCP9600 registers */
+#define MCP9600_HOT_JUNCTION 0x0
+#define MCP9600_COLD_JUNCTION 0x2
+#define MCP9600_DEVICE_ID 0x20
+
+/* MCP9600 device id value */
+#define MCP9600_DEVICE_ID_MCP9600 0x40
+
+static const struct iio_chan_spec mcp9600_channels[] = {
+ {
+ .type = IIO_TEMP,
+ .address = MCP9600_HOT_JUNCTION,
+ .info_mask_separate =
+ BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+ },
+ {
+ .type = IIO_TEMP,
+ .address = MCP9600_COLD_JUNCTION,
+ .channel2 = IIO_MOD_TEMP_AMBIENT,
+ .modified = 1,
+ .info_mask_separate =
+ BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE),
+ },
+ IIO_CHAN_SOFT_TIMESTAMP(2),
+};
+
+struct mcp9600_data {
+ struct i2c_client *client;
+ struct mutex read_lock; /* lock to prevent concurrent reads */
+};
+
+static int mcp9600_read(struct mcp9600_data *data,
+ struct iio_chan_spec const *chan, int *val)
+{
+ __be16 buf;
+ int ret;
+
+ mutex_lock(&data->read_lock);
+ ret = i2c_smbus_read_i2c_block_data(data->client, chan->address, 2,
+ (u8 *)&buf);
+ mutex_unlock(&data->read_lock);
+
+ if (ret < 0)
+ return ret;
+ *val = be16_to_cpu(buf);
+
+ return 0;
+}
+
+static int mcp9600_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val,
+ int *val2, long mask)
+{
+ struct mcp9600_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = mcp9600_read(data, chan, val);
+ if (ret)
+ return ret;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = 62;
+ *val2 = 500000;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info mcp9600_info = {
+ .read_raw = mcp9600_read_raw,
+};
+
+static int mcp9600_probe(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev;
+ struct mcp9600_data *data;
+ int ret;
+
+ ret = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
+ if (ret < 0)
+ return ret;
+ if (ret != MCP9600_DEVICE_ID_MCP9600)
+ dev_warn(&client->dev, "Expected ID %x, got %x\n",
+ MCP9600_DEVICE_ID_MCP9600, ret);
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ data->client = client;
+ mutex_init(&data->read_lock);
+
+ indio_dev->info = &mcp9600_info;
+ indio_dev->name = "mcp9600";
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = mcp9600_channels;
+ indio_dev->num_channels = ARRAY_SIZE(mcp9600_channels);
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct i2c_device_id mcp9600_id[] = {
+ { "mcp9600" },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, mcp9600_id);
+
+static const struct of_device_id mcp9600_of_match[] = {
+ { .compatible = "microchip,mcp9600" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, mcp9600_of_match);
+
+static struct i2c_driver mcp9600_driver = {
+ .driver = {
+ .name = "mcp9600",
+ .of_match_table = mcp9600_of_match,
+ },
+ .probe_new = mcp9600_probe,
+ .id_table = mcp9600_id
+};
+module_i2c_driver(mcp9600_driver);
+
+MODULE_AUTHOR("Andrew Hepp <andrew.hepp@ahepp.dev>");
+MODULE_DESCRIPTION("Microchip MCP9600 thermocouple EMF converter driver");
+MODULE_LICENSE("GPL");
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3 1/2] dt-bindings: iio: Add MCP9600 thermocouple EMF converter bindings
2023-03-04 0:41 [PATCH v3 1/2] dt-bindings: iio: Add MCP9600 thermocouple EMF converter bindings Andrew Hepp
2023-03-04 0:41 ` [PATCH v3 2/2] iio: temperature: Add MCP9600 thermocouple EMF converter driver Andrew Hepp
@ 2023-03-04 11:20 ` Krzysztof Kozlowski
2023-03-04 12:36 ` Andrew Hepp
2023-03-04 17:09 ` Jonathan Cameron
2 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-04 11:20 UTC (permalink / raw)
To: Andrew Hepp, devicetree, linux-iio
Cc: Rob Herring, Krzysztof Kozlowski, Jonathan Cameron
On 04/03/2023 01:41, Andrew Hepp wrote:
> Add support for the MCP9600 thermocouple EMF converter.
>
> Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/MCP960X-Data-Sheet-20005426.pdf
> Signed-off-by: Andrew Hepp <andrew.hepp@ahepp.dev>
Thank you for your patch. There is something to discuss/improve.
> +description: |
Drop '|'
> + https://ww1.microchip.com/downloads/en/DeviceDoc/MCP960X-Data-Sheet-20005426.pdf
> +
> +properties:
> + compatible:
> + const: microchip,mcp9600
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 6
> +
> + interrupt-names:
> + minItems: 1
> + maxItems: 6
> + items:
> + enum:
The interrupts should be usually strictly ordered and you allow any
combinations. Why?
Why are they optional?
> + - open
> + - short
> + - alert1
> + - alert2
> + - alert3
> + - alert4
> +
> + thermocouple-type:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Type of thermocouple (THERMOCOUPLE_TYPE_K if omitted).
> + Use defines in dt-bindings/iio/temperature/thermocouple.h.
> + Supported types are B, E, J, K, N, R, S, T.
> +
> + vdd-supply:
> + description: Regulator that provides power to the sensor.
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
No need for this header.
> + #include <dt-bindings/iio/temperature/thermocouple.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mcp9600@60 {
Node names should be generic, so temp-sensor, thermocouple or something
else matching the type.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "microchip,mcp9600";
> + reg = <0x60>;
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3 1/2] dt-bindings: iio: Add MCP9600 thermocouple EMF converter bindings
2023-03-04 0:41 [PATCH v3 1/2] dt-bindings: iio: Add MCP9600 thermocouple EMF converter bindings Andrew Hepp
2023-03-04 0:41 ` [PATCH v3 2/2] iio: temperature: Add MCP9600 thermocouple EMF converter driver Andrew Hepp
2023-03-04 11:20 ` [PATCH v3 1/2] dt-bindings: iio: Add MCP9600 thermocouple EMF converter bindings Krzysztof Kozlowski
@ 2023-03-04 17:09 ` Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2023-03-04 17:09 UTC (permalink / raw)
To: Andrew Hepp; +Cc: devicetree, linux-iio, Rob Herring, Krzysztof Kozlowski
On Fri, 3 Mar 2023 16:41:08 -0800
Andrew Hepp <andrew.hepp@ahepp.dev> wrote:
> Add support for the MCP9600 thermocouple EMF converter.
>
> Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/MCP960X-Data-Sheet-20005426.pdf
> Signed-off-by: Andrew Hepp <andrew.hepp@ahepp.dev>
> ---
> Changes for v3:
> - Added dt-bindings
> ---
> .../iio/temperature/microchip,mcp9600.yaml | 72 +++++++++++++++++++
> 1 file changed, 72 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> new file mode 100644
> index 000000000000..584d0ae42502
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9600.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MCP9600 thermocouple EMF converter
> +
> +maintainers:
> + - Andrew Hepp <andrew.hepp@ahepp.dev>
> +
> +description: |
> + https://ww1.microchip.com/downloads/en/DeviceDoc/MCP960X-Data-Sheet-20005426.pdf
> +
> +properties:
> + compatible:
> + const: microchip,mcp9600
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + maxItems: 6
> +
> + interrupt-names:
> + minItems: 1
> + maxItems: 6
> + items:
> + enum:
> + - open
Perhaps make it more explicit? open-circuit
> + - short
Same here? short-circuit
> + - alert1
> + - alert2
> + - alert3
> + - alert4
> +
> + thermocouple-type:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Type of thermocouple (THERMOCOUPLE_TYPE_K if omitted).
> + Use defines in dt-bindings/iio/temperature/thermocouple.h.
> + Supported types are B, E, J, K, N, R, S, T.
> +
> + vdd-supply:
> + description: Regulator that provides power to the sensor.
I'd count that one as so common it doesn't need a description.
vdd-supply: true
would be sufficient.
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/iio/temperature/thermocouple.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mcp9600@60 {
> + compatible = "microchip,mcp9600";
> + reg = <0x60>;
> + interrupt-parent = <&gpio>;
> + interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "open";
> + thermocouple-type = <THERMOCOUPLE_TYPE_K>;
> + vdd-supply = <&vdd>;
> + };
> + };
^ permalink raw reply [flat|nested] 6+ messages in thread