High frequency software trigger

High frequency software trigger

[Marten Svanfeldt]

Hi,

I have recently begun looking at utilizing iio for the input part of a 
robotics system but have some doubts about its usability for which I 
would like your comments.

Today we have (in our embedded system) a number of custom written 
drivers interfacing SPI attached accelerometers and AD converters, 
utilizing hrtimers for software triggering of sampling at 50 to 250 Hz. 
Herein lays my current question, has anyone used the IIO system with 
these type of sampling rates? The RTC based trigger seems to only give 
second or higher intervals.

I will be looking at converting our special drivers to be acceptable for 
upstream submission, same for a hrtimers based trigger, should that be a 
good idea.

Best regards
Marten Svanfeldt

RE: High frequency software trigger

[Hennerich, Michael]

Marten Svanfeldt wrote on 2011-03-04:
> Hi,
>
> I have recently begun looking at utilizing iio for the input part of a
> robotics system but have some doubts about its usability for which I
> would like your comments.
>
> Today we have (in our embedded system) a number of custom written
> drivers interfacing SPI attached accelerometers and AD converters,
> utilizing hrtimers for software triggering of sampling at 50 to 250 Hz.
> Herein lays my current question, has anyone used the IIO system with
> these type of sampling rates? The RTC based trigger seems to only give
> second or higher intervals.
>
> I will be looking at converting our special drivers to be acceptable
> for upstream submission, same for a hrtimers based trigger, should
> that be a good idea.
>
> Best regards
> Marten Svanfeldt

I recently submitted a trigger source driver that utilizes Blackfin hardware timer.

http://wiki.analog.com/software/linux/docs/iio/iio-trig-bfin-timer

Not sure what your platform is, but there might be similar peripherals,
that can generate sub second cyclic interrupts.

I successfully used it on a fast Blackfin processor at frequencies of 10kHz and above,
with this simple demo application.

http://wiki.analog.com/software/linux/docs/iio/iio_netscope

Greetings,
Michael

--
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368; Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif

Re: High frequency software trigger

[Marten Svanfeldt]

On 2011-03-04 17:19, Hennerich, Michael wrote:
>
> I recently submitted a trigger source driver that utilizes Blackfin hardware timer.
>
> http://wiki.analog.com/software/linux/docs/iio/iio-trig-bfin-timer

Thank you, this had passed me by. Looking at the source, it seems that a 
hrtimer based trigger would be something similar, except not tied to any 
specific platform.

>
> Not sure what your platform is, but there might be similar peripherals,
> that can generate sub second cyclic interrupts.
The platform in question is an TI OMAP3 (3530) based system, but my goal 
would be to utilize hrtimers as they are not tied to a specific 
platform. They are supposed to work on our target platform, but I 
haven't had time to test them extensively yet.

> I successfully used it on a fast Blackfin processor at frequencies of 10kHz and above,
> with this simple demo application.
>
> http://wiki.analog.com/software/linux/docs/iio/iio_netscope
10kHz sounds reassuring, in my case it is about 250Hz max, but some 13 
or so channels. Did you get any idea where the limitations lies in terms 
of system load etc? Of course this depends on the exact setup, just 
trying to find all possible sides of moving our code over to be IIO-based.

Regards
Marten Svanfeldt

Re: High frequency software trigger

[Jonathan Cameron]

Hi Marten,
>> I recently submitted a trigger source driver that utilizes Blackfin hardware timer.
>>
>> http://wiki.analog.com/software/linux/docs/iio/iio-trig-bfin-timer
> 
> Thank you, this had passed me by. Looking at the source, it seems
> that a hrtimer based trigger would be something similar, except not
> tied to any specific platform.
Agreed.  The rtc trigger is a silly historical relic from the pxa platform I use
which does do higher frequency periodic rtc tics.  I also have a driver for that
which effectively does the same as Michaels blackfin one, but via the rtc
interface (gives me 7 additional rtcs).

A generic hrtimer approach is something I've thought about from time to time
but never gotten round to actually writing!  Hence I'd certainly be interested
in your generic driver.
>>
>> Not sure what your platform is, but there might be similar peripherals,
>> that can generate sub second cyclic interrupts.
> The platform in question is an TI OMAP3 (3530) based system, but my
> goal would be to utilize hrtimers as they are not tied to a specific
> platform. They are supposed to work on our target platform, but I
> haven't had time to test them extensively yet.
The advantage of using specific timers originally was the simplicity of setting them
up. They look very much like an interrupt coming off the sensor hence can be set up
then left alone.
>
>> I successfully used it on a fast Blackfin processor at frequencies of 10kHz and above,
>> with this simple demo application.
>>
>> http://wiki.analog.com/software/linux/docs/iio/iio_netscope
> 10kHz sounds reassuring, in my case it is about 250Hz max, but some
> 13 or so channels. Did you get any idea where the limitations lies in
> terms of system load etc? Of course this depends on the exact setup,
> just trying to find all possible sides of moving our code over to be
> IIO-based.
I'm quite happily doing 4 channels off the periodic rtc at 1Khz on a pxa271.
Things got a bit choppy in timing when we went to 2Khz but it works well enough.
The uneven timing had definite spikes so I suspect other things were hogging
the processor from time to time.  I should probably revisit these tests and see
if I can pin down exactly what is causing the issue (that was about 2 years ago
so it may well have gone away!)  

I'm only superficially familiar with the abilities of hrtimers.
Perhaps you could post your current code as an RFC so we can get an idea of exactly
how this would work?

Thanks,

Jonathan

Re: High frequency software trigger

[Marten Svanfeldt]

On 2011-03-04 19:10, Jonathan Cameron wrote:
> I'm only superficially familiar with the abilities of hrtimers.
> Perhaps you could post your current code as an RFC so we can get an idea of exactly
> how this would work?

Right now my work is a mix of driver interfacing code and triggering, 
being the result of hacking away during my thesis work. I will try to 
write a cleaner hrtimers based trigger during the next week(s) and send 
it for review, with the limit of not having access to our target 
platform right now so testing will be limited.

Regards
Marten Svanfeldt

[RFC] IIO: trigger: New hrtimer based trigger driver

[Marten Svanfeldt]

I took the time to extract the hrtimer bits of my ugly driver and
transplanted it to a base of the rtc trigger and blackfin trigger.

Consider this driver an early RFC, it has been compile tested for
x86 and ARM, but I haven't had any possibility to run it as I don't
have access to our hardware platform right now.

Regards
Marten Svanfeldt

[PATCH] IIO: trigger: New hrtimer based trigger driver

[Marten Svanfeldt]

This driver provide a way to utilize hrtimer as an IIO trigger source.

Signed-off-by: Marten Svanfeldt <marten@intuitiveaerial.com>
---
 drivers/staging/iio/trigger/Kconfig            |    6 +
 drivers/staging/iio/trigger/Makefile           |    2 +
 drivers/staging/iio/trigger/iio-trig-hrtimer.c |  208 ++++++++++++++++++++++++
 3 files changed, 216 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/iio/trigger/iio-trig-hrtimer.c

diff --git a/drivers/staging/iio/trigger/Kconfig b/drivers/staging/iio/trigger/Kconfig
index d842a58..35d676a 100644
--- a/drivers/staging/iio/trigger/Kconfig
+++ b/drivers/staging/iio/trigger/Kconfig
@@ -18,4 +18,10 @@ config IIO_GPIO_TRIGGER
 	help
 	  Provides support for using GPIO pins as IIO triggers.
 
+config IIO_HRTIMER_TRIGGER
+        tristate "hrtimer trigger"
+        help
+          Provides support for using hrtimer as periodic IIO
+          trigger.
+
 endif # IIO_TRIGGER
diff --git a/drivers/staging/iio/trigger/Makefile b/drivers/staging/iio/trigger/Makefile
index 10aeca5..d178c66 100644
--- a/drivers/staging/iio/trigger/Makefile
+++ b/drivers/staging/iio/trigger/Makefile
@@ -4,3 +4,5 @@
 
 obj-$(CONFIG_IIO_PERIODIC_RTC_TRIGGER) += iio-trig-periodic-rtc.o
 obj-$(CONFIG_IIO_GPIO_TRIGGER) += iio-trig-gpio.o
+obj-$(CONFIG_IIO_HRTIMER_TRIGGER) += iio-trig-hrtimer.o
+
diff --git a/drivers/staging/iio/trigger/iio-trig-hrtimer.c b/drivers/staging/iio/trigger/iio-trig-hrtimer.c
new file mode 100644
index 0000000..16b5af5
--- /dev/null
+++ b/drivers/staging/iio/trigger/iio-trig-hrtimer.c
@@ -0,0 +1,208 @@
+/**
+ * The industrial I/O periodic hrtimer trigger driver
+ *
+ * Copyright (C) Intuitive Aerial AB
+ * Written by Marten Svanfeldt, marten@intuitiveaerial.com
+ *
+ * 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/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/hrtimer.h>
+#include "../iio.h"
+#include "../trigger.h"
+
+static LIST_HEAD(iio_hrtimer_trigger_list);
+
+struct iio_hrtimer_trig_info {
+	struct iio_trigger *trigger;
+	unsigned int frequency;
+	struct hrtimer timer;
+};
+
+static enum hrtimer_restart iio_trig_hrtimer_trig(struct hrtimer *timer)
+{
+	struct iio_hrtimer_trig_info *trig_info = container_of(timer, struct iio_hrtimer_trig_info, timer);
+
+	iio_trigger_poll(trig_info->trigger, 0);
+	return HRTIMER_RESTART;
+}
+
+static int iio_trig_hrtimer_set_state(struct iio_trigger *trig, bool state)
+{
+	struct iio_hrtimer_trig_info *trig_info = trig->private_data;
+	if(trig_info->frequency == 0)
+		return -EINVAL;
+
+	if(state) {
+		hrtimer_start(&trig_info->timer,
+				ktime_set(0, NSEC_PER_SEC/trig_info->frequency),
+				HRTIMER_MODE_REL);
+	} else {
+		hrtimer_cancel(&trig_info->timer);
+	}
+
+	return 0;
+}
+
+static ssize_t iio_trig_hrtimer_read_freq(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	struct iio_trigger *trig = dev_get_drvdata(dev);
+	struct iio_hrtimer_trig_info *trig_info = trig->private_data;
+	return sprintf(buf, "%u\n", trig_info->frequency);
+}
+
+static ssize_t iio_trig_hrtimer_write_freq(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf,
+					   size_t len)
+{
+	struct iio_trigger *trig = dev_get_drvdata(dev);
+	struct iio_hrtimer_trig_info *trig_info = trig->private_data;
+	unsigned long val;
+	int ret;
+
+	ret = strict_strtoul(buf, 10, &val);
+	if(ret)
+		goto error_ret;
+
+	trig_info->frequency = val;
+
+	return len;
+
+error_ret:
+	return ret;
+}
+
+static IIO_TRIGGER_NAME_ATTR;
+static DEVICE_ATTR(frequency, S_IRUGO | S_IWUSR,
+		iio_trig_hrtimer_read_freq,
+		iio_trig_hrtimer_write_freq);
+
+static struct attribute *iio_trig_hrtimer_attrs[] = {
+	&dev_attr_name.attr,
+	&dev_attr_frequency.attr,
+	NULL,
+};
+
+static const struct attribute_group iio_trig_hrtimer_attr_group = {
+	.attrs = iio_trig_hrtimer_attrs,
+};
+
+static int iio_trig_hrtimer_probe(struct platform_device *dev)
+{
+	char **pdata = dev->dev.platform_data;
+	struct iio_hrtimer_trig_info *trig_info;
+	struct iio_trigger *trig, *trig2;
+
+	int i, ret;
+
+	for(i = 0;; i++) {
+		if(pdata[i] == NULL)
+			break;
+
+		trig = iio_allocate_trigger();
+		if(!trig) {
+			ret = -ENOMEM;
+			goto err_free_completed_reg;
+		}
+		list_add(&trig->alloc_list, &iio_hrtimer_trigger_list);
+
+		trig_info = kzalloc(sizeof(struct iio_hrtimer_trig_info), GFP_KERNEL);
+		if(!trig_info) {
+			ret = -ENOMEM;
+			goto err_put_trigger;
+		}
+		trig_info->trigger = trig;
+		trig->private_data = trig_info;
+		trig->owner = THIS_MODULE;
+		trig->set_trigger_state = &iio_trig_hrtimer_set_state;
+		trig->name = kasprintf(GFP_KERNEL, "hrtimer%s", pdata[i]);
+		if(!trig->name) {
+			ret = -ENOMEM;
+			goto err_free_trig_info;
+		}
+
+		// Setup a hrtimer
+		hrtimer_init(&trig_info->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+		trig_info->timer.function = iio_trig_hrtimer_trig;
+
+		ret = iio_trigger_register(trig);
+		if(ret)
+			goto err_free_name;
+	}
+
+	return 0;
+err_free_name:
+	kfree(trig->name);
+err_free_trig_info:
+	kfree(trig_info);
+err_put_trigger:
+	list_del(&trig->alloc_list);
+	iio_put_trigger(trig);
+err_free_completed_reg:
+	list_for_each_entry_safe(trig,
+				 trig2,
+				 &iio_hrtimer_trigger_list,
+				 alloc_list) {
+		trig_info = trig->private_data;
+		kfree(trig->name);
+		kfree(trig_info);
+		iio_trigger_unregister(trig);
+	}
+
+	return ret;
+}
+
+static int iio_trig_hrtimer_remove(struct platform_device *dev)
+{
+	struct iio_trigger *trig, *trig2;
+	struct iio_hrtimer_trig_info *trig_info;
+
+	list_for_each_entry_safe(trig,
+				 trig2,
+				 &iio_hrtimer_trigger_list,
+				 alloc_list) {
+		trig_info = trig->private_data;
+		hrtimer_cancel(&trig_info->timer);
+		kfree(trig->name);
+		kfree(trig_info);
+		iio_trigger_unregister(trig);
+	}
+
+	return 0;
+}
+
+static struct platform_driver iio_trig_hrtimer_driver = {
+	.driver = {
+		.name = "iio_hrtimer_trigger",
+		.owner = THIS_MODULE
+	},
+	.probe = iio_trig_hrtimer_probe,
+	.remove = iio_trig_hrtimer_remove,
+};
+
+static int __init iio_trig_hrtimer_init(void)
+{
+	return platform_driver_register(&iio_trig_hrtimer_driver);
+}
+
+static void __exit iio_trig_hrtimer_exit(void)
+{
+	return platform_driver_unregister(&iio_trig_hrtimer_driver);
+}
+
+module_init(iio_trig_hrtimer_init);
+module_exit(iio_trig_hrtimer_exit);
+MODULE_AUTHOR("Marten Svanfeldt <marten@intuitiveaerial.com>");
+MODULE_DESCRIPTION("Periodic hrtimer trigger for the iio subsystem");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platofrm:iio-trig-hrtimer");
+
-- 
1.7.0.4

Re: [PATCH] IIO: trigger: New hrtimer based trigger driver

[Jonathan Cameron]

On 03/09/11 14:00, Marten Svanfeldt wrote:
> This driver provide a way to utilize hrtimer as an IIO trigger source.
Looks quite nice.  Some comments inline. I know this is only a quick untested
example, but as I was reading anyway...

Obviously run checkpatch over it at some point.

Otherwise, nice clean little trigger driver.  After cleanup and
testing I would be happy to see this in IIO.  Never realised it
would be this simple :)

Thanks.

Jonathan
> 
> Signed-off-by: Marten Svanfeldt <marten@intuitiveaerial.com>
> ---
>  drivers/staging/iio/trigger/Kconfig            |    6 +
>  drivers/staging/iio/trigger/Makefile           |    2 +
>  drivers/staging/iio/trigger/iio-trig-hrtimer.c |  208 ++++++++++++++++++++++++
>  3 files changed, 216 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/staging/iio/trigger/iio-trig-hrtimer.c
> 
> diff --git a/drivers/staging/iio/trigger/Kconfig b/drivers/staging/iio/trigger/Kconfig
> index d842a58..35d676a 100644
> --- a/drivers/staging/iio/trigger/Kconfig
> +++ b/drivers/staging/iio/trigger/Kconfig
> @@ -18,4 +18,10 @@ config IIO_GPIO_TRIGGER
>  	help
>  	  Provides support for using GPIO pins as IIO triggers.
>  
> +config IIO_HRTIMER_TRIGGER
> +        tristate "hrtimer trigger"
Is it worth spelling out high resolution as not every person
who builds a kernel will know what hr stands for?
> +        help
> +          Provides support for using hrtimer as periodic IIO
> +          trigger.
> +
>  endif # IIO_TRIGGER
> diff --git a/drivers/staging/iio/trigger/Makefile b/drivers/staging/iio/trigger/Makefile
> index 10aeca5..d178c66 100644
> --- a/drivers/staging/iio/trigger/Makefile
> +++ b/drivers/staging/iio/trigger/Makefile
> @@ -4,3 +4,5 @@
>  
>  obj-$(CONFIG_IIO_PERIODIC_RTC_TRIGGER) += iio-trig-periodic-rtc.o
>  obj-$(CONFIG_IIO_GPIO_TRIGGER) += iio-trig-gpio.o
> +obj-$(CONFIG_IIO_HRTIMER_TRIGGER) += iio-trig-hrtimer.o
Oops, I ought to resort these into alphabetical order at some point!
> +
> diff --git a/drivers/staging/iio/trigger/iio-trig-hrtimer.c b/drivers/staging/iio/trigger/iio-trig-hrtimer.c
> new file mode 100644
> index 0000000..16b5af5
> --- /dev/null
> +++ b/drivers/staging/iio/trigger/iio-trig-hrtimer.c
> @@ -0,0 +1,208 @@
> +/**
> + * The industrial I/O periodic hrtimer trigger driver
> + *
> + * Copyright (C) Intuitive Aerial AB
> + * Written by Marten Svanfeldt, marten@intuitiveaerial.com
> + *
> + * 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/platform_device.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/hrtimer.h>
> +#include "../iio.h"
> +#include "../trigger.h"
> +
> +static LIST_HEAD(iio_hrtimer_trigger_list);
> +
> +struct iio_hrtimer_trig_info {
hmm.. I should rework the trigger allocation code to allow for this structure
to be directly embedded in another. Would make things a bit cleaner for you
and probably a number of other users.
> +	struct iio_trigger *trigger;
> +	unsigned int frequency;
> +	struct hrtimer timer;
> +};
> +
> +static enum hrtimer_restart iio_trig_hrtimer_trig(struct hrtimer *timer)
> +{
> +	struct iio_hrtimer_trig_info *trig_info = container_of(timer, struct iio_hrtimer_trig_info, timer);
> +
> +	iio_trigger_poll(trig_info->trigger, 0);
Out of curiosity, how suceptible to drift is this?  I guess the timer doesn't take
into account the amout of time this function takes, e.g how late are we by the time
the timer is restarted?
> +	return HRTIMER_RESTART;
> +}
> +
> +static int iio_trig_hrtimer_set_state(struct iio_trigger *trig, bool state)
> +{
> +	struct iio_hrtimer_trig_info *trig_info = trig->private_data;
> +	if(trig_info->frequency == 0)
> +		return -EINVAL;
> +
> +	if(state) {
> +		hrtimer_start(&trig_info->timer,
> +				ktime_set(0, NSEC_PER_SEC/trig_info->frequency),
> +				HRTIMER_MODE_REL);
> +	} else {
> +		hrtimer_cancel(&trig_info->timer);
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t iio_trig_hrtimer_read_freq(struct device *dev,
> +					  struct device_attribute *attr,
> +					  char *buf)
> +{
> +	struct iio_trigger *trig = dev_get_drvdata(dev);
> +	struct iio_hrtimer_trig_info *trig_info = trig->private_data;
> +	return sprintf(buf, "%u\n", trig_info->frequency);
> +}
> +
> +static ssize_t iio_trig_hrtimer_write_freq(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf,
> +					   size_t len)
> +{
> +	struct iio_trigger *trig = dev_get_drvdata(dev);
> +	struct iio_hrtimer_trig_info *trig_info = trig->private_data;
> +	unsigned long val;
> +	int ret;
> +
> +	ret = strict_strtoul(buf, 10, &val);
> +	if(ret)
> +		goto error_ret;
> +
> +	trig_info->frequency = val;
Only applied on trigger being stopped and restarted I think?
Maybe return -EBUSY if the trigger is running to make this
obvious to userspace.
> +
> +	return len;
> +
> +error_ret:
> +	return ret;
> +}
> +
> +static IIO_TRIGGER_NAME_ATTR;
> +static DEVICE_ATTR(frequency, S_IRUGO | S_IWUSR,
> +		iio_trig_hrtimer_read_freq,
> +		iio_trig_hrtimer_write_freq);
> +
> +static struct attribute *iio_trig_hrtimer_attrs[] = {
> +	&dev_attr_name.attr,
> +	&dev_attr_frequency.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group iio_trig_hrtimer_attr_group = {
> +	.attrs = iio_trig_hrtimer_attrs,
> +};
> +
> +static int iio_trig_hrtimer_probe(struct platform_device *dev)
> +{
> +	char **pdata = dev->dev.platform_data;
> +	struct iio_hrtimer_trig_info *trig_info;
> +	struct iio_trigger *trig, *trig2;
> +
> +	int i, ret;
> +
while(pdata != NULL) then increment pdata pointer?
> +	for(i = 0;; i++) {
> +		if(pdata[i] == NULL)
> +			break;
> +
> +		trig = iio_allocate_trigger();
> +		if(!trig) {
> +			ret = -ENOMEM;
> +			goto err_free_completed_reg;
> +		}
> +		list_add(&trig->alloc_list, &iio_hrtimer_trigger_list);
> +
> +		trig_info = kzalloc(sizeof(struct iio_hrtimer_trig_info), GFP_KERNEL);
> +		if(!trig_info) {
> +			ret = -ENOMEM;
> +			goto err_put_trigger;
> +		}
> +		trig_info->trigger = trig;
> +		trig->private_data = trig_info;
> +		trig->owner = THIS_MODULE;
> +		trig->set_trigger_state = &iio_trig_hrtimer_set_state;
> +		trig->name = kasprintf(GFP_KERNEL, "hrtimer%s", pdata[i]);
> +		if(!trig->name) {
> +			ret = -ENOMEM;
> +			goto err_free_trig_info;
> +		}
> +
> +		// Setup a hrtimer
> +		hrtimer_init(&trig_info->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> +		trig_info->timer.function = iio_trig_hrtimer_trig;
> +
> +		ret = iio_trigger_register(trig);
> +		if(ret)
> +			goto err_free_name;
> +	}
> +
> +	return 0;
> +err_free_name:
> +	kfree(trig->name);
> +err_free_trig_info:
> +	kfree(trig_info);
> +err_put_trigger:
> +	list_del(&trig->alloc_list);
> +	iio_put_trigger(trig);
> +err_free_completed_reg:
> +	list_for_each_entry_safe(trig,
> +				 trig2,
> +				 &iio_hrtimer_trigger_list,
> +				 alloc_list) {
> +		trig_info = trig->private_data;
> +		kfree(trig->name);
> +		kfree(trig_info);
> +		iio_trigger_unregister(trig);
> +	}
> +
> +	return ret;
> +}
> +
> +static int iio_trig_hrtimer_remove(struct platform_device *dev)
> +{
> +	struct iio_trigger *trig, *trig2;
> +	struct iio_hrtimer_trig_info *trig_info;
> +
> +	list_for_each_entry_safe(trig,
> +				 trig2,
> +				 &iio_hrtimer_trigger_list,
> +				 alloc_list) {
> +		trig_info = trig->private_data;
> +		hrtimer_cancel(&trig_info->timer);
> +		kfree(trig->name);
> +		kfree(trig_info);
> +		iio_trigger_unregister(trig);
> +	}
> +
> +	return 0;
> +}
> +
> +static struct platform_driver iio_trig_hrtimer_driver = {
> +	.driver = {
> +		.name = "iio_hrtimer_trigger",
> +		.owner = THIS_MODULE
> +	},
> +	.probe = iio_trig_hrtimer_probe,
> +	.remove = iio_trig_hrtimer_remove,
> +};
> +
> +static int __init iio_trig_hrtimer_init(void)
> +{
> +	return platform_driver_register(&iio_trig_hrtimer_driver);
> +}
> +
> +static void __exit iio_trig_hrtimer_exit(void)
> +{
> +	return platform_driver_unregister(&iio_trig_hrtimer_driver);
> +}
> +
> +module_init(iio_trig_hrtimer_init);
> +module_exit(iio_trig_hrtimer_exit);
> +MODULE_AUTHOR("Marten Svanfeldt <marten@intuitiveaerial.com>");
> +MODULE_DESCRIPTION("Periodic hrtimer trigger for the iio subsystem");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platofrm:iio-trig-hrtimer");
> +

Re: [PATCH] IIO: trigger: New hrtimer based trigger driver

[Marten Svanfeldt]

On 2011-03-10 03:31, Jonathan Cameron wrote:
> Obviously run checkpatch over it at some point.
Will be done.

> Otherwise, nice clean little trigger driver.  After cleanup and
> testing I would be happy to see this in IIO.  Never realised it
> would be this simple :)
Looking over your comments I realized I forgot to pull over one part (or 
rather, thought I didn't need it but I do), which makes it a little bit 
more complex..

hrtimer callbacks are sent in hardirq context, which was OK in my 
driver, but as some IIO things involve SPI/I2C communications etc it 
needs to be moved over to a softirq setup. I will look into getting this 
fixed for a v2 patch.

>> +static enum hrtimer_restart iio_trig_hrtimer_trig(struct hrtimer *timer)
>> +{
>> +	struct iio_hrtimer_trig_info *trig_info = container_of(timer, struct iio_hrtimer_trig_info, timer);
>> +
>> +	iio_trigger_poll(trig_info->trigger, 0);
> Out of curiosity, how suceptible to drift is this?  I guess the timer doesn't take
> into account the amout of time this function takes, e.g how late are we by the time
> the timer is restarted?
This function is missing one line that updates the expiry time of the 
timer.. and the new expiry time is calculated based on the old time 
which means it should not drift at all. It can however "miss ticks" if 
the trigger poll takes longer than the interval.

>> +
>> +	trig_info->frequency = val;
> Only applied on trigger being stopped and restarted I think?
> Maybe return -EBUSY if the trigger is running to make this
> obvious to userspace.
Yes, I was unsure how to handle this properly.. I think I can make it 
update it properly if it is running though.

I'll have a v2 patch out in a couple of days, still untested as seems to 
be a few weeks before I get the second spin of our board on which I can 
run it.

Regards
Marten Svanfeldt