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

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

[Stefan Popa]

This series adds support for the Maxim MAX40080, 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.

The driver operates in direct (INDIO_DIRECT_MODE) mode. Each raw read
triggers a single on-demand conversion (SMBus Quick Command) and reads
back the matched current/voltage pair, so results are always fresh. It
exposes 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.

Tested on hardware with four MAX40080 devices on an I2C bus.

Regarding Andy's question about *iv being updated even on error in
read_poll_timeout: yes, this is intentional. The poll loop must update
*iv on each iteration to check the valid bit. On timeout (conversion
never completes), *iv contains the last-read value with the valid bit
still clear, but the caller always checks the return value first —
"if (ret) return ret;" prevents any use of the stale data.

Changes in v3:
  - Remove mod_devicetable.h include (Andy, Uwe's rework)
  - Use lowercase _mV suffix for unit defines: MAX40080_INTER_VREF_mV,
    MAX40080_CSA_50mV_GAIN, MAX40080_CSA_10mV_GAIN (Andy)
  - Remove MAX40080_NUM_RANGES macro, use ARRAY_SIZE() directly (Andy)
  - Fix reversed xmas tree ordering in get_oversampling_ratio() (Andy)
  - Split semantically different variable declarations in
    set_oversampling_ratio() and max40080_init() (Andy)
  - Join *val, *val2, long mask on same line in read_raw signature (Andy)
  - Use const int * cast in read_avail (Andy)
  - Join FIFO_CFG write onto single line in max40080_init() (Andy)
  - Return i2c_smbus_write_word_data() directly at end of
    max40080_init() (Andy)
  - Use device_property_present() pattern for optional shunt-resistor
    property (Andy)
  - Make vdd-supply required in DT bindings (Conor)
  - Restore i2c_set_clientdata() needed for PM resume (Sashiko)
  - Add bounds check in get_oversampling_ratio() for hardware register
    values outside expected range (Sashiko)

Changes in v2:
  - Add vdd-supply and interrupts properties to the binding (David)
  - Add types.h and time.h includes (Andy)
  - Use USEC_PER_MSEC for poll timeout readability (Andy)
  - Use 1 * MICRO for default shunt resistor (Andy)
  - Rename field macros to include register name, e.g. MAX40080_CFG_MODE_MSK
    (Jonathan, David)
  - Add indexed defines for RANGE field values and use in gain array
    (Jonathan)
  - Use array lookup in get_oversampling_ratio instead of formula (Andy)
  - Use switch statement for chan->type in read_raw (David)
  - Simplify update_bits: one-liner RMW, return write directly (Andy)
  - Add local client variable in trigger_measurement (Andy)
  - Add braces to for loops and use C99 loop variables (Siratul, Andy)
  - Flip if/else in reg_access, separate declaration from assignment (Andy)
  - Add .name= in i2c_device_id (David, Siratul)
  - Reorder declarations to reverse christmas tree (Siratul)
  - Return directly from oversampling case in write_raw (Andy)
  - Add blank lines before return in read_avail (Siratul)
  - Add Co-developed-by tag for Ciprian (Andy)
  - Use Link: tag for datasheet URL in commit message

Stefan Popa (2):
  dt-bindings: iio: adc: add maxim,max40080
  iio: adc: add MAX40080 current-sense amplifier driver

 .../bindings/iio/adc/maxim,max40080.yaml      |  64 ++
 MAINTAINERS                                   |   9 +
 drivers/iio/adc/Kconfig                       |  11 +
 drivers/iio/adc/Makefile                      |   1 +
 drivers/iio/adc/max40080.c                    | 630 ++++++++++++++++++
 5 files changed, 715 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml
 create mode 100644 drivers/iio/adc/max40080.c

--
2.53.0

[PATCH v3 1/2] dt-bindings: iio: adc: add maxim,max40080

[Stefan Popa]

Add device tree bindings for the Maxim MAX40080 bidirectional
current-sense amplifier with a 12-bit ADC and an I2C/SMBus interface.

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
 .../bindings/iio/adc/maxim,max40080.yaml      | 64 +++++++++++++++++++
 1 file changed, 64 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml
new file mode 100644
index 0000000000000..3b367071b5d3f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/maxim,max40080.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim MAX40080 bidirectional current-sense amplifier
+
+maintainers:
+  - Ciprian Hegbeli <ciprian.hegbeli@analog.com>
+  - Stefan Popa <stefan.popa@analog.com>
+
+description: |
+  The MAX40080 is a high-precision, 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, and stores the
+  results in an internal FIFO.
+
+  Datasheet:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/MAX40080.pdf
+
+properties:
+  compatible:
+    const: maxim,max40080
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: Power supply for the device.
+
+  interrupts:
+    maxItems: 1
+    description: ALERT output for overcurrent/undervoltage threshold events.
+
+  "#io-channel-cells":
+    const: 1
+
+  shunt-resistor-micro-ohms:
+    description:
+      Value of the current-sense shunt resistor connected between the IN+ and
+      IN- inputs. Used to scale the reported current.
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@20 {
+            compatible = "maxim,max40080";
+            reg = <0x20>;
+            vdd-supply = <&vdd_3v3>;
+            #io-channel-cells = <1>;
+            shunt-resistor-micro-ohms = <100000>;
+        };
+    };
-- 
2.53.0

Re: [PATCH v3 1/2] dt-bindings: iio: adc: add maxim,max40080

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `shunt-resistor-micro-ohms` property is missing from the `required` list, allowing incomplete hardware descriptions to pass validation.
--

--- Patch [1]: [PATCH v3 1/2] dt-bindings: iio: adc: add maxim,max40080 ---
commit 102dd7d43e2e646eee1b098788f579fc92431947
Author: Stefan Popa <stefan.popa@analog.com>

dt-bindings: iio: adc: add maxim,max40080

This commit adds device tree bindings for the Maxim MAX40080 bidirectional
current-sense amplifier. It defines the required and optional properties for
the device, including an integrated 12-bit ADC and an I2C/SMBus interface.

> diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max40080.yaml

[ ... ]

> +  shunt-resistor-micro-ohms:
> +    description:
> +      Value of the current-sense shunt resistor connected between the IN+ and
> +      IN- inputs. Used to scale the reported current.
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply

[Severity: Medium]
Should shunt-resistor-micro-ohms be added to this required list?

Without this property, the schema will silently pass incomplete hardware
descriptions during a dt_binding_check run. A current-sense amplifier
requires a shunt resistor value to correctly scale the measured voltage
drop into a current measurement.

> +
> +additionalProperties: false

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715063652.368501-1-stefan.popa@analog.com?part=1

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

[Stefan Popa]

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@analog.com>
Signed-off-by: Ciprian Hegbeli <ciprian.hegbeli@analog.com>
Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
 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@vger.kernel.org
 S:	Supported
 F:	drivers/iio/temperature/max30208.c
 
+MAXIM MAX40080 CURRENT SENSE AMPLIFIER DRIVER
+M:	Ciprian Hegbeli <ciprian.hegbeli@analog.com>
+M:	Stefan Popa <stefan.popa@analog.com>
+L:	linux-iio@vger.kernel.org
+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@bootlin.com>
 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>
+#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);
+}
+
+/*
+ * 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);
+}
+
+/*
+ * 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);
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(buf))
+		return -EIO;
+
+	*iv = get_unaligned_le32(buf);
+
+	return 0;
+}
+
+static int max40080_read_iv(struct max40080_state *st, u32 *iv)
+{
+	int ret, io_ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = max40080_trigger_measurement(st);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Wait for the conversion to complete by polling the FIFO valid bit
+	 * (or bail out on an I2C error). Polling the device's own status makes
+	 * this independent of the actual conversion time, which varies with the
+	 * oversampling ratio and the bus speed. The timeout is only a safety
+	 * ceiling: the worst case is the maximum 128x averaging on both the
+	 * current and voltage channels at the slowest 15 ksps base rate plus the
+	 * inter-channel switching time, i.e. roughly 20 ms; 50 ms leaves ample
+	 * margin.
+	 */
+	ret = read_poll_timeout(max40080_read_iv_once, io_ret,
+				io_ret || (*iv & MAX40080_IV_VALID_MSK),
+				1 * USEC_PER_MSEC, 50 * USEC_PER_MSEC,
+				false, st, iv);
+	if (ret)
+		return ret;
+
+	return io_ret;
+}
+
+static int max40080_get_current(struct max40080_state *st, int *val)
+{
+	u32 iv;
+	int ret;
+
+	ret = max40080_read_iv(st, &iv);
+	if (ret)
+		return ret;
+
+	*val = sign_extend32(FIELD_GET(MAX40080_IV_I_MSK, iv),
+			     MAX40080_IV_I_SIGN_BIT);
+
+	return 0;
+}
+
+static int max40080_get_voltage(struct max40080_state *st, int *val)
+{
+	u32 iv;
+	int ret;
+
+	ret = max40080_read_iv(st, &iv);
+	if (ret)
+		return ret;
+
+	*val = FIELD_GET(MAX40080_IV_V_MAG_MSK, iv);
+
+	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;
+
+	return 0;
+}
+
+/*
+ * Precompute the current scale (mA per code) for each RANGE setting as
+ * {integer, nano} pairs. The shunt drop for a full-scale code is
+ *   Vref[mV] / (ADC_RES * gain)
+ * and current = Vshunt / Rshunt, so with Rshunt in micro-ohms the scale in
+ * mA/code is
+ *   Vref[mV] * NANO * MICRO / (ADC_RES * gain * Rshunt[uohm])
+ * expressed as an integer part plus a nano fractional part.
+ */
+static void max40080_calc_current_scale(struct max40080_state *st)
+{
+	unsigned int i;
+	u32 rem;
+	u64 tmp;
+
+	for (i = 0; i < ARRAY_SIZE(max40080_csa_gain); i++) {
+		tmp = (u64)MAX40080_INTER_VREF_mV * NANO * MICRO;
+		tmp = div64_u64(tmp, (u64)MAX40080_ADC_RES * max40080_csa_gain[i] *
+				st->shunt_resistor_uohm);
+		st->current_scale[i][0] = div_u64_rem(tmp, NANO, &rem);
+		st->current_scale[i][1] = rem;
+	}
+}
+
+/*
+ * The FILTER field selects digital averaging of N consecutive conversions
+ * (no averaging, 8, 16, 32, 64 or 128), which maps directly to the IIO
+ * oversampling ratio. Averaging reduces the effective output data rate by the
+ * same factor; the conversion rate itself is set by the separate ADC_RATE
+ * field.
+ */
+static int max40080_get_oversampling_ratio(struct max40080_state *st, int *val)
+{
+	u8 filter;
+	int tmp;
+
+	tmp = i2c_smbus_read_word_data(st->client, MAX40080_REG_CFG);
+	if (tmp < 0)
+		return tmp;
+
+	filter = FIELD_GET(MAX40080_CFG_FILTER_MSK, tmp);
+	if (filter >= ARRAY_SIZE(max40080_oversampling_avail))
+		return -EIO;
+
+	*val = max40080_oversampling_avail[filter];
+
+	return 0;
+}
+
+/*
+ * max40080_oversampling_avail[] is ordered so that its index is the FILTER
+ * field value (index 0 = no averaging, index 1 = 8x, ...). Return that index
+ * for an exact match, or -EINVAL for a value that is not on the list.
+ */
+static int max40080_oversampling_to_filter(int val)
+{
+	for (int i = 0; i < ARRAY_SIZE(max40080_oversampling_avail); i++) {
+		if (max40080_oversampling_avail[i] == val)
+			return i;
+	}
+
+	return -EINVAL;
+}
+
+static int max40080_set_oversampling_ratio(struct max40080_state *st, int val)
+{
+	int filter;
+	int ret;
+
+	filter = max40080_oversampling_to_filter(val);
+	if (filter < 0)
+		return filter;
+
+	ret = max40080_update_bits(st, MAX40080_REG_CFG, MAX40080_CFG_FILTER_MSK,
+				   FIELD_PREP(MAX40080_CFG_FILTER_MSK, filter));
+	if (ret)
+		return ret;
+
+	st->oversampling_ratio = val;
+
+	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;
+	case IIO_CHAN_INFO_SCALE:
+		/*
+		 * The selectable current-sense range is exposed through
+		 * scale: each RANGE setting has its own precomputed
+		 * mA-per-code value. Userspace picks the range by writing
+		 * the matching scale.
+		 */
+		switch (chan->type) {
+		case IIO_CURRENT:
+			*val = st->current_scale[st->range][0];
+			*val2 = st->current_scale[st->range][1];
+			return IIO_VAL_INT_PLUS_NANO;
+		case IIO_VOLTAGE:
+			/* voltage[mV] = raw * Vref[mV] * buffer_gain / ADC_RES */
+			*val = MAX40080_INTER_VREF_mV * MAX40080_V_BUFF_GAIN;
+			*val2 = MAX40080_ADC_RES;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		ret = max40080_get_oversampling_ratio(st, val);
+		if (ret)
+			return ret;
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max40080_write_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);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		/* Only the current channel has a selectable range/scale. */
+		if (chan->type != IIO_CURRENT)
+			return -EINVAL;
+
+		for (unsigned int i = 0; i < ARRAY_SIZE(max40080_csa_gain); i++) {
+			if (val == st->current_scale[i][0] &&
+			    val2 == st->current_scale[i][1])
+				return max40080_set_range(st, i);
+		}
+
+		return -EINVAL;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return max40080_set_oversampling_ratio(st, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max40080_write_raw_get_fmt(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	default:
+		return IIO_VAL_INT;
+	}
+}
+
+static int max40080_read_avail(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       const int **vals, int *type, int *length,
+			       long info)
+{
+	struct max40080_state *st = iio_priv(indio_dev);
+
+	switch (info) {
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type != IIO_CURRENT)
+			return -EINVAL;
+
+		*vals = (const int *)st->current_scale;
+		*length = ARRAY_SIZE(max40080_csa_gain) * 2;
+		*type = IIO_VAL_INT_PLUS_NANO;
+
+		return IIO_AVAIL_LIST;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*vals = max40080_oversampling_avail;
+		*length = ARRAY_SIZE(max40080_oversampling_avail);
+		*type = IIO_VAL_INT;
+
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max40080_reg_access(struct iio_dev *indio_dev,
+			       unsigned int reg,
+			       unsigned int write_val,
+			       unsigned int *read_val)
+{
+	struct max40080_state *st = iio_priv(indio_dev);
+	int val;
+
+	if (!read_val)
+		return i2c_smbus_write_word_data(st->client, reg, write_val);
+
+	val = i2c_smbus_read_word_data(st->client, reg);
+	if (val < 0)
+		return val;
+
+	*read_val = val;
+
+	return 0;
+}
+
+static const struct iio_info max40080_info = {
+	.read_raw = max40080_read_raw,
+	.write_raw = max40080_write_raw,
+	.write_raw_get_fmt = max40080_write_raw_get_fmt,
+	.read_avail = max40080_read_avail,
+	.debugfs_reg_access = &max40080_reg_access,
+};
+
+static const struct iio_chan_spec max40080_channels[] = {
+	{
+		.type = IIO_CURRENT,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_shared_by_all_available =
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+	},
+	{
+		.type = IIO_VOLTAGE,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_shared_by_all_available =
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+	},
+};
+
+/*
+ * Configure (or, after a suspend that cut power, restore) the device from the
+ * cached state. The device powers up in standby with PEC enabled (CFG POR =
+ * 0x0060), so PEC is kept enabled throughout.
+ */
+static int max40080_init(struct max40080_state *st)
+{
+	u16 fifo_cfg, cfg;
+	int filter;
+	int ret;
+
+	filter = max40080_oversampling_to_filter(st->oversampling_ratio);
+	if (filter < 0)
+		return filter;
+
+	/*
+	 * Put the device in standby before (re)configuring the FIFO: the FIFO
+	 * configuration register can only be written while the device is not
+	 * converting.
+	 */
+	cfg = FIELD_PREP(MAX40080_CFG_MODE_MSK, MAX40080_CFG_MODE_STDBY) |
+	      FIELD_PREP(MAX40080_CFG_PEC_EN_MSK, 1);
+	ret = i2c_smbus_write_word_data(st->client, MAX40080_REG_CFG, cfg);
+	if (ret)
+		return ret;
+
+	/* Store a matched current+voltage pair per conversion. */
+	fifo_cfg = FIELD_PREP(MAX40080_FIFO_CFG_STORE_IV_MSK, MAX40080_FIFO_CFG_STORE_I_V);
+	ret = i2c_smbus_write_word_data(st->client, MAX40080_REG_FIFO_CFG, fifo_cfg);
+	if (ret)
+		return ret;
+
+	/*
+	 * Use single-measurement mode: the device stays idle and converts once
+	 * per SMBus Quick Command (see max40080_trigger_measurement()), so each
+	 * read returns a fresh sample rather than a queued FIFO entry.
+	 */
+	cfg = FIELD_PREP(MAX40080_CFG_MODE_MSK, MAX40080_CFG_MODE_SINGLE) |
+	      FIELD_PREP(MAX40080_CFG_PEC_EN_MSK, 1) |
+	      FIELD_PREP(MAX40080_CFG_RANGE_MSK, st->range) |
+	      FIELD_PREP(MAX40080_CFG_FILTER_MSK, filter);
+
+	return i2c_smbus_write_word_data(st->client, MAX40080_REG_CFG, cfg);
+}
+
+static int max40080_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct max40080_state *st;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	/*
+	 * The device powers up with PEC enabled (CFG POR = 0x0060) and rejects
+	 * unprotected transactions, so PEC support is mandatory, along with word
+	 * access, the I2C block read used for the current/voltage pair, and the
+	 * Quick Command used to trigger a conversion.
+	 */
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_WORD_DATA |
+				     I2C_FUNC_SMBUS_I2C_BLOCK |
+				     I2C_FUNC_SMBUS_QUICK |
+				     I2C_FUNC_SMBUS_PEC))
+		return -EOPNOTSUPP;
+
+	client->flags |= I2C_CLIENT_PEC;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	i2c_set_clientdata(client, indio_dev);
+
+	st = iio_priv(indio_dev);
+	st->client = client;
+
+	ret = devm_mutex_init(dev, &st->lock);
+	if (ret)
+		return ret;
+
+	if (device_property_present(dev, "shunt-resistor-micro-ohms")) {
+		ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms",
+					       &st->shunt_resistor_uohm);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "can't read shunt-resistor-micro-ohms\n");
+		if (!st->shunt_resistor_uohm)
+			return dev_err_probe(dev, -EINVAL,
+					     "shunt-resistor-micro-ohms must be non-zero\n");
+	} else {
+		st->shunt_resistor_uohm = 1 * MICRO;
+	}
+
+	max40080_calc_current_scale(st);
+
+	/* Defaults: 50 mV range (index 0), no averaging. */
+	st->range = 0;
+	st->oversampling_ratio = 1;
+
+	indio_dev->name = "max40080";
+	indio_dev->info = &max40080_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = max40080_channels;
+	indio_dev->num_channels = ARRAY_SIZE(max40080_channels);
+
+	ret = max40080_init(st);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+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);
+
+	return max40080_init(st);
+}
+
+static DEFINE_SIMPLE_DEV_PM_OPS(max40080_pm_ops, NULL, max40080_resume);
+
+static const struct i2c_device_id max40080_i2c_ids[] = {
+	{ .name = "max40080" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max40080_i2c_ids);
+
+static const struct of_device_id max40080_of_match[] = {
+	{ .compatible = "maxim,max40080" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max40080_of_match);
+
+static struct i2c_driver max40080_driver = {
+	.driver = {
+		.name = "max40080",
+		.of_match_table = max40080_of_match,
+		.pm = pm_sleep_ptr(&max40080_pm_ops),
+	},
+	.probe = max40080_probe,
+	.id_table = max40080_i2c_ids,
+};
+module_i2c_driver(max40080_driver);
+
+MODULE_AUTHOR("Ciprian Hegbeli <ciprian.hegbeli@analog.com>");
+MODULE_AUTHOR("Stefan Popa <stefan.popa@analog.com>");
+MODULE_DESCRIPTION("Analog Devices MAX40080 current-sense amplifier driver");
+MODULE_LICENSE("GPL");
-- 
2.53.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing lock protection around `st->range` and `st->oversampling_ratio` updates causes hardware state desynchronization and torn reads.
--

--- Patch [2]: [PATCH v3 2/2] iio: adc: add MAX40080 current-sense amplifier driver ---
commit 6f9d9d6fa7d25018a7143edc9a8c9183463f1900
Author: Stefan Popa <stefan.popa@analog.com>

iio: adc: add MAX40080 current-sense amplifier driver

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.

Link: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX40080.pdf

> 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
[ ... ]
> +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;

[Severity: Medium]
Could a race condition here cause the software cache to become desynchronized
with the hardware state?

Since max40080_update_bits() releases st->lock before returning, concurrent
writes to the scale attribute could interleave their hardware updates and
software cache updates, leaving the hardware set to one range and the cache
set to another.

> +
> +	return 0;
> +}
[ ... ]
> +static int max40080_set_oversampling_ratio(struct max40080_state *st, int val)
> +{
> +	int filter;
> +	int ret;
> +
> +	filter = max40080_oversampling_to_filter(val);
> +	if (filter < 0)
> +		return filter;
> +
> +	ret = max40080_update_bits(st, MAX40080_REG_CFG, MAX40080_CFG_FILTER_MSK,
> +				   FIELD_PREP(MAX40080_CFG_FILTER_MSK, filter));
> +	if (ret)
> +		return ret;
> +
> +	st->oversampling_ratio = val;

[Severity: Medium]
Similar to st->range above, could st->oversampling_ratio become desynchronized
with the hardware state due to being updated outside the lock?

> +
> +	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_SCALE:
> +		/*
> +		 * The selectable current-sense range is exposed through
> +		 * scale: each RANGE setting has its own precomputed
> +		 * mA-per-code value. Userspace picks the range by writing
> +		 * the matching scale.
> +		 */
> +		switch (chan->type) {
> +		case IIO_CURRENT:
> +			*val = st->current_scale[st->range][0];
> +			*val2 = st->current_scale[st->range][1];

[Severity: Medium]
Can this cause a torn read if st->range is updated concurrently?

Since st->range is used twice as an array index without taking st->lock,
concurrent writes could cause val and val2 to be mixed from different ranges.

> +			return IIO_VAL_INT_PLUS_NANO;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715063652.368501-1-stefan.popa@analog.com?part=2

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

[Andy Shevchenko]

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
> 

No blank line here, in a tag block. Also you can use Datasheet: tag
(but it's up to you).

> Co-developed-by: Ciprian Hegbeli <ciprian.hegbeli@analog.com>
> Signed-off-by: Ciprian Hegbeli <ciprian.hegbeli@analog.com>
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
> ---

...

> +MAXIM MAX40080 CURRENT SENSE AMPLIFIER DRIVER
> +M:	Ciprian Hegbeli <ciprian.hegbeli@analog.com>
> +M:	Stefan Popa <stefan.popa@analog.com>
> +L:	linux-iio@vger.kernel.org
> +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

David usually asks this to be split between patches to avoid "orphaned" files
from the MAINTAINERS perspective.

...

> +/* Current is a 13-bit two's-complement value (magnitude + sign bit). */

Please, choose a single style for _one-line_ comments, id est
period in all or no period, capital first letter in all or small letter.

> +#define MAX40080_CFG_MODE_SINGLE	0x02	/* one conversion per Quick Command */
> +
> +/* CFG.range field values */

(Three comments on one page of code and three different styles.)

...

> +	u32 shunt_resistor_uohm;

I think this also would be good as uOhm.

https://web.archive.org/web/20250629194735/http://poynton.ca/notes/units/
mentions this:

"... except that its initial letter is capitalized if the unit is named after a person."

...

> +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);

Perhaps even

	return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
			      I2C_SMBUS_WRITE, 0, I2C_SMBUS_QUICK, NULL);

> +}

...

> +static int max40080_read_iv(struct max40080_state *st, u32 *iv)
> +{
> +	int ret, io_ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = max40080_trigger_measurement(st);
> +	if (ret < 0)
> +		return ret;

What I meant is this

	u32 tmp = *iv;


> +	/*
> +	 * Wait for the conversion to complete by polling the FIFO valid bit
> +	 * (or bail out on an I2C error). Polling the device's own status makes
> +	 * this independent of the actual conversion time, which varies with the
> +	 * oversampling ratio and the bus speed. The timeout is only a safety
> +	 * ceiling: the worst case is the maximum 128x averaging on both the
> +	 * current and voltage channels at the slowest 15 ksps base rate plus the
> +	 * inter-channel switching time, i.e. roughly 20 ms; 50 ms leaves ample
> +	 * margin.
> +	 */
> +	ret = read_poll_timeout(max40080_read_iv_once, io_ret,
> +				io_ret || (*iv & MAX40080_IV_VALID_MSK),
> +				1 * USEC_PER_MSEC, 50 * USEC_PER_MSEC,
> +				false, st, iv);

	ret = read_poll_timeout(max40080_read_iv_once, io_ret,
				io_ret || (tmp & MAX40080_IV_VALID_MSK),
				1 * USEC_PER_MSEC, 50 * USEC_PER_MSEC,
				false, st, &tmp);

	/* ...the comment why we need to update iv even in error case... */
	*iv = tmp;

> +	if (ret)
> +		return ret;
> +
> +	return io_ret;
> +}

...

> +static void max40080_calc_current_scale(struct max40080_state *st)
> +{
> +	unsigned int i;
> +	u32 rem;
> +	u64 tmp;
> +
> +	for (i = 0; i < ARRAY_SIZE(max40080_csa_gain); i++) {

	for (unsigned int i = 0; i < ARRAY_SIZE(max40080_csa_gain); i++) {

> +		tmp = (u64)MAX40080_INTER_VREF_mV * NANO * MICRO;

I would even make another temporary for the numerator and denominator.

		u64 numerator, denominator;

		numerator = (u64)MAX40080_INTER_VREF_mV * NANO * MICRO;
		denominator = (u64)MAX40080_ADC_RES * max40080_csa_gain[i];

// Also possible to use a trick "1ULL * " instead of castings.

		numerator = 1ULL * MAX40080_INTER_VREF_mV * NANO * MICRO;
		denominator = 1ULL * MAX40080_ADC_RES * max40080_csa_gain[i];

> +		tmp = div64_u64(tmp, (u64)MAX40080_ADC_RES * max40080_csa_gain[i] *
> +				st->shunt_resistor_uohm);

		tmp = div64_u64(numerator, denominator * st->shunt_resistor_uOhm);

> +		st->current_scale[i][0] = div_u64_rem(tmp, NANO, &rem);
> +		st->current_scale[i][1] = rem;
> +	}
> +}

...

> +static int max40080_oversampling_to_filter(int val)
> +{
> +	for (int i = 0; i < ARRAY_SIZE(max40080_oversampling_avail); i++) {

unsigned int ?

> +		if (max40080_oversampling_avail[i] == val)
> +			return i;
> +	}
> +
> +	return -EINVAL;
> +}

...

> +	if (device_property_present(dev, "shunt-resistor-micro-ohms")) {
> +		ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms",
> +					       &st->shunt_resistor_uohm);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "can't read shunt-resistor-micro-ohms\n");
> +		if (!st->shunt_resistor_uohm)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "shunt-resistor-micro-ohms must be non-zero\n");

You can reduce data footprint by string literal deduplication. That's why in my example I used

	const char *propname;

and respective assignment. Currently you have three copies of the property
name: two in different error messages and one as a parameter to property APIs.

> +	} else {
> +		st->shunt_resistor_uohm = 1 * MICRO;
> +	}

-- 
With Best Regards,
Andy Shevchenko

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

[Nuno Sá]

On Wed, Jul 15, 2026 at 11:34:09AM +0300, Andy Shevchenko wrote:
> 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
> > 
> 
> No blank line here, in a tag block. Also you can use Datasheet: tag
> (but it's up to you).
> 
> > Co-developed-by: Ciprian Hegbeli <ciprian.hegbeli@analog.com>
> > Signed-off-by: Ciprian Hegbeli <ciprian.hegbeli@analog.com>
> > Signed-off-by: Stefan Popa <stefan.popa@analog.com>
> > ---
> 
> ...
> 
> > +MAXIM MAX40080 CURRENT SENSE AMPLIFIER DRIVER
> > +M:	Ciprian Hegbeli <ciprian.hegbeli@analog.com>
> > +M:	Stefan Popa <stefan.popa@analog.com>
> > +L:	linux-iio@vger.kernel.org
> > +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
> 
> David usually asks this to be split between patches to avoid "orphaned" files
> from the MAINTAINERS perspective.

I think checkpatch also complains if you do it like the above.

- Nuno Sá

> 
> ...
> 
> > +/* Current is a 13-bit two's-complement value (magnitude + sign bit). */
> 
> Please, choose a single style for _one-line_ comments, id est
> period in all or no period, capital first letter in all or small letter.
> 
> > +#define MAX40080_CFG_MODE_SINGLE	0x02	/* one conversion per Quick Command */
> > +
> > +/* CFG.range field values */
> 
> (Three comments on one page of code and three different styles.)
> 
> ...
> 
> > +	u32 shunt_resistor_uohm;
> 
> I think this also would be good as uOhm.
> 
> https://web.archive.org/web/20250629194735/http://poynton.ca/notes/units/
> mentions this:
> 
> "... except that its initial letter is capitalized if the unit is named after a person."
> 
> ...
> 
> > +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);
> 
> Perhaps even
> 
> 	return i2c_smbus_xfer(client->adapter, client->addr, client->flags,
> 			      I2C_SMBUS_WRITE, 0, I2C_SMBUS_QUICK, NULL);
> 
> > +}
> 
> ...
> 
> > +static int max40080_read_iv(struct max40080_state *st, u32 *iv)
> > +{
> > +	int ret, io_ret;
> > +
> > +	guard(mutex)(&st->lock);
> > +
> > +	ret = max40080_trigger_measurement(st);
> > +	if (ret < 0)
> > +		return ret;
> 
> What I meant is this
> 
> 	u32 tmp = *iv;
> 
> 
> > +	/*
> > +	 * Wait for the conversion to complete by polling the FIFO valid bit
> > +	 * (or bail out on an I2C error). Polling the device's own status makes
> > +	 * this independent of the actual conversion time, which varies with the
> > +	 * oversampling ratio and the bus speed. The timeout is only a safety
> > +	 * ceiling: the worst case is the maximum 128x averaging on both the
> > +	 * current and voltage channels at the slowest 15 ksps base rate plus the
> > +	 * inter-channel switching time, i.e. roughly 20 ms; 50 ms leaves ample
> > +	 * margin.
> > +	 */
> > +	ret = read_poll_timeout(max40080_read_iv_once, io_ret,
> > +				io_ret || (*iv & MAX40080_IV_VALID_MSK),
> > +				1 * USEC_PER_MSEC, 50 * USEC_PER_MSEC,
> > +				false, st, iv);
> 
> 	ret = read_poll_timeout(max40080_read_iv_once, io_ret,
> 				io_ret || (tmp & MAX40080_IV_VALID_MSK),
> 				1 * USEC_PER_MSEC, 50 * USEC_PER_MSEC,
> 				false, st, &tmp);
> 
> 	/* ...the comment why we need to update iv even in error case... */
> 	*iv = tmp;
> 
> > +	if (ret)
> > +		return ret;
> > +
> > +	return io_ret;
> > +}
> 
> ...
> 
> > +static void max40080_calc_current_scale(struct max40080_state *st)
> > +{
> > +	unsigned int i;
> > +	u32 rem;
> > +	u64 tmp;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(max40080_csa_gain); i++) {
> 
> 	for (unsigned int i = 0; i < ARRAY_SIZE(max40080_csa_gain); i++) {
> 
> > +		tmp = (u64)MAX40080_INTER_VREF_mV * NANO * MICRO;
> 
> I would even make another temporary for the numerator and denominator.
> 
> 		u64 numerator, denominator;
> 
> 		numerator = (u64)MAX40080_INTER_VREF_mV * NANO * MICRO;
> 		denominator = (u64)MAX40080_ADC_RES * max40080_csa_gain[i];
> 
> // Also possible to use a trick "1ULL * " instead of castings.
> 
> 		numerator = 1ULL * MAX40080_INTER_VREF_mV * NANO * MICRO;
> 		denominator = 1ULL * MAX40080_ADC_RES * max40080_csa_gain[i];
> 
> > +		tmp = div64_u64(tmp, (u64)MAX40080_ADC_RES * max40080_csa_gain[i] *
> > +				st->shunt_resistor_uohm);
> 
> 		tmp = div64_u64(numerator, denominator * st->shunt_resistor_uOhm);
> 
> > +		st->current_scale[i][0] = div_u64_rem(tmp, NANO, &rem);
> > +		st->current_scale[i][1] = rem;
> > +	}
> > +}
> 
> ...
> 
> > +static int max40080_oversampling_to_filter(int val)
> > +{
> > +	for (int i = 0; i < ARRAY_SIZE(max40080_oversampling_avail); i++) {
> 
> unsigned int ?
> 
> > +		if (max40080_oversampling_avail[i] == val)
> > +			return i;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> 
> ...
> 
> > +	if (device_property_present(dev, "shunt-resistor-micro-ohms")) {
> > +		ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms",
> > +					       &st->shunt_resistor_uohm);
> > +		if (ret)
> > +			return dev_err_probe(dev, ret,
> > +					     "can't read shunt-resistor-micro-ohms\n");
> > +		if (!st->shunt_resistor_uohm)
> > +			return dev_err_probe(dev, -EINVAL,
> > +					     "shunt-resistor-micro-ohms must be non-zero\n");
> 
> You can reduce data footprint by string literal deduplication. That's why in my example I used
> 
> 	const char *propname;
> 
> and respective assignment. Currently you have three copies of the property
> name: two in different error messages and one as a parameter to property APIs.
> 
> > +	} else {
> > +		st->shunt_resistor_uohm = 1 * MICRO;
> > +	}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

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

[Andy Shevchenko]

On Wed, Jul 15, 2026 at 09:36:15AM +0300, Stefan Popa wrote:
> This series adds support for the Maxim MAX40080, 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.
> 
> The driver operates in direct (INDIO_DIRECT_MODE) mode. Each raw read
> triggers a single on-demand conversion (SMBus Quick Command) and reads
> back the matched current/voltage pair, so results are always fresh. It
> exposes 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.
> 
> Tested on hardware with four MAX40080 devices on an I2C bus.
> 
> Regarding Andy's question about *iv being updated even on error in
> read_poll_timeout: yes, this is intentional. The poll loop must update
> *iv on each iteration to check the valid bit. On timeout (conversion
> never completes), *iv contains the last-read value with the valid bit
> still clear, but the caller always checks the return value first —
> "if (ret) return ret;" prevents any use of the stale data.

My point also is that we can do it only once. And since the above a comment is
a must. See more in the comment I'm going to give against individual piece of
code.

-- 
With Best Regards,
Andy Shevchenko