arm: socfpga: fix issue with warm reset when CSEL is 0

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

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(-)

arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0

index c45edea..c9c0b33 100644
@@ -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

arch/arm/mach-socfpga/misc.c +12 -1

index dd6b53b..9792138 100644
@@ -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]);

Quoted 7 lines · Dalon Westergreen

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.

Quoted 1 line

Signed-off-by Dalon Westergreen <[email protected]>

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.

Quoted 65 lines

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(-)

arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0

index c45edea..c9c0b33 100644
@@ -137,6 +137,9 @@ struct socfpga_system_manager {

?
?#define SYSMGR_SDMMC_DRVSEL_SHIFT 0
?

arch/arm/mach-socfpga/include/mach/system_manager.h +3 -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

arch/arm/mach-socfpga/misc.c +1 -0

index dd6b53b..9792138 100644
@@ -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

arch/arm/mach-socfpga/misc.c +0 -0

@@ -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.

arch/arm/mach-socfpga/misc.c +3 -0

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

arch/arm/mach-socfpga/misc.c +1 -0

+	?* the bootrom, for CSEL=0, leaves the clocks untouched.??If the

clocks

arch/arm/mach-socfpga/misc.c +1 -0

+	?* and plls are not reset, the bootrom will fail to load the spl

image.
? ?*/

arch/arm/mach-socfpga/misc.c +2 -1

-	writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable);
+
+	csel = (readl(&sysmgr_regs->bootinfo) & SYSMGR_BOOTINFO_CSEL_MASK)

Quoted 1 line

Quoted 1 line

arch/arm/mach-socfpga/misc.c +4 -0

+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	if (csel)
+		writel(0xae9efebc, &sysmgr_regs-

Quoted 1 line

romcodegrp_warmramgrp_enable);

?
? for (i = 0; i < 8; i++) /* Cache initial SW setting regs */
? iswgrp_handoff[i] = readl(&sysmgr_regs->iswgrp_handoff[i]);

Quoted 10 lines · Marek Vasut

Quoted 8 lines · Dalon Westergreen

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

Quoted 2 lines

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).

Quoted 2 lines

btw --- missing before the changelog, without it the changelog will land in git history.

Thanks

Quoted 67 lines

Quoted 65 lines

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(-)

arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0

index c45edea..c9c0b33 100644
@@ -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

arch/arm/mach-socfpga/misc.c +4 -0

index dd6b53b..9792138 100644
@@ -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

arch/arm/mach-socfpga/misc.c +1 -0

+	 * the bootrom, for CSEL=0, leaves the clocks untouched.  If the

clocks

arch/arm/mach-socfpga/misc.c +1 -0

+	 * and plls are not reset, the bootrom will fail to load the spl

image.

arch/arm/mach-socfpga/misc.c +2 -1

 	 */
-	writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable);
+
+	csel = (readl(&sysmgr_regs->bootinfo) & SYSMGR_BOOTINFO_CSEL_MASK)

Quoted 1 line

Quoted 1 line

arch/arm/mach-socfpga/misc.c +4 -0

+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	if (csel)
+		writel(0xae9efebc, &sysmgr_regs-

Quoted 1 line

romcodegrp_warmramgrp_enable);

arch/arm/mach-socfpga/misc.c +0 -0

 	for (i = 0; i < 8; i++)	/* Cache initial SW setting regs */
 		iswgrp_handoff[i] = readl(&sysmgr_regs->iswgrp_handoff[i]);

Quoted 17 lines · Dalon Westergreen

Quoted 10 lines · Marek Vasut

Quoted 8 lines · Dalon Westergreen

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.

Quoted 4 lines

Quoted 1 line

Signed-off-by Dalon Westergreen <[email protected]>

What about my suggestion for V2 about just loading function pointer into the reset jump address register ?

Quoted 7 lines

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 :)

Quoted 3 lines

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 ?

Quoted 6 lines

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.

Quoted 86 lines

Quoted 4 lines

btw --- missing before the changelog, without it the changelog will land in git history.

Thanks

Quoted 79 lines

Quoted 77 lines

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(-)

arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0

index c45edea..c9c0b33 100644
@@ -137,6 +137,9 @@ struct socfpga_system_manager {

?
?#define SYSMGR_SDMMC_DRVSEL_SHIFT 0
?

arch/arm/mach-socfpga/include/mach/system_manager.h +3 -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

arch/arm/mach-socfpga/misc.c +1 -0

index dd6b53b..9792138 100644
@@ -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

arch/arm/mach-socfpga/misc.c +0 -0

@@ -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.

arch/arm/mach-socfpga/misc.c +2 -0

+	?*
+	?* For CSEL = 0 we do not want to enable warm resets to ensure

that

arch/arm/mach-socfpga/misc.c +1 -0

+	?* on reset the clocks and plls are reset to their default

states
as

arch/arm/mach-socfpga/misc.c +1 -0

+	?* the bootrom, for CSEL=0, leaves the clocks untouched.??If

the
clocks

arch/arm/mach-socfpga/misc.c +1 -0

+	?* and plls are not reset, the bootrom will fail to load the

spl
image.
? ?*/

arch/arm/mach-socfpga/misc.c +2 -1

-	writel(0xae9efebc, &sysmgr_regs->romcodegrp_warmramgrp_enable);
+
+	csel = (readl(&sysmgr_regs->bootinfo) &

SYSMGR_BOOTINFO_CSEL_MASK)

Quoted 3 lines

Quoted 3 lines

arch/arm/mach-socfpga/misc.c +4 -0

+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	if (csel)
+		writel(0xae9efebc, &sysmgr_regs-

Quoted 2 lines

romcodegrp_warmramgrp_enable);

?
? for (i = 0; i < 8; i++) /* Cache initial SW setting regs
*/
? iswgrp_handoff[i] = readl(&sysmgr_regs-

Quoted 1 line

iswgrp_handoff[i]);

Quoted 26 lines · Marek Vasut

Quoted 24 lines · Dalon Westergreen

Quoted 10 lines · Marek Vasut

Quoted 8 lines · Dalon Westergreen

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.

Quoted 4 lines

Quoted 1 line

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.

Quoted 2 lines

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.

Quoted 5 lines

Quoted 3 lines

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.

Quoted 34 lines · Dalon Westergreen

Quoted 32 lines · Marek Vasut

Quoted 29 lines · Dalon Westergreen

Quoted 12 lines · Marek Vasut

Quoted 9 lines · Dalon Westergreen

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.

Quoted 5 lines

Quoted 1 line

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.

Quoted 6 lines

Quoted 2 lines

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

Quoted 8 lines

Quoted 8 lines

Quoted 7 lines

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.

Quoted 4 lines

Quoted 1 line

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.

[...]

Quoted 54 lines · Marek Vasut

Quoted 34 lines · Dalon Westergreen

Quoted 32 lines · Marek Vasut

Quoted 29 lines · Dalon Westergreen

Quoted 12 lines · Marek Vasut

Quoted 9 lines · Dalon Westergreen

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.

Quoted 5 lines

Quoted 1 line

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.

Quoted 6 lines

Quoted 2 lines

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

Quoted 10 lines

Quoted 9 lines

Quoted 7 lines

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 ?

Quoted 5 lines

Quoted 2 lines

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?

Quoted 70 lines · Marek Vasut

Quoted 41 lines · Dalon Westergreen

Quoted 38 lines · Marek Vasut

Quoted 34 lines · Dalon Westergreen

Quoted 14 lines · Marek Vasut

Quoted 10 lines · Dalon Westergreen

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.

Quoted 5 lines

Quoted 1 line

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.

Quoted 6 lines

Quoted 2 lines

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

Quoted 17 lines

Quoted 7 lines

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

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.

Quoted 2 lines

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?

Quoted 4 lines

[...]

Quoted 85 lines · Dalon Westergreen

Quoted 83 lines · Marek Vasut

Quoted 47 lines · Dalon Westergreen

Quoted 43 lines · Marek Vasut

Quoted 38 lines · Dalon Westergreen

Quoted 16 lines · Marek Vasut

Quoted 11 lines · Dalon Westergreen

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.

Quoted 6 lines

Quoted 1 line

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.

Quoted 8 lines

Quoted 3 lines

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

Quoted 21 lines

Quoted 10 lines

Quoted 10 lines

Quoted 7 lines

Quoted 5 lines

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.

Quoted 5 lines

Quoted 2 lines

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?

Quoted 7 lines

Quoted 7 lines

[...]

Quoted 89 lines · Dalon Westergreen

Quoted 88 lines · Dalon Westergreen

Quoted 85 lines · Marek Vasut

Quoted 47 lines · Dalon Westergreen

Quoted 43 lines · Marek Vasut

Quoted 38 lines · Dalon Westergreen

Quoted 16 lines · Marek Vasut

Quoted 11 lines · Dalon Westergreen

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.

Quoted 6 lines

Quoted 1 line

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.

Quoted 8 lines

Quoted 3 lines

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

Quoted 16 lines

Quoted 16 lines

Quoted 15 lines

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.

Quoted 5 lines

Quoted 1 line

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.

Quoted 4 lines

Quoted 2 lines

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?

Quoted 114 lines · Dalon Westergreen

Quoted 112 lines · Dalon Westergreen

Quoted 109 lines · Dalon Westergreen

Quoted 94 lines · Marek Vasut

Quoted 61 lines · Dalon Westergreen

Quoted 51 lines · Marek Vasut

Quoted 32 lines · Dalon Westergreen

Quoted 19 lines · Marek Vasut

Quoted 13 lines · Dalon Westergreen

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.

Quoted 6 lines

Quoted 1 line

Signed-off-by Dalon Westergreen <[email protected]>

What about my suggestion for V2 about just loading function pointer
into
the reset jump address register ?

Quoted 13 lines

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.

Quoted 4 lines

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 :)

Quoted 5 lines

True.  I would argue to just use cold resets but i think arria 10 has
more
use
for the warm reset case.

OK

Quoted 21 lines

Quoted 20 lines

Quoted 13 lines

Quoted 5 lines

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 ?

Quoted 8 lines

Quoted 4 lines

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?

Quoted 3 lines

Quoted 1 line

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.

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/listinfo/u-boot

Quoted 119 lines · Marek Vasut

Quoted 117 lines · Dalon Westergreen

Quoted 108 lines · Dalon Westergreen

Quoted 106 lines · Dalon Westergreen

Quoted 102 lines · Marek Vasut

Quoted 61 lines · Dalon Westergreen

Quoted 51 lines · Marek Vasut

Quoted 32 lines · Dalon Westergreen

Quoted 19 lines · Marek Vasut

Quoted 13 lines · Dalon Westergreen

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.

Quoted 6 lines

Quoted 1 line

Signed-off-by Dalon Westergreen <[email protected]>

What about my suggestion for V2 about just loading function pointer
into
the reset jump address register ?

Quoted 13 lines

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.

Quoted 4 lines

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 :)

Quoted 5 lines

True. I would argue to just use cold resets but i think arria 10 has
more
use
for the warm reset case.

OK

Quoted 19 lines

Quoted 19 lines

Quoted 18 lines

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.

Quoted 6 lines

Quoted 1 line

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.

Quoted 5 lines

Quoted 3 lines

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?

Quoted 4 lines

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.