From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Dooks Date: Fri, 07 Mar 2014 15:44:20 +0000 Subject: Re: [PATCH RFC] ARM: shmobile: r8a7791 legacy: Disable RMSTP clocks Message-Id: <5319E954.7070407@codethink.co.uk> List-Id: References: <1394135648-10193-1-git-send-email-geert@linux-m68k.org> In-Reply-To: <1394135648-10193-1-git-send-email-geert@linux-m68k.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-sh@vger.kernel.org On 07/03/14 15:23, Laurent Pinchart wrote: > Hi Geert, > > On Thursday 06 March 2014 20:54:08 Geert Uytterhoeven wrote: >> From: Geert Uytterhoeven >> >> Each module clock has actually two disable bits: one for the System Core >> (ARM) in an SMSTPCRx register, and one for the Realtime Core (SH) in an >> RMSTPRx register. Currently we don't touch the bits meant for the >> Realtime Core, so some clocks may inadvertently be enabled and still run, >> wasting power, while they're disabled in the SMSTPCRx register. >> The actual state of the clock is indicated in the corresponding status >> register. >> >> Set the disable bits for the Realtime Core for all module clocks we >> currently care about to fix this. After this, e.g. QSPI fails to work if >> we deliberately keep its clock gated. >> >> Thanks to Laurent for the Realtime Core hint. >> >> Signed-off-by: Geert Uytterhoeven >> --- >> This is a quick hack, NOT to be applied! >> >> Notes/Questions/...: >> - The good news is that the system booted fine, hence so far I didn't >> notice any clocks that were not enabled correctly in our driver code, >> for both legacy and DT case (I used a slightly different version of >> this hack to test the DT case). >> - After bootup, the following clocks seem to be enabled for the Realtime >> Core: >> SCIFA0-2, SCIFB0-2, QSPI, MSIOF0-2, I2C0-5 >> - The following clocks were disabled for the Realtime Core: >> LVDS0, DU0-1, SCIF0-5, SCIFA3-5, SDHI0-2, CMT0, Thermal, Ether, >> VIN0-1, SATA0-1 >> - These are only the clocks we're currently using. There exist other >> clocks, I did not check their states. >> - Just setting all RMSTPCRx registers to 0xffffffff is not an option, as >> reserved bits should be written back using their read values. >> - Should we add a new enable_reg to struct clk for the legacy case? Or >> just disable them at initialization time, without enlarging struct clk? >> - Add more registers to the bindings for the DT case? >> - How to tell the system that it should (not) disable the RMSTP clocks? >> Someone may want to run something on the SH core, and we don't want to >> interfere with that. > > Those are tricky questions. RMSTPCRs are supposed to be managed by the SH-4A > core. If the SH-4A core isn't booted the only option we have is to manage them > elsewhere. In that case the registers should be configured at boot time to > disable all modules from the SH-4A side. This could be done either when > booting the kernel, or in the boot loader. > > If the SH-4A core is booted by Linux we could require the SH-4A code to > disable all unused clocks, but we could also do so before booting it without > any drawback (I expect the SH-4A firmware to enable the clocks it needs). > > The SH-4A core could be booted first, and then only boot the ARM cores running > Linux. I don't know if that configuration is commonly used, or even supported, > but in theory that should be possible. In that case the Linux kernel must not > touch the RMSTPCRs. > > We could disable all realtime clocks from the Linux kernel at boot time (with > a way to bypass that depending on whether booting the SH-4A first is a use > case we need to support), or decide to leave this to the boot loader. I agree with Laurent, we don't really have a good way yet to know if the SH-4A is going to be used. My preference would be to either fix it in the bootloader (possibly add a script in before loading the kernel). -- Ben Dooks http://www.codethink.co.uk/ Senior Engineer Codethink - Providing Genius