Store the debounce period for a requested line locally, rather than in
the debounce_period_us field in the gpiolib struct gpio_desc.
Add a global tree of lines containing supplemental line information
to make the debounce period available to be reported by the
GPIO_V2_GET_LINEINFO_IOCTL and the line change notifier.
Signed-off-by: Kent Gibson <[email protected]>
---
drivers/gpio/gpiolib-cdev.c | 152 ++++++++++++++++++++++++++++++------
1 file changed, 130 insertions(+), 22 deletions(-)
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 02ffda6c1e51..5ba900e5461a 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -21,6 +21,7 @@
#include <linux/mutex.h>
#include <linux/pinctrl/consumer.h>
#include <linux/poll.h>
+#include <linux/rbtree.h>
#include <linux/seq_file.h>
#include <linux/spinlock.h>
#include <linux/timekeeping.h>
@@ -461,6 +462,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
/**
* struct line - contains the state of a requested line
+ * @node: to store the object in supinfo_tree if supplemental
* @desc: the GPIO descriptor for this line.
* @req: the corresponding line request
* @irq: the interrupt triggered in response to events on this GPIO
@@ -473,6 +475,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
* @line_seqno: the seqno for the current edge event in the sequence of
* events for this line.
* @work: the worker that implements software debouncing
+ * @debounce_period_us: the debounce period in microseconds
* @sw_debounced: flag indicating if the software debouncer is active
* @level: the current debounced physical level of the line
* @hdesc: the Hardware Timestamp Engine (HTE) descriptor
@@ -481,6 +484,7 @@ static int linehandle_create(struct gpio_device *gdev, void __user *ip)
* @last_seqno: the last sequence number before debounce period expires
*/
struct line {
+ struct rb_node node;
struct gpio_desc *desc;
/*
* -- edge detector specific fields --
@@ -514,6 +518,15 @@ struct line {
* -- debouncer specific fields --
*/
struct delayed_work work;
+ /*
+ * debounce_period_us is accessed by debounce_irq_handler() and
+ * process_hw_ts() which are disabled when modified by
+ * debounce_setup(), edge_detector_setup() or edge_detector_stop()
+ * or can live with a stale version when updated by
+ * edge_detector_update().
+ * The modifying functions are themselves mutually exclusive.
+ */
+ unsigned int debounce_period_us;
/*
* sw_debounce is accessed by linereq_set_config(), which is the
* only setter, and linereq_get_values(), which can live with a
@@ -546,6 +559,17 @@ struct line {
#endif /* CONFIG_HTE */
};
+/*
+ * a rbtree of the struct lines containing supplemental info.
+ * Used to populate gpio_v2_line_info with cdev specific fields not contained
+ * in the struct gpio_desc.
+ * A line is determined to contain supplemental information by
+ * line_is_supplemental().
+ */
+static struct rb_root supinfo_tree = RB_ROOT;
+/* covers supinfo_tree */
+DEFINE_SPINLOCK(supinfo_lock);
+
/**
* struct linereq - contains the state of a userspace line request
* @gdev: the GPIO device the line request pertains to
@@ -575,6 +599,94 @@ struct linereq {
struct line lines[] __counted_by(num_lines);
};
+static void supinfo_insert(struct line *line)
+{
+ struct rb_node **new = &(supinfo_tree.rb_node), *parent = NULL;
+ struct line *entry;
+
+ guard(spinlock)(&supinfo_lock);
+
+ while (*new) {
+ entry = container_of(*new, struct line, node);
+
+ parent = *new;
+ if (line->desc < entry->desc) {
+ new = &((*new)->rb_left);
+ } else if (line->desc > entry->desc) {
+ new = &((*new)->rb_right);
+ } else {
+ /* this should never happen */
+ WARN(1, "duplicate line inserted");
+ return;
+ }
+ }
+
+ rb_link_node(&line->node, parent, new);
+ rb_insert_color(&line->node, &supinfo_tree);
+}
+
+static void supinfo_erase(struct line *line)
+{
+ guard(spinlock)(&supinfo_lock);
+
+ rb_erase(&line->node, &supinfo_tree);
+}
+
+static struct line *supinfo_find(struct gpio_desc *desc)
+{
+ struct rb_node *node = supinfo_tree.rb_node;
+ struct line *line;
+
+ while (node) {
+ line = container_of(node, struct line, node);
+ if (desc < line->desc)
+ node = node->rb_left;
+ else if (desc > line->desc)
+ node = node->rb_right;
+ else
+ return line;
+ }
+ return NULL;
+}
+
+static void supinfo_to_lineinfo(struct gpio_desc *desc,
+ struct gpio_v2_line_info *info)
+{
+ struct gpio_v2_line_attribute *attr;
+ struct line *line;
+
+ guard(spinlock)(&supinfo_lock);
+
+ line = supinfo_find(desc);
+ if (line) {
+ attr = &info->attrs[info->num_attrs];
+ attr->id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE;
+ attr->debounce_period_us = READ_ONCE(line->debounce_period_us);
+ info->num_attrs++;
+ }
+}
+
+static inline bool line_is_supplemental(struct line *line)
+{
+ return READ_ONCE(line->debounce_period_us);
+}
+
+static void line_set_debounce_period(struct line *line,
+ unsigned int debounce_period_us)
+{
+ bool was_suppl = line_is_supplemental(line);
+
+ WRITE_ONCE(line->debounce_period_us, debounce_period_us);
+
+ if (line_is_supplemental(line) == was_suppl)
+ return;
+
+ if (was_suppl)
+ supinfo_erase(line);
+ else
+ supinfo_insert(line);
+}
+
#define GPIO_V2_LINE_BIAS_FLAGS \
(GPIO_V2_LINE_FLAG_BIAS_PULL_UP | \
GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN | \
@@ -723,7 +835,7 @@ static enum hte_return process_hw_ts(struct hte_ts_data *ts, void *p)
line->total_discard_seq++;
line->last_seqno = ts->seq;
mod_delayed_work(system_wq, &line->work,
- usecs_to_jiffies(READ_ONCE(line->desc->debounce_period_us)));
+ usecs_to_jiffies(READ_ONCE(line->debounce_period_us)));
} else {
if (unlikely(ts->seq < line->line_seqno))
return HTE_CB_HANDLED;
@@ -864,7 +976,7 @@ static irqreturn_t debounce_irq_handler(int irq, void *p)
struct line *line = p;
mod_delayed_work(system_wq, &line->work,
- usecs_to_jiffies(READ_ONCE(line->desc->debounce_period_us)));
+ usecs_to_jiffies(READ_ONCE(line->debounce_period_us)));
return IRQ_HANDLED;
}
@@ -946,7 +1058,7 @@ static int debounce_setup(struct line *line, unsigned int debounce_period_us)
/* try hardware */
ret = gpiod_set_debounce(line->desc, debounce_period_us);
if (!ret) {
- WRITE_ONCE(line->desc->debounce_period_us, debounce_period_us);
+ line_set_debounce_period(line, debounce_period_us);
return ret;
}
if (ret != -ENOTSUPP)
@@ -1025,8 +1137,7 @@ static void edge_detector_stop(struct line *line)
cancel_delayed_work_sync(&line->work);
WRITE_ONCE(line->sw_debounced, 0);
WRITE_ONCE(line->edflags, 0);
- if (line->desc)
- WRITE_ONCE(line->desc->debounce_period_us, 0);
+ line_set_debounce_period(line, 0);
/* do not change line->level - see comment in debounced_value() */
}
@@ -1051,7 +1162,7 @@ static int edge_detector_setup(struct line *line,
ret = debounce_setup(line, debounce_period_us);
if (ret)
return ret;
- WRITE_ONCE(line->desc->debounce_period_us, debounce_period_us);
+ line_set_debounce_period(line, debounce_period_us);
}
/* detection disabled or sw debouncer will provide edge detection */
@@ -1093,12 +1204,12 @@ static int edge_detector_update(struct line *line,
gpio_v2_line_config_debounce_period(lc, line_idx);
if ((active_edflags == edflags) &&
- (READ_ONCE(line->desc->debounce_period_us) == debounce_period_us))
+ (READ_ONCE(line->debounce_period_us) == debounce_period_us))
return 0;
/* sw debounced and still will be...*/
if (debounce_period_us && READ_ONCE(line->sw_debounced)) {
- WRITE_ONCE(line->desc->debounce_period_us, debounce_period_us);
+ line_set_debounce_period(line, debounce_period_us);
return 0;
}
@@ -1573,6 +1684,7 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
static void linereq_free(struct linereq *lr)
{
+ struct line *line;
unsigned int i;
if (lr->device_unregistered_nb.notifier_call)
@@ -1580,10 +1692,14 @@ static void linereq_free(struct linereq *lr)
&lr->device_unregistered_nb);
for (i = 0; i < lr->num_lines; i++) {
- if (lr->lines[i].desc) {
- edge_detector_stop(&lr->lines[i]);
- gpiod_free(lr->lines[i].desc);
- }
+ line = &lr->lines[i];
+ if (!line->desc)
+ continue;
+
+ edge_detector_stop(line);
+ if (line_is_supplemental(line))
+ supinfo_erase(line);
+ gpiod_free(line->desc);
}
kfifo_free(&lr->events);
kfree(lr->label);
@@ -2274,8 +2390,6 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
struct gpio_chip *gc = desc->gdev->chip;
bool ok_for_pinctrl;
unsigned long flags;
- u32 debounce_period_us;
- unsigned int num_attrs = 0;
memset(info, 0, sizeof(*info));
info->offset = gpio_chip_hwgpio(desc);
@@ -2341,14 +2455,6 @@ static void gpio_desc_to_lineinfo(struct gpio_desc *desc,
else if (test_bit(FLAG_EVENT_CLOCK_HTE, &desc->flags))
info->flags |= GPIO_V2_LINE_FLAG_EVENT_CLOCK_HTE;
- debounce_period_us = READ_ONCE(desc->debounce_period_us);
- if (debounce_period_us) {
- info->attrs[num_attrs].id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE;
- info->attrs[num_attrs].debounce_period_us = debounce_period_us;
- num_attrs++;
- }
- info->num_attrs = num_attrs;
-
spin_unlock_irqrestore(&gpio_lock, flags);
}
@@ -2455,6 +2561,7 @@ static int lineinfo_get(struct gpio_chardev_data *cdev, void __user *ip,
return -EBUSY;
}
gpio_desc_to_lineinfo(desc, &lineinfo);
+ supinfo_to_lineinfo(desc, &lineinfo);
if (copy_to_user(ip, &lineinfo, sizeof(lineinfo))) {
if (watch)
@@ -2545,6 +2652,7 @@ static int lineinfo_changed_notify(struct notifier_block *nb,
chg.event_type = action;
chg.timestamp_ns = ktime_get_ns();
gpio_desc_to_lineinfo(desc, &chg.info);
+ supinfo_to_lineinfo(desc, &chg.info);
ret = kfifo_in_spinlocked(&cdev->events, &chg, 1, &cdev->wait.lock);
if (ret)
--
2.39.2
On Fri, Dec 15, 2023 at 10:38:01AM +0800, Kent Gibson wrote:
> Store the debounce period for a requested line locally, rather than in
> the debounce_period_us field in the gpiolib struct gpio_desc.
>
> Add a global tree of lines containing supplemental line information
> to make the debounce period available to be reported by the
> GPIO_V2_GET_LINEINFO_IOCTL and the line change notifier.
LGTM, a few minor comments below.
...
> +/*
(now you can have a kernel doc :-)
> + * a rbtree of the struct lines containing supplemental info.
> + * Used to populate gpio_v2_line_info with cdev specific fields not contained
> + * in the struct gpio_desc.
> + * A line is determined to contain supplemental information by
> + * line_is_supplemental().
> + */
> +static struct rb_root supinfo_tree = RB_ROOT;
> +/* covers supinfo_tree */
> +DEFINE_SPINLOCK(supinfo_lock);
Shouldn't this also be static?
...
> + guard(spinlock)(&supinfo_lock);
+ cleanup.h ?
...
> +static void supinfo_to_lineinfo(struct gpio_desc *desc,
> + struct gpio_v2_line_info *info)
> +{
> + struct gpio_v2_line_attribute *attr;
> + struct line *line;
> +
> + guard(spinlock)(&supinfo_lock);
> +
> + line = supinfo_find(desc);
> + if (line) {
if (!line)
return;
?
> + attr = &info->attrs[info->num_attrs];
> + attr->id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE;
> + attr->debounce_period_us = READ_ONCE(line->debounce_period_us);
> + info->num_attrs++;
> + }
> +}
--
With Best Regards,
Andy Shevchenko
On Fri, Dec 15, 2023 at 06:35:14PM +0200, Andy Shevchenko wrote:
> On Fri, Dec 15, 2023 at 10:38:01AM +0800, Kent Gibson wrote:
> > Store the debounce period for a requested line locally, rather than in
> > the debounce_period_us field in the gpiolib struct gpio_desc.
> >
> > Add a global tree of lines containing supplemental line information
> > to make the debounce period available to be reported by the
> > GPIO_V2_GET_LINEINFO_IOCTL and the line change notifier.
>
> LGTM, a few minor comments below.
>
> ...
>
> > +/*
>
> (now you can have a kernel doc :-)
>
Maybe you can, but I can't.
> > + * a rbtree of the struct lines containing supplemental info.
> > + * Used to populate gpio_v2_line_info with cdev specific fields not contained
> > + * in the struct gpio_desc.
> > + * A line is determined to contain supplemental information by
> > + * line_is_supplemental().
> > + */
> > +static struct rb_root supinfo_tree = RB_ROOT;
> > +/* covers supinfo_tree */
> > +DEFINE_SPINLOCK(supinfo_lock);
>
> Shouldn't this also be static?
>
Indeed.
> ...
>
> > + guard(spinlock)(&supinfo_lock);
>
> + cleanup.h ?
>
Bah, I could've sworn I added that in, but it isn't evident in any of
the patches all the way back to v1, so apparently not.
> ...
>
> > +static void supinfo_to_lineinfo(struct gpio_desc *desc,
> > + struct gpio_v2_line_info *info)
> > +{
> > + struct gpio_v2_line_attribute *attr;
> > + struct line *line;
> > +
> > + guard(spinlock)(&supinfo_lock);
> > +
> > + line = supinfo_find(desc);
> > + if (line) {
>
> if (!line)
> return;
>
> ?
Will do.
>
> > + attr = &info->attrs[info->num_attrs];
> > + attr->id = GPIO_V2_LINE_ATTR_ID_DEBOUNCE;
> > + attr->debounce_period_us = READ_ONCE(line->debounce_period_us);
> > + info->num_attrs++;
> > + }
> > +}
>
Cheers,
Kent.
> --
> With Best Regards,
> Andy Shevchenko
>
>