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

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

[Ben Collins]

From: Ben Collins <bcollins@kernel.org>

ChangeLog:
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
    * FIELD_PREP -> FIELD_MODIFY
    * Remove explicit setting of 0 value in filter_level
  - Based on feedback from David Lechner <dlechner@baylibre.com>
    * Rework IIR values exposed to sysfs. Using the ratios, there was no
      way to represent "disabled" (i.e. infinity). Based on the bmp280
      driver I went with using the power coefficients (e.g. 1, 2, 4, 8,
      ...) where 1 is disabled (n=0).

v1 -> v2:
  - Break into individual patches

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

Ben Collins (5):
  dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601
  iio: mcp9600: White space cleanup for tab alignment
  iio: mcp9600: Recognize chip id for mcp9601
  iio: mcp9600: Add support for thermocouple-type
  iio: mcp9600: Add support for IIR filter

 .../iio/temperature/microchip,mcp9600.yaml    |   6 +-
 drivers/iio/temperature/Kconfig               |   8 +-
 drivers/iio/temperature/mcp9600.c             | 209 ++++++++++++++++--
 3 files changed, 201 insertions(+), 22 deletions(-)

-- 
2.50.1

[PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

The mcp9600 driver supports the mcp9601 chip, but complains about not
recognizing the device id on probe. A separate patch...

	iio: mcp9600: Recognize chip id for mcp9601

...addresses this. This patch updates the dt-bindings for this chip to
reflect the change to allow explicitly setting microchip,mcp9601 as
the expected chip type.

The mcp9601 also supports features not found on the mcp9600, so this
will also allow the driver to differentiate the support of these
features.

Signed-off-by: Ben Collins <bcollins@watter.com>
---
 .../bindings/iio/temperature/microchip,mcp9600.yaml         | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
index d2cafa38a5442..d8af0912ce886 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,9 @@ description:
 
 properties:
   compatible:
-    const: microchip,mcp9600
+    enum:
+      - microchip,mcp9600
+      - microchip,mcp9601
 
   reg:
     maxItems: 1
-- 
2.50.1

[PATCH 2/5] iio: mcp9600: White space cleanup for tab alignment

[Ben Collins]

Purely to align tabs for #defines.

Signed-off-by: Ben Collins <bcollins@watter.com>
---
 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 6e9108d5cf75f..40906bb200ec9 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.50.1

[PATCH 3/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.

Signed-off-by: Ben Collins <bcollins@watter.com>
---
 drivers/iio/temperature/Kconfig   |  8 +++--
 drivers/iio/temperature/mcp9600.c | 51 +++++++++++++++++++++++++------
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 1244d8e17d504..e14ea6ebc7a24 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
+	  and similar 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 40906bb200ec9..c18ae17b6d82f 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
 
@@ -123,6 +124,11 @@ static const struct iio_chan_spec mcp9600_channels[][2] = {
 	MCP9600_CHANNELS(2, 0, 2, 0), /* Alerts: 1 2 3 4 */
 };
 
+struct mcp_chip_info {
+	u8 chip_id;
+	const char *chip_name;
+};
+
 struct mcp9600_data {
 	struct i2c_client *client;
 };
@@ -416,16 +422,29 @@ static int mcp9600_probe_alerts(struct iio_dev *indio_dev)
 
 static int mcp9600_probe(struct i2c_client *client)
 {
+	const struct mcp_chip_info *chip_info = i2c_get_match_data(client);
 	struct iio_dev *indio_dev;
 	struct mcp9600_data *data;
-	int ret, ch_sel;
+	int ch_sel, dev_id, ret;
+
+	dev_id = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
+	if (dev_id < 0)
+		return dev_err_probe(&client->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(&client->dev,
+				"Expected id %02x, but device responded with %02\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(&client->dev, "Unknown id %x, using %x\n", dev_id,
+			 chip_info->chip_id);
+	}
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (!indio_dev)
@@ -439,7 +458,7 @@ 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]);
@@ -447,14 +466,26 @@ static int mcp9600_probe(struct i2c_client *client)
 	return devm_iio_device_register(&client->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.50.1

[PATCH 4/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>
---
 drivers/iio/temperature/mcp9600.c | 69 +++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index c18ae17b6d82f..361572a241f06 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 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,
@@ -89,6 +117,7 @@ static const struct iio_event_spec mcp9600_events[] = {
 			.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,			       \
@@ -131,6 +160,7 @@ struct mcp_chip_info {
 
 struct mcp9600_data {
 	struct i2c_client *client;
+	u32 thermocouple_type;
 };
 
 static int mcp9600_read(struct mcp9600_data *data,
@@ -165,11 +195,32 @@ 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;
 	}
 }
 
+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;
+}
+
 static int mcp9600_get_alert_index(int channel2, enum iio_event_direction dir)
 {
 	if (channel2 == IIO_MOD_TEMP_AMBIENT) {
@@ -453,6 +504,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(&client->dev, "thermocouple-type",
+				       &data->thermocouple_type);
+	if (ret < 0 && ret != -EINVAL)
+		return dev_err_probe(&client->dev, ret,
+				     "Error reading thermocouple-type property\n");
+
+	if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map))
+		return dev_err_probe(&client->dev, -EINVAL,
+				     "Invalid thermocouple-type property %u.\n",
+				     data->thermocouple_type);
+
+	/* Set initial config. */
+	ret = mcp9600_config(data);
+	if (ret < 0)
+		return ret;
+
 	ch_sel = mcp9600_probe_alerts(indio_dev);
 	if (ch_sel < 0)
 		return ch_sel;
-- 
2.50.1

[PATCH 5/5] iio: mcp9600: Add support for IIR filter

[Ben Collins]

MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
to allow get/set of this value.

Signed-off-by: Ben Collins <bcollins@watter.com>
---
 drivers/iio/temperature/mcp9600.c | 73 +++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index 361572a241f06..896520ddf6d3c 100644
--- a/drivers/iio/temperature/mcp9600.c
+++ b/drivers/iio/temperature/mcp9600.c
@@ -31,6 +31,7 @@
 #define MCP9600_STATUS_ALERT(x)		BIT(x)
 #define MCP9600_SENSOR_CFG		0x05
 #define MCP9600_SENSOR_TYPE_MASK	GENMASK(6, 4)
+#define MCP9600_FILTER_MASK		GENMASK(2, 0)
 #define MCP9600_ALERT_CFG1		0x08
 #define MCP9600_ALERT_CFG(x)		(MCP9600_ALERT_CFG1 + (x - 1))
 #define MCP9600_ALERT_CFG_ENABLE	BIT(0)
@@ -94,6 +95,10 @@ static const int mcp9600_tc_types[] = {
 	[THERMOCOUPLE_TYPE_R] = 'R',
 };
 
+static const int mcp_iir_coefficients_avail[] = {
+	1, 2, 4, 8, 16, 32, 64, 128,
+};
+
 static const struct iio_event_spec mcp9600_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -118,7 +123,10 @@ static const struct iio_event_spec mcp9600_events[] = {
 			.address = MCP9600_HOT_JUNCTION,		       \
 			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	       \
 					      BIT(IIO_CHAN_INFO_THERMOCOUPLE_TYPE) | \
+					      BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) |  \
 					      BIT(IIO_CHAN_INFO_SCALE),	       \
+			.info_mask_separate_available =                        \
+					      BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
 			.event_spec = &mcp9600_events[hj_ev_spec_off],	       \
 			.num_event_specs = hj_num_ev,			       \
 		},							       \
@@ -161,6 +169,7 @@ struct mcp_chip_info {
 struct mcp9600_data {
 	struct i2c_client *client;
 	u32 thermocouple_type;
+	u8 filter_level; /* Chip default is 0 */
 };
 
 static int mcp9600_read(struct mcp9600_data *data,
@@ -191,13 +200,36 @@ static int mcp9600_read_raw(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_SCALE:
 		*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;
+
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		*val = mcp_iir_coefficients_avail[data->filter_level];
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mcp9600_read_avail(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length,
+			      long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		*vals = mcp_iir_coefficients_avail;
+		*type = IIO_VAL_INT;
+		*length = ARRAY_SIZE(mcp_iir_coefficients_avail);
+		return IIO_AVAIL_LIST;
 	default:
 		return -EINVAL;
 	}
@@ -211,6 +243,7 @@ static int mcp9600_config(struct mcp9600_data *data)
 
 	cfg  = FIELD_PREP(MCP9600_SENSOR_TYPE_MASK,
 			  mcp9600_type_map[data->thermocouple_type]);
+	FIELD_MODIFY(MCP9600_FILTER_MASK, &cfg, data->filter_level);
 
 	ret = i2c_smbus_write_byte_data(client, MCP9600_SENSOR_CFG, cfg);
 	if (ret < 0) {
@@ -221,6 +254,43 @@ static int mcp9600_config(struct mcp9600_data *data)
 	return 0;
 }
 
+static int mcp9600_write_raw_get_fmt(struct iio_dev *indio_dev,
+				     struct iio_chan_spec const *chan,
+				     long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mcp9600_write_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 i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		for (i = 0; i < ARRAY_SIZE(mcp_iir_coefficients_avail); i++) {
+			if (mcp_iir_coefficients_avail[i] == val)
+				break;
+		}
+
+		if (i == ARRAY_SIZE(mcp_iir_coefficients_avail))
+			return -EINVAL;
+
+		data->filter_level = i;
+		return mcp9600_config(data);
+
+	default:
+		return -EINVAL;
+	}
+}
+
 static int mcp9600_get_alert_index(int channel2, enum iio_event_direction dir)
 {
 	if (channel2 == IIO_MOD_TEMP_AMBIENT) {
@@ -358,6 +428,9 @@ static int mcp9600_write_thresh(struct iio_dev *indio_dev,
 
 static const struct iio_info mcp9600_info = {
 	.read_raw = mcp9600_read_raw,
+	.read_avail = mcp9600_read_avail,
+	.write_raw = mcp9600_write_raw,
+	.write_raw_get_fmt = mcp9600_write_raw_get_fmt,
 	.read_event_config = mcp9600_read_event_config,
 	.write_event_config = mcp9600_write_event_config,
 	.read_event_value = mcp9600_read_thresh,
-- 
2.50.1

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

[kernel test robot]

Hi Ben,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.17-rc1 next-20250815]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ben-Collins/dt-bindings-iio-mcp9600-Add-compatible-for-microchip-mcp9601/20250816-005705
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20250815164627.22002-4-bcollins%40watter.com
patch subject: [PATCH 3/5] iio: mcp9600: Recognize chip id for mcp9601
config: riscv-randconfig-001-20250816 (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508161646.PDl6V4EU-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:27:
   In file included from include/linux/kernel_stat.h:8:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:12:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:820:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     820 |         insl(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl'
     106 | #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count)
         |                                          ~~~~~~~~~~ ^
   In file included from drivers/iio/temperature/mcp9600.c:13:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:27:
   In file included from include/linux/kernel_stat.h:8:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:12:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:829:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     829 |         outsb(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb'
     118 | #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count)
         |                                            ~~~~~~~~~~ ^
   In file included from drivers/iio/temperature/mcp9600.c:13:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:27:
   In file included from include/linux/kernel_stat.h:8:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:12:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:838:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     838 |         outsw(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:119:55: note: expanded from macro 'outsw'
     119 | #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count)
         |                                            ~~~~~~~~~~ ^
   In file included from drivers/iio/temperature/mcp9600.c:13:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:27:
   In file included from include/linux/kernel_stat.h:8:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:12:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:847:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
     847 |         outsl(addr, buffer, count);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/riscv/include/asm/io.h:120:55: note: expanded from macro 'outsl'
     120 | #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count)
         |                                            ~~~~~~~~~~ ^
   In file included from drivers/iio/temperature/mcp9600.c:13:
   In file included from include/linux/i2c.h:19:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:13:
   In file included from include/linux/cgroup.h:27:
   In file included from include/linux/kernel_stat.h:8:
   In file included from include/linux/interrupt.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:12:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
         |                                                   ~~~~~~~~~~ ^
>> drivers/iio/temperature/mcp9600.c:440:53: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]
     440 |                                 "Expected id %02x, but device responded with %02\n",
         |                                                                              ~~~^
   include/linux/dev_printk.h:156:62: note: expanded from macro 'dev_warn'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                                     ^~~
   include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
      19 | #define dev_fmt(fmt) fmt
         |                      ^~~
   include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ^~~
>> drivers/iio/temperature/mcp9600.c:441:26: warning: data argument not used by format string [-Wformat-extra-args]
     440 |                                 "Expected id %02x, but device responded with %02\n",
         |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     441 |                                  chip_info->chip_id, dev_id);
         |                                                      ^
   include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn'
     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                                     ~~~     ^
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
         |                              ~~~    ^
   drivers/iio/temperature/mcp9600.c:428:22: warning: unused variable 'ret' [-Wunused-variable]
     428 |         int ch_sel, dev_id, ret;
         |                             ^~~
   10 warnings generated.


vim +/x0a +440 drivers/iio/temperature/mcp9600.c

   422	
   423	static int mcp9600_probe(struct i2c_client *client)
   424	{
   425		const struct mcp_chip_info *chip_info = i2c_get_match_data(client);
   426		struct iio_dev *indio_dev;
   427		struct mcp9600_data *data;
   428		int ch_sel, dev_id, ret;
   429	
   430		dev_id = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
   431		if (dev_id < 0)
   432			return dev_err_probe(&client->dev, dev_id,
   433					     "Failed to read device ID\n");
   434	
   435		switch (dev_id) {
   436		case MCP9600_DEVICE_ID_MCP9600:
   437		case MCP9600_DEVICE_ID_MCP9601:
   438			if (dev_id != chip_info->chip_id)
   439				dev_warn(&client->dev,
 > 440					"Expected id %02x, but device responded with %02\n",
 > 441					 chip_info->chip_id, dev_id);
   442			break;
   443	
   444		default:
   445			dev_warn(&client->dev, "Unknown id %x, using %x\n", dev_id,
   446				 chip_info->chip_id);
   447		}
   448	
   449		indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
   450		if (!indio_dev)
   451			return -ENOMEM;
   452	
   453		data = iio_priv(indio_dev);
   454		data->client = client;
   455	
   456		ch_sel = mcp9600_probe_alerts(indio_dev);
   457		if (ch_sel < 0)
   458			return ch_sel;
   459	
   460		indio_dev->info = &mcp9600_info;
   461		indio_dev->name = chip_info->chip_name;
   462		indio_dev->modes = INDIO_DIRECT_MODE;
   463		indio_dev->channels = mcp9600_channels[ch_sel];
   464		indio_dev->num_channels = ARRAY_SIZE(mcp9600_channels[ch_sel]);
   465	
   466		return devm_iio_device_register(&client->dev, indio_dev);
   467	}
   468	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Jonathan Cameron]

On Fri, 15 Aug 2025 16:46:03 +0000
Ben Collins <bcollins@watter.com> wrote:

> The mcp9600 driver supports the mcp9601 chip, but complains about not
> recognizing the device id on probe. A separate patch...
> 
> 	iio: mcp9600: Recognize chip id for mcp9601
> 
> ...addresses this. This patch updates the dt-bindings for this chip to
> reflect the change to allow explicitly setting microchip,mcp9601 as
> the expected chip type.
> 
> The mcp9601 also supports features not found on the mcp9600, so this
> will also allow the driver to differentiate the support of these
> features.

If it's additional features only then you can still use a fallback
compatible.  Intent being that a new DT vs old kernel still 'works'.

Then for the driver on new kernels we match on the new compatible and
support those new features.  Old kernel users get to keep the ID
mismatch warning - they can upgrade if they want that to go away ;)

Krzysztof raised the same point on v2 but I'm not seeing it addressed
in that discussion.

Jonathan

> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
>  .../bindings/iio/temperature/microchip,mcp9600.yaml         | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
> index d2cafa38a5442..d8af0912ce886 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,9 @@ description:
>  
>  properties:
>    compatible:
> -    const: microchip,mcp9600
> +    enum:
> +      - microchip,mcp9600
> +      - microchip,mcp9601
>  
>    reg:
>      maxItems: 1

Re: [PATCH 2/5] iio: mcp9600: White space cleanup for tab alignment

[Jonathan Cameron]

On Fri, 15 Aug 2025 16:46:04 +0000
Ben Collins <bcollins@watter.com> wrote:

> Purely to align tabs for #defines.
Hi Ben,

Mention the changes to two digits for register addresses to.
Good change to roll in here given what you are touching anyway so
just make sure to call it out in the patch description.

Jonathan

> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
>  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 6e9108d5cf75f..40906bb200ec9 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
>  

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

[Jonathan Cameron]

On Sat, 16 Aug 2025 16:46:12 +0800
kernel test robot <lkp@intel.com> wrote:

> Hi Ben,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on jic23-iio/togreg]
> [also build test WARNING on linus/master v6.17-rc1 next-20250815]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Ben-Collins/dt-bindings-iio-mcp9600-Add-compatible-for-microchip-mcp9601/20250816-005705
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> patch link:    https://lore.kernel.org/r/20250815164627.22002-4-bcollins%40watter.com
> patch subject: [PATCH 3/5] iio: mcp9600: Recognize chip id for mcp9601
> config: riscv-randconfig-001-20250816 (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/config)
> compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202508161646.PDl6V4EU-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from include/linux/regulator/consumer.h:35:
>    In file included from include/linux/suspend.h:5:
>    In file included from include/linux/swap.h:9:
>    In file included from include/linux/memcontrol.h:13:
>    In file included from include/linux/cgroup.h:27:
>    In file included from include/linux/kernel_stat.h:8:
>    In file included from include/linux/interrupt.h:11:
>    In file included from include/linux/hardirq.h:11:
>    In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
>    In file included from include/asm-generic/hardirq.h:17:
>    In file included from include/linux/irq.h:20:
>    In file included from include/linux/io.h:12:
>    In file included from arch/riscv/include/asm/io.h:136:
>    include/asm-generic/io.h:820:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>      820 |         insl(addr, buffer, count);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
>    arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl'
>      106 | #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count)
>          |                                          ~~~~~~~~~~ ^
>    In file included from drivers/iio/temperature/mcp9600.c:13:
>    In file included from include/linux/i2c.h:19:
>    In file included from include/linux/regulator/consumer.h:35:
>    In file included from include/linux/suspend.h:5:
>    In file included from include/linux/swap.h:9:
>    In file included from include/linux/memcontrol.h:13:
>    In file included from include/linux/cgroup.h:27:
>    In file included from include/linux/kernel_stat.h:8:
>    In file included from include/linux/interrupt.h:11:
>    In file included from include/linux/hardirq.h:11:
>    In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
>    In file included from include/asm-generic/hardirq.h:17:
>    In file included from include/linux/irq.h:20:
>    In file included from include/linux/io.h:12:
>    In file included from arch/riscv/include/asm/io.h:136:
>    include/asm-generic/io.h:829:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>      829 |         outsb(addr, buffer, count);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
>    arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb'
>      118 | #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count)
>          |                                            ~~~~~~~~~~ ^
>    In file included from drivers/iio/temperature/mcp9600.c:13:
>    In file included from include/linux/i2c.h:19:
>    In file included from include/linux/regulator/consumer.h:35:
>    In file included from include/linux/suspend.h:5:
>    In file included from include/linux/swap.h:9:
>    In file included from include/linux/memcontrol.h:13:
>    In file included from include/linux/cgroup.h:27:
>    In file included from include/linux/kernel_stat.h:8:
>    In file included from include/linux/interrupt.h:11:
>    In file included from include/linux/hardirq.h:11:
>    In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
>    In file included from include/asm-generic/hardirq.h:17:
>    In file included from include/linux/irq.h:20:
>    In file included from include/linux/io.h:12:
>    In file included from arch/riscv/include/asm/io.h:136:
>    include/asm-generic/io.h:838:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>      838 |         outsw(addr, buffer, count);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
>    arch/riscv/include/asm/io.h:119:55: note: expanded from macro 'outsw'
>      119 | #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count)
>          |                                            ~~~~~~~~~~ ^
>    In file included from drivers/iio/temperature/mcp9600.c:13:
>    In file included from include/linux/i2c.h:19:
>    In file included from include/linux/regulator/consumer.h:35:
>    In file included from include/linux/suspend.h:5:
>    In file included from include/linux/swap.h:9:
>    In file included from include/linux/memcontrol.h:13:
>    In file included from include/linux/cgroup.h:27:
>    In file included from include/linux/kernel_stat.h:8:
>    In file included from include/linux/interrupt.h:11:
>    In file included from include/linux/hardirq.h:11:
>    In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
>    In file included from include/asm-generic/hardirq.h:17:
>    In file included from include/linux/irq.h:20:
>    In file included from include/linux/io.h:12:
>    In file included from arch/riscv/include/asm/io.h:136:
>    include/asm-generic/io.h:847:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>      847 |         outsl(addr, buffer, count);
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
>    arch/riscv/include/asm/io.h:120:55: note: expanded from macro 'outsl'
>      120 | #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count)
>          |                                            ~~~~~~~~~~ ^
>    In file included from drivers/iio/temperature/mcp9600.c:13:
>    In file included from include/linux/i2c.h:19:
>    In file included from include/linux/regulator/consumer.h:35:
>    In file included from include/linux/suspend.h:5:
>    In file included from include/linux/swap.h:9:
>    In file included from include/linux/memcontrol.h:13:
>    In file included from include/linux/cgroup.h:27:
>    In file included from include/linux/kernel_stat.h:8:
>    In file included from include/linux/interrupt.h:11:
>    In file included from include/linux/hardirq.h:11:
>    In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
>    In file included from include/asm-generic/hardirq.h:17:
>    In file included from include/linux/irq.h:20:
>    In file included from include/linux/io.h:12:
>    In file included from arch/riscv/include/asm/io.h:136:
>    include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
>     1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
>          |                                                   ~~~~~~~~~~ ^
> >> drivers/iio/temperature/mcp9600.c:440:53: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]  
>      440 |                                 "Expected id %02x, but device responded with %02\n",
>          |                                                                              ~~~^
>    include/linux/dev_printk.h:156:62: note: expanded from macro 'dev_warn'
>      156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
>          |                                                                     ^~~
>    include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
>       19 | #define dev_fmt(fmt) fmt
>          |                      ^~~
>    include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
>      110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
>          |                              ^~~
> >> drivers/iio/temperature/mcp9600.c:441:26: warning: data argument not used by format string [-Wformat-extra-args]  
>      440 |                                 "Expected id %02x, but device responded with %02\n",
>          |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      441 |                                  chip_info->chip_id, dev_id);
>          |                                                      ^
>    include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn'
>      156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
>          |                                                                     ~~~     ^
>    include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
>      110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
>          |                              ~~~    ^
>    drivers/iio/temperature/mcp9600.c:428:22: warning: unused variable 'ret' [-Wunused-variable]
>      428 |         int ch_sel, dev_id, ret;
>          |                             ^~~
>    10 warnings generated.
> 
> 
> vim +/x0a +440 drivers/iio/temperature/mcp9600.c
> 
>    422	
>    423	static int mcp9600_probe(struct i2c_client *client)
>    424	{
>    425		const struct mcp_chip_info *chip_info = i2c_get_match_data(client);

Probably a false positive as I don't think we can probe without something matching and hence
that not being NULL but an error check on that match is still a nice to have and should
resolve this build warning.  Note there is very little chance a compiler could ever figure
out if this can be NULL or not so it's a reasonable warning!

Jonathan


>    426		struct iio_dev *indio_dev;
>    427		struct mcp9600_data *data;
>    428		int ch_sel, dev_id, ret;
>    429	
>    430		dev_id = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
>    431		if (dev_id < 0)
>    432			return dev_err_probe(&client->dev, dev_id,
>    433					     "Failed to read device ID\n");
>    434	
>    435		switch (dev_id) {
>    436		case MCP9600_DEVICE_ID_MCP9600:
>    437		case MCP9600_DEVICE_ID_MCP9601:
>    438			if (dev_id != chip_info->chip_id)
>    439				dev_warn(&client->dev,
>  > 440					"Expected id %02x, but device responded with %02\n",
>  > 441					 chip_info->chip_id, dev_id);  
>    442			break;
>    443	
>    444		default:
>    445			dev_warn(&client->dev, "Unknown id %x, using %x\n", dev_id,
>    446				 chip_info->chip_id);
>    447		}
>    448	
>    449		indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>    450		if (!indio_dev)
>    451			return -ENOMEM;
>    452	
>    453		data = iio_priv(indio_dev);
>    454		data->client = client;
>    455	
>    456		ch_sel = mcp9600_probe_alerts(indio_dev);
>    457		if (ch_sel < 0)
>    458			return ch_sel;
>    459	
>    460		indio_dev->info = &mcp9600_info;
>    461		indio_dev->name = chip_info->chip_name;
>    462		indio_dev->modes = INDIO_DIRECT_MODE;
>    463		indio_dev->channels = mcp9600_channels[ch_sel];
>    464		indio_dev->num_channels = ARRAY_SIZE(mcp9600_channels[ch_sel]);
>    465	
>    466		return devm_iio_device_register(&client->dev, indio_dev);
>    467	}
>    468	
> 

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

[Jonathan Cameron]

On Fri, 15 Aug 2025 16:46:05 +0000
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.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
>  drivers/iio/temperature/Kconfig   |  8 +++--
>  drivers/iio/temperature/mcp9600.c | 51 +++++++++++++++++++++++++------
>  2 files changed, 46 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 1244d8e17d504..e14ea6ebc7a24 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
> +	  and similar thermocouple EMF converters connected via I2C.

If we are updating this list every time, do we need to add the 'and similar'
to the help text?  It makes sense only in the short description above where
we aren't listing them.

>  
>  	  This driver can also be built as a module. If so, the module
>  	  will be called mcp9600.
>

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

[Jonathan Cameron]

On Fri, 15 Aug 2025 16:46:02 +0000
Ben Collins <bcollins@watter.com> wrote:

> From: Ben Collins <bcollins@kernel.org>

See submitting patches documentation.  Should have a version number in the [] in the patch title.

Jonathan

> 
> ChangeLog:
> 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
>     * FIELD_PREP -> FIELD_MODIFY
>     * Remove explicit setting of 0 value in filter_level
>   - Based on feedback from David Lechner <dlechner@baylibre.com>
>     * Rework IIR values exposed to sysfs. Using the ratios, there was no
>       way to represent "disabled" (i.e. infinity). Based on the bmp280
>       driver I went with using the power coefficients (e.g. 1, 2, 4, 8,
>       ...) where 1 is disabled (n=0).
> 
> v1 -> v2:
>   - Break into individual patches
> 
> v1:
>   - Initial patch to enable IIR and thermocouple-type
>   - Recognize mcp9601
> 
> Ben Collins (5):
>   dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601
>   iio: mcp9600: White space cleanup for tab alignment
>   iio: mcp9600: Recognize chip id for mcp9601
>   iio: mcp9600: Add support for thermocouple-type
>   iio: mcp9600: Add support for IIR filter
> 
>  .../iio/temperature/microchip,mcp9600.yaml    |   6 +-
>  drivers/iio/temperature/Kconfig               |   8 +-
>  drivers/iio/temperature/mcp9600.c             | 209 ++++++++++++++++--
>  3 files changed, 201 insertions(+), 22 deletions(-)
> 

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

[Jonathan Cameron]

On Fri, 15 Aug 2025 16:46:06 +0000
Ben Collins <bcollins@watter.com> wrote:

> 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>
Hi Ben,

Just one trivial thing inline.

Thanks

Jonathan

> ---
>  drivers/iio/temperature/mcp9600.c | 69 +++++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
> 
> diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
> index c18ae17b6d82f..361572a241f06 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 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,
> @@ -89,6 +117,7 @@ static const struct iio_event_spec mcp9600_events[] = {
>  			.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,			       \
> @@ -131,6 +160,7 @@ struct mcp_chip_info {
>  
>  struct mcp9600_data {
>  	struct i2c_client *client;
> +	u32 thermocouple_type;
>  };
>  
>  static int mcp9600_read(struct mcp9600_data *data,
> @@ -165,11 +195,32 @@ 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;
>  	}
>  }
>  
> +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");

Only called from probe so use return dev_err_probe() here


> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  static int mcp9600_get_alert_index(int channel2, enum iio_event_direction dir)
>  {
>  	if (channel2 == IIO_MOD_TEMP_AMBIENT) {
> @@ -453,6 +504,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(&client->dev, "thermocouple-type",
> +				       &data->thermocouple_type);
> +	if (ret < 0 && ret != -EINVAL)
> +		return dev_err_probe(&client->dev, ret,
> +				     "Error reading thermocouple-type property\n");
> +
> +	if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map))
> +		return dev_err_probe(&client->dev, -EINVAL,
> +				     "Invalid thermocouple-type property %u.\n",
> +				     data->thermocouple_type);
> +
> +	/* Set initial config. */
> +	ret = mcp9600_config(data);
> +	if (ret < 0)
> +		return ret;
> +
>  	ch_sel = mcp9600_probe_alerts(indio_dev);
>  	if (ch_sel < 0)
>  		return ch_sel;

Re: [PATCH 5/5] iio: mcp9600: Add support for IIR filter

[Jonathan Cameron]

On Fri, 15 Aug 2025 16:46:07 +0000
Ben Collins <bcollins@watter.com> wrote:

> MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
> to allow get/set of this value.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>

This needs a lot more description given these should be frequencies.
We identified recently that some other drivers have this wrong but
we should be looking to fix those if possible, not replicate it.

The infinite value does need some more discussion. Lets carry that
on in the v2 thread.

Jonathan


> ---
>  drivers/iio/temperature/mcp9600.c | 73 +++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
> index 361572a241f06..896520ddf6d3c 100644
> --- a/drivers/iio/temperature/mcp9600.c
> +++ b/drivers/iio/temperature/mcp9600.c
> @@ -31,6 +31,7 @@
>  #define MCP9600_STATUS_ALERT(x)		BIT(x)
>  #define MCP9600_SENSOR_CFG		0x05
>  #define MCP9600_SENSOR_TYPE_MASK	GENMASK(6, 4)
> +#define MCP9600_FILTER_MASK		GENMASK(2, 0)
>  #define MCP9600_ALERT_CFG1		0x08
>  #define MCP9600_ALERT_CFG(x)		(MCP9600_ALERT_CFG1 + (x - 1))
>  #define MCP9600_ALERT_CFG_ENABLE	BIT(0)
> @@ -94,6 +95,10 @@ static const int mcp9600_tc_types[] = {
>  	[THERMOCOUPLE_TYPE_R] = 'R',
>  };
>  
> +static const int mcp_iir_coefficients_avail[] = {
> +	1, 2, 4, 8, 16, 32, 64, 128,
> +};
> +
>  static const struct iio_event_spec mcp9600_events[] = {
>  	{
>  		.type = IIO_EV_TYPE_THRESH,
> @@ -118,7 +123,10 @@ static const struct iio_event_spec mcp9600_events[] = {
>  			.address = MCP9600_HOT_JUNCTION,		       \
>  			.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	       \
>  					      BIT(IIO_CHAN_INFO_THERMOCOUPLE_TYPE) | \
> +					      BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) |  \
>  					      BIT(IIO_CHAN_INFO_SCALE),	       \
> +			.info_mask_separate_available =                        \
> +					      BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY), \
>  			.event_spec = &mcp9600_events[hj_ev_spec_off],	       \
>  			.num_event_specs = hj_num_ev,			       \
>  		},							       \
> @@ -161,6 +169,7 @@ struct mcp_chip_info {
>  struct mcp9600_data {
>  	struct i2c_client *client;
>  	u32 thermocouple_type;
> +	u8 filter_level; /* Chip default is 0 */
>  };
>  
>  static int mcp9600_read(struct mcp9600_data *data,
> @@ -191,13 +200,36 @@ static int mcp9600_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  		return IIO_VAL_INT;
> +
>  	case IIO_CHAN_INFO_SCALE:
>  		*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;
> +
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		*val = mcp_iir_coefficients_avail[data->filter_level];
> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int mcp9600_read_avail(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      const int **vals, int *type, int *length,
> +			      long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		*vals = mcp_iir_coefficients_avail;
> +		*type = IIO_VAL_INT;
> +		*length = ARRAY_SIZE(mcp_iir_coefficients_avail);
> +		return IIO_AVAIL_LIST;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -211,6 +243,7 @@ static int mcp9600_config(struct mcp9600_data *data)
>  
>  	cfg  = FIELD_PREP(MCP9600_SENSOR_TYPE_MASK,
>  			  mcp9600_type_map[data->thermocouple_type]);
> +	FIELD_MODIFY(MCP9600_FILTER_MASK, &cfg, data->filter_level);
>  
>  	ret = i2c_smbus_write_byte_data(client, MCP9600_SENSOR_CFG, cfg);
>  	if (ret < 0) {
> @@ -221,6 +254,43 @@ static int mcp9600_config(struct mcp9600_data *data)
>  	return 0;
>  }
>  
> +static int mcp9600_write_raw_get_fmt(struct iio_dev *indio_dev,
> +				     struct iio_chan_spec const *chan,
> +				     long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int mcp9600_write_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 i;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		for (i = 0; i < ARRAY_SIZE(mcp_iir_coefficients_avail); i++) {
> +			if (mcp_iir_coefficients_avail[i] == val)
> +				break;
> +		}
> +
> +		if (i == ARRAY_SIZE(mcp_iir_coefficients_avail))
> +			return -EINVAL;
> +
> +		data->filter_level = i;
> +		return mcp9600_config(data);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int mcp9600_get_alert_index(int channel2, enum iio_event_direction dir)
>  {
>  	if (channel2 == IIO_MOD_TEMP_AMBIENT) {
> @@ -358,6 +428,9 @@ static int mcp9600_write_thresh(struct iio_dev *indio_dev,
>  
>  static const struct iio_info mcp9600_info = {
>  	.read_raw = mcp9600_read_raw,
> +	.read_avail = mcp9600_read_avail,
> +	.write_raw = mcp9600_write_raw,
> +	.write_raw_get_fmt = mcp9600_write_raw_get_fmt,
>  	.read_event_config = mcp9600_read_event_config,
>  	.write_event_config = mcp9600_write_event_config,
>  	.read_event_value = mcp9600_read_thresh,

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

[Ben Collins]

> On Aug 16, 2025, at 6:11 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> On Fri, 15 Aug 2025 16:46:06 +0000
> Ben Collins <bcollins@watter.com> wrote:
> 
>> 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>
> Hi Ben,
> 
> Just one trivial thing inline.
...
>> 
>> +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");
> 
> Only called from probe so use return dev_err_probe() here

Hi Johnathan. That’s correct in this patch. However, in the IIR patch
I call this when the filter is set, so it didn’t seem to make sense to
do it that way, just to change it in the next patch.

I appreciate all the feedback. I’ll get things cleaned in for v4.

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

[Jonathan Cameron]

On Sat, 16 Aug 2025 09:18:09 -0400
Ben Collins <bcollins@watter.com> wrote:

> > On Aug 16, 2025, at 6:11 AM, Jonathan Cameron <jic23@kernel.org> wrote:
> > 
> > On Fri, 15 Aug 2025 16:46:06 +0000
> > Ben Collins <bcollins@watter.com> wrote:
> >   
> >> 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>  
> > Hi Ben,
> > 
> > Just one trivial thing inline.  
> ...
> >> 
> >> +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");  
> > 
> > Only called from probe so use return dev_err_probe() here  
> 
> Hi Johnathan. That’s correct in this patch. However, in the IIR patch
> I call this when the filter is set, so it didn’t seem to make sense to
> do it that way, just to change it in the next patch.
> 
> I appreciate all the feedback. I’ll get things cleaned in for v4.
Ah. Fair enough then. Leave it as it is and thanks for pointing this out.

Jonathan

Re: [PATCH 5/5] iio: mcp9600: Add support for IIR filter

[David Lechner]

On 8/15/25 11:46 AM, Ben Collins wrote:
> MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
> to allow get/set of this value.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---

...

>  static int mcp9600_read(struct mcp9600_data *data,
> @@ -191,13 +200,36 @@ static int mcp9600_read_raw(struct iio_dev *indio_dev,
>  		if (ret)
>  			return ret;
>  		return IIO_VAL_INT;
> +
>  	case IIO_CHAN_INFO_SCALE:
>  		*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;
> +
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		*val = mcp_iir_coefficients_avail[data->filter_level];

We already calculated the correct values, so we should be using
those instead of made-up values.

And the suggestion of using filter_type takes care of the
turning the filter off so we don't need an "infinity" value
here.

That does bring up the question though, if the filter is off,
should this attribute return an error in that case?

> +		return IIO_VAL_INT;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}

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

[David Lechner]

On 8/15/25 11:46 AM, Ben Collins wrote:
> 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>
> ---

...

> @@ -453,6 +504,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. */

We still also need a dt-bindings patch to specify the default there as well.

> +	data->thermocouple_type = THERMOCOUPLE_TYPE_K;
> +	ret = device_property_read_u32(&client->dev, "thermocouple-type",
> +				       &data->thermocouple_type);
> +	if (ret < 0 && ret != -EINVAL)
> +		return dev_err_probe(&client->dev, ret,
> +				     "Error reading thermocouple-type property\n");
> +
> +	if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map))
> +		return dev_err_probe(&client->dev, -EINVAL,
> +				     "Invalid thermocouple-type property %u.\n",
> +				     data->thermocouple_type);
> +
> +	/* Set initial config. */
> +	ret = mcp9600_config(data);
> +	if (ret < 0)
> +		return ret;
> +
>  	ch_sel = mcp9600_probe_alerts(indio_dev);
>  	if (ch_sel < 0)
>  		return ch_sel;

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[David Lechner]

On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> On Fri, 15 Aug 2025 16:46:03 +0000
> Ben Collins <bcollins@watter.com> wrote:
> 
>> The mcp9600 driver supports the mcp9601 chip, but complains about not
>> recognizing the device id on probe. A separate patch...
>>
>> 	iio: mcp9600: Recognize chip id for mcp9601
>>
>> ...addresses this. This patch updates the dt-bindings for this chip to
>> reflect the change to allow explicitly setting microchip,mcp9601 as
>> the expected chip type.
>>
>> The mcp9601 also supports features not found on the mcp9600, so this
>> will also allow the driver to differentiate the support of these
>> features.
> 
> If it's additional features only then you can still use a fallback
> compatible.  Intent being that a new DT vs old kernel still 'works'.
> 
> Then for the driver on new kernels we match on the new compatible and
> support those new features.  Old kernel users get to keep the ID
> mismatch warning - they can upgrade if they want that to go away ;)
> 
> Krzysztof raised the same point on v2 but I'm not seeing it addressed
> in that discussion.

One could make the argument that these are not entirely fallback
compatible since bit 4 of the STATUS register has a different
meaning depending on if the chip is MCP9601/L01/RL01 or not.

Interestingly, the existing bindings include interrupts for
open circuit and short circuit alert pins. But these pins
also only exist on MCP9601/L01/RL01. If we decide these aren't
fallback compatible, then those properties should have the
proper constraints added as well.

> 
> Jonathan
> 
>>
>> Signed-off-by: Ben Collins <bcollins@watter.com>
>> ---
>>  .../bindings/iio/temperature/microchip,mcp9600.yaml         | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml
>> index d2cafa38a5442..d8af0912ce886 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,9 @@ description:
>>  
>>  properties:
>>    compatible:
>> -    const: microchip,mcp9600
>> +    enum:
>> +      - microchip,mcp9600
>> +      - microchip,mcp9601
>>  
>>    reg:
>>      maxItems: 1
> 

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

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

On Sat, Aug 16, 2025 at 01:24:24PM -0500, David Lechner wrote:
> On 8/15/25 11:46 AM, Ben Collins wrote:
> > 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>
> > ---
> 
> ...
> 
> > @@ -453,6 +504,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. */
> 
> We still also need a dt-bindings patch to specify the default there as well.

The existing bindings file for this already states type-k is the
default. Is there something else it needs?

> > +	data->thermocouple_type = THERMOCOUPLE_TYPE_K;
> > +	ret = device_property_read_u32(&client->dev, "thermocouple-type",
> > +				       &data->thermocouple_type);
> > +	if (ret < 0 && ret != -EINVAL)
> > +		return dev_err_probe(&client->dev, ret,
> > +				     "Error reading thermocouple-type property\n");
> > +
> > +	if (data->thermocouple_type >= ARRAY_SIZE(mcp9600_type_map))
> > +		return dev_err_probe(&client->dev, -EINVAL,
> > +				     "Invalid thermocouple-type property %u.\n",
> > +				     data->thermocouple_type);
> > +
> > +	/* Set initial config. */
> > +	ret = mcp9600_config(data);
> > +	if (ret < 0)
> > +		return ret;
> > +
> >  	ch_sel = mcp9600_probe_alerts(indio_dev);
> >  	if (ch_sel < 0)
> >  		return ch_sel;
> 
> 

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[David Lechner]

On 8/16/25 9:54 PM, Ben Collins wrote:
> On Sat, Aug 16, 2025 at 01:24:24PM -0500, David Lechner wrote:
>> On 8/15/25 11:46 AM, Ben Collins wrote:
>>> 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>
>>> ---
>>
>> ...
>>
>>> @@ -453,6 +504,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. */
>>
>> We still also need a dt-bindings patch to specify the default there as well.
> 
> The existing bindings file for this already states type-k is the
> default. Is there something else it needs?
> 
  default: 3

in the YAML.

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> > On Fri, 15 Aug 2025 16:46:03 +0000
> > Ben Collins <bcollins@watter.com> wrote:
> > 
> >> The mcp9600 driver supports the mcp9601 chip, but complains about not
> >> recognizing the device id on probe. A separate patch...
> >>
> >> 	iio: mcp9600: Recognize chip id for mcp9601
> >>
> >> ...addresses this. This patch updates the dt-bindings for this chip to
> >> reflect the change to allow explicitly setting microchip,mcp9601 as
> >> the expected chip type.
> >>
> >> The mcp9601 also supports features not found on the mcp9600, so this
> >> will also allow the driver to differentiate the support of these
> >> features.
> > 
> > If it's additional features only then you can still use a fallback
> > compatible.  Intent being that a new DT vs old kernel still 'works'.
> > 
> > Then for the driver on new kernels we match on the new compatible and
> > support those new features.  Old kernel users get to keep the ID
> > mismatch warning - they can upgrade if they want that to go away ;)
> > 
> > Krzysztof raised the same point on v2 but I'm not seeing it addressed
> > in that discussion.
> 
> One could make the argument that these are not entirely fallback
> compatible since bit 4 of the STATUS register has a different
> meaning depending on if the chip is MCP9601/L01/RL01 or not.

There are some nuances to this register between the two, but it can be
used generically as "not in range" for both.

My understanding from the docs is if VSENSE is connected on mcp9601,
then it is explicitly open-circuit detection vs. short-circuit, which
is bit 5.

> Interestingly, the existing bindings include interrupts for
> open circuit and short circuit alert pins. But these pins
> also only exist on MCP9601/L01/RL01. If we decide these aren't
> fallback compatible, then those properties should have the
> proper constraints added as well.

In my v4 patch, I'm going to remove the short/open circuit interrupts
since they are not implemented, yet.

I have VSENSE wired on my board so I can work on those interrupts and
register support in a later patch series.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[David Lechner]

On 8/17/25 11:37 AM, Ben Collins wrote:
> On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
>> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
>>> On Fri, 15 Aug 2025 16:46:03 +0000
>>> Ben Collins <bcollins@watter.com> wrote:
>>>
>>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
>>>> recognizing the device id on probe. A separate patch...
>>>>
>>>> 	iio: mcp9600: Recognize chip id for mcp9601
>>>>
>>>> ...addresses this. This patch updates the dt-bindings for this chip to
>>>> reflect the change to allow explicitly setting microchip,mcp9601 as
>>>> the expected chip type.
>>>>
>>>> The mcp9601 also supports features not found on the mcp9600, so this
>>>> will also allow the driver to differentiate the support of these
>>>> features.
>>>
>>> If it's additional features only then you can still use a fallback
>>> compatible.  Intent being that a new DT vs old kernel still 'works'.
>>>
>>> Then for the driver on new kernels we match on the new compatible and
>>> support those new features.  Old kernel users get to keep the ID
>>> mismatch warning - they can upgrade if they want that to go away ;)
>>>
>>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
>>> in that discussion.
>>
>> One could make the argument that these are not entirely fallback
>> compatible since bit 4 of the STATUS register has a different
>> meaning depending on if the chip is MCP9601/L01/RL01 or not.
> 
> There are some nuances to this register between the two, but it can be
> used generically as "not in range" for both.
> 
> My understanding from the docs is if VSENSE is connected on mcp9601,
> then it is explicitly open-circuit detection vs. short-circuit, which
> is bit 5.
> 
>> Interestingly, the existing bindings include interrupts for
>> open circuit and short circuit alert pins. But these pins
>> also only exist on MCP9601/L01/RL01. If we decide these aren't
>> fallback compatible, then those properties should have the
>> proper constraints added as well.
> 
> In my v4 patch, I'm going to remove the short/open circuit interrupts
> since they are not implemented, yet.

Don't remove them from the devicetree bindings. Even if the Linux driver
doesn't use it, the bindings should be as complete as possible.

https://docs.kernel.org/devicetree/bindings/writing-bindings.html

> 
> I have VSENSE wired on my board so I can work on those interrupts and
> register support in a later patch series.
> 

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 3330 bytes --]

On Sun, Aug 17, 2025 at 11:51:22AM -0500, David Lechner wrote:
> On 8/17/25 11:37 AM, Ben Collins wrote:
> > On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
> >> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> >>> On Fri, 15 Aug 2025 16:46:03 +0000
> >>> Ben Collins <bcollins@watter.com> wrote:
> >>>
> >>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
> >>>> recognizing the device id on probe. A separate patch...
> >>>>
> >>>> 	iio: mcp9600: Recognize chip id for mcp9601
> >>>>
> >>>> ...addresses this. This patch updates the dt-bindings for this chip to
> >>>> reflect the change to allow explicitly setting microchip,mcp9601 as
> >>>> the expected chip type.
> >>>>
> >>>> The mcp9601 also supports features not found on the mcp9600, so this
> >>>> will also allow the driver to differentiate the support of these
> >>>> features.
> >>>
> >>> If it's additional features only then you can still use a fallback
> >>> compatible.  Intent being that a new DT vs old kernel still 'works'.
> >>>
> >>> Then for the driver on new kernels we match on the new compatible and
> >>> support those new features.  Old kernel users get to keep the ID
> >>> mismatch warning - they can upgrade if they want that to go away ;)
> >>>
> >>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
> >>> in that discussion.
> >>
> >> One could make the argument that these are not entirely fallback
> >> compatible since bit 4 of the STATUS register has a different
> >> meaning depending on if the chip is MCP9601/L01/RL01 or not.
> > 
> > There are some nuances to this register between the two, but it can be
> > used generically as "not in range" for both.
> > 
> > My understanding from the docs is if VSENSE is connected on mcp9601,
> > then it is explicitly open-circuit detection vs. short-circuit, which
> > is bit 5.
> > 
> >> Interestingly, the existing bindings include interrupts for
> >> open circuit and short circuit alert pins. But these pins
> >> also only exist on MCP9601/L01/RL01. If we decide these aren't
> >> fallback compatible, then those properties should have the
> >> proper constraints added as well.
> > 
> > In my v4 patch, I'm going to remove the short/open circuit interrupts
> > since they are not implemented, yet.
> 
> Don't remove them from the devicetree bindings. Even if the Linux driver
> doesn't use it, the bindings should be as complete as possible.
> 
> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
> 

I couldn't find anything that would easily describe this type of layout:

properties:
...
  interrupts:
    minItems: 1
    maxItems: 4
  interrupt-names:
    minItems: 1
    items:
      - const: alert1
      - const: alert2
      - const: alert3
      - const: alert4

allOf:
  - if:
      properties:
        compatible:
          contains:
            const: microchip,mcp9601
    then:
      # Override maxItems
      interrupts:
        maxItems: 6
      # XXX Add items to existing list???
      interrupt-names:
        items:
          - const: open-circuit
          - const: short-circuit

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[David Lechner]

On 8/17/25 12:34 PM, Ben Collins wrote:
> On Sun, Aug 17, 2025 at 11:51:22AM -0500, David Lechner wrote:
>> On 8/17/25 11:37 AM, Ben Collins wrote:
>>> On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
>>>> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
>>>>> On Fri, 15 Aug 2025 16:46:03 +0000
>>>>> Ben Collins <bcollins@watter.com> wrote:
>>>>>
>>>>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
>>>>>> recognizing the device id on probe. A separate patch...
>>>>>>
>>>>>> 	iio: mcp9600: Recognize chip id for mcp9601
>>>>>>
>>>>>> ...addresses this. This patch updates the dt-bindings for this chip to
>>>>>> reflect the change to allow explicitly setting microchip,mcp9601 as
>>>>>> the expected chip type.
>>>>>>
>>>>>> The mcp9601 also supports features not found on the mcp9600, so this
>>>>>> will also allow the driver to differentiate the support of these
>>>>>> features.
>>>>>
>>>>> If it's additional features only then you can still use a fallback
>>>>> compatible.  Intent being that a new DT vs old kernel still 'works'.
>>>>>
>>>>> Then for the driver on new kernels we match on the new compatible and
>>>>> support those new features.  Old kernel users get to keep the ID
>>>>> mismatch warning - they can upgrade if they want that to go away ;)
>>>>>
>>>>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
>>>>> in that discussion.
>>>>
>>>> One could make the argument that these are not entirely fallback
>>>> compatible since bit 4 of the STATUS register has a different
>>>> meaning depending on if the chip is MCP9601/L01/RL01 or not.
>>>
>>> There are some nuances to this register between the two, but it can be
>>> used generically as "not in range" for both.
>>>
>>> My understanding from the docs is if VSENSE is connected on mcp9601,
>>> then it is explicitly open-circuit detection vs. short-circuit, which
>>> is bit 5.
>>>
>>>> Interestingly, the existing bindings include interrupts for
>>>> open circuit and short circuit alert pins. But these pins
>>>> also only exist on MCP9601/L01/RL01. If we decide these aren't
>>>> fallback compatible, then those properties should have the
>>>> proper constraints added as well.
>>>
>>> In my v4 patch, I'm going to remove the short/open circuit interrupts
>>> since they are not implemented, yet.
>>
>> Don't remove them from the devicetree bindings. Even if the Linux driver
>> doesn't use it, the bindings should be as complete as possible.
>>
>> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
>>
> 
> I couldn't find anything that would easily describe this type of layout:
> 
> properties:
> ...
>   interrupts:
>     minItems: 1
>     maxItems: 4
>   interrupt-names:
>     minItems: 1
>     items:
>       - const: alert1
>       - const: alert2
>       - const: alert3
>       - const: alert4
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             const: microchip,mcp9601
>     then:
>       # Override maxItems
>       interrupts:
>         maxItems: 6
>       # XXX Add items to existing list???
>       interrupt-names:
>         items:
>           - const: open-circuit
>           - const: short-circuit
> 

We usually do this the other way around. The base binding lists
all of the possibilities then an -if: constraint limits them
if needed.


So don't change what is there already and then add:


allOf:
  - if:
      properties:
        compatible:
          not:
            contains:
              const: microchip,mcp9601
    then:
      properties:
        interrupts:
          maxItems: 4
        interrupt-names:
          maxItems: 4
          enum:
            - alert1
            - alert2
            - alert3
            - alert4

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 5768 bytes --]

On Sun, Aug 17, 2025 at 12:59:48PM -0500, David Lechner wrote:
> On 8/17/25 12:34 PM, Ben Collins wrote:
> > On Sun, Aug 17, 2025 at 11:51:22AM -0500, David Lechner wrote:
> >> On 8/17/25 11:37 AM, Ben Collins wrote:
> >>> On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
> >>>> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> >>>>> On Fri, 15 Aug 2025 16:46:03 +0000
> >>>>> Ben Collins <bcollins@watter.com> wrote:
> >>>>>
> >>>>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
> >>>>>> recognizing the device id on probe. A separate patch...
> >>>>>>
> >>>>>> 	iio: mcp9600: Recognize chip id for mcp9601
> >>>>>>
> >>>>>> ...addresses this. This patch updates the dt-bindings for this chip to
> >>>>>> reflect the change to allow explicitly setting microchip,mcp9601 as
> >>>>>> the expected chip type.
> >>>>>>
> >>>>>> The mcp9601 also supports features not found on the mcp9600, so this
> >>>>>> will also allow the driver to differentiate the support of these
> >>>>>> features.
> >>>>>
> >>>>> If it's additional features only then you can still use a fallback
> >>>>> compatible.  Intent being that a new DT vs old kernel still 'works'.
> >>>>>
> >>>>> Then for the driver on new kernels we match on the new compatible and
> >>>>> support those new features.  Old kernel users get to keep the ID
> >>>>> mismatch warning - they can upgrade if they want that to go away ;)
> >>>>>
> >>>>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
> >>>>> in that discussion.
> >>>>
> >>>> One could make the argument that these are not entirely fallback
> >>>> compatible since bit 4 of the STATUS register has a different
> >>>> meaning depending on if the chip is MCP9601/L01/RL01 or not.
> >>>
> >>> There are some nuances to this register between the two, but it can be
> >>> used generically as "not in range" for both.
> >>>
> >>> My understanding from the docs is if VSENSE is connected on mcp9601,
> >>> then it is explicitly open-circuit detection vs. short-circuit, which
> >>> is bit 5.
> >>>
> >>>> Interestingly, the existing bindings include interrupts for
> >>>> open circuit and short circuit alert pins. But these pins
> >>>> also only exist on MCP9601/L01/RL01. If we decide these aren't
> >>>> fallback compatible, then those properties should have the
> >>>> proper constraints added as well.
> >>>
> >>> In my v4 patch, I'm going to remove the short/open circuit interrupts
> >>> since they are not implemented, yet.
> >>
> >> Don't remove them from the devicetree bindings. Even if the Linux driver
> >> doesn't use it, the bindings should be as complete as possible.
> >>
> >> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
> >>
> > 
> > I couldn't find anything that would easily describe this type of layout:
> > 
> > properties:
> > ...
> >   interrupts:
> >     minItems: 1
> >     maxItems: 4
> >   interrupt-names:
> >     minItems: 1
> >     items:
> >       - const: alert1
> >       - const: alert2
> >       - const: alert3
> >       - const: alert4
> > 
> > allOf:
> >   - if:
> >       properties:
> >         compatible:
> >           contains:
> >             const: microchip,mcp9601
> >     then:
> >       # Override maxItems
> >       interrupts:
> >         maxItems: 6
> >       # XXX Add items to existing list???
> >       interrupt-names:
> >         items:
> >           - const: open-circuit
> >           - const: short-circuit
> > 
> 
> We usually do this the other way around. The base binding lists
> all of the possibilities then an -if: constraint limits them
> if needed.
> 
> 
> So don't change what is there already and then add:
> 
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           not:
>             contains:
>               const: microchip,mcp9601
>     then:
>       properties:
>         interrupts:
>           maxItems: 4
>         interrupt-names:
>           maxItems: 4
>           enum:
>             - alert1
>             - alert2
>             - alert3
>             - alert4

This might be a little more complicated. I want to add a boolean for
microchip,vsense so the SC/OC aren't even available without that flag
being true (default false).

I could just assume that having the interrupts means this flag is true,
but that doesn't cover the case where the interrupts might not be used
or even wired up, but the SC/OC detection in the status register can be
used.

I was going with this:

  interrupts:
    minItems: 1
    maxItems: 4

  interrupt-names:
    minItems: 1
    items:
      - const: alert1
      - const: alert2
      - const: alert3
      - const: alert4

  microchip,vsense:
    default: false
    description:
      This flag indicates that the chip has been wired with VSENSE to
      enable open and short circuit detect. By default, this is false,
      since there's no way to detect that the chip is wired correctly.
    type: boolean

...

allOf:
  - if:
      properties:
        # XXX Does this work like logical AND? Passes dt_binding_check
        microchip,vsense: true
        compatible:
          contains:
            const: microchip,mcp9601
    then:
      properties:
        interrupts:
          minItems: 1
          maxItems: 6
        interrupt-names:
          items:
            - const: alert1
            - const: alert2
            - const: alert3
            - const: alert4
            - const: open-circuit
            - const: short-circuit
-- 
 Ben Collins
 https://libjwt.io
 https://github.com/benmcollins
 --
 3EC9 7598 1672 961A 1139  173A 5D5A 57C7 242B 22CF

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Ben Collins]

[-- Attachment #1: Type: text/plain, Size: 3919 bytes --]

On Sun, Aug 17, 2025 at 05:02:49PM -0500, Ben Collins wrote:
> On Sun, Aug 17, 2025 at 12:59:48PM -0500, David Lechner wrote:
> > On 8/17/25 12:34 PM, Ben Collins wrote:
> > > On Sun, Aug 17, 2025 at 11:51:22AM -0500, David Lechner wrote:
> > >> On 8/17/25 11:37 AM, Ben Collins wrote:
> > >>> On Sat, Aug 16, 2025 at 01:55:31PM -0500, David Lechner wrote:
> > >>>> On 8/16/25 4:58 AM, Jonathan Cameron wrote:
> > >>>>> On Fri, 15 Aug 2025 16:46:03 +0000
> > >>>>> Ben Collins <bcollins@watter.com> wrote:
> > >>>>>
> > >>>>>> The mcp9600 driver supports the mcp9601 chip, but complains about not
> > >>>>>> recognizing the device id on probe. A separate patch...
> > >>>>>>
> > >>>>>> 	iio: mcp9600: Recognize chip id for mcp9601
> > >>>>>>
> > >>>>>> ...addresses this. This patch updates the dt-bindings for this chip to
> > >>>>>> reflect the change to allow explicitly setting microchip,mcp9601 as
> > >>>>>> the expected chip type.
> > >>>>>>
> > >>>>>> The mcp9601 also supports features not found on the mcp9600, so this
> > >>>>>> will also allow the driver to differentiate the support of these
> > >>>>>> features.
> > >>>>>
> > >>>>> If it's additional features only then you can still use a fallback
> > >>>>> compatible.  Intent being that a new DT vs old kernel still 'works'.
> > >>>>>
> > >>>>> Then for the driver on new kernels we match on the new compatible and
> > >>>>> support those new features.  Old kernel users get to keep the ID
> > >>>>> mismatch warning - they can upgrade if they want that to go away ;)
> > >>>>>
> > >>>>> Krzysztof raised the same point on v2 but I'm not seeing it addressed
> > >>>>> in that discussion.
> > >>>>
> > >>>> One could make the argument that these are not entirely fallback
> > >>>> compatible since bit 4 of the STATUS register has a different
> > >>>> meaning depending on if the chip is MCP9601/L01/RL01 or not.
> > >>>
> > >>> There are some nuances to this register between the two, but it can be
> > >>> used generically as "not in range" for both.
> > >>>
> > >>> My understanding from the docs is if VSENSE is connected on mcp9601,
> > >>> then it is explicitly open-circuit detection vs. short-circuit, which
> > >>> is bit 5.
> > >>>
> > >>>> Interestingly, the existing bindings include interrupts for
> > >>>> open circuit and short circuit alert pins. But these pins
> > >>>> also only exist on MCP9601/L01/RL01. If we decide these aren't
> > >>>> fallback compatible, then those properties should have the
> > >>>> proper constraints added as well.
> > >>>
> > >>> In my v4 patch, I'm going to remove the short/open circuit interrupts
> > >>> since they are not implemented, yet.
> > >>
> > >> Don't remove them from the devicetree bindings. Even if the Linux driver
> > >> doesn't use it, the bindings should be as complete as possible.
> > >>
> > >> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
> > >>
> > > 
> > > I couldn't find anything that would easily describe this type of layout:
...
> > We usually do this the other way around. The base binding lists
> > all of the possibilities then an -if: constraint limits them
> > if needed.
> > 
> > 
> > So don't change what is there already and then add:
> > 
...
> This might be a little more complicated. I want to add a boolean for
> microchip,vsense so the SC/OC aren't even available without that flag
> being true (default false).
> 
> I could just assume that having the interrupts means this flag is true,
> but that doesn't cover the case where the interrupts might not be used
> or even wired up, but the SC/OC detection in the status register can be
> used.
> 
> I was going with this:
> 

Nevermind, I figured this out. I'll send v4 soon.


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/5] dt-bindings: iio: mcp9600: Add compatible for microchip,mcp9601

[Krzysztof Kozlowski]

On 17/08/2025 23:10, Ben Collins wrote:
>>>>>
>>>>
>>>> I couldn't find anything that would easily describe this type of layout:
> ...
>>> We usually do this the other way around. The base binding lists
>>> all of the possibilities then an -if: constraint limits them
>>> if needed.
>>>
>>>
>>> So don't change what is there already and then add:
>>>
> ...
>> This might be a little more complicated. I want to add a boolean for
>> microchip,vsense so the SC/OC aren't even available without that flag
>> being true (default false).
>>
>> I could just assume that having the interrupts means this flag is true,
>> but that doesn't cover the case where the interrupts might not be used
>> or even wired up, but the SC/OC detection in the status register can be
>> used.
>>
>> I was going with this:
>>
> 
> Nevermind, I figured this out. I'll send v4 soon.

You received from David correct code, good idea... yet you ignored it
and sent something incorrect - breaking ABI.
Best regards,
Krzysztof

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

[Nathan Chancellor]

Hi Jonathan,

On Sat, Aug 16, 2025 at 11:02:43AM +0100, Jonathan Cameron wrote:
> On Sat, 16 Aug 2025 16:46:12 +0800
> kernel test robot <lkp@intel.com> wrote:
> 
> > Hi Ben,
> > 
> > kernel test robot noticed the following build warnings:
> > 
> > [auto build test WARNING on jic23-iio/togreg]
> > [also build test WARNING on linus/master v6.17-rc1 next-20250815]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Ben-Collins/dt-bindings-iio-mcp9600-Add-compatible-for-microchip-mcp9601/20250816-005705
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> > patch link:    https://lore.kernel.org/r/20250815164627.22002-4-bcollins%40watter.com
> > patch subject: [PATCH 3/5] iio: mcp9600: Recognize chip id for mcp9601
> > config: riscv-randconfig-001-20250816 (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/config)
> > compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/reproduce)
> > 
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202508161646.PDl6V4EU-lkp@intel.com/
> > 
> > All warnings (new ones prefixed by >>):

<trim unrelated -Wnull-pointer-arithmetic>

> > >> drivers/iio/temperature/mcp9600.c:440:53: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]  
> >      440 |                                 "Expected id %02x, but device responded with %02\n",
> >          |                                                                              ~~~^
> >    include/linux/dev_printk.h:156:62: note: expanded from macro 'dev_warn'
> >      156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
> >          |                                                                     ^~~
> >    include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
> >       19 | #define dev_fmt(fmt) fmt
> >          |                      ^~~
> >    include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
> >      110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
> >          |                              ^~~
> > >> drivers/iio/temperature/mcp9600.c:441:26: warning: data argument not used by format string [-Wformat-extra-args]  
> >      440 |                                 "Expected id %02x, but device responded with %02\n",
> >          |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >      441 |                                  chip_info->chip_id, dev_id);
> >          |                                                      ^
> >    include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn'
> >      156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
> >          |                                                                     ~~~     ^
> >    include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> >      110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
> >          |                              ~~~    ^
> >    drivers/iio/temperature/mcp9600.c:428:22: warning: unused variable 'ret' [-Wunused-variable]
> >      428 |         int ch_sel, dev_id, ret;
> >          |                             ^~~
> >    10 warnings generated.
> > 
> > 
> > vim +/x0a +440 drivers/iio/temperature/mcp9600.c
> > 
> >    422	
> >    423	static int mcp9600_probe(struct i2c_client *client)
> >    424	{
> >    425		const struct mcp_chip_info *chip_info = i2c_get_match_data(client);
> 
> Probably a false positive as I don't think we can probe without something matching and hence
> that not being NULL but an error check on that match is still a nice to have and should
> resolve this build warning.  Note there is very little chance a compiler could ever figure
> out if this can be NULL or not so it's a reasonable warning!

I am not sure I follow if you are referring to the -Wformat warnings
above. Isn't it pointing out that the second specifier is missing the
actual type? Shouldn't it be '%02x' or something of the sort?

Cheers,
Nathan

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

[Ben Collins]

> On Aug 18, 2025, at 11:06 AM, Nathan Chancellor <nathan@kernel.org> wrote:
> 
> Hi Jonathan,
> 
> On Sat, Aug 16, 2025 at 11:02:43AM +0100, Jonathan Cameron wrote:
>> On Sat, 16 Aug 2025 16:46:12 +0800
>> kernel test robot <lkp@intel.com> wrote:
>> 
>>> Hi Ben,
>>> 
>>> kernel test robot noticed the following build warnings:
>>> 
>>> [auto build test WARNING on jic23-iio/togreg]
>>> [also build test WARNING on linus/master v6.17-rc1 next-20250815]
>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>> And when submitting patch, we suggest to use '--base' as documented in
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>> 
>>> url:    https://github.com/intel-lab-lkp/linux/commits/Ben-Collins/dt-bindings-iio-mcp9600-Add-compatible-for-microchip-mcp9601/20250816-005705
>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
>>> patch link:    https://lore.kernel.org/r/20250815164627.22002-4-bcollins%40watter.com
>>> patch subject: [PATCH 3/5] iio: mcp9600: Recognize chip id for mcp9601
>>> config: riscv-randconfig-001-20250816 (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/config)
>>> compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
>>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/reproduce)
>>> 
>>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>>> the same patch/commit), kindly add following tags
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202508161646.PDl6V4EU-lkp@intel.com/
>>> 
>>> All warnings (new ones prefixed by >>):
> 
> <trim unrelated -Wnull-pointer-arithmetic>
> 
>>>>> drivers/iio/temperature/mcp9600.c:440:53: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]  
>>>     440 |                                 "Expected id %02x, but device responded with %02\n",
>>>         |                                                                              ~~~^
>>>   include/linux/dev_printk.h:156:62: note: expanded from macro 'dev_warn'
>>>     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
>>>         |                                                                     ^~~
>>>   include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
>>>      19 | #define dev_fmt(fmt) fmt
>>>         |                      ^~~
>>>   include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
>>>     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
>>>         |                              ^~~
>>>>> drivers/iio/temperature/mcp9600.c:441:26: warning: data argument not used by format string [-Wformat-extra-args]  
>>>     440 |                                 "Expected id %02x, but device responded with %02\n",
>>>         |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>     441 |                                  chip_info->chip_id, dev_id);
>>>         |                                                      ^
>>>   include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn'
>>>     156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
>>>         |                                                                     ~~~     ^
>>>   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
>>>     110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
>>>         |                              ~~~    ^
>>>   drivers/iio/temperature/mcp9600.c:428:22: warning: unused variable 'ret' [-Wunused-variable]
>>>     428 |         int ch_sel, dev_id, ret;
>>>         |                             ^~~
>>>   10 warnings generated.
>>> 
>>> 
>>> vim +/x0a +440 drivers/iio/temperature/mcp9600.c
>>> 
>>>   422 
>>>   423 static int mcp9600_probe(struct i2c_client *client)
>>>   424 {
>>>   425 const struct mcp_chip_info *chip_info = i2c_get_match_data(client);
>> 
>> Probably a false positive as I don't think we can probe without something matching and hence
>> that not being NULL but an error check on that match is still a nice to have and should
>> resolve this build warning.  Note there is very little chance a compiler could ever figure
>> out if this can be NULL or not so it's a reasonable warning!
> 
> I am not sure I follow if you are referring to the -Wformat warnings
> above. Isn't it pointing out that the second specifier is missing the
> actual type? Shouldn't it be '%02x' or something of the sort?

That actually was the issue and has already been fixed in follow up.

Thanks

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

[Jonathan Cameron]

On Mon, 18 Aug 2025 08:06:59 -0700
Nathan Chancellor <nathan@kernel.org> wrote:

> Hi Jonathan,
> 
> On Sat, Aug 16, 2025 at 11:02:43AM +0100, Jonathan Cameron wrote:
> > On Sat, 16 Aug 2025 16:46:12 +0800
> > kernel test robot <lkp@intel.com> wrote:
> >   
> > > Hi Ben,
> > > 
> > > kernel test robot noticed the following build warnings:
> > > 
> > > [auto build test WARNING on jic23-iio/togreg]
> > > [also build test WARNING on linus/master v6.17-rc1 next-20250815]
> > > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > > And when submitting patch, we suggest to use '--base' as documented in
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > > 
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Ben-Collins/dt-bindings-iio-mcp9600-Add-compatible-for-microchip-mcp9601/20250816-005705
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> > > patch link:    https://lore.kernel.org/r/20250815164627.22002-4-bcollins%40watter.com
> > > patch subject: [PATCH 3/5] iio: mcp9600: Recognize chip id for mcp9601
> > > config: riscv-randconfig-001-20250816 (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/config)
> > > compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
> > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508161646.PDl6V4EU-lkp@intel.com/reproduce)
> > > 
> > > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > > the same patch/commit), kindly add following tags
> > > | Reported-by: kernel test robot <lkp@intel.com>
> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202508161646.PDl6V4EU-lkp@intel.com/
> > > 
> > > All warnings (new ones prefixed by >>):  
> 
> <trim unrelated -Wnull-pointer-arithmetic>
> 
> > > >> drivers/iio/temperature/mcp9600.c:440:53: warning: invalid conversion specifier '\x0a' [-Wformat-invalid-specifier]    
> > >      440 |                                 "Expected id %02x, but device responded with %02\n",
> > >          |                                                                              ~~~^
> > >    include/linux/dev_printk.h:156:62: note: expanded from macro 'dev_warn'
> > >      156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
> > >          |                                                                     ^~~
> > >    include/linux/dev_printk.h:19:22: note: expanded from macro 'dev_fmt'
> > >       19 | #define dev_fmt(fmt) fmt
> > >          |                      ^~~
> > >    include/linux/dev_printk.h:110:16: note: expanded from macro 'dev_printk_index_wrap'
> > >      110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
> > >          |                              ^~~  
> > > >> drivers/iio/temperature/mcp9600.c:441:26: warning: data argument not used by format string [-Wformat-extra-args]    
> > >      440 |                                 "Expected id %02x, but device responded with %02\n",
> > >          |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >      441 |                                  chip_info->chip_id, dev_id);
> > >          |                                                      ^
> > >    include/linux/dev_printk.h:156:70: note: expanded from macro 'dev_warn'
> > >      156 |         dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
> > >          |                                                                     ~~~     ^
> > >    include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
> > >      110 |                 _p_func(dev, fmt, ##__VA_ARGS__);                       \
> > >          |                              ~~~    ^
> > >    drivers/iio/temperature/mcp9600.c:428:22: warning: unused variable 'ret' [-Wunused-variable]
> > >      428 |         int ch_sel, dev_id, ret;
> > >          |                             ^~~
> > >    10 warnings generated.
> > > 
> > > 
> > > vim +/x0a +440 drivers/iio/temperature/mcp9600.c
> > > 
> > >    422	
> > >    423	static int mcp9600_probe(struct i2c_client *client)
> > >    424	{
> > >    425		const struct mcp_chip_info *chip_info = i2c_get_match_data(client);  
> > 
> > Probably a false positive as I don't think we can probe without something matching and hence
> > that not being NULL but an error check on that match is still a nice to have and should
> > resolve this build warning.  Note there is very little chance a compiler could ever figure
> > out if this can be NULL or not so it's a reasonable warning!  
> 
> I am not sure I follow if you are referring to the -Wformat warnings
> above. Isn't it pointing out that the second specifier is missing the
> actual type? Shouldn't it be '%02x' or something of the sort?

I think I completely misread the report!  Sorry about that. Ignore my comment.

Jonathan

> 
> Cheers,
> Nathan
>