diff mbox series

[v1,02/43] dt-bindings: soc: Add Cirrus EP93xx

Message ID 20230601053546.9574-3-nikita.shubin@maquefel.me
State Changes Requested, archived
Headers show
Series ep93xx device tree conversion | expand

Checks

Context Check Description
robh/checkpatch warning total: 0 errors, 1 warnings, 160 lines checked
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Nikita Shubin June 1, 2023, 5:33 a.m. UTC
This adds device tree bindings for the Cirrus Logic EP93xx.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---

Notes:
    v0 -> v1:
    
    - fixed compatible - now it specifies three boards
    	- ts7250
    	- bk3
    	- edb9302
    - fixed identation in example
    - dropped labels

 .../devicetree/bindings/arm/ep93xx.yaml       | 107 ++++++++++++++++++
 .../dt-bindings/clock/cirrus,ep93xx-clock.h   |  53 +++++++++
 2 files changed, 160 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/ep93xx.yaml
 create mode 100644 include/dt-bindings/clock/cirrus,ep93xx-clock.h

Comments

Krzysztof Kozlowski June 1, 2023, 6:37 a.m. UTC | #1
On 01/06/2023 07:33, Nikita Shubin wrote:
> This adds device tree bindings for the Cirrus Logic EP93xx.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>

You already sent v1. This patchset is attached to the previous thread
making it more complicated for me to process. This buries it deep in the
mailbox and might interfere with applying entire sets.

Is this the next version, so v3? You already had at least two versions
before, so this cannot be v1.

> ---
> 
> Notes:
>     v0 -> v1:
>     
>     - fixed compatible - now it specifies three boards
>     	- ts7250
>     	- bk3
>     	- edb9302
>     - fixed identation in example
>     - dropped labels
> 
>  .../devicetree/bindings/arm/ep93xx.yaml       | 107 ++++++++++++++++++
>  .../dt-bindings/clock/cirrus,ep93xx-clock.h   |  53 +++++++++
>  2 files changed, 160 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..bcf9754d0763
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/ep93xx.yaml
> @@ -0,0 +1,107 @@
> +# 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 Logic EP93xx device tree bindings

No improvements.

> +
> +description: |+

no improvements. Do not need '|+' unless you need to preserve formatting.


> +  The EP93xx SoC is a ARMv4T-based with 200 MHz ARM9 CPU.
> +
> +maintainers:
> +  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
> +  - Nikita Shubin <nikita.shubin@maquefel.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: cirrus,ep9301
> +
> +      - description: The Liebherr BK3 is a derivate from ts7250 board
> +        items:
> +          - const: liebherr,bk3
> +          - const: cirrus,ep9301
> +
> +      - description: EDB302 is an evaluation board by Cirrus Logic,
> +          based on a Cirrus Logic EP9302 CPU
> +        items:
> +          - const: cirrus,edb9302
> +          - const: cirrus,ep9301
> +
> +  soc:
> +    type: object
> +    patternProperties:
> +      "^.*syscon@80930000$":
> +        type: object
> +        properties:
> +          compatible:
> +            items:
> +              - const: cirrus,ep9301-syscon
> +              - const: syscon
> +              - const: simple-mfd
> +          ep9301-reboot:
> +            type: object
> +            properties:
> +              compatible:
> +                const: cirrus,ep9301-reboot
> +        required:
> +          - compatible
> +          - reg
> +          - ep9301-reboot
> +
> +      "^.*timer@80810000$":
> +        type: object
> +        properties:
> +          compatible:
> +            const: cirrus,ep9301-timer
> +
> +    required:
> +      - syscon@80930000
> +      - timer@80810000

I don't understand what are you putting here. Why addresses are in
bindings (they should not be), why some nodes are documented in
top-level compatible. Drop all this.

Open existing files and look how it is done there.

> +
> +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@80930000 {
> +          compatible = "cirrus,ep9301-syscon",
> +                        "syscon", "simple-mfd";
> +          reg = <0x80930000 0x1000>;
> +
> +          ep9301-reboot {
> +            compatible = "cirrus,ep9301-reboot";
> +          };
> +        };
> +
> +        timer@80810000 {
> +          compatible = "cirrus,ep9301-timer";
> +          reg = <0x80810000 0x100>;
> +          interrupt-parent = <&vic1>;
> +          interrupts = <19>;
> +        };
> +      };
> +    };

Drop all this. There is no existing binding like that.

> +
> +...
> diff --git a/include/dt-bindings/clock/cirrus,ep93xx-clock.h b/include/dt-bindings/clock/cirrus,ep93xx-clock.h

Not related to top level compatible.

> new file mode 100644
> index 000000000000..6a8cf33d811b
> --- /dev/null
> +++ b/include/dt-bindings/clock/cirrus,ep93xx-clock.h
> @@ -0,0 +1,53 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */

Dual license.

> +#ifndef DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H
> +#define DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H
> +
> +#define EP93XX_CLK_XTALI	0
> +


Best regards,
Krzysztof
Nikita Shubin June 1, 2023, 7:04 a.m. UTC | #2
Hello Krzysztof!

On Thu, 1 Jun 2023 08:37:02 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 01/06/2023 07:33, Nikita Shubin wrote:
> > This adds device tree bindings for the Cirrus Logic EP93xx.
> > 
> > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>  
> 
> You already sent v1. This patchset is attached to the previous thread
> making it more complicated for me to process. This buries it deep in
> the mailbox and might interfere with applying entire sets.

Sorry for that, i've already realized my mistake looking at my own
mailbox.

> 
> Is this the next version, so v3? You already had at least two versions
> before, so this cannot be v1.

It's second on the mail lists, the first one was closed RFC.

The first was without any version, i.e. v0, this one is v1 (should be
v2).

I promise to be more careful next series.

All other comments acknowledged.

> 
> > ---
> > 
> > Notes:
> >     v0 -> v1:
> >     
> >     - fixed compatible - now it specifies three boards
> >     	- ts7250
> >     	- bk3
> >     	- edb9302
> >     - fixed identation in example
> >     - dropped labels
> > 
> >  .../devicetree/bindings/arm/ep93xx.yaml       | 107
> > ++++++++++++++++++ .../dt-bindings/clock/cirrus,ep93xx-clock.h   |
> > 53 +++++++++ 2 files changed, 160 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..bcf9754d0763
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/ep93xx.yaml
> > @@ -0,0 +1,107 @@
> > +# 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 Logic EP93xx device tree bindings  
> 
> No improvements.
> 
> > +
> > +description: |+  
> 
> no improvements. Do not need '|+' unless you need to preserve
> formatting.
> 
> 
> > +  The EP93xx SoC is a ARMv4T-based with 200 MHz ARM9 CPU.
> > +
> > +maintainers:
> > +  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
> > +  - Nikita Shubin <nikita.shubin@maquefel.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: cirrus,ep9301
> > +
> > +      - description: The Liebherr BK3 is a derivate from ts7250
> > board
> > +        items:
> > +          - const: liebherr,bk3
> > +          - const: cirrus,ep9301
> > +
> > +      - description: EDB302 is an evaluation board by Cirrus Logic,
> > +          based on a Cirrus Logic EP9302 CPU
> > +        items:
> > +          - const: cirrus,edb9302
> > +          - const: cirrus,ep9301
> > +
> > +  soc:
> > +    type: object
> > +    patternProperties:
> > +      "^.*syscon@80930000$":
> > +        type: object
> > +        properties:
> > +          compatible:
> > +            items:
> > +              - const: cirrus,ep9301-syscon
> > +              - const: syscon
> > +              - const: simple-mfd
> > +          ep9301-reboot:
> > +            type: object
> > +            properties:
> > +              compatible:
> > +                const: cirrus,ep9301-reboot
> > +        required:
> > +          - compatible
> > +          - reg
> > +          - ep9301-reboot
> > +
> > +      "^.*timer@80810000$":
> > +        type: object
> > +        properties:
> > +          compatible:
> > +            const: cirrus,ep9301-timer
> > +
> > +    required:
> > +      - syscon@80930000
> > +      - timer@80810000  
> 
> I don't understand what are you putting here. Why addresses are in
> bindings (they should not be), why some nodes are documented in
> top-level compatible. Drop all this.
> 
> Open existing files and look how it is done there.
> 
> > +
> > +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@80930000 {
> > +          compatible = "cirrus,ep9301-syscon",
> > +                        "syscon", "simple-mfd";
> > +          reg = <0x80930000 0x1000>;
> > +
> > +          ep9301-reboot {
> > +            compatible = "cirrus,ep9301-reboot";
> > +          };
> > +        };
> > +
> > +        timer@80810000 {
> > +          compatible = "cirrus,ep9301-timer";
> > +          reg = <0x80810000 0x100>;
> > +          interrupt-parent = <&vic1>;
> > +          interrupts = <19>;
> > +        };
> > +      };
> > +    };  
> 
> Drop all this. There is no existing binding like that.
> 
> > +
> > +...
> > diff --git a/include/dt-bindings/clock/cirrus,ep93xx-clock.h
> > b/include/dt-bindings/clock/cirrus,ep93xx-clock.h  
> 
> Not related to top level compatible.
> 
> > new file mode 100644
> > index 000000000000..6a8cf33d811b
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/cirrus,ep93xx-clock.h
> > @@ -0,0 +1,53 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */  
> 
> Dual license.
> 
> > +#ifndef DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H
> > +#define DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H
> > +
> > +#define EP93XX_CLK_XTALI	0
> > +  
> 
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/ep93xx.yaml b/Documentation/devicetree/bindings/arm/ep93xx.yaml
new file mode 100644
index 000000000000..bcf9754d0763
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/ep93xx.yaml
@@ -0,0 +1,107 @@ 
+# 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 Logic EP93xx device tree bindings
+
+description: |+
+  The EP93xx SoC is a ARMv4T-based with 200 MHz ARM9 CPU.
+
+maintainers:
+  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
+  - Nikita Shubin <nikita.shubin@maquefel.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: cirrus,ep9301
+
+      - description: The Liebherr BK3 is a derivate from ts7250 board
+        items:
+          - const: liebherr,bk3
+          - const: cirrus,ep9301
+
+      - description: EDB302 is an evaluation board by Cirrus Logic,
+          based on a Cirrus Logic EP9302 CPU
+        items:
+          - const: cirrus,edb9302
+          - const: cirrus,ep9301
+
+  soc:
+    type: object
+    patternProperties:
+      "^.*syscon@80930000$":
+        type: object
+        properties:
+          compatible:
+            items:
+              - const: cirrus,ep9301-syscon
+              - const: syscon
+              - const: simple-mfd
+          ep9301-reboot:
+            type: object
+            properties:
+              compatible:
+                const: cirrus,ep9301-reboot
+        required:
+          - compatible
+          - reg
+          - 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@80930000 {
+          compatible = "cirrus,ep9301-syscon",
+                        "syscon", "simple-mfd";
+          reg = <0x80930000 0x1000>;
+
+          ep9301-reboot {
+            compatible = "cirrus,ep9301-reboot";
+          };
+        };
+
+        timer@80810000 {
+          compatible = "cirrus,ep9301-timer";
+          reg = <0x80810000 0x100>;
+          interrupt-parent = <&vic1>;
+          interrupts = <19>;
+        };
+      };
+    };
+
+...
diff --git a/include/dt-bindings/clock/cirrus,ep93xx-clock.h b/include/dt-bindings/clock/cirrus,ep93xx-clock.h
new file mode 100644
index 000000000000..6a8cf33d811b
--- /dev/null
+++ b/include/dt-bindings/clock/cirrus,ep93xx-clock.h
@@ -0,0 +1,53 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H
+#define DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H
+
+#define EP93XX_CLK_XTALI	0
+
+#define EP93XX_CLK_PLL1		1
+#define EP93XX_CLK_FCLK		2
+#define EP93XX_CLK_HCLK		3
+#define EP93XX_CLK_PCLK		4
+#define EP93XX_CLK_PLL2		5
+
+#define EP93XX_CLK_UART		6
+
+#define EP93XX_CLK_UART1	7
+#define EP93XX_CLK_UART2	8
+#define EP93XX_CLK_UART3	9
+
+#define EP93XX_CLK_M2M0		10
+#define EP93XX_CLK_M2M1		11
+
+#define EP93XX_CLK_M2P0		12
+#define EP93XX_CLK_M2P1		13
+#define EP93XX_CLK_M2P2		14
+#define EP93XX_CLK_M2P3		15
+#define EP93XX_CLK_M2P4		16
+#define EP93XX_CLK_M2P5		17
+#define EP93XX_CLK_M2P6		18
+#define EP93XX_CLK_M2P7		19
+#define EP93XX_CLK_M2P8		20
+#define EP93XX_CLK_M2P9		21
+
+#define EP93XX_CLK_SPI		22
+
+#define EP93XX_CLK_USB		23
+
+#define EP93XX_CLK_ADC		24
+#define EP93XX_CLK_ADC_EN	25
+
+#define EP93XX_CLK_KEYPAD       26
+
+#define EP93XX_CLK_PWM		27
+
+#define EP93XX_CLK_VIDEO	28
+
+#define EP93XX_CLK_I2S_MCLK	29
+#define EP93XX_CLK_I2S_SCLK	30
+#define EP93XX_CLK_I2S_LRCLK	31
+
+
+#define EP93XX_NUM_CLKS (EP93XX_CLK_I2S_LRCLK + 1)
+
+#endif /* DT_BINDINGS_CIRRUS_EP93XX_CLOCK_H */