[U-Boot] [PATCH v2] arm: socfpga: fix issue with warm reset when CSEL is 0
Dalon Westergreen
dwesterg at gmail.com
Fri Feb 17 18:05:22 UTC 2017
On Wed, 2017-02-15 at 18:53 -0800, Dalon Westergreen wrote:
> 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 0x0000
> > > > > 0e
> > > > > 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_SYSMG
> > > > > R_
> > > > > 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
Any comments on this Marek? I am not sure there is a reasonable way
do this without the assembly snippet. The snippet is not run during
uboot or spl, it is un on a warm reset. I do agree this setup during
spl, before i do that though i would like to understand if you have
any better ways to do this? When CSEL=0 code needs to be run after
a warm reset and before the bootrom code is run again to place
the clocks in a known configuration.
Thanks,
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_AD
> > > > > DR
> > > > > ESS
> > > > > +SOCFPGA_RSTMGR: .word SOCFPGA_RSTMGR_AD
> > > > > DR
> > > > > ESS
> > > > > +SYSMGR_WARMRAMGRP_ENABLE: .word CONFIG_SYSMGR_WARMRAMGR
> > > > > P_
> > > > > ENAB
> > > > > LE
> > > > > +
> > > > > +.globl reset_clock_manager_size
> > > > > +reset_clock_manager_size:
> > > > > + .word . - reset_clock_manager
> > > > > +
> > > > >
> > > >
> > > >
> >
> >
More information about the U-Boot
mailing list