| Message ID | 20171229122812.29743-1-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: omap: Give unique labels to each GPIO bank/chip | expand |
On Fri, Dec 29, 2017 at 01:28:12PM +0100, Linus Walleij wrote: > As we need to add GPIO lookup tables to the OMAP platforms, we > need to reference each GPIO chip with a unique label. Use the GPIO > base to name each chip, "gpio-0-31", "gpio-32-63" etc. Also fix > a small error on the error path of the probe() where the chip > would not be free()ed proberly under all circumstances. Since these two changes appear to be entirely unrelated, I suggest splitting this one up into two patches. > Cc: Grygorii Strashko <grygorii.strashko@ti.com> > Cc: Santosh Shilimkar <ssantosh@kernel.org> > Cc: Kevin Hilman <khilman@kernel.org> > Cc: linux-omap@vger.kernel.org > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/gpio/gpio-omap.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c > index e136d666f1e5..14fc3fb6f96e 100644 > --- a/drivers/gpio/gpio-omap.c > +++ b/drivers/gpio/gpio-omap.c > @@ -1059,6 +1059,7 @@ static void omap_gpio_mod_init(struct gpio_bank *bank) > static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > { > static int gpio; > + const char *label; > int irq_base = 0; > int ret; > > @@ -1080,7 +1081,11 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > bank->chip.parent = &omap_mpuio_device.dev; > bank->chip.base = OMAP_MPUIO(0); > } else { > - bank->chip.label = "gpio"; > + label = devm_kasprintf(bank->chip.parent, GFP_KERNEL, "gpio-%d-%d", > + gpio, gpio + bank->width - 1); > + if (!label) > + return -ENOMEM; > + bank->chip.label = label; > bank->chip.base = gpio; > } > bank->chip.ngpio = bank->width; > @@ -1104,7 +1109,8 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > -1, 0, bank->width, 0); > if (irq_base < 0) { > dev_err(bank->chip.parent, "Couldn't allocate IRQ numbers\n"); > - return -ENODEV; > + ret = -ENODEV; > + goto out_err_chip; > } > #endif > > @@ -1123,7 +1129,8 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > dev_err(bank->chip.parent, > "Couldn't add irqchip to gpiochip %d\n", ret); > gpiochip_remove(&bank->chip); > - return -ENODEV; > + ret = -ENODEV; > + goto out_err_chip; Here it looks like you are now introducing a double free (gpiochip_remove) instead. > } > > gpiochip_set_chained_irqchip(&bank->chip, irqc, bank->irq, NULL); > @@ -1132,8 +1139,12 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) > omap_gpio_irq_handler, > 0, dev_name(bank->chip.parent), bank); > if (ret) > - gpiochip_remove(&bank->chip); > + goto out_err_chip; > + > + return ret; I'd use "return 0" here for readability reasons. > > +out_err_chip: And name this label after what it does, for example, "err_remove_chip". > + gpiochip_remove(&bank->chip); > return ret; > } Johan -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c index e136d666f1e5..14fc3fb6f96e 100644 --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -1059,6 +1059,7 @@ static void omap_gpio_mod_init(struct gpio_bank *bank) static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) { static int gpio; + const char *label; int irq_base = 0; int ret; @@ -1080,7 +1081,11 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) bank->chip.parent = &omap_mpuio_device.dev; bank->chip.base = OMAP_MPUIO(0); } else { - bank->chip.label = "gpio"; + label = devm_kasprintf(bank->chip.parent, GFP_KERNEL, "gpio-%d-%d", + gpio, gpio + bank->width - 1); + if (!label) + return -ENOMEM; + bank->chip.label = label; bank->chip.base = gpio; } bank->chip.ngpio = bank->width; @@ -1104,7 +1109,8 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) -1, 0, bank->width, 0); if (irq_base < 0) { dev_err(bank->chip.parent, "Couldn't allocate IRQ numbers\n"); - return -ENODEV; + ret = -ENODEV; + goto out_err_chip; } #endif @@ -1123,7 +1129,8 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) dev_err(bank->chip.parent, "Couldn't add irqchip to gpiochip %d\n", ret); gpiochip_remove(&bank->chip); - return -ENODEV; + ret = -ENODEV; + goto out_err_chip; } gpiochip_set_chained_irqchip(&bank->chip, irqc, bank->irq, NULL); @@ -1132,8 +1139,12 @@ static int omap_gpio_chip_init(struct gpio_bank *bank, struct irq_chip *irqc) omap_gpio_irq_handler, 0, dev_name(bank->chip.parent), bank); if (ret) - gpiochip_remove(&bank->chip); + goto out_err_chip; + + return ret; +out_err_chip: + gpiochip_remove(&bank->chip); return ret; }
As we need to add GPIO lookup tables to the OMAP platforms, we need to reference each GPIO chip with a unique label. Use the GPIO base to name each chip, "gpio-0-31", "gpio-32-63" etc. Also fix a small error on the error path of the probe() where the chip would not be free()ed proberly under all circumstances. Cc: Grygorii Strashko <grygorii.strashko@ti.com> Cc: Santosh Shilimkar <ssantosh@kernel.org> Cc: Kevin Hilman <khilman@kernel.org> Cc: linux-omap@vger.kernel.org Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/gpio/gpio-omap.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) -- 2.14.3 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html