[U-Boot] [PATCH v3 1/4] core: Add functions to set properties in live-tree

Implement a set of functions to manipulate properties in a live device tree:
* ofnode_write_prop() to set generic properties of a node * ofnode_write_string() to set string properties of a node * ofnode_set_enabled() to either enable or disable a node
Signed-off-by: Mario Six mario.six@gdsys.cc
---
v2 -> v3: * Removed #ifdef, and used if (!of_live_active()) instead * Removed allocation of property value (caller is now responsible for that); this also fixes a potential memory leak * Added error handling for malloc and strdup * Fixed style violations
v1 -> v2: * Fix potential NULL pointer dereference in ofnode_write_property * Squashed the enable/disable functions into one * Renamed ofnode_set_property to ofnode_write_prop
--- drivers/core/ofnode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/ofnode.h | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+)
diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c index 3cf3205a2f1..e8becf7f04d 100644 --- a/drivers/core/ofnode.c +++ b/drivers/core/ofnode.c @@ -697,3 +697,73 @@ int ofnode_device_is_compatible(ofnode node, const char *compat) ofnode_to_offset(node), compat); } + +int ofnode_write_prop(ofnode node, const char *propname, int len, + const void *value) +{ + const struct device_node *np = ofnode_to_np(node); + struct property *pp; + struct property *pp_last = NULL; + struct property *new; + + if (!of_live_active()) + return -ENOSYS; + + if (!np) + return -EINVAL; + + for (pp = np->properties; pp; pp = pp->next) { + if (strcmp(pp->name, propname) == 0) { + /* Property exists -> change value */ + pp->value = (void *)value; + pp->length = len; + return 0; + } + pp_last = pp; + } + + if (!pp_last) + return -ENOENT; + + /* Property does not exist -> append new property */ + new = malloc(sizeof(struct property)); + if (!new) + return -ENOMEM; + + new->name = strdup(propname); + if (!new->name) + return -ENOMEM; + + new->value = (void *)value; + new->length = len; + new->next = NULL; + + pp_last->next = new; + + return 0; +} + +int ofnode_write_string(ofnode node, const char *propname, const char *value) +{ + if (!of_live_active()) + return -ENOSYS; + + assert(ofnode_valid(node)); + + debug("%s: %s = %s", __func__, propname, value); + + return ofnode_write_prop(node, propname, strlen(value) + 1, value); +} + +int ofnode_set_enabled(ofnode node, bool value) +{ + if (!of_live_active()) + return -ENOSYS; + + assert(ofnode_valid(node)); + + if (value) + return ofnode_write_string(node, "status", "okay"); + else + return ofnode_write_string(node, "status", "disable"); +} diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 5af6b7e616a..1d60858c337 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -691,4 +691,50 @@ u64 ofnode_translate_address(ofnode node, const fdt32_t *in_addr); * @return true if OK, false if the compatible is not found */ int ofnode_device_is_compatible(ofnode node, const char *compat); + +/** + * ofnode_write_prop() - Set a property of a ofnode + * + * Note that the value passed to the function is *not* allocated by the + * function itself, but must be allocated by the caller if necessary. + * + * @node: The node for whose property should be set + * @propname: The name of the property to set + * @len: The length of the new value of the property + * @value: The new value of the property (must be valid prior to calling + * the function) + * @return 0 if successful, -ve on error + */ +int ofnode_write_prop(ofnode node, const char *propname, int len, + const void *value); + +/** + * ofnode_write_string() - Set a string property of a ofnode + * + * Note that the value passed to the function is *not* allocated by the + * function itself, but must be allocated by the caller if necessary. + * + * @node: The node for whose string property should be set + * @propname: The name of the string property to set + * @value: The new value of the string property (must be valid prior to + * calling the function) + * @return 0 if successful, -ve on error + */ +int ofnode_write_string(ofnode node, const char *propname, const char *value); + +/** + * ofnode_set_enabled() - Enable or disable a device tree node given by its + * ofnode + * + * This function effectively sets the node's "status" property to either "okay" + * or "disable", hence making it available for driver model initialization or + * not. + * + * @node: The node to enable + * @value: Flag that tells the function to either disable or enable the + * node + * @return 0 if successful, -ve on error + */ +int ofnode_set_enabled(ofnode node, bool value); + #endif -- 2.11.0

Add tests for the ofnode_set_enabled, ofnode_write_string, and ofnode_write_property functions.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Mario Six mario.six@gdsys.cc
---
v2 -> v3: * Fixed style violation
v1 -> v2: New in v2
--- test/dm/test-fdt.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 8196844e89a..1e3faf15227 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -14,6 +14,8 @@ #include <dm/device-internal.h> #include <dm/uclass-internal.h> #include <dm/util.h> +#include <dm/lists.h> +#include <dm/of_access.h> #include <test/ut.h>
DECLARE_GLOBAL_DATA_PTR; @@ -461,3 +463,55 @@ static int dm_test_fdt_translation(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_fdt_translation, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_fdt_livetree_writing(struct unit_test_state *uts) +{ + struct udevice *dev; + ofnode node; + + if (!of_live_active()) { + printf("Live tree not active; ignore test\n"); + return 0; + } + + /* Test enabling devices */ + + node = ofnode_path("/usb@2"); + + ut_assert(!of_device_is_available(ofnode_to_np(node))); + ofnode_set_enabled(node, true); + ut_assert(of_device_is_available(ofnode_to_np(node))); + + device_bind_driver_to_node(dm_root(), "usb_sandbox", "usb@2", node, + &dev); + ut_assertok(uclass_find_device_by_seq(UCLASS_USB, 2, true, &dev)); + + /* Test string property setting */ + + ut_assert(device_is_compatible(dev, "sandbox,usb")); + ofnode_write_string(node, "compatible", "gdsys,super-usb"); + ut_assert(device_is_compatible(dev, "gdsys,super-usb")); + ofnode_write_string(node, "compatible", "sandbox,usb"); + ut_assert(device_is_compatible(dev, "sandbox,usb")); + + /* Test setting generic properties */ + + /* Non-existent in DTB */ + ut_asserteq(FDT_ADDR_T_NONE, dev_read_addr(dev)); + /* reg = 0x42, size = 0x100 */ + ut_assertok(ofnode_write_prop(node, "reg", 8, + "\x00\x00\x00\x42\x00\x00\x01\x00")); + ut_asserteq(0x42, dev_read_addr(dev)); + + /* Test disabling devices */ + + device_remove(dev, DM_REMOVE_NORMAL); + device_unbind(dev); + + ut_assert(of_device_is_available(ofnode_to_np(node))); + ofnode_set_enabled(node, false); + ut_assert(!of_device_is_available(ofnode_to_np(node))); + + return 0; +} +DM_TEST(dm_test_fdt_livetree_writing, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); -- 2.11.0

On 25 June 2018 at 23:46, Mario Six mario.six@gdsys.cc wrote:
Add tests for the ofnode_set_enabled, ofnode_write_string, and ofnode_write_property functions.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Fixed style violation
v1 -> v2: New in v2
test/dm/test-fdt.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)
Applied to u-boot-dm, and now in mainline, thanks!

We cannot use device structures to disable devices, since getting them with the API functions would bind and activate the device, which would fail if the underlying device does not exist.
---
Hence, add a function to disable devices by path in a live device tree.
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3: * Renamed res to ret * Introduced device_find_by_ofnode() function * Fixed error handling in some places
v1 -> v2: * Simplified np_to_ofnode(of_find_node_by_path(path)) to ofnode_path(path) * Switched to returning -ENOSYS if livetree is not enabled
--- drivers/core/device.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/device.h | 16 +++++++++++ 2 files changed, 94 insertions(+)
diff --git a/drivers/core/device.c b/drivers/core/device.c index e048e1a6595..c0037a2eeaf 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -501,6 +501,33 @@ static int device_get_device_tail(struct udevice *dev, int ret, return 0; }
+/** + * device_find_by_ofnode() - Return device associated with given ofnode + * + * The returned device is *not* activated. + * + * @node: The ofnode for which a associated device should be looked up + * @devp: Pointer to structure to hold the found device + * Return: 0 if OK, -ve on error + */ +static int device_find_by_ofnode(ofnode node, struct udevice **devp) +{ + struct uclass *uc; + struct udevice *dev; + int ret; + + list_for_each_entry(uc, &gd->uclass_root, sibling_node) { + ret = uclass_find_device_by_ofnode(uc->uc_drv->id, node, + &dev); + if (!ret || dev) { + *devp = dev; + return 0; + } + } + + return -ENODEV; +} + int device_get_child(struct udevice *parent, int index, struct udevice **devp) { struct udevice *dev; @@ -717,3 +744,54 @@ bool of_machine_is_compatible(const char *compat)
return !fdt_node_check_compatible(fdt, 0, compat); } + +int dev_disable_by_path(const char *path) +{ + struct uclass *uc; + ofnode node = ofnode_path(path); + struct udevice *dev; + int ret = 1; + + if (!of_live_active()) + return -ENOSYS; + + list_for_each_entry(uc, &gd->uclass_root, sibling_node) { + ret = uclass_find_device_by_ofnode(uc->uc_drv->id, node, &dev); + if (!ret) + break; + } + + if (ret) + return ret; + + ret = device_remove(dev, DM_REMOVE_NORMAL); + if (ret) + return ret; + + ret = device_unbind(dev); + if (ret) + return ret; + + return ofnode_set_enabled(node, false); +} + +int dev_enable_by_path(const char *path) +{ + ofnode node = ofnode_path(path); + ofnode pnode = ofnode_get_parent(node); + struct udevice *parent; + int ret = 1; + + if (!of_live_active()) + return -ENOSYS; + + ret = device_find_by_ofnode(pnode, &parent); + if (ret) + return ret; + + ret = ofnode_set_enabled(node, true); + if (ret) + return ret; + + return lists_bind_fdt(parent, node, NULL); +} diff --git a/include/dm/device.h b/include/dm/device.h index 49078bc6b36..7d569f91979 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -586,6 +586,22 @@ bool device_is_compatible(struct udevice *dev, const char *compat); bool of_machine_is_compatible(const char *compat);
/** + * dev_disable_by_path() - Disable a device given its device tree path + * + * @path: The device tree path identifying the device to be disabled + * @return 0 on success, -ve on error + */ +int dev_disable_by_path(const char *path); + +/** + * dev_enable_by_path() - Enable a device given its device tree path + * + * @path: The device tree path identifying the device to be enabled + * @return 0 on success, -ve on error + */ +int dev_enable_by_path(const char *path); + +/** * device_is_on_pci_bus - Test if a device is on a PCI bus * * @dev: device to test -- 2.11.0

On 26 June 2018 at 00:46, Mario Six mario.six@gdsys.cc wrote:
We cannot use device structures to disable devices, since getting them with the API functions would bind and activate the device, which would fail if the underlying device does not exist.
Hence, add a function to disable devices by path in a live device tree.
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Renamed res to ret
- Introduced device_find_by_ofnode() function
- Fixed error handling in some places
v1 -> v2:
- Simplified np_to_ofnode(of_find_node_by_path(path)) to ofnode_path(path)
- Switched to returning -ENOSYS if livetree is not enabled
drivers/core/device.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/device.h | 16 +++++++++++ 2 files changed, 94 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On 26 June 2018 at 16:18, Simon Glass sjg@chromium.org wrote:
On 26 June 2018 at 00:46, Mario Six mario.six@gdsys.cc wrote:
We cannot use device structures to disable devices, since getting them with the API functions would bind and activate the device, which would fail if the underlying device does not exist.
Hence, add a function to disable devices by path in a live device tree.
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Renamed res to ret
- Introduced device_find_by_ofnode() function
- Fixed error handling in some places
v1 -> v2:
- Simplified np_to_ofnode(of_find_node_by_path(path)) to ofnode_path(path)
- Switched to returning -ENOSYS if livetree is not enabled
drivers/core/device.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/device.h | 16 +++++++++++ 2 files changed, 94 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, and now in mainline, thanks!

Add tests for the dev_{enable,disable}_by_path functions.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Mario Six mario.six@gdsys.cc
---
v2 -> v3: * Fixed style violations
v1 -> v2: New in v2
--- test/dm/test-fdt.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c index 1e3faf15227..dae99ed7e5c 100644 --- a/test/dm/test-fdt.c +++ b/test/dm/test-fdt.c @@ -515,3 +515,31 @@ static int dm_test_fdt_livetree_writing(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_fdt_livetree_writing, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); + +static int dm_test_fdt_disable_enable_by_path(struct unit_test_state *uts) +{ + ofnode node; + + if (!of_live_active()) { + printf("Live tree not active; ignore test\n"); + return 0; + } + + node = ofnode_path("/usb@2"); + + /* Test enabling devices */ + + ut_assert(!of_device_is_available(ofnode_to_np(node))); + dev_enable_by_path("/usb@2"); + ut_assert(of_device_is_available(ofnode_to_np(node))); + + /* Test disabling devices */ + + ut_assert(of_device_is_available(ofnode_to_np(node))); + dev_disable_by_path("/usb@2"); + ut_assert(!of_device_is_available(ofnode_to_np(node))); + + return 0; +} +DM_TEST(dm_test_fdt_disable_enable_by_path, DM_TESTF_SCAN_PDATA | + DM_TESTF_SCAN_FDT); -- 2.11.0

On 25 June 2018 at 23:46, Mario Six mario.six@gdsys.cc wrote:
Add tests for the dev_{enable,disable}_by_path functions.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Fixed style violations
v1 -> v2: New in v2
test/dm/test-fdt.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
Applied to u-boot-dm, and now in mainline, thanks!

On 26 June 2018 at 00:46, Mario Six mario.six@gdsys.cc wrote:
Implement a set of functions to manipulate properties in a live device tree:
- ofnode_write_prop() to set generic properties of a node
- ofnode_write_string() to set string properties of a node
- ofnode_set_enabled() to either enable or disable a node
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Removed #ifdef, and used if (!of_live_active()) instead
- Removed allocation of property value (caller is now responsible for that); this also fixes a potential memory leak
- Added error handling for malloc and strdup
- Fixed style violations
v1 -> v2:
- Fix potential NULL pointer dereference in ofnode_write_property
- Squashed the enable/disable functions into one
- Renamed ofnode_set_property to ofnode_write_prop
drivers/core/ofnode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/ofnode.h | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

Hi Simon, On Wed, Jun 27, 2018 at 1:18 AM Simon Glass sjg@chromium.org wrote:
On 26 June 2018 at 00:46, Mario Six mario.six@gdsys.cc wrote:
Implement a set of functions to manipulate properties in a live device tree:
- ofnode_write_prop() to set generic properties of a node
- ofnode_write_string() to set string properties of a node
- ofnode_set_enabled() to either enable or disable a node
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Removed #ifdef, and used if (!of_live_active()) instead
- Removed allocation of property value (caller is now responsible for that); this also fixes a potential memory leak
- Added error handling for malloc and strdup
- Fixed style violations
v1 -> v2:
- Fix potential NULL pointer dereference in ofnode_write_property
- Squashed the enable/disable functions into one
- Renamed ofnode_set_property to ofnode_write_prop
drivers/core/ofnode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/ofnode.h | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
This series seems to be a candidate for the u-boot-dm tree.
I can assign it to you in Patchwork if you want.
Best regards, Mario

Hi Mario,
On 26 September 2018 at 06:34, Mario Six mario.six@gdsys.cc wrote:
Hi Simon, On Wed, Jun 27, 2018 at 1:18 AM Simon Glass sjg@chromium.org wrote:
On 26 June 2018 at 00:46, Mario Six mario.six@gdsys.cc wrote:
Implement a set of functions to manipulate properties in a live device tree:
- ofnode_write_prop() to set generic properties of a node
- ofnode_write_string() to set string properties of a node
- ofnode_set_enabled() to either enable or disable a node
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Removed #ifdef, and used if (!of_live_active()) instead
- Removed allocation of property value (caller is now responsible for that); this also fixes a potential memory leak
- Added error handling for malloc and strdup
- Fixed style violations
v1 -> v2:
- Fix potential NULL pointer dereference in ofnode_write_property
- Squashed the enable/disable functions into one
- Renamed ofnode_set_property to ofnode_write_prop
drivers/core/ofnode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/ofnode.h | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
This series seems to be a candidate for the u-boot-dm tree.
I can assign it to you in Patchwork if you want.
OK that's fine with me.
Regards, Simon

Hi Simon, On Thu, Sep 27, 2018 at 3:42 PM Simon Glass sjg@chromium.org wrote:
Hi Mario,
On 26 September 2018 at 06:34, Mario Six mario.six@gdsys.cc wrote:
Hi Simon, On Wed, Jun 27, 2018 at 1:18 AM Simon Glass sjg@chromium.org wrote:
On 26 June 2018 at 00:46, Mario Six mario.six@gdsys.cc wrote:
Implement a set of functions to manipulate properties in a live device tree:
- ofnode_write_prop() to set generic properties of a node
- ofnode_write_string() to set string properties of a node
- ofnode_set_enabled() to either enable or disable a node
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Removed #ifdef, and used if (!of_live_active()) instead
- Removed allocation of property value (caller is now responsible for that); this also fixes a potential memory leak
- Added error handling for malloc and strdup
- Fixed style violations
v1 -> v2:
- Fix potential NULL pointer dereference in ofnode_write_property
- Squashed the enable/disable functions into one
- Renamed ofnode_set_property to ofnode_write_prop
drivers/core/ofnode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/ofnode.h | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
This series seems to be a candidate for the u-boot-dm tree.
I can assign it to you in Patchwork if you want.
OK that's fine with me.
Thanks. I reassigned the series.
Regards, Simon
Best regards, Mario

On 27 September 2018 at 06:52, Mario Six mario.six@gdsys.cc wrote:
Hi Simon, On Thu, Sep 27, 2018 at 3:42 PM Simon Glass sjg@chromium.org wrote:
Hi Mario,
On 26 September 2018 at 06:34, Mario Six mario.six@gdsys.cc wrote:
Hi Simon, On Wed, Jun 27, 2018 at 1:18 AM Simon Glass sjg@chromium.org wrote:
On 26 June 2018 at 00:46, Mario Six mario.six@gdsys.cc wrote:
Implement a set of functions to manipulate properties in a live device tree:
- ofnode_write_prop() to set generic properties of a node
- ofnode_write_string() to set string properties of a node
- ofnode_set_enabled() to either enable or disable a node
Signed-off-by: Mario Six mario.six@gdsys.cc
v2 -> v3:
- Removed #ifdef, and used if (!of_live_active()) instead
- Removed allocation of property value (caller is now responsible for that); this also fixes a potential memory leak
- Added error handling for malloc and strdup
- Fixed style violations
v1 -> v2:
- Fix potential NULL pointer dereference in ofnode_write_property
- Squashed the enable/disable functions into one
- Renamed ofnode_set_property to ofnode_write_prop
drivers/core/ofnode.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/dm/ofnode.h | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
This series seems to be a candidate for the u-boot-dm tree.
I can assign it to you in Patchwork if you want.
OK that's fine with me.
Thanks. I reassigned the series.
Applied to u-boot-dm, and now in mainline, thanks!
participants (2)
-
Mario Six
-
Simon Glass