On Tue, Mar 06, 2018 at 06:49:15PM -0600, Rob Herring wrote:
> 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@xxxxxxxxxx>
Applied, thanks.
> ---
> v2:
> - Don't bail if 'ranges' is present. This was leftover copy-n-paste.
> - Drop inner loop strlen check. It had a typo, but isn't necessary.
> The streq check will always be false if addr_b length is 0.
>
> checks.c | 31 +++++++++++++++++++++++++++++++
> tests/run_tests.sh | 1 +
> tests/unit-addr-unique.dts | 14 ++++++++++++++
> 3 files changed, 46 insertions(+)
> create mode 100644 tests/unit-addr-unique.dts
>
> diff --git a/checks.c b/checks.c
> index c07ba4da9e36..5a6397fb4a8f 100644
> --- a/checks.c
> +++ b/checks.c
> @@ -1018,6 +1018,36 @@ static void check_avoid_unnecessary_addr_size(struct check *c, struct dt_info *d
> }
> WARNING(avoid_unnecessary_addr_size, check_avoid_unnecessary_addr_size, NULL, &avoid_default_addr_size);
>
> +static void check_unique_unit_address(struct check *c, struct dt_info *dti,
> + struct node *node)
> +{
> + struct node *childa;
> +
> + if (node->addr_cells < 0 || node->size_cells < 0)
> + return;
> +
> + if (!node->children)
> + return;
> +
> + for_each_child(node, childa) {
> + struct node *childb;
> + const char *addr_a = get_unitname(childa);
> +
> + if (!strlen(addr_a))
> + continue;
> +
> + for_each_child(node, childb) {
> + const char *addr_b = get_unitname(childb);
> + if (childa == childb)
> + break;
> +
> + if (streq(addr_a, addr_b))
> + FAIL(c, dti, childb, "duplicate unit-address (also used in node %s)", childa->fullpath);
> + }
> + }
> +}
> +WARNING(unique_unit_address, check_unique_unit_address, NULL, &avoid_default_addr_size);
> +
> static void check_obsolete_chosen_interrupt_controller(struct check *c,
> struct dt_info *dti,
> struct node *node)
> @@ -1391,6 +1421,7 @@ static struct check *check_table[] = {
>
> &avoid_default_addr_size,
> &avoid_unnecessary_addr_size,
> + &unique_unit_address,
> &obsolete_chosen_interrupt_controller,
> &chosen_node_is_root, &chosen_node_bootargs, &chosen_node_stdout_path,
>
> diff --git a/tests/run_tests.sh b/tests/run_tests.sh
> index 0d30edfc0bc4..eebc38558e1a 100755
> --- a/tests/run_tests.sh
> +++ b/tests/run_tests.sh
> @@ -571,6 +571,7 @@ dtc_tests () {
> check_tests unit-addr-without-reg.dts unit_address_vs_reg
> check_tests unit-addr-leading-0x.dts unit_address_format
> check_tests unit-addr-leading-0s.dts unit_address_format
> + check_tests unit-addr-unique.dts unique_unit_address
> check_tests bad-phandle-cells.dts interrupts_extended_property
> check_tests bad-gpio.dts gpios_property
> run_sh_test dtc-checkfails.sh deprecated_gpio_property -- -Wdeprecated_gpio_property -I dts -O dtb bad-gpio.dts
> diff --git a/tests/unit-addr-unique.dts b/tests/unit-addr-unique.dts
> new file mode 100644
> index 000000000000..7cc650b400c5
> --- /dev/null
> +++ b/tests/unit-addr-unique.dts
> @@ -0,0 +1,14 @@
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + foo@1 {
> + reg = <1>;
> + };
> +
> + bar@1 {
> + reg = <1>;
> + };
> +};
--
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
Attachment:
signature.asc
Description: PGP signature
