From: Kent Gibson <warthog618@gmail.com>
To: linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
bgolaszewski@baylibre.com, linus.walleij@linaro.org
Cc: Kent Gibson <warthog618@gmail.com>
Subject: [PATCH v2] gpiolib: cdev: document that line eflags are shared
Date: Wed, 14 Oct 2020 14:29:21 +0800 [thread overview]
Message-ID: <20201014062921.79112-1-warthog618@gmail.com> (raw)
The line.eflags field is shared so document this fact and highlight it
throughout using READ_ONCE() and WRITE_ONCE() accessors.
Also use a local copy of the eflags in edge_irq_thread() to ensure
consistent control flow even if eflags changes. This is only a defensive
measure as edge_irq_thread() is currently disabled when the eflags are
changed.
Signed-off-by: Kent Gibson <warthog618@gmail.com>
---
Changes for v2:
- fixed typo in commit comment.
drivers/gpio/gpiolib-cdev.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 192721f829a3..678de9264617 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -428,6 +428,12 @@ struct line {
*/
struct linereq *req;
unsigned int irq;
+ /*
+ * eflags is set by edge_detector_setup(), edge_detector_stop() and
+ * edge_detector_update(), which are themselves mutually exclusive,
+ * and is accessed by edge_irq_thread() and debounce_work_func(),
+ * which can both live with a slightly stale value.
+ */
u64 eflags;
/*
* timestamp_ns and req_seqno are accessed only by
@@ -534,6 +540,7 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
struct line *line = p;
struct linereq *lr = line->req;
struct gpio_v2_line_event le;
+ u64 eflags;
/* Do not leak kernel stack to userspace */
memset(&le, 0, sizeof(le));
@@ -552,8 +559,9 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
}
line->timestamp_ns = 0;
- if (line->eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
- GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
+ eflags = READ_ONCE(line->eflags);
+ if (eflags == (GPIO_V2_LINE_FLAG_EDGE_RISING |
+ GPIO_V2_LINE_FLAG_EDGE_FALLING)) {
int level = gpiod_get_value_cansleep(line->desc);
if (level)
@@ -562,10 +570,10 @@ static irqreturn_t edge_irq_thread(int irq, void *p)
else
/* Emit high-to-low event */
le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE;
- } else if (line->eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) {
+ } else if (eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) {
/* Emit low-to-high event */
le.id = GPIO_V2_LINE_EVENT_RISING_EDGE;
- } else if (line->eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) {
+ } else if (eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) {
/* Emit high-to-low event */
le.id = GPIO_V2_LINE_EVENT_FALLING_EDGE;
} else {
@@ -634,6 +642,7 @@ static void debounce_work_func(struct work_struct *work)
struct line *line = container_of(work, struct line, work.work);
struct linereq *lr;
int level;
+ u64 eflags;
level = gpiod_get_raw_value_cansleep(line->desc);
if (level < 0) {
@@ -647,7 +656,8 @@ static void debounce_work_func(struct work_struct *work)
WRITE_ONCE(line->level, level);
/* -- edge detection -- */
- if (!line->eflags)
+ eflags = READ_ONCE(line->eflags);
+ if (!eflags)
return;
/* switch from physical level to logical - if they differ */
@@ -655,8 +665,8 @@ static void debounce_work_func(struct work_struct *work)
level = !level;
/* ignore edges that are not being monitored */
- if (((line->eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) && !level) ||
- ((line->eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) && level))
+ if (((eflags == GPIO_V2_LINE_FLAG_EDGE_RISING) && !level) ||
+ ((eflags == GPIO_V2_LINE_FLAG_EDGE_FALLING) && level))
return;
/* Do not leak kernel stack to userspace */
@@ -755,7 +765,7 @@ static void edge_detector_stop(struct line *line)
cancel_delayed_work_sync(&line->work);
WRITE_ONCE(line->sw_debounced, 0);
- line->eflags = 0;
+ WRITE_ONCE(line->eflags, 0);
/* do not change line->level - see comment in debounced_value() */
}
@@ -774,7 +784,7 @@ static int edge_detector_setup(struct line *line,
if (ret)
return ret;
}
- line->eflags = eflags;
+ WRITE_ONCE(line->eflags, eflags);
if (gpio_v2_line_config_debounced(lc, line_idx)) {
debounce_period_us = gpio_v2_line_config_debounce_period(lc, line_idx);
ret = debounce_setup(line, debounce_period_us);
@@ -817,13 +827,13 @@ static int edge_detector_update(struct line *line,
unsigned int debounce_period_us =
gpio_v2_line_config_debounce_period(lc, line_idx);
- if ((line->eflags == eflags) && !polarity_change &&
+ if ((READ_ONCE(line->eflags) == eflags) && !polarity_change &&
(READ_ONCE(line->desc->debounce_period_us) == debounce_period_us))
return 0;
/* sw debounced and still will be...*/
if (debounce_period_us && READ_ONCE(line->sw_debounced)) {
- line->eflags = eflags;
+ WRITE_ONCE(line->eflags, eflags);
WRITE_ONCE(line->desc->debounce_period_us, debounce_period_us);
return 0;
}
--
2.28.0
next reply other threads:[~2020-10-14 8:54 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-14 6:29 Kent Gibson [this message]
2020-10-16 14:24 ` [PATCH v2] gpiolib: cdev: document that line eflags are shared Andy Shevchenko
2020-10-16 23:39 ` Kent Gibson
2020-10-26 14:26 ` Bartosz Golaszewski
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=20201014062921.79112-1-warthog618@gmail.com \
--to=warthog618@gmail.com \
--cc=bgolaszewski@baylibre.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.