Re: [PATCH v2] clk: hisilicon: Free clk_data and unmap region obtained by of_iomap

[Dongjiu Geng]

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