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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 20/10/2025 14:31, akemnade@kernel.org wrote:
> From: Andreas Kemnade <andreas@kemnade.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@kemnade.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