2014-12-19 17:40:55

by Karol Wrona

[permalink] [raw]
Subject: [PATCH v2 0/3] iio: Add resource management kfifo alloc/free

Hello,

iio kfifo alloc took one argument struct iio_dev * indio_dev which was not used
so it was removed in allocate function and in several drivers which used it.
Second patch adds devm_ alloc/free.

Karol

changes from RFC:
- rebased on next-20141219
- changes in clients
- add minor doc fix to iio core


Karol Wrona (3):
iio: kfifo: Remove unused argument in iio_kfifo_allocate
iio: kfifo: Add resource management devm_iio_kfifo_allocate/free
iio: core: Get rid of misleading comment

drivers/iio/adc/ti_am335x_adc.c | 2 +-
drivers/iio/industrialio-core.c | 1 -
drivers/iio/industrialio-triggered-buffer.c | 2 +-
drivers/iio/kfifo_buf.c | 60 ++++++++++++++++++++++-
drivers/staging/iio/accel/lis3l02dq_ring.c | 2 +-
drivers/staging/iio/iio_simple_dummy_buffer.c | 2 +-
drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
drivers/staging/iio/meter/ade7758_ring.c | 2 +-
include/linux/iio/kfifo_buf.h | 5 +-
9 files changed, 68 insertions(+), 10 deletions(-)

--
1.7.9.5


2014-12-19 17:41:12

by Karol Wrona

[permalink] [raw]
Subject: [PATCH v2 3/3] iio: core: Get rid of misleading comment

This comment did not fit here. It explains why devm_kmalloc
uses dr_alloc. Generally is not needed at all.

Signed-off-by: Karol Wrona <[email protected]>
---
drivers/iio/industrialio-core.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index af3e76d..1d030ae 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1035,7 +1035,6 @@ struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
if (!ptr)
return NULL;

- /* use raw alloc_dr for kmalloc caller tracing */
iio_dev = iio_device_alloc(sizeof_priv);
if (iio_dev) {
*ptr = iio_dev;
--
1.7.9.5

2014-12-19 17:41:10

by Karol Wrona

[permalink] [raw]
Subject: [PATCH v2 2/3] iio: kfifo: Add resource management devm_iio_kfifo_allocate/free

iio kfifo allocate/free gained their devm_ wrappers.

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 a383291..5d44099 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

2014-12-19 17:41:44

by Karol Wrona

[permalink] [raw]
Subject: [PATCH v2 1/3] iio: kfifo: Remove unused argument in iio_kfifo_allocate

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.

The argument was removed also in its client.

Signed-off-by: Karol Wrona <[email protected]>
---
drivers/iio/adc/ti_am335x_adc.c | 2 +-
drivers/iio/industrialio-triggered-buffer.c | 2 +-
drivers/iio/kfifo_buf.c | 6 ++++--
drivers/staging/iio/accel/lis3l02dq_ring.c | 2 +-
drivers/staging/iio/iio_simple_dummy_buffer.c | 2 +-
drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
drivers/staging/iio/meter/ade7758_ring.c | 2 +-
include/linux/iio/kfifo_buf.h | 2 +-
8 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index b730864..bf87993 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -250,7 +250,7 @@ static int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
struct iio_buffer *buffer;
int ret;

- buffer = iio_kfifo_allocate(indio_dev);
+ buffer = iio_kfifo_allocate();
if (!buffer)
return -ENOMEM;

diff --git a/drivers/iio/industrialio-triggered-buffer.c b/drivers/iio/industrialio-triggered-buffer.c
index d6f54930..6c6307a 100644
--- a/drivers/iio/industrialio-triggered-buffer.c
+++ b/drivers/iio/industrialio-triggered-buffer.c
@@ -49,7 +49,7 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
struct iio_buffer *buffer;
int ret;

- buffer = iio_kfifo_allocate(indio_dev);
+ buffer = iio_kfifo_allocate();
if (!buffer) {
ret = -ENOMEM;
goto error_ret;
diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
index 7134e8a..a383291 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);
+ 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/drivers/staging/iio/accel/lis3l02dq_ring.c b/drivers/staging/iio/accel/lis3l02dq_ring.c
index 9efc77b..1fd9009 100644
--- a/drivers/staging/iio/accel/lis3l02dq_ring.c
+++ b/drivers/staging/iio/accel/lis3l02dq_ring.c
@@ -393,7 +393,7 @@ int lis3l02dq_configure_buffer(struct iio_dev *indio_dev)
int ret;
struct iio_buffer *buffer;

- buffer = iio_kfifo_allocate(indio_dev);
+ buffer = iio_kfifo_allocate();
if (!buffer)
return -ENOMEM;

diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers/staging/iio/iio_simple_dummy_buffer.c
index fd74f91..df765c9 100644
--- a/drivers/staging/iio/iio_simple_dummy_buffer.c
+++ b/drivers/staging/iio/iio_simple_dummy_buffer.c
@@ -122,7 +122,7 @@ int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
struct iio_buffer *buffer;

/* Allocate a buffer to use - here a kfifo */
- buffer = iio_kfifo_allocate(indio_dev);
+ buffer = iio_kfifo_allocate();
if (buffer == NULL) {
ret = -ENOMEM;
goto error_ret;
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index b6bd609..ace9ef8 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -626,7 +626,7 @@ static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
{
struct iio_buffer *buffer;

- buffer = iio_kfifo_allocate(indio_dev);
+ buffer = iio_kfifo_allocate();
if (!buffer)
return -ENOMEM;

diff --git a/drivers/staging/iio/meter/ade7758_ring.c b/drivers/staging/iio/meter/ade7758_ring.c
index 6e90064..31d2cf3 100644
--- a/drivers/staging/iio/meter/ade7758_ring.c
+++ b/drivers/staging/iio/meter/ade7758_ring.c
@@ -118,7 +118,7 @@ int ade7758_configure_ring(struct iio_dev *indio_dev)
struct iio_buffer *buffer;
int ret = 0;

- buffer = iio_kfifo_allocate(indio_dev);
+ buffer = iio_kfifo_allocate();
if (!buffer) {
ret = -ENOMEM;
return ret;
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

2014-12-26 11:35:37

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] iio: kfifo: Remove unused argument in iio_kfifo_allocate

On 19/12/14 17:39, 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.
>
> The argument was removed also in its client.
>
> Signed-off-by: Karol Wrona <[email protected]>
Good cleanup - some fuzz applying and required a bit of hand editting
in kfifo_buf.c for reasons I couldn't immediately spot. Please sanity
check I haven't messed anything up.

Thanks,

Jonathan
> ---
> drivers/iio/adc/ti_am335x_adc.c | 2 +-
> drivers/iio/industrialio-triggered-buffer.c | 2 +-
> drivers/iio/kfifo_buf.c | 6 ++++--
> drivers/staging/iio/accel/lis3l02dq_ring.c | 2 +-
> drivers/staging/iio/iio_simple_dummy_buffer.c | 2 +-
> drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
> drivers/staging/iio/meter/ade7758_ring.c | 2 +-
> include/linux/iio/kfifo_buf.h | 2 +-
> 8 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
> index b730864..bf87993 100644
> --- a/drivers/iio/adc/ti_am335x_adc.c
> +++ b/drivers/iio/adc/ti_am335x_adc.c
> @@ -250,7 +250,7 @@ static int tiadc_iio_buffered_hardware_setup(struct iio_dev *indio_dev,
> struct iio_buffer *buffer;
> int ret;
>
> - buffer = iio_kfifo_allocate(indio_dev);
> + buffer = iio_kfifo_allocate();
> if (!buffer)
> return -ENOMEM;
>
> diff --git a/drivers/iio/industrialio-triggered-buffer.c b/drivers/iio/industrialio-triggered-buffer.c
> index d6f54930..6c6307a 100644
> --- a/drivers/iio/industrialio-triggered-buffer.c
> +++ b/drivers/iio/industrialio-triggered-buffer.c
> @@ -49,7 +49,7 @@ int iio_triggered_buffer_setup(struct iio_dev *indio_dev,
> struct iio_buffer *buffer;
> int ret;
>
> - buffer = iio_kfifo_allocate(indio_dev);
> + buffer = iio_kfifo_allocate();
> if (!buffer) {
> ret = -ENOMEM;
> goto error_ret;
> diff --git a/drivers/iio/kfifo_buf.c b/drivers/iio/kfifo_buf.c
> index 7134e8a..a383291 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);
> + 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/drivers/staging/iio/accel/lis3l02dq_ring.c b/drivers/staging/iio/accel/lis3l02dq_ring.c
> index 9efc77b..1fd9009 100644
> --- a/drivers/staging/iio/accel/lis3l02dq_ring.c
> +++ b/drivers/staging/iio/accel/lis3l02dq_ring.c
> @@ -393,7 +393,7 @@ int lis3l02dq_configure_buffer(struct iio_dev *indio_dev)
> int ret;
> struct iio_buffer *buffer;
>
> - buffer = iio_kfifo_allocate(indio_dev);
> + buffer = iio_kfifo_allocate();
> if (!buffer)
> return -ENOMEM;
>
> diff --git a/drivers/staging/iio/iio_simple_dummy_buffer.c b/drivers/staging/iio/iio_simple_dummy_buffer.c
> index fd74f91..df765c9 100644
> --- a/drivers/staging/iio/iio_simple_dummy_buffer.c
> +++ b/drivers/staging/iio/iio_simple_dummy_buffer.c
> @@ -122,7 +122,7 @@ int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev,
> struct iio_buffer *buffer;
>
> /* Allocate a buffer to use - here a kfifo */
> - buffer = iio_kfifo_allocate(indio_dev);
> + buffer = iio_kfifo_allocate();
> if (buffer == NULL) {
> ret = -ENOMEM;
> goto error_ret;
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index b6bd609..ace9ef8 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -626,7 +626,7 @@ static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
> {
> struct iio_buffer *buffer;
>
> - buffer = iio_kfifo_allocate(indio_dev);
> + buffer = iio_kfifo_allocate();
> if (!buffer)
> return -ENOMEM;
>
> diff --git a/drivers/staging/iio/meter/ade7758_ring.c b/drivers/staging/iio/meter/ade7758_ring.c
> index 6e90064..31d2cf3 100644
> --- a/drivers/staging/iio/meter/ade7758_ring.c
> +++ b/drivers/staging/iio/meter/ade7758_ring.c
> @@ -118,7 +118,7 @@ int ade7758_configure_ring(struct iio_dev *indio_dev)
> struct iio_buffer *buffer;
> int ret = 0;
>
> - buffer = iio_kfifo_allocate(indio_dev);
> + buffer = iio_kfifo_allocate();
> if (!buffer) {
> ret = -ENOMEM;
> return ret;
> 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
>

2014-12-26 11:41:03

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] iio: kfifo: Add resource management devm_iio_kfifo_allocate/free

On 19/12/14 17:39, Karol Wrona wrote:
> iio kfifo allocate/free gained their devm_ wrappers.
>
> Signed-off-by: Karol Wrona <[email protected]>
> Suggested-by: Jonathan Cameron <[email protected]>
Applied to the togreg branch of iio.git.

One addition - added to the list of devm functions in
Documentation/device-model/devres.txt


> ---
> 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 a383291..5d44099 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
>

2014-12-26 11:41:46

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] iio: core: Get rid of misleading comment

On 19/12/14 17:39, Karol Wrona wrote:
> This comment did not fit here. It explains why devm_kmalloc
> uses dr_alloc. Generally is not needed at all.
>
A classic bit of cut and paste I guess. Anyhow, applied.

Jonathan
> Signed-off-by: Karol Wrona <[email protected]>
> ---
> drivers/iio/industrialio-core.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index af3e76d..1d030ae 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1035,7 +1035,6 @@ struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
> if (!ptr)
> return NULL;
>
> - /* use raw alloc_dr for kmalloc caller tracing */
> iio_dev = iio_device_alloc(sizeof_priv);
> if (iio_dev) {
> *ptr = iio_dev;
>