Re: [PATCH 1/2] soc: sifive: ccache: fix missing iounmap() in error path in sifive_ccache_init()

[Conor Dooley <conor@kernel.org>]

On Mon, Oct 17, 2022 at 04:44:10PM +0800, Yang Yingliang wrote:
> Add missing iounmap() before return error from sifive_ccache_init().
> 
> Fixes: a967a289f169 ("RISC-V: sifive_l2_cache: Add L2 cache controller driver for SiFive SoCs")

Hey Yang Yangliang,

Both of these patches look good to me.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

One question - of_find_matching_node() increments the refcount of np
right? It seems to me like we need an of_node_put() here, at the very
least in the error paths (do we need it in the success path too?).
If we are going to add cleanup to this driver, we may as well go the
whole hog I think.

In terms of the success paths, I assume the first place we can safely
let the reference go is just before the call to ccache_config_read()?

Please lmk if I am misunderstanding anything here...
Thanks,
Conor.

> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/soc/sifive/sifive_ccache.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
> index 1c171150e878..25019c16d8ae 100644
> --- a/drivers/soc/sifive/sifive_ccache.c
> +++ b/drivers/soc/sifive/sifive_ccache.c
> @@ -222,13 +222,16 @@ static int __init sifive_ccache_init(void)
>  	if (!ccache_base)
>  		return -ENOMEM;
>  
> -	if (of_property_read_u32(np, "cache-level", &level))
> -		return -ENOENT;
> +	if (of_property_read_u32(np, "cache-level", &level)) {
> +		rc = -ENOENT;
> +		goto err_unmap;
> +	}
>  
>  	intr_num = of_property_count_u32_elems(np, "interrupts");
>  	if (!intr_num) {
>  		pr_err("No interrupts property\n");
> -		return -ENODEV;
> +		rc = -ENODEV;
> +		goto err_unmap;
>  	}
>  
>  	for (i = 0; i < intr_num; i++) {
> @@ -237,7 +240,7 @@ static int __init sifive_ccache_init(void)
>  				 NULL);
>  		if (rc) {
>  			pr_err("Could not request IRQ %d\n", g_irq[i]);
> -			return rc;
> +			goto err_unmap;
>  		}
>  	}
>  
> @@ -250,6 +253,10 @@ static int __init sifive_ccache_init(void)
>  	setup_sifive_debug();
>  #endif
>  	return 0;
> +
> +err_unmap:
> +	iounmap(ccache_base);
> +	return rc;
>  }
>  
>  device_initcall(sifive_ccache_init);
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv