* [PATCH 0/2] Adding support for Microchip PAC1711
@ 2025-10-15 10:12 Ariana Lazar
2025-10-15 10:12 ` [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 Ariana Lazar
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Ariana Lazar @ 2025-10-15 10:12 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-iio, devicetree, linux-kernel, Ariana Lazar
The PAC1711 product is a single-channel power monitor with accumulator.
The device uses 12-bit resolution for voltage and current measurements and
24 bits power calculations. The accumulator register (56-bit) could
accumulate power (energy), current (Coulomb counter) or voltage.
PAC1711 measures up to 42V Full-Scale Range.
Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
---
Ariana Lazar (2):
dt-bindings: iio: adc: adding support for PAC1711
iio: adc: adding support for PAC1711
.../ABI/testing/sysfs-bus-iio-adc-pac1711 | 57 +
.../bindings/iio/adc/microchip,pac1711.yaml | 195 +++
MAINTAINERS | 8 +
drivers/iio/adc/Kconfig | 10 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/pac1711.c | 1448 ++++++++++++++++++++
6 files changed, 1719 insertions(+)
---
base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
change-id: 20250901-pac1711-d3bacda400fd
Best regards,
--
Ariana Lazar <ariana.lazar@microchip.com>
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 2025-10-15 10:12 [PATCH 0/2] Adding support for Microchip PAC1711 Ariana Lazar @ 2025-10-15 10:12 ` Ariana Lazar 2025-10-16 16:19 ` Conor Dooley 2025-10-19 9:04 ` Jonathan Cameron 2025-10-15 10:12 ` [PATCH 2/2] " Ariana Lazar 2025-10-19 10:31 ` [PATCH 0/2] Adding support for Microchip PAC1711 Jonathan Cameron 2 siblings, 2 replies; 11+ messages in thread From: Ariana Lazar @ 2025-10-15 10:12 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel, Ariana Lazar This is the device tree schema for Microchip PAC1711 single-channel power monitor with accumulator. The device uses 12-bit resolution for voltage and current measurements and 24 bits power calculations. The device supports one 56-bit accumulator register. PAC1711 measures up to 42V Full-Scale Range. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- .../bindings/iio/adc/microchip,pac1711.yaml | 195 +++++++++++++++++++++ MAINTAINERS | 6 + 2 files changed, 201 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..67edd778981c2f0ed21dda02f14e383a153169b1 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml @@ -0,0 +1,195 @@ +# 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. + The datasheet for PAC1711 can be found here: + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/PAC1711-Data-Sheet-DS20007058.pdf + +properties: + compatible: + enum: + - microchip,pac1711 + + reg: + maxItems: 1 + + vdd-supply: true + + "#io-channel-cells": + const: 1 + + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + gpio-controller: + description: Marks the device node as a GPIO controller. + + "#gpio-cells": + const: 2 + description: + The first cell is the GPIO number and the second cell specifies + GPIO flags, as defined in <dt-bindings/gpio/gpio.h>. + + powerdown-gpios: + description: + Active low puts the device in power-down state. When the PWRDN pin is + pulled high, measurement and accumulation will resume using the default + register settings. + maxItems: 1 + + interrupts: + maxItems: 2 + + interrupt-names: + description: + Could be triggered by overvoltage, undervoltage, overcurrent, overpower, + undercurrent, step limit, accumulator overflow and accumulator count + overflow. + items: + - const: alert0 + - const: alert1 + + shunt-resistor-micro-ohms: + description: + Value in micro Ohms of the shunt resistor connected between + the VSENSEP and VSENSEN inputs, across which the current is measured. + Value is needed to compute the scaling of the measured current. + + label: + description: Unique name to identify which device this is. + + microchip,gpio: + type: boolean + description: + In default mode, A0 pin is a GPIO 0 input pin, respectively A1 pin is + GPIO 1. The pins can be used for the SLOW function, the device will sample + at 8 samples/second if pulled high. A0 also function as the Alert0 and A1 + as Alert1, but can no longer be used to control conversion rate or SLOW. + + 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: + - VBUS has unipolar 0V to 42V FSR (default) + - VBUS has bipolar -42V to 42V FSR + - VBUS has bipolar -21V to 21V FSR + items: + - enum: [-42000000, -21000000, 0] + - enum: [21000000, 42000000] + + 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 100V FSR (default) + - VSENSE has bipolar -100 mV to 100 mV FSR + - VSENSE has bipolar -50 mV to 50 mV FSR + items: + - enum: [-100000, -50000, 0] + - enum: [50000, 100000] + + microchip,accumulation-mode: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + The Hardware Accumulator may be used to accumulate VPOWER, VSENSE or + VBUS values for any channel. By setting the accumulator for a channel + to accumulate the VPOWER values gives a measure of accumulated power + into 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: + <0> - Accumulator accumulates VPOWER (default) + <1> - Accumulator accumulates VSENSE + <2> - Accumulator accumulates VBUS + maximum: 2 + default: 0 + +required: + - compatible + - reg + - vdd-supply + - shunt-resistor-micro-ohms + +allOf: + - if: + properties: + compatible: + contains: + enum: + - microchip,pac1711 + then: + properties: + microchip,vbus-input-range-microvolt: + oneOf: + - items: + - const: 0 + - const: 42000000 + - items: + - const: -42000000 + - const: 42000000 + - items: + - const: -21000000 + - const: 21000000 + default: + items: + - const: 0 + - const: 42000000 + + microchip,vsense-input-range-microvolt: + oneOf: + - items: + - const: 0 + - const: 100000 + - items: + - const: -100000 + - const: 100000 + - items: + - const: -50000 + - const: 50000 + default: + items: + - const: 0 + - const: 100000 + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + pac1711@10 { + compatible = "microchip,pac1711"; + reg = <0x10>; + + shunt-resistor-micro-ohms = <10000>; + label = "VDD3V3"; + vdd-supply = <&vdd>; + microchip,vbus-input-range-microvolt = <(-21000000) 21000000>; + microchip,vsense-input-range-microvolt = <(-50000) 50000>; + microchip,accumulation-mode = <0>; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index a92290fffa163f9fe8fe3f04bf66426f9a894409..7686e2516c90442aa3e23d19cfb08e280a44ba76 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16337,6 +16337,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 DAC 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
* Re: [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 2025-10-15 10:12 ` [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 Ariana Lazar @ 2025-10-16 16:19 ` Conor Dooley 2025-10-19 9:04 ` Jonathan Cameron 1 sibling, 0 replies; 11+ messages in thread From: Conor Dooley @ 2025-10-16 16:19 UTC (permalink / raw) To: Ariana Lazar Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel [-- Attachment #1: Type: text/plain, Size: 9809 bytes --] Hey, On Wed, Oct 15, 2025 at 01:12:15PM +0300, Ariana Lazar wrote: > This is the device tree schema for Microchip PAC1711 single-channel power > monitor with accumulator. The device uses 12-bit resolution for voltage and > current measurements and 24 bits power calculations. The device supports > one 56-bit accumulator register. > > PAC1711 measures up to 42V Full-Scale Range. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> > --- > .../bindings/iio/adc/microchip,pac1711.yaml | 195 +++++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 201 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..67edd778981c2f0ed21dda02f14e383a153169b1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml > @@ -0,0 +1,195 @@ > +# 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. > + The datasheet for PAC1711 can be found here: > + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/PAC1711-Data-Sheet-DS20007058.pdf > + > +properties: > + compatible: > + enum: > + - microchip,pac1711 > + > + reg: > + maxItems: 1 > + > + vdd-supply: true > + > + "#io-channel-cells": > + const: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 There are no child nodes here, so I don't think size or address cells are needed. > + > + gpio-controller: > + description: Marks the device node as a GPIO controller. > + > + "#gpio-cells": > + const: 2 > + description: > + The first cell is the GPIO number and the second cell specifies > + GPIO flags, as defined in <dt-bindings/gpio/gpio.h>. > + > + powerdown-gpios: > + description: > + Active low puts the device in power-down state. When the PWRDN pin is > + pulled high, measurement and accumulation will resume using the default > + register settings. > + maxItems: 1 > + > + interrupts: > + maxItems: 2 > + > + interrupt-names: > + description: > + Could be triggered by overvoltage, undervoltage, overcurrent, overpower, > + undercurrent, step limit, accumulator overflow and accumulator count > + overflow. > + items: > + - const: alert0 > + - const: alert1 This won't allow using alert0 but not alert1, or vice versa. Is that intended? I would imagine that only using one of the interrupt pins is a valid behaviour. > + > + shunt-resistor-micro-ohms: > + description: > + Value in micro Ohms of the shunt resistor connected between > + the VSENSEP and VSENSEN inputs, across which the current is measured. > + Value is needed to compute the scaling of the measured current. > + > + label: > + description: Unique name to identify which device this is. > + > + microchip,gpio: > + type: boolean > + description: > + In default mode, A0 pin is a GPIO 0 input pin, respectively A1 pin is > + GPIO 1. The pins can be used for the SLOW function, the device will sample > + at 8 samples/second if pulled high. A0 also function as the Alert0 and A1 > + as Alert1, but can no longer be used to control conversion rate or SLOW. This description provides zero detail about what the property actually does. The driver doesn't appear to use it either, so no hints for me there. If it is something to do with deciding if pins are interrupts or are free for gpio use, can't that just be determined based on what interrupts are in use? > + > + 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: > + - VBUS has unipolar 0V to 42V FSR (default) > + - VBUS has bipolar -42V to 42V FSR > + - VBUS has bipolar -21V to 21V FSR > + items: > + - enum: [-42000000, -21000000, 0] > + - enum: [21000000, 42000000] > + > + 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 100V FSR (default) > + - VSENSE has bipolar -100 mV to 100 mV FSR > + - VSENSE has bipolar -50 mV to 50 mV FSR > + items: > + - enum: [-100000, -50000, 0] > + - enum: [50000, 100000] > + > + microchip,accumulation-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + The Hardware Accumulator may be used to accumulate VPOWER, VSENSE or > + VBUS values for any channel. By setting the accumulator for a channel > + to accumulate the VPOWER values gives a measure of accumulated power > + into 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: > + <0> - Accumulator accumulates VPOWER (default) > + <1> - Accumulator accumulates VSENSE > + <2> - Accumulator accumulates VBUS Please make this a string, probably with values "vpower", "vsense" and "vbus". > + maximum: 2 > + default: 0 > + > +required: > + - compatible > + - reg > + - vdd-supply > + - shunt-resistor-micro-ohms > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - microchip,pac1711 This is the only compatible, using an if/then/else section here is not required. This should be describable in the property itself. > + then: > + properties: > + microchip,vbus-input-range-microvolt: > + oneOf: > + - items: > + - const: 0 > + - const: 42000000 > + - items: > + - const: -42000000 > + - const: 42000000 > + - items: > + - const: -21000000 > + - const: 21000000 > + default: > + items: > + - const: 0 > + - const: 42000000 > + > + microchip,vsense-input-range-microvolt: > + oneOf: > + - items: > + - const: 0 > + - const: 100000 > + - items: > + - const: -100000 > + - const: 100000 > + - items: > + - const: -50000 > + - const: 50000 > + default: > + items: > + - const: 0 > + - const: 100000 > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pac1711@10 { "pac1711" is not a class of device, so probably dac@10? pw-bot: changes-requested > + compatible = "microchip,pac1711"; > + reg = <0x10>; > + > + shunt-resistor-micro-ohms = <10000>; > + label = "VDD3V3"; > + vdd-supply = <&vdd>; > + microchip,vbus-input-range-microvolt = <(-21000000) 21000000>; > + microchip,vsense-input-range-microvolt = <(-50000) 50000>; > + microchip,accumulation-mode = <0>; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index a92290fffa163f9fe8fe3f04bf66426f9a894409..7686e2516c90442aa3e23d19cfb08e280a44ba76 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -16337,6 +16337,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 DAC DRIVER > +M: Ariana Lazar <ariana.lazar@microchip.com> I also work for Microchip (in the FPGA BU), so whenever I see people from Microchip whose name I don't recognise I look them up. I noticed that Teams lists you as being an intern, so I have to ask what your plan is for maintaining this after your internship has completed. Do you intend passing it over to someone else from your team or continuing yourself? If it's the former, it'd be good to CC them and/or add them here as a co-maintainer. Cheers, Conor. > +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 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 2025-10-15 10:12 ` [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 Ariana Lazar 2025-10-16 16:19 ` Conor Dooley @ 2025-10-19 9:04 ` Jonathan Cameron 1 sibling, 0 replies; 11+ messages in thread From: Jonathan Cameron @ 2025-10-19 9:04 UTC (permalink / raw) To: Ariana Lazar Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Wed, 15 Oct 2025 13:12:15 +0300 Ariana Lazar <ariana.lazar@microchip.com> wrote: > This is the device tree schema for Microchip PAC1711 single-channel power > monitor with accumulator. The device uses 12-bit resolution for voltage and > current measurements and 24 bits power calculations. The device supports > one 56-bit accumulator register. > > PAC1711 measures up to 42V Full-Scale Range. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> > --- > .../bindings/iio/adc/microchip,pac1711.yaml | 195 +++++++++++++++++++++ > MAINTAINERS | 6 + > 2 files changed, 201 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..67edd778981c2f0ed21dda02f14e383a153169b1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml > + 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: > + - VBUS has unipolar 0V to 42V FSR (default) > + - VBUS has bipolar -42V to 42V FSR > + - VBUS has bipolar -21V to 21V FSR > + items: > + - enum: [-42000000, -21000000, 0] > + - enum: [21000000, 42000000] > + > + 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 100V FSR (default) > + - VSENSE has bipolar -100 mV to 100 mV FSR > + - VSENSE has bipolar -50 mV to 50 mV FSR > + items: > + - enum: [-100000, -50000, 0] > + - enum: [50000, 100000] These range setting things are common enough perhaps it's time to standardize them as properties of channel sub nodes (the stuff in adc.yaml). > + > + microchip,accumulation-mode: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + The Hardware Accumulator may be used to accumulate VPOWER, VSENSE or > + VBUS values for any channel. By setting the accumulator for a channel > + to accumulate the VPOWER values gives a measure of accumulated power > + into 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: > + <0> - Accumulator accumulates VPOWER (default) > + <1> - Accumulator accumulates VSENSE > + <2> - Accumulator accumulates VBUS This feels like a runtime thing to control. To be in DT it should be related to the board wiring rather than being a choice between multiple things being measured. > + maximum: 2 > + default: 0 > + > +required: > + - compatible > + - reg > + - vdd-supply > + - shunt-resistor-micro-ohms ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] iio: adc: adding support for PAC1711 2025-10-15 10:12 [PATCH 0/2] Adding support for Microchip PAC1711 Ariana Lazar 2025-10-15 10:12 ` [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 Ariana Lazar @ 2025-10-15 10:12 ` Ariana Lazar 2025-10-16 5:31 ` kernel test robot ` (2 more replies) 2025-10-19 10:31 ` [PATCH 0/2] Adding support for Microchip PAC1711 Jonathan Cameron 2 siblings, 3 replies; 11+ messages in thread From: Ariana Lazar @ 2025-10-15 10:12 UTC (permalink / raw) To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: linux-iio, devicetree, linux-kernel, Ariana Lazar This is the iio driver for Microchip PAC1711 single-channel power monitor with accumulator. The device uses 12-bit resolution for voltage and current measurements and 24 bits power calculations. The device supports one 56-bit accumulator register. PAC1711 measures up to 42V Full-Scale Range. Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> --- .../ABI/testing/sysfs-bus-iio-adc-pac1711 | 57 + MAINTAINERS | 2 + drivers/iio/adc/Kconfig | 10 + drivers/iio/adc/Makefile | 1 + drivers/iio/adc/pac1711.c | 1448 ++++++++++++++++++++ 5 files changed, 1518 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-pac1711 b/Documentation/ABI/testing/sysfs-bus-iio-adc-pac1711 new file mode 100644 index 0000000000000000000000000000000000000000..7f6ab50d29ff064d57b80df0a0c162a4c98764f8 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-pac1711 @@ -0,0 +1,57 @@ +What: /sys/bus/iio/devices/iio:deviceX/in_shunt_resistor +KernelVersion: 6.18 +Contact: linux-iio@vger.kernel.org +Description: + 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. X is the IIO index of the device. The value + is used to calculate current, power and accumulated energy. + +What: /sys/bus/iio/devices/iio:deviceX/in_current_acc_raw +KernelVersion: 6.18 +Contact: linux-iio@vger.kernel.org +Description: + This attribute is used to read the accumulated voltage measured + on the shunt resistor (Coulomb counter). Units after application + of scale are mA. X is the IIO index of the device. + +What: /sys/bus/iio/devices/iio:deviceX/in_current_acc_scale +KernelVersion: 6.18 +Contact: linux-iio@vger.kernel.org +Description: + If known for a device, scale to be applied to in_current_raw in + order to obtain the measured value in mA units. X is the IIO + index of the device. + +What: /sys/bus/iio/devices/iio:deviceX/in_current_acc_en +KernelVersion: 6.18 +Contact: linux-iio@vger.kernel.org +Description: + This attribute, if available, is used to enable digital + accumulation of VSENSE measurements. X is the IIO index of the + device. + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_acc_raw +KernelVersion: 6.18 +Contact: linux-iio@vger.kernel.org +Description: + This attribute is used to read the accumulated voltage on VBUS. + Units after application of scale are mV. X is the IIO index of + the device. + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_acc_scale +KernelVersion: 6.18 +Contact: linux-iio@vger.kernel.org +Description: + If known for a device, scale to be applied to in_voltage_raw + in order to obtain the measured value in mV units. X is the IIO + index of the device. + +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_acc_en +KernelVersion: 6.18 +Contact: linux-iio@vger.kernel.org +Description: + This attribute, if available, is used to enable digital + accumulation of VBUS measurements. X is the IIO index of the + device. + diff --git a/MAINTAINERS b/MAINTAINERS index 7686e2516c90442aa3e23d19cfb08e280a44ba76..d4d4e303ad278cbf8f628b52e3b5ebe60fb1bf09 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16341,7 +16341,9 @@ MICROCHIP PAC1711 DAC DRIVER M: Ariana Lazar <ariana.lazar@microchip.com> L: linux-iio@vger.kernel.org S: Supported +F: Documentation/ABI/testing/sysfs-bus-iio-adc-pac1711 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 ea3ba139739281de82848e25fd2b6ca479a939dc..c91c66e583d1ce07aa1813a84d461ebd3fb6344d 100644 --- a/drivers/iio/adc/Kconfig +++ b/drivers/iio/adc/Kconfig @@ -1125,6 +1125,16 @@ config NPCM_ADC This driver can also be built as a module. If so, the module will be called npcm_adc. +config PAC1711 + tristate "Microchip Technology PAC1711 driver" + depends on I2C + help + Say yes here to build support for Microchip Technology's PAC1711 + Single-Channel Power Monitor 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 09ae6edb26504991f011def6618efc3f4cf4df4c..d039a23cde02d442b161730ad2c939c7d035a4c6 100644 --- a/drivers/iio/adc/Makefile +++ b/drivers/iio/adc/Makefile @@ -101,6 +101,7 @@ obj-$(CONFIG_MXS_LRADC_ADC) += mxs-lradc-adc.o obj-$(CONFIG_NAU7802) += nau7802.o obj-$(CONFIG_NCT7201) += nct7201.o obj-$(CONFIG_NPCM_ADC) += npcm_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..30929af202e009c2d80ba91d5c137a26a03f09d7 --- /dev/null +++ b/drivers/iio/adc/pac1711.c @@ -0,0 +1,1448 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * IIO driver for PAC1711 Multi-Channel DC Power/Energy Monitor + * + * Copyright (C) 2024 Microchip Technology Inc. and its subsidiaries + * + * Author: Ariana Lazar <ariana.lazar@microchip.com> + * + */ +#include <linux/bitfield.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/i2c.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> +#include <linux/unaligned.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 60000 +/* 50msec is the timeout for validity of the cached registers */ +#define PAC1711_MIN_POLLING_TIME_MS 50 +/* + * 1000usec is the minimum wait time for normal conversions when sample + * rate doesn't change + */ +#define PAC1711_MIN_UPDATE_WAIT_TIME_US 1000 + +/* 42000mV */ +#define PAC1711_VOLTAGE_MILLIVOLTS_MAX 42000 + +/* voltage bits resolution when set for unsigned values */ +#define PAC1711_VOLTAGE_12B_RES 12 +/* voltage bits resolution when set for signed values */ +#define PAC1711_VOLTAGE_11B_RES 11 + +/* Power resolution is 24 bits when contains unsigned values */ +#define PAC1711_POWER_24B_RES 24 +/* Power resolution is 24 bits when contains signed values */ +#define PAC1711_POWER_23B_RES 23 + +/* Maximum power-product value - 42 V * 0.1 V */ +#define PAC1711_PRODUCT_VOLTAGE_PV_FSR 4200000000000UL + +/* Scale constant = (10^3 * 4.2 * 10^9 / 2^24) for mili Watt-second */ +#define PAC1711_SCALE_CONSTANT 250340 + +/* (100mV * 1000000) / (2^12) used to calculate the scale for current */ +#define PAC1711_MAX_VSENSE_RSHIFTED_BY_12B 24414 + +/* + * [(100mV * 1000000) / (2^12)]*10^9 used to calculate the scale + * for accumulated current/Coulomb counter + */ +#define PAC1711_MAX_VSENSE_NANO 24414062500000UL + +/* I2C address map */ +#define PAC1711_REFRESH_REG_ADDR 0x00 +#define PAC1711_CTRL_REG_ADDR 0x01 +#define PAC1711_ACC_COUNT_REG_ADDR 0x02 +#define PAC1711_VACC_ADDR 0x03 +#define PAC1711_VBUS_ADDR 0x04 +#define PAC1711_VSENSE_ADDR 0x05 +#define PAC1711_VBUS_AVG_ADDR 0x06 +#define PAC1711_VSENSE_AVG_ADDR 0x07 +#define PAC1711_VPOWER_ADDR 0x08 + +/* Start of configurations registers */ +#define PAC1711_CTRL_LAT_REG_ADDR 0x0F +#define PAC1711_NEG_PWR_FSR_REG_ADDR 0x13 +#define PAC1711_SLOW_REG_ADDR 0x16 +#define PAC1711_CTRL_ACT_REG_ADDR 0x17 +#define PAC1711_PID_REG_ADDR 0xFD +#define PAC1711_REVISION_ID_REG_ADDR 0xFF + +#define PAC1711_ACC_DEVATTR 3 +#define PAC1711_COMMON_DEVATTR 1 +#define PAC1711_MEAS_REG_SNAPSHOT_LEN 23 +#define PAC1711_ACC_REG_LEN 4 +#define PAC1711_VACC_REG_LEN 7 +#define PAC1711_VBUS_SENSE_REG_LEN 2 + +/* PRODUCT IDs */ +#define PAC_PRODUCT_ID_1711 0x80 + +/* CTRL reg */ +#define PAC1711_CTRL_SAMPLE_MODE_MASK GENMASK(15, 12) + +/* NEG_PWR_CFG reg */ +#define PAC1711_NEG_PWR_CFG_VS_MASK GENMASK(3, 2) +#define PAC1711_NEG_PWR_CFG_VB_MASK GENMASK(1, 0) +#define PAC1711_SAMPLING_MODE_MASK GENMASK(11, 0) + +#define PAC1711_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr) + +enum pac1711_ch_idx { + PAC1711_CH_POWER, + PAC1711_CH_VOLTAGE, + PAC1711_CH_CURRENT, + PAC1711_CH_VOLTAGE_AVERAGE, + PAC1711_CH_CURRENT_AVERAGE, +}; + +enum pac1711_acc_mode { + PAC1711_ACCMODE_VPOWER, + PAC1711_ACCMODE_VSENSE, + PAC1711_ACCMODE_VBUS, +}; + +enum pac1711_fsr { + PAC1711_FULL_RANGE_UNIPOLAR, + PAC1711_FULL_RANGE_BIPOLAR, + PAC1711_HALF_RANGE_BIPOLAR, +}; + +static const int pac1711_vbus_range_tbl[][2] = { + [PAC1711_FULL_RANGE_UNIPOLAR] = {0, 42000000}, + [PAC1711_FULL_RANGE_BIPOLAR] = {-42000000, 42000000}, + [PAC1711_HALF_RANGE_BIPOLAR] = {-21000000, 21000000}, +}; + +static const int pac1711_vsense_range_tbl[][2] = { + [PAC1711_FULL_RANGE_UNIPOLAR] = {0, 100000}, + [PAC1711_FULL_RANGE_BIPOLAR] = {-100000, 100000}, + [PAC1711_HALF_RANGE_BIPOLAR] = {-50000, 50000}, +}; + +enum pac1711_samps { + PAC1711_SAMP_8192SPS, + PAC1711_SAMP_4096SPS, + PAC1711_SAMP_1024SPS, + PAC1711_SAMP_256SPS, + PAC1711_SAMP_64SPS, + PAC1711_SAMP_8SPS, +}; + +static const unsigned 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, +}; + +/* Available Sample Modes */ +static const char * const pac1711_frequency_avail[] = { + "8192", + "4096", + "1024", + "256", + "64", + "8", +}; + +/** + * struct pac1711_features - features of a pac1711 instance + * @prod_id: hardware ID + * @name: chip's name + */ +struct pac1711_features { + u8 prod_id; + const char *name; +}; + +static const struct pac1711_features pac1711_chip_features = { + .name = "pac1711", + .prod_id = PAC_PRODUCT_ID_1711, +}; + +/** + * struct reg_data - data from the registers + * @meas_regs: snapshot of raw measurements registers + * @jiffies_tstamp: timestamp + * @total_samples_nr: total number of samples + * @ctrl_act_reg: the ctrl_act register + * @ctrl_lat_reg: the ctrl_lat register + * @vsense_avg: averages of vsense registers + * @acc_count: the acc_count register + * @vbus_avg: averages of vbus registers + * @acc_val: accumulated values per second + * @vpower: vpower registers + * @vsense: vsense registers + * @vacc: accumulated vpower value + * @vbus: vbus registers + */ +struct reg_data { + u8 meas_regs[PAC1711_MEAS_REG_SNAPSHOT_LEN]; + unsigned long jiffies_tstamp; + u32 total_samples_nr; + u16 ctrl_act_reg; + u16 ctrl_lat_reg; + s32 vsense_avg; + u32 acc_count; + s32 vbus_avg; + s64 acc_val; + s32 vpower; + s32 vsense; + s32 vbus; + s64 vacc; +}; + +/** + * struct pac1711_chip_info - information about the chip + * @accumulation_mode: accumulation mode for hardware accumulator + * @sample_rate_value: sampling frequency + * @work_chip_rfsh: work queue used for refresh commands + * @sampling_mode: sampling mode used by the device + * @chip_reg_data: chip reg data + * @chip_variant: chip variant + * @vsense_mode: Full Scale Range (FSR) mode for VSense + * @enable_acc: true means that accumulation channel is measured + * @vbus_mode: Full Scale Range (FSR) mode for VBus + * @iio_info: iio_info + * @client: the i2c-client attached to the device + * @label: channel label + * @shunt: shunt resistor value + * @lock: synchronize access to driver's state members + */ +struct pac1711_chip_info { + u8 accumulation_mode; + s32 sample_rate_value; + struct delayed_work work_chip_rfsh; + u8 sampling_mode; + struct reg_data chip_reg_data; + u8 chip_variant; + u8 vsense_mode; + bool enable_acc; + u8 vbus_mode; + struct iio_info iio_info; + struct i2c_client *client; + char *label; + u32 shunt; + struct mutex lock; /* synchronize access to driver's state members */ +}; + +static inline u64 pac1711_get_unaligned_be56(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 int pac1711_send_refresh(struct pac1711_chip_info *info, u8 refresh_cmd, + 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, refresh_cmd); + if (ret) { + dev_err(&client->dev, "%s - cannot send Refresh cmd (0x%02X)\n", + __func__, refresh_cmd); + return ret; + } + + /* Register data retrieval timestamp */ + info->chip_reg_data.jiffies_tstamp = jiffies; + /* Wait till the data is available */ + usleep_range(wait_time, wait_time + 100); + + return 0; +} + +static int pac1711_reg_snapshot(struct pac1711_chip_info *info, + bool do_refresh, u8 refresh_cmd, u32 wait_time) +{ + struct i2c_client *client = info->client; + struct device *dev = &client->dev; + s64 stored_value, tmp_s64; + u8 *offset_reg_data_p; + u32 count, inc_count; + bool is_bipolar; + s64 inc = 0; + u16 tmp_u16; + u8 shift; + int ret; + + guard(mutex)(&info->lock); + + if (do_refresh) { + ret = pac1711_send_refresh(info, refresh_cmd, 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(tmp_u16), (u8 *)&tmp_u16); + if (ret < 0) { + dev_err(dev, "%s - cannot read regs from 0x%02X\n", + __func__, PAC1711_CTRL_ACT_REG_ADDR); + return ret; + } + + be16_to_cpus(&tmp_u16); + info->chip_reg_data.ctrl_act_reg = tmp_u16; + + ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_LAT_REG_ADDR, + sizeof(tmp_u16), (u8 *)&tmp_u16); + if (ret < 0) { + dev_err(dev, "%s - cannot read regs from 0x%02X\n", + __func__, PAC1711_CTRL_LAT_REG_ADDR); + return ret; + } + + be16_to_cpus(&tmp_u16); + info->chip_reg_data.ctrl_lat_reg = tmp_u16; + + /* 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 < 0) { + dev_err(dev, "%s - cannot read regs from 0x%02X\n", + __func__, PAC1711_ACC_COUNT_REG_ADDR); + return ret; + } + + 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 accumulation is disabled */ + if (info->enable_acc) { + stored_value = info->chip_reg_data.acc_val; + + info->chip_reg_data.vacc = pac1711_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_VBUS: + if (info->vbus_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); + + if (info->accumulation_mode != PAC1711_ACCMODE_VBUS) { + /* VACC */ + /* + * Integrate the accumulated power or current over + * the elapsed interval. + */ + tmp_u16 = FIELD_GET(PAC1711_CTRL_SAMPLE_MODE_MASK, + info->chip_reg_data.ctrl_lat_reg); + tmp_s64 = info->chip_reg_data.vacc; + + if (tmp_u16 <= PAC1711_SAMP_8SPS) { + shift = pac1711_shift_map_tbl[tmp_u16]; + inc = tmp_s64 >> shift; + } else { + dev_err(dev, "Invalid sample rate index: %d!\n", tmp_u16); + } + } else { + count = info->chip_reg_data.total_samples_nr; + inc_count = info->chip_reg_data.acc_count; + + /* Check if total number of samples will overflow */ + if (check_add_overflow(count, inc_count, &count)) { + dev_err(dev, "Number of samples overflow!\n"); + info->chip_reg_data.total_samples_nr = 0; + info->chip_reg_data.acc_val = 0; + } + + info->chip_reg_data.total_samples_nr += inc_count; + inc = info->chip_reg_data.vacc; + } + + if (check_add_overflow(stored_value, inc, &stored_value)) { + if (is_negative(stored_value)) + info->chip_reg_data.acc_val = S64_MIN; + else + info->chip_reg_data.acc_val = S64_MAX; + + dev_err(dev, "Overflow detected!\n"); + } else { + info->chip_reg_data.acc_val += inc; + } + } + + offset_reg_data_p += PAC1711_VACC_REG_LEN; + + /* VBUS */ + info->chip_reg_data.vbus = get_unaligned_be16(offset_reg_data_p) >> 4; + + if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR) + info->chip_reg_data.vbus = sign_extend32(info->chip_reg_data.vbus, 11); + + offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN; + + /* VSENSE */ + info->chip_reg_data.vsense = get_unaligned_be16(offset_reg_data_p) >> 4; + + if (info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR) + info->chip_reg_data.vsense = sign_extend32(info->chip_reg_data.vsense, 11); + + offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN; + + /* VBUS_AVG */ + info->chip_reg_data.vbus_avg = get_unaligned_be16(offset_reg_data_p) >> 4; + + if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR) + info->chip_reg_data.vbus_avg = sign_extend32(info->chip_reg_data.vbus_avg, 11); + + offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN; + + /* VSENSE_AVG */ + info->chip_reg_data.vsense_avg = get_unaligned_be16(offset_reg_data_p) >> 4; + + if (info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR) + info->chip_reg_data.vsense_avg = sign_extend32(info->chip_reg_data.vsense_avg, 11); + + offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN; + + /* VPOWER */ + info->chip_reg_data.vpower = get_unaligned_be32(offset_reg_data_p) >> 8; + + if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR || + info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR) + info->chip_reg_data.vpower = sign_extend32(info->chip_reg_data.vpower, 23); + + return 0; +} + +static int pac1711_retrieve_data(struct pac1711_chip_info *info, + u32 wait_time) +{ + int ret; + + /* + * check if the minimal elapsed time has passed and if so, + * re-read the chip, otherwise the cached info is just fine + */ + if (time_after(jiffies, info->chip_reg_data.jiffies_tstamp + + msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS))) { + ret = pac1711_reg_snapshot(info, true, PAC1711_REFRESH_REG_ADDR, + wait_time); + + /* + * Re-schedule the work for the read registers timeout + * (to prevent chip regs saturation) + */ + cancel_delayed_work_sync(&info->work_chip_rfsh); + schedule_delayed_work(&info->work_chip_rfsh, + msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS)); + } + + return ret; +} + +static int pac1711_frequency_set(struct iio_dev *indio_dev, const struct iio_chan_spec *chan, + unsigned int mode) +{ + struct pac1711_chip_info *info = iio_priv(indio_dev); + struct i2c_client *client = info->client; + u16 tmp_u16; + int ret; + + guard(mutex)(&info->lock); + + ret = i2c_smbus_read_word_swapped(client, PAC1711_CTRL_ACT_REG_ADDR); + if (ret < 0) { + dev_err(&client->dev, "cannot read regs from 0x%02X\n", + PAC1711_CTRL_ACT_REG_ADDR); + return ret; + } + + tmp_u16 = ret; + tmp_u16 &= ~PAC1711_CTRL_SAMPLE_MODE_MASK; + tmp_u16 |= FIELD_PREP(PAC1711_CTRL_SAMPLE_MODE_MASK, mode); + + ret = i2c_smbus_write_word_swapped(client, PAC1711_CTRL_REG_ADDR, tmp_u16); + if (ret < 0) { + dev_err(&client->dev, "Failed to configure sampling mode\n"); + return ret; + } + + info->sampling_mode = mode; + info->chip_reg_data.ctrl_act_reg = tmp_u16; + + return pac1711_retrieve_data(info, PAC1711_MIN_UPDATE_WAIT_TIME_US); +} + +static int pac1711_frequency_get(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan) +{ + struct pac1711_chip_info *info; + + info = iio_priv(indio_dev); + + return info->sampling_mode; +} + +static const struct iio_enum sampling_mode_enum = { + .items = pac1711_frequency_avail, + .num_items = ARRAY_SIZE(pac1711_frequency_avail), + .set = pac1711_frequency_set, + .get = pac1711_frequency_get, +}; + +static const struct iio_chan_spec_ext_info pac1711_ext_info[] = { + IIO_ENUM("sampling_frequency", IIO_SHARED_BY_ALL, &sampling_mode_enum), + { + .name = "sampling_frequency_available", + .shared = IIO_SHARED_BY_ALL, + .read = iio_enum_available_read, + .private = (uintptr_t)&sampling_mode_enum, + }, + { } +}; + +#define TO_PAC1711_CHIP_INFO(d) container_of(d, struct pac1711_chip_info, work_chip_rfsh) + +#define PAC1711_VBUS_CHANNEL(_index, _si, _address) { \ + .type = IIO_VOLTAGE, \ + .address = (_address), \ + .indexed = 1, \ + .channel = (_index), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = (_si), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 12, \ + .storagebits = 16, \ + .shift = 4, \ + .endianness = IIO_CPU, \ + }, \ + .ext_info = pac1711_ext_info \ +} + +#define PAC1711_VBUS_AVG_CHANNEL(_index, _si, _address) { \ + .type = IIO_VOLTAGE, \ + .address = (_address), \ + .indexed = 1, \ + .channel = (_index) + 1, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_AVERAGE_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = (_si), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 12, \ + .storagebits = 16, \ + .shift = 4, \ + .endianness = IIO_CPU, \ + }, \ + .ext_info = pac1711_ext_info \ +} + +#define PAC1711_VSENSE_CHANNEL(_index, _si, _address) { \ + .type = IIO_CURRENT, \ + .address = (_address), \ + .indexed = 1, \ + .channel = (_index), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = (_si), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 12, \ + .storagebits = 16, \ + .shift = 4, \ + .endianness = IIO_CPU, \ + }, \ + .ext_info = pac1711_ext_info \ +} + +#define PAC1711_VSENSE_AVG_CHANNEL(_index, _si, _address) { \ + .type = IIO_CURRENT, \ + .address = (_address), \ + .indexed = 1, \ + .channel = (_index) + 1, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_AVERAGE_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = (_si), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 12, \ + .storagebits = 16, \ + .shift = 4, \ + .endianness = IIO_CPU, \ + }, \ + .ext_info = pac1711_ext_info \ +} + +#define PAC1711_VPOWER_CHANNEL(_index, _si, _address) { \ + .type = IIO_POWER, \ + .address = (_address), \ + .indexed = 1, \ + .channel = (_index), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = (_si), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 24, \ + .storagebits = 32, \ + .shift = 8, \ + .endianness = IIO_CPU, \ + }, \ + .ext_info = pac1711_ext_info \ +} + +static int pac1711_get_samp_rate_idx(struct pac1711_chip_info *info, u32 new_samp_rate) +{ + int cnt; + + for (cnt = 0; cnt < ARRAY_SIZE(pac1711_samp_rate_map_tbl); cnt++) + if (new_samp_rate == pac1711_samp_rate_map_tbl[cnt]) + return cnt; + + /* not a valid sample rate value */ + return -EINVAL; +} + +static ssize_t pac1711_shunt_value_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + + return sysfs_emit(buf, "%u\n", info->shunt); +} + +static ssize_t pac1711_shunt_value_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + int sh_val; + + if (kstrtouint(buf, 10, &sh_val)) { + dev_err(dev, "Shunt value is not valid\n"); + return -EINVAL; + } + + scoped_guard(mutex, &info->lock) + info->shunt = sh_val; + + return count; +} + +static ssize_t pac1711_in_power_acc_raw_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + s64 curr_energy, int_part; + int ret, rem; + + ret = pac1711_retrieve_data(info, PAC1711_MIN_UPDATE_WAIT_TIME_US); + if (ret < 0) + return 0; + + /* + * Expresses the 64 bit energy value as a + * 64 bit integer and a 32 bit nano value + */ + curr_energy = info->chip_reg_data.acc_val; + int_part = div_s64_rem(curr_energy, 1000000000, &rem); + + if (rem < 0) + return sysfs_emit(buf, "-%lld.%09u\n", abs(int_part), -rem); + else + return sysfs_emit(buf, "%lld.%09u\n", int_part, abs(rem)); +} + +static ssize_t pac1711_in_power_acc_scale_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + unsigned int rem; + u64 tmp, ref; + + ref = (u64)PAC1711_SCALE_CONSTANT; + tmp = div_u64(ref * 1000000000LL, info->shunt); + rem = do_div(tmp, 1000000000LL); + + return sysfs_emit(buf, "%lld.%09u\n", tmp, rem); +} + +static ssize_t pac1711_in_enable_acc_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + + return sysfs_emit(buf, "%d\n", info->enable_acc); +} + +static ssize_t pac1711_in_enable_acc_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + int val; + + if (kstrtouint(buf, 10, &val)) { + dev_err(dev, "Value is not valid\n"); + return -EINVAL; + } + + scoped_guard(mutex, &info->lock) { + info->enable_acc = val ? true : false; + if (!val) { + info->chip_reg_data.acc_val = 0; + info->chip_reg_data.total_samples_nr = 0; + } + } + + return count; +} + +static ssize_t pac1711_in_current_acc_raw_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + int ret; + + ret = pac1711_retrieve_data(info, PAC1711_MIN_UPDATE_WAIT_TIME_US); + if (ret < 0) + return 0; + + return sysfs_emit(buf, "%lld\n", info->chip_reg_data.acc_val); +} + +static ssize_t pac1711_in_current_acc_scale_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + u64 tmp_u64, ref; + int rem; + + /* + * Currents - scale for mA - depends on the channel's shunt value + * (100mV * 1000000) / (2^12 * shunt(uOhm)) + */ + ref = (u64)PAC1711_MAX_VSENSE_NANO; + + if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR) + ref = ref << 1; + + /* + * Increasing precision + * (100mV * 1000000 * 1000000000) / 2^12) + */ + tmp_u64 = div_u64(ref, info->shunt); + rem = do_div(tmp_u64, 1000000000LL); + + return sysfs_emit(buf, "%lld.%09u\n", tmp_u64, rem); +} + +static ssize_t pac1711_in_voltage_acc_raw_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + u32 samples_count; + s64 acc_voltage; + u64 tmp_u64; + int ret; + + ret = pac1711_retrieve_data(info, PAC1711_MIN_UPDATE_WAIT_TIME_US); + if (ret < 0) + return 0; + + acc_voltage = info->chip_reg_data.acc_val; + samples_count = info->chip_reg_data.total_samples_nr; + + tmp_u64 = div_u64(abs(acc_voltage), samples_count); + + if (is_negative(acc_voltage)) + return sysfs_emit(buf, "-%lld\n", tmp_u64); + else + return sysfs_emit(buf, "%lld\n", tmp_u64); +} + +static ssize_t pac1711_in_voltage_acc_scale_show(struct device *dev, struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct pac1711_chip_info *info = iio_priv(indio_dev); + unsigned long long tmp; + int vals[2]; + + vals[0] = PAC1711_VOLTAGE_MILLIVOLTS_MAX; + + if (info->vbus_mode == PAC1711_FULL_RANGE_BIPOLAR) + vals[1] = PAC1711_VOLTAGE_12B_RES; + else + /* PAC1711_FULL_RANGE_UNIPOLAR or PAC1711_HALF_RANGE_BIPOLAR */ + vals[1] = PAC1711_VOLTAGE_11B_RES; + + tmp = (s64)vals[0] * 1000000000LL >> vals[1]; + vals[1] = do_div(tmp, 1000000000LL); + vals[0] = tmp; + + return sysfs_emit(buf, "%d.%09u\n", vals[0], vals[1]); +} + +static IIO_DEVICE_ATTR(in_shunt_resistor, 0644, pac1711_shunt_value_show, + pac1711_shunt_value_store, 0); + +static struct attribute *pac1711_all_attrs[] = { + PAC1711_DEV_ATTR(in_shunt_resistor), + NULL +}; + +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; + u64 tmp; + + ret = pac1711_retrieve_data(info, PAC1711_MIN_UPDATE_WAIT_TIME_US); + if (ret < 0) + return ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + switch (chan->type) { + case IIO_VOLTAGE: + *val = info->chip_reg_data.vbus; + return IIO_VAL_INT; + case IIO_CURRENT: + *val = info->chip_reg_data.vsense; + return IIO_VAL_INT; + case IIO_POWER: + *val = info->chip_reg_data.vpower; + return IIO_VAL_INT; + default: + return -EINVAL; + } + case IIO_CHAN_INFO_AVERAGE_RAW: + switch (chan->type) { + case IIO_VOLTAGE: + *val = info->chip_reg_data.vbus_avg; + return IIO_VAL_INT; + case IIO_CURRENT: + *val = info->chip_reg_data.vsense_avg; + return IIO_VAL_INT; + default: + return -EINVAL; + } + case IIO_CHAN_INFO_SCALE: + switch (chan->address) { + /* Voltages - scale for millivolts */ + case PAC1711_VBUS_ADDR: + case PAC1711_VBUS_AVG_ADDR: + switch (info->chip_variant) { + case PAC_PRODUCT_ID_1711: + *val = PAC1711_VOLTAGE_MILLIVOLTS_MAX; + break; + default: + return -EINVAL; + } + switch (info->vbus_mode) { + case PAC1711_FULL_RANGE_UNIPOLAR: + case PAC1711_HALF_RANGE_BIPOLAR: + *val2 = PAC1711_VOLTAGE_12B_RES; + break; + case PAC1711_FULL_RANGE_BIPOLAR: + *val2 = PAC1711_VOLTAGE_11B_RES; + break; + default: + return -EINVAL; + } + return IIO_VAL_FRACTIONAL_LOG2; + /* + * Currents - scale for mA - depends on the + * channel's shunt value + * (100mV * 1000000) / (2^16 * shunt(uohm)) + */ + case PAC1711_VSENSE_ADDR: + case PAC1711_VSENSE_AVG_ADDR: + *val = PAC1711_MAX_VSENSE_RSHIFTED_BY_12B; + *val2 = info->shunt; + + if (info->vsense_mode == PAC1711_FULL_RANGE_BIPOLAR) + *val = *val << 1; + + return IIO_VAL_FRACTIONAL; + /* + * Power - uW - it will use the combined scale + * for current and voltage + * current(mA) * voltage(mV) = power (uW) + */ + case PAC1711_VPOWER_ADDR: + switch (info->chip_variant) { + case PAC_PRODUCT_ID_1711: + tmp = PAC1711_PRODUCT_VOLTAGE_PV_FSR; + break; + } + + do_div(tmp, info->shunt); + *val = (int)tmp; + if (info->vsense_mode == PAC1711_FULL_RANGE_UNIPOLAR && + info->vbus_mode == PAC1711_FULL_RANGE_UNIPOLAR) + *val2 = PAC1711_POWER_24B_RES; + else + *val2 = PAC1711_POWER_23B_RES; + + return IIO_VAL_FRACTIONAL_LOG2; + default: + return -EINVAL; + } + case IIO_CHAN_INFO_SAMP_FREQ: + *val = info->sample_rate_value; + 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 ret = -EINVAL; + s32 old_samp_rate; + u8 ctrl_reg; + + switch (mask) { + case IIO_CHAN_INFO_SAMP_FREQ: + ret = pac1711_get_samp_rate_idx(info, val); + if (ret < 0) + return ret; + + /* write the new sampling value and trigger a snapshot */ + scoped_guard(mutex, &info->lock) { + ctrl_reg = FIELD_PREP(PAC1711_CTRL_SAMPLE_MODE_MASK, ret); + ret = i2c_smbus_write_byte_data(client, PAC1711_CTRL_REG_ADDR, ctrl_reg); + if (ret) { + dev_err(dev, "%s - can't update sample rate\n", __func__); + return ret; + } + } + + old_samp_rate = info->sample_rate_value; + info->sample_rate_value = val; + + /* + * now, force a snapshot with refresh - call retrieve + * data in order to update the refresh timer + * alter the timestamp in order to force trigger a + * register snapshot and a timestamp update + */ + info->chip_reg_data.jiffies_tstamp -= msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS); + ret = pac1711_retrieve_data(info, (1024 / old_samp_rate) * 1000); + if (ret < 0) { + dev_err(dev, "%s - cannot snapshot ctrl and measurement regs\n", __func__); + return ret; + } + + return 0; + default: + return -EINVAL; + } +} + +static int pac1711_read_label(struct iio_dev *indio_dev, + struct iio_chan_spec const *chan, char *label) +{ + struct pac1711_chip_info *info = iio_priv(indio_dev); + + switch (chan->address) { + case PAC1711_VBUS_ADDR: + return sysfs_emit(label, "%s_VBUS\n", info->label); + case PAC1711_VBUS_AVG_ADDR: + return sysfs_emit(label, "%s_VBUS_AVG\n", info->label); + case PAC1711_VSENSE_ADDR: + return sysfs_emit(label, "%s_IBUS\n", info->label); + case PAC1711_VSENSE_AVG_ADDR: + return sysfs_emit(label, "%s_IBUS_AVG\n", info->label); + case PAC1711_VPOWER_ADDR: + return sysfs_emit(label, "%s_POWER\n", info->label); + } + + return 0; +} + +static void pac1711_work_periodic_rfsh(struct work_struct *work) +{ + struct pac1711_chip_info *info = TO_PAC1711_CHIP_INFO((struct delayed_work *)work); + struct device *dev = &info->client->dev; + + dev_dbg(dev, "%s - Periodic refresh\n", __func__); + + /* do a REFRESH, then read */ + pac1711_reg_snapshot(info, true, PAC1711_REFRESH_REG_ADDR, + PAC1711_MIN_UPDATE_WAIT_TIME_US); + + schedule_delayed_work(&info->work_chip_rfsh, + msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS)); +} + +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 < 0) + return dev_err_probe(&client->dev, ret, "cannot read product ID reg\n"); + + info->chip_variant = chip_rev_info[0]; + switch (info->chip_variant) { + case PAC_PRODUCT_ID_1711: + indio_dev->name = pac1711_chip_features.name; + break; + default: + return dev_err_probe(dev, -EINVAL, + "product ID (0x%02X, 0x%02X, 0x%02X) not recognized\n", + chip_rev_info[0], chip_rev_info[1], chip_rev_info[2]); + } + + return 0; +} + +static int pac1711_check_range(s32 *vals, const int ranges[][2], int num_ranges) +{ + int i; + + for (i = 0; i < num_ranges; i++) { + if (vals[0] == ranges[i][0] && vals[1] == ranges[i][1]) + return i; + } + + return -EINVAL; +} + +static int pac1711_setup_vbus_range(struct pac1711_chip_info *info) +{ + const char *prop_name = "microchip,vbus-input-range-microvolt"; + struct i2c_client *client = info->client; + struct device *dev = &client->dev; + unsigned int tbl_len, ret; + s32 vals[2]; + + /* default value is unipolar and Full Scale Range */ + ret = device_property_read_u32_array(dev, prop_name, vals, 2); + if (!ret) { + tbl_len = ARRAY_SIZE(pac1711_vbus_range_tbl); + ret = pac1711_check_range(vals, pac1711_vbus_range_tbl, tbl_len); + if (ret < 0) + return dev_err_probe(dev, -EINVAL, "Invalid value %d, %d for prop %s\n", + vals[0], vals[1], prop_name); + + info->vbus_mode = ret; + } + + return 0; +} + +static int pac1711_setup_vsense_range(struct pac1711_chip_info *info) +{ + const char *prop_name = "microchip,vsense-input-range-microvolt"; + struct i2c_client *client = info->client; + struct device *dev = &client->dev; + unsigned int tbl_len, ret; + s32 vals[2]; + + /* default value is unipolar and Full Scale Range */ + ret = device_property_read_u32_array(dev, prop_name, vals, 2); + if (!ret) { + tbl_len = ARRAY_SIZE(pac1711_vsense_range_tbl); + ret = pac1711_check_range(vals, pac1711_vsense_range_tbl, tbl_len); + if (ret < 0) + return dev_err_probe(dev, -EINVAL, "Invalid value %d, %d for prop %s\n", + vals[0], vals[1], prop_name); + + info->vsense_mode = ret; + } + + return 0; +} + +static bool pac1711_of_parse_channel_config(struct i2c_client *client, + struct pac1711_chip_info *info) +{ + struct device *dev = &client->dev; + int ret, temp; + + if (device_property_present(dev, "shunt-resistor-micro-ohms")) { + ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms", &info->shunt); + if (ret) + return dev_err_probe(dev, ret, "Write default shunt-resistor value %d\n", + info->shunt); + } else { + return dev_err_probe(dev, ret, "shunt-resistor property does not exist\n"); + } + + if (device_property_present(dev, "label")) { + ret = device_property_read_string(dev, "label", (const char **)&info->label); + if (ret) + return dev_err_probe(dev, ret, "invalid rail-name value\n"); + } + + ret = pac1711_setup_vbus_range(info); + if (ret) + return ret; + + ret = pac1711_setup_vsense_range(info); + if (ret) + return ret; + + ret = device_property_read_u32(dev, "microchip,accumulation-mode", &temp); + if (ret) + return dev_err_probe(dev, ret, "invalid accumulation-mode value %d\n", temp); + + if (temp == PAC1711_ACCMODE_VPOWER || temp == PAC1711_ACCMODE_VSENSE || + temp == PAC1711_ACCMODE_VBUS) { + dev_dbg(dev, "Accumulation mode set to: %d\n", temp); + info->accumulation_mode = temp; + } else { + return dev_err_probe(dev, -EINVAL, "invalid accumulation-mode value %d\n", temp); + } + + 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; + u32 wait_time; + u16 tmp_u16; + int ret = 0; + u8 tmp_u8; + + /* get sampling rate from PAC */ + ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_REG_ADDR, + sizeof(tmp_u16), (u8 *)&tmp_u16); + if (ret < 0) + return dev_err_probe(dev, ret, "cannot read 0x%02X reg\n", PAC1711_CTRL_REG_ADDR); + + be16_to_cpus(&tmp_u16); + info->sampling_mode = FIELD_GET(PAC1711_CTRL_SAMPLE_MODE_MASK, tmp_u16); + + /* + * The current/voltage can be measured unidirectional, bidirectional or half FSR + * no SLOW triggered REFRESH, clear POR + */ + tmp_u8 = FIELD_PREP(PAC1711_NEG_PWR_CFG_VS_MASK, info->vsense_mode) | + FIELD_PREP(PAC1711_NEG_PWR_CFG_VB_MASK, info->vbus_mode); + + ret = i2c_smbus_write_byte_data(client, PAC1711_NEG_PWR_FSR_REG_ADDR, tmp_u8); + if (ret) + return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n", + PAC1711_NEG_PWR_FSR_REG_ADDR); + + tmp_u8 = 0; + ret = i2c_smbus_write_byte_data(client, PAC1711_SLOW_REG_ADDR, tmp_u8); + if (ret < 0) + return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n", PAC1711_SLOW_REG_ADDR); + + tmp_u16 = 0; + cpu_to_be16s(&tmp_u16); + + ret = i2c_smbus_write_word_data(client, PAC1711_CTRL_REG_ADDR, tmp_u16); + 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) + return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n", + PAC1711_REFRESH_REG_ADDR); + + usleep_range(125, 500); + + /* + * 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 / pac1711_samp_rate_map_tbl[info->sampling_mode]) * 1000; + usleep_range(wait_time, wait_time + 100); + + INIT_DELAYED_WORK(&info->work_chip_rfsh, pac1711_work_periodic_rfsh); + /* Setup the latest moment for reading the regs before saturation */ + schedule_delayed_work(&info->work_chip_rfsh, + msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS)); + + return devm_add_action_or_reset(&client->dev, pac1711_cancel_delayed_work, + &info->work_chip_rfsh); +} + +static struct iio_chan_spec pac1711_single_channel[] = { + PAC1711_VPOWER_CHANNEL(0, 0, PAC1711_VPOWER_ADDR), + PAC1711_VBUS_CHANNEL(0, 0, PAC1711_VBUS_ADDR), + PAC1711_VSENSE_CHANNEL(0, 0, PAC1711_VSENSE_ADDR), + PAC1711_VBUS_AVG_CHANNEL(0, 0, PAC1711_VBUS_AVG_ADDR), + PAC1711_VSENSE_AVG_CHANNEL(0, 0, PAC1711_VSENSE_AVG_ADDR), +}; + +static IIO_DEVICE_ATTR(in_energy_raw, 0444, + pac1711_in_power_acc_raw_show, NULL, 0); + +static IIO_DEVICE_ATTR(in_energy_scale, 0444, + pac1711_in_power_acc_scale_show, NULL, 0); + +static IIO_DEVICE_ATTR(in_energy_en, 0644, + pac1711_in_enable_acc_show, pac1711_in_enable_acc_store, 0); + +static IIO_DEVICE_ATTR(in_current_acc_raw, 0444, + pac1711_in_current_acc_raw_show, NULL, 0); + +static IIO_DEVICE_ATTR(in_current_acc_scale, 0444, + pac1711_in_current_acc_scale_show, NULL, 0); + +static IIO_DEVICE_ATTR(in_current_acc_en, 0644, + pac1711_in_enable_acc_show, pac1711_in_enable_acc_store, 0); + +static IIO_DEVICE_ATTR(in_voltage_acc_raw, 0444, + pac1711_in_voltage_acc_raw_show, NULL, 0); + +static IIO_DEVICE_ATTR(in_voltage_acc_scale, 0444, + pac1711_in_voltage_acc_scale_show, NULL, 0); + +static IIO_DEVICE_ATTR(in_voltage_acc_en, 0644, + pac1711_in_enable_acc_show, pac1711_in_enable_acc_store, 0); + +static struct attribute *pac1711_power_acc_attr[] = { + PAC1711_DEV_ATTR(in_energy_raw), + PAC1711_DEV_ATTR(in_energy_scale), + PAC1711_DEV_ATTR(in_energy_en), + NULL +}; + +static struct attribute *pac1711_current_acc_attr[] = { + PAC1711_DEV_ATTR(in_current_acc_raw), + PAC1711_DEV_ATTR(in_current_acc_scale), + PAC1711_DEV_ATTR(in_current_acc_en), + NULL +}; + +static struct attribute *pac1711_voltage_acc_attr[] = { + PAC1711_DEV_ATTR(in_voltage_acc_raw), + PAC1711_DEV_ATTR(in_voltage_acc_scale), + PAC1711_DEV_ATTR(in_voltage_acc_en), + NULL +}; + +static int pac1711_prep_custom_attributes(struct pac1711_chip_info *info, + struct iio_dev *indio_dev) +{ + struct attribute **pac1711_custom_attrs, **tmp_attr; + struct device *dev = &info->client->dev; + struct attribute_group *pac1711_group; + int custom_attr_cnt, j, i; + + pac1711_group = devm_kzalloc(dev, sizeof(*pac1711_group), GFP_KERNEL); + if (!pac1711_group) + return -ENOMEM; + + custom_attr_cnt = PAC1711_COMMON_DEVATTR + PAC1711_ACC_DEVATTR; + + pac1711_custom_attrs = devm_kzalloc(dev, custom_attr_cnt * sizeof(*pac1711_group) + 1, + GFP_KERNEL); + + if (!pac1711_custom_attrs) + return -ENOMEM; + + j = 0; + + for (i = 0; i < PAC1711_COMMON_DEVATTR; i++) + pac1711_custom_attrs[j++] = pac1711_all_attrs[i]; + + switch (info->accumulation_mode) { + case PAC1711_ACCMODE_VPOWER: + tmp_attr = pac1711_power_acc_attr; + break; + case PAC1711_ACCMODE_VSENSE: + tmp_attr = pac1711_current_acc_attr; + break; + case PAC1711_ACCMODE_VBUS: + tmp_attr = pac1711_voltage_acc_attr; + break; + default: + return -EINVAL; + } + + pac1711_custom_attrs[j++] = tmp_attr[0]; + pac1711_custom_attrs[j++] = tmp_attr[1]; + pac1711_custom_attrs[j++] = tmp_attr[2]; + + pac1711_group->attrs = pac1711_custom_attrs; + info->iio_info.attrs = pac1711_group; + + return 0; +} + +static const struct iio_info pac1711_info = { + .read_raw = pac1711_read_raw, + .write_raw = pac1711_write_raw, + .read_label = pac1711_read_label, +}; + +static int pac1711_probe(struct i2c_client *client) +{ + const struct pac1711_features *chip; + struct device *dev = &client->dev; + struct pac1711_chip_info *info; + struct iio_dev *indio_dev; + 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 < 0) { + /* + * If failed to identify the hardware based on internal + * registers, try using fallback compatible in device tree + * to deal with some newer part number. + */ + chip = i2c_get_match_data(client); + if (!chip) + return -EINVAL; + + info->chip_variant = chip->prod_id; + indio_dev->name = chip->name; + } + + /* always start with accumulation channels enabled */ + info->enable_acc = true; + err = pac1711_of_parse_channel_config(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; + + /* + * do now any chip specific initialization (e.g. read/write + * some registers), enable/disable certain channels, change the sampling + * rate to the requested value + */ + ret = pac1711_chip_configure(info); + if (ret < 0) + return ret; + + /* prepare the channel information */ + indio_dev->num_channels = ARRAY_SIZE(pac1711_single_channel); + indio_dev->channels = pac1711_single_channel; + info->iio_info = pac1711_info; + indio_dev->info = &info->iio_info; + indio_dev->modes = INDIO_DIRECT_MODE; + + ret = pac1711_prep_custom_attributes(info, indio_dev); + if (ret < 0) + return dev_err_probe(dev, ret, + "Can't configure custom attributes for PAC1711 device\n"); + + /* + * read whatever has been accumulated in the chip so far + * and reset the accumulators + */ + ret = pac1711_reg_snapshot(info, true, PAC1711_REFRESH_REG_ADDR, + PAC1711_MIN_UPDATE_WAIT_TIME_US); + if (ret < 0) + return ret; + + ret = devm_iio_device_register(dev, indio_dev); + if (ret < 0) + 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 }, + {} +}; +MODULE_DEVICE_TABLE(i2c, pac1711_id); + +static const struct of_device_id pac1711_of_match[] = { + { + .compatible = "microchip,pac1711", + .data = &pac1711_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/Energy Monitor"); +MODULE_LICENSE("GPL"); -- 2.43.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] iio: adc: adding support for PAC1711 2025-10-15 10:12 ` [PATCH 2/2] " Ariana Lazar @ 2025-10-16 5:31 ` kernel test robot 2025-10-19 10:28 ` Jonathan Cameron 2025-10-23 7:31 ` Dan Carpenter 2 siblings, 0 replies; 11+ messages in thread From: kernel test robot @ 2025-10-16 5:31 UTC (permalink / raw) To: Ariana Lazar, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: llvm, oe-kbuild-all, linux-iio, devicetree, linux-kernel, Ariana Lazar Hi Ariana, kernel test robot noticed the following build warnings: [auto build test WARNING on 19272b37aa4f83ca52bdf9c16d5d81bdd1354494] url: https://github.com/intel-lab-lkp/linux/commits/Ariana-Lazar/dt-bindings-iio-adc-adding-support-for-PAC1711/20251016-002337 base: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 patch link: https://lore.kernel.org/r/20251015-pac1711-v1-2-976949e36367%40microchip.com patch subject: [PATCH 2/2] iio: adc: adding support for PAC1711 config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20251016/202510161332.ntymBLFJ-lkp@intel.com/config) compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251016/202510161332.ntymBLFJ-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202510161332.ntymBLFJ-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/iio/adc/pac1711.c:459:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 459 | if (time_after(jiffies, info->chip_reg_data.jiffies_tstamp + | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 460 | msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS))) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/jiffies.h:128:2: note: expanded from macro 'time_after' 128 | (typecheck(unsigned long, a) && \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 129 | typecheck(unsigned long, b) && \ | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 130 | ((long)((b) - (a)) < 0)) | ~~~~~~~~~~~~~~~~~~~~~~~~ drivers/iio/adc/pac1711.c:473:9: note: uninitialized use occurs here 473 | return ret; | ^~~ drivers/iio/adc/pac1711.c:459:2: note: remove the 'if' if its condition is always true 459 | if (time_after(jiffies, info->chip_reg_data.jiffies_tstamp + | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 460 | msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS))) { | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/iio/adc/pac1711.c:453:9: note: initialize the variable 'ret' to silence this warning 453 | int ret; | ^ | = 0 >> drivers/iio/adc/pac1711.c:1124:29: warning: variable 'ret' is uninitialized when used here [-Wuninitialized] 1124 | return dev_err_probe(dev, ret, "shunt-resistor property does not exist\n"); | ^~~ drivers/iio/adc/pac1711.c:1116:9: note: initialize the variable 'ret' to silence this warning 1116 | int ret, temp; | ^ | = 0 2 warnings generated. vim +459 drivers/iio/adc/pac1711.c 449 450 static int pac1711_retrieve_data(struct pac1711_chip_info *info, 451 u32 wait_time) 452 { 453 int ret; 454 455 /* 456 * check if the minimal elapsed time has passed and if so, 457 * re-read the chip, otherwise the cached info is just fine 458 */ > 459 if (time_after(jiffies, info->chip_reg_data.jiffies_tstamp + 460 msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS))) { 461 ret = pac1711_reg_snapshot(info, true, PAC1711_REFRESH_REG_ADDR, 462 wait_time); 463 464 /* 465 * Re-schedule the work for the read registers timeout 466 * (to prevent chip regs saturation) 467 */ 468 cancel_delayed_work_sync(&info->work_chip_rfsh); 469 schedule_delayed_work(&info->work_chip_rfsh, 470 msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS)); 471 } 472 473 return ret; 474 } 475 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] iio: adc: adding support for PAC1711 2025-10-15 10:12 ` [PATCH 2/2] " Ariana Lazar 2025-10-16 5:31 ` kernel test robot @ 2025-10-19 10:28 ` Jonathan Cameron 2025-10-23 7:31 ` Dan Carpenter 2 siblings, 0 replies; 11+ messages in thread From: Jonathan Cameron @ 2025-10-19 10:28 UTC (permalink / raw) To: Ariana Lazar Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel On Wed, 15 Oct 2025 13:12:16 +0300 Ariana Lazar <ariana.lazar@microchip.com> wrote: > This is the iio driver for Microchip PAC1711 single-channel power monitor > with accumulator. The device uses 12-bit resolution for voltage and current > measurements and 24 bits power calculations. The device supports one 56-bit > accumulator register. > > PAC1711 measures up to 42V Full-Scale Range. > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> See comments on hwmon / iio in reply to the cover letter. I'll review this anyway as may save some time either way. Biggest questions are around what ABI is actually useful to present to userspace. Often devices do things like providing voltage accumulation but in practice that's only because it was easy to build the hardware, not because anyone actually wants it. We are safer to not introduce ABI if unsure, and wait for the usecase to become clear. New ABI has a significant burden in terms of tooling updates and long term maintenance. Thanks, Jonathan > --- > .../ABI/testing/sysfs-bus-iio-adc-pac1711 | 57 + > MAINTAINERS | 2 + > drivers/iio/adc/Kconfig | 10 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/pac1711.c | 1448 ++++++++++++++++++++ > 5 files changed, 1518 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-iio-adc-pac1711 b/Documentation/ABI/testing/sysfs-bus-iio-adc-pac1711 > new file mode 100644 > index 0000000000000000000000000000000000000000..7f6ab50d29ff064d57b80df0a0c162a4c98764f8 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-iio-adc-pac1711 > @@ -0,0 +1,57 @@ > +What: /sys/bus/iio/devices/iio:deviceX/in_shunt_resistor > +KernelVersion: 6.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + 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. X is the IIO index of the device. The value > + is used to calculate current, power and accumulated energy. This is standard IIO ABI. We don't duplicate documentation so no entry here. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_current_acc_raw > +KernelVersion: 6.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to read the accumulated voltage measured > + on the shunt resistor (Coulomb counter). Units after application > + of scale are mA. X is the IIO index of the device. As you mention it is counting charge. So not a current measurement at all. We don't currently have a charge type so we should add one. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_current_acc_scale > +KernelVersion: 6.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + If known for a device, scale to be applied to in_current_raw in > + order to obtain the measured value in mA units. X is the IIO > + index of the device. It's not because it is accumulated so with approximation of charge = sumof delta t * instantaneous current it should be in coulombs. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_current_acc_en > +KernelVersion: 6.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute, if available, is used to enable digital > + accumulation of VSENSE measurements. X is the IIO index of the > + device. Other than the type this is standard ABI for channel types that count / accumulate. So should be a new entry in the _en bit in sysfs-bus-ii > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_acc_raw Voltage integrals are a weirder thing as they don't in of themselves represent anything in particular. I am a bit curious on why anyone wants these except as part of computing something else. Still we can define a new channel type for this if we have a bit more on why it is useful. It might be more useful presented as an average voltage by dividing by number of samples.. (the datasheet calls this out as a possible usecase) The device also has short term averaging so I'm not sure how useful this is. It might not make sense to present this one at all. > +KernelVersion: 6.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute is used to read the accumulated voltage on VBUS. > + Units after application of scale are mV. X is the IIO index of > + the device. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_acc_scale > +KernelVersion: 6.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + If known for a device, scale to be applied to in_voltage_raw > + in order to obtain the measured value in mV units. X is the IIO > + index of the device. > + > +What: /sys/bus/iio/devices/iio:deviceX/in_voltage_acc_en > +KernelVersion: 6.18 > +Contact: linux-iio@vger.kernel.org > +Description: > + This attribute, if available, is used to enable digital > + accumulation of VBUS measurements. X is the IIO index of the > + device. > + > diff --git a/drivers/iio/adc/pac1711.c b/drivers/iio/adc/pac1711.c > new file mode 100644 > index 0000000000000000000000000000000000000000..30929af202e009c2d80ba91d5c137a26a03f09d7 > --- /dev/null > +++ b/drivers/iio/adc/pac1711.c > @@ -0,0 +1,1448 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * IIO driver for PAC1711 Multi-Channel DC Power/Energy Monitor > + * > + * Copyright (C) 2024 Microchip Technology Inc. and its subsidiaries Should include 2025 given this is going to change. > + * > + * Author: Ariana Lazar <ariana.lazar@microchip.com> > + * > + */ > +#include <linux/bitfield.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/i2c.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > +#include <linux/unaligned.h> There are definitely some headers missing here. mutex.h for starters. Aim to follow principles of include what you used. It is slightly fuzzy for the kernel so do check that headers you add are commonly included in drivers. A few have standard 'grouping' headers that are always used instead. > + > +/* voltage bits resolution when set for unsigned values */ > +#define PAC1711_VOLTAGE_12B_RES 12 > +/* voltage bits resolution when set for signed values */ > +#define PAC1711_VOLTAGE_11B_RES 11 > + > +/* Power resolution is 24 bits when contains unsigned values */ > +#define PAC1711_POWER_24B_RES 24 > +/* Power resolution is 24 bits when contains signed values */ > +#define PAC1711_POWER_23B_RES 23 Defining something called 24 to the value 24 doesn't seem productive. Just use 24 / 23 inline as their meaning should be clear. > + > +/* Maximum power-product value - 42 V * 0.1 V */ > +#define PAC1711_PRODUCT_VOLTAGE_PV_FSR 4200000000000UL > + > +/* Scale constant = (10^3 * 4.2 * 10^9 / 2^24) for mili Watt-second */ > +#define PAC1711_SCALE_CONSTANT 250340 > + > +/* (100mV * 1000000) / (2^12) used to calculate the scale for current */ > +#define PAC1711_MAX_VSENSE_RSHIFTED_BY_12B 24414 > + > +/* > + * [(100mV * 1000000) / (2^12)]*10^9 used to calculate the scale > + * for accumulated current/Coulomb counter I'd have this comment and the value down in the code. Not seeing huge value from having it up here as it is only used once. Check whehter the other defines benefit form up here. > + */ > +#define PAC1711_MAX_VSENSE_NANO 24414062500000UL > + > +/* I2C address map */ > +#define PAC1711_REFRESH_REG_ADDR 0x00 > +#define PAC1711_CTRL_REG_ADDR 0x01 > +#define PAC1711_ACC_COUNT_REG_ADDR 0x02 > +#define PAC1711_VACC_ADDR 0x03 > +#define PAC1711_VBUS_ADDR 0x04 > +#define PAC1711_VSENSE_ADDR 0x05 > +#define PAC1711_VBUS_AVG_ADDR 0x06 > +#define PAC1711_VSENSE_AVG_ADDR 0x07 > +#define PAC1711_VPOWER_ADDR 0x08 > + > +/* Start of configurations registers */ > +#define PAC1711_CTRL_LAT_REG_ADDR 0x0F > +#define PAC1711_NEG_PWR_FSR_REG_ADDR 0x13 > +#define PAC1711_SLOW_REG_ADDR 0x16 > +#define PAC1711_CTRL_ACT_REG_ADDR 0x17 > +#define PAC1711_PID_REG_ADDR 0xFD > +#define PAC1711_REVISION_ID_REG_ADDR 0xFF > + > +#define PAC1711_ACC_DEVATTR 3 Used in only a few places and better established via ARRAY_SIZE() or similar. > +#define PAC1711_COMMON_DEVATTR 1 > +#define PAC1711_MEAS_REG_SNAPSHOT_LEN 23 Can we derive that from register addresses. If not add a comment on why it is this. > +#define PAC1711_ACC_REG_LEN 4 Likewise. These look like values that should be derived not just encoded as numbers here. > +#define PAC1711_VACC_REG_LEN 7 > +#define PAC1711_VBUS_SENSE_REG_LEN 2 > + > +/* PRODUCT IDs */ > +#define PAC_PRODUCT_ID_1711 0x80 > + > +/* CTRL reg */ Which is this? There isn't a register simply called CTRL. I think it is CTRL_LAT_REG > +#define PAC1711_CTRL_SAMPLE_MODE_MASK GENMASK(15, 12) > + > +/* NEG_PWR_CFG reg */ There isn't one called that above. > +#define PAC1711_NEG_PWR_CFG_VS_MASK GENMASK(3, 2) > +#define PAC1711_NEG_PWR_CFG_VB_MASK GENMASK(1, 0) I'd group fields just under the register address definition. Makes it clear what they are for and avoids need for comments. > +#define PAC1711_SAMPLING_MODE_MASK GENMASK(11, 0) > + > +#define PAC1711_DEV_ATTR(name) (&iio_dev_attr_##name.dev_attr.attr) > + > +enum pac1711_ch_idx { > + PAC1711_CH_POWER, > + PAC1711_CH_VOLTAGE, > + PAC1711_CH_CURRENT, > + PAC1711_CH_VOLTAGE_AVERAGE, > + PAC1711_CH_CURRENT_AVERAGE, > +}; > + > +enum pac1711_acc_mode { > + PAC1711_ACCMODE_VPOWER, > + PAC1711_ACCMODE_VSENSE, > + PAC1711_ACCMODE_VBUS, > +}; > + > +enum pac1711_fsr { > + PAC1711_FULL_RANGE_UNIPOLAR, > + PAC1711_FULL_RANGE_BIPOLAR, > + PAC1711_HALF_RANGE_BIPOLAR, > +}; > + > +static const int pac1711_vbus_range_tbl[][2] = { > + [PAC1711_FULL_RANGE_UNIPOLAR] = {0, 42000000}, > + [PAC1711_FULL_RANGE_BIPOLAR] = {-42000000, 42000000}, > + [PAC1711_HALF_RANGE_BIPOLAR] = {-21000000, 21000000}, > +}; > + > +static const int pac1711_vsense_range_tbl[][2] = { > + [PAC1711_FULL_RANGE_UNIPOLAR] = {0, 100000}, > + [PAC1711_FULL_RANGE_BIPOLAR] = {-100000, 100000}, > + [PAC1711_HALF_RANGE_BIPOLAR] = {-50000, 50000}, Spacing convention in IIO that we are slowly moving old code to as well is space after { and before } > +}; > + > +enum pac1711_samps { > + PAC1711_SAMP_8192SPS, Given I think these map to register values in the hardware, I'd prefer to see them assigned values = 0, etc Same for all similar cases. > + PAC1711_SAMP_4096SPS, > + PAC1711_SAMP_1024SPS, > + PAC1711_SAMP_256SPS, > + PAC1711_SAMP_64SPS, > + PAC1711_SAMP_8SPS, > +}; > + > +static const unsigned int pac1711_samp_rate_map_tbl[] = { > + [PAC1711_SAMP_8192SPS] = 8192, > + [PAC1711_SAMP_4096SPS] = 4096, > + [PAC1711_SAMP_1024SPS] = 1024, // default With read_avail() callback you should be able to use this array to provide the information. > + [PAC1711_SAMP_256SPS] = 256, > + [PAC1711_SAMP_64SPS] = 64, > + [PAC1711_SAMP_8SPS] = 8, > +}; > + > +static const unsigned int pac1711_shift_map_tbl[] = { Add a comment on what this is. > + [PAC1711_SAMP_8192SPS] = 13, > + [PAC1711_SAMP_4096SPS] = 12, > + [PAC1711_SAMP_1024SPS] = 10, > + [PAC1711_SAMP_256SPS] = 8, > + [PAC1711_SAMP_64SPS] = 6, > + [PAC1711_SAMP_8SPS] = 3, > +}; > + > +/* Available Sample Modes */ > +static const char * const pac1711_frequency_avail[] = { > + "8192", > + "4096", > + "1024", > + "256", > + "64", > + "8", > +}; > + > +/** > + * struct pac1711_features - features of a pac1711 instance # I mention it below. We don't normally do this until we have multiple chips supported. So I'd rip it out for now and put the values inline. > + * @prod_id: hardware ID > + * @name: chip's name > + */ > +struct pac1711_features { > + u8 prod_id; > + const char *name; > +}; > + > +static const struct pac1711_features pac1711_chip_features = { > + .name = "pac1711", > + .prod_id = PAC_PRODUCT_ID_1711, > +}; > + > +/** > + * struct reg_data - data from the registers > + * @meas_regs: snapshot of raw measurements registers > + * @jiffies_tstamp: timestamp > + * @total_samples_nr: total number of samples > + * @ctrl_act_reg: the ctrl_act register > + * @ctrl_lat_reg: the ctrl_lat register > + * @vsense_avg: averages of vsense registers > + * @acc_count: the acc_count register > + * @vbus_avg: averages of vbus registers > + * @acc_val: accumulated values per second > + * @vpower: vpower registers > + * @vsense: vsense registers > + * @vacc: accumulated vpower value > + * @vbus: vbus registers > + */ > +struct reg_data { > + u8 meas_regs[PAC1711_MEAS_REG_SNAPSHOT_LEN]; > + unsigned long jiffies_tstamp; > + u32 total_samples_nr; > + u16 ctrl_act_reg; > + u16 ctrl_lat_reg; > + s32 vsense_avg; > + u32 acc_count; > + s32 vbus_avg; > + s64 acc_val; > + s32 vpower; > + s32 vsense; > + s32 vbus; > + s64 vacc; As below. Reorder these to improve packing + group related entries. > +}; > + > +/** > + * struct pac1711_chip_info - information about the chip > + * @accumulation_mode: accumulation mode for hardware accumulator > + * @sample_rate_value: sampling frequency > + * @work_chip_rfsh: work queue used for refresh commands > + * @sampling_mode: sampling mode used by the device > + * @chip_reg_data: chip reg data > + * @chip_variant: chip variant > + * @vsense_mode: Full Scale Range (FSR) mode for VSense > + * @enable_acc: true means that accumulation channel is measured > + * @vbus_mode: Full Scale Range (FSR) mode for VBus > + * @iio_info: iio_info > + * @client: the i2c-client attached to the device > + * @label: channel label > + * @shunt: shunt resistor value > + * @lock: synchronize access to driver's state members > + */ > +struct pac1711_chip_info { > + u8 accumulation_mode; Don't do reverse xmas tree for structure elements! The packing looks terrible. Order them first into logical related groups, then order such that packing is efficient. Pahole or a glance at rules for packing in C will help with that. > + s32 sample_rate_value; > + struct delayed_work work_chip_rfsh; > + u8 sampling_mode; > + struct reg_data chip_reg_data; > + u8 chip_variant; > + u8 vsense_mode; > + bool enable_acc; > + u8 vbus_mode; > + struct iio_info iio_info; > + struct i2c_client *client; > + char *label; > + u32 shunt; > + struct mutex lock; /* synchronize access to driver's state members */ Be more specific on this comment. Which members and why. Is it to keep them inline with hardware settings, or are there complex things that need to be done atomically? > +}; > + > +static inline u64 pac1711_get_unaligned_be56(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]; Hmm. Maybe this one is reasonable to add to the main unaligned function set. Perhaps not yet as a grep didn't find multiple of this size. > +} > + > +static int pac1711_send_refresh(struct pac1711_chip_info *info, u8 refresh_cmd, > + 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, refresh_cmd); > + if (ret) { > + dev_err(&client->dev, "%s - cannot send Refresh cmd (0x%02X)\n", > + __func__, refresh_cmd); > + return ret; > + } > + > + /* Register data retrieval timestamp */ > + info->chip_reg_data.jiffies_tstamp = jiffies; > + /* Wait till the data is available */ fsleep() for waits that are fuzzy like this one. It adds standard level of fuzz and means we don't need to think about what is appropriate. > + usleep_range(wait_time, wait_time + 100); > + > + return 0; > +} > + > +static int pac1711_reg_snapshot(struct pac1711_chip_info *info, > + bool do_refresh, u8 refresh_cmd, u32 wait_time) > +{ > + struct i2c_client *client = info->client; > + struct device *dev = &client->dev; > + s64 stored_value, tmp_s64; > + u8 *offset_reg_data_p; > + u32 count, inc_count; > + bool is_bipolar; > + s64 inc = 0; > + u16 tmp_u16; > + u8 shift; > + int ret; > + > + guard(mutex)(&info->lock); > + > + if (do_refresh) { > + ret = pac1711_send_refresh(info, refresh_cmd, 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(tmp_u16), (u8 *)&tmp_u16); > + if (ret < 0) { > + dev_err(dev, "%s - cannot read regs from 0x%02X\n", > + __func__, PAC1711_CTRL_ACT_REG_ADDR); > + return ret; > + } > + > + be16_to_cpus(&tmp_u16); As below. Use a local __be16 to keep the meaning of each local variable cleaner. > + info->chip_reg_data.ctrl_act_reg = tmp_u16; > + > + ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_LAT_REG_ADDR, > + sizeof(tmp_u16), (u8 *)&tmp_u16); > + if (ret < 0) { > + dev_err(dev, "%s - cannot read regs from 0x%02X\n", > + __func__, PAC1711_CTRL_LAT_REG_ADDR); > + return ret; > + } > + > + be16_to_cpus(&tmp_u16); Same as above. > + info->chip_reg_data.ctrl_lat_reg = tmp_u16; > + > + /* 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 < 0) { > + dev_err(dev, "%s - cannot read regs from 0x%02X\n", > + __func__, PAC1711_ACC_COUNT_REG_ADDR); > + return ret; > + } > + > + 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 accumulation is disabled */ > + if (info->enable_acc) { > + stored_value = info->chip_reg_data.acc_val; > + > + info->chip_reg_data.vacc = pac1711_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_VBUS: > + if (info->vbus_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); > + > + if (info->accumulation_mode != PAC1711_ACCMODE_VBUS) { > + /* VACC */ > + /* > + * Integrate the accumulated power or current over > + * the elapsed interval. > + */ > + tmp_u16 = FIELD_GET(PAC1711_CTRL_SAMPLE_MODE_MASK, > + info->chip_reg_data.ctrl_lat_reg); > + tmp_s64 = info->chip_reg_data.vacc; > + > + if (tmp_u16 <= PAC1711_SAMP_8SPS) { > + shift = pac1711_shift_map_tbl[tmp_u16]; > + inc = tmp_s64 >> shift; > + } else { > + dev_err(dev, "Invalid sample rate index: %d!\n", tmp_u16); return an error if this happens and give up. I'm guessing it's not supposed to happen. > + } > + } else { > + count = info->chip_reg_data.total_samples_nr; > + inc_count = info->chip_reg_data.acc_count; > + > + /* Check if total number of samples will overflow */ > + if (check_add_overflow(count, inc_count, &count)) { > + dev_err(dev, "Number of samples overflow!\n"); As above. If we get an error condition I'd communicate it up with an error return value, not just something stashed in the kernel log. > + info->chip_reg_data.total_samples_nr = 0; > + info->chip_reg_data.acc_val = 0; > + } > + > + info->chip_reg_data.total_samples_nr += inc_count; > + inc = info->chip_reg_data.vacc; > + } > + > + if (check_add_overflow(stored_value, inc, &stored_value)) { > + if (is_negative(stored_value)) > + info->chip_reg_data.acc_val = S64_MIN; > + else > + info->chip_reg_data.acc_val = S64_MAX; > + > + dev_err(dev, "Overflow detected!\n"); and again. These conditions are not supposed to happen. Make them really obvious. > + } else { > + info->chip_reg_data.acc_val += inc; > + } > + } > + > + offset_reg_data_p += PAC1711_VACC_REG_LEN; > + > + /* VBUS */ > + info->chip_reg_data.vbus = get_unaligned_be16(offset_reg_data_p) >> 4; > + > + if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR) > + info->chip_reg_data.vbus = sign_extend32(info->chip_reg_data.vbus, 11); > + > + offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN; > + > + /* VSENSE */ > + info->chip_reg_data.vsense = get_unaligned_be16(offset_reg_data_p) >> 4; > + > + if (info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR) > + info->chip_reg_data.vsense = sign_extend32(info->chip_reg_data.vsense, 11); > + > + offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN; > + > + /* VBUS_AVG */ > + info->chip_reg_data.vbus_avg = get_unaligned_be16(offset_reg_data_p) >> 4; > + > + if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR) > + info->chip_reg_data.vbus_avg = sign_extend32(info->chip_reg_data.vbus_avg, 11); > + > + offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN; > + > + /* VSENSE_AVG */ > + info->chip_reg_data.vsense_avg = get_unaligned_be16(offset_reg_data_p) >> 4; > + > + if (info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR) > + info->chip_reg_data.vsense_avg = sign_extend32(info->chip_reg_data.vsense_avg, 11); > + > + offset_reg_data_p += PAC1711_VBUS_SENSE_REG_LEN; > + > + /* VPOWER */ > + info->chip_reg_data.vpower = get_unaligned_be32(offset_reg_data_p) >> 8; > + > + if (info->vbus_mode != PAC1711_FULL_RANGE_UNIPOLAR || > + info->vsense_mode != PAC1711_FULL_RANGE_UNIPOLAR) > + info->chip_reg_data.vpower = sign_extend32(info->chip_reg_data.vpower, 23); > + > + return 0; > +} > + > +static const struct iio_chan_spec_ext_info pac1711_ext_info[] = { > + IIO_ENUM("sampling_frequency", IIO_SHARED_BY_ALL, &sampling_mode_enum), Use the info_mask_shared_by_all for sampling frequency. > + { > + .name = "sampling_frequency_available", read_avail() callback and appropriate mask for this. we have had core support for generating these attributes for a while now. > + .shared = IIO_SHARED_BY_ALL, > + .read = iio_enum_available_read, > + .private = (uintptr_t)&sampling_mode_enum, > + }, > + { } > +}; > +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; > + u64 tmp; > + > + ret = pac1711_retrieve_data(info, PAC1711_MIN_UPDATE_WAIT_TIME_US); > + if (ret < 0) > + return ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + switch (chan->type) { > + case IIO_VOLTAGE: > + *val = info->chip_reg_data.vbus; > + return IIO_VAL_INT; > + case IIO_CURRENT: > + *val = info->chip_reg_data.vsense; > + return IIO_VAL_INT; > + case IIO_POWER: > + *val = info->chip_reg_data.vpower; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > + case IIO_CHAN_INFO_AVERAGE_RAW: > + switch (chan->type) { > + case IIO_VOLTAGE: > + *val = info->chip_reg_data.vbus_avg; Mentioned elsewhere but from practical usecase point of view it is very rare that rolling average and unaveraged at the same time is actually useful. Providing the rolling average only + description of filter parameters that reflect the boxcar filter usually ends up being the cleaner interface. If you have a usecase that needs both then we can figure out how to do it cleanly. > + return IIO_VAL_INT; > + case IIO_CURRENT: > + *val = info->chip_reg_data.vsense_avg; > + 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 ret = -EINVAL; set in all paths that use it so no need to initialize here. > + s32 old_samp_rate; > + u8 ctrl_reg; > + > + switch (mask) { > + case IIO_CHAN_INFO_SAMP_FREQ: > + ret = pac1711_get_samp_rate_idx(info, val); > + if (ret < 0) > + return ret; > + > + /* write the new sampling value and trigger a snapshot */ > + scoped_guard(mutex, &info->lock) { > + ctrl_reg = FIELD_PREP(PAC1711_CTRL_SAMPLE_MODE_MASK, ret); > + ret = i2c_smbus_write_byte_data(client, PAC1711_CTRL_REG_ADDR, ctrl_reg); > + if (ret) { > + dev_err(dev, "%s - can't update sample rate\n", __func__); > + return ret; > + } > + } > + > + old_samp_rate = info->sample_rate_value; > + info->sample_rate_value = val; > + > + /* > + * now, force a snapshot with refresh - call retrieve > + * data in order to update the refresh timer > + * alter the timestamp in order to force trigger a > + * register snapshot and a timestamp update > + */ > + info->chip_reg_data.jiffies_tstamp -= msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS); > + ret = pac1711_retrieve_data(info, (1024 / old_samp_rate) * 1000); > + if (ret < 0) { > + dev_err(dev, "%s - cannot snapshot ctrl and measurement regs\n", __func__); > + return ret; > + } > + > + return 0; > + default: > + return -EINVAL; > + } > +} > + > +static int pac1711_read_label(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, char *label) > +{ > + struct pac1711_chip_info *info = iio_priv(indio_dev); > + > + switch (chan->address) { > + case PAC1711_VBUS_ADDR: > + return sysfs_emit(label, "%s_VBUS\n", info->label); I'm not sure I see the advantage in the postfixes. This seems to be building channel labels from the device label. If you want channel labels, provide the channel subnodes and put the label under that. If you want a device label set indio_dev->label > + case PAC1711_VBUS_AVG_ADDR: > + return sysfs_emit(label, "%s_VBUS_AVG\n", info->label); oh goody. Moving averages. These are a pain to support cleanly. They sound simple but they overlap too much with more complex forms of data processing. Typically a user either wants the smoothed version or the non smoothed one so we don't necessarily need separate channels. An alternative is to provide a filter control via the 3dB point of the boxcar filter that an average is. > + case PAC1711_VSENSE_ADDR: > + return sysfs_emit(label, "%s_IBUS\n", info->label); > + case PAC1711_VSENSE_AVG_ADDR: > + return sysfs_emit(label, "%s_IBUS_AVG\n", info->label); > + case PAC1711_VPOWER_ADDR: > + return sysfs_emit(label, "%s_POWER\n", info->label); > + } > + > + return 0; > +} > + > +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 < 0) > + return dev_err_probe(&client->dev, ret, "cannot read product ID reg\n"); > + > + info->chip_variant = chip_rev_info[0]; > + switch (info->chip_variant) { > + case PAC_PRODUCT_ID_1711: > + indio_dev->name = pac1711_chip_features.name; > + break; return 0 here so we know nothing else to look for after thi. > + default: > + return dev_err_probe(dev, -EINVAL, dev_info() / dev_warn() only as we are going to carry on anyway. > + "product ID (0x%02X, 0x%02X, 0x%02X) not recognized\n", > + chip_rev_info[0], chip_rev_info[1], chip_rev_info[2]); > + } > + > + return 0; > +} > +static int pac1711_setup_vbus_range(struct pac1711_chip_info *info) > +{ > + const char *prop_name = "microchip,vbus-input-range-microvolt"; > + struct i2c_client *client = info->client; > + struct device *dev = &client->dev; > + unsigned int tbl_len, ret; > + s32 vals[2]; > + > + /* default value is unipolar and Full Scale Range */ > + ret = device_property_read_u32_array(dev, prop_name, vals, 2); > + if (!ret) { > + tbl_len = ARRAY_SIZE(pac1711_vbus_range_tbl); > + ret = pac1711_check_range(vals, pac1711_vbus_range_tbl, tbl_len); > + if (ret < 0) > + return dev_err_probe(dev, -EINVAL, "Invalid value %d, %d for prop %s\n", > + vals[0], vals[1], prop_name); > + > + info->vbus_mode = ret; > + } > + > + return 0; > +} > + > +static int pac1711_setup_vsense_range(struct pac1711_chip_info *info) > +{ > + const char *prop_name = "microchip,vsense-input-range-microvolt"; Either figure out how to generalize this code to share with the function above, or just put prop_name string inline. It is a bit marginal on whether it is worth generalizing. > + struct i2c_client *client = info->client; > + struct device *dev = &client->dev; > + unsigned int tbl_len, ret; > + s32 vals[2]; > + > + /* default value is unipolar and Full Scale Range */ > + ret = device_property_read_u32_array(dev, prop_name, vals, 2); > + if (!ret) { > + tbl_len = ARRAY_SIZE(pac1711_vsense_range_tbl); > + ret = pac1711_check_range(vals, pac1711_vsense_range_tbl, tbl_len); > + if (ret < 0) > + return dev_err_probe(dev, -EINVAL, "Invalid value %d, %d for prop %s\n", > + vals[0], vals[1], prop_name); > + > + info->vsense_mode = ret; > + } > + > + return 0; > +} > + > +static bool pac1711_of_parse_channel_config(struct i2c_client *client, > + struct pac1711_chip_info *info) > +{ > + struct device *dev = &client->dev; > + int ret, temp; > + > + if (device_property_present(dev, "shunt-resistor-micro-ohms")) { > + ret = device_property_read_u32(dev, "shunt-resistor-micro-ohms", &info->shunt); checking for presence before reading is pointless. device_property_read_u32() will error out if it isn't there anyway. The error code will indicate why. > + if (ret) > + return dev_err_probe(dev, ret, "Write default shunt-resistor value %d\n", > + info->shunt); > + } else { > + return dev_err_probe(dev, ret, "shunt-resistor property does not exist\n"); > + } > + > + if (device_property_present(dev, "label")) { > + ret = device_property_read_string(dev, "label", (const char **)&info->label); The cast seems odd here. Why can't info->label be a const char * ? > + if (ret) > + return dev_err_probe(dev, ret, "invalid rail-name value\n"); > + } > + > + ret = pac1711_setup_vbus_range(info); > + if (ret) > + return ret; > + > + ret = pac1711_setup_vsense_range(info); > + if (ret) > + return ret; > + > + ret = device_property_read_u32(dev, "microchip,accumulation-mode", &temp); > + if (ret) > + return dev_err_probe(dev, ret, "invalid accumulation-mode value %d\n", temp); As in the dt-binding review. I'm not sure why this needs to be coming from DT. As above, it's also not clear to me that it is worth supporting some of the modes. > + > + if (temp == PAC1711_ACCMODE_VPOWER || temp == PAC1711_ACCMODE_VSENSE || > + temp == PAC1711_ACCMODE_VBUS) { > + dev_dbg(dev, "Accumulation mode set to: %d\n", temp); > + info->accumulation_mode = temp; > + } else { > + return dev_err_probe(dev, -EINVAL, "invalid accumulation-mode value %d\n", temp); > + } > + > + 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; > + u32 wait_time; > + u16 tmp_u16; > + int ret = 0; > + u8 tmp_u8; > + > + /* get sampling rate from PAC */ > + ret = i2c_smbus_read_i2c_block_data(client, PAC1711_CTRL_REG_ADDR, > + sizeof(tmp_u16), (u8 *)&tmp_u16); > + if (ret < 0) > + return dev_err_probe(dev, ret, "cannot read 0x%02X reg\n", PAC1711_CTRL_REG_ADDR); > + > + be16_to_cpus(&tmp_u16); Avoid messy endian confusion by reading into a __be16 and then using tmp_u16 = be16_to_cpu(&tmp_be16); > + info->sampling_mode = FIELD_GET(PAC1711_CTRL_SAMPLE_MODE_MASK, tmp_u16); > + > + /* > + * The current/voltage can be measured unidirectional, bidirectional or half FSR > + * no SLOW triggered REFRESH, clear POR > + */ > + tmp_u8 = FIELD_PREP(PAC1711_NEG_PWR_CFG_VS_MASK, info->vsense_mode) | > + FIELD_PREP(PAC1711_NEG_PWR_CFG_VB_MASK, info->vbus_mode); > + > + ret = i2c_smbus_write_byte_data(client, PAC1711_NEG_PWR_FSR_REG_ADDR, tmp_u8); > + if (ret) > + return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n", > + PAC1711_NEG_PWR_FSR_REG_ADDR); > + > + tmp_u8 = 0; Don't bother setting local variable, just pass 0 in directly to the write_byte_data() > + ret = i2c_smbus_write_byte_data(client, PAC1711_SLOW_REG_ADDR, tmp_u8); > + if (ret < 0) > + return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n", PAC1711_SLOW_REG_ADDR); > + > + tmp_u16 = 0; As above. Avoid mixing what is stored in a variable by adding a __be16 local variable. > + cpu_to_be16s(&tmp_u16); > + > + ret = i2c_smbus_write_word_data(client, PAC1711_CTRL_REG_ADDR, tmp_u16); The documentation for what is supported is a bit vague, but I'm not seeing > + 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) > + return dev_err_probe(dev, ret, "cannot write 0x%02X reg\n", > + PAC1711_REFRESH_REG_ADDR); > + > + usleep_range(125, 500); > + > + /* > + * 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 / pac1711_samp_rate_map_tbl[info->sampling_mode]) * 1000; > + usleep_range(wait_time, wait_time + 100); > + > + INIT_DELAYED_WORK(&info->work_chip_rfsh, pac1711_work_periodic_rfsh); > + /* Setup the latest moment for reading the regs before saturation */ > + schedule_delayed_work(&info->work_chip_rfsh, > + msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS)); > + > + return devm_add_action_or_reset(&client->dev, pac1711_cancel_delayed_work, > + &info->work_chip_rfsh); > +} > + > +static struct iio_chan_spec pac1711_single_channel[] = { As below. Clearly not a single channel. > + PAC1711_VPOWER_CHANNEL(0, 0, PAC1711_VPOWER_ADDR), > + PAC1711_VBUS_CHANNEL(0, 0, PAC1711_VBUS_ADDR), > + PAC1711_VSENSE_CHANNEL(0, 0, PAC1711_VSENSE_ADDR), > + PAC1711_VBUS_AVG_CHANNEL(0, 0, PAC1711_VBUS_AVG_ADDR), > + PAC1711_VSENSE_AVG_CHANNEL(0, 0, PAC1711_VSENSE_AVG_ADDR), > +}; > + > +static IIO_DEVICE_ATTR(in_energy_raw, 0444, > + pac1711_in_power_acc_raw_show, NULL, 0); > + > +static IIO_DEVICE_ATTR(in_energy_scale, 0444, > + pac1711_in_power_acc_scale_show, NULL, 0); > + > +static IIO_DEVICE_ATTR(in_energy_en, 0644, > + pac1711_in_enable_acc_show, pac1711_in_enable_acc_store, 0); > + > +static IIO_DEVICE_ATTR(in_current_acc_raw, 0444, > + pac1711_in_current_acc_raw_show, NULL, 0); > + > +static IIO_DEVICE_ATTR(in_current_acc_scale, 0444, > + pac1711_in_current_acc_scale_show, NULL, 0); > + > +static IIO_DEVICE_ATTR(in_current_acc_en, 0644, > + pac1711_in_enable_acc_show, pac1711_in_enable_acc_store, 0); > + > +static IIO_DEVICE_ATTR(in_voltage_acc_raw, 0444, > + pac1711_in_voltage_acc_raw_show, NULL, 0); > + > +static IIO_DEVICE_ATTR(in_voltage_acc_scale, 0444, > + pac1711_in_voltage_acc_scale_show, NULL, 0); > + > +static IIO_DEVICE_ATTR(in_voltage_acc_en, 0644, > + pac1711_in_enable_acc_show, pac1711_in_enable_acc_store, 0); > + > +static struct attribute *pac1711_power_acc_attr[] = { > + PAC1711_DEV_ATTR(in_energy_raw), > + PAC1711_DEV_ATTR(in_energy_scale), > + PAC1711_DEV_ATTR(in_energy_en), > + NULL > +}; > + > +static struct attribute *pac1711_current_acc_attr[] = { > + PAC1711_DEV_ATTR(in_current_acc_raw), > + PAC1711_DEV_ATTR(in_current_acc_scale), > + PAC1711_DEV_ATTR(in_current_acc_en), > + NULL > +}; > + > +static struct attribute *pac1711_voltage_acc_attr[] = { > + PAC1711_DEV_ATTR(in_voltage_acc_raw), > + PAC1711_DEV_ATTR(in_voltage_acc_scale), > + PAC1711_DEV_ATTR(in_voltage_acc_en), If these did make sense (see other comments) then they should be done as ext_info for the given channels rather than going though the complexity of custom attributes. Custom attributes make most sense when not associated with any channels. A lot of drivers have them for historical reasons as well that we haven't yet cleaned up. Even in_shunt_resistor is can be a shared_by_all extended attribute as it's still associated with a bunch of channels. > + NULL > +}; > + > +static int pac1711_probe(struct i2c_client *client) > +{ > + const struct pac1711_features *chip; > + struct device *dev = &client->dev; > + struct pac1711_chip_info *info; > + struct iio_dev *indio_dev; > + int ret, err; > + > + /* always start with accumulation channels enabled */ > + info->enable_acc = true; > + err = pac1711_of_parse_channel_config(client, info); Should really using generic property.h accessors so name it after fw rather than of as it applies to other firmware types. > + if (err) > + return dev_err_probe(dev, err, "Error parsing devicetree data\n"); > + > + ret = devm_mutex_init(dev, &info->lock); > + if (ret) > + return ret; > + > + /* > + * do now any chip specific initialization (e.g. read/write > + * some registers), enable/disable certain channels, change the sampling > + * rate to the requested value I don't think this comment adds much. The name makes it clear it's configuring the chip and if anyone cares what that involves they can go look. So I'd drop it. > + */ > + ret = pac1711_chip_configure(info); > + if (ret < 0) > + return ret; > + > + /* prepare the channel information */ Likewise. Comment is too obvious to be needed. > + indio_dev->num_channels = ARRAY_SIZE(pac1711_single_channel); > + indio_dev->channels = pac1711_single_channel; Single channel? That seems unlikely naming for an array of several channels. Rename it. pac1711_channels probably. > + info->iio_info = pac1711_info; > + indio_dev->info = &info->iio_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + > + ret = pac1711_prep_custom_attributes(info, indio_dev); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "Can't configure custom attributes for PAC1711 device\n"); > + > + /* > + * read whatever has been accumulated in the chip so far > + * and reset the accumulators > + */ > + ret = pac1711_reg_snapshot(info, true, PAC1711_REFRESH_REG_ADDR, > + PAC1711_MIN_UPDATE_WAIT_TIME_US); > + if (ret < 0) > + return ret; > + > + ret = devm_iio_device_register(dev, indio_dev); > + if (ret < 0) > + 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 }, Normally we only introduce the use of driver data in a series that adds a second supported part. Otherwise we end up carrying complexity that may never be used. If you already have code adding a second part then say that in the patch description as a justification for having this flexibility from the first. > + {} As below. > +}; > +MODULE_DEVICE_TABLE(i2c, pac1711_id); > + > +static const struct of_device_id pac1711_of_match[] = { > + { > + .compatible = "microchip,pac1711", > + .data = &pac1711_chip_features > + }, > + {} For IIO at least I'm pushing to standardize on formatting these as { } It was a random choice a while back as we had about half each way. > +}; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] iio: adc: adding support for PAC1711 2025-10-15 10:12 ` [PATCH 2/2] " Ariana Lazar 2025-10-16 5:31 ` kernel test robot 2025-10-19 10:28 ` Jonathan Cameron @ 2025-10-23 7:31 ` Dan Carpenter 2 siblings, 0 replies; 11+ messages in thread From: Dan Carpenter @ 2025-10-23 7:31 UTC (permalink / raw) To: oe-kbuild, Ariana Lazar, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley Cc: lkp, oe-kbuild-all, linux-iio, devicetree, linux-kernel, Ariana Lazar Hi Ariana, kernel test robot noticed the following build warnings: url: https://github.com/intel-lab-lkp/linux/commits/Ariana-Lazar/dt-bindings-iio-adc-adding-support-for-PAC1711/20251016-002337 base: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 patch link: https://lore.kernel.org/r/20251015-pac1711-v1-2-976949e36367%40microchip.com patch subject: [PATCH 2/2] iio: adc: adding support for PAC1711 config: loongarch-randconfig-r072-20251019 (https://download.01.org/0day-ci/archive/20251023/202510230847.CEW0lvJl-lkp@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 15.1.0 If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org> | Closes: https://lore.kernel.org/r/202510230847.CEW0lvJl-lkp@intel.com/ New smatch warnings: drivers/iio/adc/pac1711.c:473 pac1711_retrieve_data() error: uninitialized symbol 'ret'. drivers/iio/adc/pac1711.c:924 pac1711_read_raw() error: uninitialized symbol 'tmp'. drivers/iio/adc/pac1711.c:1124 pac1711_of_parse_channel_config() error: uninitialized symbol 'ret'. Old smatch warnings: drivers/iio/adc/pac1711.c:1079 pac1711_setup_vbus_range() warn: unsigned 'ret' is never less than zero. drivers/iio/adc/pac1711.c:1079 pac1711_setup_vbus_range() warn: error code type promoted to positive: 'ret' drivers/iio/adc/pac1711.c:1102 pac1711_setup_vsense_range() warn: unsigned 'ret' is never less than zero. drivers/iio/adc/pac1711.c:1102 pac1711_setup_vsense_range() warn: error code type promoted to positive: 'ret' drivers/iio/adc/pac1711.c:1377 pac1711_probe() warn: passing positive error code '1' to 'dev_err_probe' vim +/ret +473 drivers/iio/adc/pac1711.c de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 450 static int pac1711_retrieve_data(struct pac1711_chip_info *info, de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 451 u32 wait_time) de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 452 { de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 453 int ret; de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 454 de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 455 /* de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 456 * check if the minimal elapsed time has passed and if so, de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 457 * re-read the chip, otherwise the cached info is just fine de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 458 */ de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 459 if (time_after(jiffies, info->chip_reg_data.jiffies_tstamp + de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 460 msecs_to_jiffies(PAC1711_MIN_POLLING_TIME_MS))) { de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 461 ret = pac1711_reg_snapshot(info, true, PAC1711_REFRESH_REG_ADDR, de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 462 wait_time); de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 463 de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 464 /* de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 465 * Re-schedule the work for the read registers timeout de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 466 * (to prevent chip regs saturation) de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 467 */ de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 468 cancel_delayed_work_sync(&info->work_chip_rfsh); de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 469 schedule_delayed_work(&info->work_chip_rfsh, de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 470 msecs_to_jiffies(PAC1711_MAX_RFSH_LIMIT_MS)); de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 471 } ret isn't initialized on the else path. de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 472 de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 @473 return ret; de2a3cdfd6e76b2 Ariana Lazar 2025-10-15 474 } -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] Adding support for Microchip PAC1711 2025-10-15 10:12 [PATCH 0/2] Adding support for Microchip PAC1711 Ariana Lazar 2025-10-15 10:12 ` [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 Ariana Lazar 2025-10-15 10:12 ` [PATCH 2/2] " Ariana Lazar @ 2025-10-19 10:31 ` Jonathan Cameron 2025-10-19 15:02 ` Guenter Roeck 2 siblings, 1 reply; 11+ messages in thread From: Jonathan Cameron @ 2025-10-19 10:31 UTC (permalink / raw) To: Ariana Lazar Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel, Guenter Roeck, linux-hwmon On Wed, 15 Oct 2025 13:12:14 +0300 Ariana Lazar <ariana.lazar@microchip.com> wrote: > The PAC1711 product is a single-channel power monitor with accumulator. > The device uses 12-bit resolution for voltage and current measurements and > 24 bits power calculations. The accumulator register (56-bit) could > accumulate power (energy), current (Coulomb counter) or voltage. > > PAC1711 measures up to 42V Full-Scale Range. Hi Ariana, For devices like this where the datasheet explicitly calls out usecases in power monitoring e.g. for "Portable and Embedded Computing" (amongst other things) there is always a question to answer wrt to whether the correct place to support them in Linux is in hwmon or IIO. Note that, whilst this has long been an informal policy I've become more strict on this after some concerns were raised in the last cycle - the presence of similar devices in IIO isn't necessarily a sign that was the right choice, but it is worth looking at the history of those divers as it may provide more insight into why they are in IIO. To address that we ask that: 1) Drivers for this sort of potentially borderline device are +CC to hwmon list and maintainers 2) A justification for IIO making more sense is included. That can be based on what cannot be supported in hwmon (high speed capture being a typical item - that doesn't seem to apply here as it's only 200 sample/sec) Anyhow, I've +CC relevant folk so if you can reply with that info here then that would be great. Thanks, Jonathan > > Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com> > --- > Ariana Lazar (2): > dt-bindings: iio: adc: adding support for PAC1711 > iio: adc: adding support for PAC1711 > > .../ABI/testing/sysfs-bus-iio-adc-pac1711 | 57 + > .../bindings/iio/adc/microchip,pac1711.yaml | 195 +++ > MAINTAINERS | 8 + > drivers/iio/adc/Kconfig | 10 + > drivers/iio/adc/Makefile | 1 + > drivers/iio/adc/pac1711.c | 1448 ++++++++++++++++++++ > 6 files changed, 1719 insertions(+) > --- > base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494 > change-id: 20250901-pac1711-d3bacda400fd > > Best regards, ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] Adding support for Microchip PAC1711 2025-10-19 10:31 ` [PATCH 0/2] Adding support for Microchip PAC1711 Jonathan Cameron @ 2025-10-19 15:02 ` Guenter Roeck 2025-10-23 12:36 ` Ariana.Lazar 0 siblings, 1 reply; 11+ messages in thread From: Guenter Roeck @ 2025-10-19 15:02 UTC (permalink / raw) To: Jonathan Cameron, Ariana Lazar Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-iio, devicetree, linux-kernel, linux-hwmon On 10/19/25 03:31, Jonathan Cameron wrote: > On Wed, 15 Oct 2025 13:12:14 +0300 > Ariana Lazar <ariana.lazar@microchip.com> wrote: > >> The PAC1711 product is a single-channel power monitor with accumulator. >> The device uses 12-bit resolution for voltage and current measurements and >> 24 bits power calculations. The accumulator register (56-bit) could >> accumulate power (energy), current (Coulomb counter) or voltage. >> >> PAC1711 measures up to 42V Full-Scale Range. > > Hi Ariana, > > For devices like this where the datasheet explicitly calls out usecases in > power monitoring e.g. for "Portable and Embedded Computing" (amongst other > things) there is always a question to answer wrt to whether the correct > place to support them in Linux is in hwmon or IIO. Note that, whilst this > has long been an informal policy I've become more strict on this after some > concerns were raised in the last cycle - the presence of similar devices > in IIO isn't necessarily a sign that was the right choice, but it is worth > looking at the history of those divers as it may provide more insight into > why they are in IIO. > > To address that we ask that: > 1) Drivers for this sort of potentially borderline device are +CC to hwmon > list and maintainers > 2) A justification for IIO making more sense is included. That can be > based on what cannot be supported in hwmon (high speed capture being > a typical item - that doesn't seem to apply here as it's only 200 sample/sec) > > Anyhow, I've +CC relevant folk so if you can reply with that info here then > that would be great. > This should really be a hardware monitoring driver. Guenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] Adding support for Microchip PAC1711 2025-10-19 15:02 ` Guenter Roeck @ 2025-10-23 12:36 ` Ariana.Lazar 0 siblings, 0 replies; 11+ messages in thread From: Ariana.Lazar @ 2025-10-23 12:36 UTC (permalink / raw) To: linux, jic23 Cc: dlechner, linux-hwmon, nuno.sa, linux-iio, devicetree, robh, linux-kernel, andy, krzk+dt, conor+dt Hi all, Thank you for the feedback, I will make sure that for border devices I will add HWMON list. I was thinking that the PAC1711 device is more suitable for the IIO subsystem for the following reasons: - first but not the most important is to have the same API for the whole family (with similar functionalities) of the devices from Microchip; - second and the most important: We are looking at PAC1711 as a "special" ADC, that has some special capabilities like hardware limits. The PAC1711 could have a sample rate of up to 16384 samples/second if we are sampling only the voltage or only the current. In case of sampling only the current we have some request to be able to profile the current consumption of the device under test. I have started with a simple driver (this one that is more appropriate to be a HWMON) and willing to add more functionality later (like data buffering that is quite important for example if someone wants to profile current consumption of the processor itself, a peripheral device, a battery or an industrial low voltage automation, this kind of functionality that was requested by our customers). Best regards, Ariana Lazar On Sun, 2025-10-19 at 08:02 -0700, Guenter Roeck wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On 10/19/25 03:31, Jonathan Cameron wrote: > > On Wed, 15 Oct 2025 13:12:14 +0300 > > Ariana Lazar <ariana.lazar@microchip.com> wrote: > > > > > The PAC1711 product is a single-channel power monitor with > > > accumulator. > > > The device uses 12-bit resolution for voltage and current > > > measurements and > > > 24 bits power calculations. The accumulator register (56-bit) > > > could > > > accumulate power (energy), current (Coulomb counter) or voltage. > > > > > > PAC1711 measures up to 42V Full-Scale Range. > > > > Hi Ariana, > > > > For devices like this where the datasheet explicitly calls out > > usecases in > > power monitoring e.g. for "Portable and Embedded Computing" > > (amongst other > > things) there is always a question to answer wrt to whether the > > correct > > place to support them in Linux is in hwmon or IIO. Note that, > > whilst this > > has long been an informal policy I've become more strict on this > > after some > > concerns were raised in the last cycle - the presence of similar > > devices > > in IIO isn't necessarily a sign that was the right choice, but it > > is worth > > looking at the history of those divers as it may provide more > > insight into > > why they are in IIO. > > > > To address that we ask that: > > 1) Drivers for this sort of potentially borderline device are +CC > > to hwmon > > list and maintainers > > 2) A justification for IIO making more sense is included. That can > > be > > based on what cannot be supported in hwmon (high speed capture > > being > > a typical item - that doesn't seem to apply here as it's only > > 200 sample/sec) > > > > Anyhow, I've +CC relevant folk so if you can reply with that info > > here then > > that would be great. > > > This should really be a hardware monitoring driver. > > Guenter > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-10-23 12:36 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-15 10:12 [PATCH 0/2] Adding support for Microchip PAC1711 Ariana Lazar 2025-10-15 10:12 ` [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 Ariana Lazar 2025-10-16 16:19 ` Conor Dooley 2025-10-19 9:04 ` Jonathan Cameron 2025-10-15 10:12 ` [PATCH 2/2] " Ariana Lazar 2025-10-16 5:31 ` kernel test robot 2025-10-19 10:28 ` Jonathan Cameron 2025-10-23 7:31 ` Dan Carpenter 2025-10-19 10:31 ` [PATCH 0/2] Adding support for Microchip PAC1711 Jonathan Cameron 2025-10-19 15:02 ` Guenter Roeck 2025-10-23 12:36 ` Ariana.Lazar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).