| Message ID | 20221017165028.4194254-1-dario.binacchi@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Dario,
I love your patch! Yet something to improve:
[auto build test ERROR on clk/clk-next]
[also build test ERROR on linus/master v6.1-rc1 next-20221017]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Dario-Binacchi/clk-ti-dra7-atl-don-t-allocate-parent_names-variable/20221018-005059
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20221017165028.4194254-1-dario.binacchi%40amarulasolutions.com
patch subject: [PATCH] clk: ti: dra7-atl: don't allocate `parent_names' variable
config: arm-defconfig
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.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/9eca9eab009687ea2b8cc0d262de7786b6d449b9
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Dario-Binacchi/clk-ti-dra7-atl-don-t-allocate-parent_names-variable/20221018-005059
git checkout 9eca9eab009687ea2b8cc0d262de7786b6d449b9
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.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 <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/clk/ti/clk-dra7-atl.c: In function 'of_dra7_atl_clock_setup':
>> drivers/clk/ti/clk-dra7-atl.c:190:27: error: assignment to 'const char * const*' from incompatible pointer type 'const char *' [-Werror=incompatible-pointer-types]
190 | init.parent_names = of_clk_get_parent_name(node, 0);
| ^
cc1: some warnings being treated as errors
vim +190 drivers/clk/ti/clk-dra7-atl.c
162
163 static void __init of_dra7_atl_clock_setup(struct device_node *node)
164 {
165 struct dra7_atl_desc *clk_hw = NULL;
166 struct clk_init_data init = { NULL };
167 const char *name;
168 struct clk *clk;
169
170 clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
171 if (!clk_hw) {
172 pr_err("%s: could not allocate dra7_atl_desc\n", __func__);
173 return;
174 }
175
176 clk_hw->hw.init = &init;
177 clk_hw->divider = 1;
178 name = ti_dt_clk_name(node);
179 init.name = name;
180 init.ops = &atl_clk_ops;
181 init.flags = CLK_IGNORE_UNUSED;
182 init.num_parents = of_clk_get_parent_count(node);
183
184 if (init.num_parents != 1) {
185 pr_err("%s: atl clock %pOFn must have 1 parent\n", __func__,
186 node);
187 goto cleanup;
188 }
189
> 190 init.parent_names = of_clk_get_parent_name(node, 0);
191
192 clk = ti_clk_register(NULL, &clk_hw->hw, name);
193 if (!IS_ERR(clk)) {
194 of_clk_add_provider(node, of_clk_src_simple_get, clk);
195 return;
196 }
197
198 cleanup:
199 kfree(clk_hw);
200 }
201 CLK_OF_DECLARE(dra7_atl_clock, "ti,dra7-atl-clock", of_dra7_atl_clock_setup);
202
diff --git a/drivers/clk/ti/clk-dra7-atl.c b/drivers/clk/ti/clk-dra7-atl.c index ff4d6a951681..ec2473069143 100644 --- a/drivers/clk/ti/clk-dra7-atl.c +++ b/drivers/clk/ti/clk-dra7-atl.c @@ -164,7 +164,6 @@ static void __init of_dra7_atl_clock_setup(struct device_node *node) { struct dra7_atl_desc *clk_hw = NULL; struct clk_init_data init = { NULL }; - const char **parent_names = NULL; const char *name; struct clk *clk; @@ -188,24 +187,15 @@ static void __init of_dra7_atl_clock_setup(struct device_node *node) goto cleanup; } - parent_names = kzalloc(sizeof(char *), GFP_KERNEL); - - if (!parent_names) - goto cleanup; - - parent_names[0] = of_clk_get_parent_name(node, 0); - - init.parent_names = parent_names; + init.parent_names = of_clk_get_parent_name(node, 0); clk = ti_clk_register(NULL, &clk_hw->hw, name); - if (!IS_ERR(clk)) { of_clk_add_provider(node, of_clk_src_simple_get, clk); - kfree(parent_names); return; } + cleanup: - kfree(parent_names); kfree(clk_hw); } CLK_OF_DECLARE(dra7_atl_clock, "ti,dra7-atl-clock", of_dra7_atl_clock_setup);
The `parent_names' variable was freed also in case of kzalloc() error. Instead of modifying the code to perform a proper memory release, I decided to fix the bug by not allocating memory. Since only one parent name is referenced, it is not necessary to allocate this variable at runtime and therefore you can avoid calling the kzalloc() function. This simplifies the code (even calls to kfree can be removed) and improves the performance of the routine. Note: Although no operation is performed by kfree() on a NULL pointer, it was however suboptimal and semantically wrong doing it. Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> --- drivers/clk/ti/clk-dra7-atl.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)