Commit Graph

30 Commits

Author SHA1 Message Date
Rob Herring b3e46d1a05 of: Use of_node_name_eq for node name comparisons
Convert string compares of DT node names to use of_node_name_eq helper
instead. This removes direct access to the node name pointer.

Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Rob Herring <robh@kernel.org>
2018-12-05 14:45:13 -06:00
Frank Rowand 482137bf2a of: overlay: validate offset from property fixups
The smatch static checker marks the data in offset as untrusted,
leading it to warn:

  drivers/of/resolver.c:125 update_usages_of_a_phandle_reference()
  error: buffer underflow 'prop->value' 's32min-s32max'

Add check to verify that offset is within the property data.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Rob Herring <robh@kernel.org>
2018-05-23 15:07:43 -05:00
Frank Rowand 0b3ce78e90 of: cache phandle nodes to reduce cost of of_find_node_by_phandle()
Create a cache of the nodes that contain a phandle property.  Use this
cache to find the node for a given phandle value instead of scanning
the devicetree to find the node.  If the phandle value is not found
in the cache, of_find_node_by_phandle() will fall back to the tree
scan algorithm.

The cache is initialized in of_core_init().

The cache is freed via a late_initcall_sync() if modules are not
enabled.

If the devicetree is created by the dtc compiler, with all phandle
property values auto generated, then the size required by the cache
could be 4 * (1 + number of phandles) bytes.  This results in an O(1)
node lookup cost for a given phandle value.  Due to a concern that the
phandle property values might not be consistent with what is generated
by the dtc compiler, a mask has been added to the cache lookup algorithm.
To maintain the O(1) node lookup cost, the size of the cache has been
increased by rounding the number of entries up to the next power of
two.

The overhead of finding the devicetree node containing a given phandle
value has been noted by several people in the recent past, in some cases
with a patch to add a hashed index of devicetree nodes, based on the
phandle value of the node.  One concern with this approach is the extra
space added to each node.  This patch takes advantage of the phandle
property values auto generated by the dtc compiler, which begin with
one and monotonically increase by one, resulting in a range of 1..n
for n phandle values.  This implementation should also provide a good
reduction of overhead for any range of phandle values that are mostly
in a monotonic range.

Performance measurements by Chintan Pandya <cpandya@codeaurora.org>
of several implementations of patches that are similar to this one
suggest an expected reduction of boot time by ~400ms for his test
system.  If the cache size was decreased to 64 entries, the boot
time was reduced by ~340 ms.  The measurements were on a 4.9.73 kernel
for arch/arm64/boot/dts/qcom/sda670-mtp.dts, contains 2371 nodes and
814 phandle values.

Reported-by: Chintan Pandya <cpandya@codeaurora.org>
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2018-03-07 14:50:09 -06:00
Frank Rowand 39a751a4cb of: change overlay apply input data from unflattened to FDT
Move duplicating and unflattening of an overlay flattened devicetree
(FDT) into the overlay application code.  To accomplish this,
of_overlay_apply() is replaced by of_overlay_fdt_apply().

The copy of the FDT (aka "duplicate FDT") now belongs to devicetree
code, which is thus responsible for freeing the duplicate FDT.  The
caller of of_overlay_fdt_apply() remains responsible for freeing the
original FDT.

The unflattened devicetree now belongs to devicetree code, which is
thus responsible for freeing the unflattened devicetree.

These ownership changes prevent early freeing of the duplicated FDT
or the unflattened devicetree, which could result in use after free
errors.

of_overlay_fdt_apply() is a private function for the anticipated
overlay loader.

Update unittest.c to use of_overlay_fdt_apply() instead of
of_overlay_apply().

Move overlay fragments from artificial locations in
drivers/of/unittest-data/tests-overlay.dtsi into one devicetree
source file per overlay.  This led to changes in
drivers/of/unitest-data/Makefile and drivers/of/unitest.c.

  - Add overlay directives to the overlay devicetree source files so
    that dtc will compile them as true overlays into one FDT data
    chunk per overlay.

  - Set CFLAGS for drivers/of/unittest-data/testcases.dts so that
    symbols will be generated for overlay resolution of overlays
    that are no longer artificially contained in testcases.dts

  - Unflatten and apply each unittest overlay FDT using
    of_overlay_fdt_apply().

  - Enable the of_resolve_phandles() check for whether the unflattened
    overlay is detached.  This check was previously disabled because the
    overlays from tests-overlay.dtsi were not unflattened into detached
    trees.

  - Other changes to unittest.c infrastructure to manage multiple test
    FDTs built into the kernel image (access by name instead of
    arbitrary number).

  - of_unittest_overlay_high_level(): previously unused code to add
    properties from the overlay_base devicetree to the live tree
    was triggered by the restructuring of tests-overlay.dtsi and thus
    testcases.dts.  This exposed two bugs: (1) the need to dup a
    property before adding it, and (2) property 'name' is
    auto-generated in the unflatten code and thus will be a duplicate
    in the __symbols__ node - do not treat this duplicate as an error.

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
2018-03-04 00:29:24 -08:00
Rob Herring af6074fc9a of: Use SPDX license tag for DT files
Convert remaining DT files to use SPDX-License-Identifier tags.

Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Reviewed-by: Frank Rowand <frank.rowand@sony.com>
Reviewed-by: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2018-01-08 08:22:45 -06:00
Frank Rowand f948d6d8b7 of: overlay: avoid race condition between applying multiple overlays
The process of applying an overlay consists of:
  - unflatten an overlay FDT (flattened device tree) into an
    EDT (expanded device tree)
  - fixup the phandle values in the overlay EDT to fit in a
    range above the phandle values in the live device tree
  - create the overlay changeset to reflect the contents of
    the overlay EDT
  - apply the overlay changeset, to modify the live device tree,
    potentially changing the maximum phandle value in the live
    device tree

There is currently no protection against two overlay applies
concurrently determining what range of phandle values are in use
in the live device tree, and subsequently changing that range.
Add a mutex to prevent multiple overlay applies from occurring
simultaneously.

Move of_resolve_phandles() into of_overlay_apply() so that it does not
have to be duplicated by each caller of of_overlay_apply().

The test in of_resolve_phandles() that the overlay tree is detached is
temporarily disabled so that old style overlay unittests do not fail.

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2017-10-17 20:47:27 -05:00
Stephen Boyd eeb09506c5 of/resolver: Replace kmalloc + memcpy with kmemdup()
Save one line.

Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
Reviewed-by: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2017-10-16 16:43:29 -05:00
Stephen Boyd d35d623f52 of/resolver: Simplify to be32_add_cpu()
This is the same as be32_add_cpu(), so simplify the code and
remove the now unused local variable.

Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
Reviewed-by: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2017-10-16 16:42:56 -05:00
Rob Herring 27497e11b5 of: find_node_by_full_name rewrite to compare each level
find_node_by_full_name() does the same thing as of_find_node_by_path(),
but takes no locks and doesn't work on aliases. Refactor
of_find_node_opts_by_path() into __of_find_node_by_full_path() and
replace find_node_by_full_name() with it.

Signed-off-by: Rob Herring <robh@kernel.org>
2017-06-22 12:38:29 -05:00
Rob Herring 95e6b1fa33 of: use kbasename instead of open coding
Several places in DT code open code the equivalent of kbasename.
Replace them.

The behavior for root nodes in node_name_cmp will be slightly different.
Instead of comparing "/", "" will be compared. The comparison will be
the same.

Reviewed-by: Andy Shevchenko <andy.shevhchenko@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2017-06-22 12:37:46 -05:00
Rob Herring 17a70355ea of: fix sparse warnings in fdt, irq, reserved mem, and resolver code
sparse generates the following warnings in drivers/of/:

../drivers/of/fdt.c:63:36: warning: cast to restricted __be32
../drivers/of/fdt.c:68:33: warning: cast to restricted __be32
../drivers/of/irq.c:105:88: warning: incorrect type in initializer (different base types)
../drivers/of/irq.c:105:88:    expected restricted __be32
../drivers/of/irq.c:105:88:    got int
../drivers/of/irq.c:526:35: warning: incorrect type in assignment (different modifiers)
../drivers/of/irq.c:526:35:    expected int ( *const [usertype] irq_init_cb )( ... )
../drivers/of/irq.c:526:35:    got void const *const data
../drivers/of/of_reserved_mem.c:200:50: warning: incorrect type in initializer (different modifiers)
../drivers/of/of_reserved_mem.c:200:50:    expected int ( *[usertype] initfn )( ... )
../drivers/of/of_reserved_mem.c:200:50:    got void const *const data
../drivers/of/resolver.c:95:42: warning: incorrect type in assignment (different base types)
../drivers/of/resolver.c:95:42:    expected unsigned int [unsigned] [usertype] <noident>
../drivers/of/resolver.c:95:42:    got restricted __be32 [usertype] <noident>

All these are harmless type mismatches fixed by adjusting the types.

Signed-off-by: Rob Herring <robh@kernel.org>
2017-05-04 14:01:13 -05:00
Geliang Tang 30965eeab5 of: drop duplicate headers
Drop duplicate headers string.h and of_platform.h.

Signed-off-by: Geliang Tang <geliangtang@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2017-01-04 14:01:28 -06:00
Moritz Fischer 32bed310ad of: Fix issue where code would fall through to error case.
No longer fall through into the error case that prints out
an error if no error (err = 0) occurred.

Fixes d9181b20a83(of: Add back an error message, restructured)
Signed-off-by: Moritz Fischer <mdf@kernel.org>
Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-12-09 10:33:03 -06:00
Frank Rowand 5581a95f2d of: Remove unused variable overlay_symbols
Remove unused pointer to node "__symbols__".

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:18 -06:00
Frank Rowand 4458db4c9b of: Move setting of pointer to beside test for non-null
Value of pointer was calculated in an earlier block than
where it was used.  Move it down into the block where it
is used, immediately before where is is checked to be valid.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:17 -06:00
Frank Rowand d9181b20a8 of: Add back an error message, restructured
Add a single pr_err() to cover a range of errors that were reported
by several pr_err() that were removed earlier in this series.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:17 -06:00
Frank Rowand 269f1a67d0 of: Update comments to reflect changes and increase clarity
Update comments to better explain what functions are doing.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:16 -06:00
Frank Rowand ea8229b788 of: Remove redundant size check
Remove a redundant check of buffer size.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:16 -06:00
Frank Rowand 624ab2a49c of: Update structure of code to be clearer, also remove BUG_ON()
Remove BUG_ON(), which is frowned upon and not needed here.
Restructure to remove some excessive complexity.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:15 -06:00
Frank Rowand 25e1687767 of: Rename variables to better reflect purpose or follow convention
Rename variables to better reflect what their purpose is.  As a side
effect, this reduces the need for some of the comments previously
removed in this series.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:15 -06:00
Frank Rowand fad556bf3f of: Remove prefix "__of_" from local function names
Remove "__of_" prefix from local function names.  The pattern of
a leading "__" is used in drivers/of/ to signify a function that
must be called with a lock held.  These functions do not fit
that pattern.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:14 -06:00
Frank Rowand f94823f2eb of: Rename functions to more accurately reflect what they do
Some function names are misleading or do not provide a good
sense of what they do.  Rename the functions to ne more
informative.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:14 -06:00
Frank Rowand 9f27ede49d of: Convert comparisons to zero or NULL to logical expressions
Convert comparisons to zero or NULL to logical expressions.  A
small number of such comparisons remain where they provide more
clarity of the numeric nature of a variable.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:13 -06:00
Frank Rowand 96d1c8e8e3 of: Remove excessive printks to reduce clutter.
Remove extra debug and error printks.  A single pr_err() will
be added at the end of this series to replace many of these
error messages.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:13 -06:00
Frank Rowand a67976ecc8 of: Remove comments that state the obvious, to reduce clutter
Remove comments that report what is obvious from the code.

Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-11-15 09:17:12 -06:00
Michal Suchanek 5de3bbc850 of: overlay: add resolver error prints
Applying overlay fails silently in case of an error. Add error prints.
Most notably the lack of symbols in the live tree is not reported.

Signed-off-by: Michal Suchanek <hramrach@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-07-18 17:18:36 -05:00
Rob Herring 606ad42aa3 of: use pr_fmt prefix for all console printing
Clean-up all the DT printk functions to use common pr_fmt prefix.

Some print statements such as kmalloc errors were redundant, so just
drop those.

Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
Reviewed-by: Frank Rowand <frank.rowand@am.sony.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-07-18 16:57:42 -05:00
Amitoj Kaur Chawla 82f68756e9 of: resolver: Add missing of_node_get and of_node_put
In __of_find_node_by_full_name, add an of_node_get when detecting the
desired element, to ensure that it ends up with a reference count that is
one greater than on entering the function.

Also in __of_find_node_by_full_name, add an of_node_put on breaking
out of the for_each_child_of_node loop, to ensure that the reference
count of the returned value is not double incremented. This change
was made using Coccinelle.

The semantic patch used for this is as follows:

// <smpl>
@@
expression e;
local idexpression n;
@@

 for_each_child_of_node(..., n) {
   ... when != of_node_put(n)
       when != e = n
(
   return n;
|
+  of_node_put(n);
?  return ...;
)
   ...
 }
// </smpl

Finally, add an of_node_put in for_each_child_of_node in the function
__of_adjust_phandle_ref after the value returned by
__of_find_node_by_full_name is no longer useful.

Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
2016-02-13 13:35:32 -06:00
Pantelis Antoniou da56d04c80 of/resolver: Switch to new local fixups format.
The original resolver format is way too cryptic, switch
to using a tree based format that gets rid of repetitions,
is more compact and readable.

At the same time, update the selftests to using the new local fixups
format.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
[grant.likely: Squashed in testcase changes and merged similar functions]
Signed-off-by: Grant Likely <grant.likely@linaro.org>
2014-11-24 22:24:50 +00:00
Pantelis Antoniou 7941b27b16 of: Introduce Device Tree resolve support.
Introduce support for dynamic device tree resolution.
Using it, it is possible to prepare a device tree that's
been loaded on runtime to be modified and inserted at the kernel
live tree.

Export of of_resolve and bug fix of double free by
	Guenter Roeck <groeck@juniper.net>

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
[grant.likely: Don't need to select CONFIG_OF_DYNAMIC and CONFIG_OF_DEVICE]
[grant.likely: Don't need to depend on OF or !SPARC]
[grant.likely: Factor out duplicate code blocks into single function]
Signed-off-by: Grant Likely <grant.likely@linaro.org>
2014-10-04 21:24:26 +01:00