From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35689) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XIBzX-0004NM-51 for qemu-devel@nongnu.org; Fri, 15 Aug 2014 03:32:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XIBzR-0003FD-Eb for qemu-devel@nongnu.org; Fri, 15 Aug 2014 03:32:03 -0400 Received: from cantor2.suse.de ([195.135.220.15]:52867 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XIBzR-0003F5-3i for qemu-devel@nongnu.org; Fri, 15 Aug 2014 03:31:57 -0400 Message-ID: <53EDB76B.7060209@suse.de> Date: Fri, 15 Aug 2014 09:31:55 +0200 From: Alexander Graf MIME-Version: 1.0 References: <430adeb742bf80def5cc117a766fa79171e8ecc8.1407116648.git.peter.crosthwaite@xilinx.com> <53E9DD5D.3040806@suse.de> <53E9F2B2.5020501@suse.de> In-Reply-To: Content-Type: text/plain; charset=utf-8; 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 Cc: Peter Maydell , "qemu-devel@nongnu.org Developers" , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , Paolo Bonzini On 15.08.14 07:11, Peter Crosthwaite wrote: > On Tue, Aug 12, 2014 at 8:55 PM, Alexander Graf wrote: >> On 12.08.14 12:48, Peter Crosthwaite wrote: >>> On Tue, Aug 12, 2014 at 7:24 PM, Alexander Graf 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 >>>>> --- >>>>> >>>>> 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"; > Just done it this way. Although have used "nm" instead of "name" > instead as "name" is already used. > >>> 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. >> > Hmm not sure what this wins though? inline "? :" is concise and self > documenting IMO. You just need to not stuff-up the "()"s (which is > admittedly quite easy to do. > > Anyways, it's a patch for another day. Yeah, probably not a big win really :). Alex