linux-kernel - Re: [PATCH 2/2] dt-bindings: hwmon: Add TI TPS389008

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: <bb1c12dc-dc71-4c7f-9969-2fe31f632f6b@kernel.org>
Date: Wed, 19 Mar 2025 20:19:27 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Flaviu Nistor <flaviu.nistor@...glemail.com>
Cc: Flaviu Nistor <flaviu.nistor@...il.com>, Jean Delvare
 <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Jonathan Corbet <corbet@....net>,
 linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH 2/2] dt-bindings: hwmon: Add TI TPS389008

On 19/03/2025 18:02, Flaviu Nistor wrote:
> Add device tree bindings and an example for the
> TI TPS389008 voltage monitor.
> 
> Signed-off-by: Flaviu Nistor <flaviu.nistor@...il.com>
> ---

This wasn't ever tested, so limited review.

BTW, bindings are before the user.

> +properties:
> +  compatible:
> +    oneOf:
> +      - const: ti,tps38908
> +      - items:
> +          - enum:
> +              - ti,tps38908
> +              - ti,tps38906
> +              - ti,tps38904
> +          - const: ti,tps38908
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    description: Required only if a child node is present.

Drop description

> +    const: 1
> +
> +  "#size-cells":
> +    description: Required only if a child node is present.

Drop description


> +    const: 0
> +
> +patternProperties:
> +  "^vmon@[1-8]$":

s/vmon/channel/

> +    description: |
> +      The node contains optional child nodes for up to x channels.
> +      There are 8 channels for TPS389008, 6 channels for TPS389006,
> +      and 4 channels for TPS389004. Each child node describes the information

Then you need allOf:if:then: disallowing these for specific variants.

> +      of the input source. Input channels default to disabled in the chip.
> +      Unless channels are explicitly enabled in device-tree,
> +      input channels will be disabled.
> +    type: object
> +    additionalProperties: false
> +    properties:
> +      reg:
> +        description: |
> +          Must be 1 to 8,

Don't repeat constraints in free form text.

> corresponding to the VMONx
> +          ports of the TPS389008, TPS389006, or TPS389004, respectively.

Drop entire description

> +        enum: [ 1, 2, 3, 4, 5, 6, 7, 8 ]
> +
> +      label:
> +        description: Name of the input source.
> +
> +      ti,vrange-mult-4x:
> +        description: |
> +          Must have values 5 or 20 in order for the analog
> +          conversion to be done correctly.
> +          Value 5 indicates multiply by 1 (x1), and value 20 indicates
> +          multiply by 4 (x4).
> +        type: boolean
> +
> +      ti,vmon-enable:

Drop. Status tells it, no?

> +        description: |
> +          Sets each channel status. By default, all input channels are disabled.
> +            - 0 = channel disabled
> +            - 1 = channel active
> +        type: boolean
> +
> +    required:
> +      - reg
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        vmon@37 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


> +            compatible = "ti,tps389008", "ti,tps389006", "ti,tps389004";

Test your code before sending, not after.



Best regards,
Krzysztof