* Re: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
2024-10-05 19:35 ` [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex Drew Fustini
@ 2024-10-05 19:43 ` Christophe JAILLET
2024-10-06 0:16 ` Drew Fustini
2024-10-05 19:54 ` Markus Elfring
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Christophe JAILLET @ 2024-10-05 19:43 UTC (permalink / raw)
To: Drew Fustini, Drew Fustini, Guo Ren, Fu Wei, Linus Walleij
Cc: linux-riscv, linux-gpio, linux-kernel
Le 05/10/2024 à 21:35, Drew Fustini a écrit :
> Convert th1520_pinctrl_dt_node_to_map() to use guarded mutex for
> thp->mutex.
>
> Suggested-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Drew Fustini <dfustini@tenstorrent.com>
> ---
> drivers/pinctrl/pinctrl-th1520.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
Hi,
> diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
> index 9331f4462480..d03a0a34220a 100644
> --- a/drivers/pinctrl/pinctrl-th1520.c
> +++ b/drivers/pinctrl/pinctrl-th1520.c
> @@ -425,7 +425,7 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> int ret;
>
> nmaps = 0;
> - for_each_available_child_of_node(np, child) {
> + for_each_available_child_of_node_scoped(np, child) {
Using _scoped iterator is not described in the commit message.
Is it expected to be part of this patch?
If yes, the "of_node_put(child);" just a few lines below should be removed.
> int npins = of_property_count_strings(child, "pins");
>
> if (npins <= 0) {
> @@ -444,8 +444,8 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> return -ENOMEM;
>
> nmaps = 0;
> - mutex_lock(&thp->mutex);
> - for_each_available_child_of_node(np, child) {
> + guard(mutex)(&thp->mutex);
> + for_each_available_child_of_node_scoped(np, child) {
Same here...
> unsigned int rollback = nmaps;
> enum th1520_muxtype muxtype;
> struct property *prop;
> @@ -530,7 +530,6 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
>
> *maps = map;
> *num_maps = nmaps;
> - mutex_unlock(&thp->mutex);
> return 0;
>
> free_configs:
> @@ -538,7 +537,6 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> put_child:
> of_node_put(child);
... this should be removed, and maybe the label renamed.
CJ
> th1520_pinctrl_dt_free_map(pctldev, map, nmaps);
> - mutex_unlock(&thp->mutex);
> return ret;
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
2024-10-05 19:43 ` Christophe JAILLET
@ 2024-10-06 0:16 ` Drew Fustini
0 siblings, 0 replies; 10+ messages in thread
From: Drew Fustini @ 2024-10-06 0:16 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Drew Fustini, Guo Ren, Fu Wei, Linus Walleij, linux-riscv,
linux-gpio, linux-kernel
On Sat, Oct 05, 2024 at 09:43:06PM +0200, Christophe JAILLET wrote:
> Le 05/10/2024 à 21:35, Drew Fustini a écrit :
> > Convert th1520_pinctrl_dt_node_to_map() to use guarded mutex for
> > thp->mutex.
> >
> > Suggested-by: Linus Walleij <linus.walleij@linaro.org>
> > Signed-off-by: Drew Fustini <dfustini@tenstorrent.com>
> > ---
> > drivers/pinctrl/pinctrl-th1520.c | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
>
> Hi,
>
> > diff --git a/drivers/pinctrl/pinctrl-th1520.c b/drivers/pinctrl/pinctrl-th1520.c
> > index 9331f4462480..d03a0a34220a 100644
> > --- a/drivers/pinctrl/pinctrl-th1520.c
> > +++ b/drivers/pinctrl/pinctrl-th1520.c
> > @@ -425,7 +425,7 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> > int ret;
> > nmaps = 0;
> > - for_each_available_child_of_node(np, child) {
> > + for_each_available_child_of_node_scoped(np, child) {
>
> Using _scoped iterator is not described in the commit message.
> Is it expected to be part of this patch?
Yes, it was intentional, but you are right that I should have
highlighted that. I'll make it a separate patch.
>
> If yes, the "of_node_put(child);" just a few lines below should be removed.
Thanks, will do.
>
> > int npins = of_property_count_strings(child, "pins");
> > if (npins <= 0) {
> > @@ -444,8 +444,8 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> > return -ENOMEM;
> > nmaps = 0;
> > - mutex_lock(&thp->mutex);
> > - for_each_available_child_of_node(np, child) {
> > + guard(mutex)(&thp->mutex);
> > + for_each_available_child_of_node_scoped(np, child) {
>
> Same here...
>
> > unsigned int rollback = nmaps;
> > enum th1520_muxtype muxtype;
> > struct property *prop;
> > @@ -530,7 +530,6 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> > *maps = map;
> > *num_maps = nmaps;
> > - mutex_unlock(&thp->mutex);
> > return 0;
> > free_configs:
> > @@ -538,7 +537,6 @@ static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> > put_child:
> > of_node_put(child);
>
> ... this should be removed, and maybe the label renamed.
Thanks, will do.
Drew
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
2024-10-05 19:35 ` [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex Drew Fustini
2024-10-05 19:43 ` Christophe JAILLET
@ 2024-10-05 19:54 ` Markus Elfring
2024-10-06 20:04 ` kernel test robot
2024-10-07 15:41 ` Dan Carpenter
3 siblings, 0 replies; 10+ messages in thread
From: Markus Elfring @ 2024-10-05 19:54 UTC (permalink / raw)
To: Drew Fustini, linux-gpio, linux-riscv, Fu Wei, Guo Ren,
Linus Walleij
Cc: Drew Fustini, LKML
> Convert th1520_pinctrl_dt_node_to_map() to use guarded mutex for
> thp->mutex.
How does the proposed usage of the programming interface “for_each_available_child_of_node_scoped”
fit to such a change description?
Would you like to omit the first word “to” from the summary phrase?
Would you generally like to increase the application of scope-based resource management
(also in this software area)?
Regards,
Markus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
2024-10-05 19:35 ` [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex Drew Fustini
2024-10-05 19:43 ` Christophe JAILLET
2024-10-05 19:54 ` Markus Elfring
@ 2024-10-06 20:04 ` kernel test robot
2024-10-07 15:41 ` Dan Carpenter
3 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2024-10-06 20:04 UTC (permalink / raw)
To: Drew Fustini, Drew Fustini, Guo Ren, Fu Wei, Linus Walleij
Cc: oe-kbuild-all, linux-riscv, linux-gpio, linux-kernel
Hi Drew,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 2694868880705e8f6bb61b24b1b25adc42a4a217]
url: https://github.com/intel-lab-lkp/linux/commits/Drew-Fustini/pinctrl-th1520-Convert-to-thp-mutex-to-guarded-mutex/20241006-033647
base: 2694868880705e8f6bb61b24b1b25adc42a4a217
patch link: https://lore.kernel.org/r/20241005-th1520-pinctrl-fixes-v1-1-5c65dffa0d00%40tenstorrent.com
patch subject: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
config: powerpc64-randconfig-r073-20241007 (https://download.01.org/0day-ci/archive/20241007/202410070352.6BZrRWQU-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241007/202410070352.6BZrRWQU-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410070352.6BZrRWQU-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/pinctrl/pinctrl-th1520.c:538:14: warning: variable 'child' is uninitialized when used here [-Wuninitialized]
of_node_put(child);
^~~~~
drivers/pinctrl/pinctrl-th1520.c:420:27: note: initialize the variable 'child' to silence this warning
struct device_node *child;
^
= NULL
1 warning generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for MODVERSIONS
Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
Selected by [y]:
- RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]
vim +/child +538 drivers/pinctrl/pinctrl-th1520.c
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 413
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 414 static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 415 struct device_node *np,
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 416 struct pinctrl_map **maps,
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 417 unsigned int *num_maps)
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 418 {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 419 struct th1520_pinctrl *thp = pinctrl_dev_get_drvdata(pctldev);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 420 struct device_node *child;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 421 struct pinctrl_map *map;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 422 unsigned long *configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 423 unsigned int nconfigs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 424 unsigned int nmaps;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 425 int ret;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 426
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 427 nmaps = 0;
fb310b5cb13ad2 Drew Fustini 2024-10-05 428 for_each_available_child_of_node_scoped(np, child) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 429 int npins = of_property_count_strings(child, "pins");
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 430
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 431 if (npins <= 0) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 432 of_node_put(child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 433 dev_err(thp->pctl->dev, "no pins selected for %pOFn.%pOFn\n",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 434 np, child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 435 return -EINVAL;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 436 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 437 nmaps += npins;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 438 if (of_property_present(child, "function"))
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 439 nmaps += npins;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 440 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 441
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 442 map = kcalloc(nmaps, sizeof(*map), GFP_KERNEL);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 443 if (!map)
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 444 return -ENOMEM;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 445
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 446 nmaps = 0;
fb310b5cb13ad2 Drew Fustini 2024-10-05 447 guard(mutex)(&thp->mutex);
fb310b5cb13ad2 Drew Fustini 2024-10-05 448 for_each_available_child_of_node_scoped(np, child) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 449 unsigned int rollback = nmaps;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 450 enum th1520_muxtype muxtype;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 451 struct property *prop;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 452 const char *funcname;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 453 const char **pgnames;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 454 const char *pinname;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 455 int npins;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 456
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 457 ret = pinconf_generic_parse_dt_config(child, pctldev, &configs, &nconfigs);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 458 if (ret) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 459 dev_err(thp->pctl->dev, "%pOFn.%pOFn: error parsing pin config\n",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 460 np, child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 461 goto put_child;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 462 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 463
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 464 if (!of_property_read_string(child, "function", &funcname)) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 465 muxtype = th1520_muxtype_get(funcname);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 466 if (!muxtype) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 467 dev_err(thp->pctl->dev, "%pOFn.%pOFn: unknown function '%s'\n",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 468 np, child, funcname);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 469 ret = -EINVAL;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 470 goto free_configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 471 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 472
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 473 funcname = devm_kasprintf(thp->pctl->dev, GFP_KERNEL, "%pOFn.%pOFn",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 474 np, child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 475 if (!funcname) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 476 ret = -ENOMEM;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 477 goto free_configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 478 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 479
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 480 npins = of_property_count_strings(child, "pins");
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 481 pgnames = devm_kcalloc(thp->pctl->dev, npins, sizeof(*pgnames), GFP_KERNEL);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 482 if (!pgnames) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 483 ret = -ENOMEM;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 484 goto free_configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 485 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 486 } else {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 487 funcname = NULL;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 488 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 489
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 490 npins = 0;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 491 of_property_for_each_string(child, "pins", prop, pinname) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 492 unsigned int i;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 493
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 494 for (i = 0; i < thp->desc.npins; i++) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 495 if (!strcmp(pinname, thp->desc.pins[i].name))
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 496 break;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 497 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 498 if (i == thp->desc.npins) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 499 nmaps = rollback;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 500 dev_err(thp->pctl->dev, "%pOFn.%pOFn: unknown pin '%s'\n",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 501 np, child, pinname);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 502 goto free_configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 503 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 504
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 505 if (nconfigs) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 506 map[nmaps].type = PIN_MAP_TYPE_CONFIGS_PIN;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 507 map[nmaps].data.configs.group_or_pin = thp->desc.pins[i].name;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 508 map[nmaps].data.configs.configs = configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 509 map[nmaps].data.configs.num_configs = nconfigs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 510 nmaps += 1;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 511 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 512 if (funcname) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 513 pgnames[npins++] = thp->desc.pins[i].name;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 514 map[nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 515 map[nmaps].data.mux.function = funcname;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 516 map[nmaps].data.mux.group = thp->desc.pins[i].name;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 517 nmaps += 1;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 518 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 519 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 520
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 521 if (funcname) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 522 ret = pinmux_generic_add_function(pctldev, funcname, pgnames,
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 523 npins, (void *)muxtype);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 524 if (ret < 0) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 525 dev_err(thp->pctl->dev, "error adding function %s\n", funcname);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 526 goto put_child;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 527 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 528 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 529 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 530
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 531 *maps = map;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 532 *num_maps = nmaps;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 533 return 0;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 534
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 535 free_configs:
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 536 kfree(configs);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 537 put_child:
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 @538 of_node_put(child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 539 th1520_pinctrl_dt_free_map(pctldev, map, nmaps);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 540 return ret;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 541 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 542
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
2024-10-05 19:35 ` [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex Drew Fustini
` (2 preceding siblings ...)
2024-10-06 20:04 ` kernel test robot
@ 2024-10-07 15:41 ` Dan Carpenter
2024-10-07 16:33 ` Drew Fustini
3 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2024-10-07 15:41 UTC (permalink / raw)
To: oe-kbuild, Drew Fustini, Drew Fustini, Guo Ren, Fu Wei,
Linus Walleij
Cc: lkp, oe-kbuild-all, linux-riscv, linux-gpio, linux-kernel
Hi Drew,
kernel test robot noticed the following build warnings:
url: https://github.com/intel-lab-lkp/linux/commits/Drew-Fustini/pinctrl-th1520-Convert-to-thp-mutex-to-guarded-mutex/20241006-033647
base: 2694868880705e8f6bb61b24b1b25adc42a4a217
patch link: https://lore.kernel.org/r/20241005-th1520-pinctrl-fixes-v1-1-5c65dffa0d00%40tenstorrent.com
patch subject: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
config: parisc-randconfig-r072-20241007 (https://download.01.org/0day-ci/archive/20241007/202410072108.uG2sVTN4-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 14.1.0
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202410072108.uG2sVTN4-lkp@intel.com/
New smatch warnings:
drivers/pinctrl/pinctrl-th1520.c:538 th1520_pinctrl_dt_node_to_map() error: uninitialized symbol 'child'.
Old smatch warnings:
drivers/pinctrl/pinctrl-th1520.c:502 th1520_pinctrl_dt_node_to_map() warn: missing error code 'ret'
vim +/child +538 drivers/pinctrl/pinctrl-th1520.c
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 414 static int th1520_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 415 struct device_node *np,
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 416 struct pinctrl_map **maps,
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 417 unsigned int *num_maps)
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 418 {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 419 struct th1520_pinctrl *thp = pinctrl_dev_get_drvdata(pctldev);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 420 struct device_node *child;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 421 struct pinctrl_map *map;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 422 unsigned long *configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 423 unsigned int nconfigs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 424 unsigned int nmaps;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 425 int ret;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 426
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 427 nmaps = 0;
fb310b5cb13ad2 Drew Fustini 2024-10-05 428 for_each_available_child_of_node_scoped(np, child) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 429 int npins = of_property_count_strings(child, "pins");
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 430
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 431 if (npins <= 0) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 432 of_node_put(child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 433 dev_err(thp->pctl->dev, "no pins selected for %pOFn.%pOFn\n",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 434 np, child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 435 return -EINVAL;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 436 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 437 nmaps += npins;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 438 if (of_property_present(child, "function"))
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 439 nmaps += npins;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 440 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 441
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 442 map = kcalloc(nmaps, sizeof(*map), GFP_KERNEL);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 443 if (!map)
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 444 return -ENOMEM;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 445
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 446 nmaps = 0;
fb310b5cb13ad2 Drew Fustini 2024-10-05 447 guard(mutex)(&thp->mutex);
fb310b5cb13ad2 Drew Fustini 2024-10-05 448 for_each_available_child_of_node_scoped(np, child) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 449 unsigned int rollback = nmaps;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 450 enum th1520_muxtype muxtype;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 451 struct property *prop;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 452 const char *funcname;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 453 const char **pgnames;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 454 const char *pinname;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 455 int npins;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 456
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 457 ret = pinconf_generic_parse_dt_config(child, pctldev, &configs, &nconfigs);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 458 if (ret) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 459 dev_err(thp->pctl->dev, "%pOFn.%pOFn: error parsing pin config\n",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 460 np, child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 461 goto put_child;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 462 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 463
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 464 if (!of_property_read_string(child, "function", &funcname)) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 465 muxtype = th1520_muxtype_get(funcname);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 466 if (!muxtype) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 467 dev_err(thp->pctl->dev, "%pOFn.%pOFn: unknown function '%s'\n",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 468 np, child, funcname);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 469 ret = -EINVAL;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 470 goto free_configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 471 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 472
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 473 funcname = devm_kasprintf(thp->pctl->dev, GFP_KERNEL, "%pOFn.%pOFn",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 474 np, child);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 475 if (!funcname) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 476 ret = -ENOMEM;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 477 goto free_configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 478 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 479
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 480 npins = of_property_count_strings(child, "pins");
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 481 pgnames = devm_kcalloc(thp->pctl->dev, npins, sizeof(*pgnames), GFP_KERNEL);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 482 if (!pgnames) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 483 ret = -ENOMEM;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 484 goto free_configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 485 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 486 } else {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 487 funcname = NULL;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 488 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 489
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 490 npins = 0;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 491 of_property_for_each_string(child, "pins", prop, pinname) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 492 unsigned int i;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 493
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 494 for (i = 0; i < thp->desc.npins; i++) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 495 if (!strcmp(pinname, thp->desc.pins[i].name))
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 496 break;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 497 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 498 if (i == thp->desc.npins) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 499 nmaps = rollback;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 500 dev_err(thp->pctl->dev, "%pOFn.%pOFn: unknown pin '%s'\n",
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 501 np, child, pinname);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 502 goto free_configs;
err = -EINVAL?
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 503 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 504
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 505 if (nconfigs) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 506 map[nmaps].type = PIN_MAP_TYPE_CONFIGS_PIN;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 507 map[nmaps].data.configs.group_or_pin = thp->desc.pins[i].name;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 508 map[nmaps].data.configs.configs = configs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 509 map[nmaps].data.configs.num_configs = nconfigs;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 510 nmaps += 1;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 511 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 512 if (funcname) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 513 pgnames[npins++] = thp->desc.pins[i].name;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 514 map[nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 515 map[nmaps].data.mux.function = funcname;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 516 map[nmaps].data.mux.group = thp->desc.pins[i].name;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 517 nmaps += 1;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 518 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 519 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 520
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 521 if (funcname) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 522 ret = pinmux_generic_add_function(pctldev, funcname, pgnames,
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 523 npins, (void *)muxtype);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 524 if (ret < 0) {
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 525 dev_err(thp->pctl->dev, "error adding function %s\n", funcname);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 526 goto put_child;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 527 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 528 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 529 }
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 530
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 531 *maps = map;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 532 *num_maps = nmaps;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 533 return 0;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 534
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 535 free_configs:
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 536 kfree(configs);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 537 put_child:
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 @538 of_node_put(child);
We're using _scoped() loops so this is a double put.
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 539 th1520_pinctrl_dt_free_map(pctldev, map, nmaps);
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 540 return ret;
bed5cd6f8a9883 Emil Renner Berthing 2024-09-30 541 }
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
2024-10-07 15:41 ` Dan Carpenter
@ 2024-10-07 16:33 ` Drew Fustini
0 siblings, 0 replies; 10+ messages in thread
From: Drew Fustini @ 2024-10-07 16:33 UTC (permalink / raw)
To: Dan Carpenter
Cc: oe-kbuild, Drew Fustini, Guo Ren, Fu Wei, Linus Walleij, lkp,
oe-kbuild-all, linux-riscv, linux-gpio, linux-kernel
On Mon, Oct 07, 2024 at 06:41:36PM +0300, Dan Carpenter wrote:
> Hi Drew,
>
> kernel test robot noticed the following build warnings:
>
> url: https://github.com/intel-lab-lkp/linux/commits/Drew-Fustini/pinctrl-th1520-Convert-to-thp-mutex-to-guarded-mutex/20241006-033647
> base: 2694868880705e8f6bb61b24b1b25adc42a4a217
> patch link: https://lore.kernel.org/r/20241005-th1520-pinctrl-fixes-v1-1-5c65dffa0d00%40tenstorrent.com
> patch subject: [PATCH 1/2] pinctrl: th1520: Convert to thp->mutex to guarded mutex
> config: parisc-randconfig-r072-20241007 (https://download.01.org/0day-ci/archive/20241007/202410072108.uG2sVTN4-lkp@intel.com/config)
> compiler: hppa-linux-gcc (GCC) 14.1.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> | Closes: https://lore.kernel.org/r/202410072108.uG2sVTN4-lkp@intel.com/
>
> New smatch warnings:
> drivers/pinctrl/pinctrl-th1520.c:538 th1520_pinctrl_dt_node_to_map() error: uninitialized symbol 'child'.
It seems this is because the scoped iterator declares *child in the
macro and thus no separate declaration is needed:
#define for_each_available_child_of_node_scoped(parent, child) \
for (struct device_node *child __free(device_node) = \
of_get_next_available_child(parent, NULL); \
child != NULL; \
child = of_get_next_available_child(parent, child))
I'll fix in future revision.
>
> Old smatch warnings:
> drivers/pinctrl/pinctrl-th1520.c:502 th1520_pinctrl_dt_node_to_map() warn: missing error code 'ret'
This has been fixed in the v2 series [1]
Thanks,
Drew
[1] https://lore.kernel.org/linux-riscv/20241006-th1520-pinctrl-fixes-v2-0-b1822ae3a6d7@tenstorrent.com/
^ permalink raw reply [flat|nested] 10+ messages in thread