linux-kernel - Re: [PATCHv1 01/11] dt-bindings: power: supply: adc-battery: add binding

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: <7a8d38bb-cfc1-f13b-e108-be0c61928ff0@gmail.com>
Date:   Mon, 13 Mar 2023 08:13:06 +0200
From:   Matti Vaittinen <mazziesaccount@...il.com>
To:     Sebastian Reichel <sre@...nel.org>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCHv1 01/11] dt-bindings: power: supply: adc-battery: add
 binding

Hi Sebastian,

On 3/10/23 00:50, Sebastian Reichel wrote:
> Add binding for a battery that is only monitored via ADC
> channels and simple status GPIOs.
> 
> Signed-off-by: Sebastian Reichel <sre@...nel.org>
> ---
>   .../bindings/power/supply/adc-battery.yaml    | 67 +++++++++++++++++++
>   1 file changed, 67 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/power/supply/adc-battery.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/adc-battery.yaml b/Documentation/devicetree/bindings/power/supply/adc-battery.yaml
> new file mode 100644
> index 000000000000..9d478bf9d2ee
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/adc-battery.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/adc-battery.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADC battery
> +
> +maintainers:
> +  - Sebastian Reichel <sre@...nel.org>
> +
> +description: |
> +  Basic Battery, which only reports (in circuit) voltage and optionally
> +  current via an ADC channel.
> +
> +allOf:
> +  - $ref: power-supply.yaml#
> +
> +properties:
> +  compatible:
> +    const: adc-battery
> +
> +  charged-gpios:
> +    description:
> +      GPIO which signals that the battery is fully charged.
> +    maxItems: 1
> +
> +  io-channels:
> +    minItems: 1
> +    maxItems: 3
> +
> +  io-channel-names:
> +    oneOf:
> +      - const: voltage
> +      - items:
> +          - const: voltage
> +          - enum:
> +              - current
> +              - power
> +      - items:
> +          - const: voltage
> +          - const: current
> +          - const: power

Good side of not knowing things is being able to asking for more 
information ;)

So, just by judging these bindings, we have a battery which provides 
fuel-gauge information via analog line connected to ADC(?)

Reading the description you have here and comments by Linus allows me to 
assume the line can represent current flowing out of the battery, or the 
battery voltage.

My guess then is that the io-channel-names property is going to tell 
which if these properties is being informed by the specific lines, 
right(?). Do you think you could add some small description for 
io-channel-names if you respin the series? I'd like to be more certain I 
"guessed" things right. ;) Maybe also add the 'power' option in the main 
description which currently just states voltage and power. (Assuming 
some devices do actually "expose" power levels via these "channels"?

> +
> +  monitored-battery: true
> +
> +required:
> +  - compatible
> +  - io-channels
> +  - io-channel-names
> +  - monitored-battery
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    fuel-gauge {
> +        compatible = "adc-battery";
> +        charged-gpios = <&gpio 42 GPIO_ACTIVE_HIGH>;
> +        io-channels = <&adc 13>, <&adc 37>;
> +        io-channel-names = "voltage", "current";
> +
> +        power-supplies = <&charger>;
> +        monitored-battery = <&battery>;
> +    };

-- 
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~