| Message ID | 1452093205-30167-4-git-send-email-eric.auger@linaro.org |
|---|---|
| State | New |
| Headers | show |
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? > > 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/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, >
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. 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.
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 >
On Tue, Jan 12, 2016 at 06:02:00PM +0100, Eric Auger wrote: > 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. One case where this might occur in practice for platform devices is if you have a system built with multiple instances of a SoC. A peripheral on SoC 0 is likely to have the same name and compatible as the same peripheral on Soc 1, just inside a different parent node. > > 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(+)