From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ppsw-41.csi.cam.ac.uk ([131.111.8.141]:42306 "EHLO ppsw-41.csi.cam.ac.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751290Ab2FRMJg (ORCPT ); Mon, 18 Jun 2012 08:09:36 -0400 Message-ID: <4FDF1A7C.8000300@cam.ac.uk> Date: Mon, 18 Jun 2012 13:09:32 +0100 From: Jonathan Cameron MIME-Version: 1.0 To: Lars-Peter Clausen CC: linux-iio@vger.kernel.org Subject: Re: [PATCH v4 05/10] staging:iio:adc:ad7298: Use new triggered buffer setup helper function References: <1340019903-13305-1-git-send-email-lars@metafoo.de> <1340019903-13305-5-git-send-email-lars@metafoo.de> In-Reply-To: <1340019903-13305-5-git-send-email-lars@metafoo.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Sender: linux-iio-owner@vger.kernel.org List-Id: linux-iio@vger.kernel.org On 6/18/2012 12:44 PM, Lars-Peter Clausen wrote: > Use the new triggered buffer setup helper function to allocate and register > buffer and pollfunc. > > The previous code was not passing the temperature channel to iio_buffer_register > since the temperature channel can not be used in buffered mode. With the new > helper functions it is not possible to do this, instead the scan index for the > temperature channel is set to -1 which will cause iio_buffer_register to ignore > the channel. (Also While we are at it also assign the symbolic constant instead > of the raw value to the channel address for the temperature channel). > > Also as part of the conversion drop scan_timestamp being enabled by default, > since it is a left over of an earlier cleanup. > > Signed-off-by: Lars-Peter Clausen Acked-by: Jonathan Cameron > --- > Changes since v3: > * Set temperature channel scan index to -1 so it can not be used in buffered > mode. This is the same behaviour as without this patch. > --- > drivers/staging/iio/adc/Kconfig | 2 +- > drivers/staging/iio/adc/ad7298.h | 5 +++ > drivers/staging/iio/adc/ad7298_core.c | 15 +++----- > drivers/staging/iio/adc/ad7298_ring.c | 64 ++++++--------------------------- > 4 files changed, 20 insertions(+), 66 deletions(-) > > diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig > index 7cdf421..bb6fffd 100644 > --- a/drivers/staging/iio/adc/Kconfig > +++ b/drivers/staging/iio/adc/Kconfig > @@ -13,7 +13,7 @@ config AD7291 > config AD7298 > tristate "Analog Devices AD7298 ADC driver" > depends on SPI > - select IIO_KFIFO_BUF if IIO_BUFFER > + select IIO_TRIGGERED_BUFFER if IIO_BUFFER > help > Say yes here to build support for Analog Devices AD7298 > 8 Channel ADC with temperature sensor. > diff --git a/drivers/staging/iio/adc/ad7298.h b/drivers/staging/iio/adc/ad7298.h > index 5051a7e..18f2787 100644 > --- a/drivers/staging/iio/adc/ad7298.h > +++ b/drivers/staging/iio/adc/ad7298.h > @@ -55,6 +55,8 @@ struct ad7298_state { > #ifdef CONFIG_IIO_BUFFER > int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev); > void ad7298_ring_cleanup(struct iio_dev *indio_dev); > +int ad7298_update_scan_mode(struct iio_dev *indio_dev, > + const unsigned long *active_scan_mask); > #else /* CONFIG_IIO_BUFFER */ > > static inline int > @@ -66,5 +68,8 @@ ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev) > static inline void ad7298_ring_cleanup(struct iio_dev *indio_dev) > { > } > + > +#define ad7298_update_scan_mode NULL > + > #endif /* CONFIG_IIO_BUFFER */ > #endif /* IIO_ADC_AD7298_H_ */ > diff --git a/drivers/staging/iio/adc/ad7298_core.c b/drivers/staging/iio/adc/ad7298_core.c > index c90f2b3..6141f4a 100644 > --- a/drivers/staging/iio/adc/ad7298_core.c > +++ b/drivers/staging/iio/adc/ad7298_core.c > @@ -45,8 +45,8 @@ static struct iio_chan_spec ad7298_channels[] = { > .channel = 0, > .info_mask = IIO_CHAN_INFO_RAW_SEPARATE_BIT | > IIO_CHAN_INFO_SCALE_SEPARATE_BIT, > - .address = 9, > - .scan_index = AD7298_CH_TEMP, > + .address = AD7298_CH_TEMP, > + .scan_index = -1, > .scan_type = { > .sign = 's', > .realbits = 32, > @@ -171,6 +171,7 @@ static int ad7298_read_raw(struct iio_dev *indio_dev, > > static const struct iio_info ad7298_info = { > .read_raw =&ad7298_read_raw, > + .update_scan_mode = ad7298_update_scan_mode, > .driver_module = THIS_MODULE, > }; > > @@ -231,19 +232,12 @@ static int __devinit ad7298_probe(struct spi_device *spi) > if (ret) > goto error_disable_reg; > > - ret = iio_buffer_register(indio_dev, > - &ad7298_channels[1], /* skip temp0 */ > - ARRAY_SIZE(ad7298_channels) - 1); > - if (ret) > - goto error_cleanup_ring; > ret = iio_device_register(indio_dev); > if (ret) > - goto error_unregister_ring; > + goto error_cleanup_ring; > > return 0; > > -error_unregister_ring: > - iio_buffer_unregister(indio_dev); > error_cleanup_ring: > ad7298_ring_cleanup(indio_dev); > error_disable_reg: > @@ -263,7 +257,6 @@ static int __devexit ad7298_remove(struct spi_device *spi) > struct ad7298_state *st = iio_priv(indio_dev); > > iio_device_unregister(indio_dev); > - iio_buffer_unregister(indio_dev); > ad7298_ring_cleanup(indio_dev); > if (!IS_ERR(st->reg)) { > regulator_disable(st->reg); > diff --git a/drivers/staging/iio/adc/ad7298_ring.c b/drivers/staging/iio/adc/ad7298_ring.c > index 908a3e5..cd3e9cb 100644 > --- a/drivers/staging/iio/adc/ad7298_ring.c > +++ b/drivers/staging/iio/adc/ad7298_ring.c > @@ -13,37 +13,29 @@ > > #include > #include > -#include > #include > +#include > > #include "ad7298.h" > > /** > - * ad7298_ring_preenable() setup the parameters of the ring before enabling > - * > - * The complex nature of the setting of the number of bytes per datum is due > - * to this driver currently ensuring that the timestamp is stored at an 8 > - * byte boundary. > + * ad7298_update_scan_mode() setup the spi transfer buffer for the new scan mask > **/ > -static int ad7298_ring_preenable(struct iio_dev *indio_dev) > +int ad7298_update_scan_mode(struct iio_dev *indio_dev, > + const unsigned long *active_scan_mask) > { > struct ad7298_state *st = iio_priv(indio_dev); > int i, m; > unsigned short command; > - int scan_count, ret; > - > - ret = iio_sw_buffer_preenable(indio_dev); > - if (ret< 0) > - return ret; > + int scan_count; > > /* Now compute overall size */ > - scan_count = bitmap_weight(indio_dev->active_scan_mask, > - indio_dev->masklength); > + scan_count = bitmap_weight(active_scan_mask, indio_dev->masklength); > > command = AD7298_WRITE | st->ext_ref; > > for (i = 0, m = AD7298_CH(0); i< AD7298_MAX_CHAN; i++, m>>= 1) > - if (test_bit(i, indio_dev->active_scan_mask)) > + if (test_bit(i, active_scan_mask)) > command |= m; > > st->tx_buf[0] = cpu_to_be16(command); > @@ -108,49 +100,13 @@ static irqreturn_t ad7298_trigger_handler(int irq, void *p) > return IRQ_HANDLED; > } > > -static const struct iio_buffer_setup_ops ad7298_ring_setup_ops = { > - .preenable =&ad7298_ring_preenable, > - .postenable =&iio_triggered_buffer_postenable, > - .predisable =&iio_triggered_buffer_predisable, > -}; > - > int ad7298_register_ring_funcs_and_init(struct iio_dev *indio_dev) > { > - int ret; > - > - indio_dev->buffer = iio_kfifo_allocate(indio_dev); > - if (!indio_dev->buffer) { > - ret = -ENOMEM; > - goto error_ret; > - } > - indio_dev->pollfunc = iio_alloc_pollfunc(NULL, > - &ad7298_trigger_handler, > - IRQF_ONESHOT, > - indio_dev, > - "ad7298_consumer%d", > - indio_dev->id); > - > - if (indio_dev->pollfunc == NULL) { > - ret = -ENOMEM; > - goto error_deallocate_kfifo; > - } > - > - /* Ring buffer functions - here trigger setup related */ > - indio_dev->setup_ops =&ad7298_ring_setup_ops; > - indio_dev->buffer->scan_timestamp = true; > - > - /* Flag that polled ring buffering is possible */ > - indio_dev->modes |= INDIO_BUFFER_TRIGGERED; > - return 0; > - > -error_deallocate_kfifo: > - iio_kfifo_free(indio_dev->buffer); > -error_ret: > - return ret; > + return iio_triggered_buffer_setup(indio_dev, NULL, > + &ad7298_trigger_handler, NULL); > } > > void ad7298_ring_cleanup(struct iio_dev *indio_dev) > { > - iio_dealloc_pollfunc(indio_dev->pollfunc); > - iio_kfifo_free(indio_dev->buffer); > + iio_triggered_buffer_cleanup(indio_dev); > }