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

PATCH Superseded Dalon Westergreen 3 participants 13 messages Feb 14, 2017 – Feb 20, 2017

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm/socfpga architecture

lore

Share & tools

lore

Dalon Westergreen

author

2017-02-14 18:28 UTC #0 lore raw

PATCH v2 1/1 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. 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 <[email protected]>

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

arch/arm/mach-socfpga/Makefile +1 -1

index 809cd47..6876ccf 100644
@@ -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

arch/arm/mach-socfpga/include/mach/clock_manager.h +25 -1

index 803c926..78f63a4 100644
@@ -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
+#define CLKMGR_BYPASS_ADDRESS 0x4
+#define CLKMGR_INTER_ADDRESS 0x8
+#define CLKMGR_INTREN_ADDRESS 0xc
+#define CLKMGR_DBCTRL_ADDRESS 0x10
+#define CLKMGR_STAT_ADDRESS 0x14
+#define CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS 0x54
+#define CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS 0x58
+#define CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS 0x90
+#define CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS 0x94
+
 
 #define CLKMGR_CTRL_SAFEMODE				(1 << 0)
 #define CLKMGR_CTRL_SAFEMODE_OFFSET			0

@@ -314,4 +330,12 @@ struct socfpga_clock_manager {
 #define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_OFFSET	9
 #define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK		0x00000e00
 
+/* Bypass Main and Per PLL, bypass source per input mux */
+#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK         0x19
+                                                                                
+#define CLKMGR_MAINQSPICLK_RESET_VALUE          0x3
+#define CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE     0x3
+#define CLKMGR_PERQSPICLK_RESET_VALUE           0x1
+#define CLKMGR_PERNANDSDMMCCLK_RESET_VALUE      0x1
+
 #endif /* _CLOCK_MANAGER_H_ */

arch/arm/mach-socfpga/include/mach/reset_manager.h +4 -0

index 2f070f2..58d77fb 100644
@@ -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

arch/arm/mach-socfpga/include/mach/system_manager.h +6 -1

index c45edea..b89f269 100644
@@ -13,7 +13,6 @@ void sysmgr_pinmux_init(void);
 void sysmgr_config_warmrstcfgio(int enable);
 
 void sysmgr_get_pinmux_table(const u8 **table, unsigned int *table_len);
-#endif
 
 struct socfpga_system_manager {
 	/* System Manager Module */

@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 	u32	_pad_0x734;
 	u32	spim0usefpga;			/* 0x738 */
 };
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_SYSMGR_ADDRESS + 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)

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

index dd6b53b..57e3334 100644
@@ -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);
+
+		/* 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

arch/arm/mach-socfpga/reset_clock_manager.S new file +71 -0

71 lines not on this page · raw message

Signature · 1 line

2.7.4

Chin Liang See

2017-02-15 06:56 UTC #1 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 19 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.??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 <[email protected]>
--
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

Quoted 5 lines

++++++++++++++++++++++
?6 files changed, 134 insertions(+), 3 deletions(-) ?create mode 100644 arch/arm/mach-socfpga/reset_clock_manager.S

Acked-by Chin Liang See <[email protected]>

Thanks
Chin Liang

Marek Vasut

2017-02-15 21:16 UTC #2 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 26 lines · Chin Liang See

Quoted 12 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. 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 <[email protected]>
--
Changes in V2:
- Fix checkpatch issues predominently due to whitespace issues
Quoted 12 lines
---
 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

Quoted 1 line

Acked-by Chin Liang See <[email protected]>

Sorry, in this state, definitelly

Naked-by Marek Vasut <[email protected]>

Signature · 2 lines

Best regards,
Marek Vasut

Marek Vasut

2017-02-15 21:15 UTC #3 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

In reply to Dalon Westergreen

Quoted 54 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. 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 <[email protected]>
--
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
arch/arm/mach-socfpga/Makefile +1 -1
index 809cd47..6876ccf 100644
@@ -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
 
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -1
index 803c926..78f63a4 100644
@@ -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 */

arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135

Quoted 6 lines

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

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

Quoted 107 lines

arch/arm/mach-socfpga/include/mach/clock_manager.h +18 -0

+#define CLKMGR_BYPASS_ADDRESS 0x4
+#define CLKMGR_INTER_ADDRESS 0x8
+#define CLKMGR_INTREN_ADDRESS 0xc
+#define CLKMGR_DBCTRL_ADDRESS 0x10
+#define CLKMGR_STAT_ADDRESS 0x14
+#define CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS 0x54
+#define CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS 0x58
+#define CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS 0x90
+#define CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS 0x94
+
 
 #define CLKMGR_CTRL_SAFEMODE				(1 << 0)
 #define CLKMGR_CTRL_SAFEMODE_OFFSET			0

@@ -314,4 +330,12 @@ struct socfpga_clock_manager {
 #define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_OFFSET	9
 #define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK		0x00000e00
 
+/* Bypass Main and Per PLL, bypass source per input mux */
+#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK         0x19
+                                                                                
+#define CLKMGR_MAINQSPICLK_RESET_VALUE          0x3
+#define CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE     0x3
+#define CLKMGR_PERQSPICLK_RESET_VALUE           0x1
+#define CLKMGR_PERNANDSDMMCCLK_RESET_VALUE      0x1
+
 #endif /* _CLOCK_MANAGER_H_ */

arch/arm/mach-socfpga/include/mach/reset_manager.h +4 -0

index 2f070f2..58d77fb 100644
@@ -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

arch/arm/mach-socfpga/include/mach/system_manager.h +6 -1

index c45edea..b89f269 100644
@@ -13,7 +13,6 @@ void sysmgr_pinmux_init(void);
 void sysmgr_config_warmrstcfgio(int enable);
 
 void sysmgr_get_pinmux_table(const u8 **table, unsigned int *table_len);
-#endif
 
 struct socfpga_system_manager {
 	/* System Manager Module */

@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 	u32	_pad_0x734;
 	u32	spim0usefpga;			/* 0x738 */
 };
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_SYSMGR_ADDRESS + 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)

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

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

arch/arm/mach-socfpga/misc.c static uint32_t iswgrp_handoff[8]; · line 380

Quoted 10 lines

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

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

Quoted 18 lines

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

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

arch/arm/mach-socfpga/reset_clock_manager.S new file +5 -0

new file mode 100644
index 0000000..1818b2d
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2017, Intel Corporation
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ */

arch/arm/mach-socfpga/reset_clock_manager.S line 15

Quoted 10 lines

arch/arm/mach-socfpga/reset_clock_manager.S new file +10 -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...

Quoted 57 lines

arch/arm/mach-socfpga/reset_clock_manager.S new file +56 -0

+	/* Put Main PLL and Peripheral PLL in bypass */
+	ldr	r0, SOCFPGA_CLKMGR
+	mov	r1, #CLKMGR_BYPASS_ADDRESS
+	mov	r2, #CLKMGR_BYPASS_MAIN_PER_PLL_MASK
+	add	r3, r0, r1
+	ldr	r4, [r3]
+	orr	r5, r4, r2
+	str	r5, [r3]
+	dsb
+	isb
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_MAINQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_PERQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_PERNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+
+	/* Disable the RAM boot */
+	ldr	r0, SOCFPGA_RSTMGR
+	ldr	r1, SYSMGR_WARMRAMGRP_ENABLE
+	mov	r2, #0
+	str	r2, [r1]
+
+	/* Trigger warm reset to continue boot normally */
+	mov	r1, #RSTMGR_CTRL_OFFSET
+	add	r2, r0, r1
+	mov	r3, #1
+	mov	r3, r3, LSL #RSTMGR_CTRL_SWWARMRSTREQ_LSB
+	ldr	r4, [r2]
+	orr	r4, r3, r4
+	str	r4, [r2]
+
+reset_clock_manager_loop:
+	dsb
+	isb
+	b	reset_clock_manager_loop
+ENDPROC(reset_clock_manager)
+
+SOCFPGA_CLKMGR:			.word	SOCFPGA_CLKMGR_ADDRESS
+SOCFPGA_RSTMGR:			.word	SOCFPGA_RSTMGR_ADDRESS
+SYSMGR_WARMRAMGRP_ENABLE:	.word	CONFIG_SYSMGR_WARMRAMGRP_ENABLE
+
+.globl reset_clock_manager_size
+reset_clock_manager_size:
+	.word	. - reset_clock_manager
+

Signature · 2 lines

Best regards,
Marek Vasut

Dalon Westergreen

author

2017-02-15 21:48 UTC #4 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 60 lines · Marek Vasut

Quoted 59 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.??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 <[email protected]>
--
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
arch/arm/mach-socfpga/Makefile +0 -0
index 809cd47..6876ccf 100644
@@ -8,7 +8,7 @@
?#
?
?obj-y += misc.o timer.o reset_manager.o system_manager.o
clock_manager.o \
arch/arm/mach-socfpga/Makefile +1 -1
-	???fpga_manager.o board.o
+	???fpga_manager.o board.o reset_clock_manager.o
?
?obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o ?
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
index 803c926..78f63a4 100644
@@ -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);
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -0
+/* 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 */
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -127,6 +130,19 @@ struct socfpga_clock_manager {
 ?	struct socfpga_clock_manager_altera altera;

Quoted 8 lines

arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135
Quoted 5 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
 ?	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

Quoted 121 lines

Quoted 121 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +10 -0
+#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
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -314,4 +330,12 @@ struct socfpga_clock_manager {
?#define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_OFFSET 9
?#define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK 0x00000e00
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
+/* Bypass Main and Per PLL, bypass source per input mux */
+#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK?????????0x19
+???????????????????????????????????????????????????????????????????????????
?????
arch/arm/mach-socfpga/include/mach/clock_manager.h +5 -0
+#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_ */
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
index 2f070f2..58d77fb 100644
@@ -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);
?
arch/arm/mach-socfpga/include/mach/reset_manager.h +2 -0
@@ -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
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
@@ -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
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
index c45edea..b89f269 100644
@@ -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 */
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 ?	u32	_pad_0x734;
? u32 spim0usefpga; /* 0x738 */
?};
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_SYSMGR_ADDR
ESS + 0xe0)
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+
+#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)
arch/arm/mach-socfpga/misc.c +0 -0
index dd6b53b..57e3334 100644
@@ -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>
arch/arm/mach-socfpga/misc.c +4 -0
@@ -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)
Quoted 1 line
Quoted 1 line
arch/arm/mach-socfpga/misc.c +7 -0
+		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
arch/arm/mach-socfpga/misc.c +4 -0
+	?* reset to get back to the bootrom.
+	?*/
+	if (!csel) {
+		/* Put the code snippet in the last 4KB of the onchip ram
*/
arch/arm/mach-socfpga/misc.c +1 -0
+		ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR +

Quoted 8 lines

arch/arm/mach-socfpga/misc.c static uint32_t iswgrp_handoff[8];
Quoted 5 lines
arch/arm/mach-socfpga/misc.c +5 -0
+			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.

Quoted 25 lines

Quoted 25 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		/* 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
arch/arm/mach- renamed from arch/arm/mach-socfpga/reset_clock_manager.S +0 -0
socfpga/reset_clock_manager.S
new file mode 100644
index 0000000..1818b2d
arch/arm/mach-socfpga/reset_clock_manager.S new file +9 -0
@@ -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>

Quoted 8 lines

arch/arm/mach-socfpga/reset_clock_manager.S line 15
Quoted 6 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +6 -0
+#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. ?If it is not executed, it is possible that the bootrom (when CSEL=0) will be unable to fetch SPL at all.

as always, your comments / guidance is much appreciated.

--dalon

Quoted 62 lines

Quoted 60 lines

arch/arm/mach-socfpga/reset_clock_manager.S new file +51 -0

+	/* Put Main PLL and Peripheral PLL in bypass */
+	ldr	r0, SOCFPGA_CLKMGR
+	mov	r1, #CLKMGR_BYPASS_ADDRESS
+	mov	r2, #CLKMGR_BYPASS_MAIN_PER_PLL_MASK
+	add	r3, r0, r1
+	ldr	r4, [r3]
+	orr	r5, r4, r2
+	str	r5, [r3]
+	dsb
+	isb
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_MAINQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_PERQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_PERNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+
+	/* Disable the RAM boot */
+	ldr	r0, SOCFPGA_RSTMGR
+	ldr	r1, SYSMGR_WARMRAMGRP_ENABLE
+	mov	r2, #0
+	str	r2, [r1]
+
+	/* Trigger warm reset to continue boot normally */
+	mov	r1, #RSTMGR_CTRL_OFFSET
+	add	r2, r0, r1
+	mov	r3, #1
+	mov	r3, r3, LSL #RSTMGR_CTRL_SWWARMRSTREQ_LSB
+	ldr	r4, [r2]
+	orr	r4, r3, r4
+	str	r4, [r2]
+
+reset_clock_manager_loop:
+	dsb
+	isb
+	b	reset_clock_manager_loop
+ENDPROC(reset_clock_manager)
+
+SOCFPGA_CLKMGR:			.word	SOCFPGA_CLKMGR_ADDRESS
+SOCFPGA_RSTMGR:			.word	SOCFPGA_RSTMGR_ADDRESS
+SYSMGR_WARMRAMGRP_ENABLE:	.word	CONFIG_SYSMGR_WARMRAMGRP_ENAB

arch/arm/mach-socfpga/reset_clock_manager.S new file +5 -0

+
+.globl reset_clock_manager_size
+reset_clock_manager_size:
+	.word	. - reset_clock_manager
+

Marek Vasut

2017-02-15 22:20 UTC #5 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 69 lines · Dalon Westergreen

Quoted 56 lines · Marek Vasut
Quoted 55 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. 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 <[email protected]>
--
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
arch/arm/mach-socfpga/Makefile +1 -1
index 809cd47..6876ccf 100644
@@ -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
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -1
index 803c926..78f63a4 100644
@@ -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
 
Quoted 12 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135
Quoted 9 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
 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 ?

Quoted 1 line

It is, just defining offsets to use in the assembly calls

The asm is IMO not needed

Quoted 130 lines

Quoted 121 lines

arch/arm/mach-socfpga/include/mach/clock_manager.h +18 -0

+#define CLKMGR_BYPASS_ADDRESS 0x4
+#define CLKMGR_INTER_ADDRESS 0x8
+#define CLKMGR_INTREN_ADDRESS 0xc
+#define CLKMGR_DBCTRL_ADDRESS 0x10
+#define CLKMGR_STAT_ADDRESS 0x14
+#define CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS 0x54
+#define CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS 0x58
+#define CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS 0x90
+#define CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS 0x94
+
 
 #define CLKMGR_CTRL_SAFEMODE				(1 << 0)
 #define CLKMGR_CTRL_SAFEMODE_OFFSET			0

@@ -314,4 +330,12 @@ struct socfpga_clock_manager {
 #define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_OFFSET	9
 #define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK		0x00000e00
 
+/* Bypass Main and Per PLL, bypass source per input mux */
+#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK         0x19
+                                                                           
 
+#define CLKMGR_MAINQSPICLK_RESET_VALUE          0x3
+#define CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE     0x3
+#define CLKMGR_PERQSPICLK_RESET_VALUE           0x1
+#define CLKMGR_PERNANDSDMMCCLK_RESET_VALUE      0x1
+

#endif /* _CLOCK_MANAGER_H_ */

arch/arm/mach-socfpga/include/mach/reset_manager.h +4 -0

index 2f070f2..58d77fb 100644
@@ -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

arch/arm/mach-socfpga/include/mach/system_manager.h +6 -1

index c45edea..b89f269 100644
@@ -13,7 +13,6 @@ void sysmgr_pinmux_init(void);
 void sysmgr_config_warmrstcfgio(int enable);
 
 void sysmgr_get_pinmux_table(const u8 **table, unsigned int *table_len);
-#endif
 
 struct socfpga_system_manager {
 	/* System Manager Module */

@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 	u32	_pad_0x734;
 	u32	spim0usefpga;			/* 0x738 */
 };
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_SYSMGR_ADDR
 ESS + 0xe0)
+
+#define SYSMGR_BOOTINFO_CSEL_MASK	0x18
+#define SYSMGR_BOOTINFO_CSEL_LSB	3
 
 #define SYSMGR_ROMCODEGRP_CTRL_WARMRSTCFGPINMUX	(1 << 0)

#define SYSMGR_ROMCODEGRP_CTRL_WARMRSTCFGIO (1 << 1)

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

index dd6b53b..57e3334 100644
@@ -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)

Quoted 1 line

Quoted 1 line

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

+		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

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

+	 * reset to get back to the bootrom.
+	 */
+	if (!csel) {
+		/* Put the code snippet in the last 4KB of the onchip ram

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

+		ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR +

Quoted 8 lines

arch/arm/mach-socfpga/misc.c static uint32_t iswgrp_handoff[8]; · line 380
Quoted 5 lines
arch/arm/mach-socfpga/misc.c +5 -0
+			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 ...

Quoted 5 lines

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.

Quoted 33 lines

Quoted 24 lines
Quoted 24 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		/* 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
arch/arm/mach- renamed from arch/arm/mach-socfpga/reset_clock_manager.S +0 -0
socfpga/reset_clock_manager.S
new file mode 100644
index 0000000..1818b2d
arch/arm/mach-socfpga/reset_clock_manager.S new file +9 -0
@@ -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>
Quoted 8 lines
arch/arm/mach-socfpga/reset_clock_manager.S line 15
Quoted 6 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +6 -0
+#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...

Quoted 3 lines

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 ?

Quoted 3 lines

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 ?

Quoted 65 lines

as always, your comments / guidance is much appreciated.

--dalon

Quoted 62 lines

Quoted 60 lines

arch/arm/mach-socfpga/reset_clock_manager.S new file +51 -0

+	/* Put Main PLL and Peripheral PLL in bypass */
+	ldr	r0, SOCFPGA_CLKMGR
+	mov	r1, #CLKMGR_BYPASS_ADDRESS
+	mov	r2, #CLKMGR_BYPASS_MAIN_PER_PLL_MASK
+	add	r3, r0, r1
+	ldr	r4, [r3]
+	orr	r5, r4, r2
+	str	r5, [r3]
+	dsb
+	isb
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_MAINQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_PERQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_PERNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+
+	/* Disable the RAM boot */
+	ldr	r0, SOCFPGA_RSTMGR
+	ldr	r1, SYSMGR_WARMRAMGRP_ENABLE
+	mov	r2, #0
+	str	r2, [r1]
+
+	/* Trigger warm reset to continue boot normally */
+	mov	r1, #RSTMGR_CTRL_OFFSET
+	add	r2, r0, r1
+	mov	r3, #1
+	mov	r3, r3, LSL #RSTMGR_CTRL_SWWARMRSTREQ_LSB
+	ldr	r4, [r2]
+	orr	r4, r3, r4
+	str	r4, [r2]
+
+reset_clock_manager_loop:
+	dsb
+	isb
+	b	reset_clock_manager_loop
+ENDPROC(reset_clock_manager)
+
+SOCFPGA_CLKMGR:			.word	SOCFPGA_CLKMGR_ADDRESS
+SOCFPGA_RSTMGR:			.word	SOCFPGA_RSTMGR_ADDRESS
+SYSMGR_WARMRAMGRP_ENABLE:	.word	CONFIG_SYSMGR_WARMRAMGRP_ENAB

arch/arm/mach-socfpga/reset_clock_manager.S new file +5 -0

+
+.globl reset_clock_manager_size
+reset_clock_manager_size:
+	.word	. - reset_clock_manager
+

Signature · 2 lines

Best regards,
Marek Vasut

Dalon Westergreen

author

2017-02-16 02:53 UTC #6 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 274 lines · Marek Vasut

Quoted 73 lines · Dalon Westergreen
Quoted 63 lines · Marek Vasut
Quoted 61 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.??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 <[email protected]>
--
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
arch/arm/mach- renamed from arch/arm/mach-socfpga/Makefile +0 -0
socfpga/Makefile
index 809cd47..6876ccf 100644
arch/arm/mach-socfpga/Makefile +0 -0
@@ -8,7 +8,7 @@
?#
?
?obj-y += misc.o timer.o reset_manager.o system_manager.o
clock_manager.o \
arch/arm/mach-socfpga/Makefile +1 -1
-	???fpga_manager.o board.o
+	???fpga_manager.o board.o reset_clock_manager.o
?
?obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o ?
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
index 803c926..78f63a4 100644
@@ -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);
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -0
+/* 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 */
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -127,6 +130,19 @@ struct socfpga_clock_manager {
 ?	struct socfpga_clock_manager_altera altera;
Quoted 8 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135
Quoted 5 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
 ?	u32	_pad_0xe8_0x200[70];
 ?};
+#endif
+
+#define CLKMGR_CTRL_ADDRESS 0x0
Is this the same as struct socfpga_clock_manager {} ? Why ?
Quoted 1 line
It is, just defining offsets to use in the assembly calls
The asm is IMO not needed
Quoted 144 lines
Quoted 135 lines
Quoted 135 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +10 -0
+#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
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -314,4 +330,12 @@ struct socfpga_clock_manager {
?#define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_OFFSET 9
?#define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK 0x00000e
00
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
+/* Bypass Main and Per PLL, bypass source per input mux */
+#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK?????????0x19
+???????????????????????????????????????????????????????????????????????
????
?????
arch/arm/mach-socfpga/include/mach/clock_manager.h +5 -0
+#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_ */
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
index 2f070f2..58d77fb 100644
@@ -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);
?
arch/arm/mach-socfpga/include/mach/reset_manager.h +2 -0
@@ -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
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
@@ -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
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
index c45edea..b89f269 100644
@@ -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 */
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 ?	u32	_pad_0x734;
? u32 spim0usefpga; /* 0x738 */
?};
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_SYSMGR_
ADDR
ESS + 0xe0)
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+
+#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)
arch/arm/mach-socfpga/misc.c +0 -0
index dd6b53b..57e3334 100644
@@ -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>
arch/arm/mach-socfpga/misc.c +4 -0
@@ -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)
Quoted 3 lines
Quoted 3 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	/*
+	?* For CSEL = 0 the bootrom does not configure the clocks which
can
arch/arm/mach-socfpga/misc.c +1 -0
+	?* result in a boot failure on warm resets.??To remedy this a
small
arch/arm/mach-socfpga/misc.c +1 -0
+	?* bit of code is placed at the end of the onchip ram and run
on
arch/arm/mach-socfpga/misc.c +1 -0
+	?* a warm reset.??It puts the PLLs in bypass and issues another
warm
arch/arm/mach-socfpga/misc.c +4 -0
+	?* reset to get back to the bootrom.
+	?*/
+	if (!csel) {
+		/* Put the code snippet in the last 4KB of the onchip
ram
*/
arch/arm/mach-socfpga/misc.c +1 -0
+		ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR +
Quoted 8 lines
arch/arm/mach-socfpga/misc.c static uint32_t iswgrp_handoff[8];
Quoted 5 lines
arch/arm/mach-socfpga/misc.c +5 -0
+			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 ...
Quoted 5 lines
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.
Quoted 37 lines
Quoted 28 lines
Quoted 28 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		/* 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
arch/arm/mach- renamed from arch/arm/mach-socfpga/reset_clock_manager.S +0 -0
socfpga/reset_clock_manager.S
new file mode 100644
index 0000000..1818b2d
arch/arm/mach-socfpga/reset_clock_manager.S new file +9 -0
@@ -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>
Quoted 8 lines
arch/arm/mach-socfpga/reset_clock_manager.S line 15
Quoted 6 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +6 -0
+#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...
Quoted 3 lines
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 ?
Quoted 1 line

Quoted 7 lines

Quoted 3 lines
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

Quoted 74 lines

Quoted 72 lines

as always, your comments / guidance is much appreciated.

--dalon

Quoted 67 lines

Quoted 65 lines

arch/arm/mach-socfpga/reset_clock_manager.S new file +49 -0

+	/* Put Main PLL and Peripheral PLL in bypass */
+	ldr	r0, SOCFPGA_CLKMGR
+	mov	r1, #CLKMGR_BYPASS_ADDRESS
+	mov	r2, #CLKMGR_BYPASS_MAIN_PER_PLL_MASK
+	add	r3, r0, r1
+	ldr	r4, [r3]
+	orr	r5, r4, r2
+	str	r5, [r3]
+	dsb
+	isb
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_MAINQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_PERQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_PERNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+
+	/* Disable the RAM boot */
+	ldr	r0, SOCFPGA_RSTMGR
+	ldr	r1, SYSMGR_WARMRAMGRP_ENABLE
+	mov	r2, #0
+	str	r2, [r1]
+
+	/* Trigger warm reset to continue boot normally */
+	mov	r1, #RSTMGR_CTRL_OFFSET
+	add	r2, r0, r1
+	mov	r3, #1
+	mov	r3, r3, LSL #RSTMGR_CTRL_SWWARMRSTREQ_LSB
+	ldr	r4, [r2]
+	orr	r4, r3, r4
+	str	r4, [r2]
+
+reset_clock_manager_loop:
+	dsb
+	isb
+	b	reset_clock_manager_loop
+ENDPROC(reset_clock_manager)
+
+SOCFPGA_CLKMGR:			.word	SOCFPGA_CLKMGR_ADDR

ESS
+SOCFPGA_RSTMGR: .word SOCFPGA_RSTMGR_ADDR
ESS
+SYSMGR_WARMRAMGRP_ENABLE: .word CONFIG_SYSMGR_WARMRAMGRP_
ENAB
LE

arch/arm/mach-socfpga/reset_clock_manager.S new file +5 -0

+
+.globl reset_clock_manager_size
+reset_clock_manager_size:
+	.word	. - reset_clock_manager
+

Dalon Westergreen

author

2017-02-17 18:05 UTC #7 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 323 lines · Dalon Westergreen

Quoted 296 lines · Marek Vasut
Quoted 77 lines · Dalon Westergreen
Quoted 66 lines · Marek Vasut
Quoted 63 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.??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 <[email protected]>
--
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
arch/arm/mach- renamed from arch/arm/mach-socfpga/Makefile +0 -0
socfpga/Makefile
index 809cd47..6876ccf 100644
arch/arm/mach-socfpga/Makefile +0 -0
@@ -8,7 +8,7 @@
?#
?
?obj-y += misc.o timer.o reset_manager.o system_manager.o
clock_manager.o \
arch/arm/mach-socfpga/Makefile +1 -1
-	???fpga_manager.o board.o
+	???fpga_manager.o board.o reset_clock_manager.o
?
?obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o ?
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
index 803c926..78f63a4 100644
@@ -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);
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -0
+/* 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 */
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -127,6 +130,19 @@ struct socfpga_clock_manager {
 ?	struct socfpga_clock_manager_altera altera;
Quoted 8 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135
Quoted 5 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
 ?	u32	_pad_0xe8_0x200[70];
 ?};
+#endif
+
+#define CLKMGR_CTRL_ADDRESS 0x0
Is this the same as struct socfpga_clock_manager {} ? Why ?
Quoted 1 line
It is, just defining offsets to use in the assembly calls
The asm is IMO not needed
Quoted 155 lines
Quoted 146 lines
Quoted 146 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +10 -0
+#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
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -314,4 +330,12 @@ struct socfpga_clock_manager {
?#define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_OFFSET 9
?#define CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK 0x0000
0e
00
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
+/* Bypass Main and Per PLL, bypass source per input mux */
+#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK?????????0x19
+?????????????????????????????????????????????????????????????????????
??
????
?????
arch/arm/mach-socfpga/include/mach/clock_manager.h +5 -0
+#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_ */
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
index 2f070f2..58d77fb 100644
@@ -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);
?
arch/arm/mach-socfpga/include/mach/reset_manager.h +2 -0
@@ -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
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
@@ -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
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
index c45edea..b89f269 100644
@@ -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 */
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 ?	u32	_pad_0x734;
? u32 spim0usefpga; /* 0x738 */
?};
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_SYSMG
R_
ADDR
ESS + 0xe0)
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+
+#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)
arch/arm/mach- renamed from arch/arm/mach-socfpga/misc.c +0 -0
socfpga/misc.c
index dd6b53b..57e3334 100644
arch/arm/mach-socfpga/misc.c +0 -0
@@ -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>
arch/arm/mach-socfpga/misc.c +4 -0
@@ -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)
Quoted 5 lines
Quoted 5 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	/*
+	?* For CSEL = 0 the bootrom does not configure the clocks
which
can
arch/arm/mach-socfpga/misc.c +1 -0
+	?* result in a boot failure on warm resets.??To remedy this a
small
arch/arm/mach-socfpga/misc.c +1 -0
+	?* bit of code is placed at the end of the onchip ram and run
on
arch/arm/mach-socfpga/misc.c +1 -0
+	?* a warm reset.??It puts the PLLs in bypass and issues
another
warm
arch/arm/mach-socfpga/misc.c +4 -0
+	?* reset to get back to the bootrom.
+	?*/
+	if (!csel) {
+		/* Put the code snippet in the last 4KB of the onchip
ram
*/
arch/arm/mach-socfpga/misc.c +1 -0
+		ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR +
Quoted 8 lines
arch/arm/mach-socfpga/misc.c static uint32_t iswgrp_handoff[8];
Quoted 5 lines
arch/arm/mach-socfpga/misc.c +5 -0
+			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 ...
Quoted 5 lines
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.
Quoted 42 lines
Quoted 33 lines
Quoted 33 lines
arch/arm/mach-socfpga/misc.c +1 -0
+		/* Set the bootrom to run the code snippet on reset
*/
arch/arm/mach-socfpga/misc.c +3 -0
+		writel(ramboot_addr,
+		??????&sysmgr_regs->romcodegrp_warmramgrp_execution);
+	}
?
? /*
? ?* Write magic value into magic register to unlock support
for
arch/arm/mach- renamed from arch/arm/mach-socfpga/reset_clock_manager.S +0 -0
socfpga/reset_clock_manager.S
new file mode 100644
index 0000000..1818b2d
arch/arm/mach-socfpga/reset_clock_manager.S new file +9 -0
@@ -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>
Quoted 8 lines
arch/arm/mach-socfpga/reset_clock_manager.S line 15
Quoted 6 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +6 -0
+#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...
Quoted 3 lines
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 ?
Quoted 2 lines
Quoted 7 lines
Quoted 3 lines
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.

Quoted 7 lines

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.

Thanks,
Dalon

Quoted 81 lines

Quoted 79 lines

as always, your comments / guidance is much appreciated.

--dalon

Quoted 72 lines

Quoted 70 lines

arch/arm/mach-socfpga/reset_clock_manager.S new file +49 -0

+	/* Put Main PLL and Peripheral PLL in bypass */
+	ldr	r0, SOCFPGA_CLKMGR
+	mov	r1, #CLKMGR_BYPASS_ADDRESS
+	mov	r2, #CLKMGR_BYPASS_MAIN_PER_PLL_MASK
+	add	r3, r0, r1
+	ldr	r4, [r3]
+	orr	r5, r4, r2
+	str	r5, [r3]
+	dsb
+	isb
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_MAINQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_MAINPLLGRP_MAINNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_MAINNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERQSPICLK_ADDRESS
+	mov	r2, #CLKMGR_PERQSPICLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+	mov	r1, #CLKMGR_PERPLLGRP_PERNANDSDMMCCLK_ADDRESS
+	mov	r2, #CLKMGR_PERNANDSDMMCCLK_RESET_VALUE
+	add	r3, r0, r1
+	str	r2, [r3]
+
+	/* Disable the RAM boot */
+	ldr	r0, SOCFPGA_RSTMGR
+	ldr	r1, SYSMGR_WARMRAMGRP_ENABLE
+	mov	r2, #0
+	str	r2, [r1]
+
+	/* Trigger warm reset to continue boot normally */
+	mov	r1, #RSTMGR_CTRL_OFFSET
+	add	r2, r0, r1
+	mov	r3, #1
+	mov	r3, r3, LSL #RSTMGR_CTRL_SWWARMRSTREQ_LSB
+	ldr	r4, [r2]
+	orr	r4, r3, r4
+	str	r4, [r2]
+
+reset_clock_manager_loop:
+	dsb
+	isb
+	b	reset_clock_manager_loop
+ENDPROC(reset_clock_manager)
+
+SOCFPGA_CLKMGR:			.word	SOCFPGA_CLKMGR_AD

DR
ESS
+SOCFPGA_RSTMGR: .word SOCFPGA_RSTMGR_AD
DR
ESS
+SYSMGR_WARMRAMGRP_ENABLE: .word CONFIG_SYSMGR_WARMRAMGR
P_
ENAB
LE

arch/arm/mach-socfpga/reset_clock_manager.S new file +5 -0

+
+.globl reset_clock_manager_size
+reset_clock_manager_size:
+	.word	. - reset_clock_manager
+

Marek Vasut

2017-02-17 21:16 UTC #8 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 332 lines · Dalon Westergreen

Quoted 323 lines · Dalon Westergreen
Quoted 296 lines · Marek Vasut
Quoted 77 lines · Dalon Westergreen
Quoted 62 lines · Marek Vasut
Quoted 59 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. 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 <[email protected]>
--
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
arch/arm/mach- renamed from arch/arm/mach-socfpga/Makefile +0 -0
socfpga/Makefile
index 809cd47..6876ccf 100644
arch/arm/mach-socfpga/Makefile +1 -1
@@ -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
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -1
index 803c926..78f63a4 100644
@@ -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
 
Quoted 12 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135
Quoted 9 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
 struct cm_config {
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
 	/* 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 ?
Quoted 1 line
It is, just defining offsets to use in the assembly calls
The asm is IMO not needed
Quoted 155 lines
Quoted 146 lines
Quoted 146 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +10 -0
+#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		0x0000
0e
00
arch/arm/mach-socfpga/include/mach/clock_manager.h +8 -0
+/* 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_ */
arch/arm/mach-socfpga/include/mach/reset_manager.h +4 -0
index 2f070f2..58d77fb 100644
@@ -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
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -1
index c45edea..b89f269 100644
@@ -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 {
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
 	/* System Manager Module */
@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 	u32	_pad_0x734;
 	u32	spim0usefpga;			/* 0x738 */
 };
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_SYSMG
R_
ADDR
ESS + 0xe0)
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+
+#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)
arch/arm/mach- renamed from arch/arm/mach-socfpga/misc.c +0 -0
socfpga/misc.c
index dd6b53b..57e3334 100644
arch/arm/mach-socfpga/misc.c +5 -0
@@ -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)
Quoted 5 lines
Quoted 5 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	/*
+	 * For CSEL = 0 the bootrom does not configure the clocks
which
can
arch/arm/mach-socfpga/misc.c +1 -0
+	 * result in a boot failure on warm resets.  To remedy this a
small
arch/arm/mach-socfpga/misc.c +1 -0
+	 * bit of code is placed at the end of the onchip ram and run
on
arch/arm/mach-socfpga/misc.c +1 -0
+	 * a warm reset.  It puts the PLLs in bypass and issues
another
warm
arch/arm/mach-socfpga/misc.c +4 -0
+	 * reset to get back to the bootrom.
+	 */
+	if (!csel) {
+		/* Put the code snippet in the last 4KB of the onchip
ram
*/
arch/arm/mach-socfpga/misc.c +1 -0
+		ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR +
Quoted 8 lines
arch/arm/mach-socfpga/misc.c static uint32_t iswgrp_handoff[8]; · line 380
Quoted 5 lines
arch/arm/mach-socfpga/misc.c +5 -0
+			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 ...
Quoted 5 lines
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.
Quoted 42 lines
Quoted 33 lines
Quoted 33 lines
arch/arm/mach-socfpga/misc.c +1 -0
+		/* Set the bootrom to run the code snippet on reset
*/
arch/arm/mach-socfpga/misc.c +3 -0
+		writel(ramboot_addr,
+		      &sysmgr_regs->romcodegrp_warmramgrp_execution);
+	}
 
 	/*
 	 * Write magic value into magic register to unlock support
for
arch/arm/mach- renamed from arch/arm/mach-socfpga/reset_clock_manager.S +0 -0
socfpga/reset_clock_manager.S
new file mode 100644
index 0000000..1818b2d
arch/arm/mach-socfpga/reset_clock_manager.S new file +9 -0
@@ -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>
Quoted 8 lines
arch/arm/mach-socfpga/reset_clock_manager.S line 15
Quoted 6 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +6 -0
+#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...
Quoted 3 lines
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 ?
Quoted 2 lines
Quoted 7 lines
Quoted 3 lines
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.
Quoted 7 lines
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

Quoted 7 lines

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

How do you point bootrom to run that snippet instead of whatever is in the OCRAM ?

Signature · 2 lines

Best regards,
Marek Vasut

Dalon Westergreen

author

2017-02-17 23:24 UTC #9 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 370 lines · Marek Vasut

Quoted 361 lines · Dalon Westergreen
Quoted 351 lines · Dalon Westergreen
Quoted 323 lines · Marek Vasut
Quoted 82 lines · Dalon Westergreen
Quoted 70 lines · Marek Vasut
Quoted 66 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.??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 <[email protected]>
--
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
arch/arm/mach- renamed from arch/arm/mach-socfpga/Makefile +0 -0
socfpga/Makefile
index 809cd47..6876ccf 100644
arch/arm/mach-socfpga/Makefile +0 -0
@@ -8,7 +8,7 @@
?#
?
?obj-y += misc.o timer.o reset_manager.o system_manager.o
clock_manager.o \
arch/arm/mach-socfpga/Makefile +1 -1
-	???fpga_manager.o board.o
+	???fpga_manager.o board.o reset_clock_manager.o
?
?obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o ?
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
index 803c926..78f63a4 100644
@@ -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);
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -0
+/* 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 */
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -127,6 +130,19 @@ struct socfpga_clock_manager {
 ?	struct socfpga_clock_manager_altera altera;
Quoted 8 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135
Quoted 5 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
 ?	u32	_pad_0xe8_0x200[70];
 ?};
+#endif
+
+#define CLKMGR_CTRL_ADDRESS 0x0
Is this the same as struct socfpga_clock_manager {} ? Why ?
Quoted 1 line
It is, just defining offsets to use in the assembly calls
The asm is IMO not needed
Quoted 169 lines
Quoted 160 lines
Quoted 160 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +10 -0
+#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
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -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
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
+/* Bypass Main and Per PLL, bypass source per input mux */
+#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK?????????0x19
+?????????????????????????????????????????????????????????????????
????
??
????
?????
arch/arm/mach-socfpga/include/mach/clock_manager.h +5 -0
+#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_ */
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
index 2f070f2..58d77fb 100644
@@ -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);
?
arch/arm/mach-socfpga/include/mach/reset_manager.h +2 -0
@@ -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
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
@@ -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
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
index c45edea..b89f269 100644
@@ -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 */
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 ?	u32	_pad_0x734;
? u32 spim0usefpga; /* 0x738
*/
?};
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_S
YSMG
R_
ADDR
ESS + 0xe0)
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+
+#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)
arch/arm/mach- renamed from arch/arm/mach-socfpga/misc.c +0 -0
socfpga/misc.c
index dd6b53b..57e3334 100644
arch/arm/mach-socfpga/misc.c +0 -0
@@ -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>
arch/arm/mach-socfpga/misc.c +4 -0
@@ -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)
Quoted 7 lines
Quoted 7 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	/*
+	?* For CSEL = 0 the bootrom does not configure the clocks
which
can
arch/arm/mach-socfpga/misc.c +1 -0
+	?* result in a boot failure on warm resets.??To remedy
this a
small
arch/arm/mach-socfpga/misc.c +1 -0
+	?* bit of code is placed at the end of the onchip ram and
run
on
arch/arm/mach-socfpga/misc.c +1 -0
+	?* a warm reset.??It puts the PLLs in bypass and issues
another
warm
arch/arm/mach-socfpga/misc.c +4 -0
+	?* reset to get back to the bootrom.
+	?*/
+	if (!csel) {
+		/* Put the code snippet in the last 4KB of the
onchip
ram
*/
arch/arm/mach-socfpga/misc.c +2 -0
+		ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR +
+			CONFIG_SYS_INIT_RAM_SIZE - 0x1000;
Quoted 8 lines
arch/arm/mach-socfpga/misc.c
Quoted 4 lines
arch/arm/mach-socfpga/misc.c +4 -0
+
+		/* 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 ...
Quoted 5 lines
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.
Quoted 48 lines
Quoted 39 lines
Quoted 39 lines
arch/arm/mach-socfpga/misc.c +1 -0
+		/* Set the bootrom to run the code snippet on
reset
*/
arch/arm/mach-socfpga/misc.c +2 -0
+		writel(ramboot_addr,
+		??????&sysmgr_regs-
Quoted 1 line
romcodegrp_warmramgrp_execution);
arch/arm/mach-socfpga/misc.c +1 -0
+	}
?
? /*
? ?* Write magic value into magic register to unlock
support
for
arch/arm/mach- renamed from arch/arm/mach-socfpga/reset_clock_manager.S +0 -0
socfpga/reset_clock_manager.S
new file mode 100644
index 0000000..1818b2d
arch/arm/mach-socfpga/reset_clock_manager.S new file +9 -0
@@ -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>
Quoted 8 lines
arch/arm/mach-socfpga/reset_clock_manager.S line 15
Quoted 6 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +6 -0
+#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...
Quoted 3 lines
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 ?
Quoted 3 lines
Quoted 7 lines
Quoted 3 lines
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.
Quoted 7 lines
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
Quoted 7 lines
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.

Quoted 3 lines

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.

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.

Quoted 3 lines

How do you point bootrom to run that snippet instead of whatever is in the OCRAM ?

This code here

Quoted 3 lines

?>?>?>?>?>?>?+ writel(ramboot_addr,
?>?>?>?>?>?>?+ ??????&sysmgr_regs-
?>?>?>?>?>?>?>romcodegrp_warmramgrp_execution);

writes the address to jump to on warm reset. ?The register value is preserved through a warm reset.

Quoted 1 line

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)

--dalon

Marek Vasut

2017-02-19 21:31 UTC #10 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 375 lines · Dalon Westergreen

Quoted 370 lines · Marek Vasut
Quoted 361 lines · Dalon Westergreen
Quoted 351 lines · Dalon Westergreen
Quoted 323 lines · Marek Vasut
Quoted 82 lines · Dalon Westergreen
Quoted 66 lines · Marek Vasut
Quoted 62 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. 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 <[email protected]>
--
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
arch/arm/mach- renamed from arch/arm/mach-socfpga/Makefile +0 -0
socfpga/Makefile
index 809cd47..6876ccf 100644
arch/arm/mach-socfpga/Makefile +1 -1
@@ -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
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -1
index 803c926..78f63a4 100644
@@ -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
 
Quoted 12 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135
Quoted 9 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
 struct cm_config {
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
 	/* 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 ?
Quoted 1 line
It is, just defining offsets to use in the assembly calls
The asm is IMO not needed
Quoted 169 lines
Quoted 160 lines
Quoted 160 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +10 -0
+#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
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -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
arch/arm/mach-socfpga/include/mach/clock_manager.h +8 -0
+/* 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_ */
arch/arm/mach-socfpga/include/mach/reset_manager.h +4 -0
index 2f070f2..58d77fb 100644
@@ -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
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -1
index c45edea..b89f269 100644
@@ -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 {
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
 	/* System Manager Module */
@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 	u32	_pad_0x734;
 	u32	spim0usefpga;			/* 0x738
 */
 };
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFPGA_S
YSMG
R_
ADDR
ESS + 0xe0)
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+
+#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)
arch/arm/mach- renamed from arch/arm/mach-socfpga/misc.c +0 -0
socfpga/misc.c
index dd6b53b..57e3334 100644
arch/arm/mach-socfpga/misc.c +5 -0
@@ -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)
Quoted 7 lines
Quoted 7 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	/*
+	 * For CSEL = 0 the bootrom does not configure the clocks
which
can
arch/arm/mach-socfpga/misc.c +1 -0
+	 * result in a boot failure on warm resets.  To remedy
this a
small
arch/arm/mach-socfpga/misc.c +1 -0
+	 * bit of code is placed at the end of the onchip ram and
run
on
arch/arm/mach-socfpga/misc.c +1 -0
+	 * a warm reset.  It puts the PLLs in bypass and issues
another
warm
arch/arm/mach-socfpga/misc.c +4 -0
+	 * reset to get back to the bootrom.
+	 */
+	if (!csel) {
+		/* Put the code snippet in the last 4KB of the
onchip
ram
*/
arch/arm/mach-socfpga/misc.c +2 -0
+		ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR +
+			CONFIG_SYS_INIT_RAM_SIZE - 0x1000;
Quoted 8 lines
arch/arm/mach-socfpga/misc.c
Quoted 4 lines
arch/arm/mach-socfpga/misc.c +4 -0
+
+		/* 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 ...
Quoted 5 lines
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.
Quoted 48 lines
Quoted 39 lines
Quoted 39 lines
arch/arm/mach-socfpga/misc.c +1 -0
+		/* Set the bootrom to run the code snippet on
reset
*/
arch/arm/mach-socfpga/misc.c +2 -0
+		writel(ramboot_addr,
+		      &sysmgr_regs-
Quoted 1 line
romcodegrp_warmramgrp_execution);
arch/arm/mach-socfpga/misc.c +1 -0
+	}
 
 	/*
 	 * Write magic value into magic register to unlock
support
for
arch/arm/mach- renamed from arch/arm/mach-socfpga/reset_clock_manager.S +0 -0
socfpga/reset_clock_manager.S
new file mode 100644
index 0000000..1818b2d
arch/arm/mach-socfpga/reset_clock_manager.S new file +9 -0
@@ -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>
Quoted 8 lines
arch/arm/mach-socfpga/reset_clock_manager.S line 15
Quoted 6 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +6 -0
+#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...
Quoted 3 lines
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 ?
Quoted 3 lines
Quoted 7 lines
Quoted 3 lines
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.
Quoted 7 lines
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
Quoted 7 lines
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.
Quoted 3 lines
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 ...

Quoted 5 lines

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 ?

Quoted 3 lines

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

Quoted 9 lines

Quoted 3 lines
How do you point bootrom to run that snippet instead of whatever is in the OCRAM ?
This code here
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +2 -0
+		writel(ramboot_addr,
+		      &sysmgr_regs-
Quoted 1 line
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 ?

Quoted 2 lines

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.

Quoted 7 lines

Quoted 1 line
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.

Quoted 7 lines

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

Signature · 2 lines

Best regards,
Marek Vasut

Dalon Westergreen

author

2017-02-20 14:07 UTC #11 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

Quoted 420 lines · Marek Vasut

Quoted 419 lines · Dalon Westergreen
Quoted 416 lines · Marek Vasut
Quoted 403 lines · Dalon Westergreen
Quoted 392 lines · Dalon Westergreen
Quoted 366 lines · Marek Vasut
Quoted 93 lines · Dalon Westergreen
Quoted 80 lines · Marek Vasut
Quoted 75 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.??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 <[email protected]>
--
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
arch/arm/mach- renamed from arch/arm/mach-socfpga/Makefile +0 -0
socfpga/Makefile
index 809cd47..6876ccf 100644
arch/arm/mach-socfpga/Makefile +0 -0
@@ -8,7 +8,7 @@
?#
?
?obj-y += misc.o timer.o reset_manager.o
system_manager.o
clock_manager.o \
arch/arm/mach-socfpga/Makefile +1 -1
-	???fpga_manager.o board.o
+	???fpga_manager.o board.o reset_clock_manager.o
?
?obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o ?
(quoted diff) +0 -0
socfpga/include/mach/clock_manager.h b/arch/arm/mach-socfpga/include/mach/clock_manager.h index 803c926..78f63a4 100644
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -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); ?
arch/arm/mach-socfpga/include/mach/clock_manager.h +4 -0
+/* 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 */
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -0
@@ -127,6 +130,19 @@ struct socfpga_clock_manager {
 ?	struct socfpga_clock_manager_altera altera;
Quoted 8 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h struct socfpga_clock_manager { · line 135
Quoted 5 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
 ?	u32	_pad_0xe8_0x200[70];
 ?};
+#endif
+
+#define CLKMGR_CTRL_ADDRESS 0x0
Is this the same as struct socfpga_clock_manager {} ? Why ?
Quoted 1 line
It is, just defining offsets to use in the assembly calls
The asm is IMO not needed
Quoted 188 lines
Quoted 179 lines
Quoted 179 lines
arch/arm/mach-socfpga/include/mach/clock_manager.h +10 -0
+#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
arch/arm/mach-socfpga/include/mach/clock_manager.h +0 -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
?
arch/arm/mach-socfpga/include/mach/clock_manager.h +3 -0
+/* Bypass Main and Per PLL, bypass source per input mux */
+#define CLKMGR_BYPASS_MAIN_PER_PLL_MASK?????????0x19
+?????????????????????????????????????????????????????????????
????
????
??
????
?????
arch/arm/mach-socfpga/include/mach/clock_manager.h +5 -0
+#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_ */
(quoted diff) +0 -0
socfpga/include/mach/reset_manager.h b/arch/arm/mach-socfpga/include/mach/reset_manager.h index 2f070f2..58d77fb 100644
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
@@ -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);
?
arch/arm/mach-socfpga/include/mach/reset_manager.h +2 -0
@@ -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
arch/arm/mach-socfpga/include/mach/reset_manager.h +0 -0
@@ -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
(quoted diff) +0 -0
socfpga/include/mach/system_manager.h b/arch/arm/mach-socfpga/include/mach/system_manager.h index c45edea..b89f269 100644
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
@@ -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 */
arch/arm/mach-socfpga/include/mach/system_manager.h +0 -0
@@ -115,6 +114,12 @@ struct socfpga_system_manager {
 ?	u32	_pad_0x734;
? u32 spim0usefpga; /*
0x738
*/
?};
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+#endif
+
+#define CONFIG_SYSMGR_WARMRAMGRP_ENABLE		(SOCFP
GA_S
YSMG
R_
ADDR
ESS + 0xe0)
arch/arm/mach-socfpga/include/mach/system_manager.h +3 -0
+
+#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)
arch/arm/mach- renamed from arch/arm/mach-socfpga/misc.c +0 -0
socfpga/misc.c
index dd6b53b..57e3334 100644
arch/arm/mach-socfpga/misc.c +0 -0
@@ -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>
arch/arm/mach-socfpga/misc.c +4 -0
@@ -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)
Quoted 9 lines
Quoted 9 lines
arch/arm/mach-socfpga/misc.c +4 -0
+		SYSMGR_BOOTINFO_CSEL_LSB;
+
+	/*
+	?* For CSEL = 0 the bootrom does not configure the
clocks
which
can
arch/arm/mach-socfpga/misc.c +1 -0
+	?* result in a boot failure on warm resets.??To
remedy
this a
small
arch/arm/mach-socfpga/misc.c +1 -0
+	?* bit of code is placed at the end of the onchip ram
and
run
on
arch/arm/mach-socfpga/misc.c +1 -0
+	?* a warm reset.??It puts the PLLs in bypass and
issues
another
warm
arch/arm/mach-socfpga/misc.c +4 -0
+	?* reset to get back to the bootrom.
+	?*/
+	if (!csel) {
+		/* Put the code snippet in the last 4KB of
the
onchip
ram
*/
arch/arm/mach-socfpga/misc.c +3 -0
+		ramboot_addr = CONFIG_SYS_INIT_RAM_ADDR +
+			CONFIG_SYS_INIT_RAM_SIZE - 0x1000;
+
Quoted 8 lines
arch/arm/mach-socfpga/misc.c
Quoted 4 lines
arch/arm/mach-socfpga/misc.c +2 -0
+		/* Copy the code to the onchip ramlocation */
+		memcpy((void *)ramboot_addr,
reset_clock_manager,
arch/arm/mach-socfpga/misc.c +1 -0
+		??????reset_clock_manager_size);
So uh, why don't you put this code into SPL and execute it from there ?
This is b/s ...
Quoted 6 lines
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.
Quoted 53 lines
Quoted 44 lines
Quoted 44 lines
arch/arm/mach-socfpga/misc.c +1 -0
+		/* Set the bootrom to run the code snippet on
reset
*/
arch/arm/mach-socfpga/misc.c +2 -0
+		writel(ramboot_addr,
+		??????&sysmgr_regs-
Quoted 2 lines
romcodegrp_warmramgrp_execution);
arch/arm/mach-socfpga/misc.c +1 -0
+	}
?
? /*
? ?* Write magic value into magic register to unlock
support
for
arch/arm/mach- renamed from arch/arm/mach-socfpga/reset_clock_manager.S +0 -0
socfpga/reset_clock_manager.S
new file mode 100644
index 0000000..1818b2d
arch/arm/mach-socfpga/reset_clock_manager.S new file +10 -0
@@ -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>
Quoted 8 lines
arch/arm/mach-socfpga/reset_clock_manager.S line 15
Quoted 5 lines
arch/arm/mach-socfpga/reset_clock_manager.S new file +5 -0
+#include <asm/arch/clock_manager.h>
+
+/*
+ */
+ENTRY(reset_clock_manager)
This is just a few writel() calls in SPL , right ? Put it there...
Quoted 3 lines
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 ?
Quoted 8 lines
If it
is not executed, it is possible that the bootrom (when CSEL=0) will
be unable to fetch SPL at all.
Quoted 4 lines
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.
Quoted 7 lines
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
Quoted 7 lines
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 ...

Quoted 8 lines

Quoted 5 lines
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.

Quoted 19 lines

Quoted 4 lines
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 ;-)
Quoted 9 lines
Quoted 5 lines
How do you point bootrom to run that snippet instead of whatever is in the OCRAM ?
This code here
Quoted 1 line

Quoted 7 lines

Quoted 3 lines
Quoted 3 lines
?> > > > > > + 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.

Quoted 20 lines

Quoted 3 lines
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.
Quoted 14 lines
Quoted 3 lines
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.

Quoted 5 lines

Quoted 2 lines
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

Marek Vasut

2017-02-20 14:12 UTC #12 lore raw

PATCH v2 1/1 arm: socfpga: fix issue with warm reset when CSEL is 0

On 02/20/2017 03:07 PM, Dalon Westergreen wrote:
[...]

Quoted 10 lines

Quoted 8 lines
Quoted 5 lines
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.

I see.

Quoted 23 lines

Quoted 23 lines
Quoted 4 lines
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 ;-)
Quoted 12 lines
Quoted 5 lines
How do you point bootrom to run that snippet instead of whatever is in the OCRAM ?
This code here
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
(quoted diff) +2 -0
+		writel(ramboot_addr,
+		      &sysmgr_regs-
Quoted 1 line
romcodegrp_warmramgrp_execution);

Quoted 6 lines

Quoted 3 lines
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.

Well if you corrupt SPL and do a warm-reset, your system won't boot. We had that already (and that's another +1 for just doing cold reset, always).

Quoted 20 lines

Quoted 20 lines
Quoted 3 lines
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.
Quoted 14 lines
Quoted 3 lines
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.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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.

Hm, OK, it's fine either way for me.

Signature · 2 lines

Best regards,
Marek Vasut