| Message ID | 1338940402-28502-3-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote: > We have device tree helpers that allow us to create single cell (u32) > wide properties. However, when creating properties that contain an array of > cells, we need to jump through hoops, manually passing in an array with > converted endianness. > > To ease the pain of this a bit, create helpers for the most common array > sizes, namely 2, 3 and 4 cells wide properties. > > Signed-off-by: Alexander Graf <agraf@suse.de> > --- > device_tree.c | 30 ++++++++++++++++++++++++++++++ > device_tree.h | 9 +++++++++ > 2 files changed, 39 insertions(+), 0 deletions(-) > > diff --git a/device_tree.c b/device_tree.c > index 94a239e..b1dff4f 100644 > --- a/device_tree.c > +++ b/device_tree.c > @@ -117,6 +117,36 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path, > return r; > } > > +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path, > + const char *property, uint32_t val, > + uint32_t val2) > +{ > + uint32_t tmp[] = { cpu_to_be32(val), > + cpu_to_be32(val2) }; > + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); > +} > + > +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path, > + const char *property, uint32_t val, > + uint32_t val2, uint32_t val3) > +{ > + uint32_t tmp[] = { cpu_to_be32(val), > + cpu_to_be32(val2), > + cpu_to_be32(val3) }; > + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); > +} > + > +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path, > + const char *property, uint32_t val, > + uint32_t val2, uint32_t val3, uint32_t val4) > +{ > + uint32_t tmp[] = { cpu_to_be32(val), > + cpu_to_be32(val2), > + cpu_to_be32(val3), > + cpu_to_be32(val4) }; > + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); > +} > + Cant this be generalised to the n case rather than having functional replication for 2/3/4 word props? +int qemu_devtree_setprop_celln(void *fdt, const char *node_path, + const char *property, uint32_t *vals, int n) > int qemu_devtree_setprop_string(void *fdt, const char *node_path, > const char *property, const char *string) > { > diff --git a/device_tree.h b/device_tree.h > index 4378685..9db7f86 100644 > --- a/device_tree.h > +++ b/device_tree.h > @@ -20,6 +20,15 @@ int qemu_devtree_setprop(void *fdt, const char *node_path, > const char *property, void *val_array, int size); > int qemu_devtree_setprop_cell(void *fdt, const char *node_path, > const char *property, uint32_t val); > +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path, > + const char *property, uint32_t val, > + uint32_t val2); > +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path, > + const char *property, uint32_t val, > + uint32_t val2, uint32_t val3); > +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path, > + const char *property, uint32_t val, > + uint32_t val2, uint32_t val3, uint32_t val4); > int qemu_devtree_setprop_string(void *fdt, const char *node_path, > const char *property, const char *string); > int qemu_devtree_nop_node(void *fdt, const char *node_path);
On 06/06/2012 07:01 AM, Peter Crosthwaite wrote: > On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote: >> We have device tree helpers that allow us to create single cell (u32) >> wide properties. However, when creating properties that contain an array of >> cells, we need to jump through hoops, manually passing in an array with >> converted endianness. >> >> To ease the pain of this a bit, create helpers for the most common array >> sizes, namely 2, 3 and 4 cells wide properties. >> >> Signed-off-by: Alexander Graf<agraf@suse.de> >> --- >> device_tree.c | 30 ++++++++++++++++++++++++++++++ >> device_tree.h | 9 +++++++++ >> 2 files changed, 39 insertions(+), 0 deletions(-) >> >> diff --git a/device_tree.c b/device_tree.c >> index 94a239e..b1dff4f 100644 >> --- a/device_tree.c >> +++ b/device_tree.c >> @@ -117,6 +117,36 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path, >> return r; >> } >> >> +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path, >> + const char *property, uint32_t val, >> + uint32_t val2) >> +{ >> + uint32_t tmp[] = { cpu_to_be32(val), >> + cpu_to_be32(val2) }; >> + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); >> +} >> + >> +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path, >> + const char *property, uint32_t val, >> + uint32_t val2, uint32_t val3) >> +{ >> + uint32_t tmp[] = { cpu_to_be32(val), >> + cpu_to_be32(val2), >> + cpu_to_be32(val3) }; >> + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); >> +} >> + >> +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path, >> + const char *property, uint32_t val, >> + uint32_t val2, uint32_t val3, uint32_t val4) >> +{ >> + uint32_t tmp[] = { cpu_to_be32(val), >> + cpu_to_be32(val2), >> + cpu_to_be32(val3), >> + cpu_to_be32(val4) }; >> + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); >> +} >> + > Cant this be generalised to the n case rather than having functional > replication for 2/3/4 word props? > > +int qemu_devtree_setprop_celln(void *fdt, const char *node_path, > + const char *property, uint32_t *vals, int n) You mean internally? Yeah, probably. Externally? The point of these helpers is to make the code look less cluttered. We can already pass in an array just fine, but C is quite annoying about generating those on the fly, while it's easy to pass in ints as parameters :) Alex
On 06/06/2012 10:55 AM, Alexander Graf wrote: > On 06/06/2012 07:01 AM, Peter Crosthwaite wrote: >> On Wed, 2012-06-06 at 01:52 +0200, Alexander Graf wrote: >>> We have device tree helpers that allow us to create single cell (u32) >>> wide properties. However, when creating properties that contain an >>> array of >>> cells, we need to jump through hoops, manually passing in an array with >>> converted endianness. >>> >>> To ease the pain of this a bit, create helpers for the most common array >>> sizes, namely 2, 3 and 4 cells wide properties. >>> >>> Signed-off-by: Alexander Graf<agraf@suse.de> >>> --- >>> device_tree.c | 30 ++++++++++++++++++++++++++++++ >>> device_tree.h | 9 +++++++++ >>> 2 files changed, 39 insertions(+), 0 deletions(-) >>> >>> diff --git a/device_tree.c b/device_tree.c >>> index 94a239e..b1dff4f 100644 >>> --- a/device_tree.c >>> +++ b/device_tree.c >>> @@ -117,6 +117,36 @@ int qemu_devtree_setprop_cell(void *fdt, const >>> char *node_path, >>> return r; >>> } >>> >>> +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path, >>> + const char *property, uint32_t val, >>> + uint32_t val2) >>> +{ >>> + uint32_t tmp[] = { cpu_to_be32(val), >>> + cpu_to_be32(val2) }; >>> + return qemu_devtree_setprop(fdt, node_path, property, tmp, >>> sizeof(tmp)); >>> +} From the subject line I was expecting these to add multi-cell numbers together (e.g. for ranges parsing). >>> + >>> +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path, >>> + const char *property, uint32_t val, >>> + uint32_t val2, uint32_t val3) >>> +{ >>> + uint32_t tmp[] = { cpu_to_be32(val), >>> + cpu_to_be32(val2), >>> + cpu_to_be32(val3) }; >>> + return qemu_devtree_setprop(fdt, node_path, property, tmp, >>> sizeof(tmp)); >>> +} >>> + >>> +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path, >>> + const char *property, uint32_t val, >>> + uint32_t val2, uint32_t val3, >>> uint32_t val4) >>> +{ >>> + uint32_t tmp[] = { cpu_to_be32(val), >>> + cpu_to_be32(val2), >>> + cpu_to_be32(val3), >>> + cpu_to_be32(val4) }; >>> + return qemu_devtree_setprop(fdt, node_path, property, tmp, >>> sizeof(tmp)); >>> +} >>> + >> Cant this be generalised to the n case rather than having functional >> replication for 2/3/4 word props? >> >> +int qemu_devtree_setprop_celln(void *fdt, const char *node_path, >> + const char *property, uint32_t *vals, >> int n) > > You mean internally? Yeah, probably. Externally? The point of these > helpers is to make the code look less cluttered. We can already pass in > an array just fine, but C is quite annoying about generating those on > the fly, while it's easy to pass in ints as parameters :) Varargs? -Scott
[snip] > > You mean internally? Yeah, probably. Externally? The point of these > > helpers is to make the code look less cluttered. We can already pass in > > an array just fine, but C is quite annoying about generating those on > > the fly, while it's easy to pass in ints as parameters :) > > Varargs? Ugly and risky with standard C varargs (because an explicit length would be needed). Could be done neatly with gcc macro varargs.
On 07.06.2012, at 01:45, David Gibson wrote: > [snip] >>> You mean internally? Yeah, probably. Externally? The point of these >>> helpers is to make the code look less cluttered. We can already pass in >>> an array just fine, but C is quite annoying about generating those on >>> the fly, while it's easy to pass in ints as parameters :) >> >> Varargs? > > Ugly and risky with standard C varargs (because an explicit length > would be needed). Could be done neatly with gcc macro varargs. Could a combination of both like this work? #include <stdio.h> #include <stdarg.h> #define __VA_NARG__(...) \ (__VA_NARG_(_0, ## __VA_ARGS__, __RSEQ_N()) - 1) #define __VA_NARG_(...) \ __VA_ARG_N(__VA_ARGS__) #define __VA_ARG_N( \ _1, _2, _3, _4, _5, _6, _7, _8, _9,_10, \ _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \ _21,_22,_23,_24,_25,_26,_27,_28,_29,_30, \ _31,_32,_33,_34,_35,_36,_37,_38,_39,_40, \ _41,_42,_43,_44,_45,_46,_47,_48,_49,_50, \ _51,_52,_53,_54,_55,_56,_57,_58,_59,_60, \ _61,_62,_63,N,...) N #define __RSEQ_N() \ 63, 62, 61, 60, \ 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \ 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \ 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \ 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \ 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \ 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 #define PRINT_ELEMS(fdt, ...) print_elems(fdt, __VA_NARG__(__VA_ARGS__), __VA_ARGS__) int print_elems(void *fdt, unsigned int count, ...) { int i; va_list argp; va_start(argp, count); for (i = 0; i < count; i++) printf("%d -> %#x\n", i, va_arg(argp, int)); va_end(argp); return 0; } int main(int argc, char **argv) { PRINT_ELEMS(NULL, 1, 2, 3, 4); return 0; }
On Thu, Jun 07, 2012 at 01:27:56PM +0200, Alexander Graf wrote: > > On 07.06.2012, at 01:45, David Gibson wrote: > > > [snip] > >>> You mean internally? Yeah, probably. Externally? The point of these > >>> helpers is to make the code look less cluttered. We can already pass in > >>> an array just fine, but C is quite annoying about generating those on > >>> the fly, while it's easy to pass in ints as parameters :) > >> > >> Varargs? > > > > Ugly and risky with standard C varargs (because an explicit length > > would be needed). Could be done neatly with gcc macro varargs. > > Could a combination of both like this work? > > #include <stdio.h> > #include <stdarg.h> > > #define __VA_NARG__(...) \ > (__VA_NARG_(_0, ## __VA_ARGS__, __RSEQ_N()) - 1) > #define __VA_NARG_(...) \ > __VA_ARG_N(__VA_ARGS__) > #define __VA_ARG_N( \ > _1, _2, _3, _4, _5, _6, _7, _8, _9,_10, \ > _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \ > _21,_22,_23,_24,_25,_26,_27,_28,_29,_30, \ > _31,_32,_33,_34,_35,_36,_37,_38,_39,_40, \ > _41,_42,_43,_44,_45,_46,_47,_48,_49,_50, \ > _51,_52,_53,_54,_55,_56,_57,_58,_59,_60, \ > _61,_62,_63,N,...) N > #define __RSEQ_N() \ > 63, 62, 61, 60, \ > 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \ > 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \ > 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \ > 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \ > 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \ > 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 > > #define PRINT_ELEMS(fdt, ...) print_elems(fdt, __VA_NARG__(__VA_ARGS__), __VA_ARGS__) Um.. that might work, but it's ludicrously complicated. If we're prepared to use the gcc statement expression extension and we're just going to abort on errors like findnode_nofail, it can be done much more easily using c99 variadic macros: #define setprop_ints(fdt, path, prop, ...) \ do { \ uint32_t tmp[] = {__VA_ARGS__}; \ if (fdt_setprop(findnode_nofail(fdt, path), prop, \ tmp, sizeof(tmp)) != 0) { \ /* error message */ \ abort(); \ } \ } while (0)
On 07.06.2012, at 14:13, David Gibson wrote: > On Thu, Jun 07, 2012 at 01:27:56PM +0200, Alexander Graf wrote: >> >> On 07.06.2012, at 01:45, David Gibson wrote: >> >>> [snip] >>>>> You mean internally? Yeah, probably. Externally? The point of these >>>>> helpers is to make the code look less cluttered. We can already pass in >>>>> an array just fine, but C is quite annoying about generating those on >>>>> the fly, while it's easy to pass in ints as parameters :) >>>> >>>> Varargs? >>> >>> Ugly and risky with standard C varargs (because an explicit length >>> would be needed). Could be done neatly with gcc macro varargs. >> >> Could a combination of both like this work? >> >> #include <stdio.h> >> #include <stdarg.h> >> >> #define __VA_NARG__(...) \ >> (__VA_NARG_(_0, ## __VA_ARGS__, __RSEQ_N()) - 1) >> #define __VA_NARG_(...) \ >> __VA_ARG_N(__VA_ARGS__) >> #define __VA_ARG_N( \ >> _1, _2, _3, _4, _5, _6, _7, _8, _9,_10, \ >> _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \ >> _21,_22,_23,_24,_25,_26,_27,_28,_29,_30, \ >> _31,_32,_33,_34,_35,_36,_37,_38,_39,_40, \ >> _41,_42,_43,_44,_45,_46,_47,_48,_49,_50, \ >> _51,_52,_53,_54,_55,_56,_57,_58,_59,_60, \ >> _61,_62,_63,N,...) N >> #define __RSEQ_N() \ >> 63, 62, 61, 60, \ >> 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \ >> 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \ >> 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \ >> 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \ >> 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \ >> 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 >> >> #define PRINT_ELEMS(fdt, ...) print_elems(fdt, __VA_NARG__(__VA_ARGS__), __VA_ARGS__) > > Um.. that might work, but it's ludicrously complicated. If we're > prepared to use the gcc statement expression extension and we're just > going to abort on errors like findnode_nofail, it can be done much > more easily using c99 variadic macros: > > #define setprop_ints(fdt, path, prop, ...) \ > do { \ > uint32_t tmp[] = {__VA_ARGS__}; \ > > if (fdt_setprop(findnode_nofail(fdt, path), prop, \ > tmp, sizeof(tmp)) != 0) { \ > /* error message */ \ > abort(); \ > } \ > } while (0) Hrm. But here we'd be overloading the name space, no? If anyone passes in tmp[3] as parameter to setprop_ints, it would conflict with the internal variable tmp, right? Alex
On Fri, Jun 08, 2012 at 03:00:56PM +0200, Alexander Graf wrote: > > On 07.06.2012, at 14:13, David Gibson wrote: > > > On Thu, Jun 07, 2012 at 01:27:56PM +0200, Alexander Graf wrote: > >> > >> On 07.06.2012, at 01:45, David Gibson wrote: > >> > >>> [snip] > >>>>> You mean internally? Yeah, probably. Externally? The point of these > >>>>> helpers is to make the code look less cluttered. We can already pass in > >>>>> an array just fine, but C is quite annoying about generating those on > >>>>> the fly, while it's easy to pass in ints as parameters :) > >>>> > >>>> Varargs? > >>> > >>> Ugly and risky with standard C varargs (because an explicit length > >>> would be needed). Could be done neatly with gcc macro varargs. > >> > >> Could a combination of both like this work? > >> > >> #include <stdio.h> > >> #include <stdarg.h> > >> > >> #define __VA_NARG__(...) \ > >> (__VA_NARG_(_0, ## __VA_ARGS__, __RSEQ_N()) - 1) > >> #define __VA_NARG_(...) \ > >> __VA_ARG_N(__VA_ARGS__) > >> #define __VA_ARG_N( \ > >> _1, _2, _3, _4, _5, _6, _7, _8, _9,_10, \ > >> _11,_12,_13,_14,_15,_16,_17,_18,_19,_20, \ > >> _21,_22,_23,_24,_25,_26,_27,_28,_29,_30, \ > >> _31,_32,_33,_34,_35,_36,_37,_38,_39,_40, \ > >> _41,_42,_43,_44,_45,_46,_47,_48,_49,_50, \ > >> _51,_52,_53,_54,_55,_56,_57,_58,_59,_60, \ > >> _61,_62,_63,N,...) N > >> #define __RSEQ_N() \ > >> 63, 62, 61, 60, \ > >> 59, 58, 57, 56, 55, 54, 53, 52, 51, 50, \ > >> 49, 48, 47, 46, 45, 44, 43, 42, 41, 40, \ > >> 39, 38, 37, 36, 35, 34, 33, 32, 31, 30, \ > >> 29, 28, 27, 26, 25, 24, 23, 22, 21, 20, \ > >> 19, 18, 17, 16, 15, 14, 13, 12, 11, 10, \ > >> 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 > >> > >> #define PRINT_ELEMS(fdt, ...) print_elems(fdt, __VA_NARG__(__VA_ARGS__), __VA_ARGS__) > > > > Um.. that might work, but it's ludicrously complicated. If we're > > prepared to use the gcc statement expression extension and we're just > > going to abort on errors like findnode_nofail, it can be done much > > more easily using c99 variadic macros: > > > > #define setprop_ints(fdt, path, prop, ...) \ > > do { \ > > uint32_t tmp[] = {__VA_ARGS__}; \ > > > > if (fdt_setprop(findnode_nofail(fdt, path), prop, \ > > tmp, sizeof(tmp)) != 0) { \ > > /* error message */ \ > > abort(); \ > > } \ > > } while (0) > > Hrm. But here we'd be overloading the name space, no? If anyone > passes in tmp[3] as parameter to setprop_ints, it would conflict > with the internal variable tmp, right? Standard macro problem. So, call it _tmp, whatever.
diff --git a/device_tree.c b/device_tree.c index 94a239e..b1dff4f 100644 --- a/device_tree.c +++ b/device_tree.c @@ -117,6 +117,36 @@ int qemu_devtree_setprop_cell(void *fdt, const char *node_path, return r; } +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path, + const char *property, uint32_t val, + uint32_t val2) +{ + uint32_t tmp[] = { cpu_to_be32(val), + cpu_to_be32(val2) }; + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); +} + +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path, + const char *property, uint32_t val, + uint32_t val2, uint32_t val3) +{ + uint32_t tmp[] = { cpu_to_be32(val), + cpu_to_be32(val2), + cpu_to_be32(val3) }; + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); +} + +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path, + const char *property, uint32_t val, + uint32_t val2, uint32_t val3, uint32_t val4) +{ + uint32_t tmp[] = { cpu_to_be32(val), + cpu_to_be32(val2), + cpu_to_be32(val3), + cpu_to_be32(val4) }; + return qemu_devtree_setprop(fdt, node_path, property, tmp, sizeof(tmp)); +} + int qemu_devtree_setprop_string(void *fdt, const char *node_path, const char *property, const char *string) { diff --git a/device_tree.h b/device_tree.h index 4378685..9db7f86 100644 --- a/device_tree.h +++ b/device_tree.h @@ -20,6 +20,15 @@ int qemu_devtree_setprop(void *fdt, const char *node_path, const char *property, void *val_array, int size); int qemu_devtree_setprop_cell(void *fdt, const char *node_path, const char *property, uint32_t val); +int qemu_devtree_setprop_cell2(void *fdt, const char *node_path, + const char *property, uint32_t val, + uint32_t val2); +int qemu_devtree_setprop_cell3(void *fdt, const char *node_path, + const char *property, uint32_t val, + uint32_t val2, uint32_t val3); +int qemu_devtree_setprop_cell4(void *fdt, const char *node_path, + const char *property, uint32_t val, + uint32_t val2, uint32_t val3, uint32_t val4); int qemu_devtree_setprop_string(void *fdt, const char *node_path, const char *property, const char *string); int qemu_devtree_nop_node(void *fdt, const char *node_path);
We have device tree helpers that allow us to create single cell (u32) wide properties. However, when creating properties that contain an array of cells, we need to jump through hoops, manually passing in an array with converted endianness. To ease the pain of this a bit, create helpers for the most common array sizes, namely 2, 3 and 4 cells wide properties. Signed-off-by: Alexander Graf <agraf@suse.de> --- device_tree.c | 30 ++++++++++++++++++++++++++++++ device_tree.h | 9 +++++++++ 2 files changed, 39 insertions(+), 0 deletions(-)