On Tue, Aug 12, 2014 at 8:55 PM, Alexander Graf <ag...@suse.de> wrote:
>
> On 12.08.14 12:48, Peter Crosthwaite wrote:
>>
>> On Tue, Aug 12, 2014 at 7:24 PM, Alexander Graf <ag...@suse.de> 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 <peter.crosthwa...@xilinx.com>
>>>> ---
>>>>
>>>> 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.
Regards,
Peter
