Messages in this thread | | | Date | Thu, 16 Jun 2022 07:12:21 +0200 | Subject | Re: [PATCH] drivers: soc: sifive: Add missing of_node_put() in sifive_l2_cache.c | From | Christophe JAILLET <> |
| |
Le 15/06/2022 à 14:23, Liang He a écrit : > In sifive_l2_init(), of_find_matching_node() will return a node pointer > with refcount incremented. We should use of_node_put() in each fail path > or when it is not used anymore. > > Signed-off-by: Liang He <windhl@126.com> > --- > drivers/soc/sifive/sifive_l2_cache.c | 29 ++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/drivers/soc/sifive/sifive_l2_cache.c b/drivers/soc/sifive/sifive_l2_cache.c > index 59640a1d0b28..2b9c9522ef21 100644 > --- a/drivers/soc/sifive/sifive_l2_cache.c > +++ b/drivers/soc/sifive/sifive_l2_cache.c > @@ -198,29 +198,41 @@ static int __init sifive_l2_init(void) > struct resource res; > int i, rc, intr_num; >
Hi, this empty line is not needed.
> + int ret; > + > np = of_find_matching_node(NULL, sifive_l2_ids); > if (!np) > return -ENODEV; > > if (of_address_to_resource(np, 0, &res)) > - return -ENODEV; > + {
this should be at the end of the previous line.
> + ret = -ENODEV; > + goto out_put; > + } > > l2_base = ioremap(res.start, resource_size(&res)); > if (!l2_base) > - return -ENOMEM; > + { >
Same here.
+ ret = -ENOMEM; > + goto out_put; > + } > > intr_num = of_property_count_u32_elems(np, "interrupts"); > - if (!intr_num) { > + if (!intr_num) { > pr_err("L2CACHE: no interrupts property\n"); > - return -ENODEV; > + ret = -ENODEV
Missing ";" as reported by the bot.
> + goto out_put; > } > > for (i = 0; i < intr_num; i++) { > g_irq[i] = irq_of_parse_and_map(np, i); > rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL); > + > if (rc) { > +
Why a new empty line here?
> pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]); > - return rc; > + ret = rc; > + goto out_put; > } > } > > @@ -232,6 +244,11 @@ static int __init sifive_l2_init(void) > #ifdef CONFIG_DEBUG_FS > setup_sifive_debug(); > #endif > - return 0; > + ret = 0; > + > +
No need for 2 empty lines here.
There are also some trailing white spaces on some lines.
"./scripts/checkpatch <name_of_the_patch>" catches some of these tiny issues. Using --strict catches even more of these issues.
You should also always at least compile test your patches, even if they look obvious,
CJ
> +out_put: > + of_node_put(np); > + return ret; > } > device_initcall(sifive_l2_init);
| |