Use scope based of_node_put() to simplify code. It reduces the chance
of forgetting of_node_put(), and also simplifies error handling path.
I not able to test the changes on all the hardwares, so driver owners,
please help review when you have time.
This patchset was inspired from Dan's comments on pinctrl-scmi-imx.c,
thanks.
Signed-off-by: Peng Fan <[email protected]>
---
Changes in v2:
- Drop aspeed changes per Andrew Jeffery
- Drop changes to code pattern that of_node_get(or other refcount
increasing) followed by of_node_put. That said, but I still have a
change for samsung pinctrl that drops several of_node_put places. If
this is not welcomed, patch 20/20 could be dropped.
- Add Fix tag for patch 1
- Add A-b for patch 4
- Drop unneeded {} in patch 8 Per Dan Carpenter
- Add a new patch 18.
- Moved patch [19,20]/20, in case people are not happy with the changes,
the two patch could be dropped when apply if no v3 patchset.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
Peng Fan (20):
pinctrl: ti: iodelay: Use scope based of_node_put() cleanups
pinctrl: tegra: Use scope based of_node_put() cleanups
pinctrl: stm32: Use scope based of_node_put() cleanups
pinctrl: starfive: Use scope based of_node_put() cleanups
pinctrl: sprd: Use scope based of_node_put() cleanups
pinctrl: spear: Use scope based of_node_put() cleanups
pinctrl: renesas: Use scope based of_node_put() cleanups
pinctrl: st: Use scope based of_node_put() cleanups
pinctrl: rockchip: Use scope based of_node_put() cleanups
pinctrl: equilibrium: Use scope based of_node_put() cleanups
pinctrl: at91: Use scope based of_node_put() cleanups
pinctrl: s32cc: Use scope based of_node_put() cleanups
pinctrl: nomadik: Use scope based of_node_put() cleanups
pinctrl: mediatek: Use scope based of_node_put() cleanups
pinctrl: freescale: Use scope based of_node_put() cleanups
pinctrl: bcm: bcm63xx: Use scope based of_node_put() cleanups
pinctrl: pinconf-generic: Use scope based of_node_put() cleanups
pinctrl: freescale: mxs: Fix refcount of child
pinctrl: k210: Use scope based of_node_put() cleanups
pinctrl: samsung: Use scope based of_node_put() cleanups
drivers/pinctrl/bcm/pinctrl-bcm63xx.c | 4 +--
drivers/pinctrl/freescale/pinctrl-imx.c | 25 ++++-----------
drivers/pinctrl/freescale/pinctrl-imx1-core.c | 16 +++-------
drivers/pinctrl/freescale/pinctrl-mxs.c | 18 ++++-------
drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 4 +--
drivers/pinctrl/mediatek/pinctrl-paris.c | 4 +--
drivers/pinctrl/nomadik/pinctrl-abx500.c | 4 +--
drivers/pinctrl/nomadik/pinctrl-nomadik.c | 4 +--
drivers/pinctrl/nxp/pinctrl-s32cc.c | 31 ++++++------------
drivers/pinctrl/pinconf-generic.c | 7 ++--
drivers/pinctrl/pinctrl-at91-pio4.c | 7 ++--
drivers/pinctrl/pinctrl-at91.c | 14 +++-----
drivers/pinctrl/pinctrl-equilibrium.c | 21 +++---------
drivers/pinctrl/pinctrl-k210.c | 7 ++--
drivers/pinctrl/pinctrl-rockchip.c | 11 ++-----
drivers/pinctrl/pinctrl-st.c | 37 +++++++---------------
drivers/pinctrl/renesas/pinctrl-rza1.c | 14 +++-----
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 7 ++--
drivers/pinctrl/renesas/pinctrl-rzn1.c | 23 ++++----------
drivers/pinctrl/renesas/pinctrl-rzv2m.c | 7 ++--
drivers/pinctrl/renesas/pinctrl.c | 7 ++--
drivers/pinctrl/samsung/pinctrl-exynos.c | 16 +++-------
drivers/pinctrl/samsung/pinctrl-samsung.c | 19 +++--------
drivers/pinctrl/spear/pinctrl-spear.c | 13 +++-----
drivers/pinctrl/sprd/pinctrl-sprd.c | 14 +++-----
drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c | 27 +++++++---------
drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c | 18 +++++------
drivers/pinctrl/stm32/pinctrl-stm32.c | 4 +--
drivers/pinctrl/tegra/pinctrl-tegra-xusb.c | 7 ++--
drivers/pinctrl/tegra/pinctrl-tegra.c | 4 +--
drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 37 ++++++++--------------
31 files changed, 133 insertions(+), 298 deletions(-)
---
base-commit: bb7a2467e6beef44a80a17d45ebf2931e7631083
change-id: 20240429-pinctrl-cleanup-e4d461c32648
Best regards,
--
Peng Fan <[email protected]>
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Fixes: 6118714275f0 ("pinctrl: core: Fix pinctrl_register_and_init() with pinctrl_enable()")
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 37 +++++++++++++--------------------
1 file changed, 14 insertions(+), 23 deletions(-)
diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
index 040f2c46a868..1032bc9c36aa 100644
--- a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
+++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
@@ -822,53 +822,48 @@ MODULE_DEVICE_TABLE(of, ti_iodelay_of_match);
static int ti_iodelay_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct device_node *np = of_node_get(dev->of_node);
+ struct device_node *np __free(device_node) = of_node_get(dev->of_node);
struct resource *res;
struct ti_iodelay_device *iod;
- int ret = 0;
+ int ret;
if (!np) {
- ret = -EINVAL;
dev_err(dev, "No OF node\n");
- goto exit_out;
+ return -EINVAL;
}
iod = devm_kzalloc(dev, sizeof(*iod), GFP_KERNEL);
- if (!iod) {
- ret = -ENOMEM;
- goto exit_out;
- }
+ if (!iod)
+ return -ENOMEM;
+
iod->dev = dev;
iod->reg_data = device_get_match_data(dev);
if (!iod->reg_data) {
- ret = -EINVAL;
dev_err(dev, "No DATA match\n");
- goto exit_out;
+ return -EINVAL;
}
/* So far We can assume there is only 1 bank of registers */
iod->reg_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
- if (IS_ERR(iod->reg_base)) {
- ret = PTR_ERR(iod->reg_base);
- goto exit_out;
- }
+ if (IS_ERR(iod->reg_base))
+ return PTR_ERR(iod->reg_base);
+
iod->phys_base = res->start;
iod->regmap = devm_regmap_init_mmio(dev, iod->reg_base,
iod->reg_data->regmap_config);
if (IS_ERR(iod->regmap)) {
dev_err(dev, "Regmap MMIO init failed.\n");
- ret = PTR_ERR(iod->regmap);
- goto exit_out;
+ return PTR_ERR(iod->regmap);
}
ret = ti_iodelay_pinconf_init_dev(iod);
if (ret)
- goto exit_out;
+ return ret;
ret = ti_iodelay_alloc_pins(dev, iod, res->start);
if (ret)
- goto exit_out;
+ return ret;
iod->desc.pctlops = &ti_iodelay_pinctrl_ops;
/* no pinmux ops - we are pinconf */
@@ -879,16 +874,12 @@ static int ti_iodelay_probe(struct platform_device *pdev)
ret = pinctrl_register_and_init(&iod->desc, dev, iod, &iod->pctl);
if (ret) {
dev_err(dev, "Failed to register pinctrl\n");
- goto exit_out;
+ return ret;
}
platform_set_drvdata(pdev, iod);
return pinctrl_enable(iod->pctl);
-
-exit_out:
- of_node_put(np);
- return ret;
}
/**
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/tegra/pinctrl-tegra-xusb.c | 7 ++-----
drivers/pinctrl/tegra/pinctrl-tegra.c | 4 +---
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra-xusb.c b/drivers/pinctrl/tegra/pinctrl-tegra-xusb.c
index 96ef57a7d385..49c5edeba87f 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra-xusb.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra-xusb.c
@@ -238,20 +238,17 @@ static int tegra_xusb_padctl_dt_node_to_map(struct pinctrl_dev *pinctrl,
{
struct tegra_xusb_padctl *padctl = pinctrl_dev_get_drvdata(pinctrl);
unsigned int reserved_maps = 0;
- struct device_node *np;
int err;
*num_maps = 0;
*maps = NULL;
- for_each_child_of_node(parent, np) {
+ for_each_child_of_node_scoped(parent, np) {
err = tegra_xusb_padctl_parse_subnode(padctl, np, maps,
&reserved_maps,
num_maps);
- if (err < 0) {
- of_node_put(np);
+ if (err < 0)
return err;
- }
}
return 0;
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index ccfa3870a67d..c83e5a65e680 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -188,20 +188,18 @@ static int tegra_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
unsigned *num_maps)
{
unsigned reserved_maps;
- struct device_node *np;
int ret;
reserved_maps = 0;
*map = NULL;
*num_maps = 0;
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = tegra_pinctrl_dt_subnode_to_map(pctldev, np, map,
&reserved_maps, num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map,
*num_maps);
- of_node_put(np);
return ret;
}
}
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/stm32/pinctrl-stm32.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
index 978ccdbaf3d3..a8673739871d 100644
--- a/drivers/pinctrl/stm32/pinctrl-stm32.c
+++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
@@ -670,7 +670,6 @@ static int stm32_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
struct device_node *np_config,
struct pinctrl_map **map, unsigned *num_maps)
{
- struct device_node *np;
unsigned reserved_maps;
int ret;
@@ -678,12 +677,11 @@ static int stm32_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
*num_maps = 0;
reserved_maps = 0;
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = stm32_pctrl_dt_subnode_to_map(pctldev, np, map,
&reserved_maps, num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map, *num_maps);
- of_node_put(np);
return ret;
}
}
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Acked-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c | 27 +++++++++-------------
drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c | 18 +++++++--------
2 files changed, 19 insertions(+), 26 deletions(-)
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c
index 6df7a310c7ed..27f99183d994 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c
@@ -480,7 +480,6 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
{
struct starfive_pinctrl *sfp = pinctrl_dev_get_drvdata(pctldev);
struct device *dev = sfp->gc.parent;
- struct device_node *child;
struct pinctrl_map *map;
const char **pgnames;
const char *grpname;
@@ -492,20 +491,18 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
nmaps = 0;
ngroups = 0;
- for_each_available_child_of_node(np, child) {
+ for_each_available_child_of_node_scoped(np, child) {
int npinmux = of_property_count_u32_elems(child, "pinmux");
int npins = of_property_count_u32_elems(child, "pins");
if (npinmux > 0 && npins > 0) {
dev_err(dev, "invalid pinctrl group %pOFn.%pOFn: both pinmux and pins set\n",
np, child);
- of_node_put(child);
return -EINVAL;
}
if (npinmux == 0 && npins == 0) {
dev_err(dev, "invalid pinctrl group %pOFn.%pOFn: neither pinmux nor pins set\n",
np, child);
- of_node_put(child);
return -EINVAL;
}
@@ -527,14 +524,14 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
nmaps = 0;
ngroups = 0;
mutex_lock(&sfp->mutex);
- for_each_available_child_of_node(np, child) {
+ for_each_available_child_of_node_scoped(np, child) {
int npins;
int i;
grpname = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn", np, child);
if (!grpname) {
ret = -ENOMEM;
- goto put_child;
+ goto free_map;
}
pgnames[ngroups++] = grpname;
@@ -543,18 +540,18 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
if (!pins) {
ret = -ENOMEM;
- goto put_child;
+ goto free_map;
}
pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux), GFP_KERNEL);
if (!pinmux) {
ret = -ENOMEM;
- goto put_child;
+ goto free_map;
}
ret = of_property_read_u32_array(child, "pinmux", pinmux, npins);
if (ret)
- goto put_child;
+ goto free_map;
for (i = 0; i < npins; i++) {
unsigned int gpio = starfive_pinmux_to_gpio(pinmux[i]);
@@ -570,7 +567,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
if (!pins) {
ret = -ENOMEM;
- goto put_child;
+ goto free_map;
}
pinmux = NULL;
@@ -580,18 +577,18 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
ret = of_property_read_u32_index(child, "pins", i, &v);
if (ret)
- goto put_child;
+ goto free_map;
pins[i] = v;
}
} else {
ret = -EINVAL;
- goto put_child;
+ goto free_map;
}
ret = pinctrl_generic_add_group(pctldev, grpname, pins, npins, pinmux);
if (ret < 0) {
dev_err(dev, "error adding group %s: %d\n", grpname, ret);
- goto put_child;
+ goto free_map;
}
ret = pinconf_generic_parse_dt_config(child, pctldev,
@@ -600,7 +597,7 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
if (ret) {
dev_err(dev, "error parsing pin config of group %s: %d\n",
grpname, ret);
- goto put_child;
+ goto free_map;
}
/* don't create a map if there are no pinconf settings */
@@ -623,8 +620,6 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
mutex_unlock(&sfp->mutex);
return 0;
-put_child:
- of_node_put(child);
free_map:
pinctrl_utils_free_map(pctldev, map, nmaps);
mutex_unlock(&sfp->mutex);
diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
index 9609eb1ecc3d..4ce080caa233 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
@@ -150,7 +150,7 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
nmaps = 0;
ngroups = 0;
mutex_lock(&sfp->mutex);
- for_each_available_child_of_node(np, child) {
+ for_each_available_child_of_node_scoped(np, child) {
int npins = of_property_count_u32_elems(child, "pinmux");
int *pins;
u32 *pinmux;
@@ -161,13 +161,13 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
"invalid pinctrl group %pOFn.%pOFn: pinmux not set\n",
np, child);
ret = -EINVAL;
- goto put_child;
+ goto free_map;
}
grpname = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn", np, child);
if (!grpname) {
ret = -ENOMEM;
- goto put_child;
+ goto free_map;
}
pgnames[ngroups++] = grpname;
@@ -175,18 +175,18 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
if (!pins) {
ret = -ENOMEM;
- goto put_child;
+ goto free_map;
}
pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux), GFP_KERNEL);
if (!pinmux) {
ret = -ENOMEM;
- goto put_child;
+ goto free_map;
}
ret = of_property_read_u32_array(child, "pinmux", pinmux, npins);
if (ret)
- goto put_child;
+ goto free_map;
for (i = 0; i < npins; i++)
pins[i] = jh7110_pinmux_pin(pinmux[i]);
@@ -200,7 +200,7 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
pins, npins, pinmux);
if (ret < 0) {
dev_err(dev, "error adding group %s: %d\n", grpname, ret);
- goto put_child;
+ goto free_map;
}
ret = pinconf_generic_parse_dt_config(child, pctldev,
@@ -209,7 +209,7 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
if (ret) {
dev_err(dev, "error parsing pin config of group %s: %d\n",
grpname, ret);
- goto put_child;
+ goto free_map;
}
/* don't create a map if there are no pinconf settings */
@@ -233,8 +233,6 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
*num_maps = nmaps;
return 0;
-put_child:
- of_node_put(child);
free_map:
pinctrl_utils_free_map(pctldev, map, nmaps);
mutex_unlock(&sfp->mutex);
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/sprd/pinctrl-sprd.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/pinctrl/sprd/pinctrl-sprd.c b/drivers/pinctrl/sprd/pinctrl-sprd.c
index d0b6d3e655a2..c4a1d99dfed0 100644
--- a/drivers/pinctrl/sprd/pinctrl-sprd.c
+++ b/drivers/pinctrl/sprd/pinctrl-sprd.c
@@ -934,7 +934,6 @@ static int sprd_pinctrl_parse_dt(struct sprd_pinctrl *sprd_pctl)
{
struct sprd_pinctrl_soc_info *info = sprd_pctl->info;
struct device_node *np = sprd_pctl->dev->of_node;
- struct device_node *child, *sub_child;
struct sprd_pin_group *grp;
const char **temp;
int ret;
@@ -962,25 +961,20 @@ static int sprd_pinctrl_parse_dt(struct sprd_pinctrl *sprd_pctl)
temp = info->grp_names;
grp = info->groups;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = sprd_pinctrl_parse_groups(child, sprd_pctl, grp);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
*temp++ = grp->name;
grp++;
if (of_get_child_count(child) > 0) {
- for_each_child_of_node(child, sub_child) {
+ for_each_child_of_node_scoped(child, sub_child) {
ret = sprd_pinctrl_parse_groups(sub_child,
sprd_pctl, grp);
- if (ret) {
- of_node_put(sub_child);
- of_node_put(child);
+ if (ret)
return ret;
- }
*temp++ = grp->name;
grp++;
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/spear/pinctrl-spear.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/pinctrl/spear/pinctrl-spear.c b/drivers/pinctrl/spear/pinctrl-spear.c
index b8caaa5a2d4e..a8c5fe973cd4 100644
--- a/drivers/pinctrl/spear/pinctrl-spear.c
+++ b/drivers/pinctrl/spear/pinctrl-spear.c
@@ -151,24 +151,19 @@ static int spear_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
unsigned *num_maps)
{
struct spear_pmx *pmx = pinctrl_dev_get_drvdata(pctldev);
- struct device_node *np;
struct property *prop;
const char *function, *group;
int ret, index = 0, count = 0;
/* calculate number of maps required */
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = of_property_read_string(np, "st,function", &function);
- if (ret < 0) {
- of_node_put(np);
+ if (ret < 0)
return ret;
- }
ret = of_property_count_strings(np, "st,pins");
- if (ret < 0) {
- of_node_put(np);
+ if (ret < 0)
return ret;
- }
count += ret;
}
@@ -182,7 +177,7 @@ static int spear_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
if (!*map)
return -ENOMEM;
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
of_property_read_string(np, "st,function", &function);
of_property_for_each_string(np, "st,pins", prop, group) {
(*map)[index].type = PIN_MAP_TYPE_MUX_GROUP;
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/renesas/pinctrl-rza1.c | 14 ++++----------
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 7 ++-----
drivers/pinctrl/renesas/pinctrl-rzn1.c | 23 +++++++----------------
drivers/pinctrl/renesas/pinctrl-rzv2m.c | 7 ++-----
drivers/pinctrl/renesas/pinctrl.c | 7 ++-----
5 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/drivers/pinctrl/renesas/pinctrl-rza1.c b/drivers/pinctrl/renesas/pinctrl-rza1.c
index edcbe7c9ad56..6527872813dc 100644
--- a/drivers/pinctrl/renesas/pinctrl-rza1.c
+++ b/drivers/pinctrl/renesas/pinctrl-rza1.c
@@ -852,7 +852,6 @@ static const struct gpio_chip rza1_gpiochip_template = {
*/
static int rza1_dt_node_pin_count(struct device_node *np)
{
- struct device_node *child;
struct property *of_pins;
unsigned int npins;
@@ -861,12 +860,10 @@ static int rza1_dt_node_pin_count(struct device_node *np)
return of_pins->length / sizeof(u32);
npins = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
of_pins = of_find_property(child, "pinmux", NULL);
- if (!of_pins) {
- of_node_put(child);
+ if (!of_pins)
return -EINVAL;
- }
npins += of_pins->length / sizeof(u32);
}
@@ -986,7 +983,6 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev,
struct rza1_pinctrl *rza1_pctl = pinctrl_dev_get_drvdata(pctldev);
struct rza1_mux_conf *mux_confs, *mux_conf;
unsigned int *grpins, *grpin;
- struct device_node *child;
const char *grpname;
const char **fngrps;
int ret, npins;
@@ -1023,13 +1019,11 @@ static int rza1_dt_node_to_map(struct pinctrl_dev *pctldev,
ret = rza1_parse_pinmux_node(rza1_pctl, np, mux_conf, grpin);
if (ret == -ENOENT)
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = rza1_parse_pinmux_node(rza1_pctl, child, mux_conf,
grpin);
- if (ret < 0) {
- of_node_put(child);
+ if (ret < 0)
return ret;
- }
grpin += ret;
mux_conf += ret;
diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
index c3256bfde502..fc7f33d3c613 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -745,7 +745,6 @@ static int rzg2l_dt_node_to_map(struct pinctrl_dev *pctldev,
unsigned int *num_maps)
{
struct rzg2l_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
- struct device_node *child;
unsigned int index;
int ret;
@@ -753,13 +752,11 @@ static int rzg2l_dt_node_to_map(struct pinctrl_dev *pctldev,
*num_maps = 0;
index = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = rzg2l_dt_subnode_to_map(pctldev, child, np, map,
num_maps, &index);
- if (ret < 0) {
- of_node_put(child);
+ if (ret < 0)
goto done;
- }
}
if (*num_maps == 0) {
diff --git a/drivers/pinctrl/renesas/pinctrl-rzn1.c b/drivers/pinctrl/renesas/pinctrl-rzn1.c
index 4b2f107824fe..e1b4203c66c6 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzn1.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzn1.c
@@ -404,7 +404,6 @@ static int rzn1_dt_node_to_map(struct pinctrl_dev *pctldev,
struct pinctrl_map **map,
unsigned int *num_maps)
{
- struct device_node *child;
int ret;
*map = NULL;
@@ -414,12 +413,10 @@ static int rzn1_dt_node_to_map(struct pinctrl_dev *pctldev,
if (ret < 0)
return ret;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = rzn1_dt_node_to_map_one(pctldev, child, map, num_maps);
- if (ret < 0) {
- of_node_put(child);
+ if (ret < 0)
return ret;
- }
}
return 0;
@@ -760,7 +757,6 @@ static int rzn1_pinctrl_parse_functions(struct device_node *np,
{
struct rzn1_pmx_func *func;
struct rzn1_pin_group *grp;
- struct device_node *child;
unsigned int i = 0;
int ret;
@@ -793,15 +789,13 @@ static int rzn1_pinctrl_parse_functions(struct device_node *np,
ipctl->ngroups++;
}
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
func->groups[i] = child->name;
grp = &ipctl->groups[ipctl->ngroups];
grp->func = func->name;
ret = rzn1_pinctrl_parse_groups(child, grp, ipctl);
- if (ret < 0) {
- of_node_put(child);
+ if (ret < 0)
return ret;
- }
i++;
ipctl->ngroups++;
}
@@ -816,7 +810,6 @@ static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
struct rzn1_pinctrl *ipctl)
{
struct device_node *np = pdev->dev.of_node;
- struct device_node *child;
unsigned int maxgroups = 0;
unsigned int i = 0;
int nfuncs = 0;
@@ -834,7 +827,7 @@ static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
return -ENOMEM;
ipctl->ngroups = 0;
- for_each_child_of_node(np, child)
+ for_each_child_of_node_scoped(np, child)
maxgroups += rzn1_pinctrl_count_function_groups(child);
ipctl->groups = devm_kmalloc_array(&pdev->dev,
@@ -844,12 +837,10 @@ static int rzn1_pinctrl_probe_dt(struct platform_device *pdev,
if (!ipctl->groups)
return -ENOMEM;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = rzn1_pinctrl_parse_functions(child, ipctl, i++);
- if (ret < 0) {
- of_node_put(child);
+ if (ret < 0)
return ret;
- }
}
return 0;
diff --git a/drivers/pinctrl/renesas/pinctrl-rzv2m.c b/drivers/pinctrl/renesas/pinctrl-rzv2m.c
index 0767a5ac23e0..0cae5472ac67 100644
--- a/drivers/pinctrl/renesas/pinctrl-rzv2m.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzv2m.c
@@ -388,7 +388,6 @@ static int rzv2m_dt_node_to_map(struct pinctrl_dev *pctldev,
unsigned int *num_maps)
{
struct rzv2m_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
- struct device_node *child;
unsigned int index;
int ret;
@@ -396,13 +395,11 @@ static int rzv2m_dt_node_to_map(struct pinctrl_dev *pctldev,
*num_maps = 0;
index = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = rzv2m_dt_subnode_to_map(pctldev, child, np, map,
num_maps, &index);
- if (ret < 0) {
- of_node_put(child);
+ if (ret < 0)
goto done;
- }
}
if (*num_maps == 0) {
diff --git a/drivers/pinctrl/renesas/pinctrl.c b/drivers/pinctrl/renesas/pinctrl.c
index 4d9d58fc1356..03e9bdbc82b9 100644
--- a/drivers/pinctrl/renesas/pinctrl.c
+++ b/drivers/pinctrl/renesas/pinctrl.c
@@ -241,7 +241,6 @@ static int sh_pfc_dt_node_to_map(struct pinctrl_dev *pctldev,
{
struct sh_pfc_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
struct device *dev = pmx->pfc->dev;
- struct device_node *child;
unsigned int index;
int ret;
@@ -249,13 +248,11 @@ static int sh_pfc_dt_node_to_map(struct pinctrl_dev *pctldev,
*num_maps = 0;
index = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = sh_pfc_dt_subnode_to_map(pctldev, child, map, num_maps,
&index);
- if (ret < 0) {
- of_node_put(child);
+ if (ret < 0)
goto done;
- }
}
/* If no mapping has been found in child nodes try the config node. */
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/pinctrl-st.c | 37 +++++++++++--------------------------
1 file changed, 11 insertions(+), 26 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
index 5d9abd6547d0..fe2d52e434db 100644
--- a/drivers/pinctrl/pinctrl-st.c
+++ b/drivers/pinctrl/pinctrl-st.c
@@ -1195,10 +1195,10 @@ static int st_pctl_dt_parse_groups(struct device_node *np,
struct property *pp;
struct device *dev = info->dev;
struct st_pinconf *conf;
- struct device_node *pins;
+ struct device_node *pins __free(device_node) = NULL;
phandle bank;
unsigned int offset;
- int i = 0, npins = 0, nr_props, ret = 0;
+ int i = 0, npins = 0, nr_props;
pins = of_get_child_by_name(np, "st,pins");
if (!pins)
@@ -1213,8 +1213,7 @@ static int st_pctl_dt_parse_groups(struct device_node *np,
npins++;
} else {
pr_warn("Invalid st,pins in %pOFn node\n", np);
- ret = -EINVAL;
- goto out_put_node;
+ return -EINVAL;
}
}
@@ -1223,10 +1222,8 @@ static int st_pctl_dt_parse_groups(struct device_node *np,
grp->pins = devm_kcalloc(dev, npins, sizeof(*grp->pins), GFP_KERNEL);
grp->pin_conf = devm_kcalloc(dev, npins, sizeof(*grp->pin_conf), GFP_KERNEL);
- if (!grp->pins || !grp->pin_conf) {
- ret = -ENOMEM;
- goto out_put_node;
- }
+ if (!grp->pins || !grp->pin_conf)
+ return -ENOMEM;
/* <bank offset mux direction rt_type rt_delay rt_clk> */
for_each_property_of_node(pins, pp) {
@@ -1260,17 +1257,13 @@ static int st_pctl_dt_parse_groups(struct device_node *np,
i++;
}
-out_put_node:
- of_node_put(pins);
-
- return ret;
+ return 0;
}
static int st_pctl_parse_functions(struct device_node *np,
struct st_pinctrl *info, u32 index, int *grp_index)
{
struct device *dev = info->dev;
- struct device_node *child;
struct st_pmx_func *func;
struct st_pctl_group *grp;
int ret, i;
@@ -1285,15 +1278,13 @@ static int st_pctl_parse_functions(struct device_node *np,
return -ENOMEM;
i = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
func->groups[i] = child->name;
grp = &info->groups[*grp_index];
*grp_index += 1;
ret = st_pctl_dt_parse_groups(child, grp, info, i++);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
}
dev_info(dev, "Function[%d\t name:%s,\tgroups:%d]\n", index, func->name, func->ngroups);
@@ -1601,7 +1592,6 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
int i = 0, j = 0, k = 0, bank;
struct pinctrl_pin_desc *pdesc;
struct device_node *np = dev->of_node;
- struct device_node *child;
int grp_index = 0;
int irq = 0;
@@ -1646,25 +1636,21 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
pctl_desc->pins = pdesc;
bank = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
if (of_property_read_bool(child, "gpio-controller")) {
const char *bank_name = NULL;
char **pin_names;
ret = st_gpiolib_register_bank(info, bank, child);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
k = info->banks[bank].range.pin_base;
bank_name = info->banks[bank].range.name;
pin_names = devm_kasprintf_strarray(dev, bank_name, ST_GPIO_PINS_PER_BANK);
- if (IS_ERR(pin_names)) {
- of_node_put(child);
+ if (IS_ERR(pin_names))
return PTR_ERR(pin_names);
- }
for (j = 0; j < ST_GPIO_PINS_PER_BANK; j++, k++) {
pdesc->number = k;
@@ -1678,7 +1664,6 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
i++, &grp_index);
if (ret) {
dev_err(dev, "No functions found.\n");
- of_node_put(child);
return ret;
}
}
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/pinctrl-rockchip.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
index 3bedf36a0019..68391d6497c9 100644
--- a/drivers/pinctrl/pinctrl-rockchip.c
+++ b/drivers/pinctrl/pinctrl-rockchip.c
@@ -3057,7 +3057,6 @@ static int rockchip_pinctrl_parse_functions(struct device_node *np,
u32 index)
{
struct device *dev = info->dev;
- struct device_node *child;
struct rockchip_pmx_func *func;
struct rockchip_pin_group *grp;
int ret;
@@ -3078,14 +3077,12 @@ static int rockchip_pinctrl_parse_functions(struct device_node *np,
if (!func->groups)
return -ENOMEM;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
func->groups[i] = child->name;
grp = &info->groups[grp_index++];
ret = rockchip_pinctrl_parse_groups(child, grp, info, i++);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
}
return 0;
@@ -3096,7 +3093,6 @@ static int rockchip_pinctrl_parse_dt(struct platform_device *pdev,
{
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
- struct device_node *child;
int ret;
int i;
@@ -3115,14 +3111,13 @@ static int rockchip_pinctrl_parse_dt(struct platform_device *pdev,
i = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
if (of_match_node(rockchip_bank_match, child))
continue;
ret = rockchip_pinctrl_parse_functions(child, info, i++);
if (ret) {
dev_err(dev, "failed to parse function\n");
- of_node_put(child);
return ret;
}
}
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/pinctrl-equilibrium.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-equilibrium.c b/drivers/pinctrl/pinctrl-equilibrium.c
index 6e1be38865c3..e727257bb697 100644
--- a/drivers/pinctrl/pinctrl-equilibrium.c
+++ b/drivers/pinctrl/pinctrl-equilibrium.c
@@ -588,14 +588,13 @@ static int funcs_utils(struct device *dev, struct eqbr_pmx_func *funcs,
unsigned int *nr_funcs, funcs_util_ops op)
{
struct device_node *node = dev->of_node;
- struct device_node *np;
struct property *prop;
const char *fn_name;
unsigned int fid;
int i, j;
i = 0;
- for_each_child_of_node(node, np) {
+ for_each_child_of_node_scoped(node, np) {
prop = of_find_property(np, "groups", NULL);
if (!prop)
continue;
@@ -633,7 +632,6 @@ static int funcs_utils(struct device *dev, struct eqbr_pmx_func *funcs,
break;
default:
- of_node_put(np);
return -EINVAL;
}
i++;
@@ -706,11 +704,10 @@ static int eqbr_build_groups(struct eqbr_pinctrl_drv_data *drvdata)
struct device_node *node = dev->of_node;
unsigned int *pins, *pinmux, pin_id, pinmux_id;
struct pingroup group, *grp = &group;
- struct device_node *np;
struct property *prop;
int j, err;
- for_each_child_of_node(node, np) {
+ for_each_child_of_node_scoped(node, np) {
prop = of_find_property(np, "groups", NULL);
if (!prop)
continue;
@@ -718,42 +715,35 @@ static int eqbr_build_groups(struct eqbr_pinctrl_drv_data *drvdata)
err = of_property_count_u32_elems(np, "pins");
if (err < 0) {
dev_err(dev, "No pins in the group: %s\n", prop->name);
- of_node_put(np);
return err;
}
grp->npins = err;
grp->name = prop->value;
pins = devm_kcalloc(dev, grp->npins, sizeof(*pins), GFP_KERNEL);
- if (!pins) {
- of_node_put(np);
+ if (!pins)
return -ENOMEM;
- }
+
grp->pins = pins;
pinmux = devm_kcalloc(dev, grp->npins, sizeof(*pinmux), GFP_KERNEL);
- if (!pinmux) {
- of_node_put(np);
+ if (!pinmux)
return -ENOMEM;
- }
for (j = 0; j < grp->npins; j++) {
if (of_property_read_u32_index(np, "pins", j, &pin_id)) {
dev_err(dev, "Group %s: Read intel pins id failed\n",
grp->name);
- of_node_put(np);
return -EINVAL;
}
if (pin_id >= drvdata->pctl_desc.npins) {
dev_err(dev, "Group %s: Invalid pin ID, idx: %d, pin %u\n",
grp->name, j, pin_id);
- of_node_put(np);
return -EINVAL;
}
pins[j] = pin_id;
if (of_property_read_u32_index(np, "pinmux", j, &pinmux_id)) {
dev_err(dev, "Group %s: Read intel pinmux id failed\n",
grp->name);
- of_node_put(np);
return -EINVAL;
}
pinmux[j] = pinmux_id;
@@ -764,7 +754,6 @@ static int eqbr_build_groups(struct eqbr_pinctrl_drv_data *drvdata)
pinmux);
if (err < 0) {
dev_err(dev, "Failed to register group %s\n", grp->name);
- of_node_put(np);
return err;
}
memset(&group, 0, sizeof(group));
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/pinctrl-at91-pio4.c | 7 ++-----
drivers/pinctrl/pinctrl-at91.c | 14 ++++----------
2 files changed, 6 insertions(+), 15 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c b/drivers/pinctrl/pinctrl-at91-pio4.c
index a27c01fcbb47..8b01d312305a 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -632,7 +632,6 @@ static int atmel_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
struct pinctrl_map **map,
unsigned int *num_maps)
{
- struct device_node *np;
unsigned int reserved_maps;
int ret;
@@ -648,13 +647,11 @@ static int atmel_pctl_dt_node_to_map(struct pinctrl_dev *pctldev,
ret = atmel_pctl_dt_subnode_to_map(pctldev, np_config, map,
&reserved_maps, num_maps);
if (ret) {
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = atmel_pctl_dt_subnode_to_map(pctldev, np, map,
&reserved_maps, num_maps);
- if (ret < 0) {
- of_node_put(np);
+ if (ret < 0)
break;
- }
}
}
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index 5aa9d5c533c6..b3c3f5fb2e2e 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -1244,7 +1244,6 @@ static int at91_pinctrl_parse_groups(struct device_node *np,
static int at91_pinctrl_parse_functions(struct device_node *np,
struct at91_pinctrl *info, u32 index)
{
- struct device_node *child;
struct at91_pmx_func *func;
struct at91_pin_group *grp;
int ret;
@@ -1267,14 +1266,12 @@ static int at91_pinctrl_parse_functions(struct device_node *np,
if (!func->groups)
return -ENOMEM;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
func->groups[i] = child->name;
grp = &info->groups[grp_index++];
ret = at91_pinctrl_parse_groups(child, grp, info, i++);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
}
return 0;
@@ -1296,7 +1293,6 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
int i, j, ngpio_chips_enabled = 0;
uint32_t *tmp;
struct device_node *np = dev->of_node;
- struct device_node *child;
if (!np)
return -ENODEV;
@@ -1349,14 +1345,12 @@ static int at91_pinctrl_probe_dt(struct platform_device *pdev,
i = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
if (of_device_is_compatible(child, gpio_compat))
continue;
ret = at91_pinctrl_parse_functions(child, info, i++);
- if (ret) {
- of_node_put(child);
+ if (ret)
return dev_err_probe(dev, ret, "failed to parse function\n");
- }
}
return 0;
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/nxp/pinctrl-s32cc.c | 31 ++++++++++---------------------
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index f0cad2c501f7..df3e5d82da4b 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -268,28 +268,23 @@ static int s32_dt_node_to_map(struct pinctrl_dev *pctldev,
unsigned int *num_maps)
{
unsigned int reserved_maps;
- struct device_node *np;
- int ret = 0;
+ int ret;
reserved_maps = 0;
*map = NULL;
*num_maps = 0;
- for_each_available_child_of_node(np_config, np) {
+ for_each_available_child_of_node_scoped(np_config, np) {
ret = s32_dt_group_node_to_map(pctldev, np, map,
&reserved_maps, num_maps,
np_config->name);
if (ret < 0) {
- of_node_put(np);
- break;
+ pinctrl_utils_free_map(pctldev, *map, *num_maps);
+ return ret;
}
}
- if (ret)
- pinctrl_utils_free_map(pctldev, *map, *num_maps);
-
- return ret;
-
+ return 0;
}
static const struct pinctrl_ops s32_pctrl_ops = {
@@ -786,7 +781,6 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
struct s32_pinctrl_soc_info *info,
u32 index)
{
- struct device_node *child;
struct pinfunction *func;
struct s32_pin_group *grp;
const char **groups;
@@ -810,14 +804,12 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
if (!groups)
return -ENOMEM;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
groups[i] = child->name;
grp = &info->groups[info->grp_index++];
ret = s32_pinctrl_parse_groups(child, grp, info);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
i++;
}
@@ -831,7 +823,6 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
{
struct s32_pinctrl_soc_info *info = ipctl->info;
struct device_node *np = pdev->dev.of_node;
- struct device_node *child;
struct resource *res;
struct regmap *map;
void __iomem *base;
@@ -889,7 +880,7 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
return -ENOMEM;
info->ngroups = 0;
- for_each_child_of_node(np, child)
+ for_each_child_of_node_scoped(np, child)
info->ngroups += of_get_child_count(child);
info->groups = devm_kcalloc(&pdev->dev, info->ngroups,
@@ -898,12 +889,10 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
return -ENOMEM;
i = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = s32_pinctrl_parse_functions(child, info, i++);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
}
return 0;
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/nomadik/pinctrl-abx500.c | 4 +---
drivers/pinctrl/nomadik/pinctrl-nomadik.c | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/nomadik/pinctrl-abx500.c b/drivers/pinctrl/nomadik/pinctrl-abx500.c
index 80e3ac333136..47f62c89955a 100644
--- a/drivers/pinctrl/nomadik/pinctrl-abx500.c
+++ b/drivers/pinctrl/nomadik/pinctrl-abx500.c
@@ -811,19 +811,17 @@ static int abx500_dt_node_to_map(struct pinctrl_dev *pctldev,
struct pinctrl_map **map, unsigned *num_maps)
{
unsigned reserved_maps;
- struct device_node *np;
int ret;
reserved_maps = 0;
*map = NULL;
*num_maps = 0;
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = abx500_dt_subnode_to_map(pctldev, np, map,
&reserved_maps, num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map, *num_maps);
- of_node_put(np);
return ret;
}
}
diff --git a/drivers/pinctrl/nomadik/pinctrl-nomadik.c b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
index cb0f0d5a5e45..fa78d5ecc685 100644
--- a/drivers/pinctrl/nomadik/pinctrl-nomadik.c
+++ b/drivers/pinctrl/nomadik/pinctrl-nomadik.c
@@ -804,19 +804,17 @@ static int nmk_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
unsigned int *num_maps)
{
unsigned int reserved_maps;
- struct device_node *np;
int ret;
reserved_maps = 0;
*map = NULL;
*num_maps = 0;
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = nmk_pinctrl_dt_subnode_to_map(pctldev, np, map,
&reserved_maps, num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map, *num_maps);
- of_node_put(np);
return ret;
}
}
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/mediatek/pinctrl-mtk-common.c | 4 +---
drivers/pinctrl/mediatek/pinctrl-paris.c | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
index d39afc122516..91edb539925a 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mtk-common.c
@@ -621,7 +621,6 @@ static int mtk_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
struct device_node *np_config,
struct pinctrl_map **map, unsigned *num_maps)
{
- struct device_node *np;
unsigned reserved_maps;
int ret;
@@ -629,12 +628,11 @@ static int mtk_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
*num_maps = 0;
reserved_maps = 0;
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = mtk_pctrl_dt_subnode_to_map(pctldev, np, map,
&reserved_maps, num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map, *num_maps);
- of_node_put(np);
return ret;
}
}
diff --git a/drivers/pinctrl/mediatek/pinctrl-paris.c b/drivers/pinctrl/mediatek/pinctrl-paris.c
index b19bc391705e..e12316c42698 100644
--- a/drivers/pinctrl/mediatek/pinctrl-paris.c
+++ b/drivers/pinctrl/mediatek/pinctrl-paris.c
@@ -536,7 +536,6 @@ static int mtk_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
struct pinctrl_map **map,
unsigned *num_maps)
{
- struct device_node *np;
unsigned reserved_maps;
int ret;
@@ -544,13 +543,12 @@ static int mtk_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
*num_maps = 0;
reserved_maps = 0;
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = mtk_pctrl_dt_subnode_to_map(pctldev, np, map,
&reserved_maps,
num_maps);
if (ret < 0) {
pinctrl_utils_free_map(pctldev, *map, *num_maps);
- of_node_put(np);
return ret;
}
}
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/freescale/pinctrl-imx.c | 25 +++++++------------------
drivers/pinctrl/freescale/pinctrl-imx1-core.c | 16 +++++-----------
drivers/pinctrl/freescale/pinctrl-mxs.c | 14 ++++----------
3 files changed, 16 insertions(+), 39 deletions(-)
diff --git a/drivers/pinctrl/freescale/pinctrl-imx.c b/drivers/pinctrl/freescale/pinctrl-imx.c
index 2d3d80921c0d..2b7448e3cd65 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx.c
@@ -580,7 +580,6 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
u32 index)
{
struct pinctrl_dev *pctl = ipctl->pctl;
- struct device_node *child;
struct function_desc *func;
struct group_desc *grp;
const char **group_names;
@@ -605,17 +604,15 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
if (!group_names)
return -ENOMEM;
i = 0;
- for_each_child_of_node(np, child)
+ for_each_child_of_node_scoped(np, child)
group_names[i++] = child->name;
func->group_names = group_names;
i = 0;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
grp = devm_kzalloc(ipctl->dev, sizeof(*grp), GFP_KERNEL);
- if (!grp) {
- of_node_put(child);
+ if (!grp)
return -ENOMEM;
- }
mutex_lock(&ipctl->mutex);
radix_tree_insert(&pctl->pin_group_tree,
@@ -635,21 +632,13 @@ static int imx_pinctrl_parse_functions(struct device_node *np,
*/
static bool imx_pinctrl_dt_is_flat_functions(struct device_node *np)
{
- struct device_node *function_np;
- struct device_node *pinctrl_np;
-
- for_each_child_of_node(np, function_np) {
- if (of_property_read_bool(function_np, "fsl,pins")) {
- of_node_put(function_np);
+ for_each_child_of_node_scoped(np, function_np) {
+ if (of_property_read_bool(function_np, "fsl,pins"))
return true;
- }
- for_each_child_of_node(function_np, pinctrl_np) {
- if (of_property_read_bool(pinctrl_np, "fsl,pins")) {
- of_node_put(pinctrl_np);
- of_node_put(function_np);
+ for_each_child_of_node_scoped(function_np, pinctrl_np) {
+ if (of_property_read_bool(pinctrl_np, "fsl,pins"))
return false;
- }
}
}
diff --git a/drivers/pinctrl/freescale/pinctrl-imx1-core.c b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
index 90c696046b38..af1ccfc90bff 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx1-core.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
@@ -508,7 +508,6 @@ static int imx1_pinctrl_parse_functions(struct device_node *np,
struct imx1_pinctrl_soc_info *info,
u32 index)
{
- struct device_node *child;
struct imx1_pmx_func *func;
struct imx1_pin_group *grp;
int ret;
@@ -531,14 +530,12 @@ static int imx1_pinctrl_parse_functions(struct device_node *np,
if (!func->groups)
return -ENOMEM;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
func->groups[i] = child->name;
grp = &info->groups[grp_index++];
ret = imx1_pinctrl_parse_groups(child, grp, info, i++);
- if (ret == -ENOMEM) {
- of_node_put(child);
+ if (ret == -ENOMEM)
return ret;
- }
}
return 0;
@@ -548,7 +545,6 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
struct imx1_pinctrl *pctl, struct imx1_pinctrl_soc_info *info)
{
struct device_node *np = pdev->dev.of_node;
- struct device_node *child;
int ret;
u32 nfuncs = 0;
u32 ngroups = 0;
@@ -557,7 +553,7 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
if (!np)
return -ENODEV;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
++nfuncs;
ngroups += of_get_child_count(child);
}
@@ -579,12 +575,10 @@ static int imx1_pinctrl_parse_dt(struct platform_device *pdev,
if (!info->functions || !info->groups)
return -ENOMEM;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
ret = imx1_pinctrl_parse_functions(child, info, ifunc++);
- if (ret == -ENOMEM) {
- of_node_put(child);
+ if (ret == -ENOMEM)
return -ENOMEM;
- }
}
return 0;
diff --git a/drivers/pinctrl/freescale/pinctrl-mxs.c b/drivers/pinctrl/freescale/pinctrl-mxs.c
index e77311f26262..aee70fa55bec 100644
--- a/drivers/pinctrl/freescale/pinctrl-mxs.c
+++ b/drivers/pinctrl/freescale/pinctrl-mxs.c
@@ -490,16 +490,14 @@ static int mxs_pinctrl_probe_dt(struct platform_device *pdev,
/* Get groups for each function */
idxf = 0;
fn = fnull;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
if (is_mxs_gpio(child))
continue;
if (of_property_read_u32(child, "reg", &val)) {
ret = mxs_pinctrl_parse_group(pdev, child,
idxg++, NULL);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
continue;
}
@@ -509,19 +507,15 @@ static int mxs_pinctrl_probe_dt(struct platform_device *pdev,
f->ngroups,
sizeof(*f->groups),
GFP_KERNEL);
- if (!f->groups) {
- of_node_put(child);
+ if (!f->groups)
return -ENOMEM;
- }
fn = child->name;
i = 0;
}
ret = mxs_pinctrl_parse_group(pdev, child, idxg++,
&f->groups[i++]);
- if (ret) {
- of_node_put(child);
+ if (ret)
return ret;
- }
}
return 0;
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/bcm/pinctrl-bcm63xx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pinctrl/bcm/pinctrl-bcm63xx.c b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
index e1285fe2fbc0..59d2ce8462d8 100644
--- a/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
+++ b/drivers/pinctrl/bcm/pinctrl-bcm63xx.c
@@ -67,7 +67,6 @@ int bcm63xx_pinctrl_probe(struct platform_device *pdev,
{
struct device *dev = &pdev->dev;
struct bcm63xx_pinctrl *pc;
- struct device_node *node;
int err;
pc = devm_kzalloc(dev, sizeof(*pc), GFP_KERNEL);
@@ -94,12 +93,11 @@ int bcm63xx_pinctrl_probe(struct platform_device *pdev,
if (IS_ERR(pc->pctl_dev))
return PTR_ERR(pc->pctl_dev);
- for_each_child_of_node(dev->parent->of_node, node) {
+ for_each_child_of_node_scoped(dev->parent->of_node, node) {
if (of_match_node(bcm63xx_gpio_of_match, node)) {
err = bcm63xx_gpio_probe(dev, node, soc, pc);
if (err) {
dev_err(dev, "could not add GPIO chip\n");
- of_node_put(node);
return err;
}
}
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/pinconf-generic.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 80de389199bd..a499b8af5c1f 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -382,7 +382,6 @@ int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
unsigned int *num_maps, enum pinctrl_map_type type)
{
unsigned int reserved_maps;
- struct device_node *np;
int ret;
reserved_maps = 0;
@@ -394,13 +393,11 @@ int pinconf_generic_dt_node_to_map(struct pinctrl_dev *pctldev,
if (ret < 0)
goto exit;
- for_each_available_child_of_node(np_config, np) {
+ for_each_available_child_of_node_scoped(np_config, np) {
ret = pinconf_generic_dt_subnode_to_map(pctldev, np, map,
&reserved_maps, num_maps, type);
- if (ret < 0) {
- of_node_put(np);
+ if (ret < 0)
goto exit;
- }
}
return 0;
--
2.37.1
From: Peng Fan <[email protected]>
of_get_next_child() will increase refcount of the returned node, need
use of_node_put() on it when done.
Per current implementation, 'child' will be override by
for_each_child_of_node(np, child), so use of_get_child_count to avoid
refcount leakage.
Fixes: 17723111e64f ("pinctrl: add pinctrl-mxs support")
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/freescale/pinctrl-mxs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/freescale/pinctrl-mxs.c b/drivers/pinctrl/freescale/pinctrl-mxs.c
index aee70fa55bec..edb242d30609 100644
--- a/drivers/pinctrl/freescale/pinctrl-mxs.c
+++ b/drivers/pinctrl/freescale/pinctrl-mxs.c
@@ -413,8 +413,8 @@ static int mxs_pinctrl_probe_dt(struct platform_device *pdev,
int ret;
u32 val;
- child = of_get_next_child(np, NULL);
- if (!child) {
+ val = of_get_child_count(np);
+ if (val == 0) {
dev_err(&pdev->dev, "no group is defined\n");
return -ENOENT;
}
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/pinctrl-k210.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-k210.c b/drivers/pinctrl/pinctrl-k210.c
index b6d1ed9ec9a3..2753e14c3e38 100644
--- a/drivers/pinctrl/pinctrl-k210.c
+++ b/drivers/pinctrl/pinctrl-k210.c
@@ -849,7 +849,6 @@ static int k210_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
unsigned int *num_maps)
{
unsigned int reserved_maps;
- struct device_node *np;
int ret;
reserved_maps = 0;
@@ -861,13 +860,11 @@ static int k210_pinctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
if (ret < 0)
goto err;
- for_each_available_child_of_node(np_config, np) {
+ for_each_available_child_of_node_scoped(np_config, np) {
ret = k210_pinctrl_dt_subnode_to_map(pctldev, np, map,
&reserved_maps, num_maps);
- if (ret < 0) {
- of_node_put(np);
+ if (ret < 0)
goto err;
- }
}
return 0;
--
2.37.1
From: Peng Fan <[email protected]>
Use scope based of_node_put() cleanup to simplify code.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/pinctrl/samsung/pinctrl-exynos.c | 16 ++++------------
drivers/pinctrl/samsung/pinctrl-samsung.c | 19 +++++--------------
2 files changed, 9 insertions(+), 26 deletions(-)
diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
index 871c1eb46ddf..3775999536e2 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -582,7 +582,7 @@ static void exynos_irq_demux_eint16_31(struct irq_desc *desc)
__init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)
{
struct device *dev = d->dev;
- struct device_node *wkup_np = NULL;
+ struct device_node *wkup_np __free(device_node) = NULL;
struct device_node *np;
struct samsung_pin_bank *bank;
struct exynos_weint_data *weint_data;
@@ -612,17 +612,14 @@ __init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)
bank->irq_chip = devm_kmemdup(dev, irq_chip, sizeof(*irq_chip),
GFP_KERNEL);
- if (!bank->irq_chip) {
- of_node_put(wkup_np);
+ if (!bank->irq_chip)
return -ENOMEM;
- }
bank->irq_chip->chip.name = bank->name;
bank->irq_domain = irq_domain_create_linear(bank->fwnode,
bank->nr_pins, &exynos_eint_irqd_ops, bank);
if (!bank->irq_domain) {
dev_err(dev, "wkup irq domain add failed\n");
- of_node_put(wkup_np);
return -ENXIO;
}
@@ -635,10 +632,8 @@ __init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)
weint_data = devm_kcalloc(dev,
bank->nr_pins, sizeof(*weint_data),
GFP_KERNEL);
- if (!weint_data) {
- of_node_put(wkup_np);
+ if (!weint_data)
return -ENOMEM;
- }
for (idx = 0; idx < bank->nr_pins; ++idx) {
irq = irq_of_parse_and_map(to_of_node(bank->fwnode), idx);
@@ -655,13 +650,10 @@ __init int exynos_eint_wkup_init(struct samsung_pinctrl_drv_data *d)
}
}
- if (!muxed_banks) {
- of_node_put(wkup_np);
+ if (!muxed_banks)
return 0;
- }
irq = irq_of_parse_and_map(wkup_np, 0);
- of_node_put(wkup_np);
if (!irq) {
dev_err(dev, "irq number for muxed EINTs not found\n");
return 0;
diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
index ed07e23e0912..0d4d7ebf7145 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.c
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
@@ -250,7 +250,6 @@ static int samsung_dt_node_to_map(struct pinctrl_dev *pctldev,
{
struct samsung_pinctrl_drv_data *drvdata;
unsigned reserved_maps;
- struct device_node *np;
int ret;
drvdata = pinctrl_dev_get_drvdata(pctldev);
@@ -265,12 +264,11 @@ static int samsung_dt_node_to_map(struct pinctrl_dev *pctldev,
&reserved_maps,
num_maps);
- for_each_child_of_node(np_config, np) {
+ for_each_child_of_node_scoped(np_config, np) {
ret = samsung_dt_subnode_to_map(drvdata, pctldev->dev, np, map,
&reserved_maps, num_maps);
if (ret < 0) {
samsung_dt_free_map(pctldev, *map, *num_maps);
- of_node_put(np);
return ret;
}
}
@@ -791,16 +789,12 @@ static struct samsung_pmx_func *samsung_pinctrl_create_functions(
* and create pin groups and pin function lists.
*/
func_cnt = 0;
- for_each_child_of_node(dev_np, cfg_np) {
- struct device_node *func_np;
-
+ for_each_child_of_node_scoped(dev_np, cfg_np) {
if (!of_get_child_count(cfg_np)) {
ret = samsung_pinctrl_create_function(dev, drvdata,
cfg_np, func);
- if (ret < 0) {
- of_node_put(cfg_np);
+ if (ret < 0)
return ERR_PTR(ret);
- }
if (ret > 0) {
++func;
++func_cnt;
@@ -808,14 +802,11 @@ static struct samsung_pmx_func *samsung_pinctrl_create_functions(
continue;
}
- for_each_child_of_node(cfg_np, func_np) {
+ for_each_child_of_node_scoped(cfg_np, func_np) {
ret = samsung_pinctrl_create_function(dev, drvdata,
func_np, func);
- if (ret < 0) {
- of_node_put(func_np);
- of_node_put(cfg_np);
+ if (ret < 0)
return ERR_PTR(ret);
- }
if (ret > 0) {
++func;
++func_cnt;
--
2.37.1
Hi Peng,
On Sat, May 4, 2024 at 3:14 PM Peng Fan (OSS) <[email protected]> wrote:
> From: Peng Fan <[email protected]>
>
> Use scope based of_node_put() cleanup to simplify code.
>
> Signed-off-by: Peng Fan <[email protected]>
Thanks for your patch!
Reviewed-by: Geert Uytterhoeven <[email protected]>
Acked-by: Geert Uytterhoeven <[email protected]>
> --- a/drivers/pinctrl/renesas/pinctrl-rzn1.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzn1.c
You missed one trivial conversion, presumably because no error handling
and thus no of_node_put() is involved?
@@ -737,13 +737,12 @@ static int rzn1_pinctrl_parse_groups(struct
device_node *np,
static int rzn1_pinctrl_count_function_groups(struct device_node *np)
{
- struct device_node *child;
int count = 0;
if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0)
count++;
- for_each_child_of_node(np, child) {
+ for_each_child_of_node_scoped(np, child) {
if (of_property_count_u32_elems(child, RZN1_PINS_PROP) > 0)
count++;
}
If you prefer not to include this, I will send a small patch myself later.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
> Subject: Re: [PATCH v2 07/20] pinctrl: renesas: Use scope based
> of_node_put() cleanups
>
> Hi Peng,
>
> On Sat, May 4, 2024 at 3:14 PM Peng Fan (OSS) <[email protected]>
> wrote:
> > From: Peng Fan <[email protected]>
> >
> > Use scope based of_node_put() cleanup to simplify code.
> >
> > Signed-off-by: Peng Fan <[email protected]>
>
> Thanks for your patch!
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
> Acked-by: Geert Uytterhoeven <[email protected]>
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzn1.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzn1.c
>
> You missed one trivial conversion, presumably because no error handling and
> thus no of_node_put() is involved?
You are right.
>
> @@ -737,13 +737,12 @@ static int rzn1_pinctrl_parse_groups(struct
> device_node *np,
>
> static int rzn1_pinctrl_count_function_groups(struct device_node *np) {
> - struct device_node *child;
> int count = 0;
>
> if (of_property_count_u32_elems(np, RZN1_PINS_PROP) > 0)
> count++;
>
> - for_each_child_of_node(np, child) {
> + for_each_child_of_node_scoped(np, child) {
> if (of_property_count_u32_elems(child, RZN1_PINS_PROP) > 0)
> count++;
> }
>
> If you prefer not to include this, I will send a small patch myself later.
I would not add it.
If no major comments in this patchset, I will not do a v3. So, please do that
with your follow up patch.
Thanks,
Peng.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
On Mon, May 13, 2024 at 1:59 PM Geert Uytterhoeven <[email protected]> wrote:
> On Sat, May 4, 2024 at 3:14 PM Peng Fan (OSS) <[email protected]> wrote:
> > From: Peng Fan <[email protected]>
> >
> > Use scope based of_node_put() cleanup to simplify code.
> >
> > Signed-off-by: Peng Fan <[email protected]>
>
> Thanks for your patch!
>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
> Acked-by: Geert Uytterhoeven <[email protected]>
Does this go into the Renesas patch stack?
I think the patch stands fine without the rest of the series.
Yours,
Linus Walleij
Hi Linus,
On Mon, May 13, 2024 at 10:51 PM Linus Walleij <[email protected]> wrote:
> On Mon, May 13, 2024 at 1:59 PM Geert Uytterhoeven <[email protected]> wrote:
> > On Sat, May 4, 2024 at 3:14 PM Peng Fan (OSS) <[email protected]> wrote:
> > > From: Peng Fan <[email protected]>
> > >
> > > Use scope based of_node_put() cleanup to simplify code.
> > >
> > > Signed-off-by: Peng Fan <[email protected]>
> >
> > Thanks for your patch!
> >
> > Reviewed-by: Geert Uytterhoeven <[email protected]>
> > Acked-by: Geert Uytterhoeven <[email protected]>
>
> Does this go into the Renesas patch stack?
>
> I think the patch stands fine without the rest of the series.
Sure, I can do that.
From your positive response to v1, I thought that perhaps you just
wanted to take the full series yourself?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Tue, May 14, 2024 at 8:36 AM Geert Uytterhoeven <[email protected]> wrote:
> > Does this go into the Renesas patch stack?
> >
> > I think the patch stands fine without the rest of the series.
>
> Sure, I can do that.
Please apply it!
> From your positive response to v1, I thought that perhaps you just
> wanted to take the full series yourself?
Sorry, I always prefer submaintainers to pick their stuff, they
know what they are doing and they can test the entire patch
stack properly.
Yours,
Linus Walleij
On Tue, May 14, 2024 at 9:33 AM Linus Walleij <linus.walleij@linaroorg> wrote:
> On Tue, May 14, 2024 at 8:36 AM Geert Uytterhoeven <[email protected]> wrote:
> > > Does this go into the Renesas patch stack?
> > > I think the patch stands fine without the rest of the series.
> >
> > Sure, I can do that.
>
> Please apply it!
OK, will queue in renesas-pinctrl for v6.11.
> > From your positive response to v1, I thought that perhaps you just
> > wanted to take the full series yourself?
>
> Sorry, I always prefer submaintainers to pick their stuff, they
> know what they are doing and they can test the entire patch
> stack properly.
OK, will (try to ;-) remember...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 5/4/24 15:20, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Use scope based of_node_put() cleanup to simplify code.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/pinctrl/stm32/pinctrl-stm32.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c
> index 978ccdbaf3d3..a8673739871d 100644
> --- a/drivers/pinctrl/stm32/pinctrl-stm32.c
> +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c
> @@ -670,7 +670,6 @@ static int stm32_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> struct device_node *np_config,
> struct pinctrl_map **map, unsigned *num_maps)
> {
> - struct device_node *np;
> unsigned reserved_maps;
> int ret;
>
> @@ -678,12 +677,11 @@ static int stm32_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> *num_maps = 0;
> reserved_maps = 0;
>
> - for_each_child_of_node(np_config, np) {
> + for_each_child_of_node_scoped(np_config, np) {
> ret = stm32_pctrl_dt_subnode_to_map(pctldev, np, map,
> &reserved_maps, num_maps);
> if (ret < 0) {
> pinctrl_utils_free_map(pctldev, *map, *num_maps);
> - of_node_put(np);
> return ret;
> }
> }
>
Hi
Reviewed-by: Patrice Chotard <[email protected]>
Thanks
Patrice
On 5/4/24 15:20, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Use scope based of_node_put() cleanup to simplify code.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/pinctrl/pinctrl-st.c | 37 +++++++++++--------------------------
> 1 file changed, 11 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c
> index 5d9abd6547d0..fe2d52e434db 100644
> --- a/drivers/pinctrl/pinctrl-st.c
> +++ b/drivers/pinctrl/pinctrl-st.c
> @@ -1195,10 +1195,10 @@ static int st_pctl_dt_parse_groups(struct device_node *np,
> struct property *pp;
> struct device *dev = info->dev;
> struct st_pinconf *conf;
> - struct device_node *pins;
> + struct device_node *pins __free(device_node) = NULL;
> phandle bank;
> unsigned int offset;
> - int i = 0, npins = 0, nr_props, ret = 0;
> + int i = 0, npins = 0, nr_props;
>
> pins = of_get_child_by_name(np, "st,pins");
> if (!pins)
> @@ -1213,8 +1213,7 @@ static int st_pctl_dt_parse_groups(struct device_node *np,
> npins++;
> } else {
> pr_warn("Invalid st,pins in %pOFn node\n", np);
> - ret = -EINVAL;
> - goto out_put_node;
> + return -EINVAL;
> }
> }
>
> @@ -1223,10 +1222,8 @@ static int st_pctl_dt_parse_groups(struct device_node *np,
> grp->pins = devm_kcalloc(dev, npins, sizeof(*grp->pins), GFP_KERNEL);
> grp->pin_conf = devm_kcalloc(dev, npins, sizeof(*grp->pin_conf), GFP_KERNEL);
>
> - if (!grp->pins || !grp->pin_conf) {
> - ret = -ENOMEM;
> - goto out_put_node;
> - }
> + if (!grp->pins || !grp->pin_conf)
> + return -ENOMEM;
>
> /* <bank offset mux direction rt_type rt_delay rt_clk> */
> for_each_property_of_node(pins, pp) {
> @@ -1260,17 +1257,13 @@ static int st_pctl_dt_parse_groups(struct device_node *np,
> i++;
> }
>
> -out_put_node:
> - of_node_put(pins);
> -
> - return ret;
> + return 0;
> }
>
> static int st_pctl_parse_functions(struct device_node *np,
> struct st_pinctrl *info, u32 index, int *grp_index)
> {
> struct device *dev = info->dev;
> - struct device_node *child;
> struct st_pmx_func *func;
> struct st_pctl_group *grp;
> int ret, i;
> @@ -1285,15 +1278,13 @@ static int st_pctl_parse_functions(struct device_node *np,
> return -ENOMEM;
>
> i = 0;
> - for_each_child_of_node(np, child) {
> + for_each_child_of_node_scoped(np, child) {
> func->groups[i] = child->name;
> grp = &info->groups[*grp_index];
> *grp_index += 1;
> ret = st_pctl_dt_parse_groups(child, grp, info, i++);
> - if (ret) {
> - of_node_put(child);
> + if (ret)
> return ret;
> - }
> }
> dev_info(dev, "Function[%d\t name:%s,\tgroups:%d]\n", index, func->name, func->ngroups);
>
> @@ -1601,7 +1592,6 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
> int i = 0, j = 0, k = 0, bank;
> struct pinctrl_pin_desc *pdesc;
> struct device_node *np = dev->of_node;
> - struct device_node *child;
> int grp_index = 0;
> int irq = 0;
>
> @@ -1646,25 +1636,21 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
> pctl_desc->pins = pdesc;
>
> bank = 0;
> - for_each_child_of_node(np, child) {
> + for_each_child_of_node_scoped(np, child) {
> if (of_property_read_bool(child, "gpio-controller")) {
> const char *bank_name = NULL;
> char **pin_names;
>
> ret = st_gpiolib_register_bank(info, bank, child);
> - if (ret) {
> - of_node_put(child);
> + if (ret)
> return ret;
> - }
>
> k = info->banks[bank].range.pin_base;
> bank_name = info->banks[bank].range.name;
>
> pin_names = devm_kasprintf_strarray(dev, bank_name, ST_GPIO_PINS_PER_BANK);
> - if (IS_ERR(pin_names)) {
> - of_node_put(child);
> + if (IS_ERR(pin_names))
> return PTR_ERR(pin_names);
> - }
>
> for (j = 0; j < ST_GPIO_PINS_PER_BANK; j++, k++) {
> pdesc->number = k;
> @@ -1678,7 +1664,6 @@ static int st_pctl_probe_dt(struct platform_device *pdev,
> i++, &grp_index);
> if (ret) {
> dev_err(dev, "No functions found.\n");
> - of_node_put(child);
> return ret;
> }
> }
>
Reviewed-by: Patrice Chotard <[email protected]>
Thanks
Patrice
On Sat, May 4, 2024 at 3:14 PM Peng Fan (OSS) <[email protected]> wrote:
> From: Peng Fan <[email protected]>
>
> Use scope based of_node_put() cleanup to simplify code.
>
> Signed-off-by: Peng Fan <[email protected]>
Patch applied.
Yours,
Linus Walleij
On Sat, May 4, 2024 at 3:13 PM Peng Fan (OSS) <[email protected]> wrote:
> From: Peng Fan <[email protected]>
>
> Use scope based of_node_put() cleanup to simplify code.
>
> Signed-off-by: Peng Fan <[email protected]>
Patch applied.
Yours,
Linus Walleij
> Use scope based of_node_put() cleanup to simplify code.
I see opportunities to improve affected function implementations another bit.
…
> +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c
…
> @@ -543,18 +540,18 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
> pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
> if (!pins) {
> ret = -ENOMEM;
> - goto put_child;
> + goto free_map;
> }
>
> pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux), GFP_KERNEL);
> if (!pinmux) {
> ret = -ENOMEM;
> - goto put_child;
> + goto free_map;
> }
…
> @@ -623,8 +620,6 @@ static int starfive_dt_node_to_map(struct pinctrl_dev *pctldev,
> mutex_unlock(&sfp->mutex);
> return 0;
>
> -put_child:
> - of_node_put(child);
> free_map:
> pinctrl_utils_free_map(pctldev, map, nmaps);
> mutex_unlock(&sfp->mutex);
…
> +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
…
> @@ -175,18 +175,18 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
> pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
> if (!pins) {
> ret = -ENOMEM;
> - goto put_child;
> + goto free_map;
> }
>
> pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux), GFP_KERNEL);
> if (!pinmux) {
> ret = -ENOMEM;
> - goto put_child;
> + goto free_map;
> }
…
> @@ -233,8 +233,6 @@ static int jh7110_dt_node_to_map(struct pinctrl_dev *pctldev,
> *num_maps = nmaps;
> return 0;
>
> -put_child:
> - of_node_put(child);
> free_map:
> pinctrl_utils_free_map(pctldev, map, nmaps);
> mutex_unlock(&sfp->mutex);
1. Exception handling is repeated a few times also according to memory allocation failures.
How do you think about to use a corresponding label like “e_nomem”
so that another bit of duplicate source code can be avoided?
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources
2. Will development interests grow for the usage of a statement like “guard(mutex)(&sfp->mutex);”?
Regards,
Markus
How do you think about to use the summary phrase “Fix reference counting for children in mxs_pinctrl_probe_dt()”?
…
> of_get_next_child() will increase refcount …
the reference counter?
> Per current implementation, 'child' will be override by
overridden?
> for_each_child_of_node(np, child), so use of_get_child_count to avoid
> refcount leakage.
Another wording suggestion:
for_each_child_of_node(np, child). Thus use an of_get_child_count() call
to avoid reference counting leakage.
Regards,
Markus
Hi Markus
> Subject: Re: [PATCH v2 04/20] pinctrl: starfive: Use scope based of_node_put()
> cleanups
>
> > Use scope based of_node_put() cleanup to simplify code.
>
> I see opportunities to improve affected function implementations another bit.
>
>
> ...
> > +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7100.c
> ...
> > @@ -543,18 +540,18 @@ static int starfive_dt_node_to_map(struct
> pinctrl_dev *pctldev,
> > pins = devm_kcalloc(dev, npins, sizeof(*pins),
> GFP_KERNEL);
> > if (!pins) {
> > ret = -ENOMEM;
> > - goto put_child;
> > + goto free_map;
> > }
> >
> > pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux),
> GFP_KERNEL);
> > if (!pinmux) {
> > ret = -ENOMEM;
> > - goto put_child;
> > + goto free_map;
> > }
> ...
> > @@ -623,8 +620,6 @@ static int starfive_dt_node_to_map(struct
> pinctrl_dev *pctldev,
> > mutex_unlock(&sfp->mutex);
> > return 0;
> >
> > -put_child:
> > - of_node_put(child);
> > free_map:
> > pinctrl_utils_free_map(pctldev, map, nmaps);
> > mutex_unlock(&sfp->mutex);
> ...
> > +++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
> ...
> > @@ -175,18 +175,18 @@ static int jh7110_dt_node_to_map(struct
> pinctrl_dev *pctldev,
> > pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
> > if (!pins) {
> > ret = -ENOMEM;
> > - goto put_child;
> > + goto free_map;
> > }
> >
> > pinmux = devm_kcalloc(dev, npins, sizeof(*pinmux),
> GFP_KERNEL);
> > if (!pinmux) {
> > ret = -ENOMEM;
> > - goto put_child;
> > + goto free_map;
> > }
> ...
> > @@ -233,8 +233,6 @@ static int jh7110_dt_node_to_map(struct
> pinctrl_dev *pctldev,
> > *num_maps = nmaps;
> > return 0;
> >
> > -put_child:
> > - of_node_put(child);
> > free_map:
> > pinctrl_utils_free_map(pctldev, map, nmaps);
> > mutex_unlock(&sfp->mutex);
>
>
> 1. Exception handling is repeated a few times also according to memory
> allocation failures.
> How do you think about to use a corresponding label like "e_nomem"
> so that another bit of duplicate source code can be avoided?
I have no plan to rework this series for non-accepted patches. If you have
interest and time, feel free to take it.
>
> https://wiki.se/
> i.cmu.edu%2Fconfluence%2Fdisplay%2Fc%2FMEM12-
> C.%2BConsider%2Busing%2Ba%2Bgoto%2Bchain%2Bwhen%2Bleaving%2Ba%
> 2Bfunction%2Bon%2Berror%2Bwhen%2Busing%2Band%2Breleasing%2Bresou
> rces&data=05%7C02%7Cpeng.fan%40nxp.com%7C293bafdf40524fa4655b08
> dc7e58f6b2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63852
> 4167804502915%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata
> =Kb5cz6sVxW1TNfQ8MM2F6YLIIztyjvW4wULEJLYKRM8%3D&reserved=0
>
> 2. Will development interests grow for the usage of a statement like
> "guard(mutex)(&sfp->mutex);"?
I have no plan on this.
Thanks,
Peng.
>
>
> Regards,
> Markus
> Subject: Re: [PATCH v2 18/20] pinctrl: freescale: mxs: Fix refcount of child
>
> How do you think about to use the summary phrase “Fix reference counting
> for children in mxs_pinctrl_probe_dt()”?
Thanks for reviewing. I have no plan to rework this series for non-accepted
patches. If you have interest and time, feel free to take it.
Thanks,
Peng.
>
>
> …
> > of_get_next_child() will increase refcount …
>
> the reference counter?
>
>
> > Per current implementation, 'child' will be override by
>
> overridden?
>
>
> > for_each_child_of_node(np, child), so use of_get_child_count to avoid
> > refcount leakage.
>
> Another wording suggestion:
> for_each_child_of_node(np, child). Thus use an of_get_child_count() call
> to avoid reference counting leakage.
>
>
> Regards,
> Markus
Hi Andy,
On Thu, May 30, 2024 at 11:03 AM Andy Shevchenko
<[email protected]> wrote:
> Mon, May 13, 2024 at 01:59:03PM +0200, Geert Uytterhoeven kirjoitti:
> > On Sat, May 4, 2024 at 3:14 PM Peng Fan (OSS) <[email protected]> wrote:
> > You missed one trivial conversion, presumably because no error handling
> > and thus no of_node_put() is involved?
>
> Nothing is missed. The below is a redundant change.
>
> ...
>
> > - for_each_child_of_node(np, child) {
> > + for_each_child_of_node_scoped(np, child) {
> > if (of_property_count_u32_elems(child, RZN1_PINS_PROP) > 0)
> > count++;
> > }
> >
> > If you prefer not to include this
>
> I prefer this not to be included as it will give a misleading signals to the
> use of the original API(s).
Thank you for reminding me to send this out as a separate patch :-)
https://lore.kernel.org/r/c0a28f466c42d5d59c7fadfa1fd05fd512d43b6f.1717060708.git.geert+renesas@glider.be
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Mon, May 13, 2024 at 01:59:03PM +0200, Geert Uytterhoeven kirjoitti:
> On Sat, May 4, 2024 at 3:14 PM Peng Fan (OSS) <[email protected]> wrote:
..
> You missed one trivial conversion, presumably because no error handling
> and thus no of_node_put() is involved?
Nothing is missed. The below is a redundant change.
..
> - for_each_child_of_node(np, child) {
> + for_each_child_of_node_scoped(np, child) {
> if (of_property_count_u32_elems(child, RZN1_PINS_PROP) > 0)
> count++;
> }
>
> If you prefer not to include this
I prefer this not to be included as it will give a misleading signals to the
use of the original API(s).
--
With Best Regards,
Andy Shevchenko
>> How do you think about to use the summary phrase “Fix reference counting
>> for children in mxs_pinctrl_probe_dt()”?
>
> Thanks for reviewing. I have no plan to rework this series for non-accepted patches.
…
Will development interests grow to take patch review concerns better into account
so that commit messages can be improved another bit?
Regards,
Markus
>> 1. Exception handling is repeated a few times also according to memory
>> allocation failures.
>> How do you think about to use a corresponding label like "e_nomem"
>> so that another bit of duplicate source code can be avoided?
>
> I have no plan to rework this series for non-accepted patches. If you have
> interest and time, feel free to take it.
>>
>> https://wiki.se/
>> i.cmu.edu%2Fconfluence%2Fdisplay%2Fc%2FMEM12-
>> C.%2BConsider%2Busing%2Ba%2Bgoto%2Bchain%2Bwhen%2Bleaving%2Ba%
>> 2Bfunction%2Bon%2Berror%2Bwhen%2Busing%2Band%2Breleasing%2Bresou
>> rces&data=05%7C02%7Cpeng.fan%40nxp.com%7C293bafdf40524fa4655b08
>> dc7e58f6b2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63852
>> 4167804502915%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
>> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata
>> =Kb5cz6sVxW1TNfQ8MM2F6YLIIztyjvW4wULEJLYKRM8%3D&reserved=0
I became curious how the change acceptance will evolve further
also according to such a code transformation possibility.
>> 2. Will development interests grow for the usage of a statement like
>> "guard(mutex)(&sfp->mutex);"?
>
> I have no plan on this.
Other contributors might get attracted by corresponding design adjustments.
https://elixir.bootlin.com/linux/v6.10-rc1/source/include/linux/cleanup.h#L124
See also:
Looking at guard usage (with SmPL)
https://lore.kernel.org/cocci/[email protected]/
https://sympa.inria.fr/sympa/arc/cocci/2024-05/msg00090.html
Regards,
Markus
> Subject: Re: [v2 18/20] pinctrl: freescale: mxs: Fix refcount of child
>
> >> How do you think about to use the summary phrase “Fix reference
> >> counting for children in mxs_pinctrl_probe_dt()”?
> >
> > Thanks for reviewing. I have no plan to rework this series for non-accepted
> patches.
> …
>
> Will development interests grow to take patch review concerns better into
> account so that commit messages can be improved another bit?
Yes. I had read your comments, since I not plan to redo this series, so not reply
every comment.
Your suggestions are good, I will improve in my future patch commit messages.
Thanks very much!
Peng
>
> Regards,
> Markus
Sat, May 04, 2024 at 09:20:06PM +0800, Peng Fan (OSS) kirjoitti:
> From: Peng Fan <[email protected]>
>
> Use scope based of_node_put() cleanup to simplify code.
..
> struct property *pp;
> struct device *dev = info->dev;
> struct st_pinconf *conf;
> - struct device_node *pins;
> + struct device_node *pins __free(device_node) = NULL;
It's better to move it upper to follow reversed xmas tree order (okay, to some
extent in this case).
> phandle bank;
> unsigned int offset;
> - int i = 0, npins = 0, nr_props, ret = 0;
> + int i = 0, npins = 0, nr_props;
--
With Best Regards,
Andy Shevchenko
On Sat May 4, 2024 at 3:20 PM CEST, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> Use scope based of_node_put() cleanup to simplify code.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra-xusb.c | 7 ++-----
> drivers/pinctrl/tegra/pinctrl-tegra.c | 4 +---
> 2 files changed, 3 insertions(+), 8 deletions(-)
Looks good to me:
Acked-by: Thierry Reding <[email protected]>
Hi Peng,
On Sat, May 4, 2024 at 3:12 PM Peng Fan (OSS) <[email protected]> wrote:
> Use scope based of_node_put() to simplify code. It reduces the chance
> of forgetting of_node_put(), and also simplifies error handling path.
> I not able to test the changes on all the hardwares, so driver owners,
> please help review when you have time.
>
> This patchset was inspired from Dan's comments on pinctrl-scmi-imx.c,
> thanks.
>
> Signed-off-by: Peng Fan <[email protected]>
Andy's question about code generation on a related patch made me
wonder, too.
On arm32, a conversion to for_each_child_of_node_scoped() seems to
cost ca. 48 bytes of additional code, regardless of whether there were
explicit cleanups before or not.
I checked "pinctrl: renesas: Use scope based of_node_put() cleanups",
and all but the conversions in *_dt_node_to_map() cost 48 bytes each.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert
> Subject: Re: [PATCH v2 00/20] pinctrl: Use scope based of_node_put()
> cleanups
>
> Hi Peng,
>
> On Sat, May 4, 2024 at 3:12 PM Peng Fan (OSS) <[email protected]>
> wrote:
> > Use scope based of_node_put() to simplify code. It reduces the chance
> > of forgetting of_node_put(), and also simplifies error handling path.
> > I not able to test the changes on all the hardwares, so driver owners,
> > please help review when you have time.
> >
> > This patchset was inspired from Dan's comments on pinctrl-scmi-imx.c,
> > thanks.
> >
> > Signed-off-by: Peng Fan <[email protected]>
>
> Andy's question about code generation on a related patch made me wonder,
> too.
>
> On arm32, a conversion to for_each_child_of_node_scoped() seems to cost ca.
> 48 bytes of additional code, regardless of whether there were explicit
> cleanups before or not.
>
> I checked "pinctrl: renesas: Use scope based of_node_put() cleanups", and all
> but the conversions in *_dt_node_to_map() cost 48 bytes each.
>
I am not sure this is an issue or else. What would you suggest me to do?
If you think extra 48bytes consumption is not good here, feel free to drop the
patch.
Thanks,
Peng.
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Hi Peng,
On Fri, May 31, 2024 at 5:07 AM Peng Fan <[email protected]> wrote:
> > Subject: Re: [PATCH v2 00/20] pinctrl: Use scope based of_node_put()
> > cleanups
> > On Sat, May 4, 2024 at 3:12 PM Peng Fan (OSS) <[email protected]>
> > wrote:
> > > Use scope based of_node_put() to simplify code. It reduces the chance
> > > of forgetting of_node_put(), and also simplifies error handling path.
> > > I not able to test the changes on all the hardwares, so driver owners,
> > > please help review when you have time.
> > >
> > > This patchset was inspired from Dan's comments on pinctrl-scmi-imx.c,
> > > thanks.
> > >
> > > Signed-off-by: Peng Fan <[email protected]>
> >
> > Andy's question about code generation on a related patch made me wonder,
> > too.
> >
> > On arm32, a conversion to for_each_child_of_node_scoped() seems to cost ca.
> > 48 bytes of additional code, regardless of whether there were explicit
> > cleanups before or not.
> >
> > I checked "pinctrl: renesas: Use scope based of_node_put() cleanups", and all
> > but the conversions in *_dt_node_to_map() cost 48 bytes each.
>
> I am not sure this is an issue or else. What would you suggest me to do?
> If you think extra 48bytes consumption is not good here, feel free to drop the
> patch.
I suggest doing nothing about this. I just wanted people to be aware
of the impact. I guess it's just part of the slow but steady increase
of kernel size (ca. 20-30 KiB/release)... ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds