Message ID | bf861fece970b0080814788df38aff95fd9dd014.1283417726.git.yamahata@valinux.co.jp |
---|---|
State | New |
Headers | show |
Please excuse my late reply. I'm so much behind in this list, it's not funny anymore. I agree with Anthony, this series looks nice. A few remarks inline. Isaku Yamahata <yamahata@valinux.co.jp> writes: > Make qbus_walk_children() call busfn for root bus. Please don't repeat the subject in the body. While I'm nit-picking about commit messages: subject is a heading, and as such it shouldn't end in a period. > and it fixes qbus_realize_all(). Can't see qbus_realize_all() in master; I guess it's in Anthony's tree. > The current qbus_walk_children() doesn't call busfn for root bus. > It cause qbus_relialize_all() to fail to call realize the system bus. Do you mean qbus_realize_all()? > This patch also refactor qbus_walk_children() a bit. > Another only user of busfn, qbus_find_child_bus(), isn't affected by this. > > Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> > --- > hw/qdev-core.h | 2 ++ > hw/qdev.c | 51 ++++++++++++++++++++++++++++++--------------------- > 2 files changed, 32 insertions(+), 21 deletions(-) > > diff --git a/hw/qdev-core.h b/hw/qdev-core.h > index 5fdee3a..a9b7692 100644 > --- a/hw/qdev-core.h > +++ b/hw/qdev-core.h > @@ -186,6 +186,8 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name); > /* Returns > 0 if either devfn or busfn terminate walk, 0 otherwise. */ > int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn, > qbus_walkerfn *busfn, void *opaque); > +int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn, > + qbus_walkerfn *busfn, void *opaque); > > DeviceState *qbus_find_child_dev(BusState *bus, const char *id); > BusState *qbus_find_child_bus(BusState *bus, const char *id); > diff --git a/hw/qdev.c b/hw/qdev.c > index a981e05..c1ba6b8 100644 > --- a/hw/qdev.c > +++ b/hw/qdev.c > @@ -296,33 +296,42 @@ int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn, > qbus_walkerfn *busfn, void *opaque) > { > DeviceState *dev; > + int err; > + > + if (busfn) { > + err = busfn(bus, opaque); > + if (err) { > + return err; > + } > + } > > QLIST_FOREACH(dev, &bus->children, sibling) { > - BusState *child; > - int err = 0; > + err = qdev_walk_children(dev, devfn, busfn, opaque); > + if (err < 0) { > + return err; > + } > + } > > - if (devfn) { > - err = devfn(dev, opaque); > + return 0; > +} > + > +int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn, > + qbus_walkerfn *busfn, void *opaque) > +{ > + BusState *bus; > + int err; > + > + if (devfn) { > + err = devfn(dev, opaque); > + if (err) { > + return err; > } > + } > > - if (err > 0) { > + QLIST_FOREACH(bus, &dev->child_bus, sibling) { > + err = qbus_walk_children(bus, devfn, busfn, opaque); > + if (err < 0) { > return err; > - } else if (err == 0) { > - QLIST_FOREACH(child, &dev->child_bus, sibling) { > - if (busfn) { > - err = busfn(child, opaque); > - if (err > 0) { > - return err; > - } > - } > - > - if (err == 0) { > - err = qbus_walk_children(child, devfn, busfn, opaque); > - if (err > 0) { > - return err; > - } > - } > - } > } > } Isn't it funny that functions that work on a qdev sub-trees always come in pairs? qdev_foo() and qbus_foo(). That's because of the split between device and bus nodes.
diff --git a/hw/qdev-core.h b/hw/qdev-core.h index 5fdee3a..a9b7692 100644 --- a/hw/qdev-core.h +++ b/hw/qdev-core.h @@ -186,6 +186,8 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name); /* Returns > 0 if either devfn or busfn terminate walk, 0 otherwise. */ int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn, qbus_walkerfn *busfn, void *opaque); +int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn, + qbus_walkerfn *busfn, void *opaque); DeviceState *qbus_find_child_dev(BusState *bus, const char *id); BusState *qbus_find_child_bus(BusState *bus, const char *id); diff --git a/hw/qdev.c b/hw/qdev.c index a981e05..c1ba6b8 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -296,33 +296,42 @@ int qbus_walk_children(BusState *bus, qdev_walkerfn *devfn, qbus_walkerfn *busfn, void *opaque) { DeviceState *dev; + int err; + + if (busfn) { + err = busfn(bus, opaque); + if (err) { + return err; + } + } QLIST_FOREACH(dev, &bus->children, sibling) { - BusState *child; - int err = 0; + err = qdev_walk_children(dev, devfn, busfn, opaque); + if (err < 0) { + return err; + } + } - if (devfn) { - err = devfn(dev, opaque); + return 0; +} + +int qdev_walk_children(DeviceState *dev, qdev_walkerfn *devfn, + qbus_walkerfn *busfn, void *opaque) +{ + BusState *bus; + int err; + + if (devfn) { + err = devfn(dev, opaque); + if (err) { + return err; } + } - if (err > 0) { + QLIST_FOREACH(bus, &dev->child_bus, sibling) { + err = qbus_walk_children(bus, devfn, busfn, opaque); + if (err < 0) { return err; - } else if (err == 0) { - QLIST_FOREACH(child, &dev->child_bus, sibling) { - if (busfn) { - err = busfn(child, opaque); - if (err > 0) { - return err; - } - } - - if (err == 0) { - err = qbus_walk_children(child, devfn, busfn, opaque); - if (err > 0) { - return err; - } - } - } } }
Make qbus_walk_children() call busfn for root bus. and it fixes qbus_realize_all(). The current qbus_walk_children() doesn't call busfn for root bus. It cause qbus_relialize_all() to fail to call realize the system bus. This patch also refactor qbus_walk_children() a bit. Another only user of busfn, qbus_find_child_bus(), isn't affected by this. Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp> --- hw/qdev-core.h | 2 ++ hw/qdev.c | 51 ++++++++++++++++++++++++++++++--------------------- 2 files changed, 32 insertions(+), 21 deletions(-)