| Message ID | 20240703065710.13786-4-five231003@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Do device node auto cleanup in drivers/soc/ti/ | expand |
On Wed, 3 Jul 2024 12:25:28 +0530 Kousik Sanagavarapu <five231003@gmail.com> wrote: > Use scope based cleanup instead of manual of_node_put() calls, hence > simplifying the handling of error paths. > > Suggested-by: Julia Lawall <julia.lawall@inria.fr> > Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com> I think you can make use of dev_err_probe() in here to further simplify things (a bit anyway!) Jonathan > --- > drivers/soc/ti/pm33xx.c | 20 +++++++------------- > 1 file changed, 7 insertions(+), 13 deletions(-) > > diff --git a/drivers/soc/ti/pm33xx.c b/drivers/soc/ti/pm33xx.c > index 8e983c3c4e03..40988c45ed00 100644 > --- a/drivers/soc/ti/pm33xx.c > +++ b/drivers/soc/ti/pm33xx.c > @@ -383,10 +383,9 @@ static void am33xx_pm_free_sram(void) > */ > static int am33xx_pm_alloc_sram(void) > { > - struct device_node *np; > - int ret = 0; > + struct device_node *np __free(device_node) = > + of_find_compatible_node(NULL, NULL, "ti,omap3-mpu"); > > - np = of_find_compatible_node(NULL, NULL, "ti,omap3-mpu"); > if (!np) { > np = of_find_compatible_node(NULL, NULL, "ti,omap4-mpu"); > if (!np) { > @@ -400,24 +399,21 @@ static int am33xx_pm_alloc_sram(void) > if (!sram_pool) { > dev_err(pm33xx_dev, "PM: %s: Unable to get sram pool for ocmcram\n", > __func__); > - ret = -ENODEV; > - goto mpu_put_node; > + return -ENODEV; > } > > sram_pool_data = of_gen_pool_get(np, "pm-sram", 1); > if (!sram_pool_data) { > dev_err(pm33xx_dev, "PM: %s: Unable to get sram data pool for ocmcram\n", > __func__); > - ret = -ENODEV; > - goto mpu_put_node; > + return -ENODEV; > } > > ocmcram_location = gen_pool_alloc(sram_pool, *pm_sram->do_wfi_sz); > if (!ocmcram_location) { > dev_err(pm33xx_dev, "PM: %s: Unable to allocate memory from ocmcram\n", > __func__); > - ret = -ENOMEM; > - goto mpu_put_node; > + return -ENOMEM; Why not dev_err_probe()? Seems to only be called from a probe() callback. > } > > ocmcram_location_data = gen_pool_alloc(sram_pool_data, > @@ -425,12 +421,10 @@ static int am33xx_pm_alloc_sram(void) > if (!ocmcram_location_data) { > dev_err(pm33xx_dev, "PM: Unable to allocate memory from ocmcram\n"); > gen_pool_free(sram_pool, ocmcram_location, *pm_sram->do_wfi_sz); > - ret = -ENOMEM; > + return -ENOMEM; I doubt the ordering matters so can probably do dev_err_probe() in here. > } > > -mpu_put_node: > - of_node_put(np); > - return ret; > + return 0; > } > > static int am33xx_pm_rtc_setup(void)
Hi Jonathan, On Fri, Jul 05, 2024 at 11:54:13AM +0100, Jonathan Cameron wrote: > On Wed, 3 Jul 2024 12:25:28 +0530 > Kousik Sanagavarapu <five231003@gmail.com> wrote: > > > Use scope based cleanup instead of manual of_node_put() calls, hence > > simplifying the handling of error paths. > > > > Suggested-by: Julia Lawall <julia.lawall@inria.fr> > > Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com> > I think you can make use of dev_err_probe() in here to > further simplify things (a bit anyway!) The changes proposed on all the patches make sense. I'll send out v3 soon. I hadn't caught the dev_err_probe() change in this patch ;) while preparing this series. I'll make the changes. Thanks for the review
diff --git a/drivers/soc/ti/pm33xx.c b/drivers/soc/ti/pm33xx.c index 8e983c3c4e03..40988c45ed00 100644 --- a/drivers/soc/ti/pm33xx.c +++ b/drivers/soc/ti/pm33xx.c @@ -383,10 +383,9 @@ static void am33xx_pm_free_sram(void) */ static int am33xx_pm_alloc_sram(void) { - struct device_node *np; - int ret = 0; + struct device_node *np __free(device_node) = + of_find_compatible_node(NULL, NULL, "ti,omap3-mpu"); - np = of_find_compatible_node(NULL, NULL, "ti,omap3-mpu"); if (!np) { np = of_find_compatible_node(NULL, NULL, "ti,omap4-mpu"); if (!np) { @@ -400,24 +399,21 @@ static int am33xx_pm_alloc_sram(void) if (!sram_pool) { dev_err(pm33xx_dev, "PM: %s: Unable to get sram pool for ocmcram\n", __func__); - ret = -ENODEV; - goto mpu_put_node; + return -ENODEV; } sram_pool_data = of_gen_pool_get(np, "pm-sram", 1); if (!sram_pool_data) { dev_err(pm33xx_dev, "PM: %s: Unable to get sram data pool for ocmcram\n", __func__); - ret = -ENODEV; - goto mpu_put_node; + return -ENODEV; } ocmcram_location = gen_pool_alloc(sram_pool, *pm_sram->do_wfi_sz); if (!ocmcram_location) { dev_err(pm33xx_dev, "PM: %s: Unable to allocate memory from ocmcram\n", __func__); - ret = -ENOMEM; - goto mpu_put_node; + return -ENOMEM; } ocmcram_location_data = gen_pool_alloc(sram_pool_data, @@ -425,12 +421,10 @@ static int am33xx_pm_alloc_sram(void) if (!ocmcram_location_data) { dev_err(pm33xx_dev, "PM: Unable to allocate memory from ocmcram\n"); gen_pool_free(sram_pool, ocmcram_location, *pm_sram->do_wfi_sz); - ret = -ENOMEM; + return -ENOMEM; } -mpu_put_node: - of_node_put(np); - return ret; + return 0; } static int am33xx_pm_rtc_setup(void)
Use scope based cleanup instead of manual of_node_put() calls, hence simplifying the handling of error paths. Suggested-by: Julia Lawall <julia.lawall@inria.fr> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com> --- drivers/soc/ti/pm33xx.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)