[U-Boot] [PATCH v2] arm: socfpga: fix issue with warm reset when CSEL is 0
Dalon Westergreen
dwesterg at gmail.com
Thu Feb 16 02:53:18 UTC 2017
On Wed, 2017-02-15 at 23:20 +0100, Marek Vasut wrote:
> 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 <dwesterg at gmail.com>
> > > >
> > > > --
> > > > 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 0x00000e
> > > > 00
> > > >
> > > > +/* 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 <asm/arch/reset_manager.h>
> > > > #include <asm/arch/scan_manager.h>
> > > > #include <asm/arch/system_manager.h>
> > > > +#include <asm/arch/clock_manager.h>
> > > > #include <asm/arch/nic301.h>
> > > > #include <asm/arch/scu.h>
> > > > #include <asm/pl310.h>
> > > > @@ -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 <config.h>
> > > > +#include <linux/linkage.h>
> > > > +#include <asm/arch/system_manager.h>
> > > > +#include <asm/arch/reset_manager.h>
> > > > +#include <asm/arch/clock_manager.h>
> > > > +
> > > > +/*
> > > > + */
> > > > +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 ?
I think i am not being clear. This issue is not power on reset, it
is after a warm reset. When CSEL=0 the bootrom does no configuration
or changes of the pll/clock settings. On power up, this isnt an
issue as the plls are bypased. On a warm reset, the clocks and
plls are left alone with csel=0, and as a result they are left running
at whatever speed they were set at during the initial boot. The
bootrom makes assumptions about the clock state and does not setup
the sdcard/qspi/nand appropriately for the clock configuration. as
a result, the bootrom will be unable to load the spl image on this
second warm reset.
The work around is to place a code snippet ( the asm stuff below )
in the onchip ram in a "reserved" area, allowing use of the reset
of the onchip ram for any purpose. The bootrom is then configured
to run this code snippet on a warm reset which could occur post
boot. The code snippet places the clocks in a known state (bypass)
and resets again to initiate the bootrom.
I am not sure how plausible it is just to, on warm reset, have the
bootrom run the spl image in onchip ram and just reserve the entire
ram for that purpose when csel=0.
i hope this clears up the problem, again, any thoughts are welcome.
--dalon
>
> >
> > 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_ADDR
> > > > ESS
> > > > +SOCFPGA_RSTMGR: .word SOCFPGA_RSTMGR_ADDR
> > > > ESS
> > > > +SYSMGR_WARMRAMGRP_ENABLE: .word CONFIG_SYSMGR_WARMRAMGRP_
> > > > ENAB
> > > > LE
> > > > +
> > > > +.globl reset_clock_manager_size
> > > > +reset_clock_manager_size:
> > > > + .word . - reset_clock_manager
> > > > +
> > > >
> > >
> > >
>
>
More information about the U-Boot
mailing list