Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1209240 > unrolled thread

[PATCH] Staging: iio: Move evgen interrupt generation to irq_work

Started byCristina Opriceana <cristina.opriceana@gmail.com>
First post2015-08-18 15:00 +0200
Last post2015-08-18 16:10 +0200
Articles 3 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] Staging: iio: Move evgen interrupt generation to irq_work Cristina Opriceana <cristina.opriceana@gmail.com> - 2015-08-18 15:00 +0200
    Re: [PATCH] Staging: iio: Move evgen interrupt generation to irq_work Crt Mori <cmo@melexis.com> - 2015-08-18 15:50 +0200
      Re: [PATCH] Staging: iio: Move evgen interrupt generation to irq_work Cristina Georgiana Opriceana <cristina.opriceana@gmail.com> - 2015-08-18 16:10 +0200

#1209240 — [PATCH] Staging: iio: Move evgen interrupt generation to irq_work

FromCristina Opriceana <cristina.opriceana@gmail.com>
Date2015-08-18 15:00 +0200
Subject[PATCH] Staging: iio: Move evgen interrupt generation to irq_work
Message-ID<pYOEG-6Gh-17@gated-at.bofh.it>
A more abstract way of handling interrupt generation in the dummy driver
is represented by irq_work which substitutes the irq_chip, used to
provide an IRQ line number. irq_work runs tasks from hardirq context,
but in a NMI-safe environment and deals better with synchronization
problems. An interrupt can be triggered by calling irq_work_queue()
and the work will be performed by a pre-registered handler, which
acts as a callback.

Cc: Daniel Baluta <daniel.baluta@intel.com>
Suggested-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
---
This is the first draft for the irq_work replacement.
As we want to move the iio dummy driver out of staging,
we started to implement Jonathan's suggestions from here:
<http://marc.info/?l=linux-iio&m=142403390705515&w=2>

 drivers/staging/iio/iio_dummy_evgen.c         | 171 +++++++++-----------------
 drivers/staging/iio/iio_dummy_evgen.h         |  15 ++-
 drivers/staging/iio/iio_simple_dummy.c        |   6 +-
 drivers/staging/iio/iio_simple_dummy.h        |   8 +-
 drivers/staging/iio/iio_simple_dummy_events.c |  71 ++++-------
 5 files changed, 105 insertions(+), 166 deletions(-)

diff --git a/drivers/staging/iio/iio_dummy_evgen.c b/drivers/staging/iio/iio_dummy_evgen.c
index 6d38854..974ef8a 100644
--- a/drivers/staging/iio/iio_dummy_evgen.c
+++ b/drivers/staging/iio/iio_dummy_evgen.c
@@ -25,132 +25,87 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 
-/* Fiddly bit of faking and irq without hardware */
-#define IIO_EVENTGEN_NO 10
-/**
- * struct iio_dummy_evgen - evgen state
- * @chip: irq chip we are faking
- * @base: base of irq range
- * @enabled: mask of which irqs are enabled
- * @inuse: mask of which irqs are connected
- * @regs: irq regs we are faking
- * @lock: protect the evgen state
- */
-struct iio_dummy_eventgen {
-	struct irq_chip chip;
-	int base;
-	bool enabled[IIO_EVENTGEN_NO];
-	bool inuse[IIO_EVENTGEN_NO];
-	struct iio_dummy_regs regs[IIO_EVENTGEN_NO];
-	struct mutex lock;
-};
+static LIST_HEAD(iio_dummy_event_list);
+static DEFINE_MUTEX(iio_dummy_event_list_mutex);
 
-/* We can only ever have one instance of this 'device' */
-static struct iio_dummy_eventgen *iio_evgen;
-static const char *iio_evgen_name = "iio_dummy_evgen";
-
-static void iio_dummy_event_irqmask(struct irq_data *d)
+struct iio_dummy_event *get_event(int index)
 {
-	struct irq_chip *chip = irq_data_get_irq_chip(d);
-	struct iio_dummy_eventgen *evgen =
-		container_of(chip, struct iio_dummy_eventgen, chip);
+	struct list_head *ptr, *tmp;
+	struct iio_dummy_event *iio_event = NULL;
 
-	evgen->enabled[d->irq - evgen->base] = false;
-}
-
-static void iio_dummy_event_irqunmask(struct irq_data *d)
-{
-	struct irq_chip *chip = irq_data_get_irq_chip(d);
-	struct iio_dummy_eventgen *evgen =
-		container_of(chip, struct iio_dummy_eventgen, chip);
-
-	evgen->enabled[d->irq - evgen->base] = true;
+	mutex_lock(&iio_dummy_event_list_mutex);
+	list_for_each_safe(ptr, tmp, &iio_dummy_event_list) {
+		iio_event = list_entry(ptr, struct iio_dummy_event, l);
+		if (iio_event->regs.reg_id == index)
+			break;
+	}
+	mutex_unlock(&iio_dummy_event_list_mutex);
+	return iio_event;
 }
 
-static int iio_dummy_evgen_create(void)
+int iio_dummy_evgen_create(struct iio_dev *indio_dev, int index)
 {
-	int ret, i;
+	struct iio_dummy_event *iio_event;
 
-	iio_evgen = kzalloc(sizeof(*iio_evgen), GFP_KERNEL);
-	if (!iio_evgen)
+	iio_event = kzalloc(sizeof(*iio_event), GFP_KERNEL);
+	if (!iio_event)
 		return -ENOMEM;
 
-	iio_evgen->base = irq_alloc_descs(-1, 0, IIO_EVENTGEN_NO, 0);
-	if (iio_evgen->base < 0) {
-		ret = iio_evgen->base;
-		kfree(iio_evgen);
-		return ret;
-	}
-	iio_evgen->chip.name = iio_evgen_name;
-	iio_evgen->chip.irq_mask = &iio_dummy_event_irqmask;
-	iio_evgen->chip.irq_unmask = &iio_dummy_event_irqunmask;
-	for (i = 0; i < IIO_EVENTGEN_NO; i++) {
-		irq_set_chip(iio_evgen->base + i, &iio_evgen->chip);
-		irq_set_handler(iio_evgen->base + i, &handle_simple_irq);
-		irq_modify_status(iio_evgen->base + i,
-				  IRQ_NOREQUEST | IRQ_NOAUTOEN,
-				  IRQ_NOPROBE);
-	}
-	mutex_init(&iio_evgen->lock);
+	iio_event->dev = indio_dev;
+	iio_event->regs.reg_id = index;
+	mutex_lock(&iio_dummy_event_list_mutex);
+	list_add(&iio_event->l, &iio_dummy_event_list);
+	mutex_unlock(&iio_dummy_event_list_mutex);
+
 	return 0;
 }
+EXPORT_SYMBOL_GPL(iio_dummy_evgen_create);
 
-/**
- * iio_dummy_evgen_get_irq() - get an evgen provided irq for a device
- *
- * This function will give a free allocated irq to a client device.
- * That irq can then be caused to 'fire' by using the associated sysfs file.
- */
-int iio_dummy_evgen_get_irq(void)
+void iio_dummy_init_work_handler(int index, void (*f)(struct irq_work *))
 {
-	int i, ret = 0;
-
-	if (!iio_evgen)
-		return -ENODEV;
+	struct iio_dummy_event *iio_event = get_event(index);
 
-	mutex_lock(&iio_evgen->lock);
-	for (i = 0; i < IIO_EVENTGEN_NO; i++)
-		if (!iio_evgen->inuse[i]) {
-			ret = iio_evgen->base + i;
-			iio_evgen->inuse[i] = true;
-			break;
-		}
-	mutex_unlock(&iio_evgen->lock);
-	if (i == IIO_EVENTGEN_NO)
-		return -ENOMEM;
-	return ret;
+	if (iio_event)
+		init_irq_work(&iio_event->work, f);
 }
-EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_irq);
+EXPORT_SYMBOL_GPL(iio_dummy_init_work_handler);
 
-/**
- * iio_dummy_evgen_release_irq() - give the irq back.
- * @irq: irq being returned to the pool
- *
- * Used by client driver instances to give the irqs back when they disconnect
- */
-void iio_dummy_evgen_release_irq(int irq)
+struct iio_dummy_regs *iio_dummy_evgen_get_regs(int index)
 {
-	mutex_lock(&iio_evgen->lock);
-	iio_evgen->inuse[irq - iio_evgen->base] = false;
-	mutex_unlock(&iio_evgen->lock);
-}
-EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq);
+	struct iio_dummy_event *iio_event;
+	struct iio_dummy_regs *regs = NULL;
 
-struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq)
-{
-	return &iio_evgen->regs[irq - iio_evgen->base];
+	iio_event = get_event(index);
+	if (!iio_event)
+		goto err_regs;
+
+	regs = &iio_event->regs;
+err_regs:
+	return regs;
 }
 EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs);
 
-static void iio_dummy_evgen_free(void)
+void iio_dummy_evgen_free(int index)
 {
-	irq_free_descs(iio_evgen->base, IIO_EVENTGEN_NO);
-	kfree(iio_evgen);
+	struct list_head *ptr, *tmp;
+	struct iio_dummy_event *iio_event;
+
+	mutex_lock(&iio_dummy_event_list_mutex);
+	list_for_each_safe(ptr, tmp, &iio_dummy_event_list) {
+		iio_event = list_entry(ptr, struct iio_dummy_event, l);
+		if (iio_event->regs.reg_id == index) {
+			list_del(ptr);
+			kfree(iio_event);
+			break;
+		}
+	}
+	mutex_unlock(&iio_dummy_event_list_mutex);
 }
+EXPORT_SYMBOL_GPL(iio_dummy_evgen_free);
 
+/* Do nothing upon release */
 static void iio_evgen_release(struct device *dev)
 {
-	iio_dummy_evgen_free();
 }
 
 static ssize_t iio_evgen_poke(struct device *dev,
@@ -159,6 +114,7 @@ static ssize_t iio_evgen_poke(struct device *dev,
 			      size_t len)
 {
 	struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
+	struct iio_dummy_event *iio_event = get_event(this_attr->address);
 	unsigned long event;
 	int ret;
 
@@ -166,12 +122,11 @@ static ssize_t iio_evgen_poke(struct device *dev,
 	if (ret)
 		return ret;
 
-	iio_evgen->regs[this_attr->address].reg_id   = this_attr->address;
-	iio_evgen->regs[this_attr->address].reg_data = event;
-
-	if (iio_evgen->enabled[this_attr->address])
-		handle_nested_irq(iio_evgen->base + this_attr->address);
-
+	if (iio_event) {
+		iio_event->regs.reg_data = event;
+		/* trigger interrupt */
+		irq_work_queue(&iio_event->work);
+	}
 	return len;
 }
 
@@ -217,10 +172,6 @@ static struct device iio_evgen_dev = {
 
 static __init int iio_dummy_evgen_init(void)
 {
-	int ret = iio_dummy_evgen_create();
-
-	if (ret < 0)
-		return ret;
 	device_initialize(&iio_evgen_dev);
 	dev_set_name(&iio_evgen_dev, "iio_evgen");
 	return device_add(&iio_evgen_dev);
diff --git a/drivers/staging/iio/iio_dummy_evgen.h b/drivers/staging/iio/iio_dummy_evgen.h
index d044b94..b78b1eb 100644
--- a/drivers/staging/iio/iio_dummy_evgen.h
+++ b/drivers/staging/iio/iio_dummy_evgen.h
@@ -1,13 +1,22 @@
 #ifndef _IIO_DUMMY_EVGEN_H_
 #define _IIO_DUMMY_EVGEN_H_
+#include <linux/irq_work.h>
 
 struct iio_dummy_regs {
 	u32 reg_id;
 	u32 reg_data;
 };
 
-struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq);
-int iio_dummy_evgen_get_irq(void);
-void iio_dummy_evgen_release_irq(int irq);
+struct iio_dummy_event {
+	struct iio_dev *dev;
+	struct iio_dummy_regs regs;
+	struct irq_work work;
+	struct list_head l;
+};
+
+int iio_dummy_evgen_create(struct iio_dev *indio_dev, int index);
+void iio_dummy_init_work_handler(int index, void (*f)(struct irq_work *));
+struct iio_dummy_regs *iio_dummy_evgen_get_regs(int index);
+void iio_dummy_evgen_free(int index);
 
 #endif /* _IIO_DUMMY_EVGEN_H_ */
diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
index 381f90f..1ae082d 100644
--- a/drivers/staging/iio/iio_simple_dummy.c
+++ b/drivers/staging/iio/iio_simple_dummy.c
@@ -635,7 +635,7 @@ static int iio_dummy_probe(int index)
 	/* Specify that device provides sysfs type interfaces */
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	ret = iio_simple_dummy_events_register(indio_dev);
+	ret = iio_simple_dummy_events_register(indio_dev, index);
 	if (ret < 0)
 		goto error_free_device;
 
@@ -651,7 +651,7 @@ static int iio_dummy_probe(int index)
 error_unconfigure_buffer:
 	iio_simple_dummy_unconfigure_buffer(indio_dev);
 error_unregister_events:
-	iio_simple_dummy_events_unregister(indio_dev);
+	iio_simple_dummy_events_unregister(index);
 error_free_device:
 	iio_device_free(indio_dev);
 error_ret:
@@ -682,7 +682,7 @@ static void iio_dummy_remove(int index)
 	/* Buffered capture related cleanup */
 	iio_simple_dummy_unconfigure_buffer(indio_dev);
 
-	iio_simple_dummy_events_unregister(indio_dev);
+	iio_simple_dummy_events_unregister(index);
 
 	/* Free all structures */
 	iio_device_free(indio_dev);
diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
index 8d00224..05b84f7 100644
--- a/drivers/staging/iio/iio_simple_dummy.h
+++ b/drivers/staging/iio/iio_simple_dummy.h
@@ -78,19 +78,19 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
 				       enum iio_event_info info, int val,
 				       int val2);
 
-int iio_simple_dummy_events_register(struct iio_dev *indio_dev);
-void iio_simple_dummy_events_unregister(struct iio_dev *indio_dev);
+int iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index);
+void iio_simple_dummy_events_unregister(int index);
 
 #else /* Stubs for when events are disabled at compile time */
 
 static inline int
-iio_simple_dummy_events_register(struct iio_dev *indio_dev)
+iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index)
 {
 	return 0;
 };
 
 static inline void
-iio_simple_dummy_events_unregister(struct iio_dev *indio_dev)
+iio_simple_dummy_events_unregister(int index)
 { };
 
 #endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS*/
diff --git a/drivers/staging/iio/iio_simple_dummy_events.c b/drivers/staging/iio/iio_simple_dummy_events.c
index 73108ba..cf777b1 100644
--- a/drivers/staging/iio/iio_simple_dummy_events.c
+++ b/drivers/staging/iio/iio_simple_dummy_events.c
@@ -155,23 +155,25 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
 
 /**
  * iio_simple_dummy_event_handler() - identify and pass on event
- * @irq: irq of event line
- * @private: pointer to device instance state.
+ * @work:	struct irq_work which handles interrupt generation
  *
  * This handler is responsible for querying the device to find out what
  * event occurred and for then pushing that event towards userspace.
  * Here only one event occurs so we push that directly on with locally
  * grabbed timestamp.
  */
-static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
+void iio_simple_dummy_event_handler(struct irq_work *work)
 {
-	struct iio_dev *indio_dev = private;
+	struct iio_dummy_event *iio_event = container_of(work,
+							struct iio_dummy_event,
+							work);
+	struct iio_dev *indio_dev = iio_event->dev;
 	struct iio_dummy_state *st = iio_priv(indio_dev);
 
 	dev_dbg(&indio_dev->dev, "id %x event %x\n",
-		st->regs->reg_id, st->regs->reg_data);
+		iio_event->regs.reg_id, iio_event->regs.reg_data);
 
-	switch (st->regs->reg_data) {
+	switch (iio_event->regs.reg_data) {
 	case 0:
 		iio_push_event(indio_dev,
 			       IIO_EVENT_CODE(IIO_VOLTAGE, 0, 0,
@@ -209,59 +211,36 @@ static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
 	default:
 		break;
 	}
-
-	return IRQ_HANDLED;
 }
 
 /**
  * iio_simple_dummy_events_register() - setup interrupt handling for events
- * @indio_dev: device instance data
+ * @indio_dev:	device instance data
+ * @index:	integer which identifies the dummy instance
  *
- * This function requests the threaded interrupt to handle the events.
- * Normally the irq is a hardware interrupt and the number comes
- * from board configuration files.  Here we get it from a companion
- * module that fakes the interrupt for us. Note that module in
- * no way forms part of this example. Just assume that events magically
- * appear via the provided interrupt.
+ * Normally the irq is a hardware interrupt and the number comes from
+ * board configuration files. Here, the interrupt generation is handled
+ * by a companion module that will generate hardware interrupts using
+ * the irq_work API. Note that module in no way forms part of this example.
+ * Just assume that events magically appear via the provided interrupt.
  */
-int iio_simple_dummy_events_register(struct iio_dev *indio_dev)
+int iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index)
 {
-	struct iio_dummy_state *st = iio_priv(indio_dev);
 	int ret;
 
-	/* Fire up event source - normally not present */
-	st->event_irq = iio_dummy_evgen_get_irq();
-	if (st->event_irq < 0) {
-		ret = st->event_irq;
-		goto error_ret;
-	}
-	st->regs = iio_dummy_evgen_get_regs(st->event_irq);
-
-	ret = request_threaded_irq(st->event_irq,
-				   NULL,
-				   &iio_simple_dummy_event_handler,
-				   IRQF_ONESHOT,
-				   "iio_simple_event",
-				   indio_dev);
-	if (ret < 0)
-		goto error_free_evgen;
+	ret = iio_dummy_evgen_create(indio_dev, index);
+	if (ret)
+		return ret;
+	/* register handler */
+	iio_dummy_init_work_handler(index, iio_simple_dummy_event_handler);
 	return 0;
-
-error_free_evgen:
-	iio_dummy_evgen_release_irq(st->event_irq);
-error_ret:
-	return ret;
 }
 
 /**
- * iio_simple_dummy_events_unregister() - tidy up interrupt handling on remove
- * @indio_dev: device instance data
+ * iio_simple_dummy_events_unregister()
+ * @index:	device instance identification
  */
-void iio_simple_dummy_events_unregister(struct iio_dev *indio_dev)
+void iio_simple_dummy_events_unregister(int index)
 {
-	struct iio_dummy_state *st = iio_priv(indio_dev);
-
-	free_irq(st->event_irq, indio_dev);
-	/* Not part of normal driver */
-	iio_dummy_evgen_release_irq(st->event_irq);
+	 iio_dummy_evgen_free(index);
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1209262

FromCrt Mori <cmo@melexis.com>
Date2015-08-18 15:50 +0200
Message-ID<pYPr4-7RA-19@gated-at.bofh.it>
In reply to#1209240
Hi, few small comments inline, although I have problems applying the patch
Crt

On 18 August 2015 at 14:55, Cristina Opriceana
<cristina.opriceana@gmail.com> wrote:
> A more abstract way of handling interrupt generation in the dummy driver
> is represented by irq_work which substitutes the irq_chip, used to
> provide an IRQ line number. irq_work runs tasks from hardirq context,
> but in a NMI-safe environment and deals better with synchronization
> problems. An interrupt can be triggered by calling irq_work_queue()
> and the work will be performed by a pre-registered handler, which
> acts as a callback.
>
> Cc: Daniel Baluta <daniel.baluta@intel.com>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>
> Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
> ---
> This is the first draft for the irq_work replacement.
> As we want to move the iio dummy driver out of staging,
> we started to implement Jonathan's suggestions from here:
> <http://marc.info/?l=linux-iio&m=142403390705515&w=2>
>
>  drivers/staging/iio/iio_dummy_evgen.c         | 171 +++++++++-----------------
>  drivers/staging/iio/iio_dummy_evgen.h         |  15 ++-
>  drivers/staging/iio/iio_simple_dummy.c        |   6 +-
>  drivers/staging/iio/iio_simple_dummy.h        |   8 +-
>  drivers/staging/iio/iio_simple_dummy_events.c |  71 ++++-------
>  5 files changed, 105 insertions(+), 166 deletions(-)
>
> diff --git a/drivers/staging/iio/iio_dummy_evgen.c b/drivers/staging/iio/iio_dummy_evgen.c
> index 6d38854..974ef8a 100644
> --- a/drivers/staging/iio/iio_dummy_evgen.c
> +++ b/drivers/staging/iio/iio_dummy_evgen.c
> @@ -25,132 +25,87 @@
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>
> -/* Fiddly bit of faking and irq without hardware */
> -#define IIO_EVENTGEN_NO 10
> -/**
> - * struct iio_dummy_evgen - evgen state
> - * @chip: irq chip we are faking
> - * @base: base of irq range
> - * @enabled: mask of which irqs are enabled
> - * @inuse: mask of which irqs are connected
> - * @regs: irq regs we are faking
> - * @lock: protect the evgen state
> - */
> -struct iio_dummy_eventgen {
> -       struct irq_chip chip;
> -       int base;
> -       bool enabled[IIO_EVENTGEN_NO];
> -       bool inuse[IIO_EVENTGEN_NO];
> -       struct iio_dummy_regs regs[IIO_EVENTGEN_NO];
> -       struct mutex lock;
> -};
> +static LIST_HEAD(iio_dummy_event_list);
> +static DEFINE_MUTEX(iio_dummy_event_list_mutex);
>
> -/* We can only ever have one instance of this 'device' */
> -static struct iio_dummy_eventgen *iio_evgen;
> -static const char *iio_evgen_name = "iio_dummy_evgen";
> -
> -static void iio_dummy_event_irqmask(struct irq_data *d)
> +struct iio_dummy_event *get_event(int index)
>  {
> -       struct irq_chip *chip = irq_data_get_irq_chip(d);
> -       struct iio_dummy_eventgen *evgen =
> -               container_of(chip, struct iio_dummy_eventgen, chip);
> +       struct list_head *ptr, *tmp;
> +       struct iio_dummy_event *iio_event = NULL;
>
> -       evgen->enabled[d->irq - evgen->base] = false;
> -}
> -
> -static void iio_dummy_event_irqunmask(struct irq_data *d)
> -{
> -       struct irq_chip *chip = irq_data_get_irq_chip(d);
> -       struct iio_dummy_eventgen *evgen =
> -               container_of(chip, struct iio_dummy_eventgen, chip);
> -
> -       evgen->enabled[d->irq - evgen->base] = true;
> +       mutex_lock(&iio_dummy_event_list_mutex);
> +       list_for_each_safe(ptr, tmp, &iio_dummy_event_list) {
> +               iio_event = list_entry(ptr, struct iio_dummy_event, l);
> +               if (iio_event->regs.reg_id == index)
> +                       break;
> +       }
> +       mutex_unlock(&iio_dummy_event_list_mutex);
> +       return iio_event;
>  }
>
> -static int iio_dummy_evgen_create(void)
> +int iio_dummy_evgen_create(struct iio_dev *indio_dev, int index)
>  {
> -       int ret, i;
> +       struct iio_dummy_event *iio_event;
>
> -       iio_evgen = kzalloc(sizeof(*iio_evgen), GFP_KERNEL);
> -       if (!iio_evgen)
> +       iio_event = kzalloc(sizeof(*iio_event), GFP_KERNEL);
> +       if (!iio_event)
>                 return -ENOMEM;
>
> -       iio_evgen->base = irq_alloc_descs(-1, 0, IIO_EVENTGEN_NO, 0);
> -       if (iio_evgen->base < 0) {
> -               ret = iio_evgen->base;
> -               kfree(iio_evgen);
> -               return ret;
> -       }
> -       iio_evgen->chip.name = iio_evgen_name;
> -       iio_evgen->chip.irq_mask = &iio_dummy_event_irqmask;
> -       iio_evgen->chip.irq_unmask = &iio_dummy_event_irqunmask;
> -       for (i = 0; i < IIO_EVENTGEN_NO; i++) {
> -               irq_set_chip(iio_evgen->base + i, &iio_evgen->chip);
> -               irq_set_handler(iio_evgen->base + i, &handle_simple_irq);
> -               irq_modify_status(iio_evgen->base + i,
> -                                 IRQ_NOREQUEST | IRQ_NOAUTOEN,
> -                                 IRQ_NOPROBE);
> -       }
> -       mutex_init(&iio_evgen->lock);
> +       iio_event->dev = indio_dev;
> +       iio_event->regs.reg_id = index;
> +       mutex_lock(&iio_dummy_event_list_mutex);
> +       list_add(&iio_event->l, &iio_dummy_event_list);
> +       mutex_unlock(&iio_dummy_event_list_mutex);
> +
>         return 0;
>  }
> +EXPORT_SYMBOL_GPL(iio_dummy_evgen_create);
>
> -/**
> - * iio_dum - get an evgen provided irq for a device
> - *
> - * This function will give a free allocated irq to a client device.
> - * That irq can then be caused to 'fire' by using the associated sysfs file.
> - */
> -int iio_dummy_evgen_get_irq(void)
> +void iio_dummy_init_work_handler(int index, void (*f)(struct irq_work *))
>  {
> -       int i, ret = 0;
> -
> -       if (!iio_evgen)
> -               return -ENODEV;
> +       struct iio_dummy_event *iio_event = get_event(index);
>
> -       mutex_lock(&iio_evgen->lock);
> -       for (i = 0; i < IIO_EVENTGEN_NO; i++)
> -               if (!iio_evgen->inuse[i]) {
> -                       ret = iio_evgen->base + i;
> -                       iio_evgen->inuse[i] = true;
> -                       break;
> -               }
> -       mutex_unlock(&iio_evgen->lock);
> -       if (i == IIO_EVENTGEN_NO)
> -               return -ENOMEM;
> -       return ret;
> +       if (iio_event)
> +               init_irq_work(&iio_event->work, f);
>  }
> -EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_irq);
> +EXPORT_SYMBOL_GPL(iio_dummy_init_work_handler);
>
> -/**
> - * iio_dummy_evgen_release_irq() - give the irq back.
> - * @irq: irq being returned to the pool
> - *
> - * Used by client driver instances to give the irqs back when they disconnect
> - */
> -void iio_dummy_evgen_release_irq(int irq)
> +struct iio_dummy_regs *iio_dummy_evgen_get_regs(int index)
>  {
> -       mutex_lock(&iio_evgen->lock);
> -       iio_evgen->inuse[irq - iio_evgen->base] = false;
> -       mutex_unlock(&iio_evgen->lock);
> -}
> -EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq);
> +       struct iio_dummy_event *iio_event;
> +       struct iio_dummy_regs *regs = NULL;
>
> -struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq)
> -{
> -       return &iio_evgen->regs[irq - iio_evgen->base];
> +       iio_event = get_event(index);
> +       if (!iio_event)
> +               goto err_regs;
This here is for future probably? - otherwise we can just return
> +
> +       regs = &iio_event->regs;
> +err_regs:
> +       return regs;
>  }
>  EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs);
>
> -static void iio_dummy_evgen_free(void)
> +void iio_dummy_evgen_free(int index)
>  {
> -       irq_free_descs(iio_evgen->base, IIO_EVENTGEN_NO);
> -       kfree(iio_evgen);
> +       struct list_head *ptr, *tmp;
> +       struct iio_dummy_event *iio_event;
> +
> +       mutex_lock(&iio_dummy_event_list_mutex);
> +       list_for_each_safe(ptr, tmp, &iio_dummy_event_list) {
> +               iio_event = list_entry(ptr, struct iio_dummy_event, l);
> +               if (iio_event->regs.reg_id == index) {
> +                       list_del(ptr);
> +                       kfree(iio_event);
> +                       break;
> +               }
> +       }
> +       mutex_unlock(&iio_dummy_event_list_mutex);
>  }
> +EXPORT_SYMBOL_GPL(iio_dummy_evgen_free);
>
> +/* Do nothing upon release */
>  static void iio_evgen_release(struct device *dev)
>  {
> -       iio_dummy_evgen_free();
>  }
>
>  static ssize_t iio_evgen_poke(struct device *dev,
> @@ -159,6 +114,7 @@ static ssize_t iio_evgen_poke(struct device *dev,
>                               size_t len)
>  {
>         struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
> +       struct iio_dummy_event *iio_event = get_event(this_attr->address);
>         unsigned long event;
>         int ret;
>
> @@ -166,12 +122,11 @@ static ssize_t iio_evgen_poke(struct device *dev,
>         if (ret)
>                 return ret;
>
> -       iio_evgen->regs[this_attr->address].reg_id   = this_attr->address;
> -       iio_evgen->regs[this_attr->address].reg_data = event;
> -
> -       if (iio_evgen->enabled[this_attr->address])
> -               handle_nested_irq(iio_evgen->base + this_attr->address);
> -
> +       if (iio_event) {
> +               iio_event->regs.reg_data = event;
> +               /* trigger interrupt */
> +               irq_work_queue(&iio_event->work);
> +       }
iio_event is never checked- else return -ENODEV probably here?
>         return len;
>  }
>
> @@ -217,10 +172,6 @@ static struct device iio_evgen_dev = {
>
>  static __init int iio_dummy_evgen_init(void)
>  {
> -       int ret = iio_dummy_evgen_create();
> -
> -       if (ret < 0)
> -               return ret;
>         device_initialize(&iio_evgen_dev);
>         dev_set_name(&iio_evgen_dev, "iio_evgen");
>         return device_add(&iio_evgen_dev);
> diff --git a/drivers/staging/iio/iio_dummy_evgen.h b/drivers/staging/iio/iio_dummy_evgen.h
> index d044b94..b78b1eb 100644
> --- a/drivers/staging/iio/iio_dummy_evgen.h
> +++ b/drivers/staging/iio/iio_dummy_evgen.h
> @@ -1,13 +1,22 @@
>  #ifndef _IIO_DUMMY_EVGEN_H_
>  #define _IIO_DUMMY_EVGEN_H_
> +#include <linux/irq_work.h>
>
>  struct iio_dummy_regs {
>         u32 reg_id;
>         u32 reg_data;
>  };
>
> -struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq);
> -int iio_dummy_evgen_get_irq(void);
> -void iio_dummy_evgen_release_irq(int irq);
> +struct iio_dummy_event {
> +       struct iio_dev *dev;
> +       struct iio_dummy_regs regs;
> +       struct irq_work work;
> +       struct list_head l;
> +};
> +
> +int iio_dummy_evgen_create(struct iio_dev *indio_dev, int index);
> +void iio_dummy_init_work_handler(int index, void (*f)(struct irq_work *));
> +struct iio_dummy_regs *iio_dummy_evgen_get_regs(int index);
> +void iio_dummy_evgen_free(int index);
>
>  #endif /* _IIO_DUMMY_EVGEN_H_ */
> diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
> index 381f90f..1ae082d 100644
> --- a/drivers/staging/iio/iio_simple_dummy.c
> +++ b/drivers/staging/iio/iio_simple_dummy.c
> @@ -635,7 +635,7 @@ static int iio_dummy_probe(int index)
>         /* Specify that device provides sysfs type interfaces */
>         indio_dev->modes = INDIO_DIRECT_MODE;
>
> -       ret = iio_simple_dummy_events_register(indio_dev);
> +       ret = iio_simple_dummy_events_register(indio_dev, index);
>         if (ret < 0)
>                 goto error_free_device;
>
> @@ -651,7 +651,7 @@ static int iio_dummy_probe(int index)
>  error_unconfigure_buffer:
>         iio_simple_dummy_unconfigure_buffer(indio_dev);
>  error_unregister_events:
> -       iio_simple_dummy_events_unregister(indio_dev);
> +       iio_simple_dummy_events_unregister(index);
>  error_free_device:
>         iio_device_free(indio_dev);
>  error_ret:
> @@ -682,7 +682,7 @@ static void iio_dummy_remove(int index)
>         /* Buffered capture related cleanup */
>         iio_simple_dummy_unconfigure_buffer(indio_dev);
>
> -       iio_simple_dummy_events_unregister(indio_dev);
> +       iio_simple_dummy_events_unregister(index);
>
>         /* Free all structures */
>         iio_device_free(indio_dev);
> diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
> index 8d00224..05b84f7 100644
> --- a/drivers/staging/iio/iio_simple_dummy.h
> +++ b/drivers/staging/iio/iio_simple_dummy.h
> @@ -78,19 +78,19 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
>                                        enum iio_event_info info, int val,
>                                        int val2);
>
> -int iio_simple_dummy_events_register(struct iio_dev *indio_dev);
> -void iio_simple_dummy_events_unregister(struct iio_dev *indio_dev);
> +int iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index);
> +void iio_simple_dummy_events_unregister(int index);
>
>  #else /* Stubs for when events are disabled at compile time */
>
>  static inline int
> -iio_simple_dummy_events_register(struct iio_dev *indio_dev)
> +iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index)
>  {
>         return 0;
>  };
>
>  static inline void
> -iio_simple_dummy_events_unregister(struct iio_dev *indio_dev)
> +iio_simple_dummy_events_unregister(int index)
>  { };
>
>  #endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS*/
> diff --git a/drivers/staging/iio/iio_simple_dummy_events.c b/drivers/staging/iio/iio_simple_dummy_events.c
> index 73108ba..cf777b1 100644
> --- a/drivers/staging/iio/iio_simple_dummy_events.c
> +++ b/drivers/staging/iio/iio_simple_dummy_events.c
> @@ -155,23 +155,25 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
>
>  /**
>   * iio_simple_dummy_event_handler() - identify and pass on event
> - * @irq: irq of event line
> - * @private: pointer to device instance state.
> + * @work:      struct irq_work which handles interrupt generation
>   *
>   * This handler is responsible for querying the device to find out what
>   * event occurred and for then pushing that event towards userspace.
>   * Here only one event occurs so we push that directly on with locally
>   * grabbed timestamp.
>   */
> -static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
> +void iio_simple_dummy_event_handler(struct irq_work *work)
>  {
> -       struct iio_dev *indio_dev = private;
> +       struct iio_dummy_event *iio_event = container_of(work,
> +                                                       struct iio_dummy_event,
> +                                                       work);
> +       struct iio_dev *indio_dev = iio_event->dev;
>         struct iio_dummy_state *st = iio_priv(indio_dev);
>
>         dev_dbg(&indio_dev->dev, "id %x event %x\n",
> -               st->regs->reg_id, st->regs->reg_data);
> +               iio_event->regs.reg_id, iio_event->regs.reg_data);
>
> -       switch (st->regs->reg_data) {
> +       switch (iio_event->regs.reg_data) {
>         case 0:
>                 iio_push_event(indio_dev,
>                                IIO_EVENT_CODE(IIO_VOLTAGE, 0, 0,
> @@ -209,59 +211,36 @@ static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
>         default:
>                 break;
>         }
> -
> -       return IRQ_HANDLED;
>  }
>
>  /**
>   * iio_simple_dummy_events_register() - setup interrupt handling for events
> - * @indio_dev: device instance data
> + * @indio_dev: device instance data
> + * @index:     integer which identifies the dummy instance
>   *
> - * This function requests the threaded interrupt to handle the events.
> - * Normally the irq is a hardware interrupt and the number comes
> - * from board configuration files.  Here we get it from a companion
> - * module that fakes the interrupt for us. Note that module in
> - * no way forms part of this example. Just assume that events magically
> - * appear via the provided interrupt.
> + * Normally the irq is a hardware interrupt and the number comes from
> + * board configuration files. Here, the interrupt generation is handled
> + * by a companion module that will generate hardware interrupts using
> + * the irq_work API. Note that module in no way forms part of this example.
> + * Just assume that events magically appear via the provided interrupt.
>   */
> -int iio_simple_dummy_events_register(struct iio_dev *indio_dev)
> +int iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index)
>  {
> -       struct iio_dummy_state *st = iio_priv(indio_dev);
>         int ret;
>
> -       /* Fire up event source - normally not present */
> -       st->event_irq = iio_dummy_evgen_get_irq();
> -       if (st->event_irq < 0) {
> -               ret = st->event_irq;
> -               goto error_ret;
> -       }
> -       st->regs = iio_dummy_evgen_get_regs(st->event_irq);
> -
> -       ret = request_threaded_irq(st->event_irq,
> -                                  NULL,
> -                                  &iio_simple_dummy_event_handler,
> -                                  IRQF_ONESHOT,
> -                                  "iio_simple_event",
> -                                  indio_dev);
> -       if (ret < 0)
> -               goto error_free_evgen;
> +       ret = iio_dummy_evgen_create(indio_dev, index);
> +       if (ret)
> +               return ret;
> +       /* register handler */
> +       iio_dummy_init_work_handler(index, iio_simple_dummy_event_handler);
>         return 0;
> -
> -error_free_evgen:
> -       iio_dummy_evgen_release_irq(st->event_irq);
> -error_ret:
> -       return ret;
>  }
>
>  /**
> - * iio_simple_dummy_events_unregister() - tidy up interrupt handling on remove
> - * @indio_dev: device instance data
> + * iio_simple_dummy_events_unregister()
> + * @index:     device instance identification
>   */
> -void iio_simple_dummy_events_unregister(struct iio_dev *indio_dev)
> +void iio_simple_dummy_events_unregister(int index)
>  {
> -       struct iio_dummy_state *st = iio_priv(indio_dev);
> -
> -       free_irq(st->event_irq, indio_dev);
> -       /* Not part of normal driver */
> -       iio_dummy_evgen_release_irq(st->event_irq);
> +        iio_dummy_evgen_free(index);
>  }
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1209277

FromCristina Georgiana Opriceana <cristina.opriceana@gmail.com>
Date2015-08-18 16:10 +0200
Message-ID<pYPKq-8vl-13@gated-at.bofh.it>
In reply to#1209262
On Tue, Aug 18, 2015 at 4:44 PM, Crt Mori <cmo@melexis.com> wrote:
> Hi, few small comments inline, although I have problems applying the patch

I've just reset my branch against origin/testing and it applies fine for me.

> Crt
>
> On 18 August 2015 at 14:55, Cristina Opriceana
> <cristina.opriceana@gmail.com> wrote:
>> A more abstract way of handling interrupt generation in the dummy driver
>> is represented by irq_work which substitutes the irq_chip, used to
>> provide an IRQ line number. irq_work runs tasks from hardirq context,
>> but in a NMI-safe environment and deals better with synchronization
>> problems. An interrupt can be triggered by calling irq_work_queue()
>> and the work will be performed by a pre-registered handler, which
>> acts as a callback.
>>
>> Cc: Daniel Baluta <daniel.baluta@intel.com>
>> Suggested-by: Jonathan Cameron <jic23@kernel.org>
>> Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
>> ---
>> This is the first draft for the irq_work replacement.
>> As we want to move the iio dummy driver out of staging,
>> we started to implement Jonathan's suggestions from here:
>> <http://marc.info/?l=linux-iio&m=142403390705515&w=2>
>>
>>  drivers/staging/iio/iio_dummy_evgen.c         | 171 +++++++++-----------------
>>  drivers/staging/iio/iio_dummy_evgen.h         |  15 ++-
>>  drivers/staging/iio/iio_simple_dummy.c        |   6 +-
>>  drivers/staging/iio/iio_simple_dummy.h        |   8 +-
>>  drivers/staging/iio/iio_simple_dummy_events.c |  71 ++++-------
>>  5 files changed, 105 insertions(+), 166 deletions(-)
>>
>> diff --git a/drivers/staging/iio/iio_dummy_evgen.c b/drivers/staging/iio/iio_dummy_evgen.c
>> index 6d38854..974ef8a 100644
>> --- a/drivers/staging/iio/iio_dummy_evgen.c
>> +++ b/drivers/staging/iio/iio_dummy_evgen.c
>> @@ -25,132 +25,87 @@
>>  #include <linux/iio/iio.h>
>>  #include <linux/iio/sysfs.h>
>>
>> -/* Fiddly bit of faking and irq without hardware */
>> -#define IIO_EVENTGEN_NO 10
>> -/**
>> - * struct iio_dummy_evgen - evgen state
>> - * @chip: irq chip we are faking
>> - * @base: base of irq range
>> - * @enabled: mask of which irqs are enabled
>> - * @inuse: mask of which irqs are connected
>> - * @regs: irq regs we are faking
>> - * @lock: protect the evgen state
>> - */
>> -struct iio_dummy_eventgen {
>> -       struct irq_chip chip;
>> -       int base;
>> -       bool enabled[IIO_EVENTGEN_NO];
>> -       bool inuse[IIO_EVENTGEN_NO];
>> -       struct iio_dummy_regs regs[IIO_EVENTGEN_NO];
>> -       struct mutex lock;
>> -};
>> +static LIST_HEAD(iio_dummy_event_list);
>> +static DEFINE_MUTEX(iio_dummy_event_list_mutex);
>>
>> -/* We can only ever have one instance of this 'device' */
>> -static struct iio_dummy_eventgen *iio_evgen;
>> -static const char *iio_evgen_name = "iio_dummy_evgen";
>> -
>> -static void iio_dummy_event_irqmask(struct irq_data *d)
>> +struct iio_dummy_event *get_event(int index)
>>  {
>> -       struct irq_chip *chip = irq_data_get_irq_chip(d);
>> -       struct iio_dummy_eventgen *evgen =
>> -               container_of(chip, struct iio_dummy_eventgen, chip);
>> +       struct list_head *ptr, *tmp;
>> +       struct iio_dummy_event *iio_event = NULL;
>>
>> -       evgen->enabled[d->irq - evgen->base] = false;
>> -}
>> -
>> -static void iio_dummy_event_irqunmask(struct irq_data *d)
>> -{
>> -       struct irq_chip *chip = irq_data_get_irq_chip(d);
>> -       struct iio_dummy_eventgen *evgen =
>> -               container_of(chip, struct iio_dummy_eventgen, chip);
>> -
>> -       evgen->enabled[d->irq - evgen->base] = true;
>> +       mutex_lock(&iio_dummy_event_list_mutex);
>> +       list_for_each_safe(ptr, tmp, &iio_dummy_event_list) {
>> +               iio_event = list_entry(ptr, struct iio_dummy_event, l);
>> +               if (iio_event->regs.reg_id == index)
>> +                       break;
>> +       }
>> +       mutex_unlock(&iio_dummy_event_list_mutex);
>> +       return iio_event;
>>  }
>>
>> -static int iio_dummy_evgen_create(void)
>> +int iio_dummy_evgen_create(struct iio_dev *indio_dev, int index)
>>  {
>> -       int ret, i;
>> +       struct iio_dummy_event *iio_event;
>>
>> -       iio_evgen = kzalloc(sizeof(*iio_evgen), GFP_KERNEL);
>> -       if (!iio_evgen)
>> +       iio_event = kzalloc(sizeof(*iio_event), GFP_KERNEL);
>> +       if (!iio_event)
>>                 return -ENOMEM;
>>
>> -       iio_evgen->base = irq_alloc_descs(-1, 0, IIO_EVENTGEN_NO, 0);
>> -       if (iio_evgen->base < 0) {
>> -               ret = iio_evgen->base;
>> -               kfree(iio_evgen);
>> -               return ret;
>> -       }
>> -       iio_evgen->chip.name = iio_evgen_name;
>> -       iio_evgen->chip.irq_mask = &iio_dummy_event_irqmask;
>> -       iio_evgen->chip.irq_unmask = &iio_dummy_event_irqunmask;
>> -       for (i = 0; i < IIO_EVENTGEN_NO; i++) {
>> -               irq_set_chip(iio_evgen->base + i, &iio_evgen->chip);
>> -               irq_set_handler(iio_evgen->base + i, &handle_simple_irq);
>> -               irq_modify_status(iio_evgen->base + i,
>> -                                 IRQ_NOREQUEST | IRQ_NOAUTOEN,
>> -                                 IRQ_NOPROBE);
>> -       }
>> -       mutex_init(&iio_evgen->lock);
>> +       iio_event->dev = indio_dev;
>> +       iio_event->regs.reg_id = index;
>> +       mutex_lock(&iio_dummy_event_list_mutex);
>> +       list_add(&iio_event->l, &iio_dummy_event_list);
>> +       mutex_unlock(&iio_dummy_event_list_mutex);
>> +
>>         return 0;
>>  }
>> +EXPORT_SYMBOL_GPL(iio_dummy_evgen_create);
>>
>> -/**
>> - * iio_dum - get an evgen provided irq for a device
>> - *
>> - * This function will give a free allocated irq to a client device.
>> - * That irq can then be caused to 'fire' by using the associated sysfs file.
>> - */
>> -int iio_dummy_evgen_get_irq(void)
>> +void iio_dummy_init_work_handler(int index, void (*f)(struct irq_work *))
>>  {
>> -       int i, ret = 0;
>> -
>> -       if (!iio_evgen)
>> -               return -ENODEV;
>> +       struct iio_dummy_event *iio_event = get_event(index);
>>
>> -       mutex_lock(&iio_evgen->lock);
>> -       for (i = 0; i < IIO_EVENTGEN_NO; i++)
>> -               if (!iio_evgen->inuse[i]) {
>> -                       ret = iio_evgen->base + i;
>> -                       iio_evgen->inuse[i] = true;
>> -                       break;
>> -               }
>> -       mutex_unlock(&iio_evgen->lock);
>> -       if (i == IIO_EVENTGEN_NO)
>> -               return -ENOMEM;
>> -       return ret;
>> +       if (iio_event)
>> +               init_irq_work(&iio_event->work, f);
>>  }
>> -EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_irq);
>> +EXPORT_SYMBOL_GPL(iio_dummy_init_work_handler);
>>
>> -/**
>> - * iio_dummy_evgen_release_irq() - give the irq back.
>> - * @irq: irq being returned to the pool
>> - *
>> - * Used by client driver instances to give the irqs back when they disconnect
>> - */
>> -void iio_dummy_evgen_release_irq(int irq)
>> +struct iio_dummy_regs *iio_dummy_evgen_get_regs(int index)
>>  {
>> -       mutex_lock(&iio_evgen->lock);
>> -       iio_evgen->inuse[irq - iio_evgen->base] = false;
>> -       mutex_unlock(&iio_evgen->lock);
>> -}
>> -EXPORT_SYMBOL_GPL(iio_dummy_evgen_release_irq);
>> +       struct iio_dummy_event *iio_event;
>> +       struct iio_dummy_regs *regs = NULL;
>>
>> -struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq)
>> -{
>> -       return &iio_evgen->regs[irq - iio_evgen->base];
>> +       iio_event = get_event(index);
>> +       if (!iio_event)
>> +               goto err_regs;
> This here is for future probably? - otherwise we can just return
>> +
>> +       regs = &iio_event->regs;
>> +err_regs:
>> +       return regs;
>>  }
>>  EXPORT_SYMBOL_GPL(iio_dummy_evgen_get_regs);
>>
>> -static void iio_dummy_evgen_free(void)
>> +void iio_dummy_evgen_free(int index)
>>  {
>> -       irq_free_descs(iio_evgen->base, IIO_EVENTGEN_NO);
>> -       kfree(iio_evgen);
>> +       struct list_head *ptr, *tmp;
>> +       struct iio_dummy_event *iio_event;
>> +
>> +       mutex_lock(&iio_dummy_event_list_mutex);
>> +       list_for_each_safe(ptr, tmp, &iio_dummy_event_list) {
>> +               iio_event = list_entry(ptr, struct iio_dummy_event, l);
>> +               if (iio_event->regs.reg_id == index) {
>> +                       list_del(ptr);
>> +                       kfree(iio_event);
>> +                       break;
>> +               }
>> +       }
>> +       mutex_unlock(&iio_dummy_event_list_mutex);
>>  }
>> +EXPORT_SYMBOL_GPL(iio_dummy_evgen_free);
>>
>> +/* Do nothing upon release */
>>  static void iio_evgen_release(struct device *dev)
>>  {
>> -       iio_dummy_evgen_free();
>>  }
>>
>>  static ssize_t iio_evgen_poke(struct device *dev,
>> @@ -159,6 +114,7 @@ static ssize_t iio_evgen_poke(struct device *dev,
>>                               size_t len)
>>  {
>>         struct iio_dev_attr *this_attr = to_iio_dev_attr(attr);
>> +       struct iio_dummy_event *iio_event = get_event(this_attr->address);
>>         unsigned long event;
>>         int ret;
>>
>> @@ -166,12 +122,11 @@ static ssize_t iio_evgen_poke(struct device *dev,
>>         if (ret)
>>                 return ret;
>>
>> -       iio_evgen->regs[this_attr->address].reg_id   = this_attr->address;
>> -       iio_evgen->regs[this_attr->address].reg_data = event;
>> -
>> -       if (iio_evgen->enabled[this_attr->address])
>> -               handle_nested_irq(iio_evgen->base + this_attr->address);
>> -
>> +       if (iio_event) {
>> +               iio_event->regs.reg_data = event;
>> +               /* trigger interrupt */
>> +               irq_work_queue(&iio_event->work);
>> +       }
> iio_event is never checked- else return -ENODEV probably here?

I left it like that to follow the return logic in the original driver.
Also, the reading of event would be successful even if the interrupt
generation can't be triggered.

>>         return len;
>>  }
>>
>> @@ -217,10 +172,6 @@ static struct device iio_evgen_dev = {
>>
>>  static __init int iio_dummy_evgen_init(void)
>>  {
>> -       int ret = iio_dummy_evgen_create();
>> -
>> -       if (ret < 0)
>> -               return ret;
>>         device_initialize(&iio_evgen_dev);
>>         dev_set_name(&iio_evgen_dev, "iio_evgen");
>>         return device_add(&iio_evgen_dev);
>> diff --git a/drivers/staging/iio/iio_dummy_evgen.h b/drivers/staging/iio/iio_dummy_evgen.h
>> index d044b94..b78b1eb 100644
>> --- a/drivers/staging/iio/iio_dummy_evgen.h
>> +++ b/drivers/staging/iio/iio_dummy_evgen.h
>> @@ -1,13 +1,22 @@
>>  #ifndef _IIO_DUMMY_EVGEN_H_
>>  #define _IIO_DUMMY_EVGEN_H_
>> +#include <linux/irq_work.h>
>>
>>  struct iio_dummy_regs {
>>         u32 reg_id;
>>         u32 reg_data;
>>  };
>>
>> -struct iio_dummy_regs *iio_dummy_evgen_get_regs(int irq);
>> -int iio_dummy_evgen_get_irq(void);
>> -void iio_dummy_evgen_release_irq(int irq);
>> +struct iio_dummy_event {
>> +       struct iio_dev *dev;
>> +       struct iio_dummy_regs regs;
>> +       struct irq_work work;
>> +       struct list_head l;
>> +};
>> +
>> +int iio_dummy_evgen_create(struct iio_dev *indio_dev, int index);
>> +void iio_dummy_init_work_handler(int index, void (*f)(struct irq_work *));
>> +struct iio_dummy_regs *iio_dummy_evgen_get_regs(int index);
>> +void iio_dummy_evgen_free(int index);
>>
>>  #endif /* _IIO_DUMMY_EVGEN_H_ */
>> diff --git a/drivers/staging/iio/iio_simple_dummy.c b/drivers/staging/iio/iio_simple_dummy.c
>> index 381f90f..1ae082d 100644
>> --- a/drivers/staging/iio/iio_simple_dummy.c
>> +++ b/drivers/staging/iio/iio_simple_dummy.c
>> @@ -635,7 +635,7 @@ static int iio_dummy_probe(int index)
>>         /* Specify that device provides sysfs type interfaces */
>>         indio_dev->modes = INDIO_DIRECT_MODE;
>>
>> -       ret = iio_simple_dummy_events_register(indio_dev);
>> +       ret = iio_simple_dummy_events_register(indio_dev, index);
>>         if (ret < 0)
>>                 goto error_free_device;
>>
>> @@ -651,7 +651,7 @@ static int iio_dummy_probe(int index)
>>  error_unconfigure_buffer:
>>         iio_simple_dummy_unconfigure_buffer(indio_dev);
>>  error_unregister_events:
>> -       iio_simple_dummy_events_unregister(indio_dev);
>> +       iio_simple_dummy_events_unregister(index);
>>  error_free_device:
>>         iio_device_free(indio_dev);
>>  error_ret:
>> @@ -682,7 +682,7 @@ static void iio_dummy_remove(int index)
>>         /* Buffered capture related cleanup */
>>         iio_simple_dummy_unconfigure_buffer(indio_dev);
>>
>> -       iio_simple_dummy_events_unregister(indio_dev);
>> +       iio_simple_dummy_events_unregister(index);
>>
>>         /* Free all structures */
>>         iio_device_free(indio_dev);
>> diff --git a/drivers/staging/iio/iio_simple_dummy.h b/drivers/staging/iio/iio_simple_dummy.h
>> index 8d00224..05b84f7 100644
>> --- a/drivers/staging/iio/iio_simple_dummy.h
>> +++ b/drivers/staging/iio/iio_simple_dummy.h
>> @@ -78,19 +78,19 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
>>                                        enum iio_event_info info, int val,
>>                                        int val2);
>>
>> -int iio_simple_dummy_events_register(struct iio_dev *indio_dev);
>> -void iio_simple_dummy_events_unregister(struct iio_dev *indio_dev);
>> +int iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index);
>> +void iio_simple_dummy_events_unregister(int index);
>>
>>  #else /* Stubs for when events are disabled at compile time */
>>
>>  static inline int
>> -iio_simple_dummy_events_register(struct iio_dev *indio_dev)
>> +iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index)
>>  {
>>         return 0;
>>  };
>>
>>  static inline void
>> -iio_simple_dummy_events_unregister(struct iio_dev *indio_dev)
>> +iio_simple_dummy_events_unregister(int index)
>>  { };
>>
>>  #endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS*/
>> diff --git a/drivers/staging/iio/iio_simple_dummy_events.c b/drivers/staging/iio/iio_simple_dummy_events.c
>> index 73108ba..cf777b1 100644
>> --- a/drivers/staging/iio/iio_simple_dummy_events.c
>> +++ b/drivers/staging/iio/iio_simple_dummy_events.c
>> @@ -155,23 +155,25 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
>>
>>  /**
>>   * iio_simple_dummy_event_handler() - identify and pass on event
>> - * @irq: irq of event line
>> - * @private: pointer to device instance state.
>> + * @work:      struct irq_work which handles interrupt generation
>>   *
>>   * This handler is responsible for querying the device to find out what
>>   * event occurred and for then pushing that event towards userspace.
>>   * Here only one event occurs so we push that directly on with locally
>>   * grabbed timestamp.
>>   */
>> -static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
>> +void iio_simple_dummy_event_handler(struct irq_work *work)
>>  {
>> -       struct iio_dev *indio_dev = private;
>> +       struct iio_dummy_event *iio_event = container_of(work,
>> +                                                       struct iio_dummy_event,
>> +                                                       work);
>> +       struct iio_dev *indio_dev = iio_event->dev;
>>         struct iio_dummy_state *st = iio_priv(indio_dev);
>>
>>         dev_dbg(&indio_dev->dev, "id %x event %x\n",
>> -               st->regs->reg_id, st->regs->reg_data);
>> +               iio_event->regs.reg_id, iio_event->regs.reg_data);
>>
>> -       switch (st->regs->reg_data) {
>> +       switch (iio_event->regs.reg_data) {
>>         case 0:
>>                 iio_push_event(indio_dev,
>>                                IIO_EVENT_CODE(IIO_VOLTAGE, 0, 0,
>> @@ -209,59 +211,36 @@ static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
>>         default:
>>                 break;
>>         }
>> -
>> -       return IRQ_HANDLED;
>>  }
>>
>>  /**
>>   * iio_simple_dummy_events_register() - setup interrupt handling for events
>> - * @indio_dev: device instance data
>> + * @indio_dev: device instance data
>> + * @index:     integer which identifies the dummy instance
>>   *
>> - * This function requests the threaded interrupt to handle the events.
>> - * Normally the irq is a hardware interrupt and the number comes
>> - * from board configuration files.  Here we get it from a companion
>> - * module that fakes the interrupt for us. Note that module in
>> - * no way forms part of this example. Just assume that events magically
>> - * appear via the provided interrupt.
>> + * Normally the irq is a hardware interrupt and the number comes from
>> + * board configuration files. Here, the interrupt generation is handled
>> + * by a companion module that will generate hardware interrupts using
>> + * the irq_work API. Note that module in no way forms part of this example.
>> + * Just assume that events magically appear via the provided interrupt.
>>   */
>> -int iio_simple_dummy_events_register(struct iio_dev *indio_dev)
>> +int iio_simple_dummy_events_register(struct iio_dev *indio_dev, int index)
>>  {
>> -       struct iio_dummy_state *st = iio_priv(indio_dev);
>>         int ret;
>>
>> -       /* Fire up event source - normally not present */
>> -       st->event_irq = iio_dummy_evgen_get_irq();
>> -       if (st->event_irq < 0) {
>> -               ret = st->event_irq;
>> -               goto error_ret;
>> -       }
>> -       st->regs = iio_dummy_evgen_get_regs(st->event_irq);
>> -
>> -       ret = request_threaded_irq(st->event_irq,
>> -                                  NULL,
>> -                                  &iio_simple_dummy_event_handler,
>> -                                  IRQF_ONESHOT,
>> -                                  "iio_simple_event",
>> -                                  indio_dev);
>> -       if (ret < 0)
>> -               goto error_free_evgen;
>> +       ret = iio_dummy_evgen_create(indio_dev, index);
>> +       if (ret)
>> +               return ret;
>> +       /* register handler */
>> +       iio_dummy_init_work_handler(index, iio_simple_dummy_event_handler);
>>         return 0;
>> -
>> -error_free_evgen:
>> -       iio_dummy_evgen_release_irq(st->event_irq);
>> -error_ret:
>> -       return ret;
>>  }
>>
>>  /**
>> - * iio_simple_dummy_events_unregister() - tidy up interrupt handling on remove
>> - * @indio_dev: device instance data
>> + * iio_simple_dummy_events_unregister()
>> + * @index:     device instance identification
>>   */
>> -void iio_simple_dummy_events_unregister(struct iio_dev *indio_dev)
>> +void iio_simple_dummy_events_unregister(int index)
>>  {
>> -       struct iio_dummy_state *st = iio_priv(indio_dev);
>> -
>> -       free_irq(st->event_irq, indio_dev);
>> -       /* Not part of normal driver */
>> -       iio_dummy_evgen_release_irq(st->event_irq);
>> +        iio_dummy_evgen_free(index);
>>  }
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web