Re: [PATCH 2/2] gpio: msm7200a: Add irq support to msm-gpiolib.

From: Greg Bean
Date: Fri Jun 11 2010 - 19:23:38 EST

+ /*
+ * 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);

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

I would argue against that. If you look at the previous patch, you'll see that the last thing that probe() does is gpiochip_add, which means it never has a need to call gpiochip_remove(), and thus no need to complain if gpiochip_remove behaves unexpectedly.

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

Point taken. There is definitely some stuff in here that is just "housecleaning" - I will sort that out.

Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at