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: <255e857e-c4a4-c90a-f8bc-8c20b02f0839@linaro.org>
Date:   Sun, 12 Mar 2023 12:29:09 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Sebastian Reichel <sre@...nel.org>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Matti Vaittinen <mazziesaccount@...il.com>,
        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

On 09/03/2023 23: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>

Thank you for your patch. There is something to discuss/improve.


> +
> +maintainers:
> +  - Sebastian Reichel <sre@...nel.org>
> +
> +description: |

Don't need '|'.

> +  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:

Simpler:

minItems: 1
items:
  - const: voltage
  - enum: [ current, power ]
  - const: power

> +    oneOf:
> +      - const: voltage
> +      - items:
> +          - const: voltage
> +          - enum:
> +              - current
> +              - power
> +      - items:
> +          - const: voltage
> +          - const: current
> +          - const: power
> +

What about temperature? For max17040 this was recently proposed and I
wonder whether it is desirable.

https://lore.kernel.org/all/74ba115e-9838-4983-7b93-188a8260dd8a@linaro.org/

Best regards,
Krzysztof