linux-kernel - Re: [PATCH v2] clk: exynos5420: Keep aclk66

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:	Thu, 5 Jun 2014 13:10:46 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Tomasz Figa <tomasz.figa@...il.com>
Cc:	Tomasz Figa <t.figa@...sung.com>,
	Mike Turquette <mturquette@...aro.org>,
	Kukjin Kim <kgene.kim@...sung.com>,
	linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Olof Johansson <olof@...om.net>,
	Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v2] clk: exynos5420: Keep aclk66_peric enabled during boot

Tomasz,

On Thu, Jun 5, 2014 at 12:31 PM, Tomasz Figa <tomasz.figa@...il.com> wrote:
> One more question that just came to my mind: Why it is just the
> aclk66_peri and not its children related to UARTs?

Can you re-read out the original patch description and see if it
answers that?  Basically: the UART was left on by the bootloader and
that's a requirement of earlyprintk but the common clock framework is
messing with aclk66_peric which is what's killing us.

> Exynos5420 clock driver still needs a little clean-up, so I'm okay with
> any fix for now and changing this later if needed, but is this gate
> clock for ACLK66_PERIC even needed? It seems to be marked with
> CLK_IGNORE_UNUSED which suggests that it is not needed in this driver at
> all, along with other similar clocks. AFAIK for power management
> purposes our fine-grained approach is fine and we shouldn't need to use
> the big gates.

Right, another option is to remove this gate from the clock tree (and
reparent all children on the gate's parent) and that would also fix
us.  The main advantages of the "big gate" as I see it:

1. There's a reasonable chance that our clock tree description is not
complete.  Patches to add previously undocumented clocks seems to be a
relatively common occurrence.  If one of those undocumented clocks
happens to be a child of aclk66_peric then we'll no longer be gating
it.

2. There might be a small amount of power saved by gating the clock
higher in the tree.

3. Having a more complete description of the clock tree is nice in
case someone outside the kernel does something.  If a stray pointer
(or a nonstandard bootloader) gates the clock then our tree won't show
problems, but the clock will still be gated.

Of course as soon as the first child of aclk66_peric is activated then
points #1 and #2 are invalid.  ...and #3 is a bit bogus because there
are A LOT of gates that we already don't model.

Also: the CLK_IGNORE_UNUSED is a bit bogus here.  That's not really a
useful flag in the case that you've got children that are enabled and
disabled.  Once your first child is enabled and disabled you'll be
disabled!

---

Hmmm, I think I've convinced myself that your suggestion of just
removing this gate from the table is the right thing to do.  Patch
will be coming up soon.

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