From: David Gibson <david@gibson.dropbear.id.au>
To: Ruslan Ruslichenko <ruslichenko.r@gmail.com>
Cc: qemu-devel@nongnu.org, qemu-arm@nongnu.org,
peter.maydell@linaro.org, artem_mygaiev@epam.com,
volodymyr_babchuk@epam.com, takahiro.nakata.wr@renesas.com,
"Edgar E . Iglesias" <edgar.iglesias@gmail.com>,
Ruslan_Ruslichenko@epam.com,
Alistair Francis <alistair.francis@wdc.com>
Subject: Re: [PATCH 02/27] system/device_tree: add few parsing and traversal helpers
Date: Tue, 27 Jan 2026 13:19:19 +1100 [thread overview]
Message-ID: <aXggp66YwLpvM0_4@zatzit> (raw)
In-Reply-To: <20260126174313.1418150-3-ruslichenko.r@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 11889 bytes --]
On Mon, Jan 26, 2026 at 06:42:48PM +0100, Ruslan Ruslichenko wrote:
> From: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
>
> The patch adds few utility functions for parsing FDT nodes.
> The helpers are required for upcoming Hardware device tree
> feature.
>
> Signed-off-by: Ruslan Ruslichenko <Ruslan_Ruslichenko@epam.com>
> ---
> include/system/device_tree.h | 30 ++++++
> system/device_tree.c | 203 +++++++++++++++++++++++++++++++++++
> 2 files changed, 233 insertions(+)
>
> diff --git a/include/system/device_tree.h b/include/system/device_tree.h
> index f34b8b7ef9..458dbb74b4 100644
> --- a/include/system/device_tree.h
> +++ b/include/system/device_tree.h
> @@ -90,6 +90,13 @@ int qemu_fdt_setprop_string_array(void *fdt, const char *node_path,
> int qemu_fdt_setprop_phandle(void *fdt, const char *node_path,
> const char *property,
> const char *target_node_path);
> +
> +uint64_t qemu_fdt_getprop_sized_cell(void *fdt, const char *node_path,
> + const char *property, int offset,
> + int size, Error **errp);
> +const char *qemu_fdt_getprop_string(void *fdt, const char*node_path,
> + const char *property, int cell,
> + bool inherit, Error **errp);
> /**
> * qemu_fdt_getprop: retrieve the value of a given property
> * @fdt: pointer to the device tree blob
> @@ -193,9 +200,32 @@ int qemu_fdt_setprop_sized_cells_from_array(void *fdt,
> qdt_tmp); \
> })
>
> +typedef struct QEMUDevtreeProp {
> + char *name;
> + int len;
> + void *value;
> +} QEMUDevtreeProp;
> +
> +/* node queries */
> +
> +char *qemu_devtree_get_node_name(void *fdt, const char *node_path);
> +int qemu_devtree_get_num_children(void *fdt, const char *node_path, int depth);
> +char **qemu_devtree_get_children(void *fdt, const char *node_path, int depth);
> +int qemu_devtree_num_props(void *fdt, const char *node_path);
> +QEMUDevtreeProp *qemu_devtree_get_props(void *fdt, const char *node_path);
>
> +/* node getters */
> +
> +int qemu_devtree_get_node_by_name(void *fdt, char *node_path,
> + const char *cmpname);
> +int qemu_devtree_get_node_by_phandle(void *fdt, char *node_path, int phandle);
> int qemu_devtree_getparent(void *fdt, char *node_path,
> const char *current);
> +int qemu_devtree_get_root_node(void *fdt, char *node_path);
> +
> +/* misc */
> +
> +int devtree_get_num_nodes(void *fdt);
>
> #define DT_PATH_LENGTH 1024
>
> diff --git a/system/device_tree.c b/system/device_tree.c
> index 41bde0ba5a..7091a4928e 100644
> --- a/system/device_tree.c
> +++ b/system/device_tree.c
> @@ -451,6 +451,50 @@ const void *qemu_fdt_getprop(void *fdt, const char *node_path,
> return r;
> }
>
> +const char *qemu_fdt_getprop_string(void *fdt, const char*node_path,
> + const char *property, int cell,
> + bool inherit, Error **errp)
> +{
> + int len;
> + const void *prop;
> + Error *err = NULL;
> +
> + if (!errp) {
> + errp = &err;
> + }
> +
> + prop = qemu_fdt_getprop(fdt, node_path, property, &len, inherit, errp);
> + if (*errp) {
> + return NULL;
> + }
> + while (cell) {
In DT context, "cell" refers specifically to a 32-bit integer value.
It's not the right word for which string in a stringlist you retrieve.
Also, most of this is re-implementing fdt_stringlist_get() from libfdt.
> + void *term = memchr(prop, '\0', len);
> + size_t diff;
> +
> + if (!term) {
> + error_setg(errp, "%s: Couldn't get %s/%s: %s", __func__,
> + node_path, property, fdt_strerror(len));
> + return NULL;
> + }
> + diff = term - prop + 1;
> + len -= diff;
> + assert(len >= 0);
> + prop += diff;
> + cell--;
> + }
> +
> + if (!len) {
> + return NULL;
> + }
> +
> + if (!*(char *)prop) {
> + error_setg(errp, "%s: Couldn't get %s/%s: %s", __func__,
> + node_path, property, fdt_strerror(len));
> + return NULL;
> + }
> + return prop;
> +}
> +
> uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
> const char *property, int offset,
> bool inherit, Error **errp)
> @@ -471,6 +515,22 @@ uint32_t qemu_fdt_getprop_cell(void *fdt, const char *node_path,
> return be32_to_cpu(p[offset]);
> }
>
> +uint64_t qemu_fdt_getprop_sized_cell(void *fdt, const char *node_path,
> + const char *property, int offset,
> + int size, Error **errp)
> +{
Again, "cell" is 32-bits in DT context, you need a different word.
> + uint64_t ret = 0;
Since ret is a u64, 'size' must be either 1 or 2. So you might as
well just have a get_u64() helper (getprop_cell already does u32).
> + for (; size; size--) {
> + ret <<= 32;
> + ret |= qemu_fdt_getprop_cell(fdt, node_path, property, offset++, false,
> + errp);
> + if (errp && *errp) {
> + return 0;
> + }
> + }
> + return ret;
> +}
> +
> uint32_t qemu_fdt_get_phandle(void *fdt, const char *path)
> {
> uint32_t r;
> @@ -638,6 +698,117 @@ out:
> return ret;
> }
>
> +char *qemu_devtree_get_node_name(void *fdt, const char *node_path)
> +{
> + const char *ret = fdt_get_name(fdt, fdt_path_offset(fdt, node_path), NULL);
This does a full scan of the fdt, which may be unexpected.
> + return ret ? strdup(ret) : NULL;
> +}
> +
> +int qemu_devtree_num_props(void *fdt, const char *node_path)
> +{
> + int offset = fdt_path_offset(fdt, node_path);
> + int ret = 0;
> +
> + for (offset = fdt_first_property_offset(fdt, offset);
> + offset != -FDT_ERR_NOTFOUND;
> + offset = fdt_next_property_offset(fdt, offset)) {
> + ret++;
> + }
> + return ret;
> +}
> +
> +QEMUDevtreeProp *qemu_devtree_get_props(void *fdt, const char *node_path)
> +{
> + QEMUDevtreeProp *ret = g_new0(QEMUDevtreeProp,
> + qemu_devtree_num_props(fdt, node_path) + 1);
> + int offset = fdt_path_offset(fdt, node_path);
> + int i = 0;
> +
> + for (offset = fdt_first_property_offset(fdt, offset);
> + offset != -FDT_ERR_NOTFOUND;
> + offset = fdt_next_property_offset(fdt, offset)) {
> + const char *propname;
> + const void *val = fdt_getprop_by_offset(fdt, offset, &propname,
> + &ret[i].len);
> +
> + ret[i].name = g_strdup(propname);
> + ret[i].value = g_memdup2(val, ret[i].len);
> + i++;
> + }
> + return ret;
How does the caller free the structure? Using g_free() will leak all
the copied names and values.
> +}
> +
> +static void qemu_devtree_children_info(void *fdt, const char *node_path,
> + int depth, int *num, char **returned_paths) {
> + int offset = fdt_path_offset(fdt, node_path);
> + int root_depth = fdt_node_depth(fdt, offset);
"root" implies the root of the whole tree, not of the node where you
started. Also, two full scans of the tree above.
> + int cur_depth = root_depth;
> +
> + if (num) {
> + *num = 0;
> + }
> + for (;;) {
> + offset = fdt_next_node(fdt, offset, &cur_depth);
> + if (cur_depth <= root_depth) {
> + break;
> + }
> + if (cur_depth <= root_depth + depth || depth == 0) {
> + if (returned_paths) {
> + returned_paths[*num] = g_malloc0(DT_PATH_LENGTH);
No test to avoid overrunning the length of the returned_paths[] array
here.
> + fdt_get_path(fdt, offset, returned_paths[*num], DT_PATH_LENGTH);
Another full tree scan for every entry here.
> + }
> + if (num) {
> + (*num)++;
> + }
> + }
> + }
> +}
> +
> +char **qemu_devtree_get_children(void *fdt, const char *node_path, int depth)
> +{
> + int num_children = qemu_devtree_get_num_children(fdt, node_path, depth);
> + char **ret = g_malloc0(sizeof(*ret) * num_children);
> +
> + qemu_devtree_children_info(fdt, node_path, depth, &num_children, ret);
> + return ret;
> +}
> +
> +int qemu_devtree_get_num_children(void *fdt, const char *node_path, int depth)
> +{
> + int ret;
> +
> + qemu_devtree_children_info(fdt, node_path, depth, &ret, NULL);
> + return ret;
> +}
> +
> +int qemu_devtree_get_node_by_name(void *fdt, char *node_path,
> + const char *cmpname) {
The semantics of this make no sense to me. It finds the first node
with the name after the given node, whether it's a child, sibling or
unrelated node. I'm guessing you want something based on
fdt_subnode_offset() instead.
> + int offset = 0;
> + char *name = NULL;
> +
> + do {
> + char *at;
> +
> + offset = fdt_next_node(fdt, offset, NULL);
No test for error here, before using 'offset'.
> + name = (void *)fdt_get_name(fdt, offset, NULL);
> + if (!name) {
> + continue;
The only errors from fdt_get_name() indicate either a corrupted DT, or
bad parameters. continuing at this point will not do anything useful.
> + }
> + at = memchr(name, '@', strlen(name));
> + if (!strncmp(name, cmpname, at ? at - name : strlen(name))) {
> + break;
> + }
> + } while (offset > 0);
> + return offset > 0 ?
> + fdt_get_path(fdt, offset, node_path, DT_PATH_LENGTH) : 1;
> +}
> +
> +int qemu_devtree_get_node_by_phandle(void *fdt, char *node_path, int phandle)
> +{
> + return fdt_get_path(fdt, fdt_node_offset_by_phandle(fdt, phandle),
> + node_path, DT_PATH_LENGTH);
Two full tree scans.
> +}
> +
> int qemu_devtree_getparent(void *fdt, char *node_path, const char *current)
> {
> int offset = fdt_path_offset(fdt, current);
> @@ -648,6 +819,38 @@ int qemu_devtree_getparent(void *fdt, char *node_path, const char *current)
> fdt_get_path(fdt, parent_offset, node_path, DT_PATH_LENGTH) : 1;
> }
>
> +int qemu_devtree_get_root_node(void *fdt, char *node_path)
> +{
> + return fdt_get_path(fdt, 0, node_path, DT_PATH_LENGTH);
The root node's path is "/" by definition, you don't need this
function.
> +}
> +
> +static void devtree_scan(void *fdt, int *num_nodes)
"scan" is vague. The name should reflect what this actually does.
> +{
> + int depth = 0, offset = 0;
> +
> + if (num_nodes) {
> + *num_nodes = 0;
> + }
> + for (;;) {
> + offset = fdt_next_node(fdt, offset, &depth);
> + if (num_nodes) {
> + (*num_nodes)++;
> + }
> + if (offset <= 0 || depth <= 0) {
> + break;
> + }
> + }
> +}
> +
> +int devtree_get_num_nodes(void *fdt)
> +{
> + int ret;
> +
> + devtree_scan(fdt, &ret);
> + return ret;
> +}
> +
> +
> void qmp_dumpdtb(const char *filename, Error **errp)
> {
> ERRP_GUARD();
> --
> 2.43.0
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2026-01-27 2:19 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-26 17:42 [PATCH 00/27] hw/arm: Add generic FDT-based machine and infrastructure Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 01/27] system/device_tree: update qemu_fdt_getprop/_cell Ruslan Ruslichenko
2026-01-27 1:56 ` David Gibson
2026-01-26 17:42 ` [PATCH 02/27] system/device_tree: add few parsing and traversal helpers Ruslan Ruslichenko
2026-01-27 2:19 ` David Gibson [this message]
2026-01-27 21:22 ` Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 03/27] util/log: add log entry for fdt generic utils Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 04/27] hw/core: introduce generic FDT device model registry Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 05/27] hw/core/fdt_generic: implement FDT machine creation helpers Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 06/27] hw/core/fdt_generic: add cpu clusters management Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 07/27] hw/core/fdt_generic_util: implement main fdt parse routine Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 08/27] hw/core/fdt_generic_util: implement fdt_init_qdev Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 09/27] hw/core/fdt_generic_util: initilize qdev properties from fdt Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 10/27] hw/core/fdt_generic_util: actually realize device Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 11/27] hw/core/fdt_generic_util: add TYPE_FDT_GENERIC_MMAP Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 12/27] hw/core/fdt_generic_util: add TYPE_FDT_GENERIC_INTC Ruslan Ruslichenko
2026-01-26 17:42 ` [PATCH 13/27] hw/core/fdt_generic_util: implement fdt_get_irq/_info API Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 14/27] hw/core/fdt_generic_util: map device memory Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 15/27] hw/core/fdt_generic_util: Connect device irqs Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 16/27] hw/core/fdt_generic_util: realize cpu clusters Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 17/27] hw/core: add fdt_generic to the build Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 18/27] hw/core/machine: add '-hw-dtb' option for machine Ruslan Ruslichenko
2026-01-27 8:40 ` Zhao Liu
2026-01-27 20:12 ` Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 19/27] hw/arm: add generic ARM machine initialized by FDT Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 20/27] hw/core/sysbus: implement FDT_GENERIC_MMAP_CLASS interface Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 21/27] hw/intc/arm_gic: implement FDT_GENERIC_INTC and fdt support Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 22/27] target/arm/cpu: add fdt support for armv8-timer Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 23/27] qom/object: export object_resolve_link() Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 24/27] system/memory: add setters for MemoryRegion properties Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 25/27] system/memory: implement FDT_GENERIC_MMAP interface Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 26/27] hw/core/fdt_generic_util: initialize serial devices Ruslan Ruslichenko
2026-01-26 17:43 ` [PATCH 27/27] system/memory: add QOM aliases for fdt support Ruslan Ruslichenko
2026-01-27 10:02 ` [PATCH 00/27] hw/arm: Add generic FDT-based machine and infrastructure Peter Maydell
2026-01-27 14:29 ` BALATON Zoltan
2026-01-27 18:18 ` Ruslan Ruslichenko
2026-01-28 17:48 ` Alex Bennée
2026-01-28 21:41 ` BALATON Zoltan
2026-01-29 12:23 ` Alex Bennée
2026-01-29 15:39 ` Ruslan Ruslichenko
2026-02-06 18:34 ` Alex Bennée
2026-01-29 16:11 ` Edgar E. Iglesias
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aXggp66YwLpvM0_4@zatzit \
--to=david@gibson.dropbear.id.au \
--cc=Ruslan_Ruslichenko@epam.com \
--cc=alistair.francis@wdc.com \
--cc=artem_mygaiev@epam.com \
--cc=edgar.iglesias@gmail.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=ruslichenko.r@gmail.com \
--cc=takahiro.nakata.wr@renesas.com \
--cc=volodymyr_babchuk@epam.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.