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@xxxxxxxxx>
>>
>> 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
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.
===================================================================
--- 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;
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
