| Message ID | 20181105153520.18528-1-eric.auger@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND,for-3.1] hw/arm/sysbus-fdt: Only call match_fn callback if the type matches | expand |
On 5 November 2018 at 15:35, Eric Auger <eric.auger@redhat.com> wrote: > Commit af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with DT > compatible value) introduced a match_fn callback which gets called > for each registered combo to check whether a sysbus device can be > dynamically instantiated. However the callback gets called even if > the device type does not match the binding combo typename field. > > Let's change the add_fdt_node() logic so that we first check the > type and if the match_fn callback is defined, then we also call it. > > Binding combos only requesting a type check do not define the > match_fn callback. > > Fixes: af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with > DT compatible value) > > Signed-off-by: Eric Auger <eric.auger@redhat.com> > Reported-by: Thomas Huth <thuth@redhat.com> > --- > hw/arm/sysbus-fdt.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c > index 0e24c803a1..a44cf7f255 100644 > --- a/hw/arm/sysbus-fdt.c > +++ b/hw/arm/sysbus-fdt.c > @@ -449,7 +449,7 @@ static bool type_match(SysBusDevice *sbdev, const BindingEntry *entry) > return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename); > } > > -#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), type_match} > +#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), NULL} > > /* list of supported dynamic sysbus bindings */ > static const BindingEntry bindings[] = { > @@ -481,10 +481,18 @@ static void add_fdt_node(SysBusDevice *sbdev, void *opaque) > for (i = 0; i < ARRAY_SIZE(bindings); i++) { > const BindingEntry *iter = &bindings[i]; > > - if (iter->match_fn(sbdev, iter)) { > - ret = iter->add_fn(sbdev, opaque); > - assert(!ret); > - return; > + if (type_match(sbdev, iter)) { > + if (iter->match_fn) { > + if (iter->match_fn(sbdev, iter)) { "if (!iter->match_fn || iter->match_fn(sbdev, iter)) {" would let you avoid duplicating the code in the body of this if and the else clause later. (Or you could have a match_always() function that always returns true instead of having to special case NULL.) > + ret = iter->add_fn(sbdev, opaque); > + assert(!ret); > + return; > + } > + } else { > + ret = iter->add_fn(sbdev, opaque); > + assert(!ret); > + return; > + } > } thanks -- PMM
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c index 0e24c803a1..a44cf7f255 100644 --- a/hw/arm/sysbus-fdt.c +++ b/hw/arm/sysbus-fdt.c @@ -449,7 +449,7 @@ static bool type_match(SysBusDevice *sbdev, const BindingEntry *entry) return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename); } -#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), type_match} +#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), NULL} /* list of supported dynamic sysbus bindings */ static const BindingEntry bindings[] = { @@ -481,10 +481,18 @@ static void add_fdt_node(SysBusDevice *sbdev, void *opaque) for (i = 0; i < ARRAY_SIZE(bindings); i++) { const BindingEntry *iter = &bindings[i]; - if (iter->match_fn(sbdev, iter)) { - ret = iter->add_fn(sbdev, opaque); - assert(!ret); - return; + if (type_match(sbdev, iter)) { + if (iter->match_fn) { + if (iter->match_fn(sbdev, iter)) { + ret = iter->add_fn(sbdev, opaque); + assert(!ret); + return; + } + } else { + ret = iter->add_fn(sbdev, opaque); + assert(!ret); + return; + } } } error_report("Device %s can not be dynamically instantiated",
Commit af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with DT compatible value) introduced a match_fn callback which gets called for each registered combo to check whether a sysbus device can be dynamically instantiated. However the callback gets called even if the device type does not match the binding combo typename field. Let's change the add_fdt_node() logic so that we first check the type and if the match_fn callback is defined, then we also call it. Binding combos only requesting a type check do not define the match_fn callback. Fixes: af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with DT compatible value) Signed-off-by: Eric Auger <eric.auger@redhat.com> Reported-by: Thomas Huth <thuth@redhat.com> --- hw/arm/sysbus-fdt.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)