From 1d4ba593d02e8ec41c2baaabdce9f62b71062dc7 Mon Sep 17 00:00:00 2001 From: Yoshihiro Shimoda Date: Thu, 8 Aug 2019 15:19:01 +0900 Subject: [PATCH 1/7] pinctrl: sh-pfc: Add new flags into struct sh_pfc_pin_config To clean/modify the code up later, this patch just adds new flags "mux_set" and "gpio_enabled" into the struct sh_pfc_pin_config. Signed-off-by: Yoshihiro Shimoda Reviewed-by: Simon Horman Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/sh-pfc/pinctrl.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c index 2824be4eb887d..864da24394d6f 100644 --- a/drivers/pinctrl/sh-pfc/pinctrl.c +++ b/drivers/pinctrl/sh-pfc/pinctrl.c @@ -27,6 +27,8 @@ struct sh_pfc_pin_config { u32 type; + bool mux_set; + bool gpio_enabled; }; struct sh_pfc_pinctrl { @@ -364,7 +366,15 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector, for (i = 0; i < grp->nr_pins; ++i) { ret = sh_pfc_config_mux(pfc, grp->mux[i], PINMUX_TYPE_FUNCTION); if (ret < 0) - break; + goto done; + } + + /* All group pins are configured, mark the pins as mux_set */ + for (i = 0; i < grp->nr_pins; ++i) { + int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]); + struct sh_pfc_pin_config *cfg = &pmx->configs[idx]; + + cfg->mux_set = true; } done: @@ -405,6 +415,7 @@ static int sh_pfc_gpio_request_enable(struct pinctrl_dev *pctldev, } cfg->type = PINMUX_TYPE_GPIO; + cfg->gpio_enabled = true; ret = 0; @@ -426,6 +437,7 @@ static void sh_pfc_gpio_disable_free(struct pinctrl_dev *pctldev, spin_lock_irqsave(&pfc->lock, flags); cfg->type = PINMUX_TYPE_NONE; + cfg->gpio_enabled = false; spin_unlock_irqrestore(&pfc->lock, flags); } From b13431ed6eab808affbd796a7c8caf05c7ae4cdd Mon Sep 17 00:00:00 2001 From: Yoshihiro Shimoda Date: Thu, 8 Aug 2019 15:19:02 +0900 Subject: [PATCH 2/7] pinctrl: sh-pfc: Remove incomplete flag "cfg->type" The old commit c58d9c1b26e3 ("sh-pfc: Implement generic pinconf support") broke the cfg->type flag to PINMUX_TYPE_FUNCTION because sh_pfc_pinconf_set() didn't call sh_pfc_reconfig_pin(). Now if we fix the cfg->type condition, it gets worse because: - Some drivers might be deferred so that .set_mux() will be called multiple times. - In such the case, the sh-pfc driver returns -EBUSY even if the group is the same, and then that driver fails to probe. Since the pinctrl subsystem already has such conditions according to @set_mux and @gpio_request_enable, this patch just remove the incomplete flag from sh-pfc/pinctrl.c. Fixes: c58d9c1b26e3 ("sh-pfc: Implement generic pinconf support") Signed-off-by: Yoshihiro Shimoda Reviewed-by: Simon Horman Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/sh-pfc/pinctrl.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c index 864da24394d6f..ab2aa93dd73b7 100644 --- a/drivers/pinctrl/sh-pfc/pinctrl.c +++ b/drivers/pinctrl/sh-pfc/pinctrl.c @@ -26,7 +26,6 @@ #include "../pinconf.h" struct sh_pfc_pin_config { - u32 type; bool mux_set; bool gpio_enabled; }; @@ -353,16 +352,6 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector, spin_lock_irqsave(&pfc->lock, flags); - for (i = 0; i < grp->nr_pins; ++i) { - int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]); - struct sh_pfc_pin_config *cfg = &pmx->configs[idx]; - - if (cfg->type != PINMUX_TYPE_NONE) { - ret = -EBUSY; - goto done; - } - } - for (i = 0; i < grp->nr_pins; ++i) { ret = sh_pfc_config_mux(pfc, grp->mux[i], PINMUX_TYPE_FUNCTION); if (ret < 0) @@ -395,14 +384,6 @@ static int sh_pfc_gpio_request_enable(struct pinctrl_dev *pctldev, spin_lock_irqsave(&pfc->lock, flags); - if (cfg->type != PINMUX_TYPE_NONE) { - dev_err(pfc->dev, - "Pin %u is busy, can't configure it as GPIO.\n", - offset); - ret = -EBUSY; - goto done; - } - if (!pfc->gpio) { /* If GPIOs are handled externally the pin mux type need to be * set to GPIO here. @@ -414,7 +395,6 @@ static int sh_pfc_gpio_request_enable(struct pinctrl_dev *pctldev, goto done; } - cfg->type = PINMUX_TYPE_GPIO; cfg->gpio_enabled = true; ret = 0; @@ -436,7 +416,6 @@ static void sh_pfc_gpio_disable_free(struct pinctrl_dev *pctldev, unsigned long flags; spin_lock_irqsave(&pfc->lock, flags); - cfg->type = PINMUX_TYPE_NONE; cfg->gpio_enabled = false; spin_unlock_irqrestore(&pfc->lock, flags); } @@ -450,7 +429,6 @@ static int sh_pfc_gpio_set_direction(struct pinctrl_dev *pctldev, int new_type = input ? PINMUX_TYPE_INPUT : PINMUX_TYPE_OUTPUT; int idx = sh_pfc_get_pin_index(pfc, offset); const struct sh_pfc_pin *pin = &pfc->info->pins[idx]; - struct sh_pfc_pin_config *cfg = &pmx->configs[idx]; unsigned long flags; unsigned int dir; int ret; @@ -470,8 +448,6 @@ static int sh_pfc_gpio_set_direction(struct pinctrl_dev *pctldev, if (ret < 0) goto done; - cfg->type = new_type; - done: spin_unlock_irqrestore(&pfc->lock, flags); return ret; @@ -794,13 +770,11 @@ static int sh_pfc_map_pins(struct sh_pfc *pfc, struct sh_pfc_pinctrl *pmx) for (i = 0; i < pfc->info->nr_pins; ++i) { const struct sh_pfc_pin *info = &pfc->info->pins[i]; - struct sh_pfc_pin_config *cfg = &pmx->configs[i]; struct pinctrl_pin_desc *pin = &pmx->pins[i]; /* If the pin number is equal to -1 all pins are considered */ pin->number = info->pin != (u16)-1 ? info->pin : i; pin->name = info->name; - cfg->type = PINMUX_TYPE_NONE; } return 0; From 8a0cc47ccc7c17826c669787f95ab7e5efb37444 Mon Sep 17 00:00:00 2001 From: Yoshihiro Shimoda Date: Thu, 8 Aug 2019 15:19:03 +0900 Subject: [PATCH 3/7] pinctrl: sh-pfc: Rollback to mux if required when the gpio is freed Some drivers require switching between function and gpio at run-time. Allow to roll back from gpio to mux when the gpio is freed. Signed-off-by: Yoshihiro Shimoda Reviewed-by: Simon Horman Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/sh-pfc/pinctrl.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/drivers/pinctrl/sh-pfc/pinctrl.c b/drivers/pinctrl/sh-pfc/pinctrl.c index ab2aa93dd73b7..99f4ebd698614 100644 --- a/drivers/pinctrl/sh-pfc/pinctrl.c +++ b/drivers/pinctrl/sh-pfc/pinctrl.c @@ -26,6 +26,7 @@ #include "../pinconf.h" struct sh_pfc_pin_config { + unsigned int mux_mark; bool mux_set; bool gpio_enabled; }; @@ -353,6 +354,16 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector, spin_lock_irqsave(&pfc->lock, flags); for (i = 0; i < grp->nr_pins; ++i) { + int idx = sh_pfc_get_pin_index(pfc, grp->pins[i]); + struct sh_pfc_pin_config *cfg = &pmx->configs[idx]; + + /* + * This driver cannot manage both gpio and mux when the gpio + * pin is already enabled. So, this function fails. + */ + if (cfg->gpio_enabled) + return -EBUSY; + ret = sh_pfc_config_mux(pfc, grp->mux[i], PINMUX_TYPE_FUNCTION); if (ret < 0) goto done; @@ -364,6 +375,7 @@ static int sh_pfc_func_set_mux(struct pinctrl_dev *pctldev, unsigned selector, struct sh_pfc_pin_config *cfg = &pmx->configs[idx]; cfg->mux_set = true; + cfg->mux_mark = grp->mux[i]; } done: @@ -417,6 +429,9 @@ static void sh_pfc_gpio_disable_free(struct pinctrl_dev *pctldev, spin_lock_irqsave(&pfc->lock, flags); cfg->gpio_enabled = false; + /* If mux is already set, this configures it here */ + if (cfg->mux_set) + sh_pfc_config_mux(pfc, cfg->mux_mark, PINMUX_TYPE_FUNCTION); spin_unlock_irqrestore(&pfc->lock, flags); } From df62267ddec54d93b6e280ba57052e7ecf3633f4 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Wed, 14 Aug 2019 09:20:32 +0200 Subject: [PATCH 4/7] pinctrl: sh-pfc: Include the right header This is a GPIO driver, use the appropriate header rather than the legacy header. Cc: Geert Uytterhoeven Signed-off-by: Linus Walleij Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/sh-pfc/gpio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pinctrl/sh-pfc/gpio.c b/drivers/pinctrl/sh-pfc/gpio.c index 64c09aa374ae0..5a55b8da79195 100644 --- a/drivers/pinctrl/sh-pfc/gpio.c +++ b/drivers/pinctrl/sh-pfc/gpio.c @@ -7,7 +7,7 @@ */ #include -#include +#include #include #include #include From ab6366ee887a273d2d33a204b7ea6e6fb2fe20e4 Mon Sep 17 00:00:00 2001 From: Nishka Dasgupta Date: Thu, 15 Aug 2019 11:35:03 +0530 Subject: [PATCH 5/7] pinctrl: rza1: Add of_node_put() before return Each iteration of for_each_child_of_node puts the previous node, but in the case of a return from the middle of the loop, there is no put, thus causing a memory leak. Hence add an of_node_put before the return in three places. Issue found with Coccinelle. Signed-off-by: Nishka Dasgupta Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/pinctrl-rza1.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c index 68aeefe291448..017fc6b3e27ec 100644 --- a/drivers/pinctrl/pinctrl-rza1.c +++ b/drivers/pinctrl/pinctrl-rza1.c @@ -866,8 +866,10 @@ static int rza1_dt_node_pin_count(struct device_node *np) npins = 0; for_each_child_of_node(np, child) { of_pins = of_find_property(child, "pinmux", NULL); - if (!of_pins) + if (!of_pins) { + of_node_put(child); return -EINVAL; + } npins += of_pins->length / sizeof(u32); } @@ -1025,8 +1027,10 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, for_each_child_of_node(np, child) { ret = rza1_parse_pinmux_node(rza1_pctl, child, mux_conf, grpin); - if (ret < 0) + if (ret < 0) { + of_node_put(child); return ret; + } grpin += ret; mux_conf += ret; @@ -1272,8 +1276,10 @@ static int rza1_gpio_register(struct rza1_pinctrl *rza1_pctl) ret = rza1_parse_gpiochip(rza1_pctl, child, &gpio_chips[i], &gpio_ranges[i]); - if (ret) + if (ret) { + of_node_put(child); return ret; + } ++i; } From 5b1d96e029bff720ca1067a1222f4b9387b5c214 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Tue, 20 Aug 2019 15:59:54 +0200 Subject: [PATCH 6/7] pinctrl: rza2: Drop driver use of consumer flags These flags are for consumers of GPIO lines, not for drivers. Cc: Chris Brandt Cc: Geert Uytterhoeven Signed-off-by: Linus Walleij Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/pinctrl-rza2.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/pinctrl/pinctrl-rza2.c b/drivers/pinctrl/pinctrl-rza2.c index 5b951c7422ccb..b0806667e94c6 100644 --- a/drivers/pinctrl/pinctrl-rza2.c +++ b/drivers/pinctrl/pinctrl-rza2.c @@ -115,7 +115,7 @@ static void rza2_pin_to_gpio(void __iomem *pfc_base, unsigned int offset, mask16 = RZA2_PDR_MASK << (pin * 2); reg16 &= ~mask16; - if (dir == GPIOF_DIR_IN) + if (dir) reg16 |= RZA2_PDR_INPUT << (pin * 2); /* pin as input */ else reg16 |= RZA2_PDR_OUTPUT << (pin * 2); /* pin as output */ @@ -134,18 +134,18 @@ static int rza2_chip_get_direction(struct gpio_chip *chip, unsigned int offset) reg16 = (reg16 >> (pin * 2)) & RZA2_PDR_MASK; if (reg16 == RZA2_PDR_OUTPUT) - return GPIOF_DIR_OUT; + return 0; if (reg16 == RZA2_PDR_INPUT) - return GPIOF_DIR_IN; + return 1; /* * This GPIO controller has a default Hi-Z state that is not input or * output, so force the pin to input now. */ - rza2_pin_to_gpio(priv->base, offset, GPIOF_DIR_IN); + rza2_pin_to_gpio(priv->base, offset, 1); - return GPIOF_DIR_IN; + return 1; } static int rza2_chip_direction_input(struct gpio_chip *chip, @@ -153,7 +153,7 @@ static int rza2_chip_direction_input(struct gpio_chip *chip, { struct rza2_pinctrl_priv *priv = gpiochip_get_data(chip); - rza2_pin_to_gpio(priv->base, offset, GPIOF_DIR_IN); + rza2_pin_to_gpio(priv->base, offset, 1); return 0; } @@ -191,7 +191,7 @@ static int rza2_chip_direction_output(struct gpio_chip *chip, struct rza2_pinctrl_priv *priv = gpiochip_get_data(chip); rza2_chip_set(chip, offset, val); - rza2_pin_to_gpio(priv->base, offset, GPIOF_DIR_OUT); + rza2_pin_to_gpio(priv->base, offset, 0); return 0; } From 0a6864274e4166cde21f26193350c7fcd9716ef5 Mon Sep 17 00:00:00 2001 From: Linus Walleij Date: Tue, 20 Aug 2019 15:59:55 +0200 Subject: [PATCH 7/7] pinctrl: rza2: Include the appropriate headers This driver is implementing a GPIO driver so include and not the legacy API . When testing it turns out it also relies on implicit inclusion of (readw etc) so make sure to include that as well. Cc: Chris Brandt Cc: Geert Uytterhoeven Signed-off-by: Linus Walleij Signed-off-by: Geert Uytterhoeven --- drivers/pinctrl/pinctrl-rza2.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/pinctrl/pinctrl-rza2.c b/drivers/pinctrl/pinctrl-rza2.c index b0806667e94c6..3be1d833bf25e 100644 --- a/drivers/pinctrl/pinctrl-rza2.c +++ b/drivers/pinctrl/pinctrl-rza2.c @@ -11,7 +11,8 @@ */ #include -#include +#include +#include #include #include #include