* [PATCH v5 0/5] Add support for Microchip PAC1711 Power Monitor
@ 2026-11-03 14:37 Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 1/5] unaligned: introduce be56 unaligned accessors Ariana Lazar
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Ariana Lazar @ 2026-11-03 14:37 UTC (permalink / raw)
To: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea, Ariana Lazar, Andy Shevchenko
The PAC1711, PAC1721, PAC1811 and PAC1821 products are single-channel power
monitors with accumulator. The PAC1711 and PAC1721 devices use 12-bit
resolution for voltage and current measurements and 24 bits for power
calculations, while PAC1811 and PAC1821 have 16-bit resolution and use 32
bits for power calculations. The 56-bit accumulator register accumulates
power (energy) or current (Coulomb counter).
PAC1711 and PAC1811 measure up to 42V Full-Scale Range, respectively 9V for
PAC1721 and PAC1821.
Reasons why these devices are more suitable for the IIO subsystem:
1. Hardware capabilities and sampling rates:
The device is fundamentally a high-speed ADC with advanced hardware
features (e.g., measuring low voltage over a current shunt, calculate
power and/or acumulate power/current). When
configured to sample a single channel (e.g. current exclusively),
the PAC1711 can achieve sampling rates of up to 16,384 samples/second.
High-rate sampling is specifically required to enable accurate
power/current profiling.
2. Future Features:
The currently submitted version of the driver provides the basic
hardware interface that could be suitable for HWMON, but later on it is
needed to introduce features such as IIO buffer support. High-frequency
buffered data capture is essential for profiling high-resolution current
consumption on processors, peripherals, battery systems, and industrial
low-voltage automation.
3. Consistency:
It maintains a unified API across the entire Microchip power and current
monitor device family with similar hardware capabilities.
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
Changes in v5:
- add get_unaligned_be56() precursor patch in the series
- add iio_fill_val_int_64() precursor patch in the series
- use device_property_match_property_string() instead of strcmp()
- resolve coding style review comments
- cast units defines to int for negatives bounds
- resolve possible overflow in calculating energy scale
- calculate update time based on current sampling rate
- initialize device name when no device is identified
- cast available sampling rates to the type used default by
read_avail()
- resolve TOCTOU race from pac1711_set_acc_enable() by verifying firstly
that the new value is different from the one stored in struct
- Link to v4: https://lore.kernel.org/r/20261007-pac1711-v4-0-6ef34d7a7947@microchip.com
Changes in v4:
- fix review comments device tree binding:
move the reasoning for defining accumulation-source as a DT property
from the binding description to the commit message
- fix review comments driver:
add a new patch in the series to add IIO_CHARGE as a standard channel
type (instead of using custom attributes for Coulomb Counter)
remove chip_variant from pac1711_chip_info struct
remove TO_PAC1711_CHIP_INFO define
use reverse xmas and correct other coding style issues flagged
use dev_err_probe() instead of dev_err() in pac1711_chip_identify()
correct sample rate update
restructure pac1711_features struct to reflect hardware
differencies
rewrite defines using unit defines such as MILLI, MICRO
add comment to explain sample rate update wait time
add device_property_present() check where needed
add porposed function pac1711_get_features_from_id() to be used as a
helper in pac1711_chip_identify()
remove early return in pac1711_set_acc_enable()
protected 64-bit read operations with guards
- Link to v3: https://lore.kernel.org/r/20260909-pac1711-v3-0-dff81003b82f@microchip.com
Changes in v3:
- fix review comments device tree binding:
remove interrupts and GPIO properties
change VSENSEP to VSENSE+, VSENSEN to VSENSE-
explain why we need the accumulation mode as a devicetree property
- fix review comments driver:
use NANO macro in defines
use anonymous enums
rename is_pac18x1_family to has_16bit_resolution
use more meaningful names for local variables instead of tmp
use guard instead of scoped_guard where possible
replace 'Increasing precision' comment from
pac1711_in_coulomb_counter_scale_show()
rename pac1711_work_periodic_rfsh() to pac1711_work_periodic_refresh()
remove ret initialization from pac1711_chip_configure()
add explanation for background worker instead of polling data in a
comment
remove dynamic allocation of the always available channels
return 0 in the end of pac1711_retrieve_data()
remove comma after NULL entries
use FIELD_MODIFY() instead of FIELD_PREP()
use consistent one line style
verify length instead of return code for I2C reads
remove return error code when accumulator overflows
correct refresh logic and jiffies timestamp calculation
add mutex in pac1711_in_power_acc_raw_show()
add mutex when reading power raw values
correct wait_time calculation
initialize all struct fields when identifying the chip fails
handle return code correctly for i2c_smbus_read_i2c_block_data()
correct power scale calculation to avoid overflow
change vals type from s32 to u32 in pac1711_init_vbus_vsense_ranges()
use IIO_ENERGY instead of custom attribute for vpower accumulation
add more comments to explain accumulation functionality
validate I2C transfer lengths and return -EIO for short transfers
- Link to v2: https://lore.kernel.org/r/20260728-pac1711-v2-0-609bc026093c@microchip.com
Changes in v2:
- fix review comments device tree binding:
add PAC1721, PAC1811 and PAC1821 part numbers
add Vbus/Vsense input ranges in attribute definition
change accumulation-mode from int to string type
remove size and address cells
correct interrupts definition and add attributes for the two
alerts, microchip,gpio0-mode and microchip,gpio1-mode
remove microchip,gpio attribute
remove "vbus" accumulation mode
- fix review comments driver:
add PAC1721, PAC1811 and PAC1821 part numbers
run pahole on reg_data and pac1711_chip_info structs
remove average registers - VBUS_AVG and VSENSE_AVG
add PAC1721, PAC1811, PAC1821 to features/compatible
add missing headers
remove rarely used defines like PAC1711_POWER_24B_RES and use the
numerical value inline instead
use ARRAY_SIZE() instead of define for the number of accumulator
related attributes
add explanation for bytes length defines
use spacing convention space after { and before }
remove pac1711_shift_map_tbl in order to use just
pac1711_samp_rate_map_tbl and an index saved in struct
use read_avail() for sampling_rate
change mutex comment in reg_data
use fsleep instead of usleep
use a local __be16 variable in endianess transformations
add missing error returns after dev_err
add info_mask_shared_by_all for sampling_frequency
use dev_info instead of dev_err_probe in chip_identify
generalize input setup functions into one
remove device_property_present
rename pac1711_single_channel into pac1711_chan_spec
rename pac1711_of_parse_channel_config into pac1711_parse_fw
remove unneccessary comments in probe
define in_shunt_resistor as ext_info instead of custom attribute
use calculations only with 16-bit resolution instead of multiple
shifting (12-bit registers are left shifted)
add scale computations based on new voltage of 9V for PAC1721/PAC1821
v1:
- first version committed to review
- Link to v1: https://lore.kernel.org/r/20251015-pac1711-v1-0-976949e36367@microchip.com
---
Ariana Lazar (4):
iio: core: add IIO_CHARGE channel type
iio: core: add iio_fill_val_int_64() helper
dt-bindings: iio: adc: add support for PAC1711
iio: adc: add support for PAC1711
Marius Cristea (1):
unaligned: introduce be56 unaligned accessors
Documentation/ABI/testing/sysfs-bus-iio | 13 +
.../bindings/iio/adc/microchip,pac1711.yaml | 169 +++
MAINTAINERS | 7 +
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/pac1711.c | 1233 ++++++++++++++++++++
drivers/iio/industrialio-core.c | 1 +
include/linux/iio/iio.h | 15 +
include/linux/unaligned.h | 27 +
include/uapi/linux/iio/types.h | 1 +
tools/iio/iio_event_monitor.c | 2 +
11 files changed, 1480 insertions(+)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20250901-pac1711-d3bacda400fd
Best regards,
--
Ariana Lazar <ariana.lazar@microchip.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 1/5] unaligned: introduce be56 unaligned accessors
2026-11-03 14:37 [PATCH v5 0/5] Add support for Microchip PAC1711 Power Monitor Ariana Lazar
@ 2026-11-03 14:37 ` Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 2/5] iio: core: add IIO_CHARGE channel type Ariana Lazar
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Ariana Lazar @ 2026-11-03 14:37 UTC (permalink / raw)
To: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea, Ariana Lazar, Andy Shevchenko
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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---
include/linux/unaligned.h | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/include/linux/unaligned.h b/include/linux/unaligned.h
index 4a9651017e3cb9f407994ef55973908b6917b5af..2105743b1800f1a8c3f498e939e55e4ab0521aa4 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.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 2/5] iio: core: add IIO_CHARGE channel type
2026-11-03 14:37 [PATCH v5 0/5] Add support for Microchip PAC1711 Power Monitor Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 1/5] unaligned: introduce be56 unaligned accessors Ariana Lazar
@ 2026-11-03 14:37 ` Ariana Lazar
2026-11-03 16:13 ` Andy Shevchenko
2026-11-03 14:37 ` [PATCH v5 3/5] iio: core: add iio_fill_val_int_64() helper Ariana Lazar
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ariana Lazar @ 2026-11-03 14:37 UTC (permalink / raw)
To: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea, Ariana Lazar
Add a new channel type for Coulomb counters, which accumulate current
measurements over time. The sysfs attribute is in_chargeY_raw and after
applying in_chargeY_scale, the value exposed is in milliCoulombs,
following the SI convention used by the IIO subsystem.
This new channel type will first be used by the pac1711 driver to report
Coulomb counter measurements.
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
Documentation/ABI/testing/sysfs-bus-iio | 13 +++++++++++++
drivers/iio/industrialio-core.c | 1 +
include/uapi/linux/iio/types.h | 1 +
tools/iio/iio_event_monitor.c | 2 ++
4 files changed, 17 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index a4f5595722ade77e81b49efffc68c971e60a2e5b..3d569a2169c2100b3fc81d38bd63124fb8b67e84 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -510,6 +510,8 @@ What: /sys/bus/iio/devices/iio:deviceX/in_intensity_blue_scale
What: /sys/bus/iio/devices/iio:deviceX/in_concentration_co2_scale
What: /sys/bus/iio/devices/iio:deviceX/in_volumeflow_scale
What: /sys/bus/iio/devices/iio:deviceX/in_volumeflowY_scale
+What: /sys/bus/iio/devices/iio:deviceX/in_charge_scale
+What: /sys/bus/iio/devices/iio:deviceX/in_chargeY_scale
KernelVersion: 2.6.35
Contact: linux-iio@vger.kernel.org
Description:
@@ -1781,6 +1783,7 @@ What: /sys/.../iio:deviceX/in_energy_en
What: /sys/.../iio:deviceX/in_distance_en
What: /sys/.../iio:deviceX/in_velocity_sqrt(x^2+y^2+z^2)_en
What: /sys/.../iio:deviceX/in_steps_en
+What: /sys/.../iio:deviceX/in_charge_en
KernelVersion: 3.19
Contact: linux-iio@vger.kernel.org
Description:
@@ -2509,3 +2512,13 @@ Description:
Raw (unscaled) volumetric flow rate reading from the channel.
To convert to standard units (cubic metres per second, m^3/s)
apply the channel's _scale (and _offset, when present).
+
+What: /sys/bus/iio/devices/iio:deviceX/in_charge_raw
+What: /sys/bus/iio/devices/iio:deviceX/in_chargeY_raw
+KernelVersion: 7.4
+Contact: linux-iio@vger.kernel.org
+Description:
+ Raw charge value read from the channel. The device acts as
+ a Coulomb counter by accumulating current measurements over
+ time. Units after application of scale are milliCoulombs. X
+ is the IIO index of the device.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 767a7794624a5026742087eb8885eb2a5ded05f0..90c92e57e8605d2ab443541b88944dab7084babc 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -100,6 +100,7 @@ static const char * const iio_chan_type_name_spec[] = {
[IIO_ALTCURRENT] = "altcurrent",
[IIO_COVERAGE] = "coverage",
[IIO_VOLUMEFLOW] = "volumeflow",
+ [IIO_CHARGE] = "charge",
};
static const char * const iio_modifier_names[] = {
diff --git a/include/uapi/linux/iio/types.h b/include/uapi/linux/iio/types.h
index d0fb714ca0fd97421d2c8345e3bc34329f1cb1b9..8acf4105216c11718c07f76ac67b93a8ba7fd6a0 100644
--- a/include/uapi/linux/iio/types.h
+++ b/include/uapi/linux/iio/types.h
@@ -55,6 +55,7 @@ enum iio_chan_type {
IIO_ALTCURRENT,
IIO_COVERAGE,
IIO_VOLUMEFLOW,
+ IIO_CHARGE,
};
enum iio_modifier {
diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
index b5c5479b9d78f70cab9d6939ceaab32e42b3f718..50c12395767c0c91916eabc420c62631e834a4e5 100644
--- a/tools/iio/iio_event_monitor.c
+++ b/tools/iio/iio_event_monitor.c
@@ -67,6 +67,7 @@ static const char * const iio_chan_type_name_spec[] = {
[IIO_ALTCURRENT] = "altcurrent",
[IIO_COVERAGE] = "coverage",
[IIO_VOLUMEFLOW] = "volumeflow",
+ [IIO_CHARGE] = "charge",
};
static const char * const iio_ev_type_text[] = {
@@ -198,6 +199,7 @@ static bool event_is_known(struct iio_event_data *event)
case IIO_ALTCURRENT:
case IIO_COVERAGE:
case IIO_VOLUMEFLOW:
+ case IIO_CHARGE:
break;
default:
return false;
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 3/5] iio: core: add iio_fill_val_int_64() helper
2026-11-03 14:37 [PATCH v5 0/5] Add support for Microchip PAC1711 Power Monitor Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 1/5] unaligned: introduce be56 unaligned accessors Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 2/5] iio: core: add IIO_CHARGE channel type Ariana Lazar
@ 2026-11-03 14:37 ` Ariana Lazar
2026-11-03 14:42 ` sashiko-bot
2026-11-03 16:14 ` Andy Shevchenko
2026-11-03 14:37 ` [PATCH v5 4/5] dt-bindings: iio: adc: add support for PAC1711 Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 5/5] " Ariana Lazar
4 siblings, 2 replies; 11+ messages in thread
From: Ariana Lazar @ 2026-11-03 14:37 UTC (permalink / raw)
To: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea, Ariana Lazar
Add iio_fill_val_int_64() to populate the val and val2 output parameters
with the lower and upper 32 bits of a 64-bit value, as used for
IIO_VAL_INT_64. The helper wraps lower_32_bits() and upper_32_bits() in
order to split the value with a single call.
The pac1711 driver will be the first user of this function.
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
include/linux/iio/iio.h | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 711c00f67371356c913ef0dbf2325e3f4d35266d..d8fd300a125d676bf60e0b59f84a69f99ff23c89 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -1099,6 +1099,21 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals);
int iio_str_to_fixpoint(const char *str, int fract_mult, int *integer,
int *fract);
+/**
+ * iio_fill_val_int_64() - Fill the value fields used to represent a 64-bit integer
+ * @value: The 64-bit value to be represented
+ * @val: the lower 32 bits of @value
+ * @val2: the upper 32 bits of @value
+ *
+ * Populate @val and @val2 with the lower and upper halves of @value, as
+ * expected by IIO_VAL_INT_64.
+ */
+static inline void iio_fill_val_int_64(s64 value, int *val, int *val2)
+{
+ *val = lower_32_bits(value);
+ *val2 = upper_32_bits(value);
+}
+
/**
* IIO_DEGREE_TO_RAD() - Convert degree to rad
* @deg: A value in degree
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 4/5] dt-bindings: iio: adc: add support for PAC1711
2026-11-03 14:37 [PATCH v5 0/5] Add support for Microchip PAC1711 Power Monitor Ariana Lazar
` (2 preceding siblings ...)
2026-11-03 14:37 ` [PATCH v5 3/5] iio: core: add iio_fill_val_int_64() helper Ariana Lazar
@ 2026-11-03 14:37 ` Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 5/5] " Ariana Lazar
4 siblings, 0 replies; 11+ messages in thread
From: Ariana Lazar @ 2026-11-03 14:37 UTC (permalink / raw)
To: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea, Ariana Lazar
This is the device tree schema for Microchip PAC1711, PAC1721, PAC1811 and
PAC1821 single-channel power monitor with accumulator. The PAC1711 and
PAC1721 devices use 12-bit resolution for voltage and current measurements
and 24 bits for power calculations, while PAC1811 and PAC1821 have 16-bit
resolution and use 32 bits for power calculations. The 56-bit accumulator
register accumulates power (energy) or current (Coulomb counter).
PAC1711 and PAC1811 measure up to 42V Full-Scale Range, respectively 9V for
PAC1721 and PAC1821.
The accumulation-source property aims to specify what kind of hardware is
intended to be used/available for the user. There are two main cases here:
- the user wants to measure also the current/power consumed before the
driver insertion (e.g. from the boot to user control) and if this is a
runtime setting, the hardware accumulator will be reset by the default
configuration the driver starts with.
- the driver does not know what type of hardware it's dealing with. In
case the part is monitoring the charge/discharge current it does not
make sense in user-space to change the accumulator to calculate energy.
Same if the hardware is intended to calculate energy it does not make
sense in user-space to change to Coulomb counter. Changing the setting
from one mode to another will reset the hardware accumulator inside the
chip.
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
.../bindings/iio/adc/microchip,pac1711.yaml | 169 +++++++++++++++++++++
MAINTAINERS | 6 +
2 files changed, 175 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..1ddf8a485c9c3e56dc96f82394c137259e00f541
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
@@ -0,0 +1,169 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/microchip,pac1711.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PAC1711 Power Monitors with Accumulator
+
+maintainers:
+ - Ariana Lazar <ariana.lazar@microchip.com>
+
+description: |
+ This device is part of the Microchip family of Power Monitors with Accumulator.
+ Datasheet links:
+ [PAC1711]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/PAC1711-Data-Sheet-DS20007058.pdf
+ [PAC1721]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/PAC1721-Single-Channel-Power-Monitor-with-Accumulator-DS20007088.pdf
+ [PAC1811]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC1811-Data-Sheet-DS20007066.pdf
+ [PAC1821]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC1821-Data-Sheet-DS20007097.pdf
+
+ PAC1711, PAC1721, PAC1811 and PAC1821 are Single Channel Power Monitors
+ with Accumulator, having 12-bit or 16-bit resolution. The devices PAC1711
+ and PAC1811 can measure up to 42V Full-Scale Range, respectively 9V
+ Full-Scale Range for PAC1721 and PAC1821.
+
+properties:
+ compatible:
+ enum:
+ - microchip,pac1711
+ - microchip,pac1721
+ - microchip,pac1811
+ - microchip,pac1821
+
+ reg:
+ maxItems: 1
+
+ vdd-supply: true
+
+ "#io-channel-cells":
+ const: 1
+
+ shunt-resistor-micro-ohms:
+ description:
+ Value in micro Ohms of the shunt resistor connected between the VSENSE+
+ and VSENSE- inputs, across which the current is measured. This value is
+ needed to compute the scaling of the measured current.
+
+ label:
+ description: Unique name to identify which device this is.
+
+ microchip,vbus-input-range-microvolt:
+ description: |
+ Specifies the voltage range in microvolts chosen for the voltage full
+ scale range (FSR). The range should be set as <minimum, maximum> by
+ hardware design and should not be changed during runtime.
+
+ The VBUS could be configured into the following full scale range:
+ - for PAC1711 or PAC1811:
+ - VBUS has unipolar 0V to 42V FSR (default)
+ - VBUS has bipolar -42V to 42V FSR
+ - VBUS has bipolar -21V to 21V FSR
+ - for PAC1721 or PAC1821:
+ - VBUS has unipolar 0V to 9V FSR (default)
+ - VBUS has bipolar -9V to 9V FSR
+ - VBUS has bipolar -4.5V to 4.5V FSR
+
+ microchip,vsense-input-range-microvolt:
+ description: |
+ Specifies the voltage range in microvolts chosen for the current full
+ scale range (FSR). The current is calculated by dividing the vsense
+ voltage by the value of the shunt resistor. The range should be set as
+ <minimum, maximum> by hardware design and it should not be changed
+ during runtime.
+
+ The VSENSE could be configured into the following full scale range:
+ - VSENSE has unipolar 0 mV to 100 mV FSR (default)
+ - VSENSE has bipolar -100 mV to 100 mV FSR
+ - VSENSE has bipolar -50 mV to 50 mV FSR
+ oneOf:
+ - items:
+ - const: 0
+ - const: 100000
+ - items:
+ - const: -100000
+ - const: 100000
+ - items:
+ - const: -50000
+ - const: 50000
+
+ microchip,accumulation-source:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: |
+ The Hardware Accumulator may be used to accumulate VPOWER or VSENSE values
+ for any channel. By setting the accumulator for a channel to accumulate
+ the VPOWER values gives a measure of accumulated power over a time period,
+ which is equivalent to energy. Setting the accumulator for a channel to
+ accumulate VSENSE values gives a measure of accumulated current, which is
+ equivalent to charge.
+
+ The Hardware Accumulator could be configured as:
+ "energy" - Accumulator accumulates VPOWER (energy)
+ "charge" - Accumulator accumulates VSENSE (Coulomb Counter)
+ enum: [energy, charge]
+ default: energy
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+ - shunt-resistor-micro-ohms
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ pattern: "^microchip,pac1[78]11$"
+ then:
+ properties:
+ microchip,vbus-input-range-microvolt:
+ oneOf:
+ - items:
+ - const: 0
+ - const: 42000000
+ - items:
+ - const: -42000000
+ - const: 42000000
+ - items:
+ - const: -21000000
+ - const: 21000000
+ default: [0, 42000000]
+ - if:
+ properties:
+ compatible:
+ pattern: "^microchip,pac1[78]21$"
+ then:
+ properties:
+ microchip,vbus-input-range-microvolt:
+ oneOf:
+ - items:
+ - const: 0
+ - const: 9000000
+ - items:
+ - const: -9000000
+ - const: 9000000
+ - items:
+ - const: -4500000
+ - const: 4500000
+ default: [0, 9000000]
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ power-monitor@40 {
+ compatible = "microchip,pac1711";
+ reg = <0x40>;
+
+ shunt-resistor-micro-ohms = <11000000>;
+ label = "VDD3V3";
+ vdd-supply = <&vdd>;
+ microchip,vbus-input-range-microvolt = <(-21000000) 21000000>;
+ microchip,vsense-input-range-microvolt = <(-50000) 50000>;
+ microchip,accumulation-source = "energy";
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 3a19da74d00c9dfb48e900df71a86d4686d3ec2c..ebba9b0b3f834576b422b96283f9a3cc79405c3a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17889,6 +17889,12 @@ F: Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
F: drivers/nvmem/microchip-otpc.c
F: include/dt-bindings/nvmem/microchip,sama7g5-otpc.h
+MICROCHIP PAC1711 POWER/CURRENT MONITOR DRIVER
+M: Ariana Lazar <ariana.lazar@microchip.com>
+L: linux-iio@vger.kernel.org
+S: Supported
+F: Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
+
MICROCHIP PAC1921 POWER/CURRENT MONITOR DRIVER
M: Matteo Martelli <matteomartelli3@gmail.com>
L: linux-iio@vger.kernel.org
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v5 5/5] iio: adc: add support for PAC1711
2026-11-03 14:37 [PATCH v5 0/5] Add support for Microchip PAC1711 Power Monitor Ariana Lazar
` (3 preceding siblings ...)
2026-11-03 14:37 ` [PATCH v5 4/5] dt-bindings: iio: adc: add support for PAC1711 Ariana Lazar
@ 2026-11-03 14:37 ` Ariana Lazar
2026-11-03 14:59 ` sashiko-bot
2026-11-03 16:40 ` Andy Shevchenko
4 siblings, 2 replies; 11+ messages in thread
From: Ariana Lazar @ 2026-11-03 14:37 UTC (permalink / raw)
To: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea, Ariana Lazar
This is the iio driver for Microchip PAC1711, PAC1721, PAC1811 and
PAC1821 single-channel power monitors with accumulator. The PAC1711 and
PAC1721 devices use 12-bit resolution for voltage and current measurements
and 24 bits for power calculations, while PAC1811 and PAC1821 have 16-bit
resolution and use 32 bits for power calculations. The 56-bit accumulator
register accumulates power (energy) or current (Coulomb counter).
PAC1711 and PAC1811 measure up to 42V Full-Scale Range, respectively 9V for
PAC1721 and PAC1821.
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
MAINTAINERS | 1 +
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/pac1711.c | 1233 +++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 1246 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index ebba9b0b3f834576b422b96283f9a3cc79405c3a..1b2c69a4456a3e155277b1e8593e618e2862d714 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17894,6 +17894,7 @@ M: Ariana Lazar <ariana.lazar@microchip.com>
L: linux-iio@vger.kernel.org
S: Supported
F: Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
+F: drivers/iio/adc/pac1711.c
MICROCHIP PAC1921 POWER/CURRENT MONITOR DRIVER
M: Matteo Martelli <matteomartelli3@gmail.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 415e519ad4eb48c136174e7faf35a1fe6d46d80a..8b51e0165d5ae6f732a05bdaee4085e8f71673d2 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1320,6 +1320,17 @@ config NXP_SAR_ADC
This driver can also be built as a module. If so, the module will be
called nxp_sar_adc.
+config PAC1711
+ tristate "Microchip Technology PAC1711 driver"
+ depends on I2C
+ help
+ Say yes here to build support for Microchip Technology's PAC1711,
+ PAC1721, PAC1811 and PAC1821 Single-Channel Power Monitors with
+ Accumulator.
+
+ This driver can also be built as a module. If so, the module
+ will be called pac1711.
+
config PAC1921
tristate "Microchip Technology PAC1921 driver"
depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index dcec0abb03b73b994382d7de335670f4cf2d14b3..33487568f0c138033a482ac9b01b274790d1e469 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -114,6 +114,7 @@ obj-$(CONFIG_NAU7802) += nau7802.o
obj-$(CONFIG_NCT7201) += nct7201.o
obj-$(CONFIG_NPCM_ADC) += npcm_adc.o
obj-$(CONFIG_NXP_SAR_ADC) += nxp-sar-adc.o
+obj-$(CONFIG_PAC1711) += pac1711.o
obj-$(CONFIG_PAC1921) += pac1921.o
obj-$(CONFIG_PAC1934) += pac1934.o
obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
diff --git a/drivers/iio/adc/pac1711.c b/drivers/iio/adc/pac1711.c
new file mode 100644
index 0000000000000000000000000000000000000000..6adc37a7ba0be882397a15b749bd9b1a657a96d6
--- /dev/null
+++ b/drivers/iio/adc/pac1711.c
@@ -0,0 +1,1233 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IIO driver for PAC1711 Single-Channel DC Power/Energy Monitor
+ *
+ * Copyright (C) 2025 Microchip Technology Inc. and its subsidiaries
+ *
+ * Author: Ariana Lazar <ariana.lazar@microchip.com>
+ *
+ * Datasheet links:
+ * [PAC1711]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/PAC1711-Data-Sheet-DS20007058.pdf
+ * [PAC1721]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/PAC1721-Single-Channel-Power-Monitor-with-Accumulator-DS20007088.pdf
+ * [PAC1811]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC1811-Data-Sheet-DS20007066.pdf
+ * [PAC1821]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC1821-Data-Sheet-DS20007097.pdf
+ */
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/jiffies.h>
+#include <linux/kstrtox.h>
+#include <linux/module.h>
+#include <linux/math64.h>
+#include <linux/mutex.h>
+#include <linux/overflow.h>
+#include <linux/property.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+#include <linux/workqueue.h>
+#include <asm/byteorder.h>
+
+#include <linux/iio/iio.h>
+
+/*
+ * Maximum accumulation time should be 1,165 hours at 1,024 sps
+ * till PAC1711 accumulation registers starts to saturate
+ */
+#define PAC1711_MAX_RFSH_LIMIT_MS (60 * MSEC_PER_SEC)
+/* 50msec is the timeout for validity of the cached registers */
+#define PAC1711_MIN_POLLING_TIME_MS 50
+
+#define PAC1711_VOLTAGE_MILLIVOLTS_MAX (42 * MILLI)
+#define PAC1721_VOLTAGE_MILLIVOLTS_MAX (9 * MILLI)
+
+/* Full FSR power is multiplied by 10^9 to ensure precise scale calculation. */
+#define PAC1711_PRODUCT_VOLTAGE_PV_FSR (4200ULL * NANO)
+#define PAC1721_PRODUCT_VOLTAGE_PV_FSR (900ULL * NANO)
+
+/* PAC1711/PAC1811 VBUS FSR: 42V and 21V */
+#define PAC1711_VBUS_FSR_UV (42 * MICRO)
+#define PAC1711_VBUS_FSR_HALF_UV (21 * MICRO)
+
+/* PAC1721/PAC1821 VBUS FSR: 9V and 4.5V */
+#define PAC1721_VBUS_FSR_UV (9 * MICRO)
+#define PAC1721_VBUS_FSR_HALF_UV (4500 * MILLI)
+
+/* VSENSE FSR: 100mV and 50mV */
+#define PAC1711_VSENSE_FSR_UV (100 * MILLI)
+#define PAC1711_VSENSE_FSR_HALF_UV (50 * MILLI)
+
+/* I2C address map */
+#define PAC1711_REFRESH_REG_ADDR 0x00
+
+/* Writable register, but needs refresh */
+#define PAC1711_CTRL_REG_ADDR 0x01
+#define PAC1711_CTRL_SAMPLE_MODE_MASK GENMASK(15, 12)
+#define PAC1711_CTRL_ACC_MODE_MASK GENMASK(3, 2)
+#define PAC1711_ACC_COUNT_REG_ADDR 0x02
+#define PAC1711_VACC_REG_ADDR 0x03
+#define PAC1711_VBUS_REG_ADDR 0x04
+#define PAC1711_VSENSE_REG_ADDR 0x05
+#define PAC1711_VPOWER_REG_ADDR 0x08
+
+/* Saves the settings that were active prior to the most recent refresh (any) command */
+#define PAC1711_CTRL_LAT_REG_ADDR 0x0F
+#define PAC1711_NEG_PWR_FSR_REG_ADDR 0x13
+#define PAC1711_NEG_PWR_FSR_VS_MASK GENMASK(3, 2)
+#define PAC1711_NEG_PWR_FSR_VB_MASK GENMASK(1, 0)
+#define PAC1711_SLOW_REG_ADDR 0x16
+
+/* Read-only register which stores currently active settings. */
+#define PAC1711_CTRL_ACT_REG_ADDR 0x17
+
+#define PAC1711_PID_REG_ADDR 0xFD
+
+/* Dimension of each register in bytes */
+#define PAC1711_ACC_REG_LEN 4
+#define PAC1711_VACC_REG_LEN 7
+#define PAC1711_VBUS_SENSE_REG_LEN 2
+
+/*
+ * The sum of the measurement registers' dimensions in bytes - from ACC_COUNT to
+ * VPOWER in datasheet register description.
+ */
+#define PAC1711_MEAS_REG_SNAPSHOT_LEN 23
+
+#define PAC1711_ACC_ENERGY_STR "energy"
+#define PAC1711_ACC_CHARGE_STR "charge"
+
+#define PAC1711_PRODUCT_ID_1711 0x80
+#define PAC1711_PRODUCT_ID_1721 0x81
+#define PAC1711_PRODUCT_ID_1811 0x84
+#define PAC1711_PRODUCT_ID_1821 0x85
+
+enum {
+ PAC1711_ACCMODE_VPOWER = 0,
+ PAC1711_ACCMODE_VSENSE = 1,
+};
+
+enum {
+ PAC1711_FULL_RANGE_UNIPOLAR = 0,
+ PAC1711_FULL_RANGE_BIPOLAR = 1,
+ PAC1711_HALF_RANGE_BIPOLAR = 2,
+};
+
+enum {
+ PAC1711_VOLTAGE_RANGE_IDX = 0,
+ PAC1721_VOLTAGE_RANGE_IDX = 1,
+};
+
+static const char * const pac1711_acc_sources[] = {
+ [PAC1711_ACCMODE_VPOWER] = PAC1711_ACC_ENERGY_STR,
+ [PAC1711_ACCMODE_VSENSE] = PAC1711_ACC_CHARGE_STR,
+};
+
+static const int pac1711_vbus_range_tbl[2][3][2] = {
+ [PAC1711_VOLTAGE_RANGE_IDX] = {
+ [PAC1711_FULL_RANGE_UNIPOLAR] = { 0, PAC1711_VBUS_FSR_UV },
+ [PAC1711_FULL_RANGE_BIPOLAR] = { -(int)PAC1711_VBUS_FSR_UV, PAC1711_VBUS_FSR_UV },
+ [PAC1711_HALF_RANGE_BIPOLAR] = { -(int)PAC1711_VBUS_FSR_HALF_UV,
+ PAC1711_VBUS_FSR_HALF_UV },
+ },
+ [PAC1721_VOLTAGE_RANGE_IDX] = {
+ [PAC1711_FULL_RANGE_UNIPOLAR] = { 0, PAC1721_VBUS_FSR_UV },
+ [PAC1711_FULL_RANGE_BIPOLAR] = { -(int)PAC1721_VBUS_FSR_UV, PAC1721_VBUS_FSR_UV },
+ [PAC1711_HALF_RANGE_BIPOLAR] = { -(int)PAC1721_VBUS_FSR_HALF_UV,
+ PAC1721_VBUS_FSR_HALF_UV },
+ },
+};
+
+static const int pac1711_vsense_range_tbl[3][2] = {
+ [PAC1711_FULL_RANGE_UNIPOLAR] = { 0, PAC1711_VSENSE_FSR_UV },
+ [PAC1711_FULL_RANGE_BIPOLAR] = { -(int)PAC1711_VSENSE_FSR_UV, PAC1711_VSENSE_FSR_UV },
+ [PAC1711_HALF_RANGE_BIPOLAR] = { -(int)PAC1711_VSENSE_FSR_HALF_UV,
+ PAC1711_VSENSE_FSR_HALF_UV },
+};
+
+enum {
+ PAC1711_SAMP_8192SPS = 0,
+ PAC1711_SAMP_4096SPS = 1,
+ PAC1711_SAMP_1024SPS = 2,
+ PAC1711_SAMP_256SPS = 3,
+ PAC1711_SAMP_64SPS = 4,
+ PAC1711_SAMP_8SPS = 5,
+};
+
+static const int pac1711_samp_rate_map_tbl[] = {
+ [PAC1711_SAMP_8192SPS] = 8192,
+ [PAC1711_SAMP_4096SPS] = 4096,
+ [PAC1711_SAMP_1024SPS] = 1024, /* Default */
+ [PAC1711_SAMP_256SPS] = 256,
+ [PAC1711_SAMP_64SPS] = 64,
+ [PAC1711_SAMP_8SPS] = 8,
+};
+
+static const unsigned int pac1711_shift_map_tbl[] = {
+ [PAC1711_SAMP_8192SPS] = 13,
+ [PAC1711_SAMP_4096SPS] = 12,
+ [PAC1711_SAMP_1024SPS] = 10,
+ [PAC1711_SAMP_256SPS] = 8,
+ [PAC1711_SAMP_64SPS] = 6,
+ [PAC1711_SAMP_8SPS] = 3,
+};
+
+/**
+ * struct pac1711_features - features of a pac1711 instance
+ * @voltage_pv_fsr: Maximum power-product value for FSR
+ * @voltage_millivolts_max: Scale maximum for millivolts
+ * @name: chip's name
+ * @voltage_range_idx: Voltage range based on part number
+ * @has_16bit_resolution: true if device is part of the PAC18x1 family
+ */
+struct pac1711_features {
+ u64 voltage_pv_fsr;
+ int voltage_millivolts_max;
+ const char *name;
+ u8 voltage_range_idx;
+ bool has_16bit_resolution;
+};
+
+static const struct pac1711_features pac1711_chip_features = {
+ .name = "pac1711",
+ .has_16bit_resolution = false,
+ .voltage_range_idx = PAC1711_VOLTAGE_RANGE_IDX,
+ .voltage_pv_fsr = PAC1711_PRODUCT_VOLTAGE_PV_FSR,
+ .voltage_millivolts_max = PAC1711_VOLTAGE_MILLIVOLTS_MAX,
+};
+
+static const struct pac1711_features pac1721_chip_features = {
+ .name = "pac1721",
+ .has_16bit_resolution = false,
+ .voltage_range_idx = PAC1721_VOLTAGE_RANGE_IDX,
+ .voltage_pv_fsr = PAC1721_PRODUCT_VOLTAGE_PV_FSR,
+ .voltage_millivolts_max = PAC1721_VOLTAGE_MILLIVOLTS_MAX,
+};
+
+static const struct pac1711_features pac1811_chip_features = {
+ .name = "pac1811",
+ .has_16bit_resolution = true,
+ .voltage_range_idx = PAC1711_VOLTAGE_RANGE_IDX,
+ .voltage_pv_fsr = PAC1711_PRODUCT_VOLTAGE_PV_FSR,
+ .voltage_millivolts_max = PAC1711_VOLTAGE_MILLIVOLTS_MAX,
+};
+
+static const struct pac1711_features pac1821_chip_features = {
+ .name = "pac1821",
+ .has_16bit_resolution = true,
+ .voltage_range_idx = PAC1721_VOLTAGE_RANGE_IDX,
+ .voltage_pv_fsr = PAC1721_PRODUCT_VOLTAGE_PV_FSR,
+ .voltage_millivolts_max = PAC1721_VOLTAGE_MILLIVOLTS_MAX,
+};
+
+/**
+ * struct pac1711_reg_data - data from the registers
+ * @vacc: accumulated vpower or vsense value
+ * @acc_val: accumulated values per second
+ * @vpower: vpower registers
+ * @vsense: vsense registers
+ * @vbus: vbus registers
+ * @acc_count: the acc_count register
+ * @jiffies_tstamp: timestamp
+ * @ctrl_act_reg: the ctrl_act register
+ * @ctrl_lat_reg: the ctrl_lat register
+ * @meas_regs: snapshot of raw measurements registers
+ */
+struct pac1711_reg_data {
+ s64 vacc;
+ s64 acc_val;
+ s64 vpower;
+ s32 vsense;
+ s32 vbus;
+ u32 acc_count;
+ unsigned long jiffies_tstamp;
+ u16 ctrl_act_reg;
+ u16 ctrl_lat_reg;
+ u8 meas_regs[PAC1711_MEAS_REG_SNAPSHOT_LEN];
+};
+
+/**
+ * struct pac1711_chip_info - information about the chip
+ * @chip_reg_data: measurement/control/accumulator output device registers
+ * @iio_info: device information
+ * @features: device features pointer
+ * @client: the I2C client attached to the device
+ * @work_chip_refresh: work queue used for refresh commands
+ * @lock: synchronize access to driver's state members
+ * @shunt: shunt resistor value
+ * @vbus_mode: Full Scale Range (FSR) mode for VBus
+ * @vsense_mode: Full Scale Range (FSR) mode for VSense
+ * @accumulation_mode: accumulation mode for hardware accumulator
+ * @sample_rate_idx: sampling frequency index
+ * @enable_acc: true means that accumulation channel is enabled
+ */
+struct pac1711_chip_info {
+ struct pac1711_reg_data chip_reg_data;
+ struct iio_info iio_info;
+ const struct pac1711_features *features;
+ struct i2c_client *client;
+ struct delayed_work work_chip_refresh;
+ /* Prevents concurrent writes into control, voltage measurement or accumulator registers. */
+ struct mutex lock;
+ u32 shunt;
+ u8 vbus_mode;
+ u8 vsense_mode;
+ u8 accumulation_mode;
+ u8 sample_rate_idx;
+ bool enable_acc;
+};
+
+static int pac1711_send_refresh(struct pac1711_chip_info *info, u32 wait_time)
+{
+ struct i2c_client *client = info->client;
+ int ret;
+
+ /* Writing a REFRESH or a REFRESH_V command */
+ ret = i2c_smbus_write_byte(client, PAC1711_REFRESH_REG_ADDR);
+ if (ret) {
+ dev_err(&client->dev, "%s - cannot send Refresh cmd (0x%02X)\n",
+ __func__, PAC1711_REFRESH_REG_ADDR);
+ return ret;
+ }
+
+ /* Register data retrieval timestamp */
+ info->chip_reg_data.jiffies_tstamp = jiffies;
+
+ /* Wait till the data is available */
+ fsleep(wait_time);
+
+ return 0;
+}
+
+static int pac1711_reg_snapshot_locked(struct pac1711_chip_info *info, u32 wait_time)
+{
+ struct i2c_client *client = info->client;
+ struct device *dev = &client->dev;
+ u8 *offset_reg_data_p;
+ bool is_bipolar;
+ __be16 val_be16;
+ u16 val_u16;
+ s64 inc = 0;
+ int ret;
+
+ /* Force a snapshot of the internal accumulator and measurements into the registers. */
+ ret = pac1711_send_refresh(info, wait_time);
+ if (ret < 0) {
+ dev_err(dev, "cannot send refresh\n");
+ return ret;
+ }
+
+ /* Read the ctrl/status registers for this snapshot */
+ ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_ACT_REG_ADDR,
+ sizeof(val_be16), (u8 *)&val_be16);
+ if (ret != sizeof(val_be16)) {
+ dev_err(dev, "%s - cannot read regs from 0x%02X\n",
+ __func__, PAC1711_CTRL_ACT_REG_ADDR);
+ return ret < 0 ? ret : -EIO;
+ }
+
+ info->chip_reg_data.ctrl_act_reg = be16_to_cpu(val_be16);
+
+ ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_LAT_REG_ADDR,
+ sizeof(val_be16), (u8 *)&val_be16);
+ if (ret != sizeof(val_be16)) {
+ dev_err(dev, "%s - cannot read regs from 0x%02X\n",
+ __func__, PAC1711_CTRL_LAT_REG_ADDR);
+ return ret < 0 ? ret : -EIO;
+ }
+
+ info->chip_reg_data.ctrl_lat_reg = be16_to_cpu(val_be16);
+
+ /* Read the data registers */
+ ret = i2c_smbus_read_i2c_block_data(client, PAC1711_ACC_COUNT_REG_ADDR,
+ PAC1711_MEAS_REG_SNAPSHOT_LEN,
+ (u8 *)info->chip_reg_data.meas_regs);
+ if (ret != PAC1711_MEAS_REG_SNAPSHOT_LEN) {
+ dev_err(dev, "%s - cannot read regs from 0x%02X\n",
+ __func__, PAC1711_ACC_COUNT_REG_ADDR);
+ return ret < 0 ? ret : -EIO;
+ }
+
+ offset_reg_data_p = &info->chip_reg_data.meas_regs[0];
+ info->chip_reg_data.acc_count = get_unaligned_be32(offset_reg_data_p);
+ offset_reg_data_p += PAC1711_ACC_REG_LEN;
+
+ /* skip if the energy/charge accumulation is disabled */
+ if (info->enable_acc) {
+ info->chip_reg_data.vacc = get_unaligned_be56(offset_reg_data_p);
+ is_bipolar = false;
+
+ switch (info->accumulation_mode) {
+ case PAC1711_ACCMODE_VPOWER:
+ if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR ||
+ info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR)
+ is_bipolar = true;
+ break;
+ case PAC1711_ACCMODE_VSENSE:
+ if (info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR)
+ is_bipolar = true;
+ break;
+ }
+
+ if (is_bipolar)
+ info->chip_reg_data.vacc = sign_extend64(info->chip_reg_data.vacc, 55);
+
+ /*
+ * Integrate the accumulated power or current over
+ * the elapsed interval.
+ */
+ val_u16 = FIELD_GET(PAC1711_CTRL_SAMPLE_MODE_MASK,
+ info->chip_reg_data.ctrl_lat_reg);
+
+ if (val_u16 <= PAC1711_SAMP_8SPS) {
+ inc = info->chip_reg_data.vacc >> pac1711_shift_map_tbl[val_u16];
+ } else {
+ dev_err(dev, "Invalid sample rate index: %d!\n", val_u16);
+ return -EINVAL;
+ }
+
+ /* Handle 64-bit overflow by saturating at limits. */
+ if (check_add_overflow(info->chip_reg_data.acc_val, inc,
+ &info->chip_reg_data.acc_val)) {
+ if (inc < 0)
+ info->chip_reg_data.acc_val = S64_MIN;
+ else
+ info->chip_reg_data.acc_val = S64_MAX;
+
+ dev_err(dev, "Accumulator Overflow detected!\n");
+ }
+ }
+
+ offset_reg_data_p += PAC1711_VACC_REG_LEN;
+
+ /* VBUS */
+ info->chip_reg_data.vbus = get_unaligned_be16(offset_reg_data_p);
+
+ if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR)
+ info->chip_reg_data.vbus = sign_extend32(info->chip_reg_data.vbus, 15);
+
+ offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN;
+
+ /* VSENSE */
+ info->chip_reg_data.vsense = get_unaligned_be16(offset_reg_data_p);
+
+ if (info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR)
+ info->chip_reg_data.vsense = sign_extend32(info->chip_reg_data.vsense, 15);
+
+ /* Skip VBUS_AVG and VSENSE_AVG registers */
+ offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN * 3;
+
+ /* VPOWER */
+ info->chip_reg_data.vpower = get_unaligned_be32(offset_reg_data_p);
+
+ if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR ||
+ info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR)
+ info->chip_reg_data.vpower = sign_extend64(info->chip_reg_data.vpower, 31);
+
+ return 0;
+}
+
+static int pac1711_reg_snapshot(struct pac1711_chip_info *info, bool *refreshed)
+{
+ u32 wait_time;
+ int ret;
+
+ guard(mutex)(&info->lock);
+
+ /* No hardware refresh is needed */
+ if (refreshed)
+ *refreshed = false;
+
+ /* Return early if the minimum polling time hasn't elapsed */
+ if (!time_after(jiffies, info->chip_reg_data.jiffies_tstamp +
+ msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS)))
+ return 0;
+
+ wait_time = (1024 * 1000) / pac1711_samp_rate_map_tbl[info->sample_rate_idx];
+
+ /* A hardware refresh will happen */
+ ret = pac1711_reg_snapshot_locked(info, wait_time);
+ if (!ret && refreshed)
+ *refreshed = true;
+
+ return ret;
+}
+
+static int pac1711_retrieve_data(struct pac1711_chip_info *info)
+{
+ bool refreshed = false;
+ int ret;
+
+ /*
+ * Restart the background timer to begin a new accumulation window if an
+ * actual hardware refresh took place.
+ */
+ ret = pac1711_reg_snapshot(info, &refreshed);
+ if (ret < 0)
+ return ret;
+
+ if (refreshed)
+ mod_delayed_work(system_percpu_wq, &info->work_chip_refresh,
+ msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS));
+
+ return 0;
+}
+
+static int pac1711_get_samp_rate_idx(u32 new_samp_rate)
+{
+ for (unsigned int cnt = 0; cnt < ARRAY_SIZE(pac1711_samp_rate_map_tbl); cnt++)
+ if (new_samp_rate == pac1711_samp_rate_map_tbl[cnt])
+ return cnt;
+
+ return -EINVAL;
+}
+
+static ssize_t pac1711_read_shunt_resistor(struct iio_dev *indio_dev, uintptr_t private,
+ const struct iio_chan_spec *ch, char *buf)
+{
+ struct pac1711_chip_info *info = iio_priv(indio_dev);
+
+ guard(mutex)(&info->lock);
+
+ return sysfs_emit(buf, "%u\n", info->shunt);
+}
+
+static int pac1711_set_acc_enable(struct pac1711_chip_info *info, int val)
+{
+ struct i2c_client *client = info->client;
+ struct device *dev = &client->dev;
+ u32 wait_time, sample_rate;
+ int ret = 0;
+
+ if (val != 0 && val != 1)
+ return -EINVAL;
+
+ guard(mutex)(&info->lock);
+
+ if (val == info->enable_acc)
+ return 0;
+
+ /*
+ * Force a hardware refresh to zero out the internal accumulators
+ * and start a new measurement window, as the chip accumulates
+ * continuously in the background.
+ */
+ if (val) {
+ sample_rate = pac1711_samp_rate_map_tbl[info->sample_rate_idx];
+ wait_time = (1024 * 1000) / sample_rate;
+ ret = pac1711_send_refresh(info, wait_time);
+ } else {
+ info->chip_reg_data.acc_val = 0;
+ info->chip_reg_data.vacc = 0;
+ info->chip_reg_data.acc_count = 0;
+ }
+
+ if (!ret)
+ info->enable_acc = val;
+
+ if (ret)
+ dev_err(dev, "failed to reset accumulator: %d\n", ret);
+
+ return ret;
+}
+
+/*
+ * The value of the shunt resistor may be known only at runtime and set by a client
+ * application. This attribute allows to set its value in micro-ohms. Y is the channel
+ * number. The value is used to calculate current, power and accumulated energy or
+ * Coulomb counter.
+ */
+static ssize_t pac1711_write_shunt_resistor(struct iio_dev *indio_dev, uintptr_t private,
+ const struct iio_chan_spec *ch, const char *buf,
+ size_t len)
+{
+ struct pac1711_chip_info *info = iio_priv(indio_dev);
+ struct device *dev = &info->client->dev;
+ unsigned int sh_val;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &sh_val);
+ if (ret) {
+ dev_err(dev, "Shunt value is not valid\n");
+ return ret;
+ }
+
+ if (sh_val == 0)
+ return -EINVAL;
+
+ guard(mutex)(&info->lock);
+
+ info->shunt = sh_val;
+
+ return len;
+}
+
+static const struct iio_chan_spec_ext_info pac1711_ext_info[] = {
+ {
+ .name = "in_shunt_resistor",
+ .read = pac1711_read_shunt_resistor,
+ .write = pac1711_write_shunt_resistor,
+ .shared = IIO_SHARED_BY_ALL,
+ },
+ { }
+};
+
+#define PAC1711_VBUS_CHANNEL(_index, _address) { \
+ .type = IIO_VOLTAGE, \
+ .address = (_address), \
+ .indexed = 1, \
+ .channel = (_index), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .ext_info = pac1711_ext_info, \
+}
+
+#define PAC1711_VSENSE_CHANNEL(_index, _address) { \
+ .type = IIO_CURRENT, \
+ .address = (_address), \
+ .indexed = 1, \
+ .channel = (_index), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .ext_info = pac1711_ext_info, \
+}
+
+#define PAC1711_VPOWER_CHANNEL(_index, _address) { \
+ .type = IIO_POWER, \
+ .address = (_address), \
+ .indexed = 1, \
+ .channel = (_index), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .ext_info = pac1711_ext_info, \
+}
+
+#define PAC1711_ACC_CHANNEL(_index, _address, _type) { \
+ .type = (_type), \
+ .address = (_address), \
+ .indexed = 1, \
+ .channel = (_index), \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \
+ BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_ENABLE), \
+ .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+ .ext_info = pac1711_ext_info, \
+}
+
+static int pac1711_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct pac1711_chip_info *info = iio_priv(indio_dev);
+ int ret, shift;
+ u64 tmp = 0;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ ret = pac1711_retrieve_data(info);
+ if (ret)
+ return ret;
+
+ switch (chan->type) {
+ case IIO_VOLTAGE: {
+ guard(mutex)(&info->lock);
+ *val = info->chip_reg_data.vbus;
+ return IIO_VAL_INT;
+ }
+ case IIO_CURRENT: {
+ guard(mutex)(&info->lock);
+ *val = info->chip_reg_data.vsense;
+ return IIO_VAL_INT;
+ }
+ case IIO_POWER: {
+ guard(mutex)(&info->lock);
+ iio_fill_val_int_64(info->chip_reg_data.vpower, val, val2);
+ return IIO_VAL_INT_64;
+ }
+ case IIO_ENERGY:
+ case IIO_CHARGE: {
+ guard(mutex)(&info->lock);
+ iio_fill_val_int_64(info->chip_reg_data.acc_val, val, val2);
+ return IIO_VAL_INT_64;
+ }
+ default:
+ return -EINVAL;
+ }
+ case IIO_CHAN_INFO_SCALE: {
+ u32 shunt;
+
+ guard(mutex)(&info->lock);
+ shunt = info->shunt;
+
+ switch (chan->address) {
+ case PAC1711_VBUS_REG_ADDR:
+ *val = info->features->voltage_millivolts_max;
+ *val2 = (info->vbus_mode == PAC1711_FULL_RANGE_BIPOLAR) ? 15 : 16;
+
+ return IIO_VAL_FRACTIONAL_LOG2;
+ case PAC1711_VSENSE_REG_ADDR:
+ /*
+ * Currents - scale for mA - depends on the channel's shunt value
+ * (100mV * 1000000) / (2^16 * shunt(uohm))
+ */
+ *val = 1526;
+ *val2 = shunt;
+
+ if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
+ *val = *val << 1;
+
+ return IIO_VAL_FRACTIONAL;
+ case PAC1711_VPOWER_REG_ADDR:
+ case PAC1711_VACC_REG_ADDR:
+ if (chan->type == IIO_CHARGE) {
+ /*
+ * Coulomb counter scale:
+ * 16-bit resolution: (100mV * NANO) / 2^16
+ * 12-bit resolution: (100mV * NANO) / 2^12
+ */
+ u64 ref = info->features->has_16bit_resolution ?
+ 1525878906250ULL : 24414062500000ULL;
+
+ if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
+ ref = ref << 1;
+
+ ref = div_u64(ref, shunt);
+ *val2 = do_div(ref, 1 * NANO);
+ *val = ref;
+
+ return IIO_VAL_INT_PLUS_NANO;
+ }
+
+ /*
+ * Power - uW - it will use the combined scale
+ * for current and voltage
+ * current(mA) * voltage(mV) = power (uW)
+ */
+ tmp = info->features->voltage_pv_fsr;
+ do_div(tmp, shunt);
+
+ if (chan->type == IIO_ENERGY)
+ *val2 = info->features->has_16bit_resolution ? 32 : 24;
+ else
+ *val2 = 32;
+
+ if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
+ *val2 -= 1;
+
+ if (info->vbus_mode == PAC1711_FULL_RANGE_BIPOLAR)
+ *val2 -= 1;
+
+ shift = fls64(tmp) - 31;
+ if (shift > 0) {
+ tmp >>= shift;
+ *val2 -= shift;
+ }
+
+ *val = (int)tmp;
+
+ return IIO_VAL_FRACTIONAL_LOG2;
+ default:
+ return -EINVAL;
+ }
+ }
+ case IIO_CHAN_INFO_SAMP_FREQ: {
+ guard(mutex)(&info->lock);
+ *val = pac1711_samp_rate_map_tbl[info->sample_rate_idx];
+ return IIO_VAL_INT;
+ }
+ case IIO_CHAN_INFO_ENABLE: {
+ guard(mutex)(&info->lock);
+ *val = info->enable_acc;
+ return IIO_VAL_INT;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static int pac1711_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct pac1711_chip_info *info = iio_priv(indio_dev);
+ struct i2c_client *client = info->client;
+ struct device *dev = &info->client->dev;
+ int new_idx, ret, refresh_time;
+ s32 old_samp_rate;
+ __be16 val_be16;
+ u32 wait_time;
+ u16 val_u16;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ scoped_guard(mutex, &info->lock) {
+ old_samp_rate = pac1711_samp_rate_map_tbl[info->sample_rate_idx];
+ new_idx = pac1711_get_samp_rate_idx(val);
+ if (new_idx < 0)
+ return new_idx;
+
+ ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_ACT_REG_ADDR,
+ sizeof(val_u16), (u8 *)&val_be16);
+ if (ret != sizeof(val_u16)) {
+ dev_err(&client->dev, "cannot read regs from 0x%02X\n",
+ PAC1711_CTRL_ACT_REG_ADDR);
+ return ret < 0 ? ret : -EIO;
+ }
+
+ val_u16 = be16_to_cpu(val_be16);
+ FIELD_MODIFY(PAC1711_CTRL_SAMPLE_MODE_MASK, &val_u16, new_idx);
+ val_be16 = cpu_to_be16(val_u16);
+ ret = i2c_smbus_write_i2c_block_data(client, PAC1711_CTRL_REG_ADDR,
+ sizeof(val_be16), (u8 *)&val_be16);
+ if (ret) {
+ dev_err(dev, "Failed to configure sampling mode in 0x%02X\n",
+ PAC1711_CTRL_ACT_REG_ADDR);
+ return ret;
+ }
+
+ info->sample_rate_idx = new_idx;
+ info->chip_reg_data.ctrl_act_reg = val_u16;
+
+ /* Force register snapshot and timestamp update with a refresh. */
+ refresh_time = msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS) + 1;
+ info->chip_reg_data.jiffies_tstamp = jiffies - refresh_time;
+ wait_time = (1024 * 1000) / old_samp_rate;
+
+ /*
+ * We must wait for cycle completion before updating the
+ * sample rate value.
+ */
+ ret = pac1711_reg_snapshot_locked(info, wait_time);
+ if (ret) {
+ dev_err(dev, "%s - cannot snapshot ctrl and measurement regs\n",
+ __func__);
+ return ret;
+ }
+ }
+
+ return 0;
+ case IIO_CHAN_INFO_ENABLE:
+ return pac1711_set_acc_enable(info, val);
+ default:
+ return -EINVAL;
+ }
+}
+
+static void pac1711_work_periodic_refresh(struct work_struct *work)
+{
+ struct delayed_work *dwork = to_delayed_work(work);
+ struct pac1711_chip_info *info = container_of(dwork, struct pac1711_chip_info,
+ work_chip_refresh);
+ struct device *dev = &info->client->dev;
+ int ret;
+
+ dev_dbg(dev, "%s - Periodic refresh\n", __func__);
+
+ /* Do a REFRESH, then read */
+ ret = pac1711_reg_snapshot(info, NULL);
+ if (ret)
+ dev_err(dev, "Refresh failed: %d\n", ret);
+
+ schedule_delayed_work(&info->work_chip_refresh,
+ msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS));
+}
+
+static const struct pac1711_features *pac1711_get_features_from_id(u8 id)
+{
+ switch (id) {
+ case PAC1711_PRODUCT_ID_1711:
+ return &pac1711_chip_features;
+ case PAC1711_PRODUCT_ID_1721:
+ return &pac1721_chip_features;
+ case PAC1711_PRODUCT_ID_1811:
+ return &pac1811_chip_features;
+ case PAC1711_PRODUCT_ID_1821:
+ return &pac1821_chip_features;
+ default:
+ return NULL;
+ }
+}
+
+static int pac1711_chip_identify(struct iio_dev *indio_dev, struct pac1711_chip_info *info)
+{
+ struct i2c_client *client = info->client;
+ struct device *dev = &client->dev;
+ u8 chip_rev_info[3] = { };
+ int ret;
+
+ ret = i2c_smbus_read_i2c_block_data(client, PAC1711_PID_REG_ADDR,
+ sizeof(chip_rev_info), chip_rev_info);
+ if (ret != sizeof(chip_rev_info)) {
+ ret = ret < 0 ? ret : -EIO;
+ return dev_err_probe(dev, ret, "product ID not recognized\n");
+ }
+
+ info->features = pac1711_get_features_from_id(chip_rev_info[0]);
+ if (!info->features)
+ return -ENODEV;
+
+ indio_dev->name = info->features->name;
+
+ return 0;
+}
+
+static int pac1711_check_range(s32 *vals, bool is_vbus, unsigned int voltage_range_idx)
+{
+ int num_ranges = ARRAY_SIZE(pac1711_vbus_range_tbl[PAC1711_VOLTAGE_RANGE_IDX]);
+ const int (*ranges)[3][2];
+
+ if (is_vbus) {
+ switch (voltage_range_idx) {
+ case PAC1711_VOLTAGE_RANGE_IDX:
+ ranges = &pac1711_vbus_range_tbl[PAC1711_VOLTAGE_RANGE_IDX];
+ break;
+ case PAC1721_VOLTAGE_RANGE_IDX:
+ ranges = &pac1711_vbus_range_tbl[PAC1721_VOLTAGE_RANGE_IDX];
+ break;
+ default:
+ return -EINVAL;
+ }
+ } else {
+ ranges = &pac1711_vsense_range_tbl;
+ }
+
+ for (unsigned int i = 0; i < num_ranges; i++) {
+ if (vals[0] == (*ranges)[i][0] && vals[1] == (*ranges)[i][1])
+ return i;
+ }
+
+ return -EINVAL;
+}
+
+static int pac1711_init_vbus_vsense_ranges(struct pac1711_chip_info *info, bool is_vbus)
+{
+ struct i2c_client *client = info->client;
+ struct device *dev = &client->dev;
+ const char *prop_name;
+ u32 raw_vals[2];
+ s32 vals[2];
+ int ret;
+
+ if (is_vbus)
+ prop_name = "microchip,vbus-input-range-microvolt";
+ else
+ prop_name = "microchip,vsense-input-range-microvolt";
+
+ if (device_property_present(dev, prop_name)) {
+ ret = device_property_read_u32_array(dev, prop_name, raw_vals, 2);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Cannot read %s property\n", prop_name);
+
+ /*
+ * For bipolar FSR the lower limit is a negative number. Since
+ * firmware properties are read as u32 arrays, cast them to
+ * s32 is required to correctly handle these signed values.
+ */
+ vals[0] = (s32)raw_vals[0];
+ vals[1] = (s32)raw_vals[1];
+ ret = pac1711_check_range(vals, is_vbus, info->features->voltage_range_idx);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Invalid value %d, %d for prop %s\n",
+ vals[0], vals[1], prop_name);
+ } else {
+ ret = PAC1711_FULL_RANGE_UNIPOLAR;
+ }
+
+ if (is_vbus)
+ info->vbus_mode = ret;
+ else
+ info->vsense_mode = ret;
+
+ return 0;
+}
+
+static int pac1711_parse_fw(struct i2c_client *client, struct pac1711_chip_info *info)
+{
+ struct device *dev = &client->dev;
+ int ret;
+
+ ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms", &info->shunt);
+ if (ret)
+ return dev_err_probe(dev, ret, "Shunt resistor property error\n");
+
+ if (!info->shunt)
+ return dev_err_probe(dev, -EINVAL, "Invalid value for shunt resistor\n");
+
+ ret = pac1711_init_vbus_vsense_ranges(info, true);
+ if (ret)
+ return ret;
+
+ ret = pac1711_init_vbus_vsense_ranges(info, false);
+ if (ret)
+ return ret;
+
+ if (device_property_present(dev, "microchip,accumulation-source")) {
+ ret = device_property_match_property_string(dev, "microchip,accumulation-source",
+ pac1711_acc_sources,
+ ARRAY_SIZE(pac1711_acc_sources));
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Invalid accumulation-source value\n");
+
+ info->accumulation_mode = ret;
+ } else {
+ info->accumulation_mode = PAC1711_ACCMODE_VPOWER;
+ }
+
+ dev_dbg(dev, "Accumulation source set to: %s\n",
+ pac1711_acc_sources[info->accumulation_mode]);
+
+ return 0;
+}
+
+static void pac1711_cancel_delayed_work(void *dwork)
+{
+ cancel_delayed_work_sync(dwork);
+}
+
+static int pac1711_chip_configure(struct pac1711_chip_info *info)
+{
+ struct i2c_client *client = info->client;
+ struct device *dev = &client->dev;
+ __be16 val_be16;
+ u32 wait_time;
+ u16 val_u16;
+ u8 val_u8;
+ int ret;
+
+ /*
+ * The current/voltage can be measured unidirectional, bidirectional or half FSR
+ * no SLOW triggered REFRESH, clear POR
+ */
+ val_u8 = FIELD_PREP(PAC1711_NEG_PWR_FSR_VS_MASK, info->vsense_mode) |
+ FIELD_PREP(PAC1711_NEG_PWR_FSR_VB_MASK, info->vbus_mode);
+
+ ret = i2c_smbus_write_byte_data(client, PAC1711_NEG_PWR_FSR_REG_ADDR, val_u8);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n",
+ PAC1711_NEG_PWR_FSR_REG_ADDR);
+
+ ret = i2c_smbus_write_byte_data(client, PAC1711_SLOW_REG_ADDR, 0);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n", PAC1711_SLOW_REG_ADDR);
+
+ /* Get sampling rate from PAC */
+ ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_REG_ADDR,
+ sizeof(val_u16), (u8 *)&val_be16);
+ if (ret != sizeof(val_u16))
+ return dev_err_probe(dev, ret < 0 ? ret : -EIO,
+ "cannot read 0x%02X reg\n", PAC1711_CTRL_REG_ADDR);
+
+ val_u16 = be16_to_cpu(val_be16);
+ info->sample_rate_idx = FIELD_GET(PAC1711_CTRL_SAMPLE_MODE_MASK, val_u16);
+ if (info->sample_rate_idx >= ARRAY_SIZE(pac1711_samp_rate_map_tbl)) {
+ /*
+ * Use default sample rate in case the chip is configured with an sample
+ * rate unsupported by the driver. The Control Register is updated.
+ */
+ info->sample_rate_idx = PAC1711_SAMP_1024SPS;
+ FIELD_MODIFY(PAC1711_CTRL_SAMPLE_MODE_MASK, &val_u16, info->sample_rate_idx);
+ }
+
+ /* Configure the accumulation mode */
+ FIELD_MODIFY(PAC1711_CTRL_ACC_MODE_MASK, &val_u16, info->accumulation_mode);
+ val_be16 = cpu_to_be16(val_u16);
+ ret = i2c_smbus_write_i2c_block_data(client, PAC1711_CTRL_REG_ADDR,
+ sizeof(val_be16), (u8 *)&val_be16);
+ if (ret)
+ return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n", PAC1711_CTRL_REG_ADDR);
+
+ /*
+ * Sending a REFRESH to the chip, so the new settings take place
+ * as well as resetting the accumulators
+ */
+ ret = i2c_smbus_write_byte(client, PAC1711_REFRESH_REG_ADDR);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n",
+ PAC1711_REFRESH_REG_ADDR);
+
+ /*
+ * Get the current (in the chip) sampling speed and compute the
+ * required timeout based on its value the timeout is 1/sampling_speed
+ * wait the maximum amount of time to be on the safe side - the
+ * maximum wait time is for 8sps
+ */
+ wait_time = ((1024 * 1000) / pac1711_samp_rate_map_tbl[info->sample_rate_idx]);
+ fsleep(wait_time);
+
+ /*
+ * VACC and ACC_COUNT registers increment continuously in hardware and do not
+ * roll over, so without a periodic Refresh (any) command before the worst-case
+ * overflow time is reached, the accumulated data would be silently lost.
+ *
+ * A periodic refresh works regardless of board pin configuration, while
+ * A0/A1 pins are optional (shared with SLOW/GPIO functions and may not be
+ * wired as ALERT on every board).
+ */
+ INIT_DELAYED_WORK(&info->work_chip_refresh, pac1711_work_periodic_refresh);
+ /* Setup the latest moment for reading the regs before saturation */
+ schedule_delayed_work(&info->work_chip_refresh,
+ msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS));
+
+ return devm_add_action_or_reset(&client->dev, pac1711_cancel_delayed_work,
+ &info->work_chip_refresh);
+}
+
+static const struct iio_chan_spec pac1711_energy_chan_spec[] = {
+ PAC1711_VBUS_CHANNEL(0, PAC1711_VBUS_REG_ADDR),
+ PAC1711_VSENSE_CHANNEL(0, PAC1711_VSENSE_REG_ADDR),
+ PAC1711_VPOWER_CHANNEL(0, PAC1711_VPOWER_REG_ADDR),
+ PAC1711_ACC_CHANNEL(0, PAC1711_VACC_REG_ADDR, IIO_ENERGY)
+};
+
+static const struct iio_chan_spec pac1711_charge_chan_spec[] = {
+ PAC1711_VBUS_CHANNEL(0, PAC1711_VBUS_REG_ADDR),
+ PAC1711_VSENSE_CHANNEL(0, PAC1711_VSENSE_REG_ADDR),
+ PAC1711_VPOWER_CHANNEL(0, PAC1711_VPOWER_REG_ADDR),
+ PAC1711_ACC_CHANNEL(0, PAC1711_VACC_REG_ADDR, IIO_CHARGE)
+};
+
+static int pac1711_setup_channels(struct iio_dev *indio_dev, struct pac1711_chip_info *info)
+{
+ switch (info->accumulation_mode) {
+ case PAC1711_ACCMODE_VPOWER:
+ indio_dev->channels = pac1711_energy_chan_spec;
+ indio_dev->num_channels = ARRAY_SIZE(pac1711_energy_chan_spec);
+ return 0;
+ case PAC1711_ACCMODE_VSENSE:
+ indio_dev->channels = pac1711_charge_chan_spec;
+ indio_dev->num_channels = ARRAY_SIZE(pac1711_charge_chan_spec);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int pac1711_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *channel,
+ const int **vals, int *type, int *length, long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *type = IIO_VAL_INT;
+ *vals = pac1711_samp_rate_map_tbl;
+ *length = ARRAY_SIZE(pac1711_samp_rate_map_tbl);
+ return IIO_AVAIL_LIST;
+ }
+
+ return -EINVAL;
+}
+
+static const struct iio_info pac1711_info = {
+ .read_raw = pac1711_read_raw,
+ .write_raw = pac1711_write_raw,
+ .read_avail = pac1711_read_avail,
+};
+
+static int pac1711_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct pac1711_chip_info *info;
+ struct iio_dev *indio_dev;
+ u32 wait_time;
+ int ret, err;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ info = iio_priv(indio_dev);
+ info->client = client;
+
+ ret = pac1711_chip_identify(indio_dev, info);
+ if (ret == -ENODEV) {
+ /*
+ * If it fails to identify the hardware based on internal
+ * registers, use compatible from devicetree.
+ */
+ info->features = i2c_get_match_data(client);
+ if (!info->features)
+ return -EINVAL;
+
+ indio_dev->name = info->features->name;
+ } else if (ret) {
+ return dev_err_probe(&client->dev, ret, "Failed to identify chip\n");
+ }
+
+ /* Always start with accumulation channels enabled. */
+ info->enable_acc = true;
+
+ err = pac1711_parse_fw(client, info);
+ if (err)
+ return dev_err_probe(dev, err, "Error parsing devicetree data\n");
+
+ ret = devm_mutex_init(dev, &info->lock);
+ if (ret)
+ return ret;
+
+ ret = pac1711_chip_configure(info);
+ if (ret)
+ return ret;
+
+ info->iio_info = pac1711_info;
+ indio_dev->info = &info->iio_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ret = pac1711_setup_channels(indio_dev, info);
+ if (ret)
+ return dev_err_probe(dev, ret, "Invalid channel configuration\n");
+
+ /* Read what has been accumulated in the chip so far and reset the accumulators. */
+ wait_time = (1024 * 1000) / pac1711_samp_rate_map_tbl[info->sample_rate_idx];
+ scoped_guard(mutex, &info->lock) {
+ ret = pac1711_reg_snapshot_locked(info, wait_time);
+ if (ret)
+ return ret;
+ }
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "Can't register IIO device\n");
+
+ return 0;
+}
+
+static const struct i2c_device_id pac1711_id[] = {
+ { .name = "pac1711", .driver_data = (kernel_ulong_t)&pac1711_chip_features },
+ { .name = "pac1721", .driver_data = (kernel_ulong_t)&pac1721_chip_features },
+ { .name = "pac1811", .driver_data = (kernel_ulong_t)&pac1811_chip_features },
+ { .name = "pac1821", .driver_data = (kernel_ulong_t)&pac1821_chip_features },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, pac1711_id);
+
+static const struct of_device_id pac1711_of_match[] = {
+ { .compatible = "microchip,pac1711", .data = &pac1711_chip_features },
+ { .compatible = "microchip,pac1721", .data = &pac1721_chip_features },
+ { .compatible = "microchip,pac1811", .data = &pac1811_chip_features },
+ { .compatible = "microchip,pac1821", .data = &pac1821_chip_features },
+ { }
+};
+MODULE_DEVICE_TABLE(of, pac1711_of_match);
+
+static struct i2c_driver pac1711_driver = {
+ .driver = {
+ .name = "pac1711",
+ .of_match_table = pac1711_of_match,
+ },
+ .probe = pac1711_probe,
+ .id_table = pac1711_id,
+};
+module_i2c_driver(pac1711_driver);
+
+MODULE_AUTHOR("Ariana Lazar <ariana.lazar@microchip.com>");
+MODULE_DESCRIPTION("IIO driver for PAC1711 DC Power Monitor with Accumulator");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/5] iio: core: add iio_fill_val_int_64() helper
2026-11-03 14:37 ` [PATCH v5 3/5] iio: core: add iio_fill_val_int_64() helper Ariana Lazar
@ 2026-11-03 14:42 ` sashiko-bot
2026-11-03 16:14 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-11-03 14:42 UTC (permalink / raw)
To: Ariana Lazar; +Cc: devicetree, linux-hwmon, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] iio: core: iio_fill_val_int_64() duplicates iio_val_s64_decompose()
--
commit 9e24103c43e24f77958f3d94346bd3b90fbffa99
Author: Ariana Lazar <ariana.lazar@microchip.com>
iio: core: add iio_fill_val_int_64() helper
This commit adds the iio_fill_val_int_64() inline helper to
include/linux/iio/iio.h to split a 64-bit integer into lower and upper 32-bit
output parameters for IIO_VAL_INT_64.
> diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> index 711c00f673713..d8fd300a125d6 100644
> --- a/include/linux/iio/iio.h
> +++ b/include/linux/iio/iio.h
> @@ -1099,6 +1099,21 @@ ssize_t iio_format_value(char *buf, unsigned int type, int size, int *vals);
> int iio_str_to_fixpoint(const char *str, int fract_mult, int *integer,
> int *fract);
>
> +/**
> + * iio_fill_val_int_64() - Fill the value fields used to represent a 64-bit integer
> + * @value: The 64-bit value to be represented
> + * @val: the lower 32 bits of @value
> + * @val2: the upper 32 bits of @value
> + *
> + * Populate @val and @val2 with the lower and upper halves of @value, as
> + * expected by IIO_VAL_INT_64.
> + */
> +static inline void iio_fill_val_int_64(s64 value, int *val, int *val2)
> +{
> + *val = lower_32_bits(value);
> + *val2 = upper_32_bits(value);
> +}
[Severity: Low]
Does iio_fill_val_int_64() in include/linux/iio/iio.h duplicate the existing
iio_val_s64_decompose() helper in include/linux/iio/types.h?
include/linux/iio/types.h (which is already included by
include/linux/iio/iio.h) defines:
static inline void iio_val_s64_decompose(s64 dec64, s32 *val0, s32 *val1)
{
*val0 = lower_32_bits(dec64);
*val1 = upper_32_bits(dec64);
}
iio_val_s64_decompose() is already used for IIO_VAL_INT_64 in the IIO core
(drivers/iio/industrialio-core.c), KUnit format tests
(iio_test_iio_format_value_integer_64() in drivers/iio/test/iio-test-format.c),
and existing IIO drivers (such as drivers/iio/flow/slf3s.c,
drivers/iio/frequency/adf41513.c, and drivers/iio/adc/ti-ads112c14.c) to
populate the read_raw int *val and int *val2 output parameters.
Could callers such as pac1711 use iio_val_s64_decompose() directly instead of
introducing a second helper for the same operation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-pac1711-v5-0-71c20101a52e@microchip.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 5/5] iio: adc: add support for PAC1711
2026-11-03 14:37 ` [PATCH v5 5/5] " Ariana Lazar
@ 2026-11-03 14:59 ` sashiko-bot
2026-11-03 16:40 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-11-03 14:59 UTC (permalink / raw)
To: Ariana Lazar; +Cc: devicetree, linux-hwmon, robh, conor+dt
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] iio: adc: pac1711: wrong power and energy scale units in pac1711_read_raw()
- [High] iio: adc: pac1711: missing vdd-supply regulator enable in pac1711_probe()
- [Medium] iio: adc: pac1711: in_shunt_resistor uses micro-ohms instead of Ohms
- [Medium] iio: adc: pac1711: signed int overflow of shunt in pac1711_read_raw()
- [Medium] iio: adc: pac1711: premature jiffies_tstamp update in pac1711_send_refresh()
- [Low] iio: adc: pac1711: premature sample_rate_idx update in pac1711_write_raw()
--
commit c4f8dfb4bdee1e8ce1711a47ff2fad6880c4a9a3
Author: Ariana Lazar <ariana.lazar@microchip.com>
iio: adc: add support for PAC1711
This commit adds an IIO ADC driver for the Microchip PAC1711, PAC1721,
PAC1811, and PAC1821 single-channel power and current monitors with hardware
accumulators.
> diff --git a/drivers/iio/adc/pac1711.c b/drivers/iio/adc/pac1711.c
> new file mode 100644
> index 0000000000000..6adc37a7ba0be
> --- /dev/null
> +++ b/drivers/iio/adc/pac1711.c
> @@ -0,0 +1,1233 @@
[ ... ]
> +/* Full FSR power is multiplied by 10^9 to ensure precise scale calculation. */
> +#define PAC1711_PRODUCT_VOLTAGE_PV_FSR (4200ULL * NANO)
> +#define PAC1721_PRODUCT_VOLTAGE_PV_FSR (900ULL * NANO)
[ ... ]
> +static int pac1711_send_refresh(struct pac1711_chip_info *info, u32 wait_time)
> +{
> + struct i2c_client *client = info->client;
> + int ret;
> +
> + /* Writing a REFRESH or a REFRESH_V command */
> + ret = i2c_smbus_write_byte(client, PAC1711_REFRESH_REG_ADDR);
> + if (ret) {
> + dev_err(&client->dev, "%s - cannot send Refresh cmd (0x%02X)\n",
> + __func__, PAC1711_REFRESH_REG_ADDR);
> + return ret;
> + }
> +
> + /* Register data retrieval timestamp */
> + info->chip_reg_data.jiffies_tstamp = jiffies;
> +
> + /* Wait till the data is available */
> + fsleep(wait_time);
> +
> + return 0;
> +}
[ ... ]
> +static int pac1711_reg_snapshot(struct pac1711_chip_info *info, bool *refreshed)
> +{
> + u32 wait_time;
> + int ret;
> +
> + guard(mutex)(&info->lock);
> +
> + /* No hardware refresh is needed */
> + if (refreshed)
> + *refreshed = false;
> +
> + /* Return early if the minimum polling time hasn't elapsed */
> + if (!time_after(jiffies, info->chip_reg_data.jiffies_tstamp +
> + msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS)))
> + return 0;
> +
> + wait_time = (1024 * 1000) / pac1711_samp_rate_map_tbl[info->sample_rate_idx];
> +
> + /* A hardware refresh will happen */
> + ret = pac1711_reg_snapshot_locked(info, wait_time);
> + if (!ret && refreshed)
> + *refreshed = true;
> +
> + return ret;
> +}
[ ... ]
> +static ssize_t pac1711_read_shunt_resistor(struct iio_dev *indio_dev, uintptr_t private,
> + const struct iio_chan_spec *ch, char *buf)
> +{
> + struct pac1711_chip_info *info = iio_priv(indio_dev);
> +
> + guard(mutex)(&info->lock);
> +
> + return sysfs_emit(buf, "%u\n", info->shunt);
> +}
> +
> +static int pac1711_set_acc_enable(struct pac1711_chip_info *info, int val)
> +{
> + struct i2c_client *client = info->client;
> + struct device *dev = &client->dev;
> + u32 wait_time, sample_rate;
> + int ret = 0;
> +
> + if (val != 0 && val != 1)
> + return -EINVAL;
> +
> + guard(mutex)(&info->lock);
> +
> + if (val == info->enable_acc)
> + return 0;
> +
> + /*
> + * Force a hardware refresh to zero out the internal accumulators
> + * and start a new measurement window, as the chip accumulates
> + * continuously in the background.
> + */
> + if (val) {
> + sample_rate = pac1711_samp_rate_map_tbl[info->sample_rate_idx];
> + wait_time = (1024 * 1000) / sample_rate;
> + ret = pac1711_send_refresh(info, wait_time);
> + } else {
> + info->chip_reg_data.acc_val = 0;
> + info->chip_reg_data.vacc = 0;
> + info->chip_reg_data.acc_count = 0;
> + }
[Severity: Medium]
Should info->chip_reg_data.jiffies_tstamp be updated only after
pac1711_reg_snapshot_locked() has finished reading the measurement registers,
rather than inside pac1711_send_refresh()?
Currently, pac1711_send_refresh() sets info->chip_reg_data.jiffies_tstamp =
jiffies before sleeping for wait_time and before any registers are read into
info->chip_reg_data.
When pac1711_set_acc_enable() re-enables accumulation, it calls
pac1711_send_refresh() without reading the measurement registers into
info->chip_reg_data. If userspace reads in_voltage0_raw, in_current0_raw, or
in_power0_raw within 50 ms (PAC1711_MIN_POLLING_TIME_MS),
pac1711_reg_snapshot() sees the updated timestamp and returns early with stale
cached values. The same issue occurs if an I2C block read fails in
pac1711_reg_snapshot_locked() after pac1711_send_refresh() succeeds.
Conversely, at 8 SPS where wait_time is 128 ms, recording jiffies_tstamp
before fsleep(wait_time) means the 50 ms cache window has already expired by
the time pac1711_send_refresh() returns.
> +
> + if (!ret)
> + info->enable_acc = val;
> +
> + if (ret)
> + dev_err(dev, "failed to reset accumulator: %d\n", ret);
> +
> + return ret;
> +}
> +
> +/*
> + * The value of the shunt resistor may be known only at runtime and set by a client
> + * application. This attribute allows to set its value in micro-ohms. Y is the channel
> + * number. The value is used to calculate current, power and accumulated energy or
> + * Coulomb counter.
> + */
> +static ssize_t pac1711_write_shunt_resistor(struct iio_dev *indio_dev, uintptr_t private,
> + const struct iio_chan_spec *ch, const char *buf,
> + size_t len)
> +{
> + struct pac1711_chip_info *info = iio_priv(indio_dev);
> + struct device *dev = &info->client->dev;
> + unsigned int sh_val;
> + int ret;
> +
> + ret = kstrtouint(buf, 10, &sh_val);
> + if (ret) {
> + dev_err(dev, "Shunt value is not valid\n");
> + return ret;
> + }
> +
> + if (sh_val == 0)
> + return -EINVAL;
> +
> + guard(mutex)(&info->lock);
> +
> + info->shunt = sh_val;
> +
> + return len;
> +}
> +
> +static const struct iio_chan_spec_ext_info pac1711_ext_info[] = {
> + {
> + .name = "in_shunt_resistor",
> + .read = pac1711_read_shunt_resistor,
> + .write = pac1711_write_shunt_resistor,
> + .shared = IIO_SHARED_BY_ALL,
> + },
> + { }
> +};
[Severity: Medium]
Does exposing in_shunt_resistor in micro-ohms violate the IIO sysfs ABI?
Documentation/ABI/testing/sysfs-bus-iio specifies that in_shunt_resistor is
expressed in Ohms. Here, pac1711_read_shunt_resistor() emits info->shunt
directly in micro-ohms via sysfs_emit(), and pac1711_write_shunt_resistor()
parses the input as an integer in micro-ohms via kstrtouint().
Should this format the value in Ohms using iio_format_value() with
IIO_VAL_FRACTIONAL (or IIO_VAL_INT_PLUS_MICRO) and parse writes with
iio_str_to_fixpoint(), matching other power monitor drivers such as pac1921?
[ ... ]
> + case IIO_CHAN_INFO_SCALE: {
> + u32 shunt;
> +
> + guard(mutex)(&info->lock);
> + shunt = info->shunt;
> +
> + switch (chan->address) {
> + case PAC1711_VBUS_REG_ADDR:
> + *val = info->features->voltage_millivolts_max;
> + *val2 = (info->vbus_mode == PAC1711_FULL_RANGE_BIPOLAR) ? 15 : 16;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> + case PAC1711_VSENSE_REG_ADDR:
> + /*
> + * Currents - scale for mA - depends on the channel's shunt value
> + * (100mV * 1000000) / (2^16 * shunt(uohm))
> + */
> + *val = 1526;
> + *val2 = shunt;
> +
> + if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
> + *val = *val << 1;
> +
> + return IIO_VAL_FRACTIONAL;
[Severity: Medium]
Can *val2 overflow to a negative value in pac1711_read_raw() when info->shunt
exceeds INT_MAX?
Both pac1711_parse_fw() and pac1711_write_shunt_resistor() accept 32-bit
unsigned values up to U32_MAX into info->shunt, only rejecting zero. When
shunt > INT_MAX, assigning it to the signed int *val2 for IIO_VAL_FRACTIONAL
wraps *val2 negative, causing iio_format_value() to output a negative current
scale. Should shunt values greater than INT_MAX be rejected?
> + case PAC1711_VPOWER_REG_ADDR:
> + case PAC1711_VACC_REG_ADDR:
> + if (chan->type == IIO_CHARGE) {
> + /*
> + * Coulomb counter scale:
> + * 16-bit resolution: (100mV * NANO) / 2^16
> + * 12-bit resolution: (100mV * NANO) / 2^12
> + */
> + u64 ref = info->features->has_16bit_resolution ?
> + 1525878906250ULL : 24414062500000ULL;
> +
> + if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
> + ref = ref << 1;
> +
> + ref = div_u64(ref, shunt);
> + *val2 = do_div(ref, 1 * NANO);
> + *val = ref;
> +
> + return IIO_VAL_INT_PLUS_NANO;
> + }
> +
> + /*
> + * Power - uW - it will use the combined scale
> + * for current and voltage
> + * current(mA) * voltage(mV) = power (uW)
> + */
> + tmp = info->features->voltage_pv_fsr;
> + do_div(tmp, shunt);
> +
> + if (chan->type == IIO_ENERGY)
> + *val2 = info->features->has_16bit_resolution ? 32 : 24;
> + else
> + *val2 = 32;
> +
> + if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
> + *val2 -= 1;
> +
> + if (info->vbus_mode == PAC1711_FULL_RANGE_BIPOLAR)
> + *val2 -= 1;
> +
> + shift = fls64(tmp) - 31;
> + if (shift > 0) {
> + tmp >>= shift;
> + *val2 -= shift;
> + }
> +
> + *val = (int)tmp;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
[Severity: High]
Are the scale units for IIO_POWER and IIO_ENERGY in pac1711_read_raw() off by
factors of 1,000 and 1,000,000 respectively?
According to Documentation/ABI/testing/sysfs-bus-iio, power channels
(in_powerY_raw * in_powerY_scale) must be reported in milliwatts (mW), and
energy channels (in_energy_raw * in_energy_scale) must be reported in Joules
(J).
However, PAC1711_PRODUCT_VOLTAGE_PV_FSR (4200ULL * NANO = 4.2 * 10^12) and
PAC1721_PRODUCT_VOLTAGE_PV_FSR (900ULL * NANO = 9 * 10^11) compute the
numerator in microwatts * micro-ohms, as noted in the comment above. Dividing
by shunt (in micro-ohms) and returning IIO_VAL_FRACTIONAL_LOG2 with *val2 = 32
(or 24) yields a scale in microwatts for IIO_POWER (1,000x too large for mW)
and microjoules for IIO_ENERGY (1,000,000x too large for Joules).
[ ... ]
> +static int pac1711_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct pac1711_chip_info *info = iio_priv(indio_dev);
> + struct i2c_client *client = info->client;
> + struct device *dev = &info->client->dev;
> + int new_idx, ret, refresh_time;
> + s32 old_samp_rate;
> + __be16 val_be16;
> + u32 wait_time;
> + u16 val_u16;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + scoped_guard(mutex, &info->lock) {
> + old_samp_rate = pac1711_samp_rate_map_tbl[info->sample_rate_idx];
> + new_idx = pac1711_get_samp_rate_idx(val);
> + if (new_idx < 0)
> + return new_idx;
> +
> + ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_ACT_REG_ADDR,
> + sizeof(val_u16), (u8 *)&val_be16);
> + if (ret != sizeof(val_u16)) {
> + dev_err(&client->dev, "cannot read regs from 0x%02X\n",
> + PAC1711_CTRL_ACT_REG_ADDR);
> + return ret < 0 ? ret : -EIO;
> + }
> +
> + val_u16 = be16_to_cpu(val_be16);
> + FIELD_MODIFY(PAC1711_CTRL_SAMPLE_MODE_MASK, &val_u16, new_idx);
> + val_be16 = cpu_to_be16(val_u16);
> + ret = i2c_smbus_write_i2c_block_data(client, PAC1711_CTRL_REG_ADDR,
> + sizeof(val_be16), (u8 *)&val_be16);
> + if (ret) {
> + dev_err(dev, "Failed to configure sampling mode in 0x%02X\n",
> + PAC1711_CTRL_ACT_REG_ADDR);
> + return ret;
> + }
> +
> + info->sample_rate_idx = new_idx;
> + info->chip_reg_data.ctrl_act_reg = val_u16;
> +
> + /* Force register snapshot and timestamp update with a refresh. */
> + refresh_time = msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS) + 1;
> + info->chip_reg_data.jiffies_tstamp = jiffies - refresh_time;
> + wait_time = (1024 * 1000) / old_samp_rate;
> +
> + /*
> + * We must wait for cycle completion before updating the
> + * sample rate value.
> + */
> + ret = pac1711_reg_snapshot_locked(info, wait_time);
> + if (ret) {
> + dev_err(dev, "%s - cannot snapshot ctrl and measurement regs\n",
> + __func__);
> + return ret;
> + }
> + }
[Severity: Low]
In pac1711_write_raw(), info->sample_rate_idx is updated to new_idx before
pac1711_reg_snapshot_locked() sends the refresh command that activates the
new control register settings. If pac1711_reg_snapshot_locked() fails to send
the refresh command, won't the chip remain at old_samp_rate while
info->sample_rate_idx reports new_idx?
Also, in the error path of the i2c_smbus_write_i2c_block_data() call above,
dev_err() logs PAC1711_CTRL_ACT_REG_ADDR instead of PAC1711_CTRL_REG_ADDR.
[ ... ]
> +static int pac1711_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct pac1711_chip_info *info;
> + struct iio_dev *indio_dev;
> + u32 wait_time;
> + int ret, err;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*info));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + info = iio_priv(indio_dev);
> + info->client = client;
> +
> + ret = pac1711_chip_identify(indio_dev, info);
[Severity: High]
Does pac1711_probe() need to enable the vdd-supply regulator before accessing
the chip over I2C?
Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml requires
vdd-supply, but pac1711_probe() never calls devm_regulator_get_enable(dev,
"vdd") before calling pac1711_chip_identify() and pac1711_chip_configure(). If
the regulator is not already enabled by the bootloader, I2C communication will
fail during probe, or the regulator core may disable the unclaimed supply at
late_initcall while the device is in use.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-pac1711-v5-0-71c20101a52e@microchip.com?part=5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 2/5] iio: core: add IIO_CHARGE channel type
2026-11-03 14:37 ` [PATCH v5 2/5] iio: core: add IIO_CHARGE channel type Ariana Lazar
@ 2026-11-03 16:13 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-11-03 16:13 UTC (permalink / raw)
To: Ariana Lazar
Cc: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea
On Tue, Nov 03, 2026 at 04:37:12PM +0200, Ariana Lazar wrote:
> Add a new channel type for Coulomb counters, which accumulate current
> measurements over time. The sysfs attribute is in_chargeY_raw and after
> applying in_chargeY_scale, the value exposed is in milliCoulombs,
> following the SI convention used by the IIO subsystem.
>
> This new channel type will first be used by the pac1711 driver to report
> Coulomb counter measurements.
> +What: /sys/bus/iio/devices/iio:deviceX/in_charge_raw
> +What: /sys/bus/iio/devices/iio:deviceX/in_chargeY_raw
> +KernelVersion: 7.4
7.5 at the best.
> +Contact: linux-iio@vger.kernel.org
> +Description:
> + Raw charge value read from the channel. The device acts as
> + a Coulomb counter by accumulating current measurements over
> + time. Units after application of scale are milliCoulombs. X
> + is the IIO index of the device.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 3/5] iio: core: add iio_fill_val_int_64() helper
2026-11-03 14:37 ` [PATCH v5 3/5] iio: core: add iio_fill_val_int_64() helper Ariana Lazar
2026-11-03 14:42 ` sashiko-bot
@ 2026-11-03 16:14 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-11-03 16:14 UTC (permalink / raw)
To: Ariana Lazar
Cc: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea
On Tue, Nov 03, 2026 at 04:37:13PM +0200, Ariana Lazar wrote:
> Add iio_fill_val_int_64() to populate the val and val2 output parameters
> with the lower and upper 32 bits of a 64-bit value, as used for
> IIO_VAL_INT_64. The helper wraps lower_32_bits() and upper_32_bits() in
> order to split the value with a single call.
>
> The pac1711 driver will be the first user of this function.
NAK. Use existing iio_val_s64_decompose().
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v5 5/5] iio: adc: add support for PAC1711
2026-11-03 14:37 ` [PATCH v5 5/5] " Ariana Lazar
2026-11-03 14:59 ` sashiko-bot
@ 2026-11-03 16:40 ` Andy Shevchenko
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-11-03 16:40 UTC (permalink / raw)
To: Ariana Lazar
Cc: Jonathan Cameron, Guenter Roeck, David Lechner, Nuno Sá,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel, linux-hwmon, Neil Armstrong,
Marius Cristea
On Tue, Nov 03, 2026 at 04:37:15PM +0200, Ariana Lazar wrote:
> This is the iio driver for Microchip PAC1711, PAC1721, PAC1811 and
> PAC1821 single-channel power monitors with accumulator. The PAC1711 and
> PAC1721 devices use 12-bit resolution for voltage and current measurements
> and 24 bits for power calculations, while PAC1811 and PAC1821 have 16-bit
> resolution and use 32 bits for power calculations. The 56-bit accumulator
> register accumulates power (energy) or current (Coulomb counter).
>
> PAC1711 and PAC1811 measure up to 42V Full-Scale Range, respectively 9V for
> PAC1721 and PAC1821.
...
> +config PAC1711
> + tristate "Microchip Technology PAC1711 driver"
Align this text with the Summary at the top comment and MODULE_DESCRIPTION()
that all three read the same (or quite similar, like the same with or without
word "driver").
> + depends on I2C
> + help
> + Say yes here to build support for Microchip Technology's PAC1711,
> + PAC1721, PAC1811 and PAC1821 Single-Channel Power Monitors with
> + Accumulator.
> +
> + This driver can also be built as a module. If so, the module
> + will be called pac1711.
...
> + * IIO driver for PAC1711 Single-Channel DC Power/Energy Monitor
^^^ See above.
...
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
I failed to see how it's used/ dev_printk.h, yes, but the rest of device.h?
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/jiffies.h>
> +#include <linux/kstrtox.h>
> +#include <linux/module.h>
> +#include <linux/math64.h>
> +#include <linux/mutex.h>
> +#include <linux/overflow.h>
> +#include <linux/property.h>
+ time.h // for MSEC_PER_SEC
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +#include <linux/workqueue.h>
+ blank line.
> +#include <asm/byteorder.h>
> +
> +#include <linux/iio/iio.h>
...
> +#define PAC1711_VOLTAGE_MILLIVOLTS_MAX (42 * MILLI)
> +#define PAC1721_VOLTAGE_MILLIVOLTS_MAX (9 * MILLI)
#define PAC1711_VOLTAGE_mV_MAX (42 * MILLI)
#define PAC1721_VOLTAGE_mV_MAX (9 * MILLI)
...
> +/* Full FSR power is multiplied by 10^9 to ensure precise scale calculation. */
> +#define PAC1711_PRODUCT_VOLTAGE_PV_FSR (4200ULL * NANO)
> +#define PAC1721_PRODUCT_VOLTAGE_PV_FSR (900ULL * NANO)
_pV (I assume picovolts is in mind?) Note, this also the same as above with
just a multiplier. Why do we need them separately (or independent)?
...
> +/* PAC1711/PAC1811 VBUS FSR: 42V and 21V */
> +#define PAC1711_VBUS_FSR_UV (42 * MICRO)
> +#define PAC1711_VBUS_FSR_HALF_UV (21 * MICRO)
> +/* PAC1721/PAC1821 VBUS FSR: 9V and 4.5V */
> +#define PAC1721_VBUS_FSR_UV (9 * MICRO)
> +#define PAC1721_VBUS_FSR_HALF_UV (4500 * MILLI)
> +
> +/* VSENSE FSR: 100mV and 50mV */
> +#define PAC1711_VSENSE_FSR_UV (100 * MILLI)
> +#define PAC1711_VSENSE_FSR_HALF_UV (50 * MILLI)
_UV ==> _uV
...
> +#define PAC1711_ACC_ENERGY_STR "energy"
> +#define PAC1711_ACC_CHARGE_STR "charge"
One-time use, kill the macro, it's visible from the below.
> +static const char * const pac1711_acc_sources[] = {
> + [PAC1711_ACCMODE_VPOWER] = PAC1711_ACC_ENERGY_STR,
> + [PAC1711_ACCMODE_VSENSE] = PAC1711_ACC_CHARGE_STR,
> +};
...
> +static const int pac1711_vbus_range_tbl[2][3][2] = {
> + [PAC1711_VOLTAGE_RANGE_IDX] = {
> + [PAC1711_FULL_RANGE_UNIPOLAR] = { 0, PAC1711_VBUS_FSR_UV },
> + [PAC1711_FULL_RANGE_BIPOLAR] = { -(int)PAC1711_VBUS_FSR_UV, PAC1711_VBUS_FSR_UV },
> + [PAC1711_HALF_RANGE_BIPOLAR] = { -(int)PAC1711_VBUS_FSR_HALF_UV,
> + PAC1711_VBUS_FSR_HALF_UV },
> + },
> + [PAC1721_VOLTAGE_RANGE_IDX] = {
> + [PAC1711_FULL_RANGE_UNIPOLAR] = { 0, PAC1721_VBUS_FSR_UV },
> + [PAC1711_FULL_RANGE_BIPOLAR] = { -(int)PAC1721_VBUS_FSR_UV, PAC1721_VBUS_FSR_UV },
> + [PAC1711_HALF_RANGE_BIPOLAR] = { -(int)PAC1721_VBUS_FSR_HALF_UV,
> + PAC1721_VBUS_FSR_HALF_UV },
There is an indentation issue, also no need to format '=' on the same column.
[PAC1711_FULL_RANGE_UNIPOLAR] = { 0, PAC1721_VBUS_FSR_uV },
// alternatively
[PAC1711_FULL_RANGE_UNIPOLAR] = {
0,
PAC1721_VBUS_FSR_uV,
},
[PAC1711_FULL_RANGE_BIPOLAR] = {
-(int)PAC1721_VBUS_FSR_uV,
PAC1721_VBUS_FSR_uV,
},
[PAC1711_HALF_RANGE_BIPOLAR] = {
-(int)PAC1721_VBUS_FSR_HALF_uV,
PAC1721_VBUS_FSR_HALF_uV,
},
> + },
> +};
...and so on...
...
> +enum {
> + PAC1711_SAMP_8192SPS = 0,
> + PAC1711_SAMP_4096SPS = 1,
> + PAC1711_SAMP_1024SPS = 2,
> + PAC1711_SAMP_256SPS = 3,
> + PAC1711_SAMP_64SPS = 4,
> + PAC1711_SAMP_8SPS = 5,
> +};
This enum is not directly used in HW, but okay, it might make code robust with
the explicit assignments.
...
> +struct pac1711_features {
> + u64 voltage_pv_fsr;
> + int voltage_millivolts_max;
> + const char *name;
> + u8 voltage_range_idx;
> + bool has_16bit_resolution;
Run `pahole` and act accordingly. Same for other data types where the order
is not strict.
> +};
...
> +static int pac1711_send_refresh(struct pac1711_chip_info *info, u32 wait_time)
> +{
> + struct i2c_client *client = info->client;
> + int ret;
> +
> + /* Writing a REFRESH or a REFRESH_V command */
> + ret = i2c_smbus_write_byte(client, PAC1711_REFRESH_REG_ADDR);
> + if (ret) {
> + dev_err(&client->dev, "%s - cannot send Refresh cmd (0x%02X)\n",
> + __func__, PAC1711_REFRESH_REG_ADDR);
__func__ means that the message itself is poorly written (not enough uniqueness
to understand from what flow point it comes. Ditto for all __func__ uses, revisit
all the messages and make them more precise if required and drop __func__.
> + return ret;
> + }
> +
> + /* Register data retrieval timestamp */
> + info->chip_reg_data.jiffies_tstamp = jiffies;
> +
> + /* Wait till the data is available */
> + fsleep(wait_time);
> +
> + return 0;
> +}
...
> + /* Read the ctrl/status registers for this snapshot */
> + ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_ACT_REG_ADDR,
> + sizeof(val_be16), (u8 *)&val_be16);
> + if (ret != sizeof(val_be16)) {
> + dev_err(dev, "%s - cannot read regs from 0x%02X\n",
> + __func__, PAC1711_CTRL_ACT_REG_ADDR);
> + return ret < 0 ? ret : -EIO;
> + }
No nested branches, it's harder to follow. (Here and elsewhere.)
...
> +static int pac1711_reg_snapshot(struct pac1711_chip_info *info, bool *refreshed)
> +{
> + u32 wait_time;
Units?
> + int ret;
> +
> + guard(mutex)(&info->lock);
> +
> + /* No hardware refresh is needed */
> + if (refreshed)
> + *refreshed = false;
> +
> + /* Return early if the minimum polling time hasn't elapsed */
> + if (!time_after(jiffies, info->chip_reg_data.jiffies_tstamp +
> + msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS)))
> + return 0;
> +
> + wait_time = (1024 * 1000) / pac1711_samp_rate_map_tbl[info->sample_rate_idx];
Depending on the above, 1000 can be replaced with named multiplier.
Same for elsewhere magic 1000.
> + /* A hardware refresh will happen */
> + ret = pac1711_reg_snapshot_locked(info, wait_time);
> + if (!ret && refreshed)
No. use the regular pattern
if (ret)
return ret;
if (refreshed)
...
return 0;
> + *refreshed = true;
> +
> + return ret;
> +}
...
> + if (!ret)
> + info->enable_acc = val;
Why two times to check ret?
> + if (ret)
> + dev_err(dev, "failed to reset accumulator: %d\n", ret);
> +
> + return ret;
...
> + guard(mutex)(&info->lock);
+ blank line.
> + shunt = info->shunt;
> +
> + switch (chan->address) {
> + case PAC1711_VBUS_REG_ADDR:
> + *val = info->features->voltage_millivolts_max;
> + *val2 = (info->vbus_mode == PAC1711_FULL_RANGE_BIPOLAR) ? 15 : 16;
> +
> + return IIO_VAL_FRACTIONAL_LOG2;
> + case PAC1711_VSENSE_REG_ADDR:
> + /*
> + * Currents - scale for mA - depends on the channel's shunt value
> + * (100mV * 1000000) / (2^16 * shunt(uohm))
> + */
> + *val = 1526;
> + *val2 = shunt;
> +
> + if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
> + *val = *val << 1;
*val *= 2;
> +
> + return IIO_VAL_FRACTIONAL;
> + case PAC1711_VPOWER_REG_ADDR:
> + case PAC1711_VACC_REG_ADDR:
> + if (chan->type == IIO_CHARGE) {
> + /*
> + * Coulomb counter scale:
> + * 16-bit resolution: (100mV * NANO) / 2^16
> + * 12-bit resolution: (100mV * NANO) / 2^12
> + */
> + u64 ref = info->features->has_16bit_resolution ?
> + 1525878906250ULL : 24414062500000ULL;
Split definition and assignment.
> + if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
> + ref = ref << 1;
ref *= 2;
> + ref = div_u64(ref, shunt);
> + *val2 = do_div(ref, 1 * NANO);
> + *val = ref;
> +
> + return IIO_VAL_INT_PLUS_NANO;
> + }
> +
> + /*
> + * Power - uW - it will use the combined scale
> + * for current and voltage
> + * current(mA) * voltage(mV) = power (uW)
> + */
> + tmp = info->features->voltage_pv_fsr;
> + do_div(tmp, shunt);
> +
> + if (chan->type == IIO_ENERGY)
> + *val2 = info->features->has_16bit_resolution ? 32 : 24;
> + else
> + *val2 = 32;
> +
> + if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR)
> + *val2 -= 1;
> +
> + if (info->vbus_mode == PAC1711_FULL_RANGE_BIPOLAR)
> + *val2 -= 1;
> +
> + shift = fls64(tmp) - 31;
> + if (shift > 0) {
> + tmp >>= shift;
> + *val2 -= shift;
> + }
> +
> + *val = (int)tmp;
Why casting?
> + return IIO_VAL_FRACTIONAL_LOG2;
> + default:
> + return -EINVAL;
> + }
...
Basically the last 2/3rd I sparsely checked. It's unbelievably that v5 of
the series has tons of small style issues. Also the driver length is too big,
can you split this to a few, smaller ones?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-11-03 16:40 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-03 14:37 [PATCH v5 0/5] Add support for Microchip PAC1711 Power Monitor Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 1/5] unaligned: introduce be56 unaligned accessors Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 2/5] iio: core: add IIO_CHARGE channel type Ariana Lazar
2026-11-03 16:13 ` Andy Shevchenko
2026-11-03 14:37 ` [PATCH v5 3/5] iio: core: add iio_fill_val_int_64() helper Ariana Lazar
2026-11-03 14:42 ` sashiko-bot
2026-11-03 16:14 ` Andy Shevchenko
2026-11-03 14:37 ` [PATCH v5 4/5] dt-bindings: iio: adc: add support for PAC1711 Ariana Lazar
2026-11-03 14:37 ` [PATCH v5 5/5] " Ariana Lazar
2026-11-03 14:59 ` sashiko-bot
2026-11-03 16:40 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox