gpio: uapi: documentation improvements
Kent Gibson
reviewercover gpio: uapi: documentation improvements
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(-)
Signature · 1 line
2.39.2
Kent Gibson
reviewerPATCH 1/5 gpio: uapi: improve description of fd fields
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.
--- include/uapi/linux/gpio.h | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
include/uapi/linux/gpio.h +6 -9
@@ -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.
Signature · 1 line
2.39.2
Kent Gibson
reviewerPATCH 2/5 gpio: uapi: clarify hte references
In reply to Kent GibsonThe 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.
--- include/uapi/linux/gpio.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
include/uapi/linux/gpio.h +5 -1
@@ -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;
Signature · 1 line
2.39.2
Kent Gibson
reviewerPATCH 3/5 gpio: uapi: drop trailing period from one sentence descriptions
In reply to Kent GibsonMake documentation more consistent by using trailing periods only for multi-sentence field descriptions, not single sentence descriptions.
--- include/uapi/linux/gpio.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
include/uapi/linux/gpio.h +7 -7
@@ -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
Signature · 1 line
2.39.2
Kent Gibson
reviewerPATCH 4/5 gpio: uapi: document possible values of gpioevent_data.id
In reply to Kent GibsonClarify the possible values of event id, rather than requiring the reader to infer.
--- include/uapi/linux/gpio.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
include/uapi/linux/gpio.h +2 -1
@@ -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.
Signature · 1 line
2.39.2
Kent Gibson
reviewerPATCH 5/5 gpio: uapi: clarify using v2 rather than v1
In reply to Kent GibsonThe 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.
--- include/uapi/linux/gpio.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
include/uapi/linux/gpio.h +7 -7
@@ -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;
Signature · 1 line
2.39.2
Quoted 7 lines · 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.
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.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Kent Gibson
reviewerQuoted 7 lines · Andy Shevchenko
Quoted 6 lines · 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.
Quoted 9 lines
Quoted 1 line
The patches are self contained and trivial so not much to add here.
Reviewed-by Andy Shevchenko <[email protected]>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.
Quoted 17 lines · Kent Gibson
Quoted 7 lines · Andy Shevchenko
Quoted 6 lines · 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.
Quoted 8 lines
Quoted 1 line
The patches are self contained and trivial so not much to add here.
Reviewed-by Andy Shevchenko <[email protected]>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.
Quoted 7 lines
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.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Quoted 19 lines · 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
Quoted 6 lines
include/uapi/linux/gpio.h | 52 ++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 25 deletions(-)--
2.39.2
Series applied, thanks!
Bart