linux-kernel - Re: [PATCH v7 2/3] clk: introduce the common clock framework

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:	Wed, 28 Mar 2012 15:25:46 -0700
From:	Saravana Kannan <skannan@...eaurora.org>
To:	"Turquette, Mike" <mturquette@...com>
CC:	Shawn Guo <shawn.guo@...aro.org>, Paul Walmsley <paul@...an.com>,
	Russell King <linux@....linux.org.uk>,
	Linus Walleij <linus.walleij@...ricsson.com>,
	patches@...aro.org, Magnus Damm <magnus.damm@...il.com>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	linux-kernel@...r.kernel.org, linaro-dev@...ts.linaro.org,
	Jeremy Kerr <jeremy.kerr@...onical.com>,
	linux-arm-kernel@...ts.infradead.org,
	Arnd Bergman <arnd.bergmann@...aro.org>
Subject: Re: [PATCH v7 2/3] clk: introduce the common clock framework

On 03/28/2012 10:08 AM, Turquette, Mike wrote:
> On Tue, Mar 27, 2012 at 8:06 PM, Saravana Kannan<skannan@...eaurora.org>  wrote:

snip

>> I think there is still a problem with not being able to differentiate
>> between pre-change recalc and post-change recalc. This applies for any set
>> parent and set rate on a clock that has children.
>>
>> Consider this simple example:
>> * Divider clock B is fed from clock A.
>> * Clock B can never output>  120 MHz due to downstream
>>   HW/clock limitations.
>> * Clock A is running at 200 MHz
>> * Clock B divider is set to 2.
>>
>> Now, say the rate of clock A is changing from 200 MHz to 300 MHz (due to set
>> rate or set parent). In this case, the clock B divider should be set to 3
>> pre-rate change to guarantee that the output of clock B is never>  120 MHz.
>> So the rate of clock B will go from 100 MHz (200/2) to 66 MHz (200/3) to 100
>> MHz (300/3) and everything is good.
>>
>> Assume we somehow managed to do the above. So, now clock A is at 300 MHz,
>> clock B divider is at 3 and the clock B output is 100 MHz.
>>
>> Now, say the rate of clock A changes from 300 MHz to 100 MHz. In this case
>> the clock B divider should only be changed post rate change. If we do it pre
>> rate change, then the output will go from 100 MHz (300/3) to 150 MHz (300/1)
>> to 100 MHz (100/1). We went past the 120 MHz limit of clock B's output rate.
>>
>> If we do this post rate change, we can do this without exceeding the max
>> output limit of clock B. It will go from 100 MHz (300/3) to 33 MHz (100/3)
>> to 100 MHz (100/1). We never went past the 120 MHz limit.
>>
>> So, at least for this reason above, I think we need to pass a pre/post
>> parameter to the recalc ops.

Sorry if I wasn't clear. But the case above is a separate issue from 
what I mention below. What are your thoughts on handling this? Pass 
"msg" to recalc_rates?

>> While we are at it, we should probably just add a failure option for recalc
>> to make it easy to reject unacceptable rate changes. To keep the clock
>> framework code simpler, you could decide to allow errors only for the
>> pre-change recalc calls. That way, the error case roll back code won't be
>> crazy.
>
> recalc is too late to catch this.  I think you mean round_rate.  We
> want to determine which rate changes are out-of-spec during
> clk_calc_new_rates (for clk_set_rate) which involves round_rate being
> a bit smarter about what it can and cannot do.

The case I'm referring to is set_parent(). set_rate() and set_parent() 
have a lot of common implications and it doesn't look like the clock 
framework is handling the common cases the same way for both 
set_parent() and set_rate().

It almost feels like set_parent() and set_rate() should just be wrappers 
around some common function that handles most of the work. After all, 
set_parent() is just a slimmed down version of set_rate(). Set rate is 
just a combination of set parent and set divider.

>
> Anyways I'm looking at ways to do this in my clk-dependencies branch.
>

Are you also looking into the pre/post rate change divider handling case 
I mentioned above?

Thanks,
Saravana

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
--
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/