2024-02-22 17:07:37

by marius.cristea

[permalink] [raw]
Subject: [PATCH v5 0/2] adding support for Microchip PAC193X Power Monitor

From: Marius Cristea <[email protected]>

Adding support for Microchip PAC193X series of Power Monitor with
Accumulator chip family. This driver covers the following part numbers:
- PAC1931, PAC1932, PAC1933 and PAC1934

This device is at the boundary between IIO and HWMON (if you are
looking just at the "shunt resistors, vsense, power, energy"). The
device also has ADC internally that can measure voltages (up to 4
channels) and also currents (up to 4 channels). The current is measured as
voltage across the shunt_resistor.

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 power consumption
of the processor itself, or a peripheral device, or a battery, this kind of
functionality was requested by our customers).

The above statement it's a left over comment / attempt to summarize the
discussion of whether IIO or HWMON was a better home for a driver for this
device. Based on current feature set that's not an obvious decision, but there
are other planned features that fit better in IIO.

Differences related to previous patch:
v5:
- fix review comments:
- remove | from device tree binding (not needed because there is no
formatting to preserve).
- update in_shunt_resistor_X attribute to in_shunt_resistorX
- use channel enable to reset the energy counter for each acctive channels
- update values of IIO elements to be in naturally aligned power of 2
- use address in the attribute (IIO_DEVICE_ATTR) to extract address field
- update the code to use mod_delayed_work
- change from kfree to ACPI_FREE
- fix "0-day" issue related to double counter increment
- check functions return in case of errors (like devm_kzalloc that could fail)
- fix coding style issues

v4:
- remove the "reset_accumulators" proprietary attribute
- add enable/disable for energy channels
- remove "reset_accumulators" attribute
- remove unused/redundant defines
- rename variable to be more relevant into a certain context
- make "storagebits" naturally aligned power of 2
- fix coding style issues
- use to_iio_dev_attr to access address field in the IIO_DEVICE_ATTR()
- remove unnecesarry "break" from switch case
- remove double increment and initialization of a variable
- use address as index in IIO_DEVICE_ATTR
- properly handle memory allocation failure

v3:
- this version was sent also to HWMON list
- fix review comments:
- drop redundant description from device tree bindings
- reorder "patternProperties:" to follow "properties:" in device tree bindings
- update comments to proper describe code
- use numbers instead of defines for clarity in some part of the code
- use the new "guard(mutex)"
- use "clamp()" instead of duplicating code
- remove extra layer of checking in some switch cases
- use "i2c_get_match_data()"
- replace while with for loops for the code to look cleaner
- reverse the logic to reduce indent.
- add comment related to channels numbering
- remove memory duplicate when creating dynamic channels
- add "devm_add_action_or_reset" to handle the "cancel_delayed_work_sync"
- remove "pac1934_remove()" function

v2:
- fix review comments:
- change the device tree bindings
- use label property
- fix coding style issues
- remove unused headers
- use get_unaligned_bexx instead of own functions
- change to use a system work queue
- use probe_new instead of old probe

v1:
- first version committed to review

Marius Cristea (2):
dt-bindings: iio: adc: adding support for PAC193X
iio: adc: adding support for PAC193x

.../ABI/testing/sysfs-bus-iio-adc-pac1934 | 9 +
.../bindings/iio/adc/microchip,pac1934.yaml | 120 ++
MAINTAINERS | 7 +
drivers/iio/adc/Kconfig | 11 +
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/pac1934.c | 1637 +++++++++++++++++
6 files changed, 1785 insertions(+)
create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-adc-pac1934
create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
create mode 100644 drivers/iio/adc/pac1934.c


base-commit: b1a1eaf6183697b77f7243780a25f35c7c0c8bdf
--
2.34.1



2024-02-22 17:08:06

by marius.cristea

[permalink] [raw]
Subject: [PATCH v5 1/2] dt-bindings: iio: adc: adding support for PAC193X

From: Marius Cristea <[email protected]>

This is the device tree schema for iio driver for
Microchip PAC193X series of Power Monitors with Accumulator.

Signed-off-by: Marius Cristea <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
.../bindings/iio/adc/microchip,pac1934.yaml | 120 ++++++++++++++++++
1 file changed, 120 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
new file mode 100644
index 000000000000..52be5ada1eb6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/microchip,pac1934.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PAC1934 Power Monitors with Accumulator
+
+maintainers:
+ - Marius Cristea <[email protected]>
+
+description: |
+ This device is part of the Microchip family of Power Monitors with
+ Accumulator.
+ The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934 can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
+
+properties:
+ compatible:
+ enum:
+ - microchip,pac1931
+ - microchip,pac1932
+ - microchip,pac1933
+ - microchip,pac1934
+
+ reg:
+ maxItems: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ interrupts:
+ maxItems: 1
+
+ slow-io-gpios:
+ description:
+ A GPIO used to trigger a change is sampling rate (lowering the chip power
+ consumption). If configured in SLOW mode, if this pin is forced high,
+ sampling rate is forced to eight samples/second. When it is forced low,
+ the sampling rate is 1024 samples/second unless a different sample rate
+ has been programmed.
+
+patternProperties:
+ "^channel@[1-4]+$":
+ type: object
+ $ref: adc.yaml
+ description:
+ Represents the external channels which are connected to the ADC.
+
+ properties:
+ reg:
+ items:
+ minimum: 1
+ maximum: 4
+
+ shunt-resistor-micro-ohms:
+ description:
+ Value in micro Ohms of the shunt resistor connected between
+ the SENSE+ and SENSE- inputs, across which the current is measured.
+ Value is needed to compute the scaling of the measured current.
+
+ required:
+ - reg
+ - shunt-resistor-micro-ohms
+
+ unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ power-monitor@10 {
+ compatible = "microchip,pac1934";
+ reg = <0x10>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@1 {
+ reg = <0x1>;
+ shunt-resistor-micro-ohms = <24900000>;
+ label = "CPU";
+ };
+
+ channel@2 {
+ reg = <0x2>;
+ shunt-resistor-micro-ohms = <49900000>;
+ label = "GPU";
+ };
+
+ channel@3 {
+ reg = <0x3>;
+ shunt-resistor-micro-ohms = <75000000>;
+ label = "MEM";
+ bipolar;
+ };
+
+ channel@4 {
+ reg = <0x4>;
+ shunt-resistor-micro-ohms = <100000000>;
+ label = "NET";
+ bipolar;
+ };
+ };
+ };
+
+...
--
2.34.1


2024-02-22 17:48:13

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: iio: adc: adding support for PAC193X


On Thu, 22 Feb 2024 18:42:05 +0200, [email protected] wrote:
> From: Marius Cristea <[email protected]>
>
> This is the device tree schema for iio driver for
> Microchip PAC193X series of Power Monitors with Accumulator.
>
> Signed-off-by: Marius Cristea <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> ---
> .../bindings/iio/adc/microchip,pac1934.yaml | 120 ++++++++++++++++++
> 1 file changed, 120 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml:51:9: [warning] wrong indentation: expected 6 but found 8 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


2024-02-24 07:35:43

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: iio: adc: adding support for PAC193X

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on b1a1eaf6183697b77f7243780a25f35c7c0c8bdf]

url: https://github.com/intel-lab-lkp/linux/commits/marius-cristea-microchip-com/dt-bindings-iio-adc-adding-support-for-PAC193X/20240223-004332
base: b1a1eaf6183697b77f7243780a25f35c7c0c8bdf
patch link: https://lore.kernel.org/r/20240222164206.65700-2-marius.cristea%40microchip.com
patch subject: [PATCH v5 1/2] dt-bindings: iio: adc: adding support for PAC193X
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240224/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml:51:9: [warning] wrong indentation: expected 6 but found 8 (indentation)

vim +51 Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml

8
9 maintainers:
10 - Marius Cristea <[email protected]>
11
12 description: |
13 This device is part of the Microchip family of Power Monitors with
14 Accumulator.
15 The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934 can be found here:
16 https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
17
18 properties:
19 compatible:
20 enum:
21 - microchip,pac1931
22 - microchip,pac1932
23 - microchip,pac1933
24 - microchip,pac1934
25
26 reg:
27 maxItems: 1
28
29 "#address-cells":
30 const: 1
31
32 "#size-cells":
33 const: 0
34
35 interrupts:
36 maxItems: 1
37
38 slow-io-gpios:
39 description:
40 A GPIO used to trigger a change is sampling rate (lowering the chip power
41 consumption). If configured in SLOW mode, if this pin is forced high,
42 sampling rate is forced to eight samples/second. When it is forced low,
43 the sampling rate is 1024 samples/second unless a different sample rate
44 has been programmed.
45
46 patternProperties:
47 "^channel@[1-4]+$":
48 type: object
49 $ref: adc.yaml
50 description:
> 51 Represents the external channels which are connected to the ADC.
52
53 properties:
54 reg:
55 items:
56 minimum: 1
57 maximum: 4
58
59 shunt-resistor-micro-ohms:
60 description:
61 Value in micro Ohms of the shunt resistor connected between
62 the SENSE+ and SENSE- inputs, across which the current is measured.
63 Value is needed to compute the scaling of the measured current.
64
65 required:
66 - reg
67 - shunt-resistor-micro-ohms
68
69 unevaluatedProperties: false
70
71 required:
72 - compatible
73 - reg
74 - "#address-cells"
75 - "#size-cells"
76

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-02-24 19:15:06

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: iio: adc: adding support for PAC193X

On Sat, 24 Feb 2024 15:34:51 +0800
kernel test robot <[email protected]> wrote:

> Hi,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on b1a1eaf6183697b77f7243780a25f35c7c0c8bdf]
>
> url: https://github.com/intel-lab-lkp/linux/commits/marius-cristea-microchip-com/dt-bindings-iio-adc-adding-support-for-PAC193X/20240223-004332
> base: b1a1eaf6183697b77f7243780a25f35c7c0c8bdf
> patch link: https://lore.kernel.org/r/20240222164206.65700-2-marius.cristea%40microchip.com
> patch subject: [PATCH v5 1/2] dt-bindings: iio: adc: adding support for PAC193X
> compiler: loongarch64-linux-gcc (GCC) 13.2.0
> reproduce: (https://download.01.org/0day-ci/archive/20240224/[email protected]/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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> dtcheck warnings: (new ones prefixed by >>)
> >> Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml:51:9: [warning] wrong indentation: expected 6 but found 8 (indentation)


> 45
> 46 patternProperties:
> 47 "^channel@[1-4]+$":
> 48 type: object
> 49 $ref: adc.yaml
> 50 description:
> > 51 Represents the external channels which are connected to the ADC.
I've fixed this up whilst applying.

> 52
> 53 properties:
> 54 reg:
> 55 items:
> 56 minimum: 1
> 57 maximum: 4
> 58
> 59 shunt-resistor-micro-ohms:
> 60 description:
> 61 Value in micro Ohms of the shunt resistor connected between
> 62 the SENSE+ and SENSE- inputs, across which the current is measured.
> 63 Value is needed to compute the scaling of the measured current.
> 64
> 65 required:
> 66 - reg
> 67 - shunt-resistor-micro-ohms
> 68
> 69 unevaluatedProperties: false
> 70
> 71 required:
> 72 - compatible
> 73 - reg
> 74 - "#address-cells"
> 75 - "#size-cells"
> 76
>