On Fri, Jun 8, 2012 at 10:46 PM, Alexander Graf <ag...@suse.de> wrote: > > On 07.06.2012, at 02:28, Peter Crosthwaite wrote: > >> On Thu, Jun 7, 2012 at 1:58 AM, Alexander Graf <ag...@suse.de> wrote: >>> On 06/06/2012 07:11 AM, Peter Crosthwaite wrote: >>>> >>>> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote: >>>>> >>>>> This patch adds a helper to search for a node's phandle by its path. This >>>>> is especially useful when the phandle is part of an array, not just a >>>>> single >>>>> cell in which case qemu_devtree_setprop_phandle would be the easy choice. >>>>> >>>>> Signed-off-by: Alexander Graf<ag...@suse.de> >>>>> --- >>>>> device_tree.c | 16 +++++++++++++++- >>>>> device_tree.h | 1 + >>>>> 2 files changed, 16 insertions(+), 1 deletions(-) >>>>> >>>>> diff --git a/device_tree.c b/device_tree.c >>>>> index 6cbc5af..6745d17 100644 >>>>> --- a/device_tree.c >>>>> +++ b/device_tree.c >>>>> @@ -162,10 +162,24 @@ int qemu_devtree_setprop_string(void *fdt, const >>>>> char *node_path, >>>>> return r; >>>>> } >>>>> >>>>> +uint32_t qemu_devtree_get_phandle(void *fdt, const char *path) >>>>> +{ >>>>> + uint32_t r; >>>>> + >>>>> + r = fdt_get_phandle(fdt, findnode_nofail(fdt, path)); >>>>> + if (r<= 0) { >>>>> + fprintf(stderr, "%s: Couldn't get phandle for %s: %s\n", >>>>> __func__, >>>>> + path, fdt_strerror(r)); >>>>> + exit(1); >>>> >>>> Is it really this functions job to terminate qemu on fail? There may be >>>> scenarios where a node does not have a phandle where the client can >>>> handle that. Perhaps return -1 on error and the client has to check? >>> >>> >>> If it can, what's the point in not calling libfdt directly then? >>> >> >> Its a very good question. If the point of this function is to fail of >> error though, perhaps it should have the _nofail suffix for clarity? > > If we do a global s/qemu_devtree_/qdt/g throughout the code base, I'd be open > to add _nofail to all function names at the end :). Otherwise we'll get into > even more trouble of staying within 80 characters per line... >
Since the majority of those functions are wrappers around "fdt_" API calls, perhaps it should be: s/qemu_devtree_/qemu_fdt_/g buys you 4 chars, which should minimise the incidence of 80 char violations when adding _nofail suffixes. Do we have a large number of lines already between 78-80 chars? Regards, Peter > > Alex >