| Message ID | 1452093205-30167-4-git-send-email-eric.auger@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Hi David, On 01/11/2016 03:38 AM, David Gibson wrote: > On Wed, Jan 06, 2016 at 03:13:21PM +0000, Eric Auger wrote: >> This new helper routine returns the node path of a device >> referred to by its node name and compat string. > > What if there are multiple nodes matching the name and compat? The function would return the first one. I can improve the doc comment. Do you think it is a problem stopping at the first one? Is it a real life test case I have to handle here? Thanks Eric > >> >> Signed-off-by: Eric Auger <eric.auger@linaro.org> >> >> --- >> >> v1 -> v2: >> - move doc comment in header file >> - do not use a fixed size buffer >> - break on errors in while loop >> - use strcmp instead of strncmp >> >> RFC -> v1: >> - improve error handling according to Alex' comments >> --- >> device_tree.c | 37 +++++++++++++++++++++++++++++++++++++ >> include/sysemu/device_tree.h | 14 ++++++++++++++ >> 2 files changed, 51 insertions(+) >> >> diff --git a/device_tree.c b/device_tree.c >> index b262c2d..8441e01 100644 >> --- a/device_tree.c >> +++ b/device_tree.c >> @@ -231,6 +231,43 @@ static int findnode_nofail(void *fdt, const char *node_path) >> return offset; >> } >> >> +int qemu_fdt_node_path(void *fdt, const char *name, char *compat, >> + char **node_path) >> +{ >> + int offset, len, ret; >> + const char *iter_name; >> + unsigned int path_len = 16; >> + char *path; >> + >> + *node_path = NULL; >> + offset = fdt_node_offset_by_compatible(fdt, -1, compat); >> + >> + while (offset >= 0) { >> + iter_name = fdt_get_name(fdt, offset, &len); >> + if (!iter_name) { >> + offset = len; >> + break; >> + } >> + if (!strcmp(iter_name, name)) { >> + goto found; >> + } >> + offset = fdt_node_offset_by_compatible(fdt, offset, compat); >> + } >> + return offset; >> + >> +found: >> + path = g_malloc(path_len); >> + while ((ret = fdt_get_path(fdt, offset, path, path_len)) >> + == -FDT_ERR_NOSPACE) { >> + path_len += 16; >> + path = g_realloc(path, path_len); >> + } >> + if (!ret) { >> + *node_path = path; >> + } >> + return ret; >> +} >> + >> int qemu_fdt_setprop(void *fdt, const char *node_path, >> const char *property, const void *val, int size) >> { >> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h >> index fdf25a4..269cb1c 100644 >> --- a/include/sysemu/device_tree.h >> +++ b/include/sysemu/device_tree.h >> @@ -20,6 +20,20 @@ void *load_device_tree(const char *filename_path, int *sizep); >> void *load_device_tree_from_sysfs(void); >> #endif >> >> +/** >> + * qemu_fdt_node_path: return the node path of a device, given its >> + * node name and its compat string >> + * @fdt: pointer to the dt blob >> + * @name: device node name >> + * @compat: compatibility string of the device >> + * @node_path: returned node path >> + * >> + * upon success, the path is output at node_path address >> + * returns 0 on success, < 0 on failure >> + */ >> +int qemu_fdt_node_path(void *fdt, const char *name, char *compat, >> + char **node_path); >> + >> int qemu_fdt_setprop(void *fdt, const char *node_path, >> const char *property, const void *val, int size); >> int qemu_fdt_setprop_cell(void *fdt, const char *node_path, >
Hi David, On 01/12/2016 05:28 AM, David Gibson wrote: > On Mon, Jan 11, 2016 at 11:35:50AM +0100, Eric Auger wrote: >> Hi David, >> On 01/11/2016 03:38 AM, David Gibson wrote: >>> On Wed, Jan 06, 2016 at 03:13:21PM +0000, Eric Auger wrote: >>>> This new helper routine returns the node path of a device >>>> referred to by its node name and compat string. >>> >>> What if there are multiple nodes matching the name and compat? >> The function would return the first one. I can improve the doc comment. >> Do you think it is a problem stopping at the first one? Is it a real >> life test case I have to handle here? > > Well, I don't know of a specific system which will have this, but it's > absolutely possible to get this situation: e.g. two different PCI > busses, both of which have their own slot 0 populated with different > instances of the same device. > > Whether it's possible for platform devices will depend on the > platform's specific bus toplogies, but you certainly can't rule it out > in general. OK I will handle that case then. I hope I will be able to test it. > > I could consider adding a new libfdt function like > fdt_node_offset_by_compatible() that searches by name as well. It's > just I'm not sure that matching by name and compatible isn't a sign of > a poor approach in the caller. well I can't really comment. That looked the most straightforward to me given the current libfdt API. But not sure it's worth to invest in a new function in libfdt Thanks! Eric >
diff --git a/device_tree.c b/device_tree.c index b262c2d..8441e01 100644 --- a/device_tree.c +++ b/device_tree.c @@ -231,6 +231,43 @@ static int findnode_nofail(void *fdt, const char *node_path) return offset; } +int qemu_fdt_node_path(void *fdt, const char *name, char *compat, + char **node_path) +{ + int offset, len, ret; + const char *iter_name; + unsigned int path_len = 16; + char *path; + + *node_path = NULL; + offset = fdt_node_offset_by_compatible(fdt, -1, compat); + + while (offset >= 0) { + iter_name = fdt_get_name(fdt, offset, &len); + if (!iter_name) { + offset = len; + break; + } + if (!strcmp(iter_name, name)) { + goto found; + } + offset = fdt_node_offset_by_compatible(fdt, offset, compat); + } + return offset; + +found: + path = g_malloc(path_len); + while ((ret = fdt_get_path(fdt, offset, path, path_len)) + == -FDT_ERR_NOSPACE) { + path_len += 16; + path = g_realloc(path, path_len); + } + if (!ret) { + *node_path = path; + } + return ret; +} + int qemu_fdt_setprop(void *fdt, const char *node_path, const char *property, const void *val, int size) { diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h index fdf25a4..269cb1c 100644 --- a/include/sysemu/device_tree.h +++ b/include/sysemu/device_tree.h @@ -20,6 +20,20 @@ void *load_device_tree(const char *filename_path, int *sizep); void *load_device_tree_from_sysfs(void); #endif +/** + * qemu_fdt_node_path: return the node path of a device, given its + * node name and its compat string + * @fdt: pointer to the dt blob + * @name: device node name + * @compat: compatibility string of the device + * @node_path: returned node path + * + * upon success, the path is output at node_path address + * returns 0 on success, < 0 on failure + */ +int qemu_fdt_node_path(void *fdt, const char *name, char *compat, + char **node_path); + int qemu_fdt_setprop(void *fdt, const char *node_path, const char *property, const void *val, int size); int qemu_fdt_setprop_cell(void *fdt, const char *node_path,
This new helper routine returns the node path of a device referred to by its node name and compat string. Signed-off-by: Eric Auger <eric.auger@linaro.org> --- v1 -> v2: - move doc comment in header file - do not use a fixed size buffer - break on errors in while loop - use strcmp instead of strncmp RFC -> v1: - improve error handling according to Alex' comments --- device_tree.c | 37 +++++++++++++++++++++++++++++++++++++ include/sysemu/device_tree.h | 14 ++++++++++++++ 2 files changed, 51 insertions(+) -- 1.9.1