All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sudeep Holla <sudeep.holla@arm.com>
To: Stephen Boyd <sboyd@codeaurora.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-clk@vger.kernel.org" <linux-clk@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Liviu Dudau <Liviu.Dudau@arm.com>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
	"Jon Medhurst (Tixy)" <tixy@linaro.org>,
	Arnd Bergmann <arnd@arndb.de>, Kevin Hilman <khilman@kernel.org>,
	Olof Johansson <olof@lixom.net>,
	Mike Turquette <mturquette@linaro.org>
Subject: Re: [PATCH v4 3/8] clk: add support for clocks provided by SCP(System Control Processor)
Date: Tue, 07 Jul 2015 17:03:36 +0100	[thread overview]
Message-ID: <559BF858.1020301@arm.com> (raw)
In-Reply-To: <559ADC94.4080505@codeaurora.org>



On 06/07/15 20:52, Stephen Boyd wrote:
> On 07/03/2015 07:52 AM, Sudeep Holla wrote:
>> Hi Stephen,
>>
>>
>> Thanks for the review.
>>
>> On 02/07/15 18:23, Stephen Boyd wrote:
>>> On 06/08, Sudeep Holla wrote:

[...]

>>>> +
>>>> +    clk = devm_clk_register(dev, &sclk->hw);
>>>> +    if (!IS_ERR(clk) && max)
>>>> +        clk_set_rate_range(clk, min, max);
>>>
>>> Hm.. we're planning to make clk_register() return a struct
>>> clk_hw, so this will block that. We need some sort of clk_hw API
>>> that allows us to setup min/max limits on the clock from the
>>> provider side. Care to add that?
>>>
>>
>> Can you provide pointer to the patches or the tree containing those
>> changes ? Are they targeted for v4.3 ?
>>
>
> If I have time I may try to start doing the clk_register() conversion,
> but it will take a while so I doubt it will be in v4.3. I'm asking if
> you can add a clk_hw based API that does something like
> clk_set_rate_range() without requiring a struct clk pointer. i.e.
> clk_hw_set_rate_range(struct clk_hw *hw, min, max) that constraints the
> min/max rate of the clock. This way, the driver is only using clk
> provider APIs and not clk consumer APIs.
>

I understand the intention of separating clk provider helpers/APIs
and clk consumer APIs. Since {min,max}_rate are part of struct clk
itself, I was thinking that you would have moved it to struct clk_core
as part of the rework you mentioned and hence asked about the patches.

IIUC, if {min,max}_rate remain part of struct clk, then how are we
restricting that operation to just the clk providers ? clk consumer
can still directly modify or use clk_set_rate_range.

Do we continue to provide that feature for both provider and consumer ?
If so I assume {min,max}_rate range requested by consumer should be
within the limits set by provider and do we maintain both the limits ?

Sorry if I am missing something fundamental since I don't have much
knowledge of clk layer internals.

Regards,
Sudeep

WARNING: multiple messages have this Message-ID (diff)
From: sudeep.holla@arm.com (Sudeep Holla)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 3/8] clk: add support for clocks provided by SCP(System Control Processor)
Date: Tue, 07 Jul 2015 17:03:36 +0100	[thread overview]
Message-ID: <559BF858.1020301@arm.com> (raw)
In-Reply-To: <559ADC94.4080505@codeaurora.org>



On 06/07/15 20:52, Stephen Boyd wrote:
> On 07/03/2015 07:52 AM, Sudeep Holla wrote:
>> Hi Stephen,
>>
>>
>> Thanks for the review.
>>
>> On 02/07/15 18:23, Stephen Boyd wrote:
>>> On 06/08, Sudeep Holla wrote:

[...]

>>>> +
>>>> +    clk = devm_clk_register(dev, &sclk->hw);
>>>> +    if (!IS_ERR(clk) && max)
>>>> +        clk_set_rate_range(clk, min, max);
>>>
>>> Hm.. we're planning to make clk_register() return a struct
>>> clk_hw, so this will block that. We need some sort of clk_hw API
>>> that allows us to setup min/max limits on the clock from the
>>> provider side. Care to add that?
>>>
>>
>> Can you provide pointer to the patches or the tree containing those
>> changes ? Are they targeted for v4.3 ?
>>
>
> If I have time I may try to start doing the clk_register() conversion,
> but it will take a while so I doubt it will be in v4.3. I'm asking if
> you can add a clk_hw based API that does something like
> clk_set_rate_range() without requiring a struct clk pointer. i.e.
> clk_hw_set_rate_range(struct clk_hw *hw, min, max) that constraints the
> min/max rate of the clock. This way, the driver is only using clk
> provider APIs and not clk consumer APIs.
>

I understand the intention of separating clk provider helpers/APIs
and clk consumer APIs. Since {min,max}_rate are part of struct clk
itself, I was thinking that you would have moved it to struct clk_core
as part of the rework you mentioned and hence asked about the patches.

IIUC, if {min,max}_rate remain part of struct clk, then how are we
restricting that operation to just the clk providers ? clk consumer
can still directly modify or use clk_set_rate_range.

Do we continue to provide that feature for both provider and consumer ?
If so I assume {min,max}_rate range requested by consumer should be
within the limits set by provider and do we maintain both the limits ?

Sorry if I am missing something fundamental since I don't have much
knowledge of clk layer internals.

Regards,
Sudeep

  reply	other threads:[~2015-07-07 16:03 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-08 10:39 [PATCH v4 0/8] ARM64: juno: add SCPI mailbox protocol, clock and CPUFreq support Sudeep Holla
2015-06-08 10:39 ` Sudeep Holla
2015-06-08 10:39 ` [PATCH v4 1/8] Documentation: add DT binding for ARM System Control and Power Interface(SCPI) protocol Sudeep Holla
2015-06-08 10:39   ` Sudeep Holla
2015-06-08 10:39   ` Sudeep Holla
2015-07-08 13:59   ` Sudeep Holla
2015-07-08 13:59     ` Sudeep Holla
2015-07-22  8:43   ` Liviu Dudau
2015-07-22  8:43     ` Liviu Dudau
2015-07-22  8:43     ` Liviu Dudau
2015-07-22  9:25     ` Sudeep Holla
2015-07-22  9:25       ` Sudeep Holla
2015-07-22  9:55   ` Mark Rutland
2015-07-22  9:55     ` Mark Rutland
2015-07-22 15:56     ` Sudeep Holla
2015-07-22 15:56       ` Sudeep Holla
2015-07-22 15:56       ` Sudeep Holla
2015-07-22 16:23       ` Mark Rutland
2015-07-22 16:23         ` Mark Rutland
2015-06-08 10:39 ` [PATCH v4 2/8] firmware: add support " Sudeep Holla
2015-06-08 10:39   ` Sudeep Holla
2015-06-11 11:54   ` Jassi Brar
2015-06-11 11:54     ` Jassi Brar
2015-06-11 13:23     ` Sudeep Holla
2015-06-11 13:23       ` Sudeep Holla
2015-06-08 10:39 ` [PATCH v4 3/8] clk: add support for clocks provided by SCP(System Control Processor) Sudeep Holla
2015-06-08 10:39   ` Sudeep Holla
2015-07-02 17:23   ` Stephen Boyd
2015-07-02 17:23     ` Stephen Boyd
2015-07-03 14:52     ` Sudeep Holla
2015-07-03 14:52       ` Sudeep Holla
2015-07-03 16:12       ` Sudeep Holla
2015-07-03 16:12         ` Sudeep Holla
2015-07-06 19:52       ` Stephen Boyd
2015-07-06 19:52         ` Stephen Boyd
2015-07-07 16:03         ` Sudeep Holla [this message]
2015-07-07 16:03           ` Sudeep Holla
2015-07-08  1:46           ` Stephen Boyd
2015-07-08  1:46             ` Stephen Boyd
2015-07-16 16:11             ` Sudeep Holla
2015-07-16 16:11               ` Sudeep Holla
2015-07-16 19:31               ` Stephen Boyd
2015-07-16 19:31                 ` Stephen Boyd
2015-07-17 11:17                 ` Sudeep Holla
2015-07-17 11:17                   ` Sudeep Holla
2015-07-17 18:13                   ` Stephen Boyd
2015-07-17 18:13                     ` Stephen Boyd
2015-07-20  8:54                     ` Sudeep Holla
2015-07-20  8:54                       ` Sudeep Holla
2015-07-21 18:05                       ` Stephen Boyd
2015-07-21 18:05                         ` Stephen Boyd
2015-07-22 14:19                         ` Sudeep Holla
2015-07-22 14:19                           ` Sudeep Holla
2015-06-08 10:39 ` [PATCH v4 4/8] clk: scpi: add support for cpufreq virtual device Sudeep Holla
2015-06-08 10:39   ` Sudeep Holla
2015-06-08 10:39 ` [PATCH v4 5/8] cpufreq: arm_big_little: add SCPI interface driver Sudeep Holla
2015-06-08 10:39   ` Sudeep Holla
2015-06-08 10:40 ` [PATCH v4 6/8] arm64: dts: add SRAM, MHU mailbox and SCPI support on Juno Sudeep Holla
2015-06-08 10:40   ` Sudeep Holla
2015-06-08 13:51   ` Jon Medhurst (Tixy)
2015-06-08 13:51     ` Jon Medhurst (Tixy)
2015-06-08 14:32     ` Sudeep Holla
2015-06-08 14:32       ` Sudeep Holla
2015-06-08 14:35     ` Liviu Dudau
2015-06-08 14:35       ` Liviu Dudau
2015-07-22 13:28   ` Liviu Dudau
2015-07-22 13:28     ` Liviu Dudau
2015-07-22 15:40     ` Sudeep Holla
2015-07-22 15:40       ` Sudeep Holla
2015-07-22 16:06       ` Liviu Dudau
2015-07-22 16:06         ` Liviu Dudau
2015-07-22 16:16         ` Sudeep Holla
2015-07-22 16:16           ` Sudeep Holla
2015-06-08 10:40 ` [PATCH v4 7/8] arm64: dts: add CPU topology " Sudeep Holla
2015-06-08 10:40   ` Sudeep Holla
2015-07-22 13:31   ` Liviu Dudau
2015-07-22 13:31     ` Liviu Dudau
2015-06-08 10:40 ` [PATCH v4 8/8] arm64: dts: add clock support for all the cpus Sudeep Holla
2015-06-08 10:40   ` Sudeep Holla
2015-07-22 13:32   ` Liviu Dudau
2015-07-22 13:32     ` Liviu Dudau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=559BF858.1020301@arm.com \
    --to=sudeep.holla@arm.com \
    --cc=Liviu.Dudau@arm.com \
    --cc=Lorenzo.Pieralisi@arm.com \
    --cc=arnd@arndb.de \
    --cc=khilman@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mturquette@linaro.org \
    --cc=olof@lixom.net \
    --cc=sboyd@codeaurora.org \
    --cc=tixy@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.