linux-kernel - Re: [RFC PATCH v2] clk: move check of CLK_SET_RATE_GATE flag to clk_propagate_rate

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] [day] [month] [year] [list]

Date:   Tue, 23 Aug 2016 16:16:02 +0900
From:   Jiada Wang <jiada_wang@...tor.com>
To:     Stephen Boyd <sboyd@...eaurora.org>
CC:     <mturquette@...libre.com>, <linux-clk@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <joshua_frkuska@...tor.com>,
        <vladimir_zapolskiy@...tor.com>
Subject: 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@...tor.com wrote:
>> From: Jiada Wang <jiada_wang@...tor.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@...tor.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