* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
@ 2017-02-18 1:34 Dalon Westergreen
2017-02-20 9:07 ` Marek Vasut
0 siblings, 1 reply; 13+ messages in thread
From: Dalon Westergreen @ 2017-02-18 1:34 UTC (permalink / raw)
To: u-boot
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 <dwesterg@gmail.com>
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]);
--
2.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-18 1:34 [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0 Dalon Westergreen
@ 2017-02-20 9:07 ` Marek Vasut
2017-02-20 14:10 ` Dalon Westergreen
0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2017-02-20 9:07 UTC (permalink / raw)
To: u-boot
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 <dwesterg@gmail.com>
What about my suggestion for V2 about just loading function pointer into
the reset jump address register ?
btw --- missing before the changelog, without it the changelog will land
in git history.
> 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]);
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-20 9:07 ` Marek Vasut
@ 2017-02-20 14:10 ` Dalon Westergreen
2017-02-20 14:14 ` Marek Vasut
0 siblings, 1 reply; 13+ messages in thread
From: Dalon Westergreen @ 2017-02-20 14:10 UTC (permalink / raw)
To: u-boot
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 <dwesterg@gmail.com>
>
> 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.
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. ?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).
> 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]);
> >
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-20 14:10 ` Dalon Westergreen
@ 2017-02-20 14:14 ` Marek Vasut
2017-02-20 14:21 ` Dalon Westergreen
0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2017-02-20 14:14 UTC (permalink / raw)
To: u-boot
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 <dwesterg@gmail.com>
>>
>> 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 :)
> 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
> 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]);
>>>
>>
>>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-20 14:14 ` Marek Vasut
@ 2017-02-20 14:21 ` Dalon Westergreen
2017-02-20 14:24 ` Marek Vasut
0 siblings, 1 reply; 13+ messages in thread
From: Dalon Westergreen @ 2017-02-20 14:21 UTC (permalink / raw)
To: u-boot
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 <dwesterg@gmail.com>
> > >
> > > 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]);
> > > >
> > >
> > >
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-20 14:21 ` Dalon Westergreen
@ 2017-02-20 14:24 ` Marek Vasut
2017-02-20 14:35 ` Dalon Westergreen
0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2017-02-20 14:24 UTC (permalink / raw)
To: u-boot
On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
> 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 <dwesterg@gmail.com>
>>>>
>>>> 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.
OK
>>>
>>> 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.
Hm, OK. But then your system is kinda broken, so you should expect this
I guess.
[...]
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-20 14:24 ` Marek Vasut
@ 2017-02-20 14:35 ` Dalon Westergreen
2017-02-20 14:38 ` Marek Vasut
2017-02-28 14:45 ` Dalon Westergreen
0 siblings, 2 replies; 13+ messages in thread
From: Dalon Westergreen @ 2017-02-20 14:35 UTC (permalink / raw)
To: u-boot
On Mon, 2017-02-20 at 15:24 +0100, Marek Vasut wrote:
> On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
> >
> > 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 <dwesterg@gmail.com>
> > > > >
> > > > > 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.
>
> OK
>
> >
> > >
> > > >
> > > >
> > > > 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.
>
> Hm, OK. But then your system is kinda broken, so you should expect this
> I guess.
My thought exactly... ?I would like to see if Chin Liang or Dinh have any
comments?
>
> [...]
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-20 14:35 ` Dalon Westergreen
@ 2017-02-20 14:38 ` Marek Vasut
2017-02-28 14:45 ` Dalon Westergreen
1 sibling, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2017-02-20 14:38 UTC (permalink / raw)
To: u-boot
On 02/20/2017 03:35 PM, Dalon Westergreen wrote:
> On Mon, 2017-02-20 at 15:24 +0100, Marek Vasut wrote:
>> On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
>>>
>>> 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 <dwesterg@gmail.com>
>>>>>>
>>>>>> 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.
>>
>> OK
>>
>>>
>>>>
>>>>>
>>>>>
>>>>> 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.
>>
>> Hm, OK. But then your system is kinda broken, so you should expect this
>> I guess.
>
> My thought exactly... I would like to see if Chin Liang or Dinh have any
> comments?
Agreed, this needs more input
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-20 14:35 ` Dalon Westergreen
2017-02-20 14:38 ` Marek Vasut
@ 2017-02-28 14:45 ` Dalon Westergreen
2017-03-05 17:38 ` Dalon Westergreen
1 sibling, 1 reply; 13+ messages in thread
From: Dalon Westergreen @ 2017-02-28 14:45 UTC (permalink / raw)
To: u-boot
On Mon, 2017-02-20 at 06:35 -0800, Dalon Westergreen wrote:
> On Mon, 2017-02-20 at 15:24 +0100, Marek Vasut wrote:
> >
> > On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
> > >
> > >
> > > 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 <dwesterg@gmail.com>
> > > > > >
> > > > > > 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.
> >
> > OK
> >
> > >
> > >
> > > >
> > > >
> > > > >
> > > > >
> > > > >
> > > > > 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.
> >
> > Hm, OK. But then your system is kinda broken, so you should expect this
> > I guess.
>
> My thought exactly... I would like to see if Chin Liang or Dinh have any
> comments?
Chin Liang, Dinh, any comments?
> >
> >
> > [...]
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-02-28 14:45 ` Dalon Westergreen
@ 2017-03-05 17:38 ` Dalon Westergreen
2017-03-05 17:49 ` Marek Vasut
0 siblings, 1 reply; 13+ messages in thread
From: Dalon Westergreen @ 2017-03-05 17:38 UTC (permalink / raw)
To: u-boot
On Tue, 2017-02-28 at 06:45 -0800, Dalon Westergreen wrote:
> On Mon, 2017-02-20 at 06:35 -0800, Dalon Westergreen wrote:
> >
> > On Mon, 2017-02-20 at 15:24 +0100, Marek Vasut wrote:
> > >
> > >
> > > On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
> > > >
> > > >
> > > >
> > > > 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 <dwesterg@gmail.com>
> > > > > > >
> > > > > > > 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.
> > >
> > > OK
> > >
> > > >
> > > >
> > > >
> > > > >
> > > > >
> > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > 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.
> > >
> > > Hm, OK. But then your system is kinda broken, so you should expect this
> > > I guess.
> >
> > My thought exactly... I would like to see if Chin Liang or Dinh have any
> > comments?
>
> Chin Liang, Dinh, any comments?
Marek, I would like to propose we move forward with this patch?
--dalon
>
> >
> > >
> > >
> > >
> > > [...]
> > >
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-03-05 17:38 ` Dalon Westergreen
@ 2017-03-05 17:49 ` Marek Vasut
2017-03-06 2:25 ` Dalon Westergreen
0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2017-03-05 17:49 UTC (permalink / raw)
To: u-boot
On 03/05/2017 06:38 PM, Dalon Westergreen wrote:
> On Tue, 2017-02-28 at 06:45 -0800, Dalon Westergreen wrote:
>> On Mon, 2017-02-20 at 06:35 -0800, Dalon Westergreen wrote:
>>>
>>> On Mon, 2017-02-20 at 15:24 +0100, Marek Vasut wrote:
>>>>
>>>>
>>>> On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
>>>>>
>>>>>
>>>>>
>>>>> 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 <dwesterg@gmail.com>
>>>>>>>>
>>>>>>>> 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.
>>>>
>>>> OK
>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 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.
>>>>
>>>> Hm, OK. But then your system is kinda broken, so you should expect this
>>>> I guess.
>>>
>>> My thought exactly... I would like to see if Chin Liang or Dinh have any
>>> comments?
>>
>> Chin Liang, Dinh, any comments?
>
> Marek, I would like to propose we move forward with this patch?
TBH I'm still thinking if you turned the V2 into C code and passed a
function pointer into the warm reset jump address register, that'd be
the best.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-03-05 17:49 ` Marek Vasut
@ 2017-03-06 2:25 ` Dalon Westergreen
2017-03-07 3:46 ` Marek Vasut
0 siblings, 1 reply; 13+ messages in thread
From: Dalon Westergreen @ 2017-03-06 2:25 UTC (permalink / raw)
To: u-boot
On Sun, 2017-03-05 at 18:49 +0100, Marek Vasut wrote:
> On 03/05/2017 06:38 PM, Dalon Westergreen wrote:
> >
> > On Tue, 2017-02-28 at 06:45 -0800, Dalon Westergreen wrote:
> > >
> > > On Mon, 2017-02-20 at 06:35 -0800, Dalon Westergreen wrote:
> > > >
> > > >
> > > > On Mon, 2017-02-20 at 15:24 +0100, Marek Vasut wrote:
> > > > >
> > > > >
> > > > >
> > > > > On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > 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 <dwesterg@gmail.com>
> > > > > > > > >
> > > > > > > > > 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.
> > > > >
> > > > > OK
> > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > 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.
> > > > >
> > > > > Hm, OK. But then your system is kinda broken, so you should expect
> > > > > this
> > > > > I guess.
> > > >
> > > > My thought exactly... I would like to see if Chin Liang or Dinh have
> > > > any
> > > > comments?
> > >
> > > Chin Liang, Dinh, any comments?
> >
> > Marek, I would like to propose we move forward with this patch?
>
> TBH I'm still thinking if you turned the V2 into C code and passed a
> function pointer into the warm reset jump address register, that'd be
> the best.
>
I will give it a shot, i just dont like relying on the spl image still
being present in the onchip ram. There is nothing preventing an errant
process or user from overwriting it and i have run into customers using
that memory for their own purposes.
-dalon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/listinfo/u-boot
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0
2017-03-06 2:25 ` Dalon Westergreen
@ 2017-03-07 3:46 ` Marek Vasut
0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2017-03-07 3:46 UTC (permalink / raw)
To: u-boot
On 03/06/2017 03:25 AM, Dalon Westergreen wrote:
> On Sun, 2017-03-05 at 18:49 +0100, Marek Vasut wrote:
>> On 03/05/2017 06:38 PM, Dalon Westergreen wrote:
>>>
>>> On Tue, 2017-02-28 at 06:45 -0800, Dalon Westergreen wrote:
>>>>
>>>> On Mon, 2017-02-20 at 06:35 -0800, Dalon Westergreen wrote:
>>>>>
>>>>>
>>>>> On Mon, 2017-02-20 at 15:24 +0100, Marek Vasut wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 02/20/2017 03:21 PM, Dalon Westergreen wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 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 <dwesterg@gmail.com>
>>>>>>>>>>
>>>>>>>>>> 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.
>>>>>>
>>>>>> OK
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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.
>>>>>>
>>>>>> Hm, OK. But then your system is kinda broken, so you should expect
>>>>>> this
>>>>>> I guess.
>>>>>
>>>>> My thought exactly... I would like to see if Chin Liang or Dinh have
>>>>> any
>>>>> comments?
>>>>
>>>> Chin Liang, Dinh, any comments?
>>>
>>> Marek, I would like to propose we move forward with this patch?
>>
>> TBH I'm still thinking if you turned the V2 into C code and passed a
>> function pointer into the warm reset jump address register, that'd be
>> the best.
>>
> I will give it a shot, i just dont like relying on the spl image still
> being present in the onchip ram. There is nothing preventing an errant
> process or user from overwriting it and i have run into customers using
> that memory for their own purposes.
>
That's very true .
Then again, we use warm reset all over the place, so we might as well
keep things consistent.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-03-07 3:46 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-18 1:34 [U-Boot] [PATCH v3] arm: socfpga: fix issue with warm reset when CSEL is 0 Dalon Westergreen
2017-02-20 9:07 ` Marek Vasut
2017-02-20 14:10 ` Dalon Westergreen
2017-02-20 14:14 ` Marek Vasut
2017-02-20 14:21 ` Dalon Westergreen
2017-02-20 14:24 ` Marek Vasut
2017-02-20 14:35 ` Dalon Westergreen
2017-02-20 14:38 ` Marek Vasut
2017-02-28 14:45 ` Dalon Westergreen
2017-03-05 17:38 ` Dalon Westergreen
2017-03-05 17:49 ` Marek Vasut
2017-03-06 2:25 ` Dalon Westergreen
2017-03-07 3:46 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.