linux-kernel - Re: [PATCH 1/3] dt-bindings: mfd: twl: enable power button also for twl603x

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: <5fd43d2c-3a08-4a51-abb6-38883ee86bf2@kernel.org>
Date: Tue, 21 Oct 2025 09:10:28 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: akemnade@...nel.org, Lee Jones <lee@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Andreas Kemnade <andreas@...nade.info>,
 Dmitry Torokhov <dmitry.torokhov@...il.com>, Tony Lindgren
 <tony@...mide.com>, Kevin Hilman <khilman@...nel.org>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-input@...r.kernel.org, linux-omap@...r.kernel.org
Subject: Re: [PATCH 1/3] dt-bindings: mfd: twl: enable power button also for
 twl603x

On 20/10/2025 14:31, akemnade@...nel.org wrote:
> From: Andreas Kemnade <andreas@...nade.info>
> 
> TWL603x has also a power button, so add the corresponding subnode.

No, we don't add subnodes just because there is a power button. This
needs broader explanation, see also my further comment.

> 
> Signed-off-by: Andreas Kemnade <andreas@...nade.info>
> ---
>  Documentation/devicetree/bindings/mfd/ti,twl.yaml | 40 ++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/ti,twl.yaml b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> index 776b04e182cb2..3527fee32cb07 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,twl.yaml
> @@ -55,6 +55,15 @@ allOf:
>  
>          gpadc: false
>  
> +        pwrbutton:
> +          properties:
> +            compatible:
> +              const: ti,twl4030-pwrbutton
> +            interrupts:
> +              items:
> +                - items:
> +                    const: 8

What is the point of defining const interrupts? If they are const, then
it is implied by compatible and defined in the driver.

Anyway, double items does not look right here. This is an odd syntax.

> +
>          usb-comparator: false
>  
>    - if:
> @@ -95,7 +104,14 @@ allOf:
>              compatible:
>                const: ti,twl6030-gpadc
>  
> -        pwrbutton: false
> +        pwrbutton:
> +          properties:
> +            compatible:
> +              const: ti,twl6030-pwrbutton
> +            interrupts:
> +              items:
> +                - items:
> +                    const: 0

So everywhere interrupt is defined by parent compatible.

BTW, you do not have any resources here, so the child node should be
folded into the parent.

Best regards,
Krzysztof