Re: [PATCH v2] gpiolib: make struct comments into real kernel docs

[Randy Dunlap <rdunlap@xxxxxxxxxxxxx>]

Hi--

On 2/8/22 02:48, Bartosz Golaszewski wrote:
> We have several comments that start with '/**' but don't conform to the
> kernel doc standard. Add proper detailed descriptions for the affected
> definitions and move the docs from the forward declarations to the
> struct definitions where applicable.
> 
> Reported-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> Signed-off-by: Bartosz Golaszewski <brgl@xxxxxxxx>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>

> ---
> v1 -> v2:
> - described fields not yet documented
> - added missing '@'
> - made using periods consistent
> - fixed struct references
> 
>  drivers/gpio/gpiolib.h        | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/gpio/consumer.h | 35 ++++++++++++++++-------------------
>  2 files changed, 50 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> index 30bc3f80f83e..40723a179902 100644
> --- a/drivers/gpio/gpiolib.h
> +++ b/drivers/gpio/gpiolib.h
> @@ -37,6 +37,9 @@
>   * or name of the IP component in a System on Chip.
>   * @data: per-instance data assigned by the driver
>   * @list: links gpio_device:s together for traversal
> + * @notifier: used to notify subscribers about lines being requested, released
> + *            or reconfigured
> + * @pin_ranges: range of pins served by the GPIO driver
>   *
>   * This state container holds most of the runtime variable data
>   * for a GPIO device and can hold references and live on after the
> @@ -72,6 +75,20 @@ struct gpio_device {
>  /* gpio suffixes used for ACPI and device tree lookup */
>  static __maybe_unused const char * const gpio_suffixes[] = { "gpios", "gpio" };
>  
> +/**
> + * struct gpio_array - Opaque descriptor for a structure of GPIO array attributes
> + *
> + * @desc:		Array of pointers to the GPIO descriptors
> + * @size:		Number of elements in desc
> + * @chip:		Parent GPIO chip
> + * @get_mask:		Get mask used in fastpath
> + * @set_mask:		Set mask used in fastpath
> + * @invert_mask:	Invert mask used in fastpath
> + *
> + * This structure is attached to struct gpiod_descs obtained from

maybe
                                    &struct gpiod_descs

> + * gpiod_get_array() and can be passed back to get/set array functions in order
> + * to activate fast processing path if applicable.
> + */
>  struct gpio_array {
>  	struct gpio_desc	**desc;
>  	unsigned int		size;
> @@ -96,6 +113,23 @@ int gpiod_set_array_value_complex(bool raw, bool can_sleep,
>  extern spinlock_t gpio_lock;
>  extern struct list_head gpio_devices;
>  
> +
> +/**
> + * struct gpio_desc - Opaque descriptor for a GPIO
> + *
> + * @gdev:		Pointer to the parent GPIO device
> + * @flags:		Binary descriptor flags
> + * @label:		Name of the consumer
> + * @name:		Line name
> + * @hog:		Pointer to the device node that hogs this line (if any)
> + * @debounce_period_us:	Debounce period in microseconds
> + *
> + * These are obtained using gpiod_get() and are preferable to the old
> + * integer-based handles.
> + *
> + * Contrary to integers, a pointer to a &struct gpio_desc is guaranteed to be
> + * valid until the GPIO is released.
> + */
>  struct gpio_desc {
>  	struct gpio_device	*gdev;
>  	unsigned long		flags;
> diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
> index 3ad67b4a72be..c3aa8b330e1c 100644
> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -8,27 +8,16 @@
>  #include <linux/err.h>
>  
>  struct device;
> -
> -/**
> - * Opaque descriptor for a GPIO. These are obtained using gpiod_get() and are
> - * preferable to the old integer-based handles.
> - *
> - * Contrary to integers, a pointer to a gpio_desc is guaranteed to be valid
> - * until the GPIO is released.
> - */
>  struct gpio_desc;
> -
> -/**
> - * Opaque descriptor for a structure of GPIO array attributes.  This structure
> - * is attached to struct gpiod_descs obtained from gpiod_get_array() and can be
> - * passed back to get/set array functions in order to activate fast processing
> - * path if applicable.
> - */
>  struct gpio_array;
>  
>  /**
> - * Struct containing an array of descriptors that can be obtained using
> - * gpiod_get_array().
> + * struct gpio_descs - Struct containing an array of descriptors that can be
> + *                     obtained using gpiod_get_array()
> + *
> + * @info:	Pointer to the opaque gpio_array structure
> + * @ndescs:	Number of held descriptors
> + * @desc:	Array of pointers to GPIO descriptors
>   */
>  struct gpio_descs {
>  	struct gpio_array *info;
> @@ -43,8 +32,16 @@ struct gpio_descs {
>  #define GPIOD_FLAGS_BIT_NONEXCLUSIVE	BIT(4)
>  
>  /**
> - * Optional flags that can be passed to one of gpiod_* to configure direction
> - * and output value. These values cannot be OR'd.
> + * enum gpiod_flags - Optional flags that can be passed to one of gpiod_* to
> + *                    configure direction and output value. These values
> + *                    cannot be OR'd.
> + *
> + * @GPIOD_ASIS:			Don't change anything
> + * @GPIOD_IN:			Set lines to input mode
> + * @GPIOD_OUT_LOW:		Set lines to output and drive them low
> + * @GPIOD_OUT_HIGH:		Set lines to output and drive them high
> + * @GPIOD_OUT_LOW_OPEN_DRAIN:	Set lines to open-drain output and drive them low
> + * @GPIOD_OUT_HIGH_OPEN_DRAIN:	Set lines to open-drain output and drive them high
>   */
>  enum gpiod_flags {
>  	GPIOD_ASIS	= 0,


thanks.
-- 
~Randy