From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37080) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH8K5-0005RM-EG for qemu-devel@nongnu.org; Tue, 12 Aug 2014 05:24:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XH8Jz-0005QI-4k for qemu-devel@nongnu.org; Tue, 12 Aug 2014 05:24:53 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39690 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH8Jy-0005QD-PY for qemu-devel@nongnu.org; Tue, 12 Aug 2014 05:24:47 -0400 Message-ID: <53E9DD5D.3040806@suse.de> Date: Tue, 12 Aug 2014 11:24:45 +0200 From: Alexander Graf MIME-Version: 1.0 References: <430adeb742bf80def5cc117a766fa79171e8ecc8.1407116648.git.peter.crosthwaite@xilinx.com> In-Reply-To: <430adeb742bf80def5cc117a766fa79171e8ecc8.1407116648.git.peter.crosthwaite@xilinx.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 13/16] qdev: gpio: Define qdev_pass_gpios() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite , qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, afaerber@suse.de, pbonzini@redhat.com On 04.08.14 03:58, Peter Crosthwaite wrote: > 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 > --- > > hw/core/qdev.c | 28 ++++++++++++++++++++++++++++ > include/hw/qdev-core.h | 3 +++ > 2 files changed, 31 insertions(+) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index bf2c227..708363f 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -440,6 +440,34 @@ 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++) { > + char *propname = g_strdup_printf("%s[%d]", > + ngl->name ? ngl->name : > + "unnamed-gpio-in", Really just a minor nit, but I think the code flow would look a lot nicer if you did the name check in a separate variable. const char *name = ngl->name ? ngl->name : "unnamed-gpio-in"; for (i = 0; ...) { char *propname = g_strdup_printf("%s[%d]", name, i); .... } Also I don't fully grasp what the naming scheme is supposed to be here. Who sets the name and why is there only a single global name for all GPIOs? Alex > + i); > + object_property_add_alias(OBJECT(container), propname, > + OBJECT(dev), propname, > + &error_abort); > + } > + for (i = 0; i < ngl->num_out; i++) { > + char *propname = g_strdup_printf("%s[%d]", > + ngl->name ? ngl->name : > + "unnamed-gpio-in", > + 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 d3326b1..08dafda 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. ***/