[PATCH v4 0/3] iio: adc: add support for the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[PATCH v4 0/3] iio: adc: add support for the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Neil Armstrong]

The MAX34417 is a specialized current and voltage monitor used to
determine power consumption of portable systems. The driver supports
getting the channels voltage, average power and accumulated average
power and energy over an I2C/SMBUS serial interface.

The MAX34417 datasheet only specified on how to calculate the
accumulated average power, but with the Sample Per Second rate
we can easily get accumulated Energy in mJ.

By default the IIO driver only supports single-shot Voltage
and Power sensing, but the accumulation can be enabled by
enabling the Energy IIO channel which then enables the
Continuous Accumulation Mode.

The counter is common to all channels, so when reading a single
channel we will copy all accumulators to registers, so in order
to permit reading individual channels we must accumulate the
other channels in the driver when Continuous Accumulation Mode
is enabled.

The get_unaligned_be56() helper is added since we effectly
get an unaligned 56bit value from the HW.

While Analog Devices originally provided an HWMON driver for this
IC, using IIO makes much more sense because it provides
much more flexibility to support getting the power values
and will make it easier to support trigger to dump all
channels in a single shot and then support the Bulk Update
feature to update all MAX34417 on the same bus via the
trigger mechanism.

The v3 implementation has been completely revised since
v1/v2 to better match the capabilities of the MAX34417 and
allow getting accumulated Energy plus provide single
channel read without loosing other channels accumulations.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v4:
- Fix bindings with Conor review on v2
 - Rename to Analog Devices and "adi" prefix
 - Move reg before #*-cells
 - Remove "label: true"
 - Make supplies required
 - Update DT example
- Rebase on linux-next
- Fix RSENSE defines
- Drop dev from max34417_data struct
- Move acc_count to u64
- Add regmap writeable/readable/volatile callbacks
- Use regmap_set/clear_bits() when needed
- Fix MAX34417_CHANNEL layout
- Drop useless comments
- Move i in the "for (int i...") and use i++ instead of ++i
- Move mutex guard into iio callbacks
- Use lower_32_bits/upper_32_bits instead of FIELD_GET()
- Use div_u64() when returning the Power scale to make sure it runs on 32bit systems
- Reorganize the get_label() callback
- Use devm_mutex_init
- Drop fwnode_property_present(node, "reg") before getting reg
- Rename rsense variable to rsense_microOhm
- Drop !rsense test
- Register iio device after device init
- Add comment about supporting MAX34407
- Link to v3: https://patch.msgid.link/20261013-topic-sm8x50-iio-max34417-adc-v3-0-d3604ae4a229@linaro.org

Changes in v3:
- Implement Continuous Accumulate Mode and default to single-shot mode
- Fix includes
- Better define Rsense and default rsensd in driver & bindings
- Define all read length and offsets
- Define the SPS bits
- Fix comments first letters
- Fix max register
- Add ENERGY channel with shared enable for Continuous Accumulate Mode
- By default reset the MAX34417 counters for each readout to do a single-shot measure
- Handle zero counter
- Add get_unaligned_be56
- Use get_unaligned_* macros
- Better handle the DT properties and report errors
- Reset the MAX34417 at the end of probe
- Link to v2: https://patch.msgid.link/20260924-topic-sm8x50-iio-max34417-adc-v2-0-9a0609e72f5c@linaro.org

Changes in v2:
- switch to shunt-resistor-micro-ohms no more required
- removed gpio.h from example
- Fixed max34417->MAX34417 in Kconfig and comments
- Added missing includes and remove unneeded
- Fixed typos in comments
- Switched to fsleep()
- Better aligned max34417_read_power declararation
- Handled 0 acc_count
- Switched to GENMASK_ULL() for 32bits systems
- Added missing empty lines
- Moved the input correction into a helper
- Set default input correction for all channels
- Switched to dev_err_probe() to return from probe
- Switched to device_for_each_child_node_scoped()
- Handled invalid shunt-resistor-micro-ohms value
- Link to v1: https://patch.msgid.link/20260923-topic-sm8x50-iio-max34417-adc-v1-0-41d4ba1bfc41@linaro.org

To: Jonathan Cameron <jic23@kernel.org>
To: David Lechner <dlechner@baylibre.com>
To: Nuno Sá <nuno.sa@analog.com>
To: Andy Shevchenko <andy@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Michael Hennerich <Michael.Hennerich@analog.com>
Cc: linux-iio@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux@analog.com

---
Marius Cristea (1):
      unaligned: introduce be56 unaligned accessors

Neil Armstrong (2):
      dt-bindings: iio: add: document the MAX34417 Four-Channel High Dynamic Range Power Accumulator
      iio: adc: add driver for the MAX34417 Four-Channel High Dynamic Range Power Accumulator

 .../devicetree/bindings/iio/adc/adi,max34417.yaml  | 103 +++
 drivers/iio/adc/Kconfig                            |  11 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/max34417.c                         | 788 +++++++++++++++++++++
 include/linux/unaligned.h                          |  27 +
 5 files changed, 930 insertions(+)
---
base-commit: 02aaca3296e9dea76ca3a89ef4fb6508b1bd2bff
change-id: 20260923-topic-sm8x50-iio-max34417-adc-209e880533fb

Best regards,
--  
Neil Armstrong <neil.armstrong@linaro.org>

[PATCH v4 1/3] dt-bindings: iio: add: document the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Neil Armstrong]

Document the Analog Devices MAX34417 Four-Channel High Dynamic Range Power
Accumulator used to monitor power consumption of portable systems.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../devicetree/bindings/iio/adc/adi,max34417.yaml  | 103 +++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,max34417.yaml b/Documentation/devicetree/bindings/iio/adc/adi,max34417.yaml
new file mode 100644
index 000000000000..700bc3ca7d7c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,max34417.yaml
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,max34417.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices MAX34417 Four-Channel High Dynamic Range Power Accumulator
+
+maintainers:
+  - Neil Armstrong <neil.armstrong@linaro.org>
+
+description: |
+  The MAX34417 is a specialized current and voltage monitor used to determine
+  power consumption of portable systems. The device has a very wide dynamic
+  range (20,000:1) that allows for the accurate measurement of power in such
+  systems. The device is configured and monitored with a standard I2C/SMBus
+  serial interface. The unidirectional current sensor offers precision
+  high-side operation with a low full-scale sense voltage.
+
+  Specifications about the device can be found at:
+  https://www.analog.com/media/en/technical-documentation/data-sheets/max34417.pdf
+
+properties:
+  compatible:
+    const: adi,max34417
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  vdd-supply: true
+  vio-supply: true
+
+patternProperties:
+  "^channel@[0-3]$":
+    $ref: adc.yaml
+    type: object
+    description:
+      Represents the internal channels of the sensor.
+
+    properties:
+      reg:
+        items:
+          - minimum: 0
+            maximum: 3
+
+      shunt-resistor-micro-ohms:
+        description:
+          Value in micro Ohms of the shunt resistor connected between the RS+ and RS- inputs.
+        minimum: 1000
+        maximum: 100000
+        default: 1000
+
+    required:
+      - reg
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - vio-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        sensor@10 {
+              compatible = "adi,max34417";
+              reg = <0x10>;
+
+              vdd-supply = <&max34417_vdd>;
+              vio-supply = <&max34417_vio>;
+
+              #address-cells = <1>;
+              #size-cells = <0>;
+
+              channel@0 {
+                  reg = <0>;
+                  label = "ch0";
+                  shunt-resistor-micro-ohms = <5000>;
+              };
+
+              channel@1 {
+                  reg = <1>;
+                  label = "ch1";
+             };
+
+              channel@2 {
+                  reg = <2>;
+             };
+        };
+    };

-- 
2.34.1

[PATCH v4 2/3] unaligned: introduce be56 unaligned accessors

[Neil Armstrong]

From: Marius Cristea <marius.cristea@microchip.com>

The MAX34417, IIO driver, is using some unaligned 56 bit registers.
Provide some helper accessors in preparation for the new driver.

Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
[narmstrong: update commit message and move to include/linux/unaligned.h]
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 include/linux/unaligned.h | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/include/linux/unaligned.h b/include/linux/unaligned.h
index 4a9651017e3c..2105743b1800 100644
--- a/include/linux/unaligned.h
+++ b/include/linux/unaligned.h
@@ -143,4 +143,31 @@ static inline u64 get_unaligned_be48(const void *p)
 	return __get_unaligned_be48(p);
 }
 
+static inline void __put_unaligned_be56(const u64 val, u8 *p)
+{
+	*p++ = (val >> 48) & 0xff;
+	*p++ = (val >> 40) & 0xff;
+	*p++ = (val >> 32) & 0xff;
+	*p++ = (val >> 24) & 0xff;
+	*p++ = (val >> 16) & 0xff;
+	*p++ = (val >> 8) & 0xff;
+	*p++ = val & 0xff;
+}
+
+static inline void put_unaligned_be56(const u64 val, void *p)
+{
+	__put_unaligned_be56(val, p);
+}
+
+static inline u64 __get_unaligned_be56(const u8 *p)
+{
+	return (u64)p[0] << 48 | (u64)p[1] << 40 | (u64)p[2] << 32 |
+		(u64)p[3] << 24 | p[4] << 16 | p[5] << 8 | p[6];
+}
+
+static inline u64 get_unaligned_be56(const void *p)
+{
+	return __get_unaligned_be56(p);
+}
+
 #endif /* __LINUX_UNALIGNED_H */

-- 
2.34.1

[PATCH v4 3/3] iio: adc: add driver for the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Neil Armstrong]

The MAX34417 is a specialized current and voltage monitor used to
determine power consumption of portable systems. The driver support
getting the channels voltage, average power, accumulated average
power and energy over an I2C/SMBUS serial interface.

The MAX34417 datasheet only specified on how to calculate the
accumulated average power, but with the Sample Per Second rate
we can easily get accumulated Energy in mJ.

By default the IIO driver only supports single-shot Voltage
and Power sensing, but the accumulation can be enabled by
enabling the Energy IIO channel which then enables the
Continuous Accumulation Mode.

The counter is common to all channels, so when reading a single
channel we will copy all accumulators to registers, so in order
to permit reading individual channels we must accumulate the
other channels in the driver when Continuous Accumulation Mode
is enabled.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/iio/adc/Kconfig    |  11 +
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max34417.c | 788 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 800 insertions(+)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a802d8ee4dd9..b7d66dffdb89 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1144,6 +1144,17 @@ config MAX34408
 	  To compile this driver as a module, choose M here: the module will be
 	  called max34408.
 
+config MAX34417
+	tristate "Maxim MAX34417 ADC driver"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say yes here to build ADC support for Maxim MAX34417 Four-Channel High
+	  Dynamic Range Power Accumulator.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max34417.
+
 config MAX40080
 	tristate "Analog Devices MAX40080 Current Sense Amplifier"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 9e82c21dbd37..26ddf3828fe4 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -97,6 +97,7 @@ obj-$(CONFIG_MAX1241) += max1241.o
 obj-$(CONFIG_MAX1363) += max1363.o
 obj-$(CONFIG_MAX14001) += max14001.o
 obj-$(CONFIG_MAX34408) += max34408.o
+obj-$(CONFIG_MAX34417) += max34417.o
 obj-$(CONFIG_MAX40080) += max40080.o
 obj-$(CONFIG_MAX77541_ADC) += max77541-adc.o
 obj-$(CONFIG_MAX9611) += max9611.o
diff --git a/drivers/iio/adc/max34417.c b/drivers/iio/adc/max34417.c
new file mode 100644
index 000000000000..b1064cf631b5
--- /dev/null
+++ b/drivers/iio/adc/max34417.c
@@ -0,0 +1,788 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * IIO driver for Analog Devices MAX34417 ADC, 4-Channels High Dynamic Range Power Accumulator
+ *
+ * Datasheet: https://www.analog.com/en/products/max34417.html
+ *
+ * TODO: Slow Mode, Park Feature, Bulk Update, Perr_Verr Correction
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/init.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+#define MAX34417_UPDATE_REG		0x0
+#define MAX34417_CONTROL_REG		0x1
+#define MAX34417_ACC_COUNT_REG		0x2
+
+#define MAX34417_PWR_ACC_1_REG		0x3
+#define MAX34417_PWR_ACC_2_REG		0x4
+#define MAX34417_PWR_ACC_3_REG		0x5
+#define MAX34417_PWR_ACC_4_REG		0x6
+
+#define MAX34417_V_CH1_REG		0x7
+#define MAX34417_V_CH2_REG		0x8
+#define MAX34417_V_CH3_REG		0x9
+#define MAX34417_V_CH4_REG		0xa
+
+#define MAX34417_DID_REG		0xf
+
+#define MAX34417_BULK_POWER_READOUT_REG		0x10
+#define MAX34417_BULK_VOLTAGE_READOUT_REG	0x11
+
+#define MAX34417_BULK_UPDATE_ADDRESS	0x2c
+#define MAX34417_BULK_UPDATE_REG	0x0
+
+/* Bit masks for control register */
+#define MAX34417_CONTROL_OVF		BIT(0)
+#define MAX34417_CONTROL_SLOW		BIT(1)
+#define MAX34417_CONTROL_PARK0		BIT(2)
+#define MAX34417_CONTROL_PARK1		BIT(3)
+#define MAX34417_CONTROL_PARK_EN	BIT(4)
+#define MAX34417_CONTROL_SMM		BIT(5)
+#define MAX34417_CONTROL_CAM		BIT(6)
+#define MAX34417_CONTROL_MODE		BIT(7)
+
+#define MAX34417_ACC_COUNT_READ_LENGTH	4
+#define MAX34417_ACC_COUNT_READ_OFFSET	1
+
+#define MAX34417_POWER_READ_LENGTH	8
+#define MAX34417_POWER_READ_OFFSET	1
+
+#define MAX34417_VOLTAGE_READ_LENGTH	3
+#define MAX34417_VOLTAGE_READ_OFFSET	1
+
+#define MAX34417_MIN_RSENSE		(1 * MILLI) /* 1000 microOhm */
+#define MAX34417_MAX_RSENSE		(100 * MILLI) /* 100000 microOhm */
+#define MAX34417_DEFAULT_RSENSE		(1 * MILLI) /* 1000 microOhm */
+
+#define MAX34417_PWR_CORRECTION_SCALE		24
+#define MAX34417_PWR_AVG_FULL_SCALE_BITS	30
+
+#define MAX34417_VOLTAGE_CORRECTION_SCALE	24
+#define MAX34417_VOLTAGE_FULL_SCALE_BITS	14
+
+/* TODO implement IIO_CHAN_INFO_SAMP_FREQ for slow mode */
+#define MAX34417_SLOW_SPS_BIT			3 /* 8 SPS */
+#define MAX34417_FAST_SPS_BIT			10 /* 1024 SPS */
+#define MAX34417_FAST_SPS_CAM			1536 /* 1.5 * 1024 SPS */
+
+#define MAX34417_CHANNEL_COUNT			4
+
+/**
+ * struct max34417_data - MAX34417 specific data.
+ * @indio_dev:  IIO Device.
+ * @regmap:	Device register map.
+ * @lock:	Lock for protecting access to device hardware registers, mostly
+ *		for reading common accumulator count and control register.
+ * @input_correction:	Correction based on the Rsense value from channel nodes.
+ * @input_label:	Channel label from channel nodes.
+ * @cam_mode:		State of the Continuous Accumulate Mode feature
+ * @acc_power:		Saved power accumulator for each channel when in CAM mode
+ * @acc_count:		Saved counter for each channel when in CAM mode
+ */
+struct max34417_data {
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+	struct mutex lock;
+	u32 input_correction[MAX34417_CHANNEL_COUNT];
+	const char *input_label[MAX34417_CHANNEL_COUNT];
+	bool cam_mode;
+	u64 acc_power[MAX34417_CHANNEL_COUNT];
+	u64 acc_count[MAX34417_CHANNEL_COUNT];
+};
+
+static bool max34417_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MAX34417_UPDATE_REG:
+	case MAX34417_CONTROL_REG:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool max34417_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MAX34417_UPDATE_REG:
+		return false;
+	default:
+		return true;
+	}
+}
+
+static bool max34417_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MAX34417_DID_REG:
+		return false;
+	default:
+	/* MAX34417_CONTROL_REG field OVF is volatile */
+		return true;
+	}
+}
+
+static const struct regmap_config max34417_regmap_config = {
+	.reg_bits	= 8,
+	.val_bits	= 8,
+	.max_register	= MAX34417_BULK_VOLTAGE_READOUT_REG,
+	.writeable_reg	= max34417_writeable_reg,
+	.readable_reg	= max34417_readable_reg,
+	.volatile_reg	= max34417_volatile_reg,
+	/* No cache since the only non-volatile register is read-only */
+};
+
+#define MAX34417_CHANNEL(_index, _v_address, _power_address)	\
+	{							\
+		.type = IIO_VOLTAGE,				\
+		.info_mask_separate =				\
+			BIT(IIO_CHAN_INFO_RAW) |		\
+			BIT(IIO_CHAN_INFO_SCALE),		\
+		.channel = (_index),				\
+		.address = (_v_address),			\
+		.indexed = 1,					\
+	},							\
+	{							\
+		.type = IIO_POWER,				\
+		.info_mask_separate =				\
+			BIT(IIO_CHAN_INFO_RAW) |		\
+			BIT(IIO_CHAN_INFO_AVERAGE_RAW) |	\
+			BIT(IIO_CHAN_INFO_SCALE),		\
+		.channel = (_index),				\
+		.address = (_power_address),			\
+		.indexed = 1,					\
+	},							\
+	{							\
+		.type = IIO_ENERGY,				\
+		.info_mask_separate =				\
+			BIT(IIO_CHAN_INFO_RAW) |		\
+			BIT(IIO_CHAN_INFO_SCALE),		\
+		.info_mask_shared_by_type =			\
+			BIT(IIO_CHAN_INFO_ENABLE),		\
+		.channel = (_index),				\
+		.address = (_power_address),			\
+		.indexed = 1,					\
+	}
+
+static const struct iio_chan_spec max34417_channels[] = {
+	MAX34417_CHANNEL(0, MAX34417_V_CH1_REG, MAX34417_PWR_ACC_1_REG),
+	MAX34417_CHANNEL(1, MAX34417_V_CH2_REG, MAX34417_PWR_ACC_2_REG),
+	MAX34417_CHANNEL(2, MAX34417_V_CH3_REG, MAX34417_PWR_ACC_3_REG),
+	MAX34417_CHANNEL(3, MAX34417_V_CH4_REG, MAX34417_PWR_ACC_4_REG),
+};
+
+/* TODO Implement trigger to update accumulator once and get all channels at once */
+
+static int max34417_reset_smm(struct max34417_data *max34417)
+{
+	int rc;
+
+	/* Set SMM=0 to re-init the SMM mode */
+	rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
+			       MAX34417_CONTROL_SMM);
+	if (rc)
+		return rc;
+
+	/* Reset accumulators after setting SMM=0 */
+	return regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+}
+
+static int max34417_trigger_smm(struct max34417_data *max34417)
+{
+	u32 control;
+	int rc;
+
+	rc = regmap_read(max34417->regmap, MAX34417_CONTROL_REG, &control);
+	if (rc)
+		return rc;
+
+	if (control & MAX34417_CONTROL_SMM) {
+		/* Set SMM=0 to re-init the SMM mode */
+		rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
+				       MAX34417_CONTROL_SMM);
+		if (rc)
+			return rc;
+
+		/* Reset accumulators after setting SMM=0 */
+		rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+		if (rc)
+			return rc;
+	}
+
+	/* Enable SMM to do a single accumulation */
+	rc = regmap_set_bits(max34417->regmap, MAX34417_CONTROL_REG,
+			     MAX34417_CONTROL_SMM);
+	if (rc)
+		return rc;
+
+	/*
+	 * After the SMM bit is changed, the UPDATE command should be sent
+	 * to reset the accumulators and perform the selected scan operation.
+	 */
+	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+	if (rc)
+		return rc;
+	/*
+	 * UPDATE commands should be no less than 1msec apart for reliable
+	 * measurements.
+	 */
+	fsleep(1 * USEC_PER_MSEC);
+
+	return 0;
+}
+
+static int max34417_read_acc_count(struct max34417_data *max34417, u32 *acc_count)
+{
+	u8 buf[MAX34417_ACC_COUNT_READ_LENGTH];
+	int rc;
+
+	rc = regmap_noinc_read(max34417->regmap, MAX34417_ACC_COUNT_REG,
+			       &buf, sizeof(buf));
+	if (rc)
+		return rc;
+
+	/* Ignore byte0 */
+	*acc_count = get_unaligned_be24(&buf[MAX34417_ACC_COUNT_READ_OFFSET]);
+
+	return 0;
+}
+
+static int max34417_read_acc_power(struct max34417_data *max34417, unsigned int address,
+				   u64 *acc_power)
+{
+	u8 buf[MAX34417_POWER_READ_LENGTH];
+	int rc;
+
+	rc = regmap_noinc_read(max34417->regmap, address, &buf, sizeof(buf));
+	if (rc)
+		return rc;
+
+	/* Ignore byte0 */
+	*acc_power = get_unaligned_be56(&buf[MAX34417_POWER_READ_OFFSET]);
+
+	return 0;
+}
+
+static int max34417_check_ovf(struct max34417_data *max34417)
+{
+	u32 control;
+	int rc;
+
+	rc = regmap_read(max34417->regmap, MAX34417_CONTROL_REG, &control);
+	if (rc)
+		return rc;
+
+	if (control & MAX34417_CONTROL_OVF) {
+		rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
+				       MAX34417_CONTROL_OVF);
+		if (rc)
+			return rc;
+
+		rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+		if (rc)
+			return rc;
+	}
+
+	return 0;
+}
+
+/*
+ * Save all power accumulators and counter after each UPDATE command
+ * since it will reset the internal registers for ALL channels,
+ * so in order to not loose any captured data we retrieve all registers
+ * and accumulate them in the context variables.
+ * When user gets either power or energy, those variables content
+ * will be returned and zero-ed to return the power accumulated
+ * since last read for each separate channel.
+ */
+static int max34417_save_accumulators(struct max34417_data *max34417)
+{
+	u64 acc_power;
+	u32 acc_count;
+	int rc;
+
+	rc = max34417_read_acc_count(max34417, &acc_count);
+	if (rc)
+		return rc;
+
+	for (int i = 0; i < MAX34417_CHANNEL_COUNT; i++) {
+		rc = max34417_read_acc_power(max34417, MAX34417_PWR_ACC_1_REG + i,
+					     &acc_power);
+		if (rc)
+			return rc;
+
+		/* TODO sanitize accumulators in case of overflow */
+		max34417->acc_power[i] += acc_power;
+		max34417->acc_count[i] += acc_count;
+	}
+
+	/* Check for overflow */
+	return max34417_check_ovf(max34417);
+}
+
+static int max34417_read_voltage(struct max34417_data *max34417,
+				 const struct iio_chan_spec *chan, int *val)
+{
+	u8 buf[MAX34417_VOLTAGE_READ_LENGTH];
+	int rc;
+
+	/* In SMM mode, trigger a measurement cycle manually */
+	if (!max34417->cam_mode) {
+		rc = max34417_trigger_smm(max34417);
+		if (rc)
+			return rc;
+	}
+
+	/* Move the new UPDATE data into the SMBus read registers */
+	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+	if (rc)
+		return rc;
+
+	if (max34417->cam_mode) {
+		/* Retrieve all channels to avoid missing data for other channels */
+		rc = max34417_save_accumulators(max34417);
+		if (rc)
+			return rc;
+	}
+
+	rc = regmap_noinc_read(max34417->regmap, chan->address, &buf, sizeof(buf));
+	if (rc)
+		return rc;
+
+	if (!max34417->cam_mode) {
+		rc = max34417_reset_smm(max34417);
+		if (rc)
+			return rc;
+	}
+
+	*val = get_unaligned_be16(&buf[MAX34417_VOLTAGE_READ_OFFSET]) >> 2;
+
+	return IIO_VAL_INT;
+}
+
+static int max34417_read_power(struct max34417_data *max34417,
+			       const struct iio_chan_spec *chan,
+			       int *val, int *val2)
+{
+	u64 power;
+	int rc;
+
+	/* In SMM mode, trigger a measurement cycle manually */
+	if (!max34417->cam_mode) {
+		rc = max34417_trigger_smm(max34417);
+		if (rc)
+			return rc;
+	}
+
+	/*
+	 * Move the new UPDATE data into the SMBus read registers.
+	 * In CAM mode this will also reset the accumulators, returning
+	 * the power accumulated since the last UPDATE.
+	 */
+	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+	if (rc)
+		return rc;
+
+	/*
+	 * Only get the accumulator counter in CAM mode, in SMM mode
+	 * we only accumulate only once, so the counter is 1
+	 */
+	if (max34417->cam_mode) {
+		u64 acc_count;
+		u64 acc_power;
+
+		/* Retrieve all channels to avoid missing data for other channels */
+		rc = max34417_save_accumulators(max34417);
+		if (rc)
+			return rc;
+
+		/* Get current channel power, counter and reset them */
+		acc_power = max34417->acc_power[chan->channel];
+		acc_count = max34417->acc_count[chan->channel];
+		max34417->acc_power[chan->channel] = 0;
+		max34417->acc_count[chan->channel] = 0;
+
+		if (acc_count == 0) {
+			dev_warn_ratelimited(&max34417->indio_dev->dev,
+					     "unexpected zero accumulator count\n");
+
+			*val = 0;
+			*val2 = 0;
+
+			return IIO_VAL_INT_64;
+		}
+
+		power = div64_u64(acc_power, acc_count);
+	} else {
+		rc = max34417_read_acc_power(max34417, chan->address, &power);
+		if (rc)
+			return rc;
+
+		rc = max34417_reset_smm(max34417);
+		if (rc)
+			return rc;
+	}
+
+	*val = lower_32_bits(power);
+	*val2 = upper_32_bits(power);
+
+	return IIO_VAL_INT_64;
+}
+
+static int max34417_read_energy(struct max34417_data *max34417,
+			       const struct iio_chan_spec *chan,
+			       int *val, int *val2)
+{
+	u64 acc_power;
+	int rc;
+
+	/* Move the data to the SMBus registers */
+	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+	if (rc)
+		return rc;
+
+	/* Retrieve all channels to avoid missing data for other channels */
+	rc = max34417_save_accumulators(max34417);
+	if (rc)
+		return rc;
+
+	/* Get current channel power and reset them */
+	acc_power = max34417->acc_power[chan->channel];
+	max34417->acc_power[chan->channel] = 0;
+	max34417->acc_count[chan->channel] = 0;
+
+	*val = lower_32_bits(acc_power);
+	*val2 = upper_32_bits(acc_power);
+
+	return IIO_VAL_INT_64;
+}
+
+static int max34417_setup_cam_mode(struct max34417_data *max34417, int val)
+{
+	int rc;
+
+	if (max34417->cam_mode && !val) {
+		rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
+				       MAX34417_CONTROL_CAM | MAX34417_CONTROL_SMM);
+		if (rc)
+			return rc;
+
+		/*
+		 * If CAM mode must be set, the Control byte must be set to
+		 * desired values with CAM = 0, then followed by an UPDATE
+		 */
+		rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+		if (rc)
+			return rc;
+	} else if (!max34417->cam_mode && val) {
+		/*
+		 * After this, the CAM mode bit can be set and another
+		 * UPDATE must be sent.
+		 */
+		rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
+				       MAX34417_CONTROL_SMM);
+		if (rc)
+			return rc;
+		rc = regmap_set_bits(max34417->regmap, MAX34417_CONTROL_REG,
+				     MAX34417_CONTROL_CAM);
+		if (rc)
+			return rc;
+
+		rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+		if (rc)
+			return rc;
+
+		/* Reset counters and accumulators */
+		for (int i = 0; i < MAX34417_CHANNEL_COUNT; i++) {
+			max34417->acc_power[i] = 0;
+			max34417->acc_count[i] = 0;
+		}
+	}
+
+	max34417->cam_mode = val;
+
+	return 0;
+}
+
+static int max34417_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct max34417_data *max34417 = iio_priv(indio_dev);
+
+	guard(mutex)(&max34417->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			return max34417_read_voltage(max34417, chan, val);
+		case IIO_POWER:
+			/* In default mode, we return a single measurement */
+			if (max34417->cam_mode)
+				return -EINVAL;
+
+			return max34417_read_power(max34417, chan, val, val2);
+		case IIO_ENERGY:
+			if (!max34417->cam_mode)
+				return -EINVAL;
+
+			return max34417_read_energy(max34417, chan, val, val2);
+		default:
+			return -EINVAL;
+		}
+	case IIO_CHAN_INFO_AVERAGE_RAW:
+		if (chan->type != IIO_POWER)
+			return -EINVAL;
+
+		/* In CAM mode, we return an average measurement */
+		if (!max34417->cam_mode)
+			return -EINVAL;
+
+		return max34417_read_power(max34417, chan, val, val2);
+	case IIO_CHAN_INFO_SCALE:
+		if (chan->type == IIO_VOLTAGE) {
+			/* Scale to mV */
+			*val = MAX34417_VOLTAGE_CORRECTION_SCALE * MILLI;
+			*val2 = MAX34417_VOLTAGE_FULL_SCALE_BITS;
+
+			return IIO_VAL_FRACTIONAL_LOG2;
+		} else if (chan->type == IIO_POWER) {
+			/*
+			 * Scale to mW
+			 * (1000 * (Power Accumulation / Counter) * Correction) / 2^30
+			 */
+			*val = max34417->input_correction[chan->channel] * MILLIWATT_PER_WATT;
+			*val2 = MAX34417_PWR_AVG_FULL_SCALE_BITS;
+
+			return IIO_VAL_FRACTIONAL_LOG2;
+		} else if (chan->type == IIO_ENERGY) {
+			/*
+			 * Scale to mJ
+			 * P=(1000 * (Power Accumulation / Counter) * Correction) / 2^30
+			 * t=(Counter / Samples per Second)
+			 * E=(P * t)
+			 * Can be simplified to:
+			 * (1000 * Power Accumulation * Correction) / (2^30 * SPS)
+			 * The denominator doesn't fit in 32bit so scale down both terms.
+			 */
+			*val = max34417->input_correction[chan->channel];
+			*val2 = div_u64(BIT_ULL(MAX34417_PWR_AVG_FULL_SCALE_BITS) *
+					MAX34417_FAST_SPS_CAM, MILLI);
+
+			return IIO_VAL_FRACTIONAL;
+		}
+
+		return -EINVAL;
+	case IIO_CHAN_INFO_ENABLE:
+		*val = max34417->cam_mode;
+
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max34417_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan,
+			      int val, int val2, long mask)
+{
+	struct max34417_data *max34417 = iio_priv(indio_dev);
+
+	guard(mutex)(&max34417->lock);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_ENABLE:
+		/* Setup the CAM mode for the 4 channels */
+		return max34417_setup_cam_mode(max34417, val);
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max34417_read_label(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       char *label)
+{
+	struct max34417_data *max34417 = iio_priv(indio_dev);
+	const char *input_label = max34417->input_label[chan->channel];
+
+	if (input_label) {
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			return sysfs_emit(label, "%s-voltage\n", input_label);
+		case IIO_POWER:
+			return sysfs_emit(label, "%s-power\n", input_label);
+		case IIO_ENERGY:
+			return sysfs_emit(label, "%s-energy\n", input_label);
+		default:
+			return 0;
+		}
+	}
+
+	switch (chan->type) {
+	case IIO_VOLTAGE:
+		return sysfs_emit(label, "channel%d-voltage\n", chan->channel);
+	case IIO_POWER:
+		return sysfs_emit(label, "channel%d-power\n", chan->channel);
+	case IIO_ENERGY:
+		return sysfs_emit(label, "channel%d-energy\n", chan->channel);
+	default:
+		return 0;
+	}
+}
+
+static const struct iio_info max34417_info = {
+	.read_raw	= max34417_read_raw,
+	.write_raw	= max34417_write_raw,
+	.read_label	= max34417_read_label,
+};
+
+static unsigned int max34417_calc_input_correction(u32 rsense_microOhm)
+{
+	/* (100 milliOhm / rsense) * MAX34417_PWR_CORRECTION_SCALE */
+	return (MAX34417_MAX_RSENSE * MAX34417_PWR_CORRECTION_SCALE) / rsense_microOhm;
+}
+
+static int max34417_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct max34417_data *max34417;
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+	int rc;
+
+	regmap = devm_regmap_init_i2c(client, &max34417_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap), "regmap_init failed\n");
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*max34417));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	rc = devm_regulator_get_enable(dev, "vdd");
+	if (rc)
+		return dev_err_probe(dev, rc, "failed to get vdd regulator\n");
+
+	rc = devm_regulator_get_enable(dev, "vio");
+	if (rc)
+		return dev_err_probe(dev, rc, "failed to get vio regulator\n");
+
+	max34417 = iio_priv(indio_dev);
+	max34417->regmap = regmap;
+	max34417->indio_dev = indio_dev;
+
+	rc = devm_mutex_init(dev, &max34417->lock);
+	if (rc)
+		return rc;
+
+	/* Set default input correction for all channels */
+	for (int i = 0; i < MAX34417_CHANNEL_COUNT; i++)
+		max34417->input_correction[i] =
+			max34417_calc_input_correction(MAX34417_DEFAULT_RSENSE);
+
+	device_for_each_child_node_scoped(dev, node) {
+		u32 rsense_microOhm, index;
+
+		rc = fwnode_property_read_u32(node, "reg", &index);
+		if (rc)
+			return dev_err_probe(dev, rc, "failure reading reg property of %pfwP\n",
+					     node);
+
+		if (index >= MAX34417_CHANNEL_COUNT)
+			return dev_err_probe(dev, -EINVAL, "invalid reg %d of %pfwP\n",
+					     index, node);
+
+		if (fwnode_property_present(node, "label")) {
+			rc = fwnode_property_read_string(node, "label",
+							 &max34417->input_label[index]);
+			if (rc)
+				return dev_err_probe(dev, rc,
+						"failure reading label property of %pfwP\n",
+						node);
+		}
+
+		if (fwnode_property_present(node, "shunt-resistor-micro-ohms")) {
+			rc = fwnode_property_read_u32(node, "shunt-resistor-micro-ohms",
+						      &rsense_microOhm);
+			if (rc)
+				return dev_err_probe(dev, rc,
+						"failure reading shunt property of %pfwP\n",
+						node);
+
+			if (rsense_microOhm < MAX34417_MIN_RSENSE ||
+			    rsense_microOhm > MAX34417_MAX_RSENSE)
+				return dev_err_probe(dev, -EINVAL,
+						     "invalid shunt value %d of %pfwP\n",
+						     rsense_microOhm, node);
+
+			max34417->input_correction[index] =
+				max34417_calc_input_correction(rsense_microOhm);
+		}
+	}
+
+	indio_dev->channels = max34417_channels;
+	indio_dev->num_channels = ARRAY_SIZE(max34417_channels);
+	indio_dev->name = "max34417";
+	indio_dev->info = &max34417_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	/*
+	 * Reset mode and set accumulator width to Wide
+	 *
+	 * The MAX34407 variant could be supported by setting the
+	 * MAX34417_CONTROL_MODE field to 0 and handling a smaller
+	 * accumulator width.
+	 */
+	rc = regmap_write(max34417->regmap, MAX34417_CONTROL_REG, MAX34417_CONTROL_MODE);
+	if (rc)
+		return dev_err_probe(dev, rc, "Error writing control register\n");
+
+	/* Reset the accumulators */
+	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
+	if (rc)
+		return rc;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id max34417_of_match[] = {
+	{ .compatible = "adi,max34417" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max34417_of_match);
+
+static const struct i2c_device_id max34417_id[] = {
+	{ .name = "max34417" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max34417_id);
+
+static struct i2c_driver max34417_driver = {
+	.driver = {
+		.name   = "max34417",
+		.of_match_table = max34417_of_match,
+	},
+	.probe = max34417_probe,
+	.id_table = max34417_id,
+};
+module_i2c_driver(max34417_driver);
+
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
+MODULE_DESCRIPTION("Analog Devices MAX34417 ADC driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v4 2/3] unaligned: introduce be56 unaligned accessors

[Andy Shevchenko]

On Wed, Oct 28, 2026 at 02:19:05PM +0100, Neil Armstrong wrote:

> The MAX34417, IIO driver, is using some unaligned 56 bit registers.

56-bit

(When you refer to the size as adjective, use '-' dash, otherwise space.)

> Provide some helper accessors in preparation for the new driver.

...

I think I gave a tag to something similar in the past. Anyways,
with the above being fixed,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/3] iio: adc: add driver for the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Andy Shevchenko]

On Wed, Oct 28, 2026 at 02:19:06PM +0100, Neil Armstrong wrote:
> The MAX34417 is a specialized current and voltage monitor used to
> determine power consumption of portable systems. The driver support
> getting the channels voltage, average power, accumulated average
> power and energy over an I2C/SMBUS serial interface.
> 
> The MAX34417 datasheet only specified on how to calculate the
> accumulated average power, but with the Sample Per Second rate
> we can easily get accumulated Energy in mJ.
> 
> By default the IIO driver only supports single-shot Voltage
> and Power sensing, but the accumulation can be enabled by
> enabling the Energy IIO channel which then enables the
> Continuous Accumulation Mode.
> 
> The counter is common to all channels, so when reading a single
> channel we will copy all accumulators to registers, so in order
> to permit reading individual channels we must accumulate the
> other channels in the driver when Continuous Accumulation Mode
> is enabled.

...

> +#define MAX34417_MIN_RSENSE		(1 * MILLI) /* 1000 microOhm */
> +#define MAX34417_MAX_RSENSE		(100 * MILLI) /* 100000 microOhm */
> +#define MAX34417_DEFAULT_RSENSE		(1 * MILLI) /* 1000 microOhm */

Add unit suffix. The comment versus MILLI multiplier is quite confusing.
What I read, it's milli-Ohms by value.

...

> +struct max34417_data {
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +	struct mutex lock;
> +	u32 input_correction[MAX34417_CHANNEL_COUNT];
> +	const char *input_label[MAX34417_CHANNEL_COUNT];
> +	bool cam_mode;

Don't you have 4-byte gap here in some cases? Can you confirm the layout
with `pahole`? On top you can check binary size by running `bloat-o-meter`.

> +	u64 acc_power[MAX34417_CHANNEL_COUNT];
> +	u64 acc_count[MAX34417_CHANNEL_COUNT];
> +};

...

> +static bool max34417_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case MAX34417_DID_REG:
> +		return false;
> +	default:
> +	/* MAX34417_CONTROL_REG field OVF is volatile */

Why not add a case instead of a comment?

> +		return true;
> +	}
> +}

...

> +static int max34417_reset_smm(struct max34417_data *max34417)
> +{
> +	int rc;
> +
> +	/* Set SMM=0 to re-init the SMM mode */
> +	rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
> +			       MAX34417_CONTROL_SMM);

With

	struct regmap *map = max34417->regmap;

this becomes a single line

	rc = regmap_clear_bits(map, MAX34417_CONTROL_REG, MAX34417_CONTROL_SMM);

Use this trick elsewhere where appropriate.

> +	if (rc)
> +		return rc;
> +
> +	/* Reset accumulators after setting SMM=0 */
> +	return regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
> +}

...

> +	*val = lower_32_bits(power);
> +	*val2 = upper_32_bits(power);

Have you included wordpart.h?

...

> +static int max34417_read_label(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       char *label)
> +{
> +	struct max34417_data *max34417 = iio_priv(indio_dev);
> +	const char *input_label = max34417->input_label[chan->channel];
> +
> +	if (input_label) {
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			return sysfs_emit(label, "%s-voltage\n", input_label);

sysfs.h for sysfs_emit().

> +		case IIO_POWER:
> +			return sysfs_emit(label, "%s-power\n", input_label);
> +		case IIO_ENERGY:
> +			return sysfs_emit(label, "%s-energy\n", input_label);
> +		default:
> +			return 0;
> +		}
> +	}
> +
> +	switch (chan->type) {
> +	case IIO_VOLTAGE:
> +		return sysfs_emit(label, "channel%d-voltage\n", chan->channel);
> +	case IIO_POWER:
> +		return sysfs_emit(label, "channel%d-power\n", chan->channel);
> +	case IIO_ENERGY:
> +		return sysfs_emit(label, "channel%d-energy\n", chan->channel);
> +	default:
> +		return 0;
> +	}
> +}

...

> +static unsigned int max34417_calc_input_correction(u32 rsense_microOhm)

rsense_uOhm is shorter

> +{
> +	/* (100 milliOhm / rsense) * MAX34417_PWR_CORRECTION_SCALE */

Comment in this form is almost useless. Rather explain that we except
the result in the certain units or something like this.

> +	return (MAX34417_MAX_RSENSE * MAX34417_PWR_CORRECTION_SCALE) / rsense_microOhm;
> +}

...

> +static int max34417_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct max34417_data *max34417;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;

*map is shorter

> +	int rc;
> +
> +	regmap = devm_regmap_init_i2c(client, &max34417_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap), "regmap_init failed\n");
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*max34417));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	rc = devm_regulator_get_enable(dev, "vdd");
> +	if (rc)
> +		return dev_err_probe(dev, rc, "failed to get vdd regulator\n");
> +
> +	rc = devm_regulator_get_enable(dev, "vio");
> +	if (rc)
> +		return dev_err_probe(dev, rc, "failed to get vio regulator\n");
> +
> +	max34417 = iio_priv(indio_dev);
> +	max34417->regmap = regmap;
> +	max34417->indio_dev = indio_dev;
> +
> +	rc = devm_mutex_init(dev, &max34417->lock);
> +	if (rc)
> +		return rc;
> +
> +	/* Set default input correction for all channels */
> +	for (int i = 0; i < MAX34417_CHANNEL_COUNT; i++)
> +		max34417->input_correction[i] =
> +			max34417_calc_input_correction(MAX34417_DEFAULT_RSENSE);
> +
> +	device_for_each_child_node_scoped(dev, node) {
> +		u32 rsense_microOhm, index;

rsense_uOhm

> +		rc = fwnode_property_read_u32(node, "reg", &index);
> +		if (rc)
> +			return dev_err_probe(dev, rc, "failure reading reg property of %pfwP\n",
> +					     node);
> +
> +		if (index >= MAX34417_CHANNEL_COUNT)
> +			return dev_err_probe(dev, -EINVAL, "invalid reg %d of %pfwP\n",

-ECHRNG

> +					     index, node);
> +
> +		if (fwnode_property_present(node, "label")) {
> +			rc = fwnode_property_read_string(node, "label",
> +							 &max34417->input_label[index]);
> +			if (rc)
> +				return dev_err_probe(dev, rc,
> +						"failure reading label property of %pfwP\n",
> +						node);
> +		}
> +
> +		if (fwnode_property_present(node, "shunt-resistor-micro-ohms")) {
> +			rc = fwnode_property_read_u32(node, "shunt-resistor-micro-ohms",
> +						      &rsense_microOhm);
> +			if (rc)
> +				return dev_err_probe(dev, rc,
> +						"failure reading shunt property of %pfwP\n",
> +						node);
> +
> +			if (rsense_microOhm < MAX34417_MIN_RSENSE ||
> +			    rsense_microOhm > MAX34417_MAX_RSENSE)
> +				return dev_err_probe(dev, -EINVAL,
> +						     "invalid shunt value %d of %pfwP\n",
> +						     rsense_microOhm, node);
> +
> +			max34417->input_correction[index] =
> +				max34417_calc_input_correction(rsense_microOhm);
> +		}
> +	}
> +
> +	indio_dev->channels = max34417_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(max34417_channels);
> +	indio_dev->name = "max34417";
> +	indio_dev->info = &max34417_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	/*
> +	 * Reset mode and set accumulator width to Wide
> +	 *
> +	 * The MAX34407 variant could be supported by setting the
> +	 * MAX34417_CONTROL_MODE field to 0 and handling a smaller
> +	 * accumulator width.
> +	 */
> +	rc = regmap_write(max34417->regmap, MAX34417_CONTROL_REG, MAX34417_CONTROL_MODE);

You have local variable for regmap, use it.

> +	if (rc)
> +		return dev_err_probe(dev, rc, "Error writing control register\n");
> +
> +	/* Reset the accumulators */
> +	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
> +	if (rc)
> +		return rc;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 3/3] iio: adc: add driver for the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Neil Armstrong]

On 10/28/26 16:06, Andy Shevchenko wrote:
> On Wed, Oct 28, 2026 at 02:19:06PM +0100, Neil Armstrong wrote:
>> The MAX34417 is a specialized current and voltage monitor used to
>> determine power consumption of portable systems. The driver support
>> getting the channels voltage, average power, accumulated average
>> power and energy over an I2C/SMBUS serial interface.
>>
>> The MAX34417 datasheet only specified on how to calculate the
>> accumulated average power, but with the Sample Per Second rate
>> we can easily get accumulated Energy in mJ.
>>
>> By default the IIO driver only supports single-shot Voltage
>> and Power sensing, but the accumulation can be enabled by
>> enabling the Energy IIO channel which then enables the
>> Continuous Accumulation Mode.
>>
>> The counter is common to all channels, so when reading a single
>> channel we will copy all accumulators to registers, so in order
>> to permit reading individual channels we must accumulate the
>> other channels in the driver when Continuous Accumulation Mode
>> is enabled.
> 
> ...
> 
>> +#define MAX34417_MIN_RSENSE		(1 * MILLI) /* 1000 microOhm */
>> +#define MAX34417_MAX_RSENSE		(100 * MILLI) /* 100000 microOhm */
>> +#define MAX34417_DEFAULT_RSENSE		(1 * MILLI) /* 1000 microOhm */
> 
> Add unit suffix. The comment versus MILLI multiplier is quite confusing.
> What I read, it's milli-Ohms by value.
> 
> ...
> 
>> +struct max34417_data {
>> +	struct iio_dev *indio_dev;
>> +	struct regmap *regmap;
>> +	struct mutex lock;
>> +	u32 input_correction[MAX34417_CHANNEL_COUNT];
>> +	const char *input_label[MAX34417_CHANNEL_COUNT];
>> +	bool cam_mode;
> 
> Don't you have 4-byte gap here in some cases? Can you confirm the layout
> with `pahole`? On top you can check binary size by running `bloat-o-meter`.
> 
>> +	u64 acc_power[MAX34417_CHANNEL_COUNT];
>> +	u64 acc_count[MAX34417_CHANNEL_COUNT];
>> +};
> 
> ...
> 
>> +static bool max34417_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> +	switch (reg) {
>> +	case MAX34417_DID_REG:
>> +		return false;
>> +	default:
>> +	/* MAX34417_CONTROL_REG field OVF is volatile */
> 
> Why not add a case instead of a comment?

The case is handled by the default, the comment is here to
explain why MAX34417_CONTROL_REG is considered as volatile.

> 
>> +		return true;
>> +	}
>> +}
> 
> ...
> 
>> +static int max34417_reset_smm(struct max34417_data *max34417)
>> +{
>> +	int rc;
>> +
>> +	/* Set SMM=0 to re-init the SMM mode */
>> +	rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
>> +			       MAX34417_CONTROL_SMM);
> 
> With
> 
> 	struct regmap *map = max34417->regmap;
> 
> this becomes a single line
> 
> 	rc = regmap_clear_bits(map, MAX34417_CONTROL_REG, MAX34417_CONTROL_SMM);
> 
> Use this trick elsewhere where appropriate.

Ack

> 
>> +	if (rc)
>> +		return rc;
>> +
>> +	/* Reset accumulators after setting SMM=0 */
>> +	return regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
>> +}
> 
> ...
> 
>> +	*val = lower_32_bits(power);
>> +	*val2 = upper_32_bits(power);
> 
> Have you included wordpart.h?

Will add

> 
> ...
> 
>> +static int max34417_read_label(struct iio_dev *indio_dev,
>> +			       struct iio_chan_spec const *chan,
>> +			       char *label)
>> +{
>> +	struct max34417_data *max34417 = iio_priv(indio_dev);
>> +	const char *input_label = max34417->input_label[chan->channel];
>> +
>> +	if (input_label) {
>> +		switch (chan->type) {
>> +		case IIO_VOLTAGE:
>> +			return sysfs_emit(label, "%s-voltage\n", input_label);
> 
> sysfs.h for sysfs_emit().

Will add

> 
>> +		case IIO_POWER:
>> +			return sysfs_emit(label, "%s-power\n", input_label);
>> +		case IIO_ENERGY:
>> +			return sysfs_emit(label, "%s-energy\n", input_label);
>> +		default:
>> +			return 0;
>> +		}
>> +	}
>> +
>> +	switch (chan->type) {
>> +	case IIO_VOLTAGE:
>> +		return sysfs_emit(label, "channel%d-voltage\n", chan->channel);
>> +	case IIO_POWER:
>> +		return sysfs_emit(label, "channel%d-power\n", chan->channel);
>> +	case IIO_ENERGY:
>> +		return sysfs_emit(label, "channel%d-energy\n", chan->channel);
>> +	default:
>> +		return 0;
>> +	}
>> +}
> 
> ...
> 
>> +static unsigned int max34417_calc_input_correction(u32 rsense_microOhm)
> 
> rsense_uOhm is shorter

Used that initially, will revert to that

> 
>> +{
>> +	/* (100 milliOhm / rsense) * MAX34417_PWR_CORRECTION_SCALE */
> 
> Comment in this form is almost useless. Rather explain that we except
> the result in the certain units or something like this.

Will try to improve

> 
>> +	return (MAX34417_MAX_RSENSE * MAX34417_PWR_CORRECTION_SCALE) / rsense_microOhm;
>> +}
> 
> ...
> 
>> +static int max34417_probe(struct i2c_client *client)
>> +{
>> +	struct device *dev = &client->dev;
>> +	struct max34417_data *max34417;
>> +	struct iio_dev *indio_dev;
>> +	struct regmap *regmap;
> 
> *map is shorter

Ok

> 
>> +	int rc;
>> +
>> +	regmap = devm_regmap_init_i2c(client, &max34417_regmap_config);
>> +	if (IS_ERR(regmap))
>> +		return dev_err_probe(dev, PTR_ERR(regmap), "regmap_init failed\n");
>> +
>> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*max34417));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	rc = devm_regulator_get_enable(dev, "vdd");
>> +	if (rc)
>> +		return dev_err_probe(dev, rc, "failed to get vdd regulator\n");
>> +
>> +	rc = devm_regulator_get_enable(dev, "vio");
>> +	if (rc)
>> +		return dev_err_probe(dev, rc, "failed to get vio regulator\n");
>> +
>> +	max34417 = iio_priv(indio_dev);
>> +	max34417->regmap = regmap;
>> +	max34417->indio_dev = indio_dev;
>> +
>> +	rc = devm_mutex_init(dev, &max34417->lock);
>> +	if (rc)
>> +		return rc;
>> +
>> +	/* Set default input correction for all channels */
>> +	for (int i = 0; i < MAX34417_CHANNEL_COUNT; i++)
>> +		max34417->input_correction[i] =
>> +			max34417_calc_input_correction(MAX34417_DEFAULT_RSENSE);
>> +
>> +	device_for_each_child_node_scoped(dev, node) {
>> +		u32 rsense_microOhm, index;
> 
> rsense_uOhm
> 
>> +		rc = fwnode_property_read_u32(node, "reg", &index);
>> +		if (rc)
>> +			return dev_err_probe(dev, rc, "failure reading reg property of %pfwP\n",
>> +					     node);
>> +
>> +		if (index >= MAX34417_CHANNEL_COUNT)
>> +			return dev_err_probe(dev, -EINVAL, "invalid reg %d of %pfwP\n",
> 
> -ECHRNG

Ack

> 
>> +					     index, node);
>> +
>> +		if (fwnode_property_present(node, "label")) {
>> +			rc = fwnode_property_read_string(node, "label",
>> +							 &max34417->input_label[index]);
>> +			if (rc)
>> +				return dev_err_probe(dev, rc,
>> +						"failure reading label property of %pfwP\n",
>> +						node);
>> +		}
>> +
>> +		if (fwnode_property_present(node, "shunt-resistor-micro-ohms")) {
>> +			rc = fwnode_property_read_u32(node, "shunt-resistor-micro-ohms",
>> +						      &rsense_microOhm);
>> +			if (rc)
>> +				return dev_err_probe(dev, rc,
>> +						"failure reading shunt property of %pfwP\n",
>> +						node);
>> +
>> +			if (rsense_microOhm < MAX34417_MIN_RSENSE ||
>> +			    rsense_microOhm > MAX34417_MAX_RSENSE)
>> +				return dev_err_probe(dev, -EINVAL,
>> +						     "invalid shunt value %d of %pfwP\n",
>> +						     rsense_microOhm, node);
>> +
>> +			max34417->input_correction[index] =
>> +				max34417_calc_input_correction(rsense_microOhm);
>> +		}
>> +	}
>> +
>> +	indio_dev->channels = max34417_channels;
>> +	indio_dev->num_channels = ARRAY_SIZE(max34417_channels);
>> +	indio_dev->name = "max34417";
>> +	indio_dev->info = &max34417_info;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +
>> +	/*
>> +	 * Reset mode and set accumulator width to Wide
>> +	 *
>> +	 * The MAX34407 variant could be supported by setting the
>> +	 * MAX34417_CONTROL_MODE field to 0 and handling a smaller
>> +	 * accumulator width.
>> +	 */
>> +	rc = regmap_write(max34417->regmap, MAX34417_CONTROL_REG, MAX34417_CONTROL_MODE);
> 
> You have local variable for regmap, use it.
> 
>> +	if (rc)
>> +		return dev_err_probe(dev, rc, "Error writing control register\n");
>> +
>> +	/* Reset the accumulators */
>> +	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
>> +	if (rc)
>> +		return rc;
>> +
>> +	return devm_iio_device_register(dev, indio_dev);
>> +}
> 

Thanks,
Neil

Re: [PATCH v4 1/3] dt-bindings: iio: add: document the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 75 bytes --]

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 3/3] iio: adc: add driver for the MAX34417 Four-Channel High Dynamic Range Power Accumulator

[Victor Chernyakin]

> +/**
> + * struct max34417_data - MAX34417 specific data.
> + * @indio_dev:  IIO Device.
> + * @regmap:	Device register map.
> + * @lock:	Lock for protecting access to device hardware registers, mostly
> + *		for reading common accumulator count and control register.
> + * @input_correction:	Correction based on the Rsense value from channel nodes.
> + * @input_label:	Channel label from channel nodes.
> + * @cam_mode:		State of the Continuous Accumulate Mode feature
> + * @acc_power:		Saved power accumulator for each channel when in CAM mode
> + * @acc_count:		Saved counter for each channel when in CAM mode
> + */

All the sentences should have periods at the end

> +static int max34417_check_ovf(struct max34417_data *max34417)
> +{
> +	u32 control;
> +	int rc;
> +
> +	rc = regmap_read(max34417->regmap, MAX34417_CONTROL_REG, &control);
> +	if (rc)
> +		return rc;
> +
> +	if (control & MAX34417_CONTROL_OVF) {
> +		rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
> +				       MAX34417_CONTROL_OVF);
> +		if (rc)
> +			return rc;
> +
> +		rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	return 0;
> +}

Up to you, but could reduce indentation here:

	if (!(control & MAX34417_CONTROL_OVF))
		return 0;

	rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
			       MAX34417_CONTROL_OVF);
	if (rc)
		return rc;

	return regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);


> +static int max34417_read_power(struct max34417_data *max34417,
> +			       const struct iio_chan_spec *chan,
> +			       int *val, int *val2)
> +{
> +	u64 power;
> +	int rc;
> +
> +	/* In SMM mode, trigger a measurement cycle manually */
> +	if (!max34417->cam_mode) {
> +		rc = max34417_trigger_smm(max34417);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	/*
> +	 * Move the new UPDATE data into the SMBus read registers.
> +	 * In CAM mode this will also reset the accumulators, returning
> +	 * the power accumulated since the last UPDATE.
> +	 */
> +	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
> +	if (rc)
> +		return rc;
> +
> +	/*
> +	 * Only get the accumulator counter in CAM mode, in SMM mode
> +	 * we only accumulate only once, so the counter is 1
> +	 */

"only" is duplicated

> +	if (max34417->cam_mode) {
> +		u64 acc_count;
> +		u64 acc_power;
> +
> +		/* Retrieve all channels to avoid missing data for other channels */
> +		rc = max34417_save_accumulators(max34417);
> +		if (rc)
> +			return rc;
> +
> +		/* Get current channel power, counter and reset them */
> +		acc_power = max34417->acc_power[chan->channel];
> +		acc_count = max34417->acc_count[chan->channel];
> +		max34417->acc_power[chan->channel] = 0;
> +		max34417->acc_count[chan->channel] = 0;
> +
> +		if (acc_count == 0) {
> +			dev_warn_ratelimited(&max34417->indio_dev->dev,
> +					     "unexpected zero accumulator count\n");
> +
> +			*val = 0;
> +			*val2 = 0;
> +
> +			return IIO_VAL_INT_64;
> +		}
> +
> +		power = div64_u64(acc_power, acc_count);

Shorter and makes the branches more symmetrical:

	if (acc_count == 0) {
		dev_warn_ratelimited(&max34417->indio_dev->dev,
				     "unexpected zero accumulator count\n");

		power = 0;
	} else {
		power = div64_u64(acc_power, acc_count);
	}


> +	} else {
> +		rc = max34417_read_acc_power(max34417, chan->address, &power);
> +		if (rc)
> +			return rc;
> +
> +		rc = max34417_reset_smm(max34417);
> +		if (rc)
> +			return rc;
> +	}
> +
> +	*val = lower_32_bits(power);
> +	*val2 = upper_32_bits(power);

iio_val_s64_decompose(power, val, val2)?

> +
> +	return IIO_VAL_INT_64;
> +}
> +
> +static int max34417_read_energy(struct max34417_data *max34417,
> +			       const struct iio_chan_spec *chan,
> +			       int *val, int *val2)
> +{
> +	u64 acc_power;
> +	int rc;
> +
> +	/* Move the data to the SMBus registers */
> +	rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
> +	if (rc)
> +		return rc;
> +
> +	/* Retrieve all channels to avoid missing data for other channels */
> +	rc = max34417_save_accumulators(max34417);
> +	if (rc)
> +		return rc;
> +
> +	/* Get current channel power and reset them */
> +	acc_power = max34417->acc_power[chan->channel];
> +	max34417->acc_power[chan->channel] = 0;
> +	max34417->acc_count[chan->channel] = 0;
> +
> +	*val = lower_32_bits(acc_power);
> +	*val2 = upper_32_bits(acc_power);

ditto, iio_val_s64_decompose(acc_power, val, val2)?

> +
> +	return IIO_VAL_INT_64;
> +}
> +
> +static int max34417_setup_cam_mode(struct max34417_data *max34417, int val)
> +{
> +	int rc;
> +
> +	if (max34417->cam_mode && !val) {
> +		rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
> +				       MAX34417_CONTROL_CAM | MAX34417_CONTROL_SMM);
> +		if (rc)
> +			return rc;
> +
> +		/*
> +		 * If CAM mode must be set, the Control byte must be set to
> +		 * desired values with CAM = 0, then followed by an UPDATE
> +		 */
> +		rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
> +		if (rc)
> +			return rc;
> +	} else if (!max34417->cam_mode && val) {
> +		/*
> +		 * After this, the CAM mode bit can be set and another
> +		 * UPDATE must be sent.
> +		 */
> +		rc = regmap_clear_bits(max34417->regmap, MAX34417_CONTROL_REG,
> +				       MAX34417_CONTROL_SMM);
> +		if (rc)
> +			return rc;
> +		rc = regmap_set_bits(max34417->regmap, MAX34417_CONTROL_REG,
> +				     MAX34417_CONTROL_CAM);
> +		if (rc)
> +			return rc;
> +

Can the regmap_clear_bits() and regmap_set_bits() calls be
merged into one regmap_update_bits()?

> +		rc = regmap_write(max34417->regmap, MAX34417_UPDATE_REG, 1);
> +		if (rc)
> +			return rc;
> +
> +		/* Reset counters and accumulators */
> +		for (int i = 0; i < MAX34417_CHANNEL_COUNT; i++) {
> +			max34417->acc_power[i] = 0;
> +			max34417->acc_count[i] = 0;
> +		}
> +	}
> +
> +	max34417->cam_mode = val;
> +
> +	return 0;
> +}
> +
> +static int max34417_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct max34417_data *max34417 = iio_priv(indio_dev);
> +
> +	guard(mutex)(&max34417->lock);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			return max34417_read_voltage(max34417, chan, val);
> +		case IIO_POWER:
> +			/* In default mode, we return a single measurement */
> +			if (max34417->cam_mode)
> +				return -EINVAL;
> +
> +			return max34417_read_power(max34417, chan, val, val2);
> +		case IIO_ENERGY:
> +			if (!max34417->cam_mode)
> +				return -EINVAL;
> +
> +			return max34417_read_energy(max34417, chan, val, val2);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_CHAN_INFO_AVERAGE_RAW:
> +		if (chan->type != IIO_POWER)
> +			return -EINVAL;
> +
> +		/* In CAM mode, we return an average measurement */
> +		if (!max34417->cam_mode)
> +			return -EINVAL;
> +
> +		return max34417_read_power(max34417, chan, val, val2);
> +	case IIO_CHAN_INFO_SCALE:
> +		if (chan->type == IIO_VOLTAGE) {
> +			/* Scale to mV */
> +			*val = MAX34417_VOLTAGE_CORRECTION_SCALE * MILLI;
> +			*val2 = MAX34417_VOLTAGE_FULL_SCALE_BITS;
> +
> +			return IIO_VAL_FRACTIONAL_LOG2;
> +		} else if (chan->type == IIO_POWER) {
> +			/*
> +			 * Scale to mW
> +			 * (1000 * (Power Accumulation / Counter) * Correction) / 2^30
> +			 */
> +			*val = max34417->input_correction[chan->channel] * MILLIWATT_PER_WATT;
> +			*val2 = MAX34417_PWR_AVG_FULL_SCALE_BITS;
> +
> +			return IIO_VAL_FRACTIONAL_LOG2;
> +		} else if (chan->type == IIO_ENERGY) {
> +			/*
> +			 * Scale to mJ
> +			 * P=(1000 * (Power Accumulation / Counter) * Correction) / 2^30
> +			 * t=(Counter / Samples per Second)
> +			 * E=(P * t)
> +			 * Can be simplified to:
> +			 * (1000 * Power Accumulation * Correction) / (2^30 * SPS)
> +			 * The denominator doesn't fit in 32bit so scale down both terms.
> +			 */
> +			*val = max34417->input_correction[chan->channel];
> +			*val2 = div_u64(BIT_ULL(MAX34417_PWR_AVG_FULL_SCALE_BITS) *
> +					MAX34417_FAST_SPS_CAM, MILLI);
> +
> +			return IIO_VAL_FRACTIONAL;
> +		}
> +
> +		return -EINVAL;

Maybe make this a switch instead of an if-chain?

> +	case IIO_CHAN_INFO_ENABLE:
> +		*val = max34417->cam_mode;
> +
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +static int max34417_read_label(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       char *label)
> +{
> +	struct max34417_data *max34417 = iio_priv(indio_dev);
> +	const char *input_label = max34417->input_label[chan->channel];
> +
> +	if (input_label) {
> +		switch (chan->type) {
> +		case IIO_VOLTAGE:
> +			return sysfs_emit(label, "%s-voltage\n", input_label);
> +		case IIO_POWER:
> +			return sysfs_emit(label, "%s-power\n", input_label);
> +		case IIO_ENERGY:
> +			return sysfs_emit(label, "%s-energy\n", input_label);
> +		default:
> +			return 0;
> +		}
> +	}
> +
> +	switch (chan->type) {
> +	case IIO_VOLTAGE:
> +		return sysfs_emit(label, "channel%d-voltage\n", chan->channel);
> +	case IIO_POWER:
> +		return sysfs_emit(label, "channel%d-power\n", chan->channel);
> +	case IIO_ENERGY:
> +		return sysfs_emit(label, "channel%d-energy\n", chan->channel);
> +	default:
> +		return 0;
> +	}
> +}

Up to you if you prefer it, but an alternative to duplicating this switch
is to add the following near the start of max34417_probe():

max34417->input_label[0] = "channel0";
max34417->input_label[1] = "channel1";
max34417->input_label[2] = "channel2";
max34417->input_label[3] = "channel3";

> +
> +static const struct iio_info max34417_info = {
> +	.read_raw	= max34417_read_raw,
> +	.write_raw	= max34417_write_raw,
> +	.read_label	= max34417_read_label,
> +};
> +
> +static unsigned int max34417_calc_input_correction(u32 rsense_microOhm)

Spell the return type as u32, to match the parameter