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

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

[Ben Collins]

ChangeLog:
v4 -> v5:
  - Missed a one line fix to IIR patch (5/5)

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

v2 -> v3:
  - Improve changelogs in each patch
  - Based on feedback from Andy Shevchenko <andy.shevchenko@gmail.com>
    * Set register offsets to fixed width
    * Fix typos
    * Future-proof Kconfig changes
    * Convert to using chip_info paradigm
    * Verbiage: dt -> firmware description
    * Use proper specifiers and drop castings
    * Fix register offset to be fixed-width
    * u8 for cfg var
    * Fix % type for u32 to be %u
    * Make blank lines consistent between case statements
    * 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 and fixed width cleanup
  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    |  25 +-
 drivers/iio/temperature/Kconfig               |   8 +-
 drivers/iio/temperature/mcp9600.c             | 220 ++++++++++++++++--
 3 files changed, 231 insertions(+), 22 deletions(-)

-- 
2.39.5

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

[Ben Collins]

From: Ben Collins <bcollins@watter.com>

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

v4 -> v5:
  - Missed a one line fix to IIR patch (5/5)

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

v2 -> v3:
  - Improve changelogs in each patch
  - Based on feedback from Andy Shevchenko <andy.shevchenko@gmail.com>
    * Set register offsets to fixed width
    * Fix typos
    * Future-proof Kconfig changes
    * Convert to using chip_info paradigm
    * Verbiage: dt -> firmware description
    * Use proper specifiers and drop castings
    * Fix register offset to be fixed-width
    * u8 for cfg var
    * Fix % type for u32 to be %u
    * Make blank lines consistent between case statements
    * 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 (6):
  dt-bindings: iio: mcp9600: Set default 3 for thermocouple-type
  dt-bindings: iio: mcp9600: Add microchip,mcp9601 and add constraints
  iio: mcp9600: White space and fixed width cleanup
  iio: mcp9600: Recognize chip id for mcp9601
  iio: mcp9600: Add support for thermocouple-type
  iio: mcp9600: Add support for IIR filter

 .../iio/temperature/microchip,mcp9600.yaml    |  61 +++-
 drivers/iio/temperature/Kconfig               |   8 +-
 drivers/iio/temperature/mcp9600.c             | 295 +++++++++++++++++-
 3 files changed, 341 insertions(+), 23 deletions(-)

-- 
2.39.5

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

[Ben Collins]

From: Ben Collins <bcollins@watter.com>

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

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

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

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

[Ben Collins]

From: Ben Collins <bcollins@watter.com>

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

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

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

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

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

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

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

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

[Ben Collins]

From: Ben Collins <bcollins@watter.com>

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

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

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

[Ben Collins]

From: Ben Collins <bcollins@watter.com>

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 | 55 +++++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 1244d8e17d50..9328b2250ace 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -173,11 +173,13 @@ config MAX31865
 	  will be called max31865.
 
 config MCP9600
-	tristate "MCP9600 thermocouple EMF converter"
+	tristate "MCP9600 and similar thermocouple EMF converters"
 	depends on I2C
 	help
-	  If you say yes here you get support for MCP9600
-	  thermocouple EMF converter connected via I2C.
+	  If you say yes here you get support for...
+	  - MCP9600
+	  - MCP9601
+	  ...thermocouple EMF converters connected via I2C.
 
 	  This driver can also be built as a module. If so, the module
 	  will be called mcp9600.
diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index 40906bb200ec..a5fad80250d3 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,33 @@ 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;
+
+	if (!chip_info)
+		return dev_err_probe(&client->dev, -EINVAL,
+				     "No chip-info found for device\n");
+
+	dev_id = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
+	if (dev_id < 0)
+		return dev_err_probe(&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 %02x\n",
+				 chip_info->chip_id, dev_id);
+		break;
 
-	ret = i2c_smbus_read_byte_data(client, MCP9600_DEVICE_ID);
-	if (ret < 0)
-		return dev_err_probe(&client->dev, ret, "Failed to read device ID\n");
-	if (ret != MCP9600_DEVICE_ID_MCP9600)
-		dev_warn(&client->dev, "Expected ID %x, got %x\n",
-				MCP9600_DEVICE_ID_MCP9600, ret);
+	default:
+		dev_warn(&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 +462,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 +470,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.39.5

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

[Ben Collins]

From: Ben Collins <bcollins@watter.com>

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 a5fad80250d3..9b017820efc1 100644
--- a/drivers/iio/temperature/mcp9600.c
+++ b/drivers/iio/temperature/mcp9600.c
@@ -22,11 +22,15 @@
 #include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 
+#include <dt-bindings/iio/temperature/thermocouple.h>
+
 /* MCP9600 registers */
 #define MCP9600_HOT_JUNCTION		0x00
 #define MCP9600_COLD_JUNCTION		0x02
 #define MCP9600_STATUS			0x04
 #define MCP9600_STATUS_ALERT(x)		BIT(x)
+#define MCP9600_SENSOR_CFG		0x05
+#define MCP9600_SENSOR_TYPE_MASK	GENMASK(6, 4)
 #define MCP9600_ALERT_CFG1		0x08
 #define MCP9600_ALERT_CFG(x)		(MCP9600_ALERT_CFG1 + (x - 1))
 #define MCP9600_ALERT_CFG_ENABLE	BIT(0)
@@ -66,6 +70,30 @@ static const char * const mcp9600_alert_name[MCP9600_ALERT_COUNT] = {
 	[MCP9600_ALERT4] = "alert4",
 };
 
+/* Map between dt-bindings enum and the chip's type value */
+static const unsigned int mcp9600_type_map[] = {
+	[THERMOCOUPLE_TYPE_K] = 0,
+	[THERMOCOUPLE_TYPE_J] = 1,
+	[THERMOCOUPLE_TYPE_T] = 2,
+	[THERMOCOUPLE_TYPE_N] = 3,
+	[THERMOCOUPLE_TYPE_S] = 4,
+	[THERMOCOUPLE_TYPE_E] = 5,
+	[THERMOCOUPLE_TYPE_B] = 6,
+	[THERMOCOUPLE_TYPE_R] = 7,
+};
+
+/* Map thermocouple type to a char for iio info in sysfs */
+static const int mcp9600_tc_types[] = {
+	[THERMOCOUPLE_TYPE_K] = 'K',
+	[THERMOCOUPLE_TYPE_J] = 'J',
+	[THERMOCOUPLE_TYPE_T] = 'T',
+	[THERMOCOUPLE_TYPE_N] = 'N',
+	[THERMOCOUPLE_TYPE_S] = 'S',
+	[THERMOCOUPLE_TYPE_E] = 'E',
+	[THERMOCOUPLE_TYPE_B] = 'B',
+	[THERMOCOUPLE_TYPE_R] = 'R',
+};
+
 static const struct iio_event_spec mcp9600_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -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) {
@@ -457,6 +508,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.39.5

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

[Ben Collins]

From: Ben Collins <bcollins@watter.com>

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

Use a filter_type[none, ema] for enabling the IIR filter.

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

diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
index 9b017820efc1..0ec47cbeb88c 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,27 @@ static const int mcp9600_tc_types[] = {
 	[THERMOCOUPLE_TYPE_R] = 'R',
 };
 
+enum mcp9600_filter {
+	MCP9600_FILTER_TYPE_NONE,
+	MCP9600_FILTER_TYPE_EMA,
+};
+
+static const char * const mcp9600_filter_type[] = {
+	[MCP9600_FILTER_TYPE_NONE] = "none",
+	[MCP9600_FILTER_TYPE_EMA] = "ema",
+};
+
+static const int mcp_iir_coefficients_avail[7][2] = {
+	/* Level 0 is no filter */
+	{ 0, 524549 },
+	{ 0, 243901 },
+	{ 0, 119994 },
+	{ 0,  59761 },
+	{ 0,  29851 },
+	{ 0,  14922 },
+	{ 0,   7461 },
+};
+
 static const struct iio_event_spec mcp9600_events[] = {
 	{
 		.type = IIO_EV_TYPE_THRESH,
@@ -118,7 +140,11 @@ 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), \
+			.ext_info = mcp9600_ext_filter,			       \
 			.event_spec = &mcp9600_events[hj_ev_spec_off],	       \
 			.num_event_specs = hj_num_ev,			       \
 		},							       \
@@ -134,6 +160,26 @@ static const struct iio_event_spec mcp9600_events[] = {
 		},							       \
 	}
 
+static int mcp9600_get_filter(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan);
+static int mcp9600_set_filter(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      unsigned int mode);
+
+static const struct iio_enum mcp9600_filter_enum = {
+	.items = mcp9600_filter_type,
+	.num_items = ARRAY_SIZE(mcp9600_filter_type),
+	.get = mcp9600_get_filter,
+	.set = mcp9600_set_filter,
+};
+
+static const struct iio_chan_spec_ext_info mcp9600_ext_filter[] = {
+	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &mcp9600_filter_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
+			   &mcp9600_filter_enum),
+	{ }
+};
+
 static const struct iio_chan_spec mcp9600_channels[][2] = {
 	MCP9600_CHANNELS(0, 0, 0, 0), /* Alerts: - - - - */
 	MCP9600_CHANNELS(1, 0, 0, 0), /* Alerts: 1 - - - */
@@ -161,6 +207,8 @@ struct mcp_chip_info {
 struct mcp9600_data {
 	struct i2c_client *client;
 	u32 thermocouple_type;
+	/* Filter enabled is 1-7, with 0 being off (filter_type none) */
+	u8 filter_level;
 };
 
 static int mcp9600_read(struct mcp9600_data *data,
@@ -191,13 +239,45 @@ 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:
+		if (data->filter_level == 0)
+			return -EINVAL;
+
+		*val = mcp_iir_coefficients_avail[data->filter_level - 1][0];
+		*val2 = mcp_iir_coefficients_avail[data->filter_level - 1][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+
+	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)
+{
+	struct mcp9600_data *data = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		if (data->filter_level == 0)
+			return -EINVAL;
+
+		*vals = (int *)mcp_iir_coefficients_avail;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = 2 * ARRAY_SIZE(mcp_iir_coefficients_avail);
+		return IIO_AVAIL_LIST;
 	default:
 		return -EINVAL;
 	}
@@ -211,6 +291,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 +302,79 @@ 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_PLUS_MICRO;
+	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][0] == val &&
+			    mcp_iir_coefficients_avail[i][1] == val2)
+				break;
+		}
+
+		if (i == ARRAY_SIZE(mcp_iir_coefficients_avail))
+			return -EINVAL;
+
+		data->filter_level = i + 1;
+		return mcp9600_config(data);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mcp9600_get_filter(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan)
+{
+	struct mcp9600_data *data = iio_priv(indio_dev);
+
+	if (data->filter_level == 0)
+		return MCP9600_FILTER_TYPE_NONE;
+
+	return MCP9600_FILTER_TYPE_EMA;
+}
+
+static int mcp9600_set_filter(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      unsigned int mode)
+{
+	struct mcp9600_data *data = iio_priv(indio_dev);
+
+	switch (mode) {
+	case MCP9600_FILTER_TYPE_NONE:
+		data->filter_level = 0;
+		return mcp9600_config(data);
+
+	case MCP9600_FILTER_TYPE_EMA:
+		if (data->filter_level == 0) {
+			/* Minimum filter by default */
+			data->filter_level = 1;
+			return mcp9600_config(data);
+		}
+		return 0;
+
+	default:
+		return -EINVAL;
+	}
+}
+
 static int mcp9600_get_alert_index(int channel2, enum iio_event_direction dir)
 {
 	if (channel2 == IIO_MOD_TEMP_AMBIENT) {
@@ -358,6 +512,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.39.5

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

[Rob Herring (Arm)]

On Mon, 18 Aug 2025 14:32:10 -0400, Ben Collins wrote:
> From: Ben Collins <bcollins@watter.com>
> 
> Add microchip,mcp9601 compatible in addition to the original
> microchip,mcp9600 to designate support between these two chips.
> 
> The current dt-binding has open-circuit and short-circuit as interrupt
> names, but these are only supported in mcp9601.
> 
> The OC and SC detection requires that mcp9601 VSENSE be wired up, which
> not only enables the OC SC interrupts, but also the OC and SC status
> register bits.
> 
> Add a microchip,vsense boolean to show the chip is wired for this
> support.
> 
> Add constraints so this feature only applies if the mcp9601 compatible
> is selected.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
>  .../iio/temperature/microchip,mcp9600.yaml    | 60 ++++++++++++++++++-
>  1 file changed, 57 insertions(+), 3 deletions(-)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/temperature/microchip,mcp9600.yaml: properties:interrupt-names:items:enum: ['open-circuit', 'short-circuit', 'alert1', 'alert2', 'alert3', 'alert4', 'open-circuit', 'short-circuit'] has non-unique elements
	hint: "enum" must be an array of either integers or strings
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250818183214.380847-3-bcollins@kernel.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

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

[Krzysztof Kozlowski]

On Mon, Aug 18, 2025 at 02:32:08PM -0400, Ben Collins wrote:
> From: Ben Collins <bcollins@watter.com>
> 
> ChangeLog:
> v5 -> v6:
>   - Fix accidental typo added in dt-bindings: IRQ_TYPE_EDGE_RISIN
>   - Correct some constraints in dt-bindings
>   - Reverse if/then for mcp9601 vs mcp9600 constraints in dt-bindings
>   - Updates to changelog for patch 2/6 (dt-bindings mcp9600)
>   - Cleanup tabs that were converted to spaces
>   - Split thermocouple-type default to separate patch

Please start using b4, so you will get changelogs with lore links for
free and ALL your patches will be properly versioned. git can do that
as well - git format-patch -v5 --cover-letter, if you don't want to use
b4.

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On Mon, Aug 18, 2025 at 02:32:09PM -0400, Ben Collins wrote:
> From: Ben Collins <bcollins@watter.com>
> 
> As is already documented in this file, Type-K is the default, so make
> that explicit in the dt-bindings.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
>  .../devicetree/bindings/iio/temperature/microchip,mcp9600.yaml   | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On Mon, Aug 18, 2025 at 02:32:10PM -0400, Ben Collins wrote:
>  properties:
>    compatible:
> -    const: microchip,mcp9600
> +    oneOf:
> +      - const: microchip,mcp9600
> +      - items:
> +          - const: microchip,mcp9601
> +          - const: microchip,mcp9600
>  
>    reg:
>      maxItems: 1
> @@ -34,6 +38,8 @@ properties:
>          - alert2
>          - alert3
>          - alert4
> +        - open-circuit
> +        - short-circuit

I do not understand this change. And again, if you tested it before
sending, you should see error.

Best regards,
Krzysztof

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

[David Lechner]

On 8/18/25 1:32 PM, Ben Collins wrote:
> From: Ben Collins <bcollins@watter.com>
> 
> MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
> to allow get/set of this value.
> 
> Use a filter_type[none, ema] for enabling the IIR filter.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
>  drivers/iio/temperature/mcp9600.c | 157 ++++++++++++++++++++++++++++++
>  1 file changed, 157 insertions(+)
> 
> diff --git a/drivers/iio/temperature/mcp9600.c b/drivers/iio/temperature/mcp9600.c
> index 9b017820efc1..0ec47cbeb88c 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,27 @@ static const int mcp9600_tc_types[] = {
>  	[THERMOCOUPLE_TYPE_R] = 'R',
>  };
>  
> +enum mcp9600_filter {
> +	MCP9600_FILTER_TYPE_NONE,
> +	MCP9600_FILTER_TYPE_EMA,
> +};
> +
> +static const char * const mcp9600_filter_type[] = {
> +	[MCP9600_FILTER_TYPE_NONE] = "none",
> +	[MCP9600_FILTER_TYPE_EMA] = "ema",
> +};
> +
> +static const int mcp_iir_coefficients_avail[7][2] = {
> +	/* Level 0 is no filter */
> +	{ 0, 524549 },
> +	{ 0, 243901 },
> +	{ 0, 119994 },
> +	{ 0,  59761 },
> +	{ 0,  29851 },
> +	{ 0,  14922 },
> +	{ 0,   7461 },
> +};
> +
>  static const struct iio_event_spec mcp9600_events[] = {
>  	{
>  		.type = IIO_EV_TYPE_THRESH,
> @@ -118,7 +140,11 @@ 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), \
> +			.ext_info = mcp9600_ext_filter,			       \
>  			.event_spec = &mcp9600_events[hj_ev_spec_off],	       \
>  			.num_event_specs = hj_num_ev,			       \
>  		},							       \
> @@ -134,6 +160,26 @@ static const struct iio_event_spec mcp9600_events[] = {
>  		},							       \
>  	}
>  
> +static int mcp9600_get_filter(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan);
> +static int mcp9600_set_filter(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      unsigned int mode);
> +
> +static const struct iio_enum mcp9600_filter_enum = {
> +	.items = mcp9600_filter_type,
> +	.num_items = ARRAY_SIZE(mcp9600_filter_type),
> +	.get = mcp9600_get_filter,
> +	.set = mcp9600_set_filter,
> +};
> +
> +static const struct iio_chan_spec_ext_info mcp9600_ext_filter[] = {
> +	IIO_ENUM("filter_type", IIO_SHARED_BY_ALL, &mcp9600_filter_enum),
> +	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_ALL,
> +			   &mcp9600_filter_enum),
> +	{ }
> +};
> +
>  static const struct iio_chan_spec mcp9600_channels[][2] = {
>  	MCP9600_CHANNELS(0, 0, 0, 0), /* Alerts: - - - - */
>  	MCP9600_CHANNELS(1, 0, 0, 0), /* Alerts: 1 - - - */
> @@ -161,6 +207,8 @@ struct mcp_chip_info {
>  struct mcp9600_data {
>  	struct i2c_client *client;
>  	u32 thermocouple_type;
> +	/* Filter enabled is 1-7, with 0 being off (filter_type none) */
> +	u8 filter_level;
>  };
>  
>  static int mcp9600_read(struct mcp9600_data *data,
> @@ -191,13 +239,45 @@ 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:
> +		if (data->filter_level == 0)
> +			return -EINVAL;

To implement Jonathan's request from v5, drop this error return.
We'll also need a separate bool data->is_filter_enabled field so
that we can keep the last set filter_level even when the filter
is disabled. (i.e. data->filter_level is never == 0).

This way, if you set the filter level, you can enable and disable
the filter via filter_type and still have the same filter level.

> +
> +		*val = mcp_iir_coefficients_avail[data->filter_level - 1][0];
> +		*val2 = mcp_iir_coefficients_avail[data->filter_level - 1][1];
> +		return IIO_VAL_INT_PLUS_MICRO;
> +
> +	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)
> +{
> +	struct mcp9600_data *data = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		if (data->filter_level == 0)
> +			return -EINVAL;
> +
> +		*vals = (int *)mcp_iir_coefficients_avail;
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		*length = 2 * ARRAY_SIZE(mcp_iir_coefficients_avail);
> +		return IIO_AVAIL_LIST;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -211,6 +291,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);

And change the logic here to:

	FIELD_MODIFY(MCP9600_FILTER_MASK, &cfg, data->is_filter_enabled ?
		     data->filter_level : 0);

>  
>  	ret = i2c_smbus_write_byte_data(client, MCP9600_SENSOR_CFG, cfg);
>  	if (ret < 0) {
> @@ -221,6 +302,79 @@ 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_PLUS_MICRO;
> +	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][0] == val &&
> +			    mcp_iir_coefficients_avail[i][1] == val2)
> +				break;
> +		}
> +
> +		if (i == ARRAY_SIZE(mcp_iir_coefficients_avail))
> +			return -EINVAL;
> +
> +		data->filter_level = i + 1;
> +		return mcp9600_config(data);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int mcp9600_get_filter(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan)
> +{
> +	struct mcp9600_data *data = iio_priv(indio_dev);
> +
> +	if (data->filter_level == 0)
> +		return MCP9600_FILTER_TYPE_NONE;
> +
> +	return MCP9600_FILTER_TYPE_EMA;
> +}
> +
> +static int mcp9600_set_filter(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan,
> +			      unsigned int mode)
> +{
> +	struct mcp9600_data *data = iio_priv(indio_dev);
> +
> +	switch (mode) {
> +	case MCP9600_FILTER_TYPE_NONE:
> +		data->filter_level = 0;

Don't touch data->filter_level here, just set data->is_filter_enabled = false.

> +		return mcp9600_config(data);
> +
> +	case MCP9600_FILTER_TYPE_EMA:
> +		if (data->filter_level == 0) {
> +			/* Minimum filter by default */
> +			data->filter_level = 1;

And similar, just set data->is_filter_enabled = true without changing filter_level.

> +			return mcp9600_config(data);
> +		}
> +		return 0;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
>  static int mcp9600_get_alert_index(int channel2, enum iio_event_direction dir)
>  {
>  	if (channel2 == IIO_MOD_TEMP_AMBIENT) {
> @@ -358,6 +512,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 6/6] iio: mcp9600: Add support for IIR filter

[Ben Collins]

On Tue, Aug 19, 2025 at 09:05:39AM -0500, David Lechner wrote:
> On 8/18/25 1:32 PM, Ben Collins wrote:
> > From: Ben Collins <bcollins@watter.com>
> > 
> > MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
> > to allow get/set of this value.
> > 
> > Use a filter_type[none, ema] for enabling the IIR filter.
> > 
> > Signed-off-by: Ben Collins <bcollins@watter.com>
> > ---
> > +		if (data->filter_level == 0)
> > +			return -EINVAL;
> 
> To implement Jonathan's request from v5, drop this error return.
> We'll also need a separate bool data->is_filter_enabled field so
> that we can keep the last set filter_level even when the filter
> is disabled. (i.e. data->filter_level is never == 0).
> 
> This way, if you set the filter level, you can enable and disable
> the filter via filter_type and still have the same filter level.
> 

Thanks, David. This is exactly what I've implemented, plus the
filter_enable attribute.

Adding the ABI doc updates as well.

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

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

[David Lechner]

On 8/19/25 9:11 AM, Ben Collins wrote:
> On Tue, Aug 19, 2025 at 09:05:39AM -0500, David Lechner wrote:
>> On 8/18/25 1:32 PM, Ben Collins wrote:
>>> From: Ben Collins <bcollins@watter.com>
>>>
>>> MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
>>> to allow get/set of this value.
>>>
>>> Use a filter_type[none, ema] for enabling the IIR filter.
>>>
>>> Signed-off-by: Ben Collins <bcollins@watter.com>
>>> ---
>>> +		if (data->filter_level == 0)
>>> +			return -EINVAL;
>>
>> To implement Jonathan's request from v5, drop this error return.
>> We'll also need a separate bool data->is_filter_enabled field so
>> that we can keep the last set filter_level even when the filter
>> is disabled. (i.e. data->filter_level is never == 0).
>>
>> This way, if you set the filter level, you can enable and disable
>> the filter via filter_type and still have the same filter level.
>>
> 
> Thanks, David. This is exactly what I've implemented, plus the
> filter_enable attribute.
> 
> Adding the ABI doc updates as well.
> 


Don't add the filter_enable attribute. The filter_type attribute
already does the job.

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

[Ben Collins]

On Tue, Aug 19, 2025 at 09:15:23AM -0500, David Lechner wrote:
> On 8/19/25 9:11 AM, Ben Collins wrote:
> > On Tue, Aug 19, 2025 at 09:05:39AM -0500, David Lechner wrote:
> >> On 8/18/25 1:32 PM, Ben Collins wrote:
> >>> From: Ben Collins <bcollins@watter.com>
> >>>
> >>> MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
> >>> to allow get/set of this value.
> >>>
> >>> Use a filter_type[none, ema] for enabling the IIR filter.
> >>>
> >>> Signed-off-by: Ben Collins <bcollins@watter.com>
> >>> ---
> >>> +		if (data->filter_level == 0)
> >>> +			return -EINVAL;
> >>
> >> To implement Jonathan's request from v5, drop this error return.
> >> We'll also need a separate bool data->is_filter_enabled field so
> >> that we can keep the last set filter_level even when the filter
> >> is disabled. (i.e. data->filter_level is never == 0).
> >>
> >> This way, if you set the filter level, you can enable and disable
> >> the filter via filter_type and still have the same filter level.
> >>
> > 
> > Thanks, David. This is exactly what I've implemented, plus the
> > filter_enable attribute.
> > 
> > Adding the ABI doc updates as well.
> > 
> 
> 
> Don't add the filter_enable attribute. The filter_type attribute
> already does the job.

That doesn't solve the problem at hand. An example:

- Driver has 3 possible filter_type's, plus "none"
- User cats filter_type_available and sees [none, sinc4, sinc5, sinc5+avg]
- User cats filter_type and sees "none"
- User cats frequency_available: What do they see?
- User cats frequency: What do they see?

Without filter_enable, [none, ema] driver works just fine. But the
above driver does not.

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

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

[David Lechner]

On 8/19/25 9:32 AM, Ben Collins wrote:
> On Tue, Aug 19, 2025 at 09:15:23AM -0500, David Lechner wrote:
>> On 8/19/25 9:11 AM, Ben Collins wrote:
>>> On Tue, Aug 19, 2025 at 09:05:39AM -0500, David Lechner wrote:
>>>> On 8/18/25 1:32 PM, Ben Collins wrote:
>>>>> From: Ben Collins <bcollins@watter.com>
>>>>>
>>>>> MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
>>>>> to allow get/set of this value.
>>>>>
>>>>> Use a filter_type[none, ema] for enabling the IIR filter.
>>>>>
>>>>> Signed-off-by: Ben Collins <bcollins@watter.com>
>>>>> ---
>>>>> +		if (data->filter_level == 0)
>>>>> +			return -EINVAL;
>>>>
>>>> To implement Jonathan's request from v5, drop this error return.
>>>> We'll also need a separate bool data->is_filter_enabled field so
>>>> that we can keep the last set filter_level even when the filter
>>>> is disabled. (i.e. data->filter_level is never == 0).
>>>>
>>>> This way, if you set the filter level, you can enable and disable
>>>> the filter via filter_type and still have the same filter level.
>>>>
>>>
>>> Thanks, David. This is exactly what I've implemented, plus the
>>> filter_enable attribute.
>>>
>>> Adding the ABI doc updates as well.
>>>
>>
>>
>> Don't add the filter_enable attribute. The filter_type attribute
>> already does the job.
> 
> That doesn't solve the problem at hand. An example:
> 
> - Driver has 3 possible filter_type's, plus "none"
> - User cats filter_type_available and sees [none, sinc4, sinc5, sinc5+avg]
> - User cats filter_type and sees "none"
> - User cats frequency_available: What do they see?
> - User cats frequency: What do they see?

The ones for the last selected filter before it was changed to "none".
If the driver starts in the "none" state a probe, just pick sinc4
as the default.

> 
> Without filter_enable, [none, ema] driver works just fine. But the
> above driver does not.
> 

We can wait and see what Jonathan thinks. But if we introduce a
new filter_enable attribute, then we need to think about what to
do about the ad4080 driver since it was the one that recently
introduced the filter_type = "none". Ideally we would change it
to work the same so that we are consistent between drivers. But
there is always the consideration that we can't go breaking existing
ABI.

Re: [PATCH 3/6] iio: mcp9600: White space and fixed width cleanup

[David Lechner]

On 8/18/25 1:32 PM, Ben Collins wrote:
> From: Ben Collins <bcollins@watter.com>
> 
> Make tabs consistent for register definitions and also fix width
> to byte size.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---
Reviewed-by: David Lechner <dlechner@baylibrc.com>

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

[David Lechner]

On 8/18/25 1:32 PM, Ben Collins wrote:
> From: Ben Collins <bcollins@watter.com>
> 
> 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>
> ---
Reviewed-by: David Lechner <dlechner@baylibre.com>

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

[David Lechner]

On 8/18/25 1:32 PM, Ben Collins wrote:
> From: Ben Collins <bcollins@watter.com>
> 
> dt-bindings documentation for this driver claims to support
> thermocouple-type, but the driver does not actually make use of
> the property.
> 
> Implement usage of the property to configure the chip for the
> selected thermocouple-type.
> 
> Signed-off-by: Ben Collins <bcollins@watter.com>
> ---

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

Re: [PATCH 3/6] iio: mcp9600: White space and fixed width cleanup

[David Lechner]

On Tue, Aug 19, 2025 at 9:55 AM David Lechner <dlechner@baylibre.com> wrote:
>
> On 8/18/25 1:32 PM, Ben Collins wrote:
> > From: Ben Collins <bcollins@watter.com>
> >
> > Make tabs consistent for register definitions and also fix width
> > to byte size.
> >
> > Signed-off-by: Ben Collins <bcollins@watter.com>
> > ---
> Reviewed-by: David Lechner <dlechner@baylibrc.com>

I made a typo above. Should be:

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

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

[Jonathan Cameron]

On Tue, 19 Aug 2025 08:55:44 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On Mon, Aug 18, 2025 at 02:32:08PM -0400, Ben Collins wrote:
> > From: Ben Collins <bcollins@watter.com>
> > 
> > ChangeLog:
> > v5 -> v6:
> >   - Fix accidental typo added in dt-bindings: IRQ_TYPE_EDGE_RISIN
> >   - Correct some constraints in dt-bindings
> >   - Reverse if/then for mcp9601 vs mcp9600 constraints in dt-bindings
> >   - Updates to changelog for patch 2/6 (dt-bindings mcp9600)
> >   - Cleanup tabs that were converted to spaces
> >   - Split thermocouple-type default to separate patch  
> 
> Please start using b4, so you will get changelogs with lore links for
> free and ALL your patches will be properly versioned. git can do that
> as well - git format-patch -v5 --cover-letter, if you don't want to use
> b4.

Second that.  This is what it looks like in patchwork that I use
for managing reviews / merges etc.
https://patchwork.kernel.org/project/linux-iio/list/?series=992678

version number not easy to find as it gets dropped from the series title
and is only normally listed for the patches. 

> 
> Best regards,
> Krzysztof
> 

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

[Jonathan Cameron]

On Tue, 19 Aug 2025 09:43:48 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 8/19/25 9:32 AM, Ben Collins wrote:
> > On Tue, Aug 19, 2025 at 09:15:23AM -0500, David Lechner wrote:  
> >> On 8/19/25 9:11 AM, Ben Collins wrote:  
> >>> On Tue, Aug 19, 2025 at 09:05:39AM -0500, David Lechner wrote:  
> >>>> On 8/18/25 1:32 PM, Ben Collins wrote:  
> >>>>> From: Ben Collins <bcollins@watter.com>
> >>>>>
> >>>>> MCP9600 supports an IIR filter with 7 levels. Add IIR attribute
> >>>>> to allow get/set of this value.
> >>>>>
> >>>>> Use a filter_type[none, ema] for enabling the IIR filter.
> >>>>>
> >>>>> Signed-off-by: Ben Collins <bcollins@watter.com>
> >>>>> ---
> >>>>> +		if (data->filter_level == 0)
> >>>>> +			return -EINVAL;  
> >>>>
> >>>> To implement Jonathan's request from v5, drop this error return.
> >>>> We'll also need a separate bool data->is_filter_enabled field so
> >>>> that we can keep the last set filter_level even when the filter
> >>>> is disabled. (i.e. data->filter_level is never == 0).
> >>>>
> >>>> This way, if you set the filter level, you can enable and disable
> >>>> the filter via filter_type and still have the same filter level.
> >>>>  
> >>>
> >>> Thanks, David. This is exactly what I've implemented, plus the
> >>> filter_enable attribute.
> >>>
> >>> Adding the ABI doc updates as well.
> >>>  
> >>
> >>
> >> Don't add the filter_enable attribute. The filter_type attribute
> >> already does the job.  
> > 
> > That doesn't solve the problem at hand. An example:
> > 
> > - Driver has 3 possible filter_type's, plus "none"
> > - User cats filter_type_available and sees [none, sinc4, sinc5, sinc5+avg]
> > - User cats filter_type and sees "none"
> > - User cats frequency_available: What do they see?
> > - User cats frequency: What do they see?  
> 
> The ones for the last selected filter before it was changed to "none".
> If the driver starts in the "none" state a probe, just pick sinc4
> as the default.

That works, or presenting no available frequencies if "none"
- empty list.  Though check the standard wrapper for available works
with a list of size 0. Not something we've done before.  Maybe a risk
of tripping up some userspace code?

Unlike some attribute/controls this one is unlikely to ever be destructive if we
have to pass through unusual states. Might have a slightly slower
transition to steady state if we are going through something inappropriate
briefly.

> 
> > 
> > Without filter_enable, [none, ema] driver works just fine. But the
> > above driver does not.
> >   
> 
> We can wait and see what Jonathan thinks. But if we introduce a
> new filter_enable attribute, then we need to think about what to
> do about the ad4080 driver since it was the one that recently
> introduced the filter_type = "none". Ideally we would change it
> to work the same so that we are consistent between drivers. But
> there is always the consideration that we can't go breaking existing
> ABI.
I was thinking we could paper over it (hence the email I sent
30 seconds before opening this) with a bonus attribute and basing
both filter_enable and none setting on the same underlying state.
Not that intuitive though as I think more about it. 

Setting off none would enable the filter when maybe a user was just
expecting to be able to see what was available (as will work for
any driver not implementing the 'none' value).  The ABI has always
allowed for interactions like this as sometimes we can't avoid them
but here maybe we can.

So ignore email of 2 mins ago, David's suggestion works better
(with modification for not showing any frequencies when on none possibly?)

Anyhow, lets take a little more time on this. I for one shouldn't
make any decisions quickly as is clear here!

Jonathan

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

[Ben Collins]

On Tue, Aug 19, 2025 at 07:24:55PM -0500, Jonathan Cameron wrote:
> On Tue, 19 Aug 2025 08:55:44 +0200
> Krzysztof Kozlowski <krzk@kernel.org> wrote:
> 
> > On Mon, Aug 18, 2025 at 02:32:08PM -0400, Ben Collins wrote:
> > > From: Ben Collins <bcollins@watter.com>
> > > 
> > > ChangeLog:
> > > v5 -> v6:
> > >   - Fix accidental typo added in dt-bindings: IRQ_TYPE_EDGE_RISIN
> > >   - Correct some constraints in dt-bindings
> > >   - Reverse if/then for mcp9601 vs mcp9600 constraints in dt-bindings
> > >   - Updates to changelog for patch 2/6 (dt-bindings mcp9600)
> > >   - Cleanup tabs that were converted to spaces
> > >   - Split thermocouple-type default to separate patch  
> > 
> > Please start using b4, so you will get changelogs with lore links for
> > free and ALL your patches will be properly versioned. git can do that
> > as well - git format-patch -v5 --cover-letter, if you don't want to use
> > b4.
> 
> Second that.  This is what it looks like in patchwork that I use
> for managing reviews / merges etc.
> https://patchwork.kernel.org/project/linux-iio/list/?series=992678
> 
> version number not easy to find as it gets dropped from the series title
> and is only normally listed for the patches. 

I appreciate both of you suggesting this. I've switched to b4 now and
now I don't know how anyone could not use it.

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