Re: [PATCH v2 1/8] RFC: dt-bindings: add img, pvrsgx.yaml for Imagination GPUs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Rob,

> Am 07.11.2019 um 15:35 schrieb Rob Herring <robh+dt@xxxxxxxxxx>:
> 
> On Thu, Nov 7, 2019 at 5:06 AM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> 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

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux