Skip to contentSkip to conversation

ls102xa: HYP/non-sec: for ls102xa.

PATCH Superseded Xiubo Li 3 participants 22 messages Oct 20, 2014 – Nov 21, 2014

Subsystems (2)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Xiubo Li

author
#0 lore raw

v3 cover ls102xa: HYP/non-sec: for ls102xa.

Change for V3:
- Fix the language in commit message.

Change for V2:
- All the registers are defined as a struct, here use it.
- Use CONFIG_PEN_ADDR_BIG_ENDIAN instead of CONFIG_SOC_BIG_ENDIAN.

Xiubo Li (5):
ARM: HYP/non-sec: add the pen address BE mode support.
ARM: HYP/non-sec: Fix the ARCH Timer frequency setting.
ls102xa: HYP/non-sec: support for ls102xa boards
ARM: ls102xa: allow all the peripheral access permissions as R/W.
ARM: ls102xa: Setting device's stream id for SMMUs.

 arch/arm/cpu/armv7/ls102xa/cpu.c                   |  15 +++
 arch/arm/cpu/armv7/nonsec_virt.S                   |   7 +-
 arch/arm/include/asm/arch-ls102xa/config.h         |   3 +
 arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h  |   3 +
 .../include/asm/arch-ls102xa/ls102xa_stream_id.h   |  17 +++
 arch/arm/include/asm/arch-ls102xa/ns_access.h      | 118 +++++++++++++++++++++
 board/freescale/common/Makefile                    |   4 +
 board/freescale/common/ls102xa_stream_id.c         |  18 ++++
 board/freescale/common/ns_access.c                 |  30 ++++++
 board/freescale/ls1021aqds/ls1021aqds.c            | 113 ++++++++++++++++++++
 board/freescale/ls1021atwr/ls1021atwr.c            | 112 +++++++++++++++++++
 include/configs/ls1021aqds.h                       |  10 ++
 include/configs/ls1021atwr.h                       |  10 ++
 include/configs/sun7i.h                            |   1 +
 14 files changed, 459 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h
 create mode 100644 arch/arm/include/asm/arch-ls102xa/ns_access.h
 create mode 100644 board/freescale/common/ls102xa_stream_id.c
 create mode 100644 board/freescale/common/ns_access.c
Signature · 1 line
2.1.0.27.g96db324
author
#1 lore raw

PATCH v3 1/5 ARM: HYP/non-sec: add the pen address BE mode support.

For some SoCs, the pen address register maybe in BE mode and the CPUs are in LE mode.

This patch adds BE mode support for smp pen address.

---
 arch/arm/cpu/armv7/nonsec_virt.S | 3 +++
 1 file changed, 3 insertions(+)
arch/arm/cpu/armv7/nonsec_virt.S +3 -0
index 745670e..1ab5d54 100644
@@ -191,6 +191,9 @@ ENTRY(smp_waitloop)
 	wfi
 	ldr	r1, =CONFIG_SMP_PEN_ADDR	@ load start address
 	ldr	r1, [r1]
+#ifdef CONFIG_PEN_ADDR_BIG_ENDIAN
+	rev	r1, r1
+#endif
 	cmp	r0, r1			@ make sure we dont execute this code
 	beq	smp_waitloop		@ again (due to a spurious wakeup)
 	mov	r0, r1
Signature · 1 line
2.1.0.27.g96db324
author
#3 lore raw

PATCH v3 2/5 ARM: HYP/non-sec: Fix the ARCH Timer frequency setting.

In reply to Xiubo Li

For some SoCs, the system clock frequency may not equal to the ARCH Timer's frequency.

This patch uses the CONFIG_TIMER_CLK_FREQ instead of CONFIG_SYS_CLK_FREQ, then the system clock macro and arch timer macor could be set separately and without interfering each other.

---
 arch/arm/cpu/armv7/nonsec_virt.S | 4 ++--
 include/configs/sun7i.h          | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)
arch/arm/cpu/armv7/nonsec_virt.S +2 -2
index 1ab5d54..30d81db 100644
@@ -169,11 +169,11 @@ ENTRY(_nonsec_init)
  * we do this here instead.
  * But first check if we have the generic timer.
  */
-#ifdef CONFIG_SYS_CLK_FREQ
+#ifdef CONFIG_TIMER_CLK_FREQ
 	mrc	p15, 0, r0, c0, c1, 1		@ read ID_PFR1
 	and	r0, r0, #CPUID_ARM_GENTIMER_MASK	@ mask arch timer bits
 	cmp	r0, #(1 << CPUID_ARM_GENTIMER_SHIFT)
-	ldreq	r1, =CONFIG_SYS_CLK_FREQ
+	ldreq	r1, =CONFIG_TIMER_CLK_FREQ
 	mcreq	p15, 0, r1, c14, c0, 0		@ write CNTFRQ
 #endif
 
include/configs/sun7i.h +1 -0
index a902b84..6e201f2f 100644
@@ -35,6 +35,7 @@
 #define CONFIG_ARMV7_PSCI_NR_CPUS	2
 #define CONFIG_ARMV7_SECURE_BASE	SUNXI_SRAM_B_BASE
 #define CONFIG_SYS_CLK_FREQ		24000000
+#define CONFIG_SYS_TIMER_CLK_FREQ	CONFIG_SYS_CLK_FREQ
 
 /*
  * Include common sunxi configuration where most the settings are
Signature · 1 line
2.1.0.27.g96db324
#4 lore raw

PATCH v3 2/5 ARM: HYP/non-sec: Fix the ARCH Timer frequency setting.

Quoted 9 lines · Xiubo Li

For some SoCs, the system clock frequency may not equal to the ARCH Timer's frequency.

This patch uses the CONFIG_TIMER_CLK_FREQ instead of CONFIG_SYS_CLK_FREQ, then the system clock macro and arch timer macor could be set separately and without interfering each other.

---

author
#5 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

In reply to Xiubo Li

Enable hypervisors utilizing the ARMv7 virtualization extension on the LS1021A-QDS/TWR boards with the A7 core tile, we add the required configuration variable.
Also we define the board specific smp_set_cpu_boot_addr() function to set the start address for secondary cores in the LS1021A specific manner.

---
 arch/arm/cpu/armv7/ls102xa/cpu.c                  | 15 +++++++++++++++
 arch/arm/include/asm/arch-ls102xa/config.h        |  2 ++
 arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h |  3 +++
 include/configs/ls1021aqds.h                      |  7 +++++++
 include/configs/ls1021atwr.h                      |  7 +++++++
 5 files changed, 34 insertions(+)
arch/arm/cpu/armv7/ls102xa/cpu.c +15 -0
index b7dde45..69d1801 100644
@@ -101,3 +101,18 @@ int cpu_eth_init(bd_t *bis)
 
 	return 0;
 }
+
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}
+#endif
arch/arm/include/asm/arch-ls102xa/config.h +2 -0
index ed78c33..4856388 100644
@@ -11,6 +11,8 @@
 
 #define OCRAM_BASE_ADDR				0x10000000
 #define OCRAM_SIZE				0x00020000
+#define OCRAM_BASE_S_ADDR			0x10010000
+#define OCRAM_S_SIZE				0x00010000
 
 #define CONFIG_SYS_IMMR				0x01000000
 
arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h +3 -0
index 7995fe2..0bac353 100644
@@ -17,6 +17,9 @@
 #define SOC_VER_LS1021		0x11
 #define SOC_VER_LS1022		0x12
 
+#define CCSR_BRR_OFFSET		0xe4
+#define CCSR_SCRATCHRW1_OFFSET	0x200
+
 #define RCWSR0_SYS_PLL_RAT_SHIFT	25
 #define RCWSR0_SYS_PLL_RAT_MASK		0x1f
 #define RCWSR0_MEM_PLL_RAT_SHIFT	16
include/configs/ls1021aqds.h +7 -0
index 657e3b6..6976cfa 100644
@@ -324,6 +324,13 @@ unsigned long get_board_ddr_clk(void);
 #define CONFIG_CMDLINE_EDITING
 #define CONFIG_CMD_IMLS
 
+#define CONFIG_ARMV7_NONSEC
+#define CONFIG_ARMV7_VIRT
+#define CONFIG_PEN_ADDR_BIG_ENDIAN
+#define CONFIG_SMP_PEN_ADDR		0x01ee0200
+#define CONFIG_TIMER_CLK_FREQ		12500000
+#define CONFIG_ARMV7_SECURE_BASE	OCRAM_BASE_S_ADDR
+
 #define CONFIG_HWCONFIG
 #define HWCONFIG_BUFFER_SIZE		128
 
include/configs/ls1021atwr.h +7 -0
index 45b2272..655b39a 100644
@@ -227,6 +227,13 @@
 #define CONFIG_CMDLINE_EDITING
 #define CONFIG_CMD_IMLS
 
+#define CONFIG_ARMV7_NONSEC
+#define CONFIG_ARMV7_VIRT
+#define CONFIG_PEN_ADDR_BIG_ENDIAN
+#define CONFIG_SMP_PEN_ADDR		0x01ee0200
+#define CONFIG_TIMER_CLK_FREQ		12500000
+#define CONFIG_ARMV7_SECURE_BASE	OCRAM_BASE_S_ADDR
+
 #define CONFIG_HWCONFIG
 #define HWCONFIG_BUFFER_SIZE		128
 
Signature · 1 line
2.1.0.27.g96db324
#6 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Quoted 9 lines · Xiubo Li

Enable hypervisors utilizing the ARMv7 virtualization extension on the LS1021A-QDS/TWR boards with the A7 core tile, we add the required configuration variable.
Also we define the board specific smp_set_cpu_boot_addr() function to set the start address for secondary cores in the LS1021A specific manner.

---

#7 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

In reply to Xiubo Li

Hello Xiubo,

Quoted 6 lines · Xiubo Li

Enable hypervisors utilizing the ARMv7 virtualization extension on the LS1021A-QDS/TWR boards with the A7 core tile, we add the required configuration variable.
Also we define the board specific smp_set_cpu_boot_addr() function to set the start address for secondary cores in the LS1021A specific manner.

Seems like there are two different logical changes here:

- adding a secondary core boot address function;

- changing a few targets' configurations.

Please split this patch in two.

Quoted 21 lines
---
 arch/arm/cpu/armv7/ls102xa/cpu.c                  | 15 +++++++++++++++
 arch/arm/include/asm/arch-ls102xa/config.h        |  2 ++
 arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h |  3 +++
 include/configs/ls1021aqds.h                      |  7 +++++++
 include/configs/ls1021atwr.h                      |  7 +++++++
 5 files changed, 34 insertions(+)
arch/arm/cpu/armv7/ls102xa/cpu.c +4 -0
index b7dde45..69d1801 100644
@@ -101,3 +101,18 @@ int cpu_eth_init(bd_t *bis)
 
 	return 0;
 }
+
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)

arch/arm/cpu/armv7/ls102xa/cpu.c int cpu_eth_init(bd_t *bis) · line 117

Quoted 10 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +10 -0
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which secondary cores start from"; it sets *a* secondary core's boot address, and then it *boots* it.

Why does this version of smp_set_core_boot_addr() need to boot the core in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Quoted 66 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +1 -0
+#endif
arch/arm/include/asm/arch-ls102xa/config.h +2 -0
index ed78c33..4856388 100644
@@ -11,6 +11,8 @@
 
 #define OCRAM_BASE_ADDR				0x10000000
 #define OCRAM_SIZE				0x00020000
+#define OCRAM_BASE_S_ADDR			0x10010000
+#define OCRAM_S_SIZE				0x00010000
 
 #define CONFIG_SYS_IMMR				0x01000000
 
arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h +3 -0
index 7995fe2..0bac353 100644
@@ -17,6 +17,9 @@
 #define SOC_VER_LS1021		0x11
 #define SOC_VER_LS1022		0x12
 
+#define CCSR_BRR_OFFSET		0xe4
+#define CCSR_SCRATCHRW1_OFFSET	0x200
+
 #define RCWSR0_SYS_PLL_RAT_SHIFT	25
 #define RCWSR0_SYS_PLL_RAT_MASK		0x1f
 #define RCWSR0_MEM_PLL_RAT_SHIFT	16
include/configs/ls1021aqds.h +7 -0
index 657e3b6..6976cfa 100644
@@ -324,6 +324,13 @@ unsigned long get_board_ddr_clk(void);
 #define CONFIG_CMDLINE_EDITING
 #define CONFIG_CMD_IMLS
 
+#define CONFIG_ARMV7_NONSEC
+#define CONFIG_ARMV7_VIRT
+#define CONFIG_PEN_ADDR_BIG_ENDIAN
+#define CONFIG_SMP_PEN_ADDR		0x01ee0200
+#define CONFIG_TIMER_CLK_FREQ		12500000
+#define CONFIG_ARMV7_SECURE_BASE	OCRAM_BASE_S_ADDR
+
 #define CONFIG_HWCONFIG
 #define HWCONFIG_BUFFER_SIZE		128
 
include/configs/ls1021atwr.h +7 -0
index 45b2272..655b39a 100644
@@ -227,6 +227,13 @@
 #define CONFIG_CMDLINE_EDITING
 #define CONFIG_CMD_IMLS
 
+#define CONFIG_ARMV7_NONSEC
+#define CONFIG_ARMV7_VIRT
+#define CONFIG_PEN_ADDR_BIG_ENDIAN
+#define CONFIG_SMP_PEN_ADDR		0x01ee0200
+#define CONFIG_TIMER_CLK_FREQ		12500000
+#define CONFIG_ARMV7_SECURE_BASE	OCRAM_BASE_S_ADDR
+
 #define CONFIG_HWCONFIG
 #define HWCONFIG_BUFFER_SIZE		128
 
Signature · 1 line
2.1.0.27.g96db324

Amicalement,

Signature · 1 line
Albert.
#8 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hi Albert,

Quoted 9 lines
Quoted 9 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +9 -0
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
Quoted 9 lines

arch/arm/cpu/armv7/ls102xa/cpu.c int cpu_eth_init(bd_t *bis) · line 117

Quoted 4 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +4 -0
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which secondary cores start from"; it sets *a* secondary core's boot address, and then it *boots* it.

Okay, I will fix it later.

Quoted 4 lines

Why does this version of smp_set_core_boot_addr() need to boot the core in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will be used, and then We also make sure that the secondary core must be in low power and deep sleep mode(using wfi). So I just release it here, to make sure that the wfi instruction will be executed as early as possible.

Thanks,

BRs,
Xiubo

Quoted 74 lines

Review

Quoted 2 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +1 -0
+#endif
(quoted diff) +0 -0

b/arch/arm/include/asm/arch-ls102xa/config.h

Review

Quoted 13 lines

index ed78c33..4856388 100644

arch/arm/include/asm/arch-ls102xa/config.h +2 -0
@@ -11,6 +11,8 @@
 
 #define OCRAM_BASE_ADDR				0x10000000
 #define OCRAM_SIZE				0x00020000
+#define OCRAM_BASE_S_ADDR			0x10010000
+#define OCRAM_S_SIZE				0x00010000
 
 #define CONFIG_SYS_IMMR				0x01000000
 
(quoted diff) +0 -0

b/arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h

Quoted 50 lines

index 7995fe2..0bac353 100644

arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h +3 -0
@@ -17,6 +17,9 @@
 #define SOC_VER_LS1021		0x11
 #define SOC_VER_LS1022		0x12
 
+#define CCSR_BRR_OFFSET		0xe4
+#define CCSR_SCRATCHRW1_OFFSET	0x200
+
 #define RCWSR0_SYS_PLL_RAT_SHIFT	25
 #define RCWSR0_SYS_PLL_RAT_MASK		0x1f
 #define RCWSR0_MEM_PLL_RAT_SHIFT	16
include/configs/ls1021aqds.h +7 -0
index 657e3b6..6976cfa 100644
@@ -324,6 +324,13 @@ unsigned long get_board_ddr_clk(void);
 #define CONFIG_CMDLINE_EDITING
 #define CONFIG_CMD_IMLS
 
+#define CONFIG_ARMV7_NONSEC
+#define CONFIG_ARMV7_VIRT
+#define CONFIG_PEN_ADDR_BIG_ENDIAN
+#define CONFIG_SMP_PEN_ADDR		0x01ee0200
+#define CONFIG_TIMER_CLK_FREQ		12500000
+#define CONFIG_ARMV7_SECURE_BASE	OCRAM_BASE_S_ADDR
+
 #define CONFIG_HWCONFIG
 #define HWCONFIG_BUFFER_SIZE		128
 
include/configs/ls1021atwr.h +7 -1
index 45b2272..655b39a 100644
@@ -227,6 +227,13 @@
 #define CONFIG_CMDLINE_EDITING
 #define CONFIG_CMD_IMLS
 
+#define CONFIG_ARMV7_NONSEC
+#define CONFIG_ARMV7_VIRT
+#define CONFIG_PEN_ADDR_BIG_ENDIAN
+#define CONFIG_SMP_PEN_ADDR		0x01ee0200
+#define CONFIG_TIMER_CLK_FREQ		12500000
+#define CONFIG_ARMV7_SECURE_BASE	OCRAM_BASE_S_ADDR
+
 #define CONFIG_HWCONFIG
 #define HWCONFIG_BUFFER_SIZE		128
 
--
Quoted 1 line

2.1.0.27.g96db324

Amicalement,
--
Albert.

#9 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 28 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 9 lines
Quoted 9 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +9 -0
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
Quoted 9 lines

arch/arm/cpu/armv7/ls102xa/cpu.c int cpu_eth_init(bd_t *bis) · line 117

Quoted 4 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +4 -0
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which secondary cores start from"; it sets *a* secondary core's boot address, and then it *boots* it.

Okay, I will fix it later.

Quoted 4 lines

Why does this version of smp_set_core_boot_addr() need to boot the core in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Quoted 6 lines

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will be used, and then We also make sure that the secondary core must be in low power and deep sleep mode(using wfi). So I just release it here, to make sure that the wfi instruction will be executed as early as possible.

Right after smp_set_core_boot_addr() is called, kick_all_cpus() isgoing to be called. Wouldn't that boot your CPUs just as well?

Quoted 4 lines

Thanks,

BRs,
Xiubo

Amicalement,

Signature · 1 line
Albert.
#10 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hi Albert,

Quoted 43 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Friday, November 14, 2014 7:45 PM
To: Xiubo Li-B47053
Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot at lists.denx.de
Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 23 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 9 lines
Quoted 9 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +9 -0
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
Quoted 9 lines

arch/arm/cpu/armv7/ls102xa/cpu.c int cpu_eth_init(bd_t *bis) · line 117

Quoted 4 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +4 -0
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which secondary cores start from"; it sets *a* secondary core's boot address, and then it *boots* it.

Okay, I will fix it later.

Quoted 8 lines
Quoted 4 lines

Why does this version of smp_set_core_boot_addr() need to boot the core in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will be used, and

then

Quoted 9 lines
Quoted 1 line

We also make sure that the secondary core must be in low power and deep

sleep

Quoted 1 line

mode(using wfi). So I just release it here, to make sure that the wfi

instruction

Quoted 1 line

will be executed as early as possible.

Right after smp_set_core_boot_addr() is called, kick_all_cpus() isgoing to be called. Wouldn't that boot your CPUs just as well?

Yes, it will.

But before that we must do the holdoff bit set operation as the SoC's requirement.

The BRR contains control bits for enabling boot for each core. On exiting HRESET or PORESET, the RCW BOOT_HO field optionally allows for logical core 0 to be released for booting or to remain in boot holdoff. All other cores remain in boot holdoff until their corresponding bit is set.

Maybe the comment is not very clear and a bit confusing.

Thanks,

BRs
Xiubo

Quoted 8 lines
Quoted 4 lines

Thanks,

BRs,
Xiubo

Amicalement,
--
Albert.

#11 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 58 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 43 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Friday, November 14, 2014 7:45 PM
To: Xiubo Li-B47053
Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot at lists.denx.de
Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 23 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 9 lines
Quoted 9 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +9 -0
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
Quoted 9 lines

arch/arm/cpu/armv7/ls102xa/cpu.c int cpu_eth_init(bd_t *bis) · line 117

Quoted 4 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +4 -0
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which secondary cores start from"; it sets *a* secondary core's boot address, and then it *boots* it.

Okay, I will fix it later.

Quoted 8 lines
Quoted 4 lines

Why does this version of smp_set_core_boot_addr() need to boot the core in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will be used, and

then

Quoted 9 lines
Quoted 1 line

We also make sure that the secondary core must be in low power and deep

sleep

Quoted 1 line

mode(using wfi). So I just release it here, to make sure that the wfi

instruction

Quoted 1 line

will be executed as early as possible.

Right after smp_set_core_boot_addr() is called, kick_all_cpus() isgoing to be called. Wouldn't that boot your CPUs just as well?

Yes, it will.

Quoted 8 lines

But before that we must do the holdoff bit set operation as the SoC's requirement.

The BRR contains control bits for enabling boot for each core. On exiting HRESET or PORESET, the RCW BOOT_HO field optionally allows for logical core 0 to be released for booting or to remain in boot holdoff. All other cores remain in boot holdoff until their corresponding bit is set.

Maybe the comment is not very clear and a bit confusing.

Before I'm lost entirely, do you mean that the comment:

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

arch/arm/cpu/armv7/ls102xa/cpu.c int cpu_eth_init(bd_t *bis) · line 114

Quoted 4 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +4 -0
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */

Is actually wrong, and the instructions that follow it do not actually boot the secondary core(s)?

Quoted 4 lines

Thanks,

BRs
Xiubo

Amicalement,

Signature · 1 line
Albert.
#12 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hi Albert,

Quoted 59 lines
Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
Quoted 18 lines

arch/arm/cpu/armv7/ls102xa/cpu.c int cpu_eth_init(bd_t *bis) · line 117

Quoted 13 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +13 -0
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which secondary cores start from"; it sets *a* secondary core's boot address, and then it *boots* it.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

Okay, I will fix it later.

Quoted 1 line

Why does this version of smp_set_core_boot_addr() need to boot the

core

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines

in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will be used,

and

Quoted 7 lines
Quoted 7 lines

then

Quoted 1 line

We also make sure that the secondary core must be in low power and deep

sleep

Quoted 1 line

mode(using wfi). So I just release it here, to make sure that the wfi

instruction

Quoted 1 line

will be executed as early as possible.

Quoted 7 lines
Quoted 3 lines

Right after smp_set_core_boot_addr() is called, kick_all_cpus() isgoing to be called. Wouldn't that boot your CPUs just as well?

Yes, it will.

But before that we must do the holdoff bit set operation as the SoC's

requirement.

Quoted 2 lines

The BRR contains control bits for enabling boot for each core. On exiting

HRESET or

Quoted 1 line

PORESET, the RCW BOOT_HO field optionally allows for logical core 0 to be

released

Quoted 1 line

for booting or to remain in boot holdoff. All other cores remain in boot

holdoff until their

Quoted 3 lines

corresponding bit is set.

Maybe the comment is not very clear and a bit confusing.

Before I'm lost entirely, do you mean that the comment:

Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

arch/arm/cpu/armv7/ls102xa/cpu.c int cpu_eth_init(bd_t *bis) · line 114

Quoted 4 lines
arch/arm/cpu/armv7/ls102xa/cpu.c +4 -0
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */

Is actually wrong, and the instructions that follow it do not actually boot the secondary core(s)?

The comment should be:
/*
* After setting the secondary core's start address,
* just release it from holdoff.
*/
From my tests, for most time the release instructions will boot the secondary core(s) without smp_kick_all_cpus(). One time has failed.

So I think the release can not make sure that it will boot the secondary core(s).

Thanks,

BRs
Xiubo

#13 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 79 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 59 lines
Quoted 19 lines
Quoted 19 lines
Quoted 19 lines
Quoted 18 lines

Review

Quoted 13 lines
(quoted diff) +13 -0
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which secondary cores start from"; it sets *a* secondary core's boot address, and then it *boots* it.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

Okay, I will fix it later.

Quoted 1 line

Why does this version of smp_set_core_boot_addr() need to boot the

core

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines

in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will be used,

and

Quoted 7 lines
Quoted 7 lines

then

Quoted 1 line

We also make sure that the secondary core must be in low power and deep

sleep

Quoted 1 line

mode(using wfi). So I just release it here, to make sure that the wfi

instruction

Quoted 1 line

will be executed as early as possible.

Quoted 7 lines
Quoted 3 lines

Right after smp_set_core_boot_addr() is called, kick_all_cpus() isgoing to be called. Wouldn't that boot your CPUs just as well?

Yes, it will.

But before that we must do the holdoff bit set operation as the SoC's

requirement.

Quoted 2 lines

The BRR contains control bits for enabling boot for each core. On exiting

HRESET or

Quoted 1 line

PORESET, the RCW BOOT_HO field optionally allows for logical core 0 to be

released

Quoted 1 line

for booting or to remain in boot holdoff. All other cores remain in boot

holdoff until their

Quoted 3 lines

corresponding bit is set.

Maybe the comment is not very clear and a bit confusing.

Before I'm lost entirely, do you mean that the comment:

Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */

Is actually wrong, and the instructions that follow it do not actually boot the secondary core(s)?

The comment should be:
/*
* After setting the secondary core's start address,
* just release it from holdoff.
*/
From my tests, for most time the release instructions will boot the secondary core(s) without smp_kick_all_cpus(). One time has failed.

Quoted 1 line

So I think the release can not make sure that it will boot the secondary core(s).

Thanks for clarifying.

If a holdoff release is the right way to boot a secondary core for you, then I think the right place to do it is not smp_set_core_boot_addr() but smp_kick_all_cpus(), of which you could make a strong version which would do the holdoff release instead of whatever the weak version does.

That is, unless you also need the weak version to run, in which case I /still/ think you should find a way for all of this to happen at the 'kick" stage (maybe with a .weakref in arch/arm/cpu/armv8/start.S, to give the weak smp_kick_all_cpus symbol another name that could be referred to by the strong version).

Quoted 4 lines

Thanks,

BRs
Xiubo

Amicalement,

Signature · 1 line
Albert.
#14 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hi Albert,

Quoted 101 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, November 18, 2014 3:18 PM
To: Xiubo Li-B47053
Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot at lists.denx.de
Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 8 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void

*)(CONFIG_SYS_FSL_GUTS_ADDR);

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which

secondary

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

cores start from"; it sets *a* secondary core's boot address, and

then

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines

it *boots* it.

Okay, I will fix it later.

Quoted 1 line

Why does this version of smp_set_core_boot_addr() need to boot the

Quoted 8 lines
Quoted 8 lines

core

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines

in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will be

used,

Quoted 3 lines
Quoted 3 lines

and

Quoted 2 lines
Quoted 2 lines

then

Quoted 1 line

We also make sure that the secondary core must be in low power and

deep

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

sleep

Quoted 1 line

mode(using wfi). So I just release it here, to make sure that the

wfi

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

instruction

Quoted 1 line

will be executed as early as possible.

Right after smp_set_core_boot_addr() is called, kick_all_cpus()

isgoing

Quoted 9 lines
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines

to be called. Wouldn't that boot your CPUs just as well?

Yes, it will.

But before that we must do the holdoff bit set operation as the SoC's

requirement.

Quoted 2 lines

The BRR contains control bits for enabling boot for each core. On

exiting

Quoted 2 lines
Quoted 2 lines

HRESET or

Quoted 1 line

PORESET, the RCW BOOT_HO field optionally allows for logical core 0 to

be

Quoted 18 lines
Quoted 9 lines

released

Quoted 1 line

for booting or to remain in boot holdoff. All other cores remain in boot

holdoff until their

Quoted 3 lines

corresponding bit is set.

Maybe the comment is not very clear and a bit confusing.

Before I'm lost entirely, do you mean that the comment:

Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */

Is actually wrong, and the instructions that follow it do not actually boot the secondary core(s)?

Quoted 6 lines

The comment should be:
/*
* After setting the secondary core's start address,
* just release it from holdoff.
*/
From my tests, for most time the release instructions will boot the

secondary

Quoted 3 lines

core(s) without smp_kick_all_cpus(). One time has failed.

So I think the release can not make sure that it will boot the secondary

Quoted 9 lines

core(s).

Thanks for clarifying.

If a holdoff release is the right way to boot a secondary core for you, then I think the right place to do it is not smp_set_core_boot_addr() but smp_kick_all_cpus(), of which you could make a strong version which would do the holdoff release instead of whatever the weak version does.

Yes, I do think a strong version will be okay.

In file arch/arm/cpu/armv7/ls102xa/cpu.c, add the strong version:

+/* Release the secondary core from holdoff state and boot it */ +void smp_kick_all_cpus(void)
+{
+ struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+ out_be32(&gur->brrl, 0x2);
+}
+
Is this okay ?

I have test the holdoff release in two boards(including the old one before I used) for 37 times and all has passed. I have a check the before failed logs, It is another issue led to the failure. And also get confirmation that the Holdoff release will do reset and then boot the secondary core.

Thanks,

BRs
Xiubo

Quoted 14 lines

That is, unless you also need the weak version to run, in which case I /still/ think you should find a way for all of this to happen at the 'kick" stage (maybe with a .weakref in arch/arm/cpu/armv8/start.S, to give the weak smp_kick_all_cpus symbol another name that could be referred to by the strong version).

Quoted 4 lines

Thanks,

BRs
Xiubo

Amicalement,
--
Albert.

#15 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 125 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 101 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, November 18, 2014 3:18 PM
To: Xiubo Li-B47053
Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot at lists.denx.de
Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 8 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
+/* Setting the address at which secondary cores start from.*/
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void

*)(CONFIG_SYS_FSL_GUTS_ADDR);

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which

secondary

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

cores start from"; it sets *a* secondary core's boot address, and

then

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines

it *boots* it.

Okay, I will fix it later.

Quoted 1 line

Why does this version of smp_set_core_boot_addr() need to boot the

Quoted 8 lines
Quoted 8 lines

core

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines

in addition to setting the address, whereas the existing ones in virt_v7, vexpress_common and arndale don't boot the cores?

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will be

used,

Quoted 3 lines
Quoted 3 lines

and

Quoted 2 lines
Quoted 2 lines

then

Quoted 1 line

We also make sure that the secondary core must be in low power and

deep

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

sleep

Quoted 1 line

mode(using wfi). So I just release it here, to make sure that the

wfi

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

instruction

Quoted 1 line

will be executed as early as possible.

Right after smp_set_core_boot_addr() is called, kick_all_cpus()

isgoing

Quoted 9 lines
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines

to be called. Wouldn't that boot your CPUs just as well?

Yes, it will.

But before that we must do the holdoff bit set operation as the SoC's

requirement.

Quoted 2 lines

The BRR contains control bits for enabling boot for each core. On

exiting

Quoted 2 lines
Quoted 2 lines

HRESET or

Quoted 1 line

PORESET, the RCW BOOT_HO field optionally allows for logical core 0 to

be

Quoted 18 lines
Quoted 9 lines

released

Quoted 1 line

for booting or to remain in boot holdoff. All other cores remain in boot

holdoff until their

Quoted 3 lines

corresponding bit is set.

Maybe the comment is not very clear and a bit confusing.

Before I'm lost entirely, do you mean that the comment:

Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */

Is actually wrong, and the instructions that follow it do not actually boot the secondary core(s)?

Quoted 6 lines

The comment should be:
/*
* After setting the secondary core's start address,
* just release it from holdoff.
*/
From my tests, for most time the release instructions will boot the

secondary

Quoted 3 lines

core(s) without smp_kick_all_cpus(). One time has failed.

So I think the release can not make sure that it will boot the secondary

Quoted 9 lines

core(s).

Thanks for clarifying.

If a holdoff release is the right way to boot a secondary core for you, then I think the right place to do it is not smp_set_core_boot_addr() but smp_kick_all_cpus(), of which you could make a strong version which would do the holdoff release instead of whatever the weak version does.

Yes, I do think a strong version will be okay.

In file arch/arm/cpu/armv7/ls102xa/cpu.c, add the strong version:

(quoted diff) +8 -0
+/* Release the secondary core from holdoff state and boot it */
+void smp_kick_all_cpus(void)
+{
+       struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+       out_be32(&gur->brrl, 0x2);
+}
+
Quoted 1 line

Is this okay ?

Yes, thanks!

Quoted 4 lines

I have test the holdoff release in two boards(including the old one before I used) for 37 times and all has passed. I have a check the before failed logs, It is another issue led to the failure. And also get confirmation that the Holdoff release will do reset and then boot the secondary core.

Good -- this makes smp_kick_all_cpus() the right home for holdoff releast.

Quoted 1 line

Thanks,

Thank you for your patience. :)

Quoted 2 lines

BRs
Xiubo

Amicalement,

Signature · 1 line
Albert.
#16 lore raw

PATCH v3 3/5 ls102xa: HYP/non-sec: support for ls102xa boards

Hi Albert,

If there hasn't any other problem, I will send out the V4 series.

Thanks very much,

BRs
Xiubo

Quoted 171 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Thursday, November 20, 2014 8:07 PM
To: Xiubo Li-B47053
Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot at lists.denx.de
Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards

Hello Li.Xiubo at freescale.com,

Quoted 8 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 6 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Tuesday, November 18, 2014 3:18 PM
To: Xiubo Li-B47053
Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot at lists.denx.de
Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa

boards

Quoted 9 lines
Quoted 9 lines

Hello Li.Xiubo at freescale.com,

Quoted 4 lines · Li.Xiubo at freescale.com

Hi Albert,

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+#if defined(CONFIG_ARMV7_NONSEC) ||

defined(CONFIG_ARMV7_VIRT)

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

+/* Setting the address at which secondary cores start

from.*/

Quoted 14 lines
Quoted 14 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

Review

Quoted 3 lines
(quoted diff) +3 -0
+void smp_set_core_boot_addr(unsigned long addr, int corenr)
+{
+	struct ccsr_gur __iomem *gur = (void

*)(CONFIG_SYS_FSL_GUTS_ADDR);

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines

Review

Quoted 8 lines
(quoted diff) +8 -0
+
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */
+	out_be32(&gur->scratchrw[0], addr);
+	out_be32(&gur->brrl, 0x2);
+}

This function does not exactly "[set] the address at which

Quoted 2 lines
Quoted 2 lines

secondary

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

cores start from"; it sets *a* secondary core's boot address,

and

Quoted 7 lines
Quoted 7 lines

then

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines

it *boots* it.

Okay, I will fix it later.

Quoted 1 line

Why does this version of smp_set_core_boot_addr() need to boot

the

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

core

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

in addition to setting the address, whereas the existing ones

in

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines

virt_v7, vexpress_common and arndale don't boot the cores?

Yes, they don't doing the release operation.

For Low Power Management requirement, maybe only one core will

be

Quoted 4 lines
Quoted 4 lines

used,

Quoted 3 lines
Quoted 3 lines

and

Quoted 2 lines
Quoted 2 lines

then

Quoted 1 line

We also make sure that the secondary core must be in low power

and

Quoted 3 lines
Quoted 3 lines

deep

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

sleep

Quoted 1 line

mode(using wfi). So I just release it here, to make sure that

the

Quoted 4 lines
Quoted 4 lines

wfi

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

instruction

Quoted 1 line

will be executed as early as possible.

Quoted 8 lines
Quoted 8 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Right after smp_set_core_boot_addr() is called, kick_all_cpus()

isgoing

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 2 lines

to be called. Wouldn't that boot your CPUs just as well?

Yes, it will.

But before that we must do the holdoff bit set operation as the

SoC's

Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

requirement.

Quoted 2 lines

The BRR contains control bits for enabling boot for each core. On

exiting

Quoted 2 lines
Quoted 2 lines

HRESET or

Quoted 1 line

PORESET, the RCW BOOT_HO field optionally allows for logical core 0

to

Quoted 3 lines
Quoted 3 lines

be

Quoted 2 lines
Quoted 2 lines

released

Quoted 1 line

for booting or to remain in boot holdoff. All other cores remain in

boot

Quoted 48 lines
Quoted 26 lines
Quoted 16 lines
Quoted 7 lines

holdoff until their

Quoted 3 lines

corresponding bit is set.

Maybe the comment is not very clear and a bit confusing.

Before I'm lost entirely, do you mean that the comment:

Quoted 8 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+	/*
+	 * After setting the secondary cores start address,
+	 * just release them to boot.
+	 */

Is actually wrong, and the instructions that follow it do not actually boot the secondary core(s)?

Quoted 6 lines

The comment should be:
/*
* After setting the secondary core's start address,
* just release it from holdoff.
*/
From my tests, for most time the release instructions will boot the

secondary

Quoted 3 lines

core(s) without smp_kick_all_cpus(). One time has failed.

So I think the release can not make sure that it will boot the secondary

Quoted 9 lines

core(s).

Thanks for clarifying.

If a holdoff release is the right way to boot a secondary core for you, then I think the right place to do it is not smp_set_core_boot_addr() but smp_kick_all_cpus(), of which you could make a strong version which would do the holdoff release instead of whatever the weak version does.

Yes, I do think a strong version will be okay.

In file arch/arm/cpu/armv7/ls102xa/cpu.c, add the strong version:

(quoted diff) +8 -0
+/* Release the secondary core from holdoff state and boot it */
+void smp_kick_all_cpus(void)
+{
+       struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
+
+       out_be32(&gur->brrl, 0x2);
+}
+
Quoted 1 line

Is this okay ?

Yes, thanks!

Quoted 2 lines

I have test the holdoff release in two boards(including the old one before I used) for 37 times and all has passed. I have a check the before failed

logs,

Quoted 2 lines

It is another issue led to the failure. And also get confirmation that the Holdoff release will do reset and then boot the secondary core.

Good -- this makes smp_kick_all_cpus() the right home for holdoff releast.

Quoted 1 line

Thanks,

Thank you for your patience. :)

Quoted 2 lines

BRs
Xiubo

Amicalement,
--
Albert.

author
#17 lore raw

PATCH v3 4/5 ARM: ls102xa: allow all the peripheral access permissions as R/W.

In reply to Xiubo Li

The Central Security Unit (CSU) allows secure world software to change the default access control policies of peripherals/bus slaves, determining which bus masters may access them. This allows peripherals to be separated into distinct security domains. Combined with SMMU configuration of the system masters privileges, these features provide protection against indirect unauthorized access to data.

For now we configure all the peripheral access permissions as R/W.

---
 arch/arm/include/asm/arch-ls102xa/config.h    |   1 +
 arch/arm/include/asm/arch-ls102xa/ns_access.h | 118 ++++++++++++++++++++++++++
 board/freescale/common/Makefile               |   2 +
 board/freescale/common/ns_access.c            |  30 +++++++
 board/freescale/ls1021aqds/ls1021aqds.c       |  92 ++++++++++++++++++++
 board/freescale/ls1021atwr/ls1021atwr.c       |  91 ++++++++++++++++++++
 include/configs/ls1021aqds.h                  |   1 +
 include/configs/ls1021atwr.h                  |   1 +
 8 files changed, 336 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-ls102xa/ns_access.h
 create mode 100644 board/freescale/common/ns_access.c
arch/arm/include/asm/arch-ls102xa/config.h +1 -0
index 4856388..0754296 100644
@@ -18,6 +18,7 @@
 
 #define CONFIG_SYS_FSL_DDR_ADDR			(CONFIG_SYS_IMMR + 0x00080000)
 #define CONFIG_SYS_CCI400_ADDR			(CONFIG_SYS_IMMR + 0x00180000)
+#define CONFIG_SYS_FSL_CSU_ADDR                 (CONFIG_SYS_IMMR + 0x00510000)
 #define CONFIG_SYS_IFC_ADDR			(CONFIG_SYS_IMMR + 0x00530000)
 #define CONFIG_SYS_FSL_ESDHC_ADDR		(CONFIG_SYS_IMMR + 0x00560000)
 #define CONFIG_SYS_FSL_SCFG_ADDR		(CONFIG_SYS_IMMR + 0x00570000)
arch/arm/include/asm/arch-ls102xa/ns_access.h new file +118 -0

118 lines not on this page · raw message

board/freescale/common/Makefile +2 -0
index 32b5a3b..e5aad2d 100644
@@ -60,4 +60,6 @@ obj-$(CONFIG_P3041DS)	+= p_corenet/
 obj-$(CONFIG_P4080DS)	+= p_corenet/
 obj-$(CONFIG_P5020DS)	+= p_corenet/
 obj-$(CONFIG_P5040DS)	+= p_corenet/
+
+obj-$(CONFIG_LS102XA_NS_ACESSC)	+= ns_access.o
 endif
board/freescale/common/ns_access.c new file +30 -0
new file mode 100644
index 0000000..d7de982
@@ -0,0 +1,30 @@
+/*
+ * Copyright 2014 Freescale Semiconductor
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/ns_access.h>
+
+void enable_devices_ns_access(struct csu_ns_dev *ns_dev, uint32_t num)
+{
+	u32 *base = (u32 *)CONFIG_SYS_FSL_CSU_ADDR;
+	u32 *reg;
+	uint32_t val;
+	int i;
+
+	for (i = 0; i < num; i++) {
+		reg = base + ns_dev[i].ind / 2;
+		val = in_be32(reg);
+		if (ns_dev[i].ind % 2 == 0) {
+			val &= 0x0000ffff;
+			val |= ns_dev[i].val << 16;
+		} else {
+			val &= 0xffff0000;
+			val |= ns_dev[i].val;
+		}
+		out_be32(reg, val);
+	}
+}
board/freescale/ls1021aqds/ls1021aqds.c +92 -0

110 lines not on this page · raw message

board/freescale/ls1021atwr/ls1021atwr.c +91 -0

109 lines not on this page · raw message

include/configs/ls1021aqds.h +1 -0
index 6976cfa..4067be9 100644
@@ -327,6 +327,7 @@ unsigned long get_board_ddr_clk(void);
 #define CONFIG_ARMV7_NONSEC
 #define CONFIG_ARMV7_VIRT
 #define CONFIG_PEN_ADDR_BIG_ENDIAN
+#define CONFIG_LS102XA_NS_ACCESS
 #define CONFIG_SMP_PEN_ADDR		0x01ee0200
 #define CONFIG_TIMER_CLK_FREQ		12500000
 #define CONFIG_ARMV7_SECURE_BASE	OCRAM_BASE_S_ADDR
include/configs/ls1021atwr.h +1 -0
index 655b39a..2a40d6a 100644
@@ -230,6 +230,7 @@
 #define CONFIG_ARMV7_NONSEC
 #define CONFIG_ARMV7_VIRT
 #define CONFIG_PEN_ADDR_BIG_ENDIAN
+#define CONFIG_LS102XA_NS_ACCESS
 #define CONFIG_SMP_PEN_ADDR		0x01ee0200
 #define CONFIG_TIMER_CLK_FREQ		12500000
 #define CONFIG_ARMV7_SECURE_BASE	OCRAM_BASE_S_ADDR
Signature · 1 line
2.1.0.27.g96db324
#18 lore raw

PATCH v3 4/5 ARM: ls102xa: allow all the peripheral access permissions as R/W.

Quoted 8 lines · Xiubo Li

The Central Security Unit (CSU) allows secure world software to change the default access control policies of peripherals/bus slaves, determining which bus masters may access them. This allows peripherals to be separated into distinct security domains. Combined with SMMU configuration of the system masters privileges, these features provide protection against indirect unauthorized access to data.

Quoted 4 lines

For now we configure all the peripheral access permissions as R/W.

---

author
#19 lore raw

PATCH v3 5/5 ARM: ls102xa: Setting device's stream id for SMMUs.

In reply to Xiubo Li

LS1 has 4 SMMUs for address translation of the masters. All the SMMUs' stream IDs are 8-bit. The address translation depends on the stream ID of the incoming transaction.
Each master has unique stream ID assigned to it and is configurable through SCFG registers. The stream ID for the masters is identical and share the same register field of STREAM ID registers.

---
 .../include/asm/arch-ls102xa/ls102xa_stream_id.h    | 17 +++++++++++++++++
 board/freescale/common/Makefile                     |  2 ++
 board/freescale/common/ls102xa_stream_id.c          | 18 ++++++++++++++++++
 board/freescale/ls1021aqds/ls1021aqds.c             | 21 +++++++++++++++++++++
 board/freescale/ls1021atwr/ls1021atwr.c             | 21 +++++++++++++++++++++
 include/configs/ls1021aqds.h                        |  2 ++
 include/configs/ls1021atwr.h                        |  2 ++
 7 files changed, 83 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h
 create mode 100644 board/freescale/common/ls102xa_stream_id.c
arch/arm/include/asm/arch-ls102xa/ls102xa_stream_id.h new file +17 -0
new file mode 100644
index 0000000..abd70fc
@@ -0,0 +1,17 @@
+/*
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __FSL_LS102XA_STREAM_ID_H_
+#define __FSL_LS102XA_STREAM_ID_H_
+
+struct smmu_stream_id {
+	uint16_t offset;
+	uint16_t stream_id;
+	char dev_name[32];
+};
+
+void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id, uint32_t num);
+#endif
board/freescale/common/Makefile +2 -0
index e5aad2d..2cf3963 100644
@@ -54,6 +54,8 @@ obj-$(CONFIG_VSC_CROSSBAR)    += vsc3316_3308.o
 obj-$(CONFIG_IDT8T49N222A)	+= idt8t49n222a_serdes_clk.o
 obj-$(CONFIG_ZM7300)		+= zm7300.o
 
+obj-$(CONFIG_LS102XA_STREAM_ID)	+= ls102xa_stream_id.o
+
 # deal with common files for P-series corenet based devices
 obj-$(CONFIG_P2041RDB)	+= p_corenet/
 obj-$(CONFIG_P3041DS)	+= p_corenet/
board/freescale/common/ls102xa_stream_id.c new file +18 -0
new file mode 100644
index 0000000..6154c9c
@@ -0,0 +1,18 @@
+/*
+ * Copyright 2014 Freescale Semiconductor
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm/arch/ls102xa_stream_id.h>
+
+void ls102xa_config_smmu_stream_id(struct smmu_stream_id *id, uint32_t num)
+{
+	uint32_t *scfg = (uint32_t *)CONFIG_SYS_FSL_SCFG_ADDR;
+	int i;
+
+	for (i = 0; i < num; i++)
+		out_be32(scfg + id[i].offset, id[i].stream_id);
+}
board/freescale/ls1021aqds/ls1021aqds.c +21 -0
index 07df7d2..dba0190 100644
@@ -11,6 +11,7 @@
 #include <asm/arch/ns_access.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/fsl_serdes.h>
+#include <asm/arch/ls102xa_stream_id.h>
 #include <mmc.h>
 #include <fsl_esdhc.h>
 #include <fsl_ifc.h>
@@ -300,6 +301,23 @@ static struct csu_ns_dev ns_dev[] = {
 };
 #endif
 
+struct smmu_stream_id dev_stream_id[] = {
+	{ 0x100, 0x01, "ETSEC MAC1" },
+	{ 0x104, 0x02, "ETSEC MAC2" },
+	{ 0x108, 0x03, "ETSEC MAC3" },
+	{ 0x10c, 0x04, "PEX1" },
+	{ 0x110, 0x05, "PEX2" },
+	{ 0x114, 0x06, "qDMA" },
+	{ 0x118, 0x07, "SATA" },
+	{ 0x11c, 0x08, "USB3" },
+	{ 0x120, 0x09, "QE" },
+	{ 0x124, 0x0a, "eSDHC" },
+	{ 0x128, 0x0b, "eMA" },
+	{ 0x14c, 0x0c, "2D-ACE" },
+	{ 0x150, 0x0d, "USB2" },
+	{ 0x18c, 0x0e, "DEBUG" },
+};
+
 int board_init(void)
 {
 	struct ccsr_cci400 *cci = (struct ccsr_cci400 *)CONFIG_SYS_CCI400_ADDR;
@@ -315,6 +333,9 @@ int board_init(void)
 	config_serdes_mux();
 #endif
 
+	ls102xa_config_smmu_stream_id(dev_stream_id,
+				      ARRAY_SIZE(dev_stream_id));
+
 #ifdef CONFIG_SYS_FSL_CSU_ADDR
 	enable_devices_ns_access(ns_dev, ARRAY_SIZE(ns_dev));
 #endif
board/freescale/ls1021atwr/ls1021atwr.c +21 -0
index c4d3600..a1978e8 100644
@@ -11,6 +11,7 @@
 #include <asm/arch/ns_access.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/fsl_serdes.h>
+#include <asm/arch/ls102xa_stream_id.h>
 #include <mmc.h>
 #include <fsl_esdhc.h>
 #include <fsl_ifc.h>
@@ -357,6 +358,23 @@ static struct csu_ns_dev ns_dev[] = {
 };
 #endif
 
+struct smmu_stream_id dev_stream_id[] = {
+	{ 0x100, 0x01, "ETSEC MAC1" },
+	{ 0x104, 0x02, "ETSEC MAC2" },
+	{ 0x108, 0x03, "ETSEC MAC3" },
+	{ 0x10c, 0x04, "PEX1" },
+	{ 0x110, 0x05, "PEX2" },
+	{ 0x114, 0x06, "qDMA" },
+	{ 0x118, 0x07, "SATA" },
+	{ 0x11c, 0x08, "USB3" },
+	{ 0x120, 0x09, "QE" },
+	{ 0x124, 0x0a, "eSDHC" },
+	{ 0x128, 0x0b, "eMA" },
+	{ 0x14c, 0x0c, "2D-ACE" },
+	{ 0x150, 0x0d, "USB2" },
+	{ 0x18c, 0x0e, "DEBUG" },
+};
+
 int board_init(void)
 {
 #ifndef CONFIG_SYS_FSL_NO_SERDES
@@ -364,6 +382,9 @@ int board_init(void)
 	config_serdes_mux();
 #endif
 
+	ls102xa_config_smmu_stream_id(dev_stream_id,
+				      ARRAY_SIZE(dev_stream_id));
+
 #ifdef CONFIG_SYS_FSL_CSU_ADDR
 	enable_devices_ns_access(ns_dev, ARRAY_SIZE(ns_dev));
 #endif
include/configs/ls1021aqds.h +2 -0
index 4067be9..480e129 100644
@@ -367,6 +367,8 @@ unsigned long get_board_ddr_clk(void);
 #define CONFIG_SYS_LOAD_ADDR		0x82000000
 #define CONFIG_SYS_HZ			1000
 
+#define CONFIG_LS102XA_STREAM_ID
+
 /*
  * Stack sizes
  * The stack sizes are set up in start.S using the settings below
include/configs/ls1021atwr.h +2 -0
index 2a40d6a..7f480bd 100644
@@ -269,6 +269,8 @@
 #define CONFIG_SYS_LOAD_ADDR		0x82000000
 #define CONFIG_SYS_HZ			1000
 
+#define CONFIG_LS102XA_STREAM_ID
+
 /*
  * Stack sizes
  * The stack sizes are set up in start.S using the settings below
Signature · 1 line
2.1.0.27.g96db324
#20 lore raw

PATCH v3 5/5 ARM: ls102xa: Setting device's stream id for SMMUs.

Quoted 9 lines · Xiubo Li

LS1 has 4 SMMUs for address translation of the masters. All the SMMUs' stream IDs are 8-bit. The address translation depends on the stream ID of the incoming transaction.
Each master has unique stream ID assigned to it and is configurable through SCFG registers. The stream ID for the masters is identical and share the same register field of STREAM ID registers.

---

#21 lore raw

v3 cover ls102xa: HYP/non-sec: for ls102xa.

In reply to Xiubo Li

Hello York,

Quoted 8 lines · Xiubo Li

Change for V3:
- Fix the language in commit message.

Change for V2:
- All the registers are defined as a struct, here use it.
- Use CONFIG_PEN_ADDR_BIG_ENDIAN instead of CONFIG_SOC_BIG_ENDIAN.

Quoted 6 lines

Xiubo Li (5):
ARM: HYP/non-sec: add the pen address BE mode support.
ARM: HYP/non-sec: Fix the ARCH Timer frequency setting.
ls102xa: HYP/non-sec: support for ls102xa boards
ARM: ls102xa: allow all the peripheral access permissions as R/W.
ARM: ls102xa: Setting device's stream id for SMMUs.

York,

Delegation of this patch is split among us. Again, one of us could give his acked-by and the other could apply the whole patch in a row. I'm ok with applying. :)

Amicalement,

Signature · 1 line
Albert.