| Message ID | 20210516102900.28036-3-wangyanan55@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/virt: Introduce cpu topology support | expand |
On Sun, May 16, 2021 at 06:28:53PM +0800, Yanan Wang wrote: > From: Andrew Jones <drjones@redhat.com> > > qemu_fdt_add_path() works like qemu_fdt_add_subnode(), except it > also adds all missing subnodes from the given path. We'll use it > in a coming patch where we will add cpu-map to the device tree. > > And we also tweak an error message of qemu_fdt_add_subnode(). > > Cc: David Gibson <david@gibson.dropbear.id.au> > Cc: Alistair Francis <alistair.francis@wdc.com> > Signed-off-by: Andrew Jones <drjones@redhat.com> > Co-developed-by: Yanan Wang <wangyanan55@huawei.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Wonder if I should integrate a function like this into libfdt. > --- > include/sysemu/device_tree.h | 1 + > softmmu/device_tree.c | 44 ++++++++++++++++++++++++++++++++++-- > 2 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > index 8a2fe55622..ef060a9759 100644 > --- a/include/sysemu/device_tree.h > +++ b/include/sysemu/device_tree.h > @@ -121,6 +121,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path); > uint32_t qemu_fdt_alloc_phandle(void *fdt); > int qemu_fdt_nop_node(void *fdt, const char *node_path); > int qemu_fdt_add_subnode(void *fdt, const char *name); > +int qemu_fdt_add_path(void *fdt, const char *path); > > #define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \ > do { \ > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index b621f63fba..3965c834ca 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -540,8 +540,8 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) > > retval = fdt_add_subnode(fdt, parent, basename); > if (retval < 0) { > - error_report("FDT: Failed to create subnode %s: %s", name, > - fdt_strerror(retval)); > + error_report("%s: Failed to create subnode %s: %s", > + __func__, name, fdt_strerror(retval)); > exit(1); > } > > @@ -549,6 +549,46 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) > return retval; > } > > +/* > + * qemu_fdt_add_path: Like qemu_fdt_add_subnode(), but will add > + * all missing subnodes from the given path. > + */ > +int qemu_fdt_add_path(void *fdt, const char *path) > +{ > + const char *name; > + const char *p = path; > + int namelen, retval; > + int parent = 0; > + > + if (path[0] != '/') { > + return -1; > + } > + > + while (p) { > + name = p + 1; > + p = strchr(name, '/'); > + namelen = p != NULL ? p - name : strlen(name); > + > + retval = fdt_subnode_offset_namelen(fdt, parent, name, namelen); > + if (retval < 0 && retval != -FDT_ERR_NOTFOUND) { > + error_report("%s: Unexpected error in finding subnode %.*s: %s", > + __func__, namelen, name, fdt_strerror(retval)); > + exit(1); > + } else if (retval == -FDT_ERR_NOTFOUND) { > + retval = fdt_add_subnode_namelen(fdt, parent, name, namelen); > + if (retval < 0) { > + error_report("%s: Failed to create subnode %.*s: %s", > + __func__, namelen, name, fdt_strerror(retval)); > + exit(1); > + } > + } > + > + parent = retval; > + } > + > + return retval; > +} > + > void qemu_fdt_dumpdtb(void *fdt, int size) > { > const char *dumpdtb = current_machine->dumpdtb;
On Sun, May 16, 2021 at 06:28:53PM +0800, Yanan Wang wrote: > From: Andrew Jones <drjones@redhat.com> Hi Yanan, This looks good, but the authorship is no longer correct. You've completely rewritten it, so I think the most I deserve is a Co-developed-by and maybe even just a Suggested-by. When changing the authorship and tags, you can also add a Reviewed-by: Andrew Jones <drjones@redhat.com> Thanks, drew > > qemu_fdt_add_path() works like qemu_fdt_add_subnode(), except it > also adds all missing subnodes from the given path. We'll use it > in a coming patch where we will add cpu-map to the device tree. > > And we also tweak an error message of qemu_fdt_add_subnode(). > > Cc: David Gibson <david@gibson.dropbear.id.au> > Cc: Alistair Francis <alistair.francis@wdc.com> > Signed-off-by: Andrew Jones <drjones@redhat.com> > Co-developed-by: Yanan Wang <wangyanan55@huawei.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > include/sysemu/device_tree.h | 1 + > softmmu/device_tree.c | 44 ++++++++++++++++++++++++++++++++++-- > 2 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > index 8a2fe55622..ef060a9759 100644 > --- a/include/sysemu/device_tree.h > +++ b/include/sysemu/device_tree.h > @@ -121,6 +121,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path); > uint32_t qemu_fdt_alloc_phandle(void *fdt); > int qemu_fdt_nop_node(void *fdt, const char *node_path); > int qemu_fdt_add_subnode(void *fdt, const char *name); > +int qemu_fdt_add_path(void *fdt, const char *path); > > #define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \ > do { \ > diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c > index b621f63fba..3965c834ca 100644 > --- a/softmmu/device_tree.c > +++ b/softmmu/device_tree.c > @@ -540,8 +540,8 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) > > retval = fdt_add_subnode(fdt, parent, basename); > if (retval < 0) { > - error_report("FDT: Failed to create subnode %s: %s", name, > - fdt_strerror(retval)); > + error_report("%s: Failed to create subnode %s: %s", > + __func__, name, fdt_strerror(retval)); > exit(1); > } > > @@ -549,6 +549,46 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) > return retval; > } > > +/* > + * qemu_fdt_add_path: Like qemu_fdt_add_subnode(), but will add > + * all missing subnodes from the given path. > + */ > +int qemu_fdt_add_path(void *fdt, const char *path) > +{ > + const char *name; > + const char *p = path; > + int namelen, retval; > + int parent = 0; > + > + if (path[0] != '/') { > + return -1; > + } > + > + while (p) { > + name = p + 1; > + p = strchr(name, '/'); > + namelen = p != NULL ? p - name : strlen(name); > + > + retval = fdt_subnode_offset_namelen(fdt, parent, name, namelen); > + if (retval < 0 && retval != -FDT_ERR_NOTFOUND) { > + error_report("%s: Unexpected error in finding subnode %.*s: %s", > + __func__, namelen, name, fdt_strerror(retval)); > + exit(1); > + } else if (retval == -FDT_ERR_NOTFOUND) { > + retval = fdt_add_subnode_namelen(fdt, parent, name, namelen); > + if (retval < 0) { > + error_report("%s: Failed to create subnode %.*s: %s", > + __func__, namelen, name, fdt_strerror(retval)); > + exit(1); > + } > + } > + > + parent = retval; > + } > + > + return retval; > +} > + > void qemu_fdt_dumpdtb(void *fdt, int size) > { > const char *dumpdtb = current_machine->dumpdtb; > -- > 2.19.1 >
On 2021/5/17 11:11, David Gibson wrote: > On Sun, May 16, 2021 at 06:28:53PM +0800, Yanan Wang wrote: >> From: Andrew Jones<drjones@redhat.com> >> >> qemu_fdt_add_path() works like qemu_fdt_add_subnode(), except it >> also adds all missing subnodes from the given path. We'll use it >> in a coming patch where we will add cpu-map to the device tree. >> >> And we also tweak an error message of qemu_fdt_add_subnode(). >> >> Cc: David Gibson<david@gibson.dropbear.id.au> >> Cc: Alistair Francis<alistair.francis@wdc.com> >> Signed-off-by: Andrew Jones<drjones@redhat.com> >> Co-developed-by: Yanan Wang<wangyanan55@huawei.com> >> Signed-off-by: Yanan Wang<wangyanan55@huawei.com> Hi David, > Reviewed-by: David Gibson<david@gibson.dropbear.id.au> Thanks! > Wonder if I should integrate a function like this into libfdt. I think it's meaningful to add a function in libfdt that serves as helper to add subpath but not subnode to a given parent node, like fdt_add_subpath_namelen(...). This will be useful when we need to frequently add different paths to a node. Thanks, Yanan >> --- >> include/sysemu/device_tree.h | 1 + >> softmmu/device_tree.c | 44 ++++++++++++++++++++++++++++++++++-- >> 2 files changed, 43 insertions(+), 2 deletions(-) >> >> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h >> index 8a2fe55622..ef060a9759 100644 >> --- a/include/sysemu/device_tree.h >> +++ b/include/sysemu/device_tree.h >> @@ -121,6 +121,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path); >> uint32_t qemu_fdt_alloc_phandle(void *fdt); >> int qemu_fdt_nop_node(void *fdt, const char *node_path); >> int qemu_fdt_add_subnode(void *fdt, const char *name); >> +int qemu_fdt_add_path(void *fdt, const char *path); >> >> #define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \ >> do { \ >> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c >> index b621f63fba..3965c834ca 100644 >> --- a/softmmu/device_tree.c >> +++ b/softmmu/device_tree.c >> @@ -540,8 +540,8 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) >> >> retval = fdt_add_subnode(fdt, parent, basename); >> if (retval < 0) { >> - error_report("FDT: Failed to create subnode %s: %s", name, >> - fdt_strerror(retval)); >> + error_report("%s: Failed to create subnode %s: %s", >> + __func__, name, fdt_strerror(retval)); >> exit(1); >> } >> >> @@ -549,6 +549,46 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) >> return retval; >> } >> >> +/* >> + * qemu_fdt_add_path: Like qemu_fdt_add_subnode(), but will add >> + * all missing subnodes from the given path. >> + */ >> +int qemu_fdt_add_path(void *fdt, const char *path) >> +{ >> + const char *name; >> + const char *p = path; >> + int namelen, retval; >> + int parent = 0; >> + >> + if (path[0] != '/') { >> + return -1; >> + } >> + >> + while (p) { >> + name = p + 1; >> + p = strchr(name, '/'); >> + namelen = p != NULL ? p - name : strlen(name); >> + >> + retval = fdt_subnode_offset_namelen(fdt, parent, name, namelen); >> + if (retval < 0 && retval != -FDT_ERR_NOTFOUND) { >> + error_report("%s: Unexpected error in finding subnode %.*s: %s", >> + __func__, namelen, name, fdt_strerror(retval)); >> + exit(1); >> + } else if (retval == -FDT_ERR_NOTFOUND) { >> + retval = fdt_add_subnode_namelen(fdt, parent, name, namelen); >> + if (retval < 0) { >> + error_report("%s: Failed to create subnode %.*s: %s", >> + __func__, namelen, name, fdt_strerror(retval)); >> + exit(1); >> + } >> + } >> + >> + parent = retval; >> + } >> + >> + return retval; >> +} >> + >> void qemu_fdt_dumpdtb(void *fdt, int size) >> { >> const char *dumpdtb = current_machine->dumpdtb;
On 2021/5/17 14:27, Andrew Jones wrote: > On Sun, May 16, 2021 at 06:28:53PM +0800, Yanan Wang wrote: >> From: Andrew Jones <drjones@redhat.com> > Hi Yanan, > > This looks good, but the authorship is no longer correct. You've > completely rewritten it, so I think the most I deserve is a > Co-developed-by and maybe even just a Suggested-by. When changing > the authorship and tags, you can also add a Hi Drew, Thanks for pointing out this, I will correct it. > Reviewed-by: Andrew Jones <drjones@redhat.com> Thanks, Yanan > Thanks, > drew > > >> qemu_fdt_add_path() works like qemu_fdt_add_subnode(), except it >> also adds all missing subnodes from the given path. We'll use it >> in a coming patch where we will add cpu-map to the device tree. >> >> And we also tweak an error message of qemu_fdt_add_subnode(). >> >> Cc: David Gibson <david@gibson.dropbear.id.au> >> Cc: Alistair Francis <alistair.francis@wdc.com> >> Signed-off-by: Andrew Jones <drjones@redhat.com> >> Co-developed-by: Yanan Wang <wangyanan55@huawei.com> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> --- >> include/sysemu/device_tree.h | 1 + >> softmmu/device_tree.c | 44 ++++++++++++++++++++++++++++++++++-- >> 2 files changed, 43 insertions(+), 2 deletions(-) >> >> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h >> index 8a2fe55622..ef060a9759 100644 >> --- a/include/sysemu/device_tree.h >> +++ b/include/sysemu/device_tree.h >> @@ -121,6 +121,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path); >> uint32_t qemu_fdt_alloc_phandle(void *fdt); >> int qemu_fdt_nop_node(void *fdt, const char *node_path); >> int qemu_fdt_add_subnode(void *fdt, const char *name); >> +int qemu_fdt_add_path(void *fdt, const char *path); >> >> #define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \ >> do { \ >> diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c >> index b621f63fba..3965c834ca 100644 >> --- a/softmmu/device_tree.c >> +++ b/softmmu/device_tree.c >> @@ -540,8 +540,8 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) >> >> retval = fdt_add_subnode(fdt, parent, basename); >> if (retval < 0) { >> - error_report("FDT: Failed to create subnode %s: %s", name, >> - fdt_strerror(retval)); >> + error_report("%s: Failed to create subnode %s: %s", >> + __func__, name, fdt_strerror(retval)); >> exit(1); >> } >> >> @@ -549,6 +549,46 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) >> return retval; >> } >> >> +/* >> + * qemu_fdt_add_path: Like qemu_fdt_add_subnode(), but will add >> + * all missing subnodes from the given path. >> + */ >> +int qemu_fdt_add_path(void *fdt, const char *path) >> +{ >> + const char *name; >> + const char *p = path; >> + int namelen, retval; >> + int parent = 0; >> + >> + if (path[0] != '/') { >> + return -1; >> + } >> + >> + while (p) { >> + name = p + 1; >> + p = strchr(name, '/'); >> + namelen = p != NULL ? p - name : strlen(name); >> + >> + retval = fdt_subnode_offset_namelen(fdt, parent, name, namelen); >> + if (retval < 0 && retval != -FDT_ERR_NOTFOUND) { >> + error_report("%s: Unexpected error in finding subnode %.*s: %s", >> + __func__, namelen, name, fdt_strerror(retval)); >> + exit(1); >> + } else if (retval == -FDT_ERR_NOTFOUND) { >> + retval = fdt_add_subnode_namelen(fdt, parent, name, namelen); >> + if (retval < 0) { >> + error_report("%s: Failed to create subnode %.*s: %s", >> + __func__, namelen, name, fdt_strerror(retval)); >> + exit(1); >> + } >> + } >> + >> + parent = retval; >> + } >> + >> + return retval; >> +} >> + >> void qemu_fdt_dumpdtb(void *fdt, int size) >> { >> const char *dumpdtb = current_machine->dumpdtb; >> -- >> 2.19.1 >> > .
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h index 8a2fe55622..ef060a9759 100644 --- a/include/sysemu/device_tree.h +++ b/include/sysemu/device_tree.h @@ -121,6 +121,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path); uint32_t qemu_fdt_alloc_phandle(void *fdt); int qemu_fdt_nop_node(void *fdt, const char *node_path); int qemu_fdt_add_subnode(void *fdt, const char *name); +int qemu_fdt_add_path(void *fdt, const char *path); #define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \ do { \ diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c index b621f63fba..3965c834ca 100644 --- a/softmmu/device_tree.c +++ b/softmmu/device_tree.c @@ -540,8 +540,8 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) retval = fdt_add_subnode(fdt, parent, basename); if (retval < 0) { - error_report("FDT: Failed to create subnode %s: %s", name, - fdt_strerror(retval)); + error_report("%s: Failed to create subnode %s: %s", + __func__, name, fdt_strerror(retval)); exit(1); } @@ -549,6 +549,46 @@ int qemu_fdt_add_subnode(void *fdt, const char *name) return retval; } +/* + * qemu_fdt_add_path: Like qemu_fdt_add_subnode(), but will add + * all missing subnodes from the given path. + */ +int qemu_fdt_add_path(void *fdt, const char *path) +{ + const char *name; + const char *p = path; + int namelen, retval; + int parent = 0; + + if (path[0] != '/') { + return -1; + } + + while (p) { + name = p + 1; + p = strchr(name, '/'); + namelen = p != NULL ? p - name : strlen(name); + + retval = fdt_subnode_offset_namelen(fdt, parent, name, namelen); + if (retval < 0 && retval != -FDT_ERR_NOTFOUND) { + error_report("%s: Unexpected error in finding subnode %.*s: %s", + __func__, namelen, name, fdt_strerror(retval)); + exit(1); + } else if (retval == -FDT_ERR_NOTFOUND) { + retval = fdt_add_subnode_namelen(fdt, parent, name, namelen); + if (retval < 0) { + error_report("%s: Failed to create subnode %.*s: %s", + __func__, namelen, name, fdt_strerror(retval)); + exit(1); + } + } + + parent = retval; + } + + return retval; +} + void qemu_fdt_dumpdtb(void *fdt, int size) { const char *dumpdtb = current_machine->dumpdtb;