drivers/soc/sifive/sifive_l2_cache.c | 29 ++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)
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;
+ 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;
+ {
+ ret = -ENODEV;
+ goto out_put;
+ }
l2_base = ioremap(res.start, resource_size(&res));
if (!l2_base)
- return -ENOMEM;
+ {
+ 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
+ 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) {
+
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;
+
+
+out_put:
+ of_node_put(np);
+ return ret;
}
device_initcall(sifive_l2_init);
--
2.25.1Le 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);
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.
Le 16/06/2022 à 07:33, Liang He a écrit : > > Sorry for my troubles. I will check my patch more carefully. No problem, you seem to be knew on the lists so you have to learn the tricks, coding-style, tools... Happy patching :) CJ
Hi Liang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.19-rc2 next-20220615]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Liang-He/drivers-soc-sifive-Add-missing-of_node_put-in-sifive_l2_cache-c/20220615-202528
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 018ab4fabddd94f1c96f3b59e180691b9e88d5d8
config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20220616/202206160559.fVKJx0ST-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/49c4086768b5aff410a4a19ca740f8ae8e211844
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Liang-He/drivers-soc-sifive-Add-missing-of_node_put-in-sifive_l2_cache-c/20220615-202528
git checkout 49c4086768b5aff410a4a19ca740f8ae8e211844
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/soc/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/soc/sifive/sifive_l2_cache.c: In function 'sifive_l2_init':
>> drivers/soc/sifive/sifive_l2_cache.c:224:17: error: expected ';' before 'goto'
224 | goto out_put;
| ^~~~
vim +224 drivers/soc/sifive/sifive_l2_cache.c
194
195 static int __init sifive_l2_init(void)
196 {
197 struct device_node *np;
198 struct resource res;
199 int i, rc, intr_num;
200
201 int ret;
202
203 np = of_find_matching_node(NULL, sifive_l2_ids);
204 if (!np)
205 return -ENODEV;
206
207 if (of_address_to_resource(np, 0, &res))
208 {
209 ret = -ENODEV;
210 goto out_put;
211 }
212
213 l2_base = ioremap(res.start, resource_size(&res));
214 if (!l2_base)
215 {
216 ret = -ENOMEM;
217 goto out_put;
218 }
219
220 intr_num = of_property_count_u32_elems(np, "interrupts");
221 if (!intr_num) {
222 pr_err("L2CACHE: no interrupts property\n");
223 ret = -ENODEV
> 224 goto out_put;
225 }
226
227 for (i = 0; i < intr_num; i++) {
228 g_irq[i] = irq_of_parse_and_map(np, i);
229 rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL);
230
231 if (rc) {
232
233 pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]);
234 ret = rc;
235 goto out_put;
236 }
237 }
238
239 l2_config_read();
240
241 l2_cache_ops.get_priv_group = l2_get_priv_group;
242 riscv_set_cacheinfo_ops(&l2_cache_ops);
243
--
0-DAY CI Kernel Test Service
https://01.org/lkp
At 2022-06-16 05:58:05, "kernel test robot" <lkp@intel.com> wrote: >Hi Liang, > >Thank you for the patch! Yet something to improve: > >[auto build test ERROR on linus/master] >[also build test ERROR on v5.19-rc2 next-20220615] >[If your patch is applied to the wrong git tree, kindly drop us a note. >And when submitting patch, we suggest to use '--base' as documented in >https://git-scm.com/docs/git-format-patch] > When I use --base, there are too many prerequests-patch-id caused by my local commits. I don't know if it will cause other troubles. So I resent a new patch still without this '--base'. Is it Ok? Sorry, I forget to say in new patch that is based on v5.19-rc2 mainline git repo. >url: https://github.com/intel-lab-lkp/linux/commits/Liang-He/drivers-soc-sifive-Add-missing-of_node_put-in-sifive_l2_cache-c/20220615-202528 >base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 018ab4fabddd94f1c96f3b59e180691b9e88d5d8 >config: riscv-allyesconfig (https://download.01.org/0day-ci/archive/20220616/202206160559.fVKJx0ST-lkp@intel.com/config) >compiler: riscv64-linux-gcc (GCC) 11.3.0 >reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/intel-lab-lkp/linux/commit/49c4086768b5aff410a4a19ca740f8ae8e211844 > git remote add linux-review https://github.com/intel-lab-lkp/linux > git fetch --no-tags linux-review Liang-He/drivers-soc-sifive-Add-missing-of_node_put-in-sifive_l2_cache-c/20220615-202528 > git checkout 49c4086768b5aff410a4a19ca740f8ae8e211844 > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/soc/ > >If you fix the issue, kindly add following tag where applicable >Reported-by: kernel test robot <lkp@intel.com> > Thanks for this report, now I make a new patch and add "Reported-by: kernel test robot <lkp@intel.com>" >All errors (new ones prefixed by >>): > > drivers/soc/sifive/sifive_l2_cache.c: In function 'sifive_l2_init': >>> drivers/soc/sifive/sifive_l2_cache.c:224:17: error: expected ';' before 'goto' > 224 | goto out_put; > | ^~~~ > > Thanks for all your effort to improve the patch.
© 2016 - 2026 Red Hat, Inc.