[PATCH] include/hw/qdev-core.h: Correct and clarify gpio doc comments

[PATCH] include/hw/qdev-core.h: Correct and clarify gpio doc comments

[Peter Maydell]

The doc comments for the functions for named GPIO inputs and
outputs had a couple of problems:
 * some copy-and-paste errors meant the qdev_connect_gpio_out_named()
   doc comment had references to input GPIOs that should be to
   output GPIOs
 * it wasn't very clear that named GPIOs are arrays and so the
   connect functions specify a single GPIO line by giving both
   the name of the array and the index within that array

Fix the copy-and-paste errors and slightly expand the text
to say that functions are connecting one line in a named GPIO
array, not a single named GPIO line.

Reported-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Does this help? I wrote the text and know what the functions do
to start with so it's a bit tricky for me to see where the docs
don't state things as clearly as they ought to do...
---
 include/hw/qdev-core.h | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 5336728a23f..77bfcbdf732 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -624,8 +624,9 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
  * @name: Name of the input GPIO array
  * @n: Number of the GPIO line in that array (which must be in range)
  *
- * Returns the qemu_irq corresponding to a named input GPIO line
- * (which the device has set up with qdev_init_gpio_in_named()).
+ * Returns the qemu_irq corresponding to a single input GPIO line
+ * in a named array of input GPIO lines on a device (which the device
+ * has set up with qdev_init_gpio_in_named()).
  * The @name string must correspond to an input GPIO array which exists on
  * the device, and the index @n of the GPIO line must be valid (i.e.
  * be at least 0 and less than the total number of input GPIOs in that
@@ -673,15 +674,15 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
  *                              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)
+ * @n: Number of the output GPIO line within that array (which must be in range)
  * @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
- * output GPIO line, the qemu_irq's callback is invoked.
+ * This function connects a single GPIO output in a named array of output
+ * GPIO lines on a device up to an arbitrary qemu_irq, so that when the
+ * device asserts that output GPIO line, the qemu_irq's callback is invoked.
  * The @name string must correspond to an output GPIO array which exists on
  * the device, and the index @n of the GPIO line must be valid (i.e.
- * be at least 0 and less than the total number of input GPIOs in that
+ * be at least 0 and less than the total number of output GPIOs in that
  * array); this function will assert() if passed an invalid name or index.
  *
  * Outbound GPIO lines can be connected to any qemu_irq, but the common
@@ -796,7 +797,7 @@ void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
  * @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
- * @n: Number of GPIO lines to create
+ * @n: Number of GPIO lines to create in this array
  *
  * Like qdev_init_gpio_out(), but creates an array of GPIO output lines
  * with a name. Code using the device can then connect these GPIO lines
-- 
2.34.1

Re: [PATCH] include/hw/qdev-core.h: Correct and clarify gpio doc comments

[BALATON Zoltan]

On Mon, 8 Jul 2024, Peter Maydell wrote:
> The doc comments for the functions for named GPIO inputs and
> outputs had a couple of problems:
> * some copy-and-paste errors meant the qdev_connect_gpio_out_named()
>   doc comment had references to input GPIOs that should be to
>   output GPIOs
> * it wasn't very clear that named GPIOs are arrays and so the
>   connect functions specify a single GPIO line by giving both
>   the name of the array and the index within that array
>
> Fix the copy-and-paste errors and slightly expand the text
> to say that functions are connecting one line in a named GPIO
> array, not a single named GPIO line.
>
> Reported-by: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Does this help? I wrote the text and know what the functions do
> to start with so it's a bit tricky for me to see where the docs
> don't state things as clearly as they ought to do...

Yes, I think it is clearer. Thanks for fixing these. I could figure this 
out despite the errors and I've used multiple named gpios for vt82c686 and 
mv64361 before, I just forgot how these functions work. Making the docs 
less confusing helps, for quick reference and especially newcomers who try 
to use this for the first time.

Regards,
BALATON Zoltan

> ---
> include/hw/qdev-core.h | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 5336728a23f..77bfcbdf732 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -624,8 +624,9 @@ qemu_irq qdev_get_gpio_in(DeviceState *dev, int n);
>  * @name: Name of the input GPIO array
>  * @n: Number of the GPIO line in that array (which must be in range)
>  *
> - * Returns the qemu_irq corresponding to a named input GPIO line
> - * (which the device has set up with qdev_init_gpio_in_named()).
> + * Returns the qemu_irq corresponding to a single input GPIO line
> + * in a named array of input GPIO lines on a device (which the device
> + * has set up with qdev_init_gpio_in_named()).
>  * The @name string must correspond to an input GPIO array which exists on
>  * the device, and the index @n of the GPIO line must be valid (i.e.
>  * be at least 0 and less than the total number of input GPIOs in that
> @@ -673,15 +674,15 @@ void qdev_connect_gpio_out(DeviceState *dev, int n, qemu_irq pin);
>  *                              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)
> + * @n: Number of the output GPIO line within that array (which must be in range)
>  * @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
> - * output GPIO line, the qemu_irq's callback is invoked.
> + * This function connects a single GPIO output in a named array of output
> + * GPIO lines on a device up to an arbitrary qemu_irq, so that when the
> + * device asserts that output GPIO line, the qemu_irq's callback is invoked.
>  * The @name string must correspond to an output GPIO array which exists on
>  * the device, and the index @n of the GPIO line must be valid (i.e.
> - * be at least 0 and less than the total number of input GPIOs in that
> + * be at least 0 and less than the total number of output GPIOs in that
>  * array); this function will assert() if passed an invalid name or index.
>  *
>  * Outbound GPIO lines can be connected to any qemu_irq, but the common
> @@ -796,7 +797,7 @@ void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
>  * @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
> - * @n: Number of GPIO lines to create
> + * @n: Number of GPIO lines to create in this array
>  *
>  * Like qdev_init_gpio_out(), but creates an array of GPIO output lines
>  * with a name. Code using the device can then connect these GPIO lines
>

Re: [PATCH] include/hw/qdev-core.h: Correct and clarify gpio doc comments

[Philippe Mathieu-Daudé]

On 8/7/24 17:33, Peter Maydell wrote:
> The doc comments for the functions for named GPIO inputs and
> outputs had a couple of problems:
>   * some copy-and-paste errors meant the qdev_connect_gpio_out_named()
>     doc comment had references to input GPIOs that should be to
>     output GPIOs
>   * it wasn't very clear that named GPIOs are arrays and so the
>     connect functions specify a single GPIO line by giving both
>     the name of the array and the index within that array
> 
> Fix the copy-and-paste errors and slightly expand the text
> to say that functions are connecting one line in a named GPIO
> array, not a single named GPIO line.
> 
> Reported-by: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Does this help? I wrote the text and know what the functions do
> to start with so it's a bit tricky for me to see where the docs
> don't state things as clearly as they ought to do...
> ---
>   include/hw/qdev-core.h | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)


> - * This function connects an anonymous output GPIO line on a device
> - * up to an arbitrary qemu_irq, so that when the device asserts that
> - * output GPIO line, the qemu_irq's callback is invoked.
> + * This function connects a single GPIO output in a named array of output
> + * GPIO lines on a device up to an arbitrary qemu_irq, so that when the
> + * device asserts that output GPIO line, the qemu_irq's callback is invoked.
>    * The @name string must correspond to an output GPIO array which exists on
>    * the device, and the index @n of the GPIO line must be valid (i.e.
> - * be at least 0 and less than the total number of input GPIOs in that
> + * be at least 0 and less than the total number of output GPIOs in that

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

And queued, thanks!