On Thu, 27 Jun 2024 21:17:19 +0800 "Peng Fan (OSS)" <peng.fan@xxxxxxxxxxx> wrote: > From: Peng Fan <peng.fan@xxxxxxx> > > Use scope based of_node_put() cleanup to simplify code. > > Signed-off-by: Peng Fan <peng.fan@xxxxxxx> 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@xxxxxxxxxx> > --- > 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? :) > } > } > > /**