[PATCH 0/5] staging:iio: drop sw_ring buffer implementation.

[PATCH 0/5] staging:iio: drop sw_ring buffer implementation.

[Jonathan Cameron]

sw_ring was the original buffer implementation for IIO. It kind of evolved
over a couple of late nights and even though I wrote it I was never
convinced it was right.  Then a nice kfifo implementation came along
and in the vast majority of IIO use cases whether we have a kfifo or
a ring buffer is not as it turns out that important.

We have been telling people not to use this for quite some time now.
Lets make that finally a really hard rule by dropping it.
There are 4 remaining users. This patch drops support form lis3l02dq
which had support for both this and kfifo.  For the other 3 users
it simply switches over to kfifo.  We haven't seen any real problems
doing this with any other driver so I can't see why these would cause
trouble!

Note my aim for this cycle ist to greatly cut down what we have left
in staging by removing legacy cruft and getting other stuff cleaned up and
out!

Jonathan

Jonathan Cameron (5):
  staging:iio:accel:lis3l02dq remove sw_ring support.
  staging:iio:impedance-analyzer switch from sw_ring to kfifo.
  staging:iio:meter:ade7758 switch from sw_ring to kfifo
  staging:iio:imu: adis16400 switch from sw_ring to kfifo
  staging:iio: drop sw_ring buffer implementation.

 drivers/staging/iio/Kconfig                     |  13 -
 drivers/staging/iio/Makefile                    |   2 -
 drivers/staging/iio/accel/Kconfig               |  23 +-
 drivers/staging/iio/accel/lis3l02dq.h           |   8 -
 drivers/staging/iio/accel/lis3l02dq_ring.c      |   7 +-
 drivers/staging/iio/impedance-analyzer/Kconfig  |   2 +-
 drivers/staging/iio/impedance-analyzer/ad5933.c |   8 +-
 drivers/staging/iio/imu/Kconfig                 |   2 +-
 drivers/staging/iio/imu/adis16400_ring.c        |  12 +-
 drivers/staging/iio/meter/Kconfig               |   2 +-
 drivers/staging/iio/meter/ade7758_ring.c        |  12 +-
 drivers/staging/iio/ring_sw.c                   | 366 ------------------------
 drivers/staging/iio/ring_sw.h                   |  30 --
 13 files changed, 23 insertions(+), 464 deletions(-)
 delete mode 100644 drivers/staging/iio/ring_sw.c
 delete mode 100644 drivers/staging/iio/ring_sw.h

-- 
1.8.0.1

[PATCH 1/5] staging:iio:accel:lis3l02dq remove sw_ring support.

[Jonathan Cameron]

The sw_ring buffer is being removed in favour of the kfifo version.
This is one of only a couple of driver still supporting its use.

This driver will hopefully also be removed in favour of supporting the
part in the unified ST accelerometer driver.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/accel/Kconfig          | 23 +----------------------
 drivers/staging/iio/accel/lis3l02dq.h      |  8 --------
 drivers/staging/iio/accel/lis3l02dq_ring.c |  7 +++----
 3 files changed, 4 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/iio/accel/Kconfig b/drivers/staging/iio/accel/Kconfig
index 2b54430..fa772a9 100644
--- a/drivers/staging/iio/accel/Kconfig
+++ b/drivers/staging/iio/accel/Kconfig
@@ -67,34 +67,13 @@ config LIS3L02DQ
 	tristate "ST Microelectronics LIS3L02DQ Accelerometer Driver"
 	depends on SPI
 	select IIO_TRIGGER if IIO_BUFFER
-	depends on !IIO_BUFFER || IIO_KFIFO_BUF || IIO_SW_RING
+	depends on !IIO_BUFFER || IIO_KFIFO_BUF
 	depends on GENERIC_GPIO
 	help
 	  Say yes here to build SPI support for the ST microelectronics
 	  accelerometer. The driver supplies direct access via sysfs files
 	  and an event interface via a character device.
 
-choice
-	prompt "Buffer type"
-       	depends on LIS3L02DQ && IIO_BUFFER
-
-config LIS3L02DQ_BUF_KFIFO
-       depends on IIO_KFIFO_BUF
-       bool "Simple FIFO"
-       help
-         Kfifo based FIFO.  Does not provide any events so it is up
-	 to userspace to ensure it reads often enough that data is not
-	 lost.
-
-config LIS3L02DQ_BUF_RING_SW
-       depends on IIO_SW_RING
-       bool "IIO Software Ring"
-       help
-         Original IIO ring buffer implementation.  Provides simple
-	 buffer events, half full etc.
-
-endchoice
-
 config SCA3000
 	depends on IIO_BUFFER
 	depends on SPI
diff --git a/drivers/staging/iio/accel/lis3l02dq.h b/drivers/staging/iio/accel/lis3l02dq.h
index 2bac722..0a8ea82 100644
--- a/drivers/staging/iio/accel/lis3l02dq.h
+++ b/drivers/staging/iio/accel/lis3l02dq.h
@@ -185,14 +185,6 @@ int lis3l02dq_probe_trigger(struct iio_dev *indio_dev);
 int lis3l02dq_configure_buffer(struct iio_dev *indio_dev);
 void lis3l02dq_unconfigure_buffer(struct iio_dev *indio_dev);
 
-#ifdef CONFIG_LIS3L02DQ_BUF_RING_SW
-#define lis3l02dq_free_buf iio_sw_rb_free
-#define lis3l02dq_alloc_buf iio_sw_rb_allocate
-#endif
-#ifdef CONFIG_LIS3L02DQ_BUF_KFIFO
-#define lis3l02dq_free_buf iio_kfifo_free
-#define lis3l02dq_alloc_buf iio_kfifo_allocate
-#endif
 irqreturn_t lis3l02dq_data_rdy_trig_poll(int irq, void *private);
 #define lis3l02dq_th lis3l02dq_data_rdy_trig_poll
 
diff --git a/drivers/staging/iio/accel/lis3l02dq_ring.c b/drivers/staging/iio/accel/lis3l02dq_ring.c
index bc38651..6861877 100644
--- a/drivers/staging/iio/accel/lis3l02dq_ring.c
+++ b/drivers/staging/iio/accel/lis3l02dq_ring.c
@@ -7,7 +7,6 @@
 #include <linux/export.h>
 
 #include <linux/iio/iio.h>
-#include "../ring_sw.h"
 #include <linux/iio/kfifo_buf.h>
 #include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
@@ -318,7 +317,7 @@ void lis3l02dq_remove_trigger(struct iio_dev *indio_dev)
 void lis3l02dq_unconfigure_buffer(struct iio_dev *indio_dev)
 {
 	iio_dealloc_pollfunc(indio_dev->pollfunc);
-	lis3l02dq_free_buf(indio_dev->buffer);
+	iio_kfifo_free(indio_dev->buffer);
 }
 
 static int lis3l02dq_buffer_postenable(struct iio_dev *indio_dev)
@@ -401,7 +400,7 @@ int lis3l02dq_configure_buffer(struct iio_dev *indio_dev)
 	int ret;
 	struct iio_buffer *buffer;
 
-	buffer = lis3l02dq_alloc_buf(indio_dev);
+	buffer = iio_kfifo_allocate(indio_dev);
 	if (!buffer)
 		return -ENOMEM;
 
@@ -427,6 +426,6 @@ int lis3l02dq_configure_buffer(struct iio_dev *indio_dev)
 	return 0;
 
 error_iio_sw_rb_free:
-	lis3l02dq_free_buf(indio_dev->buffer);
+	iio_kfifo_free(indio_dev->buffer);
 	return ret;
 }
-- 
1.8.0.1

[PATCH 2/5] staging:iio:impedance-analyzer switch from sw_ring to kfifo.

[Jonathan Cameron]

sw_ring buffer implementation is going away so switch to the
kfifo based alternative.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/impedance-analyzer/Kconfig  | 2 +-
 drivers/staging/iio/impedance-analyzer/ad5933.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/impedance-analyzer/Kconfig b/drivers/staging/iio/impedance-analyzer/Kconfig
index ad0ff76..dd97b6b 100644
--- a/drivers/staging/iio/impedance-analyzer/Kconfig
+++ b/drivers/staging/iio/impedance-analyzer/Kconfig
@@ -7,7 +7,7 @@ config AD5933
 	tristate "Analog Devices AD5933, AD5934 driver"
 	depends on I2C
 	select IIO_BUFFER
-	select IIO_SW_RING
+	select IIO_KFIFO_BUF
 	help
 	  Say yes here to build support for Analog Devices Impedance Converter,
 	  Network Analyzer, AD5933/4, provides direct access via sysfs.
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 779243d..440e226 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -22,7 +22,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
-#include "../ring_sw.h"
+#include <linux/iio/kfifo_buf.h>
 
 #include "ad5933.h"
 
@@ -630,7 +630,7 @@ static const struct iio_buffer_setup_ops ad5933_ring_setup_ops = {
 
 static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
 {
-	indio_dev->buffer = iio_sw_rb_allocate(indio_dev);
+	indio_dev->buffer = iio_kfifo_allocate(indio_dev);
 	if (!indio_dev->buffer)
 		return -ENOMEM;
 
@@ -774,7 +774,7 @@ static int ad5933_probe(struct i2c_client *client,
 error_uninitialize_ring:
 	iio_buffer_unregister(indio_dev);
 error_unreg_ring:
-	iio_sw_rb_free(indio_dev->buffer);
+	iio_kfifo_free(indio_dev->buffer);
 error_disable_reg:
 	if (!IS_ERR(st->reg))
 		regulator_disable(st->reg);
@@ -794,7 +794,7 @@ static int ad5933_remove(struct i2c_client *client)
 
 	iio_device_unregister(indio_dev);
 	iio_buffer_unregister(indio_dev);
-	iio_sw_rb_free(indio_dev->buffer);
+	iio_kfifo_free(indio_dev->buffer);
 	if (!IS_ERR(st->reg)) {
 		regulator_disable(st->reg);
 		regulator_put(st->reg);
-- 
1.8.0.1

[PATCH 3/5] staging:iio:meter:ade7758 switch from sw_ring to kfifo

[Jonathan Cameron]

sw ring is going away so switch over to kfifo based buffer implementation.
The only real change is that poll will return on some data there rather than
buffer 50% full.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/meter/Kconfig        |  2 +-
 drivers/staging/iio/meter/ade7758_ring.c | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/meter/Kconfig b/drivers/staging/iio/meter/Kconfig
index d290d27..e53274b 100644
--- a/drivers/staging/iio/meter/Kconfig
+++ b/drivers/staging/iio/meter/Kconfig
@@ -21,7 +21,7 @@ config ADE7758
 	tristate "Analog Devices ADE7758 Poly Phase Multifunction Energy Metering IC Driver"
 	depends on SPI
 	select IIO_TRIGGER if IIO_BUFFER
-	select IIO_SW_RING if IIO_BUFFER
+	select IIO_KFIFO_BUF if IIO_BUFFER
 	help
 	  Say yes here to build support for Analog Devices ADE7758 Polyphase
 	  Multifunction Energy Metering IC with Per Phase Information Driver.
diff --git a/drivers/staging/iio/meter/ade7758_ring.c b/drivers/staging/iio/meter/ade7758_ring.c
index 4552a4c..b29e2d5 100644
--- a/drivers/staging/iio/meter/ade7758_ring.c
+++ b/drivers/staging/iio/meter/ade7758_ring.c
@@ -13,7 +13,7 @@
 #include <asm/unaligned.h>
 
 #include <linux/iio/iio.h>
-#include "../ring_sw.h"
+#include <linux/iio/kfifo_buf.h>
 #include <linux/iio/trigger_consumer.h>
 #include "ade7758.h"
 
@@ -119,7 +119,7 @@ static const struct iio_buffer_setup_ops ade7758_ring_setup_ops = {
 void ade7758_unconfigure_ring(struct iio_dev *indio_dev)
 {
 	iio_dealloc_pollfunc(indio_dev->pollfunc);
-	iio_sw_rb_free(indio_dev->buffer);
+	iio_kfifo_free(indio_dev->buffer);
 }
 
 int ade7758_configure_ring(struct iio_dev *indio_dev)
@@ -127,7 +127,7 @@ int ade7758_configure_ring(struct iio_dev *indio_dev)
 	struct ade7758_state *st = iio_priv(indio_dev);
 	int ret = 0;
 
-	indio_dev->buffer = iio_sw_rb_allocate(indio_dev);
+	indio_dev->buffer = iio_kfifo_allocate(indio_dev);
 	if (!indio_dev->buffer) {
 		ret = -ENOMEM;
 		return ret;
@@ -143,7 +143,7 @@ int ade7758_configure_ring(struct iio_dev *indio_dev)
 						 indio_dev->id);
 	if (indio_dev->pollfunc == NULL) {
 		ret = -ENOMEM;
-		goto error_iio_sw_rb_free;
+		goto error_iio_kfifo_free;
 	}
 
 	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
@@ -183,8 +183,8 @@ int ade7758_configure_ring(struct iio_dev *indio_dev)
 
 	return 0;
 
-error_iio_sw_rb_free:
-	iio_sw_rb_free(indio_dev->buffer);
+error_iio_kfifo_free:
+	iio_kfifo_free(indio_dev->buffer);
 	return ret;
 }
 
-- 
1.8.0.1

[PATCH 4/5] staging:iio:imu: adis16400 switch from sw_ring to kfifo

[Jonathan Cameron]

sw ring is going away so switch over to kfifo based buffer implementation.
The only real change is that poll will return on some data there rather than
buffer 50% full.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/imu/Kconfig          |  2 +-
 drivers/staging/iio/imu/adis16400_ring.c | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/iio/imu/Kconfig b/drivers/staging/iio/imu/Kconfig
index 2c2f47d..b9d9c14 100644
--- a/drivers/staging/iio/imu/Kconfig
+++ b/drivers/staging/iio/imu/Kconfig
@@ -6,7 +6,7 @@ menu "Inertial measurement units"
 config ADIS16400
 	tristate "Analog Devices ADIS16400 and similar IMU SPI driver"
 	depends on SPI
-	select IIO_SW_RING if IIO_BUFFER
+	select IIO_KFIFO_BUF if IIO_BUFFER
 	select IIO_TRIGGER if IIO_BUFFER
 	help
 	  Say yes here to build support for Analog Devices adis16300, adis16344,
diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c
index d46c1e3..319c2c0 100644
--- a/drivers/staging/iio/imu/adis16400_ring.c
+++ b/drivers/staging/iio/imu/adis16400_ring.c
@@ -7,7 +7,7 @@
 #include <linux/export.h>
 
 #include <linux/iio/iio.h>
-#include "../ring_sw.h"
+#include <linux/iio/kfifo_buf.h>
 #include <linux/iio/trigger_consumer.h>
 #include "adis16400.h"
 
@@ -161,7 +161,7 @@ done:
 void adis16400_unconfigure_ring(struct iio_dev *indio_dev)
 {
 	iio_dealloc_pollfunc(indio_dev->pollfunc);
-	iio_sw_rb_free(indio_dev->buffer);
+	iio_kfifo_free(indio_dev->buffer);
 }
 
 static const struct iio_buffer_setup_ops adis16400_ring_setup_ops = {
@@ -175,7 +175,7 @@ int adis16400_configure_ring(struct iio_dev *indio_dev)
 	int ret = 0;
 	struct iio_buffer *ring;
 
-	ring = iio_sw_rb_allocate(indio_dev);
+	ring = iio_kfifo_allocate(indio_dev);
 	if (!ring) {
 		ret = -ENOMEM;
 		return ret;
@@ -193,12 +193,12 @@ int adis16400_configure_ring(struct iio_dev *indio_dev)
 						 indio_dev->id);
 	if (indio_dev->pollfunc == NULL) {
 		ret = -ENOMEM;
-		goto error_iio_sw_rb_free;
+		goto error_iio_kfifo_free;
 	}
 
 	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
 	return 0;
-error_iio_sw_rb_free:
-	iio_sw_rb_free(indio_dev->buffer);
+error_iio_kfifo_free:
+	iio_kfifo_free(indio_dev->buffer);
 	return ret;
 }
-- 
1.8.0.1

[PATCH 5/5] staging:iio: drop sw_ring buffer implementation.

[Jonathan Cameron]

Whilst this is IIO's oldest buffer implementation it is messy, poorly
implemented and whilst it works, no one is entirely sure it always will.

New IIO drivers have not been using this for some time and now all remaining
old users have been converted to use the kfifo based alternative.

Clearly a fifo isn't the same as a ring buffer but in many use cases it
really doesn't matter.  We also loose the watershed based poll implementation.
However having poll effectively report data only when the buffer was half
full was at best an 'unusual' use of the interface.

At somepoint in the future we may bring watersheds back on a different
buffer implementation, but then we will think a lot more about how to do
the interface first.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/Kconfig   |  13 --
 drivers/staging/iio/Makefile  |   2 -
 drivers/staging/iio/ring_sw.c | 366 ------------------------------------------
 drivers/staging/iio/ring_sw.h |  30 ----
 4 files changed, 411 deletions(-)

diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
index ca56c75..d7f9929 100644
--- a/drivers/staging/iio/Kconfig
+++ b/drivers/staging/iio/Kconfig
@@ -12,19 +12,6 @@ config IIO_ST_HWMON
 	  map allows IIO devices to provide  basic hwmon functionality
 	  for those channels specified in the map.
 
-if IIO_BUFFER
-
-config IIO_SW_RING
-       select IIO_TRIGGER
-	tristate "Industrial I/O lock free software ring"
-	help
-	  Example software ring buffer implementation.  The design aim
-	  of this particular realization was to minimize write locking
-	  with the intention that some devices would be able to write
-	  in interrupt context.
-
-endif # IIO_BUFFER
-
 source "drivers/staging/iio/accel/Kconfig"
 source "drivers/staging/iio/adc/Kconfig"
 source "drivers/staging/iio/addac/Kconfig"
diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
index fa6937d..12984da 100644
--- a/drivers/staging/iio/Makefile
+++ b/drivers/staging/iio/Makefile
@@ -2,8 +2,6 @@
 # Makefile for the industrial I/O core.
 #
 
-obj-$(CONFIG_IIO_SW_RING) += ring_sw.o
-
 obj-$(CONFIG_IIO_SIMPLE_DUMMY) += iio_dummy.o
 iio_dummy-y := iio_simple_dummy.o
 iio_dummy-$(CONFIG_IIO_SIMPLE_DUMMY_EVENTS) += iio_simple_dummy_events.o
diff --git a/drivers/staging/iio/ring_sw.c b/drivers/staging/iio/ring_sw.c
deleted file mode 100644
index 3a45f9a..0000000
--- a/drivers/staging/iio/ring_sw.c
+++ /dev/null
@@ -1,366 +0,0 @@
-/* The industrial I/O simple minimally locked ring buffer.
- *
- * Copyright (c) 2008 Jonathan Cameron
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published by
- * the Free Software Foundation.
- */
-
-#include <linux/slab.h>
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/device.h>
-#include <linux/workqueue.h>
-#include <linux/sched.h>
-#include <linux/poll.h>
-#include "ring_sw.h"
-#include <linux/iio/trigger.h>
-
-/**
- * struct iio_sw_ring_buffer - software ring buffer
- * @buf:		generic ring buffer elements
- * @data:		the ring buffer memory
- * @read_p:		read pointer (oldest available)
- * @write_p:		write pointer
- * @half_p:		half buffer length behind write_p (event generation)
- * @update_needed:	flag to indicate change in size requested
- *
- * Note that the first element of all ring buffers must be a
- * struct iio_buffer.
-**/
-struct iio_sw_ring_buffer {
-	struct iio_buffer  buf;
-	unsigned char		*data;
-	unsigned char		*read_p;
-	unsigned char		*write_p;
-	/* used to act as a point at which to signal an event */
-	unsigned char		*half_p;
-	int			update_needed;
-};
-
-#define iio_to_sw_ring(r) container_of(r, struct iio_sw_ring_buffer, buf)
-
-static inline int __iio_allocate_sw_ring_buffer(struct iio_sw_ring_buffer *ring,
-						int bytes_per_datum, int length)
-{
-	if ((length == 0) || (bytes_per_datum == 0))
-		return -EINVAL;
-	__iio_update_buffer(&ring->buf, bytes_per_datum, length);
-	ring->data = kmalloc(length*ring->buf.bytes_per_datum, GFP_ATOMIC);
-	ring->read_p = NULL;
-	ring->write_p = NULL;
-	ring->half_p = NULL;
-	return ring->data ? 0 : -ENOMEM;
-}
-
-static inline void __iio_free_sw_ring_buffer(struct iio_sw_ring_buffer *ring)
-{
-	kfree(ring->data);
-}
-
-/* Ring buffer related functionality */
-/* Store to ring is typically called in the bh of a data ready interrupt handler
- * in the device driver */
-/* Lock always held if their is a chance this may be called */
-/* Only one of these per ring may run concurrently - enforced by drivers */
-static int iio_store_to_sw_ring(struct iio_sw_ring_buffer *ring,
-				unsigned char *data)
-{
-	int ret = 0;
-	unsigned char *temp_ptr, *change_test_ptr;
-
-	/* initial store */
-	if (unlikely(ring->write_p == NULL)) {
-		ring->write_p = ring->data;
-		/* Doesn't actually matter if this is out of the set
-		 * as long as the read pointer is valid before this
-		 * passes it - guaranteed as set later in this function.
-		 */
-		ring->half_p = ring->data - ring->buf.length*ring->buf.bytes_per_datum/2;
-	}
-	/* Copy data to where ever the current write pointer says */
-	memcpy(ring->write_p, data, ring->buf.bytes_per_datum);
-	barrier();
-	/* Update the pointer used to get most recent value.
-	 * Always valid as either points to latest or second latest value.
-	 * Before this runs it is null and read attempts fail with -EAGAIN.
-	 */
-	barrier();
-	/* temp_ptr used to ensure we never have an invalid pointer
-	 * it may be slightly lagging, but never invalid
-	 */
-	temp_ptr = ring->write_p + ring->buf.bytes_per_datum;
-	/* End of ring, back to the beginning */
-	if (temp_ptr == ring->data + ring->buf.length*ring->buf.bytes_per_datum)
-		temp_ptr = ring->data;
-	/* Update the write pointer
-	 * always valid as long as this is the only function able to write.
-	 * Care needed with smp systems to ensure more than one ring fill
-	 * is never scheduled.
-	 */
-	ring->write_p = temp_ptr;
-
-	if (ring->read_p == NULL)
-		ring->read_p = ring->data;
-	/* Buffer full - move the read pointer and create / escalate
-	 * ring event */
-	/* Tricky case - if the read pointer moves before we adjust it.
-	 * Handle by not pushing if it has moved - may result in occasional
-	 * unnecessary buffer full events when it wasn't quite true.
-	 */
-	else if (ring->write_p == ring->read_p) {
-		change_test_ptr = ring->read_p;
-		temp_ptr = change_test_ptr + ring->buf.bytes_per_datum;
-		if (temp_ptr
-		    == ring->data + ring->buf.length*ring->buf.bytes_per_datum) {
-			temp_ptr = ring->data;
-		}
-		/* We are moving pointer on one because the ring is full.  Any
-		 * change to the read pointer will be this or greater.
-		 */
-		if (change_test_ptr == ring->read_p)
-			ring->read_p = temp_ptr;
-	}
-	/* investigate if our event barrier has been passed */
-	/* There are definite 'issues' with this and chances of
-	 * simultaneous read */
-	/* Also need to use loop count to ensure this only happens once */
-	ring->half_p += ring->buf.bytes_per_datum;
-	if (ring->half_p == ring->data + ring->buf.length*ring->buf.bytes_per_datum)
-		ring->half_p = ring->data;
-	if (ring->half_p == ring->read_p) {
-		ring->buf.stufftoread = true;
-		wake_up_interruptible(&ring->buf.pollq);
-	}
-	return ret;
-}
-
-static int iio_read_first_n_sw_rb(struct iio_buffer *r,
-				  size_t n, char __user *buf)
-{
-	struct iio_sw_ring_buffer *ring = iio_to_sw_ring(r);
-
-	u8 *initial_read_p, *initial_write_p, *current_read_p, *end_read_p;
-	u8 *data;
-	int ret, max_copied, bytes_to_rip, dead_offset;
-	size_t data_available, buffer_size;
-
-	/* A userspace program has probably made an error if it tries to
-	 * read something that is not a whole number of bpds.
-	 * Return an error.
-	 */
-	if (n % ring->buf.bytes_per_datum) {
-		ret = -EINVAL;
-		printk(KERN_INFO "Ring buffer read request not whole number of"
-		       "samples: Request bytes %zd, Current bytes per datum %d\n",
-		       n, ring->buf.bytes_per_datum);
-		goto error_ret;
-	}
-
-	buffer_size = ring->buf.bytes_per_datum*ring->buf.length;
-
-	/* Limit size to whole of ring buffer */
-	bytes_to_rip = min_t(size_t, buffer_size, n);
-
-	data = kmalloc(bytes_to_rip, GFP_KERNEL);
-	if (data == NULL) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
-
-	/* build local copy */
-	initial_read_p = ring->read_p;
-	if (unlikely(initial_read_p == NULL)) { /* No data here as yet */
-		ret = 0;
-		goto error_free_data_cpy;
-	}
-
-	initial_write_p = ring->write_p;
-
-	/* Need a consistent pair */
-	while ((initial_read_p != ring->read_p)
-	       || (initial_write_p != ring->write_p)) {
-		initial_read_p = ring->read_p;
-		initial_write_p = ring->write_p;
-	}
-	if (initial_write_p == initial_read_p) {
-		/* No new data available.*/
-		ret = 0;
-		goto error_free_data_cpy;
-	}
-
-	if (initial_write_p >= initial_read_p)
-		data_available = initial_write_p - initial_read_p;
-	else
-		data_available = buffer_size - (initial_read_p - initial_write_p);
-
-	if (data_available < bytes_to_rip)
-		bytes_to_rip = data_available;
-
-	if (initial_read_p + bytes_to_rip >= ring->data + buffer_size) {
-		max_copied = ring->data + buffer_size - initial_read_p;
-		memcpy(data, initial_read_p, max_copied);
-		memcpy(data + max_copied, ring->data, bytes_to_rip - max_copied);
-		end_read_p = ring->data + bytes_to_rip - max_copied;
-	} else {
-		memcpy(data, initial_read_p, bytes_to_rip);
-		end_read_p = initial_read_p + bytes_to_rip;
-	}
-
-	/* Now to verify which section was cleanly copied - i.e. how far
-	 * read pointer has been pushed */
-	current_read_p = ring->read_p;
-
-	if (initial_read_p <= current_read_p)
-		dead_offset = current_read_p - initial_read_p;
-	else
-		dead_offset = buffer_size - (initial_read_p - current_read_p);
-
-	/* possible issue if the initial write has been lapped or indeed
-	 * the point we were reading to has been passed */
-	/* No valid data read.
-	 * In this case the read pointer is already correct having been
-	 * pushed further than we would look. */
-	if (bytes_to_rip - dead_offset < 0) {
-		ret = 0;
-		goto error_free_data_cpy;
-	}
-
-	/* setup the next read position */
-	/* Beware, this may fail due to concurrency fun and games.
-	 * Possible that sufficient fill commands have run to push the read
-	 * pointer past where we would be after the rip. If this occurs, leave
-	 * it be.
-	 */
-	/* Tricky - deal with loops */
-
-	while (ring->read_p != end_read_p)
-		ring->read_p = end_read_p;
-
-	ret = bytes_to_rip - dead_offset;
-
-	if (copy_to_user(buf, data + dead_offset, ret))  {
-		ret =  -EFAULT;
-		goto error_free_data_cpy;
-	}
-
-	if (bytes_to_rip >= ring->buf.length*ring->buf.bytes_per_datum/2)
-		ring->buf.stufftoread = 0;
-
-error_free_data_cpy:
-	kfree(data);
-error_ret:
-
-	return ret;
-}
-
-static int iio_store_to_sw_rb(struct iio_buffer *r,
-			      u8 *data)
-{
-	struct iio_sw_ring_buffer *ring = iio_to_sw_ring(r);
-	return iio_store_to_sw_ring(ring, data);
-}
-
-static int iio_request_update_sw_rb(struct iio_buffer *r)
-{
-	int ret = 0;
-	struct iio_sw_ring_buffer *ring = iio_to_sw_ring(r);
-
-	r->stufftoread = false;
-	if (!ring->update_needed)
-		goto error_ret;
-	__iio_free_sw_ring_buffer(ring);
-	ret = __iio_allocate_sw_ring_buffer(ring, ring->buf.bytes_per_datum,
-					    ring->buf.length);
-error_ret:
-	return ret;
-}
-
-static int iio_get_bytes_per_datum_sw_rb(struct iio_buffer *r)
-{
-	struct iio_sw_ring_buffer *ring = iio_to_sw_ring(r);
-	return ring->buf.bytes_per_datum;
-}
-
-static int iio_mark_update_needed_sw_rb(struct iio_buffer *r)
-{
-	struct iio_sw_ring_buffer *ring = iio_to_sw_ring(r);
-	ring->update_needed = true;
-	return 0;
-}
-
-static int iio_set_bytes_per_datum_sw_rb(struct iio_buffer *r, size_t bpd)
-{
-	if (r->bytes_per_datum != bpd) {
-		r->bytes_per_datum = bpd;
-		iio_mark_update_needed_sw_rb(r);
-	}
-	return 0;
-}
-
-static int iio_get_length_sw_rb(struct iio_buffer *r)
-{
-	return r->length;
-}
-
-static int iio_set_length_sw_rb(struct iio_buffer *r, int length)
-{
-	if (r->length != length) {
-		r->length = length;
-		iio_mark_update_needed_sw_rb(r);
-	}
-	return 0;
-}
-
-static IIO_BUFFER_ENABLE_ATTR;
-static IIO_BUFFER_LENGTH_ATTR;
-
-/* Standard set of ring buffer attributes */
-static struct attribute *iio_ring_attributes[] = {
-	&dev_attr_length.attr,
-	&dev_attr_enable.attr,
-	NULL,
-};
-
-static struct attribute_group iio_ring_attribute_group = {
-	.attrs = iio_ring_attributes,
-	.name = "buffer",
-};
-
-static const struct iio_buffer_access_funcs ring_sw_access_funcs = {
-	.store_to = &iio_store_to_sw_rb,
-	.read_first_n = &iio_read_first_n_sw_rb,
-	.request_update = &iio_request_update_sw_rb,
-	.get_bytes_per_datum = &iio_get_bytes_per_datum_sw_rb,
-	.set_bytes_per_datum = &iio_set_bytes_per_datum_sw_rb,
-	.get_length = &iio_get_length_sw_rb,
-	.set_length = &iio_set_length_sw_rb,
-};
-
-struct iio_buffer *iio_sw_rb_allocate(struct iio_dev *indio_dev)
-{
-	struct iio_buffer *buf;
-	struct iio_sw_ring_buffer *ring;
-
-	ring = kzalloc(sizeof *ring, GFP_KERNEL);
-	if (!ring)
-		return NULL;
-	ring->update_needed = true;
-	buf = &ring->buf;
-	iio_buffer_init(buf);
-	buf->attrs = &iio_ring_attribute_group;
-	buf->access = &ring_sw_access_funcs;
-
-	return buf;
-}
-EXPORT_SYMBOL(iio_sw_rb_allocate);
-
-void iio_sw_rb_free(struct iio_buffer *r)
-{
-	kfree(iio_to_sw_ring(r));
-}
-EXPORT_SYMBOL(iio_sw_rb_free);
-
-MODULE_DESCRIPTION("Industrial I/O software ring buffer");
-MODULE_LICENSE("GPL");
diff --git a/drivers/staging/iio/ring_sw.h b/drivers/staging/iio/ring_sw.h
deleted file mode 100644
index a5857aa..0000000
--- a/drivers/staging/iio/ring_sw.h
+++ /dev/null
@@ -1,30 +0,0 @@
-/* The industrial I/O simple minimally locked ring buffer.
- *
- * Copyright (c) 2008 Jonathan Cameron
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 as published by
- * the Free Software Foundation.
- *
- * This code is deliberately kept separate from the main industrialio I/O core
- * as it is intended that in the future a number of different software ring
- * buffer implementations will exist with different characteristics to suit
- * different applications.
- *
- * This particular one was designed for a data capture application where it was
- * particularly important that no userspace reads would interrupt the capture
- * process. To this end the ring is not locked during a read.
- *
- * Comments on this buffer design welcomed. It's far from efficient and some of
- * my understanding of the effects of scheduling on this are somewhat limited.
- * Frankly, to my mind, this is the current weak point in the industrial I/O
- * patch set.
- */
-
-#ifndef _IIO_RING_SW_H_
-#define _IIO_RING_SW_H_
-#include <linux/iio/buffer.h>
-
-struct iio_buffer *iio_sw_rb_allocate(struct iio_dev *indio_dev);
-void iio_sw_rb_free(struct iio_buffer *ring);
-#endif /* _IIO_RING_SW_H_ */
-- 
1.8.0.1

Re: [PATCH 4/5] staging:iio:imu: adis16400 switch from sw_ring to kfifo

[Lars-Peter Clausen]

On 11/30/2012 03:22 PM, Jonathan Cameron wrote:
> sw ring is going away so switch over to kfifo based buffer implementation.
> The only real change is that poll will return on some data there rather than
> buffer 50% full.
> 
> Signed-off-by: Jonathan Cameron <jic23@kernel.org>

Is is ok for you to drop this one and wait with the last patch in the series
still a bit? I've patches to switch this driver over to the adislib and this
patch would probably introduce a couple of conflicts.

The rest of the series looks good.

Thanks,
- Lars

> ---
>  drivers/staging/iio/imu/Kconfig          |  2 +-
>  drivers/staging/iio/imu/adis16400_ring.c | 12 ++++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/iio/imu/Kconfig b/drivers/staging/iio/imu/Kconfig
> index 2c2f47d..b9d9c14 100644
> --- a/drivers/staging/iio/imu/Kconfig
> +++ b/drivers/staging/iio/imu/Kconfig
> @@ -6,7 +6,7 @@ menu "Inertial measurement units"
>  config ADIS16400
>  	tristate "Analog Devices ADIS16400 and similar IMU SPI driver"
>  	depends on SPI
> -	select IIO_SW_RING if IIO_BUFFER
> +	select IIO_KFIFO_BUF if IIO_BUFFER
>  	select IIO_TRIGGER if IIO_BUFFER
>  	help
>  	  Say yes here to build support for Analog Devices adis16300, adis16344,
> diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c
> index d46c1e3..319c2c0 100644
> --- a/drivers/staging/iio/imu/adis16400_ring.c
> +++ b/drivers/staging/iio/imu/adis16400_ring.c
> @@ -7,7 +7,7 @@
>  #include <linux/export.h>
>  
>  #include <linux/iio/iio.h>
> -#include "../ring_sw.h"
> +#include <linux/iio/kfifo_buf.h>
>  #include <linux/iio/trigger_consumer.h>
>  #include "adis16400.h"
>  
> @@ -161,7 +161,7 @@ done:
>  void adis16400_unconfigure_ring(struct iio_dev *indio_dev)
>  {
>  	iio_dealloc_pollfunc(indio_dev->pollfunc);
> -	iio_sw_rb_free(indio_dev->buffer);
> +	iio_kfifo_free(indio_dev->buffer);
>  }
>  
>  static const struct iio_buffer_setup_ops adis16400_ring_setup_ops = {
> @@ -175,7 +175,7 @@ int adis16400_configure_ring(struct iio_dev *indio_dev)
>  	int ret = 0;
>  	struct iio_buffer *ring;
>  
> -	ring = iio_sw_rb_allocate(indio_dev);
> +	ring = iio_kfifo_allocate(indio_dev);
>  	if (!ring) {
>  		ret = -ENOMEM;
>  		return ret;
> @@ -193,12 +193,12 @@ int adis16400_configure_ring(struct iio_dev *indio_dev)
>  						 indio_dev->id);
>  	if (indio_dev->pollfunc == NULL) {
>  		ret = -ENOMEM;
> -		goto error_iio_sw_rb_free;
> +		goto error_iio_kfifo_free;
>  	}
>  
>  	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
>  	return 0;
> -error_iio_sw_rb_free:
> -	iio_sw_rb_free(indio_dev->buffer);
> +error_iio_kfifo_free:
> +	iio_kfifo_free(indio_dev->buffer);
>  	return ret;
>  }

Re: [PATCH 4/5] staging:iio:imu: adis16400 switch from sw_ring to kfifo

[Jonathan Cameron]

On 11/30/2012 02:35 PM, Lars-Peter Clausen wrote:
> On 11/30/2012 03:22 PM, Jonathan Cameron wrote:
>> sw ring is going away so switch over to kfifo based buffer implementation.
>> The only real change is that poll will return on some data there rather than
>> buffer 50% full.
>>
>> Signed-off-by: Jonathan Cameron <jic23@kernel.org>
> 
> Is is ok for you to drop this one and wait with the last patch in the series
> still a bit? I've patches to switch this driver over to the adislib and this
> patch would probably introduce a couple of conflicts.
> 
> The rest of the series looks good.
*laughs* I guessed as much, but decided it was quicker to write the patch and
have you request I hold it than to send you an email to ask if you had such a
patch and then wait for the response.

Should have known you'd get back almost instantly.

These are very much for the next cycle, so I am happy to do as you
requested!


> 
> Thanks,
> - Lars
> 
>> ---
>>  drivers/staging/iio/imu/Kconfig          |  2 +-
>>  drivers/staging/iio/imu/adis16400_ring.c | 12 ++++++------
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/staging/iio/imu/Kconfig b/drivers/staging/iio/imu/Kconfig
>> index 2c2f47d..b9d9c14 100644
>> --- a/drivers/staging/iio/imu/Kconfig
>> +++ b/drivers/staging/iio/imu/Kconfig
>> @@ -6,7 +6,7 @@ menu "Inertial measurement units"
>>  config ADIS16400
>>  	tristate "Analog Devices ADIS16400 and similar IMU SPI driver"
>>  	depends on SPI
>> -	select IIO_SW_RING if IIO_BUFFER
>> +	select IIO_KFIFO_BUF if IIO_BUFFER
>>  	select IIO_TRIGGER if IIO_BUFFER
>>  	help
>>  	  Say yes here to build support for Analog Devices adis16300, adis16344,
>> diff --git a/drivers/staging/iio/imu/adis16400_ring.c b/drivers/staging/iio/imu/adis16400_ring.c
>> index d46c1e3..319c2c0 100644
>> --- a/drivers/staging/iio/imu/adis16400_ring.c
>> +++ b/drivers/staging/iio/imu/adis16400_ring.c
>> @@ -7,7 +7,7 @@
>>  #include <linux/export.h>
>>  
>>  #include <linux/iio/iio.h>
>> -#include "../ring_sw.h"
>> +#include <linux/iio/kfifo_buf.h>
>>  #include <linux/iio/trigger_consumer.h>
>>  #include "adis16400.h"
>>  
>> @@ -161,7 +161,7 @@ done:
>>  void adis16400_unconfigure_ring(struct iio_dev *indio_dev)
>>  {
>>  	iio_dealloc_pollfunc(indio_dev->pollfunc);
>> -	iio_sw_rb_free(indio_dev->buffer);
>> +	iio_kfifo_free(indio_dev->buffer);
>>  }
>>  
>>  static const struct iio_buffer_setup_ops adis16400_ring_setup_ops = {
>> @@ -175,7 +175,7 @@ int adis16400_configure_ring(struct iio_dev *indio_dev)
>>  	int ret = 0;
>>  	struct iio_buffer *ring;
>>  
>> -	ring = iio_sw_rb_allocate(indio_dev);
>> +	ring = iio_kfifo_allocate(indio_dev);
>>  	if (!ring) {
>>  		ret = -ENOMEM;
>>  		return ret;
>> @@ -193,12 +193,12 @@ int adis16400_configure_ring(struct iio_dev *indio_dev)
>>  						 indio_dev->id);
>>  	if (indio_dev->pollfunc == NULL) {
>>  		ret = -ENOMEM;
>> -		goto error_iio_sw_rb_free;
>> +		goto error_iio_kfifo_free;
>>  	}
>>  
>>  	indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
>>  	return 0;
>> -error_iio_sw_rb_free:
>> -	iio_sw_rb_free(indio_dev->buffer);
>> +error_iio_kfifo_free:
>> +	iio_kfifo_free(indio_dev->buffer);
>>  	return ret;
>>  }
> 
> --
> 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
> 

Re: [PATCH 0/5] staging:iio: drop sw_ring buffer implementation.

[Jonathan Cameron]

Added all but patch 4 to the togreg branch of
iio.git given the only reason this was held was the just merged
series cleaning up the adis16400 driver (which included as a
side effect removing the code patch 4 dealt with).

On 11/30/2012 02:22 PM, Jonathan Cameron wrote:
> sw_ring was the original buffer implementation for IIO. It kind of evolved
> over a couple of late nights and even though I wrote it I was never
> convinced it was right.  Then a nice kfifo implementation came along
> and in the vast majority of IIO use cases whether we have a kfifo or
> a ring buffer is not as it turns out that important.
> 
> We have been telling people not to use this for quite some time now.
> Lets make that finally a really hard rule by dropping it.
> There are 4 remaining users. This patch drops support form lis3l02dq
> which had support for both this and kfifo.  For the other 3 users
> it simply switches over to kfifo.  We haven't seen any real problems
> doing this with any other driver so I can't see why these would cause
> trouble!
> 
> Note my aim for this cycle ist to greatly cut down what we have left
> in staging by removing legacy cruft and getting other stuff cleaned up and
> out!
> 
> Jonathan
> 
> Jonathan Cameron (5):
>   staging:iio:accel:lis3l02dq remove sw_ring support.
>   staging:iio:impedance-analyzer switch from sw_ring to kfifo.
>   staging:iio:meter:ade7758 switch from sw_ring to kfifo
>   staging:iio:imu: adis16400 switch from sw_ring to kfifo
>   staging:iio: drop sw_ring buffer implementation.
> 
>  drivers/staging/iio/Kconfig                     |  13 -
>  drivers/staging/iio/Makefile                    |   2 -
>  drivers/staging/iio/accel/Kconfig               |  23 +-
>  drivers/staging/iio/accel/lis3l02dq.h           |   8 -
>  drivers/staging/iio/accel/lis3l02dq_ring.c      |   7 +-
>  drivers/staging/iio/impedance-analyzer/Kconfig  |   2 +-
>  drivers/staging/iio/impedance-analyzer/ad5933.c |   8 +-
>  drivers/staging/iio/imu/Kconfig                 |   2 +-
>  drivers/staging/iio/imu/adis16400_ring.c        |  12 +-
>  drivers/staging/iio/meter/Kconfig               |   2 +-
>  drivers/staging/iio/meter/ade7758_ring.c        |  12 +-
>  drivers/staging/iio/ring_sw.c                   | 366 ------------------------
>  drivers/staging/iio/ring_sw.h                   |  30 --
>  13 files changed, 23 insertions(+), 464 deletions(-)
>  delete mode 100644 drivers/staging/iio/ring_sw.c
>  delete mode 100644 drivers/staging/iio/ring_sw.h
>