From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Wed, 15 Feb 2017 23:20:42 +0100 Subject: [U-Boot] [PATCH v2] arm: socfpga: fix issue with warm reset when CSEL is 0 In-Reply-To: <1487195313.6617.291.camel@gmail.com> References: <1487096912-18457-1-git-send-email-dwesterg@gmail.com> <1487195313.6617.291.camel@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 02/15/2017 10:48 PM, Dalon Westergreen wrote: > On Wed, 2017-02-15 at 22:15 +0100, Marek Vasut wrote: >> On 02/14/2017 07:28 PM, Dalon Westergreen wrote: >>> >>> When CSEL=0x0 the socfpga bootrom does not touch the clock >>> configuration for the device. This can lead to a boot failure >>> on warm resets. To address this, the bootrom is configured to >>> run a bit of code in the last 4KB of onchip ram on a warm reset. >>> This code puts the PLLs in bypass, disables the bootrom configuration >>> to run the code snippet, and issues a warm reset to run the bootrom. >>> >>> Signed-off-by: Dalon Westergreen >>> >>> -- >>> Changes in V2: >>> - Fix checkpatch issues predominently due to whitespace issues >>> --- >>> arch/arm/mach-socfpga/Makefile | 2 +- >>> arch/arm/mach-socfpga/include/mach/clock_manager.h | 26 +++++++- >>> arch/arm/mach-socfpga/include/mach/reset_manager.h | 4 ++ >>> .../arm/mach-socfpga/include/mach/system_manager.h | 7 ++- >>> arch/arm/mach-socfpga/misc.c | 27 ++++++++ >>> arch/arm/mach-socfpga/reset_clock_manager.S | 71 >>> ++++++++++++++++++++++ >>> 6 files changed, 134 insertions(+), 3 deletions(-) >>> create mode 100644 arch/arm/mach-socfpga/reset_clock_manager.S >>> >>> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile >>> index 809cd47..6876ccf 100644 >>> --- a/arch/arm/mach-socfpga/Makefile >>> +++ b/arch/arm/mach-socfpga/Makefile >>> @@ -8,7 +8,7 @@ >>> # >>> >>> obj-y += misc.o timer.o reset_manager.o system_manager.o >>> clock_manager.o \ >>> - fpga_manager.o board.o >>> + fpga_manager.o board.o reset_clock_manager.o >>> >>> obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o >>> >>> diff --git a/arch/arm/mach-socfpga/include/mach/clock_manager.h >>> b/arch/arm/mach-socfpga/include/mach/clock_manager.h >>> index 803c926..78f63a4 100644 >>> --- a/arch/arm/mach-socfpga/include/mach/clock_manager.h >>> +++ b/arch/arm/mach-socfpga/include/mach/clock_manager.h >>> @@ -19,9 +19,12 @@ const unsigned int cm_get_osc_clk_hz(const int osc); >>> const unsigned int cm_get_f2s_per_ref_clk_hz(void); >>> const unsigned int cm_get_f2s_sdr_ref_clk_hz(void); >>> >>> +/* Onchip RAM functions for CSEL=0 */ >>> +void reset_clock_manager(void); >>> +extern unsigned reset_clock_manager_size; >>> + >>> /* Clock configuration accessors */ >>> const struct cm_config * const cm_get_default_config(void); >>> -#endif >>> >>> struct cm_config { >>> /* main group */ >>> @@ -127,6 +130,19 @@ struct socfpga_clock_manager { >>> struct socfpga_clock_manager_altera altera; >>> u32 _pad_0xe8_0x200[70]; >>> }; >>> +#endif >>> + >>> +#define CLKMGR_CTRL_ADDRESS 0x0 >> >> Is this the same as struct socfpga_clock_manager {} ? >> Why ? > It is, just defining offsets to use in the assembly calls The asm is IMO not needed >> >>> >>> +#define CLKMGR_BYPASS_ADDRESS 0x4 >>> +#define CLKMGR_INTER_ADDRESS 0x8 >>> +#define CLKMGR_INTREN_ADDRESS 0xc >>> +#define CLKMGR_DBCTRL_ADDRESS 0x10 >>> +#define CLKMGR_STAT_ADDRESS 0x14 >>> +#define CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS 0x54 >>> +#define CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS 0x58 >>> +#define CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS 0x90 >>> +#define CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS 0x94 >>> + >>> >>> #define CLKMGR_CTRL_SAFEMODE (1 << 0) >>> #define CLKMGR_CTRL_SAFEMODE_OFFSET 0 >>> @@ -314,4 +330,12 @@ struct socfpga_clock_manager { >>> #define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_OFFSET 9 >>> #define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK 0x00000e00 >>> >>> +/* Bypass Main and Per PLL, bypass source per input mux */ >>> +#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK 0x19 >>> + >>> >>> +#define CLKMGR_MAINQSPICLK_RESET_VALUE 0x3 >>> +#define CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE 0x3 >>> +#define CLKMGR_PERQSPICLK_RESET_VALUE 0x1 >>> +#define CLKMGR_PERNANDSDMMCCLK_RESET_VALUE 0x1 >>> + >>> #endif /* _CLOCK_MANAGER_H_ */ >>> diff --git a/arch/arm/mach-socfpga/include/mach/reset_manager.h >>> b/arch/arm/mach-socfpga/include/mach/reset_manager.h >>> index 2f070f2..58d77fb 100644 >>> --- a/arch/arm/mach-socfpga/include/mach/reset_manager.h >>> +++ b/arch/arm/mach-socfpga/include/mach/reset_manager.h >>> @@ -7,6 +7,7 @@ >>> #ifndef _RESET_MANAGER_H_ >>> #define _RESET_MANAGER_H_ >>> >>> +#ifndef __ASSEMBLY__ >>> void reset_cpu(ulong addr); >>> void reset_deassert_peripherals_handoff(void); >>> >>> @@ -28,6 +29,8 @@ struct socfpga_reset_manager { >>> u32 padding2[12]; >>> u32 tstscratch; >>> }; >>> +#endif >>> + >>> >>> #if defined(CONFIG_SOCFPGA_VIRTUAL_TARGET) >>> #define RSTMGR_CTRL_SWWARMRSTREQ_LSB 2 >>> @@ -40,6 +43,7 @@ struct socfpga_reset_manager { >>> * and reset ID can be extracted using the subsequent macros >>> * RSTMGR_RESET() and RSTMGR_BANK(). >>> */ >>> +#define RSTMGR_CTRL_OFFSET 4 >>> #define RSTMGR_BANK_OFFSET 8 >>> #define RSTMGR_BANK_MASK 0x7 >>> #define RSTMGR_RESET_OFFSET 0 >>> diff --git a/arch/arm/mach-socfpga/include/mach/system_manager.h >>> b/arch/arm/mach-socfpga/include/mach/system_manager.h >>> index c45edea..b89f269 100644 >>> --- a/arch/arm/mach-socfpga/include/mach/system_manager.h >>> +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h >>> @@ -13,7 +13,6 @@ void sysmgr_pinmux_init(void); >>> void sysmgr_config_warmrstcfgio(int enable); >>> >>> void sysmgr_get_pinmux_table(const u8 **table, unsigned int *table_len); >>> -#endif >>> >>> struct socfpga_system_manager { >>> /* System Manager Module */ >>> @@ -115,6 +114,12 @@ struct socfpga_system_manager { >>> u32 _pad_0x734; >>> u32 spim0usefpga; /* 0x738 */ >>> }; >>> +#endif >>> + >>> +#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE (SOCFPGA_SYSMGR_ADDR >>> ESS + 0xe0) >>> + >>> +#define SYSMGR_BOOTINFO_CSEL_MASK 0x18 >>> +#define SYSMGR_BOOTINFO_CSEL_LSB 3 >>> >>> #define SYSMGR_ROMCODEGRP_CTRL_WARMRSTCFGPINMUX (1 << 0) >>> #define SYSMGR_ROMCODEGRP_CTRL_WARMRSTCFGIO (1 << 1) >>> diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c >>> index dd6b53b..57e3334 100644 >>> --- a/arch/arm/mach-socfpga/misc.c >>> +++ b/arch/arm/mach-socfpga/misc.c >>> @@ -16,6 +16,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include >>> #include >>> #include >>> @@ -356,6 +357,32 @@ static uint32_t iswgrp_handoff[8]; >>> int arch_early_init_r(void) >>> { >>> int i; >>> + unsigned csel, ramboot_addr; >>> + >>> + /* Check the CSEL value */ >>> + csel = (readl(&sysmgr_regs->bootinfo) & SYSMGR_BOOTINFO_CSEL_MASK) >>>>> >>> + SYSMGR_BOOTINFO_CSEL_LSB; >>> + >>> + /* >>> + * For CSEL = 0 the bootrom does not configure the clocks which can >>> + * result in a boot failure on warm resets. To remedy this a small >>> + * bit of code is placed at the end of the onchip ram and run on >>> + * a warm reset. It puts the PLLs in bypass and issues another >>> warm >>> + * reset to get back to the bootrom. >>> + */ >>> + if (!csel) { >>> + /* Put the code snippet in the last 4KB of the onchip ram >>> */ >>> + ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR + >>> + CONFIG_SYS_INIT_RAM_SIZE - 0x1000; >>> + >>> + /* Copy the code to the onchip ramlocation */ >>> + memcpy((void *)ramboot_addr, reset_clock_manager, >>> + reset_clock_manager_size); >> >> So uh, why don't you put this code into SPL and execute it from there ? >> This is b/s ... > > you are correct, it should be setup in the SPL. that said though, > should the entire setup of the warm reset in this function be moved > to spl? the warm reset is enabled by writing that "magic" number > into a "magic" register. currently this is not done in SPL which > is why i put this where i did. Well yes, the SPL does the magic init of the platform. >>> >>> + /* Set the bootrom to run the code snippet on reset */ >>> + writel(ramboot_addr, >>> + &sysmgr_regs->romcodegrp_warmramgrp_execution); >>> + } >>> >>> /* >>> * Write magic value into magic register to unlock support for >>> diff --git a/arch/arm/mach-socfpga/reset_clock_manager.S b/arch/arm/mach- >>> socfpga/reset_clock_manager.S >>> new file mode 100644 >>> index 0000000..1818b2d >>> --- /dev/null >>> +++ b/arch/arm/mach-socfpga/reset_clock_manager.S >>> @@ -0,0 +1,71 @@ >>> +/* >>> + * Copyright (C) 2017, Intel Corporation >>> + * >>> + * SPDX-License-Identifier: GPL-2.0+ >>> + */ >>> + >>> +#include >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +/* >>> + */ >>> +ENTRY(reset_clock_manager) >> >> This is just a few writel() calls in SPL , right ? Put it there... > > Although this is just a few write calls, i dont believe just doing > this in SPL will work. The intent is to copy the code snippet > to onchip ram so on a warm reset the code below is executed. SPL is running from SRAM already , so what's the problem here ? > If it > is not executed, it is possible that the bootrom (when CSEL=0) will > be unable to fetch SPL at all. Why ? And how will you be able to enter this code (which is running from actual u-boot, which is itself loaded by SPL probably ...) if the bootrom cannot fetch SPL ? > as always, your comments / guidance is much appreciated. > > --dalon >> >>> >>> + /* Put Main PLL and Peripheral PLL in bypass */ >>> + ldr r0, SOCFPGA_CLKMGR >>> + mov r1, #CLKMGR_BYPASS_ADDRESS >>> + mov r2, #CLKMGR_BYPASS_MAIN_PER_PLL_MASK >>> + add r3, r0, r1 >>> + ldr r4, [r3] >>> + orr r5, r4, r2 >>> + str r5, [r3] >>> + dsb >>> + isb >>> + mov r1, #CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS >>> + mov r2, #CLKMGR_MAINQSPICLK_RESET_VALUE >>> + add r3, r0, r1 >>> + str r2, [r3] >>> + mov r1, #CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS >>> + mov r2, #CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE >>> + add r3, r0, r1 >>> + str r2, [r3] >>> + mov r1, #CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS >>> + mov r2, #CLKMGR_PERQSPICLK_RESET_VALUE >>> + add r3, r0, r1 >>> + str r2, [r3] >>> + mov r1, #CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS >>> + mov r2, #CLKMGR_PERNANDSDMMCCLK_RESET_VALUE >>> + add r3, r0, r1 >>> + str r2, [r3] >>> + >>> + /* Disable the RAM boot */ >>> + ldr r0, SOCFPGA_RSTMGR >>> + ldr r1, SYSMGR_WARMRAMGRP_ENABLE >>> + mov r2, #0 >>> + str r2, [r1] >>> + >>> + /* Trigger warm reset to continue boot normally */ >>> + mov r1, #RSTMGR_CTRL_OFFSET >>> + add r2, r0, r1 >>> + mov r3, #1 >>> + mov r3, r3, LSL #RSTMGR_CTRL_SWWARMRSTREQ_LSB >>> + ldr r4, [r2] >>> + orr r4, r3, r4 >>> + str r4, [r2] >>> + >>> +reset_clock_manager_loop: >>> + dsb >>> + isb >>> + b reset_clock_manager_loop >>> +ENDPROC(reset_clock_manager) >>> + >>> +SOCFPGA_CLKMGR: .word SOCFPGA_CLKMGR_ADDRESS >>> +SOCFPGA_RSTMGR: .word SOCFPGA_RSTMGR_ADDRESS >>> +SYSMGR_WARMRAMGRP_ENABLE: .word CONFIG_SYSMGR_WARMRAMGRP_ENAB >>> LE >>> + >>> +.globl reset_clock_manager_size >>> +reset_clock_manager_size: >>> + .word . - reset_clock_manager >>> + >>> >> >> -- Best regards, Marek Vasut