On Mon, 2017-02-20 at 15:14 +0100, Marek Vasut wrote:
> On 02/20/2017 03:10 PM, Dalon Westergreen wrote:
> >
> > On Mon, 2017-02-20 at 10:07 +0100, Marek Vasut wrote:
> > >
> > > On 02/18/2017 02:34 AM, 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. This patch disables warm resets when CSEL=0.
> > > > This results in the clock and pll configurations being reset
> > > > on any reset issued when CSEL=0.
> > > >
> > > > Signed-off-by: Dalon Westergreen <[email protected]>
> > >
> > > What about my suggestion for V2 about just loading function pointer into
> > > the reset jump address register ?
> >
> > Frankly, i really dont like relying on the existence of a snippet of code in
> > the
> > onchip ram being untouched to ensure a reboot/reset will occur for this
> > csel=0
> > case. i am certain this case is rarely used, and confident that it isnt
> > being
> > used while trying to preserve sdram contents.
>
> Well, you already rely on such snippet, it's SPL. If you corrupt SPL and
> do warm reset, your system hangs, I had that multiple times :)
True. I would argue to just use cold resets but i think arria 10 has more use
for the warm reset case.
> >
> > the downside is that the scorecard is reset every boot. so the bootrom will
> > retry all the spl images again resulting in possibly longer boot times.
>
> Is that significant ?
The watchdog timeout is on the order of 1.5 seconds. That would be for each
failed spl.
> >
> > The
> > other is that things like sdram content preservation is not likely to work
> > or
> > the csel=0 case (but as i mentioned, i dont see this used often in cyclone5,
> > and never have i seen it when csel=0).
>
> OK, I didn't see this requirement yet.
>
> >
> > >
> > > btw --- missing before the changelog, without it the changelog will land
> > > in git history.
> >
> > Thanks
> >
> > >
> > > >
> > > >
> > > > Changes in v3:
> > > > - Change implementation to rely on cold reset for CSEL=0. Which
> > > > is a much simpler approach to dealing with this special case
> > > > during boot.
> > > > Changes in v2:
> > > > - Fix checkpatch issues predominently due to whitespace issues
> > > > ---
> > > > arch/arm/mach-socfpga/include/mach/system_manager.h | 3 +++
> > > > arch/arm/mach-socfpga/misc.c | 13 ++++++++++++-
> > > > 2 files changed, 15 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm/mach-socfpga/include/mach/system_manager.h
> > > > b/arch/arm/mach-socfpga/include/mach/system_manager.h
> > > > index c45edea..c9c0b33 100644
> > > > --- a/arch/arm/mach-socfpga/include/mach/system_manager.h
> > > > +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
> > > > @@ -137,6 +137,9 @@ struct socfpga_system_manager {
> > > >
> > > > #define SYSMGR_SDMMC_DRVSEL_SHIFT 0
> > > >
> > > > +#define SYSMGR_BOOTINFO_CSEL_MASK 0x18
> > > > +#define SYSMGR_BOOTINFO_CSEL_LSB 3
> > > > +
> > > > /* EMAC Group Bit definitions */
> > > > #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII 0x0
> > > > #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RGMII 0x1
> > > > diff --git a/arch/arm/mach-socfpga/misc.c b/arch/arm/mach-socfpga/misc.c
> > > > index dd6b53b..9792138 100644
> > > > --- a/arch/arm/mach-socfpga/misc.c
> > > > +++ b/arch/arm/mach-socfpga/misc.c
> > > > @@ -356,6 +356,7 @@ static uint32_t iswgrp_handoff[8];
> > > > int arch_early_init_r(void)
> > > > {
> > > > int i;
> > > > + unsigned int csel;
> > > >
> > > > /*
> > > > * Write magic value into magic register to unlock support for
> > > > @@ -363,8 +364,18 @@ int arch_early_init_r(void)
> > > > * value to be written into the register by the bootloader, so
> > > > * to support that old code, we write it here instead of in the
> > > > * reset_cpu() function just before resetting the CPU.
> > > > + *
> > > > + * For CSEL = 0 we do not want to enable warm resets to ensure
> > > > that
> > > > + * on reset the clocks and plls are reset to their default
> > > > states
> > > > as
> > > > + * the bootrom, for CSEL=0, leaves the clocks untouched. If
> > > > the
> > > > clocks
> > > > + * and plls are not reset, the bootrom will fail to load the
> > > > spl
> > > > image.
> > > > */
> > > > - writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable);
> > > > +
> > > > + csel = (readl(&sysmgr_regs->bootinfo) &
> > > > SYSMGR_BOOTINFO_CSEL_MASK)
> > > > >
> > > > > >
> > > > > >
> > > > + SYSMGR_BOOTINFO_CSEL_LSB;
> > > > +
> > > > + if (csel)
> > > > + writel(0xae9efebc, &sysmgr_regs-
> > > > >
> > > > > romcodegrp_warmramgrp_enable);
> > > >
> > > > for (i = 0; i < 8; i++) /* Cache initial SW setting regs
> > > > */
> > > > iswgrp_handoff[i] = readl(&sysmgr_regs-
> > > > >iswgrp_handoff[i]);
> > > >
> > >
> > >
>
>
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
