Subject: Re: [PATCH v6 02/13] dt-bindings: mfd: Add bindings for sl28cpld

From: Lee Jones <[email-masked]>
Date: Tue, 28 Jul 2020 08:24:22 +0100

On Sun, 26 Jul 2020, Michael Walle wrote:

> Add a device tree bindings for the board management controller found on
> the Kontron SMARC-sAL28 board.
> 
> Signed-off-by: Michael Walle <[email-masked]>
> Reviewed-by: Rob Herring <[email-masked]>
> ---
> Changes since v5:
>  - none
> 
> Changes since v4:
>  - fix the regex of the unit-address
> 
> Changes since v3:
>  - see cover letter
> 
>  .../bindings/gpio/kontron,sl28cpld-gpio.yaml  |  54 +++++++
>  .../hwmon/kontron,sl28cpld-hwmon.yaml         |  27 ++++
>  .../kontron,sl28cpld-intc.yaml                |  54 +++++++
>  .../bindings/mfd/kontron,sl28cpld.yaml        | 153 ++++++++++++++++++
>  .../bindings/pwm/kontron,sl28cpld-pwm.yaml    |  35 ++++
>  .../watchdog/kontron,sl28cpld-wdt.yaml        |  35 ++++
>  6 files changed, 358 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
>  create mode 100644 Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
>  create mode 100644 Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
>  create mode 100644 Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml
>  create mode 100644 Documentation/devicetree/bindings/watchdog/kontron,sl28cpld-wdt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
> new file mode 100644
> index 000000000000..9a63a158a796
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/kontron,sl28cpld-gpio.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/kontron,sl28cpld-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO driver for the sl28cpld board management controller
> +
> +maintainers:
> +  - Michael Walle <[email-masked]>
> +
> +description: |
> +  This module is part of the sl28cpld multi-function device. For more
> +  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.

Paths are normally relative.

> +  There are three flavors of the GPIO controller, one full featured
> +  input/output with interrupt support (kontron,sl28cpld-gpio), one
> +  output-only (kontron,sl28-gpo) and one input-only (kontron,sl28-gpi).
> +
> +  Each controller supports 8 GPIO lines.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - kontron,sl28cpld-gpio
> +      - kontron,sl28cpld-gpi
> +      - kontron,sl28cpld-gpo
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-controller: true
> +
> +  gpio-line-names:
> +      minItems: 1
> +      maxItems: 8
> +
> +required:
> +  - compatible
> +  - "#gpio-cells"
> +  - gpio-controller
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
> new file mode 100644
> index 000000000000..1cebd61c6c32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/kontron,sl28cpld-hwmon.yaml
> @@ -0,0 +1,27 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/kontron,sl28cpld-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Hardware monitoring driver for the sl28cpld board management controller
> +
> +maintainers:
> +  - Michael Walle <[email-masked]>
> +
> +description: |
> +  This module is part of the sl28cpld multi-function device. For more
> +  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - kontron,sl28cpld-fan
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
> new file mode 100644
> index 000000000000..4c39e9ff9aea
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/kontron,sl28cpld-intc.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/kontron,sl28cpld-intc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Interrupt controller driver for the sl28cpld board management controller
> +
> +maintainers:
> +  - Michael Walle <[email-masked]>
> +
> +description: |
> +  This module is part of the sl28cpld multi-function device. For more
> +  details see Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml.
> +
> +  The following interrupts are available. All types and levels are fixed
> +  and handled by the board management controller.
> +
> +  ==== ============= ==================================
> +   IRQ line/device   description
> +  ==== ============= ==================================
> +    0  RTC_INT#      Interrupt line from on-board RTC
> +    1  SMB_ALERT#    Event on SMB_ALERT# line (P1)
> +    2  ESPI_ALERT0#  Event on ESPI_ALERT0# line (S43)
> +    3  ESPI_ALERT1#  Event on ESPI_ALERT1# line (S44)
> +    4  PWR_BTN#      Event on PWR_BTN# line (P128)
> +    5  SLEEP#        Event on SLEEP# line (S149)
> +    6  watchdog      Interrupt of the internal watchdog
> +    7  n/a           not used
> +  ==== ============= ==================================
> +
> +properties:
> +  compatible:
> +    enum:
> +      - kontron,sl28cpld-intc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupt-controller: true
> +
> +required:
> +  - compatible
> +  - interrupts
> +  - "#interrupt-cells"
> +  - interrupt-controller
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml b/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
> new file mode 100644
> index 000000000000..e3a62db678e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml
> @@ -0,0 +1,153 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/kontron,sl28cpld.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Kontron's sl28cpld board management controller

"S128CPLD" ?

"Board Management Controller (BMC)" ?

> +maintainers:
> +  - Michael Walle <[email-masked]>
> +
> +description: |
> +  The board management controller may contain different IP blocks like
> +  watchdog, fan monitoring, PWM controller, interrupt controller and a
> +  GPIO controller.
> +
> +properties:
> +  compatible:
> +    const: kontron,sl28cpld-r1

We don't usually code revision numbers in compatible strings.

Is there any way to pull this from the H/W?

> +  reg:
> +    description:
> +      I2C device address.
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  interrupt-controller: true
> +
> +patternProperties:
> +  "^gpio(@[0-9a-f]+)?$":
> +    $ref: ../gpio/kontron,sl28cpld-gpio.yaml
> +
> +  "^hwmon(@[0-9a-f]+)?$":
> +    $ref: ../hwmon/kontron,sl28cpld-hwmon.yaml
> +
> +  "^interrupt-controller(@[0-9a-f]+)?$":
> +    $ref: ../interrupt-controller/kontron,sl28cpld-intc.yaml
> +
> +  "^pwm(@[0-9a-f]+)?$":
> +    $ref: ../pwm/kontron,sl28cpld-pwm.yaml
> +
> +  "^watchdog(@[0-9a-f]+)?$":
> +    $ref: ../watchdog/kontron,sl28cpld-wdt.yaml
> +
> +required:
> +  - "#address-cells"
> +  - "#size-cells"
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        sl28cpld@4a {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "kontron,sl28cpld-r1";
> +            reg = <0x4a>;

Nit: Could you put the 'reg' and 'compatible' at the top please?

Same for all nodes.

> +            watchdog@4 {
> +                compatible = "kontron,sl28cpld-wdt";
> +                reg = <0x4>;
> +                kontron,assert-wdt-timeout-pin;
> +            };
> +
> +            hwmon@b {
> +                compatible = "kontron,sl28cpld-fan";
> +                reg = <0xb>;
> +            };
> +
> +            pwm@c {
> +                #pwm-cells = <2>;
> +                compatible = "kontron,sl28cpld-pwm";
> +                reg = <0xc>;
> +            };
> +
> +            pwm@e {
> +                #pwm-cells = <2>;
> +                compatible = "kontron,sl28cpld-pwm";
> +                reg = <0xe>;
> +            };
> +
> +            gpio@10 {
> +                compatible = "kontron,sl28cpld-gpio";
> +                reg = <0x10>;
> +                interrupts-extended = <&gpio2 6
> +                               IRQ_TYPE_EDGE_FALLING>;
> +
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +                gpio-line-names = "a", "b", "c";
> +
> +                interrupt-controller;
> +                #interrupt-cells = <2>;
> +            };
> +
> +            gpio@15 {
> +                compatible = "kontron,sl28cpld-gpio";
> +                reg = <0x15>;
> +                interrupts-extended = <&gpio2 6
> +                               IRQ_TYPE_EDGE_FALLING>;
> +
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +
> +                interrupt-controller;
> +                #interrupt-cells = <2>;
> +            };
> +
> +            gpio@1a {
> +                compatible = "kontron,sl28cpld-gpo";
> +                reg = <0x1a>;
> +
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +            };
> +
> +            gpio@1b {
> +                compatible = "kontron,sl28cpld-gpi";
> +                reg = <0x1b>;
> +
> +                gpio-controller;
> +                #gpio-cells = <2>;
> +            };
> +
> +            interrupt-controller@1c {
> +                compatible = "kontron,sl28cpld-intc";
> +                reg = <0x1c>;
> +                interrupts-extended = <&gpio2 6
> +                               IRQ_TYPE_EDGE_FALLING>;
> +
> +                interrupt-controller;
> +                #interrupt-cells = <2>;
> +            };
> +        };
> +    };

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel