* [RFC PATCH 2/3] iio: dummy: Convert IIO dummy to configfs
2016-04-20 15:45 [RFC PATCH 0/3] Introduce support for creating IIO devices via configfs Daniel Baluta
@ 2016-04-20 15:45 ` Daniel Baluta
2016-04-24 11:36 ` Jonathan Cameron
2016-04-20 15:45 ` [RFC PATCH 3/3] Documentation: iio: Add IIO software devices docs Daniel Baluta
2016-04-24 11:28 ` [RFC PATCH 0/3] Introduce support for creating IIO devices via configfs Jonathan Cameron
2 siblings, 1 reply; 10+ messages in thread
From: Daniel Baluta @ 2016-04-20 15:45 UTC (permalink / raw)
To: jic23; +Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel, daniel.baluta
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 <daniel.baluta@intel.com>
---
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 <jic23@kernel.org>");
MODULE_DESCRIPTION("IIO dummy driver");
--
2.5.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [RFC PATCH 0/3] Introduce support for creating IIO devices via configfs
2016-04-20 15:45 [RFC PATCH 0/3] Introduce support for creating IIO devices via configfs Daniel Baluta
2016-04-20 15:45 ` [RFC PATCH 2/3] iio: dummy: Convert IIO dummy to configfs Daniel Baluta
2016-04-20 15:45 ` [RFC PATCH 3/3] Documentation: iio: Add IIO software devices docs Daniel Baluta
@ 2016-04-24 11:28 ` Jonathan Cameron
2016-04-25 7:38 ` Daniel Baluta
2 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2016-04-24 11:28 UTC (permalink / raw)
To: Daniel Baluta; +Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel
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 <daniel.baluta@intel.com>
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 <daniel.baluta@intel.com>");
> +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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread