Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752826AbbHRMz1 (ORCPT ); Tue, 18 Aug 2015 08:55:27 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:37343 "EHLO mail-wi0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751387AbbHRMzZ (ORCPT ); Tue, 18 Aug 2015 08:55:25 -0400 Date: Tue, 18 Aug 2015 15:55:21 +0300 From: Cristina Opriceana To: jic23@kernel.org Cc: daniel.baluta@intel.com, octavian.purdila@intel.com, knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] Staging: iio: Move evgen interrupt generation to irq_work Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 15358 Lines: 463 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 Suggested-by: Jonathan Cameron Signed-off-by: Cristina Opriceana --- 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: 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 #include -/* 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 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/