devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Add support for Microchip EMC1812
@ 2025-09-17 12:21 Marius Cristea
  2025-09-17 12:21 ` [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812 Marius Cristea
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Marius Cristea @ 2025-09-17 12:21 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-kernel, Marius Cristea

This is the iio driver for EMC1812/13/14/15/33 multichannel Low-Voltage
Remote Diode Sensor Family. The chips in the family have one internal
and different numbers of external channels, ranging from 1 (EMC1812) to
4 channels (EMC1815).
Reading diodes in anti-parallel connection is supported by EMC1814, EMC1815
and EMC1833.

Current version of driver does not support interrupts, events and data
buffering.

Differences related to previous patch:

v1:
- initial version.

Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
---
Marius Cristea (2):
      dt-bindings: iio: temperature: add support for EMC1812
      iio: temperature: add support for EMC1812

 .../iio/temperature/microchip,emc1812.yaml         | 223 ++++++
 MAINTAINERS                                        |   7 +
 drivers/iio/temperature/Kconfig                    |  10 +
 drivers/iio/temperature/Makefile                   |   1 +
 drivers/iio/temperature/emc1812.c                  | 792 +++++++++++++++++++++
 5 files changed, 1033 insertions(+)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250805-iio-emc1812-e666183b07b5

Best regards,
-- 
Marius Cristea <marius.cristea@microchip.com>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812
  2025-09-17 12:21 [PATCH 0/2] Add support for Microchip EMC1812 Marius Cristea
@ 2025-09-17 12:21 ` Marius Cristea
  2025-09-17 14:31   ` Rob Herring (Arm)
  2025-09-17 23:34   ` David Lechner
  2025-09-17 12:21 ` [PATCH 2/2] " Marius Cristea
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 19+ messages in thread
From: Marius Cristea @ 2025-09-17 12:21 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-kernel, Marius Cristea

This is the devicetree schema for Microchip EMC1812/13/14/15/33
Multichannel Low-Voltage Remote Diode Sensor Family.

Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
---
 .../iio/temperature/microchip,emc1812.yaml         | 223 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 2 files changed, 229 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..898d6d246746e229cb004f447872ee6bd5a65074
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml
@@ -0,0 +1,223 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/temperature/microchip,emc1812.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip EMC1812/13/14/15/33 multichannel temperature sensor
+
+maintainers:
+  - Marius Cristea <marius.cristea@microchip.com>
+
+description: |
+  The Microchip EMC1812/13/14/15/33 is a high-accuracy 2-wire multichannel
+  low-voltage remote diode temperature monitor.
+
+  The datasheet can be found here:
+    https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/EMC1812-3-4-5-33-Data-Sheet-DS20005751.pdf
+
+properties:
+  compatible:
+    enum:
+      - microchip,emc1812
+      - microchip,emc1813
+      - microchip,emc1814
+      - microchip,emc1815
+      - microchip,emc1833
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:
+    description:
+      -alert-therm2 asserts when a diode temperature exceeds the ALERT
+      threshold.
+      -therm-addr asserts low when the hardware-set THERM limit threshold is
+      exceeded by one of the temperature sensors.
+    items:
+      - const: alert-therm2
+      - const: therm-addr
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  microchip,beta1:
+    description:
+      Set beta compensation value for external channel 1.
+      <0> 0.050
+      <1> 0.066
+      <2> 0.087
+      <3> 0.114
+      <4> 0.150
+      <5> 0.197
+      <6> 0.260
+      <7> 0.342
+      <8> 0.449
+      <9> 0.591
+      <10> 0.778
+      <11> 1.024
+      <12> 1.348
+      <13> 1.773
+      <14> 2.333
+      <15> Diode_Mode
+      <16> Auto
+      - Diode_Mode is used when measuring a discrete thermal diode
+      or a CPU diode that functions like a discrete thermal diode.
+      - Auto enables beta auto-detection. The chip monitors
+      external diode/transistor and determines the optimum
+      setting.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 16
+    default: 16
+
+  microchip,beta2:
+    description:
+      Set beta compensation value for external channel 2.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 16
+    default: 16
+
+  microchip,parasitic-res-on-channel1-2:
+    description:
+      Indicates that the chip and the diodes/transistors are sufficiently far
+      apart that a parasitic resistance is added to the wires, which can affect
+      the measurements. Due to the anti-parallel diode connections, channels
+      1 and 2 are affected together.
+    type: boolean
+
+  microchip,parasitic-res-on-channel3-4:
+    description:
+      Indicates that the chip and the diodes/transistors are sufficiently far
+      apart that a parasitic resistance is added to the wires, which can affect
+      the measurements. Due to the anti-parallel diode connections, channels
+      3 and 4 are affected together.
+    type: boolean
+
+  vdd-supply: true
+
+patternProperties:
+  "^channel@[1-4]$":
+    description:
+      Represents the external temperature channels to which
+      a remote diode is connected.
+    type: object
+
+    properties:
+      reg:
+        items:
+          minimum: 1
+          maximum: 4
+
+      microchip,ideality-factor:
+        description:
+          Each channel has an ideality factor.
+          Beta compensation and resistance error correction automatically
+          correct for most ideality errors. So ideality factor does not need
+          to be adjusted in general.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 8
+        maximum: 55
+        default: 18
+
+      label:
+        description: Unique name to identify which channel this is.
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - microchip,emc1812
+              - microchip,emc1813
+              - microchip,emc1833
+    then:
+      properties:
+        microchip,beta2: false
+        microchip,parasitic-res-on-channel3-4: false
+  - if:
+      properties:
+        compatible:
+          pattern: "^microchip,emc1812"
+    then:
+      patternProperties:
+        "^channel@1$":
+          properties:
+            reg:
+              items:
+                maximum: 1
+        "^channel@[2-4]$": false
+  - if:
+      properties:
+        compatible:
+          pattern: "^microchip,emc18[13]3"
+    then:
+      patternProperties:
+        "^channel@[12]$":
+          properties:
+            reg:
+              items:
+                maximum: 1
+        "^channel@[34]$": false
+  - if:
+      properties:
+        compatible:
+          pattern: "^microchip,emc1814"
+    then:
+      patternProperties:
+        "^channel@[1-3]$":
+          properties:
+            reg:
+              items:
+                maximum: 1
+        "^channel@4$": false
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        temperature-sensor@4c {
+            compatible = "microchip,emc1813";
+            reg = <0x4c>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            microchip,parasitic-res-on-channel1-2;
+
+            microchip,beta1 = <16>;
+            vdd-supply = <&vdd>;
+
+            channel@1 {
+                reg = <1>;
+                label = "External CH1 Temperature";
+            };
+
+            channel@2 {
+                reg = <2>;
+                label = "External CH2 Temperature";
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a92290fffa163f9fe8fe3f04bf66426f9a894409..09efda36a17e398b3ad807ac47485e530154bae4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22862,6 +22862,12 @@ M:	Nicolas Pitre <nico@fluxnic.net>
 S:	Odd Fixes
 F:	drivers/net/ethernet/smsc/smc91x.*
 
+MICROCHIP EMC1812 TEMPERATURE DRIVER
+M:	Marius Cristea <marius.cristea@microchip.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml
+
 SMSC EMC2103 HARDWARE MONITOR DRIVER
 M:	Steve Glendinning <steve.glendinning@shawell.net>
 L:	linux-hwmon@vger.kernel.org

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] iio: temperature: add support for EMC1812
  2025-09-17 12:21 [PATCH 0/2] Add support for Microchip EMC1812 Marius Cristea
  2025-09-17 12:21 ` [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812 Marius Cristea
@ 2025-09-17 12:21 ` Marius Cristea
  2025-09-18  3:07   ` kernel test robot
                     ` (2 more replies)
  2025-09-17 13:25 ` [PATCH 0/2] Add support for Microchip EMC1812 David Lechner
  2025-09-20 11:33 ` Jonathan Cameron
  3 siblings, 3 replies; 19+ messages in thread
From: Marius Cristea @ 2025-09-17 12:21 UTC (permalink / raw)
  To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-kernel, Marius Cristea

This is the iio driver for Microchip EMC1812/13/14/15/33
Multichannel Low-Voltage Remote Diode Sensor Family.

Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
---
 MAINTAINERS                       |   1 +
 drivers/iio/temperature/Kconfig   |  10 +
 drivers/iio/temperature/Makefile  |   1 +
 drivers/iio/temperature/emc1812.c | 792 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 804 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 09efda36a17e398b3ad807ac47485e530154bae4..15a40cfb7a473789086a1a37459baaff05c97ee2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22867,6 +22867,7 @@ M:	Marius Cristea <marius.cristea@microchip.com>
 L:	linux-iio@vger.kernel.org
 S:	Supported
 F:	Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml
+F:	drivers/iio/temperature/emc1812.c
 
 SMSC EMC2103 HARDWARE MONITOR DRIVER
 M:	Steve Glendinning <steve.glendinning@shawell.net>
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 1244d8e17d504ab23425f0c33f5f2d06dadba663..eb7269bd0a2ab2b6eea777a55c0c520f5e048d53 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -4,6 +4,16 @@
 #
 menu "Temperature sensors"
 
+config EMC1812
+	tristate "Microchip Technology EMC1812 driver"
+	depends on I2C
+	help
+	  Say yes here to build support for Microchip Technology's EMC181X/33
+	  Multichannel Low-Voltage Remote Diode Sensor Family.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called emc1812.
+
 config IQS620AT_TEMP
 	tristate "Azoteq IQS620AT temperature sensor"
 	depends on MFD_IQS62X || COMPILE_TEST
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 07d6e65709f7fe6e1ed51867e08112c8a68ee98a..935eafb4d465dcaa3aef61ec26a1b7710a0d140f 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -3,6 +3,7 @@
 # Makefile for industrial I/O temperature drivers
 #
 
+obj-$(CONFIG_EMC1812) += emc1812.o
 obj-$(CONFIG_IQS620AT_TEMP) += iqs620at-temp.o
 obj-$(CONFIG_LTC2983) += ltc2983.o
 obj-$(CONFIG_HID_SENSOR_TEMP) += hid-sensor-temperature.o
diff --git a/drivers/iio/temperature/emc1812.c b/drivers/iio/temperature/emc1812.c
new file mode 100644
index 0000000000000000000000000000000000000000..b1f567b222026a7138e9c29d739d0240f4dc726f
--- /dev/null
+++ b/drivers/iio/temperature/emc1812.c
@@ -0,0 +1,792 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IIO driver for Microchip EMC1812/13/14/15/33 Multichannel high-accuracy 2-wire
+ * low-voltage remote diode temperature monitor family.
+ *
+ * Copyright (C) 2025 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Marius Cristea <marius.cristea@microchip.com>
+ *
+ * Datasheet can be found here:
+ * https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/EMC1812-3-4-5-33-Data-Sheet-DS20005751.pdf
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/math64.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/string.h>
+#include <linux/units.h>
+
+/* EMC1812 Registers Addresses */
+#define EMC1812_STATUS_ADDR				0x02
+#define EMC1812_CONFIG_LO_ADDR				0x03
+
+#define EMC1812_CFG_ADDR				0x09
+#define EMC1812_CONV_ADDR				0x0A
+#define EMC1812_INT_DIODE_HIGH_LIMIT_ADDR		0x0B
+#define EMC1812_INT_DIODE_LOW_LIMIT_ADDR		0x0C
+#define EMC1812_EXT1_HIGH_LIMIT_HIGH_BYTE_ADDR		0x0D
+#define EMC1812_EXT1_LOW_LIMIT_HIGH_BYTE_ADDR		0x0E
+#define EMC1812_ONE_SHOT_ADDR				0x0F
+
+#define EMC1812_EXT1_HIGH_LIMIT_LOW_BYTE_ADDR		0x13
+#define EMC1812_EXT1_LOW_LIMIT_LOW_BYTE_ADDR		0x14
+#define EMC1812_EXT2_HIGH_LIMIT_HIGH_BYTE_ADDR		0x15
+#define EMC1812_EXT2_LOW_LIMIT_HIGH_BYTE_ADDR		0x16
+#define EMC1812_EXT2_HIGH_LIMIT_LOW_BYTE_ADDR		0x17
+#define EMC1812_EXT2_LOW_LIMIT_LOW_BYTE_ADDR		0x18
+#define EMC1812_EXT1_THERM_LIMIT_ADDR			0x19
+#define EMC1812_EXT2_THERM_LIMIT_ADDR			0x1A
+#define EMC1812_EXT_DIODE_FAULT_STATUS_ADDR		0x1B
+
+#define EMC1812_DIODE_FAULT_MASK_ADDR			0x1F
+#define EMC1812_INT_DIODE_THERM_LIMIT_ADDR		0x20
+#define EMC1812_THRM_HYS_ADDR				0x21
+#define EMC1812_CONSEC_ALERT_ADDR			0x22
+
+#define EMC1812_EXT1_BETA_CONFIG_ADDR			0x25
+#define EMC1812_EXT2_BETA_CONFIG_ADDR			0x26
+#define EMC1812_EXT1_IDEALITY_FACTOR_ADDR		0x27
+#define EMC1812_EXT2_IDEALITY_FACTOR_ADDR		0x28
+
+#define EMC1812_EXT3_HIGH_LIMIT_HIGH_BYTE_ADDR		0x2C
+#define EMC1812_EXT3_LOW_LIMIT_HIGH_BYTE_ADDR		0x2D
+#define EMC1812_EXT3_HIGH_LIMIT_LOW_BYTE_ADDR		0x2E
+#define EMC1812_EXT3_LOW_LIMIT_LOW_BYTE_ADDR		0x2F
+#define EMC1812_EXT3_THERM_LIMIT_ADDR			0x30
+#define EMC1812_EXT3_IDEALITY_FACTOR_ADDR		0x31
+
+#define EMC1812_EXT4_HIGH_LIMIT_HIGH_BYTE_ADDR		0x34
+#define EMC1812_EXT4_LOW_LIMIT_HIGH_BYTE_ADDR		0x35
+#define EMC1812_EXT4_HIGH_LIMIT_LOW_BYTE_ADDR		0x36
+#define EMC1812_EXT4_LOW_LIMIT_LOW_BYTE_ADDR		0x37
+#define EMC1812_EXT4_THERM_LIMIT_ADDR			0x38
+#define EMC1812_EXT4_IDEALITY_FACTOR_ADDR		0x39
+#define EMC1812_HIGH_LIMIT_STATUS_ADDR			0x3A
+#define EMC1812_LOW_LIMIT_STATUS_ADDR			0x3B
+#define EMC1812_THERM_LIMIT_STATUS_ADDR			0x3C
+#define EMC1812_ROC_GAIN_ADDR				0x3D
+#define EMC1812_ROC_CONFIG_ADDR				0x3E
+#define EMC1812_ROC_STATUS_ADDR				0x3F
+#define EMC1812_R1_RESH_ADDR				0x40
+#define EMC1812_R1_LIMH_ADDR				0x41
+#define EMC1812_R1_LIML_ADDR				0x42
+#define EMC1812_R1_SMPL_ADDR				0x43
+#define EMC1812_R2_RESH_ADDR				0x44
+#define EMC1812_R2_3_RESL_ADDR				0x45
+#define EMC1812_R2_LIMH_ADDR				0x46
+#define EMC1812_R2_LIML_ADDR				0x47
+#define EMC1812_R2_SMPL_ADDR				0x48
+#define EMC1812_PER_MAXTH_1_ADDR			0x49
+#define EMC1812_PER_MAXT1L_ADDR				0x4A
+#define EMC1812_PER_MAXTH_2_ADDR			0x4B
+#define EMC1812_PER_MAXT2_3L_ADDR			0x4C
+#define EMC1812_GBL_MAXT1H_ADDR				0x4D
+#define EMC1812_GBL_MAXT1L_ADDR				0x4E
+#define EMC1812_GBL_MAXT2H_ADDR				0x4F
+#define EMC1812_GBL_MAXT2L_ADDR				0x50
+#define EMC1812_FILTER_SEL_ADDR				0x51
+
+#define EMC1812_INT_HIGH_BYTE_ADDR		0x60
+#define EMC1812_INT_LOW_BYTE_ADDR		0x61
+#define EMC1812_EXT1_HIGH_BYTE_ADDR		0x62
+#define EMC1812_EXT1_LOW_BYTE_ADDR		0x63
+#define EMC1812_EXT2_HIGH_BYTE_ADDR		0x64
+#define EMC1812_EXT2_LOW_BYTE_ADDR		0x65
+#define EMC1812_EXT3_HIGH_BYTE_ADDR		0x66
+#define EMC1812_EXT3_LOW_BYTE_ADDR		0x67
+#define EMC1812_EXT4_HIGH_BYTE_ADDR		0x68
+#define EMC1812_EXT4_LOW_BYTE_ADDR		0x69
+#define EMC1812_HOTTEST_DIODE_HIGH_BYTE_ADDR	0x6A
+#define EMC1812_HOTTEST_DIODE_LOW_BYTE_ADDR	0x6B
+#define EMC1812_HOTTEST_STATUS_ADDR		0x6C
+#define EMC1812_HOTTEST_CFG_ADDR		0x6D
+
+#define EMC1812_PRODUCT_ID_ADDR			0xFD
+#define EMC1812_MANUFACTURER_ID_ADDR		0xFE
+#define EMC1812_REVISION_ADDR			0xFF
+
+/* EMC1812 Config Bits */
+#define EMC1812_CFG_MSKAL			BIT(7)
+#define EMC1812_CFG_RS				BIT(6)
+#define EMC1812_CFG_ATTHM			BIT(5)
+#define EMC1812_CFG_RECD12			BIT(4)
+#define EMC1812_CFG_RECD34			BIT(3)
+#define EMC1812_CFG_RANGE			BIT(2)
+#define EMC1812_CFG_DA_ENA			BIT(1)
+#define EMC1812_CFG_APDD			BIT(0)
+
+/* EMC1812 Status Bits */
+#define EMC1812_STATUS_ROCF			BIT(7)
+#define EMC1812_STATUS_HOTCHG			BIT(6)
+#define EMC1812_STATUS_BUSY			BIT(5)
+#define EMC1812_STATUS_HIGH			BIT(4)
+#define EMC1812_STATUS_LOW			BIT(3)
+#define EMC1812_STATUS_FAULT			BIT(2)
+#define EMC1812_STATUS_ETHRM			BIT(1)
+#define EMC1812_STATUS_ITHRM			BIT(0)
+
+#define EMC1812_BETA_CFG_ADDR(index)		(0x25 + (index))
+
+#define EMC1812_CH_ADDR(index)			(0x60 + 2 * (index))
+
+#define EMC1812_FILTER_MASK_LEN			2
+
+#define EMC1812_PID				0x81
+#define EMC1813_PID				0x87
+#define EMC1814_PID				0x84
+#define EMC1815_PID				0x85
+#define EMC1833_PID				0x83
+
+/* The maximum number of channels a member of the family can have */
+#define EMC1812_MAX_NUM_CHANNELS		5
+#define EMC1812_TEMP_OFFSET			-64
+
+#define EMC1812_CHAN(index, si, __address) ({						\
+	(struct iio_chan_spec) {                                                        \
+		.type = IIO_TEMP,							\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),				\
+		.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ) |	\
+		BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),			\
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ) |		\
+		BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) |			\
+		BIT(IIO_CHAN_INFO_OFFSET) |						\
+		BIT(IIO_CHAN_INFO_SCALE),						\
+		.channel = index,							\
+		.address = __address,							\
+		.scan_index = si,							\
+		.scan_type = {								\
+			.sign = 'u',							\
+			.realbits = 8,							\
+			.storagebits = 8,						\
+		},									\
+		.indexed = 1,								\
+	};										\
+})
+
+/**
+ * struct emc1812_features - features of a emc1812 instance
+ * @name:		chip's name
+ * @phys_channels:	number of physical channels supported by the chip
+ * @lock_beta1:		lock beta compensation on channel 1 when APDD is enabled
+ * @lock_beta2:		lock beta compensation on channel 2 when APDD is enabled
+ * @allow_apdd		whether the chip supports enabling APDD
+ */
+struct emc1812_features {
+	const char	*name;
+	u8		phys_channels;
+	bool		lock_beta1;
+	bool		lock_beta2;
+};
+
+static const struct emc1812_features emc1833_chip_config = {
+	.name = "emc1833",
+	.phys_channels = 3,
+	.lock_beta1 = true,
+	.lock_beta2 = false,
+};
+
+static const struct emc1812_features emc1812_chip_config = {
+	.name = "emc1812",
+	.phys_channels = 2,
+	.lock_beta1 = false,
+	.lock_beta2 = false,
+};
+
+static const struct emc1812_features emc1813_chip_config = {
+	.name = "emc1813",
+	.phys_channels = 3,
+	.lock_beta1 = false,
+	.lock_beta2 = false,
+};
+
+static const struct emc1812_features emc1814_chip_config = {
+	.name = "emc1814",
+	.phys_channels = 4,
+	.lock_beta1 = false,
+	.lock_beta2 = true,
+};
+
+static const struct emc1812_features emc1815_chip_config = {
+	.name = "emc1815",
+	.phys_channels = 5,
+	.lock_beta1 = true,
+	.lock_beta2 = true,
+};
+
+static const unsigned int emc1812_freq[][2] = {
+	{  0,  62500 },
+	{  0, 125000 },
+	{  0, 250000 },
+	{  0, 500000 },
+	{  1,      0 },
+	{  2,      0 },
+	{  4,      0 },
+	{  8,      0 },
+	{ 16,      0 },
+	{ 32,      0 },
+	{ 64,      0 },
+};
+
+/*
+ * For moving average filter with window size 4 and 8, constants were calculated
+ * using: F(@-3db) = Sampling_Frequency(Hz) / (Window_Size * 2)
+ * In case the moving average filter is disabled we will print the sampling frequency.
+ */
+static const unsigned int emc1812_3db_values_map_tbl[11][3][2] = {
+	{ {  0,  62500 }, { 0,   7813 }, { 0,   3906 }, },
+	{ {  0, 125000 }, { 0,  15625 }, { 0,   7813 }, },
+	{ {  0, 250000 }, { 0,  31250 }, { 0,  15625 }, },
+	{ {  0, 500000 }, { 0,  62500 }, { 0,  31250 }, },
+	{ {  1,      0 }, { 0, 125000 }, { 0,  62500 }, },
+	{ {  2,      0 }, { 0, 250000 }, { 0, 125000 }, },
+	{ {  4,      0 }, { 0, 500000 }, { 0, 250000 }, },
+	{ {  8,      0 }, { 1,      0 }, { 0, 500000 }, },
+	{ { 16,      0 }, { 2,      0 }, { 1,      0 }, },
+	{ { 32,      0 }, { 4,      0 }, { 2,      0 }, },
+	{ { 64,      0 }, { 8,      0 }, { 4,      0 }, },
+};
+
+static const unsigned int emc1812_window_size[3] = { 1, 4, 8 };
+
+/**
+ * struct emc1812_priv - information about chip parameters
+ * @labels:		labels of the channels
+ * @chip:		pointer to structure holding chip features
+ * @ideality_value:	ideality factor value for each external channel
+ * @iio_chan:		specifications for each channel
+ * @beta_values:	beta compensation value for external channel 1 and 2
+ * @freq_idx:		index representing the current sampling frequency
+ * @regmap:		device register map
+ * @recd34_en:		state of Resistance Error Correction (REC) on channels 3 and 4
+ * @recd12_en:		state of Resistance Error Correction (REC) on channels 1 and 2
+ * @lock:		synchronize access to driver's state members
+ * @apdd_en:		state of anti-parallel diode mode
+ * @num_channels:	number of active physical channels
+ */
+struct emc1812_priv {
+	const char *labels[EMC1812_MAX_NUM_CHANNELS];
+	const struct emc1812_features *chip;
+	unsigned int ideality_value[4];
+	struct iio_chan_spec iio_chan[EMC1812_MAX_NUM_CHANNELS];
+	unsigned int beta_values[2];
+	unsigned int freq_idx;
+	struct regmap *regmap;
+	bool recd34_en;
+	bool recd12_en;
+	struct mutex lock;	 /* Synchronize access to driver's state members */
+	bool apdd_en;
+	u8 num_channels;
+};
+
+/* emc1812 regmap configuration */
+static const struct regmap_range emc1812_regmap_writable_ranges[] = {
+	regmap_reg_range(EMC1812_CFG_ADDR, EMC1812_ONE_SHOT_ADDR),
+	regmap_reg_range(EMC1812_EXT1_HIGH_LIMIT_LOW_BYTE_ADDR,
+			 EMC1812_EXT_DIODE_FAULT_STATUS_ADDR),
+	regmap_reg_range(EMC1812_DIODE_FAULT_MASK_ADDR, EMC1812_CONSEC_ALERT_ADDR),
+	regmap_reg_range(EMC1812_EXT1_BETA_CONFIG_ADDR, EMC1812_FILTER_SEL_ADDR),
+	regmap_reg_range(EMC1812_HOTTEST_STATUS_ADDR, EMC1812_HOTTEST_CFG_ADDR),
+};
+
+static const struct regmap_access_table emc1812_regmap_wr_table = {
+	.yes_ranges = emc1812_regmap_writable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(emc1812_regmap_writable_ranges),
+};
+
+static const struct regmap_range emc1812_regmap_rd_ranges[] = {
+	regmap_reg_range(EMC1812_STATUS_ADDR, EMC1812_CONFIG_LO_ADDR),
+	regmap_reg_range(EMC1812_CFG_ADDR, EMC1812_ONE_SHOT_ADDR),
+	regmap_reg_range(EMC1812_EXT1_HIGH_LIMIT_LOW_BYTE_ADDR,
+			 EMC1812_EXT_DIODE_FAULT_STATUS_ADDR),
+	regmap_reg_range(EMC1812_DIODE_FAULT_MASK_ADDR, EMC1812_CONSEC_ALERT_ADDR),
+	regmap_reg_range(EMC1812_EXT1_BETA_CONFIG_ADDR, EMC1812_FILTER_SEL_ADDR),
+	regmap_reg_range(EMC1812_INT_HIGH_BYTE_ADDR, EMC1812_HOTTEST_CFG_ADDR),
+	regmap_reg_range(EMC1812_PRODUCT_ID_ADDR, EMC1812_REVISION_ADDR),
+};
+
+static const struct regmap_access_table emc1812_regmap_rd_table = {
+	.yes_ranges = emc1812_regmap_rd_ranges,
+	.n_yes_ranges = ARRAY_SIZE(emc1812_regmap_rd_ranges),
+};
+
+static bool emc1812_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case EMC1812_STATUS_ADDR:
+	case EMC1812_EXT_DIODE_FAULT_STATUS_ADDR:
+	case EMC1812_DIODE_FAULT_MASK_ADDR:
+
+	case EMC1812_EXT1_BETA_CONFIG_ADDR:
+	case EMC1812_EXT2_BETA_CONFIG_ADDR:
+
+	case EMC1812_HIGH_LIMIT_STATUS_ADDR:
+	case EMC1812_LOW_LIMIT_STATUS_ADDR:
+	case EMC1812_THERM_LIMIT_STATUS_ADDR:
+	case EMC1812_ROC_STATUS_ADDR:
+	case EMC1812_PER_MAXTH_1_ADDR:
+	case EMC1812_PER_MAXT1L_ADDR:
+	case EMC1812_PER_MAXTH_2_ADDR:
+	case EMC1812_PER_MAXT2_3L_ADDR:
+	case EMC1812_GBL_MAXT1H_ADDR:
+	case EMC1812_GBL_MAXT1L_ADDR:
+	case EMC1812_GBL_MAXT2H_ADDR:
+	case EMC1812_GBL_MAXT2L_ADDR:
+	case EMC1812_INT_HIGH_BYTE_ADDR:
+	case EMC1812_INT_LOW_BYTE_ADDR:
+	case EMC1812_EXT1_HIGH_BYTE_ADDR:
+	case EMC1812_EXT1_LOW_BYTE_ADDR:
+	case EMC1812_EXT2_HIGH_BYTE_ADDR:
+	case EMC1812_EXT2_LOW_BYTE_ADDR:
+	case EMC1812_EXT3_HIGH_BYTE_ADDR:
+	case EMC1812_EXT3_LOW_BYTE_ADDR:
+	case EMC1812_EXT4_HIGH_BYTE_ADDR:
+	case EMC1812_EXT4_LOW_BYTE_ADDR:
+	case EMC1812_HOTTEST_DIODE_HIGH_BYTE_ADDR:
+	case EMC1812_HOTTEST_DIODE_LOW_BYTE_ADDR:
+	case EMC1812_HOTTEST_STATUS_ADDR:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config emc1812_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.rd_table = &emc1812_regmap_rd_table,
+	.wr_table = &emc1812_regmap_wr_table,
+	.volatile_reg = emc1812_is_volatile_reg,
+	.max_register = EMC1812_REVISION_ADDR,
+};
+
+static int emc1812_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			      const int **vals, int *type, int *length, long mask)
+{
+	struct emc1812_priv *priv = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = emc1812_freq[0];
+		*length = ARRAY_SIZE(emc1812_freq) * 2;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		*vals = emc1812_3db_values_map_tbl[priv->freq_idx][0];
+		*length = ARRAY_SIZE(emc1812_3db_values_map_tbl[priv->freq_idx]) * 2;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int emc1812_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	struct emc1812_priv *priv = iio_priv(indio_dev);
+	unsigned int idx, tmp_reg;
+	__be16 tmp_be16;
+	int ret;
+
+	guard(mutex)(&priv->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_bulk_read(priv->regmap, EMC1812_CH_ADDR(chan->channel),
+				       &tmp_be16, sizeof(tmp_be16));
+		if (ret)
+			return ret;
+
+		*val = be16_to_cpu(tmp_be16);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = 3906250;
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = emc1812_freq[priv->freq_idx][0];
+		*val2 = emc1812_freq[priv->freq_idx][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		ret = regmap_read(priv->regmap, EMC1812_FILTER_SEL_ADDR, &tmp_reg);
+		if (ret)
+			return ret;
+
+		idx = hweight32(tmp_reg);
+		*val = emc1812_3db_values_map_tbl[priv->freq_idx][idx][0];
+		*val2 = emc1812_3db_values_map_tbl[priv->freq_idx][idx][1];
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = EMC1812_TEMP_OFFSET;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int emc1812_read_label(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan, char *label)
+{
+	struct emc1812_priv *priv = iio_priv(indio_dev);
+
+	if (chan->channel >= EMC1812_MAX_NUM_CHANNELS)
+		return -EINVAL;
+
+	return sysfs_emit(label, "%s\n", priv->labels[chan->channel]);
+}
+
+static int emc1812_write_raw_get_fmt(struct iio_dev *indio_dev,
+				     struct iio_chan_spec const *chan, long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int emc1812_write_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int val,
+			     int val2, long mask)
+{
+	struct emc1812_priv *priv = iio_priv(indio_dev);
+	unsigned int i;
+	int ret;
+
+	guard(mutex)(&priv->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		for (i = 0; i < ARRAY_SIZE(emc1812_freq); i++)
+			if (val == emc1812_freq[i][0] && val2 == emc1812_freq[i][1])
+				break;
+
+		if (i == ARRAY_SIZE(emc1812_freq))
+			return -EINVAL;
+
+		ret = regmap_write(priv->regmap, EMC1812_CONV_ADDR, i);
+		if (ret)
+			return ret;
+
+		priv->freq_idx = i;
+		break;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
+		for (i = 0; i < ARRAY_SIZE(emc1812_3db_values_map_tbl[priv->freq_idx]); i++)
+			if (val == emc1812_3db_values_map_tbl[priv->freq_idx][i][0] &&
+			    val2 == emc1812_3db_values_map_tbl[priv->freq_idx][i][1])
+				break;
+
+		if (i == ARRAY_SIZE(emc1812_3db_values_map_tbl[priv->freq_idx]))
+			return -EINVAL;
+
+		/*
+		 * In emc1812_3db_values_map_tbl the second index maps:
+		 * 0 for filter off
+		 * 1 for filter at level 1
+		 * 2 for filter at level 2
+		 */
+		if (i == 2)
+			i = 3;
+
+		ret = regmap_write(priv->regmap, EMC1812_FILTER_SEL_ADDR, i);
+		if (ret)
+			return ret;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct iio_info emc1812_info = {
+	.read_raw = emc1812_read_raw,
+	.read_label = emc1812_read_label,
+	.read_avail = emc1812_read_avail,
+	.write_raw_get_fmt = emc1812_write_raw_get_fmt,
+	.write_raw = emc1812_write_raw,
+};
+
+static int emc1812_init(struct emc1812_priv *priv)
+{
+	unsigned int i;
+	int ret;
+	u8 val;
+
+	/*
+	 * Depending on the chip, lock channel beta 1 and/or 2 to Diode Mode
+	 * when APDD is enabled.
+	 */
+	if (priv->chip->lock_beta1 && priv->apdd_en)
+		priv->beta_values[0] = 0x0F;
+	if (priv->chip->lock_beta2 && priv->apdd_en)
+		priv->beta_values[1] = 0x0F;
+
+	/*
+	 * Set default values in registers. APDD, RECD12 and RECD34 are active
+	 * on 0.
+	 * Set the device to be in Run (Active) state and converting on all
+	 * channels.
+	 * The temperature measurement range is -64°C to +191.875°C.
+	 */
+	val = FIELD_PREP(EMC1812_CFG_MSKAL, 1) |
+	      FIELD_PREP(EMC1812_CFG_RS, 0) |
+	      FIELD_PREP(EMC1812_CFG_ATTHM, 1) |
+	      FIELD_PREP(EMC1812_CFG_RECD12, !priv->recd12_en) |
+	      FIELD_PREP(EMC1812_CFG_RECD34, !priv->recd34_en) |
+	      FIELD_PREP(EMC1812_CFG_RANGE, 1) |
+	      FIELD_PREP(EMC1812_CFG_DA_ENA, 0) |
+	      FIELD_PREP(EMC1812_CFG_APDD, !priv->apdd_en);
+
+	ret = regmap_write(priv->regmap, EMC1812_CFG_ADDR, val);
+	if (ret)
+		return ret;
+
+	/* Default is 4 conversions/seconds */
+	ret = regmap_write(priv->regmap, EMC1812_CONV_ADDR, 6);
+	if (ret)
+		return ret;
+	priv->freq_idx = 6;
+
+	ret = regmap_write(priv->regmap, EMC1812_THRM_HYS_ADDR, 0x0A);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->regmap, EMC1812_CONSEC_ALERT_ADDR, 0x70);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->regmap, EMC1812_FILTER_SEL_ADDR, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->regmap, EMC1812_HOTTEST_CFG_ADDR, 0);
+	if (ret)
+		return ret;
+
+	/* Set beta1 and beta2 compensation parameters */
+	for (i = 0; i < ARRAY_SIZE(priv->beta_values); i++) {
+		ret = regmap_write(priv->regmap, EMC1812_BETA_CFG_ADDR(i),
+				   priv->beta_values[i]);
+		if (ret)
+			return ret;
+	}
+
+	/* Set ideality factor for all external channels */
+	ret = regmap_write(priv->regmap, EMC1812_EXT1_IDEALITY_FACTOR_ADDR,
+			   priv->ideality_value[0]);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->regmap, EMC1812_EXT2_IDEALITY_FACTOR_ADDR,
+			   priv->ideality_value[1]);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->regmap, EMC1812_EXT3_IDEALITY_FACTOR_ADDR,
+			   priv->ideality_value[2]);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(priv->regmap, EMC1812_EXT4_IDEALITY_FACTOR_ADDR,
+			   priv->ideality_value[3]);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int emc1812_parse_fw_config(struct emc1812_priv *priv, struct device *dev,
+				   int device_nr_channels)
+{
+	unsigned int reg_nr, iio_idx, tmp;
+	int ret;
+
+	priv->apdd_en = device_property_read_bool(dev, "microchip,enable-anti-parallel");
+	priv->recd12_en = device_property_read_bool(dev, "microchip,parasitic-res-on-channel1-2");
+	priv->recd34_en = device_property_read_bool(dev, "microchip,parasitic-res-on-channel3-4");
+
+	memset32(priv->beta_values, 16, ARRAY_SIZE(priv->beta_values));
+	device_property_read_u32(dev, "microchip,beta1", &priv->beta_values[0]);
+	device_property_read_u32(dev, "microchip,beta2", &priv->beta_values[1]);
+	if (priv->beta_values[0] > 16 || priv->beta_values[1] > 16)
+		return dev_err_probe(dev, -EINVAL, "Invalid beta value\n");
+
+	priv->num_channels = device_get_child_node_count(dev) + 1;
+
+	if (priv->num_channels > priv->chip->phys_channels)
+		return dev_err_probe(dev, -E2BIG, "More channels than the chip supports\n");
+
+	priv->iio_chan[0] = EMC1812_CHAN(0, 0, EMC1812_CH_ADDR(0));
+
+	priv->labels[0] = "internal_diode";
+	iio_idx = 1;
+	device_for_each_child_node_scoped(dev, child) {
+		ret = fwnode_property_read_u32(child, "reg", &reg_nr);
+		if (ret || reg_nr >= priv->chip->phys_channels)
+			return dev_err_probe(dev, -EINVAL,
+				     "The index of the channels does not match the chip\n");
+
+		ret = fwnode_property_read_u32(child, "microchip,ideality-factor", &tmp);
+		if (ret == 0) {
+			if (tmp < 8  || tmp > 63)
+				return dev_err_probe(dev, ret, "Invalid ideality value\n");
+			priv->ideality_value[reg_nr - 1] = tmp;
+		} else {
+			priv->ideality_value[reg_nr - 1] = 18;
+		}
+
+		fwnode_property_read_string(child, "label", &priv->labels[reg_nr]);
+
+		priv->iio_chan[iio_idx++] = EMC1812_CHAN(reg_nr, reg_nr, EMC1812_CH_ADDR(reg_nr));
+	}
+
+	return 0;
+}
+
+static int emc1812_chip_identify(struct emc1812_priv *priv, struct i2c_client *client)
+{
+	int ret, tmp;
+
+	ret = regmap_read(priv->regmap, EMC1812_PRODUCT_ID_ADDR, &tmp);
+	if (ret)
+		return ret;
+
+	switch (tmp) {
+	case EMC1812_PID:
+		priv->chip = &emc1812_chip_config;
+		break;
+	case EMC1813_PID:
+		priv->chip = &emc1813_chip_config;
+		break;
+	case EMC1814_PID:
+		priv->chip = &emc1814_chip_config;
+		break;
+	case EMC1815_PID:
+		priv->chip = &emc1815_chip_config;
+		break;
+	case EMC1833_PID:
+		priv->chip = &emc1833_chip_config;
+		break;
+	default:
+		/*
+		 * If failed to identify the hardware based on internal registers,
+		 * try using fallback compatible in device tree to deal with some
+		 * newer part number.
+		 */
+		priv->chip = i2c_get_match_data(client);
+		if (!priv->chip)
+			return -EINVAL;
+		break;
+	}
+	return 0;
+}
+
+static int emc1812_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct emc1812_priv *priv;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	priv = iio_priv(indio_dev);
+	priv->regmap = devm_regmap_init_i2c(client, &emc1812_regmap_config);
+	if (IS_ERR(priv->regmap))
+		return dev_err_probe(dev, PTR_ERR(priv->regmap),
+				     "Cannot initialize register map\n");
+
+	ret = devm_mutex_init(dev, &priv->lock);
+	if (ret)
+		return ret;
+
+	ret = emc1812_chip_identify(priv, client);
+	if (ret)
+		return dev_err_probe(dev, ret, "Chip identification fails\n");
+
+	dev_info(dev, "Device name: %s\n", priv->chip->name);
+
+	ret = emc1812_parse_fw_config(priv, dev, priv->chip->phys_channels);
+	if (ret)
+		return ret;
+
+	ret = emc1812_init(priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Cannot initialize device\n");
+
+	indio_dev->name = priv->chip->name;
+	indio_dev->info = &emc1812_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = &priv->iio_chan[0];
+	indio_dev->num_channels = priv->num_channels;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Cannot register IIO device\n");
+
+	return 0;
+}
+
+static const struct i2c_device_id emc1812_id[] = {
+	{ .name = "emc1812", .driver_data = (kernel_ulong_t)&emc1812_chip_config },
+	{ .name = "emc1813", .driver_data = (kernel_ulong_t)&emc1813_chip_config },
+	{ .name = "emc1814", .driver_data = (kernel_ulong_t)&emc1814_chip_config },
+	{ .name = "emc1815", .driver_data = (kernel_ulong_t)&emc1815_chip_config },
+	{ .name = "emc1833", .driver_data = (kernel_ulong_t)&emc1833_chip_config },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, emc1812_id);
+
+static const struct of_device_id emc1812_of_match[] = {
+	{
+		.compatible = "microchip,emc1812",
+		.data = &emc1812_chip_config
+	},
+	{
+		.compatible = "microchip,emc1813",
+		.data = &emc1813_chip_config
+	},
+	{
+		.compatible = "microchip,emc1814",
+		.data = &emc1814_chip_config
+	},
+	{
+		.compatible = "microchip,emc1815",
+		.data = &emc1815_chip_config
+	},
+	{
+		.compatible = "microchip,emc1833",
+		.data = &emc1833_chip_config
+	},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, emc1812_of_match);
+
+static struct i2c_driver emc1812_driver = {
+	.driver	 = {
+		.name = "emc1812",
+		.of_match_table = emc1812_of_match,
+	},
+	.probe = emc1812_probe,
+	.id_table = emc1812_id,
+};
+module_i2c_driver(emc1812_driver);
+
+MODULE_AUTHOR("Marius Cristea <marius.cristea@microchip.com>");
+MODULE_DESCRIPTION("EMC1812/13/14/15/33 high-accuracy remote diode temperature monitor Driver");
+MODULE_LICENSE("GPL");

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Add support for Microchip EMC1812
  2025-09-17 12:21 [PATCH 0/2] Add support for Microchip EMC1812 Marius Cristea
  2025-09-17 12:21 ` [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812 Marius Cristea
  2025-09-17 12:21 ` [PATCH 2/2] " Marius Cristea
@ 2025-09-17 13:25 ` David Lechner
  2025-09-17 13:30   ` Marius.Cristea
  2025-09-20 11:33 ` Jonathan Cameron
  3 siblings, 1 reply; 19+ messages in thread
From: David Lechner @ 2025-09-17 13:25 UTC (permalink / raw)
  To: Marius Cristea, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-kernel

On 9/17/25 7:21 AM, Marius Cristea wrote:
> This is the iio driver for EMC1812/13/14/15/33 multichannel Low-Voltage
> Remote Diode Sensor Family. The chips in the family have one internal
> and different numbers of external channels, ranging from 1 (EMC1812) to
> 4 channels (EMC1815).
> Reading diodes in anti-parallel connection is supported by EMC1814, EMC1815
> and EMC1833.
> 
> Current version of driver does not support interrupts, events and data
> buffering.
> 
> Differences related to previous patch:

This is confusing. I think this version is v1, so there is
no previous patch. So why does this say "previous patch"?
Is this actually v2?

> 
> v1:
> - initial version.
> 
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> ---
> Marius Cristea (2):
>       dt-bindings: iio: temperature: add support for EMC1812
>       iio: temperature: add support for EMC1812
> 
>  .../iio/temperature/microchip,emc1812.yaml         | 223 ++++++
>  MAINTAINERS                                        |   7 +
>  drivers/iio/temperature/Kconfig                    |  10 +
>  drivers/iio/temperature/Makefile                   |   1 +
>  drivers/iio/temperature/emc1812.c                  | 792 +++++++++++++++++++++
>  5 files changed, 1033 insertions(+)
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20250805-iio-emc1812-e666183b07b5
> 
> Best regards,


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Add support for Microchip EMC1812
  2025-09-17 13:25 ` [PATCH 0/2] Add support for Microchip EMC1812 David Lechner
@ 2025-09-17 13:30   ` Marius.Cristea
  2025-09-17 13:38     ` David Lechner
  0 siblings, 1 reply; 19+ messages in thread
From: Marius.Cristea @ 2025-09-17 13:30 UTC (permalink / raw)
  To: robh, krzk+dt, jic23, nuno.sa, dlechner, conor+dt, andy
  Cc: devicetree, linux-kernel, linux-iio

Hi David,

> > 
> > Current version of driver does not support interrupts, events and
> > data
> > buffering.
> > 
> > Differences related to previous patch:
> 
> This is confusing. I think this version is v1, so there is
> no previous patch. So why does this say "previous patch"?
> Is this actually v2?
> 

No, this is the first patch. I put it there as a patch history and fill
it up as the patch versioning progress.

> > 
> > v1:
> > - initial version.
> > 
> > Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> > ---
> > Marius Cristea (2):
> >       dt-bindings: iio: temperature: add support for EMC1812
> >       iio: temperature: add support for EMC1812
> > 
> >  .../iio/temperature/microchip,emc1812.yaml         | 223 ++++++
> >  MAINTAINERS                                        |   7 +
> >  drivers/iio/temperature/Kconfig                    |  10 +
> >  drivers/iio/temperature/Makefile                   |   1 +
> >  drivers/iio/temperature/emc1812.c                  | 792
> > +++++++++++++++++++++
> >  5 files changed, 1033 insertions(+)
> > ---
> > base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> > change-id: 20250805-iio-emc1812-e666183b07b5
> > 
> > Best regards,

Best regards,
Marius

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Add support for Microchip EMC1812
  2025-09-17 13:30   ` Marius.Cristea
@ 2025-09-17 13:38     ` David Lechner
  0 siblings, 0 replies; 19+ messages in thread
From: David Lechner @ 2025-09-17 13:38 UTC (permalink / raw)
  To: Marius.Cristea, robh, krzk+dt, jic23, nuno.sa, conor+dt, andy
  Cc: devicetree, linux-kernel, linux-iio

On 9/17/25 8:30 AM, Marius.Cristea@microchip.com wrote:
> Hi David,
> 
>>>
>>> Current version of driver does not support interrupts, events and
>>> data
>>> buffering.
>>>
>>> Differences related to previous patch:
>>
>> This is confusing. I think this version is v1, so there is
>> no previous patch. So why does this say "previous patch"?
>> Is this actually v2?
>>
> 
> No, this is the first patch. I put it there as a patch history and fill
> it up as the patch versioning progress.
> 
Got it. Next time, it would be better to not add the changelog
until v2. We are not used to seeing that in v1. :-)


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812
  2025-09-17 12:21 ` [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812 Marius Cristea
@ 2025-09-17 14:31   ` Rob Herring (Arm)
  2025-09-17 23:34   ` David Lechner
  1 sibling, 0 replies; 19+ messages in thread
From: Rob Herring (Arm) @ 2025-09-17 14:31 UTC (permalink / raw)
  To: Marius Cristea
  Cc: Nuno Sá, linux-kernel, Jonathan Cameron, Andy Shevchenko,
	Conor Dooley, devicetree, Krzysztof Kozlowski, linux-iio,
	David Lechner


On Wed, 17 Sep 2025 15:21:57 +0300, Marius Cristea wrote:
> This is the devicetree schema for Microchip EMC1812/13/14/15/33
> Multichannel Low-Voltage Remote Diode Sensor Family.
> 
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> ---
>  .../iio/temperature/microchip,emc1812.yaml         | 223 +++++++++++++++++++++
>  MAINTAINERS                                        |   6 +
>  2 files changed, 229 insertions(+)
> 

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,emc1812.yaml: allOf:1:then:patternProperties: '^channel@1$' should not be valid under {'pattern': '^\\^[a-zA-Z0-9,\\-._#@]+\\$$'}
	hint: Fixed strings belong in 'properties', not 'patternProperties'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml: allOf:3:then:patternProperties: '^channel@4$' should not be valid under {'pattern': '^\\^[a-zA-Z0-9,\\-._#@]+\\$$'}
	hint: Fixed strings belong in 'properties', not 'patternProperties'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.example.dtb: temperature-sensor@4c (microchip,emc1813): channel@2:reg:0:0: 2 is greater than the maximum of 1
	from schema $id: http://devicetree.org/schemas/iio/temperature/microchip,emc1812.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250917-iio-emc1812-v1-1-0b1f74cea7ab@microchip.com

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.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812
  2025-09-17 12:21 ` [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812 Marius Cristea
  2025-09-17 14:31   ` Rob Herring (Arm)
@ 2025-09-17 23:34   ` David Lechner
  2025-09-25 14:27     ` Marius.Cristea
  1 sibling, 1 reply; 19+ messages in thread
From: David Lechner @ 2025-09-17 23:34 UTC (permalink / raw)
  To: Marius Cristea, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: linux-iio, devicetree, linux-kernel

On 9/17/25 7:21 AM, Marius Cristea wrote:
> This is the devicetree schema for Microchip EMC1812/13/14/15/33
> Multichannel Low-Voltage Remote Diode Sensor Family.
> 
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> ---
>  .../iio/temperature/microchip,emc1812.yaml         | 223 +++++++++++++++++++++
>  MAINTAINERS                                        |   6 +
>  2 files changed, 229 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml b/Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..898d6d246746e229cb004f447872ee6bd5a65074
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/microchip,emc1812.yaml
> @@ -0,0 +1,223 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/temperature/microchip,emc1812.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip EMC1812/13/14/15/33 multichannel temperature sensor
> +
> +maintainers:
> +  - Marius Cristea <marius.cristea@microchip.com>
> +
> +description: |
> +  The Microchip EMC1812/13/14/15/33 is a high-accuracy 2-wire multichannel
> +  low-voltage remote diode temperature monitor.
> +
> +  The datasheet can be found here:
> +    https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/EMC1812-3-4-5-33-Data-Sheet-DS20005751.pdf

The pinouts of these chips look nearly identical to MCP998X.
Would it make sense to share a single bindings document for these?
Or maybe there would be too many if: blocks and keeping it separate
is fine.

https://lore.kernel.org/linux-iio/20250829143447.18893-2-victor.duicu@microchip.com/

> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,emc1812
> +      - microchip,emc1813
> +      - microchip,emc1814
> +      - microchip,emc1815
> +      - microchip,emc1833
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 2
> +
> +  interrupt-names:
> +    description:
> +      -alert-therm2 asserts when a diode temperature exceeds the ALERT
> +      threshold.
> +      -therm-addr asserts low when the hardware-set THERM limit threshold is
> +      exceeded by one of the temperature sensors.
> +    items:
> +      - const: alert-therm2
> +      - const: therm-addr
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  microchip,beta1:
> +    description:
> +      Set beta compensation value for external channel 1.
> +      <0> 0.050
> +      <1> 0.066
> +      <2> 0.087
> +      <3> 0.114
> +      <4> 0.150
> +      <5> 0.197
> +      <6> 0.260
> +      <7> 0.342
> +      <8> 0.449
> +      <9> 0.591
> +      <10> 0.778
> +      <11> 1.024
> +      <12> 1.348
> +      <13> 1.773
> +      <14> 2.333
> +      <15> Diode_Mode
> +      <16> Auto
> +      - Diode_Mode is used when measuring a discrete thermal diode
> +      or a CPU diode that functions like a discrete thermal diode.
> +      - Auto enables beta auto-detection. The chip monitors
> +      external diode/transistor and determines the optimum
> +      setting.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 16
> +    default: 16
> +
> +  microchip,beta2:
> +    description:
> +      Set beta compensation value for external channel 2.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 16
> +    default: 16

This beta value sounds like something that might not belong in the
devicetree.

The datasheet says that auto is always the best. So that makes me
wonder when would we want to use something else?

Also, it says that in auto mode, that the value is recalculated
on every conversion, so even if manually selecting a value, it
sounds like something that could change at runtime, so having a
fixed value might not cover all use cases.

Having a boolean flag to say this is wired to a discrete thermal diode
makes sense though (the driver would use this info to select diode mode).

And if we can make the case that the beta value should be in the
devicetree, then having the actual value instead of a lookup table
would be preferred. There is a "basis points" standards unit (suffix
"-bp") that can be used for non-integer values like this (assuming it
is a unit-less value). It is 1/10,000 so it would make the property
an enum with 15 values between 500 and 23330.

Both properties would not be allowed at the same time and if both
properties are omitted, the driver would know to use auto mode.

Also, it would make more sense to have these as channel properties
if they only apply to 1 channel each.

> +
> +  microchip,parasitic-res-on-channel1-2:
> +    description:
> +      Indicates that the chip and the diodes/transistors are sufficiently far
> +      apart that a parasitic resistance is added to the wires, which can affect
> +      the measurements. Due to the anti-parallel diode connections, channels
> +      1 and 2 are affected together.
> +    type: boolean
> +
> +  microchip,parasitic-res-on-channel3-4:
> +    description:
> +      Indicates that the chip and the diodes/transistors are sufficiently far
> +      apart that a parasitic resistance is added to the wires, which can affect
> +      the measurements. Due to the anti-parallel diode connections, channels
> +      3 and 4 are affected together.
> +    type: boolean
> +
> +  vdd-supply: true
> +
> +patternProperties:
> +  "^channel@[1-4]$":
> +    description:
> +      Represents the external temperature channels to which
> +      a remote diode is connected.
> +    type: object
> +
> +    properties:
> +      reg:
> +        items:
> +          minimum: 1
> +          maximum: 4
> +

I.e. beta-related properties would go here.

> +      microchip,ideality-factor:
> +        description:
> +          Each channel has an ideality factor.
> +          Beta compensation and resistance error correction automatically
> +          correct for most ideality errors. So ideality factor does not need
> +          to be adjusted in general.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 8
> +        maximum: 55
> +        default: 18
> +

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] iio: temperature: add support for EMC1812
  2025-09-17 12:21 ` [PATCH 2/2] " Marius Cristea
@ 2025-09-18  3:07   ` kernel test robot
  2025-09-20 11:51   ` Jonathan Cameron
  2025-09-23  5:31   ` Dan Carpenter
  2 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2025-09-18  3:07 UTC (permalink / raw)
  To: Marius Cristea, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
  Cc: oe-kbuild-all, linux-iio, devicetree, linux-kernel,
	Marius Cristea

Hi Marius,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 19272b37aa4f83ca52bdf9c16d5d81bdd1354494]

url:    https://github.com/intel-lab-lkp/linux/commits/Marius-Cristea/dt-bindings-iio-temperature-add-support-for-EMC1812/20250917-202833
base:   19272b37aa4f83ca52bdf9c16d5d81bdd1354494
patch link:    https://lore.kernel.org/r/20250917-iio-emc1812-v1-2-0b1f74cea7ab%40microchip.com
patch subject: [PATCH 2/2] iio: temperature: add support for EMC1812
config: loongarch-allyesconfig (https://download.01.org/0day-ci/archive/20250918/202509181058.dx3hT9N3-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 7c861bcedf61607b6c087380ac711eb7ff918ca6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250918/202509181058.dx3hT9N3-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/202509181058.dx3hT9N3-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from <built-in>:3:
   In file included from include/linux/compiler_types.h:171:
   include/linux/compiler-clang.h:28:9: warning: '__SANITIZE_ADDRESS__' macro redefined [-Wmacro-redefined]
      28 | #define __SANITIZE_ADDRESS__
         |         ^
   <built-in>:371:9: note: previous definition is here
     371 | #define __SANITIZE_ADDRESS__ 1
         |         ^
>> drivers/iio/temperature/emc1812.c:259:27: warning: unused variable 'emc1812_window_size' [-Wunused-const-variable]
     259 | static const unsigned int emc1812_window_size[3] = { 1, 4, 8 };
         |                           ^~~~~~~~~~~~~~~~~~~
   2 warnings generated.


vim +/emc1812_window_size +259 drivers/iio/temperature/emc1812.c

   258	
 > 259	static const unsigned int emc1812_window_size[3] = { 1, 4, 8 };
   260	

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Add support for Microchip EMC1812
  2025-09-17 12:21 [PATCH 0/2] Add support for Microchip EMC1812 Marius Cristea
                   ` (2 preceding siblings ...)
  2025-09-17 13:25 ` [PATCH 0/2] Add support for Microchip EMC1812 David Lechner
@ 2025-09-20 11:33 ` Jonathan Cameron
  2025-09-24  2:11   ` Guenter Roeck
  3 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2025-09-20 11:33 UTC (permalink / raw)
  To: Marius Cristea
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree,
	linux-kernel, jdelvare, linux, linux-hwmon

On Wed, 17 Sep 2025 15:21:56 +0300
Marius Cristea <marius.cristea@microchip.com> wrote:

> This is the iio driver for EMC1812/13/14/15/33 multichannel Low-Voltage
> Remote Diode Sensor Family. The chips in the family have one internal
> and different numbers of external channels, ranging from 1 (EMC1812) to
> 4 channels (EMC1815).
> Reading diodes in anti-parallel connection is supported by EMC1814, EMC1815
> and EMC1833.
> 
> Current version of driver does not support interrupts, events and data
> buffering.
Hi Marius,

For a temperature monitoring device like this, the opening question is
always why not HWMON?

There are various reasons we have temp sensors in IIO but mostly they are not
described as being monitors and this one is.

IIO may well be the right choice for this part, but good to lay out your
reasoning and +CC the hwmon list and maintainers.  There is an emc1403
driver already in hwmon, so perhaps compare and contrast with that.

I've +CC Jean, Guenter and list to save sending a v2 just to do that.

Jonathan


> 
> Differences related to previous patch:
> 
> v1:
> - initial version.
> 
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> ---
> Marius Cristea (2):
>       dt-bindings: iio: temperature: add support for EMC1812
>       iio: temperature: add support for EMC1812
> 
>  .../iio/temperature/microchip,emc1812.yaml         | 223 ++++++
>  MAINTAINERS                                        |   7 +
>  drivers/iio/temperature/Kconfig                    |  10 +
>  drivers/iio/temperature/Makefile                   |   1 +
>  drivers/iio/temperature/emc1812.c                  | 792 +++++++++++++++++++++
>  5 files changed, 1033 insertions(+)
> ---
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> change-id: 20250805-iio-emc1812-e666183b07b5
> 
> Best regards,


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] iio: temperature: add support for EMC1812
  2025-09-17 12:21 ` [PATCH 2/2] " Marius Cristea
  2025-09-18  3:07   ` kernel test robot
@ 2025-09-20 11:51   ` Jonathan Cameron
  2025-09-20 15:05     ` David Lechner
  2025-09-23  5:31   ` Dan Carpenter
  2 siblings, 1 reply; 19+ messages in thread
From: Jonathan Cameron @ 2025-09-20 11:51 UTC (permalink / raw)
  To: Marius Cristea
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree,
	linux-kernel

On Wed, 17 Sep 2025 15:21:58 +0300
Marius Cristea <marius.cristea@microchip.com> wrote:

> This is the iio driver for Microchip EMC1812/13/14/15/33
> Multichannel Low-Voltage Remote Diode Sensor Family.
> 
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>

A few minor comments inline,

Thanks,

Jonathan

> diff --git a/drivers/iio/temperature/emc1812.c b/drivers/iio/temperature/emc1812.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..b1f567b222026a7138e9c29d739d0240f4dc726f
> --- /dev/null
> +++ b/drivers/iio/temperature/emc1812.c
> @@ -0,0 +1,792 @@


> +
> +static int emc1812_write_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int val,
> +			     int val2, long mask)
> +{
> +	struct emc1812_priv *priv = iio_priv(indio_dev);
> +	unsigned int i;
> +	int ret;
> +
> +	guard(mutex)(&priv->lock);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		for (i = 0; i < ARRAY_SIZE(emc1812_freq); i++)
> +			if (val == emc1812_freq[i][0] && val2 == emc1812_freq[i][1])
> +				break;
> +
> +		if (i == ARRAY_SIZE(emc1812_freq))
> +			return -EINVAL;
> +
> +		ret = regmap_write(priv->regmap, EMC1812_CONV_ADDR, i);
> +		if (ret)
> +			return ret;
> +
> +		priv->freq_idx = i;
> +		break;
> +	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
> +		for (i = 0; i < ARRAY_SIZE(emc1812_3db_values_map_tbl[priv->freq_idx]); i++)
> +			if (val == emc1812_3db_values_map_tbl[priv->freq_idx][i][0] &&
> +			    val2 == emc1812_3db_values_map_tbl[priv->freq_idx][i][1])
> +				break;
> +
> +		if (i == ARRAY_SIZE(emc1812_3db_values_map_tbl[priv->freq_idx]))
> +			return -EINVAL;
> +
> +		/*
> +		 * In emc1812_3db_values_map_tbl the second index maps:
> +		 * 0 for filter off
> +		 * 1 for filter at level 1
> +		 * 2 for filter at level 2
> +		 */
> +		if (i == 2)
> +			i = 3;
> +
> +		ret = regmap_write(priv->regmap, EMC1812_FILTER_SEL_ADDR, i);
> +		if (ret)
> +			return ret;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
Prefer early returns above so that a reader can quickly see where a particular
code path goes without needing to scroll down here to see there is nothing else
to do.

> +}

> +static int emc1812_init(struct emc1812_priv *priv)
> +{
> +	unsigned int i;
> +	int ret;
> +	u8 val;
> +
> +	/*
> +	 * Depending on the chip, lock channel beta 1 and/or 2 to Diode Mode
> +	 * when APDD is enabled.
> +	 */
> +	if (priv->chip->lock_beta1 && priv->apdd_en)
> +		priv->beta_values[0] = 0x0F;
> +	if (priv->chip->lock_beta2 && priv->apdd_en)
> +		priv->beta_values[1] = 0x0F;
> +
> +	/*
> +	 * Set default values in registers. APDD, RECD12 and RECD34 are active
> +	 * on 0.
> +	 * Set the device to be in Run (Active) state and converting on all
> +	 * channels.
> +	 * The temperature measurement range is -64°C to +191.875°C.
> +	 */
> +	val = FIELD_PREP(EMC1812_CFG_MSKAL, 1) |
> +	      FIELD_PREP(EMC1812_CFG_RS, 0) |
> +	      FIELD_PREP(EMC1812_CFG_ATTHM, 1) |
> +	      FIELD_PREP(EMC1812_CFG_RECD12, !priv->recd12_en) |
> +	      FIELD_PREP(EMC1812_CFG_RECD34, !priv->recd34_en) |
> +	      FIELD_PREP(EMC1812_CFG_RANGE, 1) |
> +	      FIELD_PREP(EMC1812_CFG_DA_ENA, 0) |
> +	      FIELD_PREP(EMC1812_CFG_APDD, !priv->apdd_en);
> +
> +	ret = regmap_write(priv->regmap, EMC1812_CFG_ADDR, val);
> +	if (ret)
> +		return ret;
> +
> +	/* Default is 4 conversions/seconds */
/second

Odd that is the value 6.  Maybe this needs a define?
> +	ret = regmap_write(priv->regmap, EMC1812_CONV_ADDR, 6);
> +	if (ret)
> +		return ret;
> +	priv->freq_idx = 6;
> +
> +	ret = regmap_write(priv->regmap, EMC1812_THRM_HYS_ADDR, 0x0A);

These values are all rather non obvious.  So add a comment or where possible
defines to explain the values being set.

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(priv->regmap, EMC1812_CONSEC_ALERT_ADDR, 0x70);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(priv->regmap, EMC1812_FILTER_SEL_ADDR, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(priv->regmap, EMC1812_HOTTEST_CFG_ADDR, 0);
> +	if (ret)
> +		return ret;
> +
> +	/* Set beta1 and beta2 compensation parameters */
> +	for (i = 0; i < ARRAY_SIZE(priv->beta_values); i++) {
> +		ret = regmap_write(priv->regmap, EMC1812_BETA_CFG_ADDR(i),
> +				   priv->beta_values[i]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Set ideality factor for all external channels */
> +	ret = regmap_write(priv->regmap, EMC1812_EXT1_IDEALITY_FACTOR_ADDR,

Perhaps a look up table for the registers and a loop?


> +			   priv->ideality_value[0]);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(priv->regmap, EMC1812_EXT2_IDEALITY_FACTOR_ADDR,
> +			   priv->ideality_value[1]);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(priv->regmap, EMC1812_EXT3_IDEALITY_FACTOR_ADDR,
> +			   priv->ideality_value[2]);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(priv->regmap, EMC1812_EXT4_IDEALITY_FACTOR_ADDR,
> +			   priv->ideality_value[3]);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
return regmap_write()

> +}
> +
> +static int emc1812_parse_fw_config(struct emc1812_priv *priv, struct device *dev,
> +				   int device_nr_channels)
> +{
> +	unsigned int reg_nr, iio_idx, tmp;
> +	int ret;
> +
> +	priv->apdd_en = device_property_read_bool(dev, "microchip,enable-anti-parallel");
> +	priv->recd12_en = device_property_read_bool(dev, "microchip,parasitic-res-on-channel1-2");
> +	priv->recd34_en = device_property_read_bool(dev, "microchip,parasitic-res-on-channel3-4");
> +
> +	memset32(priv->beta_values, 16, ARRAY_SIZE(priv->beta_values));

I would just set the two separately. This optimisation would make sense if there were a lot
more entries, but not worth making things harder to read when it is only two values,
particularly when the type isn't a specific size int.

> +	device_property_read_u32(dev, "microchip,beta1", &priv->beta_values[0]);
> +	device_property_read_u32(dev, "microchip,beta2", &priv->beta_values[1]);
> +	if (priv->beta_values[0] > 16 || priv->beta_values[1] > 16)
> +		return dev_err_probe(dev, -EINVAL, "Invalid beta value\n");
> +
> +	priv->num_channels = device_get_child_node_count(dev) + 1;
> +
> +	if (priv->num_channels > priv->chip->phys_channels)
> +		return dev_err_probe(dev, -E2BIG, "More channels than the chip supports\n");
> +
> +	priv->iio_chan[0] = EMC1812_CHAN(0, 0, EMC1812_CH_ADDR(0));
> +

I would remove this blank line to keep the association between the channel and it's label
tighter. That avoids the need for an explanatory comment on what this first channel is.

I'll note this code is very similar to what I commented on earlier in Victors driver so
take a look at that and see if I failed to notice anything here I picked up on in that review.

> +	priv->labels[0] = "internal_diode";
> +	iio_idx = 1;
> +	device_for_each_child_node_scoped(dev, child) {
> +		ret = fwnode_property_read_u32(child, "reg", &reg_nr);
> +		if (ret || reg_nr >= priv->chip->phys_channels)
> +			return dev_err_probe(dev, -EINVAL,
> +				     "The index of the channels does not match the chip\n");
> +
> +		ret = fwnode_property_read_u32(child, "microchip,ideality-factor", &tmp);
> +		if (ret == 0) {
> +			if (tmp < 8  || tmp > 63)
> +				return dev_err_probe(dev, ret, "Invalid ideality value\n");
> +			priv->ideality_value[reg_nr - 1] = tmp;
> +		} else {
> +			priv->ideality_value[reg_nr - 1] = 18;
> +		}
> +
> +		fwnode_property_read_string(child, "label", &priv->labels[reg_nr]);
> +
> +		priv->iio_chan[iio_idx++] = EMC1812_CHAN(reg_nr, reg_nr, EMC1812_CH_ADDR(reg_nr));
> +	}
> +
> +	return 0;
> +}
> +
> +static int emc1812_chip_identify(struct emc1812_priv *priv, struct i2c_client *client)
> +{
> +	int ret, tmp;
> +
> +	ret = regmap_read(priv->regmap, EMC1812_PRODUCT_ID_ADDR, &tmp);
> +	if (ret)
> +		return ret;
> +
> +	switch (tmp) {
> +	case EMC1812_PID:
> +		priv->chip = &emc1812_chip_config;
> +		break;

When there is nothing else to do, I'm a big fan of early returns that
immediately let the reader know we are done.

> +	case EMC1813_PID:
> +		priv->chip = &emc1813_chip_config;
> +		break;
> +	case EMC1814_PID:
> +		priv->chip = &emc1814_chip_config;
> +		break;
> +	case EMC1815_PID:
> +		priv->chip = &emc1815_chip_config;
> +		break;
> +	case EMC1833_PID:
> +		priv->chip = &emc1833_chip_config;
> +		break;
> +	default:
> +		/*
> +		 * If failed to identify the hardware based on internal registers,
> +		 * try using fallback compatible in device tree to deal with some
> +		 * newer part number.
> +		 */
> +		priv->chip = i2c_get_match_data(client);
> +		if (!priv->chip)
> +			return -EINVAL;
> +		break;
> +	}
> +	return 0;
> +}
> +
> +static int emc1812_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct emc1812_priv *priv;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	priv = iio_priv(indio_dev);
> +	priv->regmap = devm_regmap_init_i2c(client, &emc1812_regmap_config);
> +	if (IS_ERR(priv->regmap))
> +		return dev_err_probe(dev, PTR_ERR(priv->regmap),
> +				     "Cannot initialize register map\n");
> +
> +	ret = devm_mutex_init(dev, &priv->lock);
> +	if (ret)
> +		return ret;
> +
> +	ret = emc1812_chip_identify(priv, client);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Chip identification fails\n");
> +
> +	dev_info(dev, "Device name: %s\n", priv->chip->name);

Too noisy. dev_dbg() at most, probably just drop it entirely as these are easy
to query if the driver probes successfully. 

> +
> +	ret = emc1812_parse_fw_config(priv, dev, priv->chip->phys_channels);
> +	if (ret)
> +		return ret;
> +
> +	ret = emc1812_init(priv);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Cannot initialize device\n");
> +
> +	indio_dev->name = priv->chip->name;
> +	indio_dev->info = &emc1812_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = &priv->iio_chan[0];

Given it's a pointer to an array to me using priv->iio_chan seems more
logical.

> +	indio_dev->num_channels = priv->num_channels;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Cannot register IIO device\n");
> +
> +	return 0;
> +}

> +static const struct of_device_id emc1812_of_match[] = {
> +	{
> +		.compatible = "microchip,emc1812",
> +		.data = &emc1812_chip_config
> +	},
> +	{
> +		.compatible = "microchip,emc1813",
> +		.data = &emc1813_chip_config
> +	},
> +	{
> +		.compatible = "microchip,emc1814",
> +		.data = &emc1814_chip_config
> +	},
> +	{
> +		.compatible = "microchip,emc1815",
> +		.data = &emc1815_chip_config
> +	},
> +	{
> +		.compatible = "microchip,emc1833",
> +		.data = &emc1833_chip_config

Trailing commas needed for these.  Reduces churn if we want
to set other fields in future.

> +	},
> +	{ }
> +};



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] iio: temperature: add support for EMC1812
  2025-09-20 11:51   ` Jonathan Cameron
@ 2025-09-20 15:05     ` David Lechner
  0 siblings, 0 replies; 19+ messages in thread
From: David Lechner @ 2025-09-20 15:05 UTC (permalink / raw)
  To: Jonathan Cameron, Marius Cristea
  Cc: Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, linux-iio, devicetree, linux-kernel

On 9/20/25 6:51 AM, Jonathan Cameron wrote:
> On Wed, 17 Sep 2025 15:21:58 +0300
> Marius Cristea <marius.cristea@microchip.com> wrote:
> 

...

>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_write(priv->regmap, EMC1812_CONSEC_ALERT_ADDR, 0x70);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_write(priv->regmap, EMC1812_FILTER_SEL_ADDR, 0);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_write(priv->regmap, EMC1812_HOTTEST_CFG_ADDR, 0);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Set beta1 and beta2 compensation parameters */
>> +	for (i = 0; i < ARRAY_SIZE(priv->beta_values); i++) {
>> +		ret = regmap_write(priv->regmap, EMC1812_BETA_CFG_ADDR(i),
>> +				   priv->beta_values[i]);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	/* Set ideality factor for all external channels */
>> +	ret = regmap_write(priv->regmap, EMC1812_EXT1_IDEALITY_FACTOR_ADDR,
> 
> Perhaps a look up table for the registers and a loop?
> 
> 

Also see regmap_multi_reg_write() and regmap_register_patch().


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] iio: temperature: add support for EMC1812
  2025-09-17 12:21 ` [PATCH 2/2] " Marius Cristea
  2025-09-18  3:07   ` kernel test robot
  2025-09-20 11:51   ` Jonathan Cameron
@ 2025-09-23  5:31   ` Dan Carpenter
  2 siblings, 0 replies; 19+ messages in thread
From: Dan Carpenter @ 2025-09-23  5:31 UTC (permalink / raw)
  To: oe-kbuild, Marius Cristea, Jonathan Cameron, David Lechner,
	Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: lkp, oe-kbuild-all, linux-iio, devicetree, linux-kernel,
	Marius Cristea

Hi Marius,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Marius-Cristea/dt-bindings-iio-temperature-add-support-for-EMC1812/20250917-202833
base:   19272b37aa4f83ca52bdf9c16d5d81bdd1354494
patch link:    https://lore.kernel.org/r/20250917-iio-emc1812-v1-2-0b1f74cea7ab%40microchip.com
patch subject: [PATCH 2/2] iio: temperature: add support for EMC1812
config: powerpc64-randconfig-r071-20250922 (https://download.01.org/0day-ci/archive/20250923/202509231015.K3B5TN1Q-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project cafc064fc7a96b3979a023ddae1da2b499d6c954)

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>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202509231015.K3B5TN1Q-lkp@intel.com/

smatch warnings:
drivers/iio/temperature/emc1812.c:645 emc1812_parse_fw_config() warn: passing zero to 'dev_err_probe'

vim +/dev_err_probe +645 drivers/iio/temperature/emc1812.c

56c661fffa897b2 Marius Cristea 2025-09-17  611  static int emc1812_parse_fw_config(struct emc1812_priv *priv, struct device *dev,
56c661fffa897b2 Marius Cristea 2025-09-17  612  				   int device_nr_channels)
56c661fffa897b2 Marius Cristea 2025-09-17  613  {
56c661fffa897b2 Marius Cristea 2025-09-17  614  	unsigned int reg_nr, iio_idx, tmp;
56c661fffa897b2 Marius Cristea 2025-09-17  615  	int ret;
56c661fffa897b2 Marius Cristea 2025-09-17  616  
56c661fffa897b2 Marius Cristea 2025-09-17  617  	priv->apdd_en = device_property_read_bool(dev, "microchip,enable-anti-parallel");
56c661fffa897b2 Marius Cristea 2025-09-17  618  	priv->recd12_en = device_property_read_bool(dev, "microchip,parasitic-res-on-channel1-2");
56c661fffa897b2 Marius Cristea 2025-09-17  619  	priv->recd34_en = device_property_read_bool(dev, "microchip,parasitic-res-on-channel3-4");
56c661fffa897b2 Marius Cristea 2025-09-17  620  
56c661fffa897b2 Marius Cristea 2025-09-17  621  	memset32(priv->beta_values, 16, ARRAY_SIZE(priv->beta_values));
56c661fffa897b2 Marius Cristea 2025-09-17  622  	device_property_read_u32(dev, "microchip,beta1", &priv->beta_values[0]);
56c661fffa897b2 Marius Cristea 2025-09-17  623  	device_property_read_u32(dev, "microchip,beta2", &priv->beta_values[1]);
56c661fffa897b2 Marius Cristea 2025-09-17  624  	if (priv->beta_values[0] > 16 || priv->beta_values[1] > 16)
56c661fffa897b2 Marius Cristea 2025-09-17  625  		return dev_err_probe(dev, -EINVAL, "Invalid beta value\n");
56c661fffa897b2 Marius Cristea 2025-09-17  626  
56c661fffa897b2 Marius Cristea 2025-09-17  627  	priv->num_channels = device_get_child_node_count(dev) + 1;
56c661fffa897b2 Marius Cristea 2025-09-17  628  
56c661fffa897b2 Marius Cristea 2025-09-17  629  	if (priv->num_channels > priv->chip->phys_channels)
56c661fffa897b2 Marius Cristea 2025-09-17  630  		return dev_err_probe(dev, -E2BIG, "More channels than the chip supports\n");
56c661fffa897b2 Marius Cristea 2025-09-17  631  
56c661fffa897b2 Marius Cristea 2025-09-17  632  	priv->iio_chan[0] = EMC1812_CHAN(0, 0, EMC1812_CH_ADDR(0));
56c661fffa897b2 Marius Cristea 2025-09-17  633  
56c661fffa897b2 Marius Cristea 2025-09-17  634  	priv->labels[0] = "internal_diode";
56c661fffa897b2 Marius Cristea 2025-09-17  635  	iio_idx = 1;
56c661fffa897b2 Marius Cristea 2025-09-17  636  	device_for_each_child_node_scoped(dev, child) {
56c661fffa897b2 Marius Cristea 2025-09-17  637  		ret = fwnode_property_read_u32(child, "reg", &reg_nr);
56c661fffa897b2 Marius Cristea 2025-09-17  638  		if (ret || reg_nr >= priv->chip->phys_channels)
56c661fffa897b2 Marius Cristea 2025-09-17  639  			return dev_err_probe(dev, -EINVAL,
56c661fffa897b2 Marius Cristea 2025-09-17  640  				     "The index of the channels does not match the chip\n");
56c661fffa897b2 Marius Cristea 2025-09-17  641  
56c661fffa897b2 Marius Cristea 2025-09-17  642  		ret = fwnode_property_read_u32(child, "microchip,ideality-factor", &tmp);
56c661fffa897b2 Marius Cristea 2025-09-17  643  		if (ret == 0) {
56c661fffa897b2 Marius Cristea 2025-09-17  644  			if (tmp < 8  || tmp > 63)
56c661fffa897b2 Marius Cristea 2025-09-17 @645  				return dev_err_probe(dev, ret, "Invalid ideality value\n");

Change ret to -EINVAL.

56c661fffa897b2 Marius Cristea 2025-09-17  646  			priv->ideality_value[reg_nr - 1] = tmp;
56c661fffa897b2 Marius Cristea 2025-09-17  647  		} else {
56c661fffa897b2 Marius Cristea 2025-09-17  648  			priv->ideality_value[reg_nr - 1] = 18;
56c661fffa897b2 Marius Cristea 2025-09-17  649  		}
56c661fffa897b2 Marius Cristea 2025-09-17  650  
56c661fffa897b2 Marius Cristea 2025-09-17  651  		fwnode_property_read_string(child, "label", &priv->labels[reg_nr]);
56c661fffa897b2 Marius Cristea 2025-09-17  652  
56c661fffa897b2 Marius Cristea 2025-09-17  653  		priv->iio_chan[iio_idx++] = EMC1812_CHAN(reg_nr, reg_nr, EMC1812_CH_ADDR(reg_nr));
56c661fffa897b2 Marius Cristea 2025-09-17  654  	}
56c661fffa897b2 Marius Cristea 2025-09-17  655  
56c661fffa897b2 Marius Cristea 2025-09-17  656  	return 0;
56c661fffa897b2 Marius Cristea 2025-09-17  657  }

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Add support for Microchip EMC1812
  2025-09-20 11:33 ` Jonathan Cameron
@ 2025-09-24  2:11   ` Guenter Roeck
  2025-09-25  9:09     ` Marius.Cristea
  0 siblings, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2025-09-24  2:11 UTC (permalink / raw)
  To: Jonathan Cameron, Marius Cristea
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree,
	linux-kernel, jdelvare, linux-hwmon

On 9/20/25 04:33, Jonathan Cameron wrote:
> On Wed, 17 Sep 2025 15:21:56 +0300
> Marius Cristea <marius.cristea@microchip.com> wrote:
> 
>> This is the iio driver for EMC1812/13/14/15/33 multichannel Low-Voltage
>> Remote Diode Sensor Family. The chips in the family have one internal
>> and different numbers of external channels, ranging from 1 (EMC1812) to
>> 4 channels (EMC1815).
>> Reading diodes in anti-parallel connection is supported by EMC1814, EMC1815
>> and EMC1833.
>>
>> Current version of driver does not support interrupts, events and data
>> buffering.
> Hi Marius,
> 
> For a temperature monitoring device like this, the opening question is
> always why not HWMON?
> 
> There are various reasons we have temp sensors in IIO but mostly they are not
> described as being monitors and this one is.
> 
> IIO may well be the right choice for this part, but good to lay out your
> reasoning and +CC the hwmon list and maintainers.  There is an emc1403
> driver already in hwmon, so perhaps compare and contrast with that.
> 
> I've +CC Jean, Guenter and list to save sending a v2 just to do that.
> 

At first glance it looks like the series is (mostly ?) register compatible
to the chips supported by the emc1403 driver, so it should be straightforward
to add support for the emc180x series to that driver.

Guenter


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Add support for Microchip EMC1812
  2025-09-24  2:11   ` Guenter Roeck
@ 2025-09-25  9:09     ` Marius.Cristea
  2025-09-25 14:32       ` Guenter Roeck
  0 siblings, 1 reply; 19+ messages in thread
From: Marius.Cristea @ 2025-09-25  9:09 UTC (permalink / raw)
  To: linux, jic23
  Cc: dlechner, linux-hwmon, jdelvare, nuno.sa, linux-iio, devicetree,
	robh, linux-kernel, andy, krzk+dt, conor+dt

Hi Guenter,

Thank you for the feedback.

On Tue, 2025-09-23 at 19:11 -0700, Guenter Roeck wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On 9/20/25 04:33, Jonathan Cameron wrote:
> > On Wed, 17 Sep 2025 15:21:56 +0300
> > Marius Cristea <marius.cristea@microchip.com> wrote:
> > 
> > > This is the iio driver for EMC1812/13/14/15/33 multichannel Low-
> > > Voltage
> > > Remote Diode Sensor Family. The chips in the family have one
> > > internal
> > > and different numbers of external channels, ranging from 1
> > > (EMC1812) to
> > > 4 channels (EMC1815).
> > > Reading diodes in anti-parallel connection is supported by
> > > EMC1814, EMC1815
> > > and EMC1833.
> > > 
> > > Current version of driver does not support interrupts, events and
> > > data
> > > buffering.
> > Hi Marius,
> > 
> > For a temperature monitoring device like this, the opening question
> > is
> > always why not HWMON?
> > 
> > There are various reasons we have temp sensors in IIO but mostly
> > they are not
> > described as being monitors and this one is.
> > 
> > IIO may well be the right choice for this part, but good to lay out
> > your
> > reasoning and +CC the hwmon list and maintainers.  There is an
> > emc1403
> > driver already in hwmon, so perhaps compare and contrast with that.
> > 
> > I've +CC Jean, Guenter and list to save sending a v2 just to do
> > that.
> > 
> 
> At first glance it looks like the series is (mostly ?) register
> compatible
> to the chips supported by the emc1403 driver, so it should be
> straightforward
> to add support for the emc180x series to that driver.
> 
> Guenter

Most of the register address are compatible. The EMC181X is an update 
(a newer generation) then the EMC1403.

The biggest improvement is that the EMC18XX has a continuous block of
registers in order to improve the temperature reading (that means some
addresses are overlapping with the older register maps) and a new set
of registers to  handle the "Rate Of Change" functionality.
Also the older EMC14XX has some hardcoded configuration/features based
on the part number.

Considering all of the above I consider that the complexity of the
EMC1403 will increase quite a lot without any real benefit and it will
be harder to be maintained.

I have submitted this as the fist iteration from a longer list of
feature that I want to add to the driver, including events and maybe
interrupts.

If nobody has anything against, I would like to add a separate driver
for EMC18XX into the IIO.

Thanks and Best Regards,
Marius


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812
  2025-09-17 23:34   ` David Lechner
@ 2025-09-25 14:27     ` Marius.Cristea
  0 siblings, 0 replies; 19+ messages in thread
From: Marius.Cristea @ 2025-09-25 14:27 UTC (permalink / raw)
  To: robh, krzk+dt, jic23, nuno.sa, dlechner, conor+dt, andy
  Cc: devicetree, linux-kernel, linux-iio

Hi David,

  Thank you for the feedback,

On Wed, 2025-09-17 at 18:34 -0500, David Lechner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On 9/17/25 7:21 AM, Marius Cristea wrote:
> > This is the devicetree schema for Microchip EMC1812/13/14/15/33
> > Multichannel Low-Voltage Remote Diode Sensor Family.
> > 
> > Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> > ---
> >  .../iio/temperature/microchip,emc1812.yaml         | 223
> > +++++++++++++++++++++
> >  MAINTAINERS                                        |   6 +
> >  2 files changed, 229 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/iio/temperature/microchip,emc18
> > 12.yaml
> > b/Documentation/devicetree/bindings/iio/temperature/microchip,emc18
> > 12.yaml
> > new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..898d6d246746e229cb004f447
> > 872ee6bd5a65074
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/iio/temperature/microchip,emc18
> > 12.yaml
> > @@ -0,0 +1,223 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > http://devicetree.org/schemas/iio/temperature/microchip,emc1812.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip EMC1812/13/14/15/33 multichannel temperature
> > sensor
> > +
> > +maintainers:
> > +  - Marius Cristea <marius.cristea@microchip.com>
> > +
> > +description: |
> > +  The Microchip EMC1812/13/14/15/33 is a high-accuracy 2-wire
> > multichannel
> > +  low-voltage remote diode temperature monitor.
> > +
> > +  The datasheet can be found here:
> > +   
> > https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/EMC1812-3-4-5-33-Data-Sheet-DS20005751.pdf
> 
> The pinouts of these chips look nearly identical to MCP998X.
> Would it make sense to share a single bindings document for these?
> Or maybe there would be too many if: blocks and keeping it separate
> is fine.
> 
> https://lore.kernel.org/linux-iio/20250829143447.18893-2-victor.duicu@microchip.com/
> 
> 

I know that the chip looks nearly identical with MCP998X, but being two
different family of devices and having different functions inside, I
would like to keep the binding separate, otherwise there will be too
many conditions.


Thanks,
Marius

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Add support for Microchip EMC1812
  2025-09-25  9:09     ` Marius.Cristea
@ 2025-09-25 14:32       ` Guenter Roeck
  2025-09-27 15:04         ` Jonathan Cameron
  0 siblings, 1 reply; 19+ messages in thread
From: Guenter Roeck @ 2025-09-25 14:32 UTC (permalink / raw)
  To: Marius.Cristea
  Cc: jic23, dlechner, linux-hwmon, jdelvare, nuno.sa, linux-iio,
	devicetree, robh, linux-kernel, andy, krzk+dt, conor+dt

On Thu, Sep 25, 2025 at 09:09:04AM +0000, Marius.Cristea@microchip.com wrote:
> Hi Guenter,
> 
> Thank you for the feedback.
> 
> On Tue, 2025-09-23 at 19:11 -0700, Guenter Roeck wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> > 
> > On 9/20/25 04:33, Jonathan Cameron wrote:
> > > On Wed, 17 Sep 2025 15:21:56 +0300
> > > Marius Cristea <marius.cristea@microchip.com> wrote:
> > > 
> > > > This is the iio driver for EMC1812/13/14/15/33 multichannel Low-
> > > > Voltage
> > > > Remote Diode Sensor Family. The chips in the family have one
> > > > internal
> > > > and different numbers of external channels, ranging from 1
> > > > (EMC1812) to
> > > > 4 channels (EMC1815).
> > > > Reading diodes in anti-parallel connection is supported by
> > > > EMC1814, EMC1815
> > > > and EMC1833.
> > > > 
> > > > Current version of driver does not support interrupts, events and
> > > > data
> > > > buffering.
> > > Hi Marius,
> > > 
> > > For a temperature monitoring device like this, the opening question
> > > is
> > > always why not HWMON?
> > > 
> > > There are various reasons we have temp sensors in IIO but mostly
> > > they are not
> > > described as being monitors and this one is.
> > > 
> > > IIO may well be the right choice for this part, but good to lay out
> > > your
> > > reasoning and +CC the hwmon list and maintainers.  There is an
> > > emc1403
> > > driver already in hwmon, so perhaps compare and contrast with that.
> > > 
> > > I've +CC Jean, Guenter and list to save sending a v2 just to do
> > > that.
> > > 
> > 
> > At first glance it looks like the series is (mostly ?) register
> > compatible
> > to the chips supported by the emc1403 driver, so it should be
> > straightforward
> > to add support for the emc180x series to that driver.
> > 
> > Guenter
> 
> Most of the register address are compatible. The EMC181X is an update 
> (a newer generation) then the EMC1403.
> 
> The biggest improvement is that the EMC18XX has a continuous block of
> registers in order to improve the temperature reading (that means some
> addresses are overlapping with the older register maps) and a new set
> of registers to  handle the "Rate Of Change" functionality.
> Also the older EMC14XX has some hardcoded configuration/features based
> on the part number.
> 
> Considering all of the above I consider that the complexity of the
> EMC1403 will increase quite a lot without any real benefit and it will
> be harder to be maintained.
> 
Ok.

> I have submitted this as the fist iteration from a longer list of
> feature that I want to add to the driver, including events and maybe
> interrupts.
> 
> If nobody has anything against, I would like to add a separate driver
> for EMC18XX into the IIO.

IMO this should be a hwmon driver.

Guenter

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] Add support for Microchip EMC1812
  2025-09-25 14:32       ` Guenter Roeck
@ 2025-09-27 15:04         ` Jonathan Cameron
  0 siblings, 0 replies; 19+ messages in thread
From: Jonathan Cameron @ 2025-09-27 15:04 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Marius.Cristea, dlechner, linux-hwmon, jdelvare, nuno.sa,
	linux-iio, devicetree, robh, linux-kernel, andy, krzk+dt,
	conor+dt

On Thu, 25 Sep 2025 07:32:22 -0700
Guenter Roeck <linux@roeck-us.net> wrote:

> On Thu, Sep 25, 2025 at 09:09:04AM +0000, Marius.Cristea@microchip.com wrote:
> > Hi Guenter,
> > 
> > Thank you for the feedback.
> > 
> > On Tue, 2025-09-23 at 19:11 -0700, Guenter Roeck wrote:  
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > know the content is safe
> > > 
> > > On 9/20/25 04:33, Jonathan Cameron wrote:  
> > > > On Wed, 17 Sep 2025 15:21:56 +0300
> > > > Marius Cristea <marius.cristea@microchip.com> wrote:
> > > >   
> > > > > This is the iio driver for EMC1812/13/14/15/33 multichannel Low-
> > > > > Voltage
> > > > > Remote Diode Sensor Family. The chips in the family have one
> > > > > internal
> > > > > and different numbers of external channels, ranging from 1
> > > > > (EMC1812) to
> > > > > 4 channels (EMC1815).
> > > > > Reading diodes in anti-parallel connection is supported by
> > > > > EMC1814, EMC1815
> > > > > and EMC1833.
> > > > > 
> > > > > Current version of driver does not support interrupts, events and
> > > > > data
> > > > > buffering.  
> > > > Hi Marius,
> > > > 
> > > > For a temperature monitoring device like this, the opening question
> > > > is
> > > > always why not HWMON?
> > > > 
> > > > There are various reasons we have temp sensors in IIO but mostly
> > > > they are not
> > > > described as being monitors and this one is.
> > > > 
> > > > IIO may well be the right choice for this part, but good to lay out
> > > > your
> > > > reasoning and +CC the hwmon list and maintainers.  There is an
> > > > emc1403
> > > > driver already in hwmon, so perhaps compare and contrast with that.
> > > > 
> > > > I've +CC Jean, Guenter and list to save sending a v2 just to do
> > > > that.
> > > >   
> > > 
> > > At first glance it looks like the series is (mostly ?) register
> > > compatible
> > > to the chips supported by the emc1403 driver, so it should be
> > > straightforward
> > > to add support for the emc180x series to that driver.
> > > 
> > > Guenter  
> > 
> > Most of the register address are compatible. The EMC181X is an update 
> > (a newer generation) then the EMC1403.
> > 
> > The biggest improvement is that the EMC18XX has a continuous block of
> > registers in order to improve the temperature reading (that means some
> > addresses are overlapping with the older register maps) and a new set
> > of registers to  handle the "Rate Of Change" functionality.
> > Also the older EMC14XX has some hardcoded configuration/features based
> > on the part number.
> > 
> > Considering all of the above I consider that the complexity of the
> > EMC1403 will increase quite a lot without any real benefit and it will
> > be harder to be maintained.
> >   
> Ok.
> 
> > I have submitted this as the fist iteration from a longer list of
> > feature that I want to add to the driver, including events and maybe
> > interrupts.
> > 
> > If nobody has anything against, I would like to add a separate driver
> > for EMC18XX into the IIO.  
> 
> IMO this should be a hwmon driver.

So far I haven't seen any reason why IIO would make more sense, so agreed.

Jonathan

> 
> Guenter


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 0/2] Add support for Microchip EMC1812
@ 2025-10-29 15:50 Marius Cristea
  0 siblings, 0 replies; 19+ messages in thread
From: Marius Cristea @ 2025-10-29 15:50 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Jonathan Corbet
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Marius Cristea

This is the hwmon driver for EMC1812/13/14/15/33 multichannel Low-Voltage
Remote Diode Sensor Family. The chips in the family have one internal
and different numbers of external channels, ranging from 1 (EMC1812) to
4 channels (EMC1815).
Reading diodes in anti-parallel connection is supported by EMC1814, EMC1815
and EMC1833.

Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
---
Marius Cristea (2):
      dt-bindings: hwmon: temperature: add support for EMC1812
      hwmon: temperature: add support for EMC1812

 .../bindings/hwmon/microchip,emc1812.yaml          | 176 ++++
 Documentation/hwmon/emc1812.rst                    |  68 ++
 MAINTAINERS                                        |   8 +
 drivers/hwmon/Kconfig                              |  11 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/emc1812.c                            | 967 +++++++++++++++++++++
 6 files changed, 1231 insertions(+)
---
base-commit: d2b2fea3503e5e12b2e28784152937e48bcca6ff
change-id: 20251002-hw_mon-emc1812-f1b806487d10

Best regards,
-- 
Marius Cristea <marius.cristea@microchip.com>


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-10-29 15:51 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-17 12:21 [PATCH 0/2] Add support for Microchip EMC1812 Marius Cristea
2025-09-17 12:21 ` [PATCH 1/2] dt-bindings: iio: temperature: add support for EMC1812 Marius Cristea
2025-09-17 14:31   ` Rob Herring (Arm)
2025-09-17 23:34   ` David Lechner
2025-09-25 14:27     ` Marius.Cristea
2025-09-17 12:21 ` [PATCH 2/2] " Marius Cristea
2025-09-18  3:07   ` kernel test robot
2025-09-20 11:51   ` Jonathan Cameron
2025-09-20 15:05     ` David Lechner
2025-09-23  5:31   ` Dan Carpenter
2025-09-17 13:25 ` [PATCH 0/2] Add support for Microchip EMC1812 David Lechner
2025-09-17 13:30   ` Marius.Cristea
2025-09-17 13:38     ` David Lechner
2025-09-20 11:33 ` Jonathan Cameron
2025-09-24  2:11   ` Guenter Roeck
2025-09-25  9:09     ` Marius.Cristea
2025-09-25 14:32       ` Guenter Roeck
2025-09-27 15:04         ` Jonathan Cameron
  -- strict thread matches above, loose matches on Subject: below --
2025-10-29 15:50 Marius Cristea

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).