[PATCH v8 0/5] iio: mcp9600: Features and improvements

[PATCH v8 0/5] iio: mcp9600: Features and improvements

[Ben Collins]

ChangeLog:
v7 -> v8:
  - Style changes in dt-bindings example
  - Simplify some return value checks
  - Move assignment to where it's checked
  - Speeling

v6 -> v7:
  - Separate out the mcp9600 IIR series into its own series as there is
    a lot of conversation around implementation (removed related
    comments from this changelog).

v5 -> v6:
  - Fix accidental typo added in dt-bindings: IRQ_TYPE_EDGE_RISIN
  - Correct some constraints in dt-bindings
  - Reverse if/then for mcp9601 vs mcp9600 constraints in dt-bindings
  - Updates to changelog for patch 2/6 (dt-bindings mcp9600)
  - Cleanup tabs that were converted to spaces
  - Split thermocouple-type default to separate patch

v4 -> v5:
  - None

v3 -> v4:
  - Based on feedback from David Lechner <dlechner@baylibre.com>
    * Allow fallback compatible in dt-bindings for mcp9601.
  - Based on feedback from Jonathan Cameron <jic23@kernel.org>
    * Be explicit in patch description for fixed width changes.
    * Check chip_info for NULL to quiet warnings from kernel-test-robot
    * Remove "and similar" for long description of MCP9600.
  - Set default 3 for thermocouple in dt-binding
  - Rework open/short circuit in dt-bindings

v2 -> v3:
  - Improve changelogs in each patch
  - Based on feedback from Andy Shevchenko <andy.shevchenko@gmail.com>
    * Set register offsets to fixed width
    * Fix typos
    * Future-proof Kconfig changes
    * Convert to using chip_info paradigm
    * Verbiage: dt -> firmware description
    * Use proper specifiers and drop castings
    * Fix register offset to be fixed-width
    * u8 for cfg var
    * Fix % type for u32 to be %u
    * Make blank lines consistent between case statements

v1 -> v2:
  - Break into individual patches

v1:
  - Initial patch to enable IIR and thermocouple-type
  - Recognize mcp9601

Signed-off-by: Ben Collins <bcollins@watter.com>
---
- Link to v7: https://lore.kernel.org/r/20250819-upstream-changes-v7-0-88a33aa78f6a@watter.com

---
Ben Collins (5):
      dt-bindings: iio: mcp9600: Set default 3 for thermocouple-type
      dt-bindings: iio: mcp9600: Add microchip,mcp9601 and add constraints
      iio: mcp9600: White space and fixed width cleanup
      iio: mcp9600: Recognize chip id for mcp9601
      iio: mcp9600: Add support for thermocouple-type

 .../iio/temperature/microchip,mcp9600.yaml         |  57 +++++++-
 drivers/iio/temperature/Kconfig                    |   8 +-
 drivers/iio/temperature/mcp9600.c                  | 151 +++++++++++++++++----
 3 files changed, 187 insertions(+), 29 deletions(-)
---
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
change-id: 20250819-upstream-changes-c89af86743fa

Best regards,
-- 
Ben Collins <bcollins@watter.com>

[PATCH v8 1/5] dt-bindings: iio: mcp9600: Set default 3 for thermocouple-type

[Ben Collins]

As is already documented in this file, Type-K is the default, so make
that explicit in the dt-bindings.

Signed-off-by: Ben Collins <bcollins@watter.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
index d2cafa38a5442e229be8befb26ae3f34bae44cdb..57b387a1accc776683500949a22ef0290fc876e8 100644
--- a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
+++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
@@ -37,6 +37,7 @@ properties:
 
   thermocouple-type:
     $ref: /schemas/types.yaml#/definitions/uint32
+    default: 3
     description:
       Type of thermocouple (THERMOCOUPLE_TYPE_K if omitted).
       Use defines in dt-bindings/iio/temperature/thermocouple.h.

-- 
2.39.5

[PATCH v8 2/5] dt-bindings: iio: mcp9600: Add microchip,mcp9601 and add constraints

[Ben Collins]

Add microchip,mcp9601 compatible in addition to the original
microchip,mcp9600 to designate support between these two chips.

The current dt-binding has open-circuit and short-circuit as interrupt
names, but these are only supported in mcp9601.

The OC and SC detection requires that mcp9601 VSENSE be wired up, which
not only enables the OC SC interrupts, but also the OC and SC status
register bits.

Add a microchip,vsense boolean to show the chip is wired for this
support.

Add constraints so this feature only applies if the mcp9601 compatible
is selected.

Signed-off-by: Ben Collins <bcollins@watter.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 .../iio/temperature/microchip,mcp9600.yaml         | 56 ++++++++++++++++++++--
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
index 57b387a1accc776683500949a22ef0290fc876e8..fb3661c805934255d35f664e1018ed2ec91d05f0 100644
--- a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
+++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
@@ -4,7 +4,7 @@
 $id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9600.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: Microchip MCP9600 thermocouple EMF converter
+title: Microchip MCP9600 and similar thermocouple EMF converters
 
 maintainers:
   - Andrew Hepp <andrew.hepp@ahepp.dev>
@@ -14,7 +14,11 @@ description:
 
 properties:
   compatible:
-    const: microchip,mcp9600
+    oneOf:
+      - const: microchip,mcp9600
+      - items:
+          - const: microchip,mcp9601
+          - const: microchip,mcp9600
 
   reg:
     maxItems: 1
@@ -43,8 +47,37 @@ properties:
       Use defines in dt-bindings/iio/temperature/thermocouple.h.
       Supported types are B, E, J, K, N, R, S, T.
 
+  microchip,vsense:
+    type: boolean
+    description:
+      This flag indicates that the chip has been wired with VSENSE to
+      enable open and short circuit detect.
+
   vdd-supply: true
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          not:
+            contains:
+              const: microchip,mcp9601
+    then:
+      properties:
+        interrupts:
+          minItems: 1
+          maxItems: 4
+        interrupt-names:
+          minItems: 1
+          maxItems: 4
+          items:
+            enum:
+              - alert1
+              - alert2
+              - alert3
+              - alert4
+        microchip,vsense: false
+
 required:
   - compatible
   - reg
@@ -64,8 +97,25 @@ examples:
             reg = <0x60>;
             interrupt-parent = <&gpio>;
             interrupts = <25 IRQ_TYPE_EDGE_RISING>;
-            interrupt-names = "open-circuit";
+            interrupt-names = "alert1";
             thermocouple-type = <THERMOCOUPLE_TYPE_K>;
             vdd-supply = <&vdd>;
         };
     };
+  - |
+    #include <dt-bindings/iio/temperature/thermocouple.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        temperature-sensor@62 {
+            compatible = "microchip,mcp9601", "microchip,mcp9600";
+            reg = <0x62>;
+            interrupt-parent = <&gpio>;
+            interrupts = <22 IRQ_TYPE_EDGE_RISING>, <23 IRQ_TYPE_EDGE_RISING>;
+            interrupt-names = "open-circuit", "short-circuit";
+            vdd-supply = <&vdd>;
+            microchip,vsense;
+        };
+    };

-- 
2.39.5

[PATCH v8 3/5] iio: mcp9600: White space and fixed width cleanup

[Ben Collins]

Make tabs consistent for register definitions and also fix width
to byte size.

Signed-off-by: Ben Collins <bcollins@watter.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
Reviewed-by: Andy Shevchenko <abdy@kernel.org>
---
 drivers/iio/temperature/mcp9600.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index 6e9108d5cf75f3b7a3c45ada1537c446c6853392..40906bb200ec92812a70c2dfb055793d94c7c060 100644
--- a/drivers/iio/temperature/mcp9600.c
+++ b/drivers/iio/temperature/mcp9600.c
@@ -23,25 +23,25 @@
 #include <linux/iio/iio.h>
 
 /* MCP9600 registers */
-#define MCP9600_HOT_JUNCTION 0x0
-#define MCP9600_COLD_JUNCTION 0x2
-#define MCP9600_STATUS			0x4
+#define MCP9600_HOT_JUNCTION		0x00
+#define MCP9600_COLD_JUNCTION		0x02
+#define MCP9600_STATUS			0x04
 #define MCP9600_STATUS_ALERT(x)		BIT(x)
-#define MCP9600_ALERT_CFG1		0x8
+#define MCP9600_ALERT_CFG1		0x08
 #define MCP9600_ALERT_CFG(x)		(MCP9600_ALERT_CFG1 + (x - 1))
 #define MCP9600_ALERT_CFG_ENABLE	BIT(0)
 #define MCP9600_ALERT_CFG_ACTIVE_HIGH	BIT(2)
 #define MCP9600_ALERT_CFG_FALLING	BIT(3)
 #define MCP9600_ALERT_CFG_COLD_JUNCTION	BIT(4)
-#define MCP9600_ALERT_HYSTERESIS1	0xc
+#define MCP9600_ALERT_HYSTERESIS1	0x0c
 #define MCP9600_ALERT_HYSTERESIS(x)	(MCP9600_ALERT_HYSTERESIS1 + (x - 1))
 #define MCP9600_ALERT_LIMIT1		0x10
 #define MCP9600_ALERT_LIMIT(x)		(MCP9600_ALERT_LIMIT1 + (x - 1))
 #define MCP9600_ALERT_LIMIT_MASK	GENMASK(15, 2)
-#define MCP9600_DEVICE_ID 0x20
+#define MCP9600_DEVICE_ID		0x20
 
 /* MCP9600 device id value */
-#define MCP9600_DEVICE_ID_MCP9600 0x40
+#define MCP9600_DEVICE_ID_MCP9600	0x40
 
 #define MCP9600_ALERT_COUNT		4
 

-- 
2.39.5

[PATCH v8 4/5] iio: mcp9600: Recognize chip id for mcp9601

[Ben Collins]

The current driver works with mcp9601, but emits a warning because it
does not recognize the chip id.

MCP9601 is a superset of MCP9600. The drivers works without changes
on this chipset.

However, the 9601 chip supports open/closed-circuit detection if wired
properly, so we'll need to be able to differentiate between them.

Moved "struct mcp9600_data" up in the file since a later patch will
need it and chip_info before the declarations.

Signed-off-by: Ben Collins <bcollins@watter.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/temperature/Kconfig   |  8 +++--
 drivers/iio/temperature/mcp9600.c | 68 ++++++++++++++++++++++++++++++---------
 2 files changed, 57 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 1244d8e17d504ab23425f0c33f5f2d06dadba663..9328b2250aced6dffb2ea509280dcd66ca772241 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -173,11 +173,13 @@ config MAX31865
 	  will be called max31865.
 
 config MCP9600
-	tristate "MCP9600 thermocouple EMF converter"
+	tristate "MCP9600 and similar thermocouple EMF converters"
 	depends on I2C
 	help
-	  If you say yes here you get support for MCP9600
-	  thermocouple EMF converter connected via I2C.
+	  If you say yes here you get support for...
+	  - MCP9600
+	  - MCP9601
+	  ...thermocouple EMF converters connected via I2C.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called mcp9600.
diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index 40906bb200ec92812a70c2dfb055793d94c7c060..51ed780ea72fa552884cf170def1450ce017b197 100644
--- a/drivers/iio/temperature/mcp9600.c
+++ b/drivers/iio/temperature/mcp9600.c
@@ -42,6 +42,7 @@
 
 /* MCP9600 device id value */
 #define MCP9600_DEVICE_ID_MCP9600	0x40
+#define MCP9600_DEVICE_ID_MCP9601	0x41
 
 #define MCP9600_ALERT_COUNT		4
 
@@ -82,6 +83,15 @@ static const struct iio_event_spec mcp9600_events[] = {
 	},
 };
 
+struct mcp_chip_info {
+	u8 chip_id;
+	const char *chip_name;
+};
+
+struct mcp9600_data {
+	struct i2c_client *client;
+};
+
 #define MCP9600_CHANNELS(hj_num_ev, hj_ev_spec_off, cj_num_ev, cj_ev_spec_off) \
 	{								       \
 		{							       \
@@ -123,10 +133,6 @@ static const struct iio_chan_spec mcp9600_channels[][2] = {
 	MCP9600_CHANNELS(2, 0, 2, 0), /* Alerts: 1 2 3 4 */
 };
 
-struct mcp9600_data {
-	struct i2c_client *client;
-};
-
 static int mcp9600_read(struct mcp9600_data *data,
 			struct iio_chan_spec const *chan, int *val)
 {
@@ -416,18 +422,36 @@ static int mcp9600_probe_alerts(struct iio_dev *indio_dev)
 
 static int mcp9600_probe(struct i2c_client *client)
 {
+	struct device *dev = &client->dev;
+	const struct mcp_chip_info *chip_info;
 	struct iio_dev *indio_dev;
 	struct mcp9600_data *data;
-	int ret, ch_sel;
+	int ch_sel, dev_id;
+
+	chip_info = i2c_get_match_data(client);
+	if (!chip_info)
+		return dev_err_probe(dev, -ENODEV,
+				     "No chip-info found for device\n");
+
+	dev_id = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
+	if (dev_id < 0)
+		return dev_err_probe(dev, dev_id, "Failed to read device ID\n");
+
+	switch (dev_id) {
+	case MCP9600_DEVICE_ID_MCP9600:
+	case MCP9600_DEVICE_ID_MCP9601:
+		if (dev_id != chip_info->chip_id)
+			dev_warn(dev,
+				 "Expected id %02x, but device responded with %02x\n",
+				 chip_info->chip_id, dev_id);
+		break;
 
-	ret = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
-	if (ret < 0)
-		return dev_err_probe(&client->dev, ret, "Failed to read device ID\n");
-	if (ret != MCP9600_DEVICE_ID_MCP9600)
-		dev_warn(&client->dev, "Expected ID %x, got %x\n",
-				MCP9600_DEVICE_ID_MCP9600, ret);
+	default:
+		dev_warn(dev, "Unknown id %x, using %x\n", dev_id,
+			 chip_info->chip_id);
+	}
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -439,22 +463,34 @@ static int mcp9600_probe(struct i2c_client *client)
 		return ch_sel;
 
 	indio_dev->info = &mcp9600_info;
-	indio_dev->name = "mcp9600";
+	indio_dev->name = chip_info->chip_name;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = mcp9600_channels[ch_sel];
 	indio_dev->num_channels = ARRAY_SIZE(mcp9600_channels[ch_sel]);
 
-	return devm_iio_device_register(&client->dev, indio_dev);
+	return devm_iio_device_register(dev, indio_dev);
 }
 
+static const struct mcp_chip_info mcp9600_chip_info = {
+	.chip_id   = MCP9600_DEVICE_ID_MCP9600,
+	.chip_name = "mcp9600",
+};
+
+static const struct mcp_chip_info mcp9601_chip_info = {
+	.chip_id   = MCP9600_DEVICE_ID_MCP9601,
+	.chip_name = "mcp9601",
+};
+
 static const struct i2c_device_id mcp9600_id[] = {
-	{ "mcp9600" },
+	{ "mcp9600", .driver_data = (kernel_ulong_t)&mcp9600_chip_info },
+	{ "mcp9601", .driver_data = (kernel_ulong_t)&mcp9601_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, mcp9600_id);
 
 static const struct of_device_id mcp9600_of_match[] = {
-	{ .compatible = "microchip,mcp9600" },
+	{ .compatible = "microchip,mcp9600", .data = &mcp9600_chip_info },
+	{ .compatible = "microchip,mcp9601", .data = &mcp9601_chip_info },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mcp9600_of_match);

-- 
2.39.5

[PATCH v8 5/5] iio: mcp9600: Add support for thermocouple-type

[Ben Collins]

dt-bindings documentation for this driver claims to support
thermocouple-type, but the driver does not actually make use of
the property.

Implement usage of the property to configure the chip for the
selected thermocouple-type.

Signed-off-by: Ben Collins <bcollins@watter.com>
Reviewed-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/temperature/mcp9600.c | 71 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index 51ed780ea72fa552884cf170def1450ce017b197..aa42c2b1a369edbd36e0d6d6d1738ed0069fd990 100644
--- a/drivers/iio/temperature/mcp9600.c
+++ b/drivers/iio/temperature/mcp9600.c
@@ -22,11 +22,15 @@
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 
+#include <dt-bindings/iio/temperature/thermocouple.h>
+
 /* MCP9600 registers */
 #define MCP9600_HOT_JUNCTION		0x00
 #define MCP9600_COLD_JUNCTION		0x02
 #define MCP9600_STATUS			0x04
 #define MCP9600_STATUS_ALERT(x)		BIT(x)
+#define MCP9600_SENSOR_CFG		0x05
+#define MCP9600_SENSOR_TYPE_MASK	GENMASK(6, 4)
 #define MCP9600_ALERT_CFG1		0x08
 #define MCP9600_ALERT_CFG(x)		(MCP9600_ALERT_CFG1 + (x - 1))
 #define MCP9600_ALERT_CFG_ENABLE	BIT(0)
@@ -66,6 +70,30 @@ static const char * const mcp9600_alert_name[MCP9600_ALERT_COUNT] = {
 	[MCP9600_ALERT4] = "alert4",
 };
 
+/* Map between dt-bindings enum and the chip's type value */
+static const unsigned int mcp9600_type_map[] = {
+	[THERMOCOUPLE_TYPE_K] = 0,
+	[THERMOCOUPLE_TYPE_J] = 1,
+	[THERMOCOUPLE_TYPE_T] = 2,
+	[THERMOCOUPLE_TYPE_N] = 3,
+	[THERMOCOUPLE_TYPE_S] = 4,
+	[THERMOCOUPLE_TYPE_E] = 5,
+	[THERMOCOUPLE_TYPE_B] = 6,
+	[THERMOCOUPLE_TYPE_R] = 7,
+};
+
+/* Map thermocouple type to a char for iio info in sysfs */
+static const int mcp9600_tc_types[] = {
+	[THERMOCOUPLE_TYPE_K] = 'K',
+	[THERMOCOUPLE_TYPE_J] = 'J',
+	[THERMOCOUPLE_TYPE_T] = 'T',
+	[THERMOCOUPLE_TYPE_N] = 'N',
+	[THERMOCOUPLE_TYPE_S] = 'S',
+	[THERMOCOUPLE_TYPE_E] = 'E',
+	[THERMOCOUPLE_TYPE_B] = 'B',
+	[THERMOCOUPLE_TYPE_R] = 'R',
+};
+
 static const struct iio_event_spec mcp9600_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -90,14 +118,34 @@ struct mcp_chip_info {
 
 struct mcp9600_data {
 	struct i2c_client *client;
+	u32 thermocouple_type;
 };
 
+static int mcp9600_config(struct mcp9600_data *data)
+{
+	struct i2c_client *client = data->client;
+	int ret;
+	u8 cfg;
+
+	cfg  = FIELD_PREP(MCP9600_SENSOR_TYPE_MASK,
+			  mcp9600_type_map[data->thermocouple_type]);
+
+	ret = i2c_smbus_write_byte_data(client, MCP9600_SENSOR_CFG, cfg);
+	if (ret < 0) {
+		dev_err(&client->dev, "Failed to set sensor configuration\n");
+		return ret;
+	}
+
+	return 0;
+}
+
 #define MCP9600_CHANNELS(hj_num_ev, hj_ev_spec_off, cj_num_ev, cj_ev_spec_off) \
 	{								       \
 		{							       \
 			.type = IIO_TEMP,				       \
 			.address = MCP9600_HOT_JUNCTION,		       \
 			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	       \
+					      BIT(IIO_CHAN_INFO_THERMOCOUPLE_TYPE) | \
 					      BIT(IIO_CHAN_INFO_SCALE),	       \
 			.event_spec = &mcp9600_events[hj_ev_spec_off],	       \
 			.num_event_specs = hj_num_ev,			       \
@@ -165,6 +213,9 @@ static int mcp9600_read_raw(struct iio_dev *indio_dev,
 		*val = 62;
 		*val2 = 500000;
 		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_THERMOCOUPLE_TYPE:
+		*val = mcp9600_tc_types[data->thermocouple_type];
+		return IIO_VAL_CHAR;
 	default:
 		return -EINVAL;
 	}
@@ -426,7 +477,7 @@ static int mcp9600_probe(struct i2c_client *client)
 	const struct mcp_chip_info *chip_info;
 	struct iio_dev *indio_dev;
 	struct mcp9600_data *data;
-	int ch_sel, dev_id;
+	int ch_sel, dev_id, ret;
 
 	chip_info = i2c_get_match_data(client);
 	if (!chip_info)
@@ -458,6 +509,24 @@ static int mcp9600_probe(struct i2c_client *client)
 	data = iio_priv(indio_dev);
 	data->client = client;
 
+	/* Accept type from dt with default of Type-K. */
+	data->thermocouple_type = THERMOCOUPLE_TYPE_K;
+	ret = device_property_read_u32(dev, "thermocouple-type",
+				       &data->thermocouple_type);
+	if (ret && ret != -EINVAL)
+		return dev_err_probe(dev, ret,
+				     "Error reading thermocouple-type property\n");
+
+	if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map))
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid thermocouple-type property %u.\n",
+				     data->thermocouple_type);
+
+	/* Set initial config. */
+	ret = mcp9600_config(data);
+	if (ret)
+		return ret;
+
 	ch_sel = mcp9600_probe_alerts(indio_dev);
 	if (ch_sel < 0)
 		return ch_sel;

-- 
2.39.5

Re: [PATCH v8 4/5] iio: mcp9600: Recognize chip id for mcp9601

[Andy Shevchenko]

On Fri, Aug 22, 2025 at 4:24 PM Ben Collins <bcollins@watter.com> wrote:
>
> The current driver works with mcp9601, but emits a warning because it
> does not recognize the chip id.
>
> MCP9601 is a superset of MCP9600. The drivers works without changes
> on this chipset.
>
> However, the 9601 chip supports open/closed-circuit detection if wired
> properly, so we'll need to be able to differentiate between them.
>
> Moved "struct mcp9600_data" up in the file since a later patch will
> need it and chip_info before the declarations.

...

> +struct mcp9600_data {
> +       struct i2c_client *client;
> +};
> +

> -struct mcp9600_data {
> -       struct i2c_client *client;
> -};
> -

Seems we discussed this. And my suggestion was to defer the change to
when it will be needed.

...

The rest LGTM.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 4/5] iio: mcp9600: Recognize chip id for mcp9601

[Ben Collins]

> On Aug 22, 2025, at 11:57 AM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> On Fri, Aug 22, 2025 at 4:24 PM Ben Collins <bcollins@watter.com> wrote:
>> 
>> The current driver works with mcp9601, but emits a warning because it
>> does not recognize the chip id.
>> 
>> MCP9601 is a superset of MCP9600. The drivers works without changes
>> on this chipset.
>> 
>> However, the 9601 chip supports open/closed-circuit detection if wired
>> properly, so we'll need to be able to differentiate between them.
>> 
>> Moved "struct mcp9600_data" up in the file since a later patch will
>> need it and chip_info before the declarations.
> 
> ...
> 
>> +struct mcp9600_data {
>> +       struct i2c_client *client;
>> +};
>> +
> 
>> -struct mcp9600_data {
>> -       struct i2c_client *client;
>> -};
>> -
> 
> Seems we discussed this. And my suggestion was to defer the change to
> when it will be needed.

And my response was that it’s needed in 5/5 where I add the mcp9600_config()
function. That function will need to be before mcp9600_channels[] in the
IIR patch series.

So either I move mcp9600_data now, or I leave it and put mcp9600_config()
below it, and then in the IIR series I’ll have to move both up.

Didn’t seem to make sense to move 30 lines of code later when I can move
3 lines now.

Regards,
  Ben

Re: [PATCH v8 4/5] iio: mcp9600: Recognize chip id for mcp9601

[Andy Shevchenko]

On Fri, Aug 22, 2025 at 7:07 PM Ben Collins <bcollins@watter.com> wrote:
> > On Aug 22, 2025, at 11:57 AM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Fri, Aug 22, 2025 at 4:24 PM Ben Collins <bcollins@watter.com> wrote:

...

> >> +struct mcp9600_data {
> >> +       struct i2c_client *client;
> >> +};
> >> +
> >
> >> -struct mcp9600_data {
> >> -       struct i2c_client *client;
> >> -};
> >> -
> >
> > Seems we discussed this. And my suggestion was to defer the change to
> > when it will be needed.
>
> And my response was that it’s needed in 5/5 where I add the mcp9600_config()
> function. That function will need to be before mcp9600_channels[] in the
> IIR patch series.
>
> So either I move mcp9600_data now, or I leave it and put mcp9600_config()
> below it, and then in the IIR series I’ll have to move both up.
>
> Didn’t seem to make sense to move 30 lines of code later when I can move
> 3 lines now.

TBH, I have no strong preference, I leave this to Jonathan and other reviewers.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v8 2/5] dt-bindings: iio: mcp9600: Add microchip,mcp9601 and add constraints

[David Lechner]

On 8/22/25 8:23 AM, Ben Collins wrote:
> Add microchip,mcp9601 compatible in addition to the original
> microchip,mcp9600 to designate support between these two chips.
> 
> The current dt-binding has open-circuit and short-circuit as interrupt
> names, but these are only supported in mcp9601.
> 
> The OC and SC detection requires that mcp9601 VSENSE be wired up, which
> not only enables the OC SC interrupts, but also the OC and SC status
> register bits.
> 
> Add a microchip,vsense boolean to show the chip is wired for this
> support.
> 
> Add constraints so this feature only applies if the mcp9601 compatible
> is selected.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  .../iio/temperature/microchip,mcp9600.yaml         | 56 ++++++++++++++++++++--
>  1 file changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> index 57b387a1accc776683500949a22ef0290fc876e8..fb3661c805934255d35f664e1018ed2ec91d05f0 100644
> --- a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> +++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/iio/temperature/microchip,mcp9600.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Microchip MCP9600 thermocouple EMF converter
> +title: Microchip MCP9600 and similar thermocouple EMF converters
>  
>  maintainers:
>    - Andrew Hepp <andrew.hepp@ahepp.dev>
> @@ -14,7 +14,11 @@ description:
>  
>  properties:
>    compatible:
> -    const: microchip,mcp9600
> +    oneOf:
> +      - const: microchip,mcp9600
> +      - items:
> +          - const: microchip,mcp9601
> +          - const: microchip,mcp9600
>  
>    reg:
>      maxItems: 1
> @@ -43,8 +47,37 @@ properties:
>        Use defines in dt-bindings/iio/temperature/thermocouple.h.
>        Supported types are B, E, J, K, N, R, S, T.
>  
> +  microchip,vsense:
> +    type: boolean
> +    description:
> +      This flag indicates that the chip has been wired with VSENSE to
> +      enable open and short circuit detect.
> +
>    vdd-supply: true
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          not:
> +            contains:
> +              const: microchip,mcp9601
> +    then:
> +      properties:
> +        interrupts:
> +          minItems: 1
> +          maxItems: 4
> +        interrupt-names:
> +          minItems: 1
> +          maxItems: 4
> +          items:
> +            enum:
> +              - alert1
> +              - alert2
> +              - alert3
> +              - alert4
> +        microchip,vsense: false
> +
>  required:
>    - compatible
>    - reg
> @@ -64,8 +97,25 @@ examples:
>              reg = <0x60>;
>              interrupt-parent = <&gpio>;
>              interrupts = <25 IRQ_TYPE_EDGE_RISING>;
> -            interrupt-names = "open-circuit";
> +            interrupt-names = "alert1";
>              thermocouple-type = <THERMOCOUPLE_TYPE_K>;
>              vdd-supply = <&vdd>;
>          };
>      };
> +  - |
> +    #include <dt-bindings/iio/temperature/thermocouple.h>

This header isn't used in this example.

> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        temperature-sensor@62 {
> +            compatible = "microchip,mcp9601", "microchip,mcp9600";
> +            reg = <0x62>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <22 IRQ_TYPE_EDGE_RISING>, <23 IRQ_TYPE_EDGE_RISING>;
> +            interrupt-names = "open-circuit", "short-circuit";
> +            vdd-supply = <&vdd>;
> +            microchip,vsense;
> +        };
> +    };
> 

With that fixed:

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v8 1/5] dt-bindings: iio: mcp9600: Set default 3 for thermocouple-type

[David Lechner]

On 8/22/25 8:23 AM, Ben Collins wrote:
> As is already documented in this file, Type-K is the default, so make
> that explicit in the dt-bindings.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> index d2cafa38a5442e229be8befb26ae3f34bae44cdb..57b387a1accc776683500949a22ef0290fc876e8 100644
> --- a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> +++ b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> @@ -37,6 +37,7 @@ properties:
>  
>    thermocouple-type:
>      $ref: /schemas/types.yaml#/definitions/uint32
> +    default: 3
>      description:
>        Type of thermocouple (THERMOCOUPLE_TYPE_K if omitted).
>        Use defines in dt-bindings/iio/temperature/thermocouple.h.
> 

Reviewed-by: David Lechner <dlechner@baylibre.com>

Re: [PATCH v8 2/5] dt-bindings: iio: mcp9600: Add microchip,mcp9601 and add constraints

[David Lechner]

On 8/23/25 11:06 AM, David Lechner wrote:
> On 8/22/25 8:23 AM, Ben Collins wrote:

...

>> +  - |
>> +    #include <dt-bindings/iio/temperature/thermocouple.h>
> 
> This header isn't used in this example.
> 
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        temperature-sensor@62 {
>> +            compatible = "microchip,mcp9601", "microchip,mcp9600";
>> +            reg = <0x62>;
>> +            interrupt-parent = <&gpio>;
>> +            interrupts = <22 IRQ_TYPE_EDGE_RISING>, <23 IRQ_TYPE_EDGE_RISING>;
>> +            interrupt-names = "open-circuit", "short-circuit";
>> +            vdd-supply = <&vdd>;
>> +            microchip,vsense;
>> +        };
>> +    };
>>
> 
> With that fixed:
> 
> Reviewed-by: David Lechner <dlechner@baylibre.com>
> 

BTW, sometimes Jonathan is nice and fixes a one-line change like this
when applying patches, so don't rush off and send a v9 just quite yet
unless there are other bigger changes that need to be done anyway.

Re: [PATCH v8 4/5] iio: mcp9600: Recognize chip id for mcp9601

[Jonathan Cameron]

On Fri, 22 Aug 2025 19:47:39 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Fri, Aug 22, 2025 at 7:07 PM Ben Collins <bcollins@watter.com> wrote:
> > > On Aug 22, 2025, at 11:57 AM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Fri, Aug 22, 2025 at 4:24 PM Ben Collins <bcollins@watter.com> wrote:  
> 
> ...
> 
> > >> +struct mcp9600_data {
> > >> +       struct i2c_client *client;
> > >> +};
> > >> +  
> > >  
> > >> -struct mcp9600_data {
> > >> -       struct i2c_client *client;
> > >> -};
> > >> -  
> > >
> > > Seems we discussed this. And my suggestion was to defer the change to
> > > when it will be needed.  
> >
> > And my response was that it’s needed in 5/5 where I add the mcp9600_config()
> > function. That function will need to be before mcp9600_channels[] in the
> > IIR patch series.
> >
> > So either I move mcp9600_data now, or I leave it and put mcp9600_config()
> > below it, and then in the IIR series I’ll have to move both up.
> >
> > Didn’t seem to make sense to move 30 lines of code later when I can move
> > 3 lines now.  
> 
> TBH, I have no strong preference, I leave this to Jonathan and other reviewers.
> 

Not significant enough to worry about. So I left it as is whilst applying.

Re: [PATCH v8 0/5] iio: mcp9600: Features and improvements

[Jonathan Cameron]

On Fri, 22 Aug 2025 09:23:49 -0400
Ben Collins <bcollins@watter.com> wrote:

Series applied with that unnecessary include that David pointed out dropped.

Applied to the togreg branch of iio.git but initially pushed out as testing
to give 0-day a few days to poke at it.

Thanks,

Jonathan


> ChangeLog:
> v7 -> v8:
>   - Style changes in dt-bindings example
>   - Simplify some return value checks
>   - Move assignment to where it's checked
>   - Speeling
> 
> v6 -> v7:
>   - Separate out the mcp9600 IIR series into its own series as there is
>     a lot of conversation around implementation (removed related
>     comments from this changelog).
> 
> v5 -> v6:
>   - Fix accidental typo added in dt-bindings: IRQ_TYPE_EDGE_RISIN
>   - Correct some constraints in dt-bindings
>   - Reverse if/then for mcp9601 vs mcp9600 constraints in dt-bindings
>   - Updates to changelog for patch 2/6 (dt-bindings mcp9600)
>   - Cleanup tabs that were converted to spaces
>   - Split thermocouple-type default to separate patch
> 
> v4 -> v5:
>   - None
> 
> v3 -> v4:
>   - Based on feedback from David Lechner <dlechner@baylibre.com>
>     * Allow fallback compatible in dt-bindings for mcp9601.
>   - Based on feedback from Jonathan Cameron <jic23@kernel.org>
>     * Be explicit in patch description for fixed width changes.
>     * Check chip_info for NULL to quiet warnings from kernel-test-robot
>     * Remove "and similar" for long description of MCP9600.
>   - Set default 3 for thermocouple in dt-binding
>   - Rework open/short circuit in dt-bindings
> 
> v2 -> v3:
>   - Improve changelogs in each patch
>   - Based on feedback from Andy Shevchenko <andy.shevchenko@gmail.com>
>     * Set register offsets to fixed width
>     * Fix typos
>     * Future-proof Kconfig changes
>     * Convert to using chip_info paradigm
>     * Verbiage: dt -> firmware description
>     * Use proper specifiers and drop castings
>     * Fix register offset to be fixed-width
>     * u8 for cfg var
>     * Fix % type for u32 to be %u
>     * Make blank lines consistent between case statements
> 
> v1 -> v2:
>   - Break into individual patches
> 
> v1:
>   - Initial patch to enable IIR and thermocouple-type
>   - Recognize mcp9601
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
> - Link to v7: https://lore.kernel.org/r/20250819-upstream-changes-v7-0-88a33aa78f6a@watter.com
> 
> ---
> Ben Collins (5):
>       dt-bindings: iio: mcp9600: Set default 3 for thermocouple-type
>       dt-bindings: iio: mcp9600: Add microchip,mcp9601 and add constraints
>       iio: mcp9600: White space and fixed width cleanup
>       iio: mcp9600: Recognize chip id for mcp9601
>       iio: mcp9600: Add support for thermocouple-type
> 
>  .../iio/temperature/microchip,mcp9600.yaml         |  57 +++++++-
>  drivers/iio/temperature/Kconfig                    |   8 +-
>  drivers/iio/temperature/mcp9600.c                  | 151 +++++++++++++++++----
>  3 files changed, 187 insertions(+), 29 deletions(-)
> ---
> base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> change-id: 20250819-upstream-changes-c89af86743fa
> 
> Best regards,

Re: [PATCH v8 0/5] iio: mcp9600: Features and improvements

[Ben Collins]

On Mon, Aug 25, 2025 at 10:38:47AM -0500, Jonathan Cameron wrote:
> On Fri, 22 Aug 2025 09:23:49 -0400
> Ben Collins <bcollins@watter.com> wrote:
> 
> Series applied with that unnecessary include that David pointed out dropped.
> 
> Applied to the togreg branch of iio.git but initially pushed out as testing
> to give 0-day a few days to poke at it.
> 

Thanks Jonathan and to everyone else for the input.

> 
> > ChangeLog:
> > v7 -> v8:
> >   - Style changes in dt-bindings example
> >   - Simplify some return value checks
> >   - Move assignment to where it's checked
> >   - Speeling
> > 
> > v6 -> v7:
> >   - Separate out the mcp9600 IIR series into its own series as there is
> >     a lot of conversation around implementation (removed related
> >     comments from this changelog).
> > 
> > v5 -> v6:
> >   - Fix accidental typo added in dt-bindings: IRQ_TYPE_EDGE_RISIN
> >   - Correct some constraints in dt-bindings
> >   - Reverse if/then for mcp9601 vs mcp9600 constraints in dt-bindings
> >   - Updates to changelog for patch 2/6 (dt-bindings mcp9600)
> >   - Cleanup tabs that were converted to spaces
> >   - Split thermocouple-type default to separate patch
> > 
> > v4 -> v5:
> >   - None
> > 
> > v3 -> v4:
> >   - Based on feedback from David Lechner <dlechner@baylibre.com>
> >     * Allow fallback compatible in dt-bindings for mcp9601.
> >   - Based on feedback from Jonathan Cameron <jic23@kernel.org>
> >     * Be explicit in patch description for fixed width changes.
> >     * Check chip_info for NULL to quiet warnings from kernel-test-robot
> >     * Remove "and similar" for long description of MCP9600.
> >   - Set default 3 for thermocouple in dt-binding
> >   - Rework open/short circuit in dt-bindings
> > 
> > v2 -> v3:
> >   - Improve changelogs in each patch
> >   - Based on feedback from Andy Shevchenko <andy.shevchenko@gmail.com>
> >     * Set register offsets to fixed width
> >     * Fix typos
> >     * Future-proof Kconfig changes
> >     * Convert to using chip_info paradigm
> >     * Verbiage: dt -> firmware description
> >     * Use proper specifiers and drop castings
> >     * Fix register offset to be fixed-width
> >     * u8 for cfg var
> >     * Fix % type for u32 to be %u
> >     * Make blank lines consistent between case statements
> > 
> > v1 -> v2:
> >   - Break into individual patches
> > 
> > v1:
> >   - Initial patch to enable IIR and thermocouple-type
> >   - Recognize mcp9601
> > 
> > Signed-off-by: Ben Collins <bcollins@watter.com>
> > ---
> > - Link to v7: https://lore.kernel.org/r/20250819-upstream-changes-v7-0-88a33aa78f6a@watter.com
> > 
> > ---
> > Ben Collins (5):
> >       dt-bindings: iio: mcp9600: Set default 3 for thermocouple-type
> >       dt-bindings: iio: mcp9600: Add microchip,mcp9601 and add constraints
> >       iio: mcp9600: White space and fixed width cleanup
> >       iio: mcp9600: Recognize chip id for mcp9601
> >       iio: mcp9600: Add support for thermocouple-type
> > 
> >  .../iio/temperature/microchip,mcp9600.yaml         |  57 +++++++-
> >  drivers/iio/temperature/Kconfig                    |   8 +-
> >  drivers/iio/temperature/mcp9600.c                  | 151 +++++++++++++++++----
> >  3 files changed, 187 insertions(+), 29 deletions(-)
> > ---
> > base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
> > change-id: 20250819-upstream-changes-c89af86743fa
> > 
> > Best regards,
> 
> 

-- 
 Ben Collins
 https://libjwt.io
 https://github.com/benmcollins
 --
 3EC9 7598 1672 961A 1139  173A 5D5A 57C7 242B 22CF