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

[Nuno Sá]

On Sat, Jul 18, 2026 at 12:41:57AM +0100, Jonathan Cameron wrote:
> 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.

To me it was not clear because of:

https://elixir.bootlin.com/linux/v7.2-rc3/source/drivers/i2c/i2c-core-smbus.c#L604

In the above case it looks like the buffer is just passed to the
controller and it won't fallback for the emaluted case where
i2c_smbus_try_get_dmabuf() seems to be always called.

Having said the above and looking (briefly)  at the .smbus_xfer_atomic() users, not
sure it will be such an issue.

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