Re: [PATCH 2/2] hw/core/sysbus: Make sysbus_pass_irq() arguments self-describing

[Peter Maydell <peter.maydell@linaro.org>]

On Tue, 18 Aug 2020 at 15:38, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Make the direction sysbus_pass_irq() pass the GPIOs more obvious
> by renaming its arguments as 'from_dev' and 'to_dev'.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/sysbus.h | 7 ++++++-
>  hw/core/sysbus.c    | 5 ++---
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
> index da9f85c58ce..65de6f6b062 100644
> --- a/include/hw/sysbus.h
> +++ b/include/hw/sysbus.h
> @@ -73,7 +73,12 @@ typedef void FindSysbusDeviceFunc(SysBusDevice *sbdev, void *opaque);
>  void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory);
>  MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n);
>  void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p);
> -void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target);
> +/**
> + * sysbus_pass_irq: Pass through IRQ/GPIO lines from one to another device
> + * @to_dev: Device which needs to expose IRQ/GPIO lines
> + * @from_dev: Device which has the IRQ/GPIO lines
> + */
> +void sysbus_pass_irq(SysBusDevice *to_dev, SysBusDevice *from_dev);

I think this is less good than what we had before, because (as
per comments on patch 1) "to" vs "from" is ambiguous.
The 'before' here is not as good as the qdev_pass_gpios()
choice of "dev" vs "container", though, so I guess we should
make this function match that one.

> -/* Pass IRQs from a target device.  */
> -void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target)
> +void sysbus_pass_irq(SysBusDevice *to_dev, SysBusDevice *from_dev)
>  {
> -    qdev_pass_gpios(DEVICE(target), DEVICE(dev), SYSBUS_DEVICE_GPIO_IRQ);
> +    qdev_pass_gpios(DEVICE(from_dev), DEVICE(to_dev), SYSBUS_DEVICE_GPIO_IRQ);
>  }

Having these two functions pass the container and target in
opposite orders seems unnecessarily confusing. We should
make them use the same order, at least...

thanks
-- PMM