2016-04-20 15:42:14

by Daniel Baluta

[permalink] [raw]
Subject: [RFC PATCH 0/3] Introduce support for creating IIO devices via configfs

For testing purposes is nice to have a quick way of creating IIO devices.
This patch series introduces support for creating IIO devices via configs
(patch 1), allowing users to register "device types". For the moment we
support "dummy" device type (patch 2).

This is just a RFC in order to see if the interface is acceptable. We also
need a way to create IIO devices with configurable number of channels.

Patch 3 introduces configfs entries documentation for easier review.

Daniel Baluta (3):
iio: Add support for creating IIO devices via configfs
iio: dummy: Convert IIO dummy to configfs
Documentation: iio: Add IIO software devices docs

Documentation/ABI/testing/configfs-iio | 13 +++
drivers/iio/Kconfig | 9 ++
drivers/iio/Makefile | 1 +
drivers/iio/dummy/iio_simple_dummy.c | 98 ++++++------------
drivers/iio/industrialio-sw-device.c | 181 +++++++++++++++++++++++++++++++++
include/linux/iio/sw_device.h | 70 +++++++++++++
6 files changed, 307 insertions(+), 65 deletions(-)
create mode 100644 drivers/iio/industrialio-sw-device.c
create mode 100644 include/linux/iio/sw_device.h

--
2.5.0


2016-04-20 15:42:21

by Daniel Baluta

[permalink] [raw]
Subject: [RFC PATCH 2/3] iio: dummy: Convert IIO dummy to configfs

We register a new device type named "dummy", this will create a
configfs entry under:
* /config/iio/devices/dummy.

Creating dummy devices is now as simple as:

$ mkdir /config/iio/devices/dummy/my_dummy_device

Signed-off-by: Daniel Baluta <[email protected]>
---
drivers/iio/dummy/iio_simple_dummy.c | 98 ++++++++++++------------------------
1 file changed, 33 insertions(+), 65 deletions(-)

diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
index 43fe4ba..65f92f8 100644
--- a/drivers/iio/dummy/iio_simple_dummy.c
+++ b/drivers/iio/dummy/iio_simple_dummy.c
@@ -22,21 +22,12 @@
#include <linux/iio/sysfs.h>
#include <linux/iio/events.h>
#include <linux/iio/buffer.h>
+#include <linux/iio/sw_device.h>
#include "iio_simple_dummy.h"

-/*
- * A few elements needed to fake a bus for this driver
- * Note instances parameter controls how many of these
- * dummy devices are registered.
- */
-static unsigned instances = 1;
-module_param(instances, uint, 0);
-
-/* Pointer array used to fake bus elements */
-static struct iio_dev **iio_dummy_devs;
-
-/* Fake a name for the part number, usually obtained from the id table */
-static const char *iio_dummy_part_number = "iio_dummy_part_no";
+static struct config_item_type iio_dummy_type = {
+ .ct_owner = THIS_MODULE,
+};

/**
* struct iio_dummy_accel_calibscale - realworld to register mapping
@@ -572,12 +563,18 @@ static int iio_dummy_init_device(struct iio_dev *indio_dev)
* const struct i2c_device_id *id)
* SPI: iio_dummy_probe(struct spi_device *spi)
*/
-static int iio_dummy_probe(int index)
+static struct iio_sw_device *iio_dummy_probe(const char *name)
{
int ret;
struct iio_dev *indio_dev;
struct iio_dummy_state *st;
+ struct iio_sw_device *swd;

+ swd = kzalloc(sizeof(*swd), GFP_KERNEL);
+ if (!swd) {
+ ret = -ENOMEM;
+ goto error_kzalloc;
+ }
/*
* Allocate an IIO device.
*
@@ -608,7 +605,7 @@ static int iio_dummy_probe(int index)
* i2c_set_clientdata(client, indio_dev);
* spi_set_drvdata(spi, indio_dev);
*/
- iio_dummy_devs[index] = indio_dev;
+ swd->device = indio_dev;

/*
* Set the device name.
@@ -619,7 +616,7 @@ static int iio_dummy_probe(int index)
* indio_dev->name = id->name;
* indio_dev->name = spi_get_device_id(spi)->name;
*/
- indio_dev->name = iio_dummy_part_number;
+ indio_dev->name = name;

/* Provide description of available channels */
indio_dev->channels = iio_dummy_channels;
@@ -646,7 +643,8 @@ static int iio_dummy_probe(int index)
if (ret < 0)
goto error_unconfigure_buffer;

- return 0;
+ iio_swd_group_init_type_name(swd, name, &iio_dummy_type);
+ return swd;
error_unconfigure_buffer:
iio_simple_dummy_unconfigure_buffer(indio_dev);
error_unregister_events:
@@ -654,16 +652,18 @@ error_unregister_events:
error_free_device:
iio_device_free(indio_dev);
error_ret:
- return ret;
+ kfree(swd);
+error_kzalloc:
+ return ERR_PTR(ret);
}

/**
* iio_dummy_remove() - device instance removal function
- * @index: device index.
+ * @swd: pointer to software IIO device abstraction
*
* Parameters follow those of iio_dummy_probe for buses.
*/
-static void iio_dummy_remove(int index)
+static int iio_dummy_remove(struct iio_sw_device *swd)
{
/*
* Get a pointer to the device instance iio_dev structure
@@ -671,7 +671,7 @@ static void iio_dummy_remove(int index)
* struct iio_dev *indio_dev = i2c_get_clientdata(client);
* struct iio_dev *indio_dev = spi_get_drvdata(spi);
*/
- struct iio_dev *indio_dev = iio_dummy_devs[index];
+ struct iio_dev *indio_dev = swd->device;

/* Unregister the device */
iio_device_unregister(indio_dev);
@@ -685,10 +685,10 @@ static void iio_dummy_remove(int index)

/* Free all structures */
iio_device_free(indio_dev);
+ return 0;
}
-
/**
- * iio_dummy_init() - device driver registration
+ * module_iio_sw_device_driver() - device driver registration
*
* Varies depending on bus type of the device. As there is no device
* here, call probe directly. For information on device registration
@@ -697,50 +697,18 @@ static void iio_dummy_remove(int index)
* spi:
* Documentation/spi/spi-summary
*/
-static __init int iio_dummy_init(void)
-{
- int i, ret;
-
- if (instances > 10) {
- instances = 1;
- return -EINVAL;
- }
-
- /* Fake a bus */
- iio_dummy_devs = kcalloc(instances, sizeof(*iio_dummy_devs),
- GFP_KERNEL);
- /* Here we have no actual device so call probe */
- for (i = 0; i < instances; i++) {
- ret = iio_dummy_probe(i);
- if (ret < 0)
- goto error_remove_devs;
- }
- return 0;
-
-error_remove_devs:
- while (i--)
- iio_dummy_remove(i);
-
- kfree(iio_dummy_devs);
- return ret;
-}
-module_init(iio_dummy_init);
+static const struct iio_sw_device_ops iio_dummy_device_ops = {
+ .probe = iio_dummy_probe,
+ .remove = iio_dummy_remove,
+};

-/**
- * iio_dummy_exit() - device driver removal
- *
- * Varies depending on bus type of the device.
- * As there is no device here, call remove directly.
- */
-static __exit void iio_dummy_exit(void)
-{
- int i;
+static struct iio_sw_device_type iio_dummy_device = {
+ .name = "dummy",
+ .owner = THIS_MODULE,
+ .ops = &iio_dummy_device_ops,
+};

- for (i = 0; i < instances; i++)
- iio_dummy_remove(i);
- kfree(iio_dummy_devs);
-}
-module_exit(iio_dummy_exit);
+module_iio_sw_device_driver(iio_dummy_device);

MODULE_AUTHOR("Jonathan Cameron <[email protected]>");
MODULE_DESCRIPTION("IIO dummy driver");
--
2.5.0

2016-04-20 15:42:19

by Daniel Baluta

[permalink] [raw]
Subject: [RFC PATCH 3/3] Documentation: iio: Add IIO software devices docs

Signed-off-by: Daniel Baluta <[email protected]>
---
Documentation/ABI/testing/configfs-iio | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-iio b/Documentation/ABI/testing/configfs-iio
index 2483756..84b6bd1 100644
--- a/Documentation/ABI/testing/configfs-iio
+++ b/Documentation/ABI/testing/configfs-iio
@@ -19,3 +19,16 @@ KernelVersion: 4.4
Description:
High resolution timers directory. Creating a directory here
will result in creating a hrtimer trigger in the IIO subsystem.
+
+What: /config/iio/devices
+Date: April 2016
+KernelVersion: 4.7
+Description:
+ Industrial IO software devices directory.
+
+What: /config/iio/triggers/dummy
+Date: April 2016
+KernelVersion: 4.7
+Description:
+ Dummy IIO devices directory. Creating a directory here will result
+ in creating a dummy IIO device in the IIO subystem.
--
2.5.0

2016-04-20 15:42:51

by Daniel Baluta

[permalink] [raw]
Subject: [RFC PATCH 1/3] iio: Add support for creating IIO devices via configfs

This is similar with support for creating triggers via configfs.
Devices will be hosted under:
* /config/iio/devices

We allow users to register "device types" under:
* /config/iio/devices/<device_types>/

Signed-off-by: Daniel Baluta <[email protected]>
---
drivers/iio/Kconfig | 9 ++
drivers/iio/Makefile | 1 +
drivers/iio/industrialio-sw-device.c | 181 +++++++++++++++++++++++++++++++++++
include/linux/iio/sw_device.h | 70 ++++++++++++++
4 files changed, 261 insertions(+)
create mode 100644 drivers/iio/industrialio-sw-device.c
create mode 100644 include/linux/iio/sw_device.h

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 505e921..77711a3 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -46,6 +46,15 @@ config IIO_CONSUMERS_PER_TRIGGER
This value controls the maximum number of consumers that a
given trigger may handle. Default is 2.

+config IIO_SW_DEVICE
+ tristate "Enable software IIO device support"
+ select IIO_CONFIGFS
+ help
+ Provides IIO core support for software device. A software
+ device can be created via configfs or directly by a driver
+ using the API provided.
+
+
config IIO_SW_TRIGGER
tristate "Enable software triggers support"
select IIO_CONFIGFS
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 20f6490..87e4c43 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -8,6 +8,7 @@ industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o

obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
+obj-$(CONFIG_IIO_SW_DEVICE) += industrialio-sw-device.o
obj-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
obj-$(CONFIG_IIO_TRIGGERED_EVENT) += industrialio-triggered-event.o

diff --git a/drivers/iio/industrialio-sw-device.c b/drivers/iio/industrialio-sw-device.c
new file mode 100644
index 0000000..243aaf4
--- /dev/null
+++ b/drivers/iio/industrialio-sw-device.c
@@ -0,0 +1,181 @@
+/*
+ * The Industrial I/O core, software IIO devices functions
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kmod.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+
+#include <linux/iio/sw_device.h>
+#include <linux/iio/configfs.h>
+#include <linux/configfs.h>
+
+static struct config_group *iio_devices_group;
+static struct config_item_type iio_device_type_group_type;
+
+static struct config_item_type iio_devices_group_type = {
+ .ct_owner = THIS_MODULE,
+};
+
+static LIST_HEAD(iio_device_types_list);
+static DEFINE_MUTEX(iio_device_types_lock);
+
+static
+struct iio_sw_device_type *__iio_find_sw_device_type(const char *name,
+ unsigned len)
+{
+ struct iio_sw_device_type *d = NULL, *iter;
+
+ list_for_each_entry(iter, &iio_device_types_list, list)
+ if (!strcmp(iter->name, name)) {
+ d = iter;
+ break;
+ }
+
+ return d;
+}
+
+int iio_register_sw_device_type(struct iio_sw_device_type *d)
+{
+ struct iio_sw_device_type *iter;
+ int ret = 0;
+
+ mutex_lock(&iio_device_types_lock);
+ iter = __iio_find_sw_device_type(d->name, strlen(d->name));
+ if (iter)
+ ret = -EBUSY;
+ else
+ list_add_tail(&d->list, &iio_device_types_list);
+ mutex_unlock(&iio_device_types_lock);
+
+ if (ret)
+ return ret;
+
+ d->group = configfs_register_default_group(iio_devices_group, d->name,
+ &iio_device_type_group_type);
+ if (IS_ERR(d->group))
+ ret = PTR_ERR(d->group);
+
+ return ret;
+}
+EXPORT_SYMBOL(iio_register_sw_device_type);
+
+void iio_unregister_sw_device_type(struct iio_sw_device_type *dt)
+{
+ struct iio_sw_device_type *iter;
+
+ mutex_lock(&iio_device_types_lock);
+ iter = __iio_find_sw_device_type(dt->name, strlen(dt->name));
+ if (iter)
+ list_del(&dt->list);
+ mutex_unlock(&iio_device_types_lock);
+
+ configfs_unregister_default_group(dt->group);
+}
+EXPORT_SYMBOL(iio_unregister_sw_device_type);
+
+static
+struct iio_sw_device_type *iio_get_sw_device_type(const char *name)
+{
+ struct iio_sw_device_type *dt;
+
+ mutex_lock(&iio_device_types_lock);
+ dt = __iio_find_sw_device_type(name, strlen(name));
+ if (dt && !try_module_get(dt->owner))
+ dt = NULL;
+ mutex_unlock(&iio_device_types_lock);
+
+ return dt;
+}
+
+struct iio_sw_device *iio_sw_device_create(const char *type, const char *name)
+{
+ struct iio_sw_device *d;
+ struct iio_sw_device_type *dt;
+
+ dt = iio_get_sw_device_type(type);
+ if (!dt) {
+ pr_err("Invalid device type: %s\n", type);
+ return ERR_PTR(-EINVAL);
+ }
+ d = dt->ops->probe(name);
+ if (IS_ERR(d))
+ goto out_module_put;
+
+ d->device_type = dt;
+
+ return d;
+out_module_put:
+ module_put(dt->owner);
+ return d;
+}
+EXPORT_SYMBOL(iio_sw_device_create);
+
+void iio_sw_device_destroy(struct iio_sw_device *d)
+{
+ struct iio_sw_device_type *dt = d->device_type;
+
+ dt->ops->remove(d);
+ module_put(dt->owner);
+}
+EXPORT_SYMBOL(iio_sw_device_destroy);
+
+static struct config_group *device_make_group(struct config_group *group,
+ const char *name)
+{
+ struct iio_sw_device *d;
+
+ d = iio_sw_device_create(group->cg_item.ci_name, name);
+ if (IS_ERR(d))
+ return ERR_CAST(d);
+
+ config_item_set_name(&d->group.cg_item, "%s", name);
+ return &d->group;
+}
+
+static void device_drop_group(struct config_group *group,
+ struct config_item *item)
+{
+ struct iio_sw_device *d = to_iio_sw_device(item);
+
+ iio_sw_device_destroy(d);
+ config_item_put(item);
+}
+
+static struct configfs_group_operations device_ops = {
+ .make_group = &device_make_group,
+ .drop_item = &device_drop_group,
+};
+
+static struct config_item_type iio_device_type_group_type = {
+ .ct_group_ops = &device_ops,
+ .ct_owner = THIS_MODULE,
+};
+
+static int __init iio_sw_device_init(void)
+{
+ iio_devices_group =
+ configfs_register_default_group(&iio_configfs_subsys.su_group,
+ "devices",
+ &iio_devices_group_type);
+ return PTR_ERR_OR_ZERO(iio_devices_group);
+}
+module_init(iio_sw_device_init);
+
+static void __exit iio_sw_device_exit(void)
+{
+ configfs_unregister_default_group(iio_devices_group);
+}
+module_exit(iio_sw_device_exit);
+
+MODULE_AUTHOR("Daniel Baluta <[email protected]>");
+MODULE_DESCRIPTION("Industrial I/O software devices support");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/iio/sw_device.h b/include/linux/iio/sw_device.h
new file mode 100644
index 0000000..672a9ad
--- /dev/null
+++ b/include/linux/iio/sw_device.h
@@ -0,0 +1,70 @@
+/*
+ * Industrial I/O software device interface
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef __IIO_SW_DEVICE
+#define __IIO_SW_DEVICE
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/iio/iio.h>
+#include <linux/configfs.h>
+
+#define module_iio_sw_device_driver(__iio_sw_device_type) \
+ module_driver(__iio_sw_device_type, iio_register_sw_device_type, \
+ iio_unregister_sw_device_type)
+
+struct iio_sw_device_ops;
+
+struct iio_sw_device_type {
+ const char *name;
+ struct module *owner;
+ const struct iio_sw_device_ops *ops;
+ struct list_head list;
+ struct config_group *group;
+};
+
+struct iio_sw_device {
+ struct iio_dev *device;
+ struct iio_sw_device_type *device_type;
+ struct config_group group;
+};
+
+struct iio_sw_device_ops {
+ struct iio_sw_device* (*probe)(const char *);
+ int (*remove)(struct iio_sw_device *);
+};
+
+static inline
+struct iio_sw_device *to_iio_sw_device(struct config_item *item)
+{
+ return container_of(to_config_group(item), struct iio_sw_device,
+ group);
+}
+
+int iio_register_sw_device_type(struct iio_sw_device_type *dt);
+void iio_unregister_sw_device_type(struct iio_sw_device_type *dt);
+
+struct iio_sw_device *iio_sw_device_create(const char *, const char *);
+void iio_sw_device_destroy(struct iio_sw_device *);
+
+int iio_sw_device_type_configfs_register(struct iio_sw_device_type *dt);
+void iio_sw_device_type_configfs_unregister(struct iio_sw_device_type *dt);
+
+static inline
+void iio_swd_group_init_type_name(struct iio_sw_device *d,
+ const char *name,
+ struct config_item_type *type)
+{
+#ifdef CONFIG_CONFIGFS_FS
+ config_group_init_type_name(&d->group, name, type);
+#endif
+}
+
+#endif /* __IIO_SW_DEVICE */
--
2.5.0

2016-04-20 15:44:46

by Lars-Peter Clausen

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] Documentation: iio: Add IIO software devices docs

On 04/20/2016 05:45 PM, Daniel Baluta wrote:

> +
> +What: /config/iio/triggers/dummy

s/triggers/devices

2016-04-20 15:51:29

by Daniel Baluta

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] Documentation: iio: Add IIO software devices docs

On Wed, Apr 20, 2016 at 6:44 PM, Lars-Peter Clausen <[email protected]> wrote:
> On 04/20/2016 05:45 PM, Daniel Baluta wrote:
>
>> +
>> +What: /config/iio/triggers/dummy
>
> s/triggers/devices

:D, will fix in v2. Obviously, one problem of this RFC is that we
have a lot of duplicate code between the triggers and devices
in configfs support.

2016-04-23 21:44:06

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] Documentation: iio: Add IIO software devices docs

On 20/04/16 16:51, Daniel Baluta wrote:
> On Wed, Apr 20, 2016 at 6:44 PM, Lars-Peter Clausen <[email protected]> wrote:
>> On 04/20/2016 05:45 PM, Daniel Baluta wrote:
>>
>>> +
>>> +What: /config/iio/triggers/dummy
>>
>> s/triggers/devices
>
> :D, will fix in v2. Obviously, one problem of this RFC is that we
> have a lot of duplicate code between the triggers and devices
> in configfs support.
>
We could take this as is, and do the refactor as a follow up series
- kind of up to you really.

The series as a whole looks pretty good to me though a bit of tidying
up is needed in one or two corners. Will take a closer look....

2016-04-24 11:28:54

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [RFC PATCH 0/3] Introduce support for creating IIO devices via configfs

On 20/04/16 16:45, Daniel Baluta wrote:
> For testing purposes is nice to have a quick way of creating IIO devices.
> This patch series introduces support for creating IIO devices via configs
> (patch 1), allowing users to register "device types". For the moment we
> support "dummy" device type (patch 2).
>
> This is just a RFC in order to see if the interface is acceptable. We also
> need a way to create IIO devices with configurable number of channels.
>
> Patch 3 introduces configfs entries documentation for easier review.
>
> Daniel Baluta (3):
> iio: Add support for creating IIO devices via configfs
> iio: dummy: Convert IIO dummy to configfs
> Documentation: iio: Add IIO software devices docs
>
> Documentation/ABI/testing/configfs-iio | 13 +++
> drivers/iio/Kconfig | 9 ++
> drivers/iio/Makefile | 1 +
> drivers/iio/dummy/iio_simple_dummy.c | 98 ++++++------------
> drivers/iio/industrialio-sw-device.c | 181 +++++++++++++++++++++++++++++++++
> include/linux/iio/sw_device.h | 70 +++++++++++++
> 6 files changed, 307 insertions(+), 65 deletions(-)
> create mode 100644 drivers/iio/industrialio-sw-device.c
> create mode 100644 include/linux/iio/sw_device.h
>
Sorry, I was a muppet and delete patch one due to a misstap on my phone...
Anyhow, pasting it in here to review...

> This is similar with support for creating triggers via configfs.
> Devices will be hosted under:
> * /config/iio/devices
>
> We allow users to register "device types" under:
> * /config/iio/devices/<device_types>/
>
> Signed-off-by: Daniel Baluta <[email protected]>
As you observed, there is room in here to share some code with the sw-trigger
support. Looks like we are going to have an iio-configfs helper library
or perhaps just move them into the industrialio-configfs module...

However, I'm not sure it's actually a good idea. Seems to me that the amount
of fiddly indirection needed would outway the advantages in reduced code
replication.

Other than a few nitpicks this looks good to me - though that's not surprising
as it's a find and replace job on the trigger version!

Jonathan
> ---
> drivers/iio/Kconfig | 9 ++
> drivers/iio/Makefile | 1 +
> drivers/iio/industrialio-sw-device.c | 181 +++++++++++++++++++++++++++++++++++
> include/linux/iio/sw_device.h | 70 ++++++++++++++
> 4 files changed, 261 insertions(+)
> create mode 100644 drivers/iio/industrialio-sw-device.c
> create mode 100644 include/linux/iio/sw_device.h
>
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 505e921..77711a3 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -46,6 +46,15 @@ config IIO_CONSUMERS_PER_TRIGGER
> This value controls the maximum number of consumers that a
> given trigger may handle. Default is 2.
>
> +config IIO_SW_DEVICE
> + tristate "Enable software IIO device support"
> + select IIO_CONFIGFS
> + help
> + Provides IIO core support for software device. A software
> + device can be created via configfs or directly by a driver
> + using the API provided.
> +
One blank line too many here...
> +
> config IIO_SW_TRIGGER
> tristate "Enable software triggers support"
> select IIO_CONFIGFS
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 20f6490..87e4c43 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -8,6 +8,7 @@ industrialio-$(CONFIG_IIO_BUFFER) += industrialio-buffer.o
> industrialio-$(CONFIG_IIO_TRIGGER) += industrialio-trigger.o
>
> obj-$(CONFIG_IIO_CONFIGFS) += industrialio-configfs.o
> +obj-$(CONFIG_IIO_SW_DEVICE) += industrialio-sw-device.o
> obj-$(CONFIG_IIO_SW_TRIGGER) += industrialio-sw-trigger.o
> obj-$(CONFIG_IIO_TRIGGERED_EVENT) += industrialio-triggered-event.o
>
> diff --git a/drivers/iio/industrialio-sw-device.c b/drivers/iio/industrialio-sw-device.c
> new file mode 100644
> index 0000000..243aaf4
> --- /dev/null
> +++ b/drivers/iio/industrialio-sw-device.c
> @@ -0,0 +1,181 @@
> +/*
> + * The Industrial I/O core, software IIO devices functions
> + *
> + * Copyright (c) 2015 Intel Corporation
I think you can reasonably claim 2016 - but up to you.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/kmod.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +
> +#include <linux/iio/sw_device.h>
> +#include <linux/iio/configfs.h>
> +#include <linux/configfs.h>
> +
> +static struct config_group *iio_devices_group;
> +static struct config_item_type iio_device_type_group_type;
> +
> +static struct config_item_type iio_devices_group_type = {
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static LIST_HEAD(iio_device_types_list);
> +static DEFINE_MUTEX(iio_device_types_lock);
> +
> +static
> +struct iio_sw_device_type *__iio_find_sw_device_type(const char *name,
> + unsigned len)
> +{
> + struct iio_sw_device_type *d = NULL, *iter;
> +
> + list_for_each_entry(iter, &iio_device_types_list, list)
> + if (!strcmp(iter->name, name)) {
> + d = iter;
> + break;
> + }
> +
> + return d;
> +}
> +
> +int iio_register_sw_device_type(struct iio_sw_device_type *d)
> +{
> + struct iio_sw_device_type *iter;
> + int ret = 0;
> +
> + mutex_lock(&iio_device_types_lock);
> + iter = __iio_find_sw_device_type(d->name, strlen(d->name));
> + if (iter)
> + ret = -EBUSY;
> + else
> + list_add_tail(&d->list, &iio_device_types_list);
> + mutex_unlock(&iio_device_types_lock);
> +
> + if (ret)
> + return ret;
> +
> + d->group = configfs_register_default_group(iio_devices_group, d->name,
> + &iio_device_type_group_type);
> + if (IS_ERR(d->group))
> + ret = PTR_ERR(d->group);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(iio_register_sw_device_type);
> +
> +void iio_unregister_sw_device_type(struct iio_sw_device_type *dt)
> +{
> + struct iio_sw_device_type *iter;
> +
> + mutex_lock(&iio_device_types_lock);
> + iter = __iio_find_sw_device_type(dt->name, strlen(dt->name));
> + if (iter)
> + list_del(&dt->list);
> + mutex_unlock(&iio_device_types_lock);
> +
> + configfs_unregister_default_group(dt->group);
> +}
> +EXPORT_SYMBOL(iio_unregister_sw_device_type);
> +
> +static
> +struct iio_sw_device_type *iio_get_sw_device_type(const char *name)
> +{
> + struct iio_sw_device_type *dt;
> +
> + mutex_lock(&iio_device_types_lock);
> + dt = __iio_find_sw_device_type(name, strlen(name));
> + if (dt && !try_module_get(dt->owner))
> + dt = NULL;
> + mutex_unlock(&iio_device_types_lock);
> +
> + return dt;
> +}
> +
> +struct iio_sw_device *iio_sw_device_create(const char *type, const char *name)
> +{
> + struct iio_sw_device *d;
> + struct iio_sw_device_type *dt;
> +
> + dt = iio_get_sw_device_type(type);
> + if (!dt) {
> + pr_err("Invalid device type: %s\n", type);
> + return ERR_PTR(-EINVAL);
> + }
> + d = dt->ops->probe(name);
> + if (IS_ERR(d))
> + goto out_module_put;
> +
> + d->device_type = dt;
> +
> + return d;
> +out_module_put:
> + module_put(dt->owner);
> + return d;
> +}
> +EXPORT_SYMBOL(iio_sw_device_create);
> +
> +void iio_sw_device_destroy(struct iio_sw_device *d)
> +{
> + struct iio_sw_device_type *dt = d->device_type;
> +
> + dt->ops->remove(d);
> + module_put(dt->owner);
> +}
> +EXPORT_SYMBOL(iio_sw_device_destroy);
> +
> +static struct config_group *device_make_group(struct config_group *group,
> + const char *name)
> +{
> + struct iio_sw_device *d;
> +
> + d = iio_sw_device_create(group->cg_item.ci_name, name);
> + if (IS_ERR(d))
> + return ERR_CAST(d);
> +
> + config_item_set_name(&d->group.cg_item, "%s", name);
blank line here (funilly enough yuo have one in the trigger version)
> + return &d->group;
> +}
> +
> +static void device_drop_group(struct config_group *group,
> + struct config_item *item)
> +{
> + struct iio_sw_device *d = to_iio_sw_device(item);
> +
> + iio_sw_device_destroy(d);
> + config_item_put(item);
> +}
> +
> +static struct configfs_group_operations device_ops = {
> + .make_group = &device_make_group,
> + .drop_item = &device_drop_group,
> +};
> +
> +static struct config_item_type iio_device_type_group_type = {
> + .ct_group_ops = &device_ops,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static int __init iio_sw_device_init(void)
> +{
> + iio_devices_group =
> + configfs_register_default_group(&iio_configfs_subsys.su_group,
> + "devices",
> + &iio_devices_group_type);
> + return PTR_ERR_OR_ZERO(iio_devices_group);
> +}
> +module_init(iio_sw_device_init);
> +
> +static void __exit iio_sw_device_exit(void)
> +{
> + configfs_unregister_default_group(iio_devices_group);
> +}
> +module_exit(iio_sw_device_exit);
> +
> +MODULE_AUTHOR("Daniel Baluta <[email protected]>");
> +MODULE_DESCRIPTION("Industrial I/O software devices support");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/iio/sw_device.h b/include/linux/iio/sw_device.h
> new file mode 100644
> index 0000000..672a9ad
> --- /dev/null
> +++ b/include/linux/iio/sw_device.h
> @@ -0,0 +1,70 @@
> +/*
> + * Industrial I/O software device interface
> + *
> + * Copyright (c) 2015 Intel Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#ifndef __IIO_SW_DEVICE
> +#define __IIO_SW_DEVICE
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/iio/iio.h>
> +#include <linux/configfs.h>
> +
> +#define module_iio_sw_device_driver(__iio_sw_device_type) \
> + module_driver(__iio_sw_device_type, iio_register_sw_device_type, \
> + iio_unregister_sw_device_type)
> +
> +struct iio_sw_device_ops;
> +
> +struct iio_sw_device_type {
> + const char *name;
> + struct module *owner;
> + const struct iio_sw_device_ops *ops;
> + struct list_head list;
> + struct config_group *group;
> +};
> +
> +struct iio_sw_device {
> + struct iio_dev *device;
> + struct iio_sw_device_type *device_type;
> + struct config_group group;
> +};
> +
> +struct iio_sw_device_ops {
> + struct iio_sw_device* (*probe)(const char *);
> + int (*remove)(struct iio_sw_device *);
> +};
> +
> +static inline
> +struct iio_sw_device *to_iio_sw_device(struct config_item *item)
> +{
> + return container_of(to_config_group(item), struct iio_sw_device,
> + group);
> +}
> +
> +int iio_register_sw_device_type(struct iio_sw_device_type *dt);
> +void iio_unregister_sw_device_type(struct iio_sw_device_type *dt);
> +
> +struct iio_sw_device *iio_sw_device_create(const char *, const char *);
> +void iio_sw_device_destroy(struct iio_sw_device *);
> +
> +int iio_sw_device_type_configfs_register(struct iio_sw_device_type *dt);
> +void iio_sw_device_type_configfs_unregister(struct iio_sw_device_type *dt);
> +
> +static inline
> +void iio_swd_group_init_type_name(struct iio_sw_device *d,
> + const char *name,
> + struct config_item_type *type)
> +{
> +#ifdef CONFIG_CONFIGFS_FS
> + config_group_init_type_name(&d->group, name, type);
> +#endif
> +}
> +
> +#endif /* __IIO_SW_DEVICE */
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


2016-04-24 11:36:32

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [RFC PATCH 2/3] iio: dummy: Convert IIO dummy to configfs

On 20/04/16 16:45, Daniel Baluta wrote:
> We register a new device type named "dummy", this will create a
> configfs entry under:
> * /config/iio/devices/dummy.
>
> Creating dummy devices is now as simple as:
>
> $ mkdir /config/iio/devices/dummy/my_dummy_device
>
> Signed-off-by: Daniel Baluta <[email protected]>
Looks very nice.

I don't think it is unreasonable to demand people have configfs for
the dummy driver so very happy to have this cleaned up.

Now to try it with a few thousand devices and see what breaks ;)

1 trivial point inline.

Jonathan
> ---
> drivers/iio/dummy/iio_simple_dummy.c | 98 ++++++++++++------------------------
> 1 file changed, 33 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/iio/dummy/iio_simple_dummy.c b/drivers/iio/dummy/iio_simple_dummy.c
> index 43fe4ba..65f92f8 100644
> --- a/drivers/iio/dummy/iio_simple_dummy.c
> +++ b/drivers/iio/dummy/iio_simple_dummy.c
> @@ -22,21 +22,12 @@
> #include <linux/iio/sysfs.h>
> #include <linux/iio/events.h>
> #include <linux/iio/buffer.h>
> +#include <linux/iio/sw_device.h>
> #include "iio_simple_dummy.h"
>
> -/*
> - * A few elements needed to fake a bus for this driver
> - * Note instances parameter controls how many of these
> - * dummy devices are registered.
> - */
> -static unsigned instances = 1;
> -module_param(instances, uint, 0);
> -
> -/* Pointer array used to fake bus elements */
> -static struct iio_dev **iio_dummy_devs;
> -
> -/* Fake a name for the part number, usually obtained from the id table */
> -static const char *iio_dummy_part_number = "iio_dummy_part_no";
> +static struct config_item_type iio_dummy_type = {
> + .ct_owner = THIS_MODULE,
> +};
>
> /**
> * struct iio_dummy_accel_calibscale - realworld to register mapping
> @@ -572,12 +563,18 @@ static int iio_dummy_init_device(struct iio_dev *indio_dev)
> * const struct i2c_device_id *id)
> * SPI: iio_dummy_probe(struct spi_device *spi)
> */
> -static int iio_dummy_probe(int index)
> +static struct iio_sw_device *iio_dummy_probe(const char *name)
> {
> int ret;
> struct iio_dev *indio_dev;
> struct iio_dummy_state *st;
> + struct iio_sw_device *swd;
>
> + swd = kzalloc(sizeof(*swd), GFP_KERNEL);
> + if (!swd) {
> + ret = -ENOMEM;
> + goto error_kzalloc;
> + }
> /*
> * Allocate an IIO device.
> *
> @@ -608,7 +605,7 @@ static int iio_dummy_probe(int index)
> * i2c_set_clientdata(client, indio_dev);
> * spi_set_drvdata(spi, indio_dev);
> */
> - iio_dummy_devs[index] = indio_dev;
> + swd->device = indio_dev;
>
> /*
> * Set the device name.
> @@ -619,7 +616,7 @@ static int iio_dummy_probe(int index)
> * indio_dev->name = id->name;
> * indio_dev->name = spi_get_device_id(spi)->name;
> */
> - indio_dev->name = iio_dummy_part_number;
> + indio_dev->name = name;
>
> /* Provide description of available channels */
> indio_dev->channels = iio_dummy_channels;
> @@ -646,7 +643,8 @@ static int iio_dummy_probe(int index)
> if (ret < 0)
> goto error_unconfigure_buffer;
>
> - return 0;
> + iio_swd_group_init_type_name(swd, name, &iio_dummy_type);
> + return swd;
> error_unconfigure_buffer:
> iio_simple_dummy_unconfigure_buffer(indio_dev);
> error_unregister_events:
> @@ -654,16 +652,18 @@ error_unregister_events:
> error_free_device:
> iio_device_free(indio_dev);
> error_ret:
> - return ret;
> + kfree(swd);
> +error_kzalloc:
> + return ERR_PTR(ret);
> }
>
> /**
> * iio_dummy_remove() - device instance removal function
> - * @index: device index.
> + * @swd: pointer to software IIO device abstraction
> *
> * Parameters follow those of iio_dummy_probe for buses.
> */
> -static void iio_dummy_remove(int index)
> +static int iio_dummy_remove(struct iio_sw_device *swd)
> {
> /*
> * Get a pointer to the device instance iio_dev structure
> @@ -671,7 +671,7 @@ static void iio_dummy_remove(int index)
> * struct iio_dev *indio_dev = i2c_get_clientdata(client);
> * struct iio_dev *indio_dev = spi_get_drvdata(spi);
> */
> - struct iio_dev *indio_dev = iio_dummy_devs[index];
> + struct iio_dev *indio_dev = swd->device;
>
> /* Unregister the device */
> iio_device_unregister(indio_dev);
> @@ -685,10 +685,10 @@ static void iio_dummy_remove(int index)
>
> /* Free all structures */
> iio_device_free(indio_dev);
blank line here (nitpick of the day ;)
> + return 0;
> }
> -
> /**
> - * iio_dummy_init() - device driver registration
> + * module_iio_sw_device_driver() - device driver registration
> *
> * Varies depending on bus type of the device. As there is no device
> * here, call probe directly. For information on device registration
> @@ -697,50 +697,18 @@ static void iio_dummy_remove(int index)
> * spi:
> * Documentation/spi/spi-summary
> */
> -static __init int iio_dummy_init(void)
> -{
> - int i, ret;
> -
> - if (instances > 10) {
> - instances = 1;
> - return -EINVAL;
> - }
> -
> - /* Fake a bus */
> - iio_dummy_devs = kcalloc(instances, sizeof(*iio_dummy_devs),
> - GFP_KERNEL);
> - /* Here we have no actual device so call probe */
> - for (i = 0; i < instances; i++) {
> - ret = iio_dummy_probe(i);
> - if (ret < 0)
> - goto error_remove_devs;
> - }
> - return 0;
> -
> -error_remove_devs:
> - while (i--)
> - iio_dummy_remove(i);
> -
> - kfree(iio_dummy_devs);
> - return ret;
> -}
> -module_init(iio_dummy_init);
> +static const struct iio_sw_device_ops iio_dummy_device_ops = {
> + .probe = iio_dummy_probe,
> + .remove = iio_dummy_remove,
> +};
>
> -/**
> - * iio_dummy_exit() - device driver removal
> - *
> - * Varies depending on bus type of the device.
> - * As there is no device here, call remove directly.
> - */
> -static __exit void iio_dummy_exit(void)
> -{
> - int i;
> +static struct iio_sw_device_type iio_dummy_device = {
> + .name = "dummy",
> + .owner = THIS_MODULE,
> + .ops = &iio_dummy_device_ops,
> +};
>
> - for (i = 0; i < instances; i++)
> - iio_dummy_remove(i);
> - kfree(iio_dummy_devs);
> -}
> -module_exit(iio_dummy_exit);
> +module_iio_sw_device_driver(iio_dummy_device);
>
> MODULE_AUTHOR("Jonathan Cameron <[email protected]>");
> MODULE_DESCRIPTION("IIO dummy driver");
>

2016-04-24 11:37:00

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [RFC PATCH 3/3] Documentation: iio: Add IIO software devices docs

On 20/04/16 16:45, Daniel Baluta wrote:
> Signed-off-by: Daniel Baluta <[email protected]>
Other than the deliberate mistake, looks good.

J
> ---
> Documentation/ABI/testing/configfs-iio | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/Documentation/ABI/testing/configfs-iio b/Documentation/ABI/testing/configfs-iio
> index 2483756..84b6bd1 100644
> --- a/Documentation/ABI/testing/configfs-iio
> +++ b/Documentation/ABI/testing/configfs-iio
> @@ -19,3 +19,16 @@ KernelVersion: 4.4
> Description:
> High resolution timers directory. Creating a directory here
> will result in creating a hrtimer trigger in the IIO subsystem.
> +
> +What: /config/iio/devices
> +Date: April 2016
> +KernelVersion: 4.7
> +Description:
> + Industrial IO software devices directory.
> +
> +What: /config/iio/triggers/dummy
> +Date: April 2016
> +KernelVersion: 4.7
> +Description:
> + Dummy IIO devices directory. Creating a directory here will result
> + in creating a dummy IIO device in the IIO subystem.
>

2016-04-25 07:38:34

by Daniel Baluta

[permalink] [raw]
Subject: Re: [RFC PATCH 0/3] Introduce support for creating IIO devices via configfs

On Sun, Apr 24, 2016 at 2:28 PM, Jonathan Cameron <[email protected]> wrote:
> On 20/04/16 16:45, Daniel Baluta wrote:
>> For testing purposes is nice to have a quick way of creating IIO devices.
>> This patch series introduces support for creating IIO devices via configs
>> (patch 1), allowing users to register "device types". For the moment we
>> support "dummy" device type (patch 2).
>>
>> This is just a RFC in order to see if the interface is acceptable. We also
>> need a way to create IIO devices with configurable number of channels.
>>
>> Patch 3 introduces configfs entries documentation for easier review.
>>
>> Daniel Baluta (3):
>> iio: Add support for creating IIO devices via configfs
>> iio: dummy: Convert IIO dummy to configfs
>> Documentation: iio: Add IIO software devices docs
>>
>> Documentation/ABI/testing/configfs-iio | 13 +++
>> drivers/iio/Kconfig | 9 ++
>> drivers/iio/Makefile | 1 +
>> drivers/iio/dummy/iio_simple_dummy.c | 98 ++++++------------
>> drivers/iio/industrialio-sw-device.c | 181 +++++++++++++++++++++++++++++++++
>> include/linux/iio/sw_device.h | 70 +++++++++++++
>> 6 files changed, 307 insertions(+), 65 deletions(-)
>> create mode 100644 drivers/iio/industrialio-sw-device.c
>> create mode 100644 include/linux/iio/sw_device.h
>>
> Sorry, I was a muppet and delete patch one due to a misstap on my phone...
> Anyhow, pasting it in here to review...
>
>> This is similar with support for creating triggers via configfs.
>> Devices will be hosted under:
>> * /config/iio/devices
>>
>> We allow users to register "device types" under:
>> * /config/iio/devices/<device_types>/
>>
>> Signed-off-by: Daniel Baluta <[email protected]>
> As you observed, there is room in here to share some code with the sw-trigger
> support. Looks like we are going to have an iio-configfs helper library
> or perhaps just move them into the industrialio-configfs module...
>
> However, I'm not sure it's actually a good idea. Seems to me that the amount
> of fiddly indirection needed would outway the advantages in reduced code
> replication.
>
> Other than a few nitpicks this looks good to me - though that's not surprising
> as it's a find and replace job on the trigger version!

Yes, I think we can live with the code as it is now.

I'm now thinking of a more flexibly interface, that will allow us creating an
IIO devices with configurable number and types of IIO channels.

This will be later useful in evaluating the choices for sensor hubs with lots
of sensors attached:

Single IIO device for all sensors VS One IIO device per sensor type.

Daniel.