[Pdbg] [PATCH v2 09/22] libpdbg: Parent retrieval should handle virtual nodes correctly
Alistair Popple
alistair at popple.id.au
Mon Sep 23 13:23:14 AEST 2019
On Friday, 20 September 2019 3:16:38 PM AEST Amitay Isaacs wrote:
> When traversing system device tree view (system == true), parent of a
> node can be a virtual node.
>
> When traversing backend device tree, parent of a node will not be a
> virtual node unless it's part of the backend device tree itself.
>
> Signed-off-by: Amitay Isaacs <amitay at ozlabs.org>
> ---
> libpdbg/libpdbg.c | 17 +++++++++++------
> libpdbg/libpdbg.h | 10 +++++-----
> libpdbg/target.c | 27 ++++++++++++++++++++++++++-
> libpdbg/target.h | 1 +
> src/thread.c | 4 ++--
> 5 files changed, 45 insertions(+), 14 deletions(-)
>
> diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c
> index 7029647..db0a564 100644
> --- a/libpdbg/libpdbg.c
> +++ b/libpdbg/libpdbg.c
> @@ -6,7 +6,7 @@
>
> static pdbg_progress_tick_t progress_tick;
>
> -struct pdbg_target *__pdbg_next_target(const char *class, struct
pdbg_target *parent, struct pdbg_target *last)
> +struct pdbg_target *__pdbg_next_target(const char *class, struct
pdbg_target *parent, struct pdbg_target *last, bool system)
> {
> struct pdbg_target *next, *tmp;
> struct pdbg_target_class *target_class;
> @@ -33,7 +33,7 @@ retry:
> return next;
Do we need to add any filtering with something like pdbg_next_target_match()
here? Or are we ok returning everything for a particular target class
regardless of which tree it falls under?
> else {
> /* Check if this target is a child of the given parent */
> - for (tmp = next; tmp && next->parent && tmp != parent; tmp = tmp-
>parent) {}
> + for (tmp = next; tmp && get_parent(tmp, system) && tmp != parent; tmp
= get_parent(tmp, system)) {}
> if (tmp == parent)
> return next;
> else {
> @@ -116,7 +116,7 @@ uint32_t pdbg_target_index(struct pdbg_target *target)
> {
> struct pdbg_target *dn;
>
> - for (dn = target; dn && dn->index == -1; dn = dn->parent);
> + for (dn = target; dn && dn->index == -1; dn = get_parent(dn, true));
>
> if (!dn)
> return -1;
> @@ -130,10 +130,15 @@ struct pdbg_target *pdbg_target_parent(const char
*class, struct pdbg_target *ta
> struct pdbg_target *parent;
>
> if (!class)
> - return target->parent;
> + return get_parent(target, true);
>
> - for (parent = target->parent; parent && parent->parent; parent = parent-
>parent) {
> - if (!strcmp(class, pdbg_target_class_name(parent)))
> + for (parent = get_parent(target, true); parent && get_parent(parent,
true); parent = get_parent(parent, true)) {
> + const char *tclass = pdbg_target_class_name(parent);
> +
> + if (!tclass)
> + continue;
> +
> + if (!strcmp(class, tclass))
> return parent;
> }
>
> diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h
> index 2ae29cc..de3fc17 100644
> --- a/libpdbg/libpdbg.h
> +++ b/libpdbg/libpdbg.h
> @@ -19,7 +19,7 @@ struct pdbg_target_class;
> struct pdbg_target *__pdbg_next_compatible_node(struct pdbg_target *root,
> struct pdbg_target *prev,
> const char *compat);
> -struct pdbg_target *__pdbg_next_target(const char *klass, struct
pdbg_target *parent, struct pdbg_target *last);
> +struct pdbg_target *__pdbg_next_target(const char *klass, struct
pdbg_target *parent, struct pdbg_target *last, bool system);
> struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent,
struct pdbg_target *last, bool system);
>
> /*
> @@ -61,14 +61,14 @@ enum pdbg_backend { PDBG_DEFAULT_BACKEND = 0,
PDBG_BACKEND_FSI, PDBG_BACKEND_I2C
> (target = __pdbg_next_compatible_node(parent, target, compat))
!= NULL;)
>
> #define pdbg_for_each_target(class, parent, target) \
> - for (target = __pdbg_next_target(class, parent, NULL); \
> + for (target = __pdbg_next_target(class, parent, NULL, true); \
> target; \
> - target = __pdbg_next_target(class, parent, target))
> + target = __pdbg_next_target(class, parent, target, true))
>
> #define pdbg_for_each_class_target(class, target) \
> - for (target = __pdbg_next_target(class, NULL, NULL); \
> + for (target = __pdbg_next_target(class, NULL, NULL, true); \
> target; \
> - target = __pdbg_next_target(class, NULL, target))
> + target = __pdbg_next_target(class, NULL, target, true))
>
> #define pdbg_for_each_child_target(parent, target) \
> for (target = __pdbg_next_child_target(parent, NULL, true); \
> diff --git a/libpdbg/target.c b/libpdbg/target.c
> index da24ba0..667e55d 100644
> --- a/libpdbg/target.c
> +++ b/libpdbg/target.c
> @@ -27,7 +27,7 @@ static struct pdbg_target *get_class_target_addr(struct
pdbg_target *target, con
> *addr += pdbg_target_address(target, NULL);
>
> /* Keep walking the tree translating addresses */
> - target = target->parent;
> + target = get_parent(target, false);
>
> /* The root node doesn't have an address space so it's
> * an error in the device tree if we hit this. */
> @@ -312,6 +312,31 @@ uint32_t sbe_ffdc_get(struct pdbg_target *target, const
uint8_t **ffdc, uint32_t
> return sbefifo->ffdc_get(sbefifo, ffdc, ffdc_len);
> }
>
> +struct pdbg_target *get_parent(struct pdbg_target *target, bool system)
> +{
> + struct pdbg_target *parent;
> +
> + if (!target)
> + return NULL;
> +
> + if (system) {
> + if (target->compatible && target->vnode)
> + target = target->vnode;
> + } else {
> + if (!target->compatible && target->vnode)
> + target = target->vnode;
> + }
Still not sure I completely follow the reasoning behind this logic so it would
be good if we could get some comments describing the rules/assumptions for
each node type.
- Alistair
> + parent = target->parent;
> + if (!parent || !parent->vnode)
> + return parent;
> +
> + if (parent->compatible)
> + return parent;
> +
> + return parent->vnode;
> +}
> +
> struct pdbg_target *require_target_parent(struct pdbg_target *target)
> {
> assert(target->parent);
> diff --git a/libpdbg/target.h b/libpdbg/target.h
> index 7e08ac3..7419ce5 100644
> --- a/libpdbg/target.h
> +++ b/libpdbg/target.h
> @@ -53,6 +53,7 @@ struct pdbg_target {
> struct pdbg_target *vnode;
> };
>
> +struct pdbg_target *get_parent(struct pdbg_target *target, bool system);
> struct pdbg_target *require_target_parent(struct pdbg_target *target);
> struct pdbg_target_class *find_target_class(const char *name);
> struct pdbg_target_class *require_target_class(const char *name);
> diff --git a/src/thread.c b/src/thread.c
> index 663f290..8ddf4ae 100644
> --- a/src/thread.c
> +++ b/src/thread.c
> @@ -277,10 +277,10 @@ static int thread_status_print(void)
> assert(path_target_add(pib));
> }
>
> - pib = __pdbg_next_target("pib", pdbg_target_root(), NULL);
> + pib = __pdbg_next_target("pib", pdbg_target_root(), NULL, true);
> assert(pib);
>
> - core = __pdbg_next_target("core", pib, NULL);
> + core = __pdbg_next_target("core", pib, NULL, true);
> assert(core);
>
> pdbg_for_each_target("thread", core, thread)
>
More information about the Pdbg
mailing list