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

[Nuno Sá]

Hi Stefan,

Some comments from me

On Wed, Jul 15, 2026 at 09:36:17AM +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.
> 
> No existing IIO driver covers this device or a register-compatible part.
> The closest relatives target different silicon with incompatible register
> maps and feature sets: max9611 is a unidirectional high-side sensor with a
> die-temperature channel and MUX-selected gain and no FIFO/PEC, while
> max34408 is an 8-bit multi-channel current monitor. The MAX40080 has a
> device-specific register map with bidirectional 13-bit current, a 64-entry
> FIFO, PEC, a single-measurement mode triggered by an SMBus Quick Command,
> and two selectable input ranges, so it warrants its own driver.
> 
> Add a direct-mode IIO driver exposing the current and voltage channels
> with raw and scale attributes, a configurable oversampling (digital
> averaging) ratio, and PEC-protected register access. The two selectable
> current-sense ranges are exposed through scale/scale_available; the
> current scale is derived from the shunt-resistor-micro-ohms device-tree
> property.
> 
> Link: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX40080.pdf
> 
> Co-developed-by: Ciprian Hegbeli <ciprian.hegbeli@xxxxxxxxxx>
> Signed-off-by: Ciprian Hegbeli <ciprian.hegbeli@xxxxxxxxxx>
> Signed-off-by: Stefan Popa <stefan.popa@xxxxxxxxxx>
> ---
>  MAINTAINERS                |   9 +
>  drivers/iio/adc/Kconfig    |  11 +
>  drivers/iio/adc/Makefile   |   1 +
>  drivers/iio/adc/max40080.c | 630 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 651 insertions(+)
>  create mode 100644 drivers/iio/adc/max40080.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e087673237636..f50c1e00e12bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15513,6 +15513,15 @@ L:	linux-iio@xxxxxxxxxxxxxxx
>  S:	Supported
>  F:	drivers/iio/temperature/max30208.c
>  
> +MAXIM MAX40080 CURRENT SENSE AMPLIFIER DRIVER
> +M:	Ciprian Hegbeli <ciprian.hegbeli@xxxxxxxxxx>
> +M:	Stefan Popa <stefan.popa@xxxxxxxxxx>
> +L:	linux-iio@xxxxxxxxxxxxxxx
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml
> +F:	drivers/iio/adc/max40080.c
> +
>  MAXIM MAX7360 KEYPAD LED MFD DRIVER
>  M:	Mathieu Dubois-Briand <mathieu.dubois-briand@xxxxxxxxxxx>
>  S:	Maintained
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 58da8255525e4..b651c57bbc3f5 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1041,6 +1041,17 @@ config MAX34408
>  	  To compile this driver as a module, choose M here: the module will be
>  	  called max34408.
>  
> +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.
> +
>  config MAX77541_ADC
>  	tristate "Analog Devices MAX77541 ADC driver"
>  	depends on MFD_MAX77541
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 7cc8f9a12f763..e1953353c68a4 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -91,6 +91,7 @@ obj-$(CONFIG_MAX1241) += max1241.o
>  obj-$(CONFIG_MAX1363) += max1363.o
>  obj-$(CONFIG_MAX14001) += max14001.o
>  obj-$(CONFIG_MAX34408) += max34408.o
> +obj-$(CONFIG_MAX40080) += max40080.o
>  obj-$(CONFIG_MAX77541_ADC) += max77541-adc.o
>  obj-$(CONFIG_MAX9611) += max9611.o
>  obj-$(CONFIG_MCP320X) += mcp320x.o
> diff --git a/drivers/iio/adc/max40080.c b/drivers/iio/adc/max40080.c
> new file mode 100644
> index 0000000000000..a0c1144cfda7c
> --- /dev/null
> +++ b/drivers/iio/adc/max40080.c
> @@ -0,0 +1,630 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * MAX40080 Digital Current-Sense Amplifier driver
> + *
> + * Copyright 2026 Analog Devices, Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/cleanup.h>
> +#include <linux/i2c.h>
> +#include <linux/iopoll.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>

Typically I would say you're missing mod_devicetable.h but now we have:

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/include/linux/device-id

You might need to base your series on linux-next though.

> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/property.h>
> +#include <linux/time.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +
> +#include <linux/iio/iio.h>
> +
> +#define MAX40080_REG_CFG		0x00
> +#define MAX40080_CFG_MODE_MSK		GENMASK(2, 0)
> +#define MAX40080_CFG_PEC_EN_MSK		BIT(5)
> +#define MAX40080_CFG_RANGE_MSK		BIT(6)
> +#define MAX40080_CFG_FILTER_MSK		GENMASK(14, 12)
> +
> +#define MAX40080_REG_FIFO_CFG		0x0A
> +#define MAX40080_FIFO_CFG_STORE_IV_MSK	GENMASK(1, 0)
> +
> +#define MAX40080_REG_IV			0x10
> +/* Current is a 13-bit two's-complement value (magnitude + sign bit). */
> +#define MAX40080_IV_I_MSK		GENMASK(12, 0)
> +#define MAX40080_IV_I_SIGN_BIT		12
> +#define MAX40080_IV_V_MAG_MSK		GENMASK(27, 16)
> +#define MAX40080_IV_VALID_MSK		BIT(31)
> +
> +/* CFG.mode field values */
> +#define MAX40080_CFG_MODE_STDBY		0x00
> +#define MAX40080_CFG_MODE_SINGLE	0x02	/* one conversion per Quick Command */
> +
> +/* CFG.range field values */
> +#define MAX40080_CFG_RANGE_50mV		0
> +#define MAX40080_CFG_RANGE_10mV		1
> +
> +/* FIFO_CFG.store_iv field values */
> +#define MAX40080_FIFO_CFG_STORE_I_V	0x02
> +
> +#define MAX40080_ADC_RES		4096
> +#define MAX40080_INTER_VREF_mV		1250
> +#define MAX40080_V_BUFF_GAIN		30
> +#define MAX40080_CSA_50mV_GAIN		25
> +#define MAX40080_CSA_10mV_GAIN		125
> +
> +/*
> + * The RANGE field (CFG bit 6) selects one of two current-sense full-scale
> + * ranges: +/-50 mV (gain 25 V/V) or +/-10 mV (gain 125 V/V).
> + */
> +static const int max40080_csa_gain[] = {
> +	[MAX40080_CFG_RANGE_50mV] = MAX40080_CSA_50mV_GAIN,
> +	[MAX40080_CFG_RANGE_10mV] = MAX40080_CSA_10mV_GAIN,
> +};
> +
> +struct max40080_state {
> +	struct i2c_client *client;
> +	/* Serializes read-modify-write access to the CFG register. */
> +	struct mutex lock;
> +	u32 shunt_resistor_uohm;
> +	/*
> +	 * Cached configuration, also used to restore the device on resume after
> +	 * a suspend that may have cut its power: the selected RANGE index and
> +	 * the oversampling ratio.
> +	 */
> +	unsigned int range;
> +	int oversampling_ratio;
> +	/*
> +	 * Precomputed current scale (mA per code) for each RANGE setting, as
> +	 * {integer, nano} pairs for IIO_VAL_INT_PLUS_NANO. The range is
> +	 * selected by writing the corresponding scale.
> +	 */
> +	int current_scale[ARRAY_SIZE(max40080_csa_gain)][2];
> +};
> +
> +static const int max40080_oversampling_avail[] = { 1, 8, 16, 32, 64, 128 };
> +
> +static int max40080_update_bits(struct max40080_state *st, u8 reg,
> +				u16 mask, u16 val)
> +{
> +	int tmp;
> +
> +	guard(mutex)(&st->lock);
> +
> +	tmp = i2c_smbus_read_word_data(st->client, reg);
> +	if (tmp < 0)
> +		return tmp;
> +
> +	tmp = (tmp & ~mask) | (val & mask);
> +
> +	return i2c_smbus_write_word_data(st->client, reg, tmp);
> +}

Any special reason to not use regmap? If this was already replied in
other version, feel free to disregard this comment :)

> +
> +/*
> + * In single-measurement mode the device sits idle until it receives an SMBus
> + * Quick Command, then performs exactly one current and one voltage conversion
> + * and returns to idle. Triggering on demand this way (rather than running the
> + * FIFO continuously in active mode) means each read returns a fresh, coherent
> + * current/voltage pair instead of the oldest queued FIFO entry.
> + */
> +static int max40080_trigger_measurement(struct max40080_state *st)
> +{
> +	struct i2c_client *client = st->client;
> +
> +	return i2c_smbus_xfer(client->adapter, client->addr,
> +			      client->flags, I2C_SMBUS_WRITE, 0,
> +			      I2C_SMBUS_QUICK, NULL);
> +}

On top of what Andy said, it seems the above is only used in one place
so I would consider to just remove it.

> +
> +/*
> + * A single measurement holds the matched current/voltage pair in one 32-bit
> + * word (MAX40080_REG_IV). Reading all four bytes in one transaction returns
> + * both from the same conversion; reading the separate current (0x0C) and
> + * voltage (0x0E) registers would decorrelate the two channels.
> + *
> + * Unlike the word accesses used elsewhere, this is a plain I2C block read: the
> + * SMBus layer does not append or verify a PEC byte for it even when PEC is
> + * otherwise enabled for the device, so this transfer is not PEC protected.
> + */
> +static int max40080_read_iv_once(struct max40080_state *st, u32 *iv)
> +{
> +	u8 buf[4];
> +	int ret;
> +
> +	ret = i2c_smbus_read_i2c_block_data(st->client, MAX40080_REG_IV,
> +					    sizeof(buf), buf);

It's not clear to me that i2c will use safe buffer all the time (from a
quick look). So I would say to make this DMA safe the usual way we do in
IIO.

> +	if (ret < 0)
> +		return ret;
> +	if (ret != sizeof(buf))
> +		return -EIO;
> +
> +	*iv = get_unaligned_le32(buf);
> +
> +	return 0;
> +}
> +

...

> +
> +static int max40080_set_range(struct max40080_state *st, unsigned int range)
> +{
> +	int ret;
> +
> +	ret = max40080_update_bits(st, MAX40080_REG_CFG, MAX40080_CFG_RANGE_MSK,
> +				   FIELD_PREP(MAX40080_CFG_RANGE_MSK, range));
> +	if (ret)
> +		return ret;
> +
> +	st->range = range;
> +

We have a lock protecting max40080_update_bits() but that is not enough
given the above store.

> +	return 0;
> +}

...
> +
> +static int max40080_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct max40080_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_CURRENT:
> +			ret = max40080_get_current(st, val);
> +			if (ret)
> +				return ret;
> +			break;
> +		case IIO_VOLTAGE:
> +			ret = max40080_get_voltage(st, val);
> +			if (ret)
> +				return ret;
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +		return IIO_VAL_INT;

You can return inline instead of the breaks.

...

> +
> +static int max40080_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct max40080_state *st = iio_priv(indio_dev);
> +
> +	/*
> +	 * A suspend may have cut power to the device, resetting it to its
> +	 * power-on defaults. Reprogram it from the cached configuration.
> +	 */
> +	guard(mutex)(&st->lock);

AFAIK, there´s no need for the lock here. But I´m puzzled about the above
anyways. Do we have a real usecase for it? Without a .suspend()
implementation it makes me wonder.

- Nuno Sá