gpio: omap: Give unique labels to each GPIO bank/chip

Message ID 20171229122812.29743-1-linus.walleij@linaro.org
State New
Headers show
Series
  • gpio: omap: Give unique labels to each GPIO bank/chip
Related show

Commit Message

Linus Walleij Dec. 29, 2017, 12:28 p.m.
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

Comments

Johan Hovold Dec. 29, 2017, 2:03 p.m. | #1
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

Patch

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;
 }