Re: [PATCH v7 2/2] iio: adc: add MAX40080 current-sense amplifier driver

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Aug 18, 2026 at 05:29:28PM +0300, Stefan Popa wrote:
> The MAX40080 is a bidirectional current-sense amplifier with an
> integrated 12-bit ADC and an I2C/SMBus interface. It measures the
> voltage across an external shunt resistor and the input bus voltage,
> storing the results in an internal FIFO.
> 
> Add a direct-mode IIO driver exposing the current and voltage channels
> with raw, scale and hardware-gain attributes, a configurable
> oversampling (digital averaging) ratio, and PEC-protected register
> access. The current scale is derived from the shunt resistor value
> described in the device tree.
> 
> The driver operates in single-measurement mode: each raw read triggers
> an on-demand conversion via SMBus Quick Command and returns a matched
> current/voltage pair. This avoids the latency and complexity of the
> continuous FIFO mode while ensuring each read reflects the current
> state. The two selectable current-sense ranges are exposed through
> scale/scale_available.
> 
> Continuous FIFO buffering, threshold events and the alert interrupt are
> intentionally left out of this initial submission and may be added
> later.

...

>  endmenu
> +

Wrong placement for a new entry.

> +config MAX40080
> +	tristate "Analog Devices MAX40080 Current Sense Amplifier"
> +	depends on I2C
> +	help
> +	  Say yes here to build support for the Analog Devices MAX40080
> +	  bidirectional current-sense amplifier with a 12-bit ADC and an I2C
> +	  interface.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called max40080.

...

> obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
>  obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
>  xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
>  obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
> +obj-$(CONFIG_MAX40080) += max40080.o

Why is not ordered?

...

+ array_size.h

I think I repeated this three times already.

> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/cleanup.h>
> +#include <linux/i2c.h>
> +#include <linux/types.h>
> +#include <linux/iopoll.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/property.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>

...

> +static int max40080_read_iv_once(struct max40080_state *st, u32 *iv)
> +{
> +	u8 buf[4];

Can this be __le32?

> +	int ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(st->client, MAX40080_REG_IV,
> +					    sizeof(buf), buf);
> +	if (ret < 0)
> +		return ret;
> +
> +	*iv = get_unaligned_le32(buf);

In that case le32_to_cpu() from asm/byteorder.h may be used.

> +	return 0;
> +}

...

> +static void max40080_calc_current_scale(struct max40080_state *st)
> +{
> +	u64 numerator, denominator;
> +	u32 rem;
> +
> +	for (unsigned int i = 0; i < ARRAY_SIZE(max40080_csa_gain); i++) {
> +		numerator = 1ULL * MAX40080_INTER_VREF_mV * NANO * MICRO;
> +		denominator = 1ULL * BIT(MAX40080_ADC_RES_BITS) * max40080_csa_gain[i] *

Btw, this 1ULL * BIT() can be replaced with BIT_ULL().

> +			      st->shunt_resistor_uOhm;
> +		numerator = div64_u64(numerator, denominator);
> +		st->current_scale[i][0] = div_u64_rem(numerator, NANO, &rem);
> +		st->current_scale[i][1] = rem;
> +	}
> +}

...

> +static const struct iio_chan_spec max40080_channels[] = {
> +	{
> +		.type = IIO_CURRENT,
> +		.indexed = 1,

> +		.channel = 0,

No need

> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all_available =
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +	{
> +		.type = IIO_VOLTAGE,
> +		.indexed = 1,

> +		.channel = 0,

Same, it's default.

> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_all_available =
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +};

...

> +static int max40080_probe(struct i2c_client *client)
> +{
> +	const char *propname = "shunt-resistor-micro-ohms";

It's better to split and use when it's required.

> +	struct device *dev = &client->dev;
> +	struct iio_dev *indio_dev;
> +	struct max40080_state *st;
> +	int ret;
> +
> +	/*
> +	 * The device powers up with PEC enabled (CFG POR = 0x0060) and rejects
> +	 * unprotected transactions, so PEC support is mandatory, along with word
> +	 * access, the I2C block read used for the current/voltage pair, and the
> +	 * Quick Command used to trigger a conversion.
> +	 */
> +	if (!i2c_check_functionality(client->adapter,
> +				     I2C_FUNC_SMBUS_WORD_DATA |
> +				     I2C_FUNC_SMBUS_I2C_BLOCK |
> +				     I2C_FUNC_SMBUS_QUICK |
> +				     I2C_FUNC_SMBUS_PEC))
> +		return -EOPNOTSUPP;
> +
> +	client->flags |= I2C_CLIENT_PEC;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +	st->client = client;
> +
> +	ret = devm_mutex_init(dev, &st->lock);
> +	if (ret)
> +		return ret;

	propname = "shunt-resistor-micro-ohms";

> +	ret = device_property_read_u32(dev, propname, &st->shunt_resistor_uOhm);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "can't read %s\n", propname);
> +	if (!st->shunt_resistor_uOhm)

> +		return dev_err_probe(dev, -EINVAL, "%s must be non-zero\n",
> +				     propname);

I would dare to put this on a single line.

> +	max40080_calc_current_scale(st);
> +
> +	/* Defaults: 50 mV range, no averaging. */
> +	st->range = MAX40080_CFG_RANGE_50mV;
> +	st->oversampling_ratio = 1;
> +
> +	indio_dev->name = "max40080";
> +	indio_dev->info = &max40080_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = max40080_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(max40080_channels);
> +
> +	ret = max40080_init(st);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

-- 
With Best Regards,
Andy Shevchenko