* [PATCH 1/6] ARM: tegra: add an assembly marco to check Tegra SoC ID
2013-05-15 10:27 [PATCH 0/6] ARM: tegra114: add CPU hotplug support Joseph Lo
@ 2013-05-15 10:27 ` Joseph Lo
2013-05-15 22:43 ` Stephen Warren
2013-05-15 10:27 ` [PATCH 2/6] ARM: tegra: skip SCU and PL310 code when CPU is not Cortex-A9 Joseph Lo
` (5 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Joseph Lo @ 2013-05-15 10:27 UTC (permalink / raw)
To: linux-arm-kernel
There are some Tegra SoC ID checking code around the low level assembly
code. Adding a marco to replace them. For the single image to support all
the Tegra series, we may also need the marco in other common code. So we
make it become a marco for the usage.
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/fuse.h | 22 ++++++++++++----------
arch/arm/mach-tegra/reset-handler.S | 24 +++++++-----------------
arch/arm/mach-tegra/sleep.h | 9 +++++++++
3 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/arch/arm/mach-tegra/fuse.h b/arch/arm/mach-tegra/fuse.h
index aacc00d..def7968 100644
--- a/arch/arm/mach-tegra/fuse.h
+++ b/arch/arm/mach-tegra/fuse.h
@@ -19,16 +19,6 @@
#ifndef __MACH_TEGRA_FUSE_H
#define __MACH_TEGRA_FUSE_H
-enum tegra_revision {
- TEGRA_REVISION_UNKNOWN = 0,
- TEGRA_REVISION_A01,
- TEGRA_REVISION_A02,
- TEGRA_REVISION_A03,
- TEGRA_REVISION_A03p,
- TEGRA_REVISION_A04,
- TEGRA_REVISION_MAX,
-};
-
#define SKU_ID_T20 8
#define SKU_ID_T25SE 20
#define SKU_ID_AP25 23
@@ -40,6 +30,17 @@ enum tegra_revision {
#define TEGRA30 0x30
#define TEGRA114 0x35
+#ifndef __ASSEMBLY__
+enum tegra_revision {
+ TEGRA_REVISION_UNKNOWN = 0,
+ TEGRA_REVISION_A01,
+ TEGRA_REVISION_A02,
+ TEGRA_REVISION_A03,
+ TEGRA_REVISION_A03p,
+ TEGRA_REVISION_A04,
+ TEGRA_REVISION_MAX,
+};
+
extern int tegra_sku_id;
extern int tegra_cpu_process_id;
extern int tegra_core_process_id;
@@ -72,5 +73,6 @@ void tegra114_init_speedo_data(void);
#else
static inline void tegra114_init_speedo_data(void) {}
#endif
+#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index e6de88a..525f1b9 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -22,11 +22,11 @@
#include <asm/hardware/cache-l2x0.h>
#include "flowctrl.h"
+#include "fuse.h"
#include "iomap.h"
#include "reset.h"
#include "sleep.h"
-#define APB_MISC_GP_HIDREV 0x804
#define PMC_SCRATCH41 0x140
#define RESET_DATA(x) ((TEGRA_RESET_##x)*4)
@@ -49,10 +49,7 @@ ENTRY(tegra_resume)
#ifdef CONFIG_ARCH_TEGRA_3x_SOC
/* Are we on Tegra20? */
- mov32 r6, TEGRA_APB_MISC_BASE
- ldr r0, [r6, #APB_MISC_GP_HIDREV]
- and r0, r0, #0xff00
- cmp r0, #(0x20 << 8)
+ tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r7
beq 1f @ Yes
/* Clear the flow controller flags for this CPU. */
mov32 r2, TEGRA_FLOW_CTRL_BASE + FLOW_CTRL_CPU0_CSR @ CPU0 CSR
@@ -115,13 +112,9 @@ ENTRY(__tegra_cpu_reset_handler)
cpsid aif, 0x13 @ SVC mode, interrupts disabled
- mov32 r6, TEGRA_APB_MISC_BASE
- ldr r6, [r6, #APB_MISC_GP_HIDREV]
- and r6, r6, #0xff00
-#ifdef CONFIG_ARCH_TEGRA_2x_SOC
-t20_check:
- cmp r6, #(0x20 << 8)
+ tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r5
bne after_t20_check
+#ifdef CONFIG_ARCH_TEGRA_2x_SOC
t20_errata:
# Tegra20 is a Cortex-A9 r1p1
mrc p15, 0, r0, c1, c0, 0 @ read system control register
@@ -132,8 +125,8 @@ t20_errata:
orr r0, r0, #1 << 11 @ erratum 751472
mcr p15, 0, r0, c15, c0, 1 @ write diagnostic register
b after_errata
-after_t20_check:
#endif
+after_t20_check:
#ifdef CONFIG_ARCH_TEGRA_3x_SOC
t30_check:
cmp r6, #(0x30 << 8)
@@ -163,7 +156,7 @@ after_errata:
#ifdef CONFIG_ARCH_TEGRA_2x_SOC
/* Are we on Tegra20? */
- cmp r6, #(0x20 << 8)
+ cmp r6, #(TEGRA20 << 8)
bne 1f
/* If not CPU0, don't let CPU0 reset CPU1 now that CPU1 is coming up. */
mov32 r5, TEGRA_PMC_BASE
@@ -210,10 +203,7 @@ __die:
mov32 r7, TEGRA_CLK_RESET_BASE
/* Are we on Tegra20? */
- mov32 r6, TEGRA_APB_MISC_BASE
- ldr r0, [r6, #APB_MISC_GP_HIDREV]
- and r0, r0, #0xff00
- cmp r0, #(0x20 << 8)
+ cmp r6, #(TEGRA20 << 8)
bne 1f
#ifdef CONFIG_ARCH_TEGRA_2x_SOC
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 2080fb1..7e9c9fe 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -85,6 +85,15 @@
dsb
.endm
+/* Macro to check Tegra revision */
+#define APB_MISC_GP_HIDREV 0x804
+.macro tegra_check_soc_id rev, base, tmp1, tmp2
+ mov32 \tmp2, \base
+ ldr \tmp1, [\tmp2, #APB_MISC_GP_HIDREV]
+ and \tmp1, \tmp1, #0xff00
+ cmp \tmp1, #(\rev << 8)
+.endm
+
/* Macro to resume & re-enable L2 cache */
#ifndef L2X0_CTRL_EN
#define L2X0_CTRL_EN 1
--
1.8.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 1/6] ARM: tegra: add an assembly marco to check Tegra SoC ID
2013-05-15 10:27 ` [PATCH 1/6] ARM: tegra: add an assembly marco to check Tegra SoC ID Joseph Lo
@ 2013-05-15 22:43 ` Stephen Warren
2013-05-16 10:09 ` Joseph Lo
0 siblings, 1 reply; 26+ messages in thread
From: Stephen Warren @ 2013-05-15 22:43 UTC (permalink / raw)
To: linux-arm-kernel
On 05/15/2013 04:27 AM, Joseph Lo wrote:
> There are some Tegra SoC ID checking code around the low level assembly
> code. Adding a marco to replace them. For the single image to support all
> the Tegra series, we may also need the marco in other common code. So we
> make it become a marco for the usage.
I'm not sure this patch doesn't obfuscate the code too much. The big
issue I see is:
> @@ -115,13 +112,9 @@ ENTRY(__tegra_cpu_reset_handler)
>
> cpsid aif, 0x13 @ SVC mode, interrupts disabled
>
> - mov32 r6, TEGRA_APB_MISC_BASE
> - ldr r6, [r6, #APB_MISC_GP_HIDREV]
> - and r6, r6, #0xff00
> -#ifdef CONFIG_ARCH_TEGRA_2x_SOC
> -t20_check:
> - cmp r6, #(0x20 << 8)
> + tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r5
Here, we replace all the code with the new macro, ...
> #ifdef CONFIG_ARCH_TEGRA_2x_SOC
> /* Are we on Tegra20? */
> - cmp r6, #(0x20 << 8)
> + cmp r6, #(TEGRA20 << 8)
But here we still have to write out the cmp instructions.
It may be reasonable to create a macro to read/mask/shift the SoC ID,
similar to the existing cpu_id macro, i.e. roughly:
/* Macro to check Tegra revision */
+#define APB_MISC_GP_HIDREV 0x804
+.macro tegra_soc_id base, tmp1, tmp2
+ mov32 \tmp2, \base
+ ldr \tmp1, [\tmp2, #APB_MISC_GP_HIDREV]
+ and \tmp1, \tmp1, #0xff00
+.endm
Also, I wonder:
(a) why the need to pass TEGRA_APB_MISC_BASE into the macro; can't the
macro just hard-code that value since it's always the same?
(b) Why need two registers passed to the macro. At least one of the code
segments you've replaced with the macro uses a single register instead:
- mov32 r6, TEGRA_APB_MISC_BASE
- ldr r6, [r6, #APB_MISC_GP_HIDREV]
- and r6, r6, #0xff00
Wouldn't that be a better implementation of the macro? I don't think
relying on \tmp2 containing "base" after the macro invocation would be a
good idea, since that's rather like looking inside the black box.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/6] ARM: tegra: add an assembly marco to check Tegra SoC ID
2013-05-15 22:43 ` Stephen Warren
@ 2013-05-16 10:09 ` Joseph Lo
2013-05-16 18:21 ` Stephen Warren
0 siblings, 1 reply; 26+ messages in thread
From: Joseph Lo @ 2013-05-16 10:09 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2013-05-16 at 06:43 +0800, Stephen Warren wrote:
> On 05/15/2013 04:27 AM, Joseph Lo wrote:
> > There are some Tegra SoC ID checking code around the low level assembly
> > code. Adding a marco to replace them. For the single image to support all
> > the Tegra series, we may also need the marco in other common code. So we
> > make it become a marco for the usage.
>
> I'm not sure this patch doesn't obfuscate the code too much. The big
> issue I see is:
>
> > @@ -115,13 +112,9 @@ ENTRY(__tegra_cpu_reset_handler)
> >
> > cpsid aif, 0x13 @ SVC mode, interrupts disabled
> >
> > - mov32 r6, TEGRA_APB_MISC_BASE
> > - ldr r6, [r6, #APB_MISC_GP_HIDREV]
> > - and r6, r6, #0xff00
> > -#ifdef CONFIG_ARCH_TEGRA_2x_SOC
> > -t20_check:
> > - cmp r6, #(0x20 << 8)
> > + tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r5
>
> Here, we replace all the code with the new macro, ...
>
> > #ifdef CONFIG_ARCH_TEGRA_2x_SOC
> > /* Are we on Tegra20? */
> > - cmp r6, #(0x20 << 8)
> > + cmp r6, #(TEGRA20 << 8)
>
> But here we still have to write out the cmp instructions.
>
The marco did "cmp" once and move the soc_id into r6. But the flags may
be changed after running some other codes, we need to "cmp" again to get
the correct flag.
> It may be reasonable to create a macro to read/mask/shift the SoC ID,
> similar to the existing cpu_id macro, i.e. roughly:
>
> /* Macro to check Tegra revision */
> +#define APB_MISC_GP_HIDREV 0x804
> +.macro tegra_soc_id base, tmp1, tmp2
> + mov32 \tmp2, \base
> + ldr \tmp1, [\tmp2, #APB_MISC_GP_HIDREV]
> + and \tmp1, \tmp1, #0xff00
> +.endm
>
> Also, I wonder:
>
> (a) why the need to pass TEGRA_APB_MISC_BASE into the macro; can't the
> macro just hard-code that value since it's always the same?
>
Because this marco will be used in virtual address space too, I need to
pass different base addr here.
> (b) Why need two registers passed to the macro. At least one of the code
> segments you've replaced with the macro uses a single register instead:
>
> - mov32 r6, TEGRA_APB_MISC_BASE
> - ldr r6, [r6, #APB_MISC_GP_HIDREV]
> - and r6, r6, #0xff00
>
> Wouldn't that be a better implementation of the macro? I don't think
> relying on \tmp2 containing "base" after the macro invocation would be a
> good idea, since that's rather like looking inside the black box.
OK. I can remove one tmp register here.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 1/6] ARM: tegra: add an assembly marco to check Tegra SoC ID
2013-05-16 10:09 ` Joseph Lo
@ 2013-05-16 18:21 ` Stephen Warren
0 siblings, 0 replies; 26+ messages in thread
From: Stephen Warren @ 2013-05-16 18:21 UTC (permalink / raw)
To: linux-arm-kernel
On 05/16/2013 04:09 AM, Joseph Lo wrote:
> On Thu, 2013-05-16 at 06:43 +0800, Stephen Warren wrote:
>> On 05/15/2013 04:27 AM, Joseph Lo wrote:
>>> There are some Tegra SoC ID checking code around the low level assembly
>>> code. Adding a marco to replace them. For the single image to support all
>>> the Tegra series, we may also need the marco in other common code. So we
>>> make it become a marco for the usage.
>>
>> I'm not sure this patch doesn't obfuscate the code too much. The big
>> issue I see is:
>>
>>> @@ -115,13 +112,9 @@ ENTRY(__tegra_cpu_reset_handler)
>>>
>>> cpsid aif, 0x13 @ SVC mode, interrupts disabled
>>>
>>> - mov32 r6, TEGRA_APB_MISC_BASE
>>> - ldr r6, [r6, #APB_MISC_GP_HIDREV]
>>> - and r6, r6, #0xff00
>>> -#ifdef CONFIG_ARCH_TEGRA_2x_SOC
>>> -t20_check:
>>> - cmp r6, #(0x20 << 8)
>>> + tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r5
>>
>> Here, we replace all the code with the new macro, ...
>>
>>> #ifdef CONFIG_ARCH_TEGRA_2x_SOC
>>> /* Are we on Tegra20? */
>>> - cmp r6, #(0x20 << 8)
>>> + cmp r6, #(TEGRA20 << 8)
>>
>> But here we still have to write out the cmp instructions.
>>
> The marco did "cmp" once and move the soc_id into r6. But the flags may
> be changed after running some other codes, we need to "cmp" again to get
> the correct flag.
Yes, I understand that. The issue is that writing the macro didn't
remove all the code, so it seems like it'd be best if the macro only
included the common code, and anything that needs to be repeated at each
test site not be part of the macro.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/6] ARM: tegra: skip SCU and PL310 code when CPU is not Cortex-A9
2013-05-15 10:27 [PATCH 0/6] ARM: tegra114: add CPU hotplug support Joseph Lo
2013-05-15 10:27 ` [PATCH 1/6] ARM: tegra: add an assembly marco to check Tegra SoC ID Joseph Lo
@ 2013-05-15 10:27 ` Joseph Lo
2013-05-15 22:48 ` Stephen Warren
2013-05-15 10:27 ` [PATCH 3/6] ARM: tegra: make tegra_resume can work for Tegra114 Joseph Lo
` (4 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Joseph Lo @ 2013-05-15 10:27 UTC (permalink / raw)
To: linux-arm-kernel
For supporting single image on all Tegra series, we need to skip some HW
support code for Cortex-A9 only.
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/sleep.S | 8 +++++---
arch/arm/mach-tegra/sleep.h | 24 ++++++++++++++++++------
2 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
index 364d845..9daaef2 100644
--- a/arch/arm/mach-tegra/sleep.S
+++ b/arch/arm/mach-tegra/sleep.S
@@ -106,9 +106,11 @@ ENTRY(tegra_shut_off_mmu)
isb
#ifdef CONFIG_CACHE_L2X0
/* Disable L2 cache */
- mov32 r4, TEGRA_ARM_PERIF_BASE + 0x3000
- mov r5, #0
- str r5, [r4, #L2X0_CTRL]
+ check_cpu_part_num 0xc09, r9, r10
+ movweq r4, #:lower16:(TEGRA_ARM_PERIF_BASE + 0x3000)
+ movteq r4, #:upper16:(TEGRA_ARM_PERIF_BASE + 0x3000)
+ moveq r5, #0
+ streq r5, [r4, #L2X0_CTRL]
#endif
mov pc, r0
ENDPROC(tegra_shut_off_mmu)
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index 7e9c9fe..c2cac9a 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -70,19 +70,31 @@
movt \reg, #:upper16:\val
.endm
+/* Marco to check CPU part num */
+.macro check_cpu_part_num part_num, tmp1, tmp2
+ mrc p15, 0, \tmp1, c0, c0, 0
+ ubfx \tmp1, \tmp1, #4, #12
+ mov32 \tmp2, \part_num
+ cmp \tmp1, \tmp2
+.endm
+
/* Macro to exit SMP coherency. */
.macro exit_smp, tmp1, tmp2
mrc p15, 0, \tmp1, c1, c0, 1 @ ACTLR
bic \tmp1, \tmp1, #(1<<6) | (1<<0) @ clear ACTLR.SMP | ACTLR.FW
mcr p15, 0, \tmp1, c1, c0, 1 @ ACTLR
isb
- cpu_id \tmp1
- mov \tmp1, \tmp1, lsl #2
- mov \tmp2, #0xf
- mov \tmp2, \tmp2, lsl \tmp1
- mov32 \tmp1, TEGRA_ARM_PERIF_VIRT + 0xC
- str \tmp2, [\tmp1] @ invalidate SCU tags for CPU
+#ifdef CONFIG_HAVE_ARM_SCU
+ check_cpu_part_num 0xc09, \tmp1, \tmp2
+ mrceq p15, 0, \tmp1, c0, c0, 5
+ andeq \tmp1, \tmp1, #0xF
+ moveq \tmp1, \tmp1, lsl #2
+ moveq \tmp2, #0xf
+ moveq \tmp2, \tmp2, lsl \tmp1
+ ldreq \tmp1, =(TEGRA_ARM_PERIF_VIRT + 0xC)
+ streq \tmp2, [\tmp1] @ invalidate SCU tags for CPU
dsb
+#endif
.endm
/* Macro to check Tegra revision */
--
1.8.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 2/6] ARM: tegra: skip SCU and PL310 code when CPU is not Cortex-A9
2013-05-15 10:27 ` [PATCH 2/6] ARM: tegra: skip SCU and PL310 code when CPU is not Cortex-A9 Joseph Lo
@ 2013-05-15 22:48 ` Stephen Warren
2013-05-16 10:13 ` Joseph Lo
0 siblings, 1 reply; 26+ messages in thread
From: Stephen Warren @ 2013-05-15 22:48 UTC (permalink / raw)
To: linux-arm-kernel
On 05/15/2013 04:27 AM, Joseph Lo wrote:
> For supporting single image on all Tegra series, we need to skip some HW
> support code for Cortex-A9 only.
> diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
> + check_cpu_part_num 0xc09, r9, r10
> + movweq r4, #:lower16:(TEGRA_ARM_PERIF_BASE + 0x3000)
> + movteq r4, #:upper16:(TEGRA_ARM_PERIF_BASE + 0x3000)
> + moveq r5, #0
> + streq r5, [r4, #L2X0_CTRL]
Do those conditional instructions need a Thumb iteq wrapped around them
in order to compile in Thumb2 mode? Same comment for the other change,
although IIRC iteq only supports 4 instructions at a time, so maybe a
branch would be better there. Using branches might also reduce the size
of the diff, and make the change more obvious.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 2/6] ARM: tegra: skip SCU and PL310 code when CPU is not Cortex-A9
2013-05-15 22:48 ` Stephen Warren
@ 2013-05-16 10:13 ` Joseph Lo
0 siblings, 0 replies; 26+ messages in thread
From: Joseph Lo @ 2013-05-16 10:13 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2013-05-16 at 06:48 +0800, Stephen Warren wrote:
> On 05/15/2013 04:27 AM, Joseph Lo wrote:
> > For supporting single image on all Tegra series, we need to skip some HW
> > support code for Cortex-A9 only.
>
> > diff --git a/arch/arm/mach-tegra/sleep.S b/arch/arm/mach-tegra/sleep.S
>
> > + check_cpu_part_num 0xc09, r9, r10
> > + movweq r4, #:lower16:(TEGRA_ARM_PERIF_BASE + 0x3000)
> > + movteq r4, #:upper16:(TEGRA_ARM_PERIF_BASE + 0x3000)
> > + moveq r5, #0
> > + streq r5, [r4, #L2X0_CTRL]
>
> Do those conditional instructions need a Thumb iteq wrapped around them
> in order to compile in Thumb2 mode? Same comment for the other change,
> although IIRC iteq only supports 4 instructions at a time, so maybe a
> branch would be better there. Using branches might also reduce the size
> of the diff, and make the change more obvious.
IIRC, I had tested this patch series with THUMB2_KERNEL enabled. It's
OK. I can double confirm again later.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 3/6] ARM: tegra: make tegra_resume can work for Tegra114
2013-05-15 10:27 [PATCH 0/6] ARM: tegra114: add CPU hotplug support Joseph Lo
2013-05-15 10:27 ` [PATCH 1/6] ARM: tegra: add an assembly marco to check Tegra SoC ID Joseph Lo
2013-05-15 10:27 ` [PATCH 2/6] ARM: tegra: skip SCU and PL310 code when CPU is not Cortex-A9 Joseph Lo
@ 2013-05-15 10:27 ` Joseph Lo
2013-05-15 22:57 ` Stephen Warren
2013-05-15 10:27 ` [PATCH 4/6] ARM: tegra114: add power up sequence for warm boot CPU Joseph Lo
` (3 subsequent siblings)
6 siblings, 1 reply; 26+ messages in thread
From: Joseph Lo @ 2013-05-15 10:27 UTC (permalink / raw)
To: linux-arm-kernel
Tegra114 is an ARM Cortex-A15 based SoC and some of the flow controller
hardware behavior and configurations are different with other Tegra series.
We fix the common resume function of tegra_resume to make it can work on
Tegra114 by checking SoC ID. And also checking CPU primary part number to
isolate the support code for Cortex A9 and A15.
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/reset-handler.S | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index 525f1b9..893f6b7 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -47,22 +47,25 @@ ENTRY(tegra_resume)
THUMB( it ne )
bne cpu_resume @ no
-#ifdef CONFIG_ARCH_TEGRA_3x_SOC
+#ifndef CONFIG_ARCH_TEGRA_2x_SOC
/* Are we on Tegra20? */
tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r7
beq 1f @ Yes
/* Clear the flow controller flags for this CPU. */
- mov32 r2, TEGRA_FLOW_CTRL_BASE + FLOW_CTRL_CPU0_CSR @ CPU0 CSR
- ldr r1, [r2]
+ cpu_to_csr_req r1, r0
+ mov32 r2, TEGRA_FLOW_CTRL_BASE
+ ldr r1, [r2, r1]
/* Clear event & intr flag */
orr r1, r1, \
#FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG
- movw r0, #0x0FFD @ enable, cluster_switch, immed, & bitmaps
+ movw r0, #0x3FFD @ enable, cluster_switch, immed, & bitmaps
bic r1, r1, r0
str r1, [r2]
1:
#endif
+ check_cpu_part_num 0xc09, r8, r9
+ bne not_ca9
#ifdef CONFIG_HAVE_ARM_SCU
/* enable SCU */
mov32 r0, TEGRA_ARM_PERIF_BASE
@@ -73,6 +76,7 @@ ENTRY(tegra_resume)
/* L2 cache resume & re-enable */
l2_cache_resume r0, r1, r2, l2x0_saved_regs_addr
+not_ca9:
b cpu_resume
ENDPROC(tegra_resume)
--
1.8.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 3/6] ARM: tegra: make tegra_resume can work for Tegra114
2013-05-15 10:27 ` [PATCH 3/6] ARM: tegra: make tegra_resume can work for Tegra114 Joseph Lo
@ 2013-05-15 22:57 ` Stephen Warren
2013-05-16 10:35 ` Joseph Lo
0 siblings, 1 reply; 26+ messages in thread
From: Stephen Warren @ 2013-05-15 22:57 UTC (permalink / raw)
To: linux-arm-kernel
On 05/15/2013 04:27 AM, Joseph Lo wrote:
> Tegra114 is an ARM Cortex-A15 based SoC and some of the flow controller
I don't think the CPU type is the issue here. The issue is simply that
Tegra114's flow controller is different. Mentioning the CPU type seems
misleading.
> hardware behavior and configurations are different with other Tegra series.
> We fix the common resume function of tegra_resume to make it can work on
> Tegra114 by checking SoC ID. And also checking CPU primary part number to
> isolate the support code for Cortex A9 and A15.
> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> -#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> +#ifndef CONFIG_ARCH_TEGRA_2x_SOC
> /* Are we on Tegra20? */
> tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r7
> beq 1f @ Yes
> /* Clear the flow controller flags for this CPU. */
> - mov32 r2, TEGRA_FLOW_CTRL_BASE + FLOW_CTRL_CPU0_CSR @ CPU0 CSR
> - ldr r1, [r2]
> + cpu_to_csr_req r1, r0
Where is cpu_to_csr_req defined? grep can't find it in next-20130513,
and I don't see it added in this series.
This presumably changes behaviour on Tegra30; will this cause problems?
> + mov32 r2, TEGRA_FLOW_CTRL_BASE
> + ldr r1, [r2, r1]
> /* Clear event & intr flag */
> orr r1, r1, \
> #FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG
> - movw r0, #0x0FFD @ enable, cluster_switch, immed, & bitmaps
> + movw r0, #0x3FFD @ enable, cluster_switch, immed, & bitmaps
What does this change do; does the commend need updating to describe the
new bits that are set?
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 3/6] ARM: tegra: make tegra_resume can work for Tegra114
2013-05-15 22:57 ` Stephen Warren
@ 2013-05-16 10:35 ` Joseph Lo
2013-05-16 18:24 ` Stephen Warren
0 siblings, 1 reply; 26+ messages in thread
From: Joseph Lo @ 2013-05-16 10:35 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2013-05-16 at 06:57 +0800, Stephen Warren wrote:
> On 05/15/2013 04:27 AM, Joseph Lo wrote:
> > Tegra114 is an ARM Cortex-A15 based SoC and some of the flow controller
>
> I don't think the CPU type is the issue here. The issue is simply that
> Tegra114's flow controller is different. Mentioning the CPU type seems
> misleading.
>
> > hardware behavior and configurations are different with other Tegra series.
> > We fix the common resume function of tegra_resume to make it can work on
> > Tegra114 by checking SoC ID. And also checking CPU primary part number to
> > isolate the support code for Cortex A9 and A15.
>
> > diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
>
> > -#ifdef CONFIG_ARCH_TEGRA_3x_SOC
> > +#ifndef CONFIG_ARCH_TEGRA_2x_SOC
> > /* Are we on Tegra20? */
> > tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r7
> > beq 1f @ Yes
> > /* Clear the flow controller flags for this CPU. */
> > - mov32 r2, TEGRA_FLOW_CTRL_BASE + FLOW_CTRL_CPU0_CSR @ CPU0 CSR
> > - ldr r1, [r2]
> > + cpu_to_csr_req r1, r0
>
> Where is cpu_to_csr_req defined? grep can't find it in next-20130513,
> and I don't see it added in this series.
>
The cpu_to_csr_reg macro was defined in "sleep.h". I was used to
translate the bit offset of CPU number in CSR register. The code here
was used for CPU0 on Tegra30 only before. Because the Tegra114 had more
advance CPU power control. Each Core need run through here when power
gate resume.
> This presumably changes behaviour on Tegra30; will this cause problems?
>
No, I had verified on Tegra20, Tegra30 and Tegra114 of this series.
> > + mov32 r2, TEGRA_FLOW_CTRL_BASE
> > + ldr r1, [r2, r1]
> > /* Clear event & intr flag */
> > orr r1, r1, \
> > #FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG
> > - movw r0, #0x0FFD @ enable, cluster_switch, immed, & bitmaps
> > + movw r0, #0x3FFD @ enable, cluster_switch, immed, & bitmaps
>
> What does this change do; does the commend need updating to describe the
> new bits that are set?
It clear the some extensions for CPU power control on Tegra114.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 3/6] ARM: tegra: make tegra_resume can work for Tegra114
2013-05-16 10:35 ` Joseph Lo
@ 2013-05-16 18:24 ` Stephen Warren
0 siblings, 0 replies; 26+ messages in thread
From: Stephen Warren @ 2013-05-16 18:24 UTC (permalink / raw)
To: linux-arm-kernel
On 05/16/2013 04:35 AM, Joseph Lo wrote:
> On Thu, 2013-05-16 at 06:57 +0800, Stephen Warren wrote:
>> On 05/15/2013 04:27 AM, Joseph Lo wrote:
>>> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
>>> + cpu_to_csr_req r1, r0
>>
>> Where is cpu_to_csr_req defined? grep can't find it in next-20130513,
>> and I don't see it added in this series.
>
> The cpu_to_csr_reg macro was defined in "sleep.h".
So it is. I fail at grep.
> I was used to
> translate the bit offset of CPU number in CSR register. The code here
> was used for CPU0 on Tegra30 only before.
...
>> This presumably changes behaviour on Tegra30; will this cause problems?
Ah, OK.
>>> + mov32 r2, TEGRA_FLOW_CTRL_BASE
>>> + ldr r1, [r2, r1]
>>> /* Clear event & intr flag */
>>> orr r1, r1, \
>>> #FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG
>>> - movw r0, #0x0FFD @ enable, cluster_switch, immed, & bitmaps
>>> + movw r0, #0x3FFD @ enable, cluster_switch, immed, & bitmaps
>>
>> What does this change do; does the commend need updating to describe the
>> new bits that are set?
>
> It clear the some extensions for CPU power control on Tegra114.
OK; I was implying that perhaps the comment needs updating?
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 4/6] ARM: tegra114: add power up sequence for warm boot CPU
2013-05-15 10:27 [PATCH 0/6] ARM: tegra114: add CPU hotplug support Joseph Lo
` (2 preceding siblings ...)
2013-05-15 10:27 ` [PATCH 3/6] ARM: tegra: make tegra_resume can work for Tegra114 Joseph Lo
@ 2013-05-15 10:27 ` Joseph Lo
2013-05-15 10:27 ` [PATCH 5/6] clk: tegra114: implement wait_for_reset and disable_clock for tegra_cpu_car_ops Joseph Lo
` (2 subsequent siblings)
6 siblings, 0 replies; 26+ messages in thread
From: Joseph Lo @ 2013-05-15 10:27 UTC (permalink / raw)
To: linux-arm-kernel
For Tegra114, once the CPUs were powered up by PMC in cold boot flow. The
flow controller will maintain the power state and control power sequence
for each CPU by setting event trigger (e.g. CPU hotplug ,idle and
suspend power down/up).
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/flowctrl.h | 1 +
arch/arm/mach-tegra/platsmp.c | 25 ++++++++++++++++++++++++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-tegra/flowctrl.h b/arch/arm/mach-tegra/flowctrl.h
index 67eab56..7a29bae 100644
--- a/arch/arm/mach-tegra/flowctrl.h
+++ b/arch/arm/mach-tegra/flowctrl.h
@@ -25,6 +25,7 @@
#define FLOW_CTRL_WAITEVENT (2 << 29)
#define FLOW_CTRL_WAIT_FOR_INTERRUPT (4 << 29)
#define FLOW_CTRL_JTAG_RESUME (1 << 28)
+#define FLOW_CTRL_SCLK_RESUME (1 << 27)
#define FLOW_CTRL_HALT_CPU_IRQ (1 << 10)
#define FLOW_CTRL_HALT_CPU_FIQ (1 << 8)
#define FLOW_CTRL_CPU0_CSR 0x8
diff --git a/arch/arm/mach-tegra/platsmp.c b/arch/arm/mach-tegra/platsmp.c
index fad4226..554aedc 100644
--- a/arch/arm/mach-tegra/platsmp.c
+++ b/arch/arm/mach-tegra/platsmp.c
@@ -140,8 +140,31 @@ remove_clamps:
static int tegra114_boot_secondary(unsigned int cpu, struct task_struct *idle)
{
+ int ret = 0;
+
cpu = cpu_logical_map(cpu);
- return tegra_pmc_cpu_power_on(cpu);
+
+ if (cpumask_test_cpu(cpu, &tegra_cpu_init_mask)) {
+ /*
+ * Warm boot flow
+ * The flow controller in charge of the power state and
+ * control for each CPU.
+ */
+ /* set SCLK as event trigger for flow controller */
+ flowctrl_write_cpu_csr(cpu, 1);
+ flowctrl_write_cpu_halt(cpu,
+ FLOW_CTRL_WAITEVENT | FLOW_CTRL_SCLK_RESUME);
+ } else {
+ /*
+ * Cold boot flow
+ * The CPU is powered up by toggling PMC directly. It will
+ * also initial power state in flow controller. After that,
+ * the CPU's power state is maintained by flow controller.
+ */
+ ret = tegra_pmc_cpu_power_on(cpu);
+ }
+
+ return ret;
}
static int __cpuinit tegra_boot_secondary(unsigned int cpu,
--
1.8.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 5/6] clk: tegra114: implement wait_for_reset and disable_clock for tegra_cpu_car_ops
2013-05-15 10:27 [PATCH 0/6] ARM: tegra114: add CPU hotplug support Joseph Lo
` (3 preceding siblings ...)
2013-05-15 10:27 ` [PATCH 4/6] ARM: tegra114: add power up sequence for warm boot CPU Joseph Lo
@ 2013-05-15 10:27 ` Joseph Lo
2013-05-15 23:02 ` Stephen Warren
2013-05-16 19:17 ` Mike Turquette
2013-05-15 10:27 ` [PATCH 6/6] ARM: tegra114: add CPU hotplug support Joseph Lo
2013-05-15 23:38 ` [PATCH 0/6] " Stephen Warren
6 siblings, 2 replies; 26+ messages in thread
From: Joseph Lo @ 2013-05-15 10:27 UTC (permalink / raw)
To: linux-arm-kernel
The conventional CPU hotplug sequence on the other Tegra chips, we will also
clock gate the CPU in tegra_cpu_kill() after the CPU was power gated. For
Tegra114, the flow controller will clock gate the CPU after the power down
sequence. But we still need to implement a empty function for disable_clock
to avoid kernel warning message.
Cc: Mike Turquette <mturquette@linaro.org>
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
drivers/clk/tegra/clk-tegra114.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index d78e16e..40d939d 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -250,6 +250,9 @@
#define CLK_SOURCE_XUSB_DEV_SRC 0x60c
#define CLK_SOURCE_EMC 0x19c
+/* Tegra CPU clock and reset control regs */
+#define CLK_RST_CONTROLLER_CPU_CMPLX_STATUS 0x470
+
static int periph_clk_enb_refcnt[CLK_OUT_ENB_NUM * 32];
static void __iomem *clk_base;
@@ -2000,7 +2003,25 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base)
}
}
-static struct tegra_cpu_car_ops tegra114_cpu_car_ops;
+/* Tegra114 CPU clock and reset control functions */
+static void tegra114_wait_cpu_in_reset(u32 cpu)
+{
+ unsigned int reg;
+
+ do {
+ reg = readl(clk_base + CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
+ cpu_relax();
+ } while (!(reg & (1 << cpu))); /* check CPU been reset or not */
+}
+static void tegra114_disable_cpu_clock(u32 cpu)
+{
+ /* flow controller would take care in the power sequence. */
+}
+
+static struct tegra_cpu_car_ops tegra114_cpu_car_ops = {
+ .wait_for_reset = tegra114_wait_cpu_in_reset,
+ .disable_clock = tegra114_disable_cpu_clock,
+};
static const struct of_device_id pmc_match[] __initconst = {
{ .compatible = "nvidia,tegra114-pmc" },
--
1.8.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 5/6] clk: tegra114: implement wait_for_reset and disable_clock for tegra_cpu_car_ops
2013-05-15 10:27 ` [PATCH 5/6] clk: tegra114: implement wait_for_reset and disable_clock for tegra_cpu_car_ops Joseph Lo
@ 2013-05-15 23:02 ` Stephen Warren
2013-05-16 19:17 ` Mike Turquette
1 sibling, 0 replies; 26+ messages in thread
From: Stephen Warren @ 2013-05-15 23:02 UTC (permalink / raw)
To: linux-arm-kernel
On 05/15/2013 04:27 AM, Joseph Lo wrote:
> The conventional CPU hotplug sequence on the other Tegra chips, we will also
> clock gate the CPU in tegra_cpu_kill() after the CPU was power gated. For
> Tegra114, the flow controller will clock gate the CPU after the power down
> sequence. But we still need to implement a empty function for disable_clock
> to avoid kernel warning message.
I assume I need to apply this patch in the Tegra tree along with all the
other patches in this series? i.e. Mike can't apply it on its own to the
clk tree?
If so, I'll wait for an ack from Mike.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 5/6] clk: tegra114: implement wait_for_reset and disable_clock for tegra_cpu_car_ops
2013-05-15 10:27 ` [PATCH 5/6] clk: tegra114: implement wait_for_reset and disable_clock for tegra_cpu_car_ops Joseph Lo
2013-05-15 23:02 ` Stephen Warren
@ 2013-05-16 19:17 ` Mike Turquette
1 sibling, 0 replies; 26+ messages in thread
From: Mike Turquette @ 2013-05-16 19:17 UTC (permalink / raw)
To: linux-arm-kernel
Quoting Joseph Lo (2013-05-15 03:27:23)
> The conventional CPU hotplug sequence on the other Tegra chips, we will also
> clock gate the CPU in tegra_cpu_kill() after the CPU was power gated. For
> Tegra114, the flow controller will clock gate the CPU after the power down
> sequence. But we still need to implement a empty function for disable_clock
> to avoid kernel warning message.
>
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Joseph Lo <josephl@nvidia.com>
Acked-by: Mike Turquette <mturquette@linaro.org>
> ---
> drivers/clk/tegra/clk-tegra114.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
> index d78e16e..40d939d 100644
> --- a/drivers/clk/tegra/clk-tegra114.c
> +++ b/drivers/clk/tegra/clk-tegra114.c
> @@ -250,6 +250,9 @@
> #define CLK_SOURCE_XUSB_DEV_SRC 0x60c
> #define CLK_SOURCE_EMC 0x19c
>
> +/* Tegra CPU clock and reset control regs */
> +#define CLK_RST_CONTROLLER_CPU_CMPLX_STATUS 0x470
> +
> static int periph_clk_enb_refcnt[CLK_OUT_ENB_NUM * 32];
>
> static void __iomem *clk_base;
> @@ -2000,7 +2003,25 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base)
> }
> }
>
> -static struct tegra_cpu_car_ops tegra114_cpu_car_ops;
> +/* Tegra114 CPU clock and reset control functions */
> +static void tegra114_wait_cpu_in_reset(u32 cpu)
> +{
> + unsigned int reg;
> +
> + do {
> + reg = readl(clk_base + CLK_RST_CONTROLLER_CPU_CMPLX_STATUS);
> + cpu_relax();
> + } while (!(reg & (1 << cpu))); /* check CPU been reset or not */
> +}
> +static void tegra114_disable_cpu_clock(u32 cpu)
> +{
> + /* flow controller would take care in the power sequence. */
> +}
> +
> +static struct tegra_cpu_car_ops tegra114_cpu_car_ops = {
> + .wait_for_reset = tegra114_wait_cpu_in_reset,
> + .disable_clock = tegra114_disable_cpu_clock,
> +};
>
> static const struct of_device_id pmc_match[] __initconst = {
> { .compatible = "nvidia,tegra114-pmc" },
> --
> 1.8.2.2
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 6/6] ARM: tegra114: add CPU hotplug support
2013-05-15 10:27 [PATCH 0/6] ARM: tegra114: add CPU hotplug support Joseph Lo
` (4 preceding siblings ...)
2013-05-15 10:27 ` [PATCH 5/6] clk: tegra114: implement wait_for_reset and disable_clock for tegra_cpu_car_ops Joseph Lo
@ 2013-05-15 10:27 ` Joseph Lo
2013-05-15 23:11 ` Stephen Warren
2013-05-15 23:38 ` [PATCH 0/6] " Stephen Warren
6 siblings, 1 reply; 26+ messages in thread
From: Joseph Lo @ 2013-05-15 10:27 UTC (permalink / raw)
To: linux-arm-kernel
The Tegra114 is a quad cores SoC. Each core can be hotplugged including
CPU0. The hotplug sequence can be controlled by setting event trigger in
flow controller. Then the flow controller will take care all the power
sequence that include CPU up and down.
Signed-off-by: Joseph Lo <josephl@nvidia.com>
---
arch/arm/mach-tegra/Makefile | 1 +
arch/arm/mach-tegra/hotplug.c | 2 ++
arch/arm/mach-tegra/reset-handler.S | 11 ++++++++++-
arch/arm/mach-tegra/sleep-tegra30.S | 27 +++++++++++++++++++++++----
arch/arm/mach-tegra/sleep.h | 2 ++
5 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
index d011f0a..98b184e 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_HOTPLUG_CPU) += hotplug.o
obj-$(CONFIG_TEGRA_PCI) += pcie.o
obj-$(CONFIG_ARCH_TEGRA_114_SOC) += tegra114_speedo.o
+obj-$(CONFIG_ARCH_TEGRA_114_SOC) += sleep-tegra30.o
ifeq ($(CONFIG_CPU_IDLE),y)
obj-$(CONFIG_ARCH_TEGRA_114_SOC) += cpuidle-tegra114.o
endif
diff --git a/arch/arm/mach-tegra/hotplug.c b/arch/arm/mach-tegra/hotplug.c
index 184914a..d07f152 100644
--- a/arch/arm/mach-tegra/hotplug.c
+++ b/arch/arm/mach-tegra/hotplug.c
@@ -55,4 +55,6 @@ void __init tegra_hotplug_init(void)
tegra_hotplug_shutdown = tegra20_hotplug_shutdown;
if (IS_ENABLED(CONFIG_ARCH_TEGRA_3x_SOC) && tegra_chip_id == TEGRA30)
tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
+ if (IS_ENABLED(CONFIG_ARCH_TEGRA_114_SOC) && tegra_chip_id == TEGRA114)
+ tegra_hotplug_shutdown = tegra30_hotplug_shutdown;
}
diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
index 893f6b7..5af3a7d 100644
--- a/arch/arm/mach-tegra/reset-handler.S
+++ b/arch/arm/mach-tegra/reset-handler.S
@@ -38,18 +38,24 @@
* CPU boot vector when restarting the a CPU following
* an LP2 transition. Also branched to by LP0 and LP1 resume after
* re-enabling sdram.
+ *
+ * r6: SoC ID << 8
*/
ENTRY(tegra_resume)
bl v7_invalidate_l1
cpu_id r0
+ tegra_check_soc_id TEGRA114, TEGRA_APB_MISC_BASE, r6, r7
+ beq no_cpu0_chk
+
cmp r0, #0 @ CPU0?
THUMB( it ne )
bne cpu_resume @ no
+no_cpu0_chk:
#ifndef CONFIG_ARCH_TEGRA_2x_SOC
/* Are we on Tegra20? */
- tegra_check_soc_id TEGRA20, TEGRA_APB_MISC_BASE, r6, r7
+ cmp r6, #(TEGRA20 << 8)
beq 1f @ Yes
/* Clear the flow controller flags for this CPU. */
cpu_to_csr_req r1, r0
@@ -186,8 +192,11 @@ __is_not_lp2:
* Can only be secondary boot (initial or hotplug) but CPU 0
* cannot be here.
*/
+ cmp r6, #(TEGRA114 <<8)
+ beq __no_cpu0_chk
cmp r10, #0
bleq __die @ CPU0 cannot be here
+__no_cpu0_chk:
ldr lr, [r12, #RESET_DATA(STARTUP_SECONDARY)]
cmp lr, #0
bleq __die @ no secondary startup handler
diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
index d29dfcc..ae36fbb 100644
--- a/arch/arm/mach-tegra/sleep-tegra30.S
+++ b/arch/arm/mach-tegra/sleep-tegra30.S
@@ -19,6 +19,7 @@
#include <asm/assembler.h>
#include <asm/asm-offsets.h>
+#include "fuse.h"
#include "sleep.h"
#include "flowctrl.h"
@@ -45,12 +46,17 @@ ENDPROC(tegra30_hotplug_shutdown)
* and powergates it -- flags (in R0) indicate the request type.
* Must never be called for CPU 0.
*
- * corrupts r0-r4, r12
+ * r10 = SoC ID << 8
+ * corrupts r0-r4, r10-r12
*/
ENTRY(tegra30_cpu_shutdown)
cpu_id r3
+ tegra_check_soc_id TEGRA30, TEGRA_APB_MISC_VIRT, r10, r11
+ bne _no_cpu0_chk @ It's not Tegra30
+
cmp r3, #0
moveq pc, lr @ Must never be called for CPU 0
+_no_cpu0_chk:
ldr r12, =TEGRA_FLOW_CTRL_VIRT
cpu_to_csr_reg r1, r3
@@ -65,7 +71,9 @@ ENTRY(tegra30_cpu_shutdown)
movw r12, \
FLOW_CTRL_CSR_INTR_FLAG | FLOW_CTRL_CSR_EVENT_FLAG | \
FLOW_CTRL_CSR_ENABLE
- mov r4, #(1 << 4)
+ cmp r10, #(TEGRA30 << 8)
+ moveq r4, #(1 << 4) @ wfe bitmap
+ movne r4, #(1 << 8) @ wfi bitmap
ARM( orr r12, r12, r4, lsl r3 )
THUMB( lsl r4, r4, r3 )
THUMB( orr r12, r12, r4 )
@@ -79,9 +87,19 @@ delay_1:
cpsid a @ disable imprecise aborts.
ldr r3, [r1] @ read CSR
str r3, [r1] @ clear CSR
+
tst r0, #TEGRA30_POWER_HOTPLUG_SHUTDOWN
+ beq flow_ctrl_setting_for_lp2
+
+ /* flow controller set up for hotplug */
+ mov r3, #FLOW_CTRL_WAITEVENT @ For hotplug
+ b flow_ctrl_done
+flow_ctrl_setting_for_lp2:
+ /* flow controller set up for LP2 */
+ cmp r10, #(TEGRA30 << 8)
moveq r3, #FLOW_CTRL_WAIT_FOR_INTERRUPT @ For LP2
- movne r3, #FLOW_CTRL_WAITEVENT @ For hotplug
+flow_ctrl_done:
+ cmp r10, #(TEGRA30 << 8)
str r3, [r2]
ldr r0, [r2]
b wfe_war
@@ -89,7 +107,8 @@ delay_1:
__cpu_reset_again:
dsb
.align 5
- wfe @ CPU should be power gated here
+ wfeeq @ CPU should be power gated here
+ wfine
wfe_war:
b __cpu_reset_again
diff --git a/arch/arm/mach-tegra/sleep.h b/arch/arm/mach-tegra/sleep.h
index c2cac9a..92a3f0c 100644
--- a/arch/arm/mach-tegra/sleep.h
+++ b/arch/arm/mach-tegra/sleep.h
@@ -25,6 +25,8 @@
+ IO_PPSB_VIRT)
#define TEGRA_CLK_RESET_VIRT (TEGRA_CLK_RESET_BASE - IO_PPSB_PHYS \
+ IO_PPSB_VIRT)
+#define TEGRA_APB_MISC_VIRT (TEGRA_APB_MISC_BASE - IO_APB_PHYS \
+ + IO_APB_VIRT)
#define TEGRA_PMC_VIRT (TEGRA_PMC_BASE - IO_APB_PHYS + IO_APB_VIRT)
/* PMC_SCRATCH37-39 and 41 are used for tegra_pen_lock and idle */
--
1.8.2.2
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 6/6] ARM: tegra114: add CPU hotplug support
2013-05-15 10:27 ` [PATCH 6/6] ARM: tegra114: add CPU hotplug support Joseph Lo
@ 2013-05-15 23:11 ` Stephen Warren
2013-05-16 11:14 ` Joseph Lo
0 siblings, 1 reply; 26+ messages in thread
From: Stephen Warren @ 2013-05-15 23:11 UTC (permalink / raw)
To: linux-arm-kernel
On 05/15/2013 04:27 AM, Joseph Lo wrote:
> The Tegra114 is a quad cores SoC. Each core can be hotplugged including
> CPU0. The hotplug sequence can be controlled by setting event trigger in
> flow controller. Then the flow controller will take care all the power
> sequence that include CPU up and down.
> diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
> +obj-$(CONFIG_ARCH_TEGRA_114_SOC) += sleep-tegra30.o
CONFIG_ARCH_TEGRA_3x_SOC also adds that to obj-y. Will including it
twice cause problems?
> diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
> * CPU boot vector when restarting the a CPU following
> * an LP2 transition. Also branched to by LP0 and LP1 resume after
> * re-enabling sdram.
> + *
> + * r6: SoC ID << 8
I meant to ask in patch 1: Perhaps the macro could do a >>8 so that code
can compare directly against the SoC IDs instead of having to compare
against shifted values.
> + tegra_check_soc_id TEGRA114, TEGRA_APB_MISC_BASE, r6, r7
> + beq no_cpu0_chk
Does that need a THUMB(it ...) added? Same elsewhere.
> + cmp r6, #(TEGRA114 <<8)
Needs a space after <<.
> diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
> @@ -45,12 +46,17 @@ ENDPROC(tegra30_hotplug_shutdown)
> * and powergates it -- flags (in R0) indicate the request type.
> * Must never be called for CPU 0.
That comment is wrong now, I think.
> tst r0, #TEGRA30_POWER_HOTPLUG_SHUTDOWN
> + beq flow_ctrl_setting_for_lp2
> +
> + /* flow controller set up for hotplug */
> + mov r3, #FLOW_CTRL_WAITEVENT @ For hotplug
> + b flow_ctrl_done
> +flow_ctrl_setting_for_lp2:
> + /* flow controller set up for LP2 */
> + cmp r10, #(TEGRA30 << 8)
> moveq r3, #FLOW_CTRL_WAIT_FOR_INTERRUPT @ For LP2
Do you need e.g. "movne r3, #0" or similar here? Otherwise, I think r3
ends up not being initialized.
> - movne r3, #FLOW_CTRL_WAITEVENT @ For hotplug
> +flow_ctrl_done:
> + cmp r10, #(TEGRA30 << 8)
> str r3, [r2]
I wonder whether creating a separate tegra114_cpu_shutdown() wouldn't be
better, to avoid all the runtime conditional code.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 6/6] ARM: tegra114: add CPU hotplug support
2013-05-15 23:11 ` Stephen Warren
@ 2013-05-16 11:14 ` Joseph Lo
2013-05-16 18:26 ` Stephen Warren
0 siblings, 1 reply; 26+ messages in thread
From: Joseph Lo @ 2013-05-16 11:14 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2013-05-16 at 07:11 +0800, Stephen Warren wrote:
> On 05/15/2013 04:27 AM, Joseph Lo wrote:
> > The Tegra114 is a quad cores SoC. Each core can be hotplugged including
> > CPU0. The hotplug sequence can be controlled by setting event trigger in
> > flow controller. Then the flow controller will take care all the power
> > sequence that include CPU up and down.
>
> > diff --git a/arch/arm/mach-tegra/Makefile b/arch/arm/mach-tegra/Makefile
>
> > +obj-$(CONFIG_ARCH_TEGRA_114_SOC) += sleep-tegra30.o
>
> CONFIG_ARCH_TEGRA_3x_SOC also adds that to obj-y. Will including it
> twice cause problems?
>
Looks the compiler can take care of this. I didn't see any problem here.
> > diff --git a/arch/arm/mach-tegra/reset-handler.S b/arch/arm/mach-tegra/reset-handler.S
>
> > * CPU boot vector when restarting the a CPU following
> > * an LP2 transition. Also branched to by LP0 and LP1 resume after
> > * re-enabling sdram.
> > + *
> > + * r6: SoC ID << 8
>
> I meant to ask in patch 1: Perhaps the macro could do a >>8 so that code
> can compare directly against the SoC IDs instead of having to compare
> against shifted values.
>
That's more easy to read the code indeed. Will do.
> > + tegra_check_soc_id TEGRA114, TEGRA_APB_MISC_BASE, r6, r7
> > + beq no_cpu0_chk
>
> Does that need a THUMB(it ...) added? Same elsewhere.
>
Will double confirm again later.
> > + cmp r6, #(TEGRA114 <<8)
>
> Needs a space after <<.
>
> > diff --git a/arch/arm/mach-tegra/sleep-tegra30.S b/arch/arm/mach-tegra/sleep-tegra30.S
>
> > @@ -45,12 +46,17 @@ ENDPROC(tegra30_hotplug_shutdown)
> > * and powergates it -- flags (in R0) indicate the request type.
> > * Must never be called for CPU 0.
>
> That comment is wrong now, I think.
Only true for Tegra30 now. Will fix.
>
> > tst r0, #TEGRA30_POWER_HOTPLUG_SHUTDOWN
> > + beq flow_ctrl_setting_for_lp2
> > +
> > + /* flow controller set up for hotplug */
> > + mov r3, #FLOW_CTRL_WAITEVENT @ For hotplug
> > + b flow_ctrl_done
> > +flow_ctrl_setting_for_lp2:
> > + /* flow controller set up for LP2 */
> > + cmp r10, #(TEGRA30 << 8)
> > moveq r3, #FLOW_CTRL_WAIT_FOR_INTERRUPT @ For LP2
>
> Do you need e.g. "movne r3, #0" or similar here? Otherwise, I think r3
> ends up not being initialized.
Yes, I will add the code when I add LP2 support for Tegra114 later.
Currently the LP2 code flow only for Tegra30, so it's OK.
>
> > - movne r3, #FLOW_CTRL_WAITEVENT @ For hotplug
> > +flow_ctrl_done:
> > + cmp r10, #(TEGRA30 << 8)
> > str r3, [r2]
>
> I wonder whether creating a separate tegra114_cpu_shutdown() wouldn't be
> better, to avoid all the runtime conditional code.
I personally think the conditional code is OK. And the ARM CPU also had
hardware for branch detection also.
I had finished some further features for both Tegra30 and Tegra114. And
most of the code here can be shared each other at least until now I
could see. Once I see if there is a significant difference, then I would
prepare maintain the code separately too.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 6/6] ARM: tegra114: add CPU hotplug support
2013-05-16 11:14 ` Joseph Lo
@ 2013-05-16 18:26 ` Stephen Warren
0 siblings, 0 replies; 26+ messages in thread
From: Stephen Warren @ 2013-05-16 18:26 UTC (permalink / raw)
To: linux-arm-kernel
On 05/16/2013 05:14 AM, Joseph Lo wrote:
> On Thu, 2013-05-16 at 07:11 +0800, Stephen Warren wrote:
>> On 05/15/2013 04:27 AM, Joseph Lo wrote:
>>> The Tegra114 is a quad cores SoC. Each core can be hotplugged including
>>> CPU0. The hotplug sequence can be controlled by setting event trigger in
>>> flow controller. Then the flow controller will take care all the power
>>> sequence that include CPU up and down.
>>> tst r0, #TEGRA30_POWER_HOTPLUG_SHUTDOWN
>>> + beq flow_ctrl_setting_for_lp2
>>> +
>>> + /* flow controller set up for hotplug */
>>> + mov r3, #FLOW_CTRL_WAITEVENT @ For hotplug
>>> + b flow_ctrl_done
>>> +flow_ctrl_setting_for_lp2:
>>> + /* flow controller set up for LP2 */
>>> + cmp r10, #(TEGRA30 << 8)
>>> moveq r3, #FLOW_CTRL_WAIT_FOR_INTERRUPT @ For LP2
>>
>> Do you need e.g. "movne r3, #0" or similar here? Otherwise, I think r3
>> ends up not being initialized.
>
> Yes, I will add the code when I add LP2 support for Tegra114 later.
> Currently the LP2 code flow only for Tegra30, so it's OK.
Ah, I see. Can we add the extra move to make sure the register is always
initialized now even though the path won't be used. That way, nobody
will be confused by this.
>>> - movne r3, #FLOW_CTRL_WAITEVENT @ For hotplug
>>> +flow_ctrl_done:
>>> + cmp r10, #(TEGRA30 << 8)
>>> str r3, [r2]
>>
>> I wonder whether creating a separate tegra114_cpu_shutdown() wouldn't be
>> better, to avoid all the runtime conditional code.
>
> I personally think the conditional code is OK. And the ARM CPU also had
> hardware for branch detection also.
>
> I had finished some further features for both Tegra30 and Tegra114. And
> most of the code here can be shared each other at least until now I
> could see. Once I see if there is a significant difference, then I would
> prepare maintain the code separately too.
OK, if the function gets larger with more shared, I guess it's fine.
Right now, the conditional-to-non-conditional code ratio is pretty high.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/6] ARM: tegra114: add CPU hotplug support
2013-05-15 10:27 [PATCH 0/6] ARM: tegra114: add CPU hotplug support Joseph Lo
` (5 preceding siblings ...)
2013-05-15 10:27 ` [PATCH 6/6] ARM: tegra114: add CPU hotplug support Joseph Lo
@ 2013-05-15 23:38 ` Stephen Warren
2013-05-16 9:53 ` Joseph Lo
6 siblings, 1 reply; 26+ messages in thread
From: Stephen Warren @ 2013-05-15 23:38 UTC (permalink / raw)
To: linux-arm-kernel
On 05/15/2013 04:27 AM, Joseph Lo wrote:
> Tegra 114 is different with other Tegra SoC chips. It using ARM Cortex-A15
> as CPU core and a enhanced flow controller for CPU power control. So
> we need to skip some code that was for Contex-A9 and some other support
> code that was for other Tegra SoC chips. Then adding the proper power up
> and hot plug control for Tegra114.
This series mostly works OK, but I see one problem: I can't hotunplug
CPU0, which the commit descriptions and code changes imply I should be
able to do:
root at localhost:~# echo 0 > /sys/devices/system/cpu/cpu0/online
-bash: echo: write error: Operation not permitted
^ permalink raw reply [flat|nested] 26+ messages in thread* [PATCH 0/6] ARM: tegra114: add CPU hotplug support
2013-05-15 23:38 ` [PATCH 0/6] " Stephen Warren
@ 2013-05-16 9:53 ` Joseph Lo
2013-05-16 18:19 ` Stephen Warren
2013-05-17 10:23 ` Russell King - ARM Linux
0 siblings, 2 replies; 26+ messages in thread
From: Joseph Lo @ 2013-05-16 9:53 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, 2013-05-16 at 07:38 +0800, Stephen Warren wrote:
> On 05/15/2013 04:27 AM, Joseph Lo wrote:
> > Tegra 114 is different with other Tegra SoC chips. It using ARM Cortex-A15
> > as CPU core and a enhanced flow controller for CPU power control. So
> > we need to skip some code that was for Contex-A9 and some other support
> > code that was for other Tegra SoC chips. Then adding the proper power up
> > and hot plug control for Tegra114.
>
> This series mostly works OK, but I see one problem: I can't hotunplug
> CPU0, which the commit descriptions and code changes imply I should be
> able to do:
>
> root at localhost:~# echo 0 > /sys/devices/system/cpu/cpu0/online
> -bash: echo: write error: Operation not permitted
I want to provide this function originally. But I found the
tegra_cpu_disable() was removed recently. It was replaced by the common
cpu_disable() function that didn't allow CPU0 to be un-plugged.
But I had verified the CPU0 is OK to be un-plugged on the older
linux-next branch that tegra_cpu_disable() watn't removed yet.
Do you want me to add them back to support this function for Tegra114?
Thanks,
Joseph
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/6] ARM: tegra114: add CPU hotplug support
2013-05-16 9:53 ` Joseph Lo
@ 2013-05-16 18:19 ` Stephen Warren
2013-05-17 10:15 ` Joseph Lo
2013-05-17 10:27 ` Russell King - ARM Linux
2013-05-17 10:23 ` Russell King - ARM Linux
1 sibling, 2 replies; 26+ messages in thread
From: Stephen Warren @ 2013-05-16 18:19 UTC (permalink / raw)
To: linux-arm-kernel
On 05/16/2013 03:53 AM, Joseph Lo wrote:
> On Thu, 2013-05-16 at 07:38 +0800, Stephen Warren wrote:
>> On 05/15/2013 04:27 AM, Joseph Lo wrote:
>>> Tegra 114 is different with other Tegra SoC chips. It using ARM Cortex-A15
>>> as CPU core and a enhanced flow controller for CPU power control. So
>>> we need to skip some code that was for Contex-A9 and some other support
>>> code that was for other Tegra SoC chips. Then adding the proper power up
>>> and hot plug control for Tegra114.
>>
>> This series mostly works OK, but I see one problem: I can't hotunplug
>> CPU0, which the commit descriptions and code changes imply I should be
>> able to do:
>>
>> root at localhost:~# echo 0 > /sys/devices/system/cpu/cpu0/online
>> -bash: echo: write error: Operation not permitted
>
> I want to provide this function originally. But I found the
> tegra_cpu_disable() was removed recently. It was replaced by the common
> cpu_disable() function that didn't allow CPU0 to be un-plugged.
Is there a specific reason for that; is there some problem in the core
ARM code that implies CPU0 should never be disabled?
> But I had verified the CPU0 is OK to be un-plugged on the older
> linux-next branch that tegra_cpu_disable() watn't removed yet.
>
> Do you want me to add them back to support this function for Tegra114?
If there is a problem removing CPU0 in the core code, the functionality
of this series is OK, although it's probably worth removing the parts
that attempt to make CPU0 hot-unpluggable to reduce the diff size.
If there's no problem removing CPU0, it'd be good to make it work,
although that could be done in followon patches.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/6] ARM: tegra114: add CPU hotplug support
2013-05-16 18:19 ` Stephen Warren
@ 2013-05-17 10:15 ` Joseph Lo
2013-05-17 10:27 ` Russell King - ARM Linux
1 sibling, 0 replies; 26+ messages in thread
From: Joseph Lo @ 2013-05-17 10:15 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2013-05-17 at 02:19 +0800, Stephen Warren wrote:
> On 05/16/2013 03:53 AM, Joseph Lo wrote:
> > On Thu, 2013-05-16 at 07:38 +0800, Stephen Warren wrote:
> >> On 05/15/2013 04:27 AM, Joseph Lo wrote:
> >>> Tegra 114 is different with other Tegra SoC chips. It using ARM Cortex-A15
> >>> as CPU core and a enhanced flow controller for CPU power control. So
> >>> we need to skip some code that was for Contex-A9 and some other support
> >>> code that was for other Tegra SoC chips. Then adding the proper power up
> >>> and hot plug control for Tegra114.
> >>
> >> This series mostly works OK, but I see one problem: I can't hotunplug
> >> CPU0, which the commit descriptions and code changes imply I should be
> >> able to do:
> >>
> >> root at localhost:~# echo 0 > /sys/devices/system/cpu/cpu0/online
> >> -bash: echo: write error: Operation not permitted
> >
> > I want to provide this function originally. But I found the
> > tegra_cpu_disable() was removed recently. It was replaced by the common
> > cpu_disable() function that didn't allow CPU0 to be un-plugged.
>
> Is there a specific reason for that; is there some problem in the core
> ARM code that implies CPU0 should never be disabled?
>
I think the CPU0 is the boot CPU for most of the case. We always need at
least one CPU online. That assumes to be the boot CPU.
> > But I had verified the CPU0 is OK to be un-plugged on the older
> > linux-next branch that tegra_cpu_disable() watn't removed yet.
> >
> > Do you want me to add them back to support this function for Tegra114?
>
> If there is a problem removing CPU0 in the core code, the functionality
> of this series is OK, although it's probably worth removing the parts
> that attempt to make CPU0 hot-unpluggable to reduce the diff size.
>
> If there's no problem removing CPU0, it'd be good to make it work,
> although that could be done in followon patches.
OK. Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/6] ARM: tegra114: add CPU hotplug support
2013-05-16 18:19 ` Stephen Warren
2013-05-17 10:15 ` Joseph Lo
@ 2013-05-17 10:27 ` Russell King - ARM Linux
1 sibling, 0 replies; 26+ messages in thread
From: Russell King - ARM Linux @ 2013-05-17 10:27 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, May 16, 2013 at 12:19:48PM -0600, Stephen Warren wrote:
> Is there a specific reason for that; is there some problem in the core
> ARM code that implies CPU0 should never be disabled?
No. This is the code I removed:
-int tegra_cpu_disable(unsigned int cpu)
-{
- /*
- * we don't allow CPU 0 to be shutdown (it is still too special
- * e.g. clock tick interrupts)
- */
- return cpu == 0 ? -EPERM : 0;
-}
This gets called from here:
static int platform_cpu_disable(unsigned int cpu)
{
if (smp_ops.cpu_disable)
return smp_ops.cpu_disable(cpu);
/*
* By default, allow disabling all CPUs except the first one,
* since this is special on a lot of platforms, e.g. because
* of clock tick interrupts.
*/
return cpu == 0 ? -EPERM : 0;
}
Notice the useless utterly nature of tegra_cpu_disable() given that it's
doing _precisely_ the same thing as the common code. It might as well
not even exist - and it doesn't for that very reason. Never provide
implementations for methods which are copies of the default action if
no method is provided.
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 0/6] ARM: tegra114: add CPU hotplug support
2013-05-16 9:53 ` Joseph Lo
2013-05-16 18:19 ` Stephen Warren
@ 2013-05-17 10:23 ` Russell King - ARM Linux
1 sibling, 0 replies; 26+ messages in thread
From: Russell King - ARM Linux @ 2013-05-17 10:23 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, May 16, 2013 at 05:53:39PM +0800, Joseph Lo wrote:
> On Thu, 2013-05-16 at 07:38 +0800, Stephen Warren wrote:
> > This series mostly works OK, but I see one problem: I can't hotunplug
> > CPU0, which the commit descriptions and code changes imply I should be
> > able to do:
> >
> > root at localhost:~# echo 0 > /sys/devices/system/cpu/cpu0/online
> > -bash: echo: write error: Operation not permitted
>
> I want to provide this function originally. But I found the
> tegra_cpu_disable() was removed recently. It was replaced by the common
> cpu_disable() function that didn't allow CPU0 to be un-plugged.
It got removed for the simple reason that the implementation of that
function was exactly the same as the default path in cpu_disable().
If you have a need to do something else other than the default, then
provide the function (and *only* provide it if you need to do something
different.)
However, if the default suffices, do not provide extra code even if you
later intend to change the function. Only add the function when you
need to do something else.
I see too much of this "I'll add this function just becase, and maybe
I'll change it later" crap getting into the kernel; this needs to stop.
Things like providing an empty function for every single method in a
struct even through the behaviour would be to do nothing had that
function not been provided. So whenever I see this, I remove the
offending functions if I'm touching that code.
^ permalink raw reply [flat|nested] 26+ messages in thread