From: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
To: Linus Walleij <linusw@kernel.org>,
Bartosz Golaszewski <brgl@kernel.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Kent Gibson <warthog618@gmail.com>,
Adrian Ambrozewicz <aambrozewicz@nvidia.com>
Cc: linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>,
stable@vger.kernel.org
Subject: [PATCH v2 5/5] gpio: cdev: don't overwrite the whole flags field when configuring lines
Date: Mon, 09 Nov 2026 11:27:09 +0100 [thread overview]
Message-ID: <20261109-gpio-desc-flags-atomic-v2-5-c4c54b476628@oss.qualcomm.com> (raw)
In-Reply-To: <20261109-gpio-desc-flags-atomic-v2-0-c4c54b476628@oss.qualcomm.com>
linehandle_flags_to_desc_flags() and
gpio_v2_line_config_flags_to_desc_flags() read the flags word with
READ_ONCE(), modify the local copy and write it back with WRITE_ONCE().
The per-request config_mutex does not exclude sysfs, the irq core or
drivers touching the same descriptor, so bits they set or clear
concurrently can be lost.
Build the mask of bits owned by the line configuration and their new
values locally and apply them with gpiod_update_flags(). The whole
group changes in a single atomic step.
While at it: rename the functions whose semantics changed from
"conversion" to "flags update" accordingly.
Cc: stable@vger.kernel.org
Fixes: b7adfb6076ff ("gpio: cdev: update flags at once when reconfiguring from user-space")
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
drivers/gpio/gpiolib-cdev.c | 111 +++++++++++++++++++++++++-------------------
1 file changed, 62 insertions(+), 49 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index f68e33cbd7d4cd5153e82ad77768cf52a90601d6..50baee455283c3aeb25ed92f3936c5f6a5b215bf 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -3,6 +3,7 @@
#include <linux/anon_inodes.h>
#include <linux/atomic.h>
#include <linux/bitmap.h>
+#include <linux/bitops.h>
#include <linux/build_bug.h>
#include <linux/cdev.h>
#include <linux/cleanup.h>
@@ -140,24 +141,30 @@ static int linehandle_validate_flags(u32 flags)
return 0;
}
-static void linehandle_flags_to_desc_flags(u32 lflags, unsigned long *flagsp)
+static void linehandle_update_desc_flags(u32 lflags, struct gpio_desc *desc)
{
- unsigned long flags = READ_ONCE(*flagsp);
+ const unsigned long mask = BIT(GPIOD_FLAG_ACTIVE_LOW) |
+ BIT(GPIOD_FLAG_OPEN_DRAIN) |
+ BIT(GPIOD_FLAG_OPEN_SOURCE) |
+ BIT(GPIOD_FLAG_PULL_UP) |
+ BIT(GPIOD_FLAG_PULL_DOWN) |
+ BIT(GPIOD_FLAG_BIAS_DISABLE);
+ unsigned long flags = 0;
- assign_bit(GPIOD_FLAG_ACTIVE_LOW, &flags,
- lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW);
- assign_bit(GPIOD_FLAG_OPEN_DRAIN, &flags,
- lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN);
- assign_bit(GPIOD_FLAG_OPEN_SOURCE, &flags,
- lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE);
- assign_bit(GPIOD_FLAG_PULL_UP, &flags,
- lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP);
- assign_bit(GPIOD_FLAG_PULL_DOWN, &flags,
- lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN);
- assign_bit(GPIOD_FLAG_BIAS_DISABLE, &flags,
- lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE);
+ __assign_bit(GPIOD_FLAG_ACTIVE_LOW, &flags,
+ lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW);
+ __assign_bit(GPIOD_FLAG_OPEN_DRAIN, &flags,
+ lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN);
+ __assign_bit(GPIOD_FLAG_OPEN_SOURCE, &flags,
+ lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE);
+ __assign_bit(GPIOD_FLAG_PULL_UP, &flags,
+ lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP);
+ __assign_bit(GPIOD_FLAG_PULL_DOWN, &flags,
+ lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN);
+ __assign_bit(GPIOD_FLAG_BIAS_DISABLE, &flags,
+ lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE);
- WRITE_ONCE(*flagsp, flags);
+ set_mask_bits(&desc->flags, mask, flags);
}
static long linehandle_set_config(struct linehandle_state *lh,
@@ -182,7 +189,7 @@ static long linehandle_set_config(struct linehandle_state *lh,
for (i = 0; i < lh->num_descs; i++) {
desc = lh->descs[i];
- linehandle_flags_to_desc_flags(lflags, &desc->flags);
+ linehandle_update_desc_flags(lflags, desc);
if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
int val = !!gcnf.default_values[i];
@@ -339,7 +346,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
if (ret)
return ret;
lh->descs[i] = desc;
- linehandle_flags_to_desc_flags(handlereq.flags, &desc->flags);
+ linehandle_update_desc_flags(handlereq.flags, desc);
ret = gpiod_set_transitory(desc, false);
if (ret < 0)
@@ -1216,37 +1223,43 @@ static int gpio_v2_line_config_validate(struct gpio_v2_line_config *lc,
return 0;
}
-static void gpio_v2_line_config_flags_to_desc_flags(u64 lflags,
- unsigned long *flagsp)
+static void linereq_update_desc_flags(u64 lflags,
+ struct gpio_desc *desc)
{
- unsigned long flags = READ_ONCE(*flagsp);
-
- assign_bit(GPIOD_FLAG_ACTIVE_LOW, &flags,
- lflags & GPIO_V2_LINE_FLAG_ACTIVE_LOW);
-
- assign_bit(GPIOD_FLAG_EDGE_RISING, &flags,
- lflags & GPIO_V2_LINE_FLAG_EDGE_RISING);
- assign_bit(GPIOD_FLAG_EDGE_FALLING, &flags,
- lflags & GPIO_V2_LINE_FLAG_EDGE_FALLING);
-
- assign_bit(GPIOD_FLAG_OPEN_DRAIN, &flags,
- lflags & GPIO_V2_LINE_FLAG_OPEN_DRAIN);
- assign_bit(GPIOD_FLAG_OPEN_SOURCE, &flags,
- lflags & GPIO_V2_LINE_FLAG_OPEN_SOURCE);
-
- assign_bit(GPIOD_FLAG_PULL_UP, &flags,
- lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP);
- assign_bit(GPIOD_FLAG_PULL_DOWN, &flags,
- lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN);
- assign_bit(GPIOD_FLAG_BIAS_DISABLE, &flags,
- lflags & GPIO_V2_LINE_FLAG_BIAS_DISABLED);
-
- assign_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &flags,
- lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME);
- assign_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &flags,
- lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE);
+ const unsigned long mask = BIT(GPIOD_FLAG_ACTIVE_LOW) |
+ BIT(GPIOD_FLAG_EDGE_RISING) |
+ BIT(GPIOD_FLAG_EDGE_FALLING) |
+ BIT(GPIOD_FLAG_OPEN_DRAIN) |
+ BIT(GPIOD_FLAG_OPEN_SOURCE) |
+ BIT(GPIOD_FLAG_PULL_UP) |
+ BIT(GPIOD_FLAG_PULL_DOWN) |
+ BIT(GPIOD_FLAG_BIAS_DISABLE) |
+ BIT(GPIOD_FLAG_EVENT_CLOCK_REALTIME) |
+ BIT(GPIOD_FLAG_EVENT_CLOCK_HTE);
+ unsigned long flags = 0;
- WRITE_ONCE(*flagsp, flags);
+ __assign_bit(GPIOD_FLAG_ACTIVE_LOW, &flags,
+ lflags & GPIO_V2_LINE_FLAG_ACTIVE_LOW);
+ __assign_bit(GPIOD_FLAG_EDGE_RISING, &flags,
+ lflags & GPIO_V2_LINE_FLAG_EDGE_RISING);
+ __assign_bit(GPIOD_FLAG_EDGE_FALLING, &flags,
+ lflags & GPIO_V2_LINE_FLAG_EDGE_FALLING);
+ __assign_bit(GPIOD_FLAG_OPEN_DRAIN, &flags,
+ lflags & GPIO_V2_LINE_FLAG_OPEN_DRAIN);
+ __assign_bit(GPIOD_FLAG_OPEN_SOURCE, &flags,
+ lflags & GPIO_V2_LINE_FLAG_OPEN_SOURCE);
+ __assign_bit(GPIOD_FLAG_PULL_UP, &flags,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP);
+ __assign_bit(GPIOD_FLAG_PULL_DOWN, &flags,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN);
+ __assign_bit(GPIOD_FLAG_BIAS_DISABLE, &flags,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_DISABLED);
+ __assign_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &flags,
+ lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME);
+ __assign_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &flags,
+ lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE);
+
+ set_mask_bits(&desc->flags, mask, flags);
}
static long linereq_get_values(struct linereq *lr, void __user *ip)
@@ -1404,7 +1417,7 @@ static long linereq_set_config(struct linereq *lr, void __user *ip)
*/
if (!(flags & GPIO_V2_LINE_DIRECTION_FLAGS))
continue;
- gpio_v2_line_config_flags_to_desc_flags(flags, &desc->flags);
+ linereq_update_desc_flags(flags, desc);
edflags = flags & GPIO_V2_LINE_EDGE_DETECTOR_FLAGS;
if (flags & GPIO_V2_LINE_FLAG_OUTPUT) {
int val = gpio_v2_line_config_output_value(&lc, i);
@@ -1660,7 +1673,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip)
lr->lines[i].desc = desc;
flags = gpio_v2_line_config_flags(lc, i);
- gpio_v2_line_config_flags_to_desc_flags(flags, &desc->flags);
+ linereq_update_desc_flags(flags, desc);
ret = gpiod_set_transitory(desc, false);
if (ret < 0)
@@ -2055,7 +2068,7 @@ static int lineevent_create(struct gpio_device *gdev, void __user *ip)
le->desc = desc;
le->eflags = eflags;
- linehandle_flags_to_desc_flags(lflags, &desc->flags);
+ linehandle_update_desc_flags(lflags, desc);
ret = gpiod_direction_input(desc);
if (ret)
--
2.47.3
next prev parent reply other threads:[~2026-11-09 10:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-09 10:27 [PATCH v2 0/5] gpiolib: improve concurrent access to the flags field of struct gpio_desc Bartosz Golaszewski
2026-11-09 10:27 ` [PATCH v2 1/5] gpiolib: don't overwrite the entire flags field in gpiod_get_direction() Bartosz Golaszewski
2026-11-09 10:27 ` [PATCH v2 2/5] gpio: shared: use atomic set_bit() for descriptor flags Bartosz Golaszewski
2026-11-09 10:27 ` [PATCH v2 3/5] gpiolib: clear debounce period before the "requested" flag Bartosz Golaszewski
2026-11-09 10:27 ` [PATCH v2 4/5] gpiolib: don't overwrite the entire flags field in gpiod_free_commit() Bartosz Golaszewski
2026-11-09 21:53 ` Andy Shevchenko
2026-11-09 10:27 ` Bartosz Golaszewski [this message]
2026-11-09 21:58 ` [PATCH v2 5/5] gpio: cdev: don't overwrite the whole flags field when configuring lines Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261109-gpio-desc-flags-atomic-v2-5-c4c54b476628@oss.qualcomm.com \
--to=bartosz.golaszewski@oss.qualcomm.com \
--cc=aambrozewicz@nvidia.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=brgl@kernel.org \
--cc=linusw@kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=warthog618@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®