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

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Fri, 17 Jul 2026 20:58:41 +0300
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Fri, Jul 17, 2026 at 03:38:49PM +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.  
> 
> ...
> 
> > +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];  
> 
> > +	/* DMA-safe buffer for i2c_smbus_read_i2c_block_data(). */
> > +	u8 buf[4] ____cacheline_aligned;  
> 
> Why don't you use the specific IIO macro for this?
> Do you have internal reviews for this patch series?

This is also wrong.  Despite the naming ___cacheline_aligned is not
sufficient on some architectures.   It's a performance hint, not
a correctness statement so can often be the size for say the l2 cache but
the l3 cache line is higher and the lack of coherency is beyond that point.
ARCH_DMA_MINALIGN is probably sufficient. The IIO_DMA_MINALIGN defintion
predates that being available on all architectures + ensures the buffers
are always aligned to at least 8 bytes which makes a few other things simpler
for very little cost.

Jonathan

> 
> > +};  
>