On 12 May 2014 02:03, Peter Crosthwaite <peter.crosthwa...@xilinx.com> wrote:
> Implement named GPIOs on the Device layer. Listifies the existing GPIOs
> stuff using string keys. Legacy un-named GPIOs are preserved by using
> a NULL name string - they are just a single matchable element in the
> name list.
>
> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
Two very minor nits involving comments below, but otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
and I would be happy for these patches to be committed
(through the QOM tree?)
> @@ -854,10 +907,19 @@ static void device_post_init(Object *obj)
> /* Unlink device from bus and free the structure. */
> static void device_finalize(Object *obj)
> {
> + NamedGPIOList *ngl, *next;
> +
> DeviceState *dev = DEVICE(obj);
> if (dev->opts) {
> qemu_opts_del(dev->opts);
> }
> +
> + QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
> + QLIST_REMOVE(ngl, node);
> + qemu_free_irqs(ngl->in);
> + g_free(ngl->name);
> + g_free(ngl);
I think we could use a comment along the lines of
/* ngl->out irqs are owned by the other end and should not be freed here */
to stop somebody later on wondering whether it's a bug
that we don't clean up that field in NamedGPIOList.
> @@ -253,10 +254,18 @@ static void qtest_process_command(CharDriverState *chr,
> gchar **words)
> return;
> }
>
> - if (words[0][14] == 'o') {
> - qemu_irq_intercept_out(&dev->gpio_out, qtest_irq_handler,
> dev->num_gpio_out);
> - } else {
> - qemu_irq_intercept_in(dev->gpio_in, qtest_irq_handler,
> dev->num_gpio_in);
> + QLIST_FOREACH(ngl, &dev->gpios, node) {
> + /* We dont support intercept of named GPIOs yet */
"don't".
thanks
-- PMM
