[U-Boot] [PATCH v2] arm: socfpga: fix issue with warm reset when CSEL is 0

Marek Vasut marex at denx.de
Fri Feb 17 21:16:11 UTC 2017 

On 02/17/2017 07:05 PM, Dalon Westergreen wrote:
> 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.

I just got back from the airport, catching up on email.
What about doing cold reset, SoCFPGA is capable of that. I was actually
pondering why the heck do we do warm reset at all ...

How do you point bootrom to run that snippet instead of whatever is in
the OCRAM ?


-- 
Best regards,
Marek Vasut

More information about the U-Boot mailing list