lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251016-idealist-railway-c9e94e22ef3c@spud>
Date: Thu, 16 Oct 2025 17:19:19 +0100
From: Conor Dooley <conor@...nel.org>
To: Ariana Lazar <ariana.lazar@...rochip.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
	David Lechner <dlechner@...libre.com>,
	Nuno Sá <nuno.sa@...log.com>,
	Andy Shevchenko <andy@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711

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@...rochip.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@...rochip.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@...rochip.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@...r.kernel.org
> +S:	Supported
> +F:	Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
> +
>  MICROCHIP PAC1921 POWER/CURRENT MONITOR DRIVER
>  M:	Matteo Martelli <matteomartelli3@...il.com>
>  L:	linux-iio@...r.kernel.org
> 
> -- 
> 2.43.0
> 

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ