Am 07.11.2019 um 15:35 schrieb Rob Herring robh+dt@kernel.org:

On Thu, Nov 7, 2019 at 5:06 AM H. Nikolaus Schaller hns@goldelico.com wrote:

...

The Imagination PVR/SGX GPU is part of several SoC from multiple vendors, e.g. TI OMAP, Ingenic JZ4780, Intel Poulsbo and others.

With this binding, we describe how the SGX processor is interfaced to the SoC (registers, interrupt etc.).

Clock, Reset and power management should be handled by a parent node or elsewhere.

That's probably TI specific...

Yes and no.

For example the img4780 seems to need a clock reference in the gpu node. But it could maybe connected in a parent node like recent TI SoC do with the target-module approach.

And our goal is to end up with a common driver for all SoC and architectures in far future. Then, probably clock, reset and power management should be handled in the same way.

...

---

I have used the doc2yaml script to get a first veryion but I am still stuggling with the yaml thing. My impression is that while it is human readable, it is not very human writable... Unfortunately I haven't found a good tutorial for Dummies (like me) for bindings in YAML.

Did you read .../bindings/example-schema.yaml? It explains the common cases and what schema are doing.

Yes.

I recently added to it, so look at the version in linux-next.

Ah, ok. I haven't read that one.

...

The big problem is not the YAML syntax but what the schema should contain and how to correctly formulate ideas in this new language.

Specific questions for this RFC:

• formatting: is space/tab indentation correct?

YAML requires spaces.

Which is quite uncommon if you aren't a python programmer...

...

• are strings with "" correct or without?

Generally only keys or values starting with '#' need quotes. There's other cases, but we simply don't hit them with DT. We tend to quote $ref values, but that's not strictly needed.

Ok. Good.

...

• how do I specify that there is a list of compatible strings required in a specific order?

An 'items' list defines the order.

I see.

...

• but there are multiple such lists, and only one of them is to be chosen?

                                           ^^^^^^

'oneOf' is the schema keyword you are looking for.

Ok.

...

• how can be described in the binding that there should be certain values in

the parent node (ranges) to make it work?

You can't. Schemas match on a node and work down from there. So you can do it, but it's more complicated. You'd need a custom 'select' select that matches on the parent node having the child node you are looking for (assuming the parent is something generic like 'simple-bus' which you can't match on). However, based on the example, I'd say checking 'ranges' is outside the scope of schema checks. 'ranges' doesn't have to be a certain value any more than every case of 'reg' (except maybe i2c devices with fixed addresses).

Ok.

It's up to the .dts author how exactly to do address translation.

Well, my concern as a regular .dts author is that I usually treat bindings as documentation and giving hints how to write a .dts and what to take care of. If it is not complete, I get into big trouble.

I would like to have more ranges/reg checks such as bounds checks and overlapping addresses, but I think we'd do those with code, not schema.

...

I was not able to run

   make dt_binding_check dtbs_check

due to some missing dependencies (which I did not want to invest time to research them) on my build host, so I could not get automated help from those.

Dependencies are documented in Documentation/devicetree/writing-schema.rst.

One says it needs a libyaml but after installing one my HOSTCC didn't find it. The other asks for another script which seems to be missing.

...

---

.../devicetree/bindings/gpu/img,pvrsgx.yaml | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml

diff --git a/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml new file mode 100644 index 000000000000..b1b021601c47 --- /dev/null +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml @@ -0,0 +1,128 @@ +# SPDX-License-Identifier: None

Obviously not valid.

:)

...

+%YAML 1.2 +--- +$id: http://devicetree.org/schemas/bindings/gpu/img,pvrsgx.yaml#

This should have been correct with the script, but you need to drop 'bindings'.

Ok.

...

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

+title: Imagination PVR/SGX GPU

+maintainers:

• • H. Nikolaus Schaller hns@goldelico.com

+description: |+

• This binding describes the Imagination SGX5 series of 3D accelerators which
• are found in several different SoC like TI OMAP, Sitara, Ingenic JZ4780,
• Allwinner A83, and Intel Poulsbo and CedarView.
• Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by
• this binding.
• The SGX node is usually a child node of some DT node belonging to the SoC
• which handles clocks, reset and general address space mapping of the SGX
• register area.

+properties:

• compatible:
• oneOf:

•  - item:
  

'item/items'

Ok, as you described above we need "items".

...

•    # BeagleBoard ABC, OpenPandora 600MHz
  

•    - const: "ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530", "img,sgx5"
  

Not valid YAML nor json-schema. Each value needs to be list item with 'const:'

Have to look up how that syntax is.

Plenty of examples in bindings/arm/ with board/soc bindings.

Ok.

...

•    # BeagleBoard XM, GTA04, OpenPandora 1GHz
  

•    - const: "ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"
  

This needs to be a new 'items' list under 'oneOf'.

Ok!

...

•    # BeagleBone Black
  

•    - const: "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"
  

•    # Pandaboard (ES)
  

•    - const: "ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5"
  

•    - const "ti,omap4-sgx544-112", "img,sgx544-112", "img,sgx544", "img,sgx5"
  

•    # OMAP5 UEVM, Pyra Handheld
  

•    "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"
  

•    "ti,dra7-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"
  

Just gave up on trying to write a schema here?

Yes...

You see into what issues a first time YAML/schema writer with 35 years C but no YAML, Python or JSON experience runs into...

Writing bindings as .txt was easy :)

...

•    # CI20
  

•    "ingenic,jz4780-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5";
  

• reg:
• items:

•  - description: physical base address and length of the register area
  

For single entries, just 'maxItems: 1' is enough. Unless you have something special about this device, you don't need a description here.

I am not sure if I understand that yet.

...

• interrupts:

• items:
  

•  - description: interrupt from SGX subsystem to core processor
  

• clocks:

• items:
  

•  - description: optional clocks
  

• required:
• • compatible
• • reg
• • interrupts

+examples: |

• gpu@fe00 {

•   compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
  

•   reg = <0xfe00 0x200>;
  

•   interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
  

• };

+historical: |

This should be dropped. It's just for reference as you write the schema.

Yes that is clear. I kept it for reference what I intended to translate from.

...

• Imagination PVR/SGX GPU
• Only the Imagination SGX530, SGX540 and SGX544 GPUs are currently covered by this binding.
• Required properties:
• • compatible: Should be one of

•           "ti,omap3-sgx530-121", "img,sgx530-121", "img,sgx530", "img,sgx5"; - BeagleBoard ABC, OpenPandora 600MHz
  

•           "ti,omap3-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"; - BeagleBoard XM, GTA04, OpenPandora 1GHz
  

•           "ti,am3517-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5";
  

•           "ti,am335x-sgx530-125", "img,sgx530-125", "img,sgx530", "img,sgx5"; - BeagleBone Black
  

•           "ti,omap4-sgx540-120", "img,sgx540-120", "img,sgx540", "img,sgx5"; - Pandaboard (ES)
  

•           "ti,omap4-sgx544-112", "img,sgx544-112", "img,sgx544", "img,sgx5";
  

•           "ti,omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5"; - OMAP5 UEVM, Pyra Handheld
  

•           "ti,dra7-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
  

•           "ti,am3517-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
  

•           "ti,am43xx-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
  

•           "ti,ti81xx-sgx530-?", "img,sgx530-?", "img,sgx530", "img,sgx5";
  

•           "img,jz4780-sgx540-?", "img,sgx540-?", "img,sgx540", "img,sgx5"; - CI20
  

•           "allwinner,sun8i-a83t-sgx544-?", "img,sgx544-116", "img,sgx544", "img,sgx5"; - Banana-Pi-M3 (Allwinner A83T)
  

•           "intel,poulsbo-gma500-sgx535", "img,sgx535-116", "img,sgx535", "img,sgx5"; - Atom Z5xx
  

•           "intel,medfield-gma-sgx540", "img,sgx540-116", "img,sgx540", "img,sgx5"; - Atom Z24xx
  

•           "intel,cedarview-gma3600-sgx545", "img,sgx545-116", "img,sgx545", "img,sgx5"; - Atom N2600, D2500
  

•           The "ti,omap..." entries are needed temporarily to handle SoC
  

•           specific builds of the kernel module.
  

•           In the long run, only the "img,sgx..." entry should suffice
  

•           to match a generic driver for all architectures and driver
  

•           code can dynamically find out on which SoC it is running.
  

• • reg: Physical base address and length of the register area.
• • interrupts: The interrupt numbers.
• / {

•   ocp {
  

•           sgx_module: target-module@56000000 {
  

•                   compatible = "ti,sysc-omap4", "ti,sysc";
  

•                   reg = <0x5600fe00 0x4>,
  

•                         <0x5600fe10 0x4>;
  

•                   reg-names = "rev", "sysc";
  

•                   ti,sysc-midle = <SYSC_IDLE_FORCE>,
  

•                                   <SYSC_IDLE_NO>,
  

•                                   <SYSC_IDLE_SMART>;
  

•                   ti,sysc-sidle = <SYSC_IDLE_FORCE>,
  

•                                   <SYSC_IDLE_NO>,
  

•                                   <SYSC_IDLE_SMART>;
  

•                   clocks = <&gpu_clkctrl OMAP5_GPU_CLKCTRL 0>;
  

•                   clock-names = "fck";
  

•                   #address-cells = <1>;
  

•                   #size-cells = <1>;
  

•                   ranges = <0 0x56000000 0x2000000>;
  

•                   gpu@fe00 {
  

•                           compatible = "ti,omap-omap5-sgx544-116", "img,sgx544-116", "img,sgx544", "img,sgx5";
  

•                           reg = <0xfe00 0x200>;
  

•                           interrupts = <GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
  

•                   };
  

•           };
  

•   };
  

• };

-- 2.23.0

BR and thanks for the help towards a PATCH v3, Nikolaus

Hi Rob,

Am 07.11.2019 um 15:35 schrieb Rob Herring robh+dt@kernel.org:

On Thu, Nov 7, 2019 at 5:06 AM H. Nikolaus Schaller hns@goldelico.com wrote:

...

I have used the doc2yaml script to get a first version but I am still stuggling with the yaml thing. My impression is that while it is human readable, it is not very human writable... Unfortunately I haven't found a good tutorial for Dummies (like me) for bindings in YAML.

Did you read .../bindings/example-schema.yaml? It explains the common cases and what schema are doing. I recently added to it, so look at the version in linux-next.

Yes, the latest version is much more helpful. Thanks!

In addition for getting a better understanding of the YAML syntax I have writen my own parser and by single-stepping though some sample yaml files, I now think I understand YAML. And the criticism of YAML on Wikipedia [1] that it is too ambiguous.. My own toy parser is certainly wrong as well.

Anyways it finally helped me to decide between YAML syntax and DT schemata which was so far a conglomerate of concepts.

...

• how can be described in the binding that there should be certain values in

the parent node (ranges) to make it work?

You can't. Schemas match on a node and work down from there. So you can do it, but it's more complicated. You'd need a custom 'select' select that matches on the parent node having the child node you are looking for

I must admit that I don't even understand completely what you are talking about...

Seems to be a complete new set of concepts and terms that is not obvious. For example: what is a "select" or "custom select" in this context?

(assuming the parent is something generic like 'simple-bus' which you can't match on). However, based on the example, I'd say checking 'ranges' is outside the scope of schema checks. 'ranges' doesn't have to be a certain value any more than every case of 'reg' (except maybe i2c devices with fixed addresses). It's up to the .dts author how exactly to do address translation.

I would like to have more ranges/reg checks such as bounds checks and overlapping addresses, but I think we'd do those with code, not schema.

...

I was not able to run

   make dt_binding_check dtbs_check

due to some missing dependencies (which I did not want to invest time to research them) on my build host, so I could not get automated help from those.

Dependencies are documented in Documentation/devicetree/writing-schema.rst.

Unfortunately the dependencies are incomplete (e.g. it depends on pip3 which itself depends on some python installation).

Now, SCHEMA and CHKDT run fine, but it seems to need a modified DTC because I got:

DTC Documentation/devicetree/bindings/arm/actions.example.dt.yaml - due to target missing FATAL ERROR: Unknown output format "yaml"

Well, I would prefer if that would be self-contained, i.e. that downloading a kernel tree is sufficient to run the tests and there are only simple dependencies like make, gcc, flex, yacc/bison? This would mean to include a copy of some libyaml in the kernel tree.

So, after finding other developers running into the same problem [2], I looked at scripts/dtc/Makefile which does

ifeq ($(wildcard /usr/include/yaml.h),)

which is a directory location that is not even writeable on my system...

Fortunately linux-next has improved on that, but

ifeq ($(shell pkg-config --exists yaml-0.1 2>/dev/null && echo yes),)

still fails on my build host because it runs the pkg-config of the cross-compiler.

So I had to replace the command by a real check if the HOSTCC knows to find yaml.h and not if the file exists at a certain location:

ifeq ($(shell echo "#include <yaml.h>" | ${HOSTCC} -E - 2>/dev/null >&2 && echo yes),)

The other issue I have not solved is that

HOSTLDLIBS_dtc := $(shell pkg-config yaml-0.1 --libs)

also runs the pkg-config of the cross-compiler.

IMHO the ideal solution would be if HOSTCFLAGS would be taken care of here since I already have them include the proper -Lpath/to/lib but it seems to be ignored. Then, a simple -lyaml would suffice.

...

---

.../devicetree/bindings/gpu/img,pvrsgx.yaml | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml

diff --git a/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml new file mode 100644 index 000000000000..b1b021601c47 --- /dev/null +++ b/Documentation/devicetree/bindings/gpu/img,pvrsgx.yaml @@ -0,0 +1,128 @@ +# SPDX-License-Identifier: None

Obviously not valid.

Interestingly, this is not checked by the scripts...

...

+%YAML 1.2 +--- +$id: http://devicetree.org/schemas/bindings/gpu/img,pvrsgx.yaml#

This should have been correct with the script, but you need to drop 'bindings'.

Done.

...

After digging through all these barriers reported above, I have finally reworked the bindings file in a way that it passes the dtc checks and I will submit a v3 now.

BR and thanks, Nikolaus

[1]: https://en.wikipedia.org/wiki/YAML#Criticism [2]: https://lkml.org/lkml/2019/8/27/1693