On 2020/11/10 1:54, Markus Elfring wrote: > … >> +++ b/drivers/clk/hisilicon/clk-hi3620.c > … >> @@ -478,6 +478,10 @@ static void __init hi3620_mmc_clk_init(struct >> device_node *node) >> >> clk_data->clk_num = num; >> of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); >> +free_clk_data: >> + kfree(clk_data); > … > > * Should any system resources be kept allocated if the execution > of this function implementation succeeded? > > * How do you think about to add the statement “return;” > after the call of the function “of_clk_add_provider”? > > * Should another return value be also checked here? sure. > > See also: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk.c?id=f8394f232b1eab649ce2df5c5f15b0e528c92091#n4414 > https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/clk/clk.c#L4414 > > * Would you like to use the function “of_clk_add_hw_provider” instead? How about we still use of_clk_add_provider()? It doesn't seem to make difference using of_clk_add_hw_provider(). > > Regards, > Markus
