From 4aca036de50ff98dbc5b403b6020e3eb03411715 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Tue, 6 Nov 2012 16:03:35 +0100 Subject: [PATCH] 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 [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 Change-Id: Idbdd912d93757c3a2bada6e4ff701ef6dcea47da --- drivers/gpio/gpiolib.c | 8 +++++--- drivers/of/gpio.c | 23 +++++++++-------------- drivers/pinctrl/devicetree.c | 4 +--- include/asm-generic/gpio.h | 5 +++++ include/linux/gpio.h | 20 ++++++++++++++++---- include/linux/pinctrl/pinctrl.h | 7 ++----- 6 files changed, 38 insertions(+), 29 deletions(-) diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 20257ce5222..ee048dd71c9 100644 --- a/drivers/gpio/gpiolib.c +++ b/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) diff --git a/drivers/of/gpio.c b/drivers/of/gpio.c index 4fe1333b9fd..7b41fd8be57 100644 --- a/drivers/of/gpio.c +++ b/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++); } diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c index 6728ec71cb6..fe2d1af7cfa 100644 --- 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 a0eb53aefbd..03efb50cd55 100644 --- a/include/asm-generic/gpio.h +++ b/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 /** diff --git a/include/linux/gpio.h b/include/linux/gpio.h index 42cfa2e2bf8..eda50bd1511 100644 --- a/include/linux/gpio.h +++ b/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 */ diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h index 488a07d4d4b..e5481813851 100644 --- 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);