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

[PATCH v2 0/5] 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.

Since v1:
- Addressed Yanan Wang and Peter Maydell comments:
- Correct qdev_init_gpio_out_named() doc
- Drop i8042_setup_a20_line() wrapper

Philippe Mathieu-Daudé (5):
  hw/qdev: Cosmetic around documentation
  hw/qdev: Correct qdev_init_gpio_out_named() documentation
  hw/qdev: Correct qdev_connect_gpio_out_named() documentation
  hw/qdev: Rename qdev_connect_gpio_out*() 'input_pin' parameter
  hw/input/pckbd: Open-code i8042_setup_a20_line() wrapper

 include/hw/input/i8042.h |  1 -
 include/hw/qdev-core.h   | 24 ++++++++++++++++++------
 hw/core/gpio.c           | 13 +++++++------
 hw/i386/pc.c             |  3 ++-
 hw/input/pckbd.c         |  5 -----
 5 files changed, 27 insertions(+), 19 deletions(-)

-- 
2.33.1

[PATCH v2 1/5] hw/qdev: Cosmetic around documentation

[Philippe Mathieu-Daudé]

Add empty lines to have a clearer distinction between different
functions declarations.

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

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 20d3066595e..59a822ffceb 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -321,6 +321,7 @@ compat_props_add(GPtrArray *arr,
  * The returned object has a reference count of 1.
  */
 DeviceState *qdev_new(const char *name);
+
 /**
  * qdev_try_new: Try to create a device on the heap
  * @name: device type to create
@@ -329,6 +330,7 @@ DeviceState *qdev_new(const char *name);
  * does not exist, rather than asserting.
  */
 DeviceState *qdev_try_new(const char *name);
+
 /**
  * qdev_realize: Realize @dev.
  * @dev: device to realize
@@ -347,6 +349,7 @@ DeviceState *qdev_try_new(const char *name);
  * qdev_realize_and_unref() instead.
  */
 bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp);
+
 /**
  * qdev_realize_and_unref: Realize @dev and drop a reference
  * @dev: device to realize
@@ -372,6 +375,7 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp);
  * would be incorrect. For that use case you want qdev_realize().
  */
 bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp);
+
 /**
  * qdev_unrealize: Unrealize a device
  * @dev: device to unrealize
@@ -450,6 +454,7 @@ typedef enum {
  * For named input GPIO lines, use qdev_get_gpio_in_named().
  */
 qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
+
 /**
  * qdev_get_gpio_in_named: Get one of a device's named input GPIO lines
  * @dev: Device whose GPIO we want
@@ -497,6 +502,7 @@ 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);
+
 /**
  * qdev_connect_gpio_out: Connect one of a device's anonymous output GPIO lines
  * @dev: Device whose GPIO to connect
@@ -524,6 +530,7 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
  */
 void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
                                  qemu_irq pin);
+
 /**
  * qdev_get_gpio_out_connector: Get the qemu_irq connected to an output GPIO
  * @dev: Device whose output GPIO we are interested in
@@ -541,6 +548,7 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
  * by the platform-bus subsystem.
  */
 qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
+
 /**
  * qdev_intercept_gpio_out: Intercept an existing GPIO connection
  * @dev: Device to intercept the outbound GPIO line from
@@ -582,6 +590,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
  * hold of an input GPIO line to manipulate it.
  */
 void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
+
 /**
  * qdev_init_gpio_out: create an array of anonymous output GPIO lines
  * @dev: Device to create output GPIOs for
@@ -610,6 +619,7 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
  * handler.
  */
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
+
 /**
  * qdev_init_gpio_out: create an array of named output GPIO lines
  * @dev: Device to create output GPIOs for
@@ -623,6 +633,7 @@ void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
  */
 void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
                               const char *name, int n);
+
 /**
  * qdev_init_gpio_in_named_with_opaque: create an array of input GPIO lines
  *   for the specified device
-- 
2.33.1

Re: [PATCH v2 1/5] hw/qdev: Cosmetic around documentation

[wangyanan (Y) via]

On 2021/12/18 21:04, Philippe Mathieu-Daudé wrote:
> Add empty lines to have a clearer distinction between different
> functions declarations.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/hw/qdev-core.h | 11 +++++++++++
>   1 file changed, 11 insertions(+)
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 20d3066595e..59a822ffceb 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -321,6 +321,7 @@ compat_props_add(GPtrArray *arr,
>    * The returned object has a reference count of 1.
>    */
>   DeviceState *qdev_new(const char *name);
> +
>   /**
>    * qdev_try_new: Try to create a device on the heap
>    * @name: device type to create
> @@ -329,6 +330,7 @@ DeviceState *qdev_new(const char *name);
>    * does not exist, rather than asserting.
>    */
>   DeviceState *qdev_try_new(const char *name);
> +
>   /**
>    * qdev_realize: Realize @dev.
>    * @dev: device to realize
> @@ -347,6 +349,7 @@ DeviceState *qdev_try_new(const char *name);
>    * qdev_realize_and_unref() instead.
>    */
>   bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp);
> +
>   /**
>    * qdev_realize_and_unref: Realize @dev and drop a reference
>    * @dev: device to realize
> @@ -372,6 +375,7 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp);
>    * would be incorrect. For that use case you want qdev_realize().
>    */
>   bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp);
> +
>   /**
>    * qdev_unrealize: Unrealize a device
>    * @dev: device to unrealize
> @@ -450,6 +454,7 @@ typedef enum {
>    * For named input GPIO lines, use qdev_get_gpio_in_named().
>    */
>   qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
> +
>   /**
>    * qdev_get_gpio_in_named: Get one of a device's named input GPIO lines
>    * @dev: Device whose GPIO we want
> @@ -497,6 +502,7 @@ 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);
> +
>   /**
>    * qdev_connect_gpio_out: Connect one of a device's anonymous output GPIO lines
>    * @dev: Device whose GPIO to connect
> @@ -524,6 +530,7 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
>    */
>   void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
>                                    qemu_irq pin);
> +
>   /**
>    * qdev_get_gpio_out_connector: Get the qemu_irq connected to an output GPIO
>    * @dev: Device whose output GPIO we are interested in
> @@ -541,6 +548,7 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
>    * by the platform-bus subsystem.
>    */
>   qemu_irq qdev_get_gpio_out_connector(DeviceState *dev, const char *name, int n);
> +
>   /**
>    * qdev_intercept_gpio_out: Intercept an existing GPIO connection
>    * @dev: Device to intercept the outbound GPIO line from
> @@ -582,6 +590,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
>    * hold of an input GPIO line to manipulate it.
>    */
>   void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
> +
>   /**
>    * qdev_init_gpio_out: create an array of anonymous output GPIO lines
>    * @dev: Device to create output GPIOs for
> @@ -610,6 +619,7 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
>    * handler.
>    */
>   void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
> +
>   /**
>    * qdev_init_gpio_out: create an array of named output GPIO lines
>    * @dev: Device to create output GPIOs for
> @@ -623,6 +633,7 @@ void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
>    */
>   void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
>                                 const char *name, int n);
> +
>   /**
>    * qdev_init_gpio_in_named_with_opaque: create an array of input GPIO lines
>    *   for the specified device

[PATCH v2 2/5] hw/qdev: Correct qdev_init_gpio_out_named() documentation

[Philippe Mathieu-Daudé]

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

Reported-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/qdev-core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 59a822ffceb..dcf20c69b89 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -621,7 +621,7 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
 void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
 
 /**
- * qdev_init_gpio_out: create an array of named output GPIO lines
+ * qdev_init_gpio_out_named: create an array of named output GPIO lines
  * @dev: Device to create output GPIOs for
  * @pins: Pointer to qemu_irq or qemu_irq array for the GPIO lines
  * @name: Name to give this array of GPIO lines
-- 
2.33.1

Re: [PATCH v2 2/5] hw/qdev: Correct qdev_init_gpio_out_named() documentation

[wangyanan (Y) via]

On 2021/12/18 21:04, Philippe Mathieu-Daudé wrote:
> qdev_init_gpio_out_named() is described as qdev_init_gpio_out(),
> and referring to itself in an endless loop, which is confusing. Fix.
>
> Reported-by: Yanan Wang <wangyanan55@huawei.com>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/hw/qdev-core.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 59a822ffceb..dcf20c69b89 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -621,7 +621,7 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n);
>   void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
>   
>   /**
> - * qdev_init_gpio_out: create an array of named output GPIO lines
> + * qdev_init_gpio_out_named: create an array of named output GPIO lines
>    * @dev: Device to create output GPIOs for
>    * @pins: Pointer to qemu_irq or qemu_irq array for the GPIO lines
>    * @name: Name to give this array of GPIO lines

[PATCH v2 3/5] 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, 3 insertions(+), 2 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index dcf20c69b89..424c48daf6e 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -504,7 +504,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)
@@ -526,7 +527,7 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
  * 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().
+ * 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);
-- 
2.33.1

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

[wangyanan (Y) via]

On 2021/12/18 21:04, 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, 3 insertions(+), 2 deletions(-)
Looks good,
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index dcf20c69b89..424c48daf6e 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -504,7 +504,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)
> @@ -526,7 +527,7 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
>    * 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().
> + * 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);

[PATCH v2 4/5] 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.

Reviewed-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/qdev-core.h |  6 +++---
 hw/core/gpio.c         | 13 +++++++------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 424c48daf6e..d19c9417520 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -476,7 +476,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
@@ -509,7 +509,7 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
  * @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
@@ -530,7 +530,7 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
  * 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);
+                                 qemu_irq input_pin);
 
 /**
  * qdev_get_gpio_out_connector: Get the qemu_irq connected to an output GPIO
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.33.1

[PATCH v2 5/5] hw/input/pckbd: Open-code i8042_setup_a20_line() wrapper

[Philippe Mathieu-Daudé]

Since the named GPIO lines are a "public" interface to the device,
we can directly call qdev_connect_gpio_out_named(), making it
consistent with how the other A20 input source (port92) is wired.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/input/i8042.h | 1 -
 hw/i386/pc.c             | 3 ++-
 hw/input/pckbd.c         | 5 -----
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
index 1d90432daef..1cd53b8d221 100644
--- a/include/hw/input/i8042.h
+++ b/include/hw/input/i8042.h
@@ -21,6 +21,5 @@ 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);
 
 #endif /* HW_INPUT_I8042_H */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a2ef40ecbc2..e2f7cd4aec6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1045,7 +1045,8 @@ 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]);
+    qdev_connect_gpio_out_named(DEVICE(i8042),
+                                I8042_A20_LINE, 0, 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..7fd4477d742 100644
--- a/hw/input/pckbd.c
+++ b/hw/input/pckbd.c
@@ -680,11 +680,6 @@ 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)
-{
-    qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, a20_out);
-}
-
 static const VMStateDescription vmstate_kbd_isa = {
     .name = "pckbd",
     .version_id = 3,
-- 
2.33.1

Re: [PATCH v2 5/5] hw/input/pckbd: Open-code i8042_setup_a20_line() wrapper

[wangyanan (Y) via]

On 2021/12/18 21:04, Philippe Mathieu-Daudé wrote:
> Since the named GPIO lines are a "public" interface to the device,
> we can directly call qdev_connect_gpio_out_named(), making it
> consistent with how the other A20 input source (port92) is wired.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   include/hw/input/i8042.h | 1 -
>   hw/i386/pc.c             | 3 ++-
>   hw/input/pckbd.c         | 5 -----
>   3 files changed, 2 insertions(+), 7 deletions(-)
Reviewed-by: Yanan Wang <wangyanan55@huawei.com>

Thanks,
Yanan
> diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
> index 1d90432daef..1cd53b8d221 100644
> --- a/include/hw/input/i8042.h
> +++ b/include/hw/input/i8042.h
> @@ -21,6 +21,5 @@ 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);
>   
>   #endif /* HW_INPUT_I8042_H */
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a2ef40ecbc2..e2f7cd4aec6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1045,7 +1045,8 @@ 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]);
> +    qdev_connect_gpio_out_named(DEVICE(i8042),
> +                                I8042_A20_LINE, 0, 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..7fd4477d742 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -680,11 +680,6 @@ 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)
> -{
> -    qdev_connect_gpio_out_named(DEVICE(dev), I8042_A20_LINE, 0, a20_out);
> -}
> -
>   static const VMStateDescription vmstate_kbd_isa = {
>       .name = "pckbd",
>       .version_id = 3,

Re: [PATCH v2 0/5] hw/qdev: Clarify qdev_connect_gpio_out() documentation

[Philippe Mathieu-Daudé]

Hi Peter.

Since you reviewed v1, and Ack-by on v2 would be welcomed.
Otherwise, if you don't object, I plan to queue this via
machine-next tree.

Thanks,

Phil.

On 12/18/21 14:04, Philippe Mathieu-Daudé wrote:
> Trivial patches clarifying qdev_connect_gpio_out() use,
> basically that the qemu_irq argument is an input.
> 
> Since v1:
> - Addressed Yanan Wang and Peter Maydell comments:
> - Correct qdev_init_gpio_out_named() doc
> - Drop i8042_setup_a20_line() wrapper
> 
> Philippe Mathieu-Daudé (5):
>   hw/qdev: Cosmetic around documentation
>   hw/qdev: Correct qdev_init_gpio_out_named() documentation
>   hw/qdev: Correct qdev_connect_gpio_out_named() documentation
>   hw/qdev: Rename qdev_connect_gpio_out*() 'input_pin' parameter
>   hw/input/pckbd: Open-code i8042_setup_a20_line() wrapper
> 
>  include/hw/input/i8042.h |  1 -
>  include/hw/qdev-core.h   | 24 ++++++++++++++++++------
>  hw/core/gpio.c           | 13 +++++++------
>  hw/i386/pc.c             |  3 ++-
>  hw/input/pckbd.c         |  5 -----
>  5 files changed, 27 insertions(+), 19 deletions(-)
> 

Re: [PATCH v2 0/5] hw/qdev: Clarify qdev_connect_gpio_out() documentation

[Peter Maydell]

On Thu, 23 Dec 2021 at 09:05, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Hi Peter.
>
> Since you reviewed v1, and Ack-by on v2 would be welcomed.
> Otherwise, if you don't object, I plan to queue this via
> machine-next tree.

Been on holiday since mid-Dec :-)  I see this is already
upstream, which is fine, I have no objections.

-- PMM

Re: [PATCH v2 0/5] hw/qdev: Clarify qdev_connect_gpio_out() documentation

[Philippe Mathieu-Daudé]

On 12/18/21 14:04, Philippe Mathieu-Daudé wrote:
> Trivial patches clarifying qdev_connect_gpio_out() use,
> basically that the qemu_irq argument is an input.
> 
> Since v1:
> - Addressed Yanan Wang and Peter Maydell comments:
> - Correct qdev_init_gpio_out_named() doc
> - Drop i8042_setup_a20_line() wrapper
> 
> Philippe Mathieu-Daudé (5):
>   hw/qdev: Cosmetic around documentation
>   hw/qdev: Correct qdev_init_gpio_out_named() documentation
>   hw/qdev: Correct qdev_connect_gpio_out_named() documentation
>   hw/qdev: Rename qdev_connect_gpio_out*() 'input_pin' parameter
>   hw/input/pckbd: Open-code i8042_setup_a20_line() wrapper

Queuing patches 1-4 via machine-next.