* [PATCH 0/5] pinctrl fixes for generic functions and groups @ 2018-06-15 11:11 Tony Lindgren 2018-06-15 11:11 ` [PATCH 1/5] pinctrl: core: Return selector to the pinctrl driver Tony Lindgren ` (4 more replies) 0 siblings, 5 replies; 12+ messages in thread From: Tony Lindgren @ 2018-06-15 11:11 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, linux-kernel Hi, Here are fixes to the race issues for generic group and functions reported by H. Nikolaus Schaller <hns@goldelico.com>. I have not seen the issue here myself, so please test to see if this is sufficient. I've also fixed rza1 in addition to pinctrl-single. Please also check the drivers pinctrl-mt7622.c and pinctrl-ingenic.c if mutex fixes are needed there. Regards, Tony Tony Lindgren (5): pinctrl: core: Return selector to the pinctrl driver pinctrl: pinmux: Return selector to the pinctrl driver pinctrl: single: Fix group and function selector use pinctrl: rza1: Fix selector use for groups and functions pinctrl: core: Remove broken remove_last group and pinmux functions drivers/pinctrl/core.c | 5 +- drivers/pinctrl/core.h | 6 --- drivers/pinctrl/pinctrl-rza1.c | 25 ++++----- drivers/pinctrl/pinctrl-single.c | 91 +++++++++++++++++++------------- drivers/pinctrl/pinmux.c | 5 +- drivers/pinctrl/pinmux.h | 7 --- 6 files changed, 74 insertions(+), 65 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/5] pinctrl: core: Return selector to the pinctrl driver 2018-06-15 11:11 [PATCH 0/5] pinctrl fixes for generic functions and groups Tony Lindgren @ 2018-06-15 11:11 ` Tony Lindgren 2018-06-17 16:50 ` Andy Shevchenko 2018-06-15 11:11 ` [PATCH 2/5] pinctrl: pinmux: " Tony Lindgren ` (3 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Tony Lindgren @ 2018-06-15 11:11 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-kernel, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, Sean Wang We must return the selector from pinctrl_generic_add_group() so pin controller device drivers can remove the right group if needed for deferred probe for example. Note that fixes are also needed for the pin controller drivers to use the selector value. Fixes: c7059c5ac70a ("pinctrl: core: Add generic pinctrl functions for managing groups") Reported-by: H. Nikolaus Schaller <hns@goldelico.com> Cc: Haojian Zhuang <haojian.zhuang@linaro.org> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> Cc: Paul Cercueil <paul@crapouillou.net> Cc: Sean Wang <sean.wang@mediatek.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/pinctrl/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -631,6 +631,7 @@ int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, int *pins, int num_pins, void *data) { struct group_desc *group; + int selector = pctldev->num_groups; group = devm_kzalloc(pctldev->dev, sizeof(*group), GFP_KERNEL); if (!group) @@ -641,12 +642,12 @@ int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, group->num_pins = num_pins; group->data = data; - radix_tree_insert(&pctldev->pin_group_tree, pctldev->num_groups, + radix_tree_insert(&pctldev->pin_group_tree, selector, group); pctldev->num_groups++; - return 0; + return selector; } EXPORT_SYMBOL_GPL(pinctrl_generic_add_group); -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/5] pinctrl: core: Return selector to the pinctrl driver 2018-06-15 11:11 ` [PATCH 1/5] pinctrl: core: Return selector to the pinctrl driver Tony Lindgren @ 2018-06-17 16:50 ` Andy Shevchenko 2018-06-18 6:48 ` Tony Lindgren 0 siblings, 1 reply; 12+ messages in thread From: Andy Shevchenko @ 2018-06-17 16:50 UTC (permalink / raw) To: Tony Lindgren Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, sean.wang On Fri, Jun 15, 2018 at 2:14 PM Tony Lindgren <tony@atomide.com> wrote: > > We must return the selector from pinctrl_generic_add_group() so > pin controller device drivers can remove the right group if needed > for deferred probe for example. > > Note that fixes are also needed for the pin controller drivers to > use the selector value. > > Fixes: c7059c5ac70a ("pinctrl: core: Add generic pinctrl functions > for managing groups") > Reported-by: H. Nikolaus Schaller <hns@goldelico.com> > Cc: Haojian Zhuang <haojian.zhuang@linaro.org> > Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> > Cc: Paul Cercueil <paul@crapouillou.net> > Cc: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/pinctrl/core.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c > --- a/drivers/pinctrl/core.c > +++ b/drivers/pinctrl/core.c > @@ -631,6 +631,7 @@ int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, > int *pins, int num_pins, void *data) > { > struct group_desc *group; > + int selector = pctldev->num_groups; > > group = devm_kzalloc(pctldev->dev, sizeof(*group), GFP_KERNEL); > if (!group) > @@ -641,12 +642,12 @@ int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, > group->num_pins = num_pins; > group->data = data; > > - radix_tree_insert(&pctldev->pin_group_tree, pctldev->num_groups, > + radix_tree_insert(&pctldev->pin_group_tree, selector, > group); A nit: Can it be now one line? > > pctldev->num_groups++; > > - return 0; > + return selector; > } > EXPORT_SYMBOL_GPL(pinctrl_generic_add_group); > > -- > 2.17.1 -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/5] pinctrl: core: Return selector to the pinctrl driver 2018-06-17 16:50 ` Andy Shevchenko @ 2018-06-18 6:48 ` Tony Lindgren 0 siblings, 0 replies; 12+ messages in thread From: Tony Lindgren @ 2018-06-18 6:48 UTC (permalink / raw) To: Andy Shevchenko Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, sean.wang * Andy Shevchenko <andy.shevchenko@gmail.com> [180617 16:53]: > On Fri, Jun 15, 2018 at 2:14 PM Tony Lindgren <tony@atomide.com> wrote: > > @@ -641,12 +642,12 @@ int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, > > group->num_pins = num_pins; > > group->data = data; > > > > - radix_tree_insert(&pctldev->pin_group_tree, pctldev->num_groups, > > + radix_tree_insert(&pctldev->pin_group_tree, selector, > > group); > > A nit: Can it be now one line? Sure will update and post an updated series of patches. It probably makes sense to wait a bit with these as there are still issues remaining for deferred probe. Regards, Tony ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/5] pinctrl: pinmux: Return selector to the pinctrl driver 2018-06-15 11:11 [PATCH 0/5] pinctrl fixes for generic functions and groups Tony Lindgren 2018-06-15 11:11 ` [PATCH 1/5] pinctrl: core: Return selector to the pinctrl driver Tony Lindgren @ 2018-06-15 11:11 ` Tony Lindgren 2018-06-17 16:51 ` Andy Shevchenko 2018-06-15 11:11 ` [PATCH 3/5] pinctrl: single: Fix group and function selector use Tony Lindgren ` (2 subsequent siblings) 4 siblings, 1 reply; 12+ messages in thread From: Tony Lindgren @ 2018-06-15 11:11 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-kernel, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, Sean Wang We must return the selector from pinmux_generic_add_function() so pin controller device drivers can remove the right group if needed for deferred probe for example. Note that fixes are also needed for the pin controller drivers to use the selector value. Fixes: a76edc89b100 ("pinctrl: core: Add generic pinctrl functions for managing groups") Reported-by: H. Nikolaus Schaller <hns@goldelico.com> Cc: Haojian Zhuang <haojian.zhuang@linaro.org> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> Cc: Paul Cercueil <paul@crapouillou.net> Cc: Sean Wang <sean.wang@mediatek.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/pinctrl/pinmux.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c --- a/drivers/pinctrl/pinmux.c +++ b/drivers/pinctrl/pinmux.c @@ -775,6 +775,7 @@ int pinmux_generic_add_function(struct pinctrl_dev *pctldev, void *data) { struct function_desc *function; + int selector = pctldev->num_functions; function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL); if (!function) @@ -785,12 +786,12 @@ int pinmux_generic_add_function(struct pinctrl_dev *pctldev, function->num_group_names = num_groups; function->data = data; - radix_tree_insert(&pctldev->pin_function_tree, pctldev->num_functions, + radix_tree_insert(&pctldev->pin_function_tree, selector, function); pctldev->num_functions++; - return 0; + return selector; } EXPORT_SYMBOL_GPL(pinmux_generic_add_function); -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/5] pinctrl: pinmux: Return selector to the pinctrl driver 2018-06-15 11:11 ` [PATCH 2/5] pinctrl: pinmux: " Tony Lindgren @ 2018-06-17 16:51 ` Andy Shevchenko 0 siblings, 0 replies; 12+ messages in thread From: Andy Shevchenko @ 2018-06-17 16:51 UTC (permalink / raw) To: Tony Lindgren Cc: Linus Walleij, open list:GPIO SUBSYSTEM, Linux Kernel Mailing List, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, sean.wang On Fri, Jun 15, 2018 at 2:15 PM Tony Lindgren <tony@atomide.com> wrote: > > We must return the selector from pinmux_generic_add_function() so > pin controller device drivers can remove the right group if needed > for deferred probe for example. > > Note that fixes are also needed for the pin controller drivers to > use the selector value. > > Fixes: a76edc89b100 ("pinctrl: core: Add generic pinctrl functions for > managing groups") > Reported-by: H. Nikolaus Schaller <hns@goldelico.com> > Cc: Haojian Zhuang <haojian.zhuang@linaro.org> > Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> > Cc: Paul Cercueil <paul@crapouillou.net> > Cc: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/pinctrl/pinmux.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c > --- a/drivers/pinctrl/pinmux.c > +++ b/drivers/pinctrl/pinmux.c > @@ -775,6 +775,7 @@ int pinmux_generic_add_function(struct pinctrl_dev *pctldev, > void *data) > { > struct function_desc *function; > + int selector = pctldev->num_functions; > > function = devm_kzalloc(pctldev->dev, sizeof(*function), GFP_KERNEL); > if (!function) > @@ -785,12 +786,12 @@ int pinmux_generic_add_function(struct pinctrl_dev *pctldev, > function->num_group_names = num_groups; > function->data = data; > > - radix_tree_insert(&pctldev->pin_function_tree, pctldev->num_functions, > + radix_tree_insert(&pctldev->pin_function_tree, selector, > function); Same nit: One line now? > > pctldev->num_functions++; > > - return 0; > + return selector; > } > EXPORT_SYMBOL_GPL(pinmux_generic_add_function); > > -- > 2.17.1 -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 3/5] pinctrl: single: Fix group and function selector use 2018-06-15 11:11 [PATCH 0/5] pinctrl fixes for generic functions and groups Tony Lindgren 2018-06-15 11:11 ` [PATCH 1/5] pinctrl: core: Return selector to the pinctrl driver Tony Lindgren 2018-06-15 11:11 ` [PATCH 2/5] pinctrl: pinmux: " Tony Lindgren @ 2018-06-15 11:11 ` Tony Lindgren 2018-06-15 11:11 ` [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions Tony Lindgren 2018-06-15 11:11 ` [PATCH 5/5] pinctrl: core: Remove broken remove_last group and pinmux functions Tony Lindgren 4 siblings, 0 replies; 12+ messages in thread From: Tony Lindgren @ 2018-06-15 11:11 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-kernel, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, Sean Wang We must use a mutex around the generic_add functions and save the function and group selector in case we need to remove them. Otherwise the selector use will be racy for deferred probe at least. Note that struct device_node *np is unused in pcs_add_function() we remove that too and fix a checkpatch warning for bare unsigned while at it. Fixes: 571aec4df5b7 ("pinctrl: single: Use generic pinmux helpers for managing functions") Reported-by: H. Nikolaus Schaller <hns@goldelico.com> Cc: Haojian Zhuang <haojian.zhuang@linaro.org> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> Cc: Paul Cercueil <paul@crapouillou.net> Cc: Sean Wang <sean.wang@mediatek.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/pinctrl/pinctrl-single.c | 91 +++++++++++++++++++------------- 1 file changed, 55 insertions(+), 36 deletions(-) diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -747,38 +747,44 @@ static int pcs_allocate_pin_table(struct pcs_device *pcs) /** * pcs_add_function() - adds a new function to the function list * @pcs: pcs driver instance - * @np: device node of the mux entry + * @fcn: new function allocated * @name: name of the function * @vals: array of mux register value pairs used by the function * @nvals: number of mux register value pairs * @pgnames: array of pingroup names for the function * @npgnames: number of pingroup names + * + * Caller must take care of locking. */ -static struct pcs_function *pcs_add_function(struct pcs_device *pcs, - struct device_node *np, - const char *name, - struct pcs_func_vals *vals, - unsigned nvals, - const char **pgnames, - unsigned npgnames) +static int pcs_add_function(struct pcs_device *pcs, + struct pcs_function **fcn, + const char *name, + struct pcs_func_vals *vals, + unsigned int nvals, + const char **pgnames, + unsigned int npgnames) { struct pcs_function *function; - int res; + int selector; function = devm_kzalloc(pcs->dev, sizeof(*function), GFP_KERNEL); if (!function) - return NULL; + return -ENOMEM; function->vals = vals; function->nvals = nvals; - res = pinmux_generic_add_function(pcs->pctl, name, - pgnames, npgnames, - function); - if (res) - return NULL; + selector = pinmux_generic_add_function(pcs->pctl, name, + pgnames, npgnames, + function); + if (selector < 0) { + devm_kfree(pcs->dev, function); + *fcn = NULL; + } else { + *fcn = function; + } - return function; + return selector; } /** @@ -979,8 +985,8 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs, { const char *name = "pinctrl-single,pins"; struct pcs_func_vals *vals; - int rows, *pins, found = 0, res = -ENOMEM, i; - struct pcs_function *function; + int rows, *pins, found = 0, res = -ENOMEM, i, fsel, gsel; + struct pcs_function *function = NULL; rows = pinctrl_count_index_with_args(np, name); if (rows <= 0) { @@ -1030,21 +1036,25 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs, } pgnames[0] = np->name; - function = pcs_add_function(pcs, np, np->name, vals, found, pgnames, 1); - if (!function) { - res = -ENOMEM; + mutex_lock(&pcs->mutex); + fsel = pcs_add_function(pcs, &function, np->name, vals, found, + pgnames, 1); + if (fsel < 0) { + res = fsel; goto free_pins; } - res = pinctrl_generic_add_group(pcs->pctl, np->name, pins, found, pcs); - if (res < 0) + gsel = pinctrl_generic_add_group(pcs->pctl, np->name, pins, found, pcs); + if (gsel < 0) { + res = gsel; goto free_function; + } (*map)->type = PIN_MAP_TYPE_MUX_GROUP; (*map)->data.mux.group = np->name; (*map)->data.mux.function = np->name; - if (PCS_HAS_PINCONF) { + if (PCS_HAS_PINCONF && function) { res = pcs_parse_pinconf(pcs, np, function, map); if (res) goto free_pingroups; @@ -1052,14 +1062,16 @@ static int pcs_parse_one_pinctrl_entry(struct pcs_device *pcs, } else { *num_maps = 1; } + mutex_unlock(&pcs->mutex); + return 0; free_pingroups: - pinctrl_generic_remove_last_group(pcs->pctl); + pinctrl_generic_remove_group(pcs->pctl, gsel); *num_maps = 1; free_function: - pinmux_generic_remove_last_function(pcs->pctl); - + pinmux_generic_remove_function(pcs->pctl, fsel); + mutex_unlock(&pcs->mutex); free_pins: devm_kfree(pcs->dev, pins); @@ -1077,9 +1089,9 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs, { const char *name = "pinctrl-single,bits"; struct pcs_func_vals *vals; - int rows, *pins, found = 0, res = -ENOMEM, i; + int rows, *pins, found = 0, res = -ENOMEM, i, fsel, gsel; int npins_in_row; - struct pcs_function *function; + struct pcs_function *function = NULL; rows = pinctrl_count_index_with_args(np, name); if (rows <= 0) { @@ -1166,15 +1178,19 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs, } pgnames[0] = np->name; - function = pcs_add_function(pcs, np, np->name, vals, found, pgnames, 1); - if (!function) { - res = -ENOMEM; + mutex_lock(&pcs->mutex); + fsel = pcs_add_function(pcs, &function, np->name, vals, found, + pgnames, 1); + if (fsel < 0) { + res = fsel; goto free_pins; } - res = pinctrl_generic_add_group(pcs->pctl, np->name, pins, found, pcs); - if (res < 0) + gsel = pinctrl_generic_add_group(pcs->pctl, np->name, pins, found, pcs); + if (gsel < 0) { + res = gsel; goto free_function; + } (*map)->type = PIN_MAP_TYPE_MUX_GROUP; (*map)->data.mux.group = np->name; @@ -1186,13 +1202,16 @@ static int pcs_parse_bits_in_pinctrl_entry(struct pcs_device *pcs, } *num_maps = 1; + mutex_unlock(&pcs->mutex); + return 0; free_pingroups: - pinctrl_generic_remove_last_group(pcs->pctl); + pinctrl_generic_remove_group(pcs->pctl, gsel); *num_maps = 1; free_function: - pinmux_generic_remove_last_function(pcs->pctl); + pinmux_generic_remove_function(pcs->pctl, fsel); + mutex_unlock(&pcs->mutex); free_pins: devm_kfree(pcs->dev, pins); -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions 2018-06-15 11:11 [PATCH 0/5] pinctrl fixes for generic functions and groups Tony Lindgren ` (2 preceding siblings ...) 2018-06-15 11:11 ` [PATCH 3/5] pinctrl: single: Fix group and function selector use Tony Lindgren @ 2018-06-15 11:11 ` Tony Lindgren 2018-06-19 4:16 ` jacopo mondi 2018-06-15 11:11 ` [PATCH 5/5] pinctrl: core: Remove broken remove_last group and pinmux functions Tony Lindgren 4 siblings, 1 reply; 12+ messages in thread From: Tony Lindgren @ 2018-06-15 11:11 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-kernel, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, Sean Wang We must use a mutex around the generic_add functions and save the function and group selector in case we need to remove them. Otherwise the selector use will be racy for deferred probe at least. Fixes: 5a49b644b307 ("pinctrl: Renesas RZ/A1 pin and gpio controller") Reported-by: H. Nikolaus Schaller <hns@goldelico.com> Cc: Haojian Zhuang <haojian.zhuang@linaro.org> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> Cc: Paul Cercueil <paul@crapouillou.net> Cc: Sean Wang <sean.wang@mediatek.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/pinctrl/pinctrl-rza1.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c --- a/drivers/pinctrl/pinctrl-rza1.c +++ b/drivers/pinctrl/pinctrl-rza1.c @@ -1005,7 +1005,7 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, struct device_node *child; const char *grpname; const char **fngrps; - int ret, npins; + int ret, npins, gsel, fsel; npins = rza1_dt_node_pin_count(np); if (npins < 0) { @@ -1055,18 +1055,19 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, fngrps[0] = grpname; mutex_lock(&rza1_pctl->mutex); - ret = pinctrl_generic_add_group(pctldev, grpname, grpins, npins, - NULL); - if (ret) { + gsel = pinctrl_generic_add_group(pctldev, grpname, grpins, npins, + NULL); + if (gsel < 0) { mutex_unlock(&rza1_pctl->mutex); - return ret; + return gsel; } - ret = pinmux_generic_add_function(pctldev, grpname, fngrps, 1, - mux_confs); - if (ret) + fsel = pinmux_generic_add_function(pctldev, grpname, fngrps, 1, + mux_confs); + if (fsel < 0) { + ret = fsel; goto remove_group; - mutex_unlock(&rza1_pctl->mutex); + } dev_info(rza1_pctl->dev, "Parsed function and group %s with %d pins\n", grpname, npins); @@ -1083,15 +1084,15 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, (*map)->data.mux.group = np->name; (*map)->data.mux.function = np->name; *num_maps = 1; + mutex_unlock(&rza1_pctl->mutex); return 0; remove_function: - mutex_lock(&rza1_pctl->mutex); - pinmux_generic_remove_last_function(pctldev); + pinmux_generic_remove_function(pctldev, fsel); remove_group: - pinctrl_generic_remove_last_group(pctldev); + pinctrl_generic_remove_group(pctldev, gsel); mutex_unlock(&rza1_pctl->mutex); dev_info(rza1_pctl->dev, "Unable to parse function and group %s\n", -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions 2018-06-15 11:11 ` [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions Tony Lindgren @ 2018-06-19 4:16 ` jacopo mondi 0 siblings, 0 replies; 12+ messages in thread From: jacopo mondi @ 2018-06-19 4:16 UTC (permalink / raw) To: Tony Lindgren Cc: Linus Walleij, linux-gpio, linux-kernel, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, Sean Wang [-- Attachment #1: Type: text/plain, Size: 3029 bytes --] Hi Tony, thanks for doing this! I cannot find the original bug report in linux-gpio, so I'll comment on this single patch only. On Fri, Jun 15, 2018 at 04:11:40AM -0700, Tony Lindgren wrote: > We must use a mutex around the generic_add functions and save the > function and group selector in case we need to remove them. Otherwise > the selector use will be racy for deferred probe at least. > > Fixes: 5a49b644b307 ("pinctrl: Renesas RZ/A1 pin and gpio controller") > Reported-by: H. Nikolaus Schaller <hns@goldelico.com> > Cc: Haojian Zhuang <haojian.zhuang@linaro.org> > Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> > Cc: Paul Cercueil <paul@crapouillou.net> > Cc: Sean Wang <sean.wang@mediatek.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/pinctrl/pinctrl-rza1.c | 25 +++++++++++++------------ > 1 file changed, 13 insertions(+), 12 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c > --- a/drivers/pinctrl/pinctrl-rza1.c > +++ b/drivers/pinctrl/pinctrl-rza1.c > @@ -1005,7 +1005,7 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, > struct device_node *child; > const char *grpname; > const char **fngrps; > - int ret, npins; > + int ret, npins, gsel, fsel; Nit: can you move this 3 lines up to keep the variable declarations in reverse xmas tree order? or if you prefer declare one variable per line... With this minor comment fixed: Acked-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > npins = rza1_dt_node_pin_count(np); > if (npins < 0) { > @@ -1055,18 +1055,19 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, > fngrps[0] = grpname; > > mutex_lock(&rza1_pctl->mutex); > - ret = pinctrl_generic_add_group(pctldev, grpname, grpins, npins, > - NULL); > - if (ret) { > + gsel = pinctrl_generic_add_group(pctldev, grpname, grpins, npins, > + NULL); > + if (gsel < 0) { > mutex_unlock(&rza1_pctl->mutex); > - return ret; > + return gsel; > } > > - ret = pinmux_generic_add_function(pctldev, grpname, fngrps, 1, > - mux_confs); > - if (ret) > + fsel = pinmux_generic_add_function(pctldev, grpname, fngrps, 1, > + mux_confs); > + if (fsel < 0) { > + ret = fsel; > goto remove_group; > - mutex_unlock(&rza1_pctl->mutex); > + } > > dev_info(rza1_pctl->dev, "Parsed function and group %s with %d pins\n", > grpname, npins); > @@ -1083,15 +1084,15 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, > (*map)->data.mux.group = np->name; > (*map)->data.mux.function = np->name; > *num_maps = 1; > + mutex_unlock(&rza1_pctl->mutex); > > return 0; > > remove_function: > - mutex_lock(&rza1_pctl->mutex); > - pinmux_generic_remove_last_function(pctldev); > + pinmux_generic_remove_function(pctldev, fsel); > > remove_group: > - pinctrl_generic_remove_last_group(pctldev); > + pinctrl_generic_remove_group(pctldev, gsel); > mutex_unlock(&rza1_pctl->mutex); > > dev_info(rza1_pctl->dev, "Unable to parse function and group %s\n", > -- > 2.17.1 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 5/5] pinctrl: core: Remove broken remove_last group and pinmux functions 2018-06-15 11:11 [PATCH 0/5] pinctrl fixes for generic functions and groups Tony Lindgren ` (3 preceding siblings ...) 2018-06-15 11:11 ` [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions Tony Lindgren @ 2018-06-15 11:11 ` Tony Lindgren 4 siblings, 0 replies; 12+ messages in thread From: Tony Lindgren @ 2018-06-15 11:11 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-kernel, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, Sean Wang With no users left for these functions let's remove them. Reported-by: H. Nikolaus Schaller <hns@goldelico.com> Cc: Haojian Zhuang <haojian.zhuang@linaro.org> Cc: Jacopo Mondi <jacopo+renesas@jmondi.org> Cc: Paul Cercueil <paul@crapouillou.net> Cc: Sean Wang <sean.wang@mediatek.com> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/pinctrl/core.h | 6 ------ drivers/pinctrl/pinmux.h | 7 ------- 2 files changed, 13 deletions(-) diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h --- a/drivers/pinctrl/core.h +++ b/drivers/pinctrl/core.h @@ -218,12 +218,6 @@ int pinctrl_generic_add_group(struct pinctrl_dev *pctldev, const char *name, int pinctrl_generic_remove_group(struct pinctrl_dev *pctldev, unsigned int group_selector); -static inline int -pinctrl_generic_remove_last_group(struct pinctrl_dev *pctldev) -{ - return pinctrl_generic_remove_group(pctldev, pctldev->num_groups - 1); -} - #endif /* CONFIG_GENERIC_PINCTRL_GROUPS */ struct pinctrl_dev *get_pinctrl_dev_from_devname(const char *dev_name); diff --git a/drivers/pinctrl/pinmux.h b/drivers/pinctrl/pinmux.h --- a/drivers/pinctrl/pinmux.h +++ b/drivers/pinctrl/pinmux.h @@ -150,13 +150,6 @@ int pinmux_generic_add_function(struct pinctrl_dev *pctldev, int pinmux_generic_remove_function(struct pinctrl_dev *pctldev, unsigned int selector); -static inline int -pinmux_generic_remove_last_function(struct pinctrl_dev *pctldev) -{ - return pinmux_generic_remove_function(pctldev, - pctldev->num_functions - 1); -} - void pinmux_generic_free_functions(struct pinctrl_dev *pctldev); #else -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv2 0/5] pinctrl fixes for generic functions and groups @ 2018-06-19 9:31 Tony Lindgren 2018-06-19 9:31 ` [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions Tony Lindgren 0 siblings, 1 reply; 12+ messages in thread From: Tony Lindgren @ 2018-06-19 9:31 UTC (permalink / raw) To: Linus Walleij; +Cc: linux-gpio, linux-kernel Here are fixes to the race issues for generic group and functions reported by H. Nikolaus Schaller <hns@goldelico.com>. I have not seen the issue here myself, so please test to see if this is sufficient. I've also fixed rza1 in addition to pinctrl-single. Please also check the drivers pinctrl-mt7622.c and pinctrl-ingenic.c if mutex fixes are needed there. Regards, Tony Changes since v1: - Check if a function or group already exists as suggested by Andy Shevchenko - Make sure we always have a valid name for functions and groups as suggested by Christ van Willegen - Prettify rza1 variables as suggested by Jacopo Mondi Tony Lindgren (5): pinctrl: core: Return selector to the pinctrl driver pinctrl: pinmux: Return selector to the pinctrl driver pinctrl: single: Fix group and function selector use pinctrl: rza1: Fix selector use for groups and functions pinctrl: core: Remove broken remove_last group and pinmux functions drivers/pinctrl/core.c | 35 ++++++++++-- drivers/pinctrl/core.h | 6 --- drivers/pinctrl/pinctrl-rza1.c | 24 +++++---- drivers/pinctrl/pinctrl-single.c | 91 +++++++++++++++++++------------- drivers/pinctrl/pinmux.c | 16 ++++-- drivers/pinctrl/pinmux.h | 7 --- 6 files changed, 112 insertions(+), 67 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions 2018-06-19 9:31 [PATCHv2 0/5] pinctrl fixes for generic functions and groups Tony Lindgren @ 2018-06-19 9:31 ` Tony Lindgren 0 siblings, 0 replies; 12+ messages in thread From: Tony Lindgren @ 2018-06-19 9:31 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-kernel, Andy Shevchenko, Christ van Willegen, Haojian Zhuang, Paul Cercueil, Sean Wang We must use a mutex around the generic_add functions and save the function and group selector in case we need to remove them. Otherwise the selector use will be racy for deferred probe at least. Fixes: 5a49b644b307 ("pinctrl: Renesas RZ/A1 pin and gpio controller") Reported-by: H. Nikolaus Schaller <hns@goldelico.com> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Christ van Willegen <cvwillegen@gmail.com> Cc: Haojian Zhuang <haojian.zhuang@linaro.org> Cc: Paul Cercueil <paul@crapouillou.net> Cc: Sean Wang <sean.wang@mediatek.com> Acked-by: Jacopo Mondi <jacopo@jmondi.org> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/pinctrl/pinctrl-rza1.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c --- a/drivers/pinctrl/pinctrl-rza1.c +++ b/drivers/pinctrl/pinctrl-rza1.c @@ -1006,6 +1006,7 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, const char *grpname; const char **fngrps; int ret, npins; + int gsel, fsel; npins = rza1_dt_node_pin_count(np); if (npins < 0) { @@ -1055,18 +1056,19 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, fngrps[0] = grpname; mutex_lock(&rza1_pctl->mutex); - ret = pinctrl_generic_add_group(pctldev, grpname, grpins, npins, - NULL); - if (ret) { + gsel = pinctrl_generic_add_group(pctldev, grpname, grpins, npins, + NULL); + if (gsel < 0) { mutex_unlock(&rza1_pctl->mutex); - return ret; + return gsel; } - ret = pinmux_generic_add_function(pctldev, grpname, fngrps, 1, - mux_confs); - if (ret) + fsel = pinmux_generic_add_function(pctldev, grpname, fngrps, 1, + mux_confs); + if (fsel < 0) { + ret = fsel; goto remove_group; - mutex_unlock(&rza1_pctl->mutex); + } dev_info(rza1_pctl->dev, "Parsed function and group %s with %d pins\n", grpname, npins); @@ -1083,15 +1085,15 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, (*map)->data.mux.group = np->name; (*map)->data.mux.function = np->name; *num_maps = 1; + mutex_unlock(&rza1_pctl->mutex); return 0; remove_function: - mutex_lock(&rza1_pctl->mutex); - pinmux_generic_remove_last_function(pctldev); + pinmux_generic_remove_function(pctldev, fsel); remove_group: - pinctrl_generic_remove_last_group(pctldev); + pinctrl_generic_remove_group(pctldev, gsel); mutex_unlock(&rza1_pctl->mutex); dev_info(rza1_pctl->dev, "Unable to parse function and group %s\n", -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCHv3 0/5] pinctrl fixes for generic functions and groups @ 2018-07-05 9:10 Tony Lindgren 2018-07-05 9:10 ` [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions Tony Lindgren 0 siblings, 1 reply; 12+ messages in thread From: Tony Lindgren @ 2018-07-05 9:10 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-kernel, Andy Shevchenko, Christ van Willegen, H. Nikolaus Schaller, Haojian Zhuang, Jacopo Mondi, Paul Cercueil, Sean Wang Here is a resend of fixes for a race issues for generic group and functions reported by H. Nikolaus Schaller <hns@goldelico.com>. Regards, Tony Changes since v2: - Added Nikolaus to Cc as Reported-by did not do it Changes since v1: - Check if a function or group already exists as suggested by Andy Shevchenko - Make sure we always have a valid name for functions and groups as suggested by Christ van Willegen - Prettify rza1 variables as suggested by Jacopo Mondi Tony Lindgren (5): pinctrl: core: Return selector to the pinctrl driver pinctrl: pinmux: Return selector to the pinctrl driver pinctrl: single: Fix group and function selector use pinctrl: rza1: Fix selector use for groups and functions pinctrl: core: Remove broken remove_last group and pinmux functions drivers/pinctrl/core.c | 35 ++++++++++-- drivers/pinctrl/core.h | 6 --- drivers/pinctrl/pinctrl-rza1.c | 24 +++++---- drivers/pinctrl/pinctrl-single.c | 91 +++++++++++++++++++------------- drivers/pinctrl/pinmux.c | 16 ++++-- drivers/pinctrl/pinmux.h | 7 --- 6 files changed, 112 insertions(+), 67 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions 2018-07-05 9:10 [PATCHv3 0/5] pinctrl fixes for generic functions and groups Tony Lindgren @ 2018-07-05 9:10 ` Tony Lindgren 0 siblings, 0 replies; 12+ messages in thread From: Tony Lindgren @ 2018-07-05 9:10 UTC (permalink / raw) To: Linus Walleij Cc: linux-gpio, linux-kernel, Andy Shevchenko, Christ van Willegen, Haojian Zhuang, H . Nikolaus Schaller, Paul Cercueil, Sean Wang, Jacopo Mondi We must use a mutex around the generic_add functions and save the function and group selector in case we need to remove them. Otherwise the selector use will be racy for deferred probe at least. Fixes: 5a49b644b307 ("pinctrl: Renesas RZ/A1 pin and gpio controller") Reported-by: H. Nikolaus Schaller <hns@goldelico.com> Cc: Andy Shevchenko <andy.shevchenko@gmail.com> Cc: Christ van Willegen <cvwillegen@gmail.com> Cc: Haojian Zhuang <haojian.zhuang@linaro.org> Cc: H. Nikolaus Schaller <hns@goldelico.com> Cc: Paul Cercueil <paul@crapouillou.net> Cc: Sean Wang <sean.wang@mediatek.com> Acked-by: Jacopo Mondi <jacopo@jmondi.org> Signed-off-by: Tony Lindgren <tony@atomide.com> --- drivers/pinctrl/pinctrl-rza1.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/pinctrl/pinctrl-rza1.c b/drivers/pinctrl/pinctrl-rza1.c --- a/drivers/pinctrl/pinctrl-rza1.c +++ b/drivers/pinctrl/pinctrl-rza1.c @@ -1006,6 +1006,7 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, const char *grpname; const char **fngrps; int ret, npins; + int gsel, fsel; npins = rza1_dt_node_pin_count(np); if (npins < 0) { @@ -1055,18 +1056,19 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, fngrps[0] = grpname; mutex_lock(&rza1_pctl->mutex); - ret = pinctrl_generic_add_group(pctldev, grpname, grpins, npins, - NULL); - if (ret) { + gsel = pinctrl_generic_add_group(pctldev, grpname, grpins, npins, + NULL); + if (gsel < 0) { mutex_unlock(&rza1_pctl->mutex); - return ret; + return gsel; } - ret = pinmux_generic_add_function(pctldev, grpname, fngrps, 1, - mux_confs); - if (ret) + fsel = pinmux_generic_add_function(pctldev, grpname, fngrps, 1, + mux_confs); + if (fsel < 0) { + ret = fsel; goto remove_group; - mutex_unlock(&rza1_pctl->mutex); + } dev_info(rza1_pctl->dev, "Parsed function and group %s with %d pins\n", grpname, npins); @@ -1083,15 +1085,15 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev, (*map)->data.mux.group = np->name; (*map)->data.mux.function = np->name; *num_maps = 1; + mutex_unlock(&rza1_pctl->mutex); return 0; remove_function: - mutex_lock(&rza1_pctl->mutex); - pinmux_generic_remove_last_function(pctldev); + pinmux_generic_remove_function(pctldev, fsel); remove_group: - pinctrl_generic_remove_last_group(pctldev); + pinctrl_generic_remove_group(pctldev, gsel); mutex_unlock(&rza1_pctl->mutex); dev_info(rza1_pctl->dev, "Unable to parse function and group %s\n", -- 2.17.1 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-07-05 9:10 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-15 11:11 [PATCH 0/5] pinctrl fixes for generic functions and groups Tony Lindgren 2018-06-15 11:11 ` [PATCH 1/5] pinctrl: core: Return selector to the pinctrl driver Tony Lindgren 2018-06-17 16:50 ` Andy Shevchenko 2018-06-18 6:48 ` Tony Lindgren 2018-06-15 11:11 ` [PATCH 2/5] pinctrl: pinmux: " Tony Lindgren 2018-06-17 16:51 ` Andy Shevchenko 2018-06-15 11:11 ` [PATCH 3/5] pinctrl: single: Fix group and function selector use Tony Lindgren 2018-06-15 11:11 ` [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions Tony Lindgren 2018-06-19 4:16 ` jacopo mondi 2018-06-15 11:11 ` [PATCH 5/5] pinctrl: core: Remove broken remove_last group and pinmux functions Tony Lindgren 2018-06-19 9:31 [PATCHv2 0/5] pinctrl fixes for generic functions and groups Tony Lindgren 2018-06-19 9:31 ` [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions Tony Lindgren 2018-07-05 9:10 [PATCHv3 0/5] pinctrl fixes for generic functions and groups Tony Lindgren 2018-07-05 9:10 ` [PATCH 4/5] pinctrl: rza1: Fix selector use for groups and functions Tony Lindgren
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.