linux-kernel - Re: [PATCH 11/43] dt-bindings: clock: add DT bindings for Cirrus EP93xx

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: <3574700189bb35bba3ad6d79f3a799c9.sboyd@kernel.org>
Date:   Fri, 28 Apr 2023 16:15:16 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Nikita Shubin <nikita.shubin@...uefel.me>
Cc:     Arnd Bergmann <arnd@...nel.org>, Linus Walleij <linusw@...nel.org>,
        Alexander Sverdlin <alexander.sverdlin@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Hartley Sweeten <hsweeten@...ionengravers.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-clk@...r.kernel.org
Subject: Re: [PATCH 11/43] dt-bindings: clock: add DT bindings for Cirrus EP93xx

Quoting Nikita Shubin (2023-04-24 05:34:27)
> This adds device tree bindings for the Cirrus Logic EP93xx
> clock block used in these SoCs.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@...uefel.me>
> ---
>  .../devicetree/bindings/arm/ep93xx.yaml       | 102 ++++++++++++++++++
>  .../dt-bindings/clock/cirrus,ep93xx-clock.h   |  53 +++++++++
>  2 files changed, 155 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/ep93xx.yaml
>  create mode 100644 include/dt-bindings/clock/cirrus,ep93xx-clock.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/ep93xx.yaml b/Documentation/devicetree/bindings/arm/ep93xx.yaml
> new file mode 100644
> index 000000000000..de7020f4f356
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/ep93xx.yaml

Why is this in arm/ directory? Isn't it a clock controller?

> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/ep93xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logick EP93xx device tree bindings
> +
> +description: |+
> +  The EP93xx SoC is a ARMv4T-based with 200 MHz ARM9 CPU.
> +
> +maintainers:
> +  - Hartley Sweeten <hsweeten@...ionengravers.com>
> +  - Alexander Sverdlin <alexander.sverdlin@...il.com>
> +  - Nikita Shubin <nikita.shubin@...uefel.me>
> +
> +properties:
> +  $nodename:
> +    const: '/'
> +  compatible:
> +    oneOf:
> +      - description: The TS-7250 is a compact, full-featured Single Board Computer (SBC)
> +          based upon the Cirrus EP9302 ARM9 CPU.
> +        items:
> +          - const: technologic,ts7250
> +          - const: liebherr,bk3
> +          - const: cirrus,ep9301
> +          - const: cirrus,edb9302
> +
> +  soc:
> +    type: object
> +    patternProperties:
> +      "^.*syscon@...30000$":
> +        type: object
> +        properties:
> +          compatible:
> +            items:
> +              - const: cirrus,ep9301-syscon
> +              - const: syscon
> +              - const: simple-mfd

Is there a reason it needs to be a syscon? Or a simple-mfd?

> +          ep9301-reboot:
> +            type: object
> +            properties:
> +              compatible:
> +                const: cirrus,ep9301-reboot
> +        required:
> +          - compatible
> +          - reg
> +          - '#clock-cells'
> +          - ep9301-reboot
> +
> +      "^.*timer@...10000$":
> +        type: object
> +        properties:
> +          compatible:
> +            const: cirrus,ep9301-timer
> +
> +    required:
> +      - syscon@...30000
> +      - timer@...10000
> +
> +required:
> +  - compatible
> +  - soc
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +    / {
> +      compatible = "technologic,ts7250", "cirrus,ep9301";
> +      model = "TS-7250 SBC";
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +      soc {
> +          #address-cells = <1>;
> +          #size-cells = <1>;
> +          ranges;
> +          compatible = "simple-bus";
> +
> +          syscon: syscon@...30000 {
> +                  compatible = "cirrus,ep9301-syscon",
> +                                  "syscon", "simple-mfd";
> +                  reg = <0x80930000 0x1000>;
> +                  #clock-cells = <1>;
> +                  #reset-cells = <1>;
> +
> +                  ep9301-reboot {
> +                          compatible = "cirrus,ep9301-reboot";
> +                  };
> +          };

The example should only be the clock controller node. No soc node, or
root node.

> +
> +          timer: timer@...10000 {
> +                  compatible = "cirrus,ep9301-timer";
> +                  reg = <0x80810000 0x100>;
> +                  interrupt-parent = <&vic1>;
> +                  interrupts = <19>;
> +          };
> +      };
> +    };
> +
> +...