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
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 <[email protected]>
---
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
iio kfifo allocate/free gained their devm_ wrappers.
Change-Id: I10c19ccd7c01491caf088b3629137425ddccd29c
Signed-off-by: Karol Wrona <[email protected]>
Suggested-by: Jonathan Cameron <[email protected]>
---
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
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.")
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 <[email protected]>
> Suggested-by: Jonathan Cameron <[email protected]>
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.
[...]