diff mbox series

[15/36] dt-bindings: arm: Convert Actions Semi bindings to jsonschema

Message ID 20181005165848.3474-16-robh@kernel.org
State New
Headers show
Series Devicetree schema | expand

Commit Message

Rob Herring Oct. 5, 2018, 4:58 p.m. UTC
Convert Actions Semi SoC bindings to DT schema format using json-schema.

Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>

---
 .../devicetree/bindings/arm/actions.txt       | 56 -------------------
 .../devicetree/bindings/arm/actions.yaml      | 34 +++++++++++
 2 files changed, 34 insertions(+), 56 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/actions.txt
 create mode 100644 Documentation/devicetree/bindings/arm/actions.yaml

-- 
2.17.1

Comments

Andreas Färber Oct. 6, 2018, 10:40 a.m. UTC | #1
Hi Rob,

Am 05.10.18 um 18:58 schrieb Rob Herring:
> Convert Actions Semi SoC bindings to DT schema format using json-schema.


This sounds like the next Yanny vs. Laurel... I fail to see any json. ;)

Also, it may help my understanding to be CC'ed on the cover letter, too?

> 

> Cc: "Andreas Färber" <afaerber@suse.de>

> Cc: Mark Rutland <mark.rutland@arm.com>

> Cc: linux-arm-kernel@lists.infradead.org

> Cc: devicetree@vger.kernel.org

> Signed-off-by: Rob Herring <robh@kernel.org>

> ---

>  .../devicetree/bindings/arm/actions.txt       | 56 -------------------

>  .../devicetree/bindings/arm/actions.yaml      | 34 +++++++++++

>  2 files changed, 34 insertions(+), 56 deletions(-)

>  delete mode 100644 Documentation/devicetree/bindings/arm/actions.txt

>  create mode 100644 Documentation/devicetree/bindings/arm/actions.yaml

> 

> diff --git a/Documentation/devicetree/bindings/arm/actions.txt b/Documentation/devicetree/bindings/arm/actions.txt

> deleted file mode 100644

> index d54f33c4e0da..000000000000

> --- a/Documentation/devicetree/bindings/arm/actions.txt

> +++ /dev/null

> @@ -1,56 +0,0 @@

> -Actions Semi platforms device tree bindings

> --------------------------------------------

> -

> -

> -S500 SoC

> -========

> -

> -Required root node properties:

> -

> - - compatible :  must contain "actions,s500"

> -

> -

> -Modules:

> -

> -Root node property compatible must contain, depending on module:

> -

> - - LeMaker Guitar: "lemaker,guitar"

> -

> -

> -Boards:

> -

> -Root node property compatible must contain, depending on board:

> -

> - - Allo.com Sparky: "allo,sparky"

> - - Cubietech CubieBoard6: "cubietech,cubieboard6"

> - - LeMaker Guitar Base Board rev. B: "lemaker,guitar-bb-rev-b", "lemaker,guitar"

> -

> -

> -S700 SoC

> -========

> -

> -Required root node properties:

> -

> -- compatible :  must contain "actions,s700"

> -

> -

> -Boards:

> -

> -Root node property compatible must contain, depending on board:

> -

> - - Cubietech CubieBoard7: "cubietech,cubieboard7"

> -

> -

> -S900 SoC

> -========

> -

> -Required root node properties:

> -

> -- compatible :  must contain "actions,s900"

> -

> -

> -Boards:

> -

> -Root node property compatible must contain, depending on board:

> -

> - - uCRobotics Bubblegum-96: "ucrobotics,bubblegum-96"

> diff --git a/Documentation/devicetree/bindings/arm/actions.yaml b/Documentation/devicetree/bindings/arm/actions.yaml

> new file mode 100644

> index 000000000000..af9345a228b4

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/arm/actions.yaml

> @@ -0,0 +1,34 @@

> +%YAML 1.2

> +---

> +$id: http://devicetree.org/schemas/soc/arm/actions.yaml#

> +$schema: http://devicetree.org/meta-schemas/core.yaml#


404 for the schema. Where does one find an explanation?

> +

> +title: Actions Semi platforms device tree bindings

> +

> +maintainers:

> +  - Andreas Färber <afaerber@suse.de>


Mani is now officially reviewer and the closest I have to a
co-maintainer. I suggest we add him here in some form. I assume this is
independent of MAINTAINERS patterns though, or will get_maintainers.pl
parse this, too?

> +

> +description: |


Does the | have any meaning, or a stray typo?

> +  The Actions Semi S500 is a quad-core ARM Cortex-A9 SoC. The Actions Semi

> +  S900 is a quad-core ARM Cortex-A53 SoC.


You forgot the S700 as another quad-core Cortex-A53 SoC.
Also, arm or Arm rather than ARM these days?

> +

> +properties:

> +  compatible:

> +    oneOf:

> +      - items:

> +          - enum:

> +              - lemaker,guitar-bb-rev-b

> +          - enum:

> +              - lemaker,guitar

> +              - allo,sparky

> +              - cubietech,cubieboard6

> +          - const: actions,s500

> +        minItems: 2

> +        maxItems: 3

> +        additionalItems: false


Objection. You've managed to turn a perfectly human-comprehensible text
into a machine-parseable representation incomprehensible for humans.

First, there should remain some free-text explanation of the values
defined here. Are comments allowed after the value or indented maybe?
Alternatively we could have a per-vendor file à la vendor-prefixes.txt,
but that would seem inefficient.

Next, the above items construct is horrible. What about nested oneOf:

+      - items:
+          - oneOf:
+              - items:
+                  - enum:
+                      - lemaker,guitar-bb-rev-b
+                  - const: lemaker,guitar
+              - items:
+                  - enum:
+                      - allo,sparky
+                      - cubietech,cubieboard6
+          - const: actions,s500

This grouping is much clearer to me and hopefully to anyone adding
further base boards for the module.
We will have the same issue for the BPi-S64 module with S700 below.

> +      - items:

> +          - const: cubietech,cubieboard7

> +          - const: actions,s700

> +      - items:

> +          - const: ucrobotics,bubblegum-96

> +          - const: actions,s900


Please make the board compatible an enum, even if only one is listed
today. That makes it clearer where/how (and easier) to add new boards.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Joe Perches Oct. 10, 2018, 1:41 a.m. UTC | #2
On Sat, 2018-10-06 at 12:40 +0200, Andreas Färber wrote:
> > +++ b/Documentation/devicetree/bindings/arm/actions.yaml

[]
> > +

> > +title: Actions Semi platforms device tree bindings

> > +

> > +maintainers:

> > +  - Andreas Färber <afaerber@suse.de>

> 

> Mani is now officially reviewer and the closest I have to a

> co-maintainer. I suggest we add him here in some form. I assume this is

> independent of MAINTAINERS patterns though, or will get_maintainers.pl

> parse this, too?


It _could_, if using the get_maintainers --file-emails option.
Ideally, it would be added to the MAINTAINERS somewhere.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/actions.txt b/Documentation/devicetree/bindings/arm/actions.txt
deleted file mode 100644
index d54f33c4e0da..000000000000
--- a/Documentation/devicetree/bindings/arm/actions.txt
+++ /dev/null
@@ -1,56 +0,0 @@ 
-Actions Semi platforms device tree bindings
--------------------------------------------
-
-
-S500 SoC
-========
-
-Required root node properties:
-
- - compatible :  must contain "actions,s500"
-
-
-Modules:
-
-Root node property compatible must contain, depending on module:
-
- - LeMaker Guitar: "lemaker,guitar"
-
-
-Boards:
-
-Root node property compatible must contain, depending on board:
-
- - Allo.com Sparky: "allo,sparky"
- - Cubietech CubieBoard6: "cubietech,cubieboard6"
- - LeMaker Guitar Base Board rev. B: "lemaker,guitar-bb-rev-b", "lemaker,guitar"
-
-
-S700 SoC
-========
-
-Required root node properties:
-
-- compatible :  must contain "actions,s700"
-
-
-Boards:
-
-Root node property compatible must contain, depending on board:
-
- - Cubietech CubieBoard7: "cubietech,cubieboard7"
-
-
-S900 SoC
-========
-
-Required root node properties:
-
-- compatible :  must contain "actions,s900"
-
-
-Boards:
-
-Root node property compatible must contain, depending on board:
-
- - uCRobotics Bubblegum-96: "ucrobotics,bubblegum-96"
diff --git a/Documentation/devicetree/bindings/arm/actions.yaml b/Documentation/devicetree/bindings/arm/actions.yaml
new file mode 100644
index 000000000000..af9345a228b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/actions.yaml
@@ -0,0 +1,34 @@ 
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/arm/actions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi platforms device tree bindings
+
+maintainers:
+  - Andreas Färber <afaerber@suse.de>
+
+description: |
+  The Actions Semi S500 is a quad-core ARM Cortex-A9 SoC. The Actions Semi
+  S900 is a quad-core ARM Cortex-A53 SoC.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - lemaker,guitar-bb-rev-b
+          - enum:
+              - lemaker,guitar
+              - allo,sparky
+              - cubietech,cubieboard6
+          - const: actions,s500
+        minItems: 2
+        maxItems: 3
+        additionalItems: false
+      - items:
+          - const: cubietech,cubieboard7
+          - const: actions,s700
+      - items:
+          - const: ucrobotics,bubblegum-96
+          - const: actions,s900