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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 7420 bytes --]

On Fri, Mar 06, 2026 at 11:52:23AM -0600, Timothy Pearson wrote:
> 
> 
> ----- Original Message -----
> > From: "Conor Dooley" <conor@kernel.org>
> > To: "Raptor Engineering Development Team" <support@raptorengineering.com>
> > Cc: "devicetree" <devicetree@vger.kernel.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Rob Herring"
> > <robh+dt@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>, "Timothy Pearson" <tpearson@raptorengineering.com>
> > Sent: Friday, March 6, 2026 11:49:18 AM
> > Subject: Re: [PATCH v7 1/4] dt-bindings: mfd: Add sony,cronos-smc
> 
> > On Thu, Jan 29, 2026 at 01:20:44PM -0600, Raptor Engineering Development Team
> > wrote:
> >> From: Shawn Anastasio <sanastasio@raptorengineering.com>
> >> 
> >> The Sony Cronos Platform Controller is a multi-purpose platform controller
> >> that provides both a watchdog timer and an LED controller for the Sony
> >> Interactive Entertainment Cronos x86 server platform. As both functions
> >> are provided by the same CPLD, a multi-function device is exposed as the
> >> parent of both functions.
> >> 
> >> Add a DT binding for this device.
> >> 
> >> Signed-off-by: Shawn Anastasio <sanastasio@raptorengineering.com>
> >> Signed-off-by: Timothy Pearson <tpearson@raptorengineering.com>
> >> ---
> >>  .../bindings/mfd/sony,cronos-smc.yaml         | 128 ++++++++++++++++++
> >>  1 file changed, 128 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/mfd/sony,cronos-smc.yaml
> >> 
> >> diff --git a/Documentation/devicetree/bindings/mfd/sony,cronos-smc.yaml
> >> b/Documentation/devicetree/bindings/mfd/sony,cronos-smc.yaml
> >> new file mode 100644
> >> index 000000000000..a7e88fd42d39
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/mfd/sony,cronos-smc.yaml
> >> @@ -0,0 +1,128 @@
> >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> >> +# Copyright 2025-2026 Raptor Engineering, LLC
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/mfd/sony,cronos-smc.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Sony Cronos Platform Controller CPLD multi-function device
> >> +
> >> +maintainers:
> >> +  - Georgy Yakovlev <Georgy.Yakovlev@sony.com>
> >> +
> >> +description:
> >> +  The Sony Cronos Platform Controller CPLD is a multi-purpose platform
> >> +  controller that provides both a watchdog timer and an LED controller for the
> >> +  Sony Interactive Entertainment Cronos x86 server platform. As both functions
> >> +  are provided by the same CPLD, a multi-function device is exposed as the
> >> +  parent of both functions.
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: sony,cronos-smc
> >> +
> >> +  reg:
> >> +    maxItems: 1
> >> +
> >> +  leds:
> >> +    type: object
> >> +    $ref: /schemas/leds/common.yaml#
> >> +    additionalProperties: false
> >> +    description: |
> >> +      The Cronos LED controller is a subfunction of the Cronos platform
> >> +      controller, which is a multi-function device.
> >> +
> >> +      Each led is represented as a child node of sony,cronos-led. Fifteen RGB
> >> +      LEDs are supported by the platform.
> >> +
> >> +    properties:
> >> +      compatible:
> >> +        const: sony,cronos-led
> >> +
> >> +      reg:
> >> +        maxItems: 1
> >> +
> >> +      "#address-cells":
> >> +        const: 1
> >> +
> >> +      "#size-cells":
> >> +        const: 0
> >> +
> >> +    patternProperties:
> >> +      "^multi-led@[0-9a-f]$":
> >> +        type: object
> >> +        $ref: /schemas/leds/leds-class-multicolor.yaml#
> >> +        unevaluatedProperties: false
> >> +
> >> +        properties:
> >> +          reg:
> >> +            description:
> >> +              LED channel number (0..14)
> >> +            minimum: 0
> >> +            maximum: 14
> >> +
> >> +        required:
> >> +          - reg
> >> +
> >> +    required:
> >> +      - compatible
> >> +      - "#address-cells"
> >> +      - "#size-cells"
> >> +
> >> +  watchdog:
> >> +    type: object
> >> +
> >> +    $ref: /schemas/watchdog/watchdog.yaml
> >> +
> >> +    properties:
> >> +      compatible:
> >> +        const: sony,cronos-watchdog
> >> +
> >> +      timeout-sec: true
> > 
> > Why is there a child node to just add timeout-sec?
> 
> When I tried Krzysztof's solution, the validation tools threw an error, and this was the only way I found to get them to stop throwing errors.

So you were not able to do what he did, and modify watchdog.yaml?
What did you try, and what were the errors?

> 
> > Didn't Krzysztof ask
> > you to come up with a generic node name for the mfd to use instead of
> > "smc", and to modify watchdog.yaml to accept the new node name? See
> > here:
> > https://lore.kernel.org/all/c7630eb1-2686-491e-81ed-fb43fff2dd31@linaro.org/
> > 
> > Something like "platform-controller" sounds generic to me, in a way that
> > "smc" doesn't.
> 
> SMC is the name of the hardware block in question.  platform-controller generates some other confusion as there would be multiple separate hardware blocks that could fit that name.

Node names do not have to be unique. If there are 10 platform controllers
on the platform, then having 10 platform-controller@<foo> nodes would be
fine. We explicitly want node-names to describe the type of device, not
be a unique identifier. That's what compatible strings are for.
I'm unsure why there would be any confusion caused by the node name,
can you elaborate on that? What other devices connected to the BMC would
be considered "platform controllers"?

There's no explanation anywhere here where "smc" even comes from, none
of the description text or commit message has something that I would
expect "smc" to be shorthand for.

> 
> > 
> >> +
> >> +    required:
> >> +      - compatible
> >> +
> >> +    additionalProperties: false
> >> +
> >> +required:
> >> +  - compatible
> >> +  - reg
> >> +
> >> +unevaluatedProperties: false
> >> +
> >> +examples:
> >> +  - |
> >> +    #include <dt-bindings/leds/common.h>
> >> +    i2c {
> >> +        #address-cells = <1>;
> >> +        #size-cells = <0>;
> >> +
> >> +        smc@3f {
> >> +            compatible = "sony,cronos-smc";
> >> +            reg = <0x3f>;
> >> +
> >> +            watchdog {
> >> +                compatible = "sony,cronos-watchdog";
> >> +
> >> +                timeout-sec = <20>;
> >> +            };
> >> +
> >> +            leds {
> >> +                compatible = "sony,cronos-led";
> >> +                #address-cells = <1>;
> >> +                #size-cells = <0>;
> >> +
> >> +                multi-led@0 {
> >> +                    /*
> >> +                     * No subnodes are needed, this controller only supports
> >> RGB
> >> +                     * LEDs.
> >> +                     */
> >> +                    reg = <0>;
> >> +                    color = <LED_COLOR_ID_MULTI>;
> >> +                    function = LED_FUNCTION_STATUS;
> >> +                };
> >> +            };
> >> +        };
> >> +    };
> >> +
> >> --
> >> 2.39.5

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]