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

[Qemu-devel] [PATCH qom v3 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.

CHanged since v2:
Remove num_irq from sysbus (P14)
Removed out of tree deps.
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    |  7 ++--
 qobject/qstring.c      | 12 ++++---
 qom/object.c           | 10 ++++--
 qtest.c                | 15 ++++++---
 9 files changed, 120 insertions(+), 49 deletions(-)

-- 
2.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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.

Reviewed-by: Alexander Graf <agraf@suse.de>
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 fcb1638..976e208 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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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!)

Reviewed-by: Alexander Graf <agraf@suse.de>
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 976e208..a140c79 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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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).

Reviewed-by: Alexander Graf <agraf@suse.de>
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 a140c79..2b42d5b 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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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.

Reviewed-by: Alexander Graf <agraf@suse.de>
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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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".

Reviewed-by: Alexander Graf <agraf@suse.de>
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 da0919a..fe2d1c9 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -870,7 +870,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);
 }
@@ -1173,7 +1173,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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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.

Reviewed-by: Alexander Graf <agraf@suse.de>
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 fe2d1c9..50e4f30 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1075,6 +1075,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,
@@ -1086,7 +1091,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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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.

Reviewed-by: Alexander Graf <agraf@suse.de>
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 2b42d5b..2cadacd 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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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).

Reviewed-by: Alexander Graf <agraf@suse.de>
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 2cadacd..a89207c 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 178fee2..31301e5 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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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).

Reviewed-by: Alexander Graf <agraf@suse.de>
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.1.0.1.g27b9230

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

[Peter Crosthwaite]

No more users left and obsoleted by qdev_intercept_gpio_out.

Reviewed-by: Alexander Graf <agraf@suse.de>
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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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.

Reviewed-by: Alexander Graf <agraf@suse.de>
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 a89207c..41208a0 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 31301e5..eac603b 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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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.

Reviewed-by: Alexander Graf <agraf@suse.de>
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 41208a0..cfd1ce6 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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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.

Reviewed-by: Alexander Graf <agraf@suse.de>
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 cfd1ce6..1cdd052 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 eac603b..77b193b 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.1.0.1.g27b9230

[Qemu-devel] [PATCH qom v3 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.

Reviewed-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
Changed since v2:
Remove num_irq field from sysbus (Alex review).
Changed since v1:
Named each IRQ individually.

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

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 414e2a1..e55c3c1 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 0bb91a8..9fb1782 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;
@@ -46,9 +48,6 @@ struct SysBusDevice {
     DeviceState parent_obj;
     /*< public >*/
 
-    int num_irq;
-    qemu_irq irqs[QDEV_MAX_IRQ];
-    qemu_irq *irqp[QDEV_MAX_IRQ];
     int num_mmio;
     struct {
         hwaddr addr;
-- 
2.1.0.1.g27b9230

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

[Paolo Bonzini]

Il 26/09/2014 07:19, Peter Crosthwaite ha scritto:
> 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".
> 
> Reviewed-by: Alexander Graf <agraf@suse.de>
> 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 da0919a..fe2d1c9 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -870,7 +870,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;

It should also work if you pass an empty string here.

If you do this, you do not need patch 4.

Paolo

>      object_property_set_str(obj, path, name, errp);
>      g_free(path);
>  }
> @@ -1173,7 +1173,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);
>      }
>  
> 

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

[Paolo Bonzini]

Il 26/09/2014 07:20, Peter Crosthwaite ha scritto:
> +    if (pin) {
> +        object_property_add_child(qdev_get_machine(), "non-qdev-gpio[*]",
> +                                  OBJECT(pin), NULL);
> +    }

So here is when you're using the "do not abort if not a child" behavior.
 It deserves at least a comment, because it's not obvious.

Paolo

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

[Paolo Bonzini]

Il 26/09/2014 07:16, Peter Crosthwaite ha scritto:
> 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.
> 
> CHanged since v2:
> Remove num_irq from sysbus (P14)
> Removed out of tree deps.
> 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    |  7 ++--
>  qobject/qstring.c      | 12 ++++---
>  qom/object.c           | 10 ++++--
>  qtest.c                | 15 ++++++---
>  9 files changed, 120 insertions(+), 49 deletions(-)
> 

Apart from the couple of patches I replied to,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Alex, do you want to pick it up and send it together with your sysbus
patches?

Paolo

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

[Andreas Färber]

Am 26.09.2014 um 15:13 schrieb Paolo Bonzini:
> Il 26/09/2014 07:16, Peter Crosthwaite ha scritto:
>> 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.
>>
>> CHanged since v2:
>> Remove num_irq from sysbus (P14)
>> Removed out of tree deps.
>> 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    |  7 ++--
>>  qobject/qstring.c      | 12 ++++---
>>  qom/object.c           | 10 ++++--
>>  qtest.c                | 15 ++++++---
>>  9 files changed, 120 insertions(+), 49 deletions(-)
>>
> 
> Apart from the couple of patches I replied to,
> 
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> Alex, do you want to pick it up and send it together with your sysbus
> patches?

I had started picking up the initial ones from v2 after Alex' ping...

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH qom v3 04/14] qmp: qstring: Handle NULL strings

[Paolo Bonzini]

Il 26/09/2014 07:18, Peter Crosthwaite ha scritto:
> 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.
> 
> Reviewed-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---

This shouldn't be necessary if you use "".

Paolo

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

[Igor Mammedov]

On Thu, 25 Sep 2014 22:19:19 -0700
Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:

> 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".
> 
> Reviewed-by: Alexander Graf <agraf@suse.de>
> 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 da0919a..fe2d1c9 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -870,7 +870,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)
Updating doc comment in include/qom/object.h with new behavior
would be nice.

>  {
> -    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);
>  }
> @@ -1173,7 +1173,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);
>      }
>  

Re: [Qemu-devel] [PATCH qom v3 04/14] qmp: qstring: Handle NULL strings

[Andreas Färber]

Am 26.09.2014 um 15:27 schrieb Paolo Bonzini:
> Il 26/09/2014 07:18, Peter Crosthwaite ha scritto:
>> 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.
>>
>> Reviewed-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>> ---
> 
> This shouldn't be necessary if you use "".

Ping? I have patches 1-3 queued and am waiting how to proceed with this
series.

Thanks,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg