[PATCH V3 0/3] pinctrl: use scope based of_node_put

[PATCH V3 0/3] pinctrl: use scope based of_node_put

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

This is to send the non-applied ones of [1] after rebase, no more changes

[1] https://lore.kernel.org/all/20240504-pinctrl-cleanup-v2-0-26c5f2dc1181@nxp.com/

Peng Fan (3):
  pinctrl: ti: iodelay: Use scope based of_node_put() cleanups
  pinctrl: equilibrium: Use scope based of_node_put() cleanups
  pinctrl: freescale: Use scope based of_node_put() cleanups

 drivers/pinctrl/freescale/pinctrl-imx.c       | 25 +++--------
 drivers/pinctrl/freescale/pinctrl-imx1-core.c | 16 +++----
 drivers/pinctrl/freescale/pinctrl-mxs.c       | 14 ++----
 drivers/pinctrl/pinctrl-equilibrium.c         | 21 +++------
 drivers/pinctrl/ti/pinctrl-ti-iodelay.c       | 43 +++++++------------
 5 files changed, 36 insertions(+), 83 deletions(-)

-- 
2.37.1

[PATCH V3 1/3] pinctrl: ti: iodelay: Use scope based of_node_put() cleanups

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Use scope based of_node_put() cleanup to simplify code.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 43 +++++++++----------------
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
index ef9758638501..f5e5a23d2226 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,20 +874,12 @@ static int ti_iodelay_probe(struct platform_device *pdev)
 	ret = devm_pinctrl_register_and_init(dev, &iod->desc, iod, &iod->pctl);
 	if (ret) {
 		dev_err(dev, "Failed to register pinctrl\n");
-		goto exit_out;
+		return ret;
 	}
 
 	platform_set_drvdata(pdev, iod);
 
-	ret = pinctrl_enable(iod->pctl);
-	if (ret)
-		goto exit_out;
-
-	return 0;
-
-exit_out:
-	of_node_put(np);
-	return ret;
+	return pinctrl_enable(iod->pctl);
 }
 
 /**
-- 
2.37.1

[PATCH V3 2/3] pinctrl: equilibrium: Use scope based of_node_put() cleanups

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Use scope based of_node_put() cleanup to simplify code.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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 a6d089eaaae5..3a9a0f059090 100644
--- a/drivers/pinctrl/pinctrl-equilibrium.c
+++ b/drivers/pinctrl/pinctrl-equilibrium.c
@@ -588,7 +588,6 @@ static int funcs_utils(struct device *dev, struct pinfunction *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;
 	const char **groups;
@@ -596,7 +595,7 @@ static int funcs_utils(struct device *dev, struct pinfunction *funcs,
 	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;
@@ -635,7 +634,6 @@ static int funcs_utils(struct device *dev, struct pinfunction *funcs,
 			break;
 
 		default:
-			of_node_put(np);
 			return -EINVAL;
 		}
 		i++;
@@ -708,11 +706,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;
@@ -720,42 +717,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;
@@ -766,7 +756,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

[PATCH V3 3/3] pinctrl: freescale: Use scope based of_node_put() cleanups

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Use scope based of_node_put() cleanup to simplify code.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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 1ccdeb86d8a7..9c2680df082c 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->func.groups = 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 4813a9e16cb3..edb242d30609 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

Re: [PATCH V3 1/3] pinctrl: ti: iodelay: Use scope based of_node_put() cleanups

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]

Hello Peng,

On Thu, Jun 27, 2024 at 09:17:19PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Use scope based of_node_put() cleanup to simplify code.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 43 +++++++++----------------
>  1 file changed, 15 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> index ef9758638501..f5e5a23d2226 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);

of_node_put? -------------------------------------------^

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH V3 1/3] pinctrl: ti: iodelay: Use scope based of_node_put() cleanups

[Peng Fan]

> Subject: Re: [PATCH V3 1/3] pinctrl: ti: iodelay: Use scope based
> of_node_put() cleanups
> 
> Hello Peng,
> 
> On Thu, Jun 27, 2024 at 09:17:19PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Use scope based of_node_put() cleanup to simplify code.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 43
> > +++++++++----------------
> >  1 file changed, 15 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> > b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> > index ef9758638501..f5e5a23d2226 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);
> 
> of_node_put? -------------------------------------------^

You mean use of_node_put here?
of_node_get should be used here. The __free will automatically
do of_node_put when function return.

Thanks,
Peng.

> 
> Best regards
> Uwe

Re: [PATCH V3 1/3] pinctrl: ti: iodelay: Use scope based of_node_put() cleanups

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]

Hello,

On Sat, Jun 29, 2024 at 01:32:15AM +0000, Peng Fan wrote:
> > Subject: Re: [PATCH V3 1/3] pinctrl: ti: iodelay: Use scope based
> > of_node_put() cleanups
> > 
> > Hello Peng,
> > 
> > On Thu, Jun 27, 2024 at 09:17:19PM +0800, Peng Fan (OSS) wrote:
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > Use scope based of_node_put() cleanup to simplify code.
> > >
> > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > ---
> > >  drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 43
> > > +++++++++----------------
> > >  1 file changed, 15 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> > > b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> > > index ef9758638501..f5e5a23d2226 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);
> > 
> > of_node_put? -------------------------------------------^
> 
> You mean use of_node_put here?
> of_node_get should be used here. The __free will automatically
> do of_node_put when function return.

Ah, I misinterpreted the syntax. I thought your code registers
of_node_get() as cleanup handler.

Sorry for the noise,
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V3 1/3] pinctrl: ti: iodelay: Use scope based of_node_put() cleanups

[Jonathan Cameron]

On Thu, 27 Jun 2024 21:17:19 +0800
"Peng Fan (OSS)" <peng.fan@oss.nxp.com> wrote:

> From: Peng Fan <peng.fan@nxp.com>
> 
> Use scope based of_node_put() cleanup to simplify code.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
A couple of comments on additional cleanups - primarily
using return dev_err_probe() - enabled by using scope based
cleanup to avoid the goto dance.

Either way LGTM
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 43 +++++++++----------------
>  1 file changed, 15 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
> index ef9758638501..f5e5a23d2226 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;
Whilst you are here can use more compact
		return dev_err_probe(dev, -EINVAL, "No OF node\n");
for all error prints in a probe() function.
They do various nice things on deferred probe but are
useful more generally.

>  	}
>  
>  	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;

return dev_err_probe() works here as well and in 
other cases below. It's an added bonus __free() magic often enables.

No idea why DATA is in capitals and to be honest it's not
a particularly useful error message.  What data? :)

>  	}


>  }
>  
>  /**

Re: [PATCH V3 2/3] pinctrl: equilibrium: Use scope based of_node_put() cleanups

[Jonathan Cameron]

On Thu, 27 Jun 2024 21:17:20 +0800
"Peng Fan (OSS)" <peng.fan@oss.nxp.com> wrote:

> From: Peng Fan <peng.fan@nxp.com>
> 
> Use scope based of_node_put() cleanup to simplify code.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  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 a6d089eaaae5..3a9a0f059090 100644
> --- a/drivers/pinctrl/pinctrl-equilibrium.c
> +++ b/drivers/pinctrl/pinctrl-equilibrium.c
> @@ -588,7 +588,6 @@ static int funcs_utils(struct device *dev, struct pinfunction *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;
>  	const char **groups;
> @@ -596,7 +595,7 @@ static int funcs_utils(struct device *dev, struct pinfunction *funcs,
>  	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;
> @@ -635,7 +634,6 @@ static int funcs_utils(struct device *dev, struct pinfunction *funcs,
>  			break;
>  
>  		default:
> -			of_node_put(np);
>  			return -EINVAL;
>  		}
>  		i++;
> @@ -708,11 +706,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;
> @@ -720,42 +717,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);
Given I think this is only called from probe, return dev_err_probe() works
nicely here as well.

>  			return err;
>  		}
Either way,
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Re: [PATCH V3 3/3] pinctrl: freescale: Use scope based of_node_put() cleanups

[Jonathan Cameron]

On Thu, 27 Jun 2024 21:17:21 +0800
"Peng Fan (OSS)" <peng.fan@oss.nxp.com> wrote:

> From: Peng Fan <peng.fan@nxp.com>
> 
> Use scope based of_node_put() cleanup to simplify code.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
dev_err_probe() might apply in here - I haven't chased through
the callers.  Other than that lgtm

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Re: [PATCH V3 0/3] pinctrl: use scope based of_node_put

[Linus Walleij]

On Thu, Jun 27, 2024 at 3:08 PM Peng Fan (OSS) <peng.fan@oss.nxp.com> wrote:

> From: Peng Fan <peng.fan@nxp.com>
>
> This is to send the non-applied ones of [1] after rebase, no more changes

Last three patches applied!

Thanks for doing this work Peng, it's really nice because this kind of
stuff tend to end up on the backburner for me, but now it gets done
with proper attention and focus!

Yours,
Linus Walleij