2017-07-08 00:29:02

by Frank Rowand

[permalink] [raw]
Subject: [PATCH 0/3] of: overlay: load overlay symbols into live device tree

From: Frank Rowand <[email protected]>

Symbols in a loaded overlay are not currently available to subsequently
loaded overlays because the properties in the overlay's __symbols__
node are not loaded into the live device tree.

Patch 1 is unittests to test patches 2 and 3.

Patch 2 fixes a problem discovered while developing patch 3. If
a node name in an overlay has a unit-address then the overlay
code does not correctly match the node name against an existing
node in the live tree.

Patch 3 adds the properties in an overlay's __symbol__ node to
the overlay changeset.

This patch set was created on top of v4.12, with Rob's pull request
for 4.13 added (https://lkml.org/lkml/2017/7/6/691). Thus this
series should apply on 4.13-rc1 when that becomes available. I will
re-test on 4.13-rc1.


Frank Rowand (3):
of: overlay: add overlay unittest data for node names and symbols
of: overlay: correctly apply overlay node with unit-address
of: overlay: add overlay symbols to live device tree

drivers/of/overlay.c | 136 ++++++++++++++++++++++--
drivers/of/unittest-data/Makefile | 3 +
drivers/of/unittest-data/overlay.dts | 19 +++-
drivers/of/unittest-data/overlay_bad_symbol.dts | 22 ++++
drivers/of/unittest-data/overlay_base.dts | 7 ++
drivers/of/unittest.c | 6 ++
6 files changed, 182 insertions(+), 11 deletions(-)
create mode 100644 drivers/of/unittest-data/overlay_bad_symbol.dts

--
Frank Rowand <[email protected]>


2017-07-08 00:29:07

by Frank Rowand

[permalink] [raw]
Subject: [PATCH 3/3] of: overlay: add overlay symbols to live device tree

From: Frank Rowand <[email protected]>

Add overlay __symbols__ properties to live tree when an overlay
is added to the live tree so that the symbols are available to
subsequent overlays.

Expected test result is new __symbols__ entries for labels from
the overlay after the patch is applied.

Before this patch is applied:

Console error message near end of unittest:
### dt-test ### FAIL of_unittest_overlay_high_level():2296 Adding overlay 'overlay_bad_symbol' failed
### dt-test ### end of unittest - 190 passed, 1 failed

The new unittest "fails" because the expected result of loading the
new overlay is an error instead of success.

$ # node hvac-medium-2 exists because the overlay loaded
$ # since the duplicate symbol was not detected
$ cd /proc/device-tree/testcase-data-2/substation@100/
$ ls
compatible hvac-medium-2 motor-8 reg
hvac-large-1 linux,phandle name status
hvac-medium-1 motor-1 phandle

$ cd /proc/device-tree/__symbols__/
$ ls
electric_1 lights_1 name rides_1 spin_ctrl_2
hvac_1 lights_2 retail_1 spin_ctrl_1

After this patch is applied:

Previous console error message no longer occurs, but expected error
occurs:
OF: overlay: Failed to apply prop @/__symbols__/hvac_1
OF: overlay: apply failed '/__symbols__'
### dt-test ### end of unittest - 191 passed, 0 failed

$ # node hvac-medium-2 does not exist because the overlay
$ # properly failed to load due to the duplicate symbol
$ cd /proc/device-tree/testcase-data-2/substation@100/
$ ls
compatible hvac-medium-1 motor-1 name reg
hvac-large-1 linux,phandle motor-8 phandle status

$ cd /proc/device-tree/__symbols__/
$ ls
electric_1 lights_1 retail_1 ride_200_right spin_ctrl_2
hvac_1 lights_2 ride_200 rides_1
hvac_2 name ride_200_left spin_ctrl_1
$ cat ride_200; echo
/testcase-data-2/fairway-1/ride@200
$ cat ride_200_left ; echo
/testcase-data-2/fairway-1/ride@200/track@1
$ cat ride_200_right ; echo
/testcase-data-2/fairway-1/ride@200/track@2

Signed-off-by: Frank Rowand <[email protected]>
---
drivers/of/overlay.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 107 insertions(+), 9 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 30aef51eeee5..c484712d124a 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -35,6 +35,7 @@
struct of_overlay_info {
struct device_node *target;
struct device_node *overlay;
+ bool is_symbols_node;
};

/**
@@ -55,7 +56,8 @@ struct of_overlay {
};

static int of_overlay_apply_one(struct of_overlay *ov,
- struct device_node *target, const struct device_node *overlay);
+ struct device_node *target, const struct device_node *overlay,
+ bool is_symbols_node);

static BLOCKING_NOTIFIER_HEAD(of_overlay_chain);

@@ -92,10 +94,74 @@ static int of_overlay_notify(struct of_overlay *ov,
return 0;
}

+static struct property *dup_and_fixup_symbol_prop(struct of_overlay *ov,
+ const struct property *prop)
+{
+ struct of_overlay_info *ovinfo;
+ struct property *new;
+ const char *overlay_name;
+ char *label_path;
+ char *symbol_path;
+ const char *target_path;
+ int k;
+ int label_path_len;
+ int overlay_name_len;
+ int target_path_len;
+
+ if (!prop->value)
+ return NULL;
+ symbol_path = prop->value;
+
+ new = kzalloc(sizeof(*new), GFP_KERNEL);
+ if (!new)
+ return NULL;
+
+ for (k = 0; k < ov->count; k++) {
+ ovinfo = &ov->ovinfo_tab[k];
+ overlay_name = ovinfo->overlay->full_name;
+ overlay_name_len = strlen(overlay_name);
+ if (!strncasecmp(symbol_path, overlay_name, overlay_name_len))
+ break;
+ }
+
+ if (k >= ov->count)
+ goto err_free;
+
+ target_path = ovinfo->target->full_name;
+ target_path_len = strlen(target_path);
+
+ label_path = symbol_path + overlay_name_len;
+ label_path_len = strlen(label_path);
+
+ new->name = kstrdup(prop->name, GFP_KERNEL);
+ new->length = target_path_len + label_path_len + 1;
+ new->value = kzalloc(new->length, GFP_KERNEL);
+
+ if (!new->name || !new->value)
+ goto err_free;
+
+ strcpy(new->value, target_path);
+ strcpy(new->value + target_path_len, label_path);
+
+ /* mark the property as dynamic */
+ of_property_set_flag(new, OF_DYNAMIC);
+
+ return new;
+
+ err_free:
+ kfree(new->name);
+ kfree(new->value);
+ kfree(new);
+ return NULL;
+
+
+}
+
static int of_overlay_apply_single_property(struct of_overlay *ov,
- struct device_node *target, struct property *prop)
+ struct device_node *target, struct property *prop,
+ bool is_symbols_node)
{
- struct property *propn, *tprop;
+ struct property *propn = NULL, *tprop;

/* NOTE: Multiple changes of single properties not supported */
tprop = of_find_property(target, prop->name, NULL);
@@ -106,7 +172,15 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
of_prop_cmp(prop->name, "linux,phandle") == 0)
return 0;

- propn = __of_prop_dup(prop, GFP_KERNEL);
+ if (is_symbols_node) {
+ /* changing a property in __symbols__ node not allowed */
+ if (tprop)
+ return -EINVAL;
+ propn = dup_and_fixup_symbol_prop(ov, prop);
+ } else {
+ propn = __of_prop_dup(prop, GFP_KERNEL);
+ }
+
if (propn == NULL)
return -ENOMEM;

@@ -155,7 +229,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
return -EINVAL;

/* apply overlay recursively */
- ret = of_overlay_apply_one(ov, tchild, child);
+ ret = of_overlay_apply_one(ov, tchild, child, 0);
of_node_put(tchild);
} else {
/* create empty tree as a target */
@@ -170,7 +244,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
if (ret)
return ret;

- ret = of_overlay_apply_one(ov, tchild, child);
+ ret = of_overlay_apply_one(ov, tchild, child, 0);
if (ret)
return ret;
}
@@ -186,14 +260,16 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
* by using the changeset.
*/
static int of_overlay_apply_one(struct of_overlay *ov,
- struct device_node *target, const struct device_node *overlay)
+ struct device_node *target, const struct device_node *overlay,
+ bool is_symbols_node)
{
struct device_node *child;
struct property *prop;
int ret;

for_each_property_of_node(overlay, prop) {
- ret = of_overlay_apply_single_property(ov, target, prop);
+ ret = of_overlay_apply_single_property(ov, target, prop,
+ is_symbols_node);
if (ret) {
pr_err("Failed to apply prop @%s/%s\n",
target->full_name, prop->name);
@@ -201,6 +277,10 @@ static int of_overlay_apply_one(struct of_overlay *ov,
}
}

+ /* do not allow symbols node to have any children */
+ if (is_symbols_node)
+ return 0;
+
for_each_child_of_node(overlay, child) {
ret = of_overlay_apply_single_device_node(ov, target, child);
if (ret != 0) {
@@ -231,7 +311,8 @@ static int of_overlay_apply(struct of_overlay *ov)
for (i = 0; i < ov->count; i++) {
struct of_overlay_info *ovinfo = &ov->ovinfo_tab[i];

- err = of_overlay_apply_one(ov, ovinfo->target, ovinfo->overlay);
+ err = of_overlay_apply_one(ov, ovinfo->target, ovinfo->overlay,
+ ovinfo->is_symbols_node);
if (err != 0) {
pr_err("apply failed '%s'\n", ovinfo->target->full_name);
return err;
@@ -329,6 +410,9 @@ static int of_build_overlay_info(struct of_overlay *ov,
for_each_child_of_node(tree, node)
cnt++;

+ if (of_get_child_by_name(tree, "__symbols__"))
+ cnt++;
+
ovinfo = kcalloc(cnt, sizeof(*ovinfo), GFP_KERNEL);
if (ovinfo == NULL)
return -ENOMEM;
@@ -340,6 +424,20 @@ static int of_build_overlay_info(struct of_overlay *ov,
cnt++;
}

+ node = of_get_child_by_name(tree, "__symbols__");
+ if (node) {
+ ovinfo[cnt].overlay = node;
+ ovinfo[cnt].target = of_find_node_by_path("/__symbols__");
+ ovinfo[cnt].is_symbols_node = 1;
+
+ if (!ovinfo[cnt].target) {
+ pr_err("no symbols in root of device tree.\n");
+ return -EINVAL;
+ }
+
+ cnt++;
+ }
+
/* if nothing filled, return error */
if (cnt == 0) {
kfree(ovinfo);
--
Frank Rowand <[email protected]>

2017-07-08 00:29:13

by Frank Rowand

[permalink] [raw]
Subject: [PATCH 2/3] of: overlay: correctly apply overlay node with unit-address

From: Frank Rowand <[email protected]>

Correct existing node name detection when overlay node name has
a unit-address.

Expected test result is overlay will update the nodes and properties
for /testcase-data-2/fairway-1/ride@100/ after the patch is applied.

Before this patch is applied:

Console error message near end of unittest:
OF: Duplicate name in fairway-1, renamed to "ride@100#1"

$ cd /proc/device-tree/testcase-data-2/fairway-1/
$ # extra node: ride@100#1
$ ls
#address-cells linux,phandle phandle ride@200
#size-cells name ride@100 status
compatible orientation ride@100#1
$ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
$ ls track@3/incline_up
ls: track@3/incline_up: No such file or directory
$ ls track@4/incline_up
ls: track@4/incline_up: No such file or directory

After this patch is applied:

Console error message no longer occurs

$ cd /proc/device-tree/testcase-data-2/fairway-1/
$ # no extra node: ride@100#1
$ ls
#address-cells compatible name phandle ride@200
#size-cells linux,phandle orientation ride@100 status
$ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
$ ls track@3/incline_up
track@3/incline_up
$ ls track@4/incline_up
track@4/incline_up

Signed-off-by: Frank Rowand <[email protected]>
---
drivers/of/overlay.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index c0e4ee1cd1ba..30aef51eeee5 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -118,6 +118,24 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
return of_changeset_update_property(&ov->cset, target, propn);
}

+static struct device_node *child_by_full_name(const struct device_node *np,
+ const char *cname)
+{
+ struct device_node *child;
+ struct device_node *prev;
+
+ child = np->child;
+ while (child) {
+ of_node_get(child);
+ if (!of_node_cmp(cname, kbasename(child->full_name)))
+ break;
+ prev = child;
+ child = child->sibling;
+ of_node_put(prev);
+ }
+ return child;
+}
+
static int of_overlay_apply_single_device_node(struct of_overlay *ov,
struct device_node *target, struct device_node *child)
{
@@ -130,7 +148,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
return -ENOMEM;

/* NOTE: Multiple mods of created nodes not supported */
- tchild = of_get_child_by_name(target, cname);
+ tchild = child_by_full_name(target, cname);
if (tchild != NULL) {
/* new overlay phandle value conflicts with existing value */
if (child->phandle)
--
Frank Rowand <[email protected]>

2017-07-08 00:31:16

by Frank Rowand

[permalink] [raw]
Subject: [PATCH 1/3] of: overlay: add overlay unittest data for node names and symbols

From: Frank Rowand <[email protected]>

Add nodes and properties to overlay_base and overlay dts files to
test for
- incorrect existing node name detection when overlay node name
has a unit-address
- adding overlay __symbols__ properties to live tree when an
overlay is added to the live tree

Expected result from patch 2/3 is overlay will update the nodes and
properties for /testcase-data-2/fairway-1/ride@100/

Before patch 2/3 is applied:

Console error message near end of unittest:
OF: Duplicate name in fairway-1, renamed to "ride@100#1"

$ cd /proc/device-tree/testcase-data-2/fairway-1/
$ # extra node: ride@100#1
$ ls
#address-cells linux,phandle phandle ride@200
#size-cells name ride@100 status
compatible orientation ride@100#1
$ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
$ ls track@3/incline_up
ls: track@3/incline_up: No such file or directory
$ ls track@4/incline_up
ls: track@4/incline_up: No such file or directory

After patch 2/3 is applied:

Console error message no longer occurs

$ cd /proc/device-tree/testcase-data-2/fairway-1/
$ # no extra node: ride@100#1
$ ls
#address-cells compatible name phandle ride@200
#size-cells linux,phandle orientation ride@100 status
$ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
$ ls track@3/incline_up
track@3/incline_up
$ ls track@4/incline_up
track@4/incline_up

Expected result from patch 3/3 is new __symbols__ entries for labels
from the overlay _after_ the add overlay symbols patch is also applied.

Before patch 3/3 is applied:

Console error message near end of unittest:
### dt-test ### FAIL of_unittest_overlay_high_level():2296 Adding overlay 'overlay_bad_symbol' failed
### dt-test ### end of unittest - 190 passed, 1 failed

The new unittest "fails" because the expected result of loading the
new overlay is an error instead of success.

$ # node hvac-medium-2 exists because the overlay loaded
$ # since the duplicate symbol was not detected
$ cd /proc/device-tree/testcase-data-2/substation@100/
$ ls
compatible hvac-medium-2 motor-8 reg
hvac-large-1 linux,phandle name status
hvac-medium-1 motor-1 phandle

$ cd /proc/device-tree/__symbols__/
$ ls
electric_1 lights_1 name rides_1 spin_ctrl_2
hvac_1 lights_2 retail_1 spin_ctrl_1

After patch 3/3 is applied:

Previous console error message no longer occurs, but expected error
occurs:
OF: overlay: Failed to apply prop @/__symbols__/hvac_1
OF: overlay: apply failed '/__symbols__'
### dt-test ### end of unittest - 191 passed, 0 failed

$ # node hvac-medium-2 does not exist because the overlay
$ # properly failed to load due to the duplicate symbol
$ cd /proc/device-tree/testcase-data-2/substation@100/
$ ls
compatible hvac-medium-1 motor-1 name reg
hvac-large-1 linux,phandle motor-8 phandle status

$ cd /proc/device-tree/__symbols__/
$ ls
electric_1 lights_1 retail_1 ride_200_right spin_ctrl_2
hvac_1 lights_2 ride_200 rides_1
hvac_2 name ride_200_left spin_ctrl_1
$ cat ride_200; echo
/testcase-data-2/fairway-1/ride@200
$ cat ride_200_left ; echo
/testcase-data-2/fairway-1/ride@200/track@1
$ cat ride_200_right ; echo
/testcase-data-2/fairway-1/ride@200/track@2

Signed-off-by: Frank Rowand <[email protected]>
---
drivers/of/unittest-data/Makefile | 3 +++
drivers/of/unittest-data/overlay.dts | 19 ++++++++++++++++++-
drivers/of/unittest-data/overlay_bad_symbol.dts | 22 ++++++++++++++++++++++
drivers/of/unittest-data/overlay_base.dts | 7 +++++++
drivers/of/unittest.c | 6 ++++++
5 files changed, 56 insertions(+), 1 deletion(-)
create mode 100644 drivers/of/unittest-data/overlay_bad_symbol.dts

diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 6e00a9c69e58..dae2fe23cd2e 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -1,11 +1,13 @@
obj-y += testcases.dtb.o
obj-y += overlay.dtb.o
obj-y += overlay_bad_phandle.dtb.o
+obj-y += overlay_bad_symbol.dtb.o
obj-y += overlay_base.dtb.o

targets += testcases.dtb testcases.dtb.S
targets += overlay.dtb overlay.dtb.S
targets += overlay_bad_phandle.dtb overlay_bad_phandle.dtb.S
+targets += overlay_bad_symbol.dtb overlay_bad_symbol.dtb.S
targets += overlay_base.dtb overlay_base.dtb.S

.PRECIOUS: \
@@ -15,4 +17,5 @@ targets += overlay_base.dtb overlay_base.dtb.S
# enable creation of __symbols__ node
DTC_FLAGS_overlay := -@
DTC_FLAGS_overlay_bad_phandle := -@
+DTC_FLAGS_overlay_bad_symbol := -@
DTC_FLAGS_overlay_base := -@
diff --git a/drivers/of/unittest-data/overlay.dts b/drivers/of/unittest-data/overlay.dts
index 6cd7e6a0c13e..81140adbe770 100644
--- a/drivers/of/unittest-data/overlay.dts
+++ b/drivers/of/unittest-data/overlay.dts
@@ -25,7 +25,18 @@
#size-cells = <1>;
status = "ok";

- ride@200 {
+ ride@100 {
+
+ track@3 {
+ incline_up = < 48 32 16 >;
+ };
+
+ track@4 {
+ incline_up = < 47 31 15 >;
+ };
+ };
+
+ ride_200: ride@200 {
compatible = "ot,ferris-wheel";
reg = < 0x00000200 0x100 >;
hvac-provider = < &hvac_2 >;
@@ -36,6 +47,12 @@
spin-rph = < 30 >;
gondolas = < 16 >;
gondola-capacity = < 6 >;
+
+ ride_200_left: track@1 {
+ };
+
+ ride_200_right: track@2 {
+ };
};
};
};
diff --git a/drivers/of/unittest-data/overlay_bad_symbol.dts b/drivers/of/unittest-data/overlay_bad_symbol.dts
new file mode 100644
index 000000000000..09261cb9a67e
--- /dev/null
+++ b/drivers/of/unittest-data/overlay_bad_symbol.dts
@@ -0,0 +1,22 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+
+ fragment@0 {
+ target = <&electric_1>;
+
+ __overlay__ {
+
+ // This label should cause an error when the overlay
+ // is applied. There is already a symbol hvac_1
+ // in the base tree
+ hvac_1: hvac-medium-2 {
+ compatible = "ot,hvac-medium";
+ heat-range = < 50 75 >;
+ cool-range = < 60 80 >;
+ };
+
+ };
+ };
+};
diff --git a/drivers/of/unittest-data/overlay_base.dts b/drivers/of/unittest-data/overlay_base.dts
index 5566b27fb61a..997d807259e6 100644
--- a/drivers/of/unittest-data/overlay_base.dts
+++ b/drivers/of/unittest-data/overlay_base.dts
@@ -53,6 +53,13 @@
spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
spin-controller-names = "track_1", "track_2";
queues = < 2 >;
+
+ track@3 {
+ };
+
+ track@4 {
+ };
+
};
};

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 0107fc680335..e56b8eb220d9 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -2010,6 +2010,7 @@ struct overlay_info {
OVERLAY_INFO_EXTERN(overlay_base);
OVERLAY_INFO_EXTERN(overlay);
OVERLAY_INFO_EXTERN(overlay_bad_phandle);
+OVERLAY_INFO_EXTERN(overlay_bad_symbol);

#ifdef CONFIG_OF_OVERLAY

@@ -2018,6 +2019,7 @@ struct overlay_info {
OVERLAY_INFO(overlay_base, -9999),
OVERLAY_INFO(overlay, 0),
OVERLAY_INFO(overlay_bad_phandle, -EINVAL),
+ OVERLAY_INFO(overlay_bad_symbol, -EINVAL),
{}
};

@@ -2289,6 +2291,10 @@ static __init void of_unittest_overlay_high_level(void)

unittest(overlay_data_add(2),
"Adding overlay 'overlay_bad_phandle' failed\n");
+
+ unittest(overlay_data_add(3),
+ "Adding overlay 'overlay_bad_symbol' failed\n");
+
return;

err_unlock:
--
Frank Rowand <[email protected]>

2017-07-10 16:09:07

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH 2/3] of: overlay: correctly apply overlay node with unit-address

On Fri, Jul 7, 2017 at 7:28 PM, <[email protected]> wrote:
> From: Frank Rowand <[email protected]>
>
> Correct existing node name detection when overlay node name has
> a unit-address.
>
> Expected test result is overlay will update the nodes and properties
> for /testcase-data-2/fairway-1/ride@100/ after the patch is applied.
>
> Before this patch is applied:
>
> Console error message near end of unittest:
> OF: Duplicate name in fairway-1, renamed to "ride@100#1"
>
> $ cd /proc/device-tree/testcase-data-2/fairway-1/
> $ # extra node: ride@100#1
> $ ls
> #address-cells linux,phandle phandle ride@200
> #size-cells name ride@100 status
> compatible orientation ride@100#1
> $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
> $ ls track@3/incline_up
> ls: track@3/incline_up: No such file or directory
> $ ls track@4/incline_up
> ls: track@4/incline_up: No such file or directory
>
> After this patch is applied:
>
> Console error message no longer occurs
>
> $ cd /proc/device-tree/testcase-data-2/fairway-1/
> $ # no extra node: ride@100#1
> $ ls
> #address-cells compatible name phandle ride@200
> #size-cells linux,phandle orientation ride@100 status
> $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
> $ ls track@3/incline_up
> track@3/incline_up
> $ ls track@4/incline_up
> track@4/incline_up
>
> Signed-off-by: Frank Rowand <[email protected]>
> ---
> drivers/of/overlay.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index c0e4ee1cd1ba..30aef51eeee5 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -118,6 +118,24 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
> return of_changeset_update_property(&ov->cset, target, propn);
> }
>
> +static struct device_node *child_by_full_name(const struct device_node *np,

It's not really the full name which currently means the whole path (my
full_name work is going to change that), but the unit_name (at least
that's what dtc calls it).

> + const char *cname)
> +{
> + struct device_node *child;
> + struct device_node *prev;
> +
> + child = np->child;
> + while (child) {

Doesn't for_each_child_of_node() work here?

> + of_node_get(child);
> + if (!of_node_cmp(cname, kbasename(child->full_name)))
> + break;
> + prev = child;
> + child = child->sibling;
> + of_node_put(prev);
> + }
> + return child;
> +}
> +
> static int of_overlay_apply_single_device_node(struct of_overlay *ov,
> struct device_node *target, struct device_node *child)
> {
> @@ -130,7 +148,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
> return -ENOMEM;
>
> /* NOTE: Multiple mods of created nodes not supported */
> - tchild = of_get_child_by_name(target, cname);
> + tchild = child_by_full_name(target, cname);
> if (tchild != NULL) {
> /* new overlay phandle value conflicts with existing value */
> if (child->phandle)
> --
> Frank Rowand <[email protected]>
>

2017-07-10 17:28:12

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH 2/3] of: overlay: correctly apply overlay node with unit-address

On 07/10/17 09:08, Rob Herring wrote:
> On Fri, Jul 7, 2017 at 7:28 PM, <[email protected]> wrote:
>> From: Frank Rowand <[email protected]>
>>
>> Correct existing node name detection when overlay node name has
>> a unit-address.
>>
>> Expected test result is overlay will update the nodes and properties
>> for /testcase-data-2/fairway-1/ride@100/ after the patch is applied.
>>
>> Before this patch is applied:
>>
>> Console error message near end of unittest:
>> OF: Duplicate name in fairway-1, renamed to "ride@100#1"
>>
>> $ cd /proc/device-tree/testcase-data-2/fairway-1/
>> $ # extra node: ride@100#1
>> $ ls
>> #address-cells linux,phandle phandle ride@200
>> #size-cells name ride@100 status
>> compatible orientation ride@100#1
>> $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
>> $ ls track@3/incline_up
>> ls: track@3/incline_up: No such file or directory
>> $ ls track@4/incline_up
>> ls: track@4/incline_up: No such file or directory
>>
>> After this patch is applied:
>>
>> Console error message no longer occurs
>>
>> $ cd /proc/device-tree/testcase-data-2/fairway-1/
>> $ # no extra node: ride@100#1
>> $ ls
>> #address-cells compatible name phandle ride@200
>> #size-cells linux,phandle orientation ride@100 status
>> $ cd /proc/device-tree/testcase-data-2/fairway-1/ride@100/
>> $ ls track@3/incline_up
>> track@3/incline_up
>> $ ls track@4/incline_up
>> track@4/incline_up
>>
>> Signed-off-by: Frank Rowand <[email protected]>
>> ---
>> drivers/of/overlay.c | 20 +++++++++++++++++++-
>> 1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>> index c0e4ee1cd1ba..30aef51eeee5 100644
>> --- a/drivers/of/overlay.c
>> +++ b/drivers/of/overlay.c
>> @@ -118,6 +118,24 @@ static int of_overlay_apply_single_property(struct of_overlay *ov,
>> return of_changeset_update_property(&ov->cset, target, propn);
>> }
>>
>> +static struct device_node *child_by_full_name(const struct device_node *np,
>
> It's not really the full name which currently means the whole path (my
> full_name work is going to change that), but the unit_name (at least
> that's what dtc calls it).

Yes, thanks. I would change this name, but your next comment below
allows me to remove this function instead.


>> + const char *cname)
>> +{
>> + struct device_node *child;
>> + struct device_node *prev;
>> +
>> + child = np->child;
>> + while (child) {
>
> Doesn't for_each_child_of_node() work here?

Yes, thanks. And it makes the code compact enough that I can just
put it inside of_overlay_apply_single_device_node() instead of
creating this extra function.


>> + of_node_get(child);
>> + if (!of_node_cmp(cname, kbasename(child->full_name)))
>> + break;
>> + prev = child;
>> + child = child->sibling;
>> + of_node_put(prev);
>> + }
>> + return child;
>> +}
>> +
>> static int of_overlay_apply_single_device_node(struct of_overlay *ov,
>> struct device_node *target, struct device_node *child)
>> {
>> @@ -130,7 +148,7 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
>> return -ENOMEM;
>>
>> /* NOTE: Multiple mods of created nodes not supported */
>> - tchild = of_get_child_by_name(target, cname);
>> + tchild = child_by_full_name(target, cname);
>> if (tchild != NULL) {
>> /* new overlay phandle value conflicts with existing value */
>> if (child->phandle)
>> --
>> Frank Rowand <[email protected]>
>>
>