| Message ID | 20241005-th1520-pinctrl-fixes-v1-1-5c65dffa0d00@tenstorrent.com |
|---|---|
| State | New |
| Headers | show |
| Series | pinctrl: th1520: Improve code quality | expand |
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; > } > >
> 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
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) { 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) { 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); th1520_pinctrl_dt_free_map(pctldev, map, nmaps); - mutex_unlock(&thp->mutex); return ret; }
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(-)