From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Date: Mon, 5 Sep 2016 09:32:15 +0100 Subject: [U-Boot] [PATCH 2/4] Revert "sunxi: Downclock AHB1 to 100MHz on Allwinner A64" In-Reply-To: <20160905074809.4c7efd3b@i7> References: <1473035561-23420-1-git-send-email-andre.przywara@arm.com> <1473035561-23420-3-git-send-email-andre.przywara@arm.com> <20160905074809.4c7efd3b@i7> Message-ID: <51d4876f-a179-cd0e-aad2-2461d46383c8@arm.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 05/09/16 05:48, Siarhei Siamashka wrote: > On Mon, 5 Sep 2016 01:32:39 +0100 > Andre Przywara wrote: > >> Now that we don't use SRAM C for the SPL stack anymore, there is no >> need to clock down AHB1 to 100 MHz. > > It's just another way to say it, but we are not clocking the AHB1 > down, but rather keeping it at a failsafe default. If I understand it > correctly, reclocking the AHB1 to 200 MHz early in the SPL code is > not really necessary for the v2016.09 release and does not fix anything. Yes, this patch is indeed somewhat optional. It was just a logical consequence of the other patch being reverted. > Moreover this revert affects USB FEL boot, which currently uses 8K > of SRAM C as a backup storage. Ah, that may explain the FEL issues I was trying to track down on the weekend. > Yes, we can also move the FEL backup > storage to SRAM A2, but the real question is whether we really want > to have it this way in the long run. > > Is it really needed to reclock AHB1 early in the SPL? Can't ATF or > U-Boot proper do this a bit later? Good point. My current ATF branch at home sets up all the basic clocks to their desired values, so indeed U-Boot wouldn't need to touch the clocks anymore (also I think it shouldn't care). In my current setup ATF is expected to run after the SPL, but before the U-Boot proper. > Also it's best to unmap the SRAM C > from the CPU address space at the same time as the AHB1 is reclocked > to 200 MHz. So that nobody can accidentally access it. Yes, that sounds reasonable. > There is a > special "bootmode" bit, which configures the SRAM C mapping: > > https://irclog.whitequark.org/linux-sunxi/2016-06-29#16863309; > > Writing 0x00000000 to 0x01c00004 hides the SRAM C from the CPU. And > writing 0x01000000 there enables it again. Some SRAM C experiments > can be done with the sunxi-fel tool: > > $ sunxi-fel hex 0x18000 64 > 00018000: 32 a6 21 9a da f7 16 d7 7c 59 e9 b3 db a2 5f 9e 2.!.....|Y...._. > 00018010: d2 0c 54 b8 79 7e 7a ff 7f 0d b5 e7 96 27 34 c8 ..T.y~z......'4. > 00018020: c7 d1 66 f8 4b dc a6 cb d5 ba e3 ce 26 18 49 c4 ..f.K.......&.I. > 00018030: 4f f6 3f a9 56 f7 92 d4 50 b6 7f e6 73 e8 e5 69 O.?.V...P...s..i > > $ sunxi-fel fill 0x18000 64 0xCC && sunxi-fel hex 0x18000 64 > 00018000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................ > 00018010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................ > 00018020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................ > 00018030: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................ > > $ sunxi-fel writel 0x01c00004 0x00000000 && sunxi-fel hex 0x18000 64 > 00018000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00018010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00018020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > 00018030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > > $ sunxi-fel writel 0x01c00004 0x01000000 && sunxi-fel hex 0x18000 64 > 00018000: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................ > 00018010: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................ > 00018020: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................ > 00018030: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc ................ > > The commands above first show the uninitialized garbage in the SRAM C, > then initialize it to 0xCC, then disable the SRAM C access from the > CPU (it is then read as all zeros), then enable it back (it is read > as 0xCC again). Nice one! Didn't know about that. I will do some experiments tonight. >> Keeping it at the recommended 200 MHz allows faster peripherals. > > I usually prefer to see some performance numbers in such commit > messages ;) Not that I doubt that something becomes faster, but it > is always interesting to know how big is the real practical effect. > >> This reverts commit 5bc88cc2be3a962005b6e5768e06ca8f6ffcb88d. >> >> Signed-off-by: Andre Przywara > > We are not fixing any bugs with this revert. Or do we? Probably not. Let me give it actually a try without this patch and then we can drop this one. Thanks, Andre. >> --- >> arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h >> index be9fcfd..f7e93b0 100644 >> --- a/arch/arm/include/asm/arch-sunxi/clock_sun6i.h >> +++ b/arch/arm/include/asm/arch-sunxi/clock_sun6i.h >> @@ -230,12 +230,7 @@ struct sunxi_ccm_reg { >> #define CCM_PLL5_TUN_INIT_FREQ(x) (((x) & 0x7f) << 16) >> #define CCM_PLL5_TUN_INIT_FREQ_MASK CCM_PLL5_TUN_INIT_FREQ(0x7f) >> >> -#if defined(CONFIG_MACH_SUN50I) >> -/* AHB1=100MHz failsafe setup from the FEL mode, usable with PMIC defaults */ >> -#define AHB1_ABP1_DIV_DEFAULT 0x00003190 /* AHB1=PLL6/6,APB1=AHB1/2 */ >> -#else >> #define AHB1_ABP1_DIV_DEFAULT 0x00003180 /* AHB1=PLL6/3,APB1=AHB1/2 */ >> -#endif >> >> #define AXI_GATE_OFFSET_DRAM 0 >> >