All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function
@ 2017-04-30 13:29 Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 01/11] arm: add atomic functions with return support Antoine Tenart
                   ` (11 more replies)
  0 siblings, 12 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Hi all,

Respin this series that was sent in October 2016, with comments and new
psci aware SoC taken into account.

This series adds an implementation of the psci suspend function for both
sun5i and sun7i. This was tested on Nextthing's CHIP, using cpuidle in
Linux.

As the sun5i does not have a GIC, I needed to remove some code from the
generic psci code. To do this I added an ARM_GIC Kconfig option which
needs to be selected by each SoC having a GIC. I already added the
relevant lines for SoCs selecting ARCH_SUPPORT_PSCI. As this Kconfig
option is currently only used in the psci code, it should be safe to add
it even if all the SoCs having a GIC do not select it, as long as they
don't have psci functions.

On multi-core systems, we need to ensure all CPU are in psci suspend
function to switch the cpu clk source. Likewise, the first CPU to exit
the psci suspend function should restore the cpu clk source; and only
the first one. This series adds a few atomic operation (with return
support) in order to solve this problem. I used the Linux
implementation.

This series is based on sunxi/master (6e39de1b337e) and can be found on:
https://github.com/atenart/u-boot sunxi/psci-suspend

Thanks!

Antoine

Since v3:
  - Reworked the atomic functions with return support, based on the Linux
    implementation.
  - Do not disable LDO anymore.
  - Added some comments.
  - Poll the pll1 lock bit before switching to it.
  - Select CONFIG_ARM_GIC directly in CONFIG_TEGRA_COMMON.
  - Select ARM_GIC for mx7 and uniphier as well.

Since v2:
  - Stopped putting the dram into self-refresh.
  - Removed pll2-7 disabling.
  - Added an atomic variable to only change the cpuclk source when all
    CPUs are in idle.
  - Did a similar implementation to be sure the first CPU to leave the
    idle mode switches back the cpuclk source to pll1.
  - Rebased on the latest master and updated the patches.

Since v1:
  - Rebased on the latest master and updated the patches.
  - Fixed a compile warning I introduced in virt-v7.c
  - Added the missing ARM_GIC Kconfig options.
  - Fixed a commit message (removed 'HYP').

Antoine Tenart (11):
  arm: add atomic functions with return support
  arm: add the ARM_GIC configuration option
  sunxi: select ARM_GIC for sun[6789]i
  arm: select ARM_GIC for SoCs having a psci implementation
  tegra: select ARM_GIC for Tegra SoCs
  mx7: select ARM_GIC
  uniphier: select ARM_GIC
  arm: psci: protect GIC specific code with ARM_GIC
  sun5/7i: add an implementation of the psci suspend function
  sun5i: add defines used by the PSCI code
  sun5i: boot in non-secure mode by default

 arch/arm/Kconfig                              |  13 ++++
 arch/arm/cpu/armv7/mx7/Kconfig                |   1 +
 arch/arm/cpu/armv7/nonsec_virt.S              |   6 ++
 arch/arm/cpu/armv7/sunxi/Makefile             |   9 ++-
 arch/arm/cpu/armv7/sunxi/psci.c               |  40 +---------
 arch/arm/cpu/armv7/sunxi/psci.h               |  58 ++++++++++++++
 arch/arm/cpu/armv7/sunxi/psci_suspend.c       | 108 ++++++++++++++++++++++++++
 arch/arm/cpu/armv7/virt-v7.c                  |  42 ++++++----
 arch/arm/include/asm/arch-sunxi/clock_sun4i.h |   2 +
 arch/arm/include/asm/atomic.h                 |  36 +++++++++
 arch/arm/mach-tegra/Kconfig                   |   1 +
 arch/arm/mach-uniphier/Kconfig                |   1 +
 board/sunxi/Kconfig                           |  12 +++
 include/configs/sun5i.h                       |   2 +
 14 files changed, 278 insertions(+), 53 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/sunxi/psci.h
 create mode 100644 arch/arm/cpu/armv7/sunxi/psci_suspend.c

-- 
2.11.0

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 01/11] arm: add atomic functions with return support
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 02/11] arm: add the ARM_GIC configuration option Antoine Tenart
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Implement three atomic functions to allow making an atomic operation
that returns the value. Adds: atomic_add_return(), atomic_sub_return(),
atomic_inc_return() and atomic_dec_return().

This is heavily based on the Linux implementation of such functions for
ARM.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 arch/arm/include/asm/atomic.h | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/arm/include/asm/atomic.h b/arch/arm/include/asm/atomic.h
index 171f4d979281..4bfe62afaca1 100644
--- a/arch/arm/include/asm/atomic.h
+++ b/arch/arm/include/asm/atomic.h
@@ -73,6 +73,42 @@ static inline void atomic_dec(volatile atomic_t *v)
 	local_irq_restore(flags);
 }
 
+#define ATOMIC_OP_RETURN(op, c_op, asm_op)				\
+static inline int atomic_##op##_return(int i, volatile atomic_t *v)	\
+{									\
+        unsigned long tmp;						\
+        int result;							\
+									\
+	/* prefetch */							\
+	__asm__ __volatile__("pld\t%a0"					\
+	:: "p" (&v->counter));						\
+									\
+        __asm__ __volatile__("@ atomic_" #op "_return\n"		\
+"1:	ldrex	%0, [%3]\n"						\
+"	" #asm_op "	%0, %0, %4\n"					\
+"	strex	%1, %0, [%3]\n"						\
+"	teq	%1, #0\n"						\
+"	bne	1b"							\
+        : "=&r" (result), "=&r" (tmp), "+Qo" (v->counter)		\
+        : "r" (&v->counter), "Ir" (i)					\
+        : "cc");							\
+									\
+        return result;							\
+}
+
+ATOMIC_OP_RETURN(add, +=, add)
+ATOMIC_OP_RETURN(sub, -=, sub)
+
+static inline int atomic_inc_return(volatile atomic_t *v)
+{
+	return atomic_add_return(1, v);
+}
+
+static inline int atomic_dec_return(volatile atomic_t *v)
+{
+	return atomic_sub_return(1, v);
+}
+
 static inline int atomic_dec_and_test(volatile atomic_t *v)
 {
 	unsigned long flags = 0;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 02/11] arm: add the ARM_GIC configuration option
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 01/11] arm: add atomic functions with return support Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 03/11] sunxi: select ARM_GIC for sun[6789]i Antoine Tenart
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Some SoC does not have a GIC. Adds a configuration option to denote
this, allowing to remove code configuring the GIC when it's not
possible.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
---
 arch/arm/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index d8669ce7d6b1..c7aebae3b039 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -13,6 +13,9 @@ config DMA_ADDR_T_64BIT
 	bool
 	default y if ARM64
 
+config ARM_GIC
+	bool
+
 config HAS_VBAR
 	bool
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 03/11] sunxi: select ARM_GIC for sun[6789]i
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 01/11] arm: add atomic functions with return support Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 02/11] arm: add the ARM_GIC configuration option Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 04/11] arm: select ARM_GIC for SoCs having a psci implementation Antoine Tenart
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Select the newly introduced ARM_GIC option to the relevant sunxi
MACH configurations.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 board/sunxi/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index 27bd42c64b98..e53cd9c21c33 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -83,6 +83,7 @@ config MACH_SUN5I
 
 config MACH_SUN6I
 	bool "sun6i (Allwinner A31)"
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
@@ -93,6 +94,7 @@ config MACH_SUN6I
 
 config MACH_SUN7I
 	bool "sun7i (Allwinner A20)"
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
@@ -103,6 +105,7 @@ config MACH_SUN7I
 
 config MACH_SUN8I_A23
 	bool "sun8i (Allwinner A23)"
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
@@ -113,6 +116,7 @@ config MACH_SUN8I_A23
 
 config MACH_SUN8I_A33
 	bool "sun8i (Allwinner A33)"
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
@@ -123,12 +127,14 @@ config MACH_SUN8I_A33
 
 config MACH_SUN8I_A83T
 	bool "sun8i (Allwinner A83T)"
+	select ARM_GIC
 	select CPU_V7
 	select SUNXI_GEN_SUN6I
 	select SUPPORT_SPL
 
 config MACH_SUN8I_H3
 	bool "sun8i (Allwinner H3)"
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
@@ -138,6 +144,7 @@ config MACH_SUN8I_H3
 
 config MACH_SUN8I_R40
 	bool "sun8i (Allwinner R40)"
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
@@ -147,6 +154,7 @@ config MACH_SUN8I_R40
 
 config MACH_SUN8I_V3S
 	bool "sun8i (Allwinner V3s)"
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
@@ -156,6 +164,7 @@ config MACH_SUN8I_V3S
 
 config MACH_SUN9I
 	bool "sun9i (Allwinner A80)"
+	select ARM_GIC
 	select CPU_V7
 	select SUNXI_HIGH_SRAM
 	select SUNXI_GEN_SUN6I
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 04/11] arm: select ARM_GIC for SoCs having a psci implementation
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (2 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 03/11] sunxi: select ARM_GIC for sun[6789]i Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-05-11  8:00   ` Masahiro Yamada
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs Antoine Tenart
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Select the newly introduced ARM_GIC option to the relevant MACH
configurations.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
---
 arch/arm/Kconfig | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c7aebae3b039..8b13f6909f9c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -472,6 +472,7 @@ config ARCH_BCM283X
 
 config TARGET_VEXPRESS_CA15_TC2
 	bool "Support vexpress_ca15_tc2"
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select CPU_V7_HAS_VIRT
@@ -556,14 +557,17 @@ config TARGET_BCM23550_W1D
 
 config TARGET_BCM28155_AP
 	bool "Support bcm28155_ap"
+	select ARM_GIC
 	select CPU_V7
 
 config TARGET_BCMCYGNUS
 	bool "Support bcmcygnus"
+	select ARM_GIC
 	select CPU_V7
 
 config TARGET_BCMNSP
 	bool "Support bcmnsp"
+	select ARM_GIC
 	select CPU_V7
 
 config TARGET_BCMNS2
@@ -622,6 +626,7 @@ config ARCH_MX7ULP
 
 config ARCH_MX7
 	bool "Freescale MX7"
+	select ARM_GIC
 	select CPU_V7
 	select SYS_FSL_HAS_SEC if SECURE_BOOT
 	select SYS_FSL_SEC_COMPAT_4
@@ -995,6 +1000,7 @@ config TARGET_LS1012AFRDM
 
 config TARGET_LS1021AQDS
 	bool "Support ls1021aqds"
+	select ARM_GIC
 	select BOARD_LATE_INIT
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
@@ -1008,6 +1014,7 @@ config TARGET_LS1021AQDS
 
 config TARGET_LS1021ATWR
 	bool "Support ls1021atwr"
+	select ARM_GIC
 	select BOARD_LATE_INIT
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
@@ -1020,6 +1027,7 @@ config TARGET_LS1021ATWR
 
 config TARGET_LS1021AIOT
 	bool "Support ls1021aiot"
+	select ARM_GIC
 	select BOARD_LATE_INIT
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
@@ -1100,7 +1108,9 @@ config TARGET_COLIBRI_PXA270
 
 config ARCH_UNIPHIER
 	bool "Socionext UniPhier SoCs"
+	select ARM_GIC
 	select BOARD_LATE_INIT
+	select BLK
 	select CLK_UNIPHIER
 	select DM
 	select DM_GPIO
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (3 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 04/11] arm: select ARM_GIC for SoCs having a psci implementation Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-05-01 15:12   ` Tom Warren
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 06/11] mx7: select ARM_GIC Antoine Tenart
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Select the newly introduced ARM_GIC option to the relevant configuration
which also have a psci implementation.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: Tom Warren <twarren@nvidia.com>
---
 arch/arm/mach-tegra/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index c67ffa5a2356..01f27b465bfe 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -22,6 +22,7 @@ config TEGRA_IVC
 
 config TEGRA_COMMON
 	bool "Tegra common options"
+	select ARM_GIC
 	select CLK
 	select DM
 	select DM_ETH
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 06/11] mx7: select ARM_GIC
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (4 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 07/11] uniphier: " Antoine Tenart
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Select the newly introduced ARM_GIC option to the relevant configuration
which also have a psci implementation.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 arch/arm/cpu/armv7/mx7/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/cpu/armv7/mx7/Kconfig b/arch/arm/cpu/armv7/mx7/Kconfig
index 8dfb4c96464f..412e4aba3c2a 100644
--- a/arch/arm/cpu/armv7/mx7/Kconfig
+++ b/arch/arm/cpu/armv7/mx7/Kconfig
@@ -2,6 +2,7 @@ if ARCH_MX7
 
 config MX7
 	bool
+	select ARM_GIC
 	select ROM_UNIFIED_SECTIONS
 	select CPU_V7_HAS_VIRT
 	select CPU_V7_HAS_NONSEC
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 07/11] uniphier: select ARM_GIC
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (5 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 06/11] mx7: select ARM_GIC Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 08/11] arm: psci: protect GIC specific code with ARM_GIC Antoine Tenart
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Select the newly introduced ARM_GIC option to the relevant configuration
which also have a psci implementation.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
---
 arch/arm/mach-uniphier/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-uniphier/Kconfig b/arch/arm/mach-uniphier/Kconfig
index 5739325da71b..e6164974db57 100644
--- a/arch/arm/mach-uniphier/Kconfig
+++ b/arch/arm/mach-uniphier/Kconfig
@@ -5,6 +5,7 @@ config SYS_CONFIG_NAME
 
 config ARCH_UNIPHIER_32BIT
 	bool
+	select ARM_GIC
 	select CPU_V7
 	select CPU_V7_HAS_NONSEC
 	select ARMV7_NONSEC
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 08/11] arm: psci: protect GIC specific code with ARM_GIC
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (6 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 07/11] uniphier: " Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function Antoine Tenart
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Introducing the ARM_GIC configuration option, use it to only use GIC
specific code in ARM PSCI function when the SoC has a GIC.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
---
 arch/arm/cpu/armv7/nonsec_virt.S |  6 ++++++
 arch/arm/cpu/armv7/virt-v7.c     | 42 ++++++++++++++++++++++++++--------------
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/arch/arm/cpu/armv7/nonsec_virt.S b/arch/arm/cpu/armv7/nonsec_virt.S
index e39aba711587..fb9902cc7253 100644
--- a/arch/arm/cpu/armv7/nonsec_virt.S
+++ b/arch/arm/cpu/armv7/nonsec_virt.S
@@ -164,6 +164,7 @@ ENDPROC(_smp_pen)
  * though, but we check this in C before calling this function.
  */
 ENTRY(_nonsec_init)
+#ifdef CONFIG_ARM_GIC
 	get_gicd_addr	r3
 
 	mvn	r1, #0				@ all bits to 1
@@ -175,6 +176,7 @@ ENTRY(_nonsec_init)
 	str	r1, [r3, #GICC_CTLR]		@ and clear all other bits
 	mov	r1, #0xff
 	str	r1, [r3, #GICC_PMR]		@ set priority mask register
+#endif
 
 	mrc	p15, 0, r0, c1, c1, 2
 	movw	r1, #0x3fff
@@ -200,7 +202,11 @@ ENTRY(_nonsec_init)
 	mcr	p15, 0, r1, c12, c0, 1		@ set MVBAR to secure vectors
 	isb
 
+#ifdef CONFIG_ARM_GIC
 	mov	r0, r3				@ return GICC address
+#else
+	mov	r0, #0
+#endif
 	bx	lr
 ENDPROC(_nonsec_init)
 
diff --git a/arch/arm/cpu/armv7/virt-v7.c b/arch/arm/cpu/armv7/virt-v7.c
index d33e5c61a9c2..a34305853a34 100644
--- a/arch/arm/cpu/armv7/virt-v7.c
+++ b/arch/arm/cpu/armv7/virt-v7.c
@@ -82,22 +82,11 @@ void __weak smp_kick_all_cpus(void)
 	kick_secondary_cpus_gic(gic_dist_addr);
 }
 
-__weak void psci_board_init(void)
-{
-}
-
-int armv7_init_nonsec(void)
+#ifdef CONFIG_ARM_GIC
+static int psci_gic_setup(void)
 {
-	unsigned int reg;
-	unsigned itlinesnr, i;
 	unsigned long gic_dist_addr;
-
-	/* check whether the CPU supports the security extensions */
-	reg = read_id_pfr1();
-	if ((reg & 0xF0) == 0) {
-		printf("nonsec: Security extensions not implemented.\n");
-		return -1;
-	}
+	unsigned itlinesnr, i;
 
 	/* the SCR register will be set directly in the monitor mode handler,
 	 * according to the spec one should not tinker with it in secure state
@@ -123,6 +112,31 @@ int armv7_init_nonsec(void)
 	for (i = 1; i <= itlinesnr; i++)
 		writel((unsigned)-1, gic_dist_addr + GICD_IGROUPRn + 4 * i);
 
+	return 0;
+}
+#endif
+
+__weak void psci_board_init(void)
+{
+}
+
+int armv7_init_nonsec(void)
+{
+	unsigned int reg;
+
+	/* check whether the CPU supports the security extensions */
+	reg = read_id_pfr1();
+	if ((reg & 0xF0) == 0) {
+		printf("nonsec: Security extensions not implemented.\n");
+		return -1;
+	}
+
+#ifdef CONFIG_ARM_GIC
+	int ret = psci_gic_setup();
+	if (ret)
+		return ret;
+#endif
+
 	psci_board_init();
 
 	/*
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (7 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 08/11] arm: psci: protect GIC specific code with ARM_GIC Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-05-01 21:13   ` Maxime Ripard
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 10/11] sun5i: add defines used by the PSCI code Antoine Tenart
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

Add the suspend psci function for sun5i and sun7i. Thus function
switches the cpu clk source to osc24M or to losc depending on the
SoC family.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm/cpu/armv7/sunxi/Makefile             |   9 ++-
 arch/arm/cpu/armv7/sunxi/psci.c               |  40 +---------
 arch/arm/cpu/armv7/sunxi/psci.h               |  58 ++++++++++++++
 arch/arm/cpu/armv7/sunxi/psci_suspend.c       | 108 ++++++++++++++++++++++++++
 arch/arm/include/asm/arch-sunxi/clock_sun4i.h |   2 +
 5 files changed, 178 insertions(+), 39 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/sunxi/psci.h
 create mode 100644 arch/arm/cpu/armv7/sunxi/psci_suspend.c

diff --git a/arch/arm/cpu/armv7/sunxi/Makefile b/arch/arm/cpu/armv7/sunxi/Makefile
index b35b9df4a9d6..80667268a0fc 100644
--- a/arch/arm/cpu/armv7/sunxi/Makefile
+++ b/arch/arm/cpu/armv7/sunxi/Makefile
@@ -13,7 +13,14 @@ obj-$(CONFIG_MACH_SUN6I)	+= tzpc.o
 obj-$(CONFIG_MACH_SUN8I_H3)	+= tzpc.o
 
 ifndef CONFIG_SPL_BUILD
-obj-$(CONFIG_ARMV7_PSCI)	+= psci.o
+ifdef CONFIG_ARMV7_PSCI
+obj-$(CONFIG_MACH_SUN6I)	+= psci.o
+obj-$(CONFIG_MACH_SUN7I)	+= psci.o
+obj-$(CONFIG_MACH_SUN8I)	+= psci.o
+
+obj-$(CONFIG_MACH_SUN5I)	+= psci_suspend.o
+obj-$(CONFIG_MACH_SUN7I)	+= psci_suspend.o
+endif
 endif
 
 ifdef CONFIG_SPL_BUILD
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index b3a34de1aafe..a86608cf3493 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -22,6 +22,8 @@
 
 #include <linux/bitops.h>
 
+#include "psci.h"
+
 #define __irq		__attribute__ ((interrupt ("IRQ")))
 
 #define	GICD_BASE	(SUNXI_GIC400_BASE + GIC_DIST_OFFSET)
@@ -38,44 +40,6 @@
 #define SUN8I_R40_PWR_CLAMP(cpu)		(0x120 + (cpu) * 0x4)
 #define SUN8I_R40_SRAMC_SOFT_ENTRY_REG0		(0xbc)
 
-static void __secure cp15_write_cntp_tval(u32 tval)
-{
-	asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
-}
-
-static void __secure cp15_write_cntp_ctl(u32 val)
-{
-	asm volatile ("mcr p15, 0, %0, c14, c2, 1" : : "r" (val));
-}
-
-static u32 __secure cp15_read_cntp_ctl(void)
-{
-	u32 val;
-
-	asm volatile ("mrc p15, 0, %0, c14, c2, 1" : "=r" (val));
-
-	return val;
-}
-
-#define ONE_MS (COUNTER_FREQUENCY / 1000)
-
-static void __secure __mdelay(u32 ms)
-{
-	u32 reg = ONE_MS * ms;
-
-	cp15_write_cntp_tval(reg);
-	isb();
-	cp15_write_cntp_ctl(3);
-
-	do {
-		isb();
-		reg = cp15_read_cntp_ctl();
-	} while (!(reg & BIT(2)));
-
-	cp15_write_cntp_ctl(0);
-	isb();
-}
-
 static void __secure clamp_release(u32 __maybe_unused *clamp)
 {
 #if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
diff --git a/arch/arm/cpu/armv7/sunxi/psci.h b/arch/arm/cpu/armv7/sunxi/psci.h
new file mode 100644
index 000000000000..248c25772e65
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/psci.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright (C) 2016
+ * Author: Chen-Yu Tsai <wens@csie.org>
+ *
+ * Based on assembly code by Marc Zyngier <marc.zyngier@arm.com>,
+ * which was based on code by Carl van Schaik <carl@ok-labs.com>.
+ *
+ * SPDX-License-Identifier:	GPL-2.0
+ */
+#ifndef _SUNXI_PSCI_H_
+#define _SUNXI_PSCI_H_
+
+#include <asm/armv7.h>
+#include <linux/bitops.h>
+
+#ifdef CONFIG_TIMER_CLK_FREQ
+static void __secure cp15_write_cntp_tval(u32 tval)
+{
+	asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
+}
+
+static void __secure cp15_write_cntp_ctl(u32 val)
+{
+	asm volatile ("mcr p15, 0, %0, c14, c2, 1" : : "r" (val));
+}
+
+static u32 __secure cp15_read_cntp_ctl(void)
+{
+	u32 val;
+
+	asm volatile ("mrc p15, 0, %0, c14, c2, 1" : "=r" (val));
+
+	return val;
+}
+
+#define ONE_MS (COUNTER_FREQUENCY / 1000)
+
+void __secure __mdelay(u32 ms)
+{
+	u32 reg = ONE_MS * ms;
+
+	cp15_write_cntp_tval(reg);
+	isb();
+	cp15_write_cntp_ctl(3);
+
+	do {
+		isb();
+		reg = cp15_read_cntp_ctl();
+	} while (!(reg & BIT(2)));
+
+	cp15_write_cntp_ctl(0);
+	isb();
+}
+#else
+#define __mdelay(ms)
+#endif
+
+#endif
diff --git a/arch/arm/cpu/armv7/sunxi/psci_suspend.c b/arch/arm/cpu/armv7/sunxi/psci_suspend.c
new file mode 100644
index 000000000000..0a8f2c165891
--- /dev/null
+++ b/arch/arm/cpu/armv7/sunxi/psci_suspend.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2017 Antoine Tenart <antoine.tenart@free-electrons.com>
+ *
+ * Based on Allwinner code.
+ * Copyright 2007-2012 (C) Allwinner Technology Co., Ltd.
+ *
+ * SPDX-License-Identifier:	GPL-2.0
+ */
+#include <config.h>
+#include <common.h>
+
+#include <asm/atomic.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/dram.h>
+#include <asm/armv7.h>
+#include <asm/io.h>
+#include <asm/psci.h>
+#include <asm/secure.h>
+#include <asm/system.h>
+
+#include <linux/bitops.h>
+
+#include "psci.h"
+
+#if defined(CONFIG_MACH_SUN5I)
+#define NR_CPUS		1
+#elif defined(CONFIG_MACH_SUN7I)
+#define NR_CPUS		2
+#endif
+
+/*
+ * The PSCI suspend function switch cpuclk to another source and disable
+ * pll1. As this function is called per-CPU, it should only do this when
+ * all the CPUs are in idle state.
+ *
+ * The 'cnt' variable keeps track of the number of CPU which are in the idle
+ * state. The last one setup cpuclk for idle.
+ *
+ * The 'clk_state' varibale holds the cpu clk state (idle or normal).
+ */
+atomic_t __secure_data cnt, clk_state;
+
+#define CLK_NORMAL	0
+#define CLK_IDLE	1
+
+static void __secure sunxi_clock_enter_idle(struct sunxi_ccm_reg *ccm)
+{
+	/* switch cpuclk to osc24m */
+	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
+			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
+
+	/* disable pll1 */
+	clrbits_le32(&ccm->pll1_cfg, CCM_PLL1_CTRL_EN);
+
+#ifndef CONFIG_MACH_SUN7I
+	/*
+	 * Switch cpuclk to losc. Based on my experience this didn't worked for
+	 * sun7i, hence the ifndef.
+	 */
+	clrbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT);
+#endif
+}
+
+static void __secure sunxi_clock_leave_idle(struct sunxi_ccm_reg *ccm)
+{
+#ifndef CONFIG_MACH_SUN7I
+	/* switch cpuclk to osc24m */
+	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
+			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
+#endif
+
+	/* enable pll1 */
+	setbits_le32(&ccm->pll1_cfg, CCM_PLL1_CTRL_EN);
+
+	/* wait for the clock to lock */
+	while (readl(&ccm->pll1_cfg) & BIT(28));
+
+	/* switch cpuclk to pll1 */
+	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
+			CPU_CLK_SRC_PLL1 << CPU_CLK_SRC_SHIFT);
+}
+
+void __secure psci_cpu_suspend(void)
+{
+	struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
+
+	if (atomic_inc_return(&cnt) == NR_CPUS) {
+		/* wait for any sunxi_clock_leave_idle() to finish */
+		while (atomic_read(&clk_state) != CLK_NORMAL)
+			__mdelay(1);
+
+		sunxi_clock_enter_idle(ccm);
+		atomic_set(&clk_state, CLK_IDLE);
+	}
+
+	/* idle */
+	DSB;
+	wfi();
+
+	if (atomic_dec_return(&cnt) == NR_CPUS - 1) {
+		/* wait for any sunxi_clock_enter_idle() to finish */
+		while (atomic_read(&clk_state) != CLK_IDLE)
+			__mdelay(1);
+
+		sunxi_clock_leave_idle(ccm);
+		atomic_set(&clk_state, CLK_NORMAL);
+	}
+}
diff --git a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
index d1c5ad0a739b..12b0f22a1368 100644
--- a/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
+++ b/arch/arm/include/asm/arch-sunxi/clock_sun4i.h
@@ -208,6 +208,8 @@ struct sunxi_ccm_reg {
 #define CCM_AHB_GATE_DLL (0x1 << 15)
 #define CCM_AHB_GATE_ACE (0x1 << 16)
 
+#define CCM_PLL1_CTRL_EN		(0x1 << 31)
+
 #define CCM_PLL3_CTRL_M_SHIFT		0
 #define CCM_PLL3_CTRL_M_MASK		(0x7f << CCM_PLL3_CTRL_M_SHIFT)
 #define CCM_PLL3_CTRL_M(n)		(((n) & 0x7f) << 0)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 10/11] sun5i: add defines used by the PSCI code
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (8 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 11/11] sun5i: boot in non-secure mode by default Antoine Tenart
  2017-04-30 16:26 ` [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Angus Ainslie
  11 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

The sun5i SoCs can take advantage of the newly introduce PSCI suspend
function. Add defines used by the PSCI code.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 include/configs/sun5i.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/configs/sun5i.h b/include/configs/sun5i.h
index ec8f3199ba34..6927c21363fb 100644
--- a/include/configs/sun5i.h
+++ b/include/configs/sun5i.h
@@ -18,6 +18,8 @@
 
 #define CONFIG_SUNXI_USB_PHYS	2
 
+#define CONFIG_ARMV7_SECURE_BASE	SUNXI_SRAM_A1_BASE
+
 /*
  * Include common sunxi configuration where most the settings are
  */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 11/11] sun5i: boot in non-secure mode by default
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (9 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 10/11] sun5i: add defines used by the PSCI code Antoine Tenart
@ 2017-04-30 13:29 ` Antoine Tenart
  2017-05-01 21:14   ` Maxime Ripard
  2017-04-30 16:26 ` [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Angus Ainslie
  11 siblings, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 13:29 UTC (permalink / raw)
  To: u-boot

sun5i now implements the psci suspend function. In order to be used by
the kernel, we should now boot in non-secure mode. Enable it by default.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 board/sunxi/Kconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
index e53cd9c21c33..043980b83831 100644
--- a/board/sunxi/Kconfig
+++ b/board/sunxi/Kconfig
@@ -77,9 +77,12 @@ config MACH_SUN4I
 config MACH_SUN5I
 	bool "sun5i (Allwinner A13)"
 	select CPU_V7
+	select CPU_V7_HAS_NONSEC
+	select ARCH_SUPPORT_PSCI
 	select ARM_CORTEX_CPU_IS_UP
 	select SUNXI_GEN_SUN4I
 	select SUPPORT_SPL
+	select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT
 
 config MACH_SUN6I
 	bool "sun6i (Allwinner A31)"
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function
  2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
                   ` (10 preceding siblings ...)
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 11/11] sun5i: boot in non-secure mode by default Antoine Tenart
@ 2017-04-30 16:26 ` Angus Ainslie
  2017-04-30 16:49   ` Antoine Tenart
  11 siblings, 1 reply; 34+ messages in thread
From: Angus Ainslie @ 2017-04-30 16:26 UTC (permalink / raw)
  To: u-boot

On 2017-04-30 07:29, Antoine Tenart wrote:
> Hi all,
> 
> Respin this series that was sent in October 2016, with comments and new
> psci aware SoC taken into account.
> 
> This series adds an implementation of the psci suspend function for 
> both
> sun5i and sun7i. This was tested on Nextthing's CHIP, using cpuidle in
> Linux.
> 

Does this get suspend to memory working on the Nextthing CHIP ?

If so which kernel version and what DTS changes and config changes are 
needed ?

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function
  2017-04-30 16:26 ` [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Angus Ainslie
@ 2017-04-30 16:49   ` Antoine Tenart
  2017-04-30 23:26     ` Angus Ainslie
  0 siblings, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-04-30 16:49 UTC (permalink / raw)
  To: u-boot

Hi Angus,

On Sun, Apr 30, 2017 at 10:26:15AM -0600, Angus Ainslie wrote:
> On 2017-04-30 07:29, Antoine Tenart wrote:
> > 
> > Respin this series that was sent in October 2016, with comments and new
> > psci aware SoC taken into account.
> > 
> > This series adds an implementation of the psci suspend function for both
> > sun5i and sun7i. This was tested on Nextthing's CHIP, using cpuidle in
> > Linux.
> > 
> 
> Does this get suspend to memory working on the Nextthing CHIP ?

No, this is used for CPU idle.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170430/c83887e1/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function
  2017-04-30 16:49   ` Antoine Tenart
@ 2017-04-30 23:26     ` Angus Ainslie
  2017-05-02  7:14       ` Antoine Tenart
  0 siblings, 1 reply; 34+ messages in thread
From: Angus Ainslie @ 2017-04-30 23:26 UTC (permalink / raw)
  To: u-boot

On 2017-04-30 10:49, Hi  Tenart wrote:
>> 
>> Does this get suspend to memory working on the Nextthing CHIP ?
> 
> No, this is used for CPU idle.

Hi Antoine

Does a different PSCI function need to be implemented in u-boot to 
support suspend or does it need some kernel work ?

Thanks
Angus

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs Antoine Tenart
@ 2017-05-01 15:12   ` Tom Warren
  2017-05-01 15:34     ` Stephen Warren
  2017-05-02  6:39     ` Antoine Tenart
  0 siblings, 2 replies; 34+ messages in thread
From: Tom Warren @ 2017-05-01 15:12 UTC (permalink / raw)
  To: u-boot

Adding Stephen and Thierry for review/input.  Antoine - what testing did you do?

Tom

> -----Original Message-----
> From: Antoine Tenart [mailto:antoine.tenart at free-electrons.com]
> Sent: Sunday, April 30, 2017 6:30 AM
> To: maxime.ripard at free-electrons.com; wens at csie.org; jagan at openedev.com
> Cc: Antoine Tenart <antoine.tenart@free-electrons.com>; u-
> boot at lists.denx.de; Tom Warren <TWarren@nvidia.com>
> Subject: [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs
> 
> Select the newly introduced ARM_GIC option to the relevant configuration
> which also have a psci implementation.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> Cc: Tom Warren <twarren@nvidia.com>
> ---
>  arch/arm/mach-tegra/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> index c67ffa5a2356..01f27b465bfe 100644
> --- a/arch/arm/mach-tegra/Kconfig
> +++ b/arch/arm/mach-tegra/Kconfig
> @@ -22,6 +22,7 @@ config TEGRA_IVC
> 
>  config TEGRA_COMMON
>  	bool "Tegra common options"
> +	select ARM_GIC
>  	select CLK
>  	select DM
>  	select DM_ETH
> --
> 2.11.0

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs
  2017-05-01 15:12   ` Tom Warren
@ 2017-05-01 15:34     ` Stephen Warren
  2017-05-02  6:51       ` Antoine Tenart
  2017-05-02  6:39     ` Antoine Tenart
  1 sibling, 1 reply; 34+ messages in thread
From: Stephen Warren @ 2017-05-01 15:34 UTC (permalink / raw)
  To: u-boot


On 05/01/2017 09:12 AM, Tom Warren wrote:
> Adding Stephen and Thierry for review/input.  Antoine - what testing did you do?
>
> Antoine Tenart wrote at Sunday, April 30, 2017 6:30 AM:
>>
>> Select the newly introduced ARM_GIC option to the relevant configuration
>> which also have a psci implementation.

It's true that all Tegra have ARM_GIC.

However, the commit description makes it sound as if enabling ARM_GIC 
enables PSCI support. That doesn't seem likely, but if it is true, I 
don't think this patch is correct. What is the actual implication of 
this patch?

>> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
>> Cc: Tom Warren <twarren@nvidia.com>
>> ---
>>  arch/arm/mach-tegra/Kconfig | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
>> index c67ffa5a2356..01f27b465bfe 100644
>> --- a/arch/arm/mach-tegra/Kconfig
>> +++ b/arch/arm/mach-tegra/Kconfig
>> @@ -22,6 +22,7 @@ config TEGRA_IVC
>>
>>  config TEGRA_COMMON
>>  	bool "Tegra common options"
>> +	select ARM_GIC
>>  	select CLK
>>  	select DM
>>  	select DM_ETH

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function Antoine Tenart
@ 2017-05-01 21:13   ` Maxime Ripard
  2017-05-02  7:04     ` Antoine Tenart
  0 siblings, 1 reply; 34+ messages in thread
From: Maxime Ripard @ 2017-05-01 21:13 UTC (permalink / raw)
  To: u-boot

Hi Antoine,

On Sun, Apr 30, 2017 at 03:29:54PM +0200, Antoine Tenart wrote:
> +/*
> + * The PSCI suspend function switch cpuclk to another source and disable
> + * pll1. As this function is called per-CPU, it should only do this when
> + * all the CPUs are in idle state.
> + *
> + * The 'cnt' variable keeps track of the number of CPU which are in the idle
> + * state. The last one setup cpuclk for idle.
> + *
> + * The 'clk_state' varibale holds the cpu clk state (idle or normal).
> + */
> +atomic_t __secure_data cnt, clk_state;
> +
> +#define CLK_NORMAL	0
> +#define CLK_IDLE	1
> +
> +static void __secure sunxi_clock_enter_idle(struct sunxi_ccm_reg *ccm)
> +{
> +	/* switch cpuclk to osc24m */
> +	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> +			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> +
> +	/* disable pll1 */
> +	clrbits_le32(&ccm->pll1_cfg, CCM_PLL1_CTRL_EN);
> +
> +#ifndef CONFIG_MACH_SUN7I
> +	/*
> +	 * Switch cpuclk to losc. Based on my experience this didn't worked for
> +	 * sun7i, hence the ifndef.
> +	 */
> +	clrbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT);
> +#endif

Do we enter idle per-core, or is it a cluster-wide state?

The A20 has a single clock for both CPUs, so that might explain why it
didn't work for you: if you enter idle for only one core, and change
the clock for both, then you're probably crashing the second core in
the process.

> +static void __secure sunxi_clock_leave_idle(struct sunxi_ccm_reg *ccm)
> +{
> +#ifndef CONFIG_MACH_SUN7I
> +	/* switch cpuclk to osc24m */
> +	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> +			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> +#endif

Is that really needed? Whatever state we're in at this point, we just
want to switch back to the PLL1, right?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170501/f42327df/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 11/11] sun5i: boot in non-secure mode by default
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 11/11] sun5i: boot in non-secure mode by default Antoine Tenart
@ 2017-05-01 21:14   ` Maxime Ripard
  2017-05-02  6:54     ` Antoine Tenart
  0 siblings, 1 reply; 34+ messages in thread
From: Maxime Ripard @ 2017-05-01 21:14 UTC (permalink / raw)
  To: u-boot

Hi,

On Sun, Apr 30, 2017 at 03:29:56PM +0200, Antoine Tenart wrote:
> sun5i now implements the psci suspend function. In order to be used by
> the kernel, we should now boot in non-secure mode. Enable it by default.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  board/sunxi/Kconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> index e53cd9c21c33..043980b83831 100644
> --- a/board/sunxi/Kconfig
> +++ b/board/sunxi/Kconfig
> @@ -77,9 +77,12 @@ config MACH_SUN4I
>  config MACH_SUN5I
>  	bool "sun5i (Allwinner A13)"
>  	select CPU_V7
> +	select CPU_V7_HAS_NONSEC
> +	select ARCH_SUPPORT_PSCI
>  	select ARM_CORTEX_CPU_IS_UP
>  	select SUNXI_GEN_SUN4I
>  	select SUPPORT_SPL
> +	select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT

It would be better to sort these options.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170501/79098e91/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs
  2017-05-01 15:12   ` Tom Warren
  2017-05-01 15:34     ` Stephen Warren
@ 2017-05-02  6:39     ` Antoine Tenart
  2017-05-02  6:40       ` Antoine Tenart
  1 sibling, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  6:39 UTC (permalink / raw)
  To: u-boot

Hi Stephen,

On Mon, May 01, 2017 at 03:12:30PM +0000, Tom Warren wrote:
> Antoine - what testing did you do?

I tested using CPU idle with a mainline kernel on both the CHIP and an
A20 board (both have an Allwinner SoC). I also measured the overall
power consumption using the ACME cape from BayLibre.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/e9ad4419/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs
  2017-05-02  6:39     ` Antoine Tenart
@ 2017-05-02  6:40       ` Antoine Tenart
  0 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  6:40 UTC (permalink / raw)
  To: u-boot

On Tue, May 02, 2017 at 08:39:03AM +0200, Antoine Tenart wrote:
> Hi Stephen,

Of course I meant Tom...  Sorry for that.

Antoine

> On Mon, May 01, 2017 at 03:12:30PM +0000, Tom Warren wrote:
> > Antoine - what testing did you do?
> 
> I tested using CPU idle with a mainline kernel on both the CHIP and an
> A20 board (both have an Allwinner SoC). I also measured the overall
> power consumption using the ACME cape from BayLibre.
> 
> Antoine
> 
> -- 
> Antoine Ténart, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com



-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/be56740a/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs
  2017-05-01 15:34     ` Stephen Warren
@ 2017-05-02  6:51       ` Antoine Tenart
  2017-05-02 14:58         ` Stephen Warren
  0 siblings, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  6:51 UTC (permalink / raw)
  To: u-boot

Hi Stephen,

On Mon, May 01, 2017 at 09:34:43AM -0600, Stephen Warren wrote:
> > Antoine Tenart wrote at Sunday, April 30, 2017 6:30 AM:
> > > 
> > > Select the newly introduced ARM_GIC option to the relevant configuration
> > > which also have a psci implementation.
> 
> It's true that all Tegra have ARM_GIC.
> 
> However, the commit description makes it sound as if enabling ARM_GIC
> enables PSCI support. That doesn't seem likely, but if it is true, I don't
> think this patch is correct. What is the actual implication of this patch?

The commit description is misleading then. Selecting ARM_GIC doesn't
enables PSCI support. However the common PSCI code is using the GIC
which is not possible on all SoC. This new ARM_GIC option helps using
(or not) the GIC specific code in PSCI. So, it's important to select
this new option on a SoC previously supporting PSCI in U-Boot but it
won't do anything if the SoC doesn't have a PSCI implementation.

As for Tegra SoCs, a few of them provide a PSCI implementation. At first
I selected ARM_GIC only for those providing a PSCI implementation but we
decided to do this directly for all Tegra SoCs (in the v3 thread) as
this option doesn't do anything not related to PSCI, and because all
Tegra SoCs do have a GIC.

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/4f8551da/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 11/11] sun5i: boot in non-secure mode by default
  2017-05-01 21:14   ` Maxime Ripard
@ 2017-05-02  6:54     ` Antoine Tenart
  2017-05-02  7:34       ` Maxime Ripard
  0 siblings, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  6:54 UTC (permalink / raw)
  To: u-boot

Hi Maxime,

On Mon, May 01, 2017 at 11:14:22PM +0200, Maxime Ripard wrote:
> On Sun, Apr 30, 2017 at 03:29:56PM +0200, Antoine Tenart wrote:
> > sun5i now implements the psci suspend function. In order to be used by
> > the kernel, we should now boot in non-secure mode. Enable it by default.
> > 
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
> >  board/sunxi/Kconfig | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> > index e53cd9c21c33..043980b83831 100644
> > --- a/board/sunxi/Kconfig
> > +++ b/board/sunxi/Kconfig
> > @@ -77,9 +77,12 @@ config MACH_SUN4I
> >  config MACH_SUN5I
> >  	bool "sun5i (Allwinner A13)"
> >  	select CPU_V7
> > +	select CPU_V7_HAS_NONSEC
> > +	select ARCH_SUPPORT_PSCI
> >  	select ARM_CORTEX_CPU_IS_UP
> >  	select SUNXI_GEN_SUN4I
> >  	select SUPPORT_SPL
> > +	select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT
> 
> It would be better to sort these options.

I followed what was done for the other MACH_SUN* definitions, assuming
there was some logic behind it. Otherwise I agree sorted options are
better.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/bc9a73cc/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-05-01 21:13   ` Maxime Ripard
@ 2017-05-02  7:04     ` Antoine Tenart
  2017-05-02  7:23       ` Antoine Tenart
  2017-05-02  7:56       ` Maxime Ripard
  0 siblings, 2 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  7:04 UTC (permalink / raw)
  To: u-boot

Hi Maxime,

On Mon, May 01, 2017 at 11:13:27PM +0200, Maxime Ripard wrote:
> On Sun, Apr 30, 2017 at 03:29:54PM +0200, Antoine Tenart wrote:
> > +
> > +static void __secure sunxi_clock_enter_idle(struct sunxi_ccm_reg *ccm)
> > +{
> > +	/* switch cpuclk to osc24m */
> > +	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> > +			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> > +
> > +	/* disable pll1 */
> > +	clrbits_le32(&ccm->pll1_cfg, CCM_PLL1_CTRL_EN);
> > +
> > +#ifndef CONFIG_MACH_SUN7I
> > +	/*
> > +	 * Switch cpuclk to losc. Based on my experience this didn't worked for
> > +	 * sun7i, hence the ifndef.
> > +	 */
> > +	clrbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT);
> > +#endif
> 
> Do we enter idle per-core, or is it a cluster-wide state?

This is cluster-wise, as sunxi_clock_enter_idle() is only called when
all CPU are in IDLE state (called the PSCI suspend function). See
psci_cpu_suspend().

> The A20 has a single clock for both CPUs, so that might explain why it
> didn't work for you: if you enter idle for only one core, and change
> the clock for both, then you're probably crashing the second core in
> the process.
> 
> > +static void __secure sunxi_clock_leave_idle(struct sunxi_ccm_reg *ccm)
> > +{
> > +#ifndef CONFIG_MACH_SUN7I
> > +	/* switch cpuclk to osc24m */
> > +	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> > +			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> > +#endif
> 
> Is that really needed? Whatever state we're in at this point, we just
> want to switch back to the PLL1, right?

I think it wasn't working for some reasons if we didn't switch to osc24m
first, but that was a while ago. I can test again.

Thanks,
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/e6855a3a/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function
  2017-04-30 23:26     ` Angus Ainslie
@ 2017-05-02  7:14       ` Antoine Tenart
  0 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  7:14 UTC (permalink / raw)
  To: u-boot

On Sun, Apr 30, 2017 at 05:26:03PM -0600, Angus Ainslie wrote:
> On 2017-04-30 10:49, Hi  Tenart wrote:
> > > 
> > > Does this get suspend to memory working on the Nextthing CHIP ?
> > 
> > No, this is used for CPU idle.
> 
> Does a different PSCI function need to be implemented in u-boot to support
> suspend or does it need some kernel work ?

I think there is another PSCI function for this. But you would also need
kernel work for all device drivers to support to be suspended and
resumed.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/3e260147/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-05-02  7:04     ` Antoine Tenart
@ 2017-05-02  7:23       ` Antoine Tenart
  2017-05-02  7:27         ` Chen-Yu Tsai
  2017-05-02  7:56       ` Maxime Ripard
  1 sibling, 1 reply; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  7:23 UTC (permalink / raw)
  To: u-boot

On Tue, May 02, 2017 at 09:04:18AM +0200, Antoine Tenart wrote:
> On Mon, May 01, 2017 at 11:13:27PM +0200, Maxime Ripard wrote:
> > On Sun, Apr 30, 2017 at 03:29:54PM +0200, Antoine Tenart wrote:
> > 
> > > +static void __secure sunxi_clock_leave_idle(struct sunxi_ccm_reg *ccm)
> > > +{
> > > +#ifndef CONFIG_MACH_SUN7I
> > > +	/* switch cpuclk to osc24m */
> > > +	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> > > +			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> > > +#endif
> > 
> > Is that really needed? Whatever state we're in at this point, we just
> > want to switch back to the PLL1, right?
> 
> I think it wasn't working for some reasons if we didn't switch to osc24m
> first, but that was a while ago. I can test again.

It seems to work without switching back to osc24m first. I'll remove
this.

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/b16e0211/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-05-02  7:23       ` Antoine Tenart
@ 2017-05-02  7:27         ` Chen-Yu Tsai
  2017-05-02  7:33           ` Maxime Ripard
  0 siblings, 1 reply; 34+ messages in thread
From: Chen-Yu Tsai @ 2017-05-02  7:27 UTC (permalink / raw)
  To: u-boot

On Tue, May 2, 2017 at 3:23 PM, Antoine Tenart
<antoine.tenart@free-electrons.com> wrote:
> On Tue, May 02, 2017 at 09:04:18AM +0200, Antoine Tenart wrote:
>> On Mon, May 01, 2017 at 11:13:27PM +0200, Maxime Ripard wrote:
>> > On Sun, Apr 30, 2017 at 03:29:54PM +0200, Antoine Tenart wrote:
>> >
>> > > +static void __secure sunxi_clock_leave_idle(struct sunxi_ccm_reg *ccm)
>> > > +{
>> > > +#ifndef CONFIG_MACH_SUN7I
>> > > + /* switch cpuclk to osc24m */
>> > > + clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
>> > > +                 CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
>> > > +#endif
>> >
>> > Is that really needed? Whatever state we're in at this point, we just
>> > want to switch back to the PLL1, right?
>>
>> I think it wasn't working for some reasons if we didn't switch to osc24m
>> first, but that was a while ago. I can test again.
>
> It seems to work without switching back to osc24m first. I'll remove
> this.

This is probably going to work badly with the sunxi-ng clock driver
in Linux, which will temporarily switch over to osc24M while changing
the clock rate of PLL1, then switch back.

ChenYu

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-05-02  7:27         ` Chen-Yu Tsai
@ 2017-05-02  7:33           ` Maxime Ripard
  2017-05-02  7:42             ` Antoine Tenart
  0 siblings, 1 reply; 34+ messages in thread
From: Maxime Ripard @ 2017-05-02  7:33 UTC (permalink / raw)
  To: u-boot

On Tue, May 02, 2017 at 03:27:41PM +0800, Chen-Yu Tsai wrote:
> On Tue, May 2, 2017 at 3:23 PM, Antoine Tenart
> <antoine.tenart@free-electrons.com> wrote:
> > On Tue, May 02, 2017 at 09:04:18AM +0200, Antoine Tenart wrote:
> >> On Mon, May 01, 2017 at 11:13:27PM +0200, Maxime Ripard wrote:
> >> > On Sun, Apr 30, 2017 at 03:29:54PM +0200, Antoine Tenart wrote:
> >> >
> >> > > +static void __secure sunxi_clock_leave_idle(struct sunxi_ccm_reg *ccm)
> >> > > +{
> >> > > +#ifndef CONFIG_MACH_SUN7I
> >> > > + /* switch cpuclk to osc24m */
> >> > > + clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> >> > > +                 CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> >> > > +#endif
> >> >
> >> > Is that really needed? Whatever state we're in at this point, we just
> >> > want to switch back to the PLL1, right?
> >>
> >> I think it wasn't working for some reasons if we didn't switch to osc24m
> >> first, but that was a while ago. I can test again.
> >
> > It seems to work without switching back to osc24m first. I'll remove
> > this.
> 
> This is probably going to work badly with the sunxi-ng clock driver
> in Linux, which will temporarily switch over to osc24M while changing
> the clock rate of PLL1, then switch back.

It doesn't do so for the A20, only the later SoCs.

I'm not sure whether using cpufreq and cpuidle at the same time is
valid either.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/bb9f9bc2/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 11/11] sun5i: boot in non-secure mode by default
  2017-05-02  6:54     ` Antoine Tenart
@ 2017-05-02  7:34       ` Maxime Ripard
  0 siblings, 0 replies; 34+ messages in thread
From: Maxime Ripard @ 2017-05-02  7:34 UTC (permalink / raw)
  To: u-boot

On Tue, May 02, 2017 at 08:54:59AM +0200, Antoine Tenart wrote:
> Hi Maxime,
> 
> On Mon, May 01, 2017 at 11:14:22PM +0200, Maxime Ripard wrote:
> > On Sun, Apr 30, 2017 at 03:29:56PM +0200, Antoine Tenart wrote:
> > > sun5i now implements the psci suspend function. In order to be used by
> > > the kernel, we should now boot in non-secure mode. Enable it by default.
> > > 
> > > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > > ---
> > >  board/sunxi/Kconfig | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig
> > > index e53cd9c21c33..043980b83831 100644
> > > --- a/board/sunxi/Kconfig
> > > +++ b/board/sunxi/Kconfig
> > > @@ -77,9 +77,12 @@ config MACH_SUN4I
> > >  config MACH_SUN5I
> > >  	bool "sun5i (Allwinner A13)"
> > >  	select CPU_V7
> > > +	select CPU_V7_HAS_NONSEC
> > > +	select ARCH_SUPPORT_PSCI
> > >  	select ARM_CORTEX_CPU_IS_UP
> > >  	select SUNXI_GEN_SUN4I
> > >  	select SUPPORT_SPL
> > > +	select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT
> > 
> > It would be better to sort these options.
> 
> I followed what was done for the other MACH_SUN* definitions, assuming
> there was some logic behind it. Otherwise I agree sorted options are
> better.

If you can explain it to me then, I'm all ears ;)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/17af5de7/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-05-02  7:33           ` Maxime Ripard
@ 2017-05-02  7:42             ` Antoine Tenart
  0 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  7:42 UTC (permalink / raw)
  To: u-boot

On Tue, May 02, 2017 at 09:33:47AM +0200, Maxime Ripard wrote:
> On Tue, May 02, 2017 at 03:27:41PM +0800, Chen-Yu Tsai wrote:
> > On Tue, May 2, 2017 at 3:23 PM, Antoine Tenart
> > <antoine.tenart@free-electrons.com> wrote:
> > > On Tue, May 02, 2017 at 09:04:18AM +0200, Antoine Tenart wrote:
> > >> On Mon, May 01, 2017 at 11:13:27PM +0200, Maxime Ripard wrote:
> > >> > On Sun, Apr 30, 2017 at 03:29:54PM +0200, Antoine Tenart wrote:
> > >> >
> > >> > > +static void __secure sunxi_clock_leave_idle(struct sunxi_ccm_reg *ccm)
> > >> > > +{
> > >> > > +#ifndef CONFIG_MACH_SUN7I
> > >> > > + /* switch cpuclk to osc24m */
> > >> > > + clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> > >> > > +                 CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> > >> > > +#endif
> > >> >
> > >> > Is that really needed? Whatever state we're in at this point, we just
> > >> > want to switch back to the PLL1, right?
> > >>
> > >> I think it wasn't working for some reasons if we didn't switch to osc24m
> > >> first, but that was a while ago. I can test again.
> > >
> > > It seems to work without switching back to osc24m first. I'll remove
> > > this.
> > 
> > This is probably going to work badly with the sunxi-ng clock driver
> > in Linux, which will temporarily switch over to osc24M while changing
> > the clock rate of PLL1, then switch back.
> 
> It doesn't do so for the A20, only the later SoCs.
> 
> I'm not sure whether using cpufreq and cpuidle at the same time is
> valid either.

This is cluster wise, cpuclk with be switched to losc (or osc24m) only
if all CPU are in the idle state. Likewise, the first CPU to leave this
state will switch back cpuclk to pll1. So that shouldn't be an issue for
the sunxi-ng clock driver in Linux.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/c453e0ee/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-05-02  7:04     ` Antoine Tenart
  2017-05-02  7:23       ` Antoine Tenart
@ 2017-05-02  7:56       ` Maxime Ripard
  2017-05-02  8:06         ` Antoine Tenart
  1 sibling, 1 reply; 34+ messages in thread
From: Maxime Ripard @ 2017-05-02  7:56 UTC (permalink / raw)
  To: u-boot

On Tue, May 02, 2017 at 09:04:18AM +0200, Antoine Tenart wrote:
> Hi Maxime,
> 
> On Mon, May 01, 2017 at 11:13:27PM +0200, Maxime Ripard wrote:
> > On Sun, Apr 30, 2017 at 03:29:54PM +0200, Antoine Tenart wrote:
> > > +
> > > +static void __secure sunxi_clock_enter_idle(struct sunxi_ccm_reg *ccm)
> > > +{
> > > +	/* switch cpuclk to osc24m */
> > > +	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> > > +			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> > > +
> > > +	/* disable pll1 */
> > > +	clrbits_le32(&ccm->pll1_cfg, CCM_PLL1_CTRL_EN);
> > > +
> > > +#ifndef CONFIG_MACH_SUN7I
> > > +	/*
> > > +	 * Switch cpuclk to losc. Based on my experience this didn't worked for
> > > +	 * sun7i, hence the ifndef.
> > > +	 */
> > > +	clrbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT);
> > > +#endif
> > 
> > Do we enter idle per-core, or is it a cluster-wide state?
> 
> This is cluster-wise, as sunxi_clock_enter_idle() is only called when
> all CPU are in IDLE state (called the PSCI suspend function). See
> psci_cpu_suspend().

Hmm, ok. That's still a bit weird. The clock documentation says that
you should wait 8 cycles when you reparent, which is something you're
not doing. Maybe that can be the issue.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/035c13eb/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function
  2017-05-02  7:56       ` Maxime Ripard
@ 2017-05-02  8:06         ` Antoine Tenart
  0 siblings, 0 replies; 34+ messages in thread
From: Antoine Tenart @ 2017-05-02  8:06 UTC (permalink / raw)
  To: u-boot

On Tue, May 02, 2017 at 09:56:19AM +0200, Maxime Ripard wrote:
> On Tue, May 02, 2017 at 09:04:18AM +0200, Antoine Tenart wrote:
> > On Mon, May 01, 2017 at 11:13:27PM +0200, Maxime Ripard wrote:
> > > On Sun, Apr 30, 2017 at 03:29:54PM +0200, Antoine Tenart wrote:
> > > > +
> > > > +static void __secure sunxi_clock_enter_idle(struct sunxi_ccm_reg *ccm)
> > > > +{
> > > > +	/* switch cpuclk to osc24m */
> > > > +	clrsetbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT,
> > > > +			CPU_CLK_SRC_OSC24M << CPU_CLK_SRC_SHIFT);
> > > > +
> > > > +	/* disable pll1 */
> > > > +	clrbits_le32(&ccm->pll1_cfg, CCM_PLL1_CTRL_EN);
> > > > +
> > > > +#ifndef CONFIG_MACH_SUN7I
> > > > +	/*
> > > > +	 * Switch cpuclk to losc. Based on my experience this didn't worked for
> > > > +	 * sun7i, hence the ifndef.
> > > > +	 */
> > > > +	clrbits_le32(&ccm->cpu_ahb_apb0_cfg, 0x3 << CPU_CLK_SRC_SHIFT);
> > > > +#endif
> > > 
> > > Do we enter idle per-core, or is it a cluster-wide state?
> > 
> > This is cluster-wise, as sunxi_clock_enter_idle() is only called when
> > all CPU are in IDLE state (called the PSCI suspend function). See
> > psci_cpu_suspend().
> 
> Hmm, ok. That's still a bit weird. The clock documentation says that
> you should wait 8 cycles when you reparent, which is something you're
> not doing. Maybe that can be the issue.

That might explain the issue.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170502/a4fe024f/attachment.sig>

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs
  2017-05-02  6:51       ` Antoine Tenart
@ 2017-05-02 14:58         ` Stephen Warren
  0 siblings, 0 replies; 34+ messages in thread
From: Stephen Warren @ 2017-05-02 14:58 UTC (permalink / raw)
  To: u-boot

On 05/02/2017 12:51 AM, Antoine Tenart wrote:
> Hi Stephen,
>
> On Mon, May 01, 2017 at 09:34:43AM -0600, Stephen Warren wrote:
>>> Antoine Tenart wrote at Sunday, April 30, 2017 6:30 AM:
>>>>
>>>> Select the newly introduced ARM_GIC option to the relevant configuration
>>>> which also have a psci implementation.
>>
>> It's true that all Tegra have ARM_GIC.
>>
>> However, the commit description makes it sound as if enabling ARM_GIC
>> enables PSCI support. That doesn't seem likely, but if it is true, I don't
>> think this patch is correct. What is the actual implication of this patch?
>
> The commit description is misleading then. Selecting ARM_GIC doesn't
> enables PSCI support. However the common PSCI code is using the GIC
> which is not possible on all SoC. This new ARM_GIC option helps using
> (or not) the GIC specific code in PSCI. So, it's important to select
> this new option on a SoC previously supporting PSCI in U-Boot but it
> won't do anything if the SoC doesn't have a PSCI implementation.
>
> As for Tegra SoCs, a few of them provide a PSCI implementation. At first
> I selected ARM_GIC only for those providing a PSCI implementation but we
> decided to do this directly for all Tegra SoCs (in the v3 thread) as
> this option doesn't do anything not related to PSCI, and because all
> Tegra SoCs do have a GIC.

OK, then the patch content sounds fine. I'd suggest rewording the commit 
description to more fully explain the situation; something similar to 
what you wrote in email above. Thanks.

^ permalink raw reply	[flat|nested] 34+ messages in thread

* [U-Boot] [PATCH v4 04/11] arm: select ARM_GIC for SoCs having a psci implementation
  2017-04-30 13:29 ` [U-Boot] [PATCH v4 04/11] arm: select ARM_GIC for SoCs having a psci implementation Antoine Tenart
@ 2017-05-11  8:00   ` Masahiro Yamada
  0 siblings, 0 replies; 34+ messages in thread
From: Masahiro Yamada @ 2017-05-11  8:00 UTC (permalink / raw)
  To: u-boot

2017-04-30 22:29 GMT+09:00 Antoine Tenart <antoine.tenart@free-electrons.com>:
> Select the newly introduced ARM_GIC option to the relevant MACH
> configurations.
>
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> ---
>  arch/arm/Kconfig | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index c7aebae3b039..8b13f6909f9c 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -472,6 +472,7 @@ config ARCH_BCM283X
>
>  config TARGET_VEXPRESS_CA15_TC2
>         bool "Support vexpress_ca15_tc2"
> +       select ARM_GIC
>         select CPU_V7
>         select CPU_V7_HAS_NONSEC
>         select CPU_V7_HAS_VIRT
> @@ -556,14 +557,17 @@ config TARGET_BCM23550_W1D
>
>  config TARGET_BCM28155_AP
>         bool "Support bcm28155_ap"
> +       select ARM_GIC
>         select CPU_V7
>
>  config TARGET_BCMCYGNUS
>         bool "Support bcmcygnus"
> +       select ARM_GIC
>         select CPU_V7
>
>  config TARGET_BCMNSP
>         bool "Support bcmnsp"
> +       select ARM_GIC
>         select CPU_V7
>
>  config TARGET_BCMNS2
> @@ -622,6 +626,7 @@ config ARCH_MX7ULP
>
>  config ARCH_MX7
>         bool "Freescale MX7"
> +       select ARM_GIC
>         select CPU_V7
>         select SYS_FSL_HAS_SEC if SECURE_BOOT
>         select SYS_FSL_SEC_COMPAT_4
> @@ -995,6 +1000,7 @@ config TARGET_LS1012AFRDM
>
>  config TARGET_LS1021AQDS
>         bool "Support ls1021aqds"
> +       select ARM_GIC
>         select BOARD_LATE_INIT
>         select CPU_V7
>         select CPU_V7_HAS_NONSEC
> @@ -1008,6 +1014,7 @@ config TARGET_LS1021AQDS
>
>  config TARGET_LS1021ATWR
>         bool "Support ls1021atwr"
> +       select ARM_GIC
>         select BOARD_LATE_INIT
>         select CPU_V7
>         select CPU_V7_HAS_NONSEC
> @@ -1020,6 +1027,7 @@ config TARGET_LS1021ATWR
>
>  config TARGET_LS1021AIOT
>         bool "Support ls1021aiot"
> +       select ARM_GIC
>         select BOARD_LATE_INIT
>         select CPU_V7
>         select CPU_V7_HAS_NONSEC
> @@ -1100,7 +1108,9 @@ config TARGET_COLIBRI_PXA270
>
>  config ARCH_UNIPHIER
>         bool "Socionext UniPhier SoCs"
> +       select ARM_GIC
>         select BOARD_LATE_INIT
> +       select BLK
>         select CLK_UNIPHIER
>         select DM
>         select DM_GPIO


"select BLK" is totally unrelated, and wrong.
Please remove.

In 07/11, you add "select ARM_GIC" to ARCH_UNIPHIER_32BIT.

Here, "select ARM_GIC" to ARCH_UNIPHER
including 32bit and 64bit.


What is your intention for CONFIG_ARM_GIC?
As far as I see 08, this option is only
effective for ARMv7.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2017-05-11  8:00 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-30 13:29 [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 01/11] arm: add atomic functions with return support Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 02/11] arm: add the ARM_GIC configuration option Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 03/11] sunxi: select ARM_GIC for sun[6789]i Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 04/11] arm: select ARM_GIC for SoCs having a psci implementation Antoine Tenart
2017-05-11  8:00   ` Masahiro Yamada
2017-04-30 13:29 ` [U-Boot] [PATCH v4 05/11] tegra: select ARM_GIC for Tegra SoCs Antoine Tenart
2017-05-01 15:12   ` Tom Warren
2017-05-01 15:34     ` Stephen Warren
2017-05-02  6:51       ` Antoine Tenart
2017-05-02 14:58         ` Stephen Warren
2017-05-02  6:39     ` Antoine Tenart
2017-05-02  6:40       ` Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 06/11] mx7: select ARM_GIC Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 07/11] uniphier: " Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 08/11] arm: psci: protect GIC specific code with ARM_GIC Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 09/11] sun5/7i: add an implementation of the psci suspend function Antoine Tenart
2017-05-01 21:13   ` Maxime Ripard
2017-05-02  7:04     ` Antoine Tenart
2017-05-02  7:23       ` Antoine Tenart
2017-05-02  7:27         ` Chen-Yu Tsai
2017-05-02  7:33           ` Maxime Ripard
2017-05-02  7:42             ` Antoine Tenart
2017-05-02  7:56       ` Maxime Ripard
2017-05-02  8:06         ` Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 10/11] sun5i: add defines used by the PSCI code Antoine Tenart
2017-04-30 13:29 ` [U-Boot] [PATCH v4 11/11] sun5i: boot in non-secure mode by default Antoine Tenart
2017-05-01 21:14   ` Maxime Ripard
2017-05-02  6:54     ` Antoine Tenart
2017-05-02  7:34       ` Maxime Ripard
2017-04-30 16:26 ` [U-Boot] [PATCH v4 00/11] sunxi: sun5/7i: add the psci suspend function Angus Ainslie
2017-04-30 16:49   ` Antoine Tenart
2017-04-30 23:26     ` Angus Ainslie
2017-05-02  7:14       ` Antoine Tenart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.