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

Wed Feb 15 22:20:42 UTC 2017

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		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 <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  ?

> 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