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.
This change tackles the simple allocations, to convert them to
device-managed calls, and tie them to the parent device.
Signed-off-by: Alexandru Ardelean <[email protected]>
---
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;
}
--
2.27.0
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 <[email protected]>
---
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
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 <[email protected]>
---
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
On Thu, 3 Dec 2020 11:50:03 +0200
Alexandru Ardelean <[email protected]> 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 <[email protected]>
> ---
> 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;
> }
>
On Sat, Dec 5, 2020 at 8:46 PM Jonathan Cameron <[email protected]> wrote:
>
> On Thu, 3 Dec 2020 11:50:03 +0200
> Alexandru Ardelean <[email protected]> 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 <[email protected]>
> > ---
> > 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;
> > }
> >
>
On Mon, 7 Dec 2020 09:22:28 +0200
Alexandru Ardelean <[email protected]> wrote:
> On Sat, Dec 5, 2020 at 8:46 PM Jonathan Cameron <[email protected]> wrote:
> >
> > On Thu, 3 Dec 2020 11:50:03 +0200
> > Alexandru Ardelean <[email protected]> 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 <[email protected]>
> > > ---
> > > 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;
> > > }
> > >
> >