From 7d71b5f4b2fb5b2a38794fe029d6245c86244de6 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Thu, 7 Jun 2018 14:24:34 +0200 Subject: pinctrl: pinctrl-single: Avoid divisions in context save/restore The divisions (and multiplications) can be avoided by changing the loops to use increments of mux_bytes instead of 1. While at it, remove the unneeded casts when assigning void pointers. This saves +100 bytes of kernel size on arm32/arm64. Signed-off-by: Geert Uytterhoeven Acked-by: Tony Lindgren Tested-by: Keerthy Reviewed-by: Keerthy Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-single.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'drivers/pinctrl/pinctrl-single.c') diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index b3153c095199..92b694675a56 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -1595,19 +1595,19 @@ static int pcs_save_context(struct pcs_device *pcs) switch (pcs->width) { case 64: - regsl = (u64 *)pcs->saved_vals; - for (i = 0; i < pcs->size / mux_bytes; i++) - regsl[i] = pcs->read(pcs->base + i * mux_bytes); + regsl = pcs->saved_vals; + for (i = 0; i < pcs->size; i += mux_bytes) + *regsl++ = pcs->read(pcs->base + i); break; case 32: - regsw = (u32 *)pcs->saved_vals; - for (i = 0; i < pcs->size / mux_bytes; i++) - regsw[i] = pcs->read(pcs->base + i * mux_bytes); + regsw = pcs->saved_vals; + for (i = 0; i < pcs->size; i += mux_bytes) + *regsw++ = pcs->read(pcs->base + i); break; case 16: - regshw = (u16 *)pcs->saved_vals; - for (i = 0; i < pcs->size / mux_bytes; i++) - regshw[i] = pcs->read(pcs->base + i * mux_bytes); + regshw = pcs->saved_vals; + for (i = 0; i < pcs->size; i += mux_bytes) + *regshw++ = pcs->read(pcs->base + i); break; } @@ -1625,19 +1625,19 @@ static void pcs_restore_context(struct pcs_device *pcs) switch (pcs->width) { case 64: - regsl = (u64 *)pcs->saved_vals; - for (i = 0; i < pcs->size / mux_bytes; i++) - pcs->write(regsl[i], pcs->base + i * mux_bytes); + regsl = pcs->saved_vals; + for (i = 0; i < pcs->size; i += mux_bytes) + pcs->write(*regsl++, pcs->base + i); break; case 32: - regsw = (u32 *)pcs->saved_vals; - for (i = 0; i < pcs->size / mux_bytes; i++) - pcs->write(regsw[i], pcs->base + i * mux_bytes); + regsw = pcs->saved_vals; + for (i = 0; i < pcs->size; i += mux_bytes) + pcs->write(*regsw++, pcs->base + i); break; case 16: - regshw = (u16 *)pcs->saved_vals; - for (i = 0; i < pcs->size / mux_bytes; i++) - pcs->write(regshw[i], pcs->base + i * mux_bytes); + regshw = pcs->saved_vals; + for (i = 0; i < pcs->size; i += mux_bytes) + pcs->write(*regshw++, pcs->base + i); break; } } -- cgit From a4ab1086072365235864151bca57230afa8bcc93 Mon Sep 17 00:00:00 2001 From: Tony Lindgren Date: Thu, 5 Jul 2018 02:10:16 -0700 Subject: pinctrl: single: Fix group and function selector use 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 Cc: Christ van Willegen Cc: Haojian Zhuang Cc: Jacopo Mondi Cc: Paul Cercueil Cc: Sean Wang Signed-off-by: Tony Lindgren Tested-By: H. Nikolaus Schaller Reviewed-by: Andy Shevchenko Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-single.c | 91 ++++++++++++++++++++++++---------------- 1 file changed, 55 insertions(+), 36 deletions(-) (limited to 'drivers/pinctrl/pinctrl-single.c') diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 92b694675a56..42d7e76baccf 100644 --- 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); -- cgit From 673ba5a05ca14c2ddef038044768a4acd0ae0a53 Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Wed, 11 Jul 2018 12:33:31 +0000 Subject: pinctrl: single: Fix missing unlock on error path Add the missing unlock before return from function in the error handling case. Fixes: 0f5972033509 ("pinctrl: single: Fix group and function selector use") Signed-off-by: Wei Yongjun Acked-by: Tony Lindgren Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-single.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/pinctrl/pinctrl-single.c') diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index 42d7e76baccf..9fa2f54bb1a3 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -1071,8 +1071,8 @@ free_pingroups: *num_maps = 1; free_function: pinmux_generic_remove_function(pcs->pctl, fsel); - mutex_unlock(&pcs->mutex); free_pins: + mutex_unlock(&pcs->mutex); devm_kfree(pcs->dev, pins); free_vals: @@ -1211,8 +1211,8 @@ free_pingroups: *num_maps = 1; free_function: pinmux_generic_remove_function(pcs->pctl, fsel); - mutex_unlock(&pcs->mutex); free_pins: + mutex_unlock(&pcs->mutex); devm_kfree(pcs->dev, pins); free_vals: -- cgit