If running on a tree with an 'interrupt-parent' property which contains
an invalid phandle (0 or -1, not merely for a node which doesn't exist),
then check_interrupts_property() will trip the assertion in
get_node_by_phandle().
There's logic that almost detects this, but it only handles the overlay
case, where we can't fully check because the links will be fixed up later.
For the non-overlay case, this is definitely a bad property, but we
shouldn't crash. Fix it by failing the check early.
Fixes: c1e55a5513 ("checks: fix handling of unresolved phandles for dts plugins")
Fixes: ee3d26f696 ("checks: add interrupts property check")
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Commit 4038fd9005 ("dtc: add ability to make nodes conditional on them
being referenced") added the new /omit-if-no-ref/ directive to mark
nodes as eligible to be discarded if not referenced. The mechanism to
process this happens before the symbol generation phase. This means even
if symbol generation is requested and the node has a label, it will be
discarded if there are no references to it within the same file.
This is probably not what people expect. When using symbol generation to
compile base device trees for applying overlays, nodes with labels could
be referenced by the overlays, and therefore should be preserved.
Check if the node has a label and symbol generation was requested before
dropping the node.
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
Message-Id: <20190327035352.24036-1-wens@csie.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Should be "endpoint" rather than "endpont"
Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
Message-Id: <6fcb6e160163467b706c312ffe307ee8a5d9255d.1552328099.git.leonard.crestez@nxp.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
There are various SoCs that have 2 different peripheral blocks at the
same register offset. However, we might have one block marked as
status = "disabled" and the other status = "ok". In such cases we
shouldn't warn about duplicate unit-address.
Here's a cut down example that we would warning about before:
/dts-v1/;
/ {
#address-cells = <0x01>;
#size-cells = <0x01>;
soc {
#address-cells = <0x01>;
#size-cells = <0x01>;
compatible = "simple-bus";
ranges;
i2c0: i2c@40003000 {
compatible = "nordic,nrf-i2c";
reg = <0x40003000 0x1000>;
status = "ok";
};
spi0: spi@40003000 {
compatible = "nordic,nrf-spi";
reg = <0x40003000 0x1000>;
status = "disabled";
};
};
};
We introduce 'unique_unit_address_if_enabled' check that is disabled by
default.
Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Commit 3616b9a811 ("checks: Use source position information for check
failures") causes crashes when there's a check message with multiple
source annotations. Drop the errant addition to the str pointer left
over from the previous version.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Now that we retain source position information of nodes and properties,
make that the preferred file name (and position) to print out in check
failures. This will greatly simplify finding and fixing check errors
because most errors are in included source .dtsi files and they get
duplicated every time the source file is included.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Printing to stderr as we build up the check message results in
interleaving of messages when multiple instances of dtc are running.
Change the message output to use an intermediate buffer for constructing
the message and then output the message to stderr with a single fputs.
While perhaps there is no guarantee that fputs will be atomic, this gets
rid of any interleaved output that previously occurred on Linux.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The entire check_msg function is under the if condition except for
va_start/va_end. Move these and invert the if condition saving a level
of indentation.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
If the SPI bus controller is being used for 'spi-slave' mode some of the
checks we have need to change:
In 'spi-slave' mode #address-cells should be 0, as any children don't
have a reg property.
Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Since commit 7975f64222 ("Fix widespread incorrect use of strneq(),
replace with new strprefixeq()") simple-bus checks have been silently
skipped. The problem was 'end - str' is one more than the string length
and the strnlen in strprefixeq fails. This can't be fixed simply by
subtracting one as it is possible to have multiple '\0' at the end of
the property. Fix this by making the 'compatible' property string list
check a dependency, and then we can assume the property is null
terminated and we can just use streq() for comparisons.
Add some tests so the problem doesn't happen again.
Fixes: 7975f64222 ("Fix widespread incorrect use of strneq(), replace with new strprefixeq()")
Reported-by: Kumar Gala <kumar.gala@linaro.org>
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add SPI bus type detection and checks. The node name is the
preferred way to find SPI buses as there is no common compatible or
property which can be used. There are a few common properties used in
child nodes, so they can be used as a fallback detection method. This
lets us warn if the SPI controller is not properly named 'spi@...'.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add I2C bus type detection and checks. The node name is used to find I2C
buses as there is no common compatible or property which can be used to
identify I2C controllers/buses. There are some common I2C properties,
but they are not used frequently enough to match on.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
A number of platforms have a need to reduce the number of DT nodes,
mostly because of two similar constraints: the size of the DT blob, and
the time it takes to parse it.
As the DT is used in more and more SoCs, and by more projects, some
constraints start to appear in bootloaders running from SRAM with an
order of magnitude of 10kB. A typical DT is in the same order of
magnitude, so any effort to reduce the blob size is welcome in such an
environment.
Some platforms also want to reach very fast boot time, and the time it
takes to parse a typical DT starts to be noticeable.
Both of these issues can be mitigated by reducing the number of nodes in
the DT. The biggest provider of nodes is usually the pin controller and
its subnodes, usually one for each valid pin configuration in a given
SoC.
Obviously, a single, fixed, set of these nodes will be used by a given
board, so we can introduce a node property that will tell the DT
compiler to drop the nodes when they are not referenced in the tree, and
as such wouldn't be useful in the targetted system.
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Having a 'bus-range' property for PCI bridges should not be required,
so remove the warning when missing. There was some confusion with the
Linux kernel printing a message that no property is present and the OS
assigned the bus number. This message was intended to be informational
rather than a warning.
When the firmware doesn't enumerate the PCI bus and leaves it up to the
OS to do, then it is perfectly fine for the OS to assign bus numbers
and bus-range is not necessary.
There are a few cases where bus-range is needed or useful as Arnd
Bergmann summarized:
- Traditionally Linux avoided using multiple PCI domains, but instead
configured separate PCI host bridges to have non-overlapping
bus ranges so we can present them to user space as a single
domain, and run the kernel without CONFIG_PCI_DOMAINS.
Specifying the bus ranges this way would and give stable bus
numbers across boots when the probe order is not fixed.
- On certain ARM64 systems, we must only use the first
128 bus numbers based on the way the IOMMU identifies
the device with truncated bus/dev/fn number. There are probably
others like this, with various limitations.
- To leave some room for hotplugged devices, each slot on
a host bridge can in theory get a range of bus numbers
that are available when assigning bus numbers at boot time
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
When refactoring node path printing, the code checking for duplicate
node names was accidentally changed to print the name of the parent
node, instead of the name of the duplicated child node.
Fixes: 88960e3989 ("checks: centralize printing of node path in check_msg")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add checks for DT graph bindings. These checks check node names,
unit-addresses and link connections on ports, port, and endpoint nodes.
The graph nodes are matched by finding nodes named 'endpoint' or with a
'remote-endpoint' property. We can't match on 'ports' or 'port' nodes
because those names are used for non-graph nodes. While the graph nodes
aren't really buses, using the bus pointer to tag matched nodes is
convenient.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Child nodes with the same unit-address (and different node names) are
either an error or just bad DT design. Typical errors are the unit-address
is just wrong (i.e. doesn't match reg value) or multiple children using the
same overlapping area. Overlapping regions are considered an error in new
bindings, but do exist in some existing trees. This check should flag
most but not all of those errors. Finding all cases would require doing
address translations and creating a full map of address spaces.
Mixing more than one address/number space at a level is bad design. It only
works if both spaces can use the same #address-cells and #size-cells sizes.
It also complicates parsing have a mixture of types of child nodes. The
best practice in this case is adding child container nodes for each
address/number space or using additional address bits/cells to encode
different address spaces.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Overlay fragments are traditionally named "fragment@NNN" but don't have
have a 'reg' property, amongst other differences from normal nodes. Really
we should treat overlay fragments fundamentally differently, but for the
moment, suppress the common warnings about the fragment names with this
simple hack.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Some failure messages apply to a specific property. Add a FAIL_PROP()
macro for failure messages which are specific to a property. With that,
failure messages can print the property name in a standard way. Once
source line numbers are supported, then the file and line number of the
property can be used instead of the node file and line number.
Convert the existing messages related to properties to use the FAIL_PROP
macro and reword the messages as necessary.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Most error/warning messages print the node path as part of their error
message. Move printing of the node path into check_msg() so the
formatting can be standardized to the form:
<output file>: (ERROR|warning) (<check name>): <full node name>: <check message>
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add some checks for /chosen node. These check that chosen is located at
the root level and that bootargs and stdout-path properties are strings.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add checks for aliases node that all properties follow alias naming
convention and the values are a valid path.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add a check for unnecessary "#{size,address}-cells" when there's neither
a 'ranges' property nor child nodes with a 'reg' property.
An exception may be an overlay that adds nodes, but this case would need
"#{size,address}-cells" in the overlay to properly compile already.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add a string list check for common properties ending in "-names" such as
reg-names or interrupt-names.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add a check for string list properties with compatible being the first
check.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add a string property check for 'label' property. 'label' is a human
readable string typically used to identify connectors or ports on devices.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The property is named "sound-dai", not "sound-dais".
Fixes: b3bbac02d5 ("checks: add phandle with arg property checks")
Signed-off-by: Peter Rosin <peda@axentia.se>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Every remaining usage of strneq() is, in fact, incorrect. They're trying
to check that the first n characters of one string exactly match another
string. But, they fall into the classic trap of strncmp() on which
strneq() is based. If n is less than the length of the second string, they
only check that the first string matches the start of the second, not the
whole of it.
To fix this, remove strneq() and replace it with a strprefixeq() function
which does what we want here.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
In a number of places, dtc and associated tools and test code use
leading _ characters on identifiers to flag them as "internal", an
idiom taken from the Linux kernel. This is a bad idea in a userspace
program, because identifiers with a leading _ are reserved for the C
library / system.
In some cases, the extra _ served no real purpose, so simply drop it. In
others move to the end of the identifier, which is a convention we're free
to use for our own purposes.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
In dts plugins, it is valid to have unresolved phandle values. The
check_property_phandle_args and check_interrupts_property checks failed to
account for this resulting in spurious warnings or asserts, respectively.
Fix this by bailing from the checks if we're checking a dts plugin as
there is no way to further validate the properties.
Fixes: ee3d26f696 ("checks: add interrupts property check")
Fixes: b3bbac02d5 ("checks: add phandle with arg property checks")
Reported-by: Alan Tull <atull@kernel.org>
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The size of size_t can vary between architectures, so using %ld isn't
going to work on 32-bit builds. Use the %zu modifier to make sure it is
always correct.
Signed-off-by: Thierry Reding <treding@nvidia.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add a check for nodes with interrupts property that they have a valid
parent, the parent has #interrupt-cells property, and the size is a
valid multiple of #interrupt-cells.
This may not handle every possible case and doesn't deal with
translation thru interrupt-map properties, but should be enough for
modern dts files.
Signed-off-by: Rob Herring <robh@kernel.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
The GPIO binding is different compared to other phandle plus args
properties in that the property name has a variable, optional prefix.
The format of the property name is [<name>-]gpio{s} where <name> can
be any legal property string. Therefore, custom matching of property
names is needed, but the common check_property_phandle_args() function
can still be used.
It's possible that there are property names matching which are not GPIO
binding specifiers. There's only been one case found in testing which is
"[<vendor>,]nr-gpio{s}". This property has been blacklisted and the same
should be done to any others we find. This check will prevent getting
any more of these, too.
Signed-off-by: Rob Herring <robh@kernel.org>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Many common bindings follow the same pattern of client properties
containing a phandle and N arg cells where N is defined in the provider
with a '#<specifier>-cells' property such as:
intc0: interrupt-controller@0 {
#interrupt-cells = <3>;
};
intc1: interrupt-controller@1 {
#interrupt-cells = <2>;
};
node {
interrupts-extended = <&intc0 1 2 3>, <&intc1 4 5>;
};
Add checks for properties following this pattern.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2a42b14 "dtc: check.c fix compile error" changed a format string using
%lx which wasn't correct for all platforms. Unfortunately it changed it to
%zx, which is wrong for a different set of platforms (and only right on
the others by accident). The parameter we're formatting here is uint64_t,
not size_t, so we need to use the PRIx64 macro from <inttypes.h> to get
this right.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Fix the following compile error found on odroid-xu4:
checks.c: In function ‘check_simple_bus_reg’:
checks.c:876:41: error: format ‘%lx’ expects argument of type
‘long unsigned int’, but argument 4 has type
‘uint64_t{aka long long unsigned int}’ [-Werror=format=]
snprintf(unit_addr, sizeof(unit_addr), "%lx", reg);
^
checks.c:876:41: error: format ‘%lx’ expects argument of type
‘long unsigned int’, but argument 4 has type
‘uint64_t {aka long long unsigned int}’ [-Werror=format=]
cc1: all warnings being treated as errors
Makefile:304: recipe for target 'checks.o' failed
make: *** [checks.o] Error 1
Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
[dwg: Correct new format to be correct in general]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Node name unit-addresses should generally never begin with 0x or leading
0s. Add warnings to check for these cases, but only for nodes without a
known bus type as there should be better bus specific checks of the
unit address in those cases. Any unit addresses that don't follow the
general rule will need to add a new bus type. There aren't any known
ones ATM.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add checks to identify simple-bus bus types and checks for child
devices. Simple-bus type is generally identified by "simple-bus"
compatible string. We also treat the root as a simple-bus, but only for
child nodes with reg property.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Add PCI bridge and device node checks. We identify PCI bridges with
'device_type = "pci"' as only PCI bridges should set that property. For
bridges, check that node name is pci or pcie, ranges and bus-range are
present, and #address-cells and #size-cells are correct.
For devices, check the reg property fields are correct for the first
element (the config address). Check that the unit address is formatted
corectly based on the reg property. Device unit addresses are in the
form DD or DD,F where DD is the device 0-0x1f and F is the function 0-7.
Also, check that the bus number is within the expected range defined by
bridge's bus-ranges.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Rob Herring <robh@kernel.org>
[dwg: Added a missing check dependency]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This fixes a great many sparse warnings on the fdt and libfdt sources.
These are mostly due to incorrect mixing of endian annotated and native
integer types.
This includes fixing a couple of quasi-bugs where we had endian conversions
the wrong way around (this will have the right effect in practice, but is
certainly conceptually incorrect).
This doesn't make the whole tree sparse clean: there are many warnings in
bison and lex generated code, and there are a handful of other remaining
warnings that are (for now) more trouble than they're worth to fix (and
are not genuine bugs).
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
We have a number of explicit __GNUC__ conditionals to tell if we want to
use some gcc extensions for extra warnings. This cleans this up to use
a single conditional, defining convenience macros for those attributes.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
For example:
src/arm/at91-ariag25.dtb: Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name
If output is to stdout then the prefix is "<stdout>: ".
This helps to direct the developer to where to look when multiple files are
being compiled in parallel.
Signed-off-by: Ian Campbell <ijc@hellion.org.uk>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
dtc defines a streq() (string equality) macro to avoid the easy confusion
of the sense of strcmp() comparison for equality. A few places where we
don't use it have slipped in, so remove them.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
While '#', '?', '.', '+', '*', and '_' are considered valid characters,
their use is discouraged in recommended practices.
Testing this found a few cases of '.'. The majority of the warnings were
all from underscores.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
While '?', '.', '+', '*', and '_' are considered valid characters their
use is discouraged in recommended practices. '#' is also only
recommended to be used at the beginning of property names.
Testing this found one typo error with '.' used instead of ','. The
rest of the warnings were all from underscores.
Signed-off-by: Rob Herring <robh@kernel.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
struct boot_info is named that for historical reasons, and isn't
particularly meaningful. Essentially it contains all the information -
in "live" form from a single dts or dtb file. As we move towards support
for dynamic dt overlays, that name will become increasingly bad.
So, in preparation, rename it to dt_info. At the same time rename the
'the_boot_info' global to 'parser_output' since that's its actual purpose.
Unfortunately we do need the global unless we switch to bison's re-entrant
parser extensions, which would introduce its own complications.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This patch enable the generation of symbols & local fixup information
for trees compiled with the -@ (--symbols) option.
Using this patch labels in the tree and their users emit information
in __symbols__ and __local_fixups__ nodes.
The __fixups__ node make possible the dynamic resolution of phandle
references which are present in the plugin tree but lie in the
tree that are applying the overlay against.
Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
As preparation for overlay support we need to pass the boot info
parameter instead of the root node to each check method.
The root node can be retrieved by accessing boot info's dt member.
No other functional changes are made.
Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Property checking functions aren't particularly useful. They're used only
in a handful of cases, and most of those really only check a small handful
of specific properties. This patches converts the few cases to node check
functions and removes property check functions entirely.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
David Gibson