Skip to contentSkip to conversation

iio: Rework the generic trigger-gpio trigger and move out of staging

PATCH In review Jonathan Cameron 2 participants 7 messages Jun 2, 2013 – Jun 4, 2013

Subsystems (3)
lore
Share & tools
lore

Jonathan Cameron

author maintainer
#0 lore raw

v2 cover iio: Rework the generic trigger-gpio trigger and move out of staging

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
Signature · 1 line
1.8.2.3
author maintainer
#1 lore raw

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

Also dropped the unneeded gpio.h header.

---
 drivers/staging/iio/trigger/iio-trig-gpio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/iio/trigger/iio-trig-gpio.c +1 -2
index 7c593d1..69dabca 100644
@@ -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);
 	}
Signature · 1 line
1.8.2.3
author maintainer
#2 lore raw

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

In reply to 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.

---
 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(-)
drivers/staging/iio/trigger/Kconfig +4 -4
index ae9fcd3..4ecb213 100644
@@ -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 GPIOLIB
+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"
drivers/staging/iio/trigger/Makefile +1 -1
index 8a53041..48f2236 100644
@@ -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
drivers/staging/iio/trigger/iio-trig-gpio.c deleted +0 -166

166 lines not on this page · raw message

drivers/staging/iio/trigger/iio-trig-interrupt.c new file +152 -0

152 lines not on this page · raw message

Signature · 1 line
1.8.2.3
author maintainer
#3 lore raw

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

In reply to 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.

---
 drivers/staging/iio/trigger/iio-trig-interrupt.c | 121 +++++++++--------------
 1 file changed, 45 insertions(+), 76 deletions(-)
drivers/staging/iio/trigger/iio-trig-interrupt.c +45 -76
index 9afba96..84bf397 100644
@@ -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;
 }
Signature · 1 line
1.8.2.3
author maintainer
#4 lore raw

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

In reply to Jonathan Cameron

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

---
 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-interrupt.c | 121 -----------------------
 6 files changed, 131 insertions(+), 128 deletions(-)
drivers/iio/trigger/Kconfig +9 -0
index a4e68db..360fd50 100644
@@ -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
drivers/iio/trigger/Makefile +1 -0
index e0b2183..ce319a5 100644
@@ -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
drivers/iio/trigger/iio-trig-interrupt.c new file +121 -0

121 lines not on this page · raw message

drivers/staging/iio/trigger/Kconfig +0 -6
index 4ecb213..2fd18c6 100644
@@ -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
drivers/staging/iio/trigger/Makefile +0 -1
index 48f2236..238481b 100644
@@ -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
drivers/staging/iio/trigger/iio-trig-interrupt.c deleted +0 -121

121 lines not on this page · raw message

Signature · 1 line
1.8.2.3
Reviewed-by on the v2 series
#5 lore raw
In reply to Jonathan Cameron
Quoted 28 lines · 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.

Quoted 1 line

All comments welcome. Particularly wrt to patch 3.

Looks all good to me. FWIW:

Quoted 17 lines

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
author maintainer
#6 lore raw
Quoted 29 lines · Lars-Peter Clausen
Quoted 28 lines · 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.

Quoted 6 lines
Quoted 1 line

All comments welcome. Particularly wrt to patch 3.

Looks all good to me. FWIW:

Thanks, gives me a warm fuzzy feeling. I hate merging my own patches unreviewed!

Will apply these to the togreg branch of iio.git in a few mins.

Quoted 23 lines
Quoted 7 lines

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.

Quoted 10 lines
 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

--
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