linux-kernel - Re: [PATCH] clk: Propagate prepare and enable when reparenting orphans

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:	Fri, 7 Nov 2014 14:52:38 -0800
From:	Doug Anderson <dianders@...omium.org>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Mike Turquette <mturquette@...aro.org>,
	Heiko Stuebner <heiko@...ech.de>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] clk: Propagate prepare and enable when reparenting orphans

Russell,

On Fri, Nov 7, 2014 at 10:58 AM, Russell King - ARM Linux
<linux@....linux.org.uk> wrote:
> On Fri, Nov 07, 2014 at 10:51:52AM -0800, Doug Anderson wrote:
>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>> index 4896ae9..a3d3d58 100644
>> --- a/drivers/clk/clk.c
>> +++ b/drivers/clk/clk.c
>> @@ -1650,6 +1650,17 @@ void __clk_reparent(struct clk *clk, struct clk *new_parent)
>>       clk_reparent(clk, new_parent);
>>       __clk_recalc_accuracies(clk);
>>       __clk_recalc_rates(clk, POST_RATE_CHANGE);
>> +
>> +     if (clk->prepare_count) {
>> +             unsigned long flags;
>> +
>> +             __clk_prepare(new_parent);
>> +
>> +             flags = clk_enable_lock();
>> +             if (clk->enable_count)
>> +                     __clk_enable(new_parent);
>> +             clk_enable_unlock(flags);
>> +     }
>
> I really don't understand why this isn't already done - I said this was
> necessary a /long/ time ago.
>
> However, the above isn't sufficient.  Think about the old parent - this
> should be disabled and unprepared if it was prepared and enabled by the
> child.

You may be referring of a different bug than I am addressing.  I can
think about the old parent, but it always a tear to my eyes since
these clocks are orphans and had no old parents (unless you count the
matron at the orphanage, but I doubt she was either prepared or
enabled).  ;)

Ah, but I see!  There are other users of this function that are not
part of "clk.c".  Doh!  Since this was a "__" function with no
documentation I assumed it was "static", but I see that it is not.  I
see two callers that are not part of the orphan code.

I'll happily move this code down so it's only called by the orphan
code and not touch the two callers of __clk_reparent(), assuming that
they don't need to deal with this scenario.

NOTE: As far as I can tell, the standard exposed API call is
clk_set_parent().  From reading comments that does move the prepared /
enabled state, but I haven't confirmed that functionality.

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