[PATCH 0/8] of: Dynamic DT updates

[PATCH 0/8] of: Dynamic DT updates

[Pantelis Antoniou]

This patchset introduces several concepts that make
working with dynamic DT and overlays considerably easier.

The first 6 patches introduce two new overlay target methods
addressing various user complaints and use cases. For both
documentation entries and unittests are provided.

The final two patches add a new API for using changeset which
makes things considerably easier.

This patchset applies against Linus's tree as of today and
is dependent on the previous patchset I've send out earlier.
"of: overlay: kobject & sysfs'ation"

Pantelis Antoniou (8):
  of: overlay: Implement indirect target support
  of: unittest: Add indirect overlay target test
  doc: dt: Document the indirect overlay method.
  of: overlay: Introduce target root capability.
  of: unittest: Unit-tests for target root overlays.
  doc: dt: Document the target root overlay method
  of: dynamic: Add __of_node_dupv()
  of: changesets: Introduce changeset helper methods

 Documentation/devicetree/overlay-notes.txt  |  21 +++
 drivers/of/dynamic.c                        | 280 +++++++++++++++++++++++++++-
 drivers/of/overlay.c                        | 219 +++++++++++++++++++---
 drivers/of/unittest-data/testcases.dts      |  14 ++
 drivers/of/unittest-data/tests-overlay.dtsi |  67 +++++++
 drivers/of/unittest.c                       | 272 +++++++++++++++++++++++++++
 include/linux/of.h                          |  89 +++++++++
 7 files changed, 929 insertions(+), 33 deletions(-)

-- 
1.7.12

[PATCH 1/8] of: overlay: Implement indirect target support

[Pantelis Antoniou]

Some applications require applying the same overlay to a different
target according to some external condition (for instance depending
on the slot a card has been inserted, the overlay target is different).

The indirect target use requires using the new
of_overlay_create_indirect() API which uses a text selector.

The format requires the use of a target-indirect node as follows:

	fragment@0 {
		target-indirect {
			foo { target = <&foo_target>; };
			bar { target = <&bar_target>; };
		};
	};

Calling of_overlay_create_indirect() with a "foo" argument selects
the foo_target and so on.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/overlay.c | 126 +++++++++++++++++++++++++++++++++++++++++----------
 include/linux/of.h   |   8 ++++
 2 files changed, 110 insertions(+), 24 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 747568f..80aab6f 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -54,6 +54,7 @@ struct of_overlay {
 	struct of_overlay_info *ovinfo_tab;
 	struct of_changeset cset;
 	struct kobject kobj;
+	char *indirect_id;
 };
 
 /* master enable switch; once set to 0 can't be re-enabled */
@@ -191,14 +192,8 @@ static int of_overlay_apply(struct of_overlay *ov)
 	return 0;
 }
 
-/*
- * Find the target node using a number of different strategies
- * in order of preference
- *
- * "target" property containing the phandle of the target
- * "target-path" property containing the path of the target
- */
-static struct device_node *find_target_node(struct device_node *info_node)
+static struct device_node *find_target_node_direct(struct of_overlay *ov,
+		struct device_node *info_node)
 {
 	const char *path;
 	u32 val;
@@ -209,17 +204,66 @@ static struct device_node *find_target_node(struct device_node *info_node)
 	if (ret == 0)
 		return of_find_node_by_phandle(val);
 
-	/* now try to locate by path */
+	/* failed, try to locate by path */
 	ret = of_property_read_string(info_node, "target-path", &path);
 	if (ret == 0)
 		return of_find_node_by_path(path);
 
-	pr_err("%s: Failed to find target for node %p (%s)\n", __func__,
-		info_node, info_node->name);
-
 	return NULL;
 }
 
+/*
+ * Find the target node using a number of different strategies
+ * in order of preference. Respects the indirect id if available.
+ *
+ * "target" property containing the phandle of the target
+ * "target-path" property containing the path of the target
+ */
+static struct device_node *find_target_node(struct of_overlay *ov,
+		struct device_node *info_node)
+{
+	struct device_node *target;
+	struct device_node *target_indirect;
+	struct device_node *indirect;
+
+	/* try direct target */
+	target = find_target_node_direct(ov, info_node);
+	if (target)
+		return target;
+
+	/* try indirect if there */
+	if (!ov->indirect_id)
+		return NULL;
+
+	target_indirect = of_get_child_by_name(info_node, "target-indirect");
+	if (!target_indirect) {
+		pr_err("%s: Failed to find target-indirect node at %s\n",
+				__func__,
+				of_node_full_name(info_node));
+		return NULL;
+	}
+
+	indirect = of_get_child_by_name(target_indirect, ov->indirect_id);
+	of_node_put(target_indirect);
+	if (!indirect) {
+		pr_err("%s: Failed to find indirect child node \"%s\" at %s\n",
+				__func__, ov->indirect_id,
+				of_node_full_name(info_node));
+		return NULL;
+	}
+
+	target = find_target_node_direct(ov, indirect);
+
+	if (!target) {
+		pr_err("%s: Failed to find target for \"%s\" at %s\n",
+				__func__, ov->indirect_id,
+				of_node_full_name(indirect));
+	}
+	of_node_put(indirect);
+
+	return target;
+}
+
 /**
  * of_fill_overlay_info() - Fill an overlay info structure
  * @ov		Overlay to fill
@@ -241,7 +285,7 @@ static int of_fill_overlay_info(struct of_overlay *ov,
 	if (ovinfo->overlay == NULL)
 		goto err_fail;
 
-	ovinfo->target = find_target_node(info_node);
+	ovinfo->target = find_target_node(ov, info_node);
 	if (ovinfo->target == NULL)
 		goto err_fail;
 
@@ -341,6 +385,7 @@ void of_overlay_release(struct kobject *kobj)
 {
 	struct of_overlay *ov = kobj_to_overlay(kobj);
 
+	kfree(ov->indirect_id);
 	kfree(ov);
 }
 
@@ -432,17 +477,8 @@ static struct kobj_type of_overlay_ktype = {
 
 static struct kset *ov_kset;
 
-/**
- * of_overlay_create() - Create and apply an overlay
- * @tree:	Device node containing all the overlays
- *
- * Creates and applies an overlay while also keeping track
- * of the overlay in a list. This list can be used to prevent
- * illegal overlay removals.
- *
- * Returns the id of the created overlay, or an negative error number
- */
-int of_overlay_create(struct device_node *tree)
+static int __of_overlay_create(struct device_node *tree,
+		const char *indirect_id)
 {
 	struct of_overlay *ov;
 	int err, id;
@@ -457,6 +493,14 @@ int of_overlay_create(struct device_node *tree)
 		return -ENOMEM;
 	ov->id = -1;
 
+	if (indirect_id) {
+		ov->indirect_id = kstrdup(indirect_id, GFP_KERNEL);
+		if (!ov->indirect_id) {
+			err = -ENOMEM;
+			goto err_no_mem;
+		}
+	}
+
 	INIT_LIST_HEAD(&ov->node);
 
 	of_changeset_init(&ov->cset);
@@ -523,13 +567,47 @@ err_free_idr:
 	idr_remove(&ov_idr, ov->id);
 err_destroy_trans:
 	of_changeset_destroy(&ov->cset);
+err_no_mem:
+	kfree(ov->indirect_id);
 	kfree(ov);
 	mutex_unlock(&of_mutex);
 
 	return err;
 }
+
+/**
+ * of_overlay_create() - Create and apply an overlay
+ * @tree:	Device node containing all the overlays
+ *
+ * Creates and applies an overlay while also keeping track
+ * of the overlay in a list. This list can be used to prevent
+ * illegal overlay removals.
+ *
+ * Returns the id of the created overlay, or an negative error number
+ */
+int of_overlay_create(struct device_node *tree)
+{
+	return __of_overlay_create(tree, NULL);
+}
 EXPORT_SYMBOL_GPL(of_overlay_create);
 
+/**
+ * of_overlay_create_indirect() - Create and apply an overlay
+ * @tree:	Device node containing all the overlays
+ * @id:		Indirect property phandle
+ *
+ * Creates and applies an overlay while also keeping track
+ * of the overlay in a list. This list can be used to prevent
+ * illegal overlay removals.
+ *
+ * Returns the id of the created overlay, or an negative error number
+ */
+int of_overlay_create_indirect(struct device_node *tree, const char *id)
+{
+	return __of_overlay_create(tree, id);
+}
+EXPORT_SYMBOL_GPL(of_overlay_create_indirect);
+
 /* check whether the given node, lies under the given tree */
 static int overlay_subtree_check(struct device_node *tree,
 		struct device_node *dn)
diff --git a/include/linux/of.h b/include/linux/of.h
index b87838e..98c9244 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1074,6 +1074,8 @@ int of_overlay_create(struct device_node *tree);
 int of_overlay_destroy(int id);
 int of_overlay_destroy_all(void);
 
+int of_overlay_create_indirect(struct device_node *tree, const char *id);
+
 #else
 
 static inline int of_overlay_create(struct device_node *tree)
@@ -1091,6 +1093,12 @@ static inline int of_overlay_destroy_all(void)
 	return -ENOTSUPP;
 }
 
+static inline int of_overlay_create_indirect(struct device_node *tree,
+		const char *id)
+{
+	return -ENOTSUPP;
+}
+
 #endif
 
 #endif /* _LINUX_OF_H */
-- 
1.7.12

Re: [PATCH 1/8] of: overlay: Implement indirect target support

[Rob Herring]

On Fri, Jun 12, 2015 at 2:54 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> Some applications require applying the same overlay to a different
> target according to some external condition (for instance depending
> on the slot a card has been inserted, the overlay target is different).
>
> The indirect target use requires using the new
> of_overlay_create_indirect() API which uses a text selector.
>
> The format requires the use of a target-indirect node as follows:
>
>         fragment@0 {
>                 target-indirect {
>                         foo { target = <&foo_target>; };
>                         bar { target = <&bar_target>; };
>                 };
>         };

The problem with this is it does not scale. The overlay has to be
changed for every new target. If you had an add-on board (possibly
providing an overlay from an EEPROM), you would not want to have to
rebuild overlays with every new host board. It also only handles
translations of where to apply the overlay, but doesn't provide
translations of phandles within the overlay. Say a node that is a
clock or regulator consumer for example. Or am I missing something.

Grant and I discussed this briefly. I think we need a connector
definition in the base dtb which can provide the target for an
overlay. The connector should provide the translation between
connector local signals/buses and host signals/buses. We need to
define what this translation would look like for each binding.

At least for GPIO, we could have something similar to interrupt-map
properties. For example, to map connector gpio 0 to host gpio 66 and
connector gpio 1 to host gpio 44:

gpio-map = <0 &host-gpio 66>, <1 &host-gpio 44>;

We'd need to define how to handle I2C, SPI, regulators, and clocks
minimally. Perhaps rather than trying to apply nodes into the base
dtb, they should be under the connector and the kernel has to learn to
not just look for child nodes for various bindings. Just thinking
aloud...

Rob

Re: [PATCH 1/8] of: overlay: Implement indirect target support

[Rob Herring]

On Fri, Jun 12, 2015 at 2:54 PM, Pantelis Antoniou
<pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
> Some applications require applying the same overlay to a different
> target according to some external condition (for instance depending
> on the slot a card has been inserted, the overlay target is different).
>
> The indirect target use requires using the new
> of_overlay_create_indirect() API which uses a text selector.
>
> The format requires the use of a target-indirect node as follows:
>
>         fragment@0 {
>                 target-indirect {
>                         foo { target = <&foo_target>; };
>                         bar { target = <&bar_target>; };
>                 };
>         };

The problem with this is it does not scale. The overlay has to be
changed for every new target. If you had an add-on board (possibly
providing an overlay from an EEPROM), you would not want to have to
rebuild overlays with every new host board. It also only handles
translations of where to apply the overlay, but doesn't provide
translations of phandles within the overlay. Say a node that is a
clock or regulator consumer for example. Or am I missing something.

Grant and I discussed this briefly. I think we need a connector
definition in the base dtb which can provide the target for an
overlay. The connector should provide the translation between
connector local signals/buses and host signals/buses. We need to
define what this translation would look like for each binding.

At least for GPIO, we could have something similar to interrupt-map
properties. For example, to map connector gpio 0 to host gpio 66 and
connector gpio 1 to host gpio 44:

gpio-map = <0 &host-gpio 66>, <1 &host-gpio 44>;

We'd need to define how to handle I2C, SPI, regulators, and clocks
minimally. Perhaps rather than trying to apply nodes into the base
dtb, they should be under the connector and the kernel has to learn to
not just look for child nodes for various bindings. Just thinking
aloud...

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] of: overlay: Implement indirect target support

[Guenter Roeck]

On 06/17/2015 05:10 PM, Rob Herring wrote:
> On Fri, Jun 12, 2015 at 2:54 PM, Pantelis Antoniou
> <pantelis.antoniou@konsulko.com> wrote:
>> Some applications require applying the same overlay to a different
>> target according to some external condition (for instance depending
>> on the slot a card has been inserted, the overlay target is different).
>>
>> The indirect target use requires using the new
>> of_overlay_create_indirect() API which uses a text selector.
>>
>> The format requires the use of a target-indirect node as follows:
>>
>>          fragment@0 {
>>                  target-indirect {
>>                          foo { target = <&foo_target>; };
>>                          bar { target = <&bar_target>; };
>>                  };
>>          };
>
> The problem with this is it does not scale. The overlay has to be
> changed for every new target. If you had an add-on board (possibly

Not really. For our use case, at least, each target is a slot in the
chassis, so we end up with something like

		target-indirect {
			slot0 { target = <&sib0i2c>; };
			slot1 { target = <&sib1i2c>; };
			slot2 { target = <&sib2i2c>; };
			slot3 { target = <&sib3i2c>; };
			slot4 { target = <&sib4i2c>; };
			slot5 { target = <&sib5i2c>; };
			slot6 { target = <&sib6i2c>; };
			slot7 { target = <&sib7i2c>; };
			slot8 { target = <&sib8i2c>; };
		};

where sib[0-8]i2c is defined in the master dts file.

Since the number of slots is well defined, the overlay will
always work. Sure, it may have to be updated if it is used in
a chassis with 20 slots, but that doesn't happen that often.

> providing an overlay from an EEPROM), you would not want to have to
> rebuild overlays with every new host board. It also only handles
> translations of where to apply the overlay, but doesn't provide
> translations of phandles within the overlay. Say a node that is a
> clock or regulator consumer for example. Or am I missing something.
>
> Grant and I discussed this briefly. I think we need a connector
> definition in the base dtb which can provide the target for an
> overlay. The connector should provide the translation between
> connector local signals/buses and host signals/buses. We need to
> define what this translation would look like for each binding.
>
> At least for GPIO, we could have something similar to interrupt-map
> properties. For example, to map connector gpio 0 to host gpio 66 and
> connector gpio 1 to host gpio 44:
>
> gpio-map = <0 &host-gpio 66>, <1 &host-gpio 44>;
>
> We'd need to define how to handle I2C, SPI, regulators, and clocks
> minimally. Perhaps rather than trying to apply nodes into the base
> dtb, they should be under the connector and the kernel has to learn to
> not just look for child nodes for various bindings. Just thinking
> aloud...
>
Anything is fine with me, as long as it is usable (and does not require
us to create 9 overlay files for sib[0-8] from the example above).

The real tricky part is pci, where it is not just about simple target
redirection but irq numbers, memory address ranges, and bus number ranges.
It would be quite useful to have a workable solution for that as well,
but at least I don't have an idea how it could be done.

Talking about connector ...

Right now we have something like

	sib0 {
             compatible = "jnx,sib-connector", "simple-bus";
	    ... (various properties)
	};

Maybe we could use something like the following ?

	sib0 {
             compatible = "jnx,sib-connector", "simple-bus";
	    ... (various attributes)
	    ref0 = <&sib0i2c>;
	    ref1 = <&sib0spi>;
	};

and then just reference ref0 and ref1 as targets in the overlay itself ?

Guenter

[PATCH 2/8] of: unittest: Add indirect overlay target test

[Pantelis Antoniou]

Add a unittest for the indirect overlay target case.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/unittest-data/testcases.dts      |  9 +++++
 drivers/of/unittest-data/tests-overlay.dtsi | 19 ++++++++++
 drivers/of/unittest.c                       | 59 +++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+)

diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
index 12f7c3d..ec17ab7 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -75,5 +75,14 @@
 				target = <0x00000000>;
 			};
 		};
+		overlay16 {
+			fragment@0 {
+				target-indirect {
+					unittest16 {
+						target = <0x00000000>;
+					};
+				};
+			};
+		};
 	};
 }; };
diff --git a/drivers/of/unittest-data/tests-overlay.dtsi b/drivers/of/unittest-data/tests-overlay.dtsi
index 02ba56c..881d863 100644
--- a/drivers/of/unittest-data/tests-overlay.dtsi
+++ b/drivers/of/unittest-data/tests-overlay.dtsi
@@ -110,6 +110,12 @@
 						};
 					};
 				};
+
+				unittest16: test-unittest16 {
+					compatible = "unittest";
+					status = "disabled";
+					reg = <16>;
+				};
 			};
 		};
 
@@ -325,5 +331,18 @@
 			};
 		};
 
+		/* test enable using indirect functionality */
+		overlay16 {
+			fragment@0 {
+				target-indirect {
+					unittest16 {
+						target = <&unittest16>;
+					};
+				};
+				__overlay__ {
+					status = "okay";
+				};
+			};
+		};
 	};
 };
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 0a27b38..ee1a3bb 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1858,6 +1858,63 @@ static inline void of_unittest_overlay_i2c_15(void) { }
 
 #endif
 
+static void of_unittest_overlay_16(void)
+{
+	int ret;
+	int overlay_nr = 16;
+	int unittest_nr = 16;
+	enum overlay_type ovtype = PDEV_OVERLAY;
+	int before = 0;
+	int after = 1;
+	struct device_node *np = NULL;
+	int id = -1;
+
+	/* unittest device must not be in before state */
+	if (of_unittest_device_exists(unittest_nr, ovtype) != before) {
+		unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n",
+				overlay_path(overlay_nr),
+				unittest_path(unittest_nr, ovtype),
+				!before ? "enabled" : "disabled");
+		return;
+	}
+
+	np = of_find_node_by_path(overlay_path(overlay_nr));
+	if (np == NULL) {
+		unittest(0, "could not find overlay node @\"%s\"\n",
+				overlay_path(overlay_nr));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = of_overlay_create_indirect(np, "unittest16");
+	if (ret < 0) {
+		unittest(0, "could not create overlay from \"%s\"\n",
+				overlay_path(overlay_nr));
+		goto out;
+	}
+	id = ret;
+	of_unittest_track_overlay(id);
+
+	ret = 0;
+
+out:
+	of_node_put(np);
+
+	if (ret)
+		return;
+
+	/* unittest device must be to set to after state */
+	if (of_unittest_device_exists(unittest_nr, ovtype) != after) {
+		unittest(0, "overlay @\"%s\" failed to create @\"%s\" %s\n",
+				overlay_path(overlay_nr),
+				unittest_path(unittest_nr, ovtype),
+				!after ? "enabled" : "disabled");
+		return;
+	}
+
+	unittest(1, "overlay test %d passed\n", 16);
+}
+
 static void __init of_unittest_overlay(void)
 {
 	struct device_node *bus_np = NULL;
@@ -1909,6 +1966,8 @@ static void __init of_unittest_overlay(void)
 	of_unittest_overlay_10();
 	of_unittest_overlay_11();
 
+	of_unittest_overlay_16();
+
 #if IS_BUILTIN(CONFIG_I2C)
 	if (unittest(of_unittest_overlay_i2c_init() == 0, "i2c init failed\n"))
 		goto out;
-- 
1.7.12

[PATCH 3/8] doc: dt: Document the indirect overlay method.

[Pantelis Antoniou]

Add a description of the indirect overlay method to the overlay
documention file.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 Documentation/devicetree/overlay-notes.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt
index d418a6c..dd595e6 100644
--- a/Documentation/devicetree/overlay-notes.txt
+++ b/Documentation/devicetree/overlay-notes.txt
@@ -100,6 +100,10 @@ Finally, if you need to remove all overlays in one-go, just call
 of_overlay_destroy_all() which will remove every single one in the correct
 order.
 
+If your board has multiple slots/places where a single overlay can work
+and each slot is defined by a node, you can use the of_overlay_create_indirect()
+method to select the target.
+
 Overlay DTS Format
 ------------------
 
@@ -113,6 +117,11 @@ The DTS of an overlay should have the following format:
 		target=<phandle>;	/* phandle target of the overlay */
 	or
 		target-path="/path";	/* target path of the overlay */
+	or
+		target-indirect {	/* indirect target selector */
+			foo { target|target-path ... };
+			bar { .... };
+		};
 
 		__overlay__ {
 			property-a;	/* add property-a to the target */
@@ -131,3 +140,7 @@ Using the non-phandle based target method allows one to use a base DT which does
 not contain a __symbols__ node, i.e. it was not compiled with the -@ option.
 The __symbols__ node is only required for the target=<phandle> method, since it
 contains the information required to map from a phandle to a tree location.
+
+The indirect target requires the use of a selector target on the call to
+of_overlay_create_indirect(). I.e. passing the "foo" id will select the target
+in the foo node, "bar" in bar node, etc.
-- 
1.7.12

[PATCH 3/8] doc: dt: Document the indirect overlay method.

[Pantelis Antoniou]

Add a description of the indirect overlay method to the overlay
documention file.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 Documentation/devicetree/overlay-notes.txt | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt
index d418a6c..dd595e6 100644
--- a/Documentation/devicetree/overlay-notes.txt
+++ b/Documentation/devicetree/overlay-notes.txt
@@ -100,6 +100,10 @@ Finally, if you need to remove all overlays in one-go, just call
 of_overlay_destroy_all() which will remove every single one in the correct
 order.
 
+If your board has multiple slots/places where a single overlay can work
+and each slot is defined by a node, you can use the of_overlay_create_indirect()
+method to select the target.
+
 Overlay DTS Format
 ------------------
 
@@ -113,6 +117,11 @@ The DTS of an overlay should have the following format:
 		target=<phandle>;	/* phandle target of the overlay */
 	or
 		target-path="/path";	/* target path of the overlay */
+	or
+		target-indirect {	/* indirect target selector */
+			foo { target|target-path ... };
+			bar { .... };
+		};
 
 		__overlay__ {
 			property-a;	/* add property-a to the target */
@@ -131,3 +140,7 @@ Using the non-phandle based target method allows one to use a base DT which does
 not contain a __symbols__ node, i.e. it was not compiled with the -@ option.
 The __symbols__ node is only required for the target=<phandle> method, since it
 contains the information required to map from a phandle to a tree location.
+
+The indirect target requires the use of a selector target on the call to
+of_overlay_create_indirect(). I.e. passing the "foo" id will select the target
+in the foo node, "bar" in bar node, etc.
-- 
1.7.12

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/8] of: overlay: Introduce target root capability.

[Pantelis Antoniou]

The target facility of an overlay allows the target to be any point
in the live kernel tree, since it usually that's required when
creating overlays for internal SoC devices. The target ends up
to be a single node in the tree.

However when we're dealing with probeable busses this is a problem
since the target node differs according to the bus the plugged
device lies.

Using an overlay creating method using a target root node allows
us to use a single overlay for those cases.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/overlay.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++----
 include/linux/of.h   |   8 ++++
 2 files changed, 102 insertions(+), 7 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 80aab6f..f9b55d1 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -55,6 +55,7 @@ struct of_overlay {
 	struct of_changeset cset;
 	struct kobject kobj;
 	char *indirect_id;
+	struct device_node *target_root;
 };
 
 /* master enable switch; once set to 0 can't be re-enabled */
@@ -195,21 +196,84 @@ static int of_overlay_apply(struct of_overlay *ov)
 static struct device_node *find_target_node_direct(struct of_overlay *ov,
 		struct device_node *info_node)
 {
+	struct device_node *target = NULL, *np;
 	const char *path;
+	char *newpath;
 	u32 val;
 	int ret;
 
 	/* first try to go by using the target as a phandle */
 	ret = of_property_read_u32(info_node, "target", &val);
-	if (ret == 0)
-		return of_find_node_by_phandle(val);
+	if (ret == 0) {
+		target = of_find_node_by_phandle(val);
+		if (!target) {
+			pr_err("%s: Could not find target phandle 0x%x\n",
+					__func__, val);
+			return NULL;
+		}
+		goto check_root;
+	}
 
 	/* failed, try to locate by path */
 	ret = of_property_read_string(info_node, "target-path", &path);
-	if (ret == 0)
-		return of_find_node_by_path(path);
+	if (ret == 0) {
+
+		if (!ov->target_root) {
+			target = of_find_node_by_path(path);
+			if (!target)
+				pr_err("%s: Could not find target path \"%s\"\n",
+						__func__, path);
+			return target;
+		}
+
+		/* remove preceding '/' from path; relative path */
+		if (*path == '/') {
+			while (*path == '/')
+				path++;
+
+			newpath = kasprintf(GFP_KERNEL, "%s%s%s",
+					of_node_full_name(ov->target_root),
+					*path ? "/" : "", path);
+			if (!newpath) {
+				pr_err("%s: Could not allocate \"%s%s%s\"\n",
+					__func__,
+					of_node_full_name(ov->target_root),
+					*path ? "/" : "", path);
+				return NULL;
+			}
+			target = of_find_node_by_path(newpath);
+			kfree(newpath);
+
+			return target;
+
+		}
+		/* target is an alias, need to check */
+		target = of_find_node_by_path(path);
+		if (!target) {
+			pr_err("%s: Could not find alias \"%s\"\n",
+					__func__, path);
+			return NULL;
+		}
+		goto check_root;
+	}
 
 	return NULL;
+
+check_root:
+	if (!ov->target_root)
+		return target;
+
+	/* got a target, but we have to check it's under target root */
+	for (np = target; np; np = np->parent) {
+		if (np == ov->target_root)
+			return target;
+	}
+	pr_err("%s: target \"%s\" not under target_root \"%s\"\n",
+			__func__, of_node_full_name(target),
+			of_node_full_name(ov->target_root));
+	/* target is not under target_root */
+	of_node_put(target);
+	return NULL;
 }
 
 /*
@@ -385,6 +449,7 @@ void of_overlay_release(struct kobject *kobj)
 {
 	struct of_overlay *ov = kobj_to_overlay(kobj);
 
+	of_node_put(ov->target_root);
 	kfree(ov->indirect_id);
 	kfree(ov);
 }
@@ -478,7 +543,7 @@ static struct kobj_type of_overlay_ktype = {
 static struct kset *ov_kset;
 
 static int __of_overlay_create(struct device_node *tree,
-		const char *indirect_id)
+		const char *indirect_id, struct device_node *target_root)
 {
 	struct of_overlay *ov;
 	int err, id;
@@ -500,6 +565,7 @@ static int __of_overlay_create(struct device_node *tree,
 			goto err_no_mem;
 		}
 	}
+	ov->target_root = of_node_get(target_root);
 
 	INIT_LIST_HEAD(&ov->node);
 
@@ -568,6 +634,7 @@ err_free_idr:
 err_destroy_trans:
 	of_changeset_destroy(&ov->cset);
 err_no_mem:
+	of_node_put(ov->target_root);
 	kfree(ov->indirect_id);
 	kfree(ov);
 	mutex_unlock(&of_mutex);
@@ -587,7 +654,7 @@ err_no_mem:
  */
 int of_overlay_create(struct device_node *tree)
 {
-	return __of_overlay_create(tree, NULL);
+	return __of_overlay_create(tree, NULL, NULL);
 }
 EXPORT_SYMBOL_GPL(of_overlay_create);
 
@@ -604,10 +671,30 @@ EXPORT_SYMBOL_GPL(of_overlay_create);
  */
 int of_overlay_create_indirect(struct device_node *tree, const char *id)
 {
-	return __of_overlay_create(tree, id);
+	return __of_overlay_create(tree, id, NULL);
 }
 EXPORT_SYMBOL_GPL(of_overlay_create_indirect);
 
+/**
+ * of_overlay_create_target_root() - Create and apply an overlay
+ *			under which will be limited to target_root
+ * @tree:		Device node containing all the overlays
+ * @target_root:	Target root for the overlay.
+ *
+ * Creates and applies an overlay while also keeping track
+ * of the overlay in a list. This list can be used to prevent
+ * illegal overlay removals. The overlay is only allowed to
+ * target nodes under the target_root node.
+ *
+ * Returns the id of the created overlay, or an negative error number
+ */
+int of_overlay_create_target_root(struct device_node *tree,
+		struct device_node *target_root)
+{
+	return __of_overlay_create(tree, NULL, target_root);
+}
+EXPORT_SYMBOL_GPL(of_overlay_create_target_root);
+
 /* check whether the given node, lies under the given tree */
 static int overlay_subtree_check(struct device_node *tree,
 		struct device_node *dn)
diff --git a/include/linux/of.h b/include/linux/of.h
index 98c9244..dd84bf1 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1075,6 +1075,8 @@ int of_overlay_destroy(int id);
 int of_overlay_destroy_all(void);
 
 int of_overlay_create_indirect(struct device_node *tree, const char *id);
+int of_overlay_create_target_root(struct device_node *tree,
+		struct device_node *target_root);
 
 #else
 
@@ -1099,6 +1101,12 @@ static inline int of_overlay_create_indirect(struct device_node *tree,
 	return -ENOTSUPP;
 }
 
+static inline int of_overlay_create_target_root(struct device_node *tree,
+		struct device_node *target_root)
+{
+	return -ENOTSUPP;
+}
+
 #endif
 
 #endif /* _LINUX_OF_H */
-- 
1.7.12

[PATCH 5/8] of: unittest: Unit-tests for target root overlays.

[Pantelis Antoniou]

Add unittests for target-root based overlays.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/unittest-data/testcases.dts      |   5 +
 drivers/of/unittest-data/tests-overlay.dtsi |  48 +++++++
 drivers/of/unittest.c                       | 213 ++++++++++++++++++++++++++++
 3 files changed, 266 insertions(+)

diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts
index ec17ab7..8052b96 100644
--- a/drivers/of/unittest-data/testcases.dts
+++ b/drivers/of/unittest-data/testcases.dts
@@ -84,5 +84,10 @@
 				};
 			};
 		};
+		overlay18 {
+			fragment@0 {
+				target = <0x00000000>;
+			};
+		};
 	};
 }; };
diff --git a/drivers/of/unittest-data/tests-overlay.dtsi b/drivers/of/unittest-data/tests-overlay.dtsi
index 881d863..e10ff5a 100644
--- a/drivers/of/unittest-data/tests-overlay.dtsi
+++ b/drivers/of/unittest-data/tests-overlay.dtsi
@@ -116,6 +116,24 @@
 					status = "disabled";
 					reg = <16>;
 				};
+
+				unittest17: test-unittest17 {
+					compatible = "unittest";
+					status = "disabled";
+					reg = <17>;
+				};
+
+				unittest18: test-unittest18 {
+					compatible = "unittest";
+					status = "disabled";
+					reg = <18>;
+				};
+
+				unittest19: test-unittest19 {
+					compatible = "unittest";
+					status = "disabled";
+					reg = <19>;
+				};
 			};
 		};
 
@@ -344,5 +362,35 @@
 				};
 			};
 		};
+
+		/* test enable using target root (relative path) */
+		overlay17 {
+			fragment@0 {
+				target-path = "/";
+				__overlay__ {
+					status = "okay";
+				};
+			};
+		};
+
+		/* test enable using target phandle */
+		overlay18 {
+			fragment@0 {
+				target = <&unittest18>;
+				__overlay__ {
+					status = "okay";
+				};
+			};
+		};
+
+		/* test trying to enable out of root (should fail) */
+		overlay19 {
+			fragment@0 {
+				target = <&unittest19>;
+				__overlay__ {
+					status = "okay";
+				};
+			};
+		};
 	};
 };
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index ee1a3bb..55ded19 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1915,6 +1915,215 @@ out:
 	unittest(1, "overlay test %d passed\n", 16);
 }
 
+static void of_unittest_overlay_17(void)
+{
+	int ret;
+	int overlay_nr = 17;
+	int unittest_nr = 17;
+	enum overlay_type ovtype = PDEV_OVERLAY;
+	int before = 0;
+	int after = 1;
+	const char *root_path;
+	struct device_node *np = NULL, *target_root = NULL;
+	int id = -1;
+
+	/* unittest device must not be in before state */
+	if (of_unittest_device_exists(unittest_nr, ovtype) != before) {
+		unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n",
+				overlay_path(overlay_nr),
+				unittest_path(unittest_nr, ovtype),
+				!before ? "enabled" : "disabled");
+		return;
+	}
+
+	np = of_find_node_by_path(overlay_path(overlay_nr));
+	if (np == NULL) {
+		unittest(0, "could not find overlay node @\"%s\"\n",
+				overlay_path(overlay_nr));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	root_path = "/testcase-data/overlay-node/test-bus/test-unittest17";
+	target_root = of_find_node_by_path(root_path);
+	if (!target_root) {
+		unittest(0, "could not find target_root node @\"%s\"\n",
+				root_path);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = of_overlay_create_target_root(np, target_root);
+	of_node_put(target_root);
+
+	if (ret < 0) {
+		unittest(0, "could not create overlay from \"%s\"\n",
+				overlay_path(overlay_nr));
+		goto out;
+	}
+	id = ret;
+	of_unittest_track_overlay(id);
+
+	ret = 0;
+
+out:
+	of_node_put(np);
+
+	if (ret)
+		return;
+
+	/* unittest device must be to set to after state */
+	if (of_unittest_device_exists(unittest_nr, ovtype) != after) {
+		unittest(0, "overlay @\"%s\" failed to create @\"%s\" %s\n",
+				overlay_path(overlay_nr),
+				unittest_path(unittest_nr, ovtype),
+				!after ? "enabled" : "disabled");
+		return;
+	}
+
+	unittest(1, "overlay test %d passed\n", 17);
+}
+
+static void of_unittest_overlay_18(void)
+{
+	int ret;
+	int overlay_nr = 18;
+	int unittest_nr = 18;
+	enum overlay_type ovtype = PDEV_OVERLAY;
+	int before = 0;
+	int after = 1;
+	const char *root_path;
+	struct device_node *np = NULL, *target_root = NULL;
+	int id = -1;
+
+	/* unittest device must not be in before state */
+	if (of_unittest_device_exists(unittest_nr, ovtype) != before) {
+		unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n",
+				overlay_path(overlay_nr),
+				unittest_path(unittest_nr, ovtype),
+				!before ? "enabled" : "disabled");
+		return;
+	}
+
+	np = of_find_node_by_path(overlay_path(overlay_nr));
+	if (np == NULL) {
+		unittest(0, "could not find overlay node @\"%s\"\n",
+				overlay_path(overlay_nr));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	root_path = "/testcase-data/overlay-node/test-bus/test-unittest18";
+	target_root = of_find_node_by_path(root_path);
+	if (!target_root) {
+		unittest(0, "could not find target_root node @\"%s\"\n",
+				root_path);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = of_overlay_create_target_root(np, target_root);
+	of_node_put(target_root);
+
+	if (ret < 0) {
+		unittest(0, "could not create overlay from \"%s\"\n",
+				overlay_path(overlay_nr));
+		goto out;
+	}
+	id = ret;
+	of_unittest_track_overlay(id);
+
+	ret = 0;
+
+out:
+	of_node_put(np);
+
+	if (ret)
+		return;
+
+	/* unittest device must be to set to after state */
+	if (of_unittest_device_exists(unittest_nr, ovtype) != after) {
+		unittest(0, "overlay @\"%s\" failed to create @\"%s\" %s\n",
+				overlay_path(overlay_nr),
+				unittest_path(unittest_nr, ovtype),
+				!after ? "enabled" : "disabled");
+		return;
+	}
+
+	unittest(1, "overlay test %d passed\n", 18);
+}
+
+static void of_unittest_overlay_19(void)
+{
+	int ret;
+	int overlay_nr = 19;
+	int unittest_nr = 19;
+	enum overlay_type ovtype = PDEV_OVERLAY;
+	int before = 0;
+	int after = 0;
+	const char *root_path;
+	struct device_node *np = NULL, *target_root = NULL;
+	int id = -1;
+
+	/* unittest device must not be in before state */
+	if (of_unittest_device_exists(unittest_nr, ovtype) != before) {
+		unittest(0, "overlay @\"%s\" with device @\"%s\" %s\n",
+				overlay_path(overlay_nr),
+				unittest_path(unittest_nr, ovtype),
+				!before ? "enabled" : "disabled");
+		return;
+	}
+
+	np = of_find_node_by_path(overlay_path(overlay_nr));
+	if (np == NULL) {
+		unittest(0, "could not find overlay node @\"%s\"\n",
+				overlay_path(overlay_nr));
+		ret = -EINVAL;
+		goto out;
+	}
+
+	root_path = "/testcase-data/overlay-node/test-bus/test-unittest19";
+	target_root = of_find_node_by_path(root_path);
+	if (!target_root) {
+		unittest(0, "could not find target_root node @\"%s\"\n",
+				root_path);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = of_overlay_create_target_root(np, target_root);
+	of_node_put(target_root);
+
+	if (ret >= 0) {
+		unittest(0, "created overlay from \"%s\" while we shouldn't\n",
+				overlay_path(overlay_nr));
+		id = ret;
+		of_unittest_track_overlay(id);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	ret = 0;
+
+out:
+	of_node_put(np);
+
+	if (ret)
+		return;
+
+	/* unittest device must be to set to after state */
+	if (of_unittest_device_exists(unittest_nr, ovtype) != after) {
+		unittest(0, "overlay @\"%s\" failed to create @\"%s\" %s\n",
+				overlay_path(overlay_nr),
+				unittest_path(unittest_nr, ovtype),
+				!after ? "enabled" : "disabled");
+		return;
+	}
+
+	unittest(1, "overlay test %d passed\n", 16);
+}
+
+
 static void __init of_unittest_overlay(void)
 {
 	struct device_node *bus_np = NULL;
@@ -1968,6 +2177,10 @@ static void __init of_unittest_overlay(void)
 
 	of_unittest_overlay_16();
 
+	of_unittest_overlay_17();
+	of_unittest_overlay_18();
+	of_unittest_overlay_19();
+
 #if IS_BUILTIN(CONFIG_I2C)
 	if (unittest(of_unittest_overlay_i2c_init() == 0, "i2c init failed\n"))
 		goto out;
-- 
1.7.12

[PATCH 6/8] doc: dt: Document the target root overlay method

[Pantelis Antoniou]

Add a description of the target root overlay method to the overlay
documention file.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 Documentation/devicetree/overlay-notes.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt
index dd595e6..00ede57 100644
--- a/Documentation/devicetree/overlay-notes.txt
+++ b/Documentation/devicetree/overlay-notes.txt
@@ -104,6 +104,10 @@ If your board has multiple slots/places where a single overlay can work
 and each slot is defined by a node, you can use the of_overlay_create_indirect()
 method to select the target.
 
+For overlays on probeable busses, use the of_overlay_create_target_root() method
+in which you supply a device node as a target root, and which all target
+references in the overlay are performed relative to that node.
+
 Overlay DTS Format
 ------------------
 
@@ -144,3 +148,7 @@ contains the information required to map from a phandle to a tree location.
 The indirect target requires the use of a selector target on the call to
 of_overlay_create_indirect(). I.e. passing the "foo" id will select the target
 in the foo node, "bar" in bar node, etc.
+
+Note that when using the target root create method all target references must
+lie under the target root node. I.e. the overlay is not allowed to 'break' out
+of the root.
-- 
1.7.12

[PATCH 6/8] doc: dt: Document the target root overlay method

[Pantelis Antoniou]

Add a description of the target root overlay method to the overlay
documention file.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
---
 Documentation/devicetree/overlay-notes.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/overlay-notes.txt b/Documentation/devicetree/overlay-notes.txt
index dd595e6..00ede57 100644
--- a/Documentation/devicetree/overlay-notes.txt
+++ b/Documentation/devicetree/overlay-notes.txt
@@ -104,6 +104,10 @@ If your board has multiple slots/places where a single overlay can work
 and each slot is defined by a node, you can use the of_overlay_create_indirect()
 method to select the target.
 
+For overlays on probeable busses, use the of_overlay_create_target_root() method
+in which you supply a device node as a target root, and which all target
+references in the overlay are performed relative to that node.
+
 Overlay DTS Format
 ------------------
 
@@ -144,3 +148,7 @@ contains the information required to map from a phandle to a tree location.
 The indirect target requires the use of a selector target on the call to
 of_overlay_create_indirect(). I.e. passing the "foo" id will select the target
 in the foo node, "bar" in bar node, etc.
+
+Note that when using the target root create method all target references must
+lie under the target root node. I.e. the overlay is not allowed to 'break' out
+of the root.
-- 
1.7.12

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 7/8] of: dynamic: Add __of_node_dupv()

[Pantelis Antoniou]

Add an __of_node_dupv() private method and make __of_node_dup() use it.
This is required for the subsequent changeset accessors which will
make use of it.

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/dynamic.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 53826b8..e452171 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -394,8 +394,9 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
 }
 
 /**
- * __of_node_dup() - Duplicate or create an empty device node dynamically.
- * @fmt: Format string (plus vargs) for new full name of the device node
+ * __of_node_dupv() - Duplicate or create an empty device node dynamically.
+ * @fmt: Format string for new full name of the device node
+ * @vargs: va_list containing the arugments for the node full name
  *
  * Create an device tree node, either by duplicating an empty node or by allocating
  * an empty one suitable for further modification.  The node data are
@@ -403,17 +404,15 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
  * OF_DETACHED bits set. Returns the newly allocated node or NULL on out of
  * memory error.
  */
-struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...)
+struct device_node *__of_node_dupv(const struct device_node *np,
+		const char *fmt, va_list vargs)
 {
-	va_list vargs;
 	struct device_node *node;
 
 	node = kzalloc(sizeof(*node), GFP_KERNEL);
 	if (!node)
 		return NULL;
-	va_start(vargs, fmt);
 	node->full_name = kvasprintf(GFP_KERNEL, fmt, vargs);
-	va_end(vargs);
 	if (!node->full_name) {
 		kfree(node);
 		return NULL;
@@ -445,6 +444,24 @@ struct device_node *__of_node_dup(const struct device_node *np, const char *fmt,
 	return NULL;
 }
 
+/**
+ * __of_node_dup() - Duplicate or create an empty device node dynamically.
+ * @fmt: Format string (plus vargs) for new full name of the device node
+ *
+ * See: __of_node_dupv()
+ */
+struct device_node *__of_node_dup(const struct device_node *np,
+		const char *fmt, ...)
+{
+	va_list vargs;
+	struct device_node *node;
+
+	va_start(vargs, fmt);
+	node = __of_node_dupv(np, fmt, vargs);
+	va_end(vargs);
+	return node;
+}
+
 static void __of_changeset_entry_destroy(struct of_changeset_entry *ce)
 {
 	of_node_put(ce->np);
-- 
1.7.12

[PATCH 8/8] of: changesets: Introduce changeset helper methods

[Pantelis Antoniou]

Changesets are very powerful, but the lack of a helper API
makes using them cumbersome. Introduce a simple copy based
API that makes things considerably easier.

To wit, adding a property using the raw API.

	struct property *prop;
	prop = kzalloc(sizeof(*prop)), GFP_KERNEL);
	prop->name = kstrdup("compatible");
	prop->value = kstrdup("foo,bar");
	prop->length = strlen(prop->value) + 1;
	of_changeset_add_property(ocs, np, prop);

while using the helper API

	of_changeset_add_property_string(ocs, np, "compatible",
			"foo,bar");

Signed-off-by: Pantelis Antoniou <pantelis.antoniou@konsulko.com>
---
 drivers/of/dynamic.c | 251 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h   |  73 +++++++++++++++
 2 files changed, 324 insertions(+)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index e452171..afa8e31 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -796,3 +796,254 @@ int of_changeset_action(struct of_changeset *ocs, unsigned long action,
 	list_add_tail(&ce->node, &ocs->entries);
 	return 0;
 }
+
+/* changeset helpers */
+
+/**
+ * of_changeset_create_device_node - Create an empty device node
+ *
+ * @ocs:	changeset pointer
+ * @parent:	parent device node
+ * @fmt:	format string for the node's full_name
+ * @args:	argument list for the format string
+ *
+ * Create an empty device node, marking it as detached and allocated.
+ *
+ * Returns a device node on success, an error encoded pointer otherwise
+ */
+struct device_node *of_changeset_create_device_nodev(
+	struct of_changeset *ocs, struct device_node *parent,
+	const char *fmt, va_list vargs)
+{
+	struct device_node *node;
+
+	node = __of_node_dupv(NULL, fmt, vargs);
+	if (!node)
+		return ERR_PTR(-ENOMEM);
+
+	node->parent = parent;
+	return node;
+}
+
+/**
+ * of_changeset_create_device_node - Create an empty device node
+ *
+ * @ocs:	changeset pointer
+ * @parent:	parent device node
+ * @fmt:	Format string for the node's full_name
+ * ...		Arguments
+ *
+ * Create an empty device node, marking it as detached and allocated.
+ *
+ * Returns a device node on success, an error encoded pointer otherwise
+ */
+struct device_node *of_changeset_create_device_node(
+	struct of_changeset *ocs, struct device_node *parent,
+	const char *fmt, ...)
+{
+	va_list vargs;
+	struct device_node *node;
+
+	va_start(vargs, fmt);
+	node = of_changeset_create_device_nodev(ocs, parent, fmt, vargs);
+	va_end(vargs);
+	return node;
+}
+
+/**
+ * of_changeset_add_property_copy - Create a new property copying name & value
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @value:	pointer to the value data
+ * @length:	length of the value in bytes
+ *
+ * Adds a property to the changeset by making copies of the name & value
+ * entries.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_add_property_copy(struct of_changeset *ocs,
+		struct device_node *np, const char *name, const void *value,
+		int length)
+{
+	struct property *prop;
+	char *new_name;
+	void *new_value;
+	int ret;
+
+	ret = -ENOMEM;
+
+	prop = kzalloc(sizeof(*prop), GFP_KERNEL);
+	if (!prop)
+		goto out_no_prop;
+
+	new_name = kstrdup(name, GFP_KERNEL);
+	if (!new_name)
+		goto out_no_name;
+
+	/*
+	 * NOTE: There is no check for zero length value.
+	 * In case of a boolean property, this will allocate a value
+	 * of zero bytes. We do this to work around the use
+	 * of of_get_property() calls on boolean values.
+	 */
+	new_value = kmemdup(value, length, GFP_KERNEL);
+	if (!new_value)
+		goto out_no_value;
+
+	of_property_set_flag(prop, OF_DYNAMIC);
+
+	prop->name = new_name;
+	prop->value = new_value;
+	prop->length = length;
+
+	ret = of_changeset_add_property(ocs, np, prop);
+	if (ret != 0)
+		goto out_no_add;
+
+	return 0;
+
+out_no_add:
+	kfree(prop->value);
+out_no_value:
+	kfree(prop->name);
+out_no_name:
+	kfree(prop);
+out_no_prop:
+	return ret;
+}
+
+/**
+ * of_changeset_add_property_string - Create a new string property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @str:	string property
+ *
+ * Adds a string property to the changeset by making copies of the name
+ * and the string value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_add_property_string(struct of_changeset *ocs,
+		struct device_node *np, const char *name, const char *str)
+{
+	return of_changeset_add_property_copy(ocs, np, name, str,
+			strlen(str) + 1);
+}
+
+/**
+ * of_changeset_add_property_stringf - Create a new formatted string property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @fmt:	format of string property
+ * ...		arguments of the format string
+ *
+ * Adds a string property to the changeset by making copies of the name
+ * and the formatted value.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_add_property_stringf(struct of_changeset *ocs,
+		struct device_node *np, const char *name, const char *fmt, ...)
+{
+	va_list vargs;
+	char *str;
+	int ret;
+
+	va_start(vargs, fmt);
+	str = kvasprintf(GFP_KERNEL, fmt, vargs);
+	va_end(vargs);
+
+	ret = of_changeset_add_property_string(ocs, np, name, str);
+
+	kfree(str);
+	return ret;
+}
+
+/**
+ * of_changeset_add_property_string_list - Create a new string list property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @strs:	pointer to the string list
+ * @count:	string count
+ *
+ * Adds a string list property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_add_property_string_list(struct of_changeset *ocs,
+		struct device_node *np, const char *name, const char **strs,
+		int count)
+{
+	int total, length, i, ret;
+	char *value, *s;
+
+	total = 0;
+	for (i = 0; i < count; i++) {
+		length = strlen(strs[i]);
+		total += length + 1;
+	}
+
+	value = kmalloc(total, GFP_KERNEL);
+	if (!value)
+		return -ENOMEM;
+
+	for (i = 0, s = value; i < count; i++) {
+		length = strlen(strs[i]);
+		memcpy(s, strs[i], length + 1);
+		s += length + 1;
+	}
+
+	ret = of_changeset_add_property_copy(ocs, np, name, value, total);
+
+	kfree(value);
+
+	return ret;
+}
+
+/**
+ * of_changeset_add_property_u32 - Create a new u32 property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ * @val:	value in host endian format
+ *
+ * Adds a u32 property to the changeset.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_add_property_u32(struct of_changeset *ocs,
+		struct device_node *np, const char *name, u32 val)
+{
+	/* in place */
+	val = cpu_to_be32(val);
+	return of_changeset_add_property_copy(ocs, np, name, &val, sizeof(val));
+}
+
+/**
+ * of_changeset_add_property_bool - Create a new u32 property
+ *
+ * @ocs:	changeset pointer
+ * @np:		device node pointer
+ * @name:	name of the property
+ *
+ * Adds a bool property to the changeset. Note that there is
+ * no option to set the value to false, since the property
+ * existing sets it to true.
+ *
+ * Returns zero on success, a negative error value otherwise.
+ */
+int of_changeset_add_property_bool(struct of_changeset *ocs,
+		struct device_node *np, const char *name)
+{
+	return of_changeset_add_property_copy(ocs, np, name, "", 0);
+}
diff --git a/include/linux/of.h b/include/linux/of.h
index dd84bf1..419e288 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1028,6 +1028,27 @@ static inline int of_changeset_update_property(struct of_changeset *ocs,
 {
 	return of_changeset_action(ocs, OF_RECONFIG_UPDATE_PROPERTY, np, prop);
 }
+
+struct device_node *of_changeset_create_device_nodev(
+	struct of_changeset *ocs, struct device_node *parent,
+	const char *fmt, va_list vargs);
+__printf(3, 4) struct device_node *of_changeset_create_device_node(
+	struct of_changeset *ocs, struct device_node *parent,
+	const char *fmt, ...);
+int of_changeset_add_property_copy(struct of_changeset *ocs,
+	struct device_node *np, const char *name,
+	const void *value, int length);
+int of_changeset_add_property_string(struct of_changeset *ocs,
+	struct device_node *np, const char *name, const char *str);
+__printf(4, 5) int of_changeset_add_property_stringf(struct of_changeset *ocs,
+		struct device_node *np, const char *name, const char *fmt, ...);
+int of_changeset_add_property_string_list(struct of_changeset *ocs,
+	struct device_node *np, const char *name, const char **strs, int count);
+int of_changeset_add_property_u32(struct of_changeset *ocs,
+	struct device_node *np, const char *name, u32 val);
+int of_changeset_add_property_bool(struct of_changeset *ocs,
+	struct device_node *np, const char *name);
+
 #else /* CONFIG_OF_DYNAMIC */
 static inline int of_reconfig_notifier_register(struct notifier_block *nb)
 {
@@ -1047,6 +1068,58 @@ static inline int of_reconfig_get_state_change(unsigned long action,
 {
 	return -EINVAL;
 }
+
+static inline int of_changeset_create_device_node(struct of_changeset *ocs,
+	struct device_node *parent, const char *fmt, ...)
+{
+	return -EINVAL;
+}
+
+int of_changeset_add_property_copy(struct of_changeset *ocs,
+	struct device_node *np, const char *name,
+	const void *value, int length)
+{
+	return -EINVAL;
+}
+
+int of_changeset_add_property_string(struct of_changeset *ocs,
+	struct device_node *np, const char *name, const char *str)
+{
+	return -EINVAL;
+}
+
+static inline struct device_node *of_changeset_create_device_nodev(
+	struct of_changeset *ocs, struct device_node *parent,
+	const char *fmt, va_list vargs)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+static inline __printf(4, 5) struct device_node *
+	of_changeset_add_property_stringf(
+		struct of_changeset *ocs, struct device_node *np,
+		const char *name, const char *fmt, ...)
+{
+	return ERR_PTR(-EINVAL);
+}
+
+int of_changeset_add_property_string_list(struct of_changeset *ocs,
+	struct device_node *np, const char *name, const char **strs, int count)
+{
+	return -EINVAL;
+}
+
+int of_changeset_add_property_u32(struct of_changeset *ocs,
+	struct device_node *np, const char *name, u32 val)
+{
+	return -EINVAL;
+}
+
+int of_changeset_add_property_bool(struct of_changeset *ocs,
+	struct device_node *np, const char *name)
+{
+	return -EINVAL;
+}
 #endif /* CONFIG_OF_DYNAMIC */
 
 /* CONFIG_OF_RESOLVE api */
-- 
1.7.12

Re: [PATCH 8/8] of: changesets: Introduce changeset helper methods

[Geert Uytterhoeven]

Hi Pantelis,

On Fri, Jun 12, 2015 at 9:55 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1028,6 +1028,27 @@ static inline int of_changeset_update_property(struct of_changeset *ocs,
>  {
>         return of_changeset_action(ocs, OF_RECONFIG_UPDATE_PROPERTY, np, prop);
>  }
> +
> +struct device_node *of_changeset_create_device_nodev(
> +       struct of_changeset *ocs, struct device_node *parent,
> +       const char *fmt, va_list vargs);
> +__printf(3, 4) struct device_node *of_changeset_create_device_node(
> +       struct of_changeset *ocs, struct device_node *parent,
> +       const char *fmt, ...);
> +int of_changeset_add_property_copy(struct of_changeset *ocs,
> +       struct device_node *np, const char *name,
> +       const void *value, int length);
> +int of_changeset_add_property_string(struct of_changeset *ocs,
> +       struct device_node *np, const char *name, const char *str);
> +__printf(4, 5) int of_changeset_add_property_stringf(struct of_changeset *ocs,
> +               struct device_node *np, const char *name, const char *fmt, ...);
> +int of_changeset_add_property_string_list(struct of_changeset *ocs,
> +       struct device_node *np, const char *name, const char **strs, int count);
> +int of_changeset_add_property_u32(struct of_changeset *ocs,
> +       struct device_node *np, const char *name, u32 val);
> +int of_changeset_add_property_bool(struct of_changeset *ocs,
> +       struct device_node *np, const char *name);
> +
>  #else /* CONFIG_OF_DYNAMIC */
>  static inline int of_reconfig_notifier_register(struct notifier_block *nb)
>  {
> @@ -1047,6 +1068,58 @@ static inline int of_reconfig_get_state_change(unsigned long action,
>  {
>         return -EINVAL;
>  }
> +
> +static inline int of_changeset_create_device_node(struct of_changeset *ocs,
> +       struct device_node *parent, const char *fmt, ...)
> +{
> +       return -EINVAL;
> +}
> +
> +int of_changeset_add_property_copy(struct of_changeset *ocs,

Missing "static inline".

If CONFIG_OF_DYNAMIC=n, you get zillions of:

    multiple definition of `of_changeset_add_property_copy'
    multiple definition of `of_changeset_add_property_string'

> +       struct device_node *np, const char *name,
> +       const void *value, int length)
> +{
> +       return -EINVAL;
> +}
> +
> +int of_changeset_add_property_string(struct of_changeset *ocs,

Likewise.

> +       struct device_node *np, const char *name, const char *str)
> +{
> +       return -EINVAL;
> +}


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds