lkml.org 
[lkml]   [2010]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] gpio: msm7200a: Add irq support to msm-gpiolib.
From
Date
On Fri, 2010-06-11 at 12:58 -0700, Gregory Bean wrote:

> static int msm_gpio_probe(struct platform_device *dev)
> {
> struct msm_gpio_dev *msm_gpio;
> struct msm7200a_gpio_platform_data *pdata =
> (struct msm7200a_gpio_platform_data *)dev->dev.platform_data;
> - int ret;
> + int i, irq, ret;
>
> if (!pdata)
> return -EINVAL;
> @@ -146,13 +306,53 @@ static int msm_gpio_probe(struct platform_device *dev)
> msm_gpio->gpio_chip.direction_output = gpio_chip_direction_output;
> msm_gpio->gpio_chip.get = gpio_chip_get;
> msm_gpio->gpio_chip.set = gpio_chip_set;
> + msm_gpio->gpio_chip.to_irq = gpio_chip_to_irq;
> + msm_gpio->irq_base = pdata->irq_base;
> + msm_gpio->irq_summary = pdata->irq_summary;
>
> ret = gpiochip_add(&msm_gpio->gpio_chip);
> if (ret < 0)
> - goto err;
> + goto err_post_malloc;
> +
> + for (i = 0; i < msm_gpio->gpio_chip.ngpio; ++i) {
> + irq = msm_gpio->irq_base + i;
> + set_irq_chip_data(irq, msm_gpio);
> + set_irq_chip(irq, &msm_gpio_irq_chip);
> + set_irq_handler(irq, handle_level_irq);
> + set_irq_flags(irq, IRQF_VALID);
> + }
> +
> + /*
> + * We use a level-triggered interrupt because of the nature
> + * of the shared GPIO-group interrupt.
> + *
> + * Many GPIO chips may be sharing the same group IRQ line, and
> + * it is possible for GPIO interrupt to re-occur while the system
> + * is still servicing the group interrupt associated with it.
> + * The group IRQ line would not de-assert and re-assert, and
> + * we'd get no second edge to cause the group IRQ to be handled again.
> + *
> + * Using a level interrupt guarantees that the group IRQ handlers
> + * will continue to be called as long as any GPIO chip in the group
> + * is asserting, even if the condition began while the group
> + * handler was in mid-pass.
> + */
> + ret = request_irq(msm_gpio->irq_summary,
> + msm_gpio_irq_handler,
> + IRQF_SHARED | IRQF_TRIGGER_HIGH,
> + dev->name,
> + msm_gpio);
> + if (ret < 0)
> + goto err_post_gpiochip_add;
>
> return ret;
> -err:
> +err_post_gpiochip_add:
> + /*
> + * Under no circumstances should a line be held on a gpiochip
> + * which hasn't finished probing.
> + */
> + BUG_ON(gpiochip_remove(&msm_gpio->gpio_chip) < 0);
> +err_post_malloc:

It looks like some of this should go in the prior patch. Like this
BUG_ON() above.

> kfree(msm_gpio);
> return ret;
> }
> @@ -160,12 +360,16 @@ err:
> static int msm_gpio_remove(struct platform_device *dev)
> {
> struct msm_gpio_dev *msm_gpio = platform_get_drvdata(dev);
> - int ret = gpiochip_remove(&msm_gpio->gpio_chip);
> + int ret;
>
> - if (ret == 0)
> - kfree(msm_gpio);
> + ret = gpiochip_remove(&msm_gpio->gpio_chip);
> + if (ret < 0)
> + return ret;
>
> - return ret;
> + free_irq(msm_gpio->irq_summary, msm_gpio);
> + kfree(msm_gpio);
> +
> + return 0;
> }

Also some of the code here (msm_gpio_remove) seems like it's cleaning up
the prior patch to some degree. So it should potentially get moved into
that patch.

Daniel



\
 
 \ /
  Last update: 2010-06-12 01:15    [W:0.076 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site