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 5/5] gpio: cdev: don't overwrite the whole flags field when configuring lines
Date: Wed, 04 Nov 2026 16:37:00 +0100 [thread overview]
Message-ID: <20261104-gpio-desc-flags-atomic-v1-5-e23cedf619ba@oss.qualcomm.com> (raw)
In-Reply-To: <20261104-gpio-desc-flags-atomic-v1-0-e23cedf619ba@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.
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 | 118 ++++++++++++++++++++++++--------------------
1 file changed, 65 insertions(+), 53 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 3f1f8c9acfc3c10d5b3e86b70009f1226793cbc7..b7f0aec12c8551e39d042a1999fdfb5683ae97d7 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -140,24 +140,30 @@ static int linehandle_validate_flags(u32 flags)
return 0;
}
-static void linehandle_flags_to_desc_flags(u32 lflags, unsigned long *flagsp)
-{
- unsigned long flags = READ_ONCE(*flagsp);
-
- 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);
+static void linehandle_flags_to_desc_flags(u32 lflags, struct gpio_desc *desc)
+{
+ 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 val = 0;
+
+ __assign_bit(GPIOD_FLAG_ACTIVE_LOW, &val,
+ lflags & GPIOHANDLE_REQUEST_ACTIVE_LOW);
+ __assign_bit(GPIOD_FLAG_OPEN_DRAIN, &val,
+ lflags & GPIOHANDLE_REQUEST_OPEN_DRAIN);
+ __assign_bit(GPIOD_FLAG_OPEN_SOURCE, &val,
+ lflags & GPIOHANDLE_REQUEST_OPEN_SOURCE);
+ __assign_bit(GPIOD_FLAG_PULL_UP, &val,
+ lflags & GPIOHANDLE_REQUEST_BIAS_PULL_UP);
+ __assign_bit(GPIOD_FLAG_PULL_DOWN, &val,
+ lflags & GPIOHANDLE_REQUEST_BIAS_PULL_DOWN);
+ __assign_bit(GPIOD_FLAG_BIAS_DISABLE, &val,
+ lflags & GPIOHANDLE_REQUEST_BIAS_DISABLE);
+
+ gpiod_update_flags(desc, mask, val);
}
static long linehandle_set_config(struct linehandle_state *lh,
@@ -182,7 +188,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_flags_to_desc_flags(lflags, desc);
if (lflags & GPIOHANDLE_REQUEST_OUTPUT) {
int val = !!gcnf.default_values[i];
@@ -346,7 +352,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_flags_to_desc_flags(handlereq.flags, desc);
ret = gpiod_set_transitory(desc, false);
if (ret < 0)
@@ -1224,36 +1230,42 @@ static int gpio_v2_line_config_validate(struct gpio_v2_line_config *lc,
}
static void gpio_v2_line_config_flags_to_desc_flags(u64 lflags,
- unsigned long *flagsp)
-{
- 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);
-
- WRITE_ONCE(*flagsp, flags);
+ struct gpio_desc *desc)
+{
+ 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 val = 0;
+
+ __assign_bit(GPIOD_FLAG_ACTIVE_LOW, &val,
+ lflags & GPIO_V2_LINE_FLAG_ACTIVE_LOW);
+ __assign_bit(GPIOD_FLAG_EDGE_RISING, &val,
+ lflags & GPIO_V2_LINE_FLAG_EDGE_RISING);
+ __assign_bit(GPIOD_FLAG_EDGE_FALLING, &val,
+ lflags & GPIO_V2_LINE_FLAG_EDGE_FALLING);
+ __assign_bit(GPIOD_FLAG_OPEN_DRAIN, &val,
+ lflags & GPIO_V2_LINE_FLAG_OPEN_DRAIN);
+ __assign_bit(GPIOD_FLAG_OPEN_SOURCE, &val,
+ lflags & GPIO_V2_LINE_FLAG_OPEN_SOURCE);
+ __assign_bit(GPIOD_FLAG_PULL_UP, &val,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_UP);
+ __assign_bit(GPIOD_FLAG_PULL_DOWN, &val,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN);
+ __assign_bit(GPIOD_FLAG_BIAS_DISABLE, &val,
+ lflags & GPIO_V2_LINE_FLAG_BIAS_DISABLED);
+ __assign_bit(GPIOD_FLAG_EVENT_CLOCK_REALTIME, &val,
+ lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_REALTIME);
+ __assign_bit(GPIOD_FLAG_EVENT_CLOCK_HTE, &val,
+ lflags & GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE);
+
+ gpiod_update_flags(desc, mask, val);
}
static long linereq_get_values(struct linereq *lr, void __user *ip)
@@ -1411,7 +1423,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);
+ gpio_v2_line_config_flags_to_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);
@@ -1667,7 +1679,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);
+ gpio_v2_line_config_flags_to_desc_flags(flags, desc);
ret = gpiod_set_transitory(desc, false);
if (ret < 0)
@@ -2062,7 +2074,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_flags_to_desc_flags(lflags, desc);
ret = gpiod_direction_input(desc);
if (ret)
--
2.47.3
next prev parent reply other threads:[~2026-11-04 15:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-04 15:36 [PATCH 0/5] gpiolib: improve concurrent access to the flags field of struct gpio_desc Bartosz Golaszewski
2026-11-04 15:36 ` [PATCH 1/5] gpiolib: don't overwrite the entire flags field in gpiod_get_direction() Bartosz Golaszewski
2026-11-04 15:36 ` [PATCH 2/5] gpio: shared: use atomic set_bit() for descriptor flags Bartosz Golaszewski
2026-11-04 15:36 ` [PATCH 3/5] gpiolib: provide gpiod_update_flags() Bartosz Golaszewski
2026-11-04 15:36 ` [PATCH 4/5] gpiolib: don't overwrite the entire flags field in gpiod_free_commit() Bartosz Golaszewski
2026-11-04 15:37 ` Bartosz Golaszewski [this message]
2026-11-04 21:55 ` [PATCH 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=20261104-gpio-desc-flags-atomic-v1-5-e23cedf619ba@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®