Patchwork [1/6] qdev: Make qbus_walk_children() call busfn for root bus.

login
register
mail settings
Submitter Isaku Yamahata
Date Sept. 2, 2010, 9:25 a.m.
Message ID <bf861fece970b0080814788df38aff95fd9dd014.1283417726.git.yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/63462/
State New
Headers show

Comments

Isaku Yamahata - Sept. 2, 2010, 9:25 a.m.
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(-)
Markus Armbruster - Sept. 23, 2010, 5:11 p.m.
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.

Patch

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;
-                    }
-                }
-            }
         }
     }