On Mon, 7 Dec 2020 09:22:28 +0200
Alexandru Ardelean <ardeleana...@gmail.com> wrote:
> On Sat, Dec 5, 2020 at 8:46 PM Jonathan Cameron <ji...@kernel.org> wrote:
> >
> > On Thu, 3 Dec 2020 11:50:03 +0200
> > Alexandru Ardelean <alexandru.ardel...@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.ardel...@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;
> > > }
> > >
> >
