| Message ID | 20200115051901.17514-8-amitay@ozlabs.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Use fdt properties directly | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | warning | Failed to apply on branch master (8b4611b5d8e7e2279fe4aa80c892fcfe10aa398d) |
| snowpatch_ozlabs/apply_patch | fail | Failed to apply to any branch |
Reviewed-by: Alistair Popple <alistair@popple.id.au> On Wednesday, 15 January 2020 4:18:55 PM AEDT Amitay Isaacs wrote: > ... instead the properties will be accessed directly from the device > tree. > > dt_add_property(), in addition to adding properties to a linked list, > assigned value to phandle if defined in device tree. So change the name > of the function to reflect the functionality. > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > --- > libpdbg/device.c | 54 +++--------------------------------------------- > 1 file changed, 3 insertions(+), 51 deletions(-) > > diff --git a/libpdbg/device.c b/libpdbg/device.c > index 91ad258..c5fdc4e 100644 > --- a/libpdbg/device.c > +++ b/libpdbg/device.c > @@ -331,40 +331,9 @@ static struct dt_property *dt_find_property(const > struct pdbg_target *node, return NULL; > } > > -static struct dt_property *new_property(struct pdbg_target *node, > - const char *name, size_t size) > +static void dt_add_phandle(struct pdbg_target *node, const char *name, > + const void *val, size_t size) > { > - struct dt_property *p = malloc(sizeof(*p) + size); > - char *path; > - > - if (!p) { > - path = dt_get_path(node); > - prerror("Failed to allocate property \"%s\" for %s of %zu bytes\n", > - name, path, size); > - free(path); > - abort(); > - } > - if (dt_find_property(node, name)) { > - path = dt_get_path(node); > - prerror("Duplicate property \"%s\" in node %s\n", > - name, path); > - free(path); > - abort(); > - > - } > - > - p->name = take_name(name); > - p->len = size; > - list_add_tail(&node->properties, &p->list); > - return p; > -} > - > -static struct dt_property *dt_add_property(struct pdbg_target *node, > - const char *name, > - const void *val, size_t size) > -{ > - struct dt_property *p; > - > /* > * Filter out phandle properties, we re-generate them > * when flattening > @@ -375,13 +344,7 @@ static struct dt_property *dt_add_property(struct > pdbg_target *node, node->phandle = *(const u32 *)val; > if (node->phandle >= last_phandle) > last_phandle = node->phandle; > - return NULL; > } > - > - p = new_property(node, name, size); > - if (size) > - memcpy(p->prop, val, size); > - return p; > } > > bool pdbg_target_set_property(struct pdbg_target *target, const char *name, > const void *val, size_t size) @@ -573,7 +536,7 @@ static int > dt_expand_node(struct pdbg_target *node, void *fdt, int fdt_node) if > (strcmp("status", name) == 0) > node->status = str_to_status(prop->data); > > - dt_add_property(node, name, prop->data, > + dt_add_phandle(node, name, prop->data, > fdt32_to_cpu(prop->len)); > break; > case FDT_BEGIN_NODE: > @@ -684,19 +647,8 @@ static struct pdbg_target *dt_new_virtual(struct > pdbg_target *root, const char * > > static void dt_link_virtual(struct pdbg_target *node, struct pdbg_target > *vnode) { > - struct dt_property *prop = NULL, *next; > - > node->vnode = vnode; > vnode->vnode = node; > - > - /* Move any properties on virtual node to real node */ > - list_for_each_safe(&vnode->properties, prop, next, list) { > - if (!strcmp(prop->name, "#address-cells") || !strcmp(prop->name, > "#size-cells")) - continue; > - > - list_del(&prop->list); > - list_add_tail(&node->properties, &prop->list); > - } > } > > static void pdbg_targets_init_virtual(struct pdbg_target *node, struct > pdbg_target *root)
> static void dt_link_virtual(struct pdbg_target *node, struct pdbg_target > *vnode) { > - struct dt_property *prop = NULL, *next; > - > node->vnode = vnode; > vnode->vnode = node; > - > - /* Move any properties on virtual node to real node */ > - list_for_each_safe(&vnode->properties, prop, next, list) { > - if (!strcmp(prop->name, "#address-cells") || !strcmp(prop->name, > "#size-cells")) - continue; > - > - list_del(&prop->list); > - list_add_tail(&node->properties, &prop->list); > - } We're changing the behaviour here right? We should probably figure out why we did this and make sure it's safe to stop doing it, or change the property reading code to make sure it looks at the virtual node as well when trying to access properties. - Alistair > } > > static void pdbg_targets_init_virtual(struct pdbg_target *node, struct > pdbg_target *root)
On Thu, 2020-01-16 at 12:38 +1100, Alistair Popple wrote: > > static void dt_link_virtual(struct pdbg_target *node, struct > > pdbg_target > > *vnode) { > > - struct dt_property *prop = NULL, *next; > > - > > node->vnode = vnode; > > vnode->vnode = node; > > - > > - /* Move any properties on virtual node to real node */ > > - list_for_each_safe(&vnode->properties, prop, next, list) { > > - if (!strcmp(prop->name, "#address-cells") || > > !strcmp(prop->name, > > "#size-cells")) - continue; > > - > > - list_del(&prop->list); > > - list_add_tail(&node->properties, &prop->list); > > - } > > We're changing the behaviour here right? We should probably figure > out why we > did this and make sure it's safe to stop doing it, or change the > property > reading code to make sure it looks at the virtual node as well when > trying to > access properties. I did think about this. The attributes will always be defined for system tree nodes and those are the nodes returned by most of the api functions. The other case is using the properties for the backend nodes as required for address translation etc. But in those cases we explicitly look for backend nodes. I am sure there might be some corner cases I have missed. We can take care of them as we encounter them. > > - Alistair > > > } > > > > static void pdbg_targets_init_virtual(struct pdbg_target *node, > > struct > > pdbg_target *root) > > > Amitay.
diff --git a/libpdbg/device.c b/libpdbg/device.c index 91ad258..c5fdc4e 100644 --- a/libpdbg/device.c +++ b/libpdbg/device.c @@ -331,40 +331,9 @@ static struct dt_property *dt_find_property(const struct pdbg_target *node, return NULL; } -static struct dt_property *new_property(struct pdbg_target *node, - const char *name, size_t size) +static void dt_add_phandle(struct pdbg_target *node, const char *name, + const void *val, size_t size) { - struct dt_property *p = malloc(sizeof(*p) + size); - char *path; - - if (!p) { - path = dt_get_path(node); - prerror("Failed to allocate property \"%s\" for %s of %zu bytes\n", - name, path, size); - free(path); - abort(); - } - if (dt_find_property(node, name)) { - path = dt_get_path(node); - prerror("Duplicate property \"%s\" in node %s\n", - name, path); - free(path); - abort(); - - } - - p->name = take_name(name); - p->len = size; - list_add_tail(&node->properties, &p->list); - return p; -} - -static struct dt_property *dt_add_property(struct pdbg_target *node, - const char *name, - const void *val, size_t size) -{ - struct dt_property *p; - /* * Filter out phandle properties, we re-generate them * when flattening @@ -375,13 +344,7 @@ static struct dt_property *dt_add_property(struct pdbg_target *node, node->phandle = *(const u32 *)val; if (node->phandle >= last_phandle) last_phandle = node->phandle; - return NULL; } - - p = new_property(node, name, size); - if (size) - memcpy(p->prop, val, size); - return p; } bool pdbg_target_set_property(struct pdbg_target *target, const char *name, const void *val, size_t size) @@ -573,7 +536,7 @@ static int dt_expand_node(struct pdbg_target *node, void *fdt, int fdt_node) if (strcmp("status", name) == 0) node->status = str_to_status(prop->data); - dt_add_property(node, name, prop->data, + dt_add_phandle(node, name, prop->data, fdt32_to_cpu(prop->len)); break; case FDT_BEGIN_NODE: @@ -684,19 +647,8 @@ static struct pdbg_target *dt_new_virtual(struct pdbg_target *root, const char * static void dt_link_virtual(struct pdbg_target *node, struct pdbg_target *vnode) { - struct dt_property *prop = NULL, *next; - node->vnode = vnode; vnode->vnode = node; - - /* Move any properties on virtual node to real node */ - list_for_each_safe(&vnode->properties, prop, next, list) { - if (!strcmp(prop->name, "#address-cells") || !strcmp(prop->name, "#size-cells")) - continue; - - list_del(&prop->list); - list_add_tail(&node->properties, &prop->list); - } } static void pdbg_targets_init_virtual(struct pdbg_target *node, struct pdbg_target *root)
... instead the properties will be accessed directly from the device tree. dt_add_property(), in addition to adding properties to a linked list, assigned value to phandle if defined in device tree. So change the name of the function to reflect the functionality. Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> --- libpdbg/device.c | 54 +++--------------------------------------------- 1 file changed, 3 insertions(+), 51 deletions(-)