diff mbox

[v4,1/2] qdev: Implement named GPIOs

Message ID bcd74548d2b6787d7e9704a5548a606a87402c8a.1399855985.git.peter.crosthwaite@xilinx.com
State New
Headers show

Commit Message

Peter Crosthwaite May 12, 2014, 1:03 a.m. UTC 
Implement named GPIOs on the Device layer. Listifies the existing GPIOs
stuff using string keys. Legacy un-named GPIOs are preserved by using
a NULL name string - they are just a single matchable element in the
name list.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed since v3 (PMM review):
Added finalize() cleanup of all allocated mem
Remove %s printf of NULL in info qtree
Restricted Qtest to unnamed GPIOs only
Changed since v2:
Reordered fn args to be name-before-number (PMM review)
changed since v1:
Use QLIST instead of RYO list implementation (AF review)
Reorder struct fields for consistency

 hw/core/qdev.c         | 82 ++++++++++++++++++++++++++++++++++++++++++++------
 include/hw/qdev-core.h | 24 ++++++++++++---
 qdev-monitor.c         | 16 +++++++---
 qtest.c                | 19 +++++++++---
 4 files changed, 117 insertions(+), 24 deletions(-)

Comments

Peter Maydell May 16, 2014, 3:18 p.m. UTC | #1 
On 12 May 2014 02:03, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> Implement named GPIOs on the Device layer. Listifies the existing GPIOs
> stuff using string keys. Legacy un-named GPIOs are preserved by using
> a NULL name string - they are just a single matchable element in the
> name list.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Two very minor nits involving comments below, but otherwise

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

and I would be happy for these patches to be committed
(through the QOM tree?)

> @@ -854,10 +907,19 @@ static void device_post_init(Object *obj)
>  /* Unlink device from bus and free the structure.  */
>  static void device_finalize(Object *obj)
>  {
> +    NamedGPIOList *ngl, *next;
> +
>      DeviceState *dev = DEVICE(obj);
>      if (dev->opts) {
>          qemu_opts_del(dev->opts);
>      }
> +
> +    QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
> +        QLIST_REMOVE(ngl, node);
> +        qemu_free_irqs(ngl->in);
> +        g_free(ngl->name);
> +        g_free(ngl);

I think we could use a comment along the lines of
    /* ngl->out irqs are owned by the other end and should not be freed here */

to stop somebody later on wondering whether it's a bug
that we don't clean up that field in NamedGPIOList.

> @@ -253,10 +254,18 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>             return;
>          }
>
> -        if (words[0][14] == 'o') {
> -            qemu_irq_intercept_out(&dev->gpio_out, qtest_irq_handler, dev->num_gpio_out);
> -        } else {
> -            qemu_irq_intercept_in(dev->gpio_in, qtest_irq_handler, dev->num_gpio_in);
> +        QLIST_FOREACH(ngl, &dev->gpios, node) {
> +            /* We dont support intercept of named GPIOs yet */

"don't".

thanks
-- PMM
diff mbox

Patch

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 936eae6..8efeb04 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -312,30 +312,82 @@  BusState *qdev_get_parent_bus(DeviceState *dev)
     return dev->parent_bus;
 }
 
+static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
+                                               const char *name)
+{
+    NamedGPIOList *ngl;
+
+    QLIST_FOREACH(ngl, &dev->gpios, node) {
+        /* NULL is a valid and matchable name, otherwise do a normal
+         * strcmp match.
+         */
+        if ((!ngl->name && !name) ||
+                (name && ngl->name && !strcmp(name, ngl->name))) {
+            return ngl;
+        }
+    }
+
+    ngl = g_malloc0(sizeof(*ngl));
+    ngl->name = g_strdup(name);
+    QLIST_INSERT_HEAD(&dev->gpios, ngl, node);
+    return ngl;
+}
+
+void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
+                             const char *name, int n)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
+                                     dev, n);
+    gpio_list->num_in += n;
+}
+
 void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
 {
-    dev->gpio_in = qemu_extend_irqs(dev->gpio_in, dev->num_gpio_in, handler,
-                                        dev, n);
-    dev->num_gpio_in += n;
+    qdev_init_gpio_in_named(dev, handler, NULL, n);
+}
+
+void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
+                              const char *name, int n)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(gpio_list->num_out == 0);
+    gpio_list->num_out = n;
+    gpio_list->out = pins;
 }
 
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
 {
-    assert(dev->num_gpio_out == 0);
-    dev->num_gpio_out = n;
-    dev->gpio_out = pins;
+    qdev_init_gpio_out_named(dev, pins, NULL, n);
+}
+
+qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(n >= 0 && n < gpio_list->num_in);
+    return gpio_list->in[n];
 }
 
 qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
 {
-    assert(n >= 0 && n < dev->num_gpio_in);
-    return dev->gpio_in[n];
+    return qdev_get_gpio_in_named(dev, NULL, n);
+}
+
+void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
+                                 qemu_irq pin)
+{
+    NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+
+    assert(n >= 0 && n < gpio_list->num_out);
+    gpio_list->out[n] = pin;
 }
 
 void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
 {
-    assert(n >= 0 && n < dev->num_gpio_out);
-    dev->gpio_out[n] = pin;
+    qdev_connect_gpio_out_named(dev, NULL, n, pin);
 }
 
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
@@ -844,6 +896,7 @@  static void device_initfn(Object *obj)
     object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
                              (Object **)&dev->parent_bus, NULL, 0,
                              &error_abort);
+    QLIST_INIT(&dev->gpios);
 }
 
 static void device_post_init(Object *obj)
@@ -854,10 +907,19 @@  static void device_post_init(Object *obj)
 /* Unlink device from bus and free the structure.  */
 static void device_finalize(Object *obj)
 {
+    NamedGPIOList *ngl, *next;
+
     DeviceState *dev = DEVICE(obj);
     if (dev->opts) {
         qemu_opts_del(dev->opts);
     }
+
+    QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
+        QLIST_REMOVE(ngl, node);
+        qemu_free_irqs(ngl->in);
+        g_free(ngl->name);
+        g_free(ngl);
+    }
 }
 
 static void device_class_base_init(ObjectClass *class, void *data)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index dbe473c..ae31575 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -131,6 +131,17 @@  typedef struct DeviceClass {
     const char *bus_type;
 } DeviceClass;
 
+typedef struct NamedGPIOList NamedGPIOList;
+
+struct NamedGPIOList {
+    char *name;
+    qemu_irq *in;
+    int num_in;
+    qemu_irq *out;
+    int num_out;
+    QLIST_ENTRY(NamedGPIOList) node;
+};
+
 /**
  * DeviceState:
  * @realized: Indicates whether the device has been fully constructed.
@@ -148,10 +159,7 @@  struct DeviceState {
     QemuOpts *opts;
     int hotplugged;
     BusState *parent_bus;
-    int num_gpio_out;
-    qemu_irq *gpio_out;
-    int num_gpio_in;
-    qemu_irq *gpio_in;
+    QLIST_HEAD(, NamedGPIOList) gpios;
     QLIST_HEAD(, BusState) child_bus;
     int num_child_bus;
     int instance_id_alias;
@@ -252,7 +260,11 @@  void qdev_machine_creation_done(void);
 bool qdev_machine_modified(void);
 
 qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
+qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
+
 void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
+void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
+                                 qemu_irq pin);
 
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 
@@ -262,6 +274,10 @@  BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 /* GPIO inputs also double as IRQ sinks.  */
 void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
+void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
+                             const char *name, int n);
+void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
+                              const char *name, int n);
 
 BusState *qdev_get_parent_bus(DeviceState *dev);
 
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 02cbe43..f87f3d8 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -613,14 +613,20 @@  static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 {
     ObjectClass *class;
     BusState *child;
+    NamedGPIOList *ngl;
+
     qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
                 dev->id ? dev->id : "");
     indent += 2;
-    if (dev->num_gpio_in) {
-        qdev_printf("gpio-in %d\n", dev->num_gpio_in);
-    }
-    if (dev->num_gpio_out) {
-        qdev_printf("gpio-out %d\n", dev->num_gpio_out);
+    QLIST_FOREACH(ngl, &dev->gpios, node) {
+        if (ngl->num_in) {
+            qdev_printf("gpio-in \"%s\" %d\n", ngl->name ? ngl->name : "",
+                        ngl->num_in);
+        }
+        if (ngl->num_out) {
+            qdev_printf("gpio-out \"%s\" %d\n", ngl->name ? ngl->name : "",
+                        ngl->num_out);
+        }
     }
     class = object_get_class(OBJECT(dev));
     do {
diff --git a/qtest.c b/qtest.c
index 2aba20d..b6b60d5 100644
--- a/qtest.c
+++ b/qtest.c
@@ -233,7 +233,8 @@  static void qtest_process_command(CharDriverState *chr, gchar **words)
     g_assert(command);
     if (strcmp(words[0], "irq_intercept_out") == 0
         || strcmp(words[0], "irq_intercept_in") == 0) {
-	DeviceState *dev;
+        DeviceState *dev;
+        NamedGPIOList *ngl;
 
         g_assert(words[1]);
         dev = DEVICE(object_resolve_path(words[1], NULL));
@@ -253,10 +254,18 @@  static void qtest_process_command(CharDriverState *chr, gchar **words)
 	    return;
         }
 
-        if (words[0][14] == 'o') {
-            qemu_irq_intercept_out(&dev->gpio_out, qtest_irq_handler, dev->num_gpio_out);
-        } else {
-            qemu_irq_intercept_in(dev->gpio_in, qtest_irq_handler, dev->num_gpio_in);
+        QLIST_FOREACH(ngl, &dev->gpios, node) {
+            /* We dont support intercept of named GPIOs yet */
+            if (ngl->name) {
+                continue;
+            }
+            if (words[0][14] == 'o') {
+                qemu_irq_intercept_out(&ngl->out, qtest_irq_handler,
+                                       ngl->num_out);
+            } else {
+                qemu_irq_intercept_in(ngl->in, qtest_irq_handler,
+                                      ngl->num_in);
+            }
         }
         irq_intercept_dev = dev;
         qtest_send_prefix(chr);