Re: [PATCH v2 07/18] gpiolib: cdev: support edge detection for uAPI v2

2020-08-04 Thread Bartosz Golaszewski
On Sat, Jul 25, 2020 at 6:21 AM Kent Gibson  wrote:
>
> Add support for edge detection to lines requested using
> GPIO_GET_LINE_IOCTL.
>
> Signed-off-by: Kent Gibson 
> ---

[snip!]

> +
> +static irqreturn_t edge_irq_thread(int irq, void *p)
> +{
> +   struct edge_detector *edet = p;
> +   struct line *line = edet->line;
> +   struct gpio_desc *desc = edge_detector_desc(edet);
> +   struct gpioline_event le;
> +   int ret;
> +
> +   /* Do not leak kernel stack to userspace */
> +   memset(&le, 0, sizeof(le));
> +
> +   /*
> +* We may be running from a nested threaded interrupt in which case
> +* we didn't get the timestamp from edge_irq_handler().
> +*/
> +   if (!edet->timestamp) {
> +   le.timestamp = ktime_get_ns();

IIRC Marc suggested using smp_rmw() here before reading the timestamp.
Do we still need it or something changed?

Bartosz


[PATCH v2 07/18] gpiolib: cdev: support edge detection for uAPI v2

2020-07-24 Thread Kent Gibson
Add support for edge detection to lines requested using
GPIO_GET_LINE_IOCTL.

Signed-off-by: Kent Gibson 
---

The edge_detector implementation is based on the V1 lineevent implementation.

 drivers/gpio/gpiolib-cdev.c | 314 +++-
 drivers/gpio/gpiolib.c  |   2 +
 drivers/gpio/gpiolib.h  |   2 +
 3 files changed, 317 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 1f282207fb70..8caebb460557 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -381,11 +381,43 @@ static int linehandle_create(struct gpio_device *gdev, 
void __user *ip)
 }
 #endif /* CONFIG_GPIO_CDEV_V1 */
 
+/**
+ * struct edge_detector - contains the state of a line edge detector
+ * @line: the corresponding line request
+ * @irq: the interrupt triggered in response to events on this GPIO
+ * @flags: the flags, GPIOLINE_FLAG_V2_EDGE_RISING and/or
+ * GPIOLINE_FLAG_V2_EDGE_FALLING, indicating the edge detection applied
+ * @timestamp: cache for the timestamp storing it between hardirq and IRQ
+ * thread, used to bring the timestamp close to the actual event
+ * @seqno: the seqno for the current edge event in the sequence of events
+ * for the corresponding line request. Ths is drawn from the @line.
+ * @line_seqno: the seqno for the current edge event in the sequence of
+ * events for this line.
+ */
+struct edge_detector {
+   struct line *line;
+   unsigned int irq;
+   u64 flags;
+   /*
+* timestamp and seqno are shared by edge_irq_handler() and
+* edge_irq_thread() which are themselves mutually exclusive.
+*/
+   u64 timestamp;
+   u32 seqno;
+   u32 line_seqno;
+};
+
 /**
  * struct line - contains the state of a userspace line request
  * @gdev: the GPIO device the line request pertains to
  * @label: consumer label used to tag descriptors
  * @num_descs: the number of descriptors held in the descs array
+ * @wait: wait queue that handles blocking reads of events
+ * @events: KFIFO for the GPIO events
+ * @seqno: the sequence number for edge events generated on all lines in
+ * this line request.  Note that this is not used when @num_descs is 1, as
+ * the line_seqno is then the same and is cheaper to calculate.
+ * @edets: an array of edge detectors, of size @num_descs
  * @descs: the GPIO descriptors held by this line request, with @num_descs
  * elements.
  */
@@ -393,10 +425,147 @@ struct line {
struct gpio_device *gdev;
const char *label;
u32 num_descs;
+   wait_queue_head_t wait;
+   DECLARE_KFIFO_PTR(events, struct gpioline_event);
+   atomic_t seqno;
+   struct edge_detector *edets;
/* descs must be last so it can be dynamically sized */
struct gpio_desc *descs[];
 };
 
+static inline struct gpio_desc *edge_detector_desc(
+   const struct edge_detector *edet)
+{
+   return edet->line->descs[edet - &edet->line->edets[0]];
+}
+
+static irqreturn_t edge_irq_thread(int irq, void *p)
+{
+   struct edge_detector *edet = p;
+   struct line *line = edet->line;
+   struct gpio_desc *desc = edge_detector_desc(edet);
+   struct gpioline_event le;
+   int ret;
+
+   /* Do not leak kernel stack to userspace */
+   memset(&le, 0, sizeof(le));
+
+   /*
+* We may be running from a nested threaded interrupt in which case
+* we didn't get the timestamp from edge_irq_handler().
+*/
+   if (!edet->timestamp) {
+   le.timestamp = ktime_get_ns();
+   if (line->num_descs != 1)
+   edet->seqno = atomic_inc_return(&line->seqno);
+   } else {
+   le.timestamp = edet->timestamp;
+   }
+   edet->timestamp = 0;
+
+   if (edet->flags == (GPIOLINE_FLAG_V2_EDGE_RISING |
+   GPIOLINE_FLAG_V2_EDGE_FALLING)) {
+   int level = gpiod_get_value_cansleep(desc);
+
+   if (level)
+   /* Emit low-to-high event */
+   le.id = GPIOLINE_EVENT_RISING_EDGE;
+   else
+   /* Emit high-to-low event */
+   le.id = GPIOLINE_EVENT_FALLING_EDGE;
+   } else if (edet->flags == GPIOLINE_FLAG_V2_EDGE_RISING) {
+   /* Emit low-to-high event */
+   le.id = GPIOLINE_EVENT_RISING_EDGE;
+   } else if (edet->flags == GPIOLINE_FLAG_V2_EDGE_FALLING) {
+   /* Emit high-to-low event */
+   le.id = GPIOLINE_EVENT_FALLING_EDGE;
+   } else {
+   return IRQ_NONE;
+   }
+   edet->line_seqno++;
+   le.line_seqno = edet->line_seqno;
+   le.seqno = (line->num_descs == 1) ? le.line_seqno : edet->seqno;
+   le.offset = gpio_chip_hwgpio(desc);
+
+   ret = kfifo_in_spinlocked_noirqsave(&line->events, &le,
+   1, &line->wait.lock);
+   if (ret)
+   w