diff mbox series

gpiolib: make struct comments into real kernel docs

Message ID 20220202104937.146827-1-brgl@bgdev.pl
State Superseded
Headers show
Series gpiolib: make struct comments into real kernel docs | expand

Commit Message

Bartosz Golaszewski Feb. 2, 2022, 10:49 a.m. UTC
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@infradead.org>
Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpiolib.h        | 31 +++++++++++++++++++++++++++++++
 include/linux/gpio/consumer.h | 35 ++++++++++++++++-------------------
 2 files changed, 47 insertions(+), 19 deletions(-)

Comments

Bartosz Golaszewski Feb. 8, 2022, 10:46 a.m. UTC | #1
On Wed, Feb 2, 2022 at 12:19 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Feb 02, 2022 at 11:49:37AM +0100, 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.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> A few comments below.
>
> > Reported-by: Randy Dunlap <rdunlap@infradead.org>
> > Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
> > ---
> >  drivers/gpio/gpiolib.h        | 31 +++++++++++++++++++++++++++++++
> >  include/linux/gpio/consumer.h | 35 ++++++++++++++++-------------------
> >  2 files changed, 47 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
> > index 30bc3f80f83e..0025701b7641 100644
> > --- a/drivers/gpio/gpiolib.h
> > +++ b/drivers/gpio/gpiolib.h
> > @@ -72,6 +72,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
>
> > + *
>
> I dunno we need these blank lines.

I think it looks better. Some docs use it some don't, there's no standard.

>
> > + * @desc:            Array of pointers to the GPIO descriptors
> > + * @size:            Number of elements in desc
>
>   in @desc
>
> or
>
>   in the array of the descriptor pointers
>
> > + * @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.
>
> Why some of the descriptions are with grammar period and some w/o?
>
> > + *
> > + * 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 gpio_desc        **desc;
> >       unsigned int            size;
> > @@ -96,6 +110,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 gpio_desc is guaranteed to be valid
>
> &struct gpio_desc
>
> > + * 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..3ffb054fafbd 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
>
> Missed @?
>
> >   */
> >  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 the direction
>
> Don't change anything
>
> (Not only direction, also the output value. Some hardware allows to change
> output buffer value even if the line is configured as input).
>
> > + * @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,
> > --
> > 2.30.1
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Fixed the rest. Thanks!

Bart
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 30bc3f80f83e..0025701b7641 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -72,6 +72,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
+ * 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 +110,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 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..3ffb054fafbd 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 the direction
+ * @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,