iio: adc: add Nuvoton NCT7201 ADC driver
Subsystems (3)
Eason Yang
maintainerv4 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
Eason Yang
maintainerPATCH 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
@@ -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
@@ -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
David Lechner
reviewerRe: [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.
Signed-off-by Eason Yang <[email protected]>--- .../bindings/iio/adc/nuvoton,nct7201.yaml | 57 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yamlDocumentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +22
-0@@ -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
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
-0F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* F: arch/arm/mach-npcm/
Yu-Hsian Yang
maintainerRe: [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.
Signed-off-by Eason Yang <[email protected]>--- .../bindings/iio/adc/nuvoton,nct7201.yaml | 57 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yamlDocumentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +26
-0@@ -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..fdc4aa5c7eff100644MAINTAINERS +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
-0F: Documentation/devicetree/bindings/rtc/nuvoton,nct3018y.yaml F: arch/arm/boot/dts/nuvoton/nuvoton-npcm* F: arch/arm/mach-npcm/
Re: [PATCH v4 1/2] dt-bindings: iio: adc: add NCT7201 ADCs
In reply to Eason YangQuoted 28 lines · Eason Yang
Add a binding specification for the Nuvoton NCT7201/NCT7202 up to 12-bit ADCs with I2C interface.
Signed-off-by Eason Yang <[email protected]>--- .../bindings/iio/adc/nuvoton,nct7201.yaml | 57 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yamlDocumentation/devicetree/bindings/iio/adc/nuvoton,nct7201.yaml new file +12
-0@@ -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@@ -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
Eason Yang
maintainerPATCH v4 2/2 iio: adc: add support for Nuvoton NCT7201
In reply to Eason YangAdd 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
@@ -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
@@ -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
@@ -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
David Lechner
reviewerRe: [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.
Signed-off-by Eason Yang <[email protected]>--- 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.cMAINTAINERS +0
-0
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
-0F: 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@@ -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 ; +} +
kernel test robot
build botRe: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201
In reply to Eason Yangkernel 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
Jonathan Cameron
maintainerRe: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201
In reply to Eason YangQuoted 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.
Signed-off-by Eason Yang <[email protected]>
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@@ -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); +}
Re: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201
In reply to Eason YangDear 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
Signed-off-by Eason Yang <[email protected]>--- 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
kernel test robot
build botRe: [PATCH v4 2/2] iio: adc: add support for Nuvoton NCT7201
In reply to Eason Yangkernel 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
Jonathan Cameron
maintainerOn 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 theFIELD_PREPandFIELD_GET
- Adduse_single_writeinregmap_config
- Useregmap_access_tableChange since version 3:
- Fix comments
- Don't put nct720"x" in the name, just call it nct7201
- Remove differential inputs until conversions are finished
- AddNCT7201_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 ifregmap_wirtefailed caseChange since version 2:
- Remvoe read-vin-data-size property, default use read word vin data
- Use regmap instead of i2c smbus API
- IIO should beIIO_CHAN_INFO_RAWand_SCALEnot_PROCESSED
- Usedev_xxx_probein probe function anddev_xxxin other functions
- Usedevm_iio_device_registerreplace ofiio_device_register
- Use guard(mutex) replace ofmutex_lock
- Useget_unaligned_le16conversion APIChanges since version 1:
- Add new property in iio:adc binding document
- Add new driver for Nuvoton NCT720x driverEason 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
Yu-Hsian Yang
maintainerDear 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 theFIELD_PREPandFIELD_GET
- Adduse_single_writeinregmap_config
- Useregmap_access_tableChange since version 3:
- Fix comments
- Don't put nct720"x" in the name, just call it nct7201
- Remove differential inputs until conversions are finished
- AddNCT7201_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 ifregmap_wirtefailed caseChange since version 2:
- Remvoe read-vin-data-size property, default use read word vin data
- Use regmap instead of i2c smbus API
- IIO should beIIO_CHAN_INFO_RAWand_SCALEnot_PROCESSED
- Usedev_xxx_probein probe function anddev_xxxin other functions
- Usedevm_iio_device_registerreplace ofiio_device_register
- Use guard(mutex) replace ofmutex_lock
- Useget_unaligned_le16conversion APIChanges since version 1:
- Add new property in iio:adc binding document
- Add new driver for Nuvoton NCT720x driverEason 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