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] [thread-next>] [day] [month] [year] [list]
Message-ID: <b5b43427-d0d7-43d5-bf8a-02a966ac25d3@wanadoo.fr>
Date: Sun, 8 Dec 2024 18:47:20 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Eason Yang <j2anfernee@...il.com>
Cc: avifishman70@...il.com, tmaimon77@...il.com, tali.perry1@...il.com,
 venture@...gle.com, yuenn@...gle.com, benjaminfair@...gle.com,
 jic23@...nel.org, lars@...afoo.de, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, nuno.sa@...log.com, dlechner@...libre.com,
 javier.carrasco.cruz@...il.com, andriy.shevchenko@...ux.intel.com,
 marcelo.schmitt@...log.com, olivier.moysan@...s.st.com,
 mitrutzceclan@...il.com, tgamblin@...libre.com, matteomartelli3@...il.com,
 alisadariana@...il.com, gstols@...libre.com, thomas.bonnefille@...tlin.com,
 ramona.nechita@...log.com, mike.looijmans@...ic.nl,
 chanh@...amperecomputing.com, KWLIU@...oton.com, yhyang2@...oton.com,
 openbmc@...ts.ozlabs.org, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 Eason Yang <j2anfernee@...il.com>
Subject: Re: [PATCH v2 2/2] iio: adc: add Nuvoton NCT720x ADC driver

Le 03/12/2024 à 10:15, Eason Yang a écrit :
> 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-Re5JQEeQqe8AvxtiuMwx3w@...lic.gmane.org>
> ---

...

> +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[] = {

const as well here?

> +	0 /* Not used */, 0, 1, 2, 3, 4, 5, 6,
> +	7, 8, 9, 10, 11,
> +};

...

> +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];
> +	u16 volt;
> +	unsigned int value;
> +	int err;
> +	struct nct720x_chip_info *chip = iio_priv(indio_dev);
> +
> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;
> +
> +	guard(mutex)(&chip->access_lock);

The IIO_CHAN_INFO_SCALE case does not seem to need the lock. Would it 
make sense to move it only in the IIO_CHAN_INFO_RAW case?

> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		err = regmap_read(chip->regmap16, REG_VIN[index], &value);
> +		if (err < 0)
> +			return err;
> +		volt = (u16)value;
> +		*val = volt >> 3;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		/* From the datasheet, we have to multiply by 0.0004995 */
> +		*val = 0;
> +		*val2 = 499500;
> +		return IIO_VAL_INT_PLUS_NANO;
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +static int 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,
> +				      bool state)
> +{
> +	int err = 0;

Harmless but useless initialisation.

> +	struct nct720x_chip_info *chip = iio_priv(indio_dev);
> +	int index = nct720x_chan_to_index[chan->address];
> +	unsigned int mask;
> +
> +	if (chan->type != IIO_VOLTAGE)
> +		return -EOPNOTSUPP;

...

> +static int nct720x_init_chip(struct nct720x_chip_info *chip)
> +{
> +	u8 data[2];
> +	unsigned int value;
> +	int err;
> +
> +	err = regmap_write(chip->regmap, REG_CONFIGURATION, BIT_CONFIGURATION_RESET);
> +	if (err) {
> +		dev_err(&chip->client->dev, "Failed to write REG_CONFIGURATION\n");
> +		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.
> +	 */
> +	mdelay(25);
> +	err  = regmap_read(chip->regmap, REG_BUSY_STATUS, &value);

double space after err.

> +	if (err < 0)
> +		return err;
> +	if (!(value & BIT_PWR_UP))
> +		return err;
> +
> +	/* Enable Channel */
> +	err = regmap_write(chip->regmap, REG_CHANNEL_ENABLE_1, REG_CHANNEL_ENABLE_1_MASK);
> +	if (err) {
> +		dev_err(&chip->client->dev, "Failed to write REG_CHANNEL_ENABLE_1\n");
> +		return err;
> +	}
> +
> +	if (chip->vin_max == 12) {
> +		err = regmap_write(chip->regmap, REG_CHANNEL_ENABLE_2, REG_CHANNEL_ENABLE_2_MASK);
> +		if (err) {
> +			dev_err(&chip->client->dev, "Failed to write REG_CHANNEL_ENABLE_2\n");
> +			return err;
> +		}
> +	}
> +
> +	guard(mutex)(&chip->access_lock);
> +	err  = regmap_read(chip->regmap, REG_CHANNEL_ENABLE_1, &value);

double space after err.

> +	if (err < 0)
> +		return err;
> +	data[0] = (u8)value;
> +
> +	err  = regmap_read(chip->regmap, REG_CHANNEL_ENABLE_2, &value);

double space after err.

> +	if (err < 0)
> +		return err;
> +	data[1] = (u8)value;
> +
> +	value = get_unaligned_le16(data);
> +	chip->vin_mask = value;
> +
> +	/* Start monitoring if needed */

...

CJ

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ