linux-kernel - Re: [PATCHv3 2/4] clk: socfpga: add a clock driver for the Arria 10 platform

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 19 May 2015 18:12:18 -0500
From:	Dinh Nguyen <dinguyen@...nsource.altera.com>
To:	Stephen Boyd <sboyd@...eaurora.org>
CC:	<mturquette@...aro.org>, <dinh.linux@...il.com>,
	<robh+dt@...nel.org>, <ijc+devicetree@...lion.org.uk>,
	<galak@...eaurora.org>, <mark.rutland@....com>,
	<pawel.moll@....com>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv3 2/4] clk: socfpga: add a clock driver for the Arria
 10 platform



On 5/19/15 4:50 PM, Stephen Boyd wrote:
> On 05/19/15 09:29, Dinh Nguyen wrote:
>>
>> On 5/15/15 7:52 PM, Stephen Boyd wrote:
>>> On 05/07, dinguyen@...nsource.altera.com wrote:
>>>> +
>>>> +static int socfpga_clk_prepare(struct clk_hw *hwclk)
>>>> +{
>>>> +	struct socfpga_gate_clk *socfpgaclk = to_socfpga_gate_clk(hwclk);
>>>> +	struct regmap *sys_mgr_base_addr;
>>>> +	int i;
>>>> +	u32 hs_timing;
>>>> +	u32 clk_phase[2];
>>>> +
>>>> +	if (socfpgaclk->clk_phase[0] || socfpgaclk->clk_phase[1]) {
>>>> +		sys_mgr_base_addr = syscon_regmap_lookup_by_compatible("altr,sys-mgr");
>>>> +		if (IS_ERR(sys_mgr_base_addr)) {
>>> Is there a reason the syscon is grabbed lazily in prepare? Why
>>> not get it before registering this clock?
>> This syscon node is only associated with clocks that have a clk-phase
>> property, which on the SoCFPGA platform, is the SD/MMC clocks. The way
>> to implement this went through quite a few rounds of discussion for the
>> Cyclone5/Arria5 platform before settling to this method.
>>
>> The reason why syscon is grabbed here is that the setting of the clock
>> phase must be done before enabling of the clock, so it seem that prepare
>> was a good place. Should this be move moved to the socfpga_gate_init()
>> instead?
> 
> I was expecting the regmap to be found before the clock is registered
> and stored away into the  socfpga_gate_clk structure. Getting the regmap
> during prepare is akin to ioremapping a register region during prepare,
> which doesn't sound right at all. Maybe there's some good reason in the
> earlier discussions? Any hints?
> 

Ah okay, the earlier discussions revolve mainly around moving the regmap
from the SD/MMC driver into the clock driver. But there weren't any
issue raised for putting the regmap in the prepare function.

If you're curious, here are the links to the discussion for adding the
clk-phase to the driver:

http://archive.arm.linux.org.uk/lurker/message/20131212.203042.d37c8ee9.en.html

http://archive.arm.linux.org.uk/lurker/message/20140109.213116.1f13b27a.en.html

But perhaps putting the regmap lookup in the init function is the
correct way to do this?

Thanks,
Dinh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/