public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Dalon Westergreen <dwesterg@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] arm: socfpga: fix issue with warm reset when CSEL is 0
Date: Mon, 20 Feb 2017 06:07:47 -0800	[thread overview]
Message-ID: <1487599667.6111.16.camel@gmail.com> (raw)
In-Reply-To: <0c7683b6-00b3-0922-6a80-746f296ec832@denx.de>

On Sun, 2017-02-19 at 22:31 +0100, Marek Vasut wrote:
> On 02/18/2017 12:24 AM, Dalon Westergreen wrote:
> > 
> > On Fri, 2017-02-17 at 22:16 +0100, Marek Vasut wrote:
> > > 
> > > 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@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		
> > > > > > > > > 0x
> > > > > > > > > 0000
> > > > > > > > > 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		(SOCFP
> > > > > > > > > GA_S
> > > > > > > > > YSMG
> > > > > > > > > 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 ...
> > 
> > Okay, after much discussion and debate with a colleague..\
> > 
> > Warm reset is preferred as the bootrom keeps a score card to determine
> > whether an spl image in the boot media failed or not.??If it failed,
> > on a warm reset it will not retry the failed image.
> 
> So what will it do ? Try a valid image from another slot at offset
> +n*64kiB ?

Yes. ?Or when the scorecard indicates 4 failures it will just stop.

> > 
> > The other reason warm resets are preferred is for preservation of the
> > dram contents.??On a warm reset it is possible to skip io configuration
> > and dram calibration so that the contents can be saved.
> 
> That's a good point.
> 
> But here's a counterargument, what if you upgrade U-Boot ? Warm reset
> will use the old SPL and the system will likely hang upon reboot ;-)
> 
> > 
> > > 
> > > How do you point bootrom to run that snippet instead of whatever is in
> > > the OCRAM ?
> > > 
> > 
> > This code here
> > 
> > > 
> > > ?> > > > > > +		writel(ramboot_addr,
> > > ?> > > > > > +		??????&sysmgr_regs-
> > > ?> > > > > > >romcodegrp_warmramgrp_execution);
> 
> Can't you just feed a function pointer pointing into some function which
> is part of the SPL into that register then ? I think that'd do the same
> trick, no ?

Yes, you could, but the idea of putting the code at the end of memory is to
allow the onchip ram to be used for other things.

> > 
> > writes the address to jump to on warm reset.??The register value
> > is preserved through a warm reset.
> 
> That's neat, I didn't know that.
> 
> > 
> > > 
> > > 
> > 
> > All that said, i frankly do not believe for the CSEL=0 case
> > there is any merit to doing the above.??Although it "preserves"
> > the behaviour as compared to other CSEL values, i think a much
> > simpler method is to, for the CSEL=0 case, just issue a cold reset.
> > 
> > As in this case we are touching the clocks, i am not sure the
> > use cases for a warm reset even hold (sdram preservation, etc).
> > So i agree with you, and suggest only enabling the warm reset
> > for cases where CSEL != 0.
> > 
> > Unless there are objections, I will do just that and resubmit a
> > patch. (which should now be just a few lines of code)
> 
> See above, if this actually fixes issue, let's get it in. But in a
> civilized fashion, no random ad-hoc asm if possible please :)

In v3 i just used the simpler method of not allowing warm resets for
csel=0. This is far cleaner, and likely more reliable.

--dalon

  reply	other threads:[~2017-02-20 14:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-14 18:28 [U-Boot] [PATCH v2] arm: socfpga: fix issue with warm reset when CSEL is 0 Dalon Westergreen
2017-02-15  6:56 ` Chin Liang See
2017-02-15 21:16   ` Marek Vasut
2017-02-15 21:15 ` Marek Vasut
2017-02-15 21:48   ` Dalon Westergreen
2017-02-15 22:20     ` Marek Vasut
2017-02-16  2:53       ` Dalon Westergreen
2017-02-17 18:05         ` Dalon Westergreen
2017-02-17 21:16           ` Marek Vasut
2017-02-17 23:24             ` Dalon Westergreen
2017-02-19 21:31               ` Marek Vasut
2017-02-20 14:07                 ` Dalon Westergreen [this message]
2017-02-20 14:12                   ` Marek Vasut

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1487599667.6111.16.camel@gmail.com \
    --to=dwesterg@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox