linux-kernel - Re: [PATCH v2 1/2] dt-bindings: iio: adc: adding dt-bindings for PAC193X

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]

Date:   Wed, 25 Oct 2023 16:08:18 +0100
From:   Conor Dooley <conor@...nel.org>
To:     marius.cristea@...rochip.com
Cc:     jic23@...nel.org, lars@...afoo.de, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: adding dt-bindings for
 PAC193X

Hey Marius,

On Wed, Oct 25, 2023 at 04:44:03PM +0300, marius.cristea@...rochip.com wrote:
> From: Marius Cristea <marius.cristea@...rochip.com>
> 
> This is the device tree schema for iio driver for
> Microchip PAC193X series of Power Monitors with Accumulator.
> 
> Signed-off-by: Marius Cristea <marius.cristea@...rochip.com>
> ---
>  .../bindings/iio/adc/microchip,pac1934.yaml   | 146 ++++++++++++++++++
>  1 file changed, 146 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..837053ed8a71
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> @@ -0,0 +1,146 @@
> +# 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 <marius.cristea@...rochip.com>
> +
> +description: |
> +  Bindings for 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:
> +    description: IRQ line of the ADC
> +    maxItems: 1
> +
> +  drive-open-drain:
> +    description: The IRQ signal is configured as open-drain.
> +    type: boolean
> +    maxItems: 1
> +
> +  microchip,slow-io:
> +    type: boolean
> +    description: |
> +      A GPIO used to trigger a change is sampling rate (lowering the chip power consumption).
> +      In default 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.

This description doesn't really make sense to me - if a GPIO is used to
drive the pin low or high, why do we need a property? A DT property
implies that this is a static configuration depending on the board, but
reading the description this seems to be something that can be toggled
at runtime.
I do note though, that this GPIO is not documented in the binding, so I
suppose what really needs to happen here is document the gpio so that
the driver can determine at runtime what state this pin is in?

Also, you say "In default mode", but don't mention what the non-default
mode is. What happens in the other mode?

Cheers,
Conor.

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