Re: [PATCH v2 14/34] dt-bindings: arm: Convert Amlogic board/soc bindings to json-schema

[Neil Armstrong]

Hi Rob,

You forgot linux-amlogic in CC...

On 03/12/2018 22:32, Rob Herring wrote:
> Convert Amlogic SoC bindings to DT schema format using json-schema.
> 
> Cc: Carlo Caione <carlo@xxxxxxxxxx>
> Cc: Kevin Hilman <khilman@xxxxxxxxxxxx>
> Cc: Mark Rutland <mark.rutland@xxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/arm/amlogic.txt       | 109 ------------------
>  .../devicetree/bindings/arm/amlogic.yaml      | 109 ++++++++++++++++++
>  2 files changed, 109 insertions(+), 109 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/arm/amlogic.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/amlogic.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/amlogic.txt b/Documentation/devicetree/bindings/arm/amlogic.txt
> deleted file mode 100644
> index f5c8d50a3506..000000000000
> --- a/Documentation/devicetree/bindings/arm/amlogic.txt
> +++ /dev/null
> @@ -1,109 +0,0 @@
> -Amlogic MesonX device tree bindings
> --------------------------------------------
> -
> -Work in progress statement:
> -
> -Device tree files and bindings applying to Amlogic SoCs and boards are
> -considered "unstable". Any Amlogic device tree binding may change at
> -any time. Be sure to use a device tree binary and a kernel image
> -generated from the same source tree.
> -
> -Please refer to Documentation/devicetree/bindings/ABI.txt for a definition of a
> -stable binding/ABI.
> -
> ----------------------------------------------------------------
> -
> -Boards with the Amlogic Meson6 SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,meson6"
> -
> -Boards with the Amlogic Meson8 SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,meson8";
> -
> -Boards with the Amlogic Meson8b SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,meson8b";
> -
> -Boards with the Amlogic Meson8m2 SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,meson8m2";
> -
> -Boards with the Amlogic Meson GXBaby SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,meson-gxbb";
> -
> -Boards with the Amlogic Meson GXL S905X SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,s905x", "amlogic,meson-gxl";
> -
> -Boards with the Amlogic Meson GXL S905D SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,s905d", "amlogic,meson-gxl";
> -
> -Boards with the Amlogic Meson GXL S805X SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,s805x", "amlogic,meson-gxl";
> -
> -Boards with the Amlogic Meson GXL S905W SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,s905w", "amlogic,meson-gxl";
> -
> -Boards with the Amlogic Meson GXM S912 SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,s912", "amlogic,meson-gxm";
> -
> -Boards with the Amlogic Meson AXG A113D SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,a113d", "amlogic,meson-axg";
> -
> -Boards with the Amlogic Meson G12A S905D2 SoC shall have the following properties:
> -  Required root node property:
> -    compatible: "amlogic,g12a";
> -
> -Board compatible values (alphabetically, grouped by SoC):
> -
> -  - "geniatech,atv1200" (Meson6)
> -
> -  - "minix,neo-x8" (Meson8)
> -
> -  - "endless,ec100" (Meson8b)
> -  - "hardkernel,odroid-c1" (Meson8b)
> -  - "tronfy,mxq" (Meson8b)
> -
> -  - "tronsmart,mxiii-plus" (Meson8m2)
> -
> -  - "amlogic,p200" (Meson gxbb)
> -  - "amlogic,p201" (Meson gxbb)
> -  - "friendlyarm,nanopi-k2" (Meson gxbb)
> -  - "hardkernel,odroid-c2" (Meson gxbb)
> -  - "nexbox,a95x" (Meson gxbb or Meson gxl s905x)
> -  - "tronsmart,vega-s95-pro", "tronsmart,vega-s95" (Meson gxbb)
> -  - "tronsmart,vega-s95-meta", "tronsmart,vega-s95" (Meson gxbb)
> -  - "tronsmart,vega-s95-telos", "tronsmart,vega-s95" (Meson gxbb)
> -  - "wetek,hub" (Meson gxbb)
> -  - "wetek,play2" (Meson gxbb)
> -
> -  - "amlogic,p212" (Meson gxl s905x)
> -  - "hwacom,amazetv" (Meson gxl s905x)
> -  - "khadas,vim" (Meson gxl s905x)
> -  - "libretech,cc" (Meson gxl s905x)
> -
> -  - "amlogic,p230" (Meson gxl s905d)
> -  - "amlogic,p231" (Meson gxl s905d)
> -
> -  - "amlogic,p241" (Meson gxl s805x)
> -
> -  - "amlogic,p281" (Meson gxl s905w)
> -  - "oranth,tx3-mini" (Meson gxl s905w)
> -
> -  - "amlogic,q200" (Meson gxm s912)
> -  - "amlogic,q201" (Meson gxm s912)
> -  - "khadas,vim2" (Meson gxm s912)
> -  - "kingnovel,r-box-pro" (Meson gxm S912)
> -  - "nexbox,a1" (Meson gxm s912)
> -  - "tronsmart,vega-s96" (Meson gxm s912)
> -
> -  - "amlogic,s400" (Meson axg a113d)
> -
> -  - "amlogic,u200" (Meson g12a s905d2)
> diff --git a/Documentation/devicetree/bindings/arm/amlogic.yaml b/Documentation/devicetree/bindings/arm/amlogic.yaml
> new file mode 100644
> index 000000000000..8b4b11194658
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/amlogic.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/amlogic.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic MesonX device tree bindings
> +
> +maintainers:
> +  - Neil Armstrong <narmstrong@xxxxxxxxxxxx>

The maintainer is Kevin Hilman.

> +  - Carlo Caione <carlo@xxxxxxxxxx>
> +description: |+
> +  Work in progress statement:
> +
> +  Device tree files and bindings applying to Amlogic SoCs and boards are
> +  considered "unstable". Any Amlogic device tree binding may change at
> +  any time. Be sure to use a device tree binary and a kernel image
> +  generated from the same source tree.
> +
> +  Please refer to Documentation/devicetree/bindings/ABI.txt for a definition of a
> +  stable binding/ABI.
> +
> +properties:
> +  $nodename:
> +    const: '/'
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - geniatech,atv1200
> +          - const: amlogic,meson6
> +      - items:
> +          - enum:
> +              - minix,neo-x8
> +          - const: amlogic,meson8
> +      - items:
> +          - enum:
> +              - tronsmart,mxiii-plus
> +          - const: amlogic,meson8m2
> +      - items:
> +          - enum:
> +              - endless,ec100
> +              - hardkernel,odroid-c1
> +              - tronfy,mxq
> +          - const: amlogic,meson8b
> +      - items:
> +          - enum:
> +              - amlogic,p200
> +              - amlogic,p201
> +              - friendlyarm,nanopi-k2
> +              - hardkernel,odroid-c2
> +              - nexbox,a95x
> +              - wetek,hub
> +              - wetek,play2
> +          - const: amlogic,meson-gxbb
> +      - items:
> +          - enum:
> +              - tronsmart,vega-s95-pro
> +              - tronsmart,vega-s95-meta
> +              - tronsmart,vega-s95-telos
> +          - const: tronsmart,vega-s95
> +          - const: amlogic,meson-gxbb
> +      - items:
> +          - enum:
> +              - amlogic,p241
> +          - const: amlogic,s805x
> +          - const: amlogic,meson-gxl
> +      - items:
> +          - enum:
> +              - amlogic,p281
> +              - oranth,tx3-mini
> +          - const: amlogic,s905w
> +          - const: amlogic,meson-gxl
> +      - items:
> +          - enum:
> +              - amlogic,p212
> +              - hwacom,amazetv
> +              - khadas,vim
> +              - libretech,cc
> +              - nexbox,a95x
> +          - const: amlogic,s905x
> +          - const: amlogic,meson-gxl
> +      - items:
> +          - enum:
> +              - amlogic,p230
> +              - amlogic,p231
> +          - const: amlogic,s905d
> +          - const: amlogic,meson-gxl
> +      - items:
> +          - enum:
> +              - amlogic,q200
> +              - amlogic,q201
> +              - khadas,vim2
> +              - kingnovel,r-box-pro
> +              - nexbox,a1
> +              - tronsmart,vega-s96
> +          - const: amlogic,s912
> +          - const: amlogic,meson-gxm
> +      - items:
> +          - enum:
> +              - amlogic,s400
> +          - const: amlogic,a113d
> +          - const: amlogic,meson-axg
> +      - items:
> +          - enum:
> +              - amlogic,u200
> +          - const: amlogic,g12a

but all this feels wrong for me.

First of all, this yaml description is not human friendly and not intuitive at all,
and secondly with this conversion we loose all the comments about the SoC family relationship
with the compatible strings !

I really understand the point to have automated verification, but really it's a pain to read
(I can't imagine newcomers... the actual DT bindings are already hard to read...) and
I feel it will be a real pain to write !

Can't we mix an "humam text" with a "yaml" part on a same document ? we are in 2018 (nearly 2019),
and it should be easy to extract a yaml description from a text document without pain and
keep all the human description, no ?

What will be the case for all the bindings with ASCII art to describe the architecture of the
HW ? will you simply drop it to replace it with cold yaml description ?

Neil

> +
> +...
>