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>

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 67403e47e4dc65e88650d476c7ca18d7605df1dd..a40cd84c5c107bb69c8bb07ae5ff4916ebf0e7f0 100644 (file)
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -221,8 +221,8 @@ EXPORT_SYMBOL(of_mm_gpiochip_add);
 static 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)
@@ -234,22 +234,17 @@ static 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++);
 }

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index bcf9b9914fb7176db78c4d5cc9f1d89469efcb42..c5f650095faafa9e3f1b1a71759671c411bb17c6 100644 (file)
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1187,8 +1187,8 @@ 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;
 
@@ -1196,7 +1196,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;
@@ -1207,6 +1207,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;
 }
 EXPORT_SYMBOL_GPL(gpiochip_add_pin_range);
 

diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
index 6728ec71cb6536b25e92c04e2b3237164fa0ec32..fe2d1af7cfa0d0bce2eeb8a8391a4567da70bdd8 100644 (file)
--- a/drivers/pinctrl/devicetree.c
+++ b/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;
 }
 

diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
index 2e60de4265ac08784f78a1528aa7867f8109e181..50d995e7e44a8d67dfcf690f45a6a9d851fb822c 100644 (file)
--- a/include/asm-generic/gpio.h
+++ b/include/asm-generic/gpio.h
@@ -63,8 +63,8 @@ struct gpio_pin_range {
        struct pinctrl_gpio_range range;
 };
 
-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);
 void gpiochip_remove_pin_ranges(struct gpio_chip *chip);
 
 #endif

diff --git a/include/linux/gpio.h b/include/linux/gpio.h
index 21d28b930dc7b95e87ac1a6bf7efd6fa24f5f3e5..81bbfe5b5de68bf732a6db5ad505e0f418a84f4d 100644 (file)
--- a/include/linux/gpio.h
+++ b/include/linux/gpio.h
@@ -233,7 +233,7 @@ static inline int irq_to_gpio(unsigned irq)
 
 #ifdef CONFIG_PINCTRL
 
-static inline void
+static inline int
 gpiochip_add_pin_range(struct gpio_chip *chip, const char *pinctl_name,
                       unsigned int pin_base, unsigned int npins)
 {

diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h
index 434e5a94e131ab5ee9b04024a0fcfa8979071da0..4a58428bc793bfad24bbf31797b388a71538ca62 100644 (file)
--- a/include/linux/pinctrl/pinctrl.h
+++ b/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);