[RFC PATCH 0/4] iio: Rework the generic trigger-gpio trigger and move out of staging

[RFC PATCH 0/4] iio: Rework the generic trigger-gpio trigger and move out of staging

[Jonathan Cameron]

Hi all,

This is an RFC for a couple of reasons.

1) I'm not sure how people feel about the move to multiple platform devices
(patch 3).
2) I don't actually have a simple test setup for this one.  I can put one
together but it might be a while before I get it done.

Anyhow, the purpose of this set is to respond to Lars-Peter's comments
when I originally asked if anyone minded it moving out of staging as it
was before this.  Lars raised the point that having one driver instance
register multiple triggers just made life more complicated, and instead
it might be better to have multiple driver instances, with one trigger each.
Lars also pointed out that the name of the driver was silly given there
was nolonger a requirement that the source of the interrupts be a gpio
and there hasn't been for quite some time.

All comments welcome. Particularly wrt to patch 3.

Jonathan Cameron (4):
  staging:iio:trigger:gpio bug in release of gpio in error path
  staging:iio:triggers: rename iio-trig-gpio to iio-trig-interrupt
  staging:iio:triggers:interrupt trigger - one per platform device.
  iio:triggers:interrupt trigger - move out of staging.

 drivers/iio/trigger/Kconfig                 |   9 ++
 drivers/iio/trigger/Makefile                |   1 +
 drivers/iio/trigger/iio-trig-interrupt.c    | 123 ++++++++++++++++++++
 drivers/staging/iio/trigger/Kconfig         |   6 -
 drivers/staging/iio/trigger/Makefile        |   1 -
 drivers/staging/iio/trigger/iio-trig-gpio.c | 167 ----------------------------
 6 files changed, 133 insertions(+), 174 deletions(-)
 create mode 100644 drivers/iio/trigger/iio-trig-interrupt.c
 delete mode 100644 drivers/staging/iio/trigger/iio-trig-gpio.c

-- 
1.8.2.1

[PATCH 1/4] staging:iio:trigger:gpio bug in release of gpio in error path

[Jonathan Cameron]

Also dropped the unneeded gpio.h header.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/trigger/iio-trig-gpio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/iio/trigger/iio-trig-gpio.c b/drivers/staging/iio/trigger/iio-trig-gpio.c
index 7c593d1..69dabca 100644
--- a/drivers/staging/iio/trigger/iio-trig-gpio.c
+++ b/drivers/staging/iio/trigger/iio-trig-gpio.c
@@ -19,7 +19,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
-#include <linux/gpio.h>
 #include <linux/slab.h>
 
 #include <linux/iio/iio.h>
@@ -122,7 +121,7 @@ error_free_completed_registrations:
 				 &iio_gpio_trigger_list,
 				 alloc_list) {
 		trig_info = iio_trigger_get_drvdata(trig);
-		free_irq(gpio_to_irq(trig_info->irq), trig);
+		free_irq(trig_info->irq, trig);
 		kfree(trig_info);
 		iio_trigger_unregister(trig);
 	}
-- 
1.8.2.1

[PATCH 2/4] staging:iio:triggers: rename iio-trig-gpio to iio-trig-interrupt

[Jonathan Cameron]

Also change all internal naming appropriately.
This trigger is no longer just for gpio provided interrupts so
change the naming to reflect this.  Also drop some now missleading
left over comments.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/trigger/Kconfig              |   8 +-
 drivers/staging/iio/trigger/Makefile             |   2 +-
 drivers/staging/iio/trigger/iio-trig-gpio.c      | 166 -----------------------
 drivers/staging/iio/trigger/iio-trig-interrupt.c | 152 +++++++++++++++++++++
 4 files changed, 157 insertions(+), 171 deletions(-)

diff --git a/drivers/staging/iio/trigger/Kconfig b/drivers/staging/iio/trigger/Kconfig
index d0ae99c..4ecb213 100644
--- a/drivers/staging/iio/trigger/Kconfig
+++ b/drivers/staging/iio/trigger/Kconfig
@@ -12,11 +12,11 @@ config IIO_PERIODIC_RTC_TRIGGER
 	  Provides support for using periodic capable real time
 	  clocks as IIO triggers.
 
-config IIO_GPIO_TRIGGER
-	tristate "GPIO trigger"
-	depends on GENERIC_GPIO
+config IIO_INTERRUPT_TRIGGER
+	tristate "Generic interrupt trigger"
 	help
-	  Provides support for using GPIO pins as IIO triggers.
+	  Provides support for using interrupts of various types as IIO
+	  triggers.  These may be provided by a gpio driver for example.
 
 config IIO_BFIN_TMR_TRIGGER
 	tristate "Blackfin TIMER trigger"
diff --git a/drivers/staging/iio/trigger/Makefile b/drivers/staging/iio/trigger/Makefile
index 8a53041..48f2236 100644
--- a/drivers/staging/iio/trigger/Makefile
+++ b/drivers/staging/iio/trigger/Makefile
@@ -3,5 +3,5 @@
 #
 
 obj-$(CONFIG_IIO_PERIODIC_RTC_TRIGGER) += iio-trig-periodic-rtc.o
-obj-$(CONFIG_IIO_GPIO_TRIGGER) += iio-trig-gpio.o
+obj-$(CONFIG_IIO_INTERRUPT_TRIGGER) += iio-trig-interrupt.o
 obj-$(CONFIG_IIO_BFIN_TMR_TRIGGER) += iio-trig-bfin-timer.o
diff --git a/drivers/staging/iio/trigger/iio-trig-gpio.c b/drivers/staging/iio/trigger/iio-trig-gpio.c
deleted file mode 100644
index 69dabca..0000000
--- a/drivers/staging/iio/trigger/iio-trig-gpio.c
+++ /dev/null
@@ -1,166 +0,0 @@
-/*
- * Industrial I/O - gpio based trigger support
- *
- * Copyright (c) 2008 Jonathan Cameron
- *
- * 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.
- *
- * Currently this is more of a functioning proof of concept than a full
- * fledged trigger driver.
- *
- * TODO:
- *
- * Add board config elements to allow specification of startup settings.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/interrupt.h>
-#include <linux/slab.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/trigger.h>
-
-static LIST_HEAD(iio_gpio_trigger_list);
-static DEFINE_MUTEX(iio_gpio_trigger_list_lock);
-
-struct iio_gpio_trigger_info {
-	struct mutex in_use;
-	unsigned int irq;
-};
-/*
- * Need to reference count these triggers and only enable gpio interrupts
- * as appropriate.
- */
-
-/* So what functionality do we want in here?... */
-/* set high / low as interrupt type? */
-
-static irqreturn_t iio_gpio_trigger_poll(int irq, void *private)
-{
-	/* Timestamp not currently provided */
-	iio_trigger_poll(private, 0);
-	return IRQ_HANDLED;
-}
-
-static const struct iio_trigger_ops iio_gpio_trigger_ops = {
-	.owner = THIS_MODULE,
-};
-
-static int iio_gpio_trigger_probe(struct platform_device *pdev)
-{
-	struct iio_gpio_trigger_info *trig_info;
-	struct iio_trigger *trig, *trig2;
-	unsigned long irqflags;
-	struct resource *irq_res;
-	int irq, ret = 0, irq_res_cnt = 0;
-
-	do {
-		irq_res = platform_get_resource(pdev,
-				IORESOURCE_IRQ, irq_res_cnt);
-
-		if (irq_res == NULL) {
-			if (irq_res_cnt == 0)
-				dev_err(&pdev->dev, "No GPIO IRQs specified");
-			break;
-		}
-		irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
-
-		for (irq = irq_res->start; irq <= irq_res->end; irq++) {
-
-			trig = iio_trigger_alloc("irqtrig%d", irq);
-			if (!trig) {
-				ret = -ENOMEM;
-				goto error_free_completed_registrations;
-			}
-
-			trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
-			if (!trig_info) {
-				ret = -ENOMEM;
-				goto error_put_trigger;
-			}
-			iio_trigger_set_drvdata(trig, trig_info);
-			trig_info->irq = irq;
-			trig->ops = &iio_gpio_trigger_ops;
-			ret = request_irq(irq, iio_gpio_trigger_poll,
-					  irqflags, trig->name, trig);
-			if (ret) {
-				dev_err(&pdev->dev,
-					"request IRQ-%d failed", irq);
-				goto error_free_trig_info;
-			}
-
-			ret = iio_trigger_register(trig);
-			if (ret)
-				goto error_release_irq;
-
-			list_add_tail(&trig->alloc_list,
-					&iio_gpio_trigger_list);
-		}
-
-		irq_res_cnt++;
-	} while (irq_res != NULL);
-
-
-	return 0;
-
-/* First clean up the partly allocated trigger */
-error_release_irq:
-	free_irq(irq, trig);
-error_free_trig_info:
-	kfree(trig_info);
-error_put_trigger:
-	iio_trigger_put(trig);
-error_free_completed_registrations:
-	/* The rest should have been added to the iio_gpio_trigger_list */
-	list_for_each_entry_safe(trig,
-				 trig2,
-				 &iio_gpio_trigger_list,
-				 alloc_list) {
-		trig_info = iio_trigger_get_drvdata(trig);
-		free_irq(trig_info->irq, trig);
-		kfree(trig_info);
-		iio_trigger_unregister(trig);
-	}
-
-	return ret;
-}
-
-static int iio_gpio_trigger_remove(struct platform_device *pdev)
-{
-	struct iio_trigger *trig, *trig2;
-	struct iio_gpio_trigger_info *trig_info;
-
-	mutex_lock(&iio_gpio_trigger_list_lock);
-	list_for_each_entry_safe(trig,
-				 trig2,
-				 &iio_gpio_trigger_list,
-				 alloc_list) {
-		trig_info = iio_trigger_get_drvdata(trig);
-		iio_trigger_unregister(trig);
-		free_irq(trig_info->irq, trig);
-		kfree(trig_info);
-		iio_trigger_put(trig);
-	}
-	mutex_unlock(&iio_gpio_trigger_list_lock);
-
-	return 0;
-}
-
-static struct platform_driver iio_gpio_trigger_driver = {
-	.probe = iio_gpio_trigger_probe,
-	.remove = iio_gpio_trigger_remove,
-	.driver = {
-		.name = "iio_gpio_trigger",
-		.owner = THIS_MODULE,
-	},
-};
-
-module_platform_driver(iio_gpio_trigger_driver);
-
-MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
-MODULE_DESCRIPTION("Example gpio trigger for the iio subsystem");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/trigger/iio-trig-interrupt.c b/drivers/staging/iio/trigger/iio-trig-interrupt.c
new file mode 100644
index 0000000..9afba96
--- /dev/null
+++ b/drivers/staging/iio/trigger/iio-trig-interrupt.c
@@ -0,0 +1,152 @@
+/*
+ * Industrial I/O - generic interrupt based trigger support
+ *
+ * Copyright (c) 2008-2013 Jonathan Cameron
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+
+static LIST_HEAD(iio_interrupt_trigger_list);
+static DEFINE_MUTEX(iio_interrupt_trigger_list_lock);
+
+struct iio_interrupt_trigger_info {
+	struct mutex in_use;
+	unsigned int irq;
+};
+
+static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
+{
+	/* Timestamp not currently provided */
+	iio_trigger_poll(private, 0);
+	return IRQ_HANDLED;
+}
+
+static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static int iio_interrupt_trigger_probe(struct platform_device *pdev)
+{
+	struct iio_interrupt_trigger_info *trig_info;
+	struct iio_trigger *trig, *trig2;
+	unsigned long irqflags;
+	struct resource *irq_res;
+	int irq, ret = 0, irq_res_cnt = 0;
+
+	do {
+		irq_res = platform_get_resource(pdev,
+				IORESOURCE_IRQ, irq_res_cnt);
+
+		if (irq_res == NULL) {
+			if (irq_res_cnt == 0)
+				dev_err(&pdev->dev, "No IRQs specified");
+			break;
+		}
+		irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
+
+		for (irq = irq_res->start; irq <= irq_res->end; irq++) {
+
+			trig = iio_trigger_alloc("irqtrig%d", irq);
+			if (!trig) {
+				ret = -ENOMEM;
+				goto error_free_completed_registrations;
+			}
+
+			trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
+			if (!trig_info) {
+				ret = -ENOMEM;
+				goto error_put_trigger;
+			}
+			iio_trigger_set_drvdata(trig, trig_info);
+			trig_info->irq = irq;
+			trig->ops = &iio_interrupt_trigger_ops;
+			ret = request_irq(irq, iio_interrupt_trigger_poll,
+					  irqflags, trig->name, trig);
+			if (ret) {
+				dev_err(&pdev->dev,
+					"request IRQ-%d failed", irq);
+				goto error_free_trig_info;
+			}
+
+			ret = iio_trigger_register(trig);
+			if (ret)
+				goto error_release_irq;
+
+			list_add_tail(&trig->alloc_list,
+					&iio_interrupt_trigger_list);
+		}
+
+		irq_res_cnt++;
+	} while (irq_res != NULL);
+
+
+	return 0;
+
+/* First clean up the partly allocated trigger */
+error_release_irq:
+	free_irq(irq, trig);
+error_free_trig_info:
+	kfree(trig_info);
+error_put_trigger:
+	iio_trigger_put(trig);
+error_free_completed_registrations:
+	/* The rest should have been added to the iio_interrupt_trigger_list */
+	list_for_each_entry_safe(trig,
+				 trig2,
+				 &iio_interrupt_trigger_list,
+				 alloc_list) {
+		trig_info = iio_trigger_get_drvdata(trig);
+		free_irq(trig_info->irq, trig);
+		kfree(trig_info);
+		iio_trigger_unregister(trig);
+	}
+
+	return ret;
+}
+
+static int iio_interrupt_trigger_remove(struct platform_device *pdev)
+{
+	struct iio_trigger *trig, *trig2;
+	struct iio_interrupt_trigger_info *trig_info;
+
+	mutex_lock(&iio_interrupt_trigger_list_lock);
+	list_for_each_entry_safe(trig,
+				 trig2,
+				 &iio_interrupt_trigger_list,
+				 alloc_list) {
+		trig_info = iio_trigger_get_drvdata(trig);
+		iio_trigger_unregister(trig);
+		free_irq(trig_info->irq, trig);
+		kfree(trig_info);
+		iio_trigger_put(trig);
+	}
+	mutex_unlock(&iio_interrupt_trigger_list_lock);
+
+	return 0;
+}
+
+static struct platform_driver iio_interrupt_trigger_driver = {
+	.probe = iio_interrupt_trigger_probe,
+	.remove = iio_interrupt_trigger_remove,
+	.driver = {
+		.name = "iio_interrupt_trigger",
+		.owner = THIS_MODULE,
+	},
+};
+
+module_platform_driver(iio_interrupt_trigger_driver);
+
+MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
+MODULE_DESCRIPTION("Interrupt trigger for the iio subsystem");
+MODULE_LICENSE("GPL v2");
-- 
1.8.2.1

[PATCH 3/4] staging:iio:triggers:interrupt trigger - one per platform device.

[Jonathan Cameron]

Switching from one platform device registering a lot of triggers
to one for each trigger simplifies the code somewhat. It would be
relatively unusual to have more than a couple of such devices
registered so this change will not result in much additional overhead.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/trigger/iio-trig-interrupt.c | 119 +++++++++--------------
 1 file changed, 45 insertions(+), 74 deletions(-)

diff --git a/drivers/staging/iio/trigger/iio-trig-interrupt.c b/drivers/staging/iio/trigger/iio-trig-interrupt.c
index 9afba96..e015de2 100644
--- a/drivers/staging/iio/trigger/iio-trig-interrupt.c
+++ b/drivers/staging/iio/trigger/iio-trig-interrupt.c
@@ -17,11 +17,8 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/trigger.h>
 
-static LIST_HEAD(iio_interrupt_trigger_list);
-static DEFINE_MUTEX(iio_interrupt_trigger_list_lock);
 
 struct iio_interrupt_trigger_info {
-	struct mutex in_use;
 	unsigned int irq;
 };
 
@@ -39,57 +36,48 @@ static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
 static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 {
 	struct iio_interrupt_trigger_info *trig_info;
-	struct iio_trigger *trig, *trig2;
+	struct iio_trigger *trig;
 	unsigned long irqflags;
 	struct resource *irq_res;
 	int irq, ret = 0, irq_res_cnt = 0;
 
-	do {
-		irq_res = platform_get_resource(pdev,
-				IORESOURCE_IRQ, irq_res_cnt);
-
-		if (irq_res == NULL) {
-			if (irq_res_cnt == 0)
-				dev_err(&pdev->dev, "No IRQs specified");
-			break;
-		}
-		irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
-
-		for (irq = irq_res->start; irq <= irq_res->end; irq++) {
-
-			trig = iio_trigger_alloc("irqtrig%d", irq);
-			if (!trig) {
-				ret = -ENOMEM;
-				goto error_free_completed_registrations;
-			}
-
-			trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
-			if (!trig_info) {
-				ret = -ENOMEM;
-				goto error_put_trigger;
-			}
-			iio_trigger_set_drvdata(trig, trig_info);
-			trig_info->irq = irq;
-			trig->ops = &iio_interrupt_trigger_ops;
-			ret = request_irq(irq, iio_interrupt_trigger_poll,
-					  irqflags, trig->name, trig);
-			if (ret) {
-				dev_err(&pdev->dev,
-					"request IRQ-%d failed", irq);
-				goto error_free_trig_info;
-			}
-
-			ret = iio_trigger_register(trig);
-			if (ret)
-				goto error_release_irq;
-
-			list_add_tail(&trig->alloc_list,
-					&iio_interrupt_trigger_list);
-		}
-
-		irq_res_cnt++;
-	} while (irq_res != NULL);
+	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, irq_res_cnt);
 
+	if (irq_res == NULL) {
+		if (irq_res_cnt == 0)
+			dev_err(&pdev->dev, "No IRQs specified");
+		return -ENODEV;
+	}
+	irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
+
+	irq = irq_res->start;
+
+	trig = iio_trigger_alloc("irqtrig%d", irq_res->start);
+	if (!trig) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+
+	trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
+	if (!trig_info) {
+		ret = -ENOMEM;
+		goto error_put_trigger;
+	}
+	iio_trigger_set_drvdata(trig, trig_info);
+	trig_info->irq = irq;
+	trig->ops = &iio_interrupt_trigger_ops;
+	ret = request_irq(irq, iio_interrupt_trigger_poll,
+			  irqflags, trig->name, trig);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"request IRQ-%d failed", irq);
+		goto error_free_trig_info;
+	}
+
+	ret = iio_trigger_register(trig);
+	if (ret)
+		goto error_release_irq;
+	platform_set_drvdata(pdev, trig);
 
 	return 0;
 
@@ -100,38 +88,21 @@ error_free_trig_info:
 	kfree(trig_info);
 error_put_trigger:
 	iio_trigger_put(trig);
-error_free_completed_registrations:
-	/* The rest should have been added to the iio_interrupt_trigger_list */
-	list_for_each_entry_safe(trig,
-				 trig2,
-				 &iio_interrupt_trigger_list,
-				 alloc_list) {
-		trig_info = iio_trigger_get_drvdata(trig);
-		free_irq(trig_info->irq, trig);
-		kfree(trig_info);
-		iio_trigger_unregister(trig);
-	}
-
+error_ret:
 	return ret;
 }
 
 static int iio_interrupt_trigger_remove(struct platform_device *pdev)
 {
-	struct iio_trigger *trig, *trig2;
+	struct iio_trigger *trig;
 	struct iio_interrupt_trigger_info *trig_info;
 
-	mutex_lock(&iio_interrupt_trigger_list_lock);
-	list_for_each_entry_safe(trig,
-				 trig2,
-				 &iio_interrupt_trigger_list,
-				 alloc_list) {
-		trig_info = iio_trigger_get_drvdata(trig);
-		iio_trigger_unregister(trig);
-		free_irq(trig_info->irq, trig);
-		kfree(trig_info);
-		iio_trigger_put(trig);
-	}
-	mutex_unlock(&iio_interrupt_trigger_list_lock);
+	trig = platform_get_drvdata(pdev);
+	trig_info = iio_trigger_get_drvdata(trig);
+	iio_trigger_unregister(trig);
+	free_irq(trig_info->irq, trig);
+	kfree(trig_info);
+	iio_trigger_put(trig);
 
 	return 0;
 }
-- 
1.8.2.1

Re: [PATCH 3/4] staging:iio:triggers:interrupt trigger - one per platform device.

[Lars-Peter Clausen]

[...]
>  
> @@ -39,57 +36,48 @@ static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
>  static int iio_interrupt_trigger_probe(struct platform_device *pdev)
>  {
>  	struct iio_interrupt_trigger_info *trig_info;
> -	struct iio_trigger *trig, *trig2;
> +	struct iio_trigger *trig;
>  	unsigned long irqflags;
>  	struct resource *irq_res;
>  	int irq, ret = 0, irq_res_cnt = 0;
>  [...]
> +	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, irq_res_cnt);

irq_res_cnt is always zero.

> [...]

Re: [PATCH 3/4] staging:iio:triggers:interrupt trigger - one per platform device.

[Jonathan Cameron]

Lars-Peter Clausen <lars@metafoo.de> wrote:

>[...]
>>  
>> @@ -39,57 +36,48 @@ static const struct iio_trigger_ops
>iio_interrupt_trigger_ops = {
>>  static int iio_interrupt_trigger_probe(struct platform_device *pdev)
>>  {
>>  	struct iio_interrupt_trigger_info *trig_info;
>> -	struct iio_trigger *trig, *trig2;
>> +	struct iio_trigger *trig;
>>  	unsigned long irqflags;
>>  	struct resource *irq_res;
>>  	int irq, ret = 0, irq_res_cnt = 0;
>>  [...]
>> +	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, irq_res_cnt);
>
>irq_res_cnt is always zero.

Oops. Thanks
>
>> [...]
>--
>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

-- 
Sent from my Android phone with K-9 Mail. Please excuse my brevity.

[PATCH 4/4] iio:triggers:interrupt trigger - move out of staging.

[Jonathan Cameron]

This is now a very simple trigger indeed but useful in many common cases.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/iio/trigger/Kconfig                      |   9 ++
 drivers/iio/trigger/Makefile                     |   1 +
 drivers/iio/trigger/iio-trig-interrupt.c         | 123 +++++++++++++++++++++++
 drivers/staging/iio/trigger/Kconfig              |   6 --
 drivers/staging/iio/trigger/Makefile             |   1 -
 drivers/staging/iio/trigger/iio-trig-interrupt.c | 123 -----------------------
 6 files changed, 133 insertions(+), 130 deletions(-)

diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
index a4e68db..360fd50 100644
--- a/drivers/iio/trigger/Kconfig
+++ b/drivers/iio/trigger/Kconfig
@@ -3,6 +3,15 @@
 #
 menu "Triggers - standalone"
 
+config IIO_INTERRUPT_TRIGGER
+	tristate "Generic interrupt trigger"
+	help
+	  Provides support for using an interrupt of any type as an IIO
+	  trigger.  This may be provided by a gpio driver for example.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called iio-trig-interrupt.
+
 config IIO_SYSFS_TRIGGER
 	tristate "SYSFS trigger"
 	depends on SYSFS
diff --git a/drivers/iio/trigger/Makefile b/drivers/iio/trigger/Makefile
index e0b2183..ce319a5 100644
--- a/drivers/iio/trigger/Makefile
+++ b/drivers/iio/trigger/Makefile
@@ -2,4 +2,5 @@
 # Makefile for triggers not associated with iio-devices
 #
 
+obj-$(CONFIG_IIO_INTERRUPT_TRIGGER) += iio-trig-interrupt.o
 obj-$(CONFIG_IIO_SYSFS_TRIGGER) += iio-trig-sysfs.o
diff --git a/drivers/iio/trigger/iio-trig-interrupt.c b/drivers/iio/trigger/iio-trig-interrupt.c
new file mode 100644
index 0000000..e015de2
--- /dev/null
+++ b/drivers/iio/trigger/iio-trig-interrupt.c
@@ -0,0 +1,123 @@
+/*
+ * Industrial I/O - generic interrupt based trigger support
+ *
+ * Copyright (c) 2008-2013 Jonathan Cameron
+ *
+ * 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/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+
+
+struct iio_interrupt_trigger_info {
+	unsigned int irq;
+};
+
+static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
+{
+	/* Timestamp not currently provided */
+	iio_trigger_poll(private, 0);
+	return IRQ_HANDLED;
+}
+
+static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
+	.owner = THIS_MODULE,
+};
+
+static int iio_interrupt_trigger_probe(struct platform_device *pdev)
+{
+	struct iio_interrupt_trigger_info *trig_info;
+	struct iio_trigger *trig;
+	unsigned long irqflags;
+	struct resource *irq_res;
+	int irq, ret = 0, irq_res_cnt = 0;
+
+	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, irq_res_cnt);
+
+	if (irq_res == NULL) {
+		if (irq_res_cnt == 0)
+			dev_err(&pdev->dev, "No IRQs specified");
+		return -ENODEV;
+	}
+	irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
+
+	irq = irq_res->start;
+
+	trig = iio_trigger_alloc("irqtrig%d", irq_res->start);
+	if (!trig) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+
+	trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
+	if (!trig_info) {
+		ret = -ENOMEM;
+		goto error_put_trigger;
+	}
+	iio_trigger_set_drvdata(trig, trig_info);
+	trig_info->irq = irq;
+	trig->ops = &iio_interrupt_trigger_ops;
+	ret = request_irq(irq, iio_interrupt_trigger_poll,
+			  irqflags, trig->name, trig);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"request IRQ-%d failed", irq);
+		goto error_free_trig_info;
+	}
+
+	ret = iio_trigger_register(trig);
+	if (ret)
+		goto error_release_irq;
+	platform_set_drvdata(pdev, trig);
+
+	return 0;
+
+/* First clean up the partly allocated trigger */
+error_release_irq:
+	free_irq(irq, trig);
+error_free_trig_info:
+	kfree(trig_info);
+error_put_trigger:
+	iio_trigger_put(trig);
+error_ret:
+	return ret;
+}
+
+static int iio_interrupt_trigger_remove(struct platform_device *pdev)
+{
+	struct iio_trigger *trig;
+	struct iio_interrupt_trigger_info *trig_info;
+
+	trig = platform_get_drvdata(pdev);
+	trig_info = iio_trigger_get_drvdata(trig);
+	iio_trigger_unregister(trig);
+	free_irq(trig_info->irq, trig);
+	kfree(trig_info);
+	iio_trigger_put(trig);
+
+	return 0;
+}
+
+static struct platform_driver iio_interrupt_trigger_driver = {
+	.probe = iio_interrupt_trigger_probe,
+	.remove = iio_interrupt_trigger_remove,
+	.driver = {
+		.name = "iio_interrupt_trigger",
+		.owner = THIS_MODULE,
+	},
+};
+
+module_platform_driver(iio_interrupt_trigger_driver);
+
+MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
+MODULE_DESCRIPTION("Interrupt trigger for the iio subsystem");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/staging/iio/trigger/Kconfig b/drivers/staging/iio/trigger/Kconfig
index 4ecb213..2fd18c6 100644
--- a/drivers/staging/iio/trigger/Kconfig
+++ b/drivers/staging/iio/trigger/Kconfig
@@ -12,12 +12,6 @@ config IIO_PERIODIC_RTC_TRIGGER
 	  Provides support for using periodic capable real time
 	  clocks as IIO triggers.
 
-config IIO_INTERRUPT_TRIGGER
-	tristate "Generic interrupt trigger"
-	help
-	  Provides support for using interrupts of various types as IIO
-	  triggers.  These may be provided by a gpio driver for example.
-
 config IIO_BFIN_TMR_TRIGGER
 	tristate "Blackfin TIMER trigger"
 	depends on BLACKFIN
diff --git a/drivers/staging/iio/trigger/Makefile b/drivers/staging/iio/trigger/Makefile
index 48f2236..238481b 100644
--- a/drivers/staging/iio/trigger/Makefile
+++ b/drivers/staging/iio/trigger/Makefile
@@ -3,5 +3,4 @@
 #
 
 obj-$(CONFIG_IIO_PERIODIC_RTC_TRIGGER) += iio-trig-periodic-rtc.o
-obj-$(CONFIG_IIO_INTERRUPT_TRIGGER) += iio-trig-interrupt.o
 obj-$(CONFIG_IIO_BFIN_TMR_TRIGGER) += iio-trig-bfin-timer.o
diff --git a/drivers/staging/iio/trigger/iio-trig-interrupt.c b/drivers/staging/iio/trigger/iio-trig-interrupt.c
deleted file mode 100644
index e015de2..0000000
--- a/drivers/staging/iio/trigger/iio-trig-interrupt.c
+++ /dev/null
@@ -1,123 +0,0 @@
-/*
- * Industrial I/O - generic interrupt based trigger support
- *
- * Copyright (c) 2008-2013 Jonathan Cameron
- *
- * 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/kernel.h>
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/interrupt.h>
-#include <linux/slab.h>
-
-#include <linux/iio/iio.h>
-#include <linux/iio/trigger.h>
-
-
-struct iio_interrupt_trigger_info {
-	unsigned int irq;
-};
-
-static irqreturn_t iio_interrupt_trigger_poll(int irq, void *private)
-{
-	/* Timestamp not currently provided */
-	iio_trigger_poll(private, 0);
-	return IRQ_HANDLED;
-}
-
-static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
-	.owner = THIS_MODULE,
-};
-
-static int iio_interrupt_trigger_probe(struct platform_device *pdev)
-{
-	struct iio_interrupt_trigger_info *trig_info;
-	struct iio_trigger *trig;
-	unsigned long irqflags;
-	struct resource *irq_res;
-	int irq, ret = 0, irq_res_cnt = 0;
-
-	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, irq_res_cnt);
-
-	if (irq_res == NULL) {
-		if (irq_res_cnt == 0)
-			dev_err(&pdev->dev, "No IRQs specified");
-		return -ENODEV;
-	}
-	irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
-
-	irq = irq_res->start;
-
-	trig = iio_trigger_alloc("irqtrig%d", irq_res->start);
-	if (!trig) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
-
-	trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
-	if (!trig_info) {
-		ret = -ENOMEM;
-		goto error_put_trigger;
-	}
-	iio_trigger_set_drvdata(trig, trig_info);
-	trig_info->irq = irq;
-	trig->ops = &iio_interrupt_trigger_ops;
-	ret = request_irq(irq, iio_interrupt_trigger_poll,
-			  irqflags, trig->name, trig);
-	if (ret) {
-		dev_err(&pdev->dev,
-			"request IRQ-%d failed", irq);
-		goto error_free_trig_info;
-	}
-
-	ret = iio_trigger_register(trig);
-	if (ret)
-		goto error_release_irq;
-	platform_set_drvdata(pdev, trig);
-
-	return 0;
-
-/* First clean up the partly allocated trigger */
-error_release_irq:
-	free_irq(irq, trig);
-error_free_trig_info:
-	kfree(trig_info);
-error_put_trigger:
-	iio_trigger_put(trig);
-error_ret:
-	return ret;
-}
-
-static int iio_interrupt_trigger_remove(struct platform_device *pdev)
-{
-	struct iio_trigger *trig;
-	struct iio_interrupt_trigger_info *trig_info;
-
-	trig = platform_get_drvdata(pdev);
-	trig_info = iio_trigger_get_drvdata(trig);
-	iio_trigger_unregister(trig);
-	free_irq(trig_info->irq, trig);
-	kfree(trig_info);
-	iio_trigger_put(trig);
-
-	return 0;
-}
-
-static struct platform_driver iio_interrupt_trigger_driver = {
-	.probe = iio_interrupt_trigger_probe,
-	.remove = iio_interrupt_trigger_remove,
-	.driver = {
-		.name = "iio_interrupt_trigger",
-		.owner = THIS_MODULE,
-	},
-};
-
-module_platform_driver(iio_interrupt_trigger_driver);
-
-MODULE_AUTHOR("Jonathan Cameron <jic23@kernel.org>");
-MODULE_DESCRIPTION("Interrupt trigger for the iio subsystem");
-MODULE_LICENSE("GPL v2");
-- 
1.8.2.1

[PATCH 0/4 V2] iio: Rework the generic trigger-gpio trigger and move out of staging

[Jonathan Cameron]

Only changes here are dropping the irq_res_cnt as it unused (thanks Lars) and
a rebase as a result of some Kconfig dependency changes.

I still don't have a hardware setup, but this is so simple that unless
someone shouts I'll probably send it upstream in fairly soon anyway.

Jonathan

Previous message:

Hi all,

This is an RFC for a couple of reasons.

1) I'm not sure how people feel about the move to multiple platform devices
(patch 3).
2) I don't actually have a simple test setup for this one.  I can put one
together but it might be a while before I get it done.

Anyhow, the purpose of this set is to respond to Lars-Peter's comments
when I originally asked if anyone minded it moving out of staging as it
was before this.  Lars raised the point that having one driver instance
register multiple triggers just made life more complicated, and instead
it might be better to have multiple driver instances, with one trigger each.
Lars also pointed out that the name of the driver was silly given there
was nolonger a requirement that the source of the interrupts be a gpio
and there hasn't been for quite some time.

All comments welcome. Particularly wrt to patch 3.

Jonathan Cameron (4):
  staging:iio:trigger:gpio bug in release of gpio in error path
  staging:iio:triggers: rename iio-trig-gpio to iio-trig-interrupt
  staging:iio:triggers:interrupt trigger - one per platform device.
  iio:triggers:interrupt trigger - move out of staging.

 drivers/iio/trigger/Kconfig                 |   9 ++
 drivers/iio/trigger/Makefile                |   1 +
 drivers/iio/trigger/iio-trig-interrupt.c    | 121 ++++++++++++++++++++
 drivers/staging/iio/trigger/Kconfig         |   6 -
 drivers/staging/iio/trigger/Makefile        |   1 -
 drivers/staging/iio/trigger/iio-trig-gpio.c | 167 ----------------------------
 6 files changed, 131 insertions(+), 174 deletions(-)
 create mode 100644 drivers/iio/trigger/iio-trig-interrupt.c
 delete mode 100644 drivers/staging/iio/trigger/iio-trig-gpio.c

-- 
1.8.2.3

[PATCH 3/4] staging:iio:triggers:interrupt trigger - one per platform device.

[Jonathan Cameron]

Switching from one platform device registering a lot of triggers
to one for each trigger simplifies the code somewhat. It would be
relatively unusual to have more than a couple of such devices
registered so this change will not result in much additional overhead.

Signed-off-by: Jonathan Cameron <jic23@kernel.org>
---
 drivers/staging/iio/trigger/iio-trig-interrupt.c | 121 +++++++++--------------
 1 file changed, 45 insertions(+), 76 deletions(-)

diff --git a/drivers/staging/iio/trigger/iio-trig-interrupt.c b/drivers/staging/iio/trigger/iio-trig-interrupt.c
index 9afba96..84bf397 100644
--- a/drivers/staging/iio/trigger/iio-trig-interrupt.c
+++ b/drivers/staging/iio/trigger/iio-trig-interrupt.c
@@ -17,11 +17,8 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/trigger.h>
 
-static LIST_HEAD(iio_interrupt_trigger_list);
-static DEFINE_MUTEX(iio_interrupt_trigger_list_lock);
 
 struct iio_interrupt_trigger_info {
-	struct mutex in_use;
 	unsigned int irq;
 };
 
@@ -39,57 +36,46 @@ static const struct iio_trigger_ops iio_interrupt_trigger_ops = {
 static int iio_interrupt_trigger_probe(struct platform_device *pdev)
 {
 	struct iio_interrupt_trigger_info *trig_info;
-	struct iio_trigger *trig, *trig2;
+	struct iio_trigger *trig;
 	unsigned long irqflags;
 	struct resource *irq_res;
-	int irq, ret = 0, irq_res_cnt = 0;
-
-	do {
-		irq_res = platform_get_resource(pdev,
-				IORESOURCE_IRQ, irq_res_cnt);
-
-		if (irq_res == NULL) {
-			if (irq_res_cnt == 0)
-				dev_err(&pdev->dev, "No IRQs specified");
-			break;
-		}
-		irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
-
-		for (irq = irq_res->start; irq <= irq_res->end; irq++) {
-
-			trig = iio_trigger_alloc("irqtrig%d", irq);
-			if (!trig) {
-				ret = -ENOMEM;
-				goto error_free_completed_registrations;
-			}
-
-			trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
-			if (!trig_info) {
-				ret = -ENOMEM;
-				goto error_put_trigger;
-			}
-			iio_trigger_set_drvdata(trig, trig_info);
-			trig_info->irq = irq;
-			trig->ops = &iio_interrupt_trigger_ops;
-			ret = request_irq(irq, iio_interrupt_trigger_poll,
-					  irqflags, trig->name, trig);
-			if (ret) {
-				dev_err(&pdev->dev,
-					"request IRQ-%d failed", irq);
-				goto error_free_trig_info;
-			}
-
-			ret = iio_trigger_register(trig);
-			if (ret)
-				goto error_release_irq;
-
-			list_add_tail(&trig->alloc_list,
-					&iio_interrupt_trigger_list);
-		}
-
-		irq_res_cnt++;
-	} while (irq_res != NULL);
+	int irq, ret = 0;
 
+	irq_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+
+	if (irq_res == NULL)
+		return -ENODEV;
+
+	irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
+
+	irq = irq_res->start;
+
+	trig = iio_trigger_alloc("irqtrig%d", irq_res->start);
+	if (!trig) {
+		ret = -ENOMEM;
+		goto error_ret;
+	}
+
+	trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
+	if (!trig_info) {
+		ret = -ENOMEM;
+		goto error_put_trigger;
+	}
+	iio_trigger_set_drvdata(trig, trig_info);
+	trig_info->irq = irq;
+	trig->ops = &iio_interrupt_trigger_ops;
+	ret = request_irq(irq, iio_interrupt_trigger_poll,
+			  irqflags, trig->name, trig);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"request IRQ-%d failed", irq);
+		goto error_free_trig_info;
+	}
+
+	ret = iio_trigger_register(trig);
+	if (ret)
+		goto error_release_irq;
+	platform_set_drvdata(pdev, trig);
 
 	return 0;
 
@@ -100,38 +86,21 @@ error_free_trig_info:
 	kfree(trig_info);
 error_put_trigger:
 	iio_trigger_put(trig);
-error_free_completed_registrations:
-	/* The rest should have been added to the iio_interrupt_trigger_list */
-	list_for_each_entry_safe(trig,
-				 trig2,
-				 &iio_interrupt_trigger_list,
-				 alloc_list) {
-		trig_info = iio_trigger_get_drvdata(trig);
-		free_irq(trig_info->irq, trig);
-		kfree(trig_info);
-		iio_trigger_unregister(trig);
-	}
-
+error_ret:
 	return ret;
 }
 
 static int iio_interrupt_trigger_remove(struct platform_device *pdev)
 {
-	struct iio_trigger *trig, *trig2;
+	struct iio_trigger *trig;
 	struct iio_interrupt_trigger_info *trig_info;
 
-	mutex_lock(&iio_interrupt_trigger_list_lock);
-	list_for_each_entry_safe(trig,
-				 trig2,
-				 &iio_interrupt_trigger_list,
-				 alloc_list) {
-		trig_info = iio_trigger_get_drvdata(trig);
-		iio_trigger_unregister(trig);
-		free_irq(trig_info->irq, trig);
-		kfree(trig_info);
-		iio_trigger_put(trig);
-	}
-	mutex_unlock(&iio_interrupt_trigger_list_lock);
+	trig = platform_get_drvdata(pdev);
+	trig_info = iio_trigger_get_drvdata(trig);
+	iio_trigger_unregister(trig);
+	free_irq(trig_info->irq, trig);
+	kfree(trig_info);
+	iio_trigger_put(trig);
 
 	return 0;
 }
-- 
1.8.2.3