[PATCH 0/5] gpio: uapi: documentation improvements

[PATCH 0/5] gpio: uapi: documentation improvements

[Kent Gibson]

This is a series of minor clarifications and formatting tidy ups for
the GPIO uAPI kernel doc.

The series is intended as a companion to my character device
uAPI documentation series, but makes sense on its own too.

The patches are self contained and trivial so not much to add here.

Cheers,
Kent.

Kent Gibson (5):
  gpio: uapi: improve description of fd fields
  gpio: uapi: clarify hte references
  gpio: uapi: drop trailing period from one sentence descriptions
  gpio: uapi: document possible values of gpioevent_data.id
  gpio: uapi: clarify using v2 rather than v1

 include/uapi/linux/gpio.h | 52 ++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 25 deletions(-)

-- 
2.39.2

[PATCH 1/5] gpio: uapi: improve description of fd fields

[Kent Gibson]

Only a successful operation modifies fd fields, but the current
documentation wording could be taken to imply that a positive fd
value after an ioctl() returns indicates a success.

Reword documentation to clarify that the fd is only valid after a
successful operation.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 include/uapi/linux/gpio.h | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index cb9966d49a16..650c1099d1a6 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -189,9 +189,8 @@ struct gpio_v2_line_config {
  * buffer. If this field is zero then the buffer size defaults to a minimum
  * of @num_lines * 16.
  * @padding: reserved for future use and must be zero filled
- * @fd: if successful this field will contain a valid anonymous file handle
- * after a %GPIO_GET_LINE_IOCTL operation, zero or negative value means
- * error
+ * @fd: after a successful %GPIO_V2_GET_LINE_IOCTL operation, contains
+ * a valid anonymous file descriptor representing the request
  */
 struct gpio_v2_line_request {
 	__u32 offsets[GPIO_V2_LINES_MAX];
@@ -402,9 +401,8 @@ struct gpioline_info_changed {
  * such as "my-bitbanged-relay"
  * @lines: number of lines requested in this request, i.e. the number of
  * valid fields in the above arrays, set to 1 to request a single line
- * @fd: if successful this field will contain a valid anonymous file handle
- * after a %GPIO_GET_LINEHANDLE_IOCTL operation, zero or negative value
- * means error
+ * @fd: after a successful %GPIO_GET_LINEHANDLE_IOCTL operation, contains
+ * a valid anonymous file descriptor representing the request
  *
  * Note: This struct is part of ABI v1 and is deprecated.
  * Use &struct gpio_v2_line_request instead.
@@ -465,9 +463,8 @@ struct gpiohandle_data {
  * %GPIOEVENT_REQUEST_RISING_EDGE or %GPIOEVENT_REQUEST_FALLING_EDGE
  * @consumer_label: a desired consumer label for the selected GPIO line(s)
  * such as "my-listener"
- * @fd: if successful this field will contain a valid anonymous file handle
- * after a %GPIO_GET_LINEEVENT_IOCTL operation, zero or negative value
- * means error
+ * @fd: after a successful %GPIO_GET_LINEEVENT_IOCTL operation, contains a
+ * valid anonymous file descriptor representing the request
  *
  * Note: This struct is part of ABI v1 and is deprecated.
  * Use &struct gpio_v2_line_request instead.
-- 
2.39.2

[PATCH 2/5] gpio: uapi: clarify hte references

[Kent Gibson]

The full name of the HTE subsystem is "hardware timestamping engine",
so correct references and highlight that this refers to the HTE
subsystem.

Extend the description of struct gpio_v2_line_event to clarify that
the timestamp_ns is sourced from the HTE if the
GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE is set.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 include/uapi/linux/gpio.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 650c1099d1a6..88c2a326940e 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -67,7 +67,7 @@ struct gpiochip_info {
  * @GPIO_V2_LINE_FLAG_BIAS_DISABLED: line has bias disabled
  * @GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME: line events contain REALTIME timestamps
  * @GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE: line events contain timestamps from
- * hardware timestamp engine
+ * the hardware timestamping engine (HTE) subsystem
  */
 enum gpio_v2_line_flag {
 	GPIO_V2_LINE_FLAG_USED			= _BITULL(0),
@@ -288,6 +288,10 @@ enum gpio_v2_line_event_id {
  *
  * If the %GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME flag is set then the
  * @timestamp_ns is read from %CLOCK_REALTIME.
+ *
+ * If the %GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE flag is set then the
+ * @timestamp_ns is provided by the hardware timestamping engine (HTE)
+ * subsystem.
  */
 struct gpio_v2_line_event {
 	__aligned_u64 timestamp_ns;
-- 
2.39.2

[PATCH 3/5] gpio: uapi: drop trailing period from one sentence descriptions

[Kent Gibson]

Make documentation more consistent by using trailing periods only for
multi-sentence field descriptions, not single sentence descriptions.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 include/uapi/linux/gpio.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 88c2a326940e..3ba8996e496c 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -88,10 +88,10 @@ enum gpio_v2_line_flag {
 /**
  * struct gpio_v2_line_values - Values of GPIO lines
  * @bits: a bitmap containing the value of the lines, set to 1 for active
- * and 0 for inactive.
+ * and 0 for inactive
  * @mask: a bitmap identifying the lines to get or set, with each bit
  * number corresponding to the index into &struct
- * gpio_v2_line_request.offsets.
+ * gpio_v2_line_request.offsets
  */
 struct gpio_v2_line_values {
 	__aligned_u64 bits;
@@ -123,7 +123,7 @@ enum gpio_v2_line_attr_id {
  * @values: if id is %GPIO_V2_LINE_ATTR_ID_OUTPUT_VALUES, a bitmap
  * containing the values to which the lines will be set, with each bit
  * number corresponding to the index into &struct
- * gpio_v2_line_request.offsets.
+ * gpio_v2_line_request.offsets
  * @debounce_period_us: if id is %GPIO_V2_LINE_ATTR_ID_DEBOUNCE, the
  * desired debounce period, in microseconds
  */
@@ -143,7 +143,7 @@ struct gpio_v2_line_attribute {
  * @attr: the configurable attribute
  * @mask: a bitmap identifying the lines to which the attribute applies,
  * with each bit number corresponding to the index into &struct
- * gpio_v2_line_request.offsets.
+ * gpio_v2_line_request.offsets
  */
 struct gpio_v2_line_config_attribute {
 	struct gpio_v2_line_attribute attr;
@@ -178,7 +178,7 @@ struct gpio_v2_line_config {
  * associated GPIO chip
  * @consumer: a desired consumer label for the selected GPIO lines such as
  * "my-bitbanged-relay"
- * @config: requested configuration for the lines.
+ * @config: requested configuration for the lines
  * @num_lines: number of lines requested in this request, i.e. the number
  * of valid fields in the %GPIO_V2_LINES_MAX sized arrays, set to 1 to
  * request a single line
@@ -216,7 +216,7 @@ struct gpio_v2_line_request {
  * @num_attrs: the number of attributes in @attrs
  * @flags: flags for this GPIO line, with values from &enum
  * gpio_v2_line_flag, such as %GPIO_V2_LINE_FLAG_ACTIVE_LOW,
- * %GPIO_V2_LINE_FLAG_OUTPUT etc, added together.
+ * %GPIO_V2_LINE_FLAG_OUTPUT etc, added together
  * @attrs: the configuration attributes associated with the line
  * @padding: reserved for future use
  */
@@ -273,7 +273,7 @@ enum gpio_v2_line_event_id {
 
 /**
  * struct gpio_v2_line_event - The actual event being pushed to userspace
- * @timestamp_ns: best estimate of time of event occurrence, in nanoseconds.
+ * @timestamp_ns: best estimate of time of event occurrence, in nanoseconds
  * @id: event identifier with value from &enum gpio_v2_line_event_id
  * @offset: the offset of the line that triggered the event
  * @seqno: the sequence number for this event in the sequence of events for
-- 
2.39.2

[PATCH 4/5] gpio: uapi: document possible values of gpioevent_data.id

[Kent Gibson]

Clarify the possible values of event id, rather than requiring the
reader to infer.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 include/uapi/linux/gpio.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index 3ba8996e496c..e68a56969f36 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -490,7 +490,8 @@ struct gpioevent_request {
 /**
  * struct gpioevent_data - The actual event being pushed to userspace
  * @timestamp: best estimate of time of event occurrence, in nanoseconds
- * @id: event identifier
+ * @id: event identifier, one of %GPIOEVENT_EVENT_RISING_EDGE or
+ *  %GPIOEVENT_EVENT_FALLING_EDGE
  *
  * Note: This struct is part of ABI v1 and is deprecated.
  * Use &struct gpio_v2_line_event instead.
-- 
2.39.2

[PATCH 5/5] gpio: uapi: clarify using v2 rather than v1

[Kent Gibson]

The documentation contains notes like
 This struct is part of ABI v1 and is deprecated.
 Use struct gpio_v2_line_info instead.

This could be interpreted to mean the structs can be directly
substituted in v1 calls.  Clarify that the user should use the
corresponding v2 ioctl() and structs.

Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
 include/uapi/linux/gpio.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/gpio.h b/include/uapi/linux/gpio.h
index e68a56969f36..103cd3c6c81e 100644
--- a/include/uapi/linux/gpio.h
+++ b/include/uapi/linux/gpio.h
@@ -333,7 +333,7 @@ struct gpio_v2_line_event {
  * also be empty if the consumer doesn't set this up
  *
  * Note: This struct is part of ABI v1 and is deprecated.
- * Use &struct gpio_v2_line_info instead.
+ * Use ABI v2 and &struct gpio_v2_line_info instead.
  */
 struct gpioline_info {
 	__u32 line_offset;
@@ -368,7 +368,7 @@ enum {
  * at the end of the structure on 64-bit architectures.
  *
  * Note: This struct is part of ABI v1 and is deprecated.
- * Use &struct gpio_v2_line_info_changed instead.
+ * Use ABI v2 and &struct gpio_v2_line_info_changed instead.
  */
 struct gpioline_info_changed {
 	struct gpioline_info info;
@@ -409,7 +409,7 @@ struct gpioline_info_changed {
  * a valid anonymous file descriptor representing the request
  *
  * Note: This struct is part of ABI v1 and is deprecated.
- * Use &struct gpio_v2_line_request instead.
+ * Use ABI v2 and &struct gpio_v2_line_request instead.
  */
 struct gpiohandle_request {
 	__u32 lineoffsets[GPIOHANDLES_MAX];
@@ -431,7 +431,7 @@ struct gpiohandle_request {
  * @padding: reserved for future use and should be zero filled
  *
  * Note: This struct is part of ABI v1 and is deprecated.
- * Use &struct gpio_v2_line_config instead.
+ * Use ABI v2 and &struct gpio_v2_line_config instead.
  */
 struct gpiohandle_config {
 	__u32 flags;
@@ -446,7 +446,7 @@ struct gpiohandle_config {
  * the desired target state
  *
  * Note: This struct is part of ABI v1 and is deprecated.
- * Use &struct gpio_v2_line_values instead.
+ * Use ABI v2 and &struct gpio_v2_line_values instead.
  */
 struct gpiohandle_data {
 	__u8 values[GPIOHANDLES_MAX];
@@ -471,7 +471,7 @@ struct gpiohandle_data {
  * valid anonymous file descriptor representing the request
  *
  * Note: This struct is part of ABI v1 and is deprecated.
- * Use &struct gpio_v2_line_request instead.
+ * Use ABI v2 and &struct gpio_v2_line_request instead.
  */
 struct gpioevent_request {
 	__u32 lineoffset;
@@ -494,7 +494,7 @@ struct gpioevent_request {
  *  %GPIOEVENT_EVENT_FALLING_EDGE
  *
  * Note: This struct is part of ABI v1 and is deprecated.
- * Use &struct gpio_v2_line_event instead.
+ * Use ABI v2 and &struct gpio_v2_line_event instead.
  */
 struct gpioevent_data {
 	__u64 timestamp;
-- 
2.39.2

Re: [PATCH 0/5] gpio: uapi: documentation improvements

[Andy Shevchenko]

On Tue, Jan 09, 2024 at 10:02:16PM +0800, Kent Gibson wrote:
> This is a series of minor clarifications and formatting tidy ups for
> the GPIO uAPI kernel doc.
> 
> The series is intended as a companion to my character device
> uAPI documentation series, but makes sense on its own too.
> 
> The patches are self contained and trivial so not much to add here.

Reviewed-by: Andy Shevchenko <andy@kernel.org>

for patches starting from the second one.

The first one I personally don't understand why, but I'm not a native speaker!
I believe, it's correct, although the original version seems okay to me.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/5] gpio: uapi: documentation improvements

[Kent Gibson]

On Sun, Jan 14, 2024 at 04:10:54PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 09, 2024 at 10:02:16PM +0800, Kent Gibson wrote:
> > This is a series of minor clarifications and formatting tidy ups for
> > the GPIO uAPI kernel doc.
> >
> > The series is intended as a companion to my character device
> > uAPI documentation series, but makes sense on its own too.
> >
> > The patches are self contained and trivial so not much to add here.
>
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
>
> for patches starting from the second one.
>
> The first one I personally don't understand why, but I'm not a native speaker!
> I believe, it's correct, although the original version seems okay to me.
>

The problem isn't the language, unless you mean I'm explaining poorly, it
is the logic.  The original says "zero or negative value means error", but
in case of an error the kernel does not actually set the fd.  So if the
user sends a request containing a positive fd they might incorrectly infer
that the positive fd being returned implies success.

The new wording is that the returned fd is only valid on success.

And thanks for the review.

Cheers,
Kent.

Re: [PATCH 0/5] gpio: uapi: documentation improvements

[Andy Shevchenko]

On Sun, Jan 14, 2024 at 4:20 PM Kent Gibson <warthog618@gmail.com> wrote:
> On Sun, Jan 14, 2024 at 04:10:54PM +0200, Andy Shevchenko wrote:
> > On Tue, Jan 09, 2024 at 10:02:16PM +0800, Kent Gibson wrote:
> > > This is a series of minor clarifications and formatting tidy ups for
> > > the GPIO uAPI kernel doc.
> > >
> > > The series is intended as a companion to my character device
> > > uAPI documentation series, but makes sense on its own too.
> > >
> > > The patches are self contained and trivial so not much to add here.
> >
> > Reviewed-by: Andy Shevchenko <andy@kernel.org>
> >
> > for patches starting from the second one.
> >
> > The first one I personally don't understand why, but I'm not a native speaker!
> > I believe, it's correct, although the original version seems okay to me.
>
> The problem isn't the language, unless you mean I'm explaining poorly, it
> is the logic.  The original says "zero or negative value means error", but
> in case of an error the kernel does not actually set the fd.  So if the
> user sends a request containing a positive fd they might incorrectly infer
> that the positive fd being returned implies success.
>
> The new wording is that the returned fd is only valid on success.

Ah, thanks for elaboration, now I understand the issue. Okay, feel
free to extend the Rb to the first patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/5] gpio: uapi: documentation improvements

[Bartosz Golaszewski]

On Tue, Jan 9, 2024 at 3:02 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> This is a series of minor clarifications and formatting tidy ups for
> the GPIO uAPI kernel doc.
>
> The series is intended as a companion to my character device
> uAPI documentation series, but makes sense on its own too.
>
> The patches are self contained and trivial so not much to add here.
>
> Cheers,
> Kent.
>
> Kent Gibson (5):
>   gpio: uapi: improve description of fd fields
>   gpio: uapi: clarify hte references
>   gpio: uapi: drop trailing period from one sentence descriptions
>   gpio: uapi: document possible values of gpioevent_data.id
>   gpio: uapi: clarify using v2 rather than v1
>
>  include/uapi/linux/gpio.h | 52 ++++++++++++++++++++-------------------
>  1 file changed, 27 insertions(+), 25 deletions(-)
>
> --
> 2.39.2
>

Series applied, thanks!

Bart