| Message ID | 20191003041909.23187-17-amitay@ozlabs.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add system device tree to libpdbg | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (8a10a05c89db666bf98734139334166da7c370a4) |
| snowpatch_ozlabs/build-multiarch | success | Test build-multiarch on branch master |
> + /* Make sure any virtual nodes are also probed */ > + vnode = target_to_virtual(target); > + if (vnode && vnode != target) > + pdbg_target_probe(vnode); Personally I think it might be clearer and less surprising if target_to_virtual/real() returned NULL in cases when they can't find a target of the correct type. That way any future callers are forced to think about what to do in those cases. - Alistair > /* At this point any parents must exist and have already been probed */ > if (target->probe && target->probe(target)) { > /* Could not find the target */ >
On Wed, 2019-10-09 at 15:16 +1100, Alistair Popple wrote: > > + /* Make sure any virtual nodes are also probed */ > > + vnode = target_to_virtual(target); > > + if (vnode && vnode != target) > > + pdbg_target_probe(vnode); > > Personally I think it might be clearer and less surprising if > target_to_virtual/real() returned NULL in cases when they can't find > a target > of the correct type. That way any future callers are forced to think > about > what to do in those cases. > > - Alistair May be I can add a "strict" argument to those functions. In that case, for a target which does not have a mapped virtual target, target_to_virtual(target, true) --> NULL target_to_virtual(target, false) --> target The function will return NULL if it strict is true. And will return the same target. This takes care of both cases. Well there is only a single instance of the second case (strict == true), but in future there may be more. > > > /* At this point any parents must exist and have already been > > probed */ > > if (target->probe && target->probe(target)) { > > /* Could not find the target */ > > > > > Amitay.
diff --git a/libpdbg/target.c b/libpdbg/target.c index a68cb6c..ca2bdb6 100644 --- a/libpdbg/target.c +++ b/libpdbg/target.c @@ -360,7 +360,7 @@ struct pdbg_target_class *get_target_class(struct pdbg_target *target) * exist but don't */ enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target) { - struct pdbg_target *parent; + struct pdbg_target *parent, *vnode; enum pdbg_target_status status; assert(target); @@ -374,7 +374,7 @@ enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target) * it's status won't have changed */ return status; - parent = target->parent; + parent = get_parent(target, false); if (parent) { /* Recurse up the tree to probe and set parent target status */ pdbg_target_probe(parent); @@ -406,6 +406,11 @@ enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target) } } + /* Make sure any virtual nodes are also probed */ + vnode = target_to_virtual(target); + if (vnode && vnode != target) + pdbg_target_probe(vnode); + /* At this point any parents must exist and have already been probed */ if (target->probe && target->probe(target)) { /* Could not find the target */
This ensures that the virtual nodes have correct status in the system device tree view. Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> --- libpdbg/target.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)