2022-06-16 03:39:50

by Liang He

[permalink] [raw]
Subject: [PATCH v2] drivers: clk: (clk-nomadik) Add missing of_node_put()

In nomadik_src_init, of_find_matching_node() return a node pointer
with refcount incremented. We should use of_node_put() in fail path
or when it is not used anymore.

Signed-off-by: Liang He <[email protected]>
---
changelog:

v2: (1) use real name for Sob (2) use goto-label patch style
(3) use proper subject title
v1: use two of_node_put() in fail path and normal-return

drivers/clk/clk-nomadik.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-nomadik.c b/drivers/clk/clk-nomadik.c
index bad2677e11ae..48d5772b36a5 100644
--- a/drivers/clk/clk-nomadik.c
+++ b/drivers/clk/clk-nomadik.c
@@ -99,7 +99,7 @@ static void __init nomadik_src_init(void)
if (!src_base) {
pr_err("%s: must have src parent node with REGS (%pOFn)\n",
__func__, np);
- return;
+ goto out_put;
}

/* Set all timers to use the 2.4 MHz TIMCLK */
@@ -132,6 +132,9 @@ static void __init nomadik_src_init(void)
}
writel(val, src_base + SRC_XTALCR);
register_reboot_notifier(&nomadik_clk_reboot_notifier);
+
+output:
+ of_node_put(np);
}

/**
--
2.25.1


2022-06-16 06:17:58

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2] drivers: clk: (clk-nomadik) Add missing of_node_put()

Same subject line comment applies here. Look at previous patches to the
file.

Quoting Liang He (2022-06-15 20:24:48)
> diff --git a/drivers/clk/clk-nomadik.c b/drivers/clk/clk-nomadik.c
> index bad2677e11ae..48d5772b36a5 100644
> --- a/drivers/clk/clk-nomadik.c
> +++ b/drivers/clk/clk-nomadik.c
> @@ -99,7 +99,7 @@ static void __init nomadik_src_init(void)
> if (!src_base) {
> pr_err("%s: must have src parent node with REGS (%pOFn)\n",
> __func__, np);
> - return;
> + goto out_put;
> }
>
> /* Set all timers to use the 2.4 MHz TIMCLK */
> @@ -132,6 +132,9 @@ static void __init nomadik_src_init(void)
> }
> writel(val, src_base + SRC_XTALCR);
> register_reboot_notifier(&nomadik_clk_reboot_notifier);
> +
> +output:
> + of_node_put(np);

tabbed incorrectly?

2022-06-16 08:44:50

by Liang He

[permalink] [raw]
Subject: Re:Re: [PATCH v2] drivers: clk: (clk-nomadik) Add missing of_node_put()



At 2022-06-16 14:02:23, "Stephen Boyd" <[email protected]> wrote:
>Same subject line comment applies here. Look at previous patches to the
>file.

Sorry, what do you mean same subject line comment?

I add a [PATCH v2] tag when I use 'git format-patch -v 2'.

You mean the two commit has same subject content?

Sorry, I am a beginner to submit patch commit.

>
>Quoting Liang He (2022-06-15 20:24:48)
>> diff --git a/drivers/clk/clk-nomadik.c b/drivers/clk/clk-nomadik.c
>> index bad2677e11ae..48d5772b36a5 100644
>> --- a/drivers/clk/clk-nomadik.c
>> +++ b/drivers/clk/clk-nomadik.c
>> @@ -99,7 +99,7 @@ static void __init nomadik_src_init(void)
>> if (!src_base) {
>> pr_err("%s: must have src parent node with REGS (%pOFn)\n",
>> __func__, np);
>> - return;
>> + goto out_put;
>> }
>>
>> /* Set all timers to use the 2.4 MHz TIMCLK */
>> @@ -132,6 +132,9 @@ static void __init nomadik_src_init(void)
>> }
>> writel(val, src_base + SRC_XTALCR);
>> register_reboot_notifier(&nomadik_clk_reboot_notifier);
>> +
>> +output:
>> + of_node_put(np);
>
>tabbed incorrectly?

Sorry for this incorrct tab setting.

2022-06-16 14:36:37

by Liang He

[permalink] [raw]
Subject: Re:Re: [PATCH v2] drivers: clk: (clk-nomadik) Add missing of_node_put()


At 2022-06-16 22:08:16, "kernel test robot" <[email protected]> wrote:
>Hi Liang,
>
>Thank you for the patch! Yet something to improve:
>
>[auto build test ERROR on clk/clk-next]
>[also build test ERROR on linus/master v5.19-rc2 next-20220616]
>[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-clk-clk-nomadik-Add-missing-of_node_put/20220616-112612
>base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
>config: arm-nhk8815_defconfig (https://download.01.org/0day-ci/archive/20220616/[email protected]/config)
>compiler: arm-linux-gnueabi-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/12a305918ea40c01116a3cab94fe62a487eac499
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Liang-He/drivers-clk-clk-nomadik-Add-missing-of_node_put/20220616-112612
> git checkout 12a305918ea40c01116a3cab94fe62a487eac499
> # 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=arm SHELL=/bin/bash
>
>If you fix the issue, kindly add following tag where applicable
>Reported-by: kernel test robot <[email protected]>
>
>All errors (new ones prefixed by >>):
>
> drivers/clk/clk-nomadik.c: In function 'nomadik_src_init':
> drivers/clk/clk-nomadik.c:136:1: warning: label 'output' defined but not used [-Wunused-label]
> 136 | output:
> | ^~~~~~
>>> drivers/clk/clk-nomadik.c:102:17: error: label 'out_put' used but not defined
> 102 | goto out_put;
> | ^~~~
>
>
>vim +/out_put +102 drivers/clk/clk-nomadik.c
>
> 87
> 88 static void __init nomadik_src_init(void)
> 89 {
> 90 struct device_node *np;
> 91 u32 val;
> 92
> 93 np = of_find_matching_node(NULL, nomadik_src_match);
> 94 if (!np) {
> 95 pr_crit("no matching node for SRC, aborting clock init\n");
> 96 return;
> 97 }
> 98 src_base = of_iomap(np, 0);
> 99 if (!src_base) {
> 100 pr_err("%s: must have src parent node with REGS (%pOFn)\n",
> 101 __func__, np);
> > 102 goto out_put;
> 103 }
> 104
> 105 /* Set all timers to use the 2.4 MHz TIMCLK */
> 106 val = readl(src_base + SRC_CR);
> 107 val |= SRC_CR_T0_ENSEL;
> 108 val |= SRC_CR_T1_ENSEL;
> 109 val |= SRC_CR_T2_ENSEL;
> 110 val |= SRC_CR_T3_ENSEL;
> 111 val |= SRC_CR_T4_ENSEL;
> 112 val |= SRC_CR_T5_ENSEL;
> 113 val |= SRC_CR_T6_ENSEL;
> 114 val |= SRC_CR_T7_ENSEL;
> 115 writel(val, src_base + SRC_CR);
> 116
> 117 val = readl(src_base + SRC_XTALCR);
> 118 pr_info("SXTALO is %s\n",
> 119 (val & SRC_XTALCR_SXTALDIS) ? "disabled" : "enabled");
> 120 pr_info("MXTAL is %s\n",
> 121 (val & SRC_XTALCR_MXTALSTAT) ? "enabled" : "disabled");
> 122 if (of_property_read_bool(np, "disable-sxtalo")) {
> 123 /* The machine uses an external oscillator circuit */
> 124 val |= SRC_XTALCR_SXTALDIS;
> 125 pr_info("disabling SXTALO\n");
> 126 }
> 127 if (of_property_read_bool(np, "disable-mxtalo")) {
> 128 /* Disable this too: also run by external oscillator */
> 129 val |= SRC_XTALCR_MXTALOVER;
> 130 val &= ~SRC_XTALCR_MXTALEN;
> 131 pr_info("disabling MXTALO\n");
> 132 }
> 133 writel(val, src_base + SRC_XTALCR);
> 134 register_reboot_notifier(&nomadik_clk_reboot_notifier);
> 135
> > 136 output:
> 137 of_node_put(np);
> 138 }
> 139
>
>--
>0-DAY CI Kernel Test Service
>https://01.org/lkp

Sorry for my early mistake. And I have merged all 'clk' bug-fixes into a new commit [PATCH v3].

Thanks for this report.

2022-06-16 15:12:19

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2] drivers: clk: (clk-nomadik) Add missing of_node_put()

Hi Liang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on linus/master v5.19-rc2 next-20220616]
[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-clk-clk-nomadik-Add-missing-of_node_put/20220616-112612
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: arm-nhk8815_defconfig (https://download.01.org/0day-ci/archive/20220616/[email protected]/config)
compiler: arm-linux-gnueabi-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/12a305918ea40c01116a3cab94fe62a487eac499
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Liang-He/drivers-clk-clk-nomadik-Add-missing-of_node_put/20220616-112612
git checkout 12a305918ea40c01116a3cab94fe62a487eac499
# 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=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/clk/clk-nomadik.c: In function 'nomadik_src_init':
drivers/clk/clk-nomadik.c:136:1: warning: label 'output' defined but not used [-Wunused-label]
136 | output:
| ^~~~~~
>> drivers/clk/clk-nomadik.c:102:17: error: label 'out_put' used but not defined
102 | goto out_put;
| ^~~~


vim +/out_put +102 drivers/clk/clk-nomadik.c

87
88 static void __init nomadik_src_init(void)
89 {
90 struct device_node *np;
91 u32 val;
92
93 np = of_find_matching_node(NULL, nomadik_src_match);
94 if (!np) {
95 pr_crit("no matching node for SRC, aborting clock init\n");
96 return;
97 }
98 src_base = of_iomap(np, 0);
99 if (!src_base) {
100 pr_err("%s: must have src parent node with REGS (%pOFn)\n",
101 __func__, np);
> 102 goto out_put;
103 }
104
105 /* Set all timers to use the 2.4 MHz TIMCLK */
106 val = readl(src_base + SRC_CR);
107 val |= SRC_CR_T0_ENSEL;
108 val |= SRC_CR_T1_ENSEL;
109 val |= SRC_CR_T2_ENSEL;
110 val |= SRC_CR_T3_ENSEL;
111 val |= SRC_CR_T4_ENSEL;
112 val |= SRC_CR_T5_ENSEL;
113 val |= SRC_CR_T6_ENSEL;
114 val |= SRC_CR_T7_ENSEL;
115 writel(val, src_base + SRC_CR);
116
117 val = readl(src_base + SRC_XTALCR);
118 pr_info("SXTALO is %s\n",
119 (val & SRC_XTALCR_SXTALDIS) ? "disabled" : "enabled");
120 pr_info("MXTAL is %s\n",
121 (val & SRC_XTALCR_MXTALSTAT) ? "enabled" : "disabled");
122 if (of_property_read_bool(np, "disable-sxtalo")) {
123 /* The machine uses an external oscillator circuit */
124 val |= SRC_XTALCR_SXTALDIS;
125 pr_info("disabling SXTALO\n");
126 }
127 if (of_property_read_bool(np, "disable-mxtalo")) {
128 /* Disable this too: also run by external oscillator */
129 val |= SRC_XTALCR_MXTALOVER;
130 val &= ~SRC_XTALCR_MXTALEN;
131 pr_info("disabling MXTALO\n");
132 }
133 writel(val, src_base + SRC_XTALCR);
134 register_reboot_notifier(&nomadik_clk_reboot_notifier);
135
> 136 output:
137 of_node_put(np);
138 }
139

--
0-DAY CI Kernel Test Service
https://01.org/lkp