[v4,0/2] adding support for Microchip PAC193X Power Monitor

Message

[e-mail] Jan. 22, 2024, 8:47 a.m. UTC

From: Marius Cristea <[e-mail]>

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).


Differences related to previous patch:
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                       |   12 +
 drivers/iio/adc/Makefile                      |    1 +
 drivers/iio/adc/pac1934.c                     | 1646 +++++++++++++++++
 6 files changed, 1795 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

Comments

Guenter Roeck Jan. 22, 2024, 2:57 p.m. UTC | #1

On 1/22/24 00:47, [e-mail] wrote:
> From: Marius Cristea <[e-mail]>
> 
> 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

Not sure I understand what you are trying to say here. This is obviously an iio
driver, not a hwmon driver. Any hwmon related concern is irrelevant.

Guenter

Jonathan Cameron Jan. 22, 2024, 7:28 p.m. UTC | #2

On Mon, 22 Jan 2024 06:57:32 -0800
Guenter Roeck <[e-mail]> wrote:

> On 1/22/24 00:47, [e-mail] wrote:
> > From: Marius Cristea <[e-mail]>
> > 
> > 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  
> 
> Not sure I understand what you are trying to say here. This is obviously an iio
> driver, not a hwmon driver. Any hwmon related concern is irrelevant.

It's a left over comment / attempt to summarise 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.


> 
> Guenter
>