Re: [PATCH 3/3] hw/input/pckbd: Rename i8042_setup_a20_line() and its a20 irq argument

[Peter Maydell]

On Fri, 5 Nov 2021 at 17:21, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> 'a20_out' is an input IRQ, rename it as 'a20_input'.
> i8042_setup_a20_line() doesn't take a Device parameter
> but an ISADevice one. Rename it as i8042_isa_*() to
> make it explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/input/i8042.h | 2 +-
>  hw/i386/pc.c             | 2 +-
>  hw/input/pckbd.c         | 4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
> index 1d90432daef..3534fcc4b43 100644
> --- a/include/hw/input/i8042.h
> +++ b/include/hw/input/i8042.h
> @@ -21,6 +21,6 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
>                     MemoryRegion *region, ram_addr_t size,
>                     hwaddr mask);
>  void i8042_isa_mouse_fake_event(ISAKBDState *isa);
> -void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
> +void i8042_isa_setup_a20_line(ISADevice *dev, qemu_irq a20_input);
>
>  #endif /* HW_INPUT_I8042_H */
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 2592a821486..06ef74ca22b 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1043,7 +1043,7 @@ static void pc_superio_init(ISABus *isa_bus, bool 
> create_fdctrl, bool no_vmport)
>      port92 = isa_create_simple(isa_bus, TYPE_PORT92);
>
>      a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2);
> -    i8042_setup_a20_line(i8042, a20_line[0]);
> +    i8042_isa_setup_a20_line(i8042, a20_line[0]);

I think these days we can directly call
    qdev_connect_gpio_out_named(DEVICE(i8042), I8042_A20_LINE, 0, a20_line[0]);
and drop the i8042_setup_a20_line() wrapper entirely,
since the named GPIO lines are a "public" interface to the device.
We only have this i8042_setup_a20_line() because the original
implementation (added in commit 956a3e6bb738) predates gpio lines
and did an assignment into the KBDState struct which needed to
be private to pckbd.c.

>      qdev_connect_gpio_out_named(DEVICE(port92),
>                                  PORT92_A20_LINE, 0, a20_line[1]);

That would then make it consistent with how we're wiring up the
other A20 input source here.

(Some day we should perhaps make the A20 input to the CPU an actual
GPIO input on the CPU device object, so we could wire the I8042_A20_LINE
and PORT92_A20_LINE to it via an OR gate, and drop the intermidate
qemu_irq array and handle_a20_line_change function. But needing
the OR gate makes that a little clunky so I'm not sure it's
really worth the effort.)

-- PMM