diff mbox series

[v1,2/2] iio: adc: add Nuvoton NCT720x ADC driver

Message ID 20241106023916.440767-3-j2anfernee@gmail.com
State New
Headers show
Series iio: adc: add Nuvoton NCT720x ADC driver | expand

Commit Message

Yu-Hsian Yang Nov. 6, 2024, 2:39 a.m. UTC
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 <j2anfernee@gmail.com>
---
 MAINTAINERS               |   1 +
 drivers/iio/adc/Kconfig   |   9 +
 drivers/iio/adc/Makefile  |   1 +
 drivers/iio/adc/nct720x.c | 617 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 628 insertions(+)
 create mode 100644 drivers/iio/adc/nct720x.c

Comments

kernel test robot Nov. 6, 2024, 12:30 p.m. UTC | #1
Hi Eason,

kernel test robot noticed the following build warnings:

[auto build test WARNING on jic23-iio/togreg]
[also build test WARNING on linus/master v6.12-rc6 next-20241106]
[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-binding-for-Nuvoton-NCT720x-ADCs/20241106-104046
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20241106023916.440767-3-j2anfernee%40gmail.com
patch subject: [PATCH v1 2/2] iio: adc: add Nuvoton NCT720x ADC driver
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241106/202411062051.TLRkJSSL-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241106/202411062051.TLRkJSSL-lkp@intel.com/reproduce)

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

All warnings (new ones prefixed by >>):

   In file included from drivers/iio/adc/nct720x.c:9:
   In file included from include/linux/device.h:32:
   In file included from include/linux/device/driver.h:21:
   In file included from include/linux/module.h:19:
   In file included from include/linux/elf.h:6:
   In file included from arch/s390/include/asm/elf.h:181:
   In file included from arch/s390/include/asm/mmu_context.h:11:
   In file included from arch/s390/include/asm/pgalloc.h:18:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/iio/adc/nct720x.c:526:16: warning: cast to smaller integer type 'enum nct720x_chips' from 'const void *' [-Wvoid-pointer-to-enum-cast]
     526 |                 chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
         |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   5 warnings generated.


vim +526 drivers/iio/adc/nct720x.c

   511	
   512	static int nct720x_probe(struct i2c_client *client)
   513	{
   514		const struct i2c_device_id *id = i2c_client_get_device_id(client);
   515		struct nct720x_chip_info *chip;
   516		struct iio_dev *indio_dev;
   517		int ret;
   518		u32 tmp;
   519	
   520		indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
   521		if (!indio_dev)
   522			return -ENOMEM;
   523		chip = iio_priv(indio_dev);
   524	
   525		if (client->dev.of_node)
 > 526			chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
   527		else
   528			chip->type = i2c_match_id(nct720x_id, client)->driver_data;
   529	
   530		chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
   531	
   532		ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
   533		if (ret < 0) {
   534			pr_err("read-vin-data-size property not found\n");
   535			return ret;
   536		}
   537	
   538		if (tmp == 8) {
   539			chip->use_read_byte_vin = true;
   540		} else if (tmp == 16) {
   541			chip->use_read_byte_vin = false;
   542		} else {
   543			pr_err("invalid read-vin-data-size (%d)\n", tmp);
   544			return -EINVAL;
   545		}
   546	
   547		mutex_init(&chip->access_lock);
   548	
   549		/* this is only used for device removal purposes */
   550		i2c_set_clientdata(client, indio_dev);
   551	
   552		chip->client = client;
   553	
   554		ret = nct720x_init_chip(chip);
   555		if (ret < 0)
   556			return ret;
   557	
   558		indio_dev->name = id->name;
   559		indio_dev->channels = nct720x_channels;
   560		indio_dev->num_channels = ARRAY_SIZE(nct720x_channels);
   561		indio_dev->info = &nct720x_info;
   562		indio_dev->modes = INDIO_DIRECT_MODE;
   563	
   564		iio_device_register(indio_dev);
   565	
   566		return 0;
   567	}
   568
Krzysztof Kozlowski Nov. 6, 2024, 1:41 p.m. UTC | #2
On 06/11/2024 03:39, Eason Yang 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 <j2anfernee@gmail.com>

...

> +
> +static int nct720x_probe(struct i2c_client *client)
> +{
> +	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> +	struct nct720x_chip_info *chip;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +	u32 tmp;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +	chip = iio_priv(indio_dev);
> +
> +	if (client->dev.of_node)
> +		chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
> +	else
> +		chip->type = i2c_match_id(nct720x_id, client)->driver_data;

I believe there is a I2C wrapper for above.

> +
> +	chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
> +
> +	ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
> +	if (ret < 0) {
> +		pr_err("read-vin-data-size property not found\n");

Please use dev_xxx in your driver code.


Best regards,
Krzysztof
Yu-Hsian Yang Nov. 7, 2024, 12:36 a.m. UTC | #3
Dear kernel test robot,

Thank you for your response.
I would check these warnings.

kernel test robot <lkp@intel.com> 於 2024年11月6日 週三 下午8:32寫道:
>
> Hi Eason,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on jic23-iio/togreg]
> [also build test WARNING on linus/master v6.12-rc6 next-20241106]
> [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-binding-for-Nuvoton-NCT720x-ADCs/20241106-104046
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
> patch link:    https://lore.kernel.org/r/20241106023916.440767-3-j2anfernee%40gmail.com
> patch subject: [PATCH v1 2/2] iio: adc: add Nuvoton NCT720x ADC driver
> config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241106/202411062051.TLRkJSSL-lkp@intel.com/config)
> compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241106/202411062051.TLRkJSSL-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202411062051.TLRkJSSL-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
>    In file included from drivers/iio/adc/nct720x.c:9:
>    In file included from include/linux/device.h:32:
>    In file included from include/linux/device/driver.h:21:
>    In file included from include/linux/module.h:19:
>    In file included from include/linux/elf.h:6:
>    In file included from arch/s390/include/asm/elf.h:181:
>    In file included from arch/s390/include/asm/mmu_context.h:11:
>    In file included from arch/s390/include/asm/pgalloc.h:18:
>    In file included from include/linux/mm.h:2213:
>    include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>      504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
>          |                            ~~~~~~~~~~~~~~~~~~~~~ ^
>      505 |                            item];
>          |                            ~~~~
>    include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>      511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
>          |                            ~~~~~~~~~~~~~~~~~~~~~ ^
>      512 |                            NR_VM_NUMA_EVENT_ITEMS +
>          |                            ~~~~~~~~~~~~~~~~~~~~~~
>    include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
>      518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
>          |                               ~~~~~~~~~~~ ^ ~~~
>    include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
>      524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
>          |                            ~~~~~~~~~~~~~~~~~~~~~ ^
>      525 |                            NR_VM_NUMA_EVENT_ITEMS +
>          |                            ~~~~~~~~~~~~~~~~~~~~~~
> >> drivers/iio/adc/nct720x.c:526:16: warning: cast to smaller integer type 'enum nct720x_chips' from 'const void *' [-Wvoid-pointer-to-enum-cast]
>      526 |                 chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
>          |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    5 warnings generated.
>
>
> vim +526 drivers/iio/adc/nct720x.c
>
>    511
>    512  static int nct720x_probe(struct i2c_client *client)
>    513  {
>    514          const struct i2c_device_id *id = i2c_client_get_device_id(client);
>    515          struct nct720x_chip_info *chip;
>    516          struct iio_dev *indio_dev;
>    517          int ret;
>    518          u32 tmp;
>    519
>    520          indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
>    521          if (!indio_dev)
>    522                  return -ENOMEM;
>    523          chip = iio_priv(indio_dev);
>    524
>    525          if (client->dev.of_node)
>  > 526                  chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
>    527          else
>    528                  chip->type = i2c_match_id(nct720x_id, client)->driver_data;
>    529
>    530          chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
>    531
>    532          ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
>    533          if (ret < 0) {
>    534                  pr_err("read-vin-data-size property not found\n");
>    535                  return ret;
>    536          }
>    537
>    538          if (tmp == 8) {
>    539                  chip->use_read_byte_vin = true;
>    540          } else if (tmp == 16) {
>    541                  chip->use_read_byte_vin = false;
>    542          } else {
>    543                  pr_err("invalid read-vin-data-size (%d)\n", tmp);
>    544                  return -EINVAL;
>    545          }
>    546
>    547          mutex_init(&chip->access_lock);
>    548
>    549          /* this is only used for device removal purposes */
>    550          i2c_set_clientdata(client, indio_dev);
>    551
>    552          chip->client = client;
>    553
>    554          ret = nct720x_init_chip(chip);
>    555          if (ret < 0)
>    556                  return ret;
>    557
>    558          indio_dev->name = id->name;
>    559          indio_dev->channels = nct720x_channels;
>    560          indio_dev->num_channels = ARRAY_SIZE(nct720x_channels);
>    561          indio_dev->info = &nct720x_info;
>    562          indio_dev->modes = INDIO_DIRECT_MODE;
>    563
>    564          iio_device_register(indio_dev);
>    565
>    566          return 0;
>    567  }
>    568
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
Yu-Hsian Yang Nov. 7, 2024, 12:41 a.m. UTC | #4
Dear Krzysztof Kozlowski,

Thank you for your response.

Krzysztof Kozlowski <krzk@kernel.org> 於 2024年11月6日 週三 下午9:41寫道:
>
> On 06/11/2024 03:39, Eason Yang 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 <j2anfernee@gmail.com>
>
> ...
>
> > +
> > +static int nct720x_probe(struct i2c_client *client)
> > +{
> > +     const struct i2c_device_id *id = i2c_client_get_device_id(client);
> > +     struct nct720x_chip_info *chip;
> > +     struct iio_dev *indio_dev;
> > +     int ret;
> > +     u32 tmp;
> > +
> > +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> > +     if (!indio_dev)
> > +             return -ENOMEM;
> > +     chip = iio_priv(indio_dev);
> > +
> > +     if (client->dev.of_node)
> > +             chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
> > +     else
> > +             chip->type = i2c_match_id(nct720x_id, client)->driver_data;
>
> I believe there is a I2C wrapper for above.
>

Got it.

> > +
> > +     chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
> > +
> > +     ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
> > +     if (ret < 0) {
> > +             pr_err("read-vin-data-size property not found\n");
>
> Please use dev_xxx in your driver code.

Got it.

>
>
> Best regards,
> Krzysztof
>
anish kumar Nov. 7, 2024, 6:13 a.m. UTC | #5
On Tue, Nov 5, 2024 at 6:39 PM Eason Yang <j2anfernee@gmail.com> 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 <j2anfernee@gmail.com>
> ---
>  MAINTAINERS               |   1 +
>  drivers/iio/adc/Kconfig   |   9 +
>  drivers/iio/adc/Makefile  |   1 +
>  drivers/iio/adc/nct720x.c | 617 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 628 insertions(+)
>  create mode 100644 drivers/iio/adc/nct720x.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 68570c58e7aa..9940de0ddca2 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2753,6 +2753,7 @@ F:        arch/arm/mach-npcm/
>  F:     arch/arm64/boot/dts/nuvoton/
>  F:     drivers/*/*/*npcm*
>  F:     drivers/*/*npcm*
> +F:     drivers/iio/adc/nct720x.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
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 6c4e74420fd2..adbbf0ca6f57 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1008,6 +1008,15 @@ config NAU7802
>           To compile this driver as a module, choose M here: the
>           module will be called nau7802.
>
> +config NCT720X
> +       tristate "Nuvoton Instruments NCT7201 and NCT7202 Power Monitor"
> +       depends on I2C
> +       help
> +         If you say yes here you get support for the Nuvoton NCT7201 and
> +         NCT7202 Voltage Monitor.
> +         This driver can also be built as a module. If so, the module
> +         will be called nct720x.
> +
>  config NPCM_ADC
>         tristate "Nuvoton NPCM ADC driver"
>         depends on ARCH_NPCM || COMPILE_TEST
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 7b91cd98c0e0..f53318e5aa04 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -91,6 +91,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_NCT720X) += nct720x.o
>  obj-$(CONFIG_NPCM_ADC) += npcm_adc.o
>  obj-$(CONFIG_PAC1921) += pac1921.o
>  obj-$(CONFIG_PAC1934) += pac1934.o
> diff --git a/drivers/iio/adc/nct720x.c b/drivers/iio/adc/nct720x.c
> new file mode 100644
> index 000000000000..e589479fd06e
> --- /dev/null
> +++ b/drivers/iio/adc/nct720x.c
> @@ -0,0 +1,617 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Driver for Nuvoton nct7201 and nct7202 power monitor chips.
> + *
> + * Copyright (c) 2022 Nuvoton Inc.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +
> +#define REG_CHIP_ID                    0xFD
> +#define NCT720X_ID                     0xD7
> +#define REG_VENDOR_ID                  0xFE
> +#define NUVOTON_ID                     0x50
> +#define REG_DEVICE_ID                  0xFF
> +#define NCT720X_DEVICE_ID              0x12
> +#define VIN_MAX                                12      /* Counted from 1 */
> +#define NCT7201_VIN_MAX                        8
> +#define NCT7202_VIN_MAX                        12
> +#define NCT720X_IN_SCALING             4995
> +
> +#define REG_INTERRUPT_STATUS_1         0x0C
> +#define REG_INTERRUPT_STATUS_2         0x0D
> +#define REG_VOLT_LOW_BYTE              0x0F
> +#define REG_CONFIGURATION              0x10
> +#define  CONFIGURATION_START           BIT(0)
> +#define  CONFIGURATION_ALERT_MSK       BIT(1)
> +#define  CONFIGURATION_CONV_RATE       BIT(2)
> +#define  CONFIGURATION_INIT            BIT(7)
> +
> +#define REG_ADVANCED_CONFIGURATION     0x11
> +#define  ADVANCED_CONF_MOD_ALERT       BIT(0)
> +#define  ADVANCED_CONF_MOD_STS         BIT(1)
> +#define  ADVANCED_CONF_FAULT_QUEUE     BIT(2)
> +#define  ADVANCED_CONF_EN_DEEP_SHUTDOWN        BIT(4)
> +#define  ADVANCED_CONF_EN_SMB_TIMEOUT  BIT(5)
> +#define  ADVANCED_CONF_MOD_RSTIN       BIT(7)
> +
> +#define REG_CHANNEL_INPUT_MODE         0x12
> +#define REG_CHANNEL_ENABLE_1           0x13
> +#define REG_CHANNEL_ENABLE_2           0x14
> +#define REG_INTERRUPT_MASK_1           0x15
> +#define REG_INTERRUPT_MASK_2           0x16
> +#define REG_BUSY_STATUS                        0x1E
> +#define REG_ONE_SHOT                   0x1F
> +#define REG_SMUS_ADDRESS               0xFC
> +
> +static const u8 REG_VIN[VIN_MAX] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05,
> +                                    0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B};
> +static const u8 REG_VIN_HIGH_LIMIT[VIN_MAX] = { 0x20, 0x22, 0x24, 0x26, 0x28, 0x2A,
> +                                               0x2C, 0x2E, 0x30, 0x32, 0x34, 0x36};
> +static const u8 REG_VIN_LOW_LIMIT[VIN_MAX] = { 0x21, 0x23, 0x25, 0x27, 0x29, 0x2B,
> +                                              0x2D, 0x2F, 0x31, 0x33, 0x35, 0x37};
> +static const u8 REG_VIN_HIGH_LIMIT_LSB[VIN_MAX] = { 0x40, 0x42, 0x44, 0x46, 0x48, 0x4A,
> +                                                   0x4C, 0x4E, 0x50, 0x52, 0x54, 0x56};
> +static const u8 REG_VIN_LOW_LIMIT_LSB[VIN_MAX] = { 0x41, 0x43, 0x45, 0x47, 0x49, 0x4B,
> +                                                  0x4D, 0x4F, 0x51, 0x53, 0x55, 0x57};
> +static u8 nct720x_chan_to_index[] = {
> +       0,      /* Not used */
> +       0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
> +};
> +
> +
> +/* List of supported devices */
> +enum nct720x_chips {
> +       nct7201, nct7202
> +};
> +
> +struct nct720x_chip_info {
> +       struct i2c_client       *client;
> +       enum nct720x_chips      type;
> +       struct mutex            access_lock;    /* for multi-byte read and write operations */
> +       int vin_max;                            /* number of VIN channels */
> +       u32 vin_mask;
> +       bool use_read_byte_vin;
> +};
> +
> +static const struct iio_event_spec nct720x_events[] = {
> +       {
> +               .type = IIO_EV_TYPE_THRESH,
> +               .dir = IIO_EV_DIR_RISING,
> +               .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +                                BIT(IIO_EV_INFO_ENABLE),
> +       }, {
> +               .type = IIO_EV_TYPE_THRESH,
> +               .dir = IIO_EV_DIR_FALLING,
> +               .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> +                                BIT(IIO_EV_INFO_ENABLE),
> +       },
> +};
> +
> +#define NCT720X_VOLTAGE_CHANNEL(chan, addr)                            \
> +       {                                                               \
> +               .type = IIO_VOLTAGE,                                    \
> +               .indexed = 1,                                           \
> +               .channel = chan,                                        \
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),     \
> +               .address = addr,                                        \
> +               .event_spec = nct720x_events,                           \
> +               .num_event_specs = ARRAY_SIZE(nct720x_events),          \
> +       }
> +
> +#define NCT720X_VOLTAGE_CHANNEL_DIFF(chan1, chan2, addr)               \
> +       {                                                               \
> +               .type = IIO_VOLTAGE,                                    \
> +               .indexed = 1,                                           \
> +               .channel = (chan1),                                     \
> +               .channel2 = (chan2),                                    \
> +               .differential = 1,                                      \
> +               .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),     \
> +               .address = addr,                                        \
> +               .event_spec = nct720x_events,                           \
> +               .num_event_specs = ARRAY_SIZE(nct720x_events),          \
> +       }
> +
> +static const struct iio_chan_spec nct720x_channels[] = {
> +       NCT720X_VOLTAGE_CHANNEL(1, 1),
> +       NCT720X_VOLTAGE_CHANNEL(2, 2),
> +       NCT720X_VOLTAGE_CHANNEL(3, 3),
> +       NCT720X_VOLTAGE_CHANNEL(4, 4),
> +       NCT720X_VOLTAGE_CHANNEL(5, 5),
> +       NCT720X_VOLTAGE_CHANNEL(6, 6),
> +       NCT720X_VOLTAGE_CHANNEL(7, 7),
> +       NCT720X_VOLTAGE_CHANNEL(8, 8),
> +       NCT720X_VOLTAGE_CHANNEL(9, 9),
> +       NCT720X_VOLTAGE_CHANNEL(10, 10),
> +       NCT720X_VOLTAGE_CHANNEL(11, 11),
> +       NCT720X_VOLTAGE_CHANNEL(12, 12),
> +       NCT720X_VOLTAGE_CHANNEL_DIFF(1, 2, 1),
> +       NCT720X_VOLTAGE_CHANNEL_DIFF(3, 4, 3),
> +       NCT720X_VOLTAGE_CHANNEL_DIFF(5, 6, 5),
> +       NCT720X_VOLTAGE_CHANNEL_DIFF(7, 8, 7),
> +       NCT720X_VOLTAGE_CHANNEL_DIFF(9, 10, 9),
> +       NCT720X_VOLTAGE_CHANNEL_DIFF(11, 12, 11),
> +};
> +
> +/* Read 1-byte register. Returns unsigned byte data or -ERRNO on error. */
> +static int nct720x_read_reg(struct nct720x_chip_info *chip, u8 reg)
> +{
> +       struct i2c_client *client = chip->client;
> +
> +       return i2c_smbus_read_byte_data(client, reg);
> +}
> +
> +/* Read 1-byte register. Returns unsigned word data or -ERRNO on error. */
> +static int nct720x_read_word_swapped_reg(struct nct720x_chip_info *chip, u8 reg)
> +{
> +       struct i2c_client *client = chip->client;
> +
> +       return i2c_smbus_read_word_swapped(client, reg);
> +}
> +
> +/*
> + * Read 2-byte register. Returns register in big-endian format or
> + * -ERRNO on error.
> + */
> +static int nct720x_read_reg16(struct nct720x_chip_info *chip, u8 reg)
> +{
> +       struct i2c_client *client = chip->client;
> +       int ret, low;
> +
> +       mutex_lock(&chip->access_lock);
> +       ret = i2c_smbus_read_byte_data(client, reg);
> +       if (ret >= 0) {
> +               low = ret;
> +               ret = i2c_smbus_read_byte_data(client, reg + 1);
> +               if (ret >= 0)
> +                       ret = low | (ret << 8);
> +       }
> +
> +       mutex_unlock(&chip->access_lock);
> +       return ret;
> +}
> +
> +/* Write 1-byte register. Returns 0 or -ERRNO on error. */
> +static int nct720x_write_reg(struct nct720x_chip_info *chip, u8 reg, u8 val)
> +{
> +       struct i2c_client *client = chip->client;
> +       int err;
> +
> +       err = i2c_smbus_write_byte_data(client, reg, val);
> +       /* wait for write command to be finished */
> +       mdelay(10);
> +
> +       return err;
> +}
> +
> +static int nct720x_read_raw(struct iio_dev *indio_dev,
> +                           struct iio_chan_spec const *chan,
> +                           int *val, int *val2, long mask)
> +{
> +       int index = nct720x_chan_to_index[chan->address];
> +       int v1, v2, volt, err;
> +       struct nct720x_chip_info *chip = iio_priv(indio_dev);
> +
> +       if (chan->type != IIO_VOLTAGE)
> +               return -EOPNOTSUPP;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_PROCESSED:
> +               mutex_lock(&chip->access_lock);
> +               if (chip->use_read_byte_vin) {
> +                       /*
> +                        * MNTVIN Low Byte together with MNTVIN High Byte
> +                        * forms the 13-bit count value. If MNTVIN High
> +                        * Byte readout is read successively, the
> +                        * NCT7201/NCT7202 will latch the MNTVIN Low Byte
> +                        * for next read.
> +                        */
> +                       v1 = nct720x_read_reg(chip, REG_VIN[index]);
> +                       if (v1 < 0) {
> +                               err = v1;
> +                               goto abort;
> +                       }
> +
> +                       v2 = nct720x_read_reg(chip, REG_VOLT_LOW_BYTE);
> +                       if (v2 < 0) {
> +                               err = v2;
> +                               goto abort;
> +                       }
> +                       volt = (v1 << 8) | v2;  /* Convert back to 16-bit value */
> +               } else {
> +                       /* NCT7201/NCT7202 also supports read word-size data */
> +                       volt = nct720x_read_word_swapped_reg(chip, REG_VIN[index]);
> +               }
> +
> +               /* Voltage(V) = 13bitCountValue * 0.0004995 */
> +               volt = (volt >> 3) * NCT720X_IN_SCALING;
> +               *val = volt / 10000;
> +               mutex_unlock(&chip->access_lock);
> +               return IIO_VAL_INT;
> +       default:
> +               return -EINVAL;
> +       }
> +abort:
> +       mutex_unlock(&chip->access_lock);
> +       return err;
> +}
> +
> +static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
> +       int v1, v2, err;
> +       int volt = 0;
> +       int index = nct720x_chan_to_index[chan->address];
> +
> +       if (chan->type != IIO_VOLTAGE)
> +               return -EOPNOTSUPP;
> +
> +       if (info == IIO_EV_INFO_VALUE) {
> +               if (dir == IIO_EV_DIR_FALLING) {
> +                       if (chip->use_read_byte_vin) {
> +                               /*
> +                                * Low limit VIN Low Byte together with Low limit VIN High Byte
> +                                  forms the 13-bit count value
> +                                */
> +                               mutex_lock(&chip->access_lock);
> +                               v1 = nct720x_read_reg(chip, REG_VIN_LOW_LIMIT[index]);
> +                               if (v1 < 0) {
> +                                       err = v1;
> +                                       goto abort;
> +                               }
> +
> +                               v2 = nct720x_read_reg(chip, REG_VIN_LOW_LIMIT_LSB[index]);
> +                               if (v2 < 0) {
> +                                       err = v2;
> +                                       goto abort;
> +                               }
> +                               mutex_unlock(&chip->access_lock);
> +                               volt = (v1 << 8) | v2;  /* Convert back to 16-bit value */
> +                       } else {
> +                               /* NCT7201/NCT7202 also supports read word-size data */
> +                               volt = nct720x_read_word_swapped_reg(chip,
> +                                       REG_VIN_LOW_LIMIT[index]);
> +                       }
> +               } else {
> +                       if (chip->use_read_byte_vin) {
> +                               /*
> +                                * High limit VIN Low Byte together with high limit VIN High Byte
> +                                * forms the 13-bit count value
> +                                */
> +                               mutex_lock(&chip->access_lock);
> +                               v1 = nct720x_read_reg(chip, REG_VIN_HIGH_LIMIT[index]);
> +                               if (v1 < 0) {
> +                                       err = v1;
> +                                       goto abort;
> +                               }
> +
> +                               v2 = nct720x_read_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index]);
> +                               if (v2 < 0) {
> +                                       err = v2;
> +                                       goto abort;
> +                               }
> +                               mutex_unlock(&chip->access_lock);
> +                               volt = (v1 << 8) | v2;  /* Convert back to 16-bit value */
> +                       } else {
> +                               /* NCT7201/NCT7202 also supports read word-size data */
> +                               volt = nct720x_read_word_swapped_reg(chip,
> +                                       REG_VIN_HIGH_LIMIT[index]);
> +                       }
> +               }
> +       }
> +       /* Voltage(V) = 13bitCountValue * 0.0004995 */
> +       volt = (volt >> 3) * NCT720X_IN_SCALING;
> +       *val = volt / 10000;
> +
> +       return IIO_VAL_INT;
> +abort:
> +       mutex_unlock(&chip->access_lock);
> +       return err;
> +}
> +
> +static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
> +       int index, err = 0;
> +       long v1, v2, volt;
> +
> +       index = nct720x_chan_to_index[chan->address];
> +       volt = (val * 10000) / NCT720X_IN_SCALING;
> +       v1 = volt >> 5;
> +       v2 = (volt & 0x1f) << 3;
> +
> +       if (chan->type != IIO_VOLTAGE)
> +               return -EOPNOTSUPP;
> +
> +       if (info == IIO_EV_INFO_VALUE) {
> +               if (dir == IIO_EV_DIR_FALLING) {
> +                       mutex_lock(&chip->access_lock);
> +                       err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT[index], v1);
> +                       if (err < 0) {
> +                               pr_err("Failed to write REG_VIN%d_LOW_LIMIT\n", index + 1);
> +                               goto abort;
> +                       }
> +
> +                       err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT_LSB[index], v2);
> +                       if (err < 0) {
> +                               pr_err("Failed to write REG_VIN%d_LOW_LIMIT_LSB\n", index + 1);
> +                               goto abort;
> +                       }
> +               } else {
> +                       mutex_lock(&chip->access_lock);
> +                       err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT[index], v1);
> +                       if (err < 0) {
> +                               pr_err("Failed to write REG_VIN%d_HIGH_LIMIT\n", index + 1);
> +                               goto abort;
> +                       }
> +
> +                       err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index], v2);
> +                       if (err < 0) {
> +                               pr_err("Failed to write REG_VIN%d_HIGH_LIMIT_LSB\n", index + 1);
> +                               goto abort;
> +                       }
> +               }
> +       }
> +abort:
> +       mutex_unlock(&chip->access_lock);
> +       return err;
> +}
> +
> +static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
> +       int index = nct720x_chan_to_index[chan->address];
> +
> +       if (chan->type != IIO_VOLTAGE)
> +               return -EOPNOTSUPP;
> +
> +       return !!(chip->vin_mask & BIT(index));
> +}
> +
> +static int nct720x_write_event_config(struct iio_dev *indio_dev,
> +                                     const struct iio_chan_spec *chan,
> +                                     enum iio_event_type type,
> +                                     enum iio_event_direction dir,
> +                                     int state)
> +{
> +       int err = 0;
> +       struct nct720x_chip_info *chip = iio_priv(indio_dev);
> +       int index = nct720x_chan_to_index[chan->address];
> +       unsigned int mask;
> +
> +       mask = BIT(index);

nitpick:
You can set the mask near to the place where it is used i.e.
just after below if statement and one added advantage will
be in case below "return -EOPNOTSUPP" gets executed,
you don't even have to set mask.

> +
> +       if (chan->type != IIO_VOLTAGE)
> +               return -EOPNOTSUPP;
> +
> +       if (!state && (chip->vin_mask & mask))
> +               chip->vin_mask &= ~mask;
> +       else if (state && !(chip->vin_mask & mask))
> +               chip->vin_mask |= mask;
> +
> +       mutex_lock(&chip->access_lock);
> +
> +       err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, chip->vin_mask & 0xff);
> +       if (err < 0) {
> +               pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
> +               goto abort;
> +       }
> +
> +       if (chip->type == nct7202) {
> +               err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, chip->vin_mask >> 8);
> +               if (err < 0) {
> +                       pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
> +                       goto abort;
> +               }
> +       }
> +abort:
> +       mutex_unlock(&chip->access_lock);
> +       return err;
> +}
> +
> +static int nct720x_detect(struct i2c_client *client,
> +                         struct i2c_board_info *info)
> +{
> +       struct i2c_adapter *adapter = client->adapter;
> +
> +       if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> +                                    I2C_FUNC_SMBUS_WORD_DATA))
> +               return -ENODEV;
> +
> +       /* Determine the chip type. */
> +       if (i2c_smbus_read_byte_data(client, REG_VENDOR_ID) != NUVOTON_ID ||
> +           i2c_smbus_read_byte_data(client, REG_CHIP_ID) != NCT720X_ID ||
> +           i2c_smbus_read_byte_data(client, REG_DEVICE_ID) != NCT720X_DEVICE_ID)
> +               return -ENODEV;
> +
> +       strscpy(info->type, "nct720x", I2C_NAME_SIZE);
> +
> +       return 0;
> +}
> +
> +static const struct iio_info nct720x_info = {
> +       .read_raw = &nct720x_read_raw,
> +       .read_event_config = &nct720x_read_event_config,
> +       .write_event_config = &nct720x_write_event_config,
> +       .read_event_value = &nct720x_read_event_value,
> +       .write_event_value = &nct720x_write_event_value,
> +};
> +
> +static const struct i2c_device_id nct720x_id[];
> +
> +static int nct720x_init_chip(struct nct720x_chip_info *chip)
> +{
> +       int value = 0;
> +       int err = 0;

Nit pick:
You don't need to initialize err and value.

> +
> +       /* Initial reset */
> +       err = nct720x_write_reg(chip, REG_CONFIGURATION, CONFIGURATION_INIT);
> +       if (err) {
> +               pr_err("Failed to write REG_CONFIGURATION\n");
> +               return err;
> +       }
> +
> +       /* Enable Channel */
> +       err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, 0xff);
> +       if (err) {
> +               pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
> +               return err;
> +       }
> +
> +       if (chip->type == nct7202) {
> +               err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, 0xf);
> +               if (err) {
> +                       pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
> +                       return err;
> +               }
> +       }
> +
> +       value = nct720x_read_reg16(chip, REG_CHANNEL_ENABLE_1);
> +       if (value < 0)
> +               return value;
> +       chip->vin_mask = value;
> +
> +       /* Start monitoring if needed */
> +       value = nct720x_read_reg(chip, REG_CONFIGURATION);
> +       if (value < 0) {
> +               pr_err("Failed to read REG_CONFIGURATION\n");
> +               return value;
> +       }
> +
> +       value |= CONFIGURATION_START;
> +       err = nct720x_write_reg(chip, REG_CONFIGURATION, value);
> +       if (err < 0) {
> +               pr_err("Failed to write REG_CONFIGURATION\n");
> +               return err;
> +       }
> +
> +       return 0;
> +}
> +
> +static int nct720x_probe(struct i2c_client *client)
> +{
> +       const struct i2c_device_id *id = i2c_client_get_device_id(client);
> +       struct nct720x_chip_info *chip;
> +       struct iio_dev *indio_dev;
> +       int ret;
> +       u32 tmp;
> +
> +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +       chip = iio_priv(indio_dev);
> +
> +       if (client->dev.of_node)
> +               chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
> +       else
> +               chip->type = i2c_match_id(nct720x_id, client)->driver_data;
> +
> +       chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
> +
> +       ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
> +       if (ret < 0) {
> +               pr_err("read-vin-data-size property not found\n");
> +               return ret;

nit: dev_err_probe

> +       }
> +
> +       if (tmp == 8) {
> +               chip->use_read_byte_vin = true;
> +       } else if (tmp == 16) {
> +               chip->use_read_byte_vin = false;
> +       } else {
> +               pr_err("invalid read-vin-data-size (%d)\n", tmp);
> +               return -EINVAL;

nit: dev_err_probe

> +       }
> +
> +       mutex_init(&chip->access_lock);
> +
> +       /* this is only used for device removal purposes */
> +       i2c_set_clientdata(client, indio_dev);
> +
> +       chip->client = client;
> +
> +       ret = nct720x_init_chip(chip);
> +       if (ret < 0)
> +               return ret;
> +
> +       indio_dev->name = id->name;
> +       indio_dev->channels = nct720x_channels;
> +       indio_dev->num_channels = ARRAY_SIZE(nct720x_channels);
> +       indio_dev->info = &nct720x_info;
> +       indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +       iio_device_register(indio_dev);

devm_iio_device_register?

> +
> +       return 0;
> +}
> +
> +static void nct720x_remove(struct i2c_client *client)
> +{
> +       struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +       iio_device_unregister(indio_dev);

wouldn't need if used devm_ version
> +}
> +
> +static const unsigned short nct720x_address_list[] = {
> +       0x1d, 0x1e, 0x35, 0x36, I2C_CLIENT_END
> +};
> +
> +static const struct i2c_device_id nct720x_id[] = {
> +       { "nct7201", nct7201 },
> +       { "nct7202", nct7202 },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(i2c, nct720x_id);
> +
> +static const struct of_device_id nct720x_of_match[] = {
> +       {
> +               .compatible = "nuvoton,nct7201",
> +               .data = (void *)nct7201
> +       },
> +       {
> +               .compatible = "nuvoton,nct7202",
> +               .data = (void *)nct7202
> +       },
> +       { },
> +};
> +
> +MODULE_DEVICE_TABLE(of, nct720x_of_match);
> +
> +static struct i2c_driver nct720x_driver = {
> +       .driver = {
> +               .name   = "nct720x",
> +               .of_match_table = nct720x_of_match,
> +       },
> +       .probe = nct720x_probe,
> +       .remove = nct720x_remove,
> +       .id_table = nct720x_id,
> +       .detect = nct720x_detect,
> +       .address_list = nct720x_address_list,
> +};
> +
> +module_i2c_driver(nct720x_driver);
> +
> +MODULE_AUTHOR("Eason Yang <YHYANG2@nuvoton.com>");
> +MODULE_DESCRIPTION("Nuvoton NCT720x voltage monitor driver");
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
>
>
Yu-Hsian Yang Nov. 7, 2024, 7:17 a.m. UTC | #6
Dear anish kumar,

Thank you for your response.

anish kumar <yesanishhere@gmail.com> 於 2024年11月7日 週四 下午2:14寫道:
>
> On Tue, Nov 5, 2024 at 6:39 PM Eason Yang <j2anfernee@gmail.com> 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 <j2anfernee@gmail.com>
> > ---
> >  MAINTAINERS               |   1 +
> >  drivers/iio/adc/Kconfig   |   9 +
> >  drivers/iio/adc/Makefile  |   1 +
> >  drivers/iio/adc/nct720x.c | 617 ++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 628 insertions(+)
> >  create mode 100644 drivers/iio/adc/nct720x.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 68570c58e7aa..9940de0ddca2 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2753,6 +2753,7 @@ F:        arch/arm/mach-npcm/
> >  F:     arch/arm64/boot/dts/nuvoton/
> >  F:     drivers/*/*/*npcm*
> >  F:     drivers/*/*npcm*
> > +F:     drivers/iio/adc/nct720x.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
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 6c4e74420fd2..adbbf0ca6f57 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -1008,6 +1008,15 @@ config NAU7802
> >           To compile this driver as a module, choose M here: the
> >           module will be called nau7802.
> >
> > +config NCT720X
> > +       tristate "Nuvoton Instruments NCT7201 and NCT7202 Power Monitor"
> > +       depends on I2C
> > +       help
> > +         If you say yes here you get support for the Nuvoton NCT7201 and
> > +         NCT7202 Voltage Monitor.
> > +         This driver can also be built as a module. If so, the module
> > +         will be called nct720x.
> > +
> >  config NPCM_ADC
> >         tristate "Nuvoton NPCM ADC driver"
> >         depends on ARCH_NPCM || COMPILE_TEST
> > diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> > index 7b91cd98c0e0..f53318e5aa04 100644
> > --- a/drivers/iio/adc/Makefile
> > +++ b/drivers/iio/adc/Makefile
> > @@ -91,6 +91,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_NCT720X) += nct720x.o
> >  obj-$(CONFIG_NPCM_ADC) += npcm_adc.o
> >  obj-$(CONFIG_PAC1921) += pac1921.o
> >  obj-$(CONFIG_PAC1934) += pac1934.o
> > diff --git a/drivers/iio/adc/nct720x.c b/drivers/iio/adc/nct720x.c
> > new file mode 100644
> > index 000000000000..e589479fd06e
> > --- /dev/null
> > +++ b/drivers/iio/adc/nct720x.c
> > @@ -0,0 +1,617 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Driver for Nuvoton nct7201 and nct7202 power monitor chips.
> > + *
> > + * Copyright (c) 2022 Nuvoton Inc.
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/i2c.h>
> > +#include <linux/iio/events.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/sysfs.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +
> > +#define REG_CHIP_ID                    0xFD
> > +#define NCT720X_ID                     0xD7
> > +#define REG_VENDOR_ID                  0xFE
> > +#define NUVOTON_ID                     0x50
> > +#define REG_DEVICE_ID                  0xFF
> > +#define NCT720X_DEVICE_ID              0x12
> > +#define VIN_MAX                                12      /* Counted from 1 */
> > +#define NCT7201_VIN_MAX                        8
> > +#define NCT7202_VIN_MAX                        12
> > +#define NCT720X_IN_SCALING             4995
> > +
> > +#define REG_INTERRUPT_STATUS_1         0x0C
> > +#define REG_INTERRUPT_STATUS_2         0x0D
> > +#define REG_VOLT_LOW_BYTE              0x0F
> > +#define REG_CONFIGURATION              0x10
> > +#define  CONFIGURATION_START           BIT(0)
> > +#define  CONFIGURATION_ALERT_MSK       BIT(1)
> > +#define  CONFIGURATION_CONV_RATE       BIT(2)
> > +#define  CONFIGURATION_INIT            BIT(7)
> > +
> > +#define REG_ADVANCED_CONFIGURATION     0x11
> > +#define  ADVANCED_CONF_MOD_ALERT       BIT(0)
> > +#define  ADVANCED_CONF_MOD_STS         BIT(1)
> > +#define  ADVANCED_CONF_FAULT_QUEUE     BIT(2)
> > +#define  ADVANCED_CONF_EN_DEEP_SHUTDOWN        BIT(4)
> > +#define  ADVANCED_CONF_EN_SMB_TIMEOUT  BIT(5)
> > +#define  ADVANCED_CONF_MOD_RSTIN       BIT(7)
> > +
> > +#define REG_CHANNEL_INPUT_MODE         0x12
> > +#define REG_CHANNEL_ENABLE_1           0x13
> > +#define REG_CHANNEL_ENABLE_2           0x14
> > +#define REG_INTERRUPT_MASK_1           0x15
> > +#define REG_INTERRUPT_MASK_2           0x16
> > +#define REG_BUSY_STATUS                        0x1E
> > +#define REG_ONE_SHOT                   0x1F
> > +#define REG_SMUS_ADDRESS               0xFC
> > +
> > +static const u8 REG_VIN[VIN_MAX] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05,
> > +                                    0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B};
> > +static const u8 REG_VIN_HIGH_LIMIT[VIN_MAX] = { 0x20, 0x22, 0x24, 0x26, 0x28, 0x2A,
> > +                                               0x2C, 0x2E, 0x30, 0x32, 0x34, 0x36};
> > +static const u8 REG_VIN_LOW_LIMIT[VIN_MAX] = { 0x21, 0x23, 0x25, 0x27, 0x29, 0x2B,
> > +                                              0x2D, 0x2F, 0x31, 0x33, 0x35, 0x37};
> > +static const u8 REG_VIN_HIGH_LIMIT_LSB[VIN_MAX] = { 0x40, 0x42, 0x44, 0x46, 0x48, 0x4A,
> > +                                                   0x4C, 0x4E, 0x50, 0x52, 0x54, 0x56};
> > +static const u8 REG_VIN_LOW_LIMIT_LSB[VIN_MAX] = { 0x41, 0x43, 0x45, 0x47, 0x49, 0x4B,
> > +                                                  0x4D, 0x4F, 0x51, 0x53, 0x55, 0x57};
> > +static u8 nct720x_chan_to_index[] = {
> > +       0,      /* Not used */
> > +       0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
> > +};
> > +
> > +
> > +/* List of supported devices */
> > +enum nct720x_chips {
> > +       nct7201, nct7202
> > +};
> > +
> > +struct nct720x_chip_info {
> > +       struct i2c_client       *client;
> > +       enum nct720x_chips      type;
> > +       struct mutex            access_lock;    /* for multi-byte read and write operations */
> > +       int vin_max;                            /* number of VIN channels */
> > +       u32 vin_mask;
> > +       bool use_read_byte_vin;
> > +};
> > +
> > +static const struct iio_event_spec nct720x_events[] = {
> > +       {
> > +               .type = IIO_EV_TYPE_THRESH,
> > +               .dir = IIO_EV_DIR_RISING,
> > +               .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> > +                                BIT(IIO_EV_INFO_ENABLE),
> > +       }, {
> > +               .type = IIO_EV_TYPE_THRESH,
> > +               .dir = IIO_EV_DIR_FALLING,
> > +               .mask_separate = BIT(IIO_EV_INFO_VALUE) |
> > +                                BIT(IIO_EV_INFO_ENABLE),
> > +       },
> > +};
> > +
> > +#define NCT720X_VOLTAGE_CHANNEL(chan, addr)                            \
> > +       {                                                               \
> > +               .type = IIO_VOLTAGE,                                    \
> > +               .indexed = 1,                                           \
> > +               .channel = chan,                                        \
> > +               .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),     \
> > +               .address = addr,                                        \
> > +               .event_spec = nct720x_events,                           \
> > +               .num_event_specs = ARRAY_SIZE(nct720x_events),          \
> > +       }
> > +
> > +#define NCT720X_VOLTAGE_CHANNEL_DIFF(chan1, chan2, addr)               \
> > +       {                                                               \
> > +               .type = IIO_VOLTAGE,                                    \
> > +               .indexed = 1,                                           \
> > +               .channel = (chan1),                                     \
> > +               .channel2 = (chan2),                                    \
> > +               .differential = 1,                                      \
> > +               .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),     \
> > +               .address = addr,                                        \
> > +               .event_spec = nct720x_events,                           \
> > +               .num_event_specs = ARRAY_SIZE(nct720x_events),          \
> > +       }
> > +
> > +static const struct iio_chan_spec nct720x_channels[] = {
> > +       NCT720X_VOLTAGE_CHANNEL(1, 1),
> > +       NCT720X_VOLTAGE_CHANNEL(2, 2),
> > +       NCT720X_VOLTAGE_CHANNEL(3, 3),
> > +       NCT720X_VOLTAGE_CHANNEL(4, 4),
> > +       NCT720X_VOLTAGE_CHANNEL(5, 5),
> > +       NCT720X_VOLTAGE_CHANNEL(6, 6),
> > +       NCT720X_VOLTAGE_CHANNEL(7, 7),
> > +       NCT720X_VOLTAGE_CHANNEL(8, 8),
> > +       NCT720X_VOLTAGE_CHANNEL(9, 9),
> > +       NCT720X_VOLTAGE_CHANNEL(10, 10),
> > +       NCT720X_VOLTAGE_CHANNEL(11, 11),
> > +       NCT720X_VOLTAGE_CHANNEL(12, 12),
> > +       NCT720X_VOLTAGE_CHANNEL_DIFF(1, 2, 1),
> > +       NCT720X_VOLTAGE_CHANNEL_DIFF(3, 4, 3),
> > +       NCT720X_VOLTAGE_CHANNEL_DIFF(5, 6, 5),
> > +       NCT720X_VOLTAGE_CHANNEL_DIFF(7, 8, 7),
> > +       NCT720X_VOLTAGE_CHANNEL_DIFF(9, 10, 9),
> > +       NCT720X_VOLTAGE_CHANNEL_DIFF(11, 12, 11),
> > +};
> > +
> > +/* Read 1-byte register. Returns unsigned byte data or -ERRNO on error. */
> > +static int nct720x_read_reg(struct nct720x_chip_info *chip, u8 reg)
> > +{
> > +       struct i2c_client *client = chip->client;
> > +
> > +       return i2c_smbus_read_byte_data(client, reg);
> > +}
> > +
> > +/* Read 1-byte register. Returns unsigned word data or -ERRNO on error. */
> > +static int nct720x_read_word_swapped_reg(struct nct720x_chip_info *chip, u8 reg)
> > +{
> > +       struct i2c_client *client = chip->client;
> > +
> > +       return i2c_smbus_read_word_swapped(client, reg);
> > +}
> > +
> > +/*
> > + * Read 2-byte register. Returns register in big-endian format or
> > + * -ERRNO on error.
> > + */
> > +static int nct720x_read_reg16(struct nct720x_chip_info *chip, u8 reg)
> > +{
> > +       struct i2c_client *client = chip->client;
> > +       int ret, low;
> > +
> > +       mutex_lock(&chip->access_lock);
> > +       ret = i2c_smbus_read_byte_data(client, reg);
> > +       if (ret >= 0) {
> > +               low = ret;
> > +               ret = i2c_smbus_read_byte_data(client, reg + 1);
> > +               if (ret >= 0)
> > +                       ret = low | (ret << 8);
> > +       }
> > +
> > +       mutex_unlock(&chip->access_lock);
> > +       return ret;
> > +}
> > +
> > +/* Write 1-byte register. Returns 0 or -ERRNO on error. */
> > +static int nct720x_write_reg(struct nct720x_chip_info *chip, u8 reg, u8 val)
> > +{
> > +       struct i2c_client *client = chip->client;
> > +       int err;
> > +
> > +       err = i2c_smbus_write_byte_data(client, reg, val);
> > +       /* wait for write command to be finished */
> > +       mdelay(10);
> > +
> > +       return err;
> > +}
> > +
> > +static int nct720x_read_raw(struct iio_dev *indio_dev,
> > +                           struct iio_chan_spec const *chan,
> > +                           int *val, int *val2, long mask)
> > +{
> > +       int index = nct720x_chan_to_index[chan->address];
> > +       int v1, v2, volt, err;
> > +       struct nct720x_chip_info *chip = iio_priv(indio_dev);
> > +
> > +       if (chan->type != IIO_VOLTAGE)
> > +               return -EOPNOTSUPP;
> > +
> > +       switch (mask) {
> > +       case IIO_CHAN_INFO_PROCESSED:
> > +               mutex_lock(&chip->access_lock);
> > +               if (chip->use_read_byte_vin) {
> > +                       /*
> > +                        * MNTVIN Low Byte together with MNTVIN High Byte
> > +                        * forms the 13-bit count value. If MNTVIN High
> > +                        * Byte readout is read successively, the
> > +                        * NCT7201/NCT7202 will latch the MNTVIN Low Byte
> > +                        * for next read.
> > +                        */
> > +                       v1 = nct720x_read_reg(chip, REG_VIN[index]);
> > +                       if (v1 < 0) {
> > +                               err = v1;
> > +                               goto abort;
> > +                       }
> > +
> > +                       v2 = nct720x_read_reg(chip, REG_VOLT_LOW_BYTE);
> > +                       if (v2 < 0) {
> > +                               err = v2;
> > +                               goto abort;
> > +                       }
> > +                       volt = (v1 << 8) | v2;  /* Convert back to 16-bit value */
> > +               } else {
> > +                       /* NCT7201/NCT7202 also supports read word-size data */
> > +                       volt = nct720x_read_word_swapped_reg(chip, REG_VIN[index]);
> > +               }
> > +
> > +               /* Voltage(V) = 13bitCountValue * 0.0004995 */
> > +               volt = (volt >> 3) * NCT720X_IN_SCALING;
> > +               *val = volt / 10000;
> > +               mutex_unlock(&chip->access_lock);
> > +               return IIO_VAL_INT;
> > +       default:
> > +               return -EINVAL;
> > +       }
> > +abort:
> > +       mutex_unlock(&chip->access_lock);
> > +       return err;
> > +}
> > +
> > +static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
> > +       int v1, v2, err;
> > +       int volt = 0;
> > +       int index = nct720x_chan_to_index[chan->address];
> > +
> > +       if (chan->type != IIO_VOLTAGE)
> > +               return -EOPNOTSUPP;
> > +
> > +       if (info == IIO_EV_INFO_VALUE) {
> > +               if (dir == IIO_EV_DIR_FALLING) {
> > +                       if (chip->use_read_byte_vin) {
> > +                               /*
> > +                                * Low limit VIN Low Byte together with Low limit VIN High Byte
> > +                                  forms the 13-bit count value
> > +                                */
> > +                               mutex_lock(&chip->access_lock);
> > +                               v1 = nct720x_read_reg(chip, REG_VIN_LOW_LIMIT[index]);
> > +                               if (v1 < 0) {
> > +                                       err = v1;
> > +                                       goto abort;
> > +                               }
> > +
> > +                               v2 = nct720x_read_reg(chip, REG_VIN_LOW_LIMIT_LSB[index]);
> > +                               if (v2 < 0) {
> > +                                       err = v2;
> > +                                       goto abort;
> > +                               }
> > +                               mutex_unlock(&chip->access_lock);
> > +                               volt = (v1 << 8) | v2;  /* Convert back to 16-bit value */
> > +                       } else {
> > +                               /* NCT7201/NCT7202 also supports read word-size data */
> > +                               volt = nct720x_read_word_swapped_reg(chip,
> > +                                       REG_VIN_LOW_LIMIT[index]);
> > +                       }
> > +               } else {
> > +                       if (chip->use_read_byte_vin) {
> > +                               /*
> > +                                * High limit VIN Low Byte together with high limit VIN High Byte
> > +                                * forms the 13-bit count value
> > +                                */
> > +                               mutex_lock(&chip->access_lock);
> > +                               v1 = nct720x_read_reg(chip, REG_VIN_HIGH_LIMIT[index]);
> > +                               if (v1 < 0) {
> > +                                       err = v1;
> > +                                       goto abort;
> > +                               }
> > +
> > +                               v2 = nct720x_read_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index]);
> > +                               if (v2 < 0) {
> > +                                       err = v2;
> > +                                       goto abort;
> > +                               }
> > +                               mutex_unlock(&chip->access_lock);
> > +                               volt = (v1 << 8) | v2;  /* Convert back to 16-bit value */
> > +                       } else {
> > +                               /* NCT7201/NCT7202 also supports read word-size data */
> > +                               volt = nct720x_read_word_swapped_reg(chip,
> > +                                       REG_VIN_HIGH_LIMIT[index]);
> > +                       }
> > +               }
> > +       }
> > +       /* Voltage(V) = 13bitCountValue * 0.0004995 */
> > +       volt = (volt >> 3) * NCT720X_IN_SCALING;
> > +       *val = volt / 10000;
> > +
> > +       return IIO_VAL_INT;
> > +abort:
> > +       mutex_unlock(&chip->access_lock);
> > +       return err;
> > +}
> > +
> > +static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
> > +       int index, err = 0;
> > +       long v1, v2, volt;
> > +
> > +       index = nct720x_chan_to_index[chan->address];
> > +       volt = (val * 10000) / NCT720X_IN_SCALING;
> > +       v1 = volt >> 5;
> > +       v2 = (volt & 0x1f) << 3;
> > +
> > +       if (chan->type != IIO_VOLTAGE)
> > +               return -EOPNOTSUPP;
> > +
> > +       if (info == IIO_EV_INFO_VALUE) {
> > +               if (dir == IIO_EV_DIR_FALLING) {
> > +                       mutex_lock(&chip->access_lock);
> > +                       err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT[index], v1);
> > +                       if (err < 0) {
> > +                               pr_err("Failed to write REG_VIN%d_LOW_LIMIT\n", index + 1);
> > +                               goto abort;
> > +                       }
> > +
> > +                       err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT_LSB[index], v2);
> > +                       if (err < 0) {
> > +                               pr_err("Failed to write REG_VIN%d_LOW_LIMIT_LSB\n", index + 1);
> > +                               goto abort;
> > +                       }
> > +               } else {
> > +                       mutex_lock(&chip->access_lock);
> > +                       err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT[index], v1);
> > +                       if (err < 0) {
> > +                               pr_err("Failed to write REG_VIN%d_HIGH_LIMIT\n", index + 1);
> > +                               goto abort;
> > +                       }
> > +
> > +                       err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index], v2);
> > +                       if (err < 0) {
> > +                               pr_err("Failed to write REG_VIN%d_HIGH_LIMIT_LSB\n", index + 1);
> > +                               goto abort;
> > +                       }
> > +               }
> > +       }
> > +abort:
> > +       mutex_unlock(&chip->access_lock);
> > +       return err;
> > +}
> > +
> > +static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
> > +       int index = nct720x_chan_to_index[chan->address];
> > +
> > +       if (chan->type != IIO_VOLTAGE)
> > +               return -EOPNOTSUPP;
> > +
> > +       return !!(chip->vin_mask & BIT(index));
> > +}
> > +
> > +static int nct720x_write_event_config(struct iio_dev *indio_dev,
> > +                                     const struct iio_chan_spec *chan,
> > +                                     enum iio_event_type type,
> > +                                     enum iio_event_direction dir,
> > +                                     int state)
> > +{
> > +       int err = 0;
> > +       struct nct720x_chip_info *chip = iio_priv(indio_dev);
> > +       int index = nct720x_chan_to_index[chan->address];
> > +       unsigned int mask;
> > +
> > +       mask = BIT(index);
>
> nitpick:
> You can set the mask near to the place where it is used i.e.
> just after below if statement and one added advantage will
> be in case below "return -EOPNOTSUPP" gets executed,
> you don't even have to set mask.
>
Got it.

> > +
> > +       if (chan->type != IIO_VOLTAGE)
> > +               return -EOPNOTSUPP;
> > +
> > +       if (!state && (chip->vin_mask & mask))
> > +               chip->vin_mask &= ~mask;
> > +       else if (state && !(chip->vin_mask & mask))
> > +               chip->vin_mask |= mask;
> > +
> > +       mutex_lock(&chip->access_lock);
> > +
> > +       err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, chip->vin_mask & 0xff);
> > +       if (err < 0) {
> > +               pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
> > +               goto abort;
> > +       }
> > +
> > +       if (chip->type == nct7202) {
> > +               err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, chip->vin_mask >> 8);
> > +               if (err < 0) {
> > +                       pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
> > +                       goto abort;
> > +               }
> > +       }
> > +abort:
> > +       mutex_unlock(&chip->access_lock);
> > +       return err;
> > +}
> > +
> > +static int nct720x_detect(struct i2c_client *client,
> > +                         struct i2c_board_info *info)
> > +{
> > +       struct i2c_adapter *adapter = client->adapter;
> > +
> > +       if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> > +                                    I2C_FUNC_SMBUS_WORD_DATA))
> > +               return -ENODEV;
> > +
> > +       /* Determine the chip type. */
> > +       if (i2c_smbus_read_byte_data(client, REG_VENDOR_ID) != NUVOTON_ID ||
> > +           i2c_smbus_read_byte_data(client, REG_CHIP_ID) != NCT720X_ID ||
> > +           i2c_smbus_read_byte_data(client, REG_DEVICE_ID) != NCT720X_DEVICE_ID)
> > +               return -ENODEV;
> > +
> > +       strscpy(info->type, "nct720x", I2C_NAME_SIZE);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct iio_info nct720x_info = {
> > +       .read_raw = &nct720x_read_raw,
> > +       .read_event_config = &nct720x_read_event_config,
> > +       .write_event_config = &nct720x_write_event_config,
> > +       .read_event_value = &nct720x_read_event_value,
> > +       .write_event_value = &nct720x_write_event_value,
> > +};
> > +
> > +static const struct i2c_device_id nct720x_id[];
> > +
> > +static int nct720x_init_chip(struct nct720x_chip_info *chip)
> > +{
> > +       int value = 0;
> > +       int err = 0;
>
> Nit pick:
> You don't need to initialize err and value.
>
Got it.

> > +
> > +       /* Initial reset */
> > +       err = nct720x_write_reg(chip, REG_CONFIGURATION, CONFIGURATION_INIT);
> > +       if (err) {
> > +               pr_err("Failed to write REG_CONFIGURATION\n");
> > +               return err;
> > +       }
> > +
> > +       /* Enable Channel */
> > +       err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, 0xff);
> > +       if (err) {
> > +               pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
> > +               return err;
> > +       }
> > +
> > +       if (chip->type == nct7202) {
> > +               err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, 0xf);
> > +               if (err) {
> > +                       pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
> > +                       return err;
> > +               }
> > +       }
> > +
> > +       value = nct720x_read_reg16(chip, REG_CHANNEL_ENABLE_1);
> > +       if (value < 0)
> > +               return value;
> > +       chip->vin_mask = value;
> > +
> > +       /* Start monitoring if needed */
> > +       value = nct720x_read_reg(chip, REG_CONFIGURATION);
> > +       if (value < 0) {
> > +               pr_err("Failed to read REG_CONFIGURATION\n");
> > +               return value;
> > +       }
> > +
> > +       value |= CONFIGURATION_START;
> > +       err = nct720x_write_reg(chip, REG_CONFIGURATION, value);
> > +       if (err < 0) {
> > +               pr_err("Failed to write REG_CONFIGURATION\n");
> > +               return err;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static int nct720x_probe(struct i2c_client *client)
> > +{
> > +       const struct i2c_device_id *id = i2c_client_get_device_id(client);
> > +       struct nct720x_chip_info *chip;
> > +       struct iio_dev *indio_dev;
> > +       int ret;
> > +       u32 tmp;
> > +
> > +       indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> > +       if (!indio_dev)
> > +               return -ENOMEM;
> > +       chip = iio_priv(indio_dev);
> > +
> > +       if (client->dev.of_node)
> > +               chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
> > +       else
> > +               chip->type = i2c_match_id(nct720x_id, client)->driver_data;
> > +
> > +       chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
> > +
> > +       ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
> > +       if (ret < 0) {
> > +               pr_err("read-vin-data-size property not found\n");
> > +               return ret;
>
> nit: dev_err_probe
>
I would use dev_err_probe to replace pr_err in probe function

> > +       }
> > +
> > +       if (tmp == 8) {
> > +               chip->use_read_byte_vin = true;
> > +       } else if (tmp == 16) {
> > +               chip->use_read_byte_vin = false;
> > +       } else {
> > +               pr_err("invalid read-vin-data-size (%d)\n", tmp);
> > +               return -EINVAL;
>
> nit: dev_err_probe
>
I would use dev_err_probe to replace pr_err in probe function.

> > +       }
> > +
> > +       mutex_init(&chip->access_lock);
> > +
> > +       /* this is only used for device removal purposes */
> > +       i2c_set_clientdata(client, indio_dev);
> > +
> > +       chip->client = client;
> > +
> > +       ret = nct720x_init_chip(chip);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       indio_dev->name = id->name;
> > +       indio_dev->channels = nct720x_channels;
> > +       indio_dev->num_channels = ARRAY_SIZE(nct720x_channels);
> > +       indio_dev->info = &nct720x_info;
> > +       indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > +       iio_device_register(indio_dev);
>
> devm_iio_device_register?
>
Yes, I would use devm_iio_device_registe to replace
iio_device_register and no need to use remove function.

> > +
> > +       return 0;
> > +}
> > +
> > +static void nct720x_remove(struct i2c_client *client)
> > +{
> > +       struct iio_dev *indio_dev = i2c_get_clientdata(client);
> > +
> > +       iio_device_unregister(indio_dev);
>
> wouldn't need if used devm_ version

Got it.

> > +}
> > +
> > +static const unsigned short nct720x_address_list[] = {
> > +       0x1d, 0x1e, 0x35, 0x36, I2C_CLIENT_END
> > +};
> > +
> > +static const struct i2c_device_id nct720x_id[] = {
> > +       { "nct7201", nct7201 },
> > +       { "nct7202", nct7202 },
> > +       {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, nct720x_id);
> > +
> > +static const struct of_device_id nct720x_of_match[] = {
> > +       {
> > +               .compatible = "nuvoton,nct7201",
> > +               .data = (void *)nct7201
> > +       },
> > +       {
> > +               .compatible = "nuvoton,nct7202",
> > +               .data = (void *)nct7202
> > +       },
> > +       { },
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, nct720x_of_match);
> > +
> > +static struct i2c_driver nct720x_driver = {
> > +       .driver = {
> > +               .name   = "nct720x",
> > +               .of_match_table = nct720x_of_match,
> > +       },
> > +       .probe = nct720x_probe,
> > +       .remove = nct720x_remove,
> > +       .id_table = nct720x_id,
> > +       .detect = nct720x_detect,
> > +       .address_list = nct720x_address_list,
> > +};
> > +
> > +module_i2c_driver(nct720x_driver);
> > +
> > +MODULE_AUTHOR("Eason Yang <YHYANG2@nuvoton.com>");
> > +MODULE_DESCRIPTION("Nuvoton NCT720x voltage monitor driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.34.1
> >
> >
Andy Shevchenko Nov. 7, 2024, 8:02 a.m. UTC | #7
On Wed, Nov 06, 2024 at 10:39:16AM +0800, Eason Yang 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.

...

> + * Copyright (c) 2022 Nuvoton Inc.

2024?

...

+ array_size.h
+ bits.h

> +#include <linux/delay.h>
> +#include <linux/device.h>

+ err.h

> +#include <linux/i2c.h>

> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>

Move these to be a separate group...

> +#include <linux/init.h>

+ mod_devicetable.h

> +#include <linux/module.h>
> +#include <linux/mutex.h>

+ types.h

...somewhere here after a blank line.

...

> +static const u8 REG_VIN[VIN_MAX] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05,
> +				     0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B};

Better to have it done like this

static const u8 REG_VIN[VIN_MAX] = {
	0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,	/* 0 - 7 */
	0x08, 0x09, 0x0A, 0x0B,				/* 8 - 11 */
};

Note the following: the values are placed on separate lines with trailing
commas on each line and with grouping by powwer-of-2 per line.

> +static const u8 REG_VIN_HIGH_LIMIT[VIN_MAX] = { 0x20, 0x22, 0x24, 0x26, 0x28, 0x2A,
> +						0x2C, 0x2E, 0x30, 0x32, 0x34, 0x36};
> +static const u8 REG_VIN_LOW_LIMIT[VIN_MAX] = { 0x21, 0x23, 0x25, 0x27, 0x29, 0x2B,
> +					       0x2D, 0x2F, 0x31, 0x33, 0x35, 0x37};
> +static const u8 REG_VIN_HIGH_LIMIT_LSB[VIN_MAX] = { 0x40, 0x42, 0x44, 0x46, 0x48, 0x4A,
> +						    0x4C, 0x4E, 0x50, 0x52, 0x54, 0x56};
> +static const u8 REG_VIN_LOW_LIMIT_LSB[VIN_MAX] = { 0x41, 0x43, 0x45, 0x47, 0x49, 0x4B,
> +						   0x4D, 0x4F, 0x51, 0x53, 0x55, 0x57};
> +static u8 nct720x_chan_to_index[] = {
> +	0,	/* Not used */
> +	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
> +};

Ditto for the above initialisers.

> +
> +

One blank line is enough.

> +/* List of supported devices */
> +enum nct720x_chips {
> +	nct7201, nct7202

Leave trailing comma.

> +};

...

> +/* Read 1-byte register. Returns unsigned byte data or -ERRNO on error. */
> +static int nct720x_read_reg(struct nct720x_chip_info *chip, u8 reg)
> +{
> +	struct i2c_client *client = chip->client;
> +
> +	return i2c_smbus_read_byte_data(client, reg);
> +}
> +
> +/* Read 1-byte register. Returns unsigned word data or -ERRNO on error. */
> +static int nct720x_read_word_swapped_reg(struct nct720x_chip_info *chip, u8 reg)
> +{
> +	struct i2c_client *client = chip->client;
> +
> +	return i2c_smbus_read_word_swapped(client, reg);
> +}
> +
> +/*
> + * Read 2-byte register. Returns register in big-endian format or
> + * -ERRNO on error.
> + */
> +static int nct720x_read_reg16(struct nct720x_chip_info *chip, u8 reg)
> +{
> +	struct i2c_client *client = chip->client;
> +	int ret, low;

> +	mutex_lock(&chip->access_lock);

If you are going with this, use cleanup.h.

> +	ret = i2c_smbus_read_byte_data(client, reg);
> +	if (ret >= 0) {
> +		low = ret;
> +		ret = i2c_smbus_read_byte_data(client, reg + 1);
> +		if (ret >= 0)
> +			ret = low | (ret << 8);

Hmm... Does it require to have two separate transactions? In case of regmap,
can't bulk transfer be performed for that?

> +	}
> +
> +	mutex_unlock(&chip->access_lock);
> +	return ret;
> +}

I have no explanations why regmap I2C can't be used.

...

> +/* Write 1-byte register. Returns 0 or -ERRNO on error. */
> +static int nct720x_write_reg(struct nct720x_chip_info *chip, u8 reg, u8 val)
> +{
> +	struct i2c_client *client = chip->client;
> +	int err;
> +
> +	err = i2c_smbus_write_byte_data(client, reg, val);
> +	/* wait for write command to be finished */

Is there any justification for the chosen value (I mean datasheet
recommendations or so)?

> +	mdelay(10);
> +
> +	return err;
> +}

...

> +static int nct720x_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	int index = nct720x_chan_to_index[chan->address];
> +	int v1, v2, volt, err;
> +	struct nct720x_chip_info *chip = iio_priv(indio_dev);
> +
> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED:
> +		mutex_lock(&chip->access_lock);
> +		if (chip->use_read_byte_vin) {
> +			/*
> +			 * MNTVIN Low Byte together with MNTVIN High Byte
> +			 * forms the 13-bit count value. If MNTVIN High
> +			 * Byte readout is read successively, the
> +			 * NCT7201/NCT7202 will latch the MNTVIN Low Byte
> +			 * for next read.
> +			 */
> +			v1 = nct720x_read_reg(chip, REG_VIN[index]);
> +			if (v1 < 0) {
> +				err = v1;
> +				goto abort;
> +			}
> +
> +			v2 = nct720x_read_reg(chip, REG_VOLT_LOW_BYTE);
> +			if (v2 < 0) {
> +				err = v2;
> +				goto abort;
> +			}
> +			volt = (v1 << 8) | v2;	/* Convert back to 16-bit value */

Hmm... I would expect volt to be __be16, but since you have two separate reads
it's probably fine.

> +		} else {
> +			/* NCT7201/NCT7202 also supports read word-size data */
> +			volt = nct720x_read_word_swapped_reg(chip, REG_VIN[index]);
> +		}
> +
> +		/* Voltage(V) = 13bitCountValue * 0.0004995 */
> +		volt = (volt >> 3) * NCT720X_IN_SCALING;
> +		*val = volt / 10000;
> +		mutex_unlock(&chip->access_lock);
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +abort:
> +	mutex_unlock(&chip->access_lock);
> +	return err;

Yeah, this will gone with cleanup.h in use.
Ditto for all other places like this.

> +}

...

> +{
> +	struct nct720x_chip_info *chip = iio_priv(indio_dev);
> +	int index, err = 0;
> +	long v1, v2, volt;

Why long? Wouldn't int / unsigned int suffice for v1 / v2?

> +
> +	index = nct720x_chan_to_index[chan->address];
> +	volt = (val * 10000) / NCT720X_IN_SCALING;

Please, use named constant for 10000 as you used it at least twice in the code.

> +	v1 = volt >> 5;
> +	v2 = (volt & 0x1f) << 3;

In the similar way it seems like some endianess and bit width manipulations.

Can v be an array of u8:s which you just fill with put_unaligned_be16() or so?

> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;
> +
> +	if (info == IIO_EV_INFO_VALUE) {
> +		if (dir == IIO_EV_DIR_FALLING) {
> +			mutex_lock(&chip->access_lock);
> +			err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT[index], v1);
> +			if (err < 0) {
> +				pr_err("Failed to write REG_VIN%d_LOW_LIMIT\n", index + 1);
> +				goto abort;
> +			}
> +
> +			err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT_LSB[index], v2);
> +			if (err < 0) {
> +				pr_err("Failed to write REG_VIN%d_LOW_LIMIT_LSB\n", index + 1);
> +				goto abort;
> +			}
> +		} else {
> +			mutex_lock(&chip->access_lock);
> +			err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT[index], v1);
> +			if (err < 0) {
> +				pr_err("Failed to write REG_VIN%d_HIGH_LIMIT\n", index + 1);
> +				goto abort;
> +			}
> +
> +			err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index], v2);
> +			if (err < 0) {
> +				pr_err("Failed to write REG_VIN%d_HIGH_LIMIT_LSB\n", index + 1);
> +				goto abort;
> +			}
> +		}
> +	}
> +abort:
> +	mutex_unlock(&chip->access_lock);
> +	return err;
> +}

...

> +	strscpy(info->type, "nct720x", I2C_NAME_SIZE);

Use 2-argument strscpy().

...

> +static const struct i2c_device_id nct720x_id[];

Remove this. It's not needed.

...

> +static int nct720x_init_chip(struct nct720x_chip_info *chip)
> +{

> +	int value = 0;

Why signed?

> +	int err = 0;


Redundant assignments.

> +	/* Initial reset */
> +	err = nct720x_write_reg(chip, REG_CONFIGURATION, CONFIGURATION_INIT);
> +	if (err) {
> +		pr_err("Failed to write REG_CONFIGURATION\n");
> +		return err;
> +	}
> +
> +	/* Enable Channel */
> +	err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, 0xff);

Magic 0xff.
Should be GENMASK() ? Anything else?

> +	if (err) {
> +		pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
> +		return err;
> +	}
> +
> +	if (chip->type == nct7202) {
> +		err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, 0xf);

Ditto.

> +		if (err) {
> +			pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
> +			return err;
> +		}
> +	}
> +
> +	value = nct720x_read_reg16(chip, REG_CHANNEL_ENABLE_1);
> +	if (value < 0)
> +		return value;
> +	chip->vin_mask = value;
> +
> +	/* Start monitoring if needed */
> +	value = nct720x_read_reg(chip, REG_CONFIGURATION);
> +	if (value < 0) {
> +		pr_err("Failed to read REG_CONFIGURATION\n");
> +		return value;
> +	}
> +
> +	value |= CONFIGURATION_START;
> +	err = nct720x_write_reg(chip, REG_CONFIGURATION, value);
> +	if (err < 0) {
> +		pr_err("Failed to write REG_CONFIGURATION\n");
> +		return err;
> +	}
> +
> +	return 0;
> +}

...

> +	ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);

No OF-centric APIs in a new code, please.
Use device_property_read_u32() and include property.h for that.

> +	if (ret < 0) {
> +		pr_err("read-vin-data-size property not found\n");
> +		return ret;
> +	}

...

> +	mutex_init(&chip->access_lock);

	ret = devm_mutex_init(...);

...

> +	/* this is only used for device removal purposes */
> +	i2c_set_clientdata(client, indio_dev);

Will be gone after removing ->remove().

...

> +	iio_device_register(indio_dev);

No error checks? In any case devm_ variant especially needs this.

> +	return 0;

	return devm_iio_device_register(...);

...

> +static const struct i2c_device_id nct720x_id[] = {
> +	{ "nct7201", nct7201 },
> +	{ "nct7202", nct7202 },

Please, use chip_info custom data structure instead of enum values.

> +	{}
> +};
> +MODULE_DEVICE_TABLE(i2c, nct720x_id);
> +
> +static const struct of_device_id nct720x_of_match[] = {
> +	{
> +		.compatible = "nuvoton,nct7201",
> +		.data = (void *)nct7201
> +	},
> +	{
> +		.compatible = "nuvoton,nct7202",
> +		.data = (void *)nct7202

As per above and leave trailing commas.

> +	},
> +	{ },

Drop trailing comma in the terminator entries.

> +};

> +

Redundant blank line.

> +MODULE_DEVICE_TABLE(of, nct720x_of_match);
> +
> +static struct i2c_driver nct720x_driver = {
> +	.driver = {
> +		.name	= "nct720x",
> +		.of_match_table = nct720x_of_match,
> +	},
> +	.probe = nct720x_probe,
> +	.remove = nct720x_remove,
> +	.id_table = nct720x_id,
> +	.detect = nct720x_detect,
> +	.address_list = nct720x_address_list,
> +};

> +

Redundant blank line.

> +module_i2c_driver(nct720x_driver);
Jonathan Cameron Nov. 9, 2024, 1:45 p.m. UTC | #8
On Thu, 7 Nov 2024 08:41:21 +0800
Yu-Hsian Yang <j2anfernee@gmail.com> wrote:

> Dear Krzysztof Kozlowski,
> 
> Thank you for your response.
> 
> Krzysztof Kozlowski <krzk@kernel.org> 於 2024年11月6日 週三 下午9:41寫道:
> >
> > On 06/11/2024 03:39, Eason Yang 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 <j2anfernee@gmail.com>  
> >
> > ...
> >  
> > > +
> > > +static int nct720x_probe(struct i2c_client *client)
> > > +{
> > > +     const struct i2c_device_id *id = i2c_client_get_device_id(client);
> > > +     struct nct720x_chip_info *chip;
> > > +     struct iio_dev *indio_dev;
> > > +     int ret;
> > > +     u32 tmp;
> > > +
> > > +     indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> > > +     if (!indio_dev)
> > > +             return -ENOMEM;
> > > +     chip = iio_priv(indio_dev);
> > > +
> > > +     if (client->dev.of_node)
> > > +             chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
> > > +     else
> > > +             chip->type = i2c_match_id(nct720x_id, client)->driver_data;  
> >
> > I believe there is a I2C wrapper for above.
> >  
> 
> Got it.

Don't pass an enum value as data.  Pass a pointer to a structure that describes the particular
variant.  The 0 value which tends to end up in enums is an error for device_get_match_data.

> 
> > > +
> > > +     chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
> > > +
> > > +     ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
> > > +     if (ret < 0) {
> > > +             pr_err("read-vin-data-size property not found\n");  
> >
> > Please use dev_xxx in your driver code.  
> 
> Got it.
> 
> >
> >
> > Best regards,
> > Krzysztof
> >
Jonathan Cameron Nov. 9, 2024, 2:37 p.m. UTC | #9
On Wed,  6 Nov 2024 10:39:16 +0800
Eason Yang <j2anfernee@gmail.com> 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 <j2anfernee@gmail.com>
You've gotten some good review already so I may well repeat stuff plus
will only take a fairly superficial look at this version.


> diff --git a/drivers/iio/adc/nct720x.c b/drivers/iio/adc/nct720x.c
> new file mode 100644
> index 000000000000..e589479fd06e
> --- /dev/null
> +++ b/drivers/iio/adc/nct720x.c

> +
> +/* List of supported devices */
> +enum nct720x_chips {
> +	nct7201, nct7202
This should be replaced by chip specific information structures.
That ends up a lot more extensible than using an enum and fixing up
what happens in all code paths on a per enum value basis.

> +};
> +
> +struct nct720x_chip_info {
> +	struct i2c_client	*client;
> +	enum nct720x_chips	type;
> +	struct mutex		access_lock;	/* for multi-byte read and write operations */
> +	int vin_max;				/* number of VIN channels */
> +	u32 vin_mask;
> +	bool use_read_byte_vin;
> +};

> +#define NCT720X_VOLTAGE_CHANNEL(chan, addr)				\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.indexed = 1,						\
> +		.channel = chan,					\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
> +		.address = addr,					\
> +		.event_spec = nct720x_events,				\
> +		.num_event_specs = ARRAY_SIZE(nct720x_events),		\
> +	}
> +
> +#define NCT720X_VOLTAGE_CHANNEL_DIFF(chan1, chan2, addr)		\
> +	{								\
> +		.type = IIO_VOLTAGE,					\
> +		.indexed = 1,						\
> +		.channel = (chan1),					\
> +		.channel2 = (chan2),					\
> +		.differential = 1,					\
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
See below - should be _RAW and _SCALE not _PROCESSED.
> +		.address = addr,					\
> +		.event_spec = nct720x_events,				\
> +		.num_event_specs = ARRAY_SIZE(nct720x_events),		\
> +	}
> +
> +static const struct iio_chan_spec nct720x_channels[] = {
> +	NCT720X_VOLTAGE_CHANNEL(1, 1),
> +	NCT720X_VOLTAGE_CHANNEL(2, 2),
> +	NCT720X_VOLTAGE_CHANNEL(3, 3),
> +	NCT720X_VOLTAGE_CHANNEL(4, 4),
> +	NCT720X_VOLTAGE_CHANNEL(5, 5),
> +	NCT720X_VOLTAGE_CHANNEL(6, 6),
> +	NCT720X_VOLTAGE_CHANNEL(7, 7),
> +	NCT720X_VOLTAGE_CHANNEL(8, 8),
> +	NCT720X_VOLTAGE_CHANNEL(9, 9),
> +	NCT720X_VOLTAGE_CHANNEL(10, 10),
> +	NCT720X_VOLTAGE_CHANNEL(11, 11),
> +	NCT720X_VOLTAGE_CHANNEL(12, 12),
> +	NCT720X_VOLTAGE_CHANNEL_DIFF(1, 2, 1),
> +	NCT720X_VOLTAGE_CHANNEL_DIFF(3, 4, 3),
> +	NCT720X_VOLTAGE_CHANNEL_DIFF(5, 6, 5),
> +	NCT720X_VOLTAGE_CHANNEL_DIFF(7, 8, 7),
> +	NCT720X_VOLTAGE_CHANNEL_DIFF(9, 10, 9),
> +	NCT720X_VOLTAGE_CHANNEL_DIFF(11, 12, 11),
> +};
> +
> +/* Read 1-byte register. Returns unsigned byte data or -ERRNO on error. */
> +static int nct720x_read_reg(struct nct720x_chip_info *chip, u8 reg)
> +{
> +	struct i2c_client *client = chip->client;
> +
> +	return i2c_smbus_read_byte_data(client, reg);
> +}
> +
> +/* Read 1-byte register. Returns unsigned word data or -ERRNO on error. */
> +static int nct720x_read_word_swapped_reg(struct nct720x_chip_info *chip, u8 reg)
> +{
> +	struct i2c_client *client = chip->client;
> +
> +	return i2c_smbus_read_word_swapped(client, reg);

Don't provide these wrappers as they don't add anything useful.
Make the calls directly inline.

> +}
> +
> +/*
> + * Read 2-byte register. Returns register in big-endian format or
> + * -ERRNO on error.
> + */
> +static int nct720x_read_reg16(struct nct720x_chip_info *chip, u8 reg)
> +{
> +	struct i2c_client *client = chip->client;
> +	int ret, low;
> +
> +	mutex_lock(&chip->access_lock);
guard() 

> +	ret = i2c_smbus_read_byte_data(client, reg);
> +	if (ret >= 0) {
> +		low = ret;
> +		ret = i2c_smbus_read_byte_data(client, reg + 1);
> +		if (ret >= 0)
> +			ret = low | (ret << 8);
		if (ret < 0)
			return ret;

			reg = get_unaligned_le16()
on an appropriate u8 data[2]; ideally filled by a bulk regmap read.


> +	}
> +
> +	mutex_unlock(&chip->access_lock);
> +	return ret;
> +}
> +
> +/* Write 1-byte register. Returns 0 or -ERRNO on error. */
> +static int nct720x_write_reg(struct nct720x_chip_info *chip, u8 reg, u8 val)
> +{
> +	struct i2c_client *client = chip->client;
> +	int err;
> +
> +	err = i2c_smbus_write_byte_data(client, reg, val);
> +	/* wait for write command to be finished */
If this is needed, provide a datasheet reference. It is very ususual to
see a significant delay needed. 
> +	mdelay(10);
> +
> +	return err;
> +}
> +
> +static int nct720x_read_raw(struct iio_dev *indio_dev,
> +			    struct iio_chan_spec const *chan,
> +			    int *val, int *val2, long mask)
> +{
> +	int index = nct720x_chan_to_index[chan->address];
> +	int v1, v2, volt, err;
> +	struct nct720x_chip_info *chip = iio_priv(indio_dev);
> +
> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_PROCESSED:
> +		mutex_lock(&chip->access_lock);
> +		if (chip->use_read_byte_vin) {

Ah.  So resolution doesn't change, you are controlling the i2c acceses?

That should come from the i2c controller capabilities, not DT for this
device.



> +			/*
> +			 * MNTVIN Low Byte together with MNTVIN High Byte
> +			 * forms the 13-bit count value. If MNTVIN High
> +			 * Byte readout is read successively, the
> +			 * NCT7201/NCT7202 will latch the MNTVIN Low Byte
> +			 * for next read.
> +			 */
> +			v1 = nct720x_read_reg(chip, REG_VIN[index]);
> +			if (v1 < 0) {
> +				err = v1;
> +				goto abort;
> +			}
> +
> +			v2 = nct720x_read_reg(chip, REG_VOLT_LOW_BYTE);
> +			if (v2 < 0) {
> +				err = v2;
> +				goto abort;
> +			}
> +			volt = (v1 << 8) | v2;	/* Convert back to 16-bit value */
> +		} else {
> +			/* NCT7201/NCT7202 also supports read word-size data */
> +			volt = nct720x_read_word_swapped_reg(chip, REG_VIN[index]);
> +		}
> +
> +		/* Voltage(V) = 13bitCountValue * 0.0004995 */
Present this as _RAW and provide _SCALE to userspace to be able to do this
maths.  Very unusual for an ADC driver to provided processed channels. Normally
only occurs it there is something non linear going on.


> +		volt = (volt >> 3) * NCT720X_IN_SCALING;
> +		*val = volt / 10000;
> +		mutex_unlock(&chip->access_lock);
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +abort:
> +	mutex_unlock(&chip->access_lock);
> +	return err;
> +}
> +
> +static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
> +	int v1, v2, err;
> +	int volt = 0;
> +	int index = nct720x_chan_to_index[chan->address];
> +
> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;
> +
> +	if (info == IIO_EV_INFO_VALUE) {
As below - flip logic.

> +		if (dir == IIO_EV_DIR_FALLING) {
> +			if (chip->use_read_byte_vin) {
> +				/*
> +				 * Low limit VIN Low Byte together with Low limit VIN High Byte
> +				   forms the 13-bit count value
> +				 */
> +				mutex_lock(&chip->access_lock);
> +				v1 = nct720x_read_reg(chip, REG_VIN_LOW_LIMIT[index]);
> +				if (v1 < 0) {
> +					err = v1;
> +					goto abort;
> +				}
> +
> +				v2 = nct720x_read_reg(chip, REG_VIN_LOW_LIMIT_LSB[index]);
> +				if (v2 < 0) {
> +					err = v2;
> +					goto abort;
> +				}
> +				mutex_unlock(&chip->access_lock);
> +				volt = (v1 << 8) | v2;	/* Convert back to 16-bit value */

rather see this as a get_unaligned_le16 on an array of u8.
In some cases that ends up quite a bit cheaper and it also documents what is going on.

> +			} else {
> +				/* NCT7201/NCT7202 also supports read word-size data */
> +				volt = nct720x_read_word_swapped_reg(chip,
> +					REG_VIN_LOW_LIMIT[index]);
> +			}
> +		} else {
> +			if (chip->use_read_byte_vin) {
> +				/*
> +				 * High limit VIN Low Byte together with high limit VIN High Byte
> +				 * forms the 13-bit count value
> +				 */
> +				mutex_lock(&chip->access_lock);
> +				v1 = nct720x_read_reg(chip, REG_VIN_HIGH_LIMIT[index]);
> +				if (v1 < 0) {
> +					err = v1;
> +					goto abort;
> +				}
> +
> +				v2 = nct720x_read_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index]);
> +				if (v2 < 0) {
> +					err = v2;
> +					goto abort;
> +				}
> +				mutex_unlock(&chip->access_lock);
> +				volt = (v1 << 8) | v2;	/* Convert back to 16-bit value */
> +			} else {
> +				/* NCT7201/NCT7202 also supports read word-size data */
> +				volt = nct720x_read_word_swapped_reg(chip,
> +					REG_VIN_HIGH_LIMIT[index]);
> +			}
> +		}
> +	}
> +	/* Voltage(V) = 13bitCountValue * 0.0004995 */
> +	volt = (volt >> 3) * NCT720X_IN_SCALING;
> +	*val = volt / 10000;
> +
> +	return IIO_VAL_INT;
> +abort:
> +	mutex_unlock(&chip->access_lock);
guard() in appropriate places.
Again, the lock and unlock should ideally be in same scope.

> +	return err;
> +}
> +
> +static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
> +	int index, err = 0;
> +	long v1, v2, volt;
> +
> +	index = nct720x_chan_to_index[chan->address];
> +	volt = (val * 10000) / NCT720X_IN_SCALING;
> +	v1 = volt >> 5;
> +	v2 = (volt & 0x1f) << 3;
Some explanatory comments for this would be good.

> +
> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;
> +
> +	if (info == IIO_EV_INFO_VALUE) {
Flip logic.
	if (info != IIO_EV_INFO_VALUE)
		return -EINVAL;

	if (dir == IIO_EVE_DIR_FALLING) etc

> +		if (dir == IIO_EV_DIR_FALLING) {
> +			mutex_lock(&chip->access_lock);
This is badly nested.  Mutex lock and unlock should be in same scope.
So I'd pull mutex_lock() out of this if stack and use guard(mutex) instead.
That way you can just return on error.

> +			err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT[index], v1);
> +			if (err < 0) {
> +				pr_err("Failed to write REG_VIN%d_LOW_LIMIT\n", index + 1);
> +				goto abort;
> +			}
> +
> +			err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT_LSB[index], v2);
> +			if (err < 0) {
> +				pr_err("Failed to write REG_VIN%d_LOW_LIMIT_LSB\n", index + 1);
> +				goto abort;
> +			}
> +		} else {
> +			mutex_lock(&chip->access_lock);
> +			err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT[index], v1);
> +			if (err < 0) {
> +				pr_err("Failed to write REG_VIN%d_HIGH_LIMIT\n", index + 1);
> +				goto abort;
> +			}
> +
> +			err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index], v2);
> +			if (err < 0) {
> +				pr_err("Failed to write REG_VIN%d_HIGH_LIMIT_LSB\n", index + 1);
> +				goto abort;
> +			}
> +		}
> +	}
> +abort:
> +	mutex_unlock(&chip->access_lock);
> +	return err;
> +}

> +
> +static int nct720x_write_event_config(struct iio_dev *indio_dev,
> +				      const struct iio_chan_spec *chan,
> +				      enum iio_event_type type,
> +				      enum iio_event_direction dir,
> +				      int state)
> +{
> +	int err = 0;
> +	struct nct720x_chip_info *chip = iio_priv(indio_dev);
> +	int index = nct720x_chan_to_index[chan->address];
> +	unsigned int mask;
> +
> +	mask = BIT(index);
> +
> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;
> +
> +	if (!state && (chip->vin_mask & mask))
> +		chip->vin_mask &= ~mask;
> +	else if (state && !(chip->vin_mask & mask))
> +		chip->vin_mask |= mask;
> +
> +	mutex_lock(&chip->access_lock);
guard(mutex)(&chip->access_lock);

> +
> +	err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, chip->vin_mask & 0xff);
> +	if (err < 0) {
> +		pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
> +		goto abort;
		dev_err()
		return err;

> +	}
> +
> +	if (chip->type == nct7202) {

Gain, base this on a chip_info flag.

> +		err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, chip->vin_mask >> 8);
> +		if (err < 0) {
> +			pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
> +			goto abort;
Same as above.
> +		}
> +	}
> +abort:
> +	mutex_unlock(&chip->access_lock);
> +	return err;
> +}
> +
> +static int nct720x_detect(struct i2c_client *client,
> +			  struct i2c_board_info *info)
> +{
> +	struct i2c_adapter *adapter = client->adapter;
> +
> +	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
> +				     I2C_FUNC_SMBUS_WORD_DATA))
> +		return -ENODEV;
> +
> +	/* Determine the chip type. */
> +	if (i2c_smbus_read_byte_data(client, REG_VENDOR_ID) != NUVOTON_ID ||
> +	    i2c_smbus_read_byte_data(client, REG_CHIP_ID) != NCT720X_ID ||
> +	    i2c_smbus_read_byte_data(client, REG_DEVICE_ID) != NCT720X_DEVICE_ID)
> +		return -ENODEV;
> +
> +	strscpy(info->type, "nct720x", I2C_NAME_SIZE);
as below. It's unusual to find a detect in an IIO driver because the firmware
normally tells us what is there.
> +
> +	return 0;
> +}
> +
> +static const struct iio_info nct720x_info = {
> +	.read_raw = &nct720x_read_raw,
> +	.read_event_config = &nct720x_read_event_config,
> +	.write_event_config = &nct720x_write_event_config,
> +	.read_event_value = &nct720x_read_event_value,
> +	.write_event_value = &nct720x_write_event_value,
> +};
> +
> +static const struct i2c_device_id nct720x_id[];
> +
> +static int nct720x_init_chip(struct nct720x_chip_info *chip)
> +{
> +	int value = 0;
> +	int err = 0;
> +
> +	/* Initial reset */
Maybe ignore datasheet naming and call that CONFIGURATION_INIT CONFIGURATION_RESET
at which point he comment is unneeded.

> +	err = nct720x_write_reg(chip, REG_CONFIGURATION, CONFIGURATION_INIT);
> +	if (err) {
> +		pr_err("Failed to write REG_CONFIGURATION\n");
> +		return err;
> +	}
> +
> +	/* Enable Channel */
> +	err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, 0xff);
What does 0xFF represent?  I guess all channels.  If so maybe need
to build with GENMASK so it is obvious this is enabling 8 channels.

> +	if (err) {
> +		pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
> +		return err;
> +	}
> +
> +	if (chip->type == nct7202) {

Make this 'data' in the chip_info structure.  Probably a flat to say
there is a REG_CHANNEL_ENABLE_2.  That chip->type wants to go away infavour
of chip->chip_info.X flags.


> +		err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, 0xf);
> +		if (err) {
> +			pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
> +			return err;
> +		}
> +	}
> +
> +	value = nct720x_read_reg16(chip, REG_CHANNEL_ENABLE_1);
> +	if (value < 0)
> +		return value;
> +	chip->vin_mask = value;
> +
> +	/* Start monitoring if needed */
> +	value = nct720x_read_reg(chip, REG_CONFIGURATION);

Using regmap would let you simple do a single bit write in one call
Definitely look at whether there is anything stopping  you using that
helpful infrastructure.

> +	if (value < 0) {
> +		pr_err("Failed to read REG_CONFIGURATION\n");
> +		return value;
> +	}
> +
> +	value |= CONFIGURATION_START;
> +	err = nct720x_write_reg(chip, REG_CONFIGURATION, value);
> +	if (err < 0) {
> +		pr_err("Failed to write REG_CONFIGURATION\n");

dev_err() if not called only from probe, return dev_err_probe() if only called
from probe()


> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int nct720x_probe(struct i2c_client *client)
> +{
> +	const struct i2c_device_id *id = i2c_client_get_device_id(client);
> +	struct nct720x_chip_info *chip;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +	u32 tmp;
> +
> +	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +	chip = iio_priv(indio_dev);
> +
> +	if (client->dev.of_node)
> +		chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
> +	else
> +		chip->type = i2c_match_id(nct720x_id, client)->driver_data;

i2c_get_match_data() but careful with zeros... It is much better to use that with
an actual pointer.


> +
> +	chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
> +
> +	ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
As in dt binding. If we keep this (I'm doubtful that it makes sense) define a default
so that the property doesn't need to be provided.  16 would be the most obvious choice.
Then just don't check the error value when reading the property. That is.
	
	tmp = 16;
	of_property_read_u32();


	
> +	if (ret < 0) {
> +		pr_err("read-vin-data-size property not found\n");
> +		return ret;
> +	}
> +
> +	if (tmp == 8) {
> +		chip->use_read_byte_vin = true;
> +	} else if (tmp == 16) {
> +		chip->use_read_byte_vin = false;
> +	} else {
> +		pr_err("invalid read-vin-data-size (%d)\n", tmp);
> +		return -EINVAL;
> +	}
> +
> +	mutex_init(&chip->access_lock);
For new code prefer
	ret = devm_mutex_init()
	if (ret)
		return ret;

It only helps in weird debug cases but also costs us very little to support those.
> +
> +	/* this is only used for device removal purposes */
> +	i2c_set_clientdata(client, indio_dev);
Won't be needed after the change below.

> +
> +	chip->client = client;
> +
> +	ret = nct720x_init_chip(chip);
> +	if (ret < 0)
> +		return ret;
> +
> +	indio_dev->name = id->name;
Put the name in the chip_info structure.  id->name is a complex thing
when other firmwares come into play, so better to just hard code the strings
somewhere so we always know what we are getting.

> +	indio_dev->channels = nct720x_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(nct720x_channels);
> +	indio_dev->info = &nct720x_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	iio_device_register(indio_dev);
	return devm_iio_device_register();

Then you can drop the remove callback.

> +
> +	return 0;
> +}
> +
> +static void nct720x_remove(struct i2c_client *client)

Don't use wildcards even within the driver. Just name everything
after one supported part.
nct7202_remove() etc.


> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(client);
> +
> +	iio_device_unregister(indio_dev);
> +}
> +
> +static const unsigned short nct720x_address_list[] = {
> +	0x1d, 0x1e, 0x35, 0x36, I2C_CLIENT_END
> +};
> +
> +static const struct i2c_device_id nct720x_id[] = {
> +	{ "nct7201", nct7201 },
> +	{ "nct7202", nct7202 },
> +	{}
	{ }

I'm trying to slowly standardise on this formatting in IIO
so keen not to introduce more cases.  It's an arbitrary choice
but I went with the space.

> +};
> +MODULE_DEVICE_TABLE(i2c, nct720x_id);
> +
> +static const struct of_device_id nct720x_of_match[] = {
> +	{
> +		.compatible = "nuvoton,nct7201",
> +		.data = (void *)nct7201
> +	},
> +	{
> +		.compatible = "nuvoton,nct7202",
> +		.data = (void *)nct7202
Use a pointer to a chip_info structure with all the chip specific
stuff encoded as data.  That is both more extensible and removes some
ambiguities around whether zero is an error or not.

> +	},
> +	{ },
No trailing comma
> +};
> +
> +MODULE_DEVICE_TABLE(of, nct720x_of_match);
> +
> +static struct i2c_driver nct720x_driver = {
> +	.driver = {
> +		.name	= "nct720x",
> +		.of_match_table = nct720x_of_match,
> +	},
> +	.probe = nct720x_probe,
> +	.remove = nct720x_remove,
> +	.id_table = nct720x_id,
> +	.detect = nct720x_detect,

Do you need detect?  That's kind of ancient infrastructure that I thought
no one used any more (same for the address list).

> +	.address_list = nct720x_address_list,
> +};
> +
> +module_i2c_driver(nct720x_driver);
> +
> +MODULE_AUTHOR("Eason Yang <YHYANG2@nuvoton.com>");
> +MODULE_DESCRIPTION("Nuvoton NCT720x voltage monitor driver");
> +MODULE_LICENSE("GPL");
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 68570c58e7aa..9940de0ddca2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2753,6 +2753,7 @@  F:	arch/arm/mach-npcm/
 F:	arch/arm64/boot/dts/nuvoton/
 F:	drivers/*/*/*npcm*
 F:	drivers/*/*npcm*
+F:	drivers/iio/adc/nct720x.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
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 6c4e74420fd2..adbbf0ca6f57 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1008,6 +1008,15 @@  config NAU7802
 	  To compile this driver as a module, choose M here: the
 	  module will be called nau7802.
 
+config NCT720X
+	tristate "Nuvoton Instruments NCT7201 and NCT7202 Power Monitor"
+	depends on I2C
+	help
+	  If you say yes here you get support for the Nuvoton NCT7201 and
+	  NCT7202 Voltage Monitor.
+	  This driver can also be built as a module. If so, the module
+	  will be called nct720x.
+
 config NPCM_ADC
 	tristate "Nuvoton NPCM ADC driver"
 	depends on ARCH_NPCM || COMPILE_TEST
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7b91cd98c0e0..f53318e5aa04 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -91,6 +91,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_NCT720X) += nct720x.o
 obj-$(CONFIG_NPCM_ADC) += npcm_adc.o
 obj-$(CONFIG_PAC1921) += pac1921.o
 obj-$(CONFIG_PAC1934) += pac1934.o
diff --git a/drivers/iio/adc/nct720x.c b/drivers/iio/adc/nct720x.c
new file mode 100644
index 000000000000..e589479fd06e
--- /dev/null
+++ b/drivers/iio/adc/nct720x.c
@@ -0,0 +1,617 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for Nuvoton nct7201 and nct7202 power monitor chips.
+ *
+ * Copyright (c) 2022 Nuvoton Inc.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+
+#define REG_CHIP_ID			0xFD
+#define NCT720X_ID			0xD7
+#define REG_VENDOR_ID			0xFE
+#define NUVOTON_ID			0x50
+#define REG_DEVICE_ID			0xFF
+#define NCT720X_DEVICE_ID		0x12
+#define VIN_MAX				12	/* Counted from 1 */
+#define NCT7201_VIN_MAX			8
+#define NCT7202_VIN_MAX			12
+#define NCT720X_IN_SCALING		4995
+
+#define REG_INTERRUPT_STATUS_1		0x0C
+#define REG_INTERRUPT_STATUS_2		0x0D
+#define REG_VOLT_LOW_BYTE		0x0F
+#define REG_CONFIGURATION		0x10
+#define  CONFIGURATION_START		BIT(0)
+#define  CONFIGURATION_ALERT_MSK	BIT(1)
+#define  CONFIGURATION_CONV_RATE	BIT(2)
+#define  CONFIGURATION_INIT		BIT(7)
+
+#define REG_ADVANCED_CONFIGURATION	0x11
+#define  ADVANCED_CONF_MOD_ALERT	BIT(0)
+#define  ADVANCED_CONF_MOD_STS		BIT(1)
+#define  ADVANCED_CONF_FAULT_QUEUE	BIT(2)
+#define  ADVANCED_CONF_EN_DEEP_SHUTDOWN	BIT(4)
+#define  ADVANCED_CONF_EN_SMB_TIMEOUT	BIT(5)
+#define  ADVANCED_CONF_MOD_RSTIN	BIT(7)
+
+#define REG_CHANNEL_INPUT_MODE		0x12
+#define REG_CHANNEL_ENABLE_1		0x13
+#define REG_CHANNEL_ENABLE_2		0x14
+#define REG_INTERRUPT_MASK_1		0x15
+#define REG_INTERRUPT_MASK_2		0x16
+#define REG_BUSY_STATUS			0x1E
+#define REG_ONE_SHOT			0x1F
+#define REG_SMUS_ADDRESS		0xFC
+
+static const u8 REG_VIN[VIN_MAX] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05,
+				     0x06, 0x07, 0x08, 0x09, 0x0A, 0x0B};
+static const u8 REG_VIN_HIGH_LIMIT[VIN_MAX] = { 0x20, 0x22, 0x24, 0x26, 0x28, 0x2A,
+						0x2C, 0x2E, 0x30, 0x32, 0x34, 0x36};
+static const u8 REG_VIN_LOW_LIMIT[VIN_MAX] = { 0x21, 0x23, 0x25, 0x27, 0x29, 0x2B,
+					       0x2D, 0x2F, 0x31, 0x33, 0x35, 0x37};
+static const u8 REG_VIN_HIGH_LIMIT_LSB[VIN_MAX] = { 0x40, 0x42, 0x44, 0x46, 0x48, 0x4A,
+						    0x4C, 0x4E, 0x50, 0x52, 0x54, 0x56};
+static const u8 REG_VIN_LOW_LIMIT_LSB[VIN_MAX] = { 0x41, 0x43, 0x45, 0x47, 0x49, 0x4B,
+						   0x4D, 0x4F, 0x51, 0x53, 0x55, 0x57};
+static u8 nct720x_chan_to_index[] = {
+	0,	/* Not used */
+	0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11
+};
+
+
+/* List of supported devices */
+enum nct720x_chips {
+	nct7201, nct7202
+};
+
+struct nct720x_chip_info {
+	struct i2c_client	*client;
+	enum nct720x_chips	type;
+	struct mutex		access_lock;	/* for multi-byte read and write operations */
+	int vin_max;				/* number of VIN channels */
+	u32 vin_mask;
+	bool use_read_byte_vin;
+};
+
+static const struct iio_event_spec nct720x_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) |
+				 BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
+#define NCT720X_VOLTAGE_CHANNEL(chan, addr)				\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = chan,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
+		.address = addr,					\
+		.event_spec = nct720x_events,				\
+		.num_event_specs = ARRAY_SIZE(nct720x_events),		\
+	}
+
+#define NCT720X_VOLTAGE_CHANNEL_DIFF(chan1, chan2, addr)		\
+	{								\
+		.type = IIO_VOLTAGE,					\
+		.indexed = 1,						\
+		.channel = (chan1),					\
+		.channel2 = (chan2),					\
+		.differential = 1,					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
+		.address = addr,					\
+		.event_spec = nct720x_events,				\
+		.num_event_specs = ARRAY_SIZE(nct720x_events),		\
+	}
+
+static const struct iio_chan_spec nct720x_channels[] = {
+	NCT720X_VOLTAGE_CHANNEL(1, 1),
+	NCT720X_VOLTAGE_CHANNEL(2, 2),
+	NCT720X_VOLTAGE_CHANNEL(3, 3),
+	NCT720X_VOLTAGE_CHANNEL(4, 4),
+	NCT720X_VOLTAGE_CHANNEL(5, 5),
+	NCT720X_VOLTAGE_CHANNEL(6, 6),
+	NCT720X_VOLTAGE_CHANNEL(7, 7),
+	NCT720X_VOLTAGE_CHANNEL(8, 8),
+	NCT720X_VOLTAGE_CHANNEL(9, 9),
+	NCT720X_VOLTAGE_CHANNEL(10, 10),
+	NCT720X_VOLTAGE_CHANNEL(11, 11),
+	NCT720X_VOLTAGE_CHANNEL(12, 12),
+	NCT720X_VOLTAGE_CHANNEL_DIFF(1, 2, 1),
+	NCT720X_VOLTAGE_CHANNEL_DIFF(3, 4, 3),
+	NCT720X_VOLTAGE_CHANNEL_DIFF(5, 6, 5),
+	NCT720X_VOLTAGE_CHANNEL_DIFF(7, 8, 7),
+	NCT720X_VOLTAGE_CHANNEL_DIFF(9, 10, 9),
+	NCT720X_VOLTAGE_CHANNEL_DIFF(11, 12, 11),
+};
+
+/* Read 1-byte register. Returns unsigned byte data or -ERRNO on error. */
+static int nct720x_read_reg(struct nct720x_chip_info *chip, u8 reg)
+{
+	struct i2c_client *client = chip->client;
+
+	return i2c_smbus_read_byte_data(client, reg);
+}
+
+/* Read 1-byte register. Returns unsigned word data or -ERRNO on error. */
+static int nct720x_read_word_swapped_reg(struct nct720x_chip_info *chip, u8 reg)
+{
+	struct i2c_client *client = chip->client;
+
+	return i2c_smbus_read_word_swapped(client, reg);
+}
+
+/*
+ * Read 2-byte register. Returns register in big-endian format or
+ * -ERRNO on error.
+ */
+static int nct720x_read_reg16(struct nct720x_chip_info *chip, u8 reg)
+{
+	struct i2c_client *client = chip->client;
+	int ret, low;
+
+	mutex_lock(&chip->access_lock);
+	ret = i2c_smbus_read_byte_data(client, reg);
+	if (ret >= 0) {
+		low = ret;
+		ret = i2c_smbus_read_byte_data(client, reg + 1);
+		if (ret >= 0)
+			ret = low | (ret << 8);
+	}
+
+	mutex_unlock(&chip->access_lock);
+	return ret;
+}
+
+/* Write 1-byte register. Returns 0 or -ERRNO on error. */
+static int nct720x_write_reg(struct nct720x_chip_info *chip, u8 reg, u8 val)
+{
+	struct i2c_client *client = chip->client;
+	int err;
+
+	err = i2c_smbus_write_byte_data(client, reg, val);
+	/* wait for write command to be finished */
+	mdelay(10);
+
+	return err;
+}
+
+static int nct720x_read_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int *val, int *val2, long mask)
+{
+	int index = nct720x_chan_to_index[chan->address];
+	int v1, v2, volt, err;
+	struct nct720x_chip_info *chip = iio_priv(indio_dev);
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_PROCESSED:
+		mutex_lock(&chip->access_lock);
+		if (chip->use_read_byte_vin) {
+			/*
+			 * MNTVIN Low Byte together with MNTVIN High Byte
+			 * forms the 13-bit count value. If MNTVIN High
+			 * Byte readout is read successively, the
+			 * NCT7201/NCT7202 will latch the MNTVIN Low Byte
+			 * for next read.
+			 */
+			v1 = nct720x_read_reg(chip, REG_VIN[index]);
+			if (v1 < 0) {
+				err = v1;
+				goto abort;
+			}
+
+			v2 = nct720x_read_reg(chip, REG_VOLT_LOW_BYTE);
+			if (v2 < 0) {
+				err = v2;
+				goto abort;
+			}
+			volt = (v1 << 8) | v2;	/* Convert back to 16-bit value */
+		} else {
+			/* NCT7201/NCT7202 also supports read word-size data */
+			volt = nct720x_read_word_swapped_reg(chip, REG_VIN[index]);
+		}
+
+		/* Voltage(V) = 13bitCountValue * 0.0004995 */
+		volt = (volt >> 3) * NCT720X_IN_SCALING;
+		*val = volt / 10000;
+		mutex_unlock(&chip->access_lock);
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+abort:
+	mutex_unlock(&chip->access_lock);
+	return err;
+}
+
+static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
+	int v1, v2, err;
+	int volt = 0;
+	int index = nct720x_chan_to_index[chan->address];
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	if (info == IIO_EV_INFO_VALUE) {
+		if (dir == IIO_EV_DIR_FALLING) {
+			if (chip->use_read_byte_vin) {
+				/*
+				 * Low limit VIN Low Byte together with Low limit VIN High Byte
+				   forms the 13-bit count value
+				 */
+				mutex_lock(&chip->access_lock);
+				v1 = nct720x_read_reg(chip, REG_VIN_LOW_LIMIT[index]);
+				if (v1 < 0) {
+					err = v1;
+					goto abort;
+				}
+
+				v2 = nct720x_read_reg(chip, REG_VIN_LOW_LIMIT_LSB[index]);
+				if (v2 < 0) {
+					err = v2;
+					goto abort;
+				}
+				mutex_unlock(&chip->access_lock);
+				volt = (v1 << 8) | v2;	/* Convert back to 16-bit value */
+			} else {
+				/* NCT7201/NCT7202 also supports read word-size data */
+				volt = nct720x_read_word_swapped_reg(chip,
+					REG_VIN_LOW_LIMIT[index]);
+			}
+		} else {
+			if (chip->use_read_byte_vin) {
+				/*
+				 * High limit VIN Low Byte together with high limit VIN High Byte
+				 * forms the 13-bit count value
+				 */
+				mutex_lock(&chip->access_lock);
+				v1 = nct720x_read_reg(chip, REG_VIN_HIGH_LIMIT[index]);
+				if (v1 < 0) {
+					err = v1;
+					goto abort;
+				}
+
+				v2 = nct720x_read_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index]);
+				if (v2 < 0) {
+					err = v2;
+					goto abort;
+				}
+				mutex_unlock(&chip->access_lock);
+				volt = (v1 << 8) | v2;	/* Convert back to 16-bit value */
+			} else {
+				/* NCT7201/NCT7202 also supports read word-size data */
+				volt = nct720x_read_word_swapped_reg(chip,
+					REG_VIN_HIGH_LIMIT[index]);
+			}
+		}
+	}
+	/* Voltage(V) = 13bitCountValue * 0.0004995 */
+	volt = (volt >> 3) * NCT720X_IN_SCALING;
+	*val = volt / 10000;
+
+	return IIO_VAL_INT;
+abort:
+	mutex_unlock(&chip->access_lock);
+	return err;
+}
+
+static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
+	int index, err = 0;
+	long v1, v2, volt;
+
+	index = nct720x_chan_to_index[chan->address];
+	volt = (val * 10000) / NCT720X_IN_SCALING;
+	v1 = volt >> 5;
+	v2 = (volt & 0x1f) << 3;
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	if (info == IIO_EV_INFO_VALUE) {
+		if (dir == IIO_EV_DIR_FALLING) {
+			mutex_lock(&chip->access_lock);
+			err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT[index], v1);
+			if (err < 0) {
+				pr_err("Failed to write REG_VIN%d_LOW_LIMIT\n", index + 1);
+				goto abort;
+			}
+
+			err = nct720x_write_reg(chip, REG_VIN_LOW_LIMIT_LSB[index], v2);
+			if (err < 0) {
+				pr_err("Failed to write REG_VIN%d_LOW_LIMIT_LSB\n", index + 1);
+				goto abort;
+			}
+		} else {
+			mutex_lock(&chip->access_lock);
+			err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT[index], v1);
+			if (err < 0) {
+				pr_err("Failed to write REG_VIN%d_HIGH_LIMIT\n", index + 1);
+				goto abort;
+			}
+
+			err = nct720x_write_reg(chip, REG_VIN_HIGH_LIMIT_LSB[index], v2);
+			if (err < 0) {
+				pr_err("Failed to write REG_VIN%d_HIGH_LIMIT_LSB\n", index + 1);
+				goto abort;
+			}
+		}
+	}
+abort:
+	mutex_unlock(&chip->access_lock);
+	return err;
+}
+
+static int nct720x_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 nct720x_chip_info *chip = iio_priv(indio_dev);
+	int index = nct720x_chan_to_index[chan->address];
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	return !!(chip->vin_mask & BIT(index));
+}
+
+static int nct720x_write_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir,
+				      int state)
+{
+	int err = 0;
+	struct nct720x_chip_info *chip = iio_priv(indio_dev);
+	int index = nct720x_chan_to_index[chan->address];
+	unsigned int mask;
+
+	mask = BIT(index);
+
+	if (chan->type != IIO_VOLTAGE)
+		return -EOPNOTSUPP;
+
+	if (!state && (chip->vin_mask & mask))
+		chip->vin_mask &= ~mask;
+	else if (state && !(chip->vin_mask & mask))
+		chip->vin_mask |= mask;
+
+	mutex_lock(&chip->access_lock);
+
+	err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, chip->vin_mask & 0xff);
+	if (err < 0) {
+		pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
+		goto abort;
+	}
+
+	if (chip->type == nct7202) {
+		err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, chip->vin_mask >> 8);
+		if (err < 0) {
+			pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
+			goto abort;
+		}
+	}
+abort:
+	mutex_unlock(&chip->access_lock);
+	return err;
+}
+
+static int nct720x_detect(struct i2c_client *client,
+			  struct i2c_board_info *info)
+{
+	struct i2c_adapter *adapter = client->adapter;
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA |
+				     I2C_FUNC_SMBUS_WORD_DATA))
+		return -ENODEV;
+
+	/* Determine the chip type. */
+	if (i2c_smbus_read_byte_data(client, REG_VENDOR_ID) != NUVOTON_ID ||
+	    i2c_smbus_read_byte_data(client, REG_CHIP_ID) != NCT720X_ID ||
+	    i2c_smbus_read_byte_data(client, REG_DEVICE_ID) != NCT720X_DEVICE_ID)
+		return -ENODEV;
+
+	strscpy(info->type, "nct720x", I2C_NAME_SIZE);
+
+	return 0;
+}
+
+static const struct iio_info nct720x_info = {
+	.read_raw = &nct720x_read_raw,
+	.read_event_config = &nct720x_read_event_config,
+	.write_event_config = &nct720x_write_event_config,
+	.read_event_value = &nct720x_read_event_value,
+	.write_event_value = &nct720x_write_event_value,
+};
+
+static const struct i2c_device_id nct720x_id[];
+
+static int nct720x_init_chip(struct nct720x_chip_info *chip)
+{
+	int value = 0;
+	int err = 0;
+
+	/* Initial reset */
+	err = nct720x_write_reg(chip, REG_CONFIGURATION, CONFIGURATION_INIT);
+	if (err) {
+		pr_err("Failed to write REG_CONFIGURATION\n");
+		return err;
+	}
+
+	/* Enable Channel */
+	err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_1, 0xff);
+	if (err) {
+		pr_err("Failed to write REG_CHANNEL_ENABLE_1\n");
+		return err;
+	}
+
+	if (chip->type == nct7202) {
+		err = nct720x_write_reg(chip, REG_CHANNEL_ENABLE_2, 0xf);
+		if (err) {
+			pr_err("Failed to write REG_CHANNEL_ENABLE_2\n");
+			return err;
+		}
+	}
+
+	value = nct720x_read_reg16(chip, REG_CHANNEL_ENABLE_1);
+	if (value < 0)
+		return value;
+	chip->vin_mask = value;
+
+	/* Start monitoring if needed */
+	value = nct720x_read_reg(chip, REG_CONFIGURATION);
+	if (value < 0) {
+		pr_err("Failed to read REG_CONFIGURATION\n");
+		return value;
+	}
+
+	value |= CONFIGURATION_START;
+	err = nct720x_write_reg(chip, REG_CONFIGURATION, value);
+	if (err < 0) {
+		pr_err("Failed to write REG_CONFIGURATION\n");
+		return err;
+	}
+
+	return 0;
+}
+
+static int nct720x_probe(struct i2c_client *client)
+{
+	const struct i2c_device_id *id = i2c_client_get_device_id(client);
+	struct nct720x_chip_info *chip;
+	struct iio_dev *indio_dev;
+	int ret;
+	u32 tmp;
+
+	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	if (!indio_dev)
+		return -ENOMEM;
+	chip = iio_priv(indio_dev);
+
+	if (client->dev.of_node)
+		chip->type = (enum nct720x_chips)device_get_match_data(&client->dev);
+	else
+		chip->type = i2c_match_id(nct720x_id, client)->driver_data;
+
+	chip->vin_max = (chip->type == nct7201) ? NCT7201_VIN_MAX : NCT7202_VIN_MAX;
+
+	ret = of_property_read_u32(client->dev.of_node, "read-vin-data-size", &tmp);
+	if (ret < 0) {
+		pr_err("read-vin-data-size property not found\n");
+		return ret;
+	}
+
+	if (tmp == 8) {
+		chip->use_read_byte_vin = true;
+	} else if (tmp == 16) {
+		chip->use_read_byte_vin = false;
+	} else {
+		pr_err("invalid read-vin-data-size (%d)\n", tmp);
+		return -EINVAL;
+	}
+
+	mutex_init(&chip->access_lock);
+
+	/* this is only used for device removal purposes */
+	i2c_set_clientdata(client, indio_dev);
+
+	chip->client = client;
+
+	ret = nct720x_init_chip(chip);
+	if (ret < 0)
+		return ret;
+
+	indio_dev->name = id->name;
+	indio_dev->channels = nct720x_channels;
+	indio_dev->num_channels = ARRAY_SIZE(nct720x_channels);
+	indio_dev->info = &nct720x_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	iio_device_register(indio_dev);
+
+	return 0;
+}
+
+static void nct720x_remove(struct i2c_client *client)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(client);
+
+	iio_device_unregister(indio_dev);
+}
+
+static const unsigned short nct720x_address_list[] = {
+	0x1d, 0x1e, 0x35, 0x36, I2C_CLIENT_END
+};
+
+static const struct i2c_device_id nct720x_id[] = {
+	{ "nct7201", nct7201 },
+	{ "nct7202", nct7202 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, nct720x_id);
+
+static const struct of_device_id nct720x_of_match[] = {
+	{
+		.compatible = "nuvoton,nct7201",
+		.data = (void *)nct7201
+	},
+	{
+		.compatible = "nuvoton,nct7202",
+		.data = (void *)nct7202
+	},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, nct720x_of_match);
+
+static struct i2c_driver nct720x_driver = {
+	.driver = {
+		.name	= "nct720x",
+		.of_match_table = nct720x_of_match,
+	},
+	.probe = nct720x_probe,
+	.remove = nct720x_remove,
+	.id_table = nct720x_id,
+	.detect = nct720x_detect,
+	.address_list = nct720x_address_list,
+};
+
+module_i2c_driver(nct720x_driver);
+
+MODULE_AUTHOR("Eason Yang <YHYANG2@nuvoton.com>");
+MODULE_DESCRIPTION("Nuvoton NCT720x voltage monitor driver");
+MODULE_LICENSE("GPL");