Messages in this thread | | | Date | Thu, 16 Jun 2022 13:33:49 +0800 (CST) | From | "Liang He" <> | Subject | Re:Re: [PATCH] drivers: soc: sifive: Add missing of_node_put() in sifive_l2_cache.c |
| |
At 2022-06-16 13:12:21, "Christophe JAILLET" <christophe.jaillet@wanadoo.fr> wrote: >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);
Sorry for my troubles. I will check my patch more carefully.
| |