* [PATCH 2/3] iio: dummy: convert events to device-managed handlers
2020-12-03 9:50 [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants Alexandru Ardelean
@ 2020-12-03 9:50 ` Alexandru Ardelean
2020-12-03 9:50 ` [PATCH 3/3] iio: dummy: use devm_iio_triggered_buffer_setup() for buffer setup Alexandru Ardelean
2020-12-05 16:36 ` [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Alexandru Ardelean @ 2020-12-03 9:50 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean
This change converts the iio_simple_dummy_events_register() function to use
device-managed functions/equivalents.
The iio_dummy_evgen_release_irq() function needs to be carried over a
devm_add_action_or_reset() handler.
With this the iio_simple_dummy_events_unregister() function can be removed,
and so can it's usage/call.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/dummy/iio_simple_dummy.c | 8 +---
drivers/iio/dummy/iio_simple_dummy.h | 9 +---
drivers/iio/dummy/iio_simple_dummy_events.c | 53 +++++++++------------
3 files changed, 26 insertions(+), 44 deletions(-)
diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
index 2a2e62f780a1..a746b34ae7a3 100644
--- a/drivers/iio/dummy/iio_simple_dummy.c
+++ b/drivers/iio/dummy/iio_simple_dummy.c
@@ -626,13 +626,13 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
/* Specify that device provides sysfs type interfaces */
indio_dev->modes = INDIO_DIRECT_MODE;
- ret = iio_simple_dummy_events_register(indio_dev);
+ ret = iio_simple_dummy_events_register(parent, indio_dev);
if (ret < 0)
return ERR_PTR(ret);
ret = iio_simple_dummy_configure_buffer(indio_dev);
if (ret < 0)
- goto error_unregister_events;
+ return ERR_PTR(ret);
ret = iio_device_register(indio_dev);
if (ret < 0)
@@ -643,8 +643,6 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
return swd;
error_unconfigure_buffer:
iio_simple_dummy_unconfigure_buffer(indio_dev);
-error_unregister_events:
- iio_simple_dummy_events_unregister(indio_dev);
return ERR_PTR(ret);
}
@@ -672,8 +670,6 @@ static int iio_dummy_remove(struct iio_sw_device *swd)
/* Buffered capture related cleanup */
iio_simple_dummy_unconfigure_buffer(indio_dev);
- iio_simple_dummy_events_unregister(indio_dev);
-
return 0;
}
diff --git a/drivers/iio/dummy/iio_simple_dummy.h b/drivers/iio/dummy/iio_simple_dummy.h
index a91622ac54e0..b1ca6e97ed3f 100644
--- a/drivers/iio/dummy/iio_simple_dummy.h
+++ b/drivers/iio/dummy/iio_simple_dummy.h
@@ -76,21 +76,16 @@ int iio_simple_dummy_write_event_value(struct iio_dev *indio_dev,
enum iio_event_info info, int val,
int val2);
-int iio_simple_dummy_events_register(struct iio_dev *indio_dev);
-void iio_simple_dummy_events_unregister(struct iio_dev *indio_dev);
+int iio_simple_dummy_events_register(struct device *parent, struct iio_dev *indio_dev);
#else /* Stubs for when events are disabled at compile time */
static inline int
-iio_simple_dummy_events_register(struct iio_dev *indio_dev)
+iio_simple_dummy_events_register(struct device *parent, struct iio_dev *indio_dev)
{
return 0;
}
-static inline void
-iio_simple_dummy_events_unregister(struct iio_dev *indio_dev)
-{}
-
#endif /* CONFIG_IIO_SIMPLE_DUMMY_EVENTS*/
/**
diff --git a/drivers/iio/dummy/iio_simple_dummy_events.c b/drivers/iio/dummy/iio_simple_dummy_events.c
index 63a2b844be50..8f51fe5cbdfc 100644
--- a/drivers/iio/dummy/iio_simple_dummy_events.c
+++ b/drivers/iio/dummy/iio_simple_dummy_events.c
@@ -222,6 +222,13 @@ static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
return IRQ_HANDLED;
}
+static void iio_simple_dummy_events_release_irq(void *data)
+{
+ struct iio_dummy_state *st = data;
+
+ iio_dummy_evgen_release_irq(st->event_irq);
+}
+
/**
* iio_simple_dummy_events_register() - setup interrupt handling for events
* @indio_dev: device instance data
@@ -233,44 +240,28 @@ static irqreturn_t iio_simple_dummy_event_handler(int irq, void *private)
* no way forms part of this example. Just assume that events magically
* appear via the provided interrupt.
*/
-int iio_simple_dummy_events_register(struct iio_dev *indio_dev)
+int iio_simple_dummy_events_register(struct device *parent,
+ struct iio_dev *indio_dev)
{
struct iio_dummy_state *st = iio_priv(indio_dev);
int ret;
/* Fire up event source - normally not present */
st->event_irq = iio_dummy_evgen_get_irq();
- if (st->event_irq < 0) {
- ret = st->event_irq;
- goto error_ret;
- }
- st->regs = iio_dummy_evgen_get_regs(st->event_irq);
-
- ret = request_threaded_irq(st->event_irq,
- &iio_simple_dummy_get_timestamp,
- &iio_simple_dummy_event_handler,
- IRQF_ONESHOT,
- "iio_simple_event",
- indio_dev);
- if (ret < 0)
- goto error_free_evgen;
- return 0;
+ if (st->event_irq < 0)
+ return st->event_irq;
-error_free_evgen:
- iio_dummy_evgen_release_irq(st->event_irq);
-error_ret:
- return ret;
-}
+ ret = devm_add_action_or_reset(parent,
+ iio_simple_dummy_events_release_irq, st);
+ if (ret)
+ return ret;
-/**
- * iio_simple_dummy_events_unregister() - tidy up interrupt handling on remove
- * @indio_dev: device instance data
- */
-void iio_simple_dummy_events_unregister(struct iio_dev *indio_dev)
-{
- struct iio_dummy_state *st = iio_priv(indio_dev);
+ st->regs = iio_dummy_evgen_get_regs(st->event_irq);
- free_irq(st->event_irq, indio_dev);
- /* Not part of normal driver */
- iio_dummy_evgen_release_irq(st->event_irq);
+ return devm_request_threaded_irq(parent, st->event_irq,
+ &iio_simple_dummy_get_timestamp,
+ &iio_simple_dummy_event_handler,
+ IRQF_ONESHOT,
+ "iio_simple_event",
+ indio_dev);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] iio: dummy: use devm_iio_triggered_buffer_setup() for buffer setup
2020-12-03 9:50 [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants Alexandru Ardelean
2020-12-03 9:50 ` [PATCH 2/3] iio: dummy: convert events to device-managed handlers Alexandru Ardelean
@ 2020-12-03 9:50 ` Alexandru Ardelean
2020-12-05 16:36 ` [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants Jonathan Cameron
2 siblings, 0 replies; 6+ messages in thread
From: Alexandru Ardelean @ 2020-12-03 9:50 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean
The iio_simple_dummy_configure_buffer() function is pretty much just the
iio_triggered_buffer_setup() function.
This change makes use of the devm_iio_triggered_buffer_setup() directly so
that we can tie the life-time and unwinding to the same parent object in
the probe function.
With this, the devm_iio_device_register() can be used directly in the probe
function, removing the iio_dummy_remove() function entirely.
One side-effect that is negligible for this driver is that the name of
the poll-function gets changed from 'iio_simple_dummy_consumer%d' to
'%s_consumer%d' where %s is 'indio_dev->name'.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/dummy/iio_simple_dummy.c | 37 +---------
drivers/iio/dummy/iio_simple_dummy.h | 11 +--
drivers/iio/dummy/iio_simple_dummy_buffer.c | 78 ++-------------------
3 files changed, 13 insertions(+), 113 deletions(-)
diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
index a746b34ae7a3..06baa356e264 100644
--- a/drivers/iio/dummy/iio_simple_dummy.c
+++ b/drivers/iio/dummy/iio_simple_dummy.c
@@ -630,47 +630,17 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
if (ret < 0)
return ERR_PTR(ret);
- ret = iio_simple_dummy_configure_buffer(indio_dev);
+ ret = iio_simple_dummy_configure_buffer(parent, indio_dev);
if (ret < 0)
return ERR_PTR(ret);
- ret = iio_device_register(indio_dev);
+ ret = devm_iio_device_register(parent, indio_dev);
if (ret < 0)
- goto error_unconfigure_buffer;
+ return ERR_PTR(ret);
iio_swd_group_init_type_name(swd, name, &iio_dummy_type);
return swd;
-error_unconfigure_buffer:
- iio_simple_dummy_unconfigure_buffer(indio_dev);
- return ERR_PTR(ret);
-}
-
-/**
- * iio_dummy_remove() - device instance removal function
- * @swd: pointer to software IIO device abstraction
- *
- * Parameters follow those of iio_dummy_probe for buses.
- */
-static int iio_dummy_remove(struct iio_sw_device *swd)
-{
- /*
- * Get a pointer to the device instance iio_dev structure
- * from the bus subsystem. E.g.
- * struct iio_dev *indio_dev = i2c_get_clientdata(client);
- * struct iio_dev *indio_dev = spi_get_drvdata(spi);
- */
- struct iio_dev *indio_dev = swd->device;
-
- /* Unregister the device */
- iio_device_unregister(indio_dev);
-
- /* Device specific code to power down etc */
-
- /* Buffered capture related cleanup */
- iio_simple_dummy_unconfigure_buffer(indio_dev);
-
- return 0;
}
/*
@@ -685,7 +655,6 @@ static int iio_dummy_remove(struct iio_sw_device *swd)
*/
static const struct iio_sw_device_ops iio_dummy_device_ops = {
.probe = iio_dummy_probe,
- .remove = iio_dummy_remove,
};
static struct iio_sw_device_type iio_dummy_device = {
diff --git a/drivers/iio/dummy/iio_simple_dummy.h b/drivers/iio/dummy/iio_simple_dummy.h
index b1ca6e97ed3f..d4fd16b8691b 100644
--- a/drivers/iio/dummy/iio_simple_dummy.h
+++ b/drivers/iio/dummy/iio_simple_dummy.h
@@ -105,17 +105,12 @@ enum iio_simple_dummy_scan_elements {
};
#ifdef CONFIG_IIO_SIMPLE_DUMMY_BUFFER
-int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev);
-void iio_simple_dummy_unconfigure_buffer(struct iio_dev *indio_dev);
+int iio_simple_dummy_configure_buffer(struct device *parent, struct iio_dev *indio_dev);
#else
-static inline int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
+static inline int iio_simple_dummy_configure_buffer(struct device *parent,
+ struct iio_dev *indio_dev)
{
return 0;
}
-
-static inline
-void iio_simple_dummy_unconfigure_buffer(struct iio_dev *indio_dev)
-{}
-
#endif /* CONFIG_IIO_SIMPLE_DUMMY_BUFFER */
#endif /* _IIO_SIMPLE_DUMMY_H_ */
diff --git a/drivers/iio/dummy/iio_simple_dummy_buffer.c b/drivers/iio/dummy/iio_simple_dummy_buffer.c
index 5512d5edc707..76476b45dc8b 100644
--- a/drivers/iio/dummy/iio_simple_dummy_buffer.c
+++ b/drivers/iio/dummy/iio_simple_dummy_buffer.c
@@ -16,9 +16,9 @@
#include <linux/bitmap.h>
#include <linux/iio/iio.h>
-#include <linux/iio/trigger_consumer.h>
#include <linux/iio/buffer.h>
-#include <linux/iio/kfifo_buf.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
#include "iio_simple_dummy.h"
@@ -101,74 +101,10 @@ static irqreturn_t iio_simple_dummy_trigger_h(int irq, void *p)
static const struct iio_buffer_setup_ops iio_simple_dummy_buffer_setup_ops = {
};
-int iio_simple_dummy_configure_buffer(struct iio_dev *indio_dev)
-{
- int ret;
- struct iio_buffer *buffer;
-
- /* Allocate a buffer to use - here a kfifo */
- buffer = iio_kfifo_allocate();
- if (!buffer) {
- ret = -ENOMEM;
- goto error_ret;
- }
-
- iio_device_attach_buffer(indio_dev, buffer);
-
- /*
- * Tell the core what device type specific functions should
- * be run on either side of buffer capture enable / disable.
- */
- indio_dev->setup_ops = &iio_simple_dummy_buffer_setup_ops;
-
- /*
- * Configure a polling function.
- * When a trigger event with this polling function connected
- * occurs, this function is run. Typically this grabs data
- * from the device.
- *
- * NULL for the bottom half. This is normally implemented only if we
- * either want to ping a capture now pin (no sleeping) or grab
- * a timestamp as close as possible to a data ready trigger firing.
- *
- * IRQF_ONESHOT ensures irqs are masked such that only one instance
- * of the handler can run at a time.
- *
- * "iio_simple_dummy_consumer%d" formatting string for the irq 'name'
- * as seen under /proc/interrupts. Remaining parameters as per printk.
- */
- indio_dev->pollfunc = iio_alloc_pollfunc(NULL,
- &iio_simple_dummy_trigger_h,
- IRQF_ONESHOT,
- indio_dev,
- "iio_simple_dummy_consumer%d",
- indio_dev->id);
-
- if (!indio_dev->pollfunc) {
- ret = -ENOMEM;
- goto error_free_buffer;
- }
-
- /*
- * Notify the core that this device is capable of buffered capture
- * driven by a trigger.
- */
- indio_dev->modes |= INDIO_BUFFER_TRIGGERED;
-
- return 0;
-
-error_free_buffer:
- iio_kfifo_free(indio_dev->buffer);
-error_ret:
- return ret;
-}
-
-/**
- * iio_simple_dummy_unconfigure_buffer() - release buffer resources
- * @indio_dev: device instance state
- */
-void iio_simple_dummy_unconfigure_buffer(struct iio_dev *indio_dev)
+int iio_simple_dummy_configure_buffer(struct device *parent, struct iio_dev *indio_dev)
{
- iio_dealloc_pollfunc(indio_dev->pollfunc);
- iio_kfifo_free(indio_dev->buffer);
+ return devm_iio_triggered_buffer_setup(parent, indio_dev,
+ &iio_simple_dummy_trigger_h,
+ NULL,
+ &iio_simple_dummy_buffer_setup_ops);
}
--
2.27.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants
2020-12-03 9:50 [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants Alexandru Ardelean
2020-12-03 9:50 ` [PATCH 2/3] iio: dummy: convert events to device-managed handlers Alexandru Ardelean
2020-12-03 9:50 ` [PATCH 3/3] iio: dummy: use devm_iio_triggered_buffer_setup() for buffer setup Alexandru Ardelean
@ 2020-12-05 16:36 ` Jonathan Cameron
2020-12-07 7:22 ` Alexandru Ardelean
2 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2020-12-05 16:36 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel
On Thu, 3 Dec 2020 11:50:03 +0200
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> Since a main requirement for an IIO device is to have a parent device
> object, it makes sense to attach more of the IIO device's objects to the
> lifetime of the parent object, to clean up the code.
> The idea is to also provide a base example that is more up-to-date with
> what's going on lately with most IIO drivers.
To some degree maybe, it's also very very handy for testing odd corner
cases with. I'd definitely not recommend it as a reference driver
because it inherently has some odd corners because we need to
fake various things that don't exist without hardware.
>
> This change tackles the simple allocations, to convert them to
> device-managed calls, and tie them to the parent device.
I'm confused or maybe I missrecall how this works.
IIRC there isn't a parent device...
Maybe we could create one via a bit of smoke and magic.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> ---
> drivers/iio/dummy/iio_simple_dummy.c | 29 ++++++++--------------------
> 1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
> index c0b7ef900735..2a2e62f780a1 100644
> --- a/drivers/iio/dummy/iio_simple_dummy.c
> +++ b/drivers/iio/dummy/iio_simple_dummy.c
> @@ -574,11 +574,9 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> * parent = &client->dev;
> */
>
> - swd = kzalloc(sizeof(*swd), GFP_KERNEL);
> - if (!swd) {
> - ret = -ENOMEM;
> - goto error_kzalloc;
> - }
> + swd = devm_kzalloc(parent, sizeof(*swd), GFP_KERNEL);
> + if (!swd)
> + return ERR_PTR(-ENOMEM);
> /*
> * Allocate an IIO device.
> *
> @@ -587,11 +585,9 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> * It also has a region (accessed by iio_priv()
> * for chip specific state information.
> */
> - indio_dev = iio_device_alloc(parent, sizeof(*st));
> - if (!indio_dev) {
> - ret = -ENOMEM;
> - goto error_ret;
> - }
> + indio_dev = devm_iio_device_alloc(parent, sizeof(*st));
> + if (!indio_dev)
> + return ERR_PTR(-ENOMEM);
>
> st = iio_priv(indio_dev);
> mutex_init(&st->lock);
> @@ -615,7 +611,7 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> * indio_dev->name = id->name;
> * indio_dev->name = spi_get_device_id(spi)->name;
> */
> - indio_dev->name = kstrdup(name, GFP_KERNEL);
> + indio_dev->name = devm_kstrdup(parent, name, GFP_KERNEL);
>
> /* Provide description of available channels */
> indio_dev->channels = iio_dummy_channels;
> @@ -632,7 +628,7 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
>
> ret = iio_simple_dummy_events_register(indio_dev);
> if (ret < 0)
> - goto error_free_device;
> + return ERR_PTR(ret);
>
> ret = iio_simple_dummy_configure_buffer(indio_dev);
> if (ret < 0)
> @@ -649,11 +645,6 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> iio_simple_dummy_unconfigure_buffer(indio_dev);
> error_unregister_events:
> iio_simple_dummy_events_unregister(indio_dev);
> -error_free_device:
> - iio_device_free(indio_dev);
> -error_ret:
> - kfree(swd);
> -error_kzalloc:
> return ERR_PTR(ret);
> }
>
> @@ -683,10 +674,6 @@ static int iio_dummy_remove(struct iio_sw_device *swd)
>
> iio_simple_dummy_events_unregister(indio_dev);
>
> - /* Free all structures */
> - kfree(indio_dev->name);
> - iio_device_free(indio_dev);
> -
> return 0;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants
2020-12-05 16:36 ` [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants Jonathan Cameron
@ 2020-12-07 7:22 ` Alexandru Ardelean
2020-12-13 14:22 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Alexandru Ardelean @ 2020-12-07 7:22 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: Alexandru Ardelean, linux-iio, LKML
On Sat, Dec 5, 2020 at 8:46 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Thu, 3 Dec 2020 11:50:03 +0200
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > Since a main requirement for an IIO device is to have a parent device
> > object, it makes sense to attach more of the IIO device's objects to the
> > lifetime of the parent object, to clean up the code.
> > The idea is to also provide a base example that is more up-to-date with
> > what's going on lately with most IIO drivers.
>
> To some degree maybe, it's also very very handy for testing odd corner
> cases with. I'd definitely not recommend it as a reference driver
> because it inherently has some odd corners because we need to
> fake various things that don't exist without hardware.
It's funny because during GSoC it seemed like some people would try to
use this as a starting point, then shift to another working ADC/DAC
example.
I was also thinking about maybe extending this a bit further, to have
it a bit more dynamic at load time [being able to load fake IIO
drivers, load fake data to be read via fake IIO device].
No idea when this will be ready, but it's on my long todo-list.
>
> >
> > This change tackles the simple allocations, to convert them to
> > device-managed calls, and tie them to the parent device.
>
> I'm confused or maybe I missrecall how this works.
>
> IIRC there isn't a parent device...
>
> Maybe we could create one via a bit of smoke and magic.
Yep, there isn't one.
I'll add one through some smoke, magic, some OF and maybe some fake
i2c/spi device IDs.
>
>
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > ---
> > drivers/iio/dummy/iio_simple_dummy.c | 29 ++++++++--------------------
> > 1 file changed, 8 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
> > index c0b7ef900735..2a2e62f780a1 100644
> > --- a/drivers/iio/dummy/iio_simple_dummy.c
> > +++ b/drivers/iio/dummy/iio_simple_dummy.c
> > @@ -574,11 +574,9 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > * parent = &client->dev;
> > */
> >
> > - swd = kzalloc(sizeof(*swd), GFP_KERNEL);
> > - if (!swd) {
> > - ret = -ENOMEM;
> > - goto error_kzalloc;
> > - }
> > + swd = devm_kzalloc(parent, sizeof(*swd), GFP_KERNEL);
> > + if (!swd)
> > + return ERR_PTR(-ENOMEM);
> > /*
> > * Allocate an IIO device.
> > *
> > @@ -587,11 +585,9 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > * It also has a region (accessed by iio_priv()
> > * for chip specific state information.
> > */
> > - indio_dev = iio_device_alloc(parent, sizeof(*st));
> > - if (!indio_dev) {
> > - ret = -ENOMEM;
> > - goto error_ret;
> > - }
> > + indio_dev = devm_iio_device_alloc(parent, sizeof(*st));
> > + if (!indio_dev)
> > + return ERR_PTR(-ENOMEM);
> >
> > st = iio_priv(indio_dev);
> > mutex_init(&st->lock);
> > @@ -615,7 +611,7 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > * indio_dev->name = id->name;
> > * indio_dev->name = spi_get_device_id(spi)->name;
> > */
> > - indio_dev->name = kstrdup(name, GFP_KERNEL);
> > + indio_dev->name = devm_kstrdup(parent, name, GFP_KERNEL);
> >
> > /* Provide description of available channels */
> > indio_dev->channels = iio_dummy_channels;
> > @@ -632,7 +628,7 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> >
> > ret = iio_simple_dummy_events_register(indio_dev);
> > if (ret < 0)
> > - goto error_free_device;
> > + return ERR_PTR(ret);
> >
> > ret = iio_simple_dummy_configure_buffer(indio_dev);
> > if (ret < 0)
> > @@ -649,11 +645,6 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > iio_simple_dummy_unconfigure_buffer(indio_dev);
> > error_unregister_events:
> > iio_simple_dummy_events_unregister(indio_dev);
> > -error_free_device:
> > - iio_device_free(indio_dev);
> > -error_ret:
> > - kfree(swd);
> > -error_kzalloc:
> > return ERR_PTR(ret);
> > }
> >
> > @@ -683,10 +674,6 @@ static int iio_dummy_remove(struct iio_sw_device *swd)
> >
> > iio_simple_dummy_events_unregister(indio_dev);
> >
> > - /* Free all structures */
> > - kfree(indio_dev->name);
> > - iio_device_free(indio_dev);
> > -
> > return 0;
> > }
> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/3] iio: dummy: convert all simple allocation devm_ variants
2020-12-07 7:22 ` Alexandru Ardelean
@ 2020-12-13 14:22 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2020-12-13 14:22 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: Alexandru Ardelean, linux-iio, LKML
On Mon, 7 Dec 2020 09:22:28 +0200
Alexandru Ardelean <ardeleanalex@gmail.com> wrote:
> On Sat, Dec 5, 2020 at 8:46 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Thu, 3 Dec 2020 11:50:03 +0200
> > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> >
> > > Since a main requirement for an IIO device is to have a parent device
> > > object, it makes sense to attach more of the IIO device's objects to the
> > > lifetime of the parent object, to clean up the code.
> > > The idea is to also provide a base example that is more up-to-date with
> > > what's going on lately with most IIO drivers.
> >
> > To some degree maybe, it's also very very handy for testing odd corner
> > cases with. I'd definitely not recommend it as a reference driver
> > because it inherently has some odd corners because we need to
> > fake various things that don't exist without hardware.
>
> It's funny because during GSoC it seemed like some people would try to
> use this as a starting point, then shift to another working ADC/DAC
> example.
Hmm. It make sense to use it if you are interested in see what actually
turns up in userspace etc as a gsoc student probably doesn't have much
hardware that is already supported. But code wise it's somewhat odd
and may be less good as an example than a i2c/spi ADC.
> I was also thinking about maybe extending this a bit further, to have
> it a bit more dynamic at load time [being able to load fake IIO
> drivers, load fake data to be read via fake IIO device].
> No idea when this will be ready, but it's on my long todo-list.
Sure. It's always been on my long term list to make this driver do
more complex stuff, but real parts often get in the way unless I'm
proving out something I don't have any real hardware for.
>
> >
> > >
> > > This change tackles the simple allocations, to convert them to
> > > device-managed calls, and tie them to the parent device.
> >
> > I'm confused or maybe I missrecall how this works.
> >
> > IIRC there isn't a parent device...
> >
> > Maybe we could create one via a bit of smoke and magic.
>
> Yep, there isn't one.
> I'll add one through some smoke, magic, some OF and maybe some fake
> i2c/spi device IDs.
Hmm. Whatever is done needs to be both non invasive and not imply
anything that isn't true. Maybe better off with a platform device
than i2c or spi as the parent.
Jonathan
>
> >
> >
> > >
> > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
> > > ---
> > > drivers/iio/dummy/iio_simple_dummy.c | 29 ++++++++--------------------
> > > 1 file changed, 8 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
> > > index c0b7ef900735..2a2e62f780a1 100644
> > > --- a/drivers/iio/dummy/iio_simple_dummy.c
> > > +++ b/drivers/iio/dummy/iio_simple_dummy.c
> > > @@ -574,11 +574,9 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > > * parent = &client->dev;
> > > */
> > >
> > > - swd = kzalloc(sizeof(*swd), GFP_KERNEL);
> > > - if (!swd) {
> > > - ret = -ENOMEM;
> > > - goto error_kzalloc;
> > > - }
> > > + swd = devm_kzalloc(parent, sizeof(*swd), GFP_KERNEL);
> > > + if (!swd)
> > > + return ERR_PTR(-ENOMEM);
> > > /*
> > > * Allocate an IIO device.
> > > *
> > > @@ -587,11 +585,9 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > > * It also has a region (accessed by iio_priv()
> > > * for chip specific state information.
> > > */
> > > - indio_dev = iio_device_alloc(parent, sizeof(*st));
> > > - if (!indio_dev) {
> > > - ret = -ENOMEM;
> > > - goto error_ret;
> > > - }
> > > + indio_dev = devm_iio_device_alloc(parent, sizeof(*st));
> > > + if (!indio_dev)
> > > + return ERR_PTR(-ENOMEM);
> > >
> > > st = iio_priv(indio_dev);
> > > mutex_init(&st->lock);
> > > @@ -615,7 +611,7 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > > * indio_dev->name = id->name;
> > > * indio_dev->name = spi_get_device_id(spi)->name;
> > > */
> > > - indio_dev->name = kstrdup(name, GFP_KERNEL);
> > > + indio_dev->name = devm_kstrdup(parent, name, GFP_KERNEL);
> > >
> > > /* Provide description of available channels */
> > > indio_dev->channels = iio_dummy_channels;
> > > @@ -632,7 +628,7 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > >
> > > ret = iio_simple_dummy_events_register(indio_dev);
> > > if (ret < 0)
> > > - goto error_free_device;
> > > + return ERR_PTR(ret);
> > >
> > > ret = iio_simple_dummy_configure_buffer(indio_dev);
> > > if (ret < 0)
> > > @@ -649,11 +645,6 @@ static struct iio_sw_device *iio_dummy_probe(const char *name)
> > > iio_simple_dummy_unconfigure_buffer(indio_dev);
> > > error_unregister_events:
> > > iio_simple_dummy_events_unregister(indio_dev);
> > > -error_free_device:
> > > - iio_device_free(indio_dev);
> > > -error_ret:
> > > - kfree(swd);
> > > -error_kzalloc:
> > > return ERR_PTR(ret);
> > > }
> > >
> > > @@ -683,10 +674,6 @@ static int iio_dummy_remove(struct iio_sw_device *swd)
> > >
> > > iio_simple_dummy_events_unregister(indio_dev);
> > >
> > > - /* Free all structures */
> > > - kfree(indio_dev->name);
> > > - iio_device_free(indio_dev);
> > > -
> > > return 0;
> > > }
> > >
> >
^ permalink raw reply [flat|nested] 6+ messages in thread