linux-kernel - Re: [PATCH 2/3] dt-bindings: mfd: sie,cronos-cpld: Add initial DT 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 3 Oct 2023 11:22:58 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Shawn Anastasio <sanastasio@...torengineering.com>,
        devicetree@...r.kernel.org, lee@...nel.org,
        Conor Dooley <conor+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Rob Herring <robh+dt@...nel.org>
Cc:     Timothy Pearson <tpearson@...torengineering.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: mfd: sie,cronos-cpld: Add initial DT
 binding

On 03/10/2023 00:32, Shawn Anastasio wrote:
> The SIE Cronos Platform Controller CPLD is a multi-purpose platform

What is SIE? Vendor prefix says sony.

What is Cronos?


> controller that provides both a watchdog timer and an LED controller. As
> both functions are provided by the same CPLD, a multi-function device is
> exposed as the parent of both functions.

A nit, subject: drop second/last, redundant "DT binding". The
"dt-bindings" prefix is already stating that these are bindings.

> 
> Add a DT binding for this device.
> 
> Signed-off-by: Shawn Anastasio <sanastasio@...torengineering.com>

Except that this was clearly no tested...

> ---

...

> +properties:
> +  compatible:
> +    const: sie,cronos-cpld
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1

Why do you need it?

> +
> +  '#size-cells':
> +    const: 1

Also looks unneeded.

> +
> +  leds:
> +    type: object
> +    description: Cronos Platform Status LEDs

Missing additionalProperties:false... but anyway this is just empty. No
resources? Drop the node.

> +
> +    properties:
> +      compatible:
> +        const: sie,cronos-leds
> +
> +  watchdog:
> +    type: object
> +    description: Cronos Platform Watchdog Timer
> +
> +    properties:
> +      compatible:
> +        const: sie,cronos-watchdog

No resources? Drop the node.

Best regards,
Krzysztof