powerpc/fsl: support low power boot for e500 and later
PATCH 1/1 powerpc/fsl: support low power boot for e500 and later
From: Wang Dongsheng <[email protected]>
low power boot means u-boot will put non-boot cpus into a low power status. Non-boot cpus don't need any more spin wait. e500, e500v2 will going to DOZE status. e500mc, e5500, e6500rev1 will going to PW10 state. e6500rev2 will going to PW20 state.
e500/e500v2 will be kicked up by MPIC-IPI, e500mc later will be kicked up by doorbell.
This feature tested on:
POWER UP TEST:
P1022DS(e500v2),96k times.
P4080(e500mc), 110k times. T1024(e5500), 83k times. T4240(e6500), 150k times.
CPU HOTPLUG TEST:
P1022DS(e500v2),1.4 million times.
P4080(e500mc), 1.8 million times. T1024(e5500), 1.3 million times. T4240(e6500), 1.1 million times.
arch/powerpc/cpu/mpc85xx/release.S +134 -1
@@ -297,10 +297,15 @@ __secondary_start_page: mtspr SPRN_MAS7,r11 tlbwe + li r6, 0 + /* * __bootpg_addr has the address of __second_half_boot_page * jump there in AS=1 space with cache enabled */ + .align 6 + .global jump_half_boot_page +jump_half_boot_page: lis r13,toreset(__bootpg_addr)@h ori r13,r13,toreset(__bootpg_addr)@l lwz r11,0(r13)
@@ -371,6 +376,9 @@ __second_half_boot_page: * }; * we pad this struct to 64 bytes so each entry is in its own cacheline */ + cmpwi r6, 1 + beq 3f + li r3,0 li r8,1 mfspr r4,SPRN_PIR
@@ -402,7 +410,132 @@ __second_half_boot_page: #endif lwz r4,ENTRY_ADDR_LOWER(r10) andi. r11,r4,1 - bne 3b + beq 6f + + li r6, 0 + addi r6, r6, 1 + + /* External Interrupt exception. */ + lis r7, toreset(jump_half_boot_page)@h + mtspr SPRN_IVPR, r7 + li r7, toreset(jump_half_boot_page)@l + +#ifdef CONFIG_E500MC + /* e500MC, e5500, e6500 will use doorbell to send ipi signal */ + mtspr SPRN_IVOR36, r7 +#endif + + /* + * For e500mc later: + * EE will open in low power state, IVOR4 make sure we can ACK + * trash interrupt and keep we can loop in wait state again until + * the desired interrupt coming. + * + * e500, e500v2: + * Kernel will use MPCI to send ipi signal, so we must set IVOR4. + */ + mtspr SPRN_IVOR4, r7 + + isync + +#ifdef CONFIG_E500MC + /* PW20 & AltiVec idle feature only exists for E6500 */ + mfspr r0, SPRN_PVR + rlwinm r11, r0, 16, 16, 31 + lis r12, 0 + ori r12, r12, PVR_VER_E6500 at l + cmpw r11, r12 + bne 5f + + /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */ + rlwinm r11, r0, 0, 16, 31 + cmpwi r11, 0x20 + blt 5f + +#define PW20_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */ + /* Core Enter PW20 State */ + mfspr r7, SPRN_PWRMGTCR0 + /* Set PW20_WAIT bit, enable pw20 state*/ + ori r7, r7, PWRMGTCR0_PW20_WAIT + li r9, PW20_WAIT_IDLE_BIT + /* Set Automatic PW20 Core Idle Count */ + rlwimi r7, r9, PWRMGTCR0_PW20_ENT_SHIFT, PWRMGTCR0_PW20_ENT + mtspr SPRN_PWRMGTCR0, r7 + +#define AV_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */ + /* Let Altivec Enter Idle State */ + mfspr r7, SPRN_PWRMGTCR0 + /* Enable Altivec Idle */ + oris r7, r7, PWRMGTCR0_AV_IDLE_PD_EN at h + li r9, AV_WAIT_IDLE_BIT + /* Set Automatic AltiVec Idle Count */ + rlwimi r7, r9, PWRMGTCR0_AV_IDLE_CNT_SHIFT, PWRMGTCR0_AV_IDLE_CNT + mtspr SPRN_PWRMGTCR0, r7 + +5: + /* + * Set all of cpu PIR-ID is 0, wait kernel send doorbell or MPIC-IPI + * signal. + * + * When kernel kick one of cpus, all cpus will be wakenup. To make + * sure that only the target cpu is effected, other cpus (by checking + * spin_table->addr_l) should go back to low power state. + * + * U-boot has renumber the cpu PIR Why we need to set all of PIR to + * the same value? + * A: Before kernel kicking cpu, the doorbell message was not configured + * for target cpu(cpu_messages->data). If we try to send a + * non-configured message to target cpu, it cannot correctly receive + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to + * let all cpus catch the interrupt. + * + * Why set PIR to zero? + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be + * there. So set PIR is zero as a default PIR ID for each CPUs. + */ + + li r7, 0 + mtspr SPRN_PIR, r7 + + wrteei 1 + wait +#else + li r5, 0 + lis r5, HID0_DOZE at h + mfspr r7, SPRN_HID0 + rlwinm r7, r7, 0, ~(HID0_DOZE | HID0_NAP | HID0_SLEEP) + or r7, r7, r5 + isync + mtspr SPRN_HID0, r7 + isync + + mfmsr r7 + oris r7, r7, MSR_WE at h + ori r7, r7, MSR_EE + msync + mtmsr r7 + isync +5: bl 5b +#endif +6: + wrteei 0 + + /* + * If proxy mode enable in MPIC, Read EPR to ACK INTERRUPT + * Or proxy mode disable, Kernel will read MPIC to ACK INTERRUPT. + */ + mfspr r7, SPRN_EPR + + /* Wait EOI finish */ +7: + lwz r7, ENTRY_PIR(r10) + andi. r7, r7, 0xffff + bne 8f + bl 7b +8: + isync /* get the upper bits of the addr */
arch/powerpc/include/asm/processor.h +8 -0
@@ -482,6 +482,7 @@#define SPRN_GIVOR8 0x1bb/* Guest Interrupt Vector Offset Register 8 */ #define SPRN_GIVOR13 0x1bc/* Guest Interrupt Vector Offset Register 13 */ #define SPRN_GIVOR14 0x1bd/* Guest Interrupt Vector Offset Register 14 */ +#define SPRN_EPR 0x2be/* External Proxy Register */ /* e500 definitions */ #define SPRN_L1CFG0 0x203/* L1 Cache Configuration Register 0 */
@@ -565,6 +566,13 @@#define SPRN_BBTAR 0x202/* Branch Buffer Target Address Register */ #define SPRN_PID1 0x279/* Process ID Register 1 */ #define SPRN_PID2 0x27a/* Process ID Register 2 */ +#define SPRN_PWRMGTCR0 0x3fb/* Power management control register 0 */ +#define PWRMGTCR0_PW20_WAIT (1 << 14)/* PW20 state enable bit */ +#define PWRMGTCR0_PW20_ENT_SHIFT 8 +#define PWRMGTCR0_PW20_ENT 0x3f00 +#define PWRMGTCR0_AV_IDLE_PD_EN (1 << 22)/* Altivec idle enable */ +#define PWRMGTCR0_AV_IDLE_CNT_SHIFT 16 +#define PWRMGTCR0_AV_IDLE_CNT 0x3f0000#define SPRN_MCSR 0x23c/* Machine Check Syndrome register */ #define SPRN_MCAR 0x23d/* Machine Check Address register */ #define MCSR_MCS 0x80000000/* Machine Check Summary */
Signature · 1 line
2.1.0.27.g96db324
PATCH 1/1 powerpc/fsl: support low power boot for e500 and later
Hi all,
Kernel patch link:
http://patchwork.ozlabs.org/patch/429259/
Regards,
-Dongsheng
Quoted 226 lines
-----Original Message-----
From: Dongsheng Wang [mailto:dongsheng.wang at freescale.com]
Sent: Thursday, January 15, 2015 2:05 PM
To: Sun York-R58495; Wood Scott-B07421; Li Yang-Leo-R58472
Cc: Jin Zhengxiong-R64188; u-boot at lists.denx.de; Wang Dongsheng-B40534
Subject: [PATCH] powerpc/fsl: support low power boot for e500 and laterFrom: Wang Dongsheng <[email protected]>
low power boot means u-boot will put non-boot cpus into a low power status. Non-boot cpus don't need any more spin wait. e500, e500v2 will going to DOZE status. e500mc, e5500, e6500rev1 will going to PW10 state. e6500rev2 will going to PW20 state.
e500/e500v2 will be kicked up by MPIC-IPI, e500mc later will be kicked up by doorbell.
This feature tested on:
POWER UP TEST:
P1022DS(e500v2),96k times.P4080(e500mc), 110k times. T1024(e5500), 83k times. T4240(e6500), 150k times.CPU HOTPLUG TEST:
P1022DS(e500v2),1.4 million times.P4080(e500mc), 1.8 million times. T1024(e5500), 1.3 million times. T4240(e6500), 1.1 million times.Signed-off-by Wang Dongsheng <[email protected]>arch/powerpc/cpu/mpc85xx/release.S +134
-1@@ -297,10 +297,15 @@ __secondary_start_page: mtspr SPRN_MAS7,r11 tlbwe + li r6, 0 + /* * __bootpg_addr has the address of __second_half_boot_page * jump there in AS=1 space with cache enabled */ + .align 6 + .global jump_half_boot_page +jump_half_boot_page: lis r13,toreset(__bootpg_addr)@h ori r13,r13,toreset(__bootpg_addr)@l lwz r11,0(r13)@@ -371,6 +376,9 @@ __second_half_boot_page: * }; * we pad this struct to 64 bytes so each entry is in its own cacheline */ + cmpwi r6, 1 + beq 3f + li r3,0 li r8,1 mfspr r4,SPRN_PIR@@ -402,7 +410,132 @@ __second_half_boot_page: #endif lwz r4,ENTRY_ADDR_LOWER(r10) andi. r11,r4,1 - bne 3b + beq 6f + + li r6, 0 + addi r6, r6, 1 + + /* External Interrupt exception. */ + lis r7, toreset(jump_half_boot_page)@h + mtspr SPRN_IVPR, r7 + li r7, toreset(jump_half_boot_page)@l + +#ifdef CONFIG_E500MC + /* e500MC, e5500, e6500 will use doorbell to send ipi signal */ + mtspr SPRN_IVOR36, r7 +#endif + + /* + * For e500mc later: + * EE will open in low power state, IVOR4 make sure we can ACK + * trash interrupt and keep we can loop in wait state again until + * the desired interrupt coming. + * + * e500, e500v2: + * Kernel will use MPCI to send ipi signal, so we must set IVOR4. + */ + mtspr SPRN_IVOR4, r7 + + isync + +#ifdef CONFIG_E500MC + /* PW20 & AltiVec idle feature only exists for E6500 */ + mfspr r0, SPRN_PVR + rlwinm r11, r0, 16, 16, 31 + lis r12, 0 + ori r12, r12, PVR_VER_E6500 at l + cmpw r11, r12 + bne 5f + + /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */ + rlwinm r11, r0, 0, 16, 31 + cmpwi r11, 0x20 + blt 5f + +#define PW20_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */ + /* Core Enter PW20 State */ + mfspr r7, SPRN_PWRMGTCR0 + /* Set PW20_WAIT bit, enable pw20 state*/ + ori r7, r7, PWRMGTCR0_PW20_WAIT + li r9, PW20_WAIT_IDLE_BIT + /* Set Automatic PW20 Core Idle Count */ + rlwimi r7, r9, PWRMGTCR0_PW20_ENT_SHIFT, PWRMGTCR0_PW20_ENT + mtspr SPRN_PWRMGTCR0, r7 + +#define AV_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */ + /* Let Altivec Enter Idle State */ + mfspr r7, SPRN_PWRMGTCR0 + /* Enable Altivec Idle */ + oris r7, r7, PWRMGTCR0_AV_IDLE_PD_EN at h + li r9, AV_WAIT_IDLE_BIT + /* Set Automatic AltiVec Idle Count */ + rlwimi r7, r9, PWRMGTCR0_AV_IDLE_CNT_SHIFT, PWRMGTCR0_AV_IDLE_CNT + mtspr SPRN_PWRMGTCR0, r7 + +5: + /* + * Set all of cpu PIR-ID is 0, wait kernel send doorbell or MPIC-IPI + * signal. + * + * When kernel kick one of cpus, all cpus will be wakenup. To make + * sure that only the target cpu is effected, other cpus (by checking + * spin_table->addr_l) should go back to low power state. + * + * U-boot has renumber the cpu PIR Why we need to set all of PIR to + * the same value? + * A: Before kernel kicking cpu, the doorbell message was not configured + * for target cpu(cpu_messages->data). If we try to send a + * non-configured message to target cpu, it cannot correctly receive + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to + * let all cpus catch the interrupt. + * + * Why set PIR to zero? + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be + * there. So set PIR is zero as a default PIR ID for each CPUs. + */ + + li r7, 0 + mtspr SPRN_PIR, r7 + + wrteei 1 + wait +#else + li r5, 0 + lis r5, HID0_DOZE at h + mfspr r7, SPRN_HID0 + rlwinm r7, r7, 0, ~(HID0_DOZE | HID0_NAP | HID0_SLEEP) + or r7, r7, r5 + isync + mtspr SPRN_HID0, r7 + isync + + mfmsr r7 + oris r7, r7, MSR_WE at h + ori r7, r7, MSR_EE + msync + mtmsr r7 + isync +5: bl 5b +#endif +6: + wrteei 0 + + /* + * If proxy mode enable in MPIC, Read EPR to ACK INTERRUPT + * Or proxy mode disable, Kernel will read MPIC to ACK INTERRUPT. + */ + mfspr r7, SPRN_EPR + + /* Wait EOI finish */ +7: + lwz r7, ENTRY_PIR(r10) + andi. r7, r7, 0xffff + bne 8f + bl 7b +8: + isync /* get the upper bits of the addr */arch/powerpc/include/asm/processor.h +8
-1@@ -482,6 +482,7 @@#define SPRN_GIVOR8 0x1bb/* Guest Interrupt Vector Offset Register 8 */ #define SPRN_GIVOR13 0x1bc/* Guest Interrupt Vector Offset Register 13 */ #define SPRN_GIVOR14 0x1bd/* Guest Interrupt Vector Offset Register 14 */ +#define SPRN_EPR 0x2be/* External Proxy Register */ /* e500 definitions */ #define SPRN_L1CFG0 0x203/* L1 Cache Configuration Register 0 */ @@ -565,6 +566,13 @@#define SPRN_BBTAR 0x202/* Branch Buffer Target Address Register */ #define SPRN_PID1 0x279/* Process ID Register 1 */ #define SPRN_PID2 0x27a/* Process ID Register 2 */ +#define SPRN_PWRMGTCR0 0x3fb/* Power management control register 0 */ +#define PWRMGTCR0_PW20_WAIT (1 << 14)/* PW20 state enable bit */ +#define PWRMGTCR0_PW20_ENT_SHIFT 8 +#define PWRMGTCR0_PW20_ENT 0x3f00 +#define PWRMGTCR0_AV_IDLE_PD_EN (1 << 22)/* Altivec idle enable */ +#define PWRMGTCR0_AV_IDLE_CNT_SHIFT 16 +#define PWRMGTCR0_AV_IDLE_CNT 0x3f0000#define SPRN_MCSR 0x23c/* Machine Check Syndrome register */ #define SPRN_MCAR 0x23d/* Machine Check Address register */ #define MCSR_MCS 0x80000000/* Machine Check Summary */ --2.1.0.27.g96db324
PATCH 1/1 powerpc/fsl: support low power boot for e500 and later
In reply to Dongsheng WangQuoted 9 lines · Dongsheng Wang
From: Wang Dongsheng <[email protected]>
low power boot means u-boot will put non-boot cpus into a low power status. Non-boot cpus don't need any more spin wait. e500, e500v2 will going to DOZE status. e500mc, e5500, e6500rev1 will going to PW10 state. e6500rev2 will going to PW20 state.
e500/e500v2 will be kicked up by MPIC-IPI, e500mc later will be kicked up by doorbell.
This will break compatibility with existing kernels (and violate ePAPR since you haven't changed the release-method string). It must be optional and (for now) off by default.
What benefits are we getting for this churn and complexity? Do we really need to optimize for the case where not all CPUs are used?
Where is this new mechanism documented?
Quoted 7 lines
arch/powerpc/cpu/mpc85xx/release.S +0
-0@@ -297,10 +297,15 @@ __secondary_start_page: mtspr SPRN_MAS7,r11 tlbwe
arch/powerpc/cpu/mpc85xx/release.S __secondary_start_page: · line 308
Quoted 10 lines
arch/powerpc/cpu/mpc85xx/release.S +5
-0+ li r6, 0 + /* * __bootpg_addr has the address of __second_half_boot_page * jump there in AS=1 space with cache enabled */ + .align 6 + .global jump_half_boot_page +jump_half_boot_page:
"half_boot_page" is confusing.
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 380
Quoted 9 lines
arch/powerpc/cpu/mpc85xx/release.S +2
-0lis r13,toreset(__bootpg_addr)@h ori r13,r13,toreset(__bootpg_addr)@l lwz r11,0(r13)@@ -371,6 +376,9 @@ __second_half_boot_page: * }; * we pad this struct to 64 bytes so each entry is in its own cacheline */ + cmpwi r6, 1 + beq 3f
What does a value of 1 mean for r6? What about 0? Could you use symbolic constants?
Why not just set the IVOR to where you really want to enter, rather than conditionally branching there based on a value that happens to correspond to whether you're entering via an interrupt?
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 416
Quoted 12 lines
arch/powerpc/cpu/mpc85xx/release.S +4
-1li r3,0 li r8,1 mfspr r4,SPRN_PIR@@ -402,7 +410,132 @@ __second_half_boot_page: #endif lwz r4,ENTRY_ADDR_LOWER(r10) andi. r11,r4,1 - bne 3b + beq 6f + + li r6, 0 + addi r6, r6, 1
Why not just "li r6, 1"?
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/release.S +5
-0+ /* External Interrupt exception. */ + lis r7, toreset(jump_half_boot_page)@h + mtspr SPRN_IVPR, r7 + li r7, toreset(jump_half_boot_page)@l +
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 432
Quoted 10 lines
arch/powerpc/cpu/mpc85xx/release.S +10
-0+#ifdef CONFIG_E500MC + /* e500MC, e5500, e6500 will use doorbell to send ipi signal */ + mtspr SPRN_IVOR36, r7 +#endif + + /* + * For e500mc later: + * EE will open in low power state, IVOR4 make sure we can ACK + * trash interrupt and keep we can loop in wait state again until + * the desired interrupt coming.
I don't understand "EE will open". Do you mean "EE will be set"? Why would we get unexpected interrupts?
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 436
Quoted 4 lines
arch/powerpc/cpu/mpc85xx/release.S +4
-0+ * + * e500, e500v2: + * Kernel will use MPCI to send ipi signal, so we must set IVOR4. + */
MPIC
Quoted 7 lines
arch/powerpc/cpu/mpc85xx/release.S +7
-0+ mtspr SPRN_IVOR4, r7 + + isync + +#ifdef CONFIG_E500MC + /* PW20 & AltiVec idle feature only exists for E6500 */ + mfspr r0, SPRN_PVR
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 453
Quoted 10 lines
arch/powerpc/cpu/mpc85xx/release.S +10
-0+ rlwinm r11, r0, 16, 16, 31 + lis r12, 0 + ori r12, r12, PVR_VER_E6500 at l + cmpw r11, r12 + bne 5f + + /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */ + rlwinm r11, r0, 0, 16, 31 + cmpwi r11, 0x20 + blt 5f
PW10 isn't enough here?
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 455
Quoted 1 line
arch/powerpc/cpu/mpc85xx/release.S +1
-0+#define PW20_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */
If you must use PW20 please set the timeout long enough that it's obvious we won't hit it during normal bootup.
Quoted 11 lines
arch/powerpc/cpu/mpc85xx/release.S +11
-0+ /* + * Set all of cpu PIR-ID is 0, wait kernel send doorbell or MPIC-IPI + * signal. + * + * When kernel kick one of cpus, all cpus will be wakenup. To make + * sure that only the target cpu is effected, other cpus (by checking + * spin_table->addr_l) should go back to low power state. + * + * U-boot has renumber the cpu PIR Why we need to set all of PIR to + * the same value? + * A: Before kernel kicking cpu, the doorbell message was not configured
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 496
Quoted 10 lines
arch/powerpc/cpu/mpc85xx/release.S +10
-0+ * for target cpu(cpu_messages->data). If we try to send a + * non-configured message to target cpu, it cannot correctly receive + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to + * let all cpus catch the interrupt. + * + * Why set PIR to zero? + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be + * there. So set PIR is zero as a default PIR ID for each CPUs.
What does NR_CPUS have to do with the appropriate PIR for each CPU?
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 529
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/release.S +5
-0+ /* + * If proxy mode enable in MPIC, Read EPR to ACK INTERRUPT + * Or proxy mode disable, Kernel will read MPIC to ACK INTERRUPT. + */ + mfspr r7, SPRN_EPR
Where do you limit this to cases when external proxy is enabled? Why would we care about external proxy at all? The only chips we use IPIs on don't have external proxy.
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 537
Quoted 8 lines
arch/powerpc/cpu/mpc85xx/release.S +8
-0+ + /* Wait EOI finish */ +7: + lwz r7, ENTRY_PIR(r10) + andi. r7, r7, 0xffff + bne 8f + bl 7b +8:
Why are you reading PIR in a loop? What does that have to do with an EOI finishing?
-Scott
A message that is not in the archive (sent off-list or not yet received): BN1PR03MB1883A87EC254EEF0CD9CBAE9D480@BN1PR03MB188.namprd03.prod.outlook.com
PATCH 1/1 powerpc/fsl: support low power boot for e500 and later
Quoted 26 lines · On Wed, 2015-01-21 at 01:23 -0600, Wang Dongsheng-B40534 wrote:
Quoted 18 lines
-----Original Message-----
From: Wood Scott-B07421
Sent: Tuesday, January 20, 2015 8:33 AM
To: Wang Dongsheng-B40534
Cc: Sun York-R58495; Li Yang-Leo-R58472; Jin Zhengxiong-R64188; u- boot at lists.denx.de
Subject: Re: [PATCH] powerpc/fsl: support low power boot for e500 and laterQuoted 7 lines · Dongsheng Wang
From: Wang Dongsheng <[email protected]>
low power boot means u-boot will put non-boot cpus into a low power status. Non-boot cpus don't need any more spin wait. e500, e500v2 will going to DOZE status. e500mc, e5500, e6500rev1 will going to PW10 state. e6500rev2 will going to PW20 state.
Quoted 7 lines
Quoted 2 lines
e500/e500v2 will be kicked up by MPIC-IPI, e500mc later will be kicked up by doorbell.
This will break compatibility with existing kernels (and violate ePAPR since you haven't changed the release-method string). It must be optional and (for now) off by default.
Quoted 8 lines
Agree.
Quoted 3 lines
What benefits are we getting for this churn and complexity? Do we really need to optimize for the case where not all CPUs are used?
Save the power is our purpose no matter at any stage. If we can save the power that we need to do it.
No, we don't "need to do it" if the savings are negligible compared to the complexity, or if it's for an obscure use case.
Worse, this could end up increasing power savings due to quickly entering and then leaving low power mode (unlikely with PW10, but if you end up in PW20 during the normal boot sequence, it could).
Quoted 4 lines
Quoted 2 lines
Where is this new mechanism documented?
I don't understand. Do you mean Hardware datasheet?
No, I mean software documentation for the new bootloader/kernel interface you're creating. The existing interface is documented in ePAPR.
Quoted 22 lines
Quoted 9 lines
Review
Quoted 1 line
(quoted diff) +0
-0
b/arch/powerpc/cpu/mpc85xx/release.SQuoted 7 lines
index a2c0ad4..a97a1b6 100644
arch/powerpc/cpu/mpc85xx/release.S +0
-0@@ -297,10 +297,15 @@ __secondary_start_page: mtspr SPRN_MAS7,r11 tlbweQuoted 12 lines
arch/powerpc/cpu/mpc85xx/release.S __secondary_start_page: · line 308
Quoted 9 lines
arch/powerpc/cpu/mpc85xx/release.S +5
-0+ li r6, 0 + /* * __bootpg_addr has the address of __second_half_boot_page * jump there in AS=1 space with cache enabled */ + .align 6 + .global jump_half_boot_page +jump_half_boot_page:"
half_boot_page" is confusing.
Quoted 1 line
Why there are confused?
I suspect you mean "jump to __second_half_boot_page" but it just doesn't read that way when you omit the "second" (and it doesn't help that the meaning of "second half" wasn't immediately obvious from context). It makes me ask questions like, "Is there a first_half_boot_page?" or "Is there a full_boot_page?"
Quoted 2 lines
The address of
__bootpg_addris not a physical address. Boot page will jump to a virtual address.
What does that have to do with the label name?
Quoted 19 lines
Quoted 4 lines
Quoted 4 lines
arch/powerpc/cpu/mpc85xx/release.S +0
-0lis r13,toreset(__bootpg_addr)@h ori r13,r13,toreset(__bootpg_addr)@l lwz r11,0(r13)@@ -371,6 +376,9 @@ __second_half_boot_page:Quoted 9 lines
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 380
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/release.S +2
-0* }; * we pad this struct to 64 bytes so each entry is in its own cacheline */ + cmpwi r6, 1 + beq 3fWhat does a value of 1 mean for r6? What about 0? Could you use symbolic constants?
Ok, replace the magic number with macro.
1 means the current process is come from kicked up flow. 0 means the current process is first boot up flow.
Quoted 8 lines
Quoted 4 lines
Why not just set the IVOR to where you really want to enter, rather than conditionally branching there based on a value that happens to correspond to whether you're entering via an interrupt?
Now the value of IVOR is
jump_half_boot_page. In__bootpg_addrI can check the waken up cpu that is kernel want to kick. If not the cpu will loop in low power state again.
Whatever it is that you want to check for, the first thing you do when you enter the interrupt is branch to 3f. How does setting IVOR to jump_half_boot_page rather than 3: help with what you're trying to do?
Quoted 16 lines
Quoted 6 lines
Quoted 6 lines
arch/powerpc/cpu/mpc85xx/release.S +0
-0li r3,0 li r8,1 mfspr r4,SPRN_PIR@@ -402,7 +410,132 @@ __second_half_boot_page: #endif lwz r4,ENTRY_ADDR_LOWER(r10)Quoted 9 lines
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 416
Quoted 6 lines
arch/powerpc/cpu/mpc85xx/release.S +4
-1andi. r11,r4,1 - bne 3b + beq 6f + + li r6, 0 + addi r6, r6, 1Why not just "li r6, 1"?
Quoted 1 line
Mask the cpu that cpu fall into low power state.
Forget the reason -- just tell me how the resulting state of "li r6,0; addi r6, r6, 1" will be different from "li r6, 1".
Quoted 18 lines
Quoted 9 lines
Quoted 9 lines
arch/powerpc/cpu/mpc85xx/release.S +9
-0+ /* External Interrupt exception. */ + lis r7, toreset(jump_half_boot_page)@h + mtspr SPRN_IVPR, r7 + li r7, toreset(jump_half_boot_page)@l + +#ifdef CONFIG_E500MC + /* e500MC, e5500, e6500 will use doorbell to send ipi signal */ + mtspr SPRN_IVOR36, r7 +#endifQuoted 8 lines
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 432
Quoted 6 lines
arch/powerpc/cpu/mpc85xx/release.S +6
-0+ + /* + * For e500mc later: + * EE will open in low power state, IVOR4 make sure we can ACK + * trash interrupt and keep we can loop in wait state again until + * the desired interrupt coming.I don't understand "EE will open". Do you mean "EE will be set"?
Quoted 6 lines
Thanks... Yes, EE will be set.
Quoted 2 lines
Why would we get unexpected interrupts?
When EE be set, cpu can receive external interrupt.
But what else would be sending an interrupt to this CPU at this point?
Quoted 2 lines
If a cpu is not kicked in kernel that be triggered by external interrupt, we need to make sure the cpu can be loop in low power again.
No, you can just say that the first interrupt will wake the CPU and it's up to the OS to program the MPIC properly.
Quoted 9 lines
Quoted 7 lines
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 453
Quoted 4 lines
arch/powerpc/cpu/mpc85xx/release.S +4
-0+ /* Fix erratum, e6500 rev1 does not support PW20 & AltiVec idle */ + rlwinm r11, r0, 0, 16, 31 + cmpwi r11, 0x20 + blt 5fPW10 isn't enough here?
e6500rev2 support PW20, the PW20 lower than PW10, Why not?
See the beginning of the mail.
Quoted 7 lines
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 455
Quoted 1 line
arch/powerpc/cpu/mpc85xx/release.S +1
-0+#define PW20_WAIT_IDLE_BIT 50 /* 1ms, TB frequency is 41.66MHZ */If you must use PW20 please set the timeout long enough that it's obvious we won't hit it during normal bootup.
Sorry, I don?t understand "obvious we won't hit it"...
What I mean is that we do not want to enter PW20 during normal boot. The precise time it takes before the OS releases the secondary CPUs can vary, so the timeout should be large enough that we are highly confident that the CPUs will be released before the PW20 timer expires.
Quoted 25 lines
Quoted 15 lines
Quoted 15 lines
arch/powerpc/cpu/mpc85xx/release.S +15
-0+ /* + * Set all of cpu PIR-ID is 0, wait kernel send doorbell or MPIC-IPI + * signal. + * + * When kernel kick one of cpus, all cpus will be wakenup. To make + * sure that only the target cpu is effected, other cpus (by checking + * spin_table->addr_l) should go back to low power state. + * + * U-boot has renumber the cpu PIR Why we need to set all of PIR to + * the same value? + * A: Before kernel kicking cpu, the doorbell message was not configured + * for target cpu(cpu_messages->data). If we try to send a + * non-configured message to target cpu, it cannot correctly receive + * doorbell interrput. So SET ALL OF CPU'S PIR to the same value to + * let all cpus catch the interrupt.Quoted 9 lines
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 496
Quoted 6 lines
arch/powerpc/cpu/mpc85xx/release.S +6
-0+ * + * Why set PIR to zero? + * A: U-boot cannot know how many cpus will be kicked up(Kernel allow us + * to configure NR_CPUS) and IPI is a per_cpu variable, u-boot cannot + * set a appropriate PIR for every cpu, but the boot cpu(CPU0) always be + * there. So set PIR is zero as a default PIR ID for each CPUs.What does
NR_CPUShave to do with the appropriate PIR for each CPU?
Quoted 1 line
PIR is for each CPU.
I am aware of this.
Quoted 1 line
The
NR_CPUSjust a number of kernel control to start the CPU.
It's the maximum number of CPUs that the kernel will support.
Quoted 1 line
We need to set the same value for each CPU PIR register.
No, we don't.
Quoted 1 line
Please see my comments for Why we need to do it.
If you think you've answered something and I ask again, it generally means I'm having a hard time understanding and it would be helpful to reword rather than just copying and pasting or telling me to read them again (I think I finally understand what you meant, but it's not clear).
Quoted 7 lines
* U-boot has renumber the cpu PIR Why we need to set all of PIR to
* the same value?
* A: Before kernel kicking cpu, the doorbell message was not configured
* for targetcpu(cpu_messages->data). If we try to send a
* non-configured message to target cpu, it cannot correctly receive
* doorbell interrput. So SET ALL OF CPU'S PIR to the same value to
* let all cpus catch the interrupt.
So the problem is that you're using a Linux function to send the msgsnd before it was meant to be called. Don't do that. Either set up the required data earlier, or use a lower level function that doesn't have that prerequisite.
Quoted 11 lines
Quoted 10 lines
arch/powerpc/cpu/mpc85xx/release.S __second_half_boot_page: · line 529
Quoted 5 lines
arch/powerpc/cpu/mpc85xx/release.S +5
-0+ /* + * If proxy mode enable in MPIC, Read EPR to ACK INTERRUPT + * Or proxy mode disable, Kernel will read MPIC to ACK INTERRUPT. + */ + mfspr r7, SPRN_EPRWhere do you limit this to cases when external proxy is enabled? Why would we care about external proxy at all? The only chips we use IPIs on don't have external proxy.
Quoted 1 line
In kernel the MPCI IPI message not be supported on CORENET platform and later.
Yes, it is. We just don't use it.
Quoted 1 line
After e500v2, proxy mode always enable in MPIC.
No, it's not. It has to be manually enabled by the kernel. U-Boot cannot know if the kernel has done this (without reading the relevant MPIC register).
Neither of the above two sentences answer my questions. What stops you from trying to read EPR on e500v2?
-Scott