* [RFC 00/15] Device Tree schemas and validation
@ 2013-09-24 16:52 Benoit Cousson
2013-10-01 8:06 ` Benoit Cousson
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Benoit Cousson @ 2013-09-24 16:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi All,
Following the discussion that happened during LCE-2013 and the email
thread started by Tomasz few months ago [1], here is a first attempt
to introduce:
- a schema language to define the bindings accurately
- DTS validation during device tree compilation in DTC itself
[1] http://www.spinics.net/lists/arm-kernel/msg262224.html
=== What it does? ===
For now device-tree bindings are defined in a not standardized
text-based format and thus any one can write the documentation for a
binding as he wish. In addition to this there is no automated way to
check if a dts is conform to the available bindings.
The goal of this series of patch is to fix this situation by adding a
well defined way to write bindings in a human-readable format. These
bindings will be written through files called "schemas".
=== What is a schema? ===
A schema is a file describing the constraints that one or several nodes
of a dts must conform to. Schemas must use the file extension ".schema".
A schema can check that:
- A node has a given property
- An array has a valid size
- A node contains the required children.
- A property has the correct type.
- A property has the correct value.
A schema can as well recursively check the constraints for parent nodes.
The syntax for a schema is the same as the one for dts. This choice has
been made to simplify its development, to maximize the code reuse and
finally because the format is human-readable.
=== How to defined a schema? ===
A binding directory has been added at the root of repository. Users can
add schema files anywhere in it but a nice way would be to keep the same
structure as the binding directory in the documentation.
To demonstrate how to write a schema and its capability I will use the
OMAP DTS schemas when applicable.
How to:
* Associate a schema to one or several nodes
As said earlier a schema can be used to validate one or several nodes
from a dts. To do this the "compatible" properties from the nodes which
should be validated must be present in the schema.
timer1: timer at 4a318000 {
compatible = "ti,omap3430-timer";
reg = <0x4a318000 0x80>;
interrupts = <0x0 0x25 0x4>;
ti,hwmods = "timer1";
ti,timer-alwon;
};
To write a schema which will validate OMAP Timers like the one above,
one may write the following schema:
/dts-v1/;
/ {
compatible = "ti,omap[0-9]+-timer";
...
};
The schema above will be used to validate every node in a dts which has
a compatible matching the following regular expression:
"ti,omap[0-9]+-timer".
It is possible to specify multiple compatible inside a schema using this
syntax:
compatible = "ti,omap[0-9]+-timer", "ti,am[0-9]+-timer";
This time the schema will be application for both OMAP Timers and AM
Timers.
* Define constraints on properties
To define constraints on a property one has to create a node in a schema
which has as name the name of the property that one want to validate.
To specify constraints on the property "ti,hwmods" of OMAP Timers one
can write this schema:
/dts-v1/;
/ {
compatible = "ti,omap[0-9]+-timer";
ti,hwmods {
...
};
};
If one want to use a regular as property name one can write this schema:
/dts-v1/;
/ {
compatible = "abc";
def {
name = "def[0-9]";
...
};
};
Above one can see that the "name" property override the node name.
* Require the presence of a property
One can require the presence of a property by using the "is-required"
constraint.
/dts-v1/;
/ {
compatible = "ti,omap[0-9]+-timer";
ti,hwmods {
is-required;
};
};
The "ti,hwmods" property above is set as required and its presence will
be checked in every OMAP timer.
* Require the presence of a property inside a node or inside one of its
parents
Sometimes a property required is not directly present inside a node but
is present in one of its parents. To check this, one can use
"can-be-inherited" in addition to "is-required".
twl {
interrupt-controller;
rtc {
compatible = "ti,twl4030-rtc";
interrupts = <0xc>;
};
}
In the case of the rtc above the interrupt-controller is not present,
but it is present in its parent. If inheriting the property from the
parent makes senses like here one can specify in the schema that
interrupt-controller is required in the rtc node and that the property
can be inherited from a parent.
/dts-v1/;
/ {
compatible = "ti,twl[0-9]+-rtc";
interrupt-controller {
is-required;
can-be-inherited;
};
};
* Require a node to contains a given list of children
One may want to check if a node has some required children nodes.
node {
compatible = "comp";
subnode1 {
};
subnode2 {
};
abc {
};
};
One can check if 'node' has the following subnode 'subnode1', 'subnode2',
and 'abc' with the schema below:
/dts-v1/;
/ {
compatible = "comp";
children = "abc", "subnode[0-9]";
};
* Constraints on array size
One can specify the following constraints on array size:
- length: specify the exact length that an array must have.
- min-length: specify the minimum number of elements an array must have.
- max-length: specify the maximum number of elements an array must have.
Usage example:
node {
compatible = "array_size";
myarray = <0 1 2 3 4>;
};
Schema:
/dts-v1/;
/ {
compatible = "array_size";
myarray {
length = <5>;
};
};
* Count limit on nodes
One can specify a count limit for the nodes matching the schema:
- count: if there is a match between a dts and a schema then there must
be exactly X match between the dts and the schema at the end of the
validation process.
- max-count: if there is a match between a dts and a schema then there
must be at most X match between the dts and the schema at the end of
the validation process.
This can be used to check if a specific node appears the right amount of
time in the dts.
/ {
timer1 {
compatible = "ti,omap-4430-timer";
...
};
timer2 {
compatible = "ti,omap-4430-timer";
...
};
};
If in the above dts there must be exactly two timer one can check this
constraints with the following schema:
/ {
compatible = "ti,omap-4430-timer";
count = <2>;
};
* Check that a property has the right type
One can check if a property has the correct type. Right now dtc only
handles the two trivial types: integer array, string array. Since at the
end everything is an array of byte which may or may not be terminated by
a null byte this was enough.
/ {
compatible = "abc";
abc = <0xa 0xb 0xc>;
def = "def", gef;
};
To check that the property abc is an integer array and that the property
def is a string array for the dts above one can use the following schema:
/ {
compatible = "abc";
abc {
type = "integer";
};
def {
type = "string";
};
};
* Check the value of properties
It is possible to check if a property has the expected value through the
"value" constraint.
abc {
prop1 = <0 1 2 3>;
prop2 = "value0", "value1", "value3";
};
To check whether an integer array contains value from a given range
use the following constraint:
prop1 {
type = "integer";
value = <0x0 0xF>;
};
To check whether a string array contains value that match a given
pattern use the following constraint:
prop2 {
type = "string";
value = "value[0-9]";
};
To check whether a particular element of an array has the correct value
one can use the following constraint:
prop1 {
type = "integer";
value at 2 = <2>;
};
or
prop2 {
type = "string";
value at 1 = "value1";
};
* Check constraints on a parent node
It is possible to set constraints on parents of a node.
node {
compatible = "abcomp";
abc = "abc";
subnode {
compatible = "comp";
abc = "def";
};
};
The schema below tests whether 'subnode' has a parent named 'node' and
whether it has a compatible property equal to "abcomp" and a 'abc'
property equal to "abc". In the case of the node above the constraints
couldn't be check by inheriting the properties via 'can-be-inherited'
since they are overwritten by the node 'subnode'.
/dts-v1/;
/ {
compatible = "comp";
parents {
node {
compatible {
type = "string";
value = "abcomp";
};
abc {
type = "string";
value = "abc";
};
};
};
};
It is possible to set conditional constraints on parents of the
following form:
if (node_compatible == "compat1")
check_this_parent_constraints();
else if (node_compatible == "compat2")
check_that_parent_constraints();
To do this one should put the parent constraints at the same place as
the compatible definition in a schema file.
/dts-v1/;
/ {
compatible {
value at 0 {
value = "compat1";
parents {
node {
myprop {
type = "int";
value at 0 = <0xf>;
};
};
};
};
value at 1 {
value = "compat2";
parents {
node {
myprop {
type = "int";
value at 0 = <0xa>;
};
};
};
};
};
};
This schema will check that if the compatible of a node is "compat1"
then it must have a parent node "node" which has an integer array
property "myprop" which has as first element the value 0xf, otherwise
if the node has the
compatible "compat2" then the first element of the same property must
have the value 0xa.
=== How is it working? ===
When a user will try to compile a dts, dtc will parse the device-tree
and will try to find schemas that will help to check the correctness of
the dts. In order to accomplish that dtc maintain a small index of all
the schemas available.
Once dtc finds a schema which can be used to validate a particular node,
it will loads it and starts performing all the check defined by it.
A set of unit-tests has been added to test that each feature is working
properly.
=== Usage ===
Two extra options are added to handle validation into DTC:
-M <schema-path> : path of the directoy containing the schemas
-B <verbose-level> : Level of verbosity from the schema validation
dtc -M ./bindings -B 1 file.dts
=== What could be done next? ===
* Save the schema index to avoid to recompute it everytime.
* The constraints capabilities for parents and children of a node is
not equal right now. A nice thing would be to bring the same feature
available for constraints on a parent for children nodes.
* The type systems uses only the most trivial types. A nice thing would
be to bring higher level types.
* May be add conditional constraints based on property values.
* Need more? Feel free to add any item :-)
Dependency: Please note that libpcre *must* be installed in order to
add the regular expression support into the schema validation process.
I'm not sure how such dependency should be handled, since the scripts
directory does not contain any dependency like that for the moment.
The series, based on 3.12-rc2, is available here:
http://git.baylibre.com/pub/bcousson/linux-omap dts_schema
Regards,
Fabien & Benoit
---
Fabien Parent (15):
scripts/dtc: fix most memory leaks in dtc
scripts/dtc: build schema index for dts validation
scripts/dtc: validate each nodes and properties
scripts/dtc: add procedure to handle dts errors
scripts/dtc: check type on properties
scripts/dtc: check for required properties
scripts/dtc: can inherit properties
scripts/dtc: check array size
scripts/dtc: check value of properties
scripts/dtc: add count limit on nodes
scripts/dtc: check for children nodes
scripts/dtc: check constraints on parents
bindings: OMAP: add new schema files
scripts/dtc: validate dts against schema bindings
scripts/dtc: add verbose options
bindings/arm/omap/counter.schema | 28 +
bindings/arm/omap/dsp.schema | 18 +
bindings/arm/omap/intc.schema | 48 +
bindings/arm/omap/iva.schema | 38 +
bindings/arm/omap/l3-noc.schema | 38 +
bindings/arm/omap/mpu.schema | 19 +
bindings/arm/omap/omap.schema | 62 +
bindings/arm/omap/timer.schema | 124 ++
scripts/Makefile.lib | 1 +
scripts/dtc/.gitignore | 2 +-
scripts/dtc/Makefile | 9 +-
scripts/dtc/data.c | 27 +-
scripts/dtc/dtc-lexer.l | 2 +-
scripts/dtc/dtc-lexer.lex.c_shipped | 2 +-
scripts/dtc/dtc-parser.tab.c_shipped | 595 ++++-----
scripts/dtc/dtc-parser.y | 9 +
scripts/dtc/dtc.c | 29 +-
scripts/dtc/dtc.h | 30 +
scripts/dtc/livetree.c | 108 +-
scripts/dtc/schema-test.c | 146 +++
scripts/dtc/schema.c | 1304 ++++++++++++++++++++
scripts/dtc/tests/schemas/array-size-1.schema | 13 +
scripts/dtc/tests/schemas/array-size-2.schema | 8 +
scripts/dtc/tests/schemas/array-size-3.schema | 8 +
scripts/dtc/tests/schemas/array-size-4.schema | 8 +
scripts/dtc/tests/schemas/children-nodes-1.schema | 5 +
scripts/dtc/tests/schemas/children-nodes-2.schema | 5 +
scripts/dtc/tests/schemas/inheritence-1.schema | 7 +
scripts/dtc/tests/schemas/inheritence-2.schema | 8 +
scripts/dtc/tests/schemas/integer-array-1.schema | 16 +
scripts/dtc/tests/schemas/integer-array-2.schema | 9 +
scripts/dtc/tests/schemas/integer-array-3.schema | 8 +
scripts/dtc/tests/schemas/nodes-count-1.schema | 5 +
scripts/dtc/tests/schemas/nodes-count-2.schema | 5 +
scripts/dtc/tests/schemas/nodes-count-3.schema | 5 +
scripts/dtc/tests/schemas/nodes-count-4.schema | 5 +
scripts/dtc/tests/schemas/parent-nodes-1.schema | 23 +
scripts/dtc/tests/schemas/parent-nodes-2.schema | 12 +
scripts/dtc/tests/schemas/parent-nodes-3.schema | 14 +
scripts/dtc/tests/schemas/parent-nodes-4.schema | 27 +
scripts/dtc/tests/schemas/parent-nodes-5.schema | 15 +
scripts/dtc/tests/schemas/parent-nodes-6.schema | 13 +
scripts/dtc/tests/schemas/parent-nodes-7.schema | 13 +
.../dtc/tests/schemas/pattern-matching-1.schema | 10 +
.../dtc/tests/schemas/pattern-matching-2.schema | 10 +
.../dtc/tests/schemas/required-property-1.schema | 7 +
.../dtc/tests/schemas/required-property-2.schema | 7 +
scripts/dtc/tests/schemas/string-array-1.schema | 20 +
scripts/dtc/tests/schemas/string-array-2.schema | 9 +
scripts/dtc/tests/schemas/types-1.schema | 12 +
scripts/dtc/tests/schemas/types-2.schema | 7 +
scripts/dtc/tests/test1.dts | 22 +
52 files changed, 2619 insertions(+), 356 deletions(-)
create mode 100644 bindings/arm/omap/counter.schema
create mode 100644 bindings/arm/omap/dsp.schema
create mode 100644 bindings/arm/omap/intc.schema
create mode 100644 bindings/arm/omap/iva.schema
create mode 100644 bindings/arm/omap/l3-noc.schema
create mode 100644 bindings/arm/omap/mpu.schema
create mode 100644 bindings/arm/omap/omap.schema
create mode 100644 bindings/arm/omap/timer.schema
create mode 100644 scripts/dtc/schema-test.c
create mode 100644 scripts/dtc/schema.c
create mode 100644 scripts/dtc/tests/schemas/array-size-1.schema
create mode 100644 scripts/dtc/tests/schemas/array-size-2.schema
create mode 100644 scripts/dtc/tests/schemas/array-size-3.schema
create mode 100644 scripts/dtc/tests/schemas/array-size-4.schema
create mode 100644 scripts/dtc/tests/schemas/children-nodes-1.schema
create mode 100644 scripts/dtc/tests/schemas/children-nodes-2.schema
create mode 100644 scripts/dtc/tests/schemas/inheritence-1.schema
create mode 100644 scripts/dtc/tests/schemas/inheritence-2.schema
create mode 100644 scripts/dtc/tests/schemas/integer-array-1.schema
create mode 100644 scripts/dtc/tests/schemas/integer-array-2.schema
create mode 100644 scripts/dtc/tests/schemas/integer-array-3.schema
create mode 100644 scripts/dtc/tests/schemas/nodes-count-1.schema
create mode 100644 scripts/dtc/tests/schemas/nodes-count-2.schema
create mode 100644 scripts/dtc/tests/schemas/nodes-count-3.schema
create mode 100644 scripts/dtc/tests/schemas/nodes-count-4.schema
create mode 100644 scripts/dtc/tests/schemas/parent-nodes-1.schema
create mode 100644 scripts/dtc/tests/schemas/parent-nodes-2.schema
create mode 100644 scripts/dtc/tests/schemas/parent-nodes-3.schema
create mode 100644 scripts/dtc/tests/schemas/parent-nodes-4.schema
create mode 100644 scripts/dtc/tests/schemas/parent-nodes-5.schema
create mode 100644 scripts/dtc/tests/schemas/parent-nodes-6.schema
create mode 100644 scripts/dtc/tests/schemas/parent-nodes-7.schema
create mode 100644 scripts/dtc/tests/schemas/pattern-matching-1.schema
create mode 100644 scripts/dtc/tests/schemas/pattern-matching-2.schema
create mode 100644 scripts/dtc/tests/schemas/required-property-1.schema
create mode 100644 scripts/dtc/tests/schemas/required-property-2.schema
create mode 100644 scripts/dtc/tests/schemas/string-array-1.schema
create mode 100644 scripts/dtc/tests/schemas/string-array-2.schema
create mode 100644 scripts/dtc/tests/schemas/types-1.schema
create mode 100644 scripts/dtc/tests/schemas/types-2.schema
create mode 100644 scripts/dtc/tests/test1.dts
--
1.8.1.2
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-09-24 16:52 [RFC 00/15] Device Tree schemas and validation Benoit Cousson
@ 2013-10-01 8:06 ` Benoit Cousson
2013-10-01 13:17 ` Rob Herring
2013-10-01 22:22 ` Stephen Warren
[not found] ` <1380041541-17529-2-git-send-email-bcousson@baylibre.com>
2 siblings, 1 reply; 21+ messages in thread
From: Benoit Cousson @ 2013-10-01 8:06 UTC (permalink / raw)
To: linux-arm-kernel
+ more DT maintainers folks
Hi all,
I know this is mostly boring user space code, but I was expecting a
little bit of comments about at least the bindings syntax:-(
I'd like to know if this is the right direction and if it worth pursuing
in that direction.
The idea was to have at least some base for further discussion during
ARM KS 2013.
I feel alone :-(
If you have any comment, go ahead!
Thanks,
Benoit
On 24/09/2013 18:52, Benoit Cousson wrote:
> Hi All,
>
> Following the discussion that happened during LCE-2013 and the email
> thread started by Tomasz few months ago [1], here is a first attempt
> to introduce:
> - a schema language to define the bindings accurately
> - DTS validation during device tree compilation in DTC itself
>
> [1] http://www.spinics.net/lists/arm-kernel/msg262224.html
>
>
> === What it does? ===
>
> For now device-tree bindings are defined in a not standardized
> text-based format and thus any one can write the documentation for a
> binding as he wish. In addition to this there is no automated way to
> check if a dts is conform to the available bindings.
>
> The goal of this series of patch is to fix this situation by adding a
> well defined way to write bindings in a human-readable format. These
> bindings will be written through files called "schemas".
>
>
> === What is a schema? ===
>
> A schema is a file describing the constraints that one or several nodes
> of a dts must conform to. Schemas must use the file extension ".schema".
> A schema can check that:
> - A node has a given property
> - An array has a valid size
> - A node contains the required children.
> - A property has the correct type.
> - A property has the correct value.
>
> A schema can as well recursively check the constraints for parent nodes.
>
> The syntax for a schema is the same as the one for dts. This choice has
> been made to simplify its development, to maximize the code reuse and
> finally because the format is human-readable.
>
>
> === How to defined a schema? ===
>
> A binding directory has been added at the root of repository. Users can
> add schema files anywhere in it but a nice way would be to keep the same
> structure as the binding directory in the documentation.
>
> To demonstrate how to write a schema and its capability I will use the
> OMAP DTS schemas when applicable.
>
> How to:
> * Associate a schema to one or several nodes
>
> As said earlier a schema can be used to validate one or several nodes
> from a dts. To do this the "compatible" properties from the nodes which
> should be validated must be present in the schema.
>
> timer1: timer at 4a318000 {
> compatible = "ti,omap3430-timer";
> reg = <0x4a318000 0x80>;
> interrupts = <0x0 0x25 0x4>;
> ti,hwmods = "timer1";
> ti,timer-alwon;
> };
>
> To write a schema which will validate OMAP Timers like the one above,
> one may write the following schema:
>
> /dts-v1/;
> / {
> compatible = "ti,omap[0-9]+-timer";
> ...
> };
>
> The schema above will be used to validate every node in a dts which has
> a compatible matching the following regular expression:
> "ti,omap[0-9]+-timer".
>
> It is possible to specify multiple compatible inside a schema using this
> syntax:
> compatible = "ti,omap[0-9]+-timer", "ti,am[0-9]+-timer";
>
> This time the schema will be application for both OMAP Timers and AM
> Timers.
>
>
> * Define constraints on properties
>
> To define constraints on a property one has to create a node in a schema
> which has as name the name of the property that one want to validate.
>
> To specify constraints on the property "ti,hwmods" of OMAP Timers one
> can write this schema:
>
> /dts-v1/;
> / {
> compatible = "ti,omap[0-9]+-timer";
> ti,hwmods {
> ...
> };
> };
>
> If one want to use a regular as property name one can write this schema:
>
> /dts-v1/;
> / {
> compatible = "abc";
> def {
> name = "def[0-9]";
> ...
> };
> };
>
> Above one can see that the "name" property override the node name.
>
>
> * Require the presence of a property
>
> One can require the presence of a property by using the "is-required"
> constraint.
>
> /dts-v1/;
> / {
> compatible = "ti,omap[0-9]+-timer";
> ti,hwmods {
> is-required;
> };
> };
>
> The "ti,hwmods" property above is set as required and its presence will
> be checked in every OMAP timer.
>
>
> * Require the presence of a property inside a node or inside one of its
> parents
>
> Sometimes a property required is not directly present inside a node but
> is present in one of its parents. To check this, one can use
> "can-be-inherited" in addition to "is-required".
>
> twl {
> interrupt-controller;
>
> rtc {
> compatible = "ti,twl4030-rtc";
> interrupts = <0xc>;
> };
> }
>
> In the case of the rtc above the interrupt-controller is not present,
> but it is present in its parent. If inheriting the property from the
> parent makes senses like here one can specify in the schema that
> interrupt-controller is required in the rtc node and that the property
> can be inherited from a parent.
>
> /dts-v1/;
> / {
> compatible = "ti,twl[0-9]+-rtc";
> interrupt-controller {
> is-required;
> can-be-inherited;
> };
> };
>
>
> * Require a node to contains a given list of children
>
> One may want to check if a node has some required children nodes.
>
> node {
> compatible = "comp";
>
> subnode1 {
> };
>
> subnode2 {
> };
>
> abc {
> };
> };
>
> One can check if 'node' has the following subnode 'subnode1', 'subnode2',
> and 'abc' with the schema below:
>
> /dts-v1/;
> / {
> compatible = "comp";
> children = "abc", "subnode[0-9]";
> };
>
>
> * Constraints on array size
>
> One can specify the following constraints on array size:
> - length: specify the exact length that an array must have.
> - min-length: specify the minimum number of elements an array must have.
> - max-length: specify the maximum number of elements an array must have.
>
> Usage example:
> node {
> compatible = "array_size";
> myarray = <0 1 2 3 4>;
> };
>
> Schema:
> /dts-v1/;
> / {
> compatible = "array_size";
>
> myarray {
> length = <5>;
> };
> };
>
>
> * Count limit on nodes
>
> One can specify a count limit for the nodes matching the schema:
> - count: if there is a match between a dts and a schema then there must
> be exactly X match between the dts and the schema at the end of the
> validation process.
> - max-count: if there is a match between a dts and a schema then there
> must be at most X match between the dts and the schema at the end of
> the validation process.
> This can be used to check if a specific node appears the right amount of
> time in the dts.
>
> / {
> timer1 {
> compatible = "ti,omap-4430-timer";
> ...
> };
>
> timer2 {
> compatible = "ti,omap-4430-timer";
> ...
> };
> };
>
> If in the above dts there must be exactly two timer one can check this
> constraints with the following schema:
>
> / {
> compatible = "ti,omap-4430-timer";
> count = <2>;
> };
>
>
> * Check that a property has the right type
>
> One can check if a property has the correct type. Right now dtc only
> handles the two trivial types: integer array, string array. Since at the
> end everything is an array of byte which may or may not be terminated by
> a null byte this was enough.
>
> / {
> compatible = "abc";
>
> abc = <0xa 0xb 0xc>;
> def = "def", gef;
> };
>
> To check that the property abc is an integer array and that the property
> def is a string array for the dts above one can use the following schema:
>
> / {
> compatible = "abc";
>
>
> abc {
> type = "integer";
> };
>
> def {
> type = "string";
> };
> };
>
>
> * Check the value of properties
>
> It is possible to check if a property has the expected value through the
> "value" constraint.
>
> abc {
> prop1 = <0 1 2 3>;
> prop2 = "value0", "value1", "value3";
> };
>
> To check whether an integer array contains value from a given range
> use the following constraint:
> prop1 {
> type = "integer";
> value = <0x0 0xF>;
> };
>
> To check whether a string array contains value that match a given
> pattern use the following constraint:
> prop2 {
> type = "string";
> value = "value[0-9]";
> };
>
> To check whether a particular element of an array has the correct value
> one can use the following constraint:
> prop1 {
> type = "integer";
> value at 2 = <2>;
> };
>
> or
>
> prop2 {
> type = "string";
> value at 1 = "value1";
> };
>
>
> * Check constraints on a parent node
>
> It is possible to set constraints on parents of a node.
>
> node {
> compatible = "abcomp";
> abc = "abc";
>
> subnode {
> compatible = "comp";
> abc = "def";
> };
> };
>
> The schema below tests whether 'subnode' has a parent named 'node' and
> whether it has a compatible property equal to "abcomp" and a 'abc'
> property equal to "abc". In the case of the node above the constraints
> couldn't be check by inheriting the properties via 'can-be-inherited'
> since they are overwritten by the node 'subnode'.
>
> /dts-v1/;
> / {
> compatible = "comp";
>
> parents {
> node {
> compatible {
> type = "string";
> value = "abcomp";
> };
>
> abc {
> type = "string";
> value = "abc";
> };
> };
> };
> };
>
> It is possible to set conditional constraints on parents of the
> following form:
> if (node_compatible == "compat1")
> check_this_parent_constraints();
> else if (node_compatible == "compat2")
> check_that_parent_constraints();
>
> To do this one should put the parent constraints at the same place as
> the compatible definition in a schema file.
>
> /dts-v1/;
> / {
> compatible {
> value at 0 {
> value = "compat1";
> parents {
> node {
> myprop {
> type = "int";
> value at 0 = <0xf>;
> };
> };
> };
> };
>
> value at 1 {
> value = "compat2";
> parents {
> node {
> myprop {
> type = "int";
> value at 0 = <0xa>;
> };
> };
> };
> };
> };
> };
>
> This schema will check that if the compatible of a node is "compat1"
> then it must have a parent node "node" which has an integer array
> property "myprop" which has as first element the value 0xf, otherwise
> if the node has the
> compatible "compat2" then the first element of the same property must
> have the value 0xa.
>
>
> === How is it working? ===
>
> When a user will try to compile a dts, dtc will parse the device-tree
> and will try to find schemas that will help to check the correctness of
> the dts. In order to accomplish that dtc maintain a small index of all
> the schemas available.
> Once dtc finds a schema which can be used to validate a particular node,
> it will loads it and starts performing all the check defined by it.
>
> A set of unit-tests has been added to test that each feature is working
> properly.
>
>
> === Usage ===
>
> Two extra options are added to handle validation into DTC:
> -M <schema-path> : path of the directoy containing the schemas
> -B <verbose-level> : Level of verbosity from the schema validation
>
> dtc -M ./bindings -B 1 file.dts
>
>
> === What could be done next? ===
>
> * Save the schema index to avoid to recompute it everytime.
> * The constraints capabilities for parents and children of a node is
> not equal right now. A nice thing would be to bring the same feature
> available for constraints on a parent for children nodes.
> * The type systems uses only the most trivial types. A nice thing would
> be to bring higher level types.
> * May be add conditional constraints based on property values.
> * Need more? Feel free to add any item :-)
>
> Dependency: Please note that libpcre *must* be installed in order to
> add the regular expression support into the schema validation process.
> I'm not sure how such dependency should be handled, since the scripts
> directory does not contain any dependency like that for the moment.
>
> The series, based on 3.12-rc2, is available here:
> http://git.baylibre.com/pub/bcousson/linux-omap dts_schema
>
> Regards,
> Fabien & Benoit
>
> ---
>
> Fabien Parent (15):
> scripts/dtc: fix most memory leaks in dtc
> scripts/dtc: build schema index for dts validation
> scripts/dtc: validate each nodes and properties
> scripts/dtc: add procedure to handle dts errors
> scripts/dtc: check type on properties
> scripts/dtc: check for required properties
> scripts/dtc: can inherit properties
> scripts/dtc: check array size
> scripts/dtc: check value of properties
> scripts/dtc: add count limit on nodes
> scripts/dtc: check for children nodes
> scripts/dtc: check constraints on parents
> bindings: OMAP: add new schema files
> scripts/dtc: validate dts against schema bindings
> scripts/dtc: add verbose options
>
> bindings/arm/omap/counter.schema | 28 +
> bindings/arm/omap/dsp.schema | 18 +
> bindings/arm/omap/intc.schema | 48 +
> bindings/arm/omap/iva.schema | 38 +
> bindings/arm/omap/l3-noc.schema | 38 +
> bindings/arm/omap/mpu.schema | 19 +
> bindings/arm/omap/omap.schema | 62 +
> bindings/arm/omap/timer.schema | 124 ++
> scripts/Makefile.lib | 1 +
> scripts/dtc/.gitignore | 2 +-
> scripts/dtc/Makefile | 9 +-
> scripts/dtc/data.c | 27 +-
> scripts/dtc/dtc-lexer.l | 2 +-
> scripts/dtc/dtc-lexer.lex.c_shipped | 2 +-
> scripts/dtc/dtc-parser.tab.c_shipped | 595 ++++-----
> scripts/dtc/dtc-parser.y | 9 +
> scripts/dtc/dtc.c | 29 +-
> scripts/dtc/dtc.h | 30 +
> scripts/dtc/livetree.c | 108 +-
> scripts/dtc/schema-test.c | 146 +++
> scripts/dtc/schema.c | 1304 ++++++++++++++++++++
> scripts/dtc/tests/schemas/array-size-1.schema | 13 +
> scripts/dtc/tests/schemas/array-size-2.schema | 8 +
> scripts/dtc/tests/schemas/array-size-3.schema | 8 +
> scripts/dtc/tests/schemas/array-size-4.schema | 8 +
> scripts/dtc/tests/schemas/children-nodes-1.schema | 5 +
> scripts/dtc/tests/schemas/children-nodes-2.schema | 5 +
> scripts/dtc/tests/schemas/inheritence-1.schema | 7 +
> scripts/dtc/tests/schemas/inheritence-2.schema | 8 +
> scripts/dtc/tests/schemas/integer-array-1.schema | 16 +
> scripts/dtc/tests/schemas/integer-array-2.schema | 9 +
> scripts/dtc/tests/schemas/integer-array-3.schema | 8 +
> scripts/dtc/tests/schemas/nodes-count-1.schema | 5 +
> scripts/dtc/tests/schemas/nodes-count-2.schema | 5 +
> scripts/dtc/tests/schemas/nodes-count-3.schema | 5 +
> scripts/dtc/tests/schemas/nodes-count-4.schema | 5 +
> scripts/dtc/tests/schemas/parent-nodes-1.schema | 23 +
> scripts/dtc/tests/schemas/parent-nodes-2.schema | 12 +
> scripts/dtc/tests/schemas/parent-nodes-3.schema | 14 +
> scripts/dtc/tests/schemas/parent-nodes-4.schema | 27 +
> scripts/dtc/tests/schemas/parent-nodes-5.schema | 15 +
> scripts/dtc/tests/schemas/parent-nodes-6.schema | 13 +
> scripts/dtc/tests/schemas/parent-nodes-7.schema | 13 +
> .../dtc/tests/schemas/pattern-matching-1.schema | 10 +
> .../dtc/tests/schemas/pattern-matching-2.schema | 10 +
> .../dtc/tests/schemas/required-property-1.schema | 7 +
> .../dtc/tests/schemas/required-property-2.schema | 7 +
> scripts/dtc/tests/schemas/string-array-1.schema | 20 +
> scripts/dtc/tests/schemas/string-array-2.schema | 9 +
> scripts/dtc/tests/schemas/types-1.schema | 12 +
> scripts/dtc/tests/schemas/types-2.schema | 7 +
> scripts/dtc/tests/test1.dts | 22 +
> 52 files changed, 2619 insertions(+), 356 deletions(-)
> create mode 100644 bindings/arm/omap/counter.schema
> create mode 100644 bindings/arm/omap/dsp.schema
> create mode 100644 bindings/arm/omap/intc.schema
> create mode 100644 bindings/arm/omap/iva.schema
> create mode 100644 bindings/arm/omap/l3-noc.schema
> create mode 100644 bindings/arm/omap/mpu.schema
> create mode 100644 bindings/arm/omap/omap.schema
> create mode 100644 bindings/arm/omap/timer.schema
> create mode 100644 scripts/dtc/schema-test.c
> create mode 100644 scripts/dtc/schema.c
> create mode 100644 scripts/dtc/tests/schemas/array-size-1.schema
> create mode 100644 scripts/dtc/tests/schemas/array-size-2.schema
> create mode 100644 scripts/dtc/tests/schemas/array-size-3.schema
> create mode 100644 scripts/dtc/tests/schemas/array-size-4.schema
> create mode 100644 scripts/dtc/tests/schemas/children-nodes-1.schema
> create mode 100644 scripts/dtc/tests/schemas/children-nodes-2.schema
> create mode 100644 scripts/dtc/tests/schemas/inheritence-1.schema
> create mode 100644 scripts/dtc/tests/schemas/inheritence-2.schema
> create mode 100644 scripts/dtc/tests/schemas/integer-array-1.schema
> create mode 100644 scripts/dtc/tests/schemas/integer-array-2.schema
> create mode 100644 scripts/dtc/tests/schemas/integer-array-3.schema
> create mode 100644 scripts/dtc/tests/schemas/nodes-count-1.schema
> create mode 100644 scripts/dtc/tests/schemas/nodes-count-2.schema
> create mode 100644 scripts/dtc/tests/schemas/nodes-count-3.schema
> create mode 100644 scripts/dtc/tests/schemas/nodes-count-4.schema
> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-1.schema
> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-2.schema
> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-3.schema
> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-4.schema
> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-5.schema
> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-6.schema
> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-7.schema
> create mode 100644 scripts/dtc/tests/schemas/pattern-matching-1.schema
> create mode 100644 scripts/dtc/tests/schemas/pattern-matching-2.schema
> create mode 100644 scripts/dtc/tests/schemas/required-property-1.schema
> create mode 100644 scripts/dtc/tests/schemas/required-property-2.schema
> create mode 100644 scripts/dtc/tests/schemas/string-array-1.schema
> create mode 100644 scripts/dtc/tests/schemas/string-array-2.schema
> create mode 100644 scripts/dtc/tests/schemas/types-1.schema
> create mode 100644 scripts/dtc/tests/schemas/types-2.schema
> create mode 100644 scripts/dtc/tests/test1.dts
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 8:06 ` Benoit Cousson
@ 2013-10-01 13:17 ` Rob Herring
2013-10-01 15:06 ` Benoit Cousson
2013-10-02 13:52 ` David Gibson
0 siblings, 2 replies; 21+ messages in thread
From: Rob Herring @ 2013-10-01 13:17 UTC (permalink / raw)
To: linux-arm-kernel
On 10/01/2013 03:06 AM, Benoit Cousson wrote:
> + more DT maintainers folks
>
> Hi all,
>
> I know this is mostly boring user space code, but I was expecting a
> little bit of comments about at least the bindings syntax:-(
>
> I'd like to know if this is the right direction and if it worth pursuing
> in that direction.
>
> The idea was to have at least some base for further discussion during
> ARM KS 2013.
>
> I feel alone :-(
>
> If you have any comment, go ahead!
Thanks for taking this on!
This is interesting approach using the dts syntax, but I worry that the
validation will only be as good as the schema written and the review of
the schema. I think the schema needs to define the binding rather than
define the checks. Then the schema can feed the validation checks. This
format does not seem to me as easily being able to generate
documentation from the schema which I believe is one of the goals. I for
one don't care to review the documentation and the schema for every binding.
I would like to ensure we can start with the basics and have some
generic rules. Some examples:
- flag compatible strings in dts files that are not documented
- check required properties
- check properties against cell sizes
- a node with reg property must have a unit address
- flag properties with "_"
- check properties are the correct data type
Rob
>
> Thanks,
> Benoit
>
> On 24/09/2013 18:52, Benoit Cousson wrote:
>> Hi All,
>>
>> Following the discussion that happened during LCE-2013 and the email
>> thread started by Tomasz few months ago [1], here is a first attempt
>> to introduce:
>> - a schema language to define the bindings accurately
>> - DTS validation during device tree compilation in DTC itself
>>
>> [1] http://www.spinics.net/lists/arm-kernel/msg262224.html
>>
>>
>> === What it does? ===
>>
>> For now device-tree bindings are defined in a not standardized
>> text-based format and thus any one can write the documentation for a
>> binding as he wish. In addition to this there is no automated way to
>> check if a dts is conform to the available bindings.
>>
>> The goal of this series of patch is to fix this situation by adding a
>> well defined way to write bindings in a human-readable format. These
>> bindings will be written through files called "schemas".
>>
>>
>> === What is a schema? ===
>>
>> A schema is a file describing the constraints that one or several nodes
>> of a dts must conform to. Schemas must use the file extension ".schema".
>> A schema can check that:
>> - A node has a given property
>> - An array has a valid size
>> - A node contains the required children.
>> - A property has the correct type.
>> - A property has the correct value.
>>
>> A schema can as well recursively check the constraints for parent nodes.
>>
>> The syntax for a schema is the same as the one for dts. This choice has
>> been made to simplify its development, to maximize the code reuse and
>> finally because the format is human-readable.
>>
>>
>> === How to defined a schema? ===
>>
>> A binding directory has been added at the root of repository. Users can
>> add schema files anywhere in it but a nice way would be to keep the same
>> structure as the binding directory in the documentation.
>>
>> To demonstrate how to write a schema and its capability I will use the
>> OMAP DTS schemas when applicable.
>>
>> How to:
>> * Associate a schema to one or several nodes
>>
>> As said earlier a schema can be used to validate one or several nodes
>> from a dts. To do this the "compatible" properties from the nodes which
>> should be validated must be present in the schema.
>>
>> timer1: timer at 4a318000 {
>> compatible = "ti,omap3430-timer";
>> reg = <0x4a318000 0x80>;
>> interrupts = <0x0 0x25 0x4>;
>> ti,hwmods = "timer1";
>> ti,timer-alwon;
>> };
>>
>> To write a schema which will validate OMAP Timers like the one above,
>> one may write the following schema:
>>
>> /dts-v1/;
>> / {
>> compatible = "ti,omap[0-9]+-timer";
>> ...
>> };
>>
>> The schema above will be used to validate every node in a dts which has
>> a compatible matching the following regular expression:
>> "ti,omap[0-9]+-timer".
>>
>> It is possible to specify multiple compatible inside a schema using this
>> syntax:
>> compatible = "ti,omap[0-9]+-timer", "ti,am[0-9]+-timer";
>>
>> This time the schema will be application for both OMAP Timers and AM
>> Timers.
>>
>>
>> * Define constraints on properties
>>
>> To define constraints on a property one has to create a node in a schema
>> which has as name the name of the property that one want to validate.
>>
>> To specify constraints on the property "ti,hwmods" of OMAP Timers one
>> can write this schema:
>>
>> /dts-v1/;
>> / {
>> compatible = "ti,omap[0-9]+-timer";
>> ti,hwmods {
>> ...
>> };
>> };
>>
>> If one want to use a regular as property name one can write this schema:
>>
>> /dts-v1/;
>> / {
>> compatible = "abc";
>> def {
>> name = "def[0-9]";
>> ...
>> };
>> };
>>
>> Above one can see that the "name" property override the node name.
>>
>>
>> * Require the presence of a property
>>
>> One can require the presence of a property by using the "is-required"
>> constraint.
>>
>> /dts-v1/;
>> / {
>> compatible = "ti,omap[0-9]+-timer";
>> ti,hwmods {
>> is-required;
>> };
>> };
>>
>> The "ti,hwmods" property above is set as required and its presence will
>> be checked in every OMAP timer.
>>
>>
>> * Require the presence of a property inside a node or inside one of its
>> parents
>>
>> Sometimes a property required is not directly present inside a node but
>> is present in one of its parents. To check this, one can use
>> "can-be-inherited" in addition to "is-required".
>>
>> twl {
>> interrupt-controller;
>>
>> rtc {
>> compatible = "ti,twl4030-rtc";
>> interrupts = <0xc>;
>> };
>> }
>>
>> In the case of the rtc above the interrupt-controller is not present,
>> but it is present in its parent. If inheriting the property from the
>> parent makes senses like here one can specify in the schema that
>> interrupt-controller is required in the rtc node and that the property
>> can be inherited from a parent.
>>
>> /dts-v1/;
>> / {
>> compatible = "ti,twl[0-9]+-rtc";
>> interrupt-controller {
>> is-required;
>> can-be-inherited;
>> };
>> };
>>
>>
>> * Require a node to contains a given list of children
>>
>> One may want to check if a node has some required children nodes.
>>
>> node {
>> compatible = "comp";
>>
>> subnode1 {
>> };
>>
>> subnode2 {
>> };
>>
>> abc {
>> };
>> };
>>
>> One can check if 'node' has the following subnode 'subnode1', 'subnode2',
>> and 'abc' with the schema below:
>>
>> /dts-v1/;
>> / {
>> compatible = "comp";
>> children = "abc", "subnode[0-9]";
>> };
>>
>>
>> * Constraints on array size
>>
>> One can specify the following constraints on array size:
>> - length: specify the exact length that an array must have.
>> - min-length: specify the minimum number of elements an array must
>> have.
>> - max-length: specify the maximum number of elements an array must
>> have.
>>
>> Usage example:
>> node {
>> compatible = "array_size";
>> myarray = <0 1 2 3 4>;
>> };
>>
>> Schema:
>> /dts-v1/;
>> / {
>> compatible = "array_size";
>>
>> myarray {
>> length = <5>;
>> };
>> };
>>
>>
>> * Count limit on nodes
>>
>> One can specify a count limit for the nodes matching the schema:
>> - count: if there is a match between a dts and a schema then there must
>> be exactly X match between the dts and the schema at the end of the
>> validation process.
>> - max-count: if there is a match between a dts and a schema then there
>> must be at most X match between the dts and the schema at the end of
>> the validation process.
>> This can be used to check if a specific node appears the right amount of
>> time in the dts.
>>
>> / {
>> timer1 {
>> compatible = "ti,omap-4430-timer";
>> ...
>> };
>>
>> timer2 {
>> compatible = "ti,omap-4430-timer";
>> ...
>> };
>> };
>>
>> If in the above dts there must be exactly two timer one can check this
>> constraints with the following schema:
>>
>> / {
>> compatible = "ti,omap-4430-timer";
>> count = <2>;
>> };
>>
>>
>> * Check that a property has the right type
>>
>> One can check if a property has the correct type. Right now dtc only
>> handles the two trivial types: integer array, string array. Since at the
>> end everything is an array of byte which may or may not be terminated by
>> a null byte this was enough.
>>
>> / {
>> compatible = "abc";
>>
>> abc = <0xa 0xb 0xc>;
>> def = "def", gef;
>> };
>>
>> To check that the property abc is an integer array and that the property
>> def is a string array for the dts above one can use the following schema:
>>
>> / {
>> compatible = "abc";
>>
>>
>> abc {
>> type = "integer";
>> };
>>
>> def {
>> type = "string";
>> };
>> };
>>
>>
>> * Check the value of properties
>>
>> It is possible to check if a property has the expected value through the
>> "value" constraint.
>>
>> abc {
>> prop1 = <0 1 2 3>;
>> prop2 = "value0", "value1", "value3";
>> };
>>
>> To check whether an integer array contains value from a given range
>> use the following constraint:
>> prop1 {
>> type = "integer";
>> value = <0x0 0xF>;
>> };
>>
>> To check whether a string array contains value that match a given
>> pattern use the following constraint:
>> prop2 {
>> type = "string";
>> value = "value[0-9]";
>> };
>>
>> To check whether a particular element of an array has the correct value
>> one can use the following constraint:
>> prop1 {
>> type = "integer";
>> value at 2 = <2>;
>> };
>>
>> or
>>
>> prop2 {
>> type = "string";
>> value at 1 = "value1";
>> };
>>
>>
>> * Check constraints on a parent node
>>
>> It is possible to set constraints on parents of a node.
>>
>> node {
>> compatible = "abcomp";
>> abc = "abc";
>>
>> subnode {
>> compatible = "comp";
>> abc = "def";
>> };
>> };
>>
>> The schema below tests whether 'subnode' has a parent named 'node' and
>> whether it has a compatible property equal to "abcomp" and a 'abc'
>> property equal to "abc". In the case of the node above the constraints
>> couldn't be check by inheriting the properties via 'can-be-inherited'
>> since they are overwritten by the node 'subnode'.
>>
>> /dts-v1/;
>> / {
>> compatible = "comp";
>>
>> parents {
>> node {
>> compatible {
>> type = "string";
>> value = "abcomp";
>> };
>>
>> abc {
>> type = "string";
>> value = "abc";
>> };
>> };
>> };
>> };
>>
>> It is possible to set conditional constraints on parents of the
>> following form:
>> if (node_compatible == "compat1")
>> check_this_parent_constraints();
>> else if (node_compatible == "compat2")
>> check_that_parent_constraints();
>>
>> To do this one should put the parent constraints at the same place as
>> the compatible definition in a schema file.
>>
>> /dts-v1/;
>> / {
>> compatible {
>> value at 0 {
>> value = "compat1";
>> parents {
>> node {
>> myprop {
>> type = "int";
>> value at 0 = <0xf>;
>> };
>> };
>> };
>> };
>>
>> value at 1 {
>> value = "compat2";
>> parents {
>> node {
>> myprop {
>> type = "int";
>> value at 0 = <0xa>;
>> };
>> };
>> };
>> };
>> };
>> };
>>
>> This schema will check that if the compatible of a node is "compat1"
>> then it must have a parent node "node" which has an integer array
>> property "myprop" which has as first element the value 0xf, otherwise
>> if the node has the
>> compatible "compat2" then the first element of the same property must
>> have the value 0xa.
>>
>>
>> === How is it working? ===
>>
>> When a user will try to compile a dts, dtc will parse the device-tree
>> and will try to find schemas that will help to check the correctness of
>> the dts. In order to accomplish that dtc maintain a small index of all
>> the schemas available.
>> Once dtc finds a schema which can be used to validate a particular node,
>> it will loads it and starts performing all the check defined by it.
>>
>> A set of unit-tests has been added to test that each feature is working
>> properly.
>>
>>
>> === Usage ===
>>
>> Two extra options are added to handle validation into DTC:
>> -M <schema-path> : path of the directoy containing the schemas
>> -B <verbose-level> : Level of verbosity from the schema validation
>>
>> dtc -M ./bindings -B 1 file.dts
>>
>>
>> === What could be done next? ===
>>
>> * Save the schema index to avoid to recompute it everytime.
>> * The constraints capabilities for parents and children of a node is
>> not equal right now. A nice thing would be to bring the same feature
>> available for constraints on a parent for children nodes.
>> * The type systems uses only the most trivial types. A nice thing would
>> be to bring higher level types.
>> * May be add conditional constraints based on property values.
>> * Need more? Feel free to add any item :-)
>>
>> Dependency: Please note that libpcre *must* be installed in order to
>> add the regular expression support into the schema validation process.
>> I'm not sure how such dependency should be handled, since the scripts
>> directory does not contain any dependency like that for the moment.
>>
>> The series, based on 3.12-rc2, is available here:
>> http://git.baylibre.com/pub/bcousson/linux-omap dts_schema
>>
>> Regards,
>> Fabien & Benoit
>>
>> ---
>>
>> Fabien Parent (15):
>> scripts/dtc: fix most memory leaks in dtc
>> scripts/dtc: build schema index for dts validation
>> scripts/dtc: validate each nodes and properties
>> scripts/dtc: add procedure to handle dts errors
>> scripts/dtc: check type on properties
>> scripts/dtc: check for required properties
>> scripts/dtc: can inherit properties
>> scripts/dtc: check array size
>> scripts/dtc: check value of properties
>> scripts/dtc: add count limit on nodes
>> scripts/dtc: check for children nodes
>> scripts/dtc: check constraints on parents
>> bindings: OMAP: add new schema files
>> scripts/dtc: validate dts against schema bindings
>> scripts/dtc: add verbose options
>>
>> bindings/arm/omap/counter.schema | 28 +
>> bindings/arm/omap/dsp.schema | 18 +
>> bindings/arm/omap/intc.schema | 48 +
>> bindings/arm/omap/iva.schema | 38 +
>> bindings/arm/omap/l3-noc.schema | 38 +
>> bindings/arm/omap/mpu.schema | 19 +
>> bindings/arm/omap/omap.schema | 62 +
>> bindings/arm/omap/timer.schema | 124 ++
>> scripts/Makefile.lib | 1 +
>> scripts/dtc/.gitignore | 2 +-
>> scripts/dtc/Makefile | 9 +-
>> scripts/dtc/data.c | 27 +-
>> scripts/dtc/dtc-lexer.l | 2 +-
>> scripts/dtc/dtc-lexer.lex.c_shipped | 2 +-
>> scripts/dtc/dtc-parser.tab.c_shipped | 595 ++++-----
>> scripts/dtc/dtc-parser.y | 9 +
>> scripts/dtc/dtc.c | 29 +-
>> scripts/dtc/dtc.h | 30 +
>> scripts/dtc/livetree.c | 108 +-
>> scripts/dtc/schema-test.c | 146 +++
>> scripts/dtc/schema.c | 1304
>> ++++++++++++++++++++
>> scripts/dtc/tests/schemas/array-size-1.schema | 13 +
>> scripts/dtc/tests/schemas/array-size-2.schema | 8 +
>> scripts/dtc/tests/schemas/array-size-3.schema | 8 +
>> scripts/dtc/tests/schemas/array-size-4.schema | 8 +
>> scripts/dtc/tests/schemas/children-nodes-1.schema | 5 +
>> scripts/dtc/tests/schemas/children-nodes-2.schema | 5 +
>> scripts/dtc/tests/schemas/inheritence-1.schema | 7 +
>> scripts/dtc/tests/schemas/inheritence-2.schema | 8 +
>> scripts/dtc/tests/schemas/integer-array-1.schema | 16 +
>> scripts/dtc/tests/schemas/integer-array-2.schema | 9 +
>> scripts/dtc/tests/schemas/integer-array-3.schema | 8 +
>> scripts/dtc/tests/schemas/nodes-count-1.schema | 5 +
>> scripts/dtc/tests/schemas/nodes-count-2.schema | 5 +
>> scripts/dtc/tests/schemas/nodes-count-3.schema | 5 +
>> scripts/dtc/tests/schemas/nodes-count-4.schema | 5 +
>> scripts/dtc/tests/schemas/parent-nodes-1.schema | 23 +
>> scripts/dtc/tests/schemas/parent-nodes-2.schema | 12 +
>> scripts/dtc/tests/schemas/parent-nodes-3.schema | 14 +
>> scripts/dtc/tests/schemas/parent-nodes-4.schema | 27 +
>> scripts/dtc/tests/schemas/parent-nodes-5.schema | 15 +
>> scripts/dtc/tests/schemas/parent-nodes-6.schema | 13 +
>> scripts/dtc/tests/schemas/parent-nodes-7.schema | 13 +
>> .../dtc/tests/schemas/pattern-matching-1.schema | 10 +
>> .../dtc/tests/schemas/pattern-matching-2.schema | 10 +
>> .../dtc/tests/schemas/required-property-1.schema | 7 +
>> .../dtc/tests/schemas/required-property-2.schema | 7 +
>> scripts/dtc/tests/schemas/string-array-1.schema | 20 +
>> scripts/dtc/tests/schemas/string-array-2.schema | 9 +
>> scripts/dtc/tests/schemas/types-1.schema | 12 +
>> scripts/dtc/tests/schemas/types-2.schema | 7 +
>> scripts/dtc/tests/test1.dts | 22 +
>> 52 files changed, 2619 insertions(+), 356 deletions(-)
>> create mode 100644 bindings/arm/omap/counter.schema
>> create mode 100644 bindings/arm/omap/dsp.schema
>> create mode 100644 bindings/arm/omap/intc.schema
>> create mode 100644 bindings/arm/omap/iva.schema
>> create mode 100644 bindings/arm/omap/l3-noc.schema
>> create mode 100644 bindings/arm/omap/mpu.schema
>> create mode 100644 bindings/arm/omap/omap.schema
>> create mode 100644 bindings/arm/omap/timer.schema
>> create mode 100644 scripts/dtc/schema-test.c
>> create mode 100644 scripts/dtc/schema.c
>> create mode 100644 scripts/dtc/tests/schemas/array-size-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/array-size-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/array-size-3.schema
>> create mode 100644 scripts/dtc/tests/schemas/array-size-4.schema
>> create mode 100644 scripts/dtc/tests/schemas/children-nodes-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/children-nodes-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/inheritence-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/inheritence-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/integer-array-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/integer-array-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/integer-array-3.schema
>> create mode 100644 scripts/dtc/tests/schemas/nodes-count-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/nodes-count-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/nodes-count-3.schema
>> create mode 100644 scripts/dtc/tests/schemas/nodes-count-4.schema
>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-3.schema
>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-4.schema
>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-5.schema
>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-6.schema
>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-7.schema
>> create mode 100644 scripts/dtc/tests/schemas/pattern-matching-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/pattern-matching-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/required-property-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/required-property-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/string-array-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/string-array-2.schema
>> create mode 100644 scripts/dtc/tests/schemas/types-1.schema
>> create mode 100644 scripts/dtc/tests/schemas/types-2.schema
>> create mode 100644 scripts/dtc/tests/test1.dts
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 13:17 ` Rob Herring
@ 2013-10-01 15:06 ` Benoit Cousson
2013-10-01 15:17 ` Jon Loeliger
2013-10-01 20:54 ` Rob Herring
2013-10-02 13:52 ` David Gibson
1 sibling, 2 replies; 21+ messages in thread
From: Benoit Cousson @ 2013-10-01 15:06 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rob,
On 01/10/2013 15:17, Rob Herring wrote:
> On 10/01/2013 03:06 AM, Benoit Cousson wrote:
>> + more DT maintainers folks
>>
>> Hi all,
>>
>> I know this is mostly boring user space code, but I was expecting a
>> little bit of comments about at least the bindings syntax:-(
>>
>> I'd like to know if this is the right direction and if it worth pursuing
>> in that direction.
>>
>> The idea was to have at least some base for further discussion during
>> ARM KS 2013.
>>
>> I feel alone :-(
>>
>> If you have any comment, go ahead!
>
> Thanks for taking this on!
>
> This is interesting approach using the dts syntax,
Well, this was discussed a little bit on the list, and it has the big
advantage of re-using the parser already included in DTC for free.
In term or readability, it avoids to re-defining a brand new syntax for
people who are already familiar with the DTS one.
> but I worry that the
> validation will only be as good as the schema written and the review of
> the schema.
Well, sure, but unfortunately, that will always be the case :-(
The bindings definition being quite open, there is no easy way to ensure
proper schema / bindings without careful review of the schema. There is
no such thing as a free beer... Unfortunately :-)
> I think the schema needs to define the binding rather than
> define the checks. Then the schema can feed the validation checks.
> This format does not seem to me as easily being able to generate
> documentation from the schema which I believe is one of the goals.
Indeed, but I think is it easy to generate any kind of readable format
for the documentation purpose if needed from the actual format.
Otherwise, we should consider a schema format based on kerneldoc type of
syntax to improve readability. I'm just afraid it will become harder
then to define complex schema.
BTW, what kind of documentation are you expecting here? Is is a text
that can be added on top of each schema?
> I for
> one don't care to review the documentation and the schema for every binding.
>
> I would like to ensure we can start with the basics and have some
> generic rules. Some examples:
>
> - flag compatible strings in dts files that are not documented
This is almost done with the last patch of the series. Nodes are already
checked, properties are missing.
> - check required properties
Done as well.
> - check properties against cell sizes
Yeah, that one is still to be done.
> - a node with reg property must have a unit address
Easy to add.
> - flag properties with "_"
Easy to add as well.
> - check properties are the correct data type
Already done for a couple of data type.
Thanks for your comments. Being the very first one, you might get a free
beer... meaning there might be such thing as a free beer :-)
Thanks,
Benoit
>> Thanks,
>> Benoit
>>
>> On 24/09/2013 18:52, Benoit Cousson wrote:
>>> Hi All,
>>>
>>> Following the discussion that happened during LCE-2013 and the email
>>> thread started by Tomasz few months ago [1], here is a first attempt
>>> to introduce:
>>> - a schema language to define the bindings accurately
>>> - DTS validation during device tree compilation in DTC itself
>>>
>>> [1] http://www.spinics.net/lists/arm-kernel/msg262224.html
>>>
>>>
>>> === What it does? ===
>>>
>>> For now device-tree bindings are defined in a not standardized
>>> text-based format and thus any one can write the documentation for a
>>> binding as he wish. In addition to this there is no automated way to
>>> check if a dts is conform to the available bindings.
>>>
>>> The goal of this series of patch is to fix this situation by adding a
>>> well defined way to write bindings in a human-readable format. These
>>> bindings will be written through files called "schemas".
>>>
>>>
>>> === What is a schema? ===
>>>
>>> A schema is a file describing the constraints that one or several nodes
>>> of a dts must conform to. Schemas must use the file extension ".schema".
>>> A schema can check that:
>>> - A node has a given property
>>> - An array has a valid size
>>> - A node contains the required children.
>>> - A property has the correct type.
>>> - A property has the correct value.
>>>
>>> A schema can as well recursively check the constraints for parent nodes.
>>>
>>> The syntax for a schema is the same as the one for dts. This choice has
>>> been made to simplify its development, to maximize the code reuse and
>>> finally because the format is human-readable.
>>>
>>>
>>> === How to defined a schema? ===
>>>
>>> A binding directory has been added at the root of repository. Users can
>>> add schema files anywhere in it but a nice way would be to keep the same
>>> structure as the binding directory in the documentation.
>>>
>>> To demonstrate how to write a schema and its capability I will use the
>>> OMAP DTS schemas when applicable.
>>>
>>> How to:
>>> * Associate a schema to one or several nodes
>>>
>>> As said earlier a schema can be used to validate one or several nodes
>>> from a dts. To do this the "compatible" properties from the nodes which
>>> should be validated must be present in the schema.
>>>
>>> timer1: timer at 4a318000 {
>>> compatible = "ti,omap3430-timer";
>>> reg = <0x4a318000 0x80>;
>>> interrupts = <0x0 0x25 0x4>;
>>> ti,hwmods = "timer1";
>>> ti,timer-alwon;
>>> };
>>>
>>> To write a schema which will validate OMAP Timers like the one above,
>>> one may write the following schema:
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "ti,omap[0-9]+-timer";
>>> ...
>>> };
>>>
>>> The schema above will be used to validate every node in a dts which has
>>> a compatible matching the following regular expression:
>>> "ti,omap[0-9]+-timer".
>>>
>>> It is possible to specify multiple compatible inside a schema using this
>>> syntax:
>>> compatible = "ti,omap[0-9]+-timer", "ti,am[0-9]+-timer";
>>>
>>> This time the schema will be application for both OMAP Timers and AM
>>> Timers.
>>>
>>>
>>> * Define constraints on properties
>>>
>>> To define constraints on a property one has to create a node in a schema
>>> which has as name the name of the property that one want to validate.
>>>
>>> To specify constraints on the property "ti,hwmods" of OMAP Timers one
>>> can write this schema:
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "ti,omap[0-9]+-timer";
>>> ti,hwmods {
>>> ...
>>> };
>>> };
>>>
>>> If one want to use a regular as property name one can write this schema:
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "abc";
>>> def {
>>> name = "def[0-9]";
>>> ...
>>> };
>>> };
>>>
>>> Above one can see that the "name" property override the node name.
>>>
>>>
>>> * Require the presence of a property
>>>
>>> One can require the presence of a property by using the "is-required"
>>> constraint.
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "ti,omap[0-9]+-timer";
>>> ti,hwmods {
>>> is-required;
>>> };
>>> };
>>>
>>> The "ti,hwmods" property above is set as required and its presence will
>>> be checked in every OMAP timer.
>>>
>>>
>>> * Require the presence of a property inside a node or inside one of its
>>> parents
>>>
>>> Sometimes a property required is not directly present inside a node but
>>> is present in one of its parents. To check this, one can use
>>> "can-be-inherited" in addition to "is-required".
>>>
>>> twl {
>>> interrupt-controller;
>>>
>>> rtc {
>>> compatible = "ti,twl4030-rtc";
>>> interrupts = <0xc>;
>>> };
>>> }
>>>
>>> In the case of the rtc above the interrupt-controller is not present,
>>> but it is present in its parent. If inheriting the property from the
>>> parent makes senses like here one can specify in the schema that
>>> interrupt-controller is required in the rtc node and that the property
>>> can be inherited from a parent.
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "ti,twl[0-9]+-rtc";
>>> interrupt-controller {
>>> is-required;
>>> can-be-inherited;
>>> };
>>> };
>>>
>>>
>>> * Require a node to contains a given list of children
>>>
>>> One may want to check if a node has some required children nodes.
>>>
>>> node {
>>> compatible = "comp";
>>>
>>> subnode1 {
>>> };
>>>
>>> subnode2 {
>>> };
>>>
>>> abc {
>>> };
>>> };
>>>
>>> One can check if 'node' has the following subnode 'subnode1', 'subnode2',
>>> and 'abc' with the schema below:
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "comp";
>>> children = "abc", "subnode[0-9]";
>>> };
>>>
>>>
>>> * Constraints on array size
>>>
>>> One can specify the following constraints on array size:
>>> - length: specify the exact length that an array must have.
>>> - min-length: specify the minimum number of elements an array must
>>> have.
>>> - max-length: specify the maximum number of elements an array must
>>> have.
>>>
>>> Usage example:
>>> node {
>>> compatible = "array_size";
>>> myarray = <0 1 2 3 4>;
>>> };
>>>
>>> Schema:
>>> /dts-v1/;
>>> / {
>>> compatible = "array_size";
>>>
>>> myarray {
>>> length = <5>;
>>> };
>>> };
>>>
>>>
>>> * Count limit on nodes
>>>
>>> One can specify a count limit for the nodes matching the schema:
>>> - count: if there is a match between a dts and a schema then there must
>>> be exactly X match between the dts and the schema at the end of the
>>> validation process.
>>> - max-count: if there is a match between a dts and a schema then there
>>> must be at most X match between the dts and the schema at the end of
>>> the validation process.
>>> This can be used to check if a specific node appears the right amount of
>>> time in the dts.
>>>
>>> / {
>>> timer1 {
>>> compatible = "ti,omap-4430-timer";
>>> ...
>>> };
>>>
>>> timer2 {
>>> compatible = "ti,omap-4430-timer";
>>> ...
>>> };
>>> };
>>>
>>> If in the above dts there must be exactly two timer one can check this
>>> constraints with the following schema:
>>>
>>> / {
>>> compatible = "ti,omap-4430-timer";
>>> count = <2>;
>>> };
>>>
>>>
>>> * Check that a property has the right type
>>>
>>> One can check if a property has the correct type. Right now dtc only
>>> handles the two trivial types: integer array, string array. Since at the
>>> end everything is an array of byte which may or may not be terminated by
>>> a null byte this was enough.
>>>
>>> / {
>>> compatible = "abc";
>>>
>>> abc = <0xa 0xb 0xc>;
>>> def = "def", gef;
>>> };
>>>
>>> To check that the property abc is an integer array and that the property
>>> def is a string array for the dts above one can use the following schema:
>>>
>>> / {
>>> compatible = "abc";
>>>
>>>
>>> abc {
>>> type = "integer";
>>> };
>>>
>>> def {
>>> type = "string";
>>> };
>>> };
>>>
>>>
>>> * Check the value of properties
>>>
>>> It is possible to check if a property has the expected value through the
>>> "value" constraint.
>>>
>>> abc {
>>> prop1 = <0 1 2 3>;
>>> prop2 = "value0", "value1", "value3";
>>> };
>>>
>>> To check whether an integer array contains value from a given range
>>> use the following constraint:
>>> prop1 {
>>> type = "integer";
>>> value = <0x0 0xF>;
>>> };
>>>
>>> To check whether a string array contains value that match a given
>>> pattern use the following constraint:
>>> prop2 {
>>> type = "string";
>>> value = "value[0-9]";
>>> };
>>>
>>> To check whether a particular element of an array has the correct value
>>> one can use the following constraint:
>>> prop1 {
>>> type = "integer";
>>> value at 2 = <2>;
>>> };
>>>
>>> or
>>>
>>> prop2 {
>>> type = "string";
>>> value at 1 = "value1";
>>> };
>>>
>>>
>>> * Check constraints on a parent node
>>>
>>> It is possible to set constraints on parents of a node.
>>>
>>> node {
>>> compatible = "abcomp";
>>> abc = "abc";
>>>
>>> subnode {
>>> compatible = "comp";
>>> abc = "def";
>>> };
>>> };
>>>
>>> The schema below tests whether 'subnode' has a parent named 'node' and
>>> whether it has a compatible property equal to "abcomp" and a 'abc'
>>> property equal to "abc". In the case of the node above the constraints
>>> couldn't be check by inheriting the properties via 'can-be-inherited'
>>> since they are overwritten by the node 'subnode'.
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "comp";
>>>
>>> parents {
>>> node {
>>> compatible {
>>> type = "string";
>>> value = "abcomp";
>>> };
>>>
>>> abc {
>>> type = "string";
>>> value = "abc";
>>> };
>>> };
>>> };
>>> };
>>>
>>> It is possible to set conditional constraints on parents of the
>>> following form:
>>> if (node_compatible == "compat1")
>>> check_this_parent_constraints();
>>> else if (node_compatible == "compat2")
>>> check_that_parent_constraints();
>>>
>>> To do this one should put the parent constraints at the same place as
>>> the compatible definition in a schema file.
>>>
>>> /dts-v1/;
>>> / {
>>> compatible {
>>> value at 0 {
>>> value = "compat1";
>>> parents {
>>> node {
>>> myprop {
>>> type = "int";
>>> value at 0 = <0xf>;
>>> };
>>> };
>>> };
>>> };
>>>
>>> value at 1 {
>>> value = "compat2";
>>> parents {
>>> node {
>>> myprop {
>>> type = "int";
>>> value at 0 = <0xa>;
>>> };
>>> };
>>> };
>>> };
>>> };
>>> };
>>>
>>> This schema will check that if the compatible of a node is "compat1"
>>> then it must have a parent node "node" which has an integer array
>>> property "myprop" which has as first element the value 0xf, otherwise
>>> if the node has the
>>> compatible "compat2" then the first element of the same property must
>>> have the value 0xa.
>>>
>>>
>>> === How is it working? ===
>>>
>>> When a user will try to compile a dts, dtc will parse the device-tree
>>> and will try to find schemas that will help to check the correctness of
>>> the dts. In order to accomplish that dtc maintain a small index of all
>>> the schemas available.
>>> Once dtc finds a schema which can be used to validate a particular node,
>>> it will loads it and starts performing all the check defined by it.
>>>
>>> A set of unit-tests has been added to test that each feature is working
>>> properly.
>>>
>>>
>>> === Usage ===
>>>
>>> Two extra options are added to handle validation into DTC:
>>> -M <schema-path> : path of the directoy containing the schemas
>>> -B <verbose-level> : Level of verbosity from the schema validation
>>>
>>> dtc -M ./bindings -B 1 file.dts
>>>
>>>
>>> === What could be done next? ===
>>>
>>> * Save the schema index to avoid to recompute it everytime.
>>> * The constraints capabilities for parents and children of a node is
>>> not equal right now. A nice thing would be to bring the same feature
>>> available for constraints on a parent for children nodes.
>>> * The type systems uses only the most trivial types. A nice thing would
>>> be to bring higher level types.
>>> * May be add conditional constraints based on property values.
>>> * Need more? Feel free to add any item :-)
>>>
>>> Dependency: Please note that libpcre *must* be installed in order to
>>> add the regular expression support into the schema validation process.
>>> I'm not sure how such dependency should be handled, since the scripts
>>> directory does not contain any dependency like that for the moment.
>>>
>>> The series, based on 3.12-rc2, is available here:
>>> http://git.baylibre.com/pub/bcousson/linux-omap dts_schema
>>>
>>> Regards,
>>> Fabien & Benoit
>>>
>>> ---
>>>
>>> Fabien Parent (15):
>>> scripts/dtc: fix most memory leaks in dtc
>>> scripts/dtc: build schema index for dts validation
>>> scripts/dtc: validate each nodes and properties
>>> scripts/dtc: add procedure to handle dts errors
>>> scripts/dtc: check type on properties
>>> scripts/dtc: check for required properties
>>> scripts/dtc: can inherit properties
>>> scripts/dtc: check array size
>>> scripts/dtc: check value of properties
>>> scripts/dtc: add count limit on nodes
>>> scripts/dtc: check for children nodes
>>> scripts/dtc: check constraints on parents
>>> bindings: OMAP: add new schema files
>>> scripts/dtc: validate dts against schema bindings
>>> scripts/dtc: add verbose options
>>>
>>> bindings/arm/omap/counter.schema | 28 +
>>> bindings/arm/omap/dsp.schema | 18 +
>>> bindings/arm/omap/intc.schema | 48 +
>>> bindings/arm/omap/iva.schema | 38 +
>>> bindings/arm/omap/l3-noc.schema | 38 +
>>> bindings/arm/omap/mpu.schema | 19 +
>>> bindings/arm/omap/omap.schema | 62 +
>>> bindings/arm/omap/timer.schema | 124 ++
>>> scripts/Makefile.lib | 1 +
>>> scripts/dtc/.gitignore | 2 +-
>>> scripts/dtc/Makefile | 9 +-
>>> scripts/dtc/data.c | 27 +-
>>> scripts/dtc/dtc-lexer.l | 2 +-
>>> scripts/dtc/dtc-lexer.lex.c_shipped | 2 +-
>>> scripts/dtc/dtc-parser.tab.c_shipped | 595 ++++-----
>>> scripts/dtc/dtc-parser.y | 9 +
>>> scripts/dtc/dtc.c | 29 +-
>>> scripts/dtc/dtc.h | 30 +
>>> scripts/dtc/livetree.c | 108 +-
>>> scripts/dtc/schema-test.c | 146 +++
>>> scripts/dtc/schema.c | 1304
>>> ++++++++++++++++++++
>>> scripts/dtc/tests/schemas/array-size-1.schema | 13 +
>>> scripts/dtc/tests/schemas/array-size-2.schema | 8 +
>>> scripts/dtc/tests/schemas/array-size-3.schema | 8 +
>>> scripts/dtc/tests/schemas/array-size-4.schema | 8 +
>>> scripts/dtc/tests/schemas/children-nodes-1.schema | 5 +
>>> scripts/dtc/tests/schemas/children-nodes-2.schema | 5 +
>>> scripts/dtc/tests/schemas/inheritence-1.schema | 7 +
>>> scripts/dtc/tests/schemas/inheritence-2.schema | 8 +
>>> scripts/dtc/tests/schemas/integer-array-1.schema | 16 +
>>> scripts/dtc/tests/schemas/integer-array-2.schema | 9 +
>>> scripts/dtc/tests/schemas/integer-array-3.schema | 8 +
>>> scripts/dtc/tests/schemas/nodes-count-1.schema | 5 +
>>> scripts/dtc/tests/schemas/nodes-count-2.schema | 5 +
>>> scripts/dtc/tests/schemas/nodes-count-3.schema | 5 +
>>> scripts/dtc/tests/schemas/nodes-count-4.schema | 5 +
>>> scripts/dtc/tests/schemas/parent-nodes-1.schema | 23 +
>>> scripts/dtc/tests/schemas/parent-nodes-2.schema | 12 +
>>> scripts/dtc/tests/schemas/parent-nodes-3.schema | 14 +
>>> scripts/dtc/tests/schemas/parent-nodes-4.schema | 27 +
>>> scripts/dtc/tests/schemas/parent-nodes-5.schema | 15 +
>>> scripts/dtc/tests/schemas/parent-nodes-6.schema | 13 +
>>> scripts/dtc/tests/schemas/parent-nodes-7.schema | 13 +
>>> .../dtc/tests/schemas/pattern-matching-1.schema | 10 +
>>> .../dtc/tests/schemas/pattern-matching-2.schema | 10 +
>>> .../dtc/tests/schemas/required-property-1.schema | 7 +
>>> .../dtc/tests/schemas/required-property-2.schema | 7 +
>>> scripts/dtc/tests/schemas/string-array-1.schema | 20 +
>>> scripts/dtc/tests/schemas/string-array-2.schema | 9 +
>>> scripts/dtc/tests/schemas/types-1.schema | 12 +
>>> scripts/dtc/tests/schemas/types-2.schema | 7 +
>>> scripts/dtc/tests/test1.dts | 22 +
>>> 52 files changed, 2619 insertions(+), 356 deletions(-)
>>> create mode 100644 bindings/arm/omap/counter.schema
>>> create mode 100644 bindings/arm/omap/dsp.schema
>>> create mode 100644 bindings/arm/omap/intc.schema
>>> create mode 100644 bindings/arm/omap/iva.schema
>>> create mode 100644 bindings/arm/omap/l3-noc.schema
>>> create mode 100644 bindings/arm/omap/mpu.schema
>>> create mode 100644 bindings/arm/omap/omap.schema
>>> create mode 100644 bindings/arm/omap/timer.schema
>>> create mode 100644 scripts/dtc/schema-test.c
>>> create mode 100644 scripts/dtc/schema.c
>>> create mode 100644 scripts/dtc/tests/schemas/array-size-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/array-size-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/array-size-3.schema
>>> create mode 100644 scripts/dtc/tests/schemas/array-size-4.schema
>>> create mode 100644 scripts/dtc/tests/schemas/children-nodes-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/children-nodes-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/inheritence-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/inheritence-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/integer-array-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/integer-array-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/integer-array-3.schema
>>> create mode 100644 scripts/dtc/tests/schemas/nodes-count-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/nodes-count-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/nodes-count-3.schema
>>> create mode 100644 scripts/dtc/tests/schemas/nodes-count-4.schema
>>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-3.schema
>>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-4.schema
>>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-5.schema
>>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-6.schema
>>> create mode 100644 scripts/dtc/tests/schemas/parent-nodes-7.schema
>>> create mode 100644 scripts/dtc/tests/schemas/pattern-matching-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/pattern-matching-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/required-property-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/required-property-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/string-array-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/string-array-2.schema
>>> create mode 100644 scripts/dtc/tests/schemas/types-1.schema
>>> create mode 100644 scripts/dtc/tests/schemas/types-2.schema
>>> create mode 100644 scripts/dtc/tests/test1.dts
>>>
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 15:06 ` Benoit Cousson
@ 2013-10-01 15:17 ` Jon Loeliger
2013-10-02 8:24 ` David Gibson
2013-10-02 9:25 ` Benoit Cousson
2013-10-01 20:54 ` Rob Herring
1 sibling, 2 replies; 21+ messages in thread
From: Jon Loeliger @ 2013-10-01 15:17 UTC (permalink / raw)
To: linux-arm-kernel
> Hi Rob,
>
> On 01/10/2013 15:17, Rob Herring wrote:
> > On 10/01/2013 03:06 AM, Benoit Cousson wrote:
> >> + more DT maintainers folks
> >>
> >> Hi all,
> >>
> >> I know this is mostly boring user space code, but I was expecting a
> >> little bit of comments about at least the bindings syntax:-(
> >>
> >> I'd like to know if this is the right direction and if it worth pursuing
> >> in that direction.
> >>
> >> The idea was to have at least some base for further discussion during
> >> ARM KS 2013.
> >>
> >> I feel alone :-(
> >>
> >> If you have any comment, go ahead!
Benoit,
Sorry, I meant to ask earlier but forgot.
Shouldn't this development be based on the
upstream DTC repository and not the in-kernel
copy of the DTC?
Thanks,
jdl
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 15:06 ` Benoit Cousson
2013-10-01 15:17 ` Jon Loeliger
@ 2013-10-01 20:54 ` Rob Herring
2013-10-02 13:54 ` David Gibson
1 sibling, 1 reply; 21+ messages in thread
From: Rob Herring @ 2013-10-01 20:54 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 1, 2013 at 10:06 AM, Benoit Cousson <bcousson@baylibre.com> wrote:
> Hi Rob,
>
>
> On 01/10/2013 15:17, Rob Herring wrote:
>>
>> On 10/01/2013 03:06 AM, Benoit Cousson wrote:
>>>
>>> + more DT maintainers folks
>>>
>>> Hi all,
>>>
>>> I know this is mostly boring user space code, but I was expecting a
>>> little bit of comments about at least the bindings syntax:-(
>>>
>>> I'd like to know if this is the right direction and if it worth pursuing
>>> in that direction.
>>>
>>> The idea was to have at least some base for further discussion during
>>> ARM KS 2013.
>>>
>>> I feel alone :-(
>>>
>>> If you have any comment, go ahead!
>>
>>
>> Thanks for taking this on!
>>
>> This is interesting approach using the dts syntax,
>
>
> Well, this was discussed a little bit on the list, and it has the big
> advantage of re-using the parser already included in DTC for free.
> In term or readability, it avoids to re-defining a brand new syntax for
> people who are already familiar with the DTS one.
>
>
>> but I worry that the
>> validation will only be as good as the schema written and the review of
>> the schema.
>
>
> Well, sure, but unfortunately, that will always be the case :-(
> The bindings definition being quite open, there is no easy way to ensure
> proper schema / bindings without careful review of the schema. There is no
> such thing as a free beer... Unfortunately :-)
>
>
>> I think the schema needs to define the binding rather than
>> define the checks. Then the schema can feed the validation checks.
>
>
>> This format does not seem to me as easily being able to generate
>> documentation from the schema which I believe is one of the goals.
>
>
> Indeed, but I think is it easy to generate any kind of readable format for
> the documentation purpose if needed from the actual format.
> Otherwise, we should consider a schema format based on kerneldoc type of
> syntax to improve readability. I'm just afraid it will become harder then to
> define complex schema.
>
> BTW, what kind of documentation are you expecting here? Is is a text that
> can be added on top of each schema?
I would expect the schema to replace
Documentation/devicetree/bindings/* over time. I think the thing that
needs to be worked out here is how to add free form multi-line text.
>> I for
>> one don't care to review the documentation and the schema for every
>> binding.
>>
>> I would like to ensure we can start with the basics and have some
>> generic rules. Some examples:
>>
>> - flag compatible strings in dts files that are not documented
>
>
> This is almost done with the last patch of the series. Nodes are already
> checked, properties are missing.
>
>> - check required properties
>
>
> Done as well.
>
>
>> - check properties against cell sizes
>
>
> Yeah, that one is still to be done.
>
>
>> - a node with reg property must have a unit address
>
>
> Easy to add.
>
>
>> - flag properties with "_"
>
>
> Easy to add as well.
>
>
>> - check properties are the correct data type
>
>
> Already done for a couple of data type.
Good to know. It wasn't really clear what common items are checked. An
example schema for clocks or gpio (or any other common) binding would
be nice.
Do you have a run with all the warnings found?
> Thanks for your comments. Being the very first one, you might get a free
> beer... meaning there might be such thing as a free beer :-)
In that case, it all looks perfect. :)
Rob
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-09-24 16:52 [RFC 00/15] Device Tree schemas and validation Benoit Cousson
2013-10-01 8:06 ` Benoit Cousson
@ 2013-10-01 22:22 ` Stephen Warren
2013-10-02 14:29 ` David Gibson
2013-10-03 13:17 ` Benoit Cousson
[not found] ` <1380041541-17529-2-git-send-email-bcousson@baylibre.com>
2 siblings, 2 replies; 21+ messages in thread
From: Stephen Warren @ 2013-10-01 22:22 UTC (permalink / raw)
To: linux-arm-kernel
On 09/24/2013 10:52 AM, Benoit Cousson wrote:
> Hi All,
>
> Following the discussion that happened during LCE-2013 and the email
> thread started by Tomasz few months ago [1], here is a first attempt
> to introduce:
> - a schema language to define the bindings accurately
> - DTS validation during device tree compilation in DTC itself
Sorry, this is probably going to sound a bit negative. Hopefully you
find it constructive though.
> The syntax for a schema is the same as the one for dts. This choice has
> been made to simplify its development, to maximize the code reuse and
> finally because the format is human-readable.
I'm not convinced that's a good decision.
DT is a language for representing data.
The validation checks described by schemas are rules, or code, and not
static data.
So, while I'm sure it's possible to shoe-horn at least some reasonable
subset of DT validation into DT syntax itself, I feel it's unlikely to
yield something that's scalable enough.
For example, it's easy to specify that a property must be 2 cells long.
What if it could be any multiple of two? That's a lot of numbers to
explicitly enumerate as data. Sure, you can then invent syntax to
represent that specific rule (parameterized by 2), but what about the
next similar-but-different rule? The only approach I can think of to
that is to allow the schema to contain arbitrary expressions, which
would likely need to morph into arbitary statements not just
expressions. Once you're there, I think the schema would be better
represented as a programming language rather than as a data structure
that could have code hooked into it.
> How to:
> * Associate a schema to one or several nodes
>
> As said earlier a schema can be used to validate one or several nodes
> from a dts. To do this the "compatible" properties from the nodes which
> should be validated must be present in the schema.
>
> timer1: timer at 4a318000 {
> compatible = "ti,omap3430-timer";
...
> To write a schema which will validate OMAP Timers like the one above,
> one may write the following schema:
>
> /dts-v1/;
> / {
> compatible = "ti,omap[0-9]+-timer";
What about DT nodes that don't have a compatible value? We certainly
have some of those already like /memory and /chosen. We should be able
to validate their schema too. This probably doesn't invalidate being
able to look things up by compatible value though; it just means we need
some additional mechanisms too.
> * Define constraints on properties
>
> To define constraints on a property one has to create a node in a schema
> which has as name the name of the property that one want to validate.
>
> To specify constraints on the property "ti,hwmods" of OMAP Timers one
> can write this schema:
>
> /dts-v1/;
> / {
> compatible = "ti,omap[0-9]+-timer";
> ti,hwmods {
> ...
> };
compatible and ti,hwmods are both properties in the DT file. However, in
the schema above, one appears as a property, and one as a node. I don't
like that inconsistency. It'd be better if compatible was a node too.
> If one want to use a regular as property name one can write this schema:
>
> /dts-v1/;
> / {
> compatible = "abc";
> def {
> name = "def[0-9]";
Isn't it valid to have a property named "name" within the node itself?
How do you differentiate between specifying the node name and the name
property?
What if the node name needs more validation than just a regex. For
example, suppose we want to validate the
unit-name-must-match-reg-address rule. We need to write some complex
expression using data extracted from reg to calculate the unit address.
Equally, the node name perhaps has to exist in some global list of
acceptable node names. It would be extremely tricky if not impossible to
do that with a regex.
> ...
> };
> };
>
> Above one can see that the "name" property override the node name.
Override implies that dtc would change the node name during compilation.
I think s/override/validate/ or s/override/overrides the validation
rules for/?
> * Require the presence of a property inside a node or inside one of its
> parents
...
> /dts-v1/;
> / {
> compatible = "ti,twl[0-9]+-rtc";
> interrupt-controller {
> is-required;
> can-be-inherited;
interrupt-controller isn't a good example here, since it isn't a
property that would typically be inherited. Why not use interrupt-parent
instead?
> One can check if 'node' has the following subnode 'subnode1', 'subnode2',
> and 'abc' with the schema below:
>
> /dts-v1/;
> / {
> compatible = "comp";
> children = "abc", "subnode[0-9]";
> };
How is the schema for each sub-node specified?
What if some nodes are optional and some required? The conditions where
a sub-node is required might be complex, and I think we'd always want to
be able to represent them in whatever schema language we chose.
The most obvious way would be to make each sub-node's schema appear as a
sub-node within the main node's schema, but then how do you tell if a
schema node describes a property or a node?
Note that the following DT file is currently accepted by dtc even if it
may not be the best choice of property and node names:
==========
/dts-v1/;
/ {
foo = <1>;
foo {};
};
==========
> * Constraints on array size
>
> One can specify the following constraints on array size:
> - length: specify the exact length that an array must have.
> - min-length: specify the minimum number of elements an array must have.
> - max-length: specify the maximum number of elements an array must have.
This seems rather inflexible; it'll cover a lot of the simple cases, but
hit a wall pretty soon. For example, how would it validate a property
that is supposed to include 3 GPIO specifiers, where the GPIO specifiers
are going to have DT-specific lengths, since the length of each
specifier is defined by the node that the phandles reference?
Overall, I believe perhaps the single most important aspect of any DT
schema is schema inheritance or instancing, and this proposal doesn't
appear to address that issue at all.
Inheritance of schemas:
For example, any node that is addressed must contain a reg property. The
constraints on that property are identical in all bindings; it must
consist of #address-cells + #size-cells integer values (cells). We don't
want to have to cut/paste that rule into every single binding
definition. Rather, we should simply say something like "this binding
uses the reg property", and the schema validation tool will look up the
definition of "reg property", and hence know how to validate it.
Similarly, any binding that describes a GPIO controller will have some
similar requirements; the gpio-controller and #gpio-cells properties
must be present. The schema should simply say "I'm a GPIO controller",
and the schema tool should add some extra requirements to nodes of that
type.
Instancing of schemas:
Any binding that uses GPIOs should be able to say that a particular
property (e.g. "enable-gpios") is-a GPIO-specifier (with parameters
"enable" for the property name, min/max/expression length, etc.), and
then the schema validation tool would know to apply rules for a
specifier list to that property (and be able to check the property name).
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 15:17 ` Jon Loeliger
@ 2013-10-02 8:24 ` David Gibson
2013-10-02 9:25 ` Benoit Cousson
1 sibling, 0 replies; 21+ messages in thread
From: David Gibson @ 2013-10-02 8:24 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 01, 2013 at 10:17:53AM -0500, Jon Loeliger wrote:
> > Hi Rob,
> >
> > On 01/10/2013 15:17, Rob Herring wrote:
> > > On 10/01/2013 03:06 AM, Benoit Cousson wrote:
> > >> + more DT maintainers folks
> > >>
> > >> Hi all,
> > >>
> > >> I know this is mostly boring user space code, but I was expecting a
> > >> little bit of comments about at least the bindings syntax:-(
> > >>
> > >> I'd like to know if this is the right direction and if it worth pursuing
> > >> in that direction.
> > >>
> > >> The idea was to have at least some base for further discussion during
> > >> ARM KS 2013.
> > >>
> > >> I feel alone :-(
> > >>
> > >> If you have any comment, go ahead!
>
>
> Benoit,
>
> Sorry, I meant to ask earlier but forgot.
> Shouldn't this development be based on the
> upstream DTC repository and not the in-kernel
> copy of the DTC?
Absolutely. Please work against upstream dtc.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131002/982f4d1c/attachment-0001.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 15:17 ` Jon Loeliger
2013-10-02 8:24 ` David Gibson
@ 2013-10-02 9:25 ` Benoit Cousson
2013-10-02 13:22 ` Jon Loeliger
1 sibling, 1 reply; 21+ messages in thread
From: Benoit Cousson @ 2013-10-02 9:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jon,
On 01/10/2013 17:17, Jon Loeliger wrote:
>> Hi Rob,
>>
>> On 01/10/2013 15:17, Rob Herring wrote:
>>> On 10/01/2013 03:06 AM, Benoit Cousson wrote:
>>>> + more DT maintainers folks
>>>>
>>>> Hi all,
>>>>
>>>> I know this is mostly boring user space code, but I was expecting a
>>>> little bit of comments about at least the bindings syntax:-(
>>>>
>>>> I'd like to know if this is the right direction and if it worth pursuing
>>>> in that direction.
>>>>
>>>> The idea was to have at least some base for further discussion during
>>>> ARM KS 2013.
>>>>
>>>> I feel alone :-(
>>>>
>>>> If you have any comment, go ahead!
>
>
> Benoit,
>
> Sorry, I meant to ask earlier but forgot.
> Shouldn't this development be based on the
> upstream DTC repository and not the in-kernel
> copy of the DTC?
Eventually, yes, but here the main point is to discuss the schema that
will be used to defined bindings.
In that case, the DTC patches code are mostly a proof of concept using
the Linux kernel as example.
Regards,
Benoit
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 01/15] scripts/dtc: fix most memory leaks in dtc
[not found] ` <1380041541-17529-2-git-send-email-bcousson@baylibre.com>
@ 2013-10-02 12:59 ` David Gibson
[not found] ` <CAOwMV_zAZG3vvWS6pkyK-FbOEg_32KRO-k1SmFSh-pc9+0JiPA@mail.gmail.com>
0 siblings, 1 reply; 21+ messages in thread
From: David Gibson @ 2013-10-02 12:59 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Sep 24, 2013 at 06:52:07PM +0200, Benoit Cousson wrote:
> From: Fabien Parent <fparent@baylibre.com>
As noted elsewhere, please write patches against upstream dtc, not the
version in the kernel. git://git.kernel.org/pub/scm/utils/dtc/dtc.git
> There are a few memory leaks in dtc which until now were not that important
> since they were all in the parser and only one instance of the parser was run
> per instance of dtc. The following commits will add a validation of dts through
> schema which have the same syntax as dts, i.e. the parser of dts will be reused
> to parse schema. The consequence is that instead of having the parser running
> only one time for an instance of the dtc process, the parser will run
> many many times and thus the leak that were not important until now becomes
> urgent to fix.
Hrm, yeah. I'm aware that I haven't been very careful with memory
leaks within the parser. Essentially, I've been treating the process
as a pool allocator with exactly one pool - I've even considered
getting rid of those free()s we do have.
If the parser's going to be invoked repeatedly, then, yes, that
certainly needs fixing. Whether individually fixing each leak or
using a explicit pool allocator is a better option is less clear.
> dtc-lexer: Do not duplicate the string which contains literals because the
> string is directly converted afterward to an integer and is never used again.
> livetree: Add a bunch of free helper functions to clean properly the
> dt.
This is no good. Making this assumption is a layering violation - if
the parser was changed so that this literal wasn't converted until
after another token was read, the yytext value copied in here would no
longer be valid and would break horribly.
>
> Signed-off-by: Fabien Parent <fparent@baylibre.com>
> Signed-off-by: Benoit Cousson <bcousson@baylibre.com>
> ---
> scripts/dtc/dtc-lexer.l | 2 +-
> scripts/dtc/dtc-lexer.lex.c_shipped | 2 +-
> scripts/dtc/dtc.c | 1 +
> scripts/dtc/dtc.h | 1 +
> scripts/dtc/livetree.c | 108 +++++++++++++++++++++++++++++++++---
> 5 files changed, 105 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/dtc/dtc-lexer.l b/scripts/dtc/dtc-lexer.l
> index 3b41bfc..4f63fbf 100644
> --- a/scripts/dtc/dtc-lexer.l
> +++ b/scripts/dtc/dtc-lexer.l
> @@ -146,7 +146,7 @@ static int pop_input_file(void);
> }
>
> <V1>([0-9]+|0[xX][0-9a-fA-F]+)(U|L|UL|LL|ULL)? {
> - yylval.literal = xstrdup(yytext);
> + yylval.literal = yytext;
> DPRINT("Literal: '%s'\n", yylval.literal);
> return DT_LITERAL;
> }
> diff --git a/scripts/dtc/dtc-lexer.lex.c_shipped b/scripts/dtc/dtc-lexer.lex.c_shipped
> index 2d30f41..5c0d27c 100644
> --- a/scripts/dtc/dtc-lexer.lex.c_shipped
> +++ b/scripts/dtc/dtc-lexer.lex.c_shipped
> @@ -1054,7 +1054,7 @@ case 10:
> YY_RULE_SETUP
> #line 148 "dtc-lexer.l"
> {
> - yylval.literal = xstrdup(yytext);
> + yylval.literal = yytext;
> DPRINT("Literal: '%s'\n", yylval.literal);
> return DT_LITERAL;
> }
> diff --git a/scripts/dtc/dtc.c b/scripts/dtc/dtc.c
> index a375683..215ae92 100644
> --- a/scripts/dtc/dtc.c
> +++ b/scripts/dtc/dtc.c
> @@ -256,5 +256,6 @@ int main(int argc, char *argv[])
> die("Unknown output format \"%s\"\n", outform);
> }
>
> + free_dt(bi);
> exit(0);
> }
> diff --git a/scripts/dtc/dtc.h b/scripts/dtc/dtc.h
> index 3e42a07..9c45fd2 100644
> --- a/scripts/dtc/dtc.h
> +++ b/scripts/dtc/dtc.h
> @@ -245,6 +245,7 @@ struct boot_info {
> struct boot_info *build_boot_info(struct reserve_info *reservelist,
> struct node *tree, uint32_t boot_cpuid_phys);
> void sort_tree(struct boot_info *bi);
> +void free_dt(struct boot_info *bi);
>
> /* Checks */
>
> diff --git a/scripts/dtc/livetree.c b/scripts/dtc/livetree.c
> index b61465f..5c8692c 100644
> --- a/scripts/dtc/livetree.c
> +++ b/scripts/dtc/livetree.c
> @@ -20,6 +20,10 @@
>
> #include "dtc.h"
>
> +static void free_node_list(struct node *n);
> +static void free_node(struct node *n);
> +static void free_property(struct property *p);
> +
> /*
> * Tree building functions
> */
> @@ -144,7 +148,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>
> /* Add new node labels to old node */
> for_each_label_withdel(new_node->labels, l)
> - add_label(&old_node->labels, l->label);
> + add_label(&old_node->labels, xstrdup(l->label));
>
> /* Move properties from the new node to the old node. If there
> * is a collision, replace the old value with the new */
> @@ -156,7 +160,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>
> if (new_prop->deleted) {
> delete_property_by_name(old_node, new_prop->name);
> - free(new_prop);
> + free_property(new_prop);
> continue;
> }
>
> @@ -165,7 +169,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
> if (streq(old_prop->name, new_prop->name)) {
> /* Add new labels to old property */
> for_each_label_withdel(new_prop->labels, l)
> - add_label(&old_prop->labels, l->label);
> + add_label(&old_prop->labels, xstrdup(l->label));
>
> old_prop->val = new_prop->val;
> old_prop->deleted = 0;
> @@ -191,7 +195,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>
> if (new_child->deleted) {
> delete_node_by_name(old_node, new_child->name);
> - free(new_child);
> + free_node(new_child);
> continue;
> }
>
> @@ -211,7 +215,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>
> /* The new node contents are now merged into the old node. Free
> * the new node. */
> - free(new_node);
> + free_node(new_node);
>
> return old_node;
> }
> @@ -532,13 +536,13 @@ cell_t get_node_phandle(struct node *root, struct node *node)
> if (!get_property(node, "linux,phandle")
> && (phandle_format & PHANDLE_LEGACY))
> add_property(node,
> - build_property("linux,phandle",
> + build_property(xstrdup("linux,phandle"),
> data_append_cell(empty_data, phandle)));
>
> if (!get_property(node, "phandle")
> && (phandle_format & PHANDLE_EPAPR))
> add_property(node,
> - build_property("phandle",
> + build_property(xstrdup("phandle"),
> data_append_cell(empty_data, phandle)));
>
> /* If the node *does* have a phandle property, we must
> @@ -707,3 +711,93 @@ void sort_tree(struct boot_info *bi)
> sort_reserve_entries(bi);
> sort_node(bi->dt);
> }
> +
> +static void free_marker_list(struct marker *m)
> +{
> + struct marker *marker, *marker_next;
> +
> + if (!m)
> + return;
> +
> + for (marker = m, marker_next = marker ? marker->next : NULL;
> + marker;
> + marker = marker_next, marker_next = marker ? marker->next : NULL) {
Rather hard to read version of safe-against-free list iteration.
> + free(marker->ref);
> + free(marker);
> + }
> +}
> +
> +static void free_label_list(struct label *l)
> +{
> + struct label *label, *label_next;
> +
> + if (!l)
> + return;
> +
> + for (label = l, label_next = label ? label->next : NULL;
> + label;
> + label = label_next, label_next = label ? label->next : NULL) {
> + free(label->label);
> + free(label);
> + }
> +}
> +
> +static void free_property(struct property *p)
> +{
> + if (!p)
> + return;
> +
> + free_label_list(p->labels);
> + free_marker_list(p->val.markers);
> + free(p->val.val);
> + free(p->name);
> + free(p);
> +}
> +
> +static void free_property_list(struct property *p)
> +{
> + struct property *next;
> +
> + if (!p)
> + return;
> +
> + for (next = p->next; p; p = next, next = p ? p->next : NULL)
> + free_property(p);
> +}
> +
> +static void free_node(struct node *n)
> +{
> + if (!n)
> + return;
> +
> + free_node_list(n->children);
> + free_label_list(n->labels);
> + free_property_list(n->proplist);
> + free(n->fullpath);
> + if (n->name && *n->name)
*n->name .. so, the name can (and must) be statically allocated if
it's exactly "". Not a useful optimization, I think.
> + free(n->name);
> + free(n);
> +}
> +
> +static void free_node_list(struct node *n)
> +{
> + struct node *next;
> +
> + if (!n)
> + return;
> +
> + for (next = n->next_sibling;
> + n;
> + n = next, next = n ? n->next_sibling : NULL) {
> + free_node(n);
> + }
> +}
> +
> +void free_dt(struct boot_info *bi)
> +{
> + if (!bi)
> + return;
> +
> + free_node_list(bi->dt);
> + free(bi);
> +}
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131002/8ca918da/attachment-0001.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-02 9:25 ` Benoit Cousson
@ 2013-10-02 13:22 ` Jon Loeliger
0 siblings, 0 replies; 21+ messages in thread
From: Jon Loeliger @ 2013-10-02 13:22 UTC (permalink / raw)
To: linux-arm-kernel
> >
> > Benoit,
> >
> > Sorry, I meant to ask earlier but forgot.
> > Shouldn't this development be based on the
> > upstream DTC repository and not the in-kernel
> > copy of the DTC?
>
> Eventually, yes,
We should *start* there, though.
> but here the main point is to discuss the schema that
> will be used to defined bindings.
> In that case, the DTC patches code are mostly a proof of concept
Sure; all that is fine.
> using the Linux kernel as example.
Example?(*) We shouldn't churn the kernel.
jdl
(*) In the sense that *this* Universe is just
one example from all known Universes? :-)
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 13:17 ` Rob Herring
2013-10-01 15:06 ` Benoit Cousson
@ 2013-10-02 13:52 ` David Gibson
1 sibling, 0 replies; 21+ messages in thread
From: David Gibson @ 2013-10-02 13:52 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 01, 2013 at 08:17:42AM -0500, Rob Herring wrote:
> On 10/01/2013 03:06 AM, Benoit Cousson wrote:
> > + more DT maintainers folks
> >
> > Hi all,
> >
> > I know this is mostly boring user space code, but I was expecting a
> > little bit of comments about at least the bindings syntax:-(
> >
> > I'd like to know if this is the right direction and if it worth pursuing
> > in that direction.
> >
> > The idea was to have at least some base for further discussion during
> > ARM KS 2013.
> >
> > I feel alone :-(
> >
> > If you have any comment, go ahead!
>
> Thanks for taking this on!
>
> This is interesting approach using the dts syntax, but I worry that the
> validation will only be as good as the schema written and the review of
> the schema. I think the schema needs to define the binding rather than
> define the checks. Then the schema can feed the validation checks. This
> format does not seem to me as easily being able to generate
> documentation from the schema which I believe is one of the goals. I for
> one don't care to review the documentation and the schema for every binding.
Hrm. I'm less optimistic about entirely replacing human-readable
bindings with machine-readable schemas. But I do think the schema
language needs to be substantially more flexible than the draft
presented here.
While I think a schema syntax which mirrors dts syntax makes a lot of
sense, actually defining schemas as "device" trees doesn't seem quite
right to me.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131002/83f7e873/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 20:54 ` Rob Herring
@ 2013-10-02 13:54 ` David Gibson
2013-10-02 18:08 ` Mark Brown
2013-10-03 6:52 ` Benoit Cousson
0 siblings, 2 replies; 21+ messages in thread
From: David Gibson @ 2013-10-02 13:54 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 01, 2013 at 03:54:20PM -0500, Rob Herring wrote:
> On Tue, Oct 1, 2013 at 10:06 AM, Benoit Cousson <bcousson@baylibre.com> wrote:
> > Hi Rob,
> >
> >
> > On 01/10/2013 15:17, Rob Herring wrote:
> >>
> >> On 10/01/2013 03:06 AM, Benoit Cousson wrote:
> >>>
> >>> + more DT maintainers folks
> >>>
> >>> Hi all,
> >>>
> >>> I know this is mostly boring user space code, but I was expecting a
> >>> little bit of comments about at least the bindings syntax:-(
> >>>
> >>> I'd like to know if this is the right direction and if it worth pursuing
> >>> in that direction.
> >>>
> >>> The idea was to have at least some base for further discussion during
> >>> ARM KS 2013.
> >>>
> >>> I feel alone :-(
> >>>
> >>> If you have any comment, go ahead!
> >>
> >>
> >> Thanks for taking this on!
> >>
> >> This is interesting approach using the dts syntax,
> >
> >
> > Well, this was discussed a little bit on the list, and it has the big
> > advantage of re-using the parser already included in DTC for free.
> > In term or readability, it avoids to re-defining a brand new syntax for
> > people who are already familiar with the DTS one.
> >
> >
> >> but I worry that the
> >> validation will only be as good as the schema written and the review of
> >> the schema.
> >
> >
> > Well, sure, but unfortunately, that will always be the case :-(
> > The bindings definition being quite open, there is no easy way to ensure
> > proper schema / bindings without careful review of the schema. There is no
> > such thing as a free beer... Unfortunately :-)
> >
> >
> >> I think the schema needs to define the binding rather than
> >> define the checks. Then the schema can feed the validation checks.
> >
> >
> >> This format does not seem to me as easily being able to generate
> >> documentation from the schema which I believe is one of the goals.
> >
> >
> > Indeed, but I think is it easy to generate any kind of readable format for
> > the documentation purpose if needed from the actual format.
> > Otherwise, we should consider a schema format based on kerneldoc type of
> > syntax to improve readability. I'm just afraid it will become harder then to
> > define complex schema.
> >
> > BTW, what kind of documentation are you expecting here? Is is a text that
> > can be added on top of each schema?
>
> I would expect the schema to replace
> Documentation/devicetree/bindings/* over time. I think the thing that
> needs to be worked out here is how to add free form multi-line text.
I'm not convinced that's a realistic goal. As I see it, the
fundamental difference between a binding document and a formal schema
is that a binding defines both the syntax required of a node, and its
semantics, whereas a schema defines only syntax - the semantics still
need to be defined somewhere.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131002/080a5fd4/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 22:22 ` Stephen Warren
@ 2013-10-02 14:29 ` David Gibson
2013-10-03 13:53 ` Benoit Cousson
2013-10-03 13:17 ` Benoit Cousson
1 sibling, 1 reply; 21+ messages in thread
From: David Gibson @ 2013-10-02 14:29 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 01, 2013 at 04:22:24PM -0600, Stephen Warren wrote:
> On 09/24/2013 10:52 AM, Benoit Cousson wrote:
> > Hi All,
> >
> > Following the discussion that happened during LCE-2013 and the email
> > thread started by Tomasz few months ago [1], here is a first attempt
> > to introduce:
> > - a schema language to define the bindings accurately
> > - DTS validation during device tree compilation in DTC itself
>
> Sorry, this is probably going to sound a bit negative. Hopefully you
> find it constructive though.
>
> > The syntax for a schema is the same as the one for dts. This choice has
> > been made to simplify its development, to maximize the code reuse and
> > finally because the format is human-readable.
>
> I'm not convinced that's a good decision.
>
> DT is a language for representing data.
>
> The validation checks described by schemas are rules, or code, and not
> static data.
>
> So, while I'm sure it's possible to shoe-horn at least some reasonable
> subset of DT validation into DT syntax itself, I feel it's unlikely to
> yield something that's scalable enough.
I tend to agree.
> For example, it's easy to specify that a property must be 2 cells long.
> What if it could be any multiple of two? That's a lot of numbers to
> explicitly enumerate as data. Sure, you can then invent syntax to
> represent that specific rule (parameterized by 2), but what about the
> next similar-but-different rule? The only approach I can think of to
> that is to allow the schema to contain arbitrary expressions, which
> would likely need to morph into arbitary statements not just
> expressions. Once you're there, I think the schema would be better
> represented as a programming language rather than as a data structure
> that could have code hooked into it.
>
> > How to:
> > * Associate a schema to one or several nodes
> >
> > As said earlier a schema can be used to validate one or several nodes
> > from a dts. To do this the "compatible" properties from the nodes which
> > should be validated must be present in the schema.
> >
> > timer1: timer at 4a318000 {
> > compatible = "ti,omap3430-timer";
> ...
> > To write a schema which will validate OMAP Timers like the one above,
> > one may write the following schema:
> >
> > /dts-v1/;
> > / {
> > compatible = "ti,omap[0-9]+-timer";
>
> What about DT nodes that don't have a compatible value? We certainly
> have some of those already like /memory and /chosen. We should be able
> to validate their schema too. This probably doesn't invalidate being
> able to look things up by compatible value though; it just means we need
> some additional mechanisms too.
More to the point, what about the properties of a node whose format is
defined not by this node's binding but by some other nodes binding.
e.g. the exact format of reg and ranges is at least partially
determined by the parent bus's binding, and interrupts is defined
partially by the interrupt parent's binding. gpio properties are
defined by a combination of a global binding and the gpio parent,
IIRC.
> > * Define constraints on properties
> >
> > To define constraints on a property one has to create a node in a schema
> > which has as name the name of the property that one want to validate.
> >
> > To specify constraints on the property "ti,hwmods" of OMAP Timers one
> > can write this schema:
> >
> > /dts-v1/;
> > / {
> > compatible = "ti,omap[0-9]+-timer";
> > ti,hwmods {
> > ...
> > };
>
> compatible and ti,hwmods are both properties in the DT file. However, in
> the schema above, one appears as a property, and one as a node. I don't
> like that inconsistency. It'd be better if compatible was a node too.
Essentially what's going on here is that to describe the constraint on
a property, a node with corresponding name is defined to encode the
parameters of that constraint. It kind of works, but it's forced. It
also hits problems since nodes and properties are technically in
different namespaces, although they rarely collide in real cases.
> > If one want to use a regular as property name one can write this schema:
> >
> > /dts-v1/;
> > / {
> > compatible = "abc";
> > def {
> > name = "def[0-9]";
>
> Isn't it valid to have a property named "name" within the node itself?
> How do you differentiate between specifying the node name and the name
> property?
Or to look at it another way, how do you differentiate between nodes
representing encoded constraints for a property, and nodes
representing nodes directly.
> What if the node name needs more validation than just a regex. For
> example, suppose we want to validate the
> unit-name-must-match-reg-address rule. We need to write some complex
> expression using data extracted from reg to calculate the unit address.
> Equally, the node name perhaps has to exist in some global list of
> acceptable node names. It would be extremely tricky if not impossible to
> do that with a regex.
>
> > ...
> > };
> > };
> >
> > Above one can see that the "name" property override the node name.
>
> Override implies that dtc would change the node name during compilation.
> I think s/override/validate/ or s/override/overrides the validation
> rules for/?
Actually, dtc already contains checks that a "name" property (if
present) matches the unit name. Name properties vs. node names work a
bit differently in the flat-tree world versus traditional OF, and this
checks ensures that flat trees don't do (at least some) things which
would break the OF traditional approach.
> > * Require the presence of a property inside a node or inside one of its
> > parents
> ...
> > /dts-v1/;
> > / {
> > compatible = "ti,twl[0-9]+-rtc";
> > interrupt-controller {
> > is-required;
> > can-be-inherited;
>
> interrupt-controller isn't a good example here, since it isn't a
> property that would typically be inherited. Why not use interrupt-parent
> instead?
>
> > One can check if 'node' has the following subnode 'subnode1', 'subnode2',
> > and 'abc' with the schema below:
> >
> > /dts-v1/;
> > / {
> > compatible = "comp";
> > children = "abc", "subnode[0-9]";
> > };
>
> How is the schema for each sub-node specified?
>
> What if some nodes are optional and some required? The conditions where
> a sub-node is required might be complex, and I think we'd always want to
> be able to represent them in whatever schema language we chose.
>
> The most obvious way would be to make each sub-node's schema appear as a
> sub-node within the main node's schema, but then how do you tell if a
> schema node describes a property or a node?
>
> Note that the following DT file is currently accepted by dtc even if it
> may not be the best choice of property and node names:
>
> ==========
> /dts-v1/;
>
> / {
> foo = <1>;
> foo {};
> };
> ==========
Note that node / property name collisions are not entirely theoretical
either. They are permitted in IEEE1275 and there are real Apple
device trees in the wild which have them. It's rare and discouraged,
obviously.
> > * Constraints on array size
> >
> > One can specify the following constraints on array size:
> > - length: specify the exact length that an array must have.
> > - min-length: specify the minimum number of elements an array must have.
> > - max-length: specify the maximum number of elements an array must have.
>
> This seems rather inflexible; it'll cover a lot of the simple cases, but
> hit a wall pretty soon. For example, how would it validate a property
> that is supposed to include 3 GPIO specifiers, where the GPIO specifiers
> are going to have DT-specific lengths, since the length of each
> specifier is defined by the node that the phandles reference?
>
>
> Overall, I believe perhaps the single most important aspect of any DT
> schema is schema inheritance or instancing, and this proposal doesn't
> appear to address that issue at all.
>
> Inheritance of schemas:
>
> For example, any node that is addressed must contain a reg property. The
> constraints on that property are identical in all bindings; it must
> consist of #address-cells + #size-cells integer values (cells). We don't
> want to have to cut/paste that rule into every single binding
> definition. Rather, we should simply say something like "this binding
> uses the reg property", and the schema validation tool will look up the
> definition of "reg property", and hence know how to validate it.
>
> Similarly, any binding that describes a GPIO controller will have some
> similar requirements; the gpio-controller and #gpio-cells properties
> must be present. The schema should simply say "I'm a GPIO controller",
> and the schema tool should add some extra requirements to nodes of that
> type.
>
> Instancing of schemas:
>
> Any binding that uses GPIOs should be able to say that a particular
> property (e.g. "enable-gpios") is-a GPIO-specifier (with parameters
> "enable" for the property name, min/max/expression length, etc.), and
> then the schema validation tool would know to apply rules for a
> specifier list to that property (and be able to check the property name).
Yes, I agree both of those are important.
So, here's a counter-proposal of at least a rough outline of how I
think schemas could work, in a way that's still based generally on dt
syntax.
First, define the notion of dt "patterns" or "templates". A dt
pattern is to a dt node or subtree as a regex is to a string - it
provides a reasonably expressive way of defining a family of dt
nodes. These would be defined in an extension / superset of dt
syntax.
A schema would then be defined as a set of implications:
If node X matches pattern A, => it must also match pattern B
For example:
If a node has a compatible property with string "foodev"
=> it must have various foodev properties.
If a node has a "reg" property (at all)
=> it must have the format required by reg
If a node has an "interrupts" property
=> it must have either "interrupt-parent" or "interrupt-map"
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131003/36be64f0/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-02 13:54 ` David Gibson
@ 2013-10-02 18:08 ` Mark Brown
2013-10-02 23:38 ` David Gibson
2013-10-03 6:52 ` Benoit Cousson
1 sibling, 1 reply; 21+ messages in thread
From: Mark Brown @ 2013-10-02 18:08 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 02, 2013 at 11:54:50PM +1000, David Gibson wrote:
> On Tue, Oct 01, 2013 at 03:54:20PM -0500, Rob Herring wrote:
> > I would expect the schema to replace
> > Documentation/devicetree/bindings/* over time. I think the thing that
> > needs to be worked out here is how to add free form multi-line text.
> I'm not convinced that's a realistic goal. As I see it, the
> fundamental difference between a binding document and a formal schema
> is that a binding defines both the syntax required of a node, and its
> semantics, whereas a schema defines only syntax - the semantics still
> need to be defined somewhere.
So long as the schema lets you include free form text to define the
semantics I'm not sure there's an incompatibility there - the same
document can cover both.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131002/af925648/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-02 18:08 ` Mark Brown
@ 2013-10-02 23:38 ` David Gibson
0 siblings, 0 replies; 21+ messages in thread
From: David Gibson @ 2013-10-02 23:38 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 02, 2013 at 07:08:41PM +0100, Mark Brown wrote:
> On Wed, Oct 02, 2013 at 11:54:50PM +1000, David Gibson wrote:
> > On Tue, Oct 01, 2013 at 03:54:20PM -0500, Rob Herring wrote:
>
> > > I would expect the schema to replace
> > > Documentation/devicetree/bindings/* over time. I think the thing that
> > > needs to be worked out here is how to add free form multi-line text.
>
> > I'm not convinced that's a realistic goal. As I see it, the
> > fundamental difference between a binding document and a formal schema
> > is that a binding defines both the syntax required of a node, and its
> > semantics, whereas a schema defines only syntax - the semantics still
> > need to be defined somewhere.
>
> So long as the schema lets you include free form text to define the
> semantics I'm not sure there's an incompatibility there - the same
> document can cover both.
True, there's no reason the machine-readable schema and human-readable
documentation can't be contained in the same file.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131003/583920cd/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-02 13:54 ` David Gibson
2013-10-02 18:08 ` Mark Brown
@ 2013-10-03 6:52 ` Benoit Cousson
1 sibling, 0 replies; 21+ messages in thread
From: Benoit Cousson @ 2013-10-03 6:52 UTC (permalink / raw)
To: linux-arm-kernel
On 02/10/2013 15:54, David Gibson wrote:
> On Tue, Oct 01, 2013 at 03:54:20PM -0500, Rob Herring wrote:
>> On Tue, Oct 1, 2013 at 10:06 AM, Benoit Cousson <bcousson@baylibre.com> wrote:
>>> Hi Rob,
>>>
>>>
>>> On 01/10/2013 15:17, Rob Herring wrote:
>>>>
>>>> On 10/01/2013 03:06 AM, Benoit Cousson wrote:
>>>>>
>>>>> + more DT maintainers folks
>>>>>
>>>>> Hi all,
>>>>>
>>>>> I know this is mostly boring user space code, but I was expecting a
>>>>> little bit of comments about at least the bindings syntax:-(
>>>>>
>>>>> I'd like to know if this is the right direction and if it worth pursuing
>>>>> in that direction.
>>>>>
>>>>> The idea was to have at least some base for further discussion during
>>>>> ARM KS 2013.
>>>>>
>>>>> I feel alone :-(
>>>>>
>>>>> If you have any comment, go ahead!
>>>>
>>>>
>>>> Thanks for taking this on!
>>>>
>>>> This is interesting approach using the dts syntax,
>>>
>>>
>>> Well, this was discussed a little bit on the list, and it has the big
>>> advantage of re-using the parser already included in DTC for free.
>>> In term or readability, it avoids to re-defining a brand new syntax for
>>> people who are already familiar with the DTS one.
>>>
>>>
>>>> but I worry that the
>>>> validation will only be as good as the schema written and the review of
>>>> the schema.
>>>
>>>
>>> Well, sure, but unfortunately, that will always be the case :-(
>>> The bindings definition being quite open, there is no easy way to ensure
>>> proper schema / bindings without careful review of the schema. There is no
>>> such thing as a free beer... Unfortunately :-)
>>>
>>>
>>>> I think the schema needs to define the binding rather than
>>>> define the checks. Then the schema can feed the validation checks.
>>>
>>>
>>>> This format does not seem to me as easily being able to generate
>>>> documentation from the schema which I believe is one of the goals.
>>>
>>>
>>> Indeed, but I think is it easy to generate any kind of readable format for
>>> the documentation purpose if needed from the actual format.
>>> Otherwise, we should consider a schema format based on kerneldoc type of
>>> syntax to improve readability. I'm just afraid it will become harder then to
>>> define complex schema.
>>>
>>> BTW, what kind of documentation are you expecting here? Is is a text that
>>> can be added on top of each schema?
>>
>> I would expect the schema to replace
>> Documentation/devicetree/bindings/* over time. I think the thing that
>> needs to be worked out here is how to add free form multi-line text.
>
> I'm not convinced that's a realistic goal. As I see it, the
> fundamental difference between a binding document and a formal schema
> is that a binding defines both the syntax required of a node, and its
> semantics, whereas a schema defines only syntax - the semantics still
> need to be defined somewhere.
>
Mmm, I do not really understand what is missing in the following schema
in term of semantic compared to the original interrupt-controller bindings?
interrupt-controller {
description = "Identifies the node as an interrupt controller";
is-required;
type = "bool";
};
#interrupt-cells {
description = "Specifies the number of cells needed to encode an
interrupt source. The type shall be a <u32>
and the value shall be 1.
The cell contains the interrupt number
in the range [0-128].";
is-required;
type = "integer";
value = <1>;
};
As soon as you have a description you can keep the content of the
original binding. But on top of that, you do have the schema validation
information.
I'm not sure to understand your point. Could you elaborate?
Regards,
Benoit
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-01 22:22 ` Stephen Warren
2013-10-02 14:29 ` David Gibson
@ 2013-10-03 13:17 ` Benoit Cousson
1 sibling, 0 replies; 21+ messages in thread
From: Benoit Cousson @ 2013-10-03 13:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Stephen,
On 02/10/2013 00:22, Stephen Warren wrote:
> On 09/24/2013 10:52 AM, Benoit Cousson wrote:
>> Hi All,
>>
>> Following the discussion that happened during LCE-2013 and the email
>> thread started by Tomasz few months ago [1], here is a first attempt
>> to introduce:
>> - a schema language to define the bindings accurately
>> - DTS validation during device tree compilation in DTC itself
>
> Sorry, this is probably going to sound a bit negative. Hopefully you
> find it constructive though.
Well, I hope so too, let's see at the end of the email :-)
>> The syntax for a schema is the same as the one for dts. This choice has
>> been made to simplify its development, to maximize the code reuse and
>> finally because the format is human-readable.
>
> I'm not convinced that's a good decision.
Me neither :-), but I gave the current rational...
> DT is a language for representing data.
>
> The validation checks described by schemas are rules, or code, and not
> static data.
I will not be that strict with what DTS is supposed to do. In that
aspect DTS is just a way to represent information in a structured
hierarchical way.
It is for my point of view no different than XML. I know that everybody
hate XML, including me, but that shows at least what is doable with such
language. The fact that we like it or not is a different topic.
> So, while I'm sure it's possible to shoe-horn at least some reasonable
> subset of DT validation into DT syntax itself, I feel it's unlikely to
> yield something that's scalable enough.
I don't think we have any limit with such representation. My concern
will be more the readability.
To be honest, a language choice is by nature completely subjective, and
nobody will have the same taste. So we can spend weeks arguing about
that :-)
> For example, it's easy to specify that a property must be 2 cells long.
> What if it could be any multiple of two? That's a lot of numbers to
> explicitly enumerate as data. Sure, you can then invent syntax to
> represent that specific rule (parameterized by 2), but what about the
> next similar-but-different rule? The only approach I can think of to
> that is to allow the schema to contain arbitrary expressions, which
> would likely need to morph into arbitary statements not just
> expressions. Once you're there, I think the schema would be better
> represented as a programming language rather than as a data structure
> that could have code hooked into it.
Sure, but how many complex cases like that do we have? I guess, we can
handle all the use-cases required by Rob with the current syntax.
Let's assume we cover 99% of the use-cases with such language, do we
really want to have a super complex language just for the corner cases?
Potentially, writing a C extension to DTC is still possible for that
specific case.
Not ideal, I do agree, but we have to be pragmatic as well.
We really need to understand how scalable we have to be before deciding
that the current representation is not good enough.
>> How to:
>> * Associate a schema to one or several nodes
>>
>> As said earlier a schema can be used to validate one or several nodes
>> from a dts. To do this the "compatible" properties from the nodes which
>> should be validated must be present in the schema.
>>
>> timer1: timer at 4a318000 {
>> compatible = "ti,omap3430-timer";
> ...
>> To write a schema which will validate OMAP Timers like the one above,
>> one may write the following schema:
>>
>> /dts-v1/;
>> / {
>> compatible = "ti,omap[0-9]+-timer";
>
> What about DT nodes that don't have a compatible value? We certainly
> have some of those already like /memory and /chosen. We should be able
> to validate their schema too. This probably doesn't invalidate being
> able to look things up by compatible value though; it just means we need
> some additional mechanisms too.
Yes, that's a good point and easy to add as well.
>> * Define constraints on properties
>>
>> To define constraints on a property one has to create a node in a schema
>> which has as name the name of the property that one want to validate.
>>
>> To specify constraints on the property "ti,hwmods" of OMAP Timers one
>> can write this schema:
>>
>> /dts-v1/;
>> / {
>> compatible = "ti,omap[0-9]+-timer";
>> ti,hwmods {
>> ...
>> };
>
> compatible and ti,hwmods are both properties in the DT file. However, in
> the schema above, one appears as a property, and one as a node. I don't
> like that inconsistency. It'd be better if compatible was a node too.
That's already possible, you can check the timer.schema. The point is to
simplify the representation for simple case and use a attribute instead
of a node. But that will make 2 different representation for the same
case, which might not be that good.
>> If one want to use a regular as property name one can write this schema:
>>
>> /dts-v1/;
>> / {
>> compatible = "abc";
>> def {
>> name = "def[0-9]";
>
> Isn't it valid to have a property named "name" within the node itself?
> How do you differentiate between specifying the node name and the name
> property?
You don't have to. In this case the attributes inside the node are
strictly the schema language keywords.
That being said, it might happen for some other casea, so maybe a prefix
like "schema,XXX" should be use to create a proper namespace.
> What if the node name needs more validation than just a regex. For
> example, suppose we want to validate the
> unit-name-must-match-reg-address rule. We need to write some complex
> expression using data extracted from reg to calculate the unit address.
> Equally, the node name perhaps has to exist in some global list of
> acceptable node names. It would be extremely tricky if not impossible to
> do that with a regex.
Sure, but again, do we have such cases already? How far do we want to go
in term of complexity for corner cases.
>> ...
>> };
>> };
>>
>> Above one can see that the "name" property override the node name.
>
> Override implies that dtc would change the node name during compilation.
> I think s/override/validate/ or s/override/overrides the validation
> rules for/?
OK
>> * Require the presence of a property inside a node or inside one of its
>> parents
> ...
>> /dts-v1/;
>> / {
>> compatible = "ti,twl[0-9]+-rtc";
>> interrupt-controller {
>> is-required;
>> can-be-inherited;
>
> interrupt-controller isn't a good example here, since it isn't a
> property that would typically be inherited. Why not use interrupt-parent
> instead?
Yeah, that's a mistake, it should have been interrupt-parent. It was
done for that attribute mainly.
>> One can check if 'node' has the following subnode 'subnode1', 'subnode2',
>> and 'abc' with the schema below:
>>
>> /dts-v1/;
>> / {
>> compatible = "comp";
>> children = "abc", "subnode[0-9]";
>> };
>
> How is the schema for each sub-node specified?
sub-node are handled like any other regular node. If needed you can set
constraints on parent node using the parents keyword.
> What if some nodes are optional and some required? The conditions where
> a sub-node is required might be complex, and I think we'd always want to
> be able to represent them in whatever schema language we chose.
>
> The most obvious way would be to make each sub-node's schema appear as a
> sub-node within the main node's schema, but then how do you tell if a
> schema node describes a property or a node?
I'm not sure about that. That was my first impression as well when we
started, but in fact I don't think this is really needed.
By doing that, you end up creating a schema that looks like your final
dts. So this become some kind of template more than a schema.
> Note that the following DT file is currently accepted by dtc even if it
> may not be the best choice of property and node names:
>
> ==========
> /dts-v1/;
>
> / {
> foo = <1>;
> foo {};
> };
> ==========
>
>> * Constraints on array size
>>
>> One can specify the following constraints on array size:
>> - length: specify the exact length that an array must have.
>> - min-length: specify the minimum number of elements an array must have.
>> - max-length: specify the maximum number of elements an array must have.
>
> This seems rather inflexible; it'll cover a lot of the simple cases, but
> hit a wall pretty soon. For example, how would it validate a property
> that is supposed to include 3 GPIO specifiers, where the GPIO specifiers
> are going to have DT-specific lengths, since the length of each
> specifier is defined by the node that the phandles reference?
sure, but that kind of check can be added.
> Overall, I believe perhaps the single most important aspect of any DT
> schema is schema inheritance or instancing, and this proposal doesn't
> appear to address that issue at all.
It does not handle inheritance completely, but that's not necessarily
needed for the cases you describe below.
> Inheritance of schemas:
>
> For example, any node that is addressed must contain a reg property. The
> constraints on that property are identical in all bindings; it must
> consist of #address-cells + #size-cells integer values (cells). We don't
> want to have to cut/paste that rule into every single binding
> definition. Rather, we should simply say something like "this binding
> uses the reg property", and the schema validation tool will look up the
> definition of "reg property", and hence know how to validate it.
That's almost doable with the current mechanism and part of the plan.
You can already add a generic rule that will apply to every nodes thanks
to a wildcard RE. Then later you can add a more specific rule that will
apply to few nodes only.
But I realized, we did not even used that in the example we did :-(
> Similarly, any binding that describes a GPIO controller will have some
> similar requirements; the gpio-controller and #gpio-cells properties
> must be present. The schema should simply say "I'm a GPIO controller",
> and the schema tool should add some extra requirements to nodes of that
> type.
Yes, agreed. Should be doable using previous mechanism. But will need
some improvement.
> Instancing of schemas:
>
> Any binding that uses GPIOs should be able to say that a particular
> property (e.g. "enable-gpios") is-a GPIO-specifier (with parameters
> "enable" for the property name, min/max/expression length, etc.), and
> then the schema validation tool would know to apply rules for a
> specifier list to that property (and be able to check the property name).
>
Thanks for your comments, that are indeed really good and constructive.
Benoit
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-02 14:29 ` David Gibson
@ 2013-10-03 13:53 ` Benoit Cousson
2013-10-06 3:02 ` Chaiken, Alison
0 siblings, 1 reply; 21+ messages in thread
From: Benoit Cousson @ 2013-10-03 13:53 UTC (permalink / raw)
To: linux-arm-kernel
Hi David,
On 02/10/2013 16:29, David Gibson wrote:
> On Tue, Oct 01, 2013 at 04:22:24PM -0600, Stephen Warren wrote:
>> On 09/24/2013 10:52 AM, Benoit Cousson wrote:
>>> Hi All,
>>>
>>> Following the discussion that happened during LCE-2013 and the email
>>> thread started by Tomasz few months ago [1], here is a first attempt
>>> to introduce:
>>> - a schema language to define the bindings accurately
>>> - DTS validation during device tree compilation in DTC itself
>>
>> Sorry, this is probably going to sound a bit negative. Hopefully you
>> find it constructive though.
>>
>>> The syntax for a schema is the same as the one for dts. This choice has
>>> been made to simplify its development, to maximize the code reuse and
>>> finally because the format is human-readable.
>>
>> I'm not convinced that's a good decision.
>>
>> DT is a language for representing data.
>>
>> The validation checks described by schemas are rules, or code, and not
>> static data.
>>
>> So, while I'm sure it's possible to shoe-horn at least some reasonable
>> subset of DT validation into DT syntax itself, I feel it's unlikely to
>> yield something that's scalable enough.
>
> I tend to agree.
>
>> For example, it's easy to specify that a property must be 2 cells long.
>> What if it could be any multiple of two? That's a lot of numbers to
>> explicitly enumerate as data. Sure, you can then invent syntax to
>> represent that specific rule (parameterized by 2), but what about the
>> next similar-but-different rule? The only approach I can think of to
>> that is to allow the schema to contain arbitrary expressions, which
>> would likely need to morph into arbitary statements not just
>> expressions. Once you're there, I think the schema would be better
>> represented as a programming language rather than as a data structure
>> that could have code hooked into it.
>>
>>> How to:
>>> * Associate a schema to one or several nodes
>>>
>>> As said earlier a schema can be used to validate one or several nodes
>>> from a dts. To do this the "compatible" properties from the nodes which
>>> should be validated must be present in the schema.
>>>
>>> timer1: timer at 4a318000 {
>>> compatible = "ti,omap3430-timer";
>> ...
>>> To write a schema which will validate OMAP Timers like the one above,
>>> one may write the following schema:
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "ti,omap[0-9]+-timer";
>>
>> What about DT nodes that don't have a compatible value? We certainly
>> have some of those already like /memory and /chosen. We should be able
>> to validate their schema too. This probably doesn't invalidate being
>> able to look things up by compatible value though; it just means we need
>> some additional mechanisms too.
>
> More to the point, what about the properties of a node whose format is
> defined not by this node's binding but by some other nodes binding.
> e.g. the exact format of reg and ranges is at least partially
> determined by the parent bus's binding, and interrupts is defined
> partially by the interrupt parent's binding. gpio properties are
> defined by a combination of a global binding and the gpio parent,
> IIRC.
Yeah, that's a general concern that Stephen raised several time as well.
We need to figure out some way to handle that.
>>> * Define constraints on properties
>>>
>>> To define constraints on a property one has to create a node in a schema
>>> which has as name the name of the property that one want to validate.
>>>
>>> To specify constraints on the property "ti,hwmods" of OMAP Timers one
>>> can write this schema:
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "ti,omap[0-9]+-timer";
>>> ti,hwmods {
>>> ...
>>> };
>>
>> compatible and ti,hwmods are both properties in the DT file. However, in
>> the schema above, one appears as a property, and one as a node. I don't
>> like that inconsistency. It'd be better if compatible was a node too.
>
> Essentially what's going on here is that to describe the constraint on
> a property, a node with corresponding name is defined to encode the
> parameters of that constraint. It kind of works, but it's forced. It
> also hits problems since nodes and properties are technically in
> different namespaces, although they rarely collide in real cases.
OK, so would you suggest keeping mapping between node / attribute in DTS
and in the schema?
>>> If one want to use a regular as property name one can write this schema:
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "abc";
>>> def {
>>> name = "def[0-9]";
>>
>> Isn't it valid to have a property named "name" within the node itself?
>> How do you differentiate between specifying the node name and the name
>> property?
>
> Or to look at it another way, how do you differentiate between nodes
> representing encoded constraints for a property, and nodes
> representing nodes directly.
>
>> What if the node name needs more validation than just a regex. For
>> example, suppose we want to validate the
>> unit-name-must-match-reg-address rule. We need to write some complex
>> expression using data extracted from reg to calculate the unit address.
>> Equally, the node name perhaps has to exist in some global list of
>> acceptable node names. It would be extremely tricky if not impossible to
>> do that with a regex.
>>
>>> ...
>>> };
>>> };
>>>
>>> Above one can see that the "name" property override the node name.
>>
>> Override implies that dtc would change the node name during compilation.
>> I think s/override/validate/ or s/override/overrides the validation
>> rules for/?
>
> Actually, dtc already contains checks that a "name" property (if
> present) matches the unit name. Name properties vs. node names work a
> bit differently in the flat-tree world versus traditional OF, and this
> checks ensures that flat trees don't do (at least some) things which
> would break the OF traditional approach.
>
>>> * Require the presence of a property inside a node or inside one of its
>>> parents
>> ...
>>> /dts-v1/;
>>> / {
>>> compatible = "ti,twl[0-9]+-rtc";
>>> interrupt-controller {
>>> is-required;
>>> can-be-inherited;
>>
>> interrupt-controller isn't a good example here, since it isn't a
>> property that would typically be inherited. Why not use interrupt-parent
>> instead?
>>
>>> One can check if 'node' has the following subnode 'subnode1', 'subnode2',
>>> and 'abc' with the schema below:
>>>
>>> /dts-v1/;
>>> / {
>>> compatible = "comp";
>>> children = "abc", "subnode[0-9]";
>>> };
>>
>> How is the schema for each sub-node specified?
>>
>> What if some nodes are optional and some required? The conditions where
>> a sub-node is required might be complex, and I think we'd always want to
>> be able to represent them in whatever schema language we chose.
>>
>> The most obvious way would be to make each sub-node's schema appear as a
>> sub-node within the main node's schema, but then how do you tell if a
>> schema node describes a property or a node?
>>
>> Note that the following DT file is currently accepted by dtc even if it
>> may not be the best choice of property and node names:
>>
>> ==========
>> /dts-v1/;
>>
>> / {
>> foo = <1>;
>> foo {};
>> };
>> ==========
>
> Note that node / property name collisions are not entirely theoretical
> either. They are permitted in IEEE1275 and there are real Apple
> device trees in the wild which have them. It's rare and discouraged,
> obviously.
>
>>> * Constraints on array size
>>>
>>> One can specify the following constraints on array size:
>>> - length: specify the exact length that an array must have.
>>> - min-length: specify the minimum number of elements an array must have.
>>> - max-length: specify the maximum number of elements an array must have.
>>
>> This seems rather inflexible; it'll cover a lot of the simple cases, but
>> hit a wall pretty soon. For example, how would it validate a property
>> that is supposed to include 3 GPIO specifiers, where the GPIO specifiers
>> are going to have DT-specific lengths, since the length of each
>> specifier is defined by the node that the phandles reference?
>>
>>
>> Overall, I believe perhaps the single most important aspect of any DT
>> schema is schema inheritance or instancing, and this proposal doesn't
>> appear to address that issue at all.
>>
>> Inheritance of schemas:
>>
>> For example, any node that is addressed must contain a reg property. The
>> constraints on that property are identical in all bindings; it must
>> consist of #address-cells + #size-cells integer values (cells). We don't
>> want to have to cut/paste that rule into every single binding
>> definition. Rather, we should simply say something like "this binding
>> uses the reg property", and the schema validation tool will look up the
>> definition of "reg property", and hence know how to validate it.
>>
>> Similarly, any binding that describes a GPIO controller will have some
>> similar requirements; the gpio-controller and #gpio-cells properties
>> must be present. The schema should simply say "I'm a GPIO controller",
>> and the schema tool should add some extra requirements to nodes of that
>> type.
>>
>> Instancing of schemas:
>>
>> Any binding that uses GPIOs should be able to say that a particular
>> property (e.g. "enable-gpios") is-a GPIO-specifier (with parameters
>> "enable" for the property name, min/max/expression length, etc.), and
>> then the schema validation tool would know to apply rules for a
>> specifier list to that property (and be able to check the property name).
>
> Yes, I agree both of those are important.
>
>
> So, here's a counter-proposal of at least a rough outline of how I
> think schemas could work, in a way that's still based generally on dt
> syntax.
That seems to be well aligned with what we tried to achieve, so I'm not
considering that as a counter-proposal but as a refinement. :-)
> First, define the notion of dt "patterns" or "templates". A dt
> pattern is to a dt node or subtree as a regex is to a string - it
> provides a reasonably expressive way of defining a family of dt
> nodes. These would be defined in an extension / superset of dt
> syntax.
OK, make sense. Are you considering a syntax similar to xpath in order
to match any node in the path, using the full path information instead
of the individual node?
I'm a little bit rusty on xpath but AFAIR it could be something like that.
match any node containing reg:
"//reg/.."
match only node containing reg for ti,omap4-gpio compatible
"//*[@compatible='ti,omap4-gpio']/reg/.."
match only node containing reg below the ocp parent node
"//ocp/*/reg/.."
> A schema would then be defined as a set of implications:
> If node X matches pattern A, => it must also match pattern B
>
> For example:
> If a node has a compatible property with string "foodev"
> => it must have various foodev properties.
>
> If a node has a "reg" property (at all)
> => it must have the format required by reg
>
> If a node has an "interrupts" property
> => it must have either "interrupt-parent" or "interrupt-map"
>
That's part is similar to what we had in mind. So we just need to find
how to express it properly using the DTS syntax.
Thanks,
Benoit
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 01/15] scripts/dtc: fix most memory leaks in dtc
[not found] ` <CAOwMV_zAZG3vvWS6pkyK-FbOEg_32KRO-k1SmFSh-pc9+0JiPA@mail.gmail.com>
@ 2013-10-03 14:26 ` Fabien Parent
0 siblings, 0 replies; 21+ messages in thread
From: Fabien Parent @ 2013-10-03 14:26 UTC (permalink / raw)
To: linux-arm-kernel
Looping back the mailing-lists.
On Thu, Oct 3, 2013 at 4:23 PM, Fabien Parent <fparent@baylibre.com> wrote:
>
> Hi David,
>
>
> On Wed, Oct 2, 2013 at 2:59 PM, David Gibson <david@gibson.dropbear.id.au> wrote:
>>
>> On Tue, Sep 24, 2013 at 06:52:07PM +0200, Benoit Cousson wrote:
>> > From: Fabien Parent <fparent@baylibre.com>
>>
>> As noted elsewhere, please write patches against upstream dtc, not the
>> version in the kernel. git://git.kernel.org/pub/scm/utils/dtc/dtc.git
>>
>> > There are a few memory leaks in dtc which until now were not that important
>> > since they were all in the parser and only one instance of the parser was run
>> > per instance of dtc. The following commits will add a validation of dts through
>> > schema which have the same syntax as dts, i.e. the parser of dts will be reused
>> > to parse schema. The consequence is that instead of having the parser running
>> > only one time for an instance of the dtc process, the parser will run
>> > many many times and thus the leak that were not important until now becomes
>> > urgent to fix.
>>
>> Hrm, yeah. I'm aware that I haven't been very careful with memory
>> leaks within the parser. Essentially, I've been treating the process
>> as a pool allocator with exactly one pool - I've even considered
>> getting rid of those free()s we do have.
>>
>> If the parser's going to be invoked repeatedly, then, yes, that
>> certainly needs fixing. Whether individually fixing each leak or
>> using a explicit pool allocator is a better option is less clear.
>
>
> I've chosen the path of using free()s since it was removing most (all?) memory leaks from
> the parser and wasn't adding much more complexity to it. I don't think a pool allocator would
> be useful to DTC in its current state, but it's true that with schemas which are using
> the DTS syntax it could make a lot of sense to switch to a pool allocator.
>
> I guess I will wait until the discussion about this proposal has progressed and see
> whether this patch should be rewritten using a pool allocator or not.
>
>>
>> > dtc-lexer: Do not duplicate the string which contains literals because the
>> > string is directly converted afterward to an integer and is never used again.
>> > livetree: Add a bunch of free helper functions to clean properly the
>> > dt.
>>
>> This is no good. Making this assumption is a layering violation - if
>> the parser was changed so that this literal wasn't converted until
>> after another token was read, the yytext value copied in here would no
>> longer be valid and would break horribly.
>>
>
> Right, I've been lazy on this one and I took the easy road.
>
>
>>
>> >
>> > Signed-off-by: Fabien Parent <fparent@baylibre.com>
>> > Signed-off-by: Benoit Cousson <bcousson@baylibre.com>
>> > ---
>> > scripts/dtc/dtc-lexer.l | 2 +-
>> > scripts/dtc/dtc-lexer.lex.c_shipped | 2 +-
>> > scripts/dtc/dtc.c | 1 +
>> > scripts/dtc/dtc.h | 1 +
>> > scripts/dtc/livetree.c | 108 +++++++++++++++++++++++++++++++++---
>> > 5 files changed, 105 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/scripts/dtc/dtc-lexer.l b/scripts/dtc/dtc-lexer.l
>> > index 3b41bfc..4f63fbf 100644
>> > --- a/scripts/dtc/dtc-lexer.l
>> > +++ b/scripts/dtc/dtc-lexer.l
>> > @@ -146,7 +146,7 @@ static int pop_input_file(void);
>> > }
>> >
>> > <V1>([0-9]+|0[xX][0-9a-fA-F]+)(U|L|UL|LL|ULL)? {
>> > - yylval.literal = xstrdup(yytext);
>> > + yylval.literal = yytext;
>> > DPRINT("Literal: '%s'\n", yylval.literal);
>> > return DT_LITERAL;
>> > }
>> > diff --git a/scripts/dtc/dtc-lexer.lex.c_shipped b/scripts/dtc/dtc-lexer.lex.c_shipped
>> > index 2d30f41..5c0d27c 100644
>> > --- a/scripts/dtc/dtc-lexer.lex.c_shipped
>> > +++ b/scripts/dtc/dtc-lexer.lex.c_shipped
>> > @@ -1054,7 +1054,7 @@ case 10:
>> > YY_RULE_SETUP
>> > #line 148 "dtc-lexer.l"
>> > {
>> > - yylval.literal = xstrdup(yytext);
>> > + yylval.literal = yytext;
>> > DPRINT("Literal: '%s'\n", yylval.literal);
>> > return DT_LITERAL;
>> > }
>> > diff --git a/scripts/dtc/dtc.c b/scripts/dtc/dtc.c
>> > index a375683..215ae92 100644
>> > --- a/scripts/dtc/dtc.c
>> > +++ b/scripts/dtc/dtc.c
>> > @@ -256,5 +256,6 @@ int main(int argc, char *argv[])
>> > die("Unknown output format \"%s\"\n", outform);
>> > }
>> >
>> > + free_dt(bi);
>> > exit(0);
>> > }
>> > diff --git a/scripts/dtc/dtc.h b/scripts/dtc/dtc.h
>> > index 3e42a07..9c45fd2 100644
>> > --- a/scripts/dtc/dtc.h
>> > +++ b/scripts/dtc/dtc.h
>> > @@ -245,6 +245,7 @@ struct boot_info {
>> > struct boot_info *build_boot_info(struct reserve_info *reservelist,
>> > struct node *tree, uint32_t boot_cpuid_phys);
>> > void sort_tree(struct boot_info *bi);
>> > +void free_dt(struct boot_info *bi);
>> >
>> > /* Checks */
>> >
>> > diff --git a/scripts/dtc/livetree.c b/scripts/dtc/livetree.c
>> > index b61465f..5c8692c 100644
>> > --- a/scripts/dtc/livetree.c
>> > +++ b/scripts/dtc/livetree.c
>> > @@ -20,6 +20,10 @@
>> >
>> > #include "dtc.h"
>> >
>> > +static void free_node_list(struct node *n);
>> > +static void free_node(struct node *n);
>> > +static void free_property(struct property *p);
>> > +
>> > /*
>> > * Tree building functions
>> > */
>> > @@ -144,7 +148,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>> >
>> > /* Add new node labels to old node */
>> > for_each_label_withdel(new_node->labels, l)
>> > - add_label(&old_node->labels, l->label);
>> > + add_label(&old_node->labels, xstrdup(l->label));
>> >
>> > /* Move properties from the new node to the old node. If there
>> > * is a collision, replace the old value with the new */
>> > @@ -156,7 +160,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>> >
>> > if (new_prop->deleted) {
>> > delete_property_by_name(old_node, new_prop->name);
>> > - free(new_prop);
>> > + free_property(new_prop);
>> > continue;
>> > }
>> >
>> > @@ -165,7 +169,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>> > if (streq(old_prop->name, new_prop->name)) {
>> > /* Add new labels to old property */
>> > for_each_label_withdel(new_prop->labels, l)
>> > - add_label(&old_prop->labels, l->label);
>> > + add_label(&old_prop->labels, xstrdup(l->label));
>> >
>> > old_prop->val = new_prop->val;
>> > old_prop->deleted = 0;
>> > @@ -191,7 +195,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>> >
>> > if (new_child->deleted) {
>> > delete_node_by_name(old_node, new_child->name);
>> > - free(new_child);
>> > + free_node(new_child);
>> > continue;
>> > }
>> >
>> > @@ -211,7 +215,7 @@ struct node *merge_nodes(struct node *old_node, struct node *new_node)
>> >
>> > /* The new node contents are now merged into the old node. Free
>> > * the new node. */
>> > - free(new_node);
>> > + free_node(new_node);
>> >
>> > return old_node;
>> > }
>> > @@ -532,13 +536,13 @@ cell_t get_node_phandle(struct node *root, struct node *node)
>> > if (!get_property(node, "linux,phandle")
>> > && (phandle_format & PHANDLE_LEGACY))
>> > add_property(node,
>> > - build_property("linux,phandle",
>> > + build_property(xstrdup("linux,phandle"),
>> > data_append_cell(empty_data, phandle)));
>> >
>> > if (!get_property(node, "phandle")
>> > && (phandle_format & PHANDLE_EPAPR))
>> > add_property(node,
>> > - build_property("phandle",
>> > + build_property(xstrdup("phandle"),
>> > data_append_cell(empty_data, phandle)));
>> >
>> > /* If the node *does* have a phandle property, we must
>> > @@ -707,3 +711,93 @@ void sort_tree(struct boot_info *bi)
>> > sort_reserve_entries(bi);
>> > sort_node(bi->dt);
>> > }
>> > +
>> > +static void free_marker_list(struct marker *m)
>> > +{
>> > + struct marker *marker, *marker_next;
>> > +
>> > + if (!m)
>> > + return;
>> > +
>> > + for (marker = m, marker_next = marker ? marker->next : NULL;
>> > + marker;
>> > + marker = marker_next, marker_next = marker ? marker->next : NULL) {
>>
>> Rather hard to read version of safe-against-free list iteration.
>>
>
> Agreed.
>
>>
>> > + free(marker->ref);
>> > + free(marker);
>> > + }
>> > +}
>> > +
>> > +static void free_label_list(struct label *l)
>> > +{
>> > + struct label *label, *label_next;
>> > +
>> > + if (!l)
>> > + return;
>> > +
>> > + for (label = l, label_next = label ? label->next : NULL;
>> > + label;
>> > + label = label_next, label_next = label ? label->next : NULL) {
>> > + free(label->label);
>> > + free(label);
>> > + }
>> > +}
>> > +
>> > +static void free_property(struct property *p)
>> > +{
>> > + if (!p)
>> > + return;
>> > +
>> > + free_label_list(p->labels);
>> > + free_marker_list(p->val.markers);
>> > + free(p->val.val);
>> > + free(p->name);
>> > + free(p);
>> > +}
>> > +
>> > +static void free_property_list(struct property *p)
>> > +{
>> > + struct property *next;
>> > +
>> > + if (!p)
>> > + return;
>> > +
>> > + for (next = p->next; p; p = next, next = p ? p->next : NULL)
>> > + free_property(p);
>> > +}
>> > +
>> > +static void free_node(struct node *n)
>> > +{
>> > + if (!n)
>> > + return;
>> > +
>> > + free_node_list(n->children);
>> > + free_label_list(n->labels);
>> > + free_property_list(n->proplist);
>> > + free(n->fullpath);
>> > + if (n->name && *n->name)
>>
>> *n->name .. so, the name can (and must) be statically allocated if
>> it's exactly "". Not a useful optimization, I think.
>
>
> True.
>
>>
>>
>> > + free(n->name);
>> > + free(n);
>> > +}
>> > +
>> > +static void free_node_list(struct node *n)
>> > +{
>> > + struct node *next;
>> > +
>> > + if (!n)
>> > + return;
>> > +
>> > + for (next = n->next_sibling;
>> > + n;
>> > + n = next, next = n ? n->next_sibling : NULL) {
>> > + free_node(n);
>> > + }
>> > +}
>> > +
>> > +void free_dt(struct boot_info *bi)
>> > +{
>> > + if (!bi)
>> > + return;
>> > +
>> > + free_node_list(bi->dt);
>> > + free(bi);
>> > +}
>>
>> --
>> David Gibson | I'll have my music baroque, and my code
>> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
>> | _way_ _around_!
>> http://www.ozlabs.org/~dgibson
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [RFC 00/15] Device Tree schemas and validation
2013-10-03 13:53 ` Benoit Cousson
@ 2013-10-06 3:02 ` Chaiken, Alison
0 siblings, 0 replies; 21+ messages in thread
From: Chaiken, Alison @ 2013-10-06 3:02 UTC (permalink / raw)
To: linux-arm-kernel
Rob Herring in devicetree/msg06598.html
> This is interesting approach using the dts syntax,
Benoit Cousson in devicetree/msg06617.html
> it has the big advantage of re-using the parser already included in DTC for
> free. In term or readability, it avoids to re-defining a brand new syntax for
> people who are already familiar with the DTS one.
Stephen Warren wrote in devicetree/msg06676.html:
> DT is a language for representing data.
> The validation checks described by schemas are rules, or code, and not static
> data.
The syntax of the existing device-tree source is strikingly similar to that of
the widely used JavaScript Object Notation, better known as JSON. JSON has many
parsers, validators and schemata already in existence. Assuredly as many coders
know how to program JSON as know C.
JSON makes some sense for representation of device-trees, because, as the
authoritative json.org explains, "JSON is a text format that is completely
language independent but uses conventions that are familiar to programmers of
the C-family of languages . . . JSON is a natural representation of data for the
C family of programming languages."
Benoit Cousson remarks in devicetree/msg06617.html:
> The bindings definition being quite open, there is no easy way to
> ensure proper schema / bindings without careful review of the
> schema.
Stephen Warren continues in devicetree/msg06676.html:
> Overall, I believe perhaps the single most important aspect of any DT
> schema is schema inheritance or instancing,
David Gibson comments in devicetree/msg06725.html:
> define the notion of dt "patterns" or "templates". A dt
> pattern is to a dt node or subtree as a regex is to a string - it
> provides a reasonably expressive way of defining a family of dt
> nodes. These would be defined in an extension / superset of dt
> syntax.
I violently agree with Stephen and David and believe that inheritance
offers a partial solution to the problem Benoit describes. What about
improving compliance by explicitly making use of inheritance with
device-tree include files? Suppose we consider supplementing the ARM
tree's skeleton.dtsi tree-root with board.dtsi, cpu.dtsi,
daughtercard.dtsi . . . Suppose then we require board-level dts files
to include board.dtsi, and furthermore that the CPU node for the board
to be described in a DTSI file that must itself include cpu.dtsi.
Then the dtc itself could in effect check perform some constraint
checking. Device vendors could offer <arch>.dtsi and <particular
chip>.dtsi that users of those products would be required to include.
A related question: what DTS files will a validator compare against a schema?
Assuredly given the existing ability of nodes and properties in a
hierarchy of include files to override and modify one another, the
post C-preprocessed and compiled single-file is the one wanted, that
is, the output from
$CC $CPP_FLAGS -o foo.tmp foo.dts
dtc -O dts -i arch/arm/boot/dts foo.tmp
Some other suggestions:
Let's not make the documentation derived from schemata and DTS files
tree-structured. Otherwise we end up with GNU info, ahem.
Stephen Warren previously contributed a useful bindings checklist. We should
try to roll that checklist into any validator.
Benoit promises in devicetree/msg06617.html:
> Being the very first one, you might get a free beer... meaning there
> might be such thing as a free beer :-)
I'm presenting a talk about device-tree for Embedded Linux Conference
Europe, http://sched.co/17ozhPE, and hope that some of you come flame
me in person there. If so, I will actually buy you a free beer, but
not until after the second seminar I must give that afternoon!
--
Alison Chaiken
Mentor Embedded Software Division
Fremont, CA
GMT-8
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2013-10-06 3:02 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-24 16:52 [RFC 00/15] Device Tree schemas and validation Benoit Cousson
2013-10-01 8:06 ` Benoit Cousson
2013-10-01 13:17 ` Rob Herring
2013-10-01 15:06 ` Benoit Cousson
2013-10-01 15:17 ` Jon Loeliger
2013-10-02 8:24 ` David Gibson
2013-10-02 9:25 ` Benoit Cousson
2013-10-02 13:22 ` Jon Loeliger
2013-10-01 20:54 ` Rob Herring
2013-10-02 13:54 ` David Gibson
2013-10-02 18:08 ` Mark Brown
2013-10-02 23:38 ` David Gibson
2013-10-03 6:52 ` Benoit Cousson
2013-10-02 13:52 ` David Gibson
2013-10-01 22:22 ` Stephen Warren
2013-10-02 14:29 ` David Gibson
2013-10-03 13:53 ` Benoit Cousson
2013-10-06 3:02 ` Chaiken, Alison
2013-10-03 13:17 ` Benoit Cousson
[not found] ` <1380041541-17529-2-git-send-email-bcousson@baylibre.com>
2013-10-02 12:59 ` [RFC 01/15] scripts/dtc: fix most memory leaks in dtc David Gibson
[not found] ` <CAOwMV_zAZG3vvWS6pkyK-FbOEg_32KRO-k1SmFSh-pc9+0JiPA@mail.gmail.com>
2013-10-03 14:26 ` Fabien Parent
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).