gpiolib: separation of pin concerns

The fact that of_gpiochip_add_pin_range() and
gpiochip_add_pin_range() share too much code is fragile and
will invariably mean that bugs need to be fixed in two places
instead of one.

So separate the concerns of gpiolib.c and gpiolib-of.c and
have the latter call the former as back-end. This is necessary
also when going forward with other device descriptions such
as ACPI.

This is done by:

- Adding a return code to gpiochip_add_pin_range() so we can
  reliably check whether this succeeds.

- Get rid of the custom of_pinctrl_add_gpio_range() from
  pinctrl. Instead create of_pinctrl_get() to just retrive the
  pin controller per se from an OF node. This composite
  function was just begging to be deleted, it was way to
  purpose-specific.

- Use pinctrl_dev_get_name() to get the name of the retrieved
  pin controller and use that to call back into the generic
  gpiochip_add_pin_range().

Now the pin range is only allocated and tied to a pin
controller from the core implementation in gpiolib.c.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
[took their side]
Git-commit: 1e63d7b9363f0c57d00991f9f2e0af374dfc591a
Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
Signed-off-by: Hanumant Singh <hanumant@codeaurora.org>

Change-Id: Idbdd912d93757c3a2bada6e4ff701ef6dcea47da

drivers/gpio/gpiolib.c

@@ -1183,8 +1183,8 @@ struct gpio_chip *gpiochip_find(const void *data,
 EXPORT_SYMBOL_GPL(gpiochip_find);
 
 #ifdef CONFIG_PINCTRL
-void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
-		unsigned int pin_base, unsigned int npins)
+int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
+			   unsigned int pin_base, unsigned int npins)
 {
 	struct gpio_pin_range *pin_range;
 
@@ -1192,7 +1192,7 @@ void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
 	if (!pin_range) {
 		pr_err("%s: GPIO chip: failed to allocate pin ranges\n",
 				chip->label);
-		return;
+		return -ENOMEM;
 	}
 
 	pin_range->range.name = chip->label;
@@ -1203,6 +1203,8 @@ void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
 			&pin_range->range);
 
 	list_add_tail(&pin_range->node, &chip->pin_ranges);
+
+	return 0;
 }
 
 void gpiochip_remove_pin_ranges(struct gpio_chip *chip)

drivers/of/gpio.c

@@ -211,8 +211,8 @@ EXPORT_SYMBOL(of_mm_gpiochip_add);
 void of_gpiochip_add_pin_range(struct gpio_chip *chip)
 {
 	struct device_node *np = chip->of_node;
-	struct gpio_pin_range *pin_range;
 	struct of_phandle_args pinspec;
+	struct pinctrl_dev *pctldev;
 	int index = 0, ret;
 
 	if (!np)
@@ -224,22 +224,17 @@ void of_gpiochip_add_pin_range(struct gpio_chip *chip)
 		if (ret)
 			break;
 
-		pin_range = devm_kzalloc(chip->dev, sizeof(*pin_range),
-				GFP_KERNEL);
-		if (!pin_range) {
-			pr_err("%s: GPIO chip: failed to allocate pin ranges\n",
-					chip->label);
+		pctldev = of_pinctrl_get(pinspec.np);
+		if (!pctldev)
 			break;
-		}
 
-		pin_range->range.name = chip->label;
-		pin_range->range.base = chip->base;
-		pin_range->range.pin_base = pinspec.args[0];
-		pin_range->range.npins = pinspec.args[1];
-		pin_range->pctldev = of_pinctrl_add_gpio_range(pinspec.np,
-				&pin_range->range);
+		ret = gpiochip_add_pin_range(chip,
+					     pinctrl_dev_get_name(pctldev),
+					     pinspec.args[0],
+					     pinspec.args[1]);
 
-		list_add_tail(&pin_range->node, &chip->pin_ranges);
+		if (ret)
+			break;
 
 	} while (index++);
 }

drivers/pinctrl/devicetree.c

@@ -106,8 +106,7 @@ static struct pinctrl_dev *find_pinctrl_by_of_node(struct device_node *np)
 	return NULL;
 }
 
-struct pinctrl_dev *of_pinctrl_add_gpio_range(struct device_node *np,
-		struct pinctrl_gpio_range *range)
+struct pinctrl_dev *of_pinctrl_get(struct device_node *np)
 {
 	struct pinctrl_dev *pctldev;
 
@@ -115,7 +114,6 @@ struct pinctrl_dev *of_pinctrl_add_gpio_range(struct device_node *np,
 	if (!pctldev)
 		return NULL;
 
-	pinctrl_add_gpio_range(pctldev, range);
 	return pctldev;
 }
 

include/asm-generic/gpio.h

@@ -61,6 +61,11 @@ struct gpio_pin_range {
 	struct pinctrl_dev *pctldev;
 	struct pinctrl_gpio_range range;
 };
+
+int gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
+			   unsigned int pin_base, unsigned int npins);
+void gpiochip_remove_pin_ranges(struct gpio_chip *chip);
+
 #endif
 
 /**

include/linux/gpio.h

@@ -172,9 +172,21 @@ static inline int irq_to_gpio(unsigned irq)
 	return -EINVAL;
 }
 
-void gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
-		unsigned int pin_base, unsigned int npins);
-void gpiochip_remove_pin_ranges(struct gpio_chip *chip);
-#endif
+#ifdef CONFIG_PINCTRL
+
+static inline int
+gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
+		       unsigned int pin_base, unsigned int npins)
+{
+}
+
+static inline void
+gpiochip_remove_pin_ranges(struct gpio_chip *chip)
+{
+}
+
+#endif /* CONFIG_PINCTRL */
+
+#endif /* ! CONFIG_GENERIC_GPIO */
 
 #endif /* __LINUX_GPIO_H */

include/linux/pinctrl/pinctrl.h

@@ -141,16 +141,13 @@ extern struct pinctrl_dev *find_pinctrl_and_add_gpio_range(const char *devname,
 		struct pinctrl_gpio_range *range);
 
 #ifdef CONFIG_OF
-extern struct pinctrl_dev *of_pinctrl_add_gpio_range(struct device_node *np,
-		struct pinctrl_gpio_range *range);
+extern struct pinctrl_dev *of_pinctrl_get(struct device_node *np);
 #else
 static inline
-struct pinctrl_dev *of_pinctrl_add_gpio_range(struct device_node *np,
-		struct pinctrl_gpio_range *range)
+struct pinctrl_dev *of_pinctrl_get(struct device_node *np)
 {
 	return NULL;
 }
-
 #endif /* CONFIG_OF */
 
 extern const char *pinctrl_dev_get_name(struct pinctrl_dev *pctldev);