linux-kernel - Re: [PATCH v4 3/8] clk: add support for clocks provided by SCP(System Control Processor)

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]

Message-ID: <55A945DF.3@codeaurora.org>
Date:	Fri, 17 Jul 2015 11:13:51 -0700
From:	Stephen Boyd <sboyd@...eaurora.org>
To:	Sudeep Holla <sudeep.holla@....com>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	"linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Liviu Dudau <Liviu.Dudau@....com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
	"Jon Medhurst (Tixy)" <tixy@...aro.org>,
	Arnd Bergmann <arnd@...db.de>,
	Kevin Hilman <khilman@...nel.org>,
	Olof Johansson <olof@...om.net>,
	Mike Turquette <mturquette@...libre.com>
Subject: Re: [PATCH v4 3/8] clk: add support for clocks provided by SCP(System
 Control Processor)

On 07/17/2015 04:17 AM, Sudeep Holla wrote:
>
>
> On 16/07/15 20:31, Stephen Boyd wrote:
>> On 07/16, Sudeep Holla wrote:
>>> On 08/07/15 02:46, Stephen Boyd wrote:
>>>>
>>>> Yes struct clk would have min/max, and struct clk_core would have
>>>> min/max. Then some sort of provider API (or possibly even
>>>> clk_init_data) would take the min/max fields and copy them over
>>>> to struct clk_core. Then during set_rate operations we would
>>>> aggregate the constraints from struct clk like we already do and
>>>> add in the constrains in struct clk_core.
>>>>
>>>> One downside to adding new fields to clk_init_data is that there
>>>> are drivers out there that aren't initializing that structure to
>>>> 0, and they're putting it on the stack, so stack junk can come
>>>> through. Furthermore, min/max would mean that every driver needs
>>>> to specify some large number for max or we have to special case
>>>> min == max == 0 and ignore it. Somehow it needs to be opt-in. If
>>>> we want to go down the clk_init_data route then perhaps we need
>>>> some sort of rate_constraint struct pointer in there that drivers
>>>> can optionally setup.
>>>>
>>>>     struct clk_rate_constraint {
>>>>         unsigned long min;
>>>>         unsigned long max;
>>>>     };
>>>>
>>>>     struct clk_init_data {
>>>>         ...
>>>>         struct clk_rate_constraint *rate_constraint;
>>>>     };
>>>>
>>>> I haven't thought it through completely, but I can probably write
>>>> up some patch tomorrow after I sleep on it.
>>>>
>>>
>>> I am hoping to get this series for v4.3. In order to avoid using
>>> consumer API, I can revert back to the min,max check I had in the
>>> round_rate earlier if that's fine with you ? Let me know so that I can
>>> post the next version based on that. All the other comments are already
>>> addressed.
>>
>> Ok. I'm fine with the consumer API being used, but it would be
>> nice if we didn't have to do so. Try out the patch below,
>> hopefully it's good enough for your purposes. It may need to be
>> more robust, and we may still want to use the init_data structure
>> to avoid races with providers and consumers, but we can leave
>> that for later after sweeping all the structure users.
>>
>
> Agreed, I would avoid using clk consumer API or use it with TODO so that
> I remember to remove it soon. Anyways, thanks for the patch, I tested it
> and works fine to me. You can add Tested-by if you decide to push it.

Thanks. I pushed it to -next last night but it probably hasn't shown up yet.

>
>>>
>>> Also since this series depends on SCPI, I was thinking to get it merged
>>> via ARM-SoC, but that might conflict with the round_rate prototype
>>> change. Do do plan to share a stable base with arm-soc guys or you
>>> expect all the changes to be contained in clk tree ?
>>>
>>
>> We can share a stable branch for the determine_rate change with
>> arm-soc. We already have it on a separate branch but haven't
>> published it so far because nobody has asked.
>>
>
> determine_rate change shouldn't affect SCPI clock driver but I remember
> seeing round_rate change too on the list which returns value using the
> argument from Boris. Is that planned for v4.3 ? I would need the stable
> branch from this clk_hw_set_rate_range if you decide to push. Let me
> know your preferences. I will post the updated version of the patch
> accordingly.
>

We're not going to change round_rate() so it sounds like you don't need 
a stable branch. But you would need this new consumer API. So you still 
need a branch right?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

--
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/