| Message ID | 20190903075934.3525-1-alistair@popple.id.au |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] libpdbg: Create struct pdbg_target_priv | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (e005e0863a44f04c931b09804f258fb1c0b6f14c) |
| snowpatch_ozlabs/build-multiarch | success | Test build-multiarch on branch master |
On Tue, 2019-09-03 at 17:59 +1000, Alistair Popple wrote: > In order to allow external programs to register their own hardware > units it is required to expose the definition of struct > pdbg_target. However this means the struct definition now becomes > part > of the library ABI/API, making it difficult to change fields without > breaking applications. > > It also means we would have to install the ccan list headers, which > themselves depend on other ccan modules. Instead create a seperate > struct definition (struct pdbg_target_priv) which encapsulates the > libpdbg specific fields which should not be accessed directly by > applications and switch the library to use those. > > This should allow a stable definition of struct pdbg_target to be > exported to applications without preventing changes to the underlying > library fields. I agree with the idea, but I have few reservations about naming. Even though internally libpdbg uses pdbg_target to store part of hardware unit, exposing pdbg_target as a hardware unit is a bad idea. From libpdbg api it's clear that pdbg_target represents an instance of a hardware unit. So overloading pdbg_target as a hardware unit adds more confusion. It would make more sense (to me at least) if we call new structure pdbg_hwunit (or similar but different from pdbg_target). This will make the libpdbg api cleaner and less confusing. (What we really need is virtual base class and every hardware unit is derived from that. Need a good way to do that in C.) Also, let's not add any structures to libpdbg API without pdbg_ prefix (e.g. hw_unit_info). I guess this will involve a bit of churn through libpdbg, but in the long run it will be better. Amitay. > > Signed-off-by: Alistair Popple <alistair@popple.id.au> > --- > libpdbg/device.c | 95 +++++++++++++++++++++++++++---------------- > ---- > libpdbg/hwunit.c | 3 ++ > libpdbg/libpdbg.c | 30 +++++++++------ > libpdbg/target.c | 12 +++--- > libpdbg/target.h | 8 +++- > 5 files changed, 89 insertions(+), 59 deletions(-) > > diff --git a/libpdbg/device.c b/libpdbg/device.c > index efa9ce4..e009a1a 100644 > --- a/libpdbg/device.c > +++ b/libpdbg/device.c > @@ -67,6 +67,7 @@ static const char *take_name(const char *name) > /* Adds information representing an actual target */ > static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int > node_offset) > { > + struct pdbg_target_priv *target_priv; > struct pdbg_target *target; > struct pdbg_target_class *target_class; > const struct hw_unit_info *hw_info = NULL; > @@ -98,19 +99,21 @@ static struct pdbg_target > *dt_pdbg_target_new(const void *fdt, int node_offset) > /* Couldn't find anything implementing this target */ > return NULL; > > - target = calloc(1, size); > - if (!target) { > + target_priv = calloc(1, sizeof(struct pdbg_target_priv) - > sizeof(struct pdbg_target) + size); > + if (!target_priv) { > prerror("Failed to allocate node\n"); > abort(); > } > > + target = &target_priv->target; > + > /* hw_info->hw_unit points to a per-target struct type. This > * works because the first member in the per-target struct is > * guaranteed to be the struct pdbg_target (see the comment > * above DECLARE_HW_UNIT). */ > memcpy(target, hw_info->hw_unit, size); > target_class = get_target_class(target); > - list_add_tail(&target_class->targets, &target->class_link); > + list_add_tail(&target_class->targets, &target_priv- > >class_link); > > return target; > } > @@ -118,31 +121,36 @@ static struct pdbg_target > *dt_pdbg_target_new(const void *fdt, int node_offset) > static struct pdbg_target *dt_new_node(const char *name, const void > *fdt, int node_offset) > { > struct pdbg_target *node = NULL; > - size_t size = sizeof(*node); > + struct pdbg_target_priv *target_priv; > > - if (fdt) > + if (fdt) { > node = dt_pdbg_target_new(fdt, node_offset); > - > - if (!node) > - node = calloc(1, size); > + target_priv = pdbg_target_to_priv(node); > + } > > if (!node) { > + target_priv = calloc(1, sizeof(*target_priv)); > + node = &target_priv->target; > + } > + > + if (!node && !target_priv) { > prerror("Failed to allocate node\n"); > abort(); > } > > - node->dn_name = take_name(name); > + target_priv->dn_name = take_name(name); > node->parent = NULL; > - list_head_init(&node->properties); > - list_head_init(&node->children); > - node->phandle = ++last_phandle; > + list_head_init(&target_priv->properties); > + list_head_init(&target_priv->children); > + target_priv->phandle = ++last_phandle; > > return node; > } > > static const char *get_unitname(const struct pdbg_target *node) > { > - const char *c = strchr(node->dn_name, '@'); > + struct pdbg_target_priv *target_priv = > pdbg_target_to_priv(node); > + const char *c = strchr(target_priv->dn_name, '@'); > > if (!c) > return NULL; > @@ -152,13 +160,14 @@ static const char *get_unitname(const struct > pdbg_target *node) > > static int dt_cmp_subnodes(const struct pdbg_target *a, const struct > pdbg_target *b) > { > + struct pdbg_target_priv *priv_a = pdbg_target_to_priv(a), > *priv_b = pdbg_target_to_priv(b); > const char *a_unit = get_unitname(a); > const char *b_unit = get_unitname(b); > > - ptrdiff_t basenamelen = a_unit - a->dn_name; > + ptrdiff_t basenamelen = a_unit - priv_a->dn_name; > > /* sort hex unit addresses by number */ > - if (a_unit && b_unit && !strncmp(a->dn_name, b->dn_name, > basenamelen)) { > + if (a_unit && b_unit && !strncmp(priv_a->dn_name, priv_b- > >dn_name, basenamelen)) { > unsigned long long a_num, b_num; > char *a_end, *b_end; > > @@ -170,29 +179,31 @@ static int dt_cmp_subnodes(const struct > pdbg_target *a, const struct pdbg_target > return (a_num > b_num) - (a_num < b_num); > } > > - return strcmp(a->dn_name, b->dn_name); > + return strcmp(priv_a->dn_name, priv_b->dn_name); > } > > static bool dt_attach_root(struct pdbg_target *parent, struct > pdbg_target *root) > { > - struct pdbg_target *node = NULL; > + struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root); > + struct pdbg_target_priv *parent_priv = > pdbg_target_to_priv(parent); > + struct pdbg_target_priv *node = NULL; > > assert(!root->parent); > > - if (list_empty(&parent->children)) { > - list_add(&parent->children, &root->list); > + if (list_empty(&parent_priv->children)) { > + list_add(&parent_priv->children, &root_priv->list); > root->parent = parent; > > return true; > } > > - dt_for_each_child(parent, node) { > - int cmp = dt_cmp_subnodes(node, root); > + dt_for_each_child(parent_priv, node) { > + int cmp = dt_cmp_subnodes(&node->target, root); > > /* Look for duplicates */ > if (cmp == 0) { > prerror("DT: %s failed, duplicate %s\n", > - __func__, root->dn_name); > + __func__, root_priv->dn_name); > return false; > } > > @@ -202,7 +213,7 @@ static bool dt_attach_root(struct pdbg_target > *parent, struct pdbg_target *root) > break; > } > > - list_add_before(&parent->children, &root->list, &node->list); > + list_add_before(&parent_priv->children, &root_priv->list, > &node->list); > root->parent = parent; > > return true; > @@ -219,7 +230,7 @@ static char *dt_get_path(const struct pdbg_target > *node) > return strdup("<NULL>"); > > for (n = node; n; n = n->parent) { > - len += strlen(n->dn_name); > + len += strlen(pdbg_target_to_priv(n)->dn_name); > if (n->parent || n == node) > len++; > } > @@ -227,9 +238,9 @@ static char *dt_get_path(const struct pdbg_target > *node) > assert(path); > p = path + len; > for (n = node; n; n = n->parent) { > - len = strlen(n->dn_name); > + len = strlen(pdbg_target_to_priv(n)->dn_name); > p -= len; > - memcpy(p, n->dn_name, len); > + memcpy(p, pdbg_target_to_priv(n)->dn_name, len); > if (n->parent || n == node) > *(--p) = '/'; > } > @@ -272,7 +283,8 @@ static const char *__dt_path_split(const char *p, > > static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, > const char *path) > { > - struct pdbg_target *n; > + struct pdbg_target_priv *n; > + struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root); > const char *pn, *pa = NULL, *p = path, *nn = NULL, *na = NULL; > unsigned int pnl, pal, nnl, nal; > bool match; > @@ -286,7 +298,7 @@ static struct pdbg_target *dt_find_by_path(struct > pdbg_target *root, const char > > /* Compare with each child node */ > match = false; > - list_for_each(&root->children, n, list) { > + list_for_each(&root_priv->children, n, list) { > match = true; > __dt_path_split(n->dn_name, &nn, &nnl, &na, > &nal); > if (pnl && (pnl != nnl || strncmp(pn, nn, > pnl))) > @@ -294,7 +306,7 @@ static struct pdbg_target *dt_find_by_path(struct > pdbg_target *root, const char > if (pal && (pal != nal || strncmp(pa, na, > pal))) > match = false; > if (match) { > - root = n; > + root = &n->target; > break; > } > } > @@ -311,7 +323,7 @@ static struct dt_property *dt_find_property(const > struct pdbg_target *node, > { > struct dt_property *i = NULL; > > - list_for_each(&node->properties, i, list) > + list_for_each(&pdbg_target_to_priv(node)->properties, i, list) > if (strcmp(i->name, name) == 0) > return i; > return NULL; > @@ -341,7 +353,7 @@ static struct dt_property *new_property(struct > pdbg_target *node, > > p->name = take_name(name); > p->len = size; > - list_add_tail(&node->properties, &p->list); > + list_add_tail(&pdbg_target_to_priv(node)->properties, &p- > >list); > return p; > } > > @@ -349,6 +361,7 @@ static struct dt_property *dt_add_property(struct > pdbg_target *node, > const char *name, > const void *val, size_t size) > { > + struct pdbg_target_priv *node_priv = pdbg_target_to_priv(node); > struct dt_property *p; > > /* > @@ -358,9 +371,9 @@ static struct dt_property *dt_add_property(struct > pdbg_target *node, > if (strcmp(name, "linux,phandle") == 0 || > strcmp(name, "phandle") == 0) { > assert(size == 4); > - node->phandle = *(const u32 *)val; > - if (node->phandle >= last_phandle) > - last_phandle = node->phandle; > + node_priv->phandle = *(const u32 *)val; > + if (node_priv->phandle >= last_phandle) > + last_phandle = node_priv->phandle; > return NULL; > } > > @@ -422,21 +435,23 @@ static u32 dt_property_get_cell(const struct > dt_property *prop, u32 index) > /* First child of this node. */ > static struct pdbg_target *dt_first(const struct pdbg_target *root) > { > - return list_top(&root->children, struct pdbg_target, list); > + return &list_top(&pdbg_target_to_priv(root)->children, struct > pdbg_target_priv, list)->target; > } > > /* Return next node, or NULL. */ > static struct pdbg_target *dt_next(const struct pdbg_target *root, > const struct pdbg_target *prev) > { > + struct pdbg_target_priv *prev_priv = pdbg_target_to_priv(prev); > + > /* Children? */ > - if (!list_empty(&prev->children)) > + if (!list_empty(&prev_priv->children)) > return dt_first(prev); > > do { > /* More siblings? */ > - if (prev->list.next != &prev->parent->children.n) > - return list_entry(prev->list.next, struct > pdbg_target,list); > + if (prev_priv->list.next != &pdbg_target_to_priv(prev- > >parent)->children.n) > + return &list_entry(prev_priv->list.next, struct > pdbg_target_priv, list)->target; > > /* No more siblings, move up to parent. */ > prev = prev->parent; > @@ -563,11 +578,11 @@ static int dt_expand_node(struct pdbg_target > *node, const void *fdt, int fdt_nod > name = fdt_string(fdt, fdt32_to_cpu(prop- > >nameoff)); > if (strcmp("index", name) == 0) { > memcpy(&data, prop->data, > sizeof(data)); > - node->index = fdt32_to_cpu(data); > + pdbg_target_to_priv(node)->index = > fdt32_to_cpu(data); > } > > if (strcmp("status", name) == 0) > - node->status = str_to_status(prop- > >data); > + pdbg_target_to_priv(node)->status = > str_to_status(prop->data); > > dt_add_property(node, name, prop->data, > fdt32_to_cpu(prop->len)); > diff --git a/libpdbg/hwunit.c b/libpdbg/hwunit.c > index c7ec63d..5c40d23 100644 > --- a/libpdbg/hwunit.c > +++ b/libpdbg/hwunit.c > @@ -21,6 +21,9 @@ > > #define MAX_HW_UNITS 1024 > > +struct pdbg_target pdbg_target_size; > +uint8_t pdbg_target_size_test[sizeof(struct pdbg_target)]; > + > static const struct hw_unit_info *g_hw_unit[MAX_HW_UNITS]; > static int g_hw_unit_count; > > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c > index 0c5a451..bec2488 100644 > --- a/libpdbg/libpdbg.c > +++ b/libpdbg/libpdbg.c > @@ -9,6 +9,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 *next, *tmp; > + struct pdbg_target_priv *last_priv; > struct pdbg_target_class *target_class; > > if (class && !find_target_class(class)) > @@ -17,15 +18,17 @@ struct pdbg_target *__pdbg_next_target(const char > *class, struct pdbg_target *pa > target_class = require_target_class(class); > > retry: > + last_priv = pdbg_target_to_priv(last); > + > /* No more targets left to check in this class */ > - if ((last && last->class_link.next == &target_class->targets.n) > || > + if ((last && last_priv->class_link.next == &target_class- > >targets.n) || > list_empty(&target_class->targets)) > return NULL; > > if (last) > - next = list_entry(last->class_link.next, struct > pdbg_target, class_link); > + next = &list_entry(last_priv->class_link.next, struct > pdbg_target_priv, class_link)->target; > else > - if (!(next = list_top(&target_class->targets, struct > pdbg_target, class_link))) > + if (!(next = &list_top(&target_class->targets, struct > pdbg_target_priv, class_link)->target)) > return NULL; > > if (!parent) > @@ -45,21 +48,24 @@ retry: > > struct pdbg_target *__pdbg_next_child_target(struct pdbg_target > *parent, struct pdbg_target *last) > { > - if (!parent || list_empty(&parent->children)) > + struct pdbg_target_priv *parent_priv = > pdbg_target_to_priv(parent); > + struct pdbg_target_priv *last_priv = pdbg_target_to_priv(last); > + > + if (!parent || list_empty(&parent_priv->children)) > return NULL; > > if (!last) > - return list_top(&parent->children, struct pdbg_target, > list); > + return &list_top(&parent_priv->children, struct > pdbg_target_priv, list)->target; > > - if (last->list.next == &parent->children.n) > + if (last_priv->list.next == &parent_priv->children.n) > return NULL; > > - return list_entry(last->list.next, struct pdbg_target, list); > + return &list_entry(last_priv->list.next, struct > pdbg_target_priv, list)->target; > } > > enum pdbg_target_status pdbg_target_status(struct pdbg_target > *target) > { > - return target->status; > + return pdbg_target_to_priv(target)->status; > } > > void pdbg_target_status_set(struct pdbg_target *target, enum > pdbg_target_status status) > @@ -68,7 +74,7 @@ void pdbg_target_status_set(struct pdbg_target > *target, enum pdbg_target_status > * blow up obviously if this happens */ > assert(status == PDBG_TARGET_DISABLED || status == > PDBG_TARGET_MUSTEXIST); > > - target->status = status; > + pdbg_target_to_priv(target)->status = status; > } > > /* Searches up the tree and returns the first valid index found */ > @@ -76,12 +82,12 @@ 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 && pdbg_target_to_priv(dn)->index == -1; > dn = dn->parent); > > if (!dn) > return -1; > else > - return dn->index; > + return pdbg_target_to_priv(dn)->index; > } > > /* Find a target parent from the given class */ > @@ -132,7 +138,7 @@ char *pdbg_target_name(struct pdbg_target > *target) > > const char *pdbg_target_dn_name(struct pdbg_target *target) > { > - return target->dn_name; > + return pdbg_target_to_priv(target)->dn_name; > } > > int pdbg_target_u32_property(struct pdbg_target *target, const char > *name, uint32_t *val) > diff --git a/libpdbg/target.c b/libpdbg/target.c > index 5808d02..ce2bb2d 100644 > --- a/libpdbg/target.c > +++ b/libpdbg/target.c > @@ -400,7 +400,7 @@ enum pdbg_target_status pdbg_target_probe(struct > pdbg_target *target) > case PDBG_TARGET_NONEXISTENT: > /* The parent doesn't exist neither does it's > * children */ > - target->status = PDBG_TARGET_NONEXISTENT; > + pdbg_target_to_priv(target)->status = > PDBG_TARGET_NONEXISTENT; > return PDBG_TARGET_NONEXISTENT; > > case PDBG_TARGET_DISABLED: > @@ -427,11 +427,11 @@ enum pdbg_target_status > pdbg_target_probe(struct pdbg_target *target) > if (target->probe && target->probe(target)) { > /* Could not find the target */ > assert(pdbg_target_status(target) != > PDBG_TARGET_MUSTEXIST); > - target->status = PDBG_TARGET_NONEXISTENT; > + pdbg_target_to_priv(target)->status = > PDBG_TARGET_NONEXISTENT; > return PDBG_TARGET_NONEXISTENT; > } > > - target->status = PDBG_TARGET_ENABLED; > + pdbg_target_to_priv(target)->status = PDBG_TARGET_ENABLED; > return PDBG_TARGET_ENABLED; > } > > @@ -449,7 +449,7 @@ void pdbg_target_release(struct pdbg_target > *target) > /* Release the target */ > if (target->release) > target->release(target); > - target->status = PDBG_TARGET_RELEASED; > + pdbg_target_to_priv(target)->status = PDBG_TARGET_RELEASED; > } > > /* > @@ -477,10 +477,10 @@ bool pdbg_target_is_class(struct pdbg_target > *target, const char *class) > > void *pdbg_target_priv(struct pdbg_target *target) > { > - return target->priv; > + return pdbg_target_to_priv(target)->priv; > } > > void pdbg_target_priv_set(struct pdbg_target *target, void *priv) > { > - target->priv = priv; > + pdbg_target_to_priv(target)->priv = priv; > } > diff --git a/libpdbg/target.h b/libpdbg/target.h > index 2ecbf98..ba2b20d 100644 > --- a/libpdbg/target.h > +++ b/libpdbg/target.h > @@ -39,19 +39,25 @@ struct pdbg_target { > int (*probe)(struct pdbg_target *target); > void (*release)(struct pdbg_target *target); > uint64_t (*translate)(struct pdbg_target *target, uint64_t > addr); > + struct pdbg_target *parent; > +}; > + > +struct pdbg_target_priv { > int index; > enum pdbg_target_status status; > const char *dn_name; > struct list_node list; > struct list_head properties; > struct list_head children; > - struct pdbg_target *parent; > u32 phandle; > bool probed; > struct list_node class_link; > + struct pdbg_target target; > void *priv; > }; > > +#define pdbg_target_to_priv(x) (x ? container_of(x, struct > pdbg_target_priv, target) : NULL) > + > 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); > -- > 2.20.1 > Amitay.
Amitay, > Even though internally libpdbg uses pdbg_target to store part of > hardware unit, exposing pdbg_target as a hardware unit is a bad idea. > From libpdbg api it's clear that pdbg_target represents an instance of > a hardware unit. So overloading pdbg_target as a hardware unit adds > more confusion. > > It would make more sense (to me at least) if we call new structure > pdbg_hwunit (or similar but different from pdbg_target). This will > make the libpdbg api cleaner and less confusing. (What we really need > is virtual base class and every hardware unit is derived from that. > Need a good way to do that in C.) Ok, it took me a moment to work out what the issue was and what you were suggesting but now that I've figured it out I agree - it would be much clearer/ cleaner to differentiate between the two. Will do a v2 that fixes it. Next up, converting to C++ (-: > Also, let's not add any structures to libpdbg API without pdbg_ prefix > (e.g. hw_unit_info). I guess this will involve a bit of churn through > libpdbg, but in the long run it will be better. Thanks, that is also my intent but I missed this one. As they all (should) only get created by a macro I doubt it will involve too much churn. - Alistair > Amitay. > > > > > Signed-off-by: Alistair Popple <alistair@popple.id.au> > > --- > > libpdbg/device.c | 95 +++++++++++++++++++++++++++---------------- > > ---- > > libpdbg/hwunit.c | 3 ++ > > libpdbg/libpdbg.c | 30 +++++++++------ > > libpdbg/target.c | 12 +++--- > > libpdbg/target.h | 8 +++- > > 5 files changed, 89 insertions(+), 59 deletions(-) > > > > diff --git a/libpdbg/device.c b/libpdbg/device.c > > index efa9ce4..e009a1a 100644 > > --- a/libpdbg/device.c > > +++ b/libpdbg/device.c > > @@ -67,6 +67,7 @@ static const char *take_name(const char *name) > > /* Adds information representing an actual target */ > > static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int > > node_offset) > > { > > + struct pdbg_target_priv *target_priv; > > struct pdbg_target *target; > > struct pdbg_target_class *target_class; > > const struct hw_unit_info *hw_info = NULL; > > @@ -98,19 +99,21 @@ static struct pdbg_target > > *dt_pdbg_target_new(const void *fdt, int node_offset) > > /* Couldn't find anything implementing this target */ > > return NULL; > > > > - target = calloc(1, size); > > - if (!target) { > > + target_priv = calloc(1, sizeof(struct pdbg_target_priv) - > > sizeof(struct pdbg_target) + size); > > + if (!target_priv) { > > prerror("Failed to allocate node\n"); > > abort(); > > } > > > > + target = &target_priv->target; > > + > > /* hw_info->hw_unit points to a per-target struct type. This > > * works because the first member in the per-target struct is > > * guaranteed to be the struct pdbg_target (see the comment > > * above DECLARE_HW_UNIT). */ > > memcpy(target, hw_info->hw_unit, size); > > target_class = get_target_class(target); > > - list_add_tail(&target_class->targets, &target->class_link); > > + list_add_tail(&target_class->targets, &target_priv- > > >class_link); > > > > return target; > > } > > @@ -118,31 +121,36 @@ static struct pdbg_target > > *dt_pdbg_target_new(const void *fdt, int node_offset) > > static struct pdbg_target *dt_new_node(const char *name, const void > > *fdt, int node_offset) > > { > > struct pdbg_target *node = NULL; > > - size_t size = sizeof(*node); > > + struct pdbg_target_priv *target_priv; > > > > - if (fdt) > > + if (fdt) { > > node = dt_pdbg_target_new(fdt, node_offset); > > - > > - if (!node) > > - node = calloc(1, size); > > + target_priv = pdbg_target_to_priv(node); > > + } > > > > if (!node) { > > + target_priv = calloc(1, sizeof(*target_priv)); > > + node = &target_priv->target; > > + } > > + > > + if (!node && !target_priv) { > > prerror("Failed to allocate node\n"); > > abort(); > > } > > > > - node->dn_name = take_name(name); > > + target_priv->dn_name = take_name(name); > > node->parent = NULL; > > - list_head_init(&node->properties); > > - list_head_init(&node->children); > > - node->phandle = ++last_phandle; > > + list_head_init(&target_priv->properties); > > + list_head_init(&target_priv->children); > > + target_priv->phandle = ++last_phandle; > > > > return node; > > } > > > > static const char *get_unitname(const struct pdbg_target *node) > > { > > - const char *c = strchr(node->dn_name, '@'); > > + struct pdbg_target_priv *target_priv = > > pdbg_target_to_priv(node); > > + const char *c = strchr(target_priv->dn_name, '@'); > > > > if (!c) > > return NULL; > > @@ -152,13 +160,14 @@ static const char *get_unitname(const struct > > pdbg_target *node) > > > > static int dt_cmp_subnodes(const struct pdbg_target *a, const struct > > pdbg_target *b) > > { > > + struct pdbg_target_priv *priv_a = pdbg_target_to_priv(a), > > *priv_b = pdbg_target_to_priv(b); > > const char *a_unit = get_unitname(a); > > const char *b_unit = get_unitname(b); > > > > - ptrdiff_t basenamelen = a_unit - a->dn_name; > > + ptrdiff_t basenamelen = a_unit - priv_a->dn_name; > > > > /* sort hex unit addresses by number */ > > - if (a_unit && b_unit && !strncmp(a->dn_name, b->dn_name, > > basenamelen)) { > > + if (a_unit && b_unit && !strncmp(priv_a->dn_name, priv_b- > > >dn_name, basenamelen)) { > > unsigned long long a_num, b_num; > > char *a_end, *b_end; > > > > @@ -170,29 +179,31 @@ static int dt_cmp_subnodes(const struct > > pdbg_target *a, const struct pdbg_target > > return (a_num > b_num) - (a_num < b_num); > > } > > > > - return strcmp(a->dn_name, b->dn_name); > > + return strcmp(priv_a->dn_name, priv_b->dn_name); > > } > > > > static bool dt_attach_root(struct pdbg_target *parent, struct > > pdbg_target *root) > > { > > - struct pdbg_target *node = NULL; > > + struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root); > > + struct pdbg_target_priv *parent_priv = > > pdbg_target_to_priv(parent); > > + struct pdbg_target_priv *node = NULL; > > > > assert(!root->parent); > > > > - if (list_empty(&parent->children)) { > > - list_add(&parent->children, &root->list); > > + if (list_empty(&parent_priv->children)) { > > + list_add(&parent_priv->children, &root_priv->list); > > root->parent = parent; > > > > return true; > > } > > > > - dt_for_each_child(parent, node) { > > - int cmp = dt_cmp_subnodes(node, root); > > + dt_for_each_child(parent_priv, node) { > > + int cmp = dt_cmp_subnodes(&node->target, root); > > > > /* Look for duplicates */ > > if (cmp == 0) { > > prerror("DT: %s failed, duplicate %s\n", > > - __func__, root->dn_name); > > + __func__, root_priv->dn_name); > > return false; > > } > > > > @@ -202,7 +213,7 @@ static bool dt_attach_root(struct pdbg_target > > *parent, struct pdbg_target *root) > > break; > > } > > > > - list_add_before(&parent->children, &root->list, &node->list); > > + list_add_before(&parent_priv->children, &root_priv->list, > > &node->list); > > root->parent = parent; > > > > return true; > > @@ -219,7 +230,7 @@ static char *dt_get_path(const struct pdbg_target > > *node) > > return strdup("<NULL>"); > > > > for (n = node; n; n = n->parent) { > > - len += strlen(n->dn_name); > > + len += strlen(pdbg_target_to_priv(n)->dn_name); > > if (n->parent || n == node) > > len++; > > } > > @@ -227,9 +238,9 @@ static char *dt_get_path(const struct pdbg_target > > *node) > > assert(path); > > p = path + len; > > for (n = node; n; n = n->parent) { > > - len = strlen(n->dn_name); > > + len = strlen(pdbg_target_to_priv(n)->dn_name); > > p -= len; > > - memcpy(p, n->dn_name, len); > > + memcpy(p, pdbg_target_to_priv(n)->dn_name, len); > > if (n->parent || n == node) > > *(--p) = '/'; > > } > > @@ -272,7 +283,8 @@ static const char *__dt_path_split(const char *p, > > > > static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, > > const char *path) > > { > > - struct pdbg_target *n; > > + struct pdbg_target_priv *n; > > + struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root); > > const char *pn, *pa = NULL, *p = path, *nn = NULL, *na = NULL; > > unsigned int pnl, pal, nnl, nal; > > bool match; > > @@ -286,7 +298,7 @@ static struct pdbg_target *dt_find_by_path(struct > > pdbg_target *root, const char > > > > /* Compare with each child node */ > > match = false; > > - list_for_each(&root->children, n, list) { > > + list_for_each(&root_priv->children, n, list) { > > match = true; > > __dt_path_split(n->dn_name, &nn, &nnl, &na, > > &nal); > > if (pnl && (pnl != nnl || strncmp(pn, nn, > > pnl))) > > @@ -294,7 +306,7 @@ static struct pdbg_target *dt_find_by_path(struct > > pdbg_target *root, const char > > if (pal && (pal != nal || strncmp(pa, na, > > pal))) > > match = false; > > if (match) { > > - root = n; > > + root = &n->target; > > break; > > } > > } > > @@ -311,7 +323,7 @@ static struct dt_property *dt_find_property(const > > struct pdbg_target *node, > > { > > struct dt_property *i = NULL; > > > > - list_for_each(&node->properties, i, list) > > + list_for_each(&pdbg_target_to_priv(node)->properties, i, list) > > if (strcmp(i->name, name) == 0) > > return i; > > return NULL; > > @@ -341,7 +353,7 @@ static struct dt_property *new_property(struct > > pdbg_target *node, > > > > p->name = take_name(name); > > p->len = size; > > - list_add_tail(&node->properties, &p->list); > > + list_add_tail(&pdbg_target_to_priv(node)->properties, &p- > > >list); > > return p; > > } > > > > @@ -349,6 +361,7 @@ static struct dt_property *dt_add_property(struct > > pdbg_target *node, > > const char *name, > > const void *val, size_t size) > > { > > + struct pdbg_target_priv *node_priv = pdbg_target_to_priv(node); > > struct dt_property *p; > > > > /* > > @@ -358,9 +371,9 @@ static struct dt_property *dt_add_property(struct > > pdbg_target *node, > > if (strcmp(name, "linux,phandle") == 0 || > > strcmp(name, "phandle") == 0) { > > assert(size == 4); > > - node->phandle = *(const u32 *)val; > > - if (node->phandle >= last_phandle) > > - last_phandle = node->phandle; > > + node_priv->phandle = *(const u32 *)val; > > + if (node_priv->phandle >= last_phandle) > > + last_phandle = node_priv->phandle; > > return NULL; > > } > > > > @@ -422,21 +435,23 @@ static u32 dt_property_get_cell(const struct > > dt_property *prop, u32 index) > > /* First child of this node. */ > > static struct pdbg_target *dt_first(const struct pdbg_target *root) > > { > > - return list_top(&root->children, struct pdbg_target, list); > > + return &list_top(&pdbg_target_to_priv(root)->children, struct > > pdbg_target_priv, list)->target; > > } > > > > /* Return next node, or NULL. */ > > static struct pdbg_target *dt_next(const struct pdbg_target *root, > > const struct pdbg_target *prev) > > { > > + struct pdbg_target_priv *prev_priv = pdbg_target_to_priv(prev); > > + > > /* Children? */ > > - if (!list_empty(&prev->children)) > > + if (!list_empty(&prev_priv->children)) > > return dt_first(prev); > > > > do { > > /* More siblings? */ > > - if (prev->list.next != &prev->parent->children.n) > > - return list_entry(prev->list.next, struct > > pdbg_target,list); > > + if (prev_priv->list.next != &pdbg_target_to_priv(prev- > > >parent)->children.n) > > + return &list_entry(prev_priv->list.next, struct > > pdbg_target_priv, list)->target; > > > > /* No more siblings, move up to parent. */ > > prev = prev->parent; > > @@ -563,11 +578,11 @@ static int dt_expand_node(struct pdbg_target > > *node, const void *fdt, int fdt_nod > > name = fdt_string(fdt, fdt32_to_cpu(prop- > > >nameoff)); > > if (strcmp("index", name) == 0) { > > memcpy(&data, prop->data, > > sizeof(data)); > > - node->index = fdt32_to_cpu(data); > > + pdbg_target_to_priv(node)->index = > > fdt32_to_cpu(data); > > } > > > > if (strcmp("status", name) == 0) > > - node->status = str_to_status(prop- > > >data); > > + pdbg_target_to_priv(node)->status = > > str_to_status(prop->data); > > > > dt_add_property(node, name, prop->data, > > fdt32_to_cpu(prop->len)); > > diff --git a/libpdbg/hwunit.c b/libpdbg/hwunit.c > > index c7ec63d..5c40d23 100644 > > --- a/libpdbg/hwunit.c > > +++ b/libpdbg/hwunit.c > > @@ -21,6 +21,9 @@ > > > > #define MAX_HW_UNITS 1024 > > > > +struct pdbg_target pdbg_target_size; > > +uint8_t pdbg_target_size_test[sizeof(struct pdbg_target)]; > > + > > static const struct hw_unit_info *g_hw_unit[MAX_HW_UNITS]; > > static int g_hw_unit_count; > > > > diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c > > index 0c5a451..bec2488 100644 > > --- a/libpdbg/libpdbg.c > > +++ b/libpdbg/libpdbg.c > > @@ -9,6 +9,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 *next, *tmp; > > + struct pdbg_target_priv *last_priv; > > struct pdbg_target_class *target_class; > > > > if (class && !find_target_class(class)) > > @@ -17,15 +18,17 @@ struct pdbg_target *__pdbg_next_target(const char > > *class, struct pdbg_target *pa > > target_class = require_target_class(class); > > > > retry: > > + last_priv = pdbg_target_to_priv(last); > > + > > /* No more targets left to check in this class */ > > - if ((last && last->class_link.next == &target_class->targets.n) > > || > > + if ((last && last_priv->class_link.next == &target_class- > > >targets.n) || > > list_empty(&target_class->targets)) > > return NULL; > > > > if (last) > > - next = list_entry(last->class_link.next, struct > > pdbg_target, class_link); > > + next = &list_entry(last_priv->class_link.next, struct > > pdbg_target_priv, class_link)->target; > > else > > - if (!(next = list_top(&target_class->targets, struct > > pdbg_target, class_link))) > > + if (!(next = &list_top(&target_class->targets, struct > > pdbg_target_priv, class_link)->target)) > > return NULL; > > > > if (!parent) > > @@ -45,21 +48,24 @@ retry: > > > > struct pdbg_target *__pdbg_next_child_target(struct pdbg_target > > *parent, struct pdbg_target *last) > > { > > - if (!parent || list_empty(&parent->children)) > > + struct pdbg_target_priv *parent_priv = > > pdbg_target_to_priv(parent); > > + struct pdbg_target_priv *last_priv = pdbg_target_to_priv(last); > > + > > + if (!parent || list_empty(&parent_priv->children)) > > return NULL; > > > > if (!last) > > - return list_top(&parent->children, struct pdbg_target, > > list); > > + return &list_top(&parent_priv->children, struct > > pdbg_target_priv, list)->target; > > > > - if (last->list.next == &parent->children.n) > > + if (last_priv->list.next == &parent_priv->children.n) > > return NULL; > > > > - return list_entry(last->list.next, struct pdbg_target, list); > > + return &list_entry(last_priv->list.next, struct > > pdbg_target_priv, list)->target; > > } > > > > enum pdbg_target_status pdbg_target_status(struct pdbg_target > > *target) > > { > > - return target->status; > > + return pdbg_target_to_priv(target)->status; > > } > > > > void pdbg_target_status_set(struct pdbg_target *target, enum > > pdbg_target_status status) > > @@ -68,7 +74,7 @@ void pdbg_target_status_set(struct pdbg_target > > *target, enum pdbg_target_status > > * blow up obviously if this happens */ > > assert(status == PDBG_TARGET_DISABLED || status == > > PDBG_TARGET_MUSTEXIST); > > > > - target->status = status; > > + pdbg_target_to_priv(target)->status = status; > > } > > > > /* Searches up the tree and returns the first valid index found */ > > @@ -76,12 +82,12 @@ 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 && pdbg_target_to_priv(dn)->index == -1; > > dn = dn->parent); > > > > if (!dn) > > return -1; > > else > > - return dn->index; > > + return pdbg_target_to_priv(dn)->index; > > } > > > > /* Find a target parent from the given class */ > > @@ -132,7 +138,7 @@ char *pdbg_target_name(struct pdbg_target > > *target) > > > > const char *pdbg_target_dn_name(struct pdbg_target *target) > > { > > - return target->dn_name; > > + return pdbg_target_to_priv(target)->dn_name; > > } > > > > int pdbg_target_u32_property(struct pdbg_target *target, const char > > *name, uint32_t *val) > > diff --git a/libpdbg/target.c b/libpdbg/target.c > > index 5808d02..ce2bb2d 100644 > > --- a/libpdbg/target.c > > +++ b/libpdbg/target.c > > @@ -400,7 +400,7 @@ enum pdbg_target_status pdbg_target_probe(struct > > pdbg_target *target) > > case PDBG_TARGET_NONEXISTENT: > > /* The parent doesn't exist neither does it's > > * children */ > > - target->status = PDBG_TARGET_NONEXISTENT; > > + pdbg_target_to_priv(target)->status = > > PDBG_TARGET_NONEXISTENT; > > return PDBG_TARGET_NONEXISTENT; > > > > case PDBG_TARGET_DISABLED: > > @@ -427,11 +427,11 @@ enum pdbg_target_status > > pdbg_target_probe(struct pdbg_target *target) > > if (target->probe && target->probe(target)) { > > /* Could not find the target */ > > assert(pdbg_target_status(target) != > > PDBG_TARGET_MUSTEXIST); > > - target->status = PDBG_TARGET_NONEXISTENT; > > + pdbg_target_to_priv(target)->status = > > PDBG_TARGET_NONEXISTENT; > > return PDBG_TARGET_NONEXISTENT; > > } > > > > - target->status = PDBG_TARGET_ENABLED; > > + pdbg_target_to_priv(target)->status = PDBG_TARGET_ENABLED; > > return PDBG_TARGET_ENABLED; > > } > > > > @@ -449,7 +449,7 @@ void pdbg_target_release(struct pdbg_target > > *target) > > /* Release the target */ > > if (target->release) > > target->release(target); > > - target->status = PDBG_TARGET_RELEASED; > > + pdbg_target_to_priv(target)->status = PDBG_TARGET_RELEASED; > > } > > > > /* > > @@ -477,10 +477,10 @@ bool pdbg_target_is_class(struct pdbg_target > > *target, const char *class) > > > > void *pdbg_target_priv(struct pdbg_target *target) > > { > > - return target->priv; > > + return pdbg_target_to_priv(target)->priv; > > } > > > > void pdbg_target_priv_set(struct pdbg_target *target, void *priv) > > { > > - target->priv = priv; > > + pdbg_target_to_priv(target)->priv = priv; > > } > > diff --git a/libpdbg/target.h b/libpdbg/target.h > > index 2ecbf98..ba2b20d 100644 > > --- a/libpdbg/target.h > > +++ b/libpdbg/target.h > > @@ -39,19 +39,25 @@ struct pdbg_target { > > int (*probe)(struct pdbg_target *target); > > void (*release)(struct pdbg_target *target); > > uint64_t (*translate)(struct pdbg_target *target, uint64_t > > addr); > > + struct pdbg_target *parent; > > +}; > > + > > +struct pdbg_target_priv { > > int index; > > enum pdbg_target_status status; > > const char *dn_name; > > struct list_node list; > > struct list_head properties; > > struct list_head children; > > - struct pdbg_target *parent; > > u32 phandle; > > bool probed; > > struct list_node class_link; > > + struct pdbg_target target; > > void *priv; > > }; > > > > +#define pdbg_target_to_priv(x) (x ? container_of(x, struct > > pdbg_target_priv, target) : NULL) > > + > > 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); > > Amitay. >
diff --git a/libpdbg/device.c b/libpdbg/device.c index efa9ce4..e009a1a 100644 --- a/libpdbg/device.c +++ b/libpdbg/device.c @@ -67,6 +67,7 @@ static const char *take_name(const char *name) /* Adds information representing an actual target */ static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset) { + struct pdbg_target_priv *target_priv; struct pdbg_target *target; struct pdbg_target_class *target_class; const struct hw_unit_info *hw_info = NULL; @@ -98,19 +99,21 @@ static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset) /* Couldn't find anything implementing this target */ return NULL; - target = calloc(1, size); - if (!target) { + target_priv = calloc(1, sizeof(struct pdbg_target_priv) - sizeof(struct pdbg_target) + size); + if (!target_priv) { prerror("Failed to allocate node\n"); abort(); } + target = &target_priv->target; + /* hw_info->hw_unit points to a per-target struct type. This * works because the first member in the per-target struct is * guaranteed to be the struct pdbg_target (see the comment * above DECLARE_HW_UNIT). */ memcpy(target, hw_info->hw_unit, size); target_class = get_target_class(target); - list_add_tail(&target_class->targets, &target->class_link); + list_add_tail(&target_class->targets, &target_priv->class_link); return target; } @@ -118,31 +121,36 @@ static struct pdbg_target *dt_pdbg_target_new(const void *fdt, int node_offset) static struct pdbg_target *dt_new_node(const char *name, const void *fdt, int node_offset) { struct pdbg_target *node = NULL; - size_t size = sizeof(*node); + struct pdbg_target_priv *target_priv; - if (fdt) + if (fdt) { node = dt_pdbg_target_new(fdt, node_offset); - - if (!node) - node = calloc(1, size); + target_priv = pdbg_target_to_priv(node); + } if (!node) { + target_priv = calloc(1, sizeof(*target_priv)); + node = &target_priv->target; + } + + if (!node && !target_priv) { prerror("Failed to allocate node\n"); abort(); } - node->dn_name = take_name(name); + target_priv->dn_name = take_name(name); node->parent = NULL; - list_head_init(&node->properties); - list_head_init(&node->children); - node->phandle = ++last_phandle; + list_head_init(&target_priv->properties); + list_head_init(&target_priv->children); + target_priv->phandle = ++last_phandle; return node; } static const char *get_unitname(const struct pdbg_target *node) { - const char *c = strchr(node->dn_name, '@'); + struct pdbg_target_priv *target_priv = pdbg_target_to_priv(node); + const char *c = strchr(target_priv->dn_name, '@'); if (!c) return NULL; @@ -152,13 +160,14 @@ static const char *get_unitname(const struct pdbg_target *node) static int dt_cmp_subnodes(const struct pdbg_target *a, const struct pdbg_target *b) { + struct pdbg_target_priv *priv_a = pdbg_target_to_priv(a), *priv_b = pdbg_target_to_priv(b); const char *a_unit = get_unitname(a); const char *b_unit = get_unitname(b); - ptrdiff_t basenamelen = a_unit - a->dn_name; + ptrdiff_t basenamelen = a_unit - priv_a->dn_name; /* sort hex unit addresses by number */ - if (a_unit && b_unit && !strncmp(a->dn_name, b->dn_name, basenamelen)) { + if (a_unit && b_unit && !strncmp(priv_a->dn_name, priv_b->dn_name, basenamelen)) { unsigned long long a_num, b_num; char *a_end, *b_end; @@ -170,29 +179,31 @@ static int dt_cmp_subnodes(const struct pdbg_target *a, const struct pdbg_target return (a_num > b_num) - (a_num < b_num); } - return strcmp(a->dn_name, b->dn_name); + return strcmp(priv_a->dn_name, priv_b->dn_name); } static bool dt_attach_root(struct pdbg_target *parent, struct pdbg_target *root) { - struct pdbg_target *node = NULL; + struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root); + struct pdbg_target_priv *parent_priv = pdbg_target_to_priv(parent); + struct pdbg_target_priv *node = NULL; assert(!root->parent); - if (list_empty(&parent->children)) { - list_add(&parent->children, &root->list); + if (list_empty(&parent_priv->children)) { + list_add(&parent_priv->children, &root_priv->list); root->parent = parent; return true; } - dt_for_each_child(parent, node) { - int cmp = dt_cmp_subnodes(node, root); + dt_for_each_child(parent_priv, node) { + int cmp = dt_cmp_subnodes(&node->target, root); /* Look for duplicates */ if (cmp == 0) { prerror("DT: %s failed, duplicate %s\n", - __func__, root->dn_name); + __func__, root_priv->dn_name); return false; } @@ -202,7 +213,7 @@ static bool dt_attach_root(struct pdbg_target *parent, struct pdbg_target *root) break; } - list_add_before(&parent->children, &root->list, &node->list); + list_add_before(&parent_priv->children, &root_priv->list, &node->list); root->parent = parent; return true; @@ -219,7 +230,7 @@ static char *dt_get_path(const struct pdbg_target *node) return strdup("<NULL>"); for (n = node; n; n = n->parent) { - len += strlen(n->dn_name); + len += strlen(pdbg_target_to_priv(n)->dn_name); if (n->parent || n == node) len++; } @@ -227,9 +238,9 @@ static char *dt_get_path(const struct pdbg_target *node) assert(path); p = path + len; for (n = node; n; n = n->parent) { - len = strlen(n->dn_name); + len = strlen(pdbg_target_to_priv(n)->dn_name); p -= len; - memcpy(p, n->dn_name, len); + memcpy(p, pdbg_target_to_priv(n)->dn_name, len); if (n->parent || n == node) *(--p) = '/'; } @@ -272,7 +283,8 @@ static const char *__dt_path_split(const char *p, static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char *path) { - struct pdbg_target *n; + struct pdbg_target_priv *n; + struct pdbg_target_priv *root_priv = pdbg_target_to_priv(root); const char *pn, *pa = NULL, *p = path, *nn = NULL, *na = NULL; unsigned int pnl, pal, nnl, nal; bool match; @@ -286,7 +298,7 @@ static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char /* Compare with each child node */ match = false; - list_for_each(&root->children, n, list) { + list_for_each(&root_priv->children, n, list) { match = true; __dt_path_split(n->dn_name, &nn, &nnl, &na, &nal); if (pnl && (pnl != nnl || strncmp(pn, nn, pnl))) @@ -294,7 +306,7 @@ static struct pdbg_target *dt_find_by_path(struct pdbg_target *root, const char if (pal && (pal != nal || strncmp(pa, na, pal))) match = false; if (match) { - root = n; + root = &n->target; break; } } @@ -311,7 +323,7 @@ static struct dt_property *dt_find_property(const struct pdbg_target *node, { struct dt_property *i = NULL; - list_for_each(&node->properties, i, list) + list_for_each(&pdbg_target_to_priv(node)->properties, i, list) if (strcmp(i->name, name) == 0) return i; return NULL; @@ -341,7 +353,7 @@ static struct dt_property *new_property(struct pdbg_target *node, p->name = take_name(name); p->len = size; - list_add_tail(&node->properties, &p->list); + list_add_tail(&pdbg_target_to_priv(node)->properties, &p->list); return p; } @@ -349,6 +361,7 @@ static struct dt_property *dt_add_property(struct pdbg_target *node, const char *name, const void *val, size_t size) { + struct pdbg_target_priv *node_priv = pdbg_target_to_priv(node); struct dt_property *p; /* @@ -358,9 +371,9 @@ static struct dt_property *dt_add_property(struct pdbg_target *node, if (strcmp(name, "linux,phandle") == 0 || strcmp(name, "phandle") == 0) { assert(size == 4); - node->phandle = *(const u32 *)val; - if (node->phandle >= last_phandle) - last_phandle = node->phandle; + node_priv->phandle = *(const u32 *)val; + if (node_priv->phandle >= last_phandle) + last_phandle = node_priv->phandle; return NULL; } @@ -422,21 +435,23 @@ static u32 dt_property_get_cell(const struct dt_property *prop, u32 index) /* First child of this node. */ static struct pdbg_target *dt_first(const struct pdbg_target *root) { - return list_top(&root->children, struct pdbg_target, list); + return &list_top(&pdbg_target_to_priv(root)->children, struct pdbg_target_priv, list)->target; } /* Return next node, or NULL. */ static struct pdbg_target *dt_next(const struct pdbg_target *root, const struct pdbg_target *prev) { + struct pdbg_target_priv *prev_priv = pdbg_target_to_priv(prev); + /* Children? */ - if (!list_empty(&prev->children)) + if (!list_empty(&prev_priv->children)) return dt_first(prev); do { /* More siblings? */ - if (prev->list.next != &prev->parent->children.n) - return list_entry(prev->list.next, struct pdbg_target,list); + if (prev_priv->list.next != &pdbg_target_to_priv(prev->parent)->children.n) + return &list_entry(prev_priv->list.next, struct pdbg_target_priv, list)->target; /* No more siblings, move up to parent. */ prev = prev->parent; @@ -563,11 +578,11 @@ static int dt_expand_node(struct pdbg_target *node, const void *fdt, int fdt_nod name = fdt_string(fdt, fdt32_to_cpu(prop->nameoff)); if (strcmp("index", name) == 0) { memcpy(&data, prop->data, sizeof(data)); - node->index = fdt32_to_cpu(data); + pdbg_target_to_priv(node)->index = fdt32_to_cpu(data); } if (strcmp("status", name) == 0) - node->status = str_to_status(prop->data); + pdbg_target_to_priv(node)->status = str_to_status(prop->data); dt_add_property(node, name, prop->data, fdt32_to_cpu(prop->len)); diff --git a/libpdbg/hwunit.c b/libpdbg/hwunit.c index c7ec63d..5c40d23 100644 --- a/libpdbg/hwunit.c +++ b/libpdbg/hwunit.c @@ -21,6 +21,9 @@ #define MAX_HW_UNITS 1024 +struct pdbg_target pdbg_target_size; +uint8_t pdbg_target_size_test[sizeof(struct pdbg_target)]; + static const struct hw_unit_info *g_hw_unit[MAX_HW_UNITS]; static int g_hw_unit_count; diff --git a/libpdbg/libpdbg.c b/libpdbg/libpdbg.c index 0c5a451..bec2488 100644 --- a/libpdbg/libpdbg.c +++ b/libpdbg/libpdbg.c @@ -9,6 +9,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 *next, *tmp; + struct pdbg_target_priv *last_priv; struct pdbg_target_class *target_class; if (class && !find_target_class(class)) @@ -17,15 +18,17 @@ struct pdbg_target *__pdbg_next_target(const char *class, struct pdbg_target *pa target_class = require_target_class(class); retry: + last_priv = pdbg_target_to_priv(last); + /* No more targets left to check in this class */ - if ((last && last->class_link.next == &target_class->targets.n) || + if ((last && last_priv->class_link.next == &target_class->targets.n) || list_empty(&target_class->targets)) return NULL; if (last) - next = list_entry(last->class_link.next, struct pdbg_target, class_link); + next = &list_entry(last_priv->class_link.next, struct pdbg_target_priv, class_link)->target; else - if (!(next = list_top(&target_class->targets, struct pdbg_target, class_link))) + if (!(next = &list_top(&target_class->targets, struct pdbg_target_priv, class_link)->target)) return NULL; if (!parent) @@ -45,21 +48,24 @@ retry: struct pdbg_target *__pdbg_next_child_target(struct pdbg_target *parent, struct pdbg_target *last) { - if (!parent || list_empty(&parent->children)) + struct pdbg_target_priv *parent_priv = pdbg_target_to_priv(parent); + struct pdbg_target_priv *last_priv = pdbg_target_to_priv(last); + + if (!parent || list_empty(&parent_priv->children)) return NULL; if (!last) - return list_top(&parent->children, struct pdbg_target, list); + return &list_top(&parent_priv->children, struct pdbg_target_priv, list)->target; - if (last->list.next == &parent->children.n) + if (last_priv->list.next == &parent_priv->children.n) return NULL; - return list_entry(last->list.next, struct pdbg_target, list); + return &list_entry(last_priv->list.next, struct pdbg_target_priv, list)->target; } enum pdbg_target_status pdbg_target_status(struct pdbg_target *target) { - return target->status; + return pdbg_target_to_priv(target)->status; } void pdbg_target_status_set(struct pdbg_target *target, enum pdbg_target_status status) @@ -68,7 +74,7 @@ void pdbg_target_status_set(struct pdbg_target *target, enum pdbg_target_status * blow up obviously if this happens */ assert(status == PDBG_TARGET_DISABLED || status == PDBG_TARGET_MUSTEXIST); - target->status = status; + pdbg_target_to_priv(target)->status = status; } /* Searches up the tree and returns the first valid index found */ @@ -76,12 +82,12 @@ 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 && pdbg_target_to_priv(dn)->index == -1; dn = dn->parent); if (!dn) return -1; else - return dn->index; + return pdbg_target_to_priv(dn)->index; } /* Find a target parent from the given class */ @@ -132,7 +138,7 @@ char *pdbg_target_name(struct pdbg_target *target) const char *pdbg_target_dn_name(struct pdbg_target *target) { - return target->dn_name; + return pdbg_target_to_priv(target)->dn_name; } int pdbg_target_u32_property(struct pdbg_target *target, const char *name, uint32_t *val) diff --git a/libpdbg/target.c b/libpdbg/target.c index 5808d02..ce2bb2d 100644 --- a/libpdbg/target.c +++ b/libpdbg/target.c @@ -400,7 +400,7 @@ enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target) case PDBG_TARGET_NONEXISTENT: /* The parent doesn't exist neither does it's * children */ - target->status = PDBG_TARGET_NONEXISTENT; + pdbg_target_to_priv(target)->status = PDBG_TARGET_NONEXISTENT; return PDBG_TARGET_NONEXISTENT; case PDBG_TARGET_DISABLED: @@ -427,11 +427,11 @@ enum pdbg_target_status pdbg_target_probe(struct pdbg_target *target) if (target->probe && target->probe(target)) { /* Could not find the target */ assert(pdbg_target_status(target) != PDBG_TARGET_MUSTEXIST); - target->status = PDBG_TARGET_NONEXISTENT; + pdbg_target_to_priv(target)->status = PDBG_TARGET_NONEXISTENT; return PDBG_TARGET_NONEXISTENT; } - target->status = PDBG_TARGET_ENABLED; + pdbg_target_to_priv(target)->status = PDBG_TARGET_ENABLED; return PDBG_TARGET_ENABLED; } @@ -449,7 +449,7 @@ void pdbg_target_release(struct pdbg_target *target) /* Release the target */ if (target->release) target->release(target); - target->status = PDBG_TARGET_RELEASED; + pdbg_target_to_priv(target)->status = PDBG_TARGET_RELEASED; } /* @@ -477,10 +477,10 @@ bool pdbg_target_is_class(struct pdbg_target *target, const char *class) void *pdbg_target_priv(struct pdbg_target *target) { - return target->priv; + return pdbg_target_to_priv(target)->priv; } void pdbg_target_priv_set(struct pdbg_target *target, void *priv) { - target->priv = priv; + pdbg_target_to_priv(target)->priv = priv; } diff --git a/libpdbg/target.h b/libpdbg/target.h index 2ecbf98..ba2b20d 100644 --- a/libpdbg/target.h +++ b/libpdbg/target.h @@ -39,19 +39,25 @@ struct pdbg_target { int (*probe)(struct pdbg_target *target); void (*release)(struct pdbg_target *target); uint64_t (*translate)(struct pdbg_target *target, uint64_t addr); + struct pdbg_target *parent; +}; + +struct pdbg_target_priv { int index; enum pdbg_target_status status; const char *dn_name; struct list_node list; struct list_head properties; struct list_head children; - struct pdbg_target *parent; u32 phandle; bool probed; struct list_node class_link; + struct pdbg_target target; void *priv; }; +#define pdbg_target_to_priv(x) (x ? container_of(x, struct pdbg_target_priv, target) : NULL) + 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);
In order to allow external programs to register their own hardware units it is required to expose the definition of struct pdbg_target. However this means the struct definition now becomes part of the library ABI/API, making it difficult to change fields without breaking applications. It also means we would have to install the ccan list headers, which themselves depend on other ccan modules. Instead create a seperate struct definition (struct pdbg_target_priv) which encapsulates the libpdbg specific fields which should not be accessed directly by applications and switch the library to use those. This should allow a stable definition of struct pdbg_target to be exported to applications without preventing changes to the underlying library fields. Signed-off-by: Alistair Popple <alistair@popple.id.au> --- libpdbg/device.c | 95 +++++++++++++++++++++++++++-------------------- libpdbg/hwunit.c | 3 ++ libpdbg/libpdbg.c | 30 +++++++++------ libpdbg/target.c | 12 +++--- libpdbg/target.h | 8 +++- 5 files changed, 89 insertions(+), 59 deletions(-)