Deep sleep patches for Freescale QorIQ platforms

PATCH New Yuantian.Tang at freescale.com 3 participants 16 messages Oct 9, 2014 – Nov 25, 2014 Arm (32-bit)

lore

Share & tools

lore

Yuantian.Tang at freescale.com

author

2014-10-09 08:11 UTC #0 lore raw

cover Deep sleep patches for Freescale QorIQ platforms

From: Tang Yuantian <[email protected]>

These patches depend on the following patches:
https://patchwork.ozlabs.org/patch/389949/
https://patchwork.ozlabs.org/patch/389950/
https://patchwork.ozlabs.org/patch/389951/
https://patchwork.ozlabs.org/patch/389952/

Tang Yuantian (4):
Add deep sleep framework support for Freescale QorIQ platforms
ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation
arm: ls102xa: Fixed a register definition error
arm: ls1021qds: Add deep sleep support

 arch/arm/cpu/armv7/virt-v7.c                      | 14 +++---
 arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h |  2 +-
 board/freescale/ls1021aqds/ddr.c                  |  7 +++
 board/freescale/ls1021aqds/ls1021aqds.c           | 60 +++++++++++++++++++++++
 common/board_f.c                                  | 10 ++++
 drivers/ddr/fsl/arm_ddr_gen3.c                    | 48 ++++++++++++++++--
 include/configs/ls1021aqds.h                      |  4 ++
 include/fsl_ddr_sdram.h                           |  2 +
 include/fsl_sleep.h                               | 32 ++++++++++++
 9 files changed, 167 insertions(+), 12 deletions(-)
 create mode 100644 include/fsl_sleep.h

Signature · 1 line

2.1.0.27.g96db324

Yuantian.Tang at freescale.com

author

2014-10-09 08:11 UTC #1 lore raw

PATCH 1/4 Add deep sleep framework support for Freescale QorIQ platforms

From: Tang Yuantian <[email protected]>

When Freescale QorIQ SoCs wake up from deep sleep, control is passed to the primary core that starts executing uboot. After re-initialized some IP blocks, like DDRC, kernel will take responsibility to continue to restore environment it leaves before.

This patch adds the deep sleep framework support for all Freescale QorIQ platforms that use generic_board configuation.

Signed-off-by Tang Yuantian <[email protected]>

---
 common/board_f.c               | 10 +++++++++
 drivers/ddr/fsl/arm_ddr_gen3.c | 48 +++++++++++++++++++++++++++++++++++++-----
 include/fsl_ddr_sdram.h        |  2 ++
 include/fsl_sleep.h            | 32 ++++++++++++++++++++++++++++
 4 files changed, 87 insertions(+), 5 deletions(-)
 create mode 100644 include/fsl_sleep.h

common/board_f.c +10 -0

index e6aa298..b736d29 100644
@@ -56,6 +56,9 @@
 #endif
 #include <dm/root.h>
 #include <linux/compiler.h>
+#ifdef CONFIG_FSL_DEEP_SLEEP
+#include <fsl_sleep.h>
+#endif
 
 /*
  * Pointer to initial global data area

@@ -921,6 +924,9 @@ static init_fnc_t init_sequence_f[] = {
 #if defined(CONFIG_MIPS) || defined(CONFIG_PPC)
 	init_func_ram,
 #endif
+#ifdef CONFIG_FSL_DEEP_SLEEP
+	fsl_dp_resume,
+#endif
 #ifdef CONFIG_POST
 	post_init_f,
 #endif

@@ -1027,6 +1033,10 @@ void board_init_f(ulong boot_flags)
 	gd->flags = boot_flags;
 	gd->have_console = 0;
 
+#ifdef CONFIG_FSL_DEEP_SLEEP
+	if (is_warm_boot())
+		gd->flags |= GD_FLG_SILENT | GD_FLG_DISABLE_CONSOLE;
+#endif
 	if (initcall_run_list(init_sequence_f))
 		hang();

drivers/ddr/fsl/arm_ddr_gen3.c +43 -5

83 lines not on this page · raw message

include/fsl_ddr_sdram.h +2 -0

index 5b03c14..b69b1dc 100644
@@ -114,6 +114,7 @@ typedef struct ddr4_spd_eeprom_s generic_spd_eeprom_t;
 #define SDRAM_CFG_2T_EN			0x00008000
 #define SDRAM_CFG_BI			0x00000001
 
+#define SDRAM_CFG2_FRC_SR		0x80000000
 #define SDRAM_CFG2_D_INIT		0x00000010
 #define SDRAM_CFG2_ODT_CFG_MASK		0x00600000
 #define SDRAM_CFG2_ODT_NEVER		0

@@ -163,6 +164,7 @@ typedef struct ddr4_spd_eeprom_s generic_spd_eeprom_t;
 #define DDR_CDR1_ODT(x) ((x & DDR_CDR1_ODT_MASK) << DDR_CDR1_ODT_SHIFT)
 #define DDR_CDR2_ODT(x) (x & DDR_CDR2_ODT_MASK)
 #define DDR_CDR2_VREF_OVRD(x)	(0x00008080 | ((((x) - 37) & 0x3F) << 8))
+#define DDR_CDR2_VREF_TRAIN_EN	0x00000080
 
 #if (defined(CONFIG_SYS_FSL_DDR_VER) && \
 	(CONFIG_SYS_FSL_DDR_VER >= FSL_DDR_VER_4_7))

include/fsl_sleep.h new file +32 -0

new file mode 100644
index 0000000..429e302
@@ -0,0 +1,32 @@
+/*
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef __DEEP_SLEEP_H
+#define __DEEP_SLEEP_H
+
+/*
+ * Functions prototype for sleep.
+ * Each platform that supports deep sleep needs to
+ * implement them on their own.
+ */
+
+/* determine if it is a wakeup from deep sleep */
+bool is_warm_boot(void);
+
+/* board specific memory setup, like CKE isolation */
+void fsl_dp_mem_setup(void);
+
+/* clean up everything and jump to kernel */
+int fsl_dp_resume(void);
+
+/*
+ * When wakeup from deep sleep, the first 128 bytes space
+ * will be used to do DDR training which corrupts the data
+ * in there. This function will restore them.
+ */
+void fsl_dp_ddr_restore(void);
+
+#endif

Signature · 1 line

2.1.0.27.g96db324

Yuantian.Tang at freescale.com

author

2014-10-09 08:11 UTC #2 lore raw

PATCH 2/4 ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

In reply to Yuantian.Tang at freescale.com

From: Tang Yuantian <[email protected]>

Defining variable gic_dist_addr as a globe one prevents function armv7_init_nonsec() from being used before relocation which is the case in the deep sleep resume process on Freescale QorIQ SoC platforms.
This patch removes this limitation by adding a extra same meaning local variable. In this way, no exsiting codes get corrupts.

Signed-off-by Tang Yuantian <[email protected]>

---
 arch/arm/cpu/armv7/virt-v7.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

arch/arm/cpu/armv7/virt-v7.c +8 -6

index 651ca40..e1dfce9 100644
@@ -75,6 +75,7 @@ int armv7_init_nonsec(void)
 {
 	unsigned int reg;
 	unsigned itlinesnr, i;
+	unsigned long gic_base_addr;
 
 	/* check whether the CPU supports the security extensions */
 	reg = read_id_pfr1();

@@ -89,23 +90,24 @@ int armv7_init_nonsec(void)
 	 * any access to it will trap.
 	 */
 
-	gic_dist_addr = get_gicd_base_address();
-	if (gic_dist_addr == -1)
+	gic_base_addr = get_gicd_base_address();
+	gic_dist_addr = gic_base_addr;
+	if (gic_base_addr == -1)
 		return -1;
 
 	/* enable the GIC distributor */
-	writel(readl(gic_dist_addr + GICD_CTLR) | 0x03,
-	       gic_dist_addr + GICD_CTLR);
+	writel(readl(gic_base_addr + GICD_CTLR) | 0x03,
+	       gic_base_addr + GICD_CTLR);
 
 	/* TYPER[4:0] contains an encoded number of available interrupts */
-	itlinesnr = readl(gic_dist_addr + GICD_TYPER) & 0x1f;
+	itlinesnr = readl(gic_base_addr + GICD_TYPER) & 0x1f;
 
 	/* set all bits in the GIC group registers to one to allow access
 	 * from non-secure state. The first 32 interrupts are private per
 	 * CPU and will be set later when enabling the GIC for each core
 	 */
 	for (i = 1; i <= itlinesnr; i++)
-		writel((unsigned)-1, gic_dist_addr + GICD_IGROUPRn + 4 * i);
+		writel((unsigned)-1, gic_base_addr + GICD_IGROUPRn + 4 * i);
 
 #ifndef CONFIG_ARMV7_PSCI
 	smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen), -1);

Signature · 1 line

2.1.0.27.g96db324

York Sun

2014-10-15 15:44 UTC #3 lore raw

PATCH 2/4 ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

Quoted 48 lines · Yuantian.Tang at freescale.com

From: Tang Yuantian <[email protected]>

Signed-off-by Tang Yuantian <[email protected]>

---
 arch/arm/cpu/armv7/virt-v7.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

arch/arm/cpu/armv7/virt-v7.c +7 -5

index 651ca40..e1dfce9 100644
@@ -75,6 +75,7 @@ int armv7_init_nonsec(void)
 {
 	unsigned int reg;
 	unsigned itlinesnr, i;
+	unsigned long gic_base_addr;
 
 	/* check whether the CPU supports the security extensions */
 	reg = read_id_pfr1();

@@ -89,23 +90,24 @@ int armv7_init_nonsec(void)
 	 * any access to it will trap.
 	 */
 
-	gic_dist_addr = get_gicd_base_address();
-	if (gic_dist_addr == -1)
+	gic_base_addr = get_gicd_base_address();
+	gic_dist_addr = gic_base_addr;
+	if (gic_base_addr == -1)
 		return -1;
 
 	/* enable the GIC distributor */
-	writel(readl(gic_dist_addr + GICD_CTLR) | 0x03,
-	       gic_dist_addr + GICD_CTLR);
+	writel(readl(gic_base_addr + GICD_CTLR) | 0x03,
+	       gic_base_addr + GICD_CTLR);
 
 	/* TYPER[4:0] contains an encoded number of available interrupts */
-	itlinesnr = readl(gic_dist_addr + GICD_TYPER) & 0x1f;
+	itlinesnr = readl(gic_base_addr + GICD_TYPER) & 0x1f;

arch/arm/cpu/armv7/virt-v7.c int armv7_init_nonsec(void) · line 113

Quoted 10 lines

arch/arm/cpu/armv7/virt-v7.c +1 -1

 	/* set all bits in the GIC group registers to one to allow access
 	 * from non-secure state. The first 32 interrupts are private per
 	 * CPU and will be set later when enabling the GIC for each core
 	 */
 	for (i = 1; i <= itlinesnr; i++)
-		writel((unsigned)-1, gic_dist_addr + GICD_IGROUPRn + 4 * i);
+		writel((unsigned)-1, gic_base_addr + GICD_IGROUPRn + 4 * i);
 
 #ifndef CONFIG_ARMV7_PSCI
 	smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen), -1);

Wouldn't it be better to declare gic_dist_base as a local variable? It is only used once outside function armv7_switch_nonsec(). It could be replaced with get_gicd_base_address() call.

York

Yuantian Tang

author

2014-10-16 03:02 UTC #4 lore raw

PATCH 2/4 ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

Quoted 63 lines

-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, October 15, 2014 11:44 PM
To: Tang Yuantian-B29983; albert.u.boot at aribaud.net
Cc: u-boot at lists.denx.de; Jin Zhengxiong-R64188
Subject: Re: [PATCH 2/4] ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation
Quoted 54 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Defining variable gic_dist_addr as a globe one prevents function armv7_init_nonsec() from being used before relocation which is the case in the deep sleep resume process on Freescale QorIQ SoC platforms.
This patch removes this limitation by adding a extra same meaning local variable. In this way, no exsiting codes get corrupts.
Signed-off-by Tang Yuantian <[email protected]>
---
 arch/arm/cpu/armv7/virt-v7.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/cpu/armv7/virt-v7.c index 651ca40..e1dfce9 100644
arch/arm/cpu/armv7/virt-v7.c +8 -6
@@ -75,6 +75,7 @@ int armv7_init_nonsec(void)  {
 	unsigned int reg;
 	unsigned itlinesnr, i;
+	unsigned long gic_base_addr;
 
 	/* check whether the CPU supports the security extensions */
 	reg = read_id_pfr1();
@@ -89,23 +90,24 @@ int armv7_init_nonsec(void)
 	 * any access to it will trap.
 	 */
 
-	gic_dist_addr = get_gicd_base_address();
-	if (gic_dist_addr == -1)
+	gic_base_addr = get_gicd_base_address();
+	gic_dist_addr = gic_base_addr;
+	if (gic_base_addr == -1)
 		return -1;
 
 	/* enable the GIC distributor */
-	writel(readl(gic_dist_addr + GICD_CTLR) | 0x03,
-	       gic_dist_addr + GICD_CTLR);
+	writel(readl(gic_base_addr + GICD_CTLR) | 0x03,
+	       gic_base_addr + GICD_CTLR);
 
 	/* TYPER[4:0] contains an encoded number of available interrupts */
-	itlinesnr = readl(gic_dist_addr + GICD_TYPER) & 0x1f;
+	itlinesnr = readl(gic_base_addr + GICD_TYPER) & 0x1f;
 
 	/* set all bits in the GIC group registers to one to allow access
 	 * from non-secure state. The first 32 interrupts are private per
 	 * CPU and will be set later when enabling the GIC for each core
 	 */
 	for (i = 1; i <= itlinesnr; i++)
-		writel((unsigned)-1, gic_dist_addr + GICD_IGROUPRn + 4 * i);
+		writel((unsigned)-1, gic_base_addr + GICD_IGROUPRn + 4 * i);
 

Quoted 9 lines

arch/arm/cpu/armv7/virt-v7.c int armv7_init_nonsec(void)
Quoted 4 lines
arch/arm/cpu/armv7/virt-v7.c +0 -1
 #ifndef CONFIG_ARMV7_PSCI
 	smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen),
-1);
Wouldn't it be better to declare gic_dist_base as a local variable? It is only used once outside function armv7_switch_nonsec(). It could be replaced with get_gicd_base_address() call.

I am with you. That's what I did in the first version of this patch. Patch links is at: http://patchwork.ozlabs.org/patch/391065/
But Albert seems have some concerns. The attached is what we discussed.

Now on the second thought, I prefer the way this patch proposed because if we define gic_dist_base as local variable, That means function get_gicd_base_address() should be usable at any time in any mode. Can we make sure of that in the future?

Regards,
Yuantian

Quoted 1 line

York

-------------- next part --------------
An embedded message was scrubbed...
From: Yuantian Tang <[email protected]>
Subject: RE: [PATCH] ARM: HYP/non-sec: Make a variable as a local one
Date: Mon, 22 Sep 2014 08:46:09 +0000
Size: 1951
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141016/2cf013f6/attachment.mht>

York Sun

2014-10-16 03:36 UTC #5 lore raw

PATCH 2/4 ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

Quoted 74 lines · On 10/15/14 8:02 PM, "Tang Yuantian-B29983" <[email protected]> wrote:

Quoted 64 lines
-----Original Message-----
From: Sun York-R58495
Sent: Wednesday, October 15, 2014 11:44 PM
To: Tang Yuantian-B29983; albert.u.boot at aribaud.net
Cc: u-boot at lists.denx.de; Jin Zhengxiong-R64188
Subject: Re: [PATCH 2/4] ARM: HYP/non-sec: Make armv7_init_nonsec() usable
before relocation
Quoted 54 lines · Yuantian.Tang at freescale.com
From: Tang Yuantian <[email protected]>
Defining variable gic_dist_addr as a globe one prevents function armv7_init_nonsec() from being used before relocation which is the case in the deep sleep resume process on Freescale QorIQ SoC platforms.
This patch removes this limitation by adding a extra same meaning local variable. In this way, no exsiting codes get corrupts.
Signed-off-by Tang Yuantian <[email protected]>
---
 arch/arm/cpu/armv7/virt-v7.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/arch/arm/cpu/armv7/virt-v7.c index 651ca40..e1dfce9 100644
arch/arm/cpu/armv7/virt-v7.c +8 -6
@@ -75,6 +75,7 @@ int armv7_init_nonsec(void)  {
 	unsigned int reg;
 	unsigned itlinesnr, i;
+	unsigned long gic_base_addr;
 
 	/* check whether the CPU supports the security extensions */
 	reg = read_id_pfr1();
@@ -89,23 +90,24 @@ int armv7_init_nonsec(void)
 	 * any access to it will trap.
 	 */
 
-	gic_dist_addr = get_gicd_base_address();
-	if (gic_dist_addr == -1)
+	gic_base_addr = get_gicd_base_address();
+	gic_dist_addr = gic_base_addr;
+	if (gic_base_addr == -1)
 		return -1;
 
 	/* enable the GIC distributor */
-	writel(readl(gic_dist_addr + GICD_CTLR) | 0x03,
-	       gic_dist_addr + GICD_CTLR);
+	writel(readl(gic_base_addr + GICD_CTLR) | 0x03,
+	       gic_base_addr + GICD_CTLR);
 
 	/* TYPER[4:0] contains an encoded number of available interrupts */
-	itlinesnr = readl(gic_dist_addr + GICD_TYPER) & 0x1f;
+	itlinesnr = readl(gic_base_addr + GICD_TYPER) & 0x1f;
 
 	/* set all bits in the GIC group registers to one to allow access
 	 * from non-secure state. The first 32 interrupts are private per
 	 * CPU and will be set later when enabling the GIC for each core
 	 */
 	for (i = 1; i <= itlinesnr; i++)
-		writel((unsigned)-1, gic_dist_addr + GICD_IGROUPRn + 4 * i);
+		writel((unsigned)-1, gic_base_addr + GICD_IGROUPRn + 4 * i);
 
Quoted 10 lines
arch/arm/cpu/armv7/virt-v7.c int armv7_init_nonsec(void)
Quoted 4 lines
arch/arm/cpu/armv7/virt-v7.c +0 -1
 #ifndef CONFIG_ARMV7_PSCI
 	smp_set_core_boot_addr((unsigned long)secure_ram_addr(_smp_pen),
-1);
Wouldn't it be better to declare gic_dist_base as a local variable? It is only used
once outside function armv7_switch_nonsec(). It could be replaced with get_gicd_base_address() call.

Quoted 8 lines

I am with you. That's what I did in the first version of this patch. Patch links is at: http://patchwork.ozlabs.org/patch/391065/
But Albert seems have some concerns. The attached is what we discussed.
Now on the second thought, I prefer the way this patch proposed because if we define gic_dist_base as local variable,
That means function get_gicd_base_address() should be usable at any time in any mode. Can we make sure of that in the future?

I don't strongly object introducing a new local variable. But I don't see how the global variable is useful. Function get_gicd_base_address() should be available all the time. It reads PERIPHBASE register, or return a macro. It hasn't changed since the first patch added it in 2013. Not sure if the original author Andre Przywara is available to comments.

York

Yuantian Tang

author

2014-10-16 04:42 UTC #6 lore raw

PATCH 2/4 ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

Quoted 14 lines

Quoted 5 lines
Quoted 5 lines
Wouldn't it be better to declare gic_dist_base as a local variable? It is only used once outside function armv7_switch_nonsec(). It could be replaced with
get_gicd_base_address() call.
Quoted 8 lines
I am with you. That's what I did in the first version of this patch. Patch links is at: http://patchwork.ozlabs.org/patch/391065/
But Albert seems have some concerns. The attached is what we discussed.
Now on the second thought, I prefer the way this patch proposed because if we define gic_dist_base as local variable, That means function get_gicd_base_address() should be usable at any time in any mode. Can we make sure of that in the future?

Quoted 6 lines

I don't strongly object introducing a new local variable. But I don't see how the global variable is useful. Function get_gicd_base_address() should be available all the time. It reads PERIPHBASE register, or return a macro. It hasn't changed since the first patch added it in 2013. Not sure if the original author Andre Przywara is available to comments.

Thanks for your comments.
If no one objects the original patch, I like to resubmit it.

Hi Albert, what's your opinion on this?

Regards,
Yuantian

Quoted 2 lines

York

Albert ARIBAUD

2014-10-27 09:41 UTC #7 lore raw

PATCH 2/4 ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

Hello Yuantian,

Quoted 8 lines · Yuantian Tang

Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Wouldn't it be better to declare gic_dist_base as a local variable? It is only used once outside function armv7_switch_nonsec(). It could be replaced with
get_gicd_base_address() call.
I am with you. That's what I did in the first version of this patch. Patch links is at: http://patchwork.ozlabs.org/patch/391065/
But Albert seems have some concerns. The attached is what we discussed.

FTR, I only had concerns with the patch subject / commit summary. Regarding the patch itself, I just asked whether the global was not used as some means of coordination which would have been broken by turning it into a local, but you had checked, so that was fine.

Quoted 11 lines

Quoted 11 lines
Quoted 4 lines
Now on the second thought, I prefer the way this patch proposed because if we define gic_dist_base as local variable, That means function get_gicd_base_address() should be usable at any time in any mode. Can we make sure of that in the future?
I don't strongly object introducing a new local variable. But I don't see how the global variable is useful. Function get_gicd_base_address() should be available all the time. It reads PERIPHBASE register, or return a macro. It hasn't changed since the first patch added it in 2013. Not sure if the original author Andre Przywara is available to comments.

Quoted 4 lines

Thanks for your comments.
If no one objects the original patch, I like to resubmit it.
Hi Albert, what's your opinion on this?

Which 'original patch' do you mean?

If it is http://patchwork.ozlabs.org/patch/391065/ then I'm fine with it and will apply it.

Quoted 5 lines

Regards,
Yuantian
Quoted 2 lines
York

Amicalement,

Signature · 1 line

Albert.

Yuantian Tang

author

2014-10-28 02:24 UTC #8 lore raw

PATCH 2/4 ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

Quoted 43 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Monday, October 27, 2014 5:42 PM
To: Tang Yuantian-B29983
Cc: Sun York-R58495; u-boot at lists.denx.de; Jin Zhengxiong-R64188
Subject: Re: [PATCH 2/4] ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation
Hello Yuantian,
Quoted 8 lines · Yuantian Tang
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Wouldn't it be better to declare gic_dist_base as a local variable? It is only used once outside function armv7_switch_nonsec(). It could be replaced with
get_gicd_base_address() call.
I am with you. That's what I did in the first version of this patch. Patch links is at: http://patchwork.ozlabs.org/patch/391065/
But Albert seems have some concerns. The attached is what we discussed.
FTR, I only had concerns with the patch subject / commit summary. Regarding the patch itself, I just asked whether the global was not used as some means of coordination which would have been broken by turning it into a local, but you had checked, so that was fine.
Quoted 11 lines
Quoted 11 lines
Quoted 5 lines
Now on the second thought, I prefer the way this patch proposed because if we define gic_dist_base as local variable, That means function
get_gicd_base_address() should be usable at any time in any mode. Can we make sure of that in the future?
I don't strongly object introducing a new local variable. But I don't see how the global variable is useful. Function get_gicd_base_address() should be available all the time. It reads PERIPHBASE register, or return a macro. It hasn't changed since the first patch added it in 2013. Not sure if the original author Andre Przywara is
available to comments.
Quoted 4 lines
Quoted 1 line
Thanks for your comments.
If no one objects the original patch, I like to resubmit it.

Quoted 8 lines

Quoted 1 line
Hi Albert, what's your opinion on this?
Which 'original patch' do you mean?
If it is http://patchwork.ozlabs.org/patch/391065/ then I'm fine with it and will apply it.

Yes, it is.
But I marked it as superseded because, as you suggested, this patch is resent as part of "deep sleep" patch set. I will send deep sleep patch set v2 to address TOM's concerns. You can apply them all together.

Thanks,
Yuantian

Quoted 9 lines

Quoted 5 lines
Regards,
Yuantian
Quoted 2 lines
York
Amicalement,
--
Albert.

Albert ARIBAUD

2014-10-28 06:48 UTC #9 lore raw

PATCH 2/4 ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation

Hello Yuantian,

Quoted 46 lines · Yuantian Tang

Quoted 46 lines
-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Monday, October 27, 2014 5:42 PM
To: Tang Yuantian-B29983
Cc: Sun York-R58495; u-boot at lists.denx.de; Jin Zhengxiong-R64188
Subject: Re: [PATCH 2/4] ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation
Hello Yuantian,
Quoted 8 lines · Yuantian Tang
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
Wouldn't it be better to declare gic_dist_base as a local variable? It is only used once outside function armv7_switch_nonsec(). It could be replaced with
get_gicd_base_address() call.
I am with you. That's what I did in the first version of this patch. Patch links is at: http://patchwork.ozlabs.org/patch/391065/
But Albert seems have some concerns. The attached is what we discussed.
FTR, I only had concerns with the patch subject / commit summary. Regarding the patch itself, I just asked whether the global was not used as some means of coordination which would have been broken by turning it into a local, but you had checked, so that was fine.
Quoted 11 lines
Quoted 11 lines
Quoted 5 lines
Now on the second thought, I prefer the way this patch proposed because if we define gic_dist_base as local variable, That means function
get_gicd_base_address() should be usable at any time in any mode. Can we make sure of that in the future?
I don't strongly object introducing a new local variable. But I don't see how the global variable is useful. Function get_gicd_base_address() should be available all the time. It reads PERIPHBASE register, or return a macro. It hasn't changed since the first patch added it in 2013. Not sure if the original author Andre Przywara is
available to comments.
Quoted 5 lines
Quoted 1 line
Thanks for your comments.
If no one objects the original patch, I like to resubmit it.
Hi Albert, what's your opinion on this?

Quoted 8 lines

Quoted 5 lines
Which 'original patch' do you mean?
If it is http://patchwork.ozlabs.org/patch/391065/ then I'm fine with it and will apply it.
Yes, it is.
But I marked it as superseded because, as you suggested, this patch is resent as part of "deep sleep" patch set. I will send deep sleep patch set v2 to address TOM's concerns. You can apply them all together.

Ok, thanks for the clarification.

Quoted 2 lines

Thanks,
Yuantian

Amicalement,

Signature · 1 line

Albert.

Yuantian.Tang at freescale.com

author

2014-10-09 08:11 UTC #10 lore raw

PATCH 3/4 arm: ls102xa: Fixed a register definition error

In reply to Yuantian.Tang at freescale.com

From: Tang Yuantian <[email protected]>

There are 8 SCFG_SPARECR registers in SCFG memory block, not one.

Signed-off-by Tang Yuantian <[email protected]>

---
 arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h +1 -1

index 7995fe2..b5db720 100644
@@ -182,7 +182,7 @@ struct ccsr_scfg {
 	u32 etsecmcr;
 	u32 sdhciovserlcr;
 	u32 resv14[61];
-	u32 sparecr;
+	u32 sparecr[8];
 };
 
 /* Clocking */

Signature · 1 line

2.1.0.27.g96db324

York Sun

2014-11-25 17:51 UTC #11 lore raw

PATCH 3/4 arm: ls102xa: Fixed a register definition error

Quoted 9 lines · Yuantian.Tang at freescale.com

From: Tang Yuantian <[email protected]>
There are 8 SCFG_SPARECR registers in SCFG memory block, not one.
Signed-off-by Tang Yuantian <[email protected]>
---
 arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Applied to u-boot-fsl-qoriq master. Awaiting upstream. Thanks.

York

Yuantian.Tang at freescale.com

author

2014-10-09 08:11 UTC #12 lore raw

PATCH 4/4 arm: ls1021qds: Add deep sleep support

In reply to Yuantian.Tang at freescale.com

From: Tang Yuantian <[email protected]>

Add deep sleep support on Freescale LS1021QDS platform.

Signed-off-by Tang Yuantian <[email protected]>

---
 board/freescale/ls1021aqds/ddr.c        |  7 ++++
 board/freescale/ls1021aqds/ls1021aqds.c | 60 +++++++++++++++++++++++++++++++++
 include/configs/ls1021aqds.h            |  4 +++
 3 files changed, 71 insertions(+)

board/freescale/ls1021aqds/ddr.c +7 -0

index 5898e33..6dad4cc 100644
@@ -8,6 +8,9 @@
 #include <fsl_ddr_sdram.h>
 #include <fsl_ddr_dimm_params.h>
 #include "ddr.h"
+#ifdef CONFIG_FSL_DEEP_SLEEP
+#include <fsl_sleep.h>
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;

@@ -156,6 +159,10 @@ phys_size_t initdram(int board_type)
 	puts("Initializing DDR....using SPD\n");
 	dram_size = fsl_ddr_sdram();
 
+#ifdef CONFIG_FSL_DEEP_SLEEP
+	fsl_dp_ddr_restore();
+#endif
+
 	return dram_size;
 }

board/freescale/ls1021aqds/ls1021aqds.c +60 -0

63 lines not on this page · raw message

include/configs/ls1021aqds.h +4 -0

index bb47813..448a07e 100644
@@ -19,6 +19,10 @@
 #define CONFIG_SKIP_LOWLEVEL_INIT
 #define CONFIG_BOARD_EARLY_INIT_F
 
+#define CONFIG_FSL_DEEP_SLEEP
+#ifdef CONFIG_FSL_DEEP_SLEEP
+#define CONFIG_SILENT_CONSOLE
+#endif
 /*
  * Size of malloc() pool
  */

Signature · 1 line

2.1.0.27.g96db324

Yuantian Tang

author

2014-10-13 09:41 UTC #13 lore raw

cover Deep sleep patches for Freescale QorIQ platforms

In reply to Yuantian.Tang at freescale.com

PING.

Thanks,
Yuantian

Quoted 38 lines

-----Original Message-----
From: Yuantian.Tang at freescale.com [mailto:Yuantian.Tang at freescale.com]
Sent: Thursday, October 09, 2014 4:12 PM
To: albert.u.boot at aribaud.net
Cc: u-boot at lists.denx.de; Jin Zhengxiong-R64188; Sun York-R58495; Tang Yuantian-B29983
Subject: [PATCH 0/4] Deep sleep patches for Freescale QorIQ platforms
From: Tang Yuantian <[email protected]>
These patches depend on the following patches:
https://patchwork.ozlabs.org/patch/389949/
https://patchwork.ozlabs.org/patch/389950/
https://patchwork.ozlabs.org/patch/389951/
https://patchwork.ozlabs.org/patch/389952/
Tang Yuantian (4):
Add deep sleep framework support for Freescale QorIQ platforms
ARM: HYP/non-sec: Make armv7_init_nonsec() usable before relocation
arm: ls102xa: Fixed a register definition error
arm: ls1021qds: Add deep sleep support
 arch/arm/cpu/armv7/virt-v7.c                      | 14 +++---
 arch/arm/include/asm/arch-ls102xa/immap_ls102xa.h |  2 +-
 board/freescale/ls1021aqds/ddr.c                  |  7 +++
 board/freescale/ls1021aqds/ls1021aqds.c           | 60
+++++++++++++++++++++++
 common/board_f.c                                  | 10 ++++
 drivers/ddr/fsl/arm_ddr_gen3.c                    | 48
++++++++++++++++--
 include/configs/ls1021aqds.h                      |  4 ++
 include/fsl_ddr_sdram.h                           |  2 +
 include/fsl_sleep.h                               | 32 ++++++++++++
 9 files changed, 167 insertions(+), 12 deletions(-)  create mode 100644
include/fsl_sleep.h
--
2.1.0.27.g96db324

Albert ARIBAUD

2014-10-13 09:46 UTC #14 lore raw

cover Deep sleep patches for Freescale QorIQ platforms

Hi Yuantian,

Quoted 1 line · Yuantian Tang

PING.

This submission is only 4 days old, across a week ending. Can you please wait a bit longer before pinging?

Quoted 2 lines

Thanks,
Yuantian

Amicalement,

Signature · 1 line

Albert.

Yuantian Tang

author

2014-10-14 02:30 UTC #15 lore raw

cover Deep sleep patches for Freescale QorIQ platforms

Quoted 9 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Monday, October 13, 2014 5:46 PM
To: Tang Yuantian-B29983
Cc: u-boot at lists.denx.de; Jin Zhengxiong-R64188; Sun York-R58495
Subject: Re: [PATCH 0/4] Deep sleep patches for Freescale QorIQ platforms
Hi Yuantian,

Quoted 8 lines

Quoted 1 line · Yuantian Tang
PING.
This submission is only 4 days old, across a week ending. Can you please wait a bit longer before pinging?

Yeah, I should've waited longer.
That you responded so quickly to the patches I sent earlier makes me think you are always a quick responder. Take your time.

Thanks,
Yuantian

Quoted 6 lines

Quoted 2 lines
Thanks,
Yuantian
Amicalement,
--
Albert.