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

[Jonathan Cameron]

> 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.
> 
> Co-developed-by: Ciprian Hegbeli <ciprian.hegbeli@xxxxxxxxxx>
> Signed-off-by: Ciprian Hegbeli <ciprian.hegbeli@xxxxxxxxxx>
> Signed-off-by: Stefan Popa <stefan.popa@xxxxxxxxxx>
> Reviewed-by: Siratul Islam <siratul.islam@xxxxxxxxx>

One thing inline

> diff --git a/drivers/iio/adc/max40080.c b/drivers/iio/adc/max40080.c
> new file mode 100644
> index 000000000000..1a6568eabcaa
> --- /dev/null
> +++ b/drivers/iio/adc/max40080.c

> +/*
> + * 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)
> +{
> +	__le32 buf;

Whilst it is vanishingly unlikely we'd get a short read, setting
buf = 0; will keep the checkers happy. Sashiko did comment on this
again and I changed my mind a bit on whether we care about that warning.

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

Jonathan

-- 
Jonathan Cameron <jonathan.cameron@xxxxxxxxxxxxxxxx>