| Message ID | 20190923084841.18057-9-amitay@ozlabs.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add system device tree to libpdbg | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (1a147598c63a3db8e97c654fe1e46ef806515f4d) |
| snowpatch_ozlabs/build-multiarch | success | Test build-multiarch on branch master |
Ok, I think with the comments I was able to convince myself the traversal was correct. I assume a real target is one which has a compatible property right? Reviewed-by: Alistair Popple <alistair@popple.id.au> On Monday, 23 September 2019 6:48:27 PM AEST Amitay Isaacs wrote: > When traversing system device tree view (system == true), the children > are calculated based on the system device tree view (using the virtual > node attachments). > > When traversing backend device tree (system == false), the virtual nodes > are ignored except if they are part of the backend device tree itself. > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > --- > libpdbg/libpdbg.c | 96 ++++++++++++++++++++++++++++++++++++++++++++--- > libpdbg/libpdbg.h | 6 +-- > 2 files changed, 93 insertions(+), 9 deletions(-) > > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c > index aade5d3..1f23208 100644 > --- a/libpdbg/libpdbg.c > +++ b/libpdbg/libpdbg.c > @@ -43,18 +43,102 @@ retry: > } > } > > -struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last) > +static struct pdbg_target *target_map_child(struct pdbg_target *next, bool system) > { > - if (!parent || list_empty(&parent->children)) > + /* > + * Map a target in system tree: > + * > + * - If there is no virtual node assiociated, then return the target > + * (the target itself can be virtual or real) > + * > + * - If there is virtual node associated, > + * - If the target is virtual, return the associated node > + * - If the target is real, return NULL > + * (this target is already covered by previous condition) > + * > + * Map a target in backend tree: > + * > + * - If the target is real, return the target > + * > + * - If the target is virtual, return NULL > + * (no virtual nodes in backend tree) > + */ > + if (system) { > + if (!next->vnode) > + return next; > + else > + if (!next->compatible) > + return next->vnode; > + } else { > + if (next->compatible) > + return next; > + } > + > + return NULL; > +} > + > +struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last, bool system) > +{ > + struct pdbg_target *next, *child = NULL; > + > + if (!parent) > return NULL; > > - if (!last) > - return list_top(&parent->children, struct pdbg_target, list); > + /* > + * Parent node can be virtual or real. > + * > + * If the parent node doesn't have any children, > + * - If there is associated virtual node, > + * Use that node as parent > + * > + * - If there is no associated virtual node, > + * No children > + */ > + if (list_empty(&parent->children)) { > + if (parent->vnode) > + parent = parent->vnode; > + else > + return NULL; > + } > + > + /* > + * If the parent node has children, > + * - Traverse the children > + * (map the children in system or backend tree) > + */ > + if (!last) { > + list_for_each(&parent->children, child, list) > + if ((next = target_map_child(child, system))) > + return next; > > - if (last->list.next == &parent->children.n) > return NULL; > + } > > - return list_entry(last->list.next, struct pdbg_target, list); > + /* > + * In a system tree traversal, virtual targets with associated > + * nodes, get mapped to real targets. > + * > + * When the last child is specified: > + * - If in a system tree traverse, and > + * the last child has associated node, and > + * the last child is real > + * Then the last child is not the actual child of the parent > + * (the associated virtual node is the actual child) > + */ > + if (system && last->vnode && last->compatible) > + last = last->vnode; > + > + if (last == list_tail(&parent->children, struct pdbg_target, list)) > + return NULL; > + > + child = last; > + do { > + child = list_entry(child->list.next, struct pdbg_target, list); > + if ((next = target_map_child(child, system))) > + return next; > + } while (child != list_tail(&parent->children, struct pdbg_target, list)); > + > + return NULL; > } > > enum pdbg_target_status pdbg_target_status(struct pdbg_target *target) > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h > index 2bd3aa0..2ae29cc 100644 > --- a/libpdbg/libpdbg.h > +++ b/libpdbg/libpdbg.h > @@ -20,7 +20,7 @@ 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_child_target(struct pdbg_target *parent, struct pdbg_target *last); > +struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last, bool system); > > /* > * Each target has a status associated with it. This is what each status means: > @@ -71,9 +71,9 @@ enum pdbg_backend { PDBG_DEFAULT_BACKEND = 0, PDBG_BACKEND_FSI, PDBG_BACKEND_I2C > target = __pdbg_next_target(class, NULL, target)) > > #define pdbg_for_each_child_target(parent, target) \ > - for (target = __pdbg_next_child_target(parent, NULL); \ > + for (target = __pdbg_next_child_target(parent, NULL, true); \ > target; \ > - target = __pdbg_next_child_target(parent, target)) > + target = __pdbg_next_child_target(parent, target, true)) > > /* Return the first parent target of the given class, or NULL if the given > * target does not have a parent of the given class. */ >
On Thu, 2019-09-26 at 15:22 +1000, Alistair Popple wrote: > Ok, I think with the comments I was able to convince myself the > traversal was > correct. I assume a real target is one which has a compatible > property right? Correct. > > Reviewed-by: Alistair Popple <alistair@popple.id.au> > > On Monday, 23 September 2019 6:48:27 PM AEST Amitay Isaacs wrote: > > When traversing system device tree view (system == true), the > > children > > are calculated based on the system device tree view (using the > > virtual > > node attachments). > > > > When traversing backend device tree (system == false), the virtual > > nodes > > are ignored except if they are part of the backend device tree > > itself. > > > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > > --- > > libpdbg/libpdbg.c | 96 > > ++++++++++++++++++++++++++++++++++++++++++++--- > > libpdbg/libpdbg.h | 6 +-- > > 2 files changed, 93 insertions(+), 9 deletions(-) > > > > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c > > index aade5d3..1f23208 100644 > > --- a/libpdbg/libpdbg.c > > +++ b/libpdbg/libpdbg.c > > @@ -43,18 +43,102 @@ retry: > > } > > } > > > > -struct pdbg_target *__pdbg_next_child_target(struct pdbg_target > > *parent, > struct pdbg_target *last) > > +static struct pdbg_target *target_map_child(struct pdbg_target > > *next, bool > system) > > { > > - if (!parent || list_empty(&parent->children)) > > + /* > > + * Map a target in system tree: > > + * > > + * - If there is no virtual node assiociated, then return the > > target > > + * (the target itself can be virtual or real) > > + * > > + * - If there is virtual node associated, > > + * - If the target is virtual, return the associated node > > + * - If the target is real, return NULL > > + * (this target is already covered by previous condition) > > + * > > + * Map a target in backend tree: > > + * > > + * - If the target is real, return the target > > + * > > + * - If the target is virtual, return NULL > > + * (no virtual nodes in backend tree) > > + */ > > + if (system) { > > + if (!next->vnode) > > + return next; > > + else > > + if (!next->compatible) > > + return next->vnode; > > + } else { > > + if (next->compatible) > > + return next; > > + } > > + > > + return NULL; > > +} > > + > > +struct pdbg_target *__pdbg_next_child_target(struct pdbg_target > > *parent, > struct pdbg_target *last, bool system) > > +{ > > + struct pdbg_target *next, *child = NULL; > > + > > + if (!parent) > > return NULL; > > > > - if (!last) > > - return list_top(&parent->children, struct pdbg_target, > > list); > > + /* > > + * Parent node can be virtual or real. > > + * > > + * If the parent node doesn't have any children, > > + * - If there is associated virtual node, > > + * Use that node as parent > > + * > > + * - If there is no associated virtual node, > > + * No children > > + */ > > + if (list_empty(&parent->children)) { > > + if (parent->vnode) > > + parent = parent->vnode; > > + else > > + return NULL; > > + } > > + > > + /* > > + * If the parent node has children, > > + * - Traverse the children > > + * (map the children in system or backend tree) > > + */ > > + if (!last) { > > + list_for_each(&parent->children, child, list) > > + if ((next = target_map_child(child, system))) > > + return next; > > > > - if (last->list.next == &parent->children.n) > > return NULL; > > + } > > > > - return list_entry(last->list.next, struct pdbg_target, list); > > + /* > > + * In a system tree traversal, virtual targets with associated > > + * nodes, get mapped to real targets. > > + * > > + * When the last child is specified: > > + * - If in a system tree traverse, and > > + * the last child has associated node, and > > + * the last child is real > > + * Then the last child is not the actual child of the > > parent > > + * (the associated virtual node is the actual child) > > + */ > > + if (system && last->vnode && last->compatible) > > + last = last->vnode; > > + > > + if (last == list_tail(&parent->children, struct pdbg_target, > > list)) > > + return NULL; > > + > > + child = last; > > + do { > > + child = list_entry(child->list.next, struct > > pdbg_target, list); > > + if ((next = target_map_child(child, system))) > > + return next; > > + } while (child != list_tail(&parent->children, struct > > pdbg_target, list)); > > + > > + return NULL; > > } > > > > enum pdbg_target_status pdbg_target_status(struct pdbg_target > > *target) > > diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h > > index 2bd3aa0..2ae29cc 100644 > > --- a/libpdbg/libpdbg.h > > +++ b/libpdbg/libpdbg.h > > @@ -20,7 +20,7 @@ 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_child_target(struct pdbg_target > > *parent, > struct pdbg_target *last); > > +struct pdbg_target *__pdbg_next_child_target(struct pdbg_target > > *parent, > struct pdbg_target *last, bool system); > > > > /* > > * Each target has a status associated with it. This is what each > > status > means: > > @@ -71,9 +71,9 @@ enum pdbg_backend { PDBG_DEFAULT_BACKEND = 0, > PDBG_BACKEND_FSI, PDBG_BACKEND_I2C > > target = __pdbg_next_target(class, NULL, target)) > > > > #define pdbg_for_each_child_target(parent, target) \ > > - for (target = __pdbg_next_child_target(parent, NULL); \ > > + for (target = __pdbg_next_child_target(parent, NULL, true); \ > > target; \ > > - target = __pdbg_next_child_target(parent, target)) > > + target = __pdbg_next_child_target(parent, target, true)) > > > > /* Return the first parent target of the given class, or NULL if > > the given > > * target does not have a parent of the given class. */ > > > > > Amitay.
diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c index aade5d3..1f23208 100644 --- a/libpdbg/libpdbg.c +++ b/libpdbg/libpdbg.c @@ -43,18 +43,102 @@ retry: } } -struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last) +static struct pdbg_target *target_map_child(struct pdbg_target *next, bool system) { - if (!parent || list_empty(&parent->children)) + /* + * Map a target in system tree: + * + * - If there is no virtual node assiociated, then return the target + * (the target itself can be virtual or real) + * + * - If there is virtual node associated, + * - If the target is virtual, return the associated node + * - If the target is real, return NULL + * (this target is already covered by previous condition) + * + * Map a target in backend tree: + * + * - If the target is real, return the target + * + * - If the target is virtual, return NULL + * (no virtual nodes in backend tree) + */ + if (system) { + if (!next->vnode) + return next; + else + if (!next->compatible) + return next->vnode; + } else { + if (next->compatible) + return next; + } + + return NULL; +} + +struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last, bool system) +{ + struct pdbg_target *next, *child = NULL; + + if (!parent) return NULL; - if (!last) - return list_top(&parent->children, struct pdbg_target, list); + /* + * Parent node can be virtual or real. + * + * If the parent node doesn't have any children, + * - If there is associated virtual node, + * Use that node as parent + * + * - If there is no associated virtual node, + * No children + */ + if (list_empty(&parent->children)) { + if (parent->vnode) + parent = parent->vnode; + else + return NULL; + } + + /* + * If the parent node has children, + * - Traverse the children + * (map the children in system or backend tree) + */ + if (!last) { + list_for_each(&parent->children, child, list) + if ((next = target_map_child(child, system))) + return next; - if (last->list.next == &parent->children.n) return NULL; + } - return list_entry(last->list.next, struct pdbg_target, list); + /* + * In a system tree traversal, virtual targets with associated + * nodes, get mapped to real targets. + * + * When the last child is specified: + * - If in a system tree traverse, and + * the last child has associated node, and + * the last child is real + * Then the last child is not the actual child of the parent + * (the associated virtual node is the actual child) + */ + if (system && last->vnode && last->compatible) + last = last->vnode; + + if (last == list_tail(&parent->children, struct pdbg_target, list)) + return NULL; + + child = last; + do { + child = list_entry(child->list.next, struct pdbg_target, list); + if ((next = target_map_child(child, system))) + return next; + } while (child != list_tail(&parent->children, struct pdbg_target, list)); + + return NULL; } enum pdbg_target_status pdbg_target_status(struct pdbg_target *target) diff --git a/libpdbg/libpdbg.h b/libpdbg/libpdbg.h index 2bd3aa0..2ae29cc 100644 --- a/libpdbg/libpdbg.h +++ b/libpdbg/libpdbg.h @@ -20,7 +20,7 @@ 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_child_target(struct pdbg_target *parent, struct pdbg_target *last); +struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last, bool system); /* * Each target has a status associated with it. This is what each status means: @@ -71,9 +71,9 @@ enum pdbg_backend { PDBG_DEFAULT_BACKEND = 0, PDBG_BACKEND_FSI, PDBG_BACKEND_I2C target = __pdbg_next_target(class, NULL, target)) #define pdbg_for_each_child_target(parent, target) \ - for (target = __pdbg_next_child_target(parent, NULL); \ + for (target = __pdbg_next_child_target(parent, NULL, true); \ target; \ - target = __pdbg_next_child_target(parent, target)) + target = __pdbg_next_child_target(parent, target, true)) /* Return the first parent target of the given class, or NULL if the given * target does not have a parent of the given class. */
When traversing system device tree view (system == true), the children are calculated based on the system device tree view (using the virtual node attachments). When traversing backend device tree (system == false), the virtual nodes are ignored except if they are part of the backend device tree itself. Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> --- libpdbg/libpdbg.c | 96 ++++++++++++++++++++++++++++++++++++++++++++--- libpdbg/libpdbg.h | 6 +-- 2 files changed, 93 insertions(+), 9 deletions(-)