linux-kernel - Re: [PATCH v7 2/2] iio: adc: add support for Nuvoton NCT7201

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aAoU6iWGPkqjon7Z@smile.fi.intel.com>
Date: Thu, 24 Apr 2025 13:39:38 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Eason Yang <j2anfernee@...il.com>
Cc: jic23@...nel.org, lars@...afoo.de, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, dlechner@...libre.com, nuno.sa@...log.com,
	javier.carrasco.cruz@...il.com, gstols@...libre.com,
	olivier.moysan@...s.st.com, alisadariana@...il.com,
	tgamblin@...libre.com, antoniu.miclaus@...log.com,
	eblanc@...libre.com, joao.goncalves@...adex.com,
	ramona.gradinariu@...log.com, marcelo.schmitt@...log.com,
	matteomartelli3@...il.com, chanh@...amperecomputing.com,
	KWLIU@...oton.com, yhyang2@...oton.com, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 2/2] iio: adc: add support for Nuvoton NCT7201

On Thu, Apr 24, 2025 at 04:30:00PM +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 all the 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 are
> supported.

Very good, from my point of view it's almost ready, a few nit-picks below.

...

> +#include <linux/array_size.h>
> +#include <linux/bitfield.h>
> +#include <linux/bits.h>
> +#include <linux/delay.h>

> +#include <linux/device.h>

It seems this is not used, but missing
dev_printk.h
Am I mistaken?

> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>

...

> +static int nct7201_write_event_value(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     enum iio_event_type type,
> +				     enum iio_event_direction dir,
> +				     enum iio_event_info info,
> +				     int val, int val2)
> +{
> +	struct nct7201_chip_info *chip = iio_priv(indio_dev);
> +	int  err;
> +
> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;
> +
> +	if (info != IIO_EV_INFO_VALUE)
> +		return -EOPNOTSUPP;
> +
> +	if (dir == IIO_EV_DIR_FALLING)
> +		err = regmap_write(chip->regmap16, NCT7201_REG_VIN_LOW_LIMIT(chan->address),
> +				   FIELD_PREP(NCT7201_REG_VIN_MASK, val));
> +	else
> +		err = regmap_write(chip->regmap16, NCT7201_REG_VIN_HIGH_LIMIT(chan->address),
> +				   FIELD_PREP(NCT7201_REG_VIN_MASK, val));

> +	if (err)
> +		return err;
> +
> +	return 0;

	return err;

> +}

...

> +	/*
> +	 * After about 25 msecs, the device should be ready and then the power-up
> +	 * bit will be set to 1. If not, wait for it.
> +	 */
> +	fsleep(25000);

25 * USEC_PER_MSEC ?

...

> +	/* Enable Channel */
> +	if (chip->num_vin_channels <= 8) {
> +		err = regmap_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE,
> +				   GENMASK(chip->num_vin_channels - 1, 0));

> +		if (err)
> +			return dev_err_probe(dev, err, "Failed to enable channel\n");

This...

> +	} else {
> +		err = regmap_bulk_write(chip->regmap, NCT7201_REG_CHANNEL_ENABLE,
> +					&data, sizeof(data));

> +		if (err)
> +			return dev_err_probe(dev2, err, "Failed to enable channel\n");

...and this are identical, deduplicate by moving outside of if-else.

> +	}


-- 
With Best Regards,
Andy Shevchenko