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

[Jonathan Cameron]

On Wed, 15 Jul 2026 16:23:11 +0200
Nuno Sá <noname.nuno@xxxxxxxxx> wrote:

> 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>

A couple of follow ups even though v4 is on list.  

That should indicate clearly why you should slow down and let people
get to earlier versions.

Jonathan

> > 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.

I merged rc2 into the IIO togreg (and so testing as well) branches to resolve
merge conflicts with that series.  Upshot is just don't bother
including mod_devicetable.h in any new code.  Also don't worry
about including any of the linux/device-id headers unless you don't
get them via i2c.h, spi.h etc

> 
> > +#include <linux/mutex.h>
> > +#include <linux/pm.h>
> > +#include <linux/property.h>
> > +#include <linux/time.h>
> > +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.

I2c always bounces unless you use specific dmasafe functions to indicate
that your particular buffer is DMA safe.

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