linux-kernel - Re: [PATCH 2/6] pinctrl: Update clock handling for the pinctrl-nomadik GPIO driver

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, 25 Oct 2012 14:33:01 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Ulf Hansson <ulf.hansson@...aro.org>
Cc:	Lee Jones <lee.jones@...aro.org>,
	Linus Walleij <linus.walleij@...ricsson.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"arnd@...db.de" <arnd@...db.de>
Subject: Re: [PATCH 2/6] pinctrl: Update clock handling for the
 pinctrl-nomadik GPIO driver

On Thu, Oct 25, 2012 at 11:29 AM, Ulf Hansson <ulf.hansson@...aro.org> wrote:

> Depending on clock type, a clk_disable is actually not going to "gate"
> the clock, that might happen only in unprepare. This depends on if the
> clock is a fast or slow clock.

Hm thats interesting. Now I need to drill down into this.
So looking at it in this case:

clk_disable() from the GPIO block of the pin controller will
hit "gpio.0", "gpio.1" etc in drivers/clk/ux500/u8500_clk.c.

These are PRCC (Programmable Clock Controller) clocks
registered using clk_reg_prcc_pclk() from clk-prcc.c.

pclk:s are using the clk_prcc_pclk_ops and these point to
clk_prcc_pclk_enable()/clk_prcc_pclk_disable() for
enable/disable respectively.

These will just write a PRCC register.

And prepare() and unprepare() are not implemented for
this clock.

So far we can conclude that clk_enable()/disable()
will indeed achieve the desired effect of gating the
clock to the GPIO block per se, so we are saving
some power for sure.

However the prepare()/unprepare() calls will of course
also accumulate upwards and in e.g. the example of
"gpio.0" and "gpio.1" the parent is "per1clk" which is the
clock for the entire peripheral group.

(At this point I can stick in a reminder of the idea to
restructure the device tree in peripheral groups, because
that exercise will certainly pay off the day we try to encode
clocks in the device tree, I think the point can be clearly
seen as we proceed...)

This "per1clk" is registered using clk_req_prcmu_gate()
from clk-prcmu.c.

Looking at that clock type we find it's a plain software
dummy for the clk_enable()/clk_disable() path.
The real action is happening in the prepare()/unprepare()
path.

So to be able to shut down the entire peripheral cluster,
indeed both functions need to be called.

So in accordance with this we can see that the patch
should be applied, in some form.

However it is not removing the initial clk_prepare()
so the entire patch will be pointless, the prepare count
will currently always be > 0.

I'll mangle Lee's patch a bit, hold on..

Yours,
Linus Walleij
--
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/