[PATCH 0/3] hw/qdev: Clarify qdev_connect_gpio_out() documentation

[PATCH 0/3] hw/qdev: Clarify qdev_connect_gpio_out() documentation

[Philippe Mathieu-Daudé]

Trivial patches clarifying qdev_connect_gpio_out() use,
basically that the qemu_irq argument is an input.
No logical change, only function name / parameter changed.

Philippe Mathieu-Daudé (3):
  hw/qdev: Correct qdev_connect_gpio_out_named() documentation
  hw/qdev: Rename qdev_connect_gpio_out*() 'input_pin' parameter
  hw/input/pckbd: Rename i8042_setup_a20_line() and its a20 irq argument

 include/hw/input/i8042.h |  2 +-
 include/hw/qdev-core.h   | 13 ++++++-------
 hw/core/gpio.c           | 13 +++++++------
 hw/i386/pc.c             |  2 +-
 hw/input/pckbd.c         |  4 ++--
 5 files changed, 17 insertions(+), 17 deletions(-)

-- 
2.31.1

[PATCH 1/3] hw/qdev: Correct qdev_connect_gpio_out_named() documentation

[Philippe Mathieu-Daudé]

qdev_connect_gpio_out_named() is described as qdev_connect_gpio_out(),
and referring to itself in an endless loop, which is confusing. Fix.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/qdev-core.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 72622bd3370..5b88c8b9dd3 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -497,7 +497,8 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
  */
 void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
 /**
- * qdev_connect_gpio_out: Connect one of a device's anonymous output GPIO lines
+ * qdev_connect_gpio_out_named: Connect one of a device's named output
+ *                              GPIO lines
  * @dev: Device whose GPIO to connect
  * @name: Name of the output GPIO array
  * @n: Number of the anonymous output GPIO line (which must be in range)
@@ -518,8 +519,6 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
  * It is not valid to try to connect one outbound GPIO to multiple
  * qemu_irqs at once, or to connect multiple outbound GPIOs to the
  * same qemu_irq; see qdev_connect_gpio_out() for details.
- *
- * For named output GPIO lines, use qdev_connect_gpio_out_named().
  */
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
                                  qemu_irq pin);
-- 
2.31.1

Re: [PATCH 1/3] hw/qdev: Correct qdev_connect_gpio_out_named() documentation

[wangyanan (Y)]

Hi Philippe,

On 2021/11/6 1:21, Philippe Mathieu-Daudé wrote:
> qdev_connect_gpio_out_named() is described as qdev_connect_gpio_out(),
> and referring to itself in an endless loop, which is confusing. Fix.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/hw/qdev-core.h | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 72622bd3370..5b88c8b9dd3 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -497,7 +497,8 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
>    */
>   void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
>   /**
> - * qdev_connect_gpio_out: Connect one of a device's anonymous output GPIO lines
> + * qdev_connect_gpio_out_named: Connect one of a device's named output
> + *                              GPIO lines
>    * @dev: Device whose GPIO to connect
>    * @name: Name of the output GPIO array
>    * @n: Number of the anonymous output GPIO line (which must be in range)
> @@ -518,8 +519,6 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
>    * It is not valid to try to connect one outbound GPIO to multiple
>    * qemu_irqs at once, or to connect multiple outbound GPIOs to the
>    * same qemu_irq; see qdev_connect_gpio_out() for details.
> - *
> - * For named output GPIO lines, use qdev_connect_gpio_out_named().
After deleting the wrong context, we can also add the right one, like:
"For anonymous output GPIO lines, use qdev_connect_gpio_out()."
>    */
>   void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
>                                    qemu_irq pin);
Having a further, the description of qdev_init_gpio_out_named() also needs
a tweak. Maybe we can squash that fix in this patch too.

Thanks,
Yanan

[PATCH 2/3] hw/qdev: Rename qdev_connect_gpio_out*() 'input_pin' parameter

[Philippe Mathieu-Daudé]

@pin is an input where we connect a device output.
Rename it @input_pin to simplify the documentation.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/qdev-core.h |  8 ++++----
 hw/core/gpio.c         | 13 +++++++------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 5b88c8b9dd3..3a0fe643a73 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -470,7 +470,7 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
  * qdev_connect_gpio_out: Connect one of a device's anonymous output GPIO lines
  * @dev: Device whose GPIO to connect
  * @n: Number of the anonymous output GPIO line (which must be in range)
- * @pin: qemu_irq to connect the output line to
+ * @input_pin: qemu_irq to connect the output line to
  *
  * This function connects an anonymous output GPIO line on a device
  * up to an arbitrary qemu_irq, so that when the device asserts that
@@ -495,14 +495,14 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
  *
  * For named output GPIO lines, use qdev_connect_gpio_out_named().
  */
-void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
+void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq input_pin);
 /**
  * qdev_connect_gpio_out_named: Connect one of a device's named output
  *                              GPIO lines
  * @dev: Device whose GPIO to connect
  * @name: Name of the output GPIO array
  * @n: Number of the anonymous output GPIO line (which must be in range)
- * @pin: qemu_irq to connect the output line to
+ * @input_pin: qemu_irq to connect the output line to
  *
  * This function connects an anonymous output GPIO line on a device
  * up to an arbitrary qemu_irq, so that when the device asserts that
@@ -521,7 +521,7 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
  * same qemu_irq; see qdev_connect_gpio_out() for details.
  */
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
-                                 qemu_irq pin);
+                                 qemu_irq input_pin);
 /**
  * qdev_get_gpio_out_connector: Get the qemu_irq connected to an output GPIO
  * @dev: Device whose output GPIO we are interested in
diff --git a/hw/core/gpio.c b/hw/core/gpio.c
index 8e6b4f5edf3..80d07a6ec99 100644
--- a/hw/core/gpio.c
+++ b/hw/core/gpio.c
@@ -115,17 +115,18 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
 }
 
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
-                                 qemu_irq pin)
+                                 qemu_irq input_pin)
 {
     char *propname = g_strdup_printf("%s[%d]",
                                      name ? name : "unnamed-gpio-out", n);
-    if (pin && !OBJECT(pin)->parent) {
+    if (input_pin && !OBJECT(input_pin)->parent) {
         /* We need a name for object_property_set_link to work */
         object_property_add_child(container_get(qdev_get_machine(),
                                                 "/unattached"),
-                                  "non-qdev-gpio[*]", OBJECT(pin));
+                                  "non-qdev-gpio[*]", OBJECT(input_pin));
     }
-    object_property_set_link(OBJECT(dev), propname, OBJECT(pin), &error_abort);
+    object_property_set_link(OBJECT(dev), propname,
+                             OBJECT(input_pin), &error_abort);
     g_free(propname);
 }
 
@@ -165,9 +166,9 @@ qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
     return disconnected;
 }
 
-void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin)
+void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq input_pin)
 {
-    qdev_connect_gpio_out_named(dev, NULL, n, pin);
+    qdev_connect_gpio_out_named(dev, NULL, n, input_pin);
 }
 
 void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
-- 
2.31.1

Re: [PATCH 2/3] hw/qdev: Rename qdev_connect_gpio_out*() 'input_pin' parameter

[wangyanan (Y)]

On 2021/11/6 1:21, Philippe Mathieu-Daudé wrote:
> @pin is an input where we connect a device output.
> Rename it @input_pin to simplify the documentation.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/hw/qdev-core.h |  8 ++++----
>   hw/core/gpio.c         | 13 +++++++------
>   2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 5b88c8b9dd3..3a0fe643a73 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -470,7 +470,7 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
>    * qdev_connect_gpio_out: Connect one of a device's anonymous output GPIO lines
>    * @dev: Device whose GPIO to connect
>    * @n: Number of the anonymous output GPIO line (which must be in range)
> - * @pin: qemu_irq to connect the output line to
> + * @input_pin: qemu_irq to connect the output line to
>    *
>    * This function connects an anonymous output GPIO line on a device
>    * up to an arbitrary qemu_irq, so that when the device asserts that
> @@ -495,14 +495,14 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n);
>    *
>    * For named output GPIO lines, use qdev_connect_gpio_out_named().
>    */
> -void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
> +void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq input_pin);
>   /**
>    * qdev_connect_gpio_out_named: Connect one of a device's named output
>    *                              GPIO lines
>    * @dev: Device whose GPIO to connect
>    * @name: Name of the output GPIO array
>    * @n: Number of the anonymous output GPIO line (which must be in range)
> - * @pin: qemu_irq to connect the output line to
> + * @input_pin: qemu_irq to connect the output line to
>    *
>    * This function connects an anonymous output GPIO line on a device
>    * up to an arbitrary qemu_irq, so that when the device asserts that
> @@ -521,7 +521,7 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
>    * same qemu_irq; see qdev_connect_gpio_out() for details.
>    */
>   void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
> -                                 qemu_irq pin);
> +                                 qemu_irq input_pin);
>   /**
>    * qdev_get_gpio_out_connector: Get the qemu_irq connected to an output GPIO
>    * @dev: Device whose output GPIO we are interested in
> diff --git a/hw/core/gpio.c b/hw/core/gpio.c
> index 8e6b4f5edf3..80d07a6ec99 100644
> --- a/hw/core/gpio.c
> +++ b/hw/core/gpio.c
> @@ -115,17 +115,18 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n)
>   }
>   
>   void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
> -                                 qemu_irq pin)
> +                                 qemu_irq input_pin)
>   {
>       char *propname = g_strdup_printf("%s[%d]",
>                                        name ? name : "unnamed-gpio-out", n);
> -    if (pin && !OBJECT(pin)->parent) {
> +    if (input_pin && !OBJECT(input_pin)->parent) {
>           /* We need a name for object_property_set_link to work */
>           object_property_add_child(container_get(qdev_get_machine(),
>                                                   "/unattached"),
> -                                  "non-qdev-gpio[*]", OBJECT(pin));
> +                                  "non-qdev-gpio[*]", OBJECT(input_pin));
>       }
> -    object_property_set_link(OBJECT(dev), propname, OBJECT(pin), &error_abort);
> +    object_property_set_link(OBJECT(dev), propname,
> +                             OBJECT(input_pin), &error_abort);
>       g_free(propname);
>   }
>   
> @@ -165,9 +166,9 @@ qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq icpt,
>       return disconnected;
>   }
>   
> -void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin)
> +void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq input_pin)
>   {
> -    qdev_connect_gpio_out_named(dev, NULL, n, pin);
> +    qdev_connect_gpio_out_named(dev, NULL, n, input_pin);
>   }
>   
>   void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan

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

[Philippe Mathieu-Daudé]

'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]);
     qdev_connect_gpio_out_named(DEVICE(port92),
                                 PORT92_A20_LINE, 0, a20_line[1]);
     g_free(a20_line);
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index baba62f357a..7b842c8f3b5 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -680,9 +680,9 @@ void i8042_isa_mouse_fake_event(ISAKBDState *isa)
     ps2_mouse_fake_event(s->mouse);
 }
 
-void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out)
+void i8042_isa_setup_a20_line(ISADevice *dev, qemu_irq a20_input)
 {
-    qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, a20_out);
+    qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, a20_input);
 }
 
 static const VMStateDescription vmstate_kbd_isa = {
-- 
2.31.1

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

[wangyanan (Y)]

On 2021/11/6 1:21, Philippe Mathieu-Daudé 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]);
>       qdev_connect_gpio_out_named(DEVICE(port92),
>                                   PORT92_A20_LINE, 0, a20_line[1]);
>       g_free(a20_line);
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index baba62f357a..7b842c8f3b5 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -680,9 +680,9 @@ void i8042_isa_mouse_fake_event(ISAKBDState *isa)
>       ps2_mouse_fake_event(s->mouse);
>   }
>   
> -void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out)
> +void i8042_isa_setup_a20_line(ISADevice *dev, qemu_irq a20_input)
>   {
> -    qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, a20_out);
> +    qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, a20_input);
>   }
>   
>   static const VMStateDescription vmstate_kbd_isa = {
Looks reasonable:
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan

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

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

[Philippe Mathieu-Daudé]

On 11/22/21 12:14, Peter Maydell wrote:
> 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.

Very good point, thank you.

> (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.)

I once worked on an "info irqtree" HMP command; if I finish it
then would be nice to display.