gpiolib: cdev: directionless line reconfiguration

PATCH Applied Kent Gibson 2 participants 12 messages Jun 26, 2024 – Jul 1, 2024

Subsystems (3)

Documentation GPIO GPIO uapi

lore

Share & tools

lore

Kent Gibson

author reviewer

2024-06-26 05:29 UTC #0 lore raw

cover gpiolib: cdev: directionless line reconfiguration

The behaviour of request reconfiguration without a direction flag set is ill-defined and badly behaved, for both uAPI v1 and v2. I'll will refer to such a configuration as 'directionless' here. That refers to the configuration requested, not the actual state of the line.

The configuration validation used during reconfiguration is borrowed from the line request operation, where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But that validation does not allow for the directionless case, making it possible to clear flags set previously without specifying the line direction.

Adding to the inconsistency, those changes are not immediately applied, but will take effect when the line value is next get or set.

For example, by requesting a reconfiguration with no flags set, an output line requested with active low and open drain flags set could have those flags cleared, inverting the sense of the line and changing the line drive to push-pull on the next line value set.

This series addresses directionless reconfiguration behaviour for both uAPI versions.

Patch 1 disallows reconfiguration without direction for uAPI v1.

Patch 2 ignores reconfiguration of a line without direction for uAPI v2. A different approach is used, compared to uAPI v1, as v2 allows for reconfiguration of multiple lines with different configurations. It is more useful to skip directionless lines rather than returning an error, as it allows for reconfiguration of a subset of requested lines.

Patches 3 and 4 update the documentation for uAPI v1 and v2, respectively, to describe the updated behaviour.

Cheers,
Kent.

Kent Gibson (4):
gpiolib: cdev: Disallow reconfiguration without direction (uAPI v1)
gpiolib: cdev: Ignore reconfiguration without direction
Documentation: gpio: Reconfiguration with unset direction (uAPI v1)
Documentation: gpio: Reconfiguration with unset direction (uAPI v2)

 .../gpio/gpio-handle-set-config-ioctl.rst     |  5 +++-
 .../gpio/gpio-v2-line-set-config-ioctl.rst    |  7 +++--
 drivers/gpio/gpiolib-cdev.c                   | 28 +++++++++++--------
 3 files changed, 26 insertions(+), 14 deletions(-)

Signature · 1 line

2.39.2

Kent Gibson

author reviewer

2024-06-26 05:29 UTC #1 lore raw

PATCH 1/4 gpiolib: cdev: Disallow reconfiguration without direction (uAPI v1)

linehandle_set_config() behaves badly when direction is not set. The configuration validation is borrowed from linehandle_create(), where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But, when applied to reconfiguration, that validation does not allow for the unset direction case, making it possible to clear flags set previously without specifying the line direction.

Adding to the inconsistency, those changes are not immediately applied by linehandle_set_config(), but will take effect when the line value is next get or set.

For example, by requesting a configuration with no flags set, an output line with GPIOHANDLE_REQUEST_ACTIVE_LOW and GPIOHANDLE_REQUEST_OPEN_DRAIN requested could have those flags cleared, inverting the sense of the line and changing the line drive to push-pull on the next line value set.

Ensure the intent of the user by disallowing configurations which do not have direction set, returning an error to userspace to indicate that the configuration is invalid.

And, for clarity, use lflags, a local copy of gcnf.flags, throughout when dealing with the requested flags, rather than a mixture of both.

Fixes e588bb1eae31 ("gpio: add new SET_CONFIG ioctl() to gpio chardev")

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpiolib-cdev.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

drivers/gpio/gpiolib-cdev.c +10 -6

index 1cb952daacfb..f7a129d67b7d 100644
@@ -89,6 +89,10 @@ struct linehandle_state {
 	GPIOHANDLE_REQUEST_OPEN_DRAIN | \
 	GPIOHANDLE_REQUEST_OPEN_SOURCE)
 
+#define GPIOHANDLE_REQUEST_DIRECTION_FLAGS \
+	(GPIOHANDLE_REQUEST_INPUT | \
+	 GPIOHANDLE_REQUEST_OUTPUT)
+
 static int linehandle_validate_flags(u32 flags)
 {
 	/* Return an error if an unknown flag is set */

@@ -169,21 +173,21 @@ static long linehandle_set_config(struct linehandle_state *lh,
 	if (ret)
 		return ret;
 
+	/* Lines must be reconfigured explicitly as input or output. */
+	if (!(lflags & GPIOHANDLE_REQUEST_DIRECTION_FLAGS))
+		return -EINVAL;
+
 	for (i = 0; i < lh->num_descs; i++) {
 		desc = lh->descs[i];
-		linehandle_flags_to_desc_flags(gcnf.flags, &desc->flags);
+		linehandle_flags_to_desc_flags(lflags, &desc->flags);
 
-		/*
-		 * Lines have to be requested explicitly for input
-		 * or output, else the line will be treated "as is".
-		 */
 		if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
 			int val = !!gcnf.default_values[i];
 
 			ret = gpiod_direction_output(desc, val);
 			if (ret)
 				return ret;
-		} else if (lflags & GPIOHANDLE_REQUEST_INPUT) {
+		} else {
 			ret = gpiod_direction_input(desc);
 			if (ret)
 				return ret;

Signature · 1 line

2.39.2

Kent Gibson

author reviewer

2024-06-26 05:29 UTC #2 lore raw

PATCH 2/4 gpiolib: cdev: Ignore reconfiguration without direction

In reply to Kent Gibson

linereq_set_config() behaves badly when direction is not set. The configuration validation is borrowed from linereq_create(), where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But, when applied to reconfiguration, that validation does not allow for the unset direction case, making it possible to clear flags set previously without specifying the line direction.

Adding to the inconsistency, those changes are not immediately applied by linereq_set_config(), but will take effect when the line value is next get or set.

For example, by requesting a configuration with no flags set, an output line with GPIO_V2_LINE_FLAG_ACTIVE_LOW and GPIO_V2_LINE_FLAG_OPEN_DRAIN set could have those flags cleared, inverting the sense of the line and changing the line drive to push-pull on the next line value set.

Skip the reconfiguration of lines for which the direction is not set, and only reconfigure the lines for which direction is set.

Fixes a54756cb24ea ("gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL")

Signed-off-by Kent Gibson <[email protected]>

---
 drivers/gpio/gpiolib-cdev.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

drivers/gpio/gpiolib-cdev.c +7 -5

index f7a129d67b7d..ef08b23a56e2 100644
@@ -1534,12 +1534,14 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
 		line = &lr->lines[i];
 		desc = lr->lines[i].desc;
 		flags = gpio_v2_line_config_flags(&lc, i);
+		/*
+		 * Lines not explicitly reconfigured as input or output
+		 * are left unchanged.
+		 */
+		if (!(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
+			continue;
 		gpio_v2_line_config_flags_to_desc_flags(flags, &desc->flags);
 		edflags = flags & GPIO_V2_LINE_EDGE_DETECTOR_FLAGS;
-		/*
-		 * Lines have to be requested explicitly for input
-		 * or output, else the line will be treated "as is".
-		 */
 		if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
 			int val = gpio_v2_line_config_output_value(&lc, i);

@@ -1547,7 +1549,7 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
 			ret = gpiod_direction_output(desc, val);
 			if (ret)
 				return ret;
-		} else if (flags & GPIO_V2_LINE_FLAG_INPUT) {
+		} else {
 			ret = gpiod_direction_input(desc);
 			if (ret)
 				return ret;

Signature · 1 line

2.39.2

Bartosz Golaszewski

2024-06-27 14:06 UTC #3 lore raw

Re: [PATCH 2/4] gpiolib: cdev: Ignore reconfiguration without direction

Quoted 32 lines · Kent Gibson

linereq_set_config() behaves badly when direction is not set. The configuration validation is borrowed from linereq_create(), where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But, when applied to reconfiguration, that validation does not allow for the unset direction case, making it possible to clear flags set previously without specifying the line direction.
Adding to the inconsistency, those changes are not immediately applied by linereq_set_config(), but will take effect when the line value is next get or set.
For example, by requesting a configuration with no flags set, an output line with GPIO_V2_LINE_FLAG_ACTIVE_LOW and GPIO_V2_LINE_FLAG_OPEN_DRAIN set could have those flags cleared, inverting the sense of the line and changing the line drive to push-pull on the next line value set.
Skip the reconfiguration of lines for which the direction is not set, and only reconfigure the lines for which direction is set.
Fixes a54756cb24ea ("gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL")
Signed-off-by Kent Gibson <[email protected]>
---
 drivers/gpio/gpiolib-cdev.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/gpio/gpiolib-cdev.c +0 -0
index f7a129d67b7d..ef08b23a56e2 100644

drivers/gpio/gpiolib-cdev.c static long linereq_set_config(struct linereq *lr, void __user *ip) · line 1542

Quoted 9 lines

drivers/gpio/gpiolib-cdev.c +6 -0

@@ -1534,12 +1534,14 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
                line = &lr->lines[i];
                desc = lr->lines[i].desc;
                flags = gpio_v2_line_config_flags(&lc, i);
+               /*
+                * Lines not explicitly reconfigured as input or output
+                * are left unchanged.
+                */
+               if (!(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
+                       continue;

Series looks good, thanks. I'd say that this bit here calls for at least a debug-level message since we don't return an error unlike v1. What do you think?

Bart

Quoted 21 lines

drivers/gpio/gpiolib-cdev.c +1 -6

                gpio_v2_line_config_flags_to_desc_flags(flags, &desc->flags);
                edflags = flags & GPIO_V2_LINE_EDGE_DETECTOR_FLAGS;
-               /*
-                * Lines have to be requested explicitly for input
-                * or output, else the line will be treated "as is".
-                */
                if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
                        int val = gpio_v2_line_config_output_value(&lc, i);

@@ -1547,7 +1549,7 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
                        ret = gpiod_direction_output(desc, val);
                        if (ret)
                                return ret;
-               } else if (flags & GPIO_V2_LINE_FLAG_INPUT) {
+               } else {
                        ret = gpiod_direction_input(desc);
                        if (ret)
                                return ret;
--

2.39.2

Kent Gibson

author reviewer

2024-06-27 14:22 UTC #4 lore raw

Re: [PATCH 2/4] gpiolib: cdev: Ignore reconfiguration without direction

Quoted 38 lines · Bartosz Golaszewski

Quoted 37 lines · Kent Gibson
linereq_set_config() behaves badly when direction is not set. The configuration validation is borrowed from linereq_create(), where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But, when applied to reconfiguration, that validation does not allow for the unset direction case, making it possible to clear flags set previously without specifying the line direction.
Adding to the inconsistency, those changes are not immediately applied by linereq_set_config(), but will take effect when the line value is next get or set.
For example, by requesting a configuration with no flags set, an output line with GPIO_V2_LINE_FLAG_ACTIVE_LOW and GPIO_V2_LINE_FLAG_OPEN_DRAIN set could have those flags cleared, inverting the sense of the line and changing the line drive to push-pull on the next line value set.
Skip the reconfiguration of lines for which the direction is not set, and only reconfigure the lines for which direction is set.
Fixes a54756cb24ea ("gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL")
Signed-off-by Kent Gibson <[email protected]>
---
 drivers/gpio/gpiolib-cdev.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/gpio/gpiolib-cdev.c +2 -0
index f7a129d67b7d..ef08b23a56e2 100644
@@ -1534,12 +1534,14 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
                line = &lr->lines[i];
                desc = lr->lines[i].desc;
                flags = gpio_v2_line_config_flags(&lc, i);
+               /*
+                * Lines not explicitly reconfigured as input or output

Quoted 9 lines

drivers/gpio/gpiolib-cdev.c static long linereq_set_config(struct linereq *lr, void __user *ip) · line 1542
Quoted 4 lines
drivers/gpio/gpiolib-cdev.c +4 -0
+                * are left unchanged.
+                */
+               if (!(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
+                       continue;
Series looks good, thanks. I'd say that this bit here calls for at least a debug-level message since we don't return an error unlike v1. What do you think?

The change to the libgpiod Python bindings makes use of this to support reconfiguration of subsets, so on its own it isn't an abnormal path and I'm not sure it warrants even a debug.

OTOH, I did consider if there should be a check that at least one line in the reconfig has a direction, returning an error if there are none, but was on the fence about it and left it out as it added complexity.

Would that make more sense?
Or do you have a problem with reconfiguring subsets?

Cheers,
Kent.

Bartosz Golaszewski

2024-06-27 14:44 UTC #5 lore raw

Re: [PATCH 2/4] gpiolib: cdev: Ignore reconfiguration without direction

Quoted 58 lines · Kent Gibson

Quoted 38 lines · Bartosz Golaszewski
Quoted 37 lines · Kent Gibson
linereq_set_config() behaves badly when direction is not set. The configuration validation is borrowed from linereq_create(), where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But, when applied to reconfiguration, that validation does not allow for the unset direction case, making it possible to clear flags set previously without specifying the line direction.
Adding to the inconsistency, those changes are not immediately applied by linereq_set_config(), but will take effect when the line value is next get or set.
For example, by requesting a configuration with no flags set, an output line with GPIO_V2_LINE_FLAG_ACTIVE_LOW and GPIO_V2_LINE_FLAG_OPEN_DRAIN set could have those flags cleared, inverting the sense of the line and changing the line drive to push-pull on the next line value set.
Skip the reconfiguration of lines for which the direction is not set, and only reconfigure the lines for which direction is set.
Fixes a54756cb24ea ("gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL")
Signed-off-by Kent Gibson <[email protected]>
---
 drivers/gpio/gpiolib-cdev.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/gpio/gpiolib-cdev.c +2 -0
index f7a129d67b7d..ef08b23a56e2 100644
@@ -1534,12 +1534,14 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
                line = &lr->lines[i];
                desc = lr->lines[i].desc;
                flags = gpio_v2_line_config_flags(&lc, i);
+               /*
+                * Lines not explicitly reconfigured as input or output
Quoted 9 lines
drivers/gpio/gpiolib-cdev.c static long linereq_set_config(struct linereq *lr, void __user *ip) · line 1542
Quoted 4 lines
drivers/gpio/gpiolib-cdev.c +4 -0
+                * are left unchanged.
+                */
+               if (!(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
+                       continue;
Series looks good, thanks. I'd say that this bit here calls for at least a debug-level message since we don't return an error unlike v1. What do you think?
The change to the libgpiod Python bindings makes use of this to support reconfiguration of subsets, so on its own it isn't an abnormal path and I'm not sure it warrants even a debug.
OTOH, I did consider if there should be a check that at least one line in the reconfig has a direction, returning an error if there are none, but was on the fence about it and left it out as it added complexity.

Quoted 5 lines

Would that make more sense?
Or do you have a problem with reconfiguring subsets?
Cheers,
Kent.

I see. Ok, I'll take it as is interpreting it as a feature.

Bart

Kent Gibson

author reviewer

2024-06-27 14:49 UTC #6 lore raw

Re: [PATCH 2/4] gpiolib: cdev: Ignore reconfiguration without direction

Quoted 59 lines · Bartosz Golaszewski

Quoted 58 lines · Kent Gibson
Quoted 38 lines · Bartosz Golaszewski
Quoted 37 lines · Kent Gibson
linereq_set_config() behaves badly when direction is not set. The configuration validation is borrowed from linereq_create(), where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But, when applied to reconfiguration, that validation does not allow for the unset direction case, making it possible to clear flags set previously without specifying the line direction.
Adding to the inconsistency, those changes are not immediately applied by linereq_set_config(), but will take effect when the line value is next get or set.
For example, by requesting a configuration with no flags set, an output line with GPIO_V2_LINE_FLAG_ACTIVE_LOW and GPIO_V2_LINE_FLAG_OPEN_DRAIN set could have those flags cleared, inverting the sense of the line and changing the line drive to push-pull on the next line value set.
Skip the reconfiguration of lines for which the direction is not set, and only reconfigure the lines for which direction is set.
Fixes a54756cb24ea ("gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL")
Signed-off-by Kent Gibson <[email protected]>
---
 drivers/gpio/gpiolib-cdev.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/gpio/gpiolib-cdev.c +2 -0
index f7a129d67b7d..ef08b23a56e2 100644
@@ -1534,12 +1534,14 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
                line = &lr->lines[i];
                desc = lr->lines[i].desc;
                flags = gpio_v2_line_config_flags(&lc, i);
+               /*
+                * Lines not explicitly reconfigured as input or output
Quoted 9 lines
drivers/gpio/gpiolib-cdev.c static long linereq_set_config(struct linereq *lr, void __user *ip) · line 1542
Quoted 4 lines
drivers/gpio/gpiolib-cdev.c +4 -0
+                * are left unchanged.
+                */
+               if (!(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
+                       continue;
Series looks good, thanks. I'd say that this bit here calls for at least a debug-level message since we don't return an error unlike v1. What do you think?
The change to the libgpiod Python bindings makes use of this to support reconfiguration of subsets, so on its own it isn't an abnormal path and I'm not sure it warrants even a debug.
OTOH, I did consider if there should be a check that at least one line in the reconfig has a direction, returning an error if there are none, but was on the fence about it and left it out as it added complexity.

Quoted 8 lines

Quoted 5 lines
Would that make more sense?
Or do you have a problem with reconfiguring subsets?
Cheers,
Kent.
I see. Ok, I'll take it as is interpreting it as a feature.

I'm totally ok with adding a check that direction is set at least once, if you would like that. Can be done with a reasonably minor change to gpio_v2_line_config_validate(). Though that would probably still double the size of this patch.

Cheers,
Kent.

Bartosz Golaszewski

2024-06-27 14:51 UTC #7 lore raw

Re: [PATCH 2/4] gpiolib: cdev: Ignore reconfiguration without direction

Quoted 70 lines · Kent Gibson

Quoted 59 lines · Bartosz Golaszewski
Quoted 58 lines · Kent Gibson
Quoted 38 lines · Bartosz Golaszewski
Quoted 37 lines · Kent Gibson
linereq_set_config() behaves badly when direction is not set. The configuration validation is borrowed from linereq_create(), where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But, when applied to reconfiguration, that validation does not allow for the unset direction case, making it possible to clear flags set previously without specifying the line direction.
Adding to the inconsistency, those changes are not immediately applied by linereq_set_config(), but will take effect when the line value is next get or set.
For example, by requesting a configuration with no flags set, an output line with GPIO_V2_LINE_FLAG_ACTIVE_LOW and GPIO_V2_LINE_FLAG_OPEN_DRAIN set could have those flags cleared, inverting the sense of the line and changing the line drive to push-pull on the next line value set.
Skip the reconfiguration of lines for which the direction is not set, and only reconfigure the lines for which direction is set.
Fixes a54756cb24ea ("gpiolib: cdev: support GPIO_V2_LINE_SET_CONFIG_IOCTL")
Signed-off-by Kent Gibson <[email protected]>
---
 drivers/gpio/gpiolib-cdev.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/gpio/gpiolib-cdev.c +2 -0
index f7a129d67b7d..ef08b23a56e2 100644
@@ -1534,12 +1534,14 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
                line = &lr->lines[i];
                desc = lr->lines[i].desc;
                flags = gpio_v2_line_config_flags(&lc, i);
+               /*
+                * Lines not explicitly reconfigured as input or output
Quoted 9 lines
drivers/gpio/gpiolib-cdev.c static long linereq_set_config(struct linereq *lr, void __user *ip) · line 1542
Quoted 4 lines
drivers/gpio/gpiolib-cdev.c +4 -0
+                * are left unchanged.
+                */
+               if (!(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
+                       continue;
Series looks good, thanks. I'd say that this bit here calls for at least a debug-level message since we don't return an error unlike v1. What do you think?
The change to the libgpiod Python bindings makes use of this to support reconfiguration of subsets, so on its own it isn't an abnormal path and I'm not sure it warrants even a debug.
OTOH, I did consider if there should be a check that at least one line in the reconfig has a direction, returning an error if there are none, but was on the fence about it and left it out as it added complexity.
Quoted 8 lines
Quoted 5 lines
Would that make more sense?
Or do you have a problem with reconfiguring subsets?
Cheers,
Kent.
I see. Ok, I'll take it as is interpreting it as a feature.

Quoted 8 lines

I'm totally ok with adding a check that direction is set at least once, if you would like that. Can be done with a reasonably minor change to gpio_v2_line_config_validate(). Though that would probably still double the size of this patch.
Cheers,
Kent.

No, it's fine. Docs are quite explicit about the behavior and there's a comment in place.

Bart

Kent Gibson

author reviewer

2024-06-26 05:30 UTC #8 lore raw

PATCH 3/4 Documentation: gpio: Reconfiguration with unset direction (uAPI v1)

In reply to Kent Gibson

Update description of reconfiguration rules, adding requirement that a direction flag be set or the configuration is considered invalid.

Signed-off-by Kent Gibson <[email protected]>

---
 .../userspace-api/gpio/gpio-handle-set-config-ioctl.rst      | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Documentation/userspace-api/gpio/gpio-handle-set-config-ioctl.rst +4 -1

index d002a84681ac..a03f30db63ab 100644
@@ -43,7 +43,10 @@ The configuration applies to all requested lines.
 
 The same :ref:`gpio-get-linehandle-config-rules` and
 :ref:`gpio-get-linehandle-config-support` that apply when requesting the
-lines also apply when updating the line configuration.
+lines also apply when updating the line configuration, with the additional
+restriction that a direction flag must be set. Requesting an invalid
+configuration, including without a direction flag set, is an error
+(**EINVAL**).
 
 The motivating use case for this command is changing direction of
 bi-directional lines between input and output, but it may be used more

Signature · 1 line

2.39.2

Kent Gibson

author reviewer

2024-06-26 05:30 UTC #9 lore raw

PATCH 4/4 Documentation: gpio: Reconfiguration with unset direction (uAPI v2)

In reply to Kent Gibson

Update description of reconfiguration rules, adding requirement that a direction flag be set to enable changing configuration for a line.

Signed-off-by Kent Gibson <[email protected]>

---
 .../userspace-api/gpio/gpio-v2-line-set-config-ioctl.rst   | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Documentation/userspace-api/gpio/gpio-v2-line-set-config-ioctl.rst +5 -2

index 9b942a8a53ca..cfaab801556c 100644
@@ -35,11 +35,14 @@ Description
 Update the configuration of previously requested lines, without releasing the
 line or introducing potential glitches.
 
-The new configuration must specify the configuration of all requested lines.
+The new configuration must specify a configuration for all requested lines.
 
 The same :ref:`gpio-v2-get-line-config-rules` and
 :ref:`gpio-v2-get-line-config-support` that apply when requesting the lines
-also apply when updating the line configuration.
+also apply when updating the line configuration, with the additional
+restriction that a direction flag must be set to enable reconfiguration.
+If no direction flag is set in the configuration for a given line then the
+configuration for that line is left unchanged.
 
 The motivating use case for this command is changing direction of
 bi-directional lines between input and output, but it may also be used to

Signature · 1 line

2.39.2

Bartosz Golaszewski

2024-06-27 15:23 UTC #10 lore raw

In reply to Kent Gibson

From: Bartosz Golaszewski <[email protected]>

Quoted 5 lines · Kent Gibson

The behaviour of request reconfiguration without a direction flag set is ill-defined and badly behaved, for both uAPI v1 and v2. I'll will refer to such a configuration as 'directionless' here. That refers to the configuration requested, not the actual state of the line.

Quoted 8 lines

The configuration validation used during reconfiguration is borrowed from the line request operation, where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But that validation does not allow for the directionless case, making it possible to clear flags set previously without specifying the line direction.
[...]

I applied the first two for fixes and will apply the remaining two for the upcoming merge window once this gets upstream.

[1/4] gpiolib: cdev: Disallow reconfiguration without direction (uAPI v1)

      commit: 9919cce62f68e6ab68dc2a975b5dc670f8ca7d40
[2/4] gpiolib: cdev: Ignore reconfiguration without direction
      commit: b440396387418fe2feaacd41ca16080e7a8bc9ad

Best regards,

Signature · 1 line

Bartosz Golaszewski <[email protected]>

Bartosz Golaszewski

Applied

2024-07-01 07:39 UTC #11 lore raw

In reply to Kent Gibson

From: Bartosz Golaszewski <[email protected]>

Quoted 5 lines · Kent Gibson

The behaviour of request reconfiguration without a direction flag set is ill-defined and badly behaved, for both uAPI v1 and v2. I'll will refer to such a configuration as 'directionless' here. That refers to the configuration requested, not the actual state of the line.

Quoted 8 lines

The configuration validation used during reconfiguration is borrowed from the line request operation, where, to verify the intent of the user, the direction must be set to in order to effect a change to the electrical configuration of a line. But that validation does not allow for the directionless case, making it possible to clear flags set previously without specifying the line direction.
[...]

Applied, thanks!

[3/4] Documentation: gpio: Reconfiguration with unset direction (uAPI v1)

      commit: e48fe75afa539d110753f7420aa398ef89f8e383
[4/4] Documentation: gpio: Reconfiguration with unset direction (uAPI v2)
      commit: 6a9c15083b1662a4b7b36e787272deb696d72c24

Best regards,

Signature · 1 line

Bartosz Golaszewski <[email protected]>