|
From: | Alexander Graf |
Subject: | Re: [Qemu-devel] [PATCH v1 13/16] qdev: gpio: Define qdev_pass_gpios() |
Date: | Tue, 12 Aug 2014 12:55:46 +0200 |
User-agent: | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Thunderbird/31.0 |
On 12.08.14 12:48, Peter Crosthwaite wrote:
On Tue, Aug 12, 2014 at 7:24 PM, Alexander Graf <address@hidden> wrote: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 <address@hidden> --- 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";I think I may even go an extra step and get it macroified. How about: #define QDEV_GPIO_IN_NAME(a) ((a) ? (a) : "unnamed-gpio-io") and then you can continue to use it inline without extra variables or nasty "?:"?
The variable will get optimized out, so for the sake of readability I would still vote to have it around. Whether you declare it via a macro or with an a ? a : b macro I don't really care much about :).
In fact, it might make more sense to literally have the typical (a ? a : b) flow macroified in a generic place somewhere.
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?Ideally, the instantiating device sets the names. There's not a global name for all GPIOs, just an over-used default. The intention is that qdev_init_gpio_in is phased out in favor of qdev_init_gpio_in_named. If NULL name is given or qdev_init_gpio_in is used, it defaults to this single global name here. All sysbus IRQs also share a single name (seperate from the qdev default) but that sharing is implemented on the sysbus level and transparent to qdev. The need for a default name is to appease QOM, which needs a valid string for canonical path.
Ah, I missed the fact that qdev_get_named_gpio_list() returns a list of elements with exactly the name you searched for in the first place.
Alex
[Prev in Thread] | Current Thread | [Next in Thread] |