Skip to contentSkip to conversation

iio: adc: add Nuvoton NCT7201 ADC driver

PATCH Superseded Eason Yang 6 participants 13 messages Feb 21, 2025 – Apr 7, 2025

Subsystems (3)

v4 of 10 Superseded Latest is v10 →

lore
Share & tools
lore

Eason Yang

author maintainer
#0 lore raw

v4 cover iio: adc: add Nuvoton NCT7201 ADC driver

Change since version 4:
- Fix comments
- Add interrupts and reset-gpios to the DT example
- Use the FIELD_PREP and FIELD_GET
- Add use_single_write in regmap_config
- Use regmap_access_table

Change since version 3:
- Fix comments
- Don't put nct720"x" in the name, just call it nct7201
- Remove differential inputs until conversions are finished
- Add NCT7201_ prefix in all macros and avoid the tables
- Correct event threshold values in raw units
- Add with and without interrupt callback function to have the event config part and one that doesn't
- Remove print an error message if regmap_wirte failed case

Change since version 2:
- Remvoe read-vin-data-size property, default use read word vin data
- Use regmap instead of i2c smbus API
- IIO should be IIO_CHAN_INFO_RAW and _SCALE not _PROCESSED
- Use dev_xxx_probe in probe function and dev_xxx in other functions
- Use devm_iio_device_register replace of iio_device_register
- Use guard(mutex) replace of mutex_lock
- Use get_unaligned_le16 conversion API

Changes since version 1:
- Add new property in iio:adc binding document
- Add new driver for Nuvoton NCT720x driver

Eason Yang (2):
dt-bindings: iio: adc: add NCT7201 ADCs
iio: adc: add support for Nuvoton NCT7201

 .../bindings/iio/adc/nuvoton,nct7201.yaml     |  57 ++
 MAINTAINERS                                   |   2 +
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/nct7201.c                     | 487 ++++++++++++++++++
 5 files changed, 558 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
 create mode 100644 drivers/iio/adc/nct7201.c
Signature · 1 line
2.34.1
author maintainer
#1 lore raw

PATCH v4 1/2 dt-bindings: iio: adc: add NCT7201 ADCs

Add a binding specification for the Nuvoton NCT7201/NCT7202 up to 12-bit ADCs with I2C interface.

---
 .../bindings/iio/adc/nuvoton,nct7201.yaml     | 57 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +57 -0
new file mode 100644
index 000000000000..830c37fd9f22
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct7201.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton nct7201 and similar ADCs
+
+maintainers:
+  - Eason Yang <[email protected]>
+
+description: |
+  The NCT7201/NCT7202 is a Nuvoton Hardware Monitor IC, contains up to 12 voltage
+  monitoring channels, with SMBus interface, and up to 4 sets SMBus address
+  selection by ADDR connection. It also provides ALERT# signal for event
+  notification and reset input RSTIN# to recover it from a fault condition.
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,nct7201
+      - nuvoton,nct7202
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@1d {
+            compatible = "nuvoton,nct7202";
+            reg = <0x1d>;
+            interrupt-parent = <&gpio3>;
+            interrupts = <30 IRQ_TYPE_LEVEL_LOW>;
+            reset-gpios = <&gpio3 28 GPIO_ACTIVE_LOW>;
+        };
+    };
+...
MAINTAINERS +1 -0
index 3864d473f52f..fdc4aa5c7eff 100644
@@ -2831,6 +2831,7 @@ L:	[email protected] (moderated for non-subscribers)
 S:	Supported
 F:	Documentation/devicetree/bindings/*/*/*npcm*
 F:	Documentation/devicetree/bindings/*/*npcm*
+F:	Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
 F:	Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml
 F:	arch/arm/boot/dts/nuvoton/nuvoton-npcm*
 F:	arch/arm/mach-npcm/
Signature · 1 line
2.34.1
reviewer
#2 lore raw

Re: [PATCH v4 1/2] dt-bindings: iio: adc: add NCT7201 ADCs

Quoted 38 lines · Eason Yang

Add a binding specification for the Nuvoton NCT7201/NCT7202 up to 12-bit ADCs with I2C interface.

---
 .../bindings/iio/adc/nuvoton,nct7201.yaml     | 57 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +22 -0
new file mode 100644
index 000000000000..830c37fd9f22
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct7201.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton nct7201 and similar ADCs
+
+maintainers:
+  - Eason Yang <[email protected]>
+
+description: |
+  The NCT7201/NCT7202 is a Nuvoton Hardware Monitor IC, contains up to 12 voltage
+  monitoring channels, with SMBus interface, and up to 4 sets SMBus address
+  selection by ADDR connection. It also provides ALERT# signal for event
+  notification and reset input RSTIN# to recover it from a fault condition.
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,nct7201
+      - nuvoton,nct7202

Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml line 32

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +10 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+

Maybe this was brought up before, but no power supply?

Quoted 30 lines
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +25 -0
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@1d {
+            compatible = "nuvoton,nct7202";
+            reg = <0x1d>;
+            interrupt-parent = <&gpio3>;
+            interrupts = <30 IRQ_TYPE_LEVEL_LOW>;
+            reset-gpios = <&gpio3 28 GPIO_ACTIVE_LOW>;
+        };
+    };
+...
MAINTAINERS +0 -0
index 3864d473f52f..fdc4aa5c7eff 100644

MAINTAINERS L: [email protected] (moderated for non-subscribers) · line 2834

Quoted 4 lines
MAINTAINERS +1 -0
@@ -2831,6 +2831,7 @@ L:	[email protected] (moderated for non-subscribers)
 S:	Supported
 F:	Documentation/devicetree/bindings/*/*/*npcm*
 F:	Documentation/devicetree/bindings/*/*npcm*
+F:	Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml

This (ARM/NUVOTON NPCM ARCHITECTURE) doesn't look like the right place for adding a stand-alone chip. You will need to start a new section like:

NUVOTON NCT7201 IIO DRIVER

Quoted 3 lines
MAINTAINERS +0 -0
 F:	Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml
 F:	arch/arm/boot/dts/nuvoton/nuvoton-npcm*
 F:	arch/arm/mach-npcm/
author maintainer
#3 lore raw

Re: [PATCH v4 1/2] dt-bindings: iio: adc: add NCT7201 ADCs

Dear David Lechner,

Thanks again for the review, I'll do the changes here and send next patch.

David Lechner <[email protected]> 於 2025年2月21日 週五 下午11:56寫道:

Quoted 44 lines · Eason Yang
Quoted 42 lines

Add a binding specification for the Nuvoton NCT7201/NCT7202 up to 12-bit ADCs with I2C interface.

---
 .../bindings/iio/adc/nuvoton,nct7201.yaml     | 57 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +26 -0
new file mode 100644
index 000000000000..830c37fd9f22
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct7201.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton nct7201 and similar ADCs
+
+maintainers:
+  - Eason Yang <[email protected]>
+
+description: |
+  The NCT7201/NCT7202 is a Nuvoton Hardware Monitor IC, contains up to 12 voltage
+  monitoring channels, with SMBus interface, and up to 4 sets SMBus address
+  selection by ADDR connection. It also provides ALERT# signal for event
+  notification and reset input RSTIN# to recover it from a fault condition.
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,nct7201
+      - nuvoton,nct7202
+
+  reg:
+    maxItems: 1
+
Quoted 9 lines

Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml line 32

Quoted 6 lines
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +6 -0
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+

Maybe this was brought up before, but no power supply?

vdd-supply:
description:
A 3.3V to supply that powers the chip.

vref-supply:
description:
The regulator supply for the ADC reference voltage.

Quoted 11 lines
Quoted 11 lines
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +11 -0
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
Quoted 8 lines

Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml line 51

Quoted 8 lines
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +8 -0
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@1d {
+            compatible = "nuvoton,nct7202";
+            reg = <0x1d>;
            vdd-supply = <&vdd>;
            vref-supply = <&vref>;
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +6 -0
+            interrupt-parent = <&gpio3>;
+            interrupts = <30 IRQ_TYPE_LEVEL_LOW>;
+            reset-gpios = <&gpio3 28 GPIO_ACTIVE_LOW>;
+        };
+    };
+...
MAINTAINERS +0 -0
Quoted 8 lines

MAINTAINERS L: [email protected] (moderated for non-subscribers) · line 2834

Quoted 8 lines

index 3864d473f52f..fdc4aa5c7eff 100644

MAINTAINERS +1 -0
@@ -2831,6 +2831,7 @@ L:      [email protected] (moderated for non-subscribers)
 S:   Supported
 F:   Documentation/devicetree/bindings/*/*/*npcm*
 F:   Documentation/devicetree/bindings/*/*npcm*
+F:   Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml

Remove F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml

Quoted 6 lines

This (ARM/NUVOTON NPCM ARCHITECTURE) doesn't look like the right place for adding a stand-alone chip. You will need to start a new section like:

NUVOTON NCT7201 IIO DRIVER

NUVOTON NCT7201 IIO DRIVER
M: Eason Yang <[email protected]>
L: [email protected]
S: Maintained
F: Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
F: drivers/iio/adc/nct7201.c

Quoted 4 lines
Quoted 3 lines
MAINTAINERS +0 -0
 F:   Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml
 F:   arch/arm/boot/dts/nuvoton/nuvoton-npcm*
 F:   arch/arm/mach-npcm/
#4 lore raw

Re: [PATCH v4 1/2] dt-bindings: iio: adc: add NCT7201 ADCs

In reply to Eason Yang
Quoted 28 lines · Eason Yang

Add a binding specification for the Nuvoton NCT7201/NCT7202 up to 12-bit ADCs with I2C interface.

---
 .../bindings/iio/adc/nuvoton,nct7201.yaml     | 57 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +12 -0
new file mode 100644
index 000000000000..830c37fd9f22
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/nuvoton,nct7201.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton nct7201 and similar ADCs
+
+maintainers:
+  - Eason Yang <[email protected]>
+
+description: |

Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml line 22

Quoted 10 lines
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +10 -0
+  The NCT7201/NCT7202 is a Nuvoton Hardware Monitor IC, contains up to 12 voltage
+  monitoring channels, with SMBus interface, and up to 4 sets SMBus address
+  selection by ADDR connection. It also provides ALERT# signal for event
+  notification and reset input RSTIN# to recover it from a fault condition.
+
+properties:
+  compatible:
+    enum:
+      - nuvoton,nct7201
+      - nuvoton,nct7202

When you respin, please add a note about what differs between these devices that requires different handling in the driver.

Cheers,
Conor.

Quoted 50 lines
Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +35 -0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@1d {
+            compatible = "nuvoton,nct7202";
+            reg = <0x1d>;
+            interrupt-parent = <&gpio3>;
+            interrupts = <30 IRQ_TYPE_LEVEL_LOW>;
+            reset-gpios = <&gpio3 28 GPIO_ACTIVE_LOW>;
+        };
+    };
+...
MAINTAINERS +1 -0
index 3864d473f52f..fdc4aa5c7eff 100644
@@ -2831,6 +2831,7 @@ L:	[email protected] (moderated for non-subscribers)
 S:	Supported
 F:	Documentation/devicetree/bindings/*/*/*npcm*
 F:	Documentation/devicetree/bindings/*/*npcm*
+F:	Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
 F:	Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml
 F:	arch/arm/boot/dts/nuvoton/nuvoton-npcm*
 F:	arch/arm/mach-npcm/
Signature · 1 line
2.34.1
author maintainer
#5 lore raw

PATCH v4 2/2 iio: adc: add support for Nuvoton NCT7201

In reply to Eason Yang

Add Nuvoton NCT7201/NCT7202 system voltage monitor 12-bit ADC driver

NCT7201/NCT7202 supports up to 12 analog voltage monitor inputs and up to 4 SMBus addresses by ADDR pin. Meanwhile, ALERT# hardware event pins for independent alarm signals, and the all threshold values could be set for system protection without any timing delay. It also supports reset input RSTIN# to recover system from a fault condition.

Currently, only single-edge mode conversion and threshold events support.

---
 MAINTAINERS               |   1 +
 drivers/iio/adc/Kconfig   |  11 +
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/nct7201.c | 487 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 500 insertions(+)
 create mode 100644 drivers/iio/adc/nct7201.c
MAINTAINERS +1 -0
index fdc4aa5c7eff..389cbbdae1a7 100644
@@ -2838,6 +2838,7 @@ F:	arch/arm/mach-npcm/
 F:	arch/arm64/boot/dts/nuvoton/
 F:	drivers/*/*/*npcm*
 F:	drivers/*/*npcm*
+F:	drivers/iio/adc/nct7201.c
 F:	drivers/rtc/rtc-nct3018y.c
 F:	include/dt-bindings/clock/nuvoton,npcm7xx-clock.h
 F:	include/dt-bindings/clock/nuvoton,npcm845-clk.h
drivers/iio/adc/Kconfig +11 -0
index 849c90203071..d97eccb1c685 100644
@@ -1048,6 +1048,17 @@ config NAU7802
 	  To compile this driver as a module, choose M here: the
 	  module will be called nau7802.
 
+config NCT7201
+        tristate "Nuvoton NCT7201 and NCT7202 Power Monitor"
+        depends on I2C
+        select REGMAP_I2C
+        help
+          Say yes here to build support for Nuvoton NCT7201 and NCT7202
+          Voltage Monitor.
+
+          This driver can also be built as a module. If so, the module
+          will be called nct7201.
+
 config NPCM_ADC
 	tristate "Nuvoton NPCM ADC driver"
 	depends on ARCH_NPCM || COMPILE_TEST
drivers/iio/adc/Makefile +1 -0
index ee19afba62b7..0108472e141c 100644
@@ -94,6 +94,7 @@ obj-$(CONFIG_MESON_SARADC) += meson_saradc.o
 obj-$(CONFIG_MP2629_ADC) += mp2629_adc.o
 obj-$(CONFIG_MXS_LRADC_ADC) += mxs-lradc-adc.o
 obj-$(CONFIG_NAU7802) += nau7802.o
+obj-$(CONFIG_NCT7201) += nct7201.o
 obj-$(CONFIG_NPCM_ADC) += npcm_adc.o
 obj-$(CONFIG_PAC1921) += pac1921.o
 obj-$(CONFIG_PAC1934) += pac1934.o
drivers/iio/adc/nct7201.c new file +487 -0

487 lines not on this page · raw message

Signature · 1 line
2.34.1
reviewer
#6 lore raw

Re: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201

Quoted 24 lines · Eason Yang

Add Nuvoton NCT7201/NCT7202 system voltage monitor 12-bit ADC driver

NCT7201/NCT7202 supports up to 12 analog voltage monitor inputs and up to 4 SMBus addresses by ADDR pin. Meanwhile, ALERT# hardware event pins for independent alarm signals, and the all threshold values could be set for system protection without any timing delay. It also supports reset input RSTIN# to recover system from a fault condition.

Currently, only single-edge mode conversion and threshold events support.

---
 MAINTAINERS               |   1 +
 drivers/iio/adc/Kconfig   |  11 +
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/nct7201.c | 487 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 500 insertions(+)
 create mode 100644 drivers/iio/adc/nct7201.c
MAINTAINERS +0 -0
index fdc4aa5c7eff..389cbbdae1a7 100644

MAINTAINERS F: arch/arm/mach-npcm/ · line 2841

Quoted 4 lines
MAINTAINERS +1 -0
@@ -2838,6 +2838,7 @@ F:	arch/arm/mach-npcm/
 F:	arch/arm64/boot/dts/nuvoton/
 F:	drivers/*/*/*npcm*
 F:	drivers/*/*npcm*
+F:	drivers/iio/adc/nct7201.c

Same comment as DT bindings, this is ARM/NUVOTON NPCM ARCHITECTURE. We need a new section for this chip since it is stand-alone.

Quoted 3 lines
MAINTAINERS +0 -0
 F:	drivers/rtc/rtc-nct3018y.c
 F:	include/dt-bindings/clock/nuvoton,npcm7xx-clock.h
 F:	include/dt-bindings/clock/nuvoton,npcm845-clk.h

...

Quoted 11 lines
drivers/iio/adc/nct7201.c new file +5 -0
new file mode 100644
index 000000000000..c5d1540bcc00
@@ -0,0 +1,487 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Nuvoton nct7201 and nct7202 power monitor chips.
+ *
+ * Copyright (c) 2024-2025 Nuvoton Technology corporation.

drivers/iio/adc/nct7201.c line 15

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/init.h>

Are we really using something from the init header?

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+

...

drivers/iio/adc/nct7201.c line 205

Quoted 13 lines
drivers/iio/adc/nct7201.c new file +13 -0
+static int nct7201_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	u16 volt;
+	unsigned int value;
+	int err;
+	struct nct7201_chip_info *chip = iio_priv(indio_dev);
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	guard(mutex)(&chip->access_lock);

The regmap should already have an intneral lock, so this mutex seems reduandnt in it's current usage.

Quoted 32 lines
drivers/iio/adc/nct7201.c new file +32 -0
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		err = regmap_read(chip->regmap16, NCT7201_REG_VIN(chan->address), &value);
+		if (err < 0)
+			return err;
+		volt = value;
+		*val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		/* From the datasheet, we have to multiply by 0.0004995 */
+		*val = 0;
+		*val2 = 499500;
+		return IIO_VAL_INT_PLUS_NANO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int nct7201_read_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
+{
+	struct nct7201_chip_info *chip = iio_priv(indio_dev);
+	u16 volt;
+	unsigned int value;
+	int err;
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;

drivers/iio/adc/nct7201.c line 247

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	if (dir == IIO_EV_DIR_FALLING) {
+		err = regmap_read(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),
+				  &value);
+		if (err < 0)
+			return err;
+		volt = value;

Assigning to volt seems reduant. We can just pass value to FIELD_GET() below.

Quoted 21 lines
drivers/iio/adc/nct7201.c new file +21 -0
+	} else {
+		err = regmap_read(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
+				  &value);
+		if (err < 0)
+			return err;
+		volt = value;
+	}
+
+	*val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
+
+	return IIO_VAL_INT;
+}
+
+static int nct7201_write_event_value(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int val, int val2)
+{
+	struct nct7201_chip_info *chip = iio_priv(indio_dev);

drivers/iio/adc/nct7201.c line 278

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EOPNOTSUPP;
+
+	if (dir == IIO_EV_DIR_FALLING)
+		regmap_write(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),
+			     FIELD_PREP(NCT7201_REG_VIN_MASK, val));

No error checking? Could just return here directly.

Quoted 7 lines
drivers/iio/adc/nct7201.c new file +7 -0
+	else
+		regmap_write(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
+			     FIELD_PREP(NCT7201_REG_VIN_MASK, val));
+
+	return 0;
+}
+

...

Quoted 8 lines
drivers/iio/adc/nct7201.c new file +8 -0
+static int nct7201_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      bool state)
+{
+	struct nct7201_chip_info *chip = iio_priv(indio_dev);
+	unsigned int mask;

drivers/iio/adc/nct7201.c line 316

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	mask = BIT(chan->address);
+
+	if (!state && (chip->vin_mask & mask))
+		chip->vin_mask &= ~mask;
+	else if (state && !(chip->vin_mask & mask))
+		chip->vin_mask |= mask;

This would be easier to read as:

	if (state)
		chip->vin_mask |= mask;
	else
		chip->vin_mask &= ~mask;

drivers/iio/adc/nct7201.c line 319

Quoted 3 lines
drivers/iio/adc/nct7201.c new file +3 -0
+
+	if (chip->num_vin_channels <= 8)
+		regmap_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1, chip->vin_mask);

No error checking?

Quoted 7 lines
drivers/iio/adc/nct7201.c new file +7 -0
+	else
+		regmap_bulk_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1,
+				  &chip->vin_mask, sizeof(chip->vin_mask));
+
+	return 0;
+}
+

...

drivers/iio/adc/nct7201.c line 360

Quoted 8 lines
drivers/iio/adc/nct7201.c new file +8 -0
+static int nct7201_init_chip(struct nct7201_chip_info *chip)
+{
+	u8 data[2];
+	unsigned int value;
+	int err;
+
+	regmap_write(chip->regmap, NCT7201_REG_CONFIGURATION,
+		     NCT7201_BIT_CONFIGURATION_RESET);

Check error return?

Quoted 21 lines
drivers/iio/adc/nct7201.c new file +21 -0
+
+	/*
+	 * After about 25 msecs, the device should be ready and then
+	 * the Power Up bit will be set to 1. If not, wait for it.
+	 */
+	mdelay(25);
+	err = regmap_read(chip->regmap, NCT7201_REG_BUSY_STATUS, &value);
+	if (err < 0)
+		return err;
+	if (!(value & NCT7201_BIT_PWR_UP))
+		return dev_err_probe(&chip->client->dev, -EIO,
+				     "Failed to power up after reset\n");
+
+	/* Enable Channel */
+	if (chip->num_vin_channels <= 8) {
+		data[0] = NCT7201_REG_CHANNEL_ENABLE_1_MASK;
+		err = regmap_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1, data[0]);
+		if (err < 0)
+			return dev_err_probe(&chip->client->dev, -EIO,
+					     "Failed to write NCT7201_REG_CHANNEL_ENABLE_1\n");
+	} else {

drivers/iio/adc/nct7201.c line 391

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+		data[0] = NCT7201_REG_CHANNEL_ENABLE_1_MASK;
+		data[1] = NCT7201_REG_CHANNEL_ENABLE_2_MASK;
+		err = regmap_bulk_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1,
+					data, ARRAY_SIZE(data));
+		if (err < 0)
+			return dev_err_probe(&chip->client->dev, -EIO,
+					    "Failed to write NCT7201_REG_CHANNEL_ENABLE_1 and NCT7201_REG_CHANNEL_ENABLE_2\n");
+	}
+
+	value = get_unaligned_le16(data);

Does it matter that data[1] may be uninitialized and contain random value here?

drivers/iio/adc/nct7201.c line 392

Quoted 1 line
drivers/iio/adc/nct7201.c new file +1 -0
+	chip->vin_mask = value;

Don't really need the intermediate value assignment here.

Quoted 12 lines
drivers/iio/adc/nct7201.c new file +12 -0
+
+	/* Start monitoring if needed */
+	err = regmap_read(chip->regmap, NCT7201_REG_CONFIGURATION, &value);
+	if (err < 0)
+		return dev_err_probe(&chip->client->dev, -EIO,
+				     "Failed to read NCT7201_REG_CONFIGURATION\n");
+
+	regmap_set_bits(chip->regmap, NCT7201_REG_CONFIGURATION, NCT7201_BIT_CONFIGURATION_START);
+
+	return 0;
+}
+
#7 lore raw

Re: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201

In reply to Eason Yang
kernel test robot noticed the following build errors:

Hi Eason,

kernel test robot noticed the following build errors:

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

url:    https://github.com/intel-lab-lkp/linux/commits/Eason-Yang/dt-bindings-iio-adc-add-NCT7201-ADCs/20250221-171244
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20250221090918.1487689-3-j2anfernee%40gmail.com
patch subject: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20250222/[email protected]/config)
compiler: sh4-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250222/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

drivers/iio/adc/nct7201.c: In function 'nct7201_read_raw':

>> drivers/iio/adc/nct7201.c:212:24: error: implicit declaration of function 'FIELD_GET' [-Wimplicit-function-declaration]
     212 |                 *val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
         |                        ^~~~~~~~~
   drivers/iio/adc/nct7201.c: In function 'nct7201_write_event_value':
>> drivers/iio/adc/nct7201.c:278:30: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
     278 |                              FIELD_PREP(NCT7201_REG_VIN_MASK, val));
         |                              ^~~~~~~~~~

vim +/FIELD_GET +212 drivers/iio/adc/nct7201.c

192
193 static int nct7201_read_raw(struct iio_dev *indio_dev,
194 struct iio_chan_spec const *chan,
195 int *val, int *val2, long mask)
196 {
197 u16 volt;
198 unsigned int value;
199 int err;
200 struct nct7201_chip_info *chip = iio_priv(indio_dev);
201
202 if (chan->type != IIO_VOLTAGE)
203 return -EOPNOTSUPP;
204
205 guard(mutex)(&chip->access_lock);
206 switch (mask) {
207 case IIO_CHAN_INFO_RAW:
208 err = regmap_read(chip->regmap16, NCT7201_REG_VIN(chan->address), &value);
209 if (err < 0)
210 return err;
211 volt = value;

 > 212			*val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
   213			return IIO_VAL_INT;
   214		case IIO_CHAN_INFO_SCALE:
   215			/* From the datasheet, we have to multiply by 0.0004995 */
   216			*val = 0;
   217			*val2 = 499500;
   218			return IIO_VAL_INT_PLUS_NANO;
   219		default:
   220			return -EINVAL;
   221		}
   222	}
   223	
   224	static int nct7201_read_event_value(struct iio_dev *indio_dev,
   225					    const struct iio_chan_spec *chan,
   226					    enum iio_event_type type,
   227					    enum iio_event_direction dir,
   228					    enum iio_event_info info,
   229					    int *val, int *val2)
   230	{
   231		struct nct7201_chip_info *chip = iio_priv(indio_dev);
   232		u16 volt;
   233		unsigned int value;
   234		int err;
   235	
   236		if (chan->type != IIO_VOLTAGE)
   237			return -EOPNOTSUPP;
   238	
   239		if (info != IIO_EV_INFO_VALUE)
   240			return -EINVAL;
   241	
   242		if (dir == IIO_EV_DIR_FALLING) {
   243			err = regmap_read(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),
   244					  &value);
   245			if (err < 0)
   246				return err;
   247			volt = value;
   248		} else {
   249			err = regmap_read(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
   250					  &value);
   251			if (err < 0)
   252				return err;
   253			volt = value;
   254		}
   255	
   256		*val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
   257	
   258		return IIO_VAL_INT;
   259	}
   260	
   261	static int nct7201_write_event_value(struct iio_dev *indio_dev,
   262					     const struct iio_chan_spec *chan,
   263					     enum iio_event_type type,
   264					     enum iio_event_direction dir,
   265					     enum iio_event_info info,
   266					     int val, int val2)
   267	{
   268		struct nct7201_chip_info *chip = iio_priv(indio_dev);
   269	
   270		if (chan->type != IIO_VOLTAGE)
   271			return -EOPNOTSUPP;
   272	
   273		if (info != IIO_EV_INFO_VALUE)
   274			return -EOPNOTSUPP;
   275	
   276		if (dir == IIO_EV_DIR_FALLING)
   277			regmap_write(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),
 > 278				     FIELD_PREP(NCT7201_REG_VIN_MASK, val));

279 else
280 regmap_write(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
281 FIELD_PREP(NCT7201_REG_VIN_MASK, val));
282
283 return 0;
284 }
285

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#8 lore raw

Re: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201

In reply to Eason Yang
Quoted 11 lines · On Fri, 21 Feb 2025 17:09:18 +0800 Eason Yang <[email protected]> wrote:

Add Nuvoton NCT7201/NCT7202 system voltage monitor 12-bit ADC driver

NCT7201/NCT7202 supports up to 12 analog voltage monitor inputs and up to 4 SMBus addresses by ADDR pin. Meanwhile, ALERT# hardware event pins for independent alarm signals, and the all threshold values could be set for system protection without any timing delay. It also supports reset input RSTIN# to recover system from a fault condition.

Currently, only single-edge mode conversion and threshold events support.

Hi Eason

A few comments from me. May well overlap in some places with other feedback.

Jonathan

Quoted 34 lines
drivers/iio/adc/nct7201.c new file +27 -0
new file mode 100644
index 000000000000..c5d1540bcc00
@@ -0,0 +1,487 @@
 
+
+#define NCT7201_VOLTAGE_CHANNEL(chan, addr)				\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = chan,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
+		.address = addr,					\
+		.event_spec = nct7201_events,				\
+		.num_event_specs = ARRAY_SIZE(nct7201_events),		\
+	}
+
+static const struct iio_chan_spec nct7201_channels[] = {
+	NCT7201_VOLTAGE_CHANNEL(1, 0),
+	NCT7201_VOLTAGE_CHANNEL(2, 1),
+	NCT7201_VOLTAGE_CHANNEL(3, 2),
+	NCT7201_VOLTAGE_CHANNEL(4, 3),
+	NCT7201_VOLTAGE_CHANNEL(5, 4),
+	NCT7201_VOLTAGE_CHANNEL(6, 5),
+	NCT7201_VOLTAGE_CHANNEL(7, 6),
+	NCT7201_VOLTAGE_CHANNEL(8, 7),
+};
+
+static const struct iio_chan_spec nct7202_channels[] = {
+	NCT7201_VOLTAGE_CHANNEL(1, 0),
+	NCT7201_VOLTAGE_CHANNEL(2, 1),

drivers/iio/adc/nct7201.c line 190

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+	NCT7201_VOLTAGE_CHANNEL(3, 2),
+	NCT7201_VOLTAGE_CHANNEL(4, 3),
+	NCT7201_VOLTAGE_CHANNEL(5, 4),
+	NCT7201_VOLTAGE_CHANNEL(6, 5),
+	NCT7201_VOLTAGE_CHANNEL(7, 6),
+	NCT7201_VOLTAGE_CHANNEL(8, 7),
+	NCT7201_VOLTAGE_CHANNEL(9, 8),
+	NCT7201_VOLTAGE_CHANNEL(10, 9),
+	NCT7201_VOLTAGE_CHANNEL(11, 10),
+	NCT7201_VOLTAGE_CHANNEL(12, 11),

We normally number channels from 0 which would simplify this but I don't really mind if you want to keep the offset of 1. Maybe just have one macro parameter though and do the +1 in the macro.

Quoted 22 lines
drivers/iio/adc/nct7201.c new file +21 -0
+};
 
+static int nct7201_read_event_value(struct iio_dev *indio_dev,
+				    const struct iio_chan_spec *chan,
+				    enum iio_event_type type,
+				    enum iio_event_direction dir,
+				    enum iio_event_info info,
+				    int *val, int *val2)
+{
+	struct nct7201_chip_info *chip = iio_priv(indio_dev);
+	u16 volt;
+	unsigned int value;
+	int err;
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EINVAL;
+
+	if (dir == IIO_EV_DIR_FALLING) {
+		err = regmap_read(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),

drivers/iio/adc/nct7201.c line 253

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+				  &value);
+		if (err < 0)
+			return err;
+		volt = value;
+	} else {
+		err = regmap_read(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
+				  &value);
+		if (err < 0)
+			return err;
+		volt = value;

No real point in assigning to volt here,

Quoted 18 lines
drivers/iio/adc/nct7201.c new file +18 -0
+	}
+
+	*val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
+
+	return IIO_VAL_INT;
+}
+
+static int nct7201_write_event_value(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     enum iio_event_info info,
+				     int val, int val2)
+{
+	struct nct7201_chip_info *chip = iio_priv(indio_dev);
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;

drivers/iio/adc/nct7201.c line 281

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+
+	if (info != IIO_EV_INFO_VALUE)
+		return -EOPNOTSUPP;
+
+	if (dir == IIO_EV_DIR_FALLING)
+		regmap_write(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),
+			     FIELD_PREP(NCT7201_REG_VIN_MASK, val));
+	else
+		regmap_write(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
+			     FIELD_PREP(NCT7201_REG_VIN_MASK, val));

Check for error returns.

Quoted 31 lines
drivers/iio/adc/nct7201.c new file +31 -0
+
+	return 0;
+}
+
+static int nct7201_read_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir)
+{
+	struct nct7201_chip_info *chip = iio_priv(indio_dev);
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	return !!(chip->vin_mask & BIT(chan->address));
+}
+
+static int nct7201_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      bool state)
+{
+	struct nct7201_chip_info *chip = iio_priv(indio_dev);
+	unsigned int mask;
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	mask = BIT(chan->address);
+

drivers/iio/adc/nct7201.c line 322

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+	if (!state && (chip->vin_mask & mask))
+		chip->vin_mask &= ~mask;
+	else if (state && !(chip->vin_mask & mask))
+		chip->vin_mask |= mask;
+
+	if (chip->num_vin_channels <= 8)
+		regmap_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1, chip->vin_mask);
+	else
+		regmap_bulk_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1,
+				  &chip->vin_mask, sizeof(chip->vin_mask));

Check errors on these writes.

drivers/iio/adc/nct7201.c line 360

Quoted 12 lines
drivers/iio/adc/nct7201.c new file +11 -0
+
+	return 0;
+}
 
+static int nct7201_init_chip(struct nct7201_chip_info *chip)
+{
+	u8 data[2];
+	unsigned int value;
+	int err;
+
+	regmap_write(chip->regmap, NCT7201_REG_CONFIGURATION,
+		     NCT7201_BIT_CONFIGURATION_RESET);

Check for errors on all accesses to the device. It can get fiddly in paths where you are already handling an error but that's not the case here.

drivers/iio/adc/nct7201.c line 364

Quoted 4 lines
drivers/iio/adc/nct7201.c new file +4 -0
+
+	/*
+	 * After about 25 msecs, the device should be ready and then
+	 * the Power Up bit will be set to 1. If not, wait for it.

Wrap to 80 chars.

Quoted 18 lines
drivers/iio/adc/nct7201.c new file +18 -0
+	 */
+	mdelay(25);
+	err = regmap_read(chip->regmap, NCT7201_REG_BUSY_STATUS, &value);
+	if (err < 0)
+		return err;
+	if (!(value & NCT7201_BIT_PWR_UP))
+		return dev_err_probe(&chip->client->dev, -EIO,
+				     "Failed to power up after reset\n");
+
+	/* Enable Channel */
+	if (chip->num_vin_channels <= 8) {
+		data[0] = NCT7201_REG_CHANNEL_ENABLE_1_MASK;
+		err = regmap_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1, data[0]);
+		if (err < 0)
+			return dev_err_probe(&chip->client->dev, -EIO,
+					     "Failed to write NCT7201_REG_CHANNEL_ENABLE_1\n");
+	} else {
+		data[0] = NCT7201_REG_CHANNEL_ENABLE_1_MASK;

drivers/iio/adc/nct7201.c line 392

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+		data[1] = NCT7201_REG_CHANNEL_ENABLE_2_MASK;
+		err = regmap_bulk_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE_1,
+					data, ARRAY_SIZE(data));
+		if (err < 0)
+			return dev_err_probe(&chip->client->dev, -EIO,
+					    "Failed to write NCT7201_REG_CHANNEL_ENABLE_1 and NCT7201_REG_CHANNEL_ENABLE_2\n");
+	}
+
+	value = get_unaligned_le16(data);
+	chip->vin_mask = value;

Local variable doesn't seem to add any benefits so just assign vin_mask directly.

Quoted 22 lines
drivers/iio/adc/nct7201.c new file +22 -0
+
+	/* Start monitoring if needed */
+	err = regmap_read(chip->regmap, NCT7201_REG_CONFIGURATION, &value);
+	if (err < 0)
+		return dev_err_probe(&chip->client->dev, -EIO,
+				     "Failed to read NCT7201_REG_CONFIGURATION\n");
+
+	regmap_set_bits(chip->regmap, NCT7201_REG_CONFIGURATION, NCT7201_BIT_CONFIGURATION_START);
+
+	return 0;
+}
+
+static int nct7201_probe(struct i2c_client *client)
+{
+	const struct nct7201_adc_model_data *model_data;
+	struct nct7201_chip_info *chip;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	model_data = i2c_get_match_data(client);
+	if (!model_data)
+		return -EINVAL;

drivers/iio/adc/nct7201.c line 424

Quoted 10 lines
drivers/iio/adc/nct7201.c new file +10 -0
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	if (!indio_dev)
+		return -ENOMEM;
+	chip = iio_priv(indio_dev);
+
+	chip->regmap = devm_regmap_init_i2c(client, &nct7201_regmap8_config);
+	if (IS_ERR(chip->regmap))
+		return dev_err_probe(&client->dev, PTR_ERR(chip->regmap),
+			"Failed to init regmap\n");

Where it doesn't lead to really long lines, align all parameters to just after the opening bracket.

Quoted 29 lines
drivers/iio/adc/nct7201.c new file +29 -0
+
+	chip->regmap16 = devm_regmap_init_i2c(client, &nct7201_regmap16_config);
+	if (IS_ERR(chip->regmap16))
+		return dev_err_probe(&client->dev, PTR_ERR(chip->regmap16),
+			"Failed to init regmap16\n");
+
+	chip->num_vin_channels = model_data->num_vin_channels;
+
+	ret = devm_mutex_init(&client->dev, &chip->access_lock);
+	if (ret)
+		return ret;
+
+	chip->client = client;
+
+	ret = nct7201_init_chip(chip);
+	if (ret < 0)
+		return ret;
+
+	indio_dev->name = model_data->model_name;
+	indio_dev->channels = model_data->channels;
+	indio_dev->num_channels = model_data->num_channels;
+	if (client->irq)
+		indio_dev->info = &nct7201_info;
+	else
+		indio_dev->info = &nct7201_info_no_irq;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	return devm_iio_device_register(&client->dev, indio_dev);
+}
#9 lore raw

Re: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201

In reply to Eason Yang

Dear Eason,

Thank you for your patch.

Am 21.02.25 um 10:09 schrieb Eason Yang:

Quoted 5 lines

Add Nuvoton NCT7201/NCT7202 system voltage monitor 12-bit ADC driver

NCT7201/NCT7202 supports up to 12 analog voltage monitor inputs and up to 4 SMBus addresses by ADDR pin. Meanwhile, ALERT# hardware event pins for independent alarm signals, and the all threshold values could be set for

… and all the threshold values …

Quoted 4 lines

system protection without any timing delay. It also supports reset input RSTIN# to recover system from a fault condition.

Currently, only single-edge mode conversion and threshold events support.

… are supported.

It’d be great if you added a datasheet name and revision, and, if publicly available, a URL.

Quoted 8 lines
---
  MAINTAINERS               |   1 +
  drivers/iio/adc/Kconfig   |  11 +
  drivers/iio/adc/Makefile  |   1 +
  drivers/iio/adc/nct7201.c | 487 ++++++++++++++++++++++++++++++++++++++
  4 files changed, 500 insertions(+)

create mode 100644 drivers/iio/adc/nct7201.c

[…]

Kind regards,

Paul

#10 lore raw

Re: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201

In reply to Eason Yang
kernel test robot noticed the following build errors:

Hi Eason,

kernel test robot noticed the following build errors:

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

url:    https://github.com/intel-lab-lkp/linux/commits/Eason-Yang/dt-bindings-iio-adc-add-NCT7201-ADCs/20250221-171244
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20250221090918.1487689-3-j2anfernee%40gmail.com
patch subject: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20250304/[email protected]/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250304/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> drivers/iio/adc/nct7201.c:212:10: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     212 |                 *val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
         |                        ^
   drivers/iio/adc/nct7201.c:256:9: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     256 |         *val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
         |                ^
>> drivers/iio/adc/nct7201.c:278:9: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     278 |                              FIELD_PREP(NCT7201_REG_VIN_MASK, val));
         |                              ^

3 errors generated.

vim +/FIELD_GET +212 drivers/iio/adc/nct7201.c

192
193 static int nct7201_read_raw(struct iio_dev *indio_dev,
194 struct iio_chan_spec const *chan,
195 int *val, int *val2, long mask)
196 {
197 u16 volt;
198 unsigned int value;
199 int err;
200 struct nct7201_chip_info *chip = iio_priv(indio_dev);
201
202 if (chan->type != IIO_VOLTAGE)
203 return -EOPNOTSUPP;
204
205 guard(mutex)(&chip->access_lock);
206 switch (mask) {
207 case IIO_CHAN_INFO_RAW:
208 err = regmap_read(chip->regmap16, NCT7201_REG_VIN(chan->address), &value);
209 if (err < 0)
210 return err;
211 volt = value;

 > 212			*val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
   213			return IIO_VAL_INT;
   214		case IIO_CHAN_INFO_SCALE:
   215			/* From the datasheet, we have to multiply by 0.0004995 */
   216			*val = 0;
   217			*val2 = 499500;
   218			return IIO_VAL_INT_PLUS_NANO;
   219		default:
   220			return -EINVAL;
   221		}
   222	}
   223	
   224	static int nct7201_read_event_value(struct iio_dev *indio_dev,
   225					    const struct iio_chan_spec *chan,
   226					    enum iio_event_type type,
   227					    enum iio_event_direction dir,
   228					    enum iio_event_info info,
   229					    int *val, int *val2)
   230	{
   231		struct nct7201_chip_info *chip = iio_priv(indio_dev);
   232		u16 volt;
   233		unsigned int value;
   234		int err;
   235	
   236		if (chan->type != IIO_VOLTAGE)
   237			return -EOPNOTSUPP;
   238	
   239		if (info != IIO_EV_INFO_VALUE)
   240			return -EINVAL;
   241	
   242		if (dir == IIO_EV_DIR_FALLING) {
   243			err = regmap_read(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),
   244					  &value);
   245			if (err < 0)
   246				return err;
   247			volt = value;
   248		} else {
   249			err = regmap_read(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
   250					  &value);
   251			if (err < 0)
   252				return err;
   253			volt = value;
   254		}
   255	
   256		*val = FIELD_GET(NCT7201_REG_VIN_MASK, volt);
   257	
   258		return IIO_VAL_INT;
   259	}
   260	
   261	static int nct7201_write_event_value(struct iio_dev *indio_dev,
   262					     const struct iio_chan_spec *chan,
   263					     enum iio_event_type type,
   264					     enum iio_event_direction dir,
   265					     enum iio_event_info info,
   266					     int val, int val2)
   267	{
   268		struct nct7201_chip_info *chip = iio_priv(indio_dev);
   269	
   270		if (chan->type != IIO_VOLTAGE)
   271			return -EOPNOTSUPP;
   272	
   273		if (info != IIO_EV_INFO_VALUE)
   274			return -EOPNOTSUPP;
   275	
   276		if (dir == IIO_EV_DIR_FALLING)
   277			regmap_write(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),
 > 278				     FIELD_PREP(NCT7201_REG_VIN_MASK, val));

279 else
280 regmap_write(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
281 FIELD_PREP(NCT7201_REG_VIN_MASK, val));
282
283 return 0;
284 }
285

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
maintainer
#11 lore raw
In reply to Eason Yang

On Fri, 21 Feb 2025 17:09:16 +0800
Eason Yang <[email protected]> wrote:

Hi Eason,

Not sure if I asked this before, but this is a device that seems to be typically used for hardware monitoring and there are a number of similar sounding device in drivers/hwmon/

That raises a couple of questions:
1) Is it compatible with any of those existing drivers?
2) Why IIO rather than HWMON?

There isn't normally a problem with having a hardware monitoring related device supported by IIO, it is just good to know if your usecase makes that a good idea. We have the iio-hwmon bridge driver to solve the problem of a device than can be used either as a generic ADC or as a hwmon type monitoring device (which tends to have more alarms etc)

Jonathan

Quoted 43 lines

Change since version 4:
- Fix comments
- Add interrupts and reset-gpios to the DT example
- Use the FIELD_PREP and FIELD_GET
- Add use_single_write in regmap_config
- Use regmap_access_table

Change since version 3:
- Fix comments
- Don't put nct720"x" in the name, just call it nct7201
- Remove differential inputs until conversions are finished
- Add NCT7201_ prefix in all macros and avoid the tables
- Correct event threshold values in raw units
- Add with and without interrupt callback function to have the event config part and one that doesn't
- Remove print an error message if regmap_wirte failed case

Change since version 2:
- Remvoe read-vin-data-size property, default use read word vin data
- Use regmap instead of i2c smbus API
- IIO should be IIO_CHAN_INFO_RAW and _SCALE not _PROCESSED
- Use dev_xxx_probe in probe function and dev_xxx in other functions
- Use devm_iio_device_register replace of iio_device_register
- Use guard(mutex) replace of mutex_lock
- Use get_unaligned_le16 conversion API

Changes since version 1:
- Add new property in iio:adc binding document
- Add new driver for Nuvoton NCT720x driver

Eason Yang (2):
dt-bindings: iio: adc: add NCT7201 ADCs
iio: adc: add support for Nuvoton NCT7201

 .../bindings/iio/adc/nuvoton,nct7201.yaml     |  57 ++
 MAINTAINERS                                   |   2 +
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/nct7201.c                     | 487 ++++++++++++++++++
 5 files changed, 558 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
 create mode 100644 drivers/iio/adc/nct7201.c
author maintainer
#12 lore raw

Dear Jonathan,

Thanks again for the review

Jonathan Cameron <[email protected]> 於 2025年2月22日 週六 下午11:25寫道:

Quoted 4 lines

On Fri, 21 Feb 2025 17:09:16 +0800
Eason Yang <[email protected]> wrote:

Quoted 8 lines

Hi Eason,

Not sure if I asked this before, but this is a device that seems to be typically used for hardware monitoring and there are a number of similar sounding device in drivers/hwmon/

That raises a couple of questions:
1) Is it compatible with any of those existing drivers?

No, we need a new driver to do the ADC conversion for NCT7201/NCT7202.

Quoted 1 line

2) Why IIO rather than HWMON?

A driver for NCT7201 with more functionality is available in the iio adc subsystem.
If the chip is used for hardware monitoring, the iio->hwmon bridge should be used.

Quoted 7 lines

There isn't normally a problem with having a hardware monitoring related device supported by IIO, it is just good to know if your usecase makes that a good idea. We have the iio-hwmon bridge driver to solve the problem of a device than can be used either as a generic ADC or as a hwmon type monitoring device (which tends to have more alarms etc)

Yes, we also verify the iio-hwmon bridge.

Quoted 48 lines

Jonathan

Quoted 43 lines

Change since version 4:
- Fix comments
- Add interrupts and reset-gpios to the DT example
- Use the FIELD_PREP and FIELD_GET
- Add use_single_write in regmap_config
- Use regmap_access_table

Change since version 3:
- Fix comments
- Don't put nct720"x" in the name, just call it nct7201
- Remove differential inputs until conversions are finished
- Add NCT7201_ prefix in all macros and avoid the tables
- Correct event threshold values in raw units
- Add with and without interrupt callback function to have the event config part and one that doesn't
- Remove print an error message if regmap_wirte failed case

Change since version 2:
- Remvoe read-vin-data-size property, default use read word vin data
- Use regmap instead of i2c smbus API
- IIO should be IIO_CHAN_INFO_RAW and _SCALE not _PROCESSED
- Use dev_xxx_probe in probe function and dev_xxx in other functions
- Use devm_iio_device_register replace of iio_device_register
- Use guard(mutex) replace of mutex_lock
- Use get_unaligned_le16 conversion API

Changes since version 1:
- Add new property in iio:adc binding document
- Add new driver for Nuvoton NCT720x driver

Eason Yang (2):
dt-bindings: iio: adc: add NCT7201 ADCs
iio: adc: add support for Nuvoton NCT7201

 .../bindings/iio/adc/nuvoton,nct7201.yaml     |  57 ++
 MAINTAINERS                                   |   2 +
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/nct7201.c                     | 487 ++++++++++++++++++
 5 files changed, 558 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml
 create mode 100644 drivers/iio/adc/nct7201.c