linux-kernel - Re: [PATCH v2 04/22] clk: samsung: exynos5410: Provide fin

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 May 2016 07:26:19 +0200
From:	Krzysztof Kozlowski <krzk@...nel.org>
To:	Javier Martinez Canillas <javier@....samsung.com>,
	Kukjin Kim <kgene@...nel.org>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Sylwester Nawrocki <s.nawrocki@...sung.com>,
	Tomasz Figa <tomasz.figa@...il.com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH v2 04/22] clk: samsung: exynos5410: Provide fin_pll
 external fixed clock

On 05/09/2016 10:19 PM, Javier Martinez Canillas wrote:
> Hello Krzysztof,
> 
> On 05/08/2016 03:05 PM, Krzysztof Kozlowski wrote:
>> Just like clock driver for Exynos542x/5800, provide the fixed clock here
>> so the clock bindings and their consumers would be consistent and
>> similar.
>>
>> However a clock named "fin_pll" is already provided by generic
>> fixed-clock and it is both referenced in the clock driver (by name) and
>> in DT (by phandle). To make the transition smooth, first introduce the
>> new external fixed clock here under temporary, different name and switch
>> internal users to it.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzk@...nel.org>
>> ---
> 
> [snip]
> 
>>  
>> +/* Same as in Exynos5420 */
>> +static const struct of_device_id ext_clk_match[] __initconst = {
>> +	{ .compatible = "samsung,exynos5420-oscclk", .data = (void *)0, },
> 
> Since using designated initializers, I was about to say that there is no need
> to explicitly set .data to 0 since omitted fields are implicitly initialized...
> 
>> +	{ },
>> +};
>> +
>>  /* register exynos5410 clocks */
>>  static void __init exynos5410_clk_init(struct device_node *np)
>>  {
>> @@ -192,6 +204,10 @@ static void __init exynos5410_clk_init(struct device_node *np)
>>  
>>  	ctx = samsung_clk_init(np, reg_base, CLK_NR_CLKS);
>>  
>> +	samsung_clk_of_register_fixed_ext(ctx, exynos5x_fixed_rate_ext_clks,
>> +			ARRAY_SIZE(exynos5x_fixed_rate_ext_clks),
>> +			ext_clk_match);
>> +
> 
> ... but then I noticed that .data is used as exynos5x_fixed_rate_ext_clks
> array index in samsung_clk_of_register_fixed_ext(). So makes sense to set
> it explicitly to make the intent clear.

Yes, that is the intention here. The '(void *)0' is a hint that clock
needs frequency from DT.

Thanks for review,
Krzysztof