From: Michael Hennerich <michael.hennerich@analog.com>
To: Jonathan Cameron <jic23@cam.ac.uk>
Cc: "linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>
Subject: Re: [PATCH 05/12] staging:iio:adis16400 replace unnecessary event line registration.
Date: Wed, 13 Apr 2011 15:55:43 +0200 [thread overview]
Message-ID: <4DA5AB5F.2060000@analog.com> (raw)
In-Reply-To: <1302702187-953-6-git-send-email-jic23@cam.ac.uk>
On 04/13/2011 03:43 PM, Jonathan Cameron wrote:
> Whilst the adis16400 does indeed support events, currently the driver
> does not. The trigger code should never use that infrastructure.
>
> Untested
>
> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> ---
> drivers/staging/iio/imu/adis16400_core.c | 26 +--------------
> drivers/staging/iio/imu/adis16400_trigger.c | 47 +++++++++++---------------
> 2 files changed, 21 insertions(+), 52 deletions(-)
>
> diff --git a/drivers/staging/iio/imu/adis16400_core.c b/drivers/staging/iio/imu/adis16400_core.c
> index 3439239..b8831dc 100644
> --- a/drivers/staging/iio/imu/adis16400_core.c
> +++ b/drivers/staging/iio/imu/adis16400_core.c
> @@ -552,14 +552,6 @@ static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("409 546 819 1638");
>
> static IIO_CONST_ATTR_NAME("adis16400");
>
> -static struct attribute *adis16400_event_attributes[] = {
> - NULL
> -};
> -
> -static struct attribute_group adis16400_event_attribute_group = {
> - .attrs = adis16400_event_attributes,
> -};
> -
> static struct attribute *adis16400_attributes[] = {
> &iio_dev_attr_gyro_x_calibbias.dev_attr.attr,
> &iio_dev_attr_gyro_y_calibbias.dev_attr.attr,
> @@ -629,8 +621,6 @@ static int __devinit adis16400_probe(struct spi_device *spi)
> }
>
> st->indio_dev->dev.parent = &spi->dev;
> - st->indio_dev->num_interrupt_lines = 1;
> - st->indio_dev->event_attrs = &adis16400_event_attribute_group;
> st->indio_dev->attrs = &adis16400_attribute_group;
> st->indio_dev->dev_data = (void *)(st);
> st->indio_dev->driver_module = THIS_MODULE;
> @@ -652,17 +642,9 @@ static int __devinit adis16400_probe(struct spi_device *spi)
> }
>
> if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0) {
> - ret = iio_register_interrupt_line(spi->irq,
> - st->indio_dev,
> - 0,
> - IRQF_TRIGGER_RISING,
> - "adis16400");
> - if (ret)
> - goto error_uninitialize_ring;
> -
> ret = adis16400_probe_trigger(st->indio_dev);
> if (ret)
> - goto error_unregister_line;
> + goto error_uninitialize_ring;
> }
>
> /* Get the device into a sane initial state */
> @@ -674,9 +656,6 @@ static int __devinit adis16400_probe(struct spi_device *spi)
> error_remove_trigger:
> if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
> adis16400_remove_trigger(st->indio_dev);
> -error_unregister_line:
> - if (st->indio_dev->modes & INDIO_RING_TRIGGERED)
> - iio_unregister_interrupt_line(st->indio_dev, 0);
> error_uninitialize_ring:
> iio_ring_buffer_unregister(st->indio_dev->ring);
> error_unreg_ring_funcs:
> @@ -710,9 +689,6 @@ static int adis16400_remove(struct spi_device *spi)
> flush_scheduled_work();
>
> adis16400_remove_trigger(indio_dev);
> - if (spi->irq && gpio_is_valid(irq_to_gpio(spi->irq)) > 0)
> - iio_unregister_interrupt_line(indio_dev, 0);
> -
> iio_ring_buffer_unregister(st->indio_dev->ring);
> adis16400_unconfigure_ring(indio_dev);
> iio_device_unregister(indio_dev);
> diff --git a/drivers/staging/iio/imu/adis16400_trigger.c b/drivers/staging/iio/imu/adis16400_trigger.c
> index 36b5ff5..afa5e74 100644
> --- a/drivers/staging/iio/imu/adis16400_trigger.c
> +++ b/drivers/staging/iio/imu/adis16400_trigger.c
> @@ -15,21 +15,13 @@
> /**
> * adis16400_data_rdy_trig_poll() the event handler for the data rdy trig
> **/
> -static int adis16400_data_rdy_trig_poll(struct iio_dev *dev_info,
> - int index,
> - s64 timestamp,
> - int no_test)
> +static irqreturn_t adis16400_data_rdy_trig_poll(int irq, void *private)
> {
> - struct adis16400_state *st = iio_dev_get_devdata(dev_info);
> - struct iio_trigger *trig = st->trig;
> -
> - iio_trigger_poll(trig, timestamp);
> -
> + disable_irq_nosync(irq);
> + iio_trigger_poll(private, iio_get_time_ns());
>
Is it save to call iio_trigger_poll() from the Top Half Hard-IRQ?
Instead of disable/enable irq shouldn't this be better a threaded_irq
with IRQF_ONESHOT?
> return IRQ_HANDLED;
> }
>
> -IIO_EVENT_SH(data_rdy_trig, &adis16400_data_rdy_trig_poll);
> -
> static IIO_TRIGGER_NAME_ATTR;
>
> static struct attribute *adis16400_trigger_attrs[] = {
> @@ -49,22 +41,9 @@ static int adis16400_data_rdy_trigger_set_state(struct iio_trigger *trig,
> {
> struct adis16400_state *st = trig->private_data;
> struct iio_dev *indio_dev = st->indio_dev;
> - int ret = 0;
>
> dev_dbg(&indio_dev->dev, "%s (%d)\n", __func__, state);
> - ret = adis16400_set_irq(&st->indio_dev->dev, state);
> - if (state == false) {
> - iio_remove_event_from_list(&iio_event_data_rdy_trig,
> - &indio_dev->interrupts[0]
> - ->ev_list);
> - /* possible quirk with handler currently worked around
> - by ensuring the work queue is empty */
> - flush_scheduled_work();
> - } else {
> - iio_add_event_to_list(&iio_event_data_rdy_trig,
> - &indio_dev->interrupts[0]->ev_list);
> - }
> - return ret;
> + return adis16400_set_irq(&st->indio_dev->dev, state);
> }
>
> /**
> @@ -85,12 +64,23 @@ int adis16400_probe_trigger(struct iio_dev *indio_dev)
> struct adis16400_state *st = indio_dev->dev_data;
>
> st->trig = iio_allocate_trigger();
> + if (st->trig == NULL) {
> + ret = -ENOMEM;
> + goto error_ret;
> + }
> + ret = request_irq(st->us->irq,
> + adis16400_data_rdy_trig_poll,
> + IRQF_TRIGGER_RISING,
> + "adis16400",
> + st->trig);
>
threaded_irq with IRQF_ONESHOT?
> + if (ret)
> + goto error_free_trig;
> st->trig->name = kasprintf(GFP_KERNEL,
> "adis16400-dev%d",
> indio_dev->id);
> if (!st->trig->name) {
> ret = -ENOMEM;
> - goto error_free_trig;
> + goto error_free_irq;
> }
> st->trig->dev.parent = &st->us->dev;
> st->trig->owner = THIS_MODULE;
> @@ -109,9 +99,11 @@ int adis16400_probe_trigger(struct iio_dev *indio_dev)
>
> error_free_trig_name:
> kfree(st->trig->name);
> +error_free_irq:
> + free_irq(st->us->irq, st->trig);
> error_free_trig:
> iio_free_trigger(st->trig);
> -
> +error_ret:
> return ret;
> }
>
> @@ -121,5 +113,6 @@ void adis16400_remove_trigger(struct iio_dev *indio_dev)
>
> iio_trigger_unregister(state->trig);
> kfree(state->trig->name);
> + free_irq(state->us->irq, state->trig);
> iio_free_trigger(state->trig);
> }
>
--
Greetings,
Michael
--
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368;
Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin,
Margaret Seif
next prev parent reply other threads:[~2011-04-13 13:55 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-13 13:42 [PATCH 00/12] IIO: precursor cleanups to even system rewrite Jonathan Cameron
2011-04-13 13:42 ` [PATCH 01/12] staging:iio:tsl2563 take advantage of new iio_device_allocate private data Jonathan Cameron
2011-04-13 13:42 ` [PATCH 02/12] staging:iio:light:tsl2563 constify gain level table Jonathan Cameron
2011-04-13 13:42 ` [PATCH 03/12] staging:iio:adis16300 replace unnecessary event line registration Jonathan Cameron
2011-04-13 13:42 ` [PATCH 04/12] staging:iio:adis16350 " Jonathan Cameron
2011-04-13 13:43 ` [PATCH 05/12] staging:iio:adis16400 " Jonathan Cameron
2011-04-13 13:55 ` Michael Hennerich [this message]
2011-04-13 14:45 ` Jonathan Cameron
2011-04-17 18:41 ` Jonathan Cameron
2011-04-18 7:16 ` Hennerich, Michael
2011-04-13 13:43 ` [PATCH 06/12] staging:iio:adis16260 " Jonathan Cameron
2011-04-13 13:43 ` [PATCH 07/12] staging:iio:adis16203 " Jonathan Cameron
2011-04-13 13:43 ` [PATCH 08/12] staging:iio:adis16204 " Jonathan Cameron
2011-04-13 13:43 ` [PATCH 09/12] staging:iio:adis16201 " Jonathan Cameron
2011-04-13 13:43 ` [PATCH 10/12] staging:iio:adis16240 " Jonathan Cameron
2011-04-13 13:43 ` [PATCH 11/12] staging:iio:adis16209 " Jonathan Cameron
2011-04-13 13:43 ` [PATCH 12/12] staging:iio:ade7758 " Jonathan Cameron
2011-04-18 11:59 ` [PATCH 00/12] IIO: precursor cleanups to even system rewrite Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4DA5AB5F.2060000@analog.com \
--to=michael.hennerich@analog.com \
--cc=jic23@cam.ac.uk \
--cc=linux-iio@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.