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

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 <[email protected]>
Suggested-by: Jonathan Cameron <[email protected]>
Signed-off-by: Cristina Opriceana <[email protected]>
---
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


2015-08-18 13:45:28

by Crt Mori

[permalink] [raw]
Subject: Re: [PATCH] Staging: iio: Move evgen interrupt generation to irq_work

Hi, few small comments inline, although I have problems applying the patch
Crt

On 18 August 2015 at 14:55, Cristina Opriceana
<[email protected]> 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 <[email protected]>
> Suggested-by: Jonathan Cameron <[email protected]>
> Signed-off-by: Cristina Opriceana <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

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

On Tue, Aug 18, 2015 at 4:44 PM, Crt Mori <[email protected]> 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
> <[email protected]> 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 <[email protected]>
>> Suggested-by: Jonathan Cameron <[email protected]>
>> Signed-off-by: Cristina Opriceana <[email protected]>
>> ---
>> 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 [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2015-08-31 15:43:31

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH] Staging: iio: Move evgen interrupt generation to irq_work

On 18/08/15 13:55, Cristina Opriceana 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 <[email protected]>
> Suggested-by: Jonathan Cameron <[email protected]>
> Signed-off-by: Cristina Opriceana <[email protected]>
Hi Cristina,

Sorry for the slow response on this!

Anyhow, my main thought here is whether we can take advantage of the irq
work approach to allow a true simulation of an irq chip rather than
using the nested_irq as in the original.

The suggestion in that original email was just to move from the 'naughty'
nested_irq call over to irq work, I wasn't suggesting dropping the irq chip.

The aim of this is to allow the dummy driver to fully support
anything that can be done in a normal driver, without it being in anyway
modified in form from that of a normal hardware driver.

The result of the change here is to move the actual example code
futher from a normal driver rather than nearer to it...

Sorry for the confusion on this,

Jonathan
> ---
> 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,
It's this null that I never liked. The one thing you 'always'
do on an event is grab a timestamp in the top half handler. With
the nested irq handler we could never do that (as not in irq context).

> - &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);
> }
>