| Message ID | 1464960552-6645-5-git-send-email-edgar.iglesias@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Edgar, On 03/06/16 14:29, Edgar E. Iglesias wrote: > From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > Make dt_match_node match for existing properties. > We only search for the existance of the properties, not s/existance/existence/ > for specific values. [..] > diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h > index b348913..f13d186 100644 > --- a/xen/include/xen/device_tree.h > +++ b/xen/include/xen/device_tree.h > @@ -30,6 +30,7 @@ struct dt_device_match { > const char *type; > const char *compatible; > const bool_t not_available; > + const char **props; I would add a comment above the field to explain the behavior. > const void *data; > }; > > @@ -37,11 +38,13 @@ struct dt_device_match { > #define __DT_MATCH_TYPE(typ) .type = typ > #define __DT_MATCH_COMPATIBLE(compat) .compatible = compat > #define __DT_MATCH_NOT_AVAILABLE() .not_available = 1 > +#define __DT_MATCH_PROPS(p...) .props = (const char *[]) { p, NULL } Why the cast? > > #define DT_MATCH_PATH(p) { __DT_MATCH_PATH(p) } > #define DT_MATCH_TYPE(typ) { __DT_MATCH_TYPE(typ) } > #define DT_MATCH_COMPATIBLE(compat) { __DT_MATCH_COMPATIBLE(compat) } > #define DT_MATCH_NOT_AVAILABLE() { __DT_MATCH_NOT_AVAILABLE() } > +#define DT_MATCH_PROPS(p...) { __DT_MATCH_PROPS(p) } > > typedef u32 dt_phandle; Regards,
On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote: > Hi Edgar, > > On 03/06/16 14:29, Edgar E. Iglesias wrote: > >From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> > > > >Make dt_match_node match for existing properties. > >We only search for the existance of the properties, not > > s/existance/existence/ Fixed > > >for specific values. > > [..] > > >diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h > >index b348913..f13d186 100644 > >--- a/xen/include/xen/device_tree.h > >+++ b/xen/include/xen/device_tree.h > >@@ -30,6 +30,7 @@ struct dt_device_match { > > const char *type; > > const char *compatible; > > const bool_t not_available; > >+ const char **props; > > I would add a comment above the field to explain the behavior. Sounds good. I've added the following: /* * NULL terminated array of property names to search for. * We only search for the properties existence. */ const char **props; > > > const void *data; > > }; > > > >@@ -37,11 +38,13 @@ struct dt_device_match { > > #define __DT_MATCH_TYPE(typ) .type = typ > > #define __DT_MATCH_COMPATIBLE(compat) .compatible = compat > > #define __DT_MATCH_NOT_AVAILABLE() .not_available = 1 > >+#define __DT_MATCH_PROPS(p...) .props = (const char *[]) { p, NULL } > > Why the cast? AFAIK, it's needed to instantiate the dynamically sized array of pointers. Another option is to make __DT_MATCH_PROPS take the char ** pointer. The descriptor declaration would instead of looking like this: { __DT_MATCH_COMPATIBLE("mmio-sram"), __DT_MATCH_PROPS("no-memory-wc"), .data = &mattr_device_rw, }, Look something like this: const char *props_no_mem_wc[] = { "no-memory-wc", NULL }; .... { __DT_MATCH_COMPATIBLE("mmio-sram"), __DT_MATCH_PROPS(props_no_mem_wc), .data = &mattr_device_rw, }, Or do you have better suggestions? Best regards, Edgar > > > > > #define DT_MATCH_PATH(p) { __DT_MATCH_PATH(p) } > > #define DT_MATCH_TYPE(typ) { __DT_MATCH_TYPE(typ) } > > #define DT_MATCH_COMPATIBLE(compat) { __DT_MATCH_COMPATIBLE(compat) } > > #define DT_MATCH_NOT_AVAILABLE() { __DT_MATCH_NOT_AVAILABLE() } > >+#define DT_MATCH_PROPS(p...) { __DT_MATCH_PROPS(p) } > > > > typedef u32 dt_phandle; > > Regards, > > -- > Julien Grall
Hi Edgar, On 07/06/2016 21:43, Edgar E. Iglesias wrote: > On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote: >> On 03/06/16 14:29, Edgar E. Iglesias wrote: [...] > AFAIK, it's needed to instantiate the dynamically sized array of pointers. > Another option is to make __DT_MATCH_PROPS take the char ** pointer. > The descriptor declaration would instead of looking like this: > { > __DT_MATCH_COMPATIBLE("mmio-sram"), > __DT_MATCH_PROPS("no-memory-wc"), > .data = &mattr_device_rw, > }, > > Look something like this: > > const char *props_no_mem_wc[] = { "no-memory-wc", NULL }; > > .... > > { > __DT_MATCH_COMPATIBLE("mmio-sram"), > __DT_MATCH_PROPS(props_no_mem_wc), > .data = &mattr_device_rw, > }, > > > Or do you have better suggestions? How about defining props with the type "const char *props[]"? Regards,
On Wed, Jun 08, 2016 at 09:44:51AM +0100, Julien Grall wrote: > Hi Edgar, Hi Julien, > > On 07/06/2016 21:43, Edgar E. Iglesias wrote: > >On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote: > >>On 03/06/16 14:29, Edgar E. Iglesias wrote: > > [...] > > >AFAIK, it's needed to instantiate the dynamically sized array of pointers. > >Another option is to make __DT_MATCH_PROPS take the char ** pointer. > >The descriptor declaration would instead of looking like this: > > { > > __DT_MATCH_COMPATIBLE("mmio-sram"), > > __DT_MATCH_PROPS("no-memory-wc"), > > .data = &mattr_device_rw, > > }, > > > >Look something like this: > > > >const char *props_no_mem_wc[] = { "no-memory-wc", NULL }; > > > >.... > > > > { > > __DT_MATCH_COMPATIBLE("mmio-sram"), > > __DT_MATCH_PROPS(props_no_mem_wc), > > .data = &mattr_device_rw, > > }, > > > > > >Or do you have better suggestions? > > How about defining props with the type "const char *props[]"? > That doesn't work for arrays of match descriptors (i.e you can't have arrays of variable sized objects)... Cheers, Edgar
Hi Edgar, On 09/06/16 17:04, Edgar E. Iglesias wrote: > On Wed, Jun 08, 2016 at 09:44:51AM +0100, Julien Grall wrote: >> >> On 07/06/2016 21:43, Edgar E. Iglesias wrote: >>> On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote: >>>> On 03/06/16 14:29, Edgar E. Iglesias wrote: >> >> [...] >> >>> AFAIK, it's needed to instantiate the dynamically sized array of pointers. >>> Another option is to make __DT_MATCH_PROPS take the char ** pointer. >>> The descriptor declaration would instead of looking like this: >>> { >>> __DT_MATCH_COMPATIBLE("mmio-sram"), >>> __DT_MATCH_PROPS("no-memory-wc"), >>> .data = &mattr_device_rw, >>> }, >>> >>> Look something like this: >>> >>> const char *props_no_mem_wc[] = { "no-memory-wc", NULL }; >>> >>> .... >>> >>> { >>> __DT_MATCH_COMPATIBLE("mmio-sram"), >>> __DT_MATCH_PROPS(props_no_mem_wc), >>> .data = &mattr_device_rw, >>> }, >>> >>> >>> Or do you have better suggestions? >> >> How about defining props with the type "const char *props[]"? >> > > That doesn't work for arrays of match descriptors (i.e you can't have arrays of variable sized objects)... Hmmmm... I would rather try to avoid the cast, but the other solution you suggested does not look appealing (i.e declare separately the properties). However do you have a use case where checking multiple properties would be useful? If not, I would just handle one property for now. Regards,
On Thu, Jun 09, 2016 at 05:23:33PM +0100, Julien Grall wrote: > Hi Edgar, > > On 09/06/16 17:04, Edgar E. Iglesias wrote: > >On Wed, Jun 08, 2016 at 09:44:51AM +0100, Julien Grall wrote: > >> > >>On 07/06/2016 21:43, Edgar E. Iglesias wrote: > >>>On Mon, Jun 06, 2016 at 06:39:39PM +0100, Julien Grall wrote: > >>>>On 03/06/16 14:29, Edgar E. Iglesias wrote: > >> > >>[...] > >> > >>>AFAIK, it's needed to instantiate the dynamically sized array of pointers. > >>>Another option is to make __DT_MATCH_PROPS take the char ** pointer. > >>>The descriptor declaration would instead of looking like this: > >>> { > >>> __DT_MATCH_COMPATIBLE("mmio-sram"), > >>> __DT_MATCH_PROPS("no-memory-wc"), > >>> .data = &mattr_device_rw, > >>> }, > >>> > >>>Look something like this: > >>> > >>>const char *props_no_mem_wc[] = { "no-memory-wc", NULL }; > >>> > >>>.... > >>> > >>> { > >>> __DT_MATCH_COMPATIBLE("mmio-sram"), > >>> __DT_MATCH_PROPS(props_no_mem_wc), > >>> .data = &mattr_device_rw, > >>> }, > >>> > >>> > >>>Or do you have better suggestions? > >> > >>How about defining props with the type "const char *props[]"? > >> > > > >That doesn't work for arrays of match descriptors (i.e you can't have arrays of variable sized objects)... > > Hmmmm... I would rather try to avoid the cast, but the other solution you > suggested does not look appealing (i.e declare separately the properties). > > However do you have a use case where checking multiple properties would be > useful? If not, I would just handle one property for now. No I don't. We can do one property for now. I'll change that for v3. Thanks, Edgar
diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index 06a2837..fbee354 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -323,8 +323,9 @@ dt_match_node(const struct dt_device_match *matches, return NULL; while ( matches->path || matches->type || - matches->compatible || matches->not_available ) + matches->compatible || matches->not_available || matches->props) { + const char **props = matches->props; bool_t match = 1; if ( matches->path ) @@ -339,6 +340,12 @@ dt_match_node(const struct dt_device_match *matches, if ( matches->not_available ) match &= !dt_device_is_available(node); + while ( match && props && *props ) + { + match &= dt_find_property(node, *props, NULL) != NULL; + props++; + } + if ( match ) return matches; matches++; diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index b348913..f13d186 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -30,6 +30,7 @@ struct dt_device_match { const char *type; const char *compatible; const bool_t not_available; + const char **props; const void *data; }; @@ -37,11 +38,13 @@ struct dt_device_match { #define __DT_MATCH_TYPE(typ) .type = typ #define __DT_MATCH_COMPATIBLE(compat) .compatible = compat #define __DT_MATCH_NOT_AVAILABLE() .not_available = 1 +#define __DT_MATCH_PROPS(p...) .props = (const char *[]) { p, NULL } #define DT_MATCH_PATH(p) { __DT_MATCH_PATH(p) } #define DT_MATCH_TYPE(typ) { __DT_MATCH_TYPE(typ) } #define DT_MATCH_COMPATIBLE(compat) { __DT_MATCH_COMPATIBLE(compat) } #define DT_MATCH_NOT_AVAILABLE() { __DT_MATCH_NOT_AVAILABLE() } +#define DT_MATCH_PROPS(p...) { __DT_MATCH_PROPS(p) } typedef u32 dt_phandle;