Re: [PATCH 1/4] dt-bindings: mfd: Add sony,cronos-cpld

[Timothy Pearson <tpearson@raptorengineering.com>]

----- Original Message -----
> From: "Krzysztof Kozlowski" <krzysztof.kozlowski@linaro.org>
> To: "Timothy Pearson" <tpearson@raptorengineering.com>, "robh" <robh@kernel.org>
> Cc: "devicetree" <devicetree@vger.kernel.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Conor Dooley"
> <conor+dt@kernel.org>, "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>, "Lee Jones" <lee@kernel.org>, "Georgy
> Yakovlev" <Georgy.Yakovlev@sony.com>, "Shawn Anastasio" <sanastasio@raptorengineering.com>
> Sent: Monday, October 20, 2025 10:58:40 AM
> Subject: Re: [PATCH 1/4] dt-bindings: mfd: Add sony,cronos-cpld

> On 20/10/2025 17:34, Timothy Pearson wrote:
>>>> +  watchdog:
>>>> +    type: object
>>>> +    description: Cronos Platform Watchdog Timer
>>>> +
>>>> +    allOf:
>>>> +      - $ref: watchdog.yaml#
>>>> +
>>>> +    properties:
>>>> +      compatible:
>>>> +        const: sony,cronos-watchdog
>>>
>>> There's no need for a child node here. 'timeout-sec' can just go in the
>>> parent node.
>> 
>> Could you elaborate on this please?  As far as I can tell we ref watchdog.yaml
>> and need some kind of compatible string, so why would I break out timeout-sec
>> directly here?
> 
> 
> Please look where the comment was placed. Under $ref? No. Under this
> compatible. $ref goes to the parent, obviously.

This is not an area of the kernel I'm normally changing, so I appreciate the insight.  None of this yaml syntax is particularly obvious the first time it's encountered, at least not to me.

> You do not need kind of compatible string. Parent already has one. You
> do not need compatible strings at all to instantiate Linux drivers.
> That's pretty common pattern for most of MFD-like devices, plenty of
> examples in the kernel.

Understood.