[Qemu-devel] [PATCH v2 00/14] GPIO/IRQ QOMification: Phase 2 - Getting rid of SYSBUS IRQs

[Qemu-devel] [PATCH v2 00/14] GPIO/IRQ QOMification: Phase 2 - Getting rid of SYSBUS IRQs

[Peter Crosthwaite]

Hi All,

So phase one was the QOMification of qemu_irq. This is the next step.
We start to setup GPIOs as proper QOM objects. Inputs are child objects
of their device. Outputs are settable Links and connection is made
via proper setting of a QOM link.

We then cleanup Sysbus to simply re-use device level GPIOs and get rid
of it's special IRQ handling code.

Depends of my pending QOM array property stuff (the "[*]" series):

https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg04116.html

Changed since v1:
Addressed Alex review
Dropped IRQ g_new0 changes



Peter Crosthwaite (14):
  qdev: gpio: Don't allow name share between I and O
  qdev: gpio: Register GPIO inputs as child objects
  qdev: gpio: Register GPIO outputs as QOM links
  qmp: qstring: Handle NULL strings
  qom: Allow clearing of a Link property
  qom: Demote already-has-a-parent to a regular error
  qdev: gpio: Re-impement qdev_connect_gpio QOM style
  qdev: gpio: Add API for intercepting a GPIO
  qtest/irq: Rework IRQ interception
  irq: Remove qemu_irq_intercept_out
  qdev: gpio: delete NamedGPIOList::out
  qdev: gpio: Remove qdev_init_gpio_out x1 restriction
  qdev: gpio: Define qdev_pass_gpios()
  sysbus: Use TYPE_DEVICE GPIO functionality

 hw/core/irq.c          |  8 +----
 hw/core/qdev.c         | 90 ++++++++++++++++++++++++++++++++++++++++++++++----
 hw/core/sysbus.c       | 20 ++---------
 include/hw/irq.h       |  1 -
 include/hw/qdev-core.h |  6 +++-
 include/hw/sysbus.h    |  6 ++--
 qobject/qstring.c      | 12 ++++---
 qom/object.c           | 10 ++++--
 qtest.c                | 15 ++++++---
 9 files changed, 120 insertions(+), 48 deletions(-)

-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 01/14] qdev: gpio: Don't allow name share between I and O

[Peter Crosthwaite]

Only allow a GPIO name to be one or the other. Inputs and outputs are
functionally different and should be in different namespaces. Prepares
support for the QOMification of IRQs as Links or Child objects.

The alternative is to munge names .e.g. with "-in" or "-out" suffixes
when giving QOM names. But that reduces clarity and if there are cases
out there where users want I and O with same name they can manually add
their own suffixes.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index da1ba48..77441aa 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -339,6 +339,7 @@ void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
 {
     NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
 
+    assert(gpio_list->num_out == 0 || !name);
     gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
                                      dev, n);
     gpio_list->num_in += n;
@@ -354,6 +355,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
 {
     NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
 
+    assert(gpio_list->num_in == 0 || !name);
     assert(gpio_list->num_out == 0);
     gpio_list->num_out = n;
     gpio_list->out = pins;
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 02/14] qdev: gpio: Register GPIO inputs as child objects

[Peter Crosthwaite]

To the device that contains them. This will allow for referencing
a GPIO input from it's canonical path (exciting for dynamic machine
generation!)

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Define propname variable at start of function.

 hw/core/qdev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 77441aa..83c3923 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -337,11 +337,20 @@ static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev,
 void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
                              const char *name, int n)
 {
+    int i;
     NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+    char *propname = g_strdup_printf("%s[*]", name ? name : "unnamed-gpio-in");
 
     assert(gpio_list->num_out == 0 || !name);
     gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler,
                                      dev, n);
+
+    for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) {
+        object_property_add_child(OBJECT(dev), propname,
+                                  OBJECT(gpio_list->in[i]), &error_abort);
+    }
+    g_free(propname);
+
     gpio_list->num_in += n;
 }
 
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 03/14] qdev: gpio: Register GPIO outputs as QOM links

[Peter Crosthwaite]

Within the object that contains the GPIO output. This allows for
connecting GPIO outputs via setting of a Link property.

Also clear the link value to zero. This catch-alls the case
where a device improperly inits a gpio_out (malloc instead of
malloc0).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed since v1:
Clear the irq when initing the GPIO.
Define propname at start of function.

 hw/core/qdev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 83c3923..6030ee5 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -362,12 +362,24 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
 void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
                               const char *name, int n)
 {
+    int i;
     NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
+    char *propname = g_strdup_printf("%s[*]", name ? name : "unnamed-gpio-out");
 
     assert(gpio_list->num_in == 0 || !name);
     assert(gpio_list->num_out == 0);
     gpio_list->num_out = n;
     gpio_list->out = pins;
+
+    for (i = 0; i < n; ++i) {
+        memset(&pins[i], 0, sizeof(*pins));
+        object_property_add_link(OBJECT(dev), propname, TYPE_IRQ,
+                                 (Object **)&pins[i],
+                                 object_property_allow_set_link,
+                                 OBJ_PROP_LINK_UNREF_ON_RELEASE,
+                                 &error_abort);
+    }
+    g_free(propname);
 }
 
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n)
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 04/14] qmp: qstring: Handle NULL strings

[Peter Crosthwaite]

Create a valid qobject even if the input string is null.
qstring->string will be NULL and length will be 0.

This prepares support for clearing of QOM Link properties
where NULL canonical path string will be passes through
this API.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 qobject/qstring.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/qobject/qstring.c b/qobject/qstring.c
index 607b7a1..a15ffed 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -48,14 +48,16 @@ QString *qstring_from_substr(const char *str, int start, int end)
 {
     QString *qstring;
 
-    qstring = g_malloc(sizeof(*qstring));
+    qstring = g_malloc0(sizeof(*qstring));
 
     qstring->length = end - start + 1;
     qstring->capacity = qstring->length;
 
-    qstring->string = g_malloc(qstring->capacity + 1);
-    memcpy(qstring->string, str + start, qstring->length);
-    qstring->string[qstring->length] = 0;
+    if (str) {
+        qstring->string = g_malloc(qstring->capacity + 1);
+        memcpy(qstring->string, str + start, qstring->length);
+        qstring->string[qstring->length] = 0;
+    }
 
     QOBJECT_INIT(qstring, &qstring_type);
 
@@ -69,7 +71,7 @@ QString *qstring_from_substr(const char *str, int start, int end)
  */
 QString *qstring_from_str(const char *str)
 {
-    return qstring_from_substr(str, 0, strlen(str) - 1);
+    return qstring_from_substr(str, 0, (str ? strlen(str) : 0) - 1);
 }
 
 static void capacity_increase(QString *qstring, size_t len)
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 05/14] qom: Allow clearing of a Link property

[Peter Crosthwaite]

By passing in NULL to object_property_set_link.

The lead user of this is the QDEV GPIO framework which will implement
GPIO disconnects via an "unlink".

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 qom/object.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index c869e8e..8821e23 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -881,7 +881,7 @@ char *object_property_get_str(Object *obj, const char *name,
 void object_property_set_link(Object *obj, Object *value,
                               const char *name, Error **errp)
 {
-    gchar *path = object_get_canonical_path(value);
+    gchar *path = value ? object_get_canonical_path(value) : NULL;
     object_property_set_str(obj, path, name, errp);
     g_free(path);
 }
@@ -1172,7 +1172,7 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque,
 
     visit_type_str(v, &path, name, &local_err);
 
-    if (!local_err && strcmp(path, "") != 0) {
+    if (!local_err && path && strcmp(path, "") != 0) {
         new_target = object_resolve_link(obj, name, path, &local_err);
     }
 
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 06/14] qom: Demote already-has-a-parent to a regular error

[Peter Crosthwaite]

Rather than an abort(). This allows callers to decide whether parenting
an already-parented object is a fatal error condition.

Useful for providing a default value for an object's parent in the case
where you want to set one iff it doesn't already have one.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 qom/object.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 8821e23..9da0064 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1074,6 +1074,11 @@ void object_property_add_child(Object *obj, const char *name,
     gchar *type;
     ObjectProperty *op;
 
+    if (child->parent != NULL) {
+        error_setg(errp, "child object is already parented");
+        return;
+    }
+
     type = g_strdup_printf("child<%s>", object_get_typename(OBJECT(child)));
 
     op = object_property_add(obj, name, type, object_get_child_property, NULL,
@@ -1085,7 +1090,6 @@ void object_property_add_child(Object *obj, const char *name,
 
     op->resolve = object_resolve_child_property;
     object_ref(child);
-    g_assert(child->parent == NULL);
     child->parent = obj;
 
 out:
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 07/14] qdev: gpio: Re-impement qdev_connect_gpio QOM style

[Peter Crosthwaite]

Re-implement as a link setter. This should allow the QOM framework to
keep track of ref counts properly etc.

We need to add a default parent for the connecting input incase it's
coming from a non-qdev source. We simply parent the IRQ to the machine
in this case.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 6030ee5..7681fe1 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -403,10 +403,14 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int 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;
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+    if (pin) {
+        object_property_add_child(qdev_get_machine(), "non-qdev-gpio[*]",
+                                  OBJECT(pin), NULL);
+    }
+    object_property_set_link(OBJECT(dev), OBJECT(pin), propname, &error_abort);
+    g_free(propname);
 }
 
 void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 08/14] qdev: gpio: Add API for intercepting a GPIO

[Peter Crosthwaite]

To replace the old qemu_irq intercept API (which had users reaching
into qdev private state for GPIOs).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1 (Alex review):
Accept an alredy inited qemu_irq as arg and return the disconnected
(allow arbitrary opaque setting).
s/irq/gpio in commit msg

 hw/core/qdev.c         | 25 +++++++++++++++++++++++++
 include/hw/qdev-core.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 7681fe1..d83e870 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -413,6 +413,31 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
     g_free(propname);
 }
 
+/* disconnect a GPIO ouput, returning the disconnected input (if any) */
+
+static qemu_irq qdev_disconnect_gpio_out_named(DeviceState *dev,
+                                               const char *name, int n)
+{
+    char *propname = g_strdup_printf("%s[%d]",
+                                     name ? name : "unnamed-gpio-out", n);
+
+    qemu_irq ret = (qemu_irq)object_property_get_link(OBJECT(dev), propname,
+                                                      NULL);
+    if (ret) {
+        object_property_set_link(OBJECT(dev), NULL, propname, NULL);
+    }
+    g_free(propname);
+    return ret;
+}
+
+qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
+                                 const char *name, int n)
+{
+    qemu_irq disconnected = qdev_disconnect_gpio_out_named(dev, name, n);
+    qdev_connect_gpio_out_named(dev, name, n, icpt);
+    return disconnected;
+}
+
 void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
 {
     qdev_connect_gpio_out_named(dev, NULL, n, pin);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 0799ff2..b28e276 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -274,6 +274,8 @@ 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);
+qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
+                                 const char *name, int n);
 
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 09/14] qtest/irq: Rework IRQ interception

[Peter Crosthwaite]

Change the qtest intercept handler to accept just the individual IRQ
being intercepted as opaque. n is still expected to be correctly set
as for the original intercepted irq. qemu_intercept_irq_in is updated
accordingly.

Then covert the qemu_irq_intercept_out call to use qdev intercept
version. This stops qtest from having to mess with the raw IRQ pointers
(still has to mess with names and counts but a step in the right
direction).

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
Create IRQ before intercept call and add extra layer of indirection on
opaque data.

 hw/core/irq.c |  2 +-
 qtest.c       | 15 +++++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index cffced0..4a580a2 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -140,7 +140,7 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n)
     for (i = 0; i < n; i++) {
         *old_irqs[i] = *gpio_in[i];
         gpio_in[i]->handler = handler;
-        gpio_in[i]->opaque = old_irqs;
+        gpio_in[i]->opaque = &old_irqs[i];
     }
 }
 
diff --git a/qtest.c b/qtest.c
index ef0d991..bc40c6c 100644
--- a/qtest.c
+++ b/qtest.c
@@ -200,8 +200,8 @@ static void GCC_FMT_ATTR(2, 3) qtest_send(CharDriverState *chr,
 
 static void qtest_irq_handler(void *opaque, int n, int level)
 {
-    qemu_irq *old_irqs = opaque;
-    qemu_set_irq(old_irqs[n], level);
+    qemu_irq old_irq = *(qemu_irq *)opaque;
+    qemu_set_irq(old_irq, level);
 
     if (irq_levels[n] != level) {
         CharDriverState *chr = qtest_chr;
@@ -263,8 +263,15 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
                 continue;
             }
             if (words[0][14] == 'o') {
-                qemu_irq_intercept_out(&ngl->out, qtest_irq_handler,
-                                       ngl->num_out);
+                int i;
+                for (i = 0; i < ngl->num_out; ++i) {
+                    qemu_irq *disconnected = g_new0(qemu_irq, 1);
+                    qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler,
+                                                      disconnected, i);
+
+                    *disconnected = qdev_intercept_gpio_out(dev, icpt,
+                                                            ngl->name, i);
+                }
             } else {
                 qemu_irq_intercept_in(ngl->in, qtest_irq_handler,
                                       ngl->num_in);
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 10/14] irq: Remove qemu_irq_intercept_out

[Peter Crosthwaite]

No more users left and obsoleted by qdev_intercept_gpio_out.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/irq.c    | 6 ------
 include/hw/irq.h | 1 -
 2 files changed, 7 deletions(-)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index 4a580a2..8a62a36 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -144,12 +144,6 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n)
     }
 }
 
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int n)
-{
-    qemu_irq *old_irqs = *gpio_out;
-    *gpio_out = qemu_allocate_irqs(handler, old_irqs, n);
-}
-
 static const TypeInfo irq_type_info = {
    .name = TYPE_IRQ,
    .parent = TYPE_OBJECT,
diff --git a/include/hw/irq.h b/include/hw/irq.h
index 6f874f5..4c4c2ea 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -61,6 +61,5 @@ qemu_irq *qemu_irq_proxy(qemu_irq **target, int n);
 /* For internal use in qtest.  Similar to qemu_irq_split, but operating
    on an existing vector of qemu_irq.  */
 void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int n);
 
 #endif
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 11/14] qdev: gpio: delete NamedGPIOList::out

[Peter Crosthwaite]

All users of GPIO outputs are fully QOMified, using QOM properties to
access the GPIO data. Delete.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c         | 1 -
 include/hw/qdev-core.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index d83e870..dbc4aad 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -369,7 +369,6 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
     assert(gpio_list->num_in == 0 || !name);
     assert(gpio_list->num_out == 0);
     gpio_list->num_out = n;
-    gpio_list->out = pins;
 
     for (i = 0; i < n; ++i) {
         memset(&pins[i], 0, sizeof(*pins));
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index b28e276..e6b1cf0 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -137,7 +137,6 @@ struct NamedGPIOList {
     char *name;
     qemu_irq *in;
     int num_in;
-    qemu_irq *out;
     int num_out;
     QLIST_ENTRY(NamedGPIOList) node;
 };
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 12/14] qdev: gpio: Remove qdev_init_gpio_out x1 restriction

[Peter Crosthwaite]

Previously this was restricted to a single call per-dev/per-name. With
the conversion of the GPIO output state to QOM the implementation can
now handle repeated calls. Remove the restriction.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/core/qdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index dbc4aad..2b96b5a 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -367,8 +367,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
     char *propname = g_strdup_printf("%s[*]", name ? name : "unnamed-gpio-out");
 
     assert(gpio_list->num_in == 0 || !name);
-    assert(gpio_list->num_out == 0);
-    gpio_list->num_out = n;
+    gpio_list->num_out += n;
 
     for (i = 0; i < n; ++i) {
         memset(&pins[i], 0, sizeof(*pins));
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 13/14] qdev: gpio: Define qdev_pass_gpios()

[Peter Crosthwaite]

Allows a container to take ownership of GPIOs in a contained
device and automatically connect them as GPIOs to the container.

This prepares for deprecation of the SYSBUS IRQ functionality, which
has this feature. We push it up to the device level instead of sysbus
level. There's nothing sysbus specific about passing GPIOs to
containers so its a legitimate device-level generic feature.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
changed since v1:
Variablised ? : string expressions (Alex review).

 hw/core/qdev.c         | 26 ++++++++++++++++++++++++++
 include/hw/qdev-core.h |  3 +++
 2 files changed, 29 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 2b96b5a..6d09d47 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -441,6 +441,32 @@ void qdev_connect_gpio_out(DeviceState * dev, int n, qemu_irq pin)
     qdev_connect_gpio_out_named(dev, NULL, n, pin);
 }
 
+void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
+                     const char *name)
+{
+    int i;
+    NamedGPIOList *ngl = qdev_get_named_gpio_list(dev, name);
+
+    for (i = 0; i < ngl->num_in; i++) {
+        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-in";
+        char *propname = g_strdup_printf("%s[%d]", nm, i);
+
+        object_property_add_alias(OBJECT(container), propname,
+                                  OBJECT(dev), propname,
+                                  &error_abort);
+    }
+    for (i = 0; i < ngl->num_out; i++) {
+        const char *nm = ngl->name ? ngl->name : "unnamed-gpio-out";
+        char *propname = g_strdup_printf("%s[%d]", nm, i);
+
+        object_property_add_alias(OBJECT(container), propname,
+                                  OBJECT(dev), propname,
+                                  &error_abort);
+    }
+    QLIST_REMOVE(ngl, node);
+    QLIST_INSERT_HEAD(&container->gpios, ngl, node);
+}
+
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
 {
     BusState *bus;
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e6b1cf0..abdbf42 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -289,6 +289,9 @@ void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler,
 void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
                               const char *name, int n);
 
+void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
+                     const char *name);
+
 BusState *qdev_get_parent_bus(DeviceState *dev);
 
 /*** BUS API. ***/
-- 
2.0.1.1.gfbfc394

[Qemu-devel] [PATCH v2 14/14] sysbus: Use TYPE_DEVICE GPIO functionality

[Peter Crosthwaite]

Re-implement the Sysbus GPIOs to use the existing TYPE_DEVICE
GPIO named framework. A constant string name is chosen to avoid
conflicts with existing unnamed GPIOs.

This unifies GPIOs are IRQs for sysbus devices and allows removal
of all Sysbus state for GPIOs.

Any existing and future-added functionality for GPIOs is now
also available for sysbus IRQs.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed since v1:
Named each IRQ individually.

 hw/core/sysbus.c    | 20 +++-----------------
 include/hw/sysbus.h |  6 +++---
 2 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index f4e760d..192a729 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -41,11 +41,7 @@ static const TypeInfo system_bus_info = {
 
 void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
 {
-    assert(n >= 0 && n < dev->num_irq);
-    dev->irqs[n] = NULL;
-    if (dev->irqp[n]) {
-        *dev->irqp[n] = irq;
-    }
+    qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);
 }
 
 static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
@@ -89,22 +85,13 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr,
 /* Request an IRQ source.  The actual IRQ object may be populated later.  */
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
 {
-    int n;
-
-    assert(dev->num_irq < QDEV_MAX_IRQ);
-    n = dev->num_irq++;
-    dev->irqp[n] = p;
+    qdev_init_gpio_out_named(DEVICE(dev), p, SYSBUS_DEVICE_GPIO_IRQ, 1);
 }
 
 /* Pass IRQs from a target device.  */
 void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target)
 {
-    int i;
-    assert(dev->num_irq == 0);
-    dev->num_irq = target->num_irq;
-    for (i = 0; i < dev->num_irq; i++) {
-        dev->irqp[i] = target->irqp[i];
-    }
+    qdev_pass_gpios(DEVICE(target), DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ);
 }
 
 void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
@@ -210,7 +197,6 @@ static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent)
     hwaddr size;
     int i;
 
-    monitor_printf(mon, "%*sirq %d\n", indent, "", s->num_irq);
     for (i = 0; i < s->num_mmio; i++) {
         size = memory_region_size(s->mmio[i].memory);
         monitor_printf(mon, "%*smmio " TARGET_FMT_plx "/" TARGET_FMT_plx "\n",
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index f5aaa05..a0baefc 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -8,7 +8,6 @@
 
 #define QDEV_MAX_MMIO 32
 #define QDEV_MAX_PIO 32
-#define QDEV_MAX_IRQ 512
 
 #define TYPE_SYSTEM_BUS "System"
 #define SYSTEM_BUS(obj) OBJECT_CHECK(IDEBus, (obj), TYPE_IDE_BUS)
@@ -33,6 +32,9 @@ typedef struct SysBusDevice SysBusDevice;
  * SysBusDeviceClass is not overriding #DeviceClass.realize, so derived
  * classes overriding it are not required to invoke its implementation.
  */
+
+#define SYSBUS_DEVICE_GPIO_IRQ "sysbus-irq"
+
 typedef struct SysBusDeviceClass {
     /*< private >*/
     DeviceClass parent_class;
@@ -47,8 +49,6 @@ struct SysBusDevice {
     /*< public >*/
 
     int num_irq;
-    qemu_irq irqs[QDEV_MAX_IRQ];
-    qemu_irq *irqp[QDEV_MAX_IRQ];
     int num_mmio;
     struct {
         hwaddr addr;
-- 
2.0.1.1.gfbfc394

Re: [Qemu-devel] [PATCH v2 00/14] GPIO/IRQ QOMification: Phase 2 - Getting rid of SYSBUS IRQs

[Alexander Graf]

On 15.08.14 07:29, Peter Crosthwaite wrote:
> Hi All,
> 
> So phase one was the QOMification of qemu_irq. This is the next step.
> We start to setup GPIOs as proper QOM objects. Inputs are child objects
> of their device. Outputs are settable Links and connection is made
> via proper setting of a QOM link.
> 
> We then cleanup Sysbus to simply re-use device level GPIOs and get rid
> of it's special IRQ handling code.
> 
> Depends of my pending QOM array property stuff (the "[*]" series):
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg04116.html
> 
> Changed since v1:
> Addressed Alex review
> Dropped IRQ g_new0 changes

Reviewed-by: Alexander Graf <agraf@suse.de>


Alex

Re: [Qemu-devel] [PATCH v2 00/14] GPIO/IRQ QOMification: Phase 2 - Getting rid of SYSBUS IRQs

[Peter Crosthwaite]

Ping!

Andreas, are you able to take this?

Regards,
Peter

On Thu, Aug 21, 2014 at 7:23 PM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 15.08.14 07:29, Peter Crosthwaite wrote:
>> Hi All,
>>
>> So phase one was the QOMification of qemu_irq. This is the next step.
>> We start to setup GPIOs as proper QOM objects. Inputs are child objects
>> of their device. Outputs are settable Links and connection is made
>> via proper setting of a QOM link.
>>
>> We then cleanup Sysbus to simply re-use device level GPIOs and get rid
>> of it's special IRQ handling code.
>>
>> Depends of my pending QOM array property stuff (the "[*]" series):
>>
>> https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg04116.html
>>
>> Changed since v1:
>> Addressed Alex review
>> Dropped IRQ g_new0 changes
>
> Reviewed-by: Alexander Graf <agraf@suse.de>
>
>
> Alex
>

Re: [Qemu-devel] [PATCH v2 14/14] sysbus: Use TYPE_DEVICE GPIO functionality

[Alexander Graf]

On 15.08.14 07:37, Peter Crosthwaite wrote:
> Re-implement the Sysbus GPIOs to use the existing TYPE_DEVICE
> GPIO named framework. A constant string name is chosen to avoid
> conflicts with existing unnamed GPIOs.
> 
> This unifies GPIOs are IRQs for sysbus devices and allows removal
> of all Sysbus state for GPIOs.
> 
> Any existing and future-added functionality for GPIOs is now
> also available for sysbus IRQs.
> 
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
> Changed since v1:
> Named each IRQ individually.
> 
>  hw/core/sysbus.c    | 20 +++-----------------
>  include/hw/sysbus.h |  6 +++---
>  2 files changed, 6 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
> index f4e760d..192a729 100644
> --- a/hw/core/sysbus.c
> +++ b/hw/core/sysbus.c
> @@ -41,11 +41,7 @@ static const TypeInfo system_bus_info = {
>  
>  void sysbus_connect_irq(SysBusDevice *dev, int n, qemu_irq irq)
>  {
> -    assert(n >= 0 && n < dev->num_irq);
> -    dev->irqs[n] = NULL;
> -    if (dev->irqp[n]) {
> -        *dev->irqp[n] = irq;
> -    }
> +    qdev_connect_gpio_out_named(DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ, n, irq);
>  }
>  
>  static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
> @@ -89,22 +85,13 @@ void sysbus_mmio_map_overlap(SysBusDevice *dev, int n, hwaddr addr,
>  /* Request an IRQ source.  The actual IRQ object may be populated later.  */
>  void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p)
>  {
> -    int n;
> -
> -    assert(dev->num_irq < QDEV_MAX_IRQ);
> -    n = dev->num_irq++;

This renders dev->num_irq to always stay 0. Better remove it from the
header.


Alex

Re: [Qemu-devel] [PATCH v2 00/14] GPIO/IRQ QOMification: Phase 2 - Getting rid of SYSBUS IRQs

[Alexander Graf]

On 15.08.14 07:29, Peter Crosthwaite wrote:
> Hi All,
> 
> So phase one was the QOMification of qemu_irq. This is the next step.
> We start to setup GPIOs as proper QOM objects. Inputs are child objects
> of their device. Outputs are settable Links and connection is made
> via proper setting of a QOM link.
> 
> We then cleanup Sysbus to simply re-use device level GPIOs and get rid
> of it's special IRQ handling code.
> 
> Depends of my pending QOM array property stuff (the "[*]" series):
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg04116.html
> 
> Changed since v1:
> Addressed Alex review
> Dropped IRQ g_new0 changes

With this patch set applied, the e500 machine fails to start:

$ ./ppc-softmmu/qemu-system-ppc -M ppce500 -nographic
qemu-system-ppc: attempt to add duplicate property 'sysbus-irq[*]' to
object (type 'openpic')
Aborted


Alex

Re: [Qemu-devel] [PATCH v2 00/14] GPIO/IRQ QOMification: Phase 2 - Getting rid of SYSBUS IRQs

[Alexander Graf]

On 28.08.14 13:59, Alexander Graf wrote:
> 
> 
> On 15.08.14 07:29, Peter Crosthwaite wrote:
>> Hi All,
>>
>> So phase one was the QOMification of qemu_irq. This is the next step.
>> We start to setup GPIOs as proper QOM objects. Inputs are child objects
>> of their device. Outputs are settable Links and connection is made
>> via proper setting of a QOM link.
>>
>> We then cleanup Sysbus to simply re-use device level GPIOs and get rid
>> of it's special IRQ handling code.
>>
>> Depends of my pending QOM array property stuff (the "[*]" series):
>>
>> https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg04116.html
>>
>> Changed since v1:
>> Addressed Alex review
>> Dropped IRQ g_new0 changes
> 
> With this patch set applied, the e500 machine fails to start:
> 
> $ ./ppc-softmmu/qemu-system-ppc -M ppce500 -nographic
> qemu-system-ppc: attempt to add duplicate property 'sysbus-irq[*]' to
> object (type 'openpic')
> Aborted

Andreas pointed me to his qom-next branch where patches were that
apparently are a prerequisite (doh, I should've read the cover letter).

Sorry for the fuss, it works like a charm ;).


Alex

Re: [Qemu-devel] [PATCH v2 00/14] GPIO/IRQ QOMification: Phase 2 - Getting rid of SYSBUS IRQs

[Alexander Graf]

On 28.08.14 14:23, Alexander Graf wrote:
> 
> 
> On 28.08.14 13:59, Alexander Graf wrote:
>>
>>
>> On 15.08.14 07:29, Peter Crosthwaite wrote:
>>> Hi All,
>>>
>>> So phase one was the QOMification of qemu_irq. This is the next step.
>>> We start to setup GPIOs as proper QOM objects. Inputs are child objects
>>> of their device. Outputs are settable Links and connection is made
>>> via proper setting of a QOM link.
>>>
>>> We then cleanup Sysbus to simply re-use device level GPIOs and get rid
>>> of it's special IRQ handling code.
>>>
>>> Depends of my pending QOM array property stuff (the "[*]" series):
>>>
>>> https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg04116.html
>>>
>>> Changed since v1:
>>> Addressed Alex review
>>> Dropped IRQ g_new0 changes
>>
>> With this patch set applied, the e500 machine fails to start:
>>
>> $ ./ppc-softmmu/qemu-system-ppc -M ppce500 -nographic
>> qemu-system-ppc: attempt to add duplicate property 'sysbus-irq[*]' to
>> object (type 'openpic')
>> Aborted
> 
> Andreas pointed me to his qom-next branch where patches were that
> apparently are a prerequisite (doh, I should've read the cover letter).
> 
> Sorry for the fuss, it works like a charm ;).

Can I expect this to land upstream anytime soon? :)


Alex