[RFC PATCH 0/2] iio: kfifo: Add resource management alloc/free

[RFC PATCH 0/2] iio: kfifo: Add resource management alloc/free

[Karol Wrona]

This is RFC because I felt a bit confused while devm_alloc/free adding.
It applies on next but I've tested it partialy on 3.16.  It can cause some
problemsi with several drivers so it does not fit for now. 

iio kfifo alloc takes one argument struct iio_dev * which is not used.
iio_buffer is attached by external function.  In such situation resource 
management alloc can be implemented in two ways:
1. indio_dev ptr in iio_kfifo_alloc stays and devm_ functions have to
provide it but it is still unused.
2.  As there are a few drivers which use kfifo it can be removed and 
iio_kfifo_allocate refactored.

I apologise if I missed something and it was done in such way intentionally.



Karol Wrona (2):
  iio: kfifo: Remove unused argument in iio_kfifo_allocate
  iio: kfifo: Add resource management devm_iio_kfifo_allocate/free

 drivers/iio/kfifo_buf.c       |   58 ++++++++++++++++++++++++++++++++++++++++-
 include/linux/iio/kfifo_buf.h |    5 +++-
 2 files changed, 61 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[RFC PATCH 1/2] iio: kfifo: Remove unused argument in iio_kfifo_allocate

[Karol Wrona]

indio_dev was unused in function body plus some small style fix - add new
lines after "if(sth) return sth" and before the last return statement.

Change-Id: I9495c778dc7bc87cc4503e890397b576966c44fd
Signed-off-by: Karol Wrona <k.wrona@samsung.com>
---
 drivers/iio/kfifo_buf.c       |    4 +++-
 include/linux/iio/kfifo_buf.h |    2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 7134e8a..5b5387c 100644
--- a/drivers/iio/kfifo_buf.c
+++ b/drivers/iio/kfifo_buf.c
@@ -166,19 +166,21 @@ static const struct iio_buffer_access_funcs kfifo_access_funcs = {
 	.release = &iio_kfifo_buffer_release,
 };
 
-struct iio_buffer *iio_kfifo_allocate(struct iio_dev *indio_dev)
+struct iio_buffer *iio_kfifo_allocate(void)
 {
 	struct iio_kfifo *kf;
 
 	kf = kzalloc(sizeof *kf, GFP_KERNEL);
 	if (!kf)
 		return NULL;
+
 	kf->update_needed = true;
 	iio_buffer_init(&kf->buffer);
 	kf->buffer.attrs = &iio_kfifo_attribute_group;
 	kf->buffer.access = &kfifo_access_funcs;
 	kf->buffer.length = 2;
 	mutex_init(&kf->user_lock);
+
 	return &kf->buffer;
 }
 EXPORT_SYMBOL(iio_kfifo_allocate);
diff --git a/include/linux/iio/kfifo_buf.h b/include/linux/iio/kfifo_buf.h
index 25eeac7..1a8d57a 100644
--- a/include/linux/iio/kfifo_buf.h
+++ b/include/linux/iio/kfifo_buf.h
@@ -5,7 +5,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
 
-struct iio_buffer *iio_kfifo_allocate(struct iio_dev *indio_dev);
+struct iio_buffer *iio_kfifo_allocate(void);
 void iio_kfifo_free(struct iio_buffer *r);
 
 #endif
-- 
1.7.9.5

Re: [RFC PATCH 1/2] iio: kfifo: Remove unused argument in iio_kfifo_allocate

[Lars-Peter Clausen]

On 12/09/2014 08:29 PM, Karol Wrona wrote:
> indio_dev was unused in function body plus some small style fix - add new
> lines after "if(sth) return sth" and before the last return statement.

Looks good, except for the missing updates for the users.

Doing some digging in the git history raveled that the user of that 
parameter in that function was removed in commit f79a909890 
("staging:iio:buffer struct iio_buffer doesn't need an indio_dev pointer.")

[RFC PATCH 2/2] iio: kfifo: Add resource management devm_iio_kfifo_allocate/free

[Karol Wrona]

iio kfifo allocate/free gained their devm_ wrappers.

Change-Id: I10c19ccd7c01491caf088b3629137425ddccd29c
Signed-off-by: Karol Wrona <k.wrona@samsung.com>
Suggested-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/iio/kfifo_buf.c       |   54 +++++++++++++++++++++++++++++++++++++++++
 include/linux/iio/kfifo_buf.h |    3 +++
 2 files changed, 57 insertions(+)

diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 5b5387c..03a133f 100644
--- a/drivers/iio/kfifo_buf.c
+++ b/drivers/iio/kfifo_buf.c
@@ -191,4 +191,58 @@ void iio_kfifo_free(struct iio_buffer *r)
 }
 EXPORT_SYMBOL(iio_kfifo_free);
 
+static void devm_iio_kfifo_release(struct device *dev, void *res)
+{
+	iio_kfifo_free(*(struct iio_buffer **)res);
+}
+
+static int devm_iio_kfifo_match(struct device *dev, void *res, void *data)
+{
+	struct iio_buffer **r = res;
+
+	if (WARN_ON(!r || !*r))
+		return 0;
+
+	return *r == data;
+}
+
+/**
+ * devm_iio_fifo_allocate - Resource-managed iio_kfifo_allocate()
+ * @dev:		Device to allocate kfifo buffer for
+ *
+ * RETURNS:
+ * Pointer to allocated iio_buffer on success, NULL on failure.
+ */
+struct iio_buffer *devm_iio_kfifo_allocate(struct device *dev)
+{
+	struct iio_buffer **ptr, *r;
+
+	ptr = devres_alloc(devm_iio_kfifo_release, sizeof *ptr, GFP_KERNEL);
+	if (!ptr)
+		return NULL;
+
+	r = iio_kfifo_allocate();
+	if (r) {
+		*ptr = r;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return r;
+}
+EXPORT_SYMBOL(devm_iio_kfifo_allocate);
+
+/**
+ * devm_iio_fifo_free - Resource-managed iio_kfifo_free()
+ * @dev:		Device the buffer belongs to
+ * @r:			the buffer associated with the device
+ */
+void devm_iio_kfifo_free(struct device *dev, struct iio_buffer *r)
+{
+	WARN_ON(devres_release(dev, devm_iio_kfifo_release,
+			       devm_iio_kfifo_match, r));
+}
+EXPORT_SYMBOL(devm_iio_kfifo_free);
+
 MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/kfifo_buf.h b/include/linux/iio/kfifo_buf.h
index 1a8d57a..1683bc7 100644
--- a/include/linux/iio/kfifo_buf.h
+++ b/include/linux/iio/kfifo_buf.h
@@ -8,4 +8,7 @@
 struct iio_buffer *iio_kfifo_allocate(void);
 void iio_kfifo_free(struct iio_buffer *r);
 
+struct iio_buffer *devm_iio_kfifo_allocate(struct device *dev);
+void devm_iio_kfifo_free(struct device *dev, struct iio_buffer *r);
+
 #endif
-- 
1.7.9.5

Re: [RFC PATCH 2/2] iio: kfifo: Add resource management devm_iio_kfifo_allocate/free

[Lars-Peter Clausen]

On 12/09/2014 08:29 PM, Karol Wrona wrote:
> iio kfifo allocate/free gained their devm_ wrappers.
>
> Change-Id: I10c19ccd7c01491caf088b3629137425ddccd29c
> Signed-off-by: Karol Wrona <k.wrona@samsung.com>
> Suggested-by: Jonathan Cameron <jic23@kernel.org>

Looks good to me.

[...]
> +struct iio_buffer *devm_iio_kfifo_allocate(struct device *dev)
> +{
> +	struct iio_buffer **ptr, *r;
> +
> +	ptr = devres_alloc(devm_iio_kfifo_release, sizeof *ptr, GFP_KERNEL);

But this should be sizeof(*ptr) to compliant with the Linux kernel coding style.

[...]