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