All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@cam.ac.uk>
To: Barry Song <21cnbao@gmail.com>
Cc: linux-iio@vger.kernel.org
Subject: Re: [PATCH 5/5 v2] staging:iio:adis16209 use iio_sw_ring_helper_state and funcs
Date: Thu, 08 Jul 2010 15:46:42 +0100	[thread overview]
Message-ID: <4C35E4D2.3090202@cam.ac.uk> (raw)
In-Reply-To: <AANLkTiknA_De-zqjjOS_1d8Fkm_PXMJCiZjurGEMpQsk@mail.gmail.com>

On 07/08/10 10:37, Barry Song wrote:
> On Thu, Jul 1, 2010 at 12:07 AM, Jonathan Cameron <jic23@cam.ac.uk> wrote:
>>  Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
> Thanks very much. Has this one been tested? If so,
> Acked-by: Barry Song <21cnbao@gmail.com>.
Nope.  Don't have one of these... I'll hold off on sending this
to Greg until I have confirmation it works. 
> And I will send related patches for other drivers too.
Excellent. 
> -barry
>> ---
>>
>>  The get element fuction should return how much of the buffer it has
>>  used.  Now it actually does this.
>>
>>  drivers/staging/iio/accel/adis16209.h         |    7 ++-
>>  drivers/staging/iio/accel/adis16209_core.c    |   54 +++++++++++----------
>>  drivers/staging/iio/accel/adis16209_ring.c    |   65 ++++++-------------------
>>  drivers/staging/iio/accel/adis16209_trigger.c |    8 ++-
>>  4 files changed, 53 insertions(+), 81 deletions(-)
>>
>> diff --git a/drivers/staging/iio/accel/adis16209.h b/drivers/staging/iio/accel/adis16209.h
>> index 92daf6f..84e1a2f 100644
>> --- a/drivers/staging/iio/accel/adis16209.h
>> +++ b/drivers/staging/iio/accel/adis16209.h
>> @@ -113,16 +113,17 @@
>>  * @buf_lock:          mutex to protect tx and rx
>>  **/
>>  struct adis16209_state {
>> +       struct iio_sw_ring_helper_state help;
>>        struct spi_device               *us;
>> -       struct work_struct              work_trigger_to_ring;
>> -       s64                             last_timestamp;
>> -       struct iio_dev                  *indio_dev;
>>        struct iio_trigger              *trig;
>>        u8                              *tx;
>>        u8                              *rx;
>>        struct mutex                    buf_lock;
>>  };
>>
>> +#define adis16209_h_to_s(_h)                           \
>> +       container_of(_h, struct adis16209_state, help)
>> +
>>  int adis16209_set_irq(struct device *dev, bool enable);
>>
>>  #ifdef CONFIG_IIO_RING_BUFFER
>> diff --git a/drivers/staging/iio/accel/adis16209_core.c b/drivers/staging/iio/accel/adis16209_core.c
>> index 6c6923f..9c7aafe 100644
>> --- a/drivers/staging/iio/accel/adis16209_core.c
>> +++ b/drivers/staging/iio/accel/adis16209_core.c
>> @@ -21,6 +21,7 @@
>>  #include "../iio.h"
>>  #include "../sysfs.h"
>>  #include "../ring_generic.h"
>> +#include "../ring_sw.h"
>>  #include "accel.h"
>>  #include "inclinometer.h"
>>  #include "../gyro/gyro.h"
>> @@ -44,7 +45,8 @@ static int adis16209_spi_write_reg_8(struct device *dev,
>>  {
>>        int ret;
>>        struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> -       struct adis16209_state *st = iio_dev_get_devdata(indio_dev);
>> +       struct iio_sw_ring_helper_state *h = iio_dev_get_devdata(indio_dev);
>> +       struct adis16209_state *st = adis16209_h_to_s(h);
>>
>>        mutex_lock(&st->buf_lock);
>>        st->tx[0] = ADIS16209_WRITE_REG(reg_address);
>> @@ -70,7 +72,8 @@ static int adis16209_spi_write_reg_16(struct device *dev,
>>        int ret;
>>        struct spi_message msg;
>>        struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> -       struct adis16209_state *st = iio_dev_get_devdata(indio_dev);
>> +       struct iio_sw_ring_helper_state *h = iio_dev_get_devdata(indio_dev);
>> +       struct adis16209_state *st = adis16209_h_to_s(h);
>>        struct spi_transfer xfers[] = {
>>                {
>>                        .tx_buf = st->tx,
>> @@ -115,7 +118,8 @@ static int adis16209_spi_read_reg_16(struct device *dev,
>>  {
>>        struct spi_message msg;
>>        struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> -       struct adis16209_state *st = iio_dev_get_devdata(indio_dev);
>> +       struct iio_sw_ring_helper_state *h = iio_dev_get_devdata(indio_dev);
>> +       struct adis16209_state *st = adis16209_h_to_s(h);
>>        int ret;
>>        struct spi_transfer xfers[] = {
>>                {
>> @@ -355,7 +359,7 @@ err_ret:
>>  static int adis16209_initial_setup(struct adis16209_state *st)
>>  {
>>        int ret;
>> -       struct device *dev = &st->indio_dev->dev;
>> +       struct device *dev = &st->help.indio_dev->dev;
>>
>>        /* Disable IRQ */
>>        ret = adis16209_set_irq(dev, false);
>> @@ -498,30 +502,30 @@ static int __devinit adis16209_probe(struct spi_device *spi)
>>        st->us = spi;
>>        mutex_init(&st->buf_lock);
>>        /* setup the industrialio driver allocated elements */
>> -       st->indio_dev = iio_allocate_device();
>> -       if (st->indio_dev == NULL) {
>> +       st->help.indio_dev = iio_allocate_device();
>> +       if (st->help.indio_dev == NULL) {
>>                ret = -ENOMEM;
>>                goto error_free_tx;
>>        }
>>
>> -       st->indio_dev->dev.parent = &spi->dev;
>> -       st->indio_dev->num_interrupt_lines = 1;
>> -       st->indio_dev->event_attrs = &adis16209_event_attribute_group;
>> -       st->indio_dev->attrs = &adis16209_attribute_group;
>> -       st->indio_dev->dev_data = (void *)(st);
>> -       st->indio_dev->driver_module = THIS_MODULE;
>> -       st->indio_dev->modes = INDIO_DIRECT_MODE;
>> +       st->help.indio_dev->dev.parent = &spi->dev;
>> +       st->help.indio_dev->num_interrupt_lines = 1;
>> +       st->help.indio_dev->event_attrs = &adis16209_event_attribute_group;
>> +       st->help.indio_dev->attrs = &adis16209_attribute_group;
>> +       st->help.indio_dev->dev_data = (void *)(&st->help);
>> +       st->help.indio_dev->driver_module = THIS_MODULE;
>> +       st->help.indio_dev->modes = INDIO_DIRECT_MODE;
>>
>> -       ret = adis16209_configure_ring(st->indio_dev);
>> +       ret = adis16209_configure_ring(st->help.indio_dev);
>>        if (ret)
>>                goto error_free_dev;
>>
>> -       ret = iio_device_register(st->indio_dev);
>> +       ret = iio_device_register(st->help.indio_dev);
>>        if (ret)
>>                goto error_unreg_ring_funcs;
>>        regdone = 1;
>>
>> -       ret = iio_ring_buffer_register(st->indio_dev->ring, 0);
>> +       ret = iio_ring_buffer_register(st->help.indio_dev->ring, 0);
>>        if (ret) {
>>                printk(KERN_ERR "failed to initialize the ring\n");
>>                goto error_unreg_ring_funcs;
>> @@ -529,14 +533,14 @@ static int __devinit adis16209_probe(struct spi_device *spi)
>>
>>        if (spi->irq) {
>>                ret = iio_register_interrupt_line(spi->irq,
>> -                               st->indio_dev,
>> +                               st->help.indio_dev,
>>                                0,
>>                                IRQF_TRIGGER_RISING,
>>                                "adis16209");
>>                if (ret)
>>                        goto error_uninitialize_ring;
>>
>> -               ret = adis16209_probe_trigger(st->indio_dev);
>> +               ret = adis16209_probe_trigger(st->help.indio_dev);
>>                if (ret)
>>                        goto error_unregister_line;
>>        }
>> @@ -548,19 +552,19 @@ static int __devinit adis16209_probe(struct spi_device *spi)
>>        return 0;
>>
>>  error_remove_trigger:
>> -       adis16209_remove_trigger(st->indio_dev);
>> +       adis16209_remove_trigger(st->help.indio_dev);
>>  error_unregister_line:
>>        if (spi->irq)
>> -               iio_unregister_interrupt_line(st->indio_dev, 0);
>> +               iio_unregister_interrupt_line(st->help.indio_dev, 0);
>>  error_uninitialize_ring:
>> -       iio_ring_buffer_unregister(st->indio_dev->ring);
>> +       iio_ring_buffer_unregister(st->help.indio_dev->ring);
>>  error_unreg_ring_funcs:
>> -       adis16209_unconfigure_ring(st->indio_dev);
>> +       adis16209_unconfigure_ring(st->help.indio_dev);
>>  error_free_dev:
>>        if (regdone)
>> -               iio_device_unregister(st->indio_dev);
>> +               iio_device_unregister(st->help.indio_dev);
>>        else
>> -               iio_free_device(st->indio_dev);
>> +               iio_free_device(st->help.indio_dev);
>>  error_free_tx:
>>        kfree(st->tx);
>>  error_free_rx:
>> @@ -574,7 +578,7 @@ error_ret:
>>  static int adis16209_remove(struct spi_device *spi)
>>  {
>>        struct adis16209_state *st = spi_get_drvdata(spi);
>> -       struct iio_dev *indio_dev = st->indio_dev;
>> +       struct iio_dev *indio_dev = st->help.indio_dev;
>>
>>        flush_scheduled_work();
>>
>> diff --git a/drivers/staging/iio/accel/adis16209_ring.c b/drivers/staging/iio/accel/adis16209_ring.c
>> index 25fde65..bb2389e 100644
>> --- a/drivers/staging/iio/accel/adis16209_ring.c
>> +++ b/drivers/staging/iio/accel/adis16209_ring.c
>> @@ -55,17 +55,6 @@ static struct attribute_group adis16209_scan_el_group = {
>>  };
>>
>>  /**
>> - * adis16209_poll_func_th() top half interrupt handler called by trigger
>> - * @private_data:      iio_dev
>> - **/
>> -static void adis16209_poll_func_th(struct iio_dev *indio_dev, s64 time)
>> -{
>> -       struct adis16209_state *st = iio_dev_get_devdata(indio_dev);
>> -       st->last_timestamp = time;
>> -       schedule_work(&st->work_trigger_to_ring);
>> -}
>> -
>> -/**
>>  * adis16209_read_ring_data() read data registers which will be placed into ring
>>  * @dev: device associated with child of actual device (iio_dev or iio_trig)
>>  * @rx: somewhere to pass back the value read
>> @@ -107,44 +96,20 @@ static int adis16209_read_ring_data(struct device *dev, u8 *rx)
>>        return ret;
>>  }
>>
>> -/* Whilst this makes a lot of calls to iio_sw_ring functions - it is to device
>> - * specific to be rolled into the core.
>> - */
>> -static void adis16209_trigger_bh_to_ring(struct work_struct *work_s)
>> +static int adis16209_get_ring_element(struct iio_sw_ring_helper_state *h,
>> +                               u8 *buf)
>>  {
>> -       struct adis16209_state *st
>> -               = container_of(work_s, struct adis16209_state,
>> -                              work_trigger_to_ring);
>> -
>> -       int i = 0;
>> -       s16 *data;
>> -       size_t datasize = st->indio_dev
>> -               ->ring->access.get_bpd(st->indio_dev->ring);
>> -
>> -       data = kmalloc(datasize , GFP_KERNEL);
>> -       if (data == NULL) {
>> -               dev_err(&st->us->dev, "memory alloc failed in ring bh");
>> -               return;
>> -       }
>> -
>> -       if (st->indio_dev->scan_count)
>> -               if (adis16209_read_ring_data(&st->indio_dev->dev, st->rx) >= 0)
>> -                       for (; i < st->indio_dev->scan_count; i++)
>> -                               data[i] = be16_to_cpup(
>> -                                       (__be16 *)&(st->rx[i*2]));
>> -
>> -       /* Guaranteed to be aligned with 8 byte boundary */
>> -       if (st->indio_dev->scan_timestamp)
>> -               *((s64 *)(data + ((i + 3)/4)*4)) = st->last_timestamp;
>> +       int i, ret;
>> +       s16 *data = (s16 *)buf;
>> +       struct adis16209_state *st = adis16209_h_to_s(h);
>>
>> -       st->indio_dev->ring->access.store_to(st->indio_dev->ring,
>> -                                           (u8 *)data,
>> -                                           st->last_timestamp);
>> -
>> -       iio_trigger_notify_done(st->indio_dev->trig);
>> -       kfree(data);
>> +       ret = adis16209_read_ring_data(&h->indio_dev->dev, st->rx);
>> +       if (ret < 0)
>> +               return ret;
>> +       for (i = 0; i < h->indio_dev->scan_count; i++)
>> +               data[i] = be16_to_cpup((__be16 *)&(st->rx[i*2]));
>>
>> -       return;
>> +       return i*sizeof(data[0]);
>>  }
>>
>>  void adis16209_unconfigure_ring(struct iio_dev *indio_dev)
>> @@ -156,11 +121,11 @@ void adis16209_unconfigure_ring(struct iio_dev *indio_dev)
>>  int adis16209_configure_ring(struct iio_dev *indio_dev)
>>  {
>>        int ret = 0;
>> -       struct adis16209_state *st = indio_dev->dev_data;
>>        struct iio_ring_buffer *ring;
>> -       INIT_WORK(&st->work_trigger_to_ring, adis16209_trigger_bh_to_ring);
>> +       struct iio_sw_ring_helper_state *h = iio_dev_get_devdata(indio_dev);
>> +       INIT_WORK(&h->work_trigger_to_ring, iio_sw_trigger_bh_to_ring);
>>        /* Set default scan mode */
>> -
>> +       h->get_ring_element = &adis16209_get_ring_element;
>>        iio_scan_mask_set(indio_dev, iio_scan_el_supply.number);
>>        iio_scan_mask_set(indio_dev, iio_scan_el_rot.number);
>>        iio_scan_mask_set(indio_dev, iio_scan_el_accel_x.number);
>> @@ -187,7 +152,7 @@ int adis16209_configure_ring(struct iio_dev *indio_dev)
>>        ring->predisable = &iio_triggered_ring_predisable;
>>        ring->owner = THIS_MODULE;
>>
>> -       ret = iio_alloc_pollfunc(indio_dev, NULL, &adis16209_poll_func_th);
>> +       ret = iio_alloc_pollfunc(indio_dev, NULL, &iio_sw_poll_func_th);
>>        if (ret)
>>                goto error_iio_sw_rb_free;
>>
>> diff --git a/drivers/staging/iio/accel/adis16209_trigger.c b/drivers/staging/iio/accel/adis16209_trigger.c
>> index 1487eff..51cdf4a 100644
>> --- a/drivers/staging/iio/accel/adis16209_trigger.c
>> +++ b/drivers/staging/iio/accel/adis16209_trigger.c
>> @@ -10,6 +10,7 @@
>>  #include "../iio.h"
>>  #include "../sysfs.h"
>>  #include "../trigger.h"
>> +#include "../ring_sw.h"
>>  #include "adis16209.h"
>>
>>  /**
>> @@ -48,11 +49,11 @@ static int adis16209_data_rdy_trigger_set_state(struct iio_trigger *trig,
>>                                                bool state)
>>  {
>>        struct adis16209_state *st = trig->private_data;
>> -       struct iio_dev *indio_dev = st->indio_dev;
>> +       struct iio_dev *indio_dev = st->help.indio_dev;
>>        int ret = 0;
>>
>>        dev_dbg(&indio_dev->dev, "%s (%d)\n", __func__, state);
>> -       ret = adis16209_set_irq(&st->indio_dev->dev, state);
>> +       ret = adis16209_set_irq(&indio_dev->dev, state);
>>        if (state == false) {
>>                iio_remove_event_from_list(&iio_event_data_rdy_trig,
>>                                           &indio_dev->interrupts[0]
>> @@ -79,7 +80,8 @@ static int adis16209_trig_try_reen(struct iio_trigger *trig)
>>  int adis16209_probe_trigger(struct iio_dev *indio_dev)
>>  {
>>        int ret;
>> -       struct adis16209_state *st = indio_dev->dev_data;
>> +       struct iio_sw_ring_helper_state *h = iio_dev_get_devdata(indio_dev);
>> +       struct adis16209_state *st = adis16209_h_to_s(h);
>>
>>        st->trig = iio_allocate_trigger();
>>        st->trig->name = kasprintf(GFP_KERNEL,
>> --
>> 1.7.0.4
>>
>> --
>> 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
>>
> 


  reply	other threads:[~2010-07-08 14:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-30 16:07 [PATCH 4/5 v2] staging:iio:lis3l02dq use iio_sw_ring_helper_state and funcs Jonathan Cameron
2010-06-30 16:07 ` [PATCH 5/5 v2] staging:iio:adis16209 " Jonathan Cameron
2010-07-08  9:37   ` Barry Song
2010-07-08 14:46     ` Jonathan Cameron [this message]
2010-07-08  9:32 ` [PATCH 4/5 v2] staging:iio:lis3l02dq " Barry Song

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=4C35E4D2.3090202@cam.ac.uk \
    --to=jic23@cam.ac.uk \
    --cc=21cnbao@gmail.com \
    --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.