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

[Stephen Boyd]

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@xxxxxxxxxxx>
> ---
>  .../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@xxxxxxxxxxxxxxxxxxx>
> +  - Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> +  - Nikita Shubin <nikita.shubin@xxxxxxxxxxx>
> +
> +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@80930000$":
> +        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@80810000$":
> +        type: object
> +        properties:
> +          compatible:
> +            const: cirrus,ep9301-timer
> +
> +    required:
> +      - syscon@80930000
> +      - timer@80810000
> +
> +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@80930000 {
> +                  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@80810000 {
> +                  compatible = "cirrus,ep9301-timer";
> +                  reg = <0x80810000 0x100>;
> +                  interrupt-parent = <&vic1>;
> +                  interrupts = <19>;
> +          };
> +      };
> +    };
> +
> +...