diff mbox series

[v4,1/5] gpiolib: cdev: relocate debounce_period_us from struct gpio_desc

Message ID 20231216001652.56276-2-warthog618@gmail.com
State Superseded
Headers show
Series gpiolib: cdev: relocate debounce_period_us | expand

Commit Message

Kent Gibson Dec. 16, 2023, 12:16 a.m. UTC
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 <warthog618@gmail.com>
Reviewed-by: Andy Shevchenko <andy@kernel.org>
---
 drivers/gpio/gpiolib-cdev.c | 154 ++++++++++++++++++++++++++++++------
 1 file changed, 132 insertions(+), 22 deletions(-)

Comments

Bartosz Golaszewski Dec. 18, 2023, 3:24 p.m. UTC | #1
On Sat, Dec 16, 2023 at 1:17 AM Kent Gibson <warthog618@gmail.com> 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.
>
> Signed-off-by: Kent Gibson <warthog618@gmail.com>
> Reviewed-by: Andy Shevchenko <andy@kernel.org>
> ---
>  drivers/gpio/gpiolib-cdev.c | 154 ++++++++++++++++++++++++++++++------
>  1 file changed, 132 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index 02ffda6c1e51..47197f6339c4 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -5,6 +5,7 @@
>  #include <linux/bitmap.h>
>  #include <linux/build_bug.h>
>  #include <linux/cdev.h>
> +#include <linux/cleanup.h>
>  #include <linux/compat.h>
>  #include <linux/compiler.h>
>  #include <linux/device.h>
> @@ -21,6 +22,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 +463,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 +476,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 +485,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 +519,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 +560,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 */
> +static 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 +600,95 @@ 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)
> +               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++;
> +}
> +
> +static inline bool line_is_supplemental(struct line *line)

Under v2 I suggested naming this line_has_suppinfo(). Any reason not
to do it? I think it's more logical than saying "line is
supplemental". The latter makes it seem as if certain line objects
would "supplement" some third party with something. What this really
checks is: does this line contain additional information.

> +{
> +       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);

Could you add a comment here saying it's called with the config mutex
taken as at first glance it looks racy but actually isn't?

Bart

> +}
> +
>  #define GPIO_V2_LINE_BIAS_FLAGS \
>         (GPIO_V2_LINE_FLAG_BIAS_PULL_UP | \
>          GPIO_V2_LINE_FLAG_BIAS_PULL_DOWN | \
> @@ -723,7 +837,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 +978,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 +1060,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 +1139,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 +1164,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 +1206,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 +1686,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 +1694,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 +2392,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 +2457,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 +2563,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 +2654,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
>
Bartosz Golaszewski Dec. 18, 2023, 4:01 p.m. UTC | #2
On Mon, Dec 18, 2023 at 4:40 PM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Mon, Dec 18, 2023 at 04:24:50PM +0100, Bartosz Golaszewski wrote:
> > On Sat, Dec 16, 2023 at 1:17 AM Kent Gibson <warthog618@gmail.com> 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.
> > >
> > > Signed-off-by: Kent Gibson <warthog618@gmail.com>
> > > Reviewed-by: Andy Shevchenko <andy@kernel.org>
> > > ---
> > >  drivers/gpio/gpiolib-cdev.c | 154 ++++++++++++++++++++++++++++++------
> > >  1 file changed, 132 insertions(+), 22 deletions(-)
> > >
> > > +static inline bool line_is_supplemental(struct line *line)
> >
> > Under v2 I suggested naming this line_has_suppinfo(). Any reason not
> > to do it? I think it's more logical than saying "line is
> > supplemental". The latter makes it seem as if certain line objects
> > would "supplement" some third party with something. What this really
> > checks is: does this line contain additional information.
> >
>
>
> My bad - responded to your first comment and then missed the rest.
>
> Agreed - the naming could be better. Will fix for v5.
>
> > > +{
> > > +       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);
> >
> > Could you add a comment here saying it's called with the config mutex
> > taken as at first glance it looks racy but actually isn't?
> >
>
> Sure.  Though it is also covered by the gdev->sem you added, right?
> So the config_mutex is now redundant?
> Should I document it is covered by both?
> Or drop the config_mutex entirely?
>

No! The semaphore is a read-write semaphore and we can have multiple
readers at once. This semaphore only protects us from the chip being
set to NULL during a system call. It will also go away once I get the
descriptor access serialized (or not - I'm figuring out a lockless
approach) and finally use SRCU to protect gdev->chip.

> And you wanted some comments to explain the logic?
> I thought this is a common "has it changed" pattern, and so didn't
> require additional explanation, but I guess not as common as I thought.
>

If line_set_debounce_period() could be called concurrently for the
same line, this approach would be racy. It cannot but I want a comment
here as I fear that if in the future we add some more attributes that
constitute "supplemental info" and which may be changed outside of the
config lock then this would in fact become racy.

Bart

> Cheers,
> Kent.
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 02ffda6c1e51..47197f6339c4 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -5,6 +5,7 @@ 
 #include <linux/bitmap.h>
 #include <linux/build_bug.h>
 #include <linux/cdev.h>
+#include <linux/cleanup.h>
 #include <linux/compat.h>
 #include <linux/compiler.h>
 #include <linux/device.h>
@@ -21,6 +22,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 +463,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 +476,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 +485,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 +519,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 +560,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 */
+static 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 +600,95 @@  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)
+		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++;
+}
+
+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 +837,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 +978,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 +1060,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 +1139,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 +1164,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 +1206,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 +1686,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 +1694,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 +2392,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 +2457,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 +2563,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 +2654,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)