* [PATCH] iio-trig-gpio:Remove redundant gpio_request
@ 2010-04-26 8:36 michael.hennerich
0 siblings, 0 replies; 11+ messages in thread
From: michael.hennerich @ 2010-04-26 8:36 UTC (permalink / raw)
To: gregkh
Cc: jic23, uclinux-dist-devel, linux-kernel, linux-iio,
Michael Hennerich
Remove redundant gpio_request:
The GPIO used as trigger IRQ, is also requested as gpio, but actually never read.
Use platform resource facility to get IRQs numbers and flags.
Make sure this driver can be used with any system IRQ, not necessarily limited to GPIO-IRQs.
Use dev_err(dev...) and friends instead of printk(KERN_ERR...)
Signed-off-by: Michael Hennerich <Michael.Hennerich@analog.com>
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
drivers/staging/iio/trigger/iio-trig-gpio.c | 127 +++++++++++++--------------
1 files changed, 61 insertions(+), 66 deletions(-)
diff --git a/drivers/staging/iio/trigger/iio-trig-gpio.c b/drivers/staging/iio/trigger/iio-trig-gpio.c
index 539e416..6e64a76 100644
--- a/drivers/staging/iio/trigger/iio-trig-gpio.c
+++ b/drivers/staging/iio/trigger/iio-trig-gpio.c
@@ -13,7 +13,6 @@
* TODO:
*
* Add board config elements to allow specification of startup settings.
- * Add configuration settings (irq type etc)
*/
#include <linux/kernel.h>
@@ -30,7 +29,7 @@ DEFINE_MUTEX(iio_gpio_trigger_list_lock);
struct iio_gpio_trigger_info {
struct mutex in_use;
- int gpio;
+ unsigned int irq;
};
/*
* Need to reference count these triggers and only enable gpio interrupts
@@ -57,78 +56,77 @@ static const struct attribute_group iio_gpio_trigger_attr_group = {
.attrs = iio_gpio_trigger_attrs,
};
-static int iio_gpio_trigger_probe(struct platform_device *dev)
+static int iio_gpio_trigger_probe(struct platform_device *pdev)
{
- int *pdata = dev->dev.platform_data;
struct iio_gpio_trigger_info *trig_info;
struct iio_trigger *trig, *trig2;
- int i, irq, ret = 0;
- if (!pdata) {
- printk(KERN_ERR "No IIO gpio trigger platform data found\n");
- goto error_ret;
- }
- for (i = 0;; i++) {
- if (!gpio_is_valid(pdata[i]))
- break;
- trig = iio_allocate_trigger();
- if (!trig) {
- ret = -ENOMEM;
- goto error_free_completed_registrations;
- }
+ unsigned long irqflags;
+ struct resource *irq_res;
+ int irq, ret = 0, irq_res_cnt = 0;
- trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
- if (!trig_info) {
- ret = -ENOMEM;
- goto error_put_trigger;
- }
- trig->control_attrs = &iio_gpio_trigger_attr_group;
- trig->private_data = trig_info;
- trig_info->gpio = pdata[i];
- trig->owner = THIS_MODULE;
- trig->name = kmalloc(IIO_TRIGGER_NAME_LENGTH, GFP_KERNEL);
- if (!trig->name) {
- ret = -ENOMEM;
- goto error_free_trig_info;
+ 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;
}
- snprintf((char *)trig->name,
- IIO_TRIGGER_NAME_LENGTH,
- "gpiotrig%d",
- pdata[i]);
- ret = gpio_request(trig_info->gpio, trig->name);
- if (ret)
- goto error_free_name;
-
- ret = gpio_direction_input(trig_info->gpio);
- if (ret)
- goto error_release_gpio;
-
- irq = gpio_to_irq(trig_info->gpio);
- if (irq < 0) {
- ret = irq;
- goto error_release_gpio;
+ irqflags = (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SHARED;
+
+ for (irq = irq_res->start; irq <= irq_res->end; irq++) {
+
+ trig = iio_allocate_trigger();
+ 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;
+ }
+ trig->control_attrs = &iio_gpio_trigger_attr_group;
+ trig->private_data = trig_info;
+ trig_info->irq = irq;
+ trig->owner = THIS_MODULE;
+ trig->name = kmalloc(IIO_TRIGGER_NAME_LENGTH,
+ GFP_KERNEL);
+ if (!trig->name) {
+ ret = -ENOMEM;
+ goto error_free_trig_info;
+ }
+ snprintf((char *)trig->name,
+ IIO_TRIGGER_NAME_LENGTH,
+ "irqtrig%d", irq);
+
+ 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_name;
+ }
+
+ ret = iio_trigger_register(trig);
+ if (ret)
+ goto error_release_irq;
+
+ list_add_tail(&trig->alloc_list,
+ &iio_gpio_trigger_list);
}
- ret = request_irq(irq, iio_gpio_trigger_poll,
- IRQF_TRIGGER_RISING,
- trig->name,
- trig);
- if (ret)
- goto error_release_gpio;
+ irq_res_cnt++;
+ } while (irq_res != NULL);
- ret = iio_trigger_register(trig);
- if (ret)
- goto error_release_irq;
- list_add_tail(&trig->alloc_list, &iio_gpio_trigger_list);
-
- }
return 0;
/* First clean up the partly allocated trigger */
error_release_irq:
free_irq(irq, trig);
-error_release_gpio:
- gpio_free(trig_info->gpio);
error_free_name:
kfree(trig->name);
error_free_trig_info:
@@ -142,18 +140,16 @@ error_free_completed_registrations:
&iio_gpio_trigger_list,
alloc_list) {
trig_info = trig->private_data;
- free_irq(gpio_to_irq(trig_info->gpio), trig);
- gpio_free(trig_info->gpio);
+ free_irq(gpio_to_irq(trig_info->irq), trig);
kfree(trig->name);
kfree(trig_info);
iio_trigger_unregister(trig);
}
-error_ret:
return ret;
}
-static int iio_gpio_trigger_remove(struct platform_device *dev)
+static int iio_gpio_trigger_remove(struct platform_device *pdev)
{
struct iio_trigger *trig, *trig2;
struct iio_gpio_trigger_info *trig_info;
@@ -165,8 +161,7 @@ static int iio_gpio_trigger_remove(struct platform_device *dev)
alloc_list) {
trig_info = trig->private_data;
iio_trigger_unregister(trig);
- free_irq(gpio_to_irq(trig_info->gpio), trig);
- gpio_free(trig_info->gpio);
+ free_irq(trig_info->irq, trig);
kfree(trig->name);
kfree(trig_info);
iio_put_trigger(trig);
^ permalink raw reply related [flat|nested] 11+ messages in thread[parent not found: <544AC56F16B56944AEC3BD4E3D5917712D6B1D9767@LIMKCMBX1.ad.analog.com>]
* Re: [PATCH] iio-trig-gpio:Remove redundant gpio_request
[not found] <544AC56F16B56944AEC3BD4E3D5917712D6B1D9767@LIMKCMBX1.ad.analog.com>
@ 2010-04-22 23:37 ` Greg KH
0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2010-04-22 23:37 UTC (permalink / raw)
To: Hennerich, Michael
Cc: gregkh@suse.de, linux-iio@vger.kernel.org, LKML,
uclinux-dist-devel@blackfin.uclinux.org, Jonathan Cameron
On Tue, Mar 09, 2010 at 12:58:29PM +0000, Hennerich, Michael wrote:
>
> Remove redundant gpio_request:
> The GPIO used as trigger IRQ, is also requested as gpio, but actually never read.
>
> Use platform resource facility to get IRQs numbers and flags.
> Make sure this driver can be used with any system IRQ, not necessarily limited to GPIO-IRQs.
> Use dev_err(dev...) and friends instead of printk(KERN_ERR...)
>
>
> Signed-off-by: Michael Hennerich <Michael.Hennerich@analog.com>
> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>
>
> Index: drivers/staging/iio/trigger/iio-trig-gpio.c
> ===================================================================
> --- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 7916)
> +++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
> @@ -13,7 +13,6 @@
> * TODO:
> *
> * Add board config elements to allow specification of startup settings.
> - * Add configuration settings (irq type etc)
> */
>
> #include <linux/kernel.h>
> @@ -30,7 +29,7 @@
>
> struct iio_gpio_trigger_info {
> struct mutex in_use;
> - int gpio;
> + unsigned int irq;
Your email client turned the tabs into spaces, and made this patch
unappliable. Can you fix this and resend your other patches as well
(they also had the same problem.)
Take a look at Documentation/SubmittingPatches for how to do this
correctly (you also need to make the diff one level down).
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] iio-trig-gpio:Remove redundant gpio_request
@ 2010-03-09 9:35 Hennerich, Michael
2010-03-09 12:45 ` Jonathan Cameron
0 siblings, 1 reply; 11+ messages in thread
From: Hennerich, Michael @ 2010-03-09 9:35 UTC (permalink / raw)
To: Jonathan Cameron, gregkh@suse.de
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Remove redundant gpio_request:
The GPIO used as trigger IRQ, is also requested as gpio, but actually never=
read.
Use platform resource facility to get IRQs numbers and flags.
Make sure this driver can be used with any system IRQ, not necessarily limi=
ted to GPIO-IRQs.
Use dev_err(dev...) and friends instead of printk(KERN_ERR...)
From: Michael Hennerich <Michael.Hennerich@analog.com>
Index: drivers/staging/iio/trigger/iio-trig-gpio.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 7916)
+++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
@@ -13,7 +13,6 @@
* TODO:
*
* Add board config elements to allow specification of startup settings.
- * Add configuration settings (irq type etc)
*/
#include <linux/kernel.h>
@@ -30,7 +29,7 @@
struct iio_gpio_trigger_info {
struct mutex in_use;
- int gpio;
+ unsigned int irq;
};
/*
* Need to reference count these triggers and only enable gpio interrupts
@@ -57,78 +56,77 @@
.attrs =3D iio_gpio_trigger_attrs,
};
-static int iio_gpio_trigger_probe(struct platform_device *dev)
+static int iio_gpio_trigger_probe(struct platform_device *pdev)
{
- int *pdata =3D dev->dev.platform_data;
struct iio_gpio_trigger_info *trig_info;
struct iio_trigger *trig, *trig2;
- int i, irq, ret =3D 0;
- if (!pdata) {
- printk(KERN_ERR "No IIO gpio trigger platform data found\n"=
);
- goto error_ret;
- }
- for (i =3D 0;; i++) {
- if (!gpio_is_valid(pdata[i]))
+ unsigned long irqflags;
+ struct resource *irq_res;
+ int irq, ret =3D 0, irq_res_cnt =3D 0;
+
+ do {
+ irq_res =3D platform_get_resource(pdev,
+ IORESOURCE_IRQ, irq_res_cnt);
+
+ if (irq_res =3D=3D NULL) {
+ if (irq_res_cnt =3D=3D 0)
+ dev_err(&pdev->dev, "No GPIO IRQs specified=
");
break;
- trig =3D iio_allocate_trigger();
- if (!trig) {
- ret =3D -ENOMEM;
- goto error_free_completed_registrations;
}
+ irqflags =3D (irq_res->flags & IRQF_TRIGGER_MASK) | IRQF_SH=
ARED;
- trig_info =3D kzalloc(sizeof(*trig_info), GFP_KERNEL);
- if (!trig_info) {
- ret =3D -ENOMEM;
- goto error_put_trigger;
- }
- trig->control_attrs =3D &iio_gpio_trigger_attr_group;
- trig->private_data =3D trig_info;
- trig_info->gpio =3D pdata[i];
- trig->owner =3D THIS_MODULE;
- trig->name =3D kmalloc(IIO_TRIGGER_NAME_LENGTH, GFP_KERNEL)=
;
- if (!trig->name) {
- ret =3D -ENOMEM;
- goto error_free_trig_info;
- }
- snprintf((char *)trig->name,
- IIO_TRIGGER_NAME_LENGTH,
- "gpiotrig%d",
- pdata[i]);
- ret =3D gpio_request(trig_info->gpio, trig->name);
- if (ret)
- goto error_free_name;
+ for (irq =3D irq_res->start; irq <=3D irq_res->end; irq++) =
{
- ret =3D gpio_direction_input(trig_info->gpio);
- if (ret)
- goto error_release_gpio;
+ trig =3D iio_allocate_trigger();
+ if (!trig) {
+ ret =3D -ENOMEM;
+ goto error_free_completed_registrations;
+ }
- irq =3D gpio_to_irq(trig_info->gpio);
- if (irq < 0) {
- ret =3D irq;
- goto error_release_gpio;
- }
+ trig_info =3D kzalloc(sizeof(*trig_info), GFP_KERNE=
L);
+ if (!trig_info) {
+ ret =3D -ENOMEM;
+ goto error_put_trigger;
+ }
+ trig->control_attrs =3D &iio_gpio_trigger_attr_grou=
p;
+ trig->private_data =3D trig_info;
+ trig_info->irq =3D irq;
+ trig->owner =3D THIS_MODULE;
+ trig->name =3D kmalloc(IIO_TRIGGER_NAME_LENGTH,
+ GFP_KERNEL);
+ if (!trig->name) {
+ ret =3D -ENOMEM;
+ goto error_free_trig_info;
+ }
+ snprintf((char *)trig->name,
+ IIO_TRIGGER_NAME_LENGTH,
+ "irqtrig%d", irq);
- ret =3D request_irq(irq, iio_gpio_trigger_poll,
- IRQF_TRIGGER_RISING,
- trig->name,
- trig);
- if (ret)
- goto error_release_gpio;
+ ret =3D 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_name;
+ }
- ret =3D iio_trigger_register(trig);
- if (ret)
- goto error_release_irq;
+ ret =3D iio_trigger_register(trig);
+ if (ret)
+ goto error_release_irq;
- list_add_tail(&trig->alloc_list, &iio_gpio_trigger_list);
+ list_add_tail(&trig->alloc_list,
+ &iio_gpio_trigger_list);
+ }
- }
+ irq_res_cnt++;
+ } while (irq_res !=3D NULL);
+
+
return 0;
/* First clean up the partly allocated trigger */
error_release_irq:
free_irq(irq, trig);
-error_release_gpio:
- gpio_free(trig_info->gpio);
error_free_name:
kfree(trig->name);
error_free_trig_info:
@@ -142,18 +140,16 @@
&iio_gpio_trigger_list,
alloc_list) {
trig_info =3D trig->private_data;
- free_irq(gpio_to_irq(trig_info->gpio), trig);
- gpio_free(trig_info->gpio);
+ free_irq(gpio_to_irq(trig_info->irq), trig);
kfree(trig->name);
kfree(trig_info);
iio_trigger_unregister(trig);
}
-error_ret:
return ret;
}
-static int iio_gpio_trigger_remove(struct platform_device *dev)
+static int iio_gpio_trigger_remove(struct platform_device *pdev)
{
struct iio_trigger *trig, *trig2;
struct iio_gpio_trigger_info *trig_info;
@@ -165,8 +161,7 @@
alloc_list) {
trig_info =3D trig->private_data;
iio_trigger_unregister(trig);
- free_irq(gpio_to_irq(trig_info->gpio), trig);
- gpio_free(trig_info->gpio);
+ free_irq(trig_info->irq, trig);
kfree(trig->name);
kfree(trig_info);
iio_put_trigger(trig);
------------------------------------------------------------------
********* Analog Devices GmbH Open Platform Solutions
** *****
** ** Wilhelm-Wagenfeld-Strasse 6
** ***** D-80807 Munich
********* Germany
Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas Wessel, W=
illiam A. Martin, Margaret K. Seif
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] iio-trig-gpio:Remove redundant gpio_request
2010-03-09 9:35 Hennerich, Michael
@ 2010-03-09 12:45 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2010-03-09 12:45 UTC (permalink / raw)
To: Hennerich, Michael
Cc: gregkh@suse.de, linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Again,
This patch probably needs.
Signed-off-by: Michael Hennerich <Michael.Hennerich@analog.com>
and you can add
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
p.s. For those not following original thread this is a fix for=20
am misunderstanding of mine when I wrote the original code.
(Thanks!)
> Remove redundant gpio_request:
> The GPIO used as trigger IRQ, is also requested as gpio, but actually=
never read.
>=20
> Use platform resource facility to get IRQs numbers and flags.
> Make sure this driver can be used with any system IRQ, not necessaril=
y limited to GPIO-IRQs.
> Use dev_err(dev...) and friends instead of printk(KERN_ERR...)
>=20
> From: Michael Hennerich <Michael.Hennerich@analog.com>
>=20
> Index: drivers/staging/iio/trigger/iio-trig-gpio.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 7916)
> +++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
> @@ -13,7 +13,6 @@
> * TODO:
> *
> * Add board config elements to allow specification of startup setti=
ngs.
> - * Add configuration settings (irq type etc)
> */
>=20
> #include <linux/kernel.h>
> @@ -30,7 +29,7 @@
>=20
> struct iio_gpio_trigger_info {
> struct mutex in_use;
> - int gpio;
> + unsigned int irq;
> };
> /*
> * Need to reference count these triggers and only enable gpio inter=
rupts
> @@ -57,78 +56,77 @@
> .attrs =3D iio_gpio_trigger_attrs,
> };
>=20
> -static int iio_gpio_trigger_probe(struct platform_device *dev)
> +static int iio_gpio_trigger_probe(struct platform_device *pdev)
> {
> - int *pdata =3D dev->dev.platform_data;
> struct iio_gpio_trigger_info *trig_info;
> struct iio_trigger *trig, *trig2;
> - int i, irq, ret =3D 0;
> - if (!pdata) {
> - printk(KERN_ERR "No IIO gpio trigger platform data fo=
und\n");
> - goto error_ret;
> - }
> - for (i =3D 0;; i++) {
> - if (!gpio_is_valid(pdata[i]))
> + unsigned long irqflags;
> + struct resource *irq_res;
> + int irq, ret =3D 0, irq_res_cnt =3D 0;
> +
> + do {
> + irq_res =3D platform_get_resource(pdev,
> + IORESOURCE_IRQ, irq_res_cnt);
> +
> + if (irq_res =3D=3D NULL) {
> + if (irq_res_cnt =3D=3D 0)
> + dev_err(&pdev->dev, "No GPIO IRQs spe=
cified");
> break;
> - trig =3D iio_allocate_trigger();
> - if (!trig) {
> - ret =3D -ENOMEM;
> - goto error_free_completed_registrations;
> }
> + irqflags =3D (irq_res->flags & IRQF_TRIGGER_MASK) | I=
RQF_SHARED;
>=20
> - trig_info =3D kzalloc(sizeof(*trig_info), GFP_KERNEL)=
;
> - if (!trig_info) {
> - ret =3D -ENOMEM;
> - goto error_put_trigger;
> - }
> - trig->control_attrs =3D &iio_gpio_trigger_attr_group;
> - trig->private_data =3D trig_info;
> - trig_info->gpio =3D pdata[i];
> - trig->owner =3D THIS_MODULE;
> - trig->name =3D kmalloc(IIO_TRIGGER_NAME_LENGTH, GFP_K=
ERNEL);
> - if (!trig->name) {
> - ret =3D -ENOMEM;
> - goto error_free_trig_info;
> - }
> - snprintf((char *)trig->name,
> - IIO_TRIGGER_NAME_LENGTH,
> - "gpiotrig%d",
> - pdata[i]);
> - ret =3D gpio_request(trig_info->gpio, trig->name);
> - if (ret)
> - goto error_free_name;
> + for (irq =3D irq_res->start; irq <=3D irq_res->end; i=
rq++) {
>=20
> - ret =3D gpio_direction_input(trig_info->gpio);
> - if (ret)
> - goto error_release_gpio;
> + trig =3D iio_allocate_trigger();
> + if (!trig) {
> + ret =3D -ENOMEM;
> + goto error_free_completed_registratio=
ns;
> + }
>=20
> - irq =3D gpio_to_irq(trig_info->gpio);
> - if (irq < 0) {
> - ret =3D irq;
> - goto error_release_gpio;
> - }
> + trig_info =3D kzalloc(sizeof(*trig_info), GFP=
_KERNEL);
> + if (!trig_info) {
> + ret =3D -ENOMEM;
> + goto error_put_trigger;
> + }
> + trig->control_attrs =3D &iio_gpio_trigger_att=
r_group;
> + trig->private_data =3D trig_info;
> + trig_info->irq =3D irq;
> + trig->owner =3D THIS_MODULE;
> + trig->name =3D kmalloc(IIO_TRIGGER_NAME_LENGT=
H,
> + GFP_KERNEL);
> + if (!trig->name) {
> + ret =3D -ENOMEM;
> + goto error_free_trig_info;
> + }
> + snprintf((char *)trig->name,
> + IIO_TRIGGER_NAME_LENGTH,
> + "irqtrig%d", irq);
>=20
> - ret =3D request_irq(irq, iio_gpio_trigger_poll,
> - IRQF_TRIGGER_RISING,
> - trig->name,
> - trig);
> - if (ret)
> - goto error_release_gpio;
> + ret =3D request_irq(irq, iio_gpio_trigger_pol=
l,
> + irqflags, trig->name, trig)=
;
> + if (ret) {
> + dev_err(&pdev->dev,
> + "request IRQ-%d failed", irq)=
;
> + goto error_free_name;
> + }
>=20
> - ret =3D iio_trigger_register(trig);
> - if (ret)
> - goto error_release_irq;
> + ret =3D iio_trigger_register(trig);
> + if (ret)
> + goto error_release_irq;
>=20
> - list_add_tail(&trig->alloc_list, &iio_gpio_trigger_li=
st);
> + list_add_tail(&trig->alloc_list,
> + &iio_gpio_trigger_list);
> + }
>=20
> - }
> + irq_res_cnt++;
> + } while (irq_res !=3D NULL);
> +
> +
> return 0;
>=20
> /* First clean up the partly allocated trigger */
> error_release_irq:
> free_irq(irq, trig);
> -error_release_gpio:
> - gpio_free(trig_info->gpio);
> error_free_name:
> kfree(trig->name);
> error_free_trig_info:
> @@ -142,18 +140,16 @@
> &iio_gpio_trigger_list,
> alloc_list) {
> trig_info =3D trig->private_data;
> - free_irq(gpio_to_irq(trig_info->gpio), trig);
> - gpio_free(trig_info->gpio);
> + free_irq(gpio_to_irq(trig_info->irq), trig);
> kfree(trig->name);
> kfree(trig_info);
> iio_trigger_unregister(trig);
> }
>=20
> -error_ret:
> return ret;
> }
>=20
> -static int iio_gpio_trigger_remove(struct platform_device *dev)
> +static int iio_gpio_trigger_remove(struct platform_device *pdev)
> {
> struct iio_trigger *trig, *trig2;
> struct iio_gpio_trigger_info *trig_info;
> @@ -165,8 +161,7 @@
> alloc_list) {
> trig_info =3D trig->private_data;
> iio_trigger_unregister(trig);
> - free_irq(gpio_to_irq(trig_info->gpio), trig);
> - gpio_free(trig_info->gpio);
> + free_irq(trig_info->irq, trig);
> kfree(trig->name);
> kfree(trig_info);
> iio_put_trigger(trig);
>=20
>=20
> ------------------------------------------------------------------
> ********* Analog Devices GmbH Open Platform Solutions
> ** *****
> ** ** Wilhelm-Wagenfeld-Strasse 6
> ** ***** D-80807 Munich
> ********* Germany
> Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas Wes=
sel, William A. Martin, Margaret K. Seif
>=20
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] iio-trig-gpio:Remove redundant gpio_request
@ 2010-03-08 9:31 Hennerich, Michael
2010-03-08 10:58 ` Jonathan Cameron
0 siblings, 1 reply; 11+ messages in thread
From: Hennerich, Michael @ 2010-03-08 9:31 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Remove redundant gpio_request.
The GPIO used as trigger IRQ, is also requested as gpio, but actually never=
read.
On some platforms a GPIO requested as IRQ can't be requested as GPIO as wel=
l.
From: Michael Hennerich <Michael.Hennerich@analog.com>
Index: drivers/staging/iio/trigger/iio-trig-gpio.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 8368)
+++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
@@ -94,18 +94,11 @@
IIO_TRIGGER_NAME_LENGTH,
"gpiotrig%d",
pdata[i]);
- ret =3D gpio_request(trig_info->gpio, trig->name);
- if (ret)
- goto error_free_name;
- ret =3D gpio_direction_input(trig_info->gpio);
- if (ret)
- goto error_release_gpio;
-
irq =3D gpio_to_irq(trig_info->gpio);
if (irq < 0) {
ret =3D irq;
- goto error_release_gpio;
+ goto error_free_name;
}
ret =3D request_irq(irq, iio_gpio_trigger_poll,
@@ -113,7 +106,7 @@
trig->name,
trig);
if (ret)
- goto error_release_gpio;
+ goto error_free_name;
ret =3D iio_trigger_register(trig);
if (ret)
@@ -127,8 +120,6 @@
/* First clean up the partly allocated trigger */
error_release_irq:
free_irq(irq, trig);
-error_release_gpio:
- gpio_free(trig_info->gpio);
error_free_name:
kfree(trig->name);
error_free_trig_info:
@@ -143,7 +134,6 @@
alloc_list) {
trig_info =3D trig->private_data;
free_irq(gpio_to_irq(trig_info->gpio), trig);
- gpio_free(trig_info->gpio);
kfree(trig->name);
kfree(trig_info);
iio_trigger_unregister(trig);
@@ -166,7 +156,6 @@
trig_info =3D trig->private_data;
iio_trigger_unregister(trig);
free_irq(gpio_to_irq(trig_info->gpio), trig);
- gpio_free(trig_info->gpio);
kfree(trig->name);
kfree(trig_info);
iio_put_trigger(trig);
------------------------------------------------------------------
********* Analog Devices GmbH Open Platform Solutions
** *****
** ** Wilhelm-Wagenfeld-Strasse 6
** ***** D-80807 Munich
********* Germany
Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas Wessel, W=
illiam A. Martin, Margaret K. Seif
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] iio-trig-gpio:Remove redundant gpio_request
2010-03-08 9:31 Hennerich, Michael
@ 2010-03-08 10:58 ` Jonathan Cameron
2010-03-08 11:38 ` Hennerich, Michael
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2010-03-08 10:58 UTC (permalink / raw)
To: Hennerich, Michael
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Hi Michael,
I'm wondering about this one. Are you sure all platforms=20
have the correct gpio setup when doing the request_irq?
(those that need it obviously, basically the question is whether
all platforms default to having gpio's as inputs).
If the do there is certainly a lot of redundant code about
based on a quick bit of grepping. (loads of it in arm/mach-pxa
for starters). In that particular case all gpio's are configured
as inputs, but I'm not sure other platforms are so well behaved.
Unless I'm completely wrong that means we have run into a
rather general issue.
Also, if this is fine, the right fix is probably to have the interrupt =
as the
platform data parameter rather than the gpio. It makes the code more g=
eneral
and moves a lot of these direction issues etc into the board specific a=
reas
rather than here.
Also, CC Greg KH on any patches like this which are fixes.
Jonathan
> Remove redundant gpio_request.
> The GPIO used as trigger IRQ, is also requested as gpio, but actually=
never read.
> On some platforms a GPIO requested as IRQ can't be requested as GPIO =
as well.
>=20
> From: Michael Hennerich <Michael.Hennerich@analog.com>
>=20
> Index: drivers/staging/iio/trigger/iio-trig-gpio.c
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 8368)
> +++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
> @@ -94,18 +94,11 @@
> IIO_TRIGGER_NAME_LENGTH,
> "gpiotrig%d",
> pdata[i]);
> - ret =3D gpio_request(trig_info->gpio, trig->name);
> - if (ret)
> - goto error_free_name;
>=20
> - ret =3D gpio_direction_input(trig_info->gpio);
> - if (ret)
> - goto error_release_gpio;
> -
> irq =3D gpio_to_irq(trig_info->gpio);
> if (irq < 0) {
> ret =3D irq;
> - goto error_release_gpio;
> + goto error_free_name;
> }
>=20
> ret =3D request_irq(irq, iio_gpio_trigger_poll,
> @@ -113,7 +106,7 @@
> trig->name,
> trig);
> if (ret)
> - goto error_release_gpio;
> + goto error_free_name;
>=20
> ret =3D iio_trigger_register(trig);
> if (ret)
> @@ -127,8 +120,6 @@
> /* First clean up the partly allocated trigger */
> error_release_irq:
> free_irq(irq, trig);
> -error_release_gpio:
> - gpio_free(trig_info->gpio);
> error_free_name:
> kfree(trig->name);
> error_free_trig_info:
> @@ -143,7 +134,6 @@
> alloc_list) {
> trig_info =3D trig->private_data;
> free_irq(gpio_to_irq(trig_info->gpio), trig);
> - gpio_free(trig_info->gpio);
> kfree(trig->name);
> kfree(trig_info);
> iio_trigger_unregister(trig);
> @@ -166,7 +156,6 @@
> trig_info =3D trig->private_data;
> iio_trigger_unregister(trig);
> free_irq(gpio_to_irq(trig_info->gpio), trig);
> - gpio_free(trig_info->gpio);
> kfree(trig->name);
> kfree(trig_info);
> iio_put_trigger(trig);
>=20
>=20
>=20
> ------------------------------------------------------------------
> ********* Analog Devices GmbH Open Platform Solutions
> ** *****
> ** ** Wilhelm-Wagenfeld-Strasse 6
> ** ***** D-80807 Munich
> ********* Germany
> Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas Wes=
sel, William A. Martin, Margaret K. Seif
>=20
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH] iio-trig-gpio:Remove redundant gpio_request
2010-03-08 10:58 ` Jonathan Cameron
@ 2010-03-08 11:38 ` Hennerich, Michael
2010-03-08 13:12 ` Jonathan Cameron
0 siblings, 1 reply; 11+ messages in thread
From: Hennerich, Michael @ 2010-03-08 11:38 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Hi Jonathan,
Jonathan Cameron wrote on 2010-03-08:
> Hi Michael,
>
> I'm wondering about this one. Are you sure all platforms have the
> correct gpio setup when doing the request_irq?
> (those that need it obviously, basically the question is whether all
> platforms default to having gpio's as inputs).
>
> If the do there is certainly a lot of redundant code about based on a
> quick bit of grepping. (loads of it in arm/mach-pxa for starters). In
> that particular case all gpio's are configured as inputs, but I'm not
> sure other platforms are so well behaved.
>
>
> Unless I'm completely wrong that means we have run into a rather
> general issue.
By no means - you have to first configure a GPIO_IRQ as GPIO-Input and then=
as IRQ.
irq_type takes care of it. In your case you gave the IRQF_TRIGGER_RISING fl=
ag with request_irq().
In case such a IRQF_TRIGGER_XXX flag is present the common IRQ infrastructu=
re will call the associated irq_chips irq_type() function.
This function is supposed to take care for all of it.(GPIO input, setting t=
he level, edge and polarity)
As an alternative you can request_irq() without the IRQF_TRIGGER_XXX flags=
and use set_irq_type() anytime later, or even in advance.
>
> Also, if this is fine, the right fix is probably to have the interrupt
> as the platform data parameter rather than the gpio. It makes the
> code more general and moves a lot of these direction issues etc into
> the board specific areas rather than here.
Of course this is an option.
Regards,
Michael
>
> Also, CC Greg KH on any patches like this which are fixes.
>
> Jonathan
>
>> Remove redundant gpio_request. The GPIO used as trigger IRQ, is also
>> requested as gpio, but actually never read. On some platforms a GPIO
>> requested as IRQ can't be requested as GPIO as well.
>>
>> From: Michael Hennerich <Michael.Hennerich@analog.com>
>>
>> Index: drivers/staging/iio/trigger/iio-trig-gpio.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 8368)
>> +++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
>> @@ -94,18 +94,11 @@
>> IIO_TRIGGER_NAME_LENGTH,
>> "gpiotrig%d",
>> pdata[i]);
>> - ret =3D gpio_request(trig_info->gpio, trig->name);
>> - if (ret)
>> - goto error_free_name;
>>
>> - ret =3D gpio_direction_input(trig_info->gpio);
>> - if (ret)
>> - goto error_release_gpio;
>> -
>> irq =3D gpio_to_irq(trig_info->gpio);
>> if (irq < 0) {
>> ret =3D irq;
>> - goto error_release_gpio;
>> + goto error_free_name;
>> }
>>
>> ret =3D request_irq(irq, iio_gpio_trigger_poll, @@
>> -113,7 +106,7 @@
>> trig->name,
>> trig);
>> if (ret)
>> - goto error_release_gpio;
>> + goto error_free_name;
>>
>> ret =3D iio_trigger_register(trig);
>> if (ret)
>> @@ -127,8 +120,6 @@
>> /* First clean up the partly allocated trigger */
>> error_release_irq:
>> free_irq(irq, trig);
>> -error_release_gpio:
>> - gpio_free(trig_info->gpio);
>> error_free_name:
>> kfree(trig->name);
>> error_free_trig_info:
>> @@ -143,7 +134,6 @@
>> alloc_list) {
>> trig_info =3D trig->private_data;
>> free_irq(gpio_to_irq(trig_info->gpio), trig); -
>> gpio_free(trig_info->gpio); kfree(trig->name);
>> kfree(trig_info); iio_trigger_unregister(trig); @@
>> -166,7 +156,6 @@ trig_info =3D trig->private_data;
>> iio_trigger_unregister(trig);
>> free_irq(gpio_to_irq(trig_info->gpio), trig); -
>> gpio_free(trig_info->gpio); kfree(trig->name);
>> kfree(trig_info); iio_put_trigger(trig);
>>
>>
>> ------------------------------------------------------------------
>> ********* Analog Devices GmbH Open Platform Solutions
>> ** *****
>> ** ** Wilhelm-Wagenfeld-Strasse 6
>> ** ***** D-80807 Munich
>> ********* Germany
>> Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas Wessel=
,
>> William A. Martin, Margaret K. Seif
>>
Greetings,
Michael
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036 Geschaeft=
sfuehrer Thomas Wessel, William A. Martin, Margaret Seif
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] iio-trig-gpio:Remove redundant gpio_request
2010-03-08 11:38 ` Hennerich, Michael
@ 2010-03-08 13:12 ` Jonathan Cameron
2010-03-08 13:45 ` Hennerich, Michael
0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Cameron @ 2010-03-08 13:12 UTC (permalink / raw)
To: Hennerich, Michael
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
On 03/08/10 11:38, Hennerich, Michael wrote:
> Hi Jonathan,
> Jonathan Cameron wrote on 2010-03-08:
>> Hi Michael,
>>
>> I'm wondering about this one. Are you sure all platforms have the
>> correct gpio setup when doing the request_irq?
>> (those that need it obviously, basically the question is whether all
>> platforms default to having gpio's as inputs).
>>
>> If the do there is certainly a lot of redundant code about based on =
a
>> quick bit of grepping. (loads of it in arm/mach-pxa for starters). =
In
>> that particular case all gpio's are configured as inputs, but I'm no=
t
>> sure other platforms are so well behaved.
>>
>>
>> Unless I'm completely wrong that means we have run into a rather
>> general issue.
>=20
> By no means - you have to first configure a GPIO_IRQ as GPIO-Input an=
d then as IRQ.
> irq_type takes care of it. In your case you gave the IRQF_TRIGGER_RIS=
ING flag with request_irq().
> In case such a IRQF_TRIGGER_XXX flag is present the common IRQ infras=
tructure will call the associated irq_chips irq_type() function.
> This function is supposed to take care for all of it.(GPIO input, set=
ting the level, edge and polarity)
>=20
> As an alternative you can request_irq() without the IRQF_TRIGGER_XXX=
flags and use set_irq_type() anytime later, or even in advance.
Ah fair enough. Thanks for cleaning that up for me.
Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
Please send it on to Greg KH
>=20
>>
>> Also, if this is fine, the right fix is probably to have the interru=
pt
>> as the platform data parameter rather than the gpio. It makes the
>> code more general and moves a lot of these direction issues etc into
>> the board specific areas rather than here.
>=20
> Of course this is an option.
>=20
> Regards,
> Michael
>=20
>>
>> Also, CC Greg KH on any patches like this which are fixes.
>>
>> Jonathan
>>
>>> Remove redundant gpio_request. The GPIO used as trigger IRQ, is als=
o
>>> requested as gpio, but actually never read. On some platforms a GPI=
O
>>> requested as IRQ can't be requested as GPIO as well.
>>>
>>> From: Michael Hennerich <Michael.Hennerich@analog.com>
>>>
>>> Index: drivers/staging/iio/trigger/iio-trig-gpio.c
>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>> --- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 8368)
>>> +++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
>>> @@ -94,18 +94,11 @@
>>> IIO_TRIGGER_NAME_LENGTH,
>>> "gpiotrig%d",
>>> pdata[i]);
>>> - ret =3D gpio_request(trig_info->gpio, trig->name);
>>> - if (ret)
>>> - goto error_free_name;
>>>
>>> - ret =3D gpio_direction_input(trig_info->gpio);
>>> - if (ret)
>>> - goto error_release_gpio;
>>> -
>>> irq =3D gpio_to_irq(trig_info->gpio);
>>> if (irq < 0) {
>>> ret =3D irq;
>>> - goto error_release_gpio;
>>> + goto error_free_name;
>>> }
>>>
>>> ret =3D request_irq(irq, iio_gpio_trigger_poll, @@
>>> -113,7 +106,7 @@
>>> trig->name,
>>> trig);
>>> if (ret)
>>> - goto error_release_gpio;
>>> + goto error_free_name;
>>>
>>> ret =3D iio_trigger_register(trig);
>>> if (ret)
>>> @@ -127,8 +120,6 @@
>>> /* First clean up the partly allocated trigger */
>>> error_release_irq:
>>> free_irq(irq, trig);
>>> -error_release_gpio:
>>> - gpio_free(trig_info->gpio);
>>> error_free_name:
>>> kfree(trig->name);
>>> error_free_trig_info:
>>> @@ -143,7 +134,6 @@
>>> alloc_list) {
>>> trig_info =3D trig->private_data;
>>> free_irq(gpio_to_irq(trig_info->gpio), trig); -
>>> gpio_free(trig_info->gpio); kfree(trig->name)=
;
>>> kfree(trig_info); iio_trigger_unregister(trig); @@
>>> -166,7 +156,6 @@ trig_info =3D trig->private_data;
>>> iio_trigger_unregister(trig);
>>> free_irq(gpio_to_irq(trig_info->gpio), trig); -
>>> gpio_free(trig_info->gpio); kfree(trig->name)=
;
>>> kfree(trig_info); iio_put_trigger(trig);
>>>
>>>
>>> ------------------------------------------------------------------
>>> ********* Analog Devices GmbH Open Platform Solutions
>>> ** *****
>>> ** ** Wilhelm-Wagenfeld-Strasse 6
>>> ** ***** D-80807 Munich
>>> ********* Germany
>>> Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas W=
essel,
>>> William A. Martin, Margaret K. Seif
>>>
> Greetings,
> Michael
>=20
> Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
> Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036 Ges=
chaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
>=20
>=20
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH] iio-trig-gpio:Remove redundant gpio_request
2010-03-08 13:12 ` Jonathan Cameron
@ 2010-03-08 13:45 ` Hennerich, Michael
2010-03-08 14:47 ` Hennerich, Michael
0 siblings, 1 reply; 11+ messages in thread
From: Hennerich, Michael @ 2010-03-08 13:45 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Jonathan Cameron wrote on 2010-03-08:
> On 03/08/10 11:38, Hennerich, Michael wrote:
>> Hi Jonathan,
>> Jonathan Cameron wrote on 2010-03-08:
>>> Hi Michael,
>>>
>>> I'm wondering about this one. Are you sure all platforms have the
>>> correct gpio setup when doing the request_irq?
>>> (those that need it obviously, basically the question is whether
>>> all platforms default to having gpio's as inputs).
>>>
>>> If the do there is certainly a lot of redundant code about based on a
>>> quick bit of grepping. (loads of it in arm/mach-pxa for starters). In
>>> that particular case all gpio's are configured as inputs, but I'm not
>>> sure other platforms are so well behaved.
>>>
>>>
>>> Unless I'm completely wrong that means we have run into a rather
>>> general issue.
>>
>> By no means - you have to first configure a GPIO_IRQ as GPIO-Input and
>> then as IRQ. irq_type takes care of it. In your case you gave the
>> IRQF_TRIGGER_RISING flag with request_irq(). In case such a
>> IRQF_TRIGGER_XXX flag is present the common IRQ infrastructure will
>> call the associated irq_chips irq_type() function. This function is
>> supposed to take care for all of it.(GPIO input, setting the level,
>> edge and polarity)
>>
>> As an alternative you can request_irq() without the
>> IRQF_TRIGGER_XXX
> flags and use set_irq_type() anytime later, or even in advance.
> Ah fair enough. Thanks for cleaning that up for me.
>
> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>
> Please send it on to Greg KH
Shall I change the driver to loop on while(irq_res !=3D NULL)?
And get the irqflags from the platform data struct resource?
unsigned long irqflags;
struct resource *irq_res;
int i =3D 0;
irq_res =3D platform_get_resource(pdev, IORESOURCE_IRQ, i++);
irqflags =3D irq_res->flags & IRQF_TRIGGER_MASK;
[ -- snip -- ]
ret =3D request_irq(irq, iio_gpio_trigger_poll,
irqflags,
trig->name,
trig);
-Michael
>>
>>>
>>> Also, if this is fine, the right fix is probably to have the interrupt
>>> as the platform data parameter rather than the gpio. It makes the
>>> code more general and moves a lot of these direction issues etc into
>>> the board specific areas rather than here.
>>
>> Of course this is an option.
>>
>> Regards,
>> Michael
>>
>>>
>>> Also, CC Greg KH on any patches like this which are fixes.
>>>
>>> Jonathan
>>>
>>>> Remove redundant gpio_request. The GPIO used as trigger IRQ, is also
>>>> requested as gpio, but actually never read. On some platforms a GPIO
>>>> requested as IRQ can't be requested as GPIO as well.
>>>>
>>>> From: Michael Hennerich <Michael.Hennerich@analog.com>
>>>>
>>>> Index: drivers/staging/iio/trigger/iio-trig-gpio.c
>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>>>> =3D
>>>> --- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 8368)
>>>> +++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
>>>> @@ -94,18 +94,11 @@
>>>> IIO_TRIGGER_NAME_LENGTH,
>>>> "gpiotrig%d",
>>>> pdata[i]);
>>>> - ret =3D gpio_request(trig_info->gpio, trig->name);
>>>> - if (ret)
>>>> - goto error_free_name;
>>>>
>>>> - ret =3D gpio_direction_input(trig_info->gpio);
>>>> - if (ret)
>>>> - goto error_release_gpio;
>>>> -
>>>> irq =3D gpio_to_irq(trig_info->gpio);
>>>> if (irq < 0) {
>>>> ret =3D irq;
>>>> - goto error_release_gpio;
>>>> + goto error_free_name;
>>>> }
>>>>
>>>> ret =3D request_irq(irq, iio_gpio_trigger_poll, @@
>>>> -113,7 +106,7 @@
>>>> trig->name,
>>>> trig);
>>>> if (ret)
>>>> - goto error_release_gpio;
>>>> + goto error_free_name;
>>>>
>>>> ret =3D iio_trigger_register(trig);
>>>> if (ret)
>>>> @@ -127,8 +120,6 @@
>>>> /* First clean up the partly allocated trigger */
>>>> error_release_irq:
>>>> free_irq(irq, trig);
>>>> -error_release_gpio:
>>>> - gpio_free(trig_info->gpio);
>>>> error_free_name:
>>>> kfree(trig->name);
>>>> error_free_trig_info:
>>>> @@ -143,7 +134,6 @@
>>>> alloc_list) {
>>>> trig_info =3D trig->private_data;
>>>> free_irq(gpio_to_irq(trig_info->gpio), trig); -
>>>> gpio_free(trig_info->gpio); kfree(trig-
>> name);
>>>> kfree(trig_info); iio_trigger_unregister(trig); @@
>>>> -166,7 +156,6 @@ trig_info =3D trig->private_data;
>>>> iio_trigger_unregister(trig);
>>>> free_irq(gpio_to_irq(trig_info->gpio), trig); -
>>>> gpio_free(trig_info->gpio); kfree(trig-
>> name);
>>>> kfree(trig_info); iio_put_trigger(trig);
>>>>
>>>> ------------------------------------------------------------------
>>>> ********* Analog Devices GmbH Open Platform Solutions
>>>> ** *****
>>>> ** ** Wilhelm-Wagenfeld-Strasse 6
>>>> ** ***** D-80807 Munich
>>>> ********* Germany
>>>> Registergericht M=FCnchen HRB 40368, Gesch=E4ftsf=FChrer: Thomas
>>>> Wessel, William A. Martin, Margaret K. Seif
>>>>
>> Greetings,
>> Michael
>>
>> Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
>> Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036
>> Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif
>>
>>
Greetings,
Michael
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 4036 Geschaeft=
sfuehrer Thomas Wessel, William A. Martin, Margaret Seif
^ permalink raw reply [flat|nested] 11+ messages in thread* RE: [PATCH] iio-trig-gpio:Remove redundant gpio_request
2010-03-08 13:45 ` Hennerich, Michael
@ 2010-03-08 14:47 ` Hennerich, Michael
2010-03-08 15:41 ` Jonathan Cameron
0 siblings, 1 reply; 11+ messages in thread
From: Hennerich, Michael @ 2010-03-08 14:47 UTC (permalink / raw)
To: Hennerich, Michael, Jonathan Cameron
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
Hennerich, Michael wrote on 2010-03-08:
> Jonathan Cameron wrote on 2010-03-08:
>> On 03/08/10 11:38, Hennerich, Michael wrote:
>>> Hi Jonathan,
>>> Jonathan Cameron wrote on 2010-03-08:
>>>> Hi Michael,
>>>>
>>>> I'm wondering about this one. Are you sure all platforms have the
>>>> correct gpio setup when doing the request_irq? (those that need it
>>>> obviously, basically the question is whether all platforms default to
>>>> having gpio's as inputs).
>>>>
>>>> If the do there is certainly a lot of redundant code about based on a
>>>> quick bit of grepping. (loads of it in arm/mach-pxa for starters). In
>>>> that particular case all gpio's are configured as inputs, but I'm not
>>>> sure other platforms are so well behaved.
>>>>
>>>>
>>>> Unless I'm completely wrong that means we have run into a rather
>>>> general issue.
>>>
>>> By no means - you have to first configure a GPIO_IRQ as GPIO-Input and
>>> then as IRQ. irq_type takes care of it. In your case you gave the
>>> IRQF_TRIGGER_RISING flag with request_irq(). In case such a
>>> IRQF_TRIGGER_XXX flag is present the common IRQ infrastructure will
>>> call the associated irq_chips irq_type() function. This function is
>>> supposed to take care for all of it.(GPIO input, setting the level,
>>> edge and polarity)
>>>
>>> As an alternative you can request_irq() without the
>>> IRQF_TRIGGER_XXX
>> flags and use set_irq_type() anytime later, or even in advance.
>> Ah fair enough. Thanks for cleaning that up for me.
>>
>> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>>
>> Please send it on to Greg KH
>
> Shall I change the driver to loop on while(irq_res !=3D NULL)?
> And get the irqflags from the platform data struct resource?
What do you think about this one?
Example Platform Data:
#if defined(CONFIG_IIO_GPIO_TRIGGER) || \
defined(CONFIG_IIO_GPIO_TRIGGER_MODULE)
static struct resource iio_gpio_trigger_resources[] =3D {
[0] =3D {
.start =3D IRQ_PF5,
.end =3D IRQ_PF5,
.flags =3D IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE,
},
[1] =3D {
.start =3D IRQ_PF2,
.end =3D IRQ_PF3,
.flags =3D IORESOURCE_IRQ | IORESOURCE_IRQ_LOWEDGE,
},
};
static struct platform_device iio_gpio_trigger =3D {
.name =3D "iio_gpio_trigger",
.num_resources =3D ARRAY_SIZE(iio_gpio_trigger_resources),
.resource =3D iio_gpio_trigger_resources,
};
#endif
This way allows you to specify different GPIO Edge/Polarity behavior as wel=
l as GPIO-IRQ sequences.
Patch below is an top of my previous one.
I wonder I we should change all references to gpio completely (replace with=
irq), since this driver can also be used with any other system IRQ as well=
.
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 8415)
+++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
@@ -57,64 +57,73 @@
.attrs =3D iio_gpio_trigger_attrs,
};
-static int iio_gpio_trigger_probe(struct platform_device *dev)
+static int iio_gpio_trigger_probe(struct platform_device *pdev)
{
- int *pdata =3D dev->dev.platform_data;
+ int *pdata =3D pdev->dev.platform_data;
struct iio_gpio_trigger_info *trig_info;
struct iio_trigger *trig, *trig2;
- int i, irq, ret =3D 0;
- if (!pdata) {
- printk(KERN_ERR "No IIO gpio trigger platform data found\n"=
);
- goto error_ret;
- }
- for (i =3D 0;; i++) {
- if (!gpio_is_valid(pdata[i]))
+ unsigned long irqflags;
+ struct resource *irq_res;
+ int irq, ret =3D 0, irq_res_cnt =3D 0;
+ unsigned gpio;
+
+ do {
+ irq_res =3D platform_get_resource(pdev, IORESOURCE_IRQ, irq=
_res_cnt);
+
+ if (irq_res =3D=3D NULL) {
+ if (irq_res_cnt =3D=3D 0)
+ dev_err(&pdev->dev, "No GPIO IRQs specified=
");
break;
- trig =3D iio_allocate_trigger();
- if (!trig) {
- ret =3D -ENOMEM;
- goto error_free_completed_registrations;
}
+ irqflags =3D irq_res->flags & IRQF_TRIGGER_MASK;
- trig_info =3D kzalloc(sizeof(*trig_info), GFP_KERNEL);
- if (!trig_info) {
- ret =3D -ENOMEM;
- goto error_put_trigger;
- }
- trig->control_attrs =3D &iio_gpio_trigger_attr_group;
- trig->private_data =3D trig_info;
- trig_info->gpio =3D pdata[i];
- trig->owner =3D THIS_MODULE;
- trig->name =3D kmalloc(IIO_TRIGGER_NAME_LENGTH, GFP_KERNEL)=
;
- if (!trig->name) {
- ret =3D -ENOMEM;
- goto error_free_trig_info;
- }
- snprintf((char *)trig->name,
- IIO_TRIGGER_NAME_LENGTH,
- "gpiotrig%d",
- pdata[i]);
+ for (irq =3D irq_res->start; irq <=3D irq_res->end; irq++) =
{
+ gpio =3D irq_to_gpio(irq);
+ if (!gpio_is_valid(gpio))
+ break;
+ trig =3D iio_allocate_trigger();
+ if (!trig) {
+ ret =3D -ENOMEM;
+ goto error_free_completed_registrations;
+ }
- irq =3D gpio_to_irq(trig_info->gpio);
- if (irq < 0) {
- ret =3D irq;
- goto error_free_name;
+ trig_info =3D kzalloc(sizeof(*trig_info), GFP_KERNE=
L);
+ if (!trig_info) {
+ ret =3D -ENOMEM;
+ goto error_put_trigger;
+ }
+ trig->control_attrs =3D &iio_gpio_trigger_attr_grou=
p;
+ trig->private_data =3D trig_info;
+ trig_info->gpio =3D gpio;
+ trig->owner =3D THIS_MODULE;
+ trig->name =3D kmalloc(IIO_TRIGGER_NAME_LENGTH, GFP=
_KERNEL);
+ if (!trig->name) {
+ ret =3D -ENOMEM;
+ goto error_free_trig_info;
+ }
+ snprintf((char *)trig->name,
+ IIO_TRIGGER_NAME_LENGTH,
+ "gpiotrig%d",
+ gpio);
+
+ ret =3D request_irq(irq, iio_gpio_trigger_poll,
+ irqflags,
+ trig->name,
+ trig);
+ if (ret)
+ goto error_free_name;
+
+ ret =3D iio_trigger_register(trig);
+ if (ret)
+ goto error_release_irq;
+
+ list_add_tail(&trig->alloc_list, &iio_gpio_trigger_=
list);
}
- ret =3D request_irq(irq, iio_gpio_trigger_poll,
- IRQF_TRIGGER_RISING,
- trig->name,
- trig);
- if (ret)
- goto error_free_name;
+ irq_res_cnt++;
+ } while (irq_res !=3D NULL);
- ret =3D iio_trigger_register(trig);
- if (ret)
- goto error_release_irq;
- list_add_tail(&trig->alloc_list, &iio_gpio_trigger_list);
-
- }
return 0;
/* First clean up the partly allocated trigger */
@@ -143,7 +152,7 @@
return ret;
}
-static int iio_gpio_trigger_remove(struct platform_device *dev)
+static int iio_gpio_trigger_remove(struct platform_device *pdev)
{
struct iio_trigger *trig, *trig2;
struct iio_gpio_trigger_info *trig_info;
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] iio-trig-gpio:Remove redundant gpio_request
2010-03-08 14:47 ` Hennerich, Michael
@ 2010-03-08 15:41 ` Jonathan Cameron
0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2010-03-08 15:41 UTC (permalink / raw)
To: Hennerich, Michael
Cc: linux-iio@vger.kernel.org,
uclinux-dist-devel@blackfin.uclinux.org
On 03/08/10 14:47, Hennerich, Michael wrote:
> Hennerich, Michael wrote on 2010-03-08:
>> Jonathan Cameron wrote on 2010-03-08:
>>> On 03/08/10 11:38, Hennerich, Michael wrote:
>>>> Hi Jonathan,
>>>> Jonathan Cameron wrote on 2010-03-08:
>>>>> Hi Michael,
>>>>>
>>>>> I'm wondering about this one. Are you sure all platforms have the
>>>>> correct gpio setup when doing the request_irq? (those that need it
>>>>> obviously, basically the question is whether all platforms default to
>>>>> having gpio's as inputs).
>>>>>
>>>>> If the do there is certainly a lot of redundant code about based on a
>>>>> quick bit of grepping. (loads of it in arm/mach-pxa for starters). In
>>>>> that particular case all gpio's are configured as inputs, but I'm not
>>>>> sure other platforms are so well behaved.
>>>>>
>>>>>
>>>>> Unless I'm completely wrong that means we have run into a rather
>>>>> general issue.
>>>>
>>>> By no means - you have to first configure a GPIO_IRQ as GPIO-Input and
>>>> then as IRQ. irq_type takes care of it. In your case you gave the
>>>> IRQF_TRIGGER_RISING flag with request_irq(). In case such a
>>>> IRQF_TRIGGER_XXX flag is present the common IRQ infrastructure will
>>>> call the associated irq_chips irq_type() function. This function is
>>>> supposed to take care for all of it.(GPIO input, setting the level,
>>>> edge and polarity)
>>>>
>>>> As an alternative you can request_irq() without the
>>>> IRQF_TRIGGER_XXX
>>> flags and use set_irq_type() anytime later, or even in advance.
>>> Ah fair enough. Thanks for cleaning that up for me.
>>>
>>> Acked-by: Jonathan Cameron <jic23@cam.ac.uk>
>>>
>>> Please send it on to Greg KH
>>
>> Shall I change the driver to loop on while(irq_res != NULL)?
>> And get the irqflags from the platform data struct resource?
>
> What do you think about this one?
>
> Example Platform Data:
>
> #if defined(CONFIG_IIO_GPIO_TRIGGER) || \
> defined(CONFIG_IIO_GPIO_TRIGGER_MODULE)
>
> static struct resource iio_gpio_trigger_resources[] = {
> [0] = {
> .start = IRQ_PF5,
> .end = IRQ_PF5,
> .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE,
> },
> [1] = {
> .start = IRQ_PF2,
> .end = IRQ_PF3,
> .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_LOWEDGE,
> },
> };
>
> static struct platform_device iio_gpio_trigger = {
> .name = "iio_gpio_trigger",
> .num_resources = ARRAY_SIZE(iio_gpio_trigger_resources),
> .resource = iio_gpio_trigger_resources,
> };
> #endif
That seems sensible.
>
> This way allows you to specify different GPIO Edge/Polarity behavior as well as GPIO-IRQ sequences.
> Patch below is an top of my previous one.
> I wonder I we should change all references to gpio completely (replace with irq), since this driver can also be used with any other system IRQ as well.
Hmm.. Could do I guess. The intention was always to fill out the driver
so one could control the rising, falling choices from userspace, but
as we can do that with an interrupt that works just as well.
Would clean things up.
The patch below looks good to me.
Jonathan
>
>
> ===================================================================
> --- drivers/staging/iio/trigger/iio-trig-gpio.c (revision 8415)
> +++ drivers/staging/iio/trigger/iio-trig-gpio.c (working copy)
> @@ -57,64 +57,73 @@
> .attrs = iio_gpio_trigger_attrs,
> };
>
> -static int iio_gpio_trigger_probe(struct platform_device *dev)
> +static int iio_gpio_trigger_probe(struct platform_device *pdev)
> {
> - int *pdata = dev->dev.platform_data;
> + int *pdata = pdev->dev.platform_data;
> struct iio_gpio_trigger_info *trig_info;
> struct iio_trigger *trig, *trig2;
> - int i, irq, ret = 0;
> - if (!pdata) {
> - printk(KERN_ERR "No IIO gpio trigger platform data found\n");
> - goto error_ret;
> - }
> - for (i = 0;; i++) {
> - if (!gpio_is_valid(pdata[i]))
> + unsigned long irqflags;
> + struct resource *irq_res;
> + int irq, ret = 0, irq_res_cnt = 0;
> + unsigned gpio;
> +
> + 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;
> - trig = iio_allocate_trigger();
> - if (!trig) {
> - ret = -ENOMEM;
> - goto error_free_completed_registrations;
> }
> + irqflags = irq_res->flags & IRQF_TRIGGER_MASK;
>
> - trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
> - if (!trig_info) {
> - ret = -ENOMEM;
> - goto error_put_trigger;
> - }
> - trig->control_attrs = &iio_gpio_trigger_attr_group;
> - trig->private_data = trig_info;
> - trig_info->gpio = pdata[i];
> - trig->owner = THIS_MODULE;
> - trig->name = kmalloc(IIO_TRIGGER_NAME_LENGTH, GFP_KERNEL);
> - if (!trig->name) {
> - ret = -ENOMEM;
> - goto error_free_trig_info;
> - }
> - snprintf((char *)trig->name,
> - IIO_TRIGGER_NAME_LENGTH,
> - "gpiotrig%d",
> - pdata[i]);
> + for (irq = irq_res->start; irq <= irq_res->end; irq++) {
> + gpio = irq_to_gpio(irq);
> + if (!gpio_is_valid(gpio))
> + break;
> + trig = iio_allocate_trigger();
> + if (!trig) {
> + ret = -ENOMEM;
> + goto error_free_completed_registrations;
> + }
>
> - irq = gpio_to_irq(trig_info->gpio);
> - if (irq < 0) {
> - ret = irq;
> - goto error_free_name;
> + trig_info = kzalloc(sizeof(*trig_info), GFP_KERNEL);
> + if (!trig_info) {
> + ret = -ENOMEM;
> + goto error_put_trigger;
> + }
> + trig->control_attrs = &iio_gpio_trigger_attr_group;
> + trig->private_data = trig_info;
> + trig_info->gpio = gpio;
> + trig->owner = THIS_MODULE;
> + trig->name = kmalloc(IIO_TRIGGER_NAME_LENGTH, GFP_KERNEL);
> + if (!trig->name) {
> + ret = -ENOMEM;
> + goto error_free_trig_info;
> + }
> + snprintf((char *)trig->name,
> + IIO_TRIGGER_NAME_LENGTH,
> + "gpiotrig%d",
> + gpio);
> +
> + ret = request_irq(irq, iio_gpio_trigger_poll,
> + irqflags,
> + trig->name,
> + trig);
> + if (ret)
> + goto error_free_name;
> +
> + ret = iio_trigger_register(trig);
> + if (ret)
> + goto error_release_irq;
> +
> + list_add_tail(&trig->alloc_list, &iio_gpio_trigger_list);
> }
>
> - ret = request_irq(irq, iio_gpio_trigger_poll,
> - IRQF_TRIGGER_RISING,
> - trig->name,
> - trig);
> - if (ret)
> - goto error_free_name;
> + irq_res_cnt++;
> + } while (irq_res != NULL);
>
> - ret = iio_trigger_register(trig);
> - if (ret)
> - goto error_release_irq;
>
> - list_add_tail(&trig->alloc_list, &iio_gpio_trigger_list);
> -
> - }
> return 0;
>
> /* First clean up the partly allocated trigger */
> @@ -143,7 +152,7 @@
> return ret;
> }
>
> -static int iio_gpio_trigger_remove(struct platform_device *dev)
> +static int iio_gpio_trigger_remove(struct platform_device *pdev)
> {
> struct iio_trigger *trig, *trig2;
> struct iio_gpio_trigger_info *trig_info;
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-04-26 8:36 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-26 8:36 [PATCH] iio-trig-gpio:Remove redundant gpio_request michael.hennerich
[not found] <544AC56F16B56944AEC3BD4E3D5917712D6B1D9767@LIMKCMBX1.ad.analog.com>
2010-04-22 23:37 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2010-03-09 9:35 Hennerich, Michael
2010-03-09 12:45 ` Jonathan Cameron
2010-03-08 9:31 Hennerich, Michael
2010-03-08 10:58 ` Jonathan Cameron
2010-03-08 11:38 ` Hennerich, Michael
2010-03-08 13:12 ` Jonathan Cameron
2010-03-08 13:45 ` Hennerich, Michael
2010-03-08 14:47 ` Hennerich, Michael
2010-03-08 15:41 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).