Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1460051 > unrolled thread

Re: [RFC PATCH v2] clk: move check of CLK_SET_RATE_GATE flag to clk_propagate_rate_change()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH v2] clk: move check of CLK_SET_RATE_GATE flag to  clk_propagate_rate_change() Stephen Boyd <sboyd@codeaurora.org> - 2016-08-11 00:20 +0200
    Re: [RFC PATCH v2] clk: move check of CLK_SET_RATE_GATE flag to clk_propagate_rate_change() Jiada Wang <jiada_wang@mentor.com> - 2016-08-23 09:20 +0200

#1460051 — Re: [RFC PATCH v2] clk: move check of CLK_SET_RATE_GATE flag to clk_propagate_rate_change()

On 07/10, jiada_wang@mentor.com wrote:
> From: Jiada Wang <jiada_wang@mentor.com>
> 
> Previously CLK_SET_RATE_GATE flag is only checked in clk_set_rate()
> which only ensures the clock being called by clk_set_rate() won't
> change rate when it has been prepared if CLK_SET_RATE_GATE flag is set.
> But a clk_set_rate() request may propagate rate change to these clocks
> from the requested clock's topmost parent clock to all its offsprings,

s/offsprings/children/ please

> when any one of these clocks has CLK_SET_RATE_GATE flag set
> and it has been prepared, the clk_set_rate() request should fail.
> 
> This patch moves check of CLK_SET_RATE_GATE flag to
> clk_propagate_rate_change() to ensure all affected clocks will
> be checked if their rate will be changed after clk_set_rate().
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>

I'm slightly worried that this will break providers that were
relying on the previous (mis)behavior of this flag. For example,
I think I have this flag set on clks in the qcom/gcc-msm8960.c
driver that have so far not triggered but will trigger now with
this patch. I suppose we should just delete the flag from those
clks because things are working fine so far anyway.

This also brings up the question about what drivers should do if
this flag is set and clk_set_rate() fails. Should drivers need to
know if they're on a platform where clk_set_rate() is going to
fail because the clk is not gated and take appropriate action?
How would they know this? Or should the framework forcibly gate
the clk and all the children, change the rate, and then ungate?

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

[toc] | [next] | [standalone]

#1468335 — Re: [RFC PATCH v2] clk: move check of CLK_SET_RATE_GATE flag to clk_propagate_rate_change()

Hello Stephen

On 08/11/2016 07:19 AM, Stephen Boyd wrote:
> On 07/10, jiada_wang@mentor.com wrote:
>> From: Jiada Wang <jiada_wang@mentor.com>
>>
>> Previously CLK_SET_RATE_GATE flag is only checked in clk_set_rate()
>> which only ensures the clock being called by clk_set_rate() won't
>> change rate when it has been prepared if CLK_SET_RATE_GATE flag is set.
>> But a clk_set_rate() request may propagate rate change to these clocks
>> from the requested clock's topmost parent clock to all its offsprings,
>
> s/offsprings/children/ please
>
will update in next version.

>> when any one of these clocks has CLK_SET_RATE_GATE flag set
>> and it has been prepared, the clk_set_rate() request should fail.
>>
>> This patch moves check of CLK_SET_RATE_GATE flag to
>> clk_propagate_rate_change() to ensure all affected clocks will
>> be checked if their rate will be changed after clk_set_rate().
>>
>> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
>
> I'm slightly worried that this will break providers that were
> relying on the previous (mis)behavior of this flag. For example,
> I think I have this flag set on clks in the qcom/gcc-msm8960.c
> driver that have so far not triggered but will trigger now with
> this patch. I suppose we should just delete the flag from those
> clks because things are working fine so far anyway.
>
I am also worrying about this, that was why I added RFC tag in my patch.
I am not sure if remove all existing CLK_SET_RATE_GATE flags will cause
any issue, for example CLK_SET_RATE_GATE flag still works for these
clocks directly called by clk_set_rate(). if remove all
CLK_SET_RATE_GATE flags, will cause functional change for these clocks.

> This also brings up the question about what drivers should do if
> this flag is set and clk_set_rate() fails. Should drivers need to
> know if they're on a platform where clk_set_rate() is going to
> fail because the clk is not gated and take appropriate action?
> How would they know this? Or should the framework forcibly gate
> the clk and all the children, change the rate, and then ungate?
>
IMO, an error message with the error'ing clock to notify user that
clk_set_rate() is necessary. but clock framework don't need to forcibly
gate the clock (as the clock with CLK_SET_RATE_GATE flag maybe owned by
some other module)

Thanks,
Jiada

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web