lkml.org 
[lkml]   [2022]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] drivers: soc: sifive: Add missing of_node_put() in sifive_l2_cache.c
From
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);

\
 
 \ /
  Last update: 2022-06-16 07:13    [W:1.034 / U:0.396 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site