linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define
@ 2013-09-11  5:56 Dongsheng Wang
  2013-09-11  5:56 ` [PATCH v3 2/4] powerpc/85xx: add hardware automatically enter altivec idle state Dongsheng Wang
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Dongsheng Wang @ 2013-09-11  5:56 UTC (permalink / raw)
  To: scottwood; +Cc: linuxppc-dev, Wang Dongsheng

From: Wang Dongsheng <dongsheng.wang@freescale.com>

E6500 PVR and SPRN_PWRMGTCR0 will be used in subsequent pw20/altivec
idle patches.

Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
---
*v3:
Add bit definitions for PWRMGTCR0.

 arch/powerpc/include/asm/reg.h       | 2 ++
 arch/powerpc/include/asm/reg_booke.h | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 64264bf..d4160ca 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -1053,6 +1053,8 @@
 #define PVR_8560	0x80200000
 #define PVR_VER_E500V1	0x8020
 #define PVR_VER_E500V2	0x8021
+#define PVR_VER_E6500	0x8040
+
 /*
  * For the 8xx processors, all of them report the same PVR family for
  * the PowerPC core. The various versions of these processors must be
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index ed8f836..4a6457e 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -170,6 +170,7 @@
 #define SPRN_L2CSR1	0x3FA	/* L2 Data Cache Control and Status Register 1 */
 #define SPRN_DCCR	0x3FA	/* Data Cache Cacheability Register */
 #define SPRN_ICCR	0x3FB	/* Instruction Cache Cacheability Register */
+#define SPRN_PWRMGTCR0	0x3FB	/* Power management control register 0 */
 #define SPRN_SVR	0x3FF	/* System Version Register */

 /*
@@ -216,6 +217,14 @@
 #define	CCR1_DPC	0x00000100 /* Disable L1 I-Cache/D-Cache parity checking */
 #define	CCR1_TCS	0x00000080 /* Timer Clock Select */

+/* Bit definitions for PWRMGTCR0. */
+#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
+
 /* Bit definitions for the MCSR. */
 #define MCSR_MCS	0x80000000 /* Machine Check Summary */
 #define MCSR_IB		0x40000000 /* Instruction PLB Error */
--
1.8.0

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

* [PATCH v3 2/4] powerpc/85xx: add hardware automatically enter altivec idle state
  2013-09-11  5:56 [PATCH v3 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define Dongsheng Wang
@ 2013-09-11  5:56 ` Dongsheng Wang
  2013-09-11 22:42   ` Scott Wood
  2013-09-11  5:56 ` [PATCH v3 3/4] powerpc/85xx: add hardware automatically enter pw20 state Dongsheng Wang
  2013-09-11  5:56 ` [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle Dongsheng Wang
  2 siblings, 1 reply; 12+ messages in thread
From: Dongsheng Wang @ 2013-09-11  5:56 UTC (permalink / raw)
  To: scottwood; +Cc: linuxppc-dev, Wang Dongsheng

From: Wang Dongsheng <dongsheng.wang@freescale.com>

Each core's AltiVec unit may be placed into a power savings mode
by turning off power to the unit. Core hardware will automatically
power down the AltiVec unit after no AltiVec instructions have
executed in N cycles. The AltiVec power-control is triggered by hardware.

Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
---
*v3:
Assembly code instead of C code.

*v2:
Remove:
delete setup_idle_hw_governor function.
delete "Fix erratum" for rev1.

Move:
move setup_* into __setup/restore_cpu_e6500.

 arch/powerpc/kernel/cpu_setup_fsl_booke.S | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
index bfb18c7..3c03c109 100644
--- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
+++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
@@ -53,11 +53,30 @@ _GLOBAL(__e500_dcache_setup)
 	isync
 	blr

+/*
+ * FIXME - We don't know the AltiVec application scenarios.
+ */
+#define AV_WAIT_IDLE_BIT		50 /* 1ms, TB frequency is 41.66MHZ */
+_GLOBAL(setup_altivec_idle)
+	mfspr	r3, SPRN_PWRMGTCR0
+
+	/* Enable Altivec Idle */
+	oris	r3, r3, PWRMGTCR0_AV_IDLE_PD_EN@h
+	li	r4, AV_WAIT_IDLE_BIT
+
+	/* Set Automatic AltiVec Idle Count */
+	rlwimi	r3, r4, PWRMGTCR0_AV_IDLE_CNT_SHIFT, PWRMGTCR0_AV_IDLE_CNT
+
+	mtspr	SPRN_PWRMGTCR0, r3
+
+	blr
+
 _GLOBAL(__setup_cpu_e6500)
 	mflr	r6
 #ifdef CONFIG_PPC64
 	bl	.setup_altivec_ivors
 #endif
+	bl	.setup_altivec_idle
 	bl	__setup_cpu_e5500
 	mtlr	r6
 	blr
@@ -119,6 +138,7 @@ _GLOBAL(__setup_cpu_e5500)
 _GLOBAL(__restore_cpu_e6500)
 	mflr	r5
 	bl	.setup_altivec_ivors
+	bl	.setup_altivec_idle
 	bl	__restore_cpu_e5500
 	mtlr	r5
 	blr
--
1.8.0

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

* [PATCH v3 3/4] powerpc/85xx: add hardware automatically enter pw20 state
  2013-09-11  5:56 [PATCH v3 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define Dongsheng Wang
  2013-09-11  5:56 ` [PATCH v3 2/4] powerpc/85xx: add hardware automatically enter altivec idle state Dongsheng Wang
@ 2013-09-11  5:56 ` Dongsheng Wang
  2013-09-11  5:56 ` [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle Dongsheng Wang
  2 siblings, 0 replies; 12+ messages in thread
From: Dongsheng Wang @ 2013-09-11  5:56 UTC (permalink / raw)
  To: scottwood; +Cc: linuxppc-dev, Wang Dongsheng

From: Wang Dongsheng <dongsheng.wang@freescale.com>

Using hardware features make core automatically enter PW20 state.
Set a TB count to hardware, the effective count begins when PW10
is entered. When the effective period has expired, the core will
proceed from PW10 to PW20 if no exit conditions have occurred during
the period.

Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
---
*v3:
Assembly code instead of C code.

*v2:
Remove:
delete setup_idle_hw_governor function.
delete "Fix erratum" for rev1.

Move:
move setup_* into __setup/restore_cpu_e6500.

 arch/powerpc/kernel/cpu_setup_fsl_booke.S | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
index 3c03c109..7389d49 100644
--- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
+++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
@@ -54,6 +54,27 @@ _GLOBAL(__e500_dcache_setup)
 	blr

 /*
+ * FIXME - We don't know, what time should we let the core into PW20 state.
+ * because we don't know the current state of the cpu load. And threads are
+ * independent, so we can not know the state of different thread has been
+ * idle.
+ */
+#define PW20_WAIT_IDLE_BIT		50 /* 1ms, TB frequency is 41.66MHZ */
+_GLOBAL(setup_pw20_idle)
+	mfspr	r3, SPRN_PWRMGTCR0
+
+	/* Set PW20_WAIT bit, enable pw20 state*/
+	ori	r3, r3, PWRMGTCR0_PW20_WAIT
+	li	r4, PW20_WAIT_IDLE_BIT
+
+	/* Set Automatic PW20 Core Idle Count */
+	rlwimi	r3, r4, PWRMGTCR0_PW20_ENT_SHIFT, PWRMGTCR0_PW20_ENT
+
+	mtspr	SPRN_PWRMGTCR0, r3
+
+	blr
+
+/*
  * FIXME - We don't know the AltiVec application scenarios.
  */
 #define AV_WAIT_IDLE_BIT		50 /* 1ms, TB frequency is 41.66MHZ */
@@ -76,6 +97,7 @@ _GLOBAL(__setup_cpu_e6500)
 #ifdef CONFIG_PPC64
 	bl	.setup_altivec_ivors
 #endif
+	bl	.setup_pw20_idle
 	bl	.setup_altivec_idle
 	bl	__setup_cpu_e5500
 	mtlr	r6
@@ -138,6 +160,7 @@ _GLOBAL(__setup_cpu_e5500)
 _GLOBAL(__restore_cpu_e6500)
 	mflr	r5
 	bl	.setup_altivec_ivors
+	bl	.setup_pw20_idle
 	bl	.setup_altivec_idle
 	bl	__restore_cpu_e5500
 	mtlr	r5
--
1.8.0

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

* [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
  2013-09-11  5:56 [PATCH v3 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define Dongsheng Wang
  2013-09-11  5:56 ` [PATCH v3 2/4] powerpc/85xx: add hardware automatically enter altivec idle state Dongsheng Wang
  2013-09-11  5:56 ` [PATCH v3 3/4] powerpc/85xx: add hardware automatically enter pw20 state Dongsheng Wang
@ 2013-09-11  5:56 ` Dongsheng Wang
  2013-09-11 23:04   ` Scott Wood
  2 siblings, 1 reply; 12+ messages in thread
From: Dongsheng Wang @ 2013-09-11  5:56 UTC (permalink / raw)
  To: scottwood; +Cc: linuxppc-dev, Wang Dongsheng

From: Wang Dongsheng <dongsheng.wang@freescale.com>

Add a sys interface to enable/diable pw20 state or altivec idle, and
control the wait entry time.

Enable/Disable interface:
0, disable. 1, enable.
/sys/devices/system/cpu/cpuX/pw20_state
/sys/devices/system/cpu/cpuX/altivec_idle

Set wait entry bit interface:
bit value range 0~63, 0 bit is Mintime, 63 bit is Maxtime.
/sys/devices/system/cpu/cpuX/pw20_wait_entry_bit
/sys/devices/system/cpu/cpuX/altivec_idle_wait_entry_bit

Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
---
diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
index 7389d49..7395d79 100644
--- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
+++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
@@ -53,6 +53,21 @@ _GLOBAL(__e500_dcache_setup)
 	isync
 	blr

+_GLOBAL(has_pw20_altivec_idle)
+	/* 0 false, 1 true */
+	li	r3, 0
+
+	/* PW20 & AltiVec idle feature only exists for E6500 */
+	mfspr	r0, SPRN_PVR
+	rlwinm	r4, r0, 16, 16, 31
+	lis	r12, 0
+	ori	r12, r12, PVR_VER_E6500@l
+	cmpw	r4, r12
+	bne	2f
+	li	r3, 1
+2:
+	blr
+
 /*
  * FIXME - We don't know, what time should we let the core into PW20 state.
  * because we don't know the current state of the cpu load. And threads are
diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c
index d0861a0..fe4d3a7 100644
--- a/arch/powerpc/platforms/85xx/common.c
+++ b/arch/powerpc/platforms/85xx/common.c
@@ -5,12 +5,16 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
+#include <linux/cpu.h>
 #include <linux/of_platform.h>

 #include <sysdev/cpm2_pic.h>

 #include "mpc85xx.h"

+#define MAX_BIT		63
+
 static struct of_device_id __initdata mpc85xx_common_ids[] = {
 	{ .type = "soc", },
 	{ .compatible = "soc", },
@@ -80,3 +84,234 @@ void __init mpc85xx_cpm2_pic_init(void)
 	irq_set_chained_handler(irq, cpm2_cascade);
 }
 #endif
+
+static void query_pwrmgtcr0(void *val)
+{
+	u32 *value = val;
+
+	*value = mfspr(SPRN_PWRMGTCR0);
+}
+
+static ssize_t show_pw20_state(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	u32 value;
+	unsigned int cpu = dev->id;
+
+	smp_call_function_single(cpu, query_pwrmgtcr0, &value, 1);
+
+	value &= PWRMGTCR0_PW20_WAIT;
+
+	return sprintf(buf, "%u\n", value ? 1 : 0);
+}
+
+static void control_pw20_state(void *val)
+{
+	u32 *value = val;
+	u32 pw20_state;
+
+	pw20_state = mfspr(SPRN_PWRMGTCR0);
+
+	if (*value)
+		pw20_state |= PWRMGTCR0_PW20_WAIT;
+	else
+		pw20_state &= ~PWRMGTCR0_PW20_WAIT;
+
+	mtspr(SPRN_PWRMGTCR0, pw20_state);
+}
+
+static ssize_t store_pw20_state(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	u32 value;
+	unsigned int cpu = dev->id;
+
+	if (kstrtou32(buf, 0, &value))
+		return -EINVAL;
+
+	if (value > 1)
+		return -EINVAL;
+
+	smp_call_function_single(cpu, control_pw20_state, &value, 1);
+
+	return count;
+}
+
+static ssize_t show_pw20_wait_entry_bit(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	u32 value;
+	unsigned int cpu = dev->id;
+
+	smp_call_function_single(cpu, query_pwrmgtcr0, &value, 1);
+
+	value = MAX_BIT - ((value & PWRMGTCR0_PW20_ENT) >>
+				PWRMGTCR0_PW20_ENT_SHIFT);
+
+	return sprintf(buf, "wait entry bit is %u\n", value);
+}
+
+static void set_pw20_wait_entry_bit(void *val)
+{
+	u32 *value = val;
+	u32 pw20_idle;
+
+	pw20_idle = mfspr(SPRN_PWRMGTCR0);
+
+	/* Set Automatic PW20 Core Idle Count */
+	/* clear count */
+	pw20_idle &= ~PWRMGTCR0_PW20_ENT;
+
+	/* set count */
+	pw20_idle |= ((MAX_BIT - *value) << PWRMGTCR0_PW20_ENT_SHIFT);
+
+	mtspr(SPRN_PWRMGTCR0, pw20_idle);
+}
+
+static ssize_t store_pw20_wait_entry_bit(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	u32 value;
+	unsigned int cpu = dev->id;
+
+	if (kstrtou32(buf, 0, &value))
+		return -EINVAL;
+
+	if (value > MAX_BIT)
+		return -EINVAL;
+
+	smp_call_function_single(cpu, set_pw20_wait_entry_bit,
+				&value, 1);
+
+	return count;
+}
+
+static ssize_t show_altivec_idle(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	u32 value;
+	unsigned int cpu = dev->id;
+
+	smp_call_function_single(cpu, query_pwrmgtcr0, &value, 1);
+
+	value &= PWRMGTCR0_AV_IDLE_PD_EN;
+
+	return sprintf(buf, "%u\n", value ? 1 : 0);
+}
+
+static void control_altivec_idle(void *val)
+{
+	u32 *value = val;
+	u32 altivec_idle;
+
+	altivec_idle = mfspr(SPRN_PWRMGTCR0);
+
+	if (*value)
+		altivec_idle |= PWRMGTCR0_AV_IDLE_PD_EN;
+	else
+		altivec_idle &= ~PWRMGTCR0_AV_IDLE_PD_EN;
+
+	mtspr(SPRN_PWRMGTCR0, altivec_idle);
+}
+
+static ssize_t store_altivec_idle(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	u32 value;
+	unsigned int cpu = dev->id;
+
+	if (kstrtou32(buf, 0, &value))
+		return -EINVAL;
+
+	if (value > 1)
+		return -EINVAL;
+
+	smp_call_function_single(cpu, control_altivec_idle, &value, 1);
+
+	return count;
+}
+
+static ssize_t show_altivec_idle_wait_entry_bit(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	u32 value;
+	unsigned int cpu = dev->id;
+
+	smp_call_function_single(cpu, query_pwrmgtcr0, &value, 1);
+
+	value = MAX_BIT - ((value & PWRMGTCR0_AV_IDLE_CNT) >>
+				PWRMGTCR0_AV_IDLE_CNT_SHIFT);
+
+	return sprintf(buf, "wait entry bit is %u\n", value);
+}
+
+static void set_altivec_idle_wait_entry_bit(void *val)
+{
+	u32 *value = val;
+	u32 altivec_idle;
+
+	altivec_idle = mfspr(SPRN_PWRMGTCR0);
+
+	/* Set Automatic AltiVec Idle Count */
+	/* clear count */
+	altivec_idle &= ~PWRMGTCR0_AV_IDLE_CNT;
+
+	/* set count */
+	altivec_idle |=
+		((MAX_BIT - *value) << PWRMGTCR0_AV_IDLE_CNT_SHIFT);
+
+	mtspr(SPRN_PWRMGTCR0, altivec_idle);
+}
+
+static ssize_t store_altivec_idle_wait_entry_bit(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	u32 value;
+	unsigned int cpu = dev->id;
+
+	if (kstrtou32(buf, 0, &value))
+		return -EINVAL;
+
+	if (value > MAX_BIT)
+		return -EINVAL;
+
+	smp_call_function_single(cpu, set_altivec_idle_wait_entry_bit,
+				&value, 1);
+
+	return count;
+}
+
+static DEVICE_ATTR(pw20_state, 0644, show_pw20_state, store_pw20_state);
+static DEVICE_ATTR(pw20_wait_entry_bit, 0644, show_pw20_wait_entry_bit,
+						store_pw20_wait_entry_bit);
+
+static DEVICE_ATTR(altivec_idle, 0644, show_altivec_idle, store_altivec_idle);
+static DEVICE_ATTR(altivec_idle_wait_entry_bit, 0644,
+			show_altivec_idle_wait_entry_bit,
+			store_altivec_idle_wait_entry_bit);
+
+static int __init create_pw20_altivec_sysfs(void)
+{
+	int i;
+	struct device *cpu_dev;
+
+	if (!has_pw20_altivec_idle())
+		return -ENODEV;
+
+	for_each_possible_cpu(i) {
+		cpu_dev = get_cpu_device(i);
+		device_create_file(cpu_dev, &dev_attr_pw20_state);
+		device_create_file(cpu_dev, &dev_attr_pw20_wait_entry_bit);
+
+		device_create_file(cpu_dev, &dev_attr_altivec_idle);
+		device_create_file(cpu_dev,
+					&dev_attr_altivec_idle_wait_entry_bit);
+	}
+
+	return 0;
+}
+device_initcall(create_pw20_altivec_sysfs);
diff --git a/arch/powerpc/platforms/85xx/mpc85xx.h b/arch/powerpc/platforms/85xx/mpc85xx.h
index 2aa7c5d..e454d4d 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx.h
+++ b/arch/powerpc/platforms/85xx/mpc85xx.h
@@ -1,6 +1,7 @@
 #ifndef MPC85xx_H
 #define MPC85xx_H
 extern int mpc85xx_common_publish_devices(void);
+extern bool has_pw20_altivec_idle(void);

 #ifdef CONFIG_CPM2
 extern void mpc85xx_cpm2_pic_init(void);
--
1.8.0

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

* Re: [PATCH v3 2/4] powerpc/85xx: add hardware automatically enter altivec idle state
  2013-09-11  5:56 ` [PATCH v3 2/4] powerpc/85xx: add hardware automatically enter altivec idle state Dongsheng Wang
@ 2013-09-11 22:42   ` Scott Wood
  2013-09-12  2:27     ` Wang Dongsheng-B40534
  0 siblings, 1 reply; 12+ messages in thread
From: Scott Wood @ 2013-09-11 22:42 UTC (permalink / raw)
  To: Dongsheng Wang; +Cc: linuxppc-dev

On Wed, 2013-09-11 at 13:56 +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
> 
> Each core's AltiVec unit may be placed into a power savings mode
> by turning off power to the unit. Core hardware will automatically
> power down the AltiVec unit after no AltiVec instructions have
> executed in N cycles. The AltiVec power-control is triggered by hardware.
> 
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
> *v3:
> Assembly code instead of C code.
> 
> *v2:
> Remove:
> delete setup_idle_hw_governor function.
> delete "Fix erratum" for rev1.
> 
> Move:
> move setup_* into __setup/restore_cpu_e6500.
> 
>  arch/powerpc/kernel/cpu_setup_fsl_booke.S | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> index bfb18c7..3c03c109 100644
> --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> @@ -53,11 +53,30 @@ _GLOBAL(__e500_dcache_setup)
>  	isync
>  	blr
> 
> +/*
> + * FIXME - We don't know the AltiVec application scenarios.
> + */
> +#define AV_WAIT_IDLE_BIT		50 /* 1ms, TB frequency is 41.66MHZ */
> +_GLOBAL(setup_altivec_idle)
> +	mfspr	r3, SPRN_PWRMGTCR0
> +
> +	/* Enable Altivec Idle */
> +	oris	r3, r3, PWRMGTCR0_AV_IDLE_PD_EN@h
> +	li	r4, AV_WAIT_IDLE_BIT
> +
> +	/* Set Automatic AltiVec Idle Count */
> +	rlwimi	r3, r4, PWRMGTCR0_AV_IDLE_CNT_SHIFT, PWRMGTCR0_AV_IDLE_CNT
> +
> +	mtspr	SPRN_PWRMGTCR0, r3
> +
> +	blr

The FIXME comment is not clear.  If you mean that we haven't yet done
testing to determine a reasonable default value for AV_WAIT_IDLE_BIT,
then just say that.  Likewise with the FIXME comment in the pw20 patch
-- the uncertainty is due to a lack of investigation, not "because we
don't know the current state of the cpu load".

While some workloads may want a different value than whatever default we
set, that's what the sysfs interface is for.  The only thing missing
here is benchmarking to determine a good overall default.

-Scott

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

* Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
  2013-09-11  5:56 ` [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle Dongsheng Wang
@ 2013-09-11 23:04   ` Scott Wood
  2013-09-12  3:48     ` Wang Dongsheng-B40534
  0 siblings, 1 reply; 12+ messages in thread
From: Scott Wood @ 2013-09-11 23:04 UTC (permalink / raw)
  To: Dongsheng Wang; +Cc: linuxppc-dev

On Wed, 2013-09-11 at 13:56 +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
> 
> Add a sys interface to enable/diable pw20 state or altivec idle, and
> control the wait entry time.
> 
> Enable/Disable interface:
> 0, disable. 1, enable.
> /sys/devices/system/cpu/cpuX/pw20_state
> /sys/devices/system/cpu/cpuX/altivec_idle
> 
> Set wait entry bit interface:
> bit value range 0~63, 0 bit is Mintime, 63 bit is Maxtime.
> /sys/devices/system/cpu/cpuX/pw20_wait_entry_bit
> /sys/devices/system/cpu/cpuX/altivec_idle_wait_entry_bit

I'm no fan of the way powerpc does bit numbering, but don't flip it
around here -- you'll just cause confusion.

Better yet, this interface should take real time units rather than a
timebase bit.

Also, you disable the power saving mode if the maximum interval is
selected, but the documentation doesn't say that -- and the
documentation should be in the code (or other easily findable place),
not just in the commit message.

> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
> diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> index 7389d49..7395d79 100644
> --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> @@ -53,6 +53,21 @@ _GLOBAL(__e500_dcache_setup)
>  	isync
>  	blr
> 
> +_GLOBAL(has_pw20_altivec_idle)
> +	/* 0 false, 1 true */
> +	li	r3, 0
> +
> +	/* PW20 & AltiVec idle feature only exists for E6500 */
> +	mfspr	r0, SPRN_PVR
> +	rlwinm	r4, r0, 16, 16, 31
> +	lis	r12, 0
> +	ori	r12, r12, PVR_VER_E6500@l
> +	cmpw	r4, r12
> +	bne	2f
> +	li	r3, 1
> +2:
> +	blr

Why is this in asm?  And shouldn't this go in the cputable somehow?

> +static void query_pwrmgtcr0(void *val)
> +{
> +	u32 *value = val;
> +
> +	*value = mfspr(SPRN_PWRMGTCR0);
> +}
> +
> +static ssize_t show_pw20_state(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	u32 value;
> +	unsigned int cpu = dev->id;
> +
> +	smp_call_function_single(cpu, query_pwrmgtcr0, &value, 1);
> +
> +	value &= PWRMGTCR0_PW20_WAIT;
> +
> +	return sprintf(buf, "%u\n", value ? 1 : 0);
> +}
> +
> +static void control_pw20_state(void *val)
> +{
> +	u32 *value = val;
> +	u32 pw20_state;
> +
> +	pw20_state = mfspr(SPRN_PWRMGTCR0);
> +
> +	if (*value)
> +		pw20_state |= PWRMGTCR0_PW20_WAIT;
> +	else
> +		pw20_state &= ~PWRMGTCR0_PW20_WAIT;
> +
> +	mtspr(SPRN_PWRMGTCR0, pw20_state);
> +}
> +
> +static ssize_t store_pw20_state(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)

The difference between query/show and control/store is a bit awkward...
I'd s/query/do_show/ and s/control/do_store/ (or local_ or other
appropriate prefix).

> +static ssize_t show_altivec_idle_wait_entry_bit(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	u32 value;
> +	unsigned int cpu = dev->id;
> +
> +	smp_call_function_single(cpu, query_pwrmgtcr0, &value, 1);
> +
> +	value = MAX_BIT - ((value & PWRMGTCR0_AV_IDLE_CNT) >>
> +				PWRMGTCR0_AV_IDLE_CNT_SHIFT);
> +
> +	return sprintf(buf, "wait entry bit is %u\n", value);
> +}

Reading a sysfs value should just return the value, not a human-readable
string.

> +static DEVICE_ATTR(pw20_state, 0644, show_pw20_state, store_pw20_state);
> +static DEVICE_ATTR(pw20_wait_entry_bit, 0644, show_pw20_wait_entry_bit,
> +						store_pw20_wait_entry_bit);
> +
> +static DEVICE_ATTR(altivec_idle, 0644, show_altivec_idle, store_altivec_idle);
> +static DEVICE_ATTR(altivec_idle_wait_entry_bit, 0644,
> +			show_altivec_idle_wait_entry_bit,
> +			store_altivec_idle_wait_entry_bit);

I'd make these 0600, given their ability to spam other CPUs with IPIs
even on read.

> +static int __init create_pw20_altivec_sysfs(void)
> +{
> +	int i;
> +	struct device *cpu_dev;
> +
> +	if (!has_pw20_altivec_idle())
> +		return -ENODEV;
> +
> +	for_each_possible_cpu(i) {
> +		cpu_dev = get_cpu_device(i);
> +		device_create_file(cpu_dev, &dev_attr_pw20_state);
> +		device_create_file(cpu_dev, &dev_attr_pw20_wait_entry_bit);
> +
> +		device_create_file(cpu_dev, &dev_attr_altivec_idle);
> +		device_create_file(cpu_dev,
> +					&dev_attr_altivec_idle_wait_entry_bit);
> +	}
> +
> +	return 0;
> +}
> +device_initcall(create_pw20_altivec_sysfs);

I think I said this earlier, but it'd be nice to have a "late cpu setup"
cputable callback -- but failing that, this should be called from
register_cpu_online() which is where other CPU sysfs attributes are
created.

-Scott

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

* RE: [PATCH v3 2/4] powerpc/85xx: add hardware automatically enter altivec idle state
  2013-09-11 22:42   ` Scott Wood
@ 2013-09-12  2:27     ` Wang Dongsheng-B40534
  0 siblings, 0 replies; 12+ messages in thread
From: Wang Dongsheng-B40534 @ 2013-09-12  2:27 UTC (permalink / raw)
  To: Wood Scott-B07421; +Cc: linuxppc-dev

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVGh1cnNkYXksIFNlcHRlbWJlciAxMiwgMjAxMyA2OjQzIEFNDQo+IFRvOiBX
YW5nIERvbmdzaGVuZy1CNDA1MzQNCj4gQ2M6IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IGxp
bnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjMgMi80
XSBwb3dlcnBjLzg1eHg6IGFkZCBoYXJkd2FyZSBhdXRvbWF0aWNhbGx5DQo+IGVudGVyIGFsdGl2
ZWMgaWRsZSBzdGF0ZQ0KPiANCj4gT24gV2VkLCAyMDEzLTA5LTExIGF0IDEzOjU2ICswODAwLCBE
b25nc2hlbmcgV2FuZyB3cm90ZToNCj4gPiBGcm9tOiBXYW5nIERvbmdzaGVuZyA8ZG9uZ3NoZW5n
LndhbmdAZnJlZXNjYWxlLmNvbT4NCj4gPg0KPiA+IEVhY2ggY29yZSdzIEFsdGlWZWMgdW5pdCBt
YXkgYmUgcGxhY2VkIGludG8gYSBwb3dlciBzYXZpbmdzIG1vZGUNCj4gPiBieSB0dXJuaW5nIG9m
ZiBwb3dlciB0byB0aGUgdW5pdC4gQ29yZSBoYXJkd2FyZSB3aWxsIGF1dG9tYXRpY2FsbHkNCj4g
PiBwb3dlciBkb3duIHRoZSBBbHRpVmVjIHVuaXQgYWZ0ZXIgbm8gQWx0aVZlYyBpbnN0cnVjdGlv
bnMgaGF2ZQ0KPiA+IGV4ZWN1dGVkIGluIE4gY3ljbGVzLiBUaGUgQWx0aVZlYyBwb3dlci1jb250
cm9sIGlzIHRyaWdnZXJlZCBieQ0KPiBoYXJkd2FyZS4NCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6
IFdhbmcgRG9uZ3NoZW5nIDxkb25nc2hlbmcud2FuZ0BmcmVlc2NhbGUuY29tPg0KPiA+IC0tLQ0K
PiA+ICp2MzoNCj4gPiBBc3NlbWJseSBjb2RlIGluc3RlYWQgb2YgQyBjb2RlLg0KPiA+DQo+ID4g
KnYyOg0KPiA+IFJlbW92ZToNCj4gPiBkZWxldGUgc2V0dXBfaWRsZV9od19nb3Zlcm5vciBmdW5j
dGlvbi4NCj4gPiBkZWxldGUgIkZpeCBlcnJhdHVtIiBmb3IgcmV2MS4NCj4gPg0KPiA+IE1vdmU6
DQo+ID4gbW92ZSBzZXR1cF8qIGludG8gX19zZXR1cC9yZXN0b3JlX2NwdV9lNjUwMC4NCj4gPg0K
PiA+ICBhcmNoL3Bvd2VycGMva2VybmVsL2NwdV9zZXR1cF9mc2xfYm9va2UuUyB8IDIwICsrKysr
KysrKysrKysrKysrKysrDQo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAyMCBpbnNlcnRpb25zKCspDQo+
ID4NCj4gPiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2tlcm5lbC9jcHVfc2V0dXBfZnNsX2Jv
b2tlLlMNCj4gYi9hcmNoL3Bvd2VycGMva2VybmVsL2NwdV9zZXR1cF9mc2xfYm9va2UuUw0KPiA+
IGluZGV4IGJmYjE4YzcuLjNjMDNjMTA5IDEwMDY0NA0KPiA+IC0tLSBhL2FyY2gvcG93ZXJwYy9r
ZXJuZWwvY3B1X3NldHVwX2ZzbF9ib29rZS5TDQo+ID4gKysrIGIvYXJjaC9wb3dlcnBjL2tlcm5l
bC9jcHVfc2V0dXBfZnNsX2Jvb2tlLlMNCj4gPiBAQCAtNTMsMTEgKzUzLDMwIEBAIF9HTE9CQUwo
X19lNTAwX2RjYWNoZV9zZXR1cCkNCj4gPiAgCWlzeW5jDQo+ID4gIAlibHINCj4gPg0KPiA+ICsv
Kg0KPiA+ICsgKiBGSVhNRSAtIFdlIGRvbid0IGtub3cgdGhlIEFsdGlWZWMgYXBwbGljYXRpb24g
c2NlbmFyaW9zLg0KPiA+ICsgKi8NCj4gPiArI2RlZmluZSBBVl9XQUlUX0lETEVfQklUCQk1MCAv
KiAxbXMsIFRCIGZyZXF1ZW5jeSBpcyA0MS42Nk1IWg0KPiAqLw0KPiA+ICtfR0xPQkFMKHNldHVw
X2FsdGl2ZWNfaWRsZSkNCj4gPiArCW1mc3ByCXIzLCBTUFJOX1BXUk1HVENSMA0KPiA+ICsNCj4g
PiArCS8qIEVuYWJsZSBBbHRpdmVjIElkbGUgKi8NCj4gPiArCW9yaXMJcjMsIHIzLCBQV1JNR1RD
UjBfQVZfSURMRV9QRF9FTkBoDQo+ID4gKwlsaQlyNCwgQVZfV0FJVF9JRExFX0JJVA0KPiA+ICsN
Cj4gPiArCS8qIFNldCBBdXRvbWF0aWMgQWx0aVZlYyBJZGxlIENvdW50ICovDQo+ID4gKwlybHdp
bWkJcjMsIHI0LCBQV1JNR1RDUjBfQVZfSURMRV9DTlRfU0hJRlQsDQo+IFBXUk1HVENSMF9BVl9J
RExFX0NOVA0KPiA+ICsNCj4gPiArCW10c3ByCVNQUk5fUFdSTUdUQ1IwLCByMw0KPiA+ICsNCj4g
PiArCWJscg0KPiANCj4gVGhlIEZJWE1FIGNvbW1lbnQgaXMgbm90IGNsZWFyLiAgSWYgeW91IG1l
YW4gdGhhdCB3ZSBoYXZlbid0IHlldCBkb25lDQo+IHRlc3RpbmcgdG8gZGV0ZXJtaW5lIGEgcmVh
c29uYWJsZSBkZWZhdWx0IHZhbHVlIGZvciBBVl9XQUlUX0lETEVfQklULA0KPiB0aGVuIGp1c3Qg
c2F5IHRoYXQuICBMaWtld2lzZSB3aXRoIHRoZSBGSVhNRSBjb21tZW50IGluIHRoZSBwdzIwIHBh
dGNoDQo+IC0tIHRoZSB1bmNlcnRhaW50eSBpcyBkdWUgdG8gYSBsYWNrIG9mIGludmVzdGlnYXRp
b24sIG5vdCAiYmVjYXVzZSB3ZQ0KPiBkb24ndCBrbm93IHRoZSBjdXJyZW50IHN0YXRlIG9mIHRo
ZSBjcHUgbG9hZCIuDQo+IA0KPiBXaGlsZSBzb21lIHdvcmtsb2FkcyBtYXkgd2FudCBhIGRpZmZl
cmVudCB2YWx1ZSB0aGFuIHdoYXRldmVyIGRlZmF1bHQgd2UNCj4gc2V0LCB0aGF0J3Mgd2hhdCB0
aGUgc3lzZnMgaW50ZXJmYWNlIGlzIGZvci4gIFRoZSBvbmx5IHRoaW5nIG1pc3NpbmcNCj4gaGVy
ZSBpcyBiZW5jaG1hcmtpbmcgdG8gZGV0ZXJtaW5lIGEgZ29vZCBvdmVyYWxsIGRlZmF1bHQuDQo+
IA0KVGhhbmtzLg0KDQo=

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

* RE: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
  2013-09-11 23:04   ` Scott Wood
@ 2013-09-12  3:48     ` Wang Dongsheng-B40534
  2013-09-12 18:06       ` Scott Wood
  0 siblings, 1 reply; 12+ messages in thread
From: Wang Dongsheng-B40534 @ 2013-09-12  3:48 UTC (permalink / raw)
  To: Wood Scott-B07421; +Cc: linuxppc-dev

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVGh1cnNkYXksIFNlcHRlbWJlciAxMiwgMjAxMyA3OjA0IEFNDQo+IFRvOiBX
YW5nIERvbmdzaGVuZy1CNDA1MzQNCj4gQ2M6IGdhbGFrQGtlcm5lbC5jcmFzaGluZy5vcmc7IGxp
bnV4cHBjLWRldkBsaXN0cy5vemxhYnMub3JnDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjMgNC80
XSBwb3dlcnBjLzg1eHg6IGFkZCBzeXNmcyBmb3IgcHcyMCBzdGF0ZSBhbmQNCj4gYWx0aXZlYyBp
ZGxlDQo+IA0KPiBPbiBXZWQsIDIwMTMtMDktMTEgYXQgMTM6NTYgKzA4MDAsIERvbmdzaGVuZyBX
YW5nIHdyb3RlOg0KPiA+IEZyb206IFdhbmcgRG9uZ3NoZW5nIDxkb25nc2hlbmcud2FuZ0BmcmVl
c2NhbGUuY29tPg0KPiA+DQo+ID4gQWRkIGEgc3lzIGludGVyZmFjZSB0byBlbmFibGUvZGlhYmxl
IHB3MjAgc3RhdGUgb3IgYWx0aXZlYyBpZGxlLCBhbmQNCj4gPiBjb250cm9sIHRoZSB3YWl0IGVu
dHJ5IHRpbWUuDQo+ID4NCj4gPiBFbmFibGUvRGlzYWJsZSBpbnRlcmZhY2U6DQo+ID4gMCwgZGlz
YWJsZS4gMSwgZW5hYmxlLg0KPiA+IC9zeXMvZGV2aWNlcy9zeXN0ZW0vY3B1L2NwdVgvcHcyMF9z
dGF0ZQ0KPiA+IC9zeXMvZGV2aWNlcy9zeXN0ZW0vY3B1L2NwdVgvYWx0aXZlY19pZGxlDQo+ID4N
Cj4gPiBTZXQgd2FpdCBlbnRyeSBiaXQgaW50ZXJmYWNlOg0KPiA+IGJpdCB2YWx1ZSByYW5nZSAw
fjYzLCAwIGJpdCBpcyBNaW50aW1lLCA2MyBiaXQgaXMgTWF4dGltZS4NCj4gPiAvc3lzL2Rldmlj
ZXMvc3lzdGVtL2NwdS9jcHVYL3B3MjBfd2FpdF9lbnRyeV9iaXQNCj4gPiAvc3lzL2RldmljZXMv
c3lzdGVtL2NwdS9jcHVYL2FsdGl2ZWNfaWRsZV93YWl0X2VudHJ5X2JpdA0KPiANCj4gSSdtIG5v
IGZhbiBvZiB0aGUgd2F5IHBvd2VycGMgZG9lcyBiaXQgbnVtYmVyaW5nLCBidXQgZG9uJ3QgZmxp
cCBpdA0KPiBhcm91bmQgaGVyZSAtLSB5b3UnbGwganVzdCBjYXVzZSBjb25mdXNpb24uDQo+IA0K
T0suIDAgYml0IGlzIG1heHRpbWUsIDYzIGJpdCBpcyBtaW50aW1lLg0KDQo+IEJldHRlciB5ZXQs
IHRoaXMgaW50ZXJmYWNlIHNob3VsZCB0YWtlIHJlYWwgdGltZSB1bml0cyByYXRoZXIgdGhhbiBh
DQo+IHRpbWViYXNlIGJpdC4NCj4gDQpJIHRoaW5rIHRoZSByZWFsIHRpbWUgaXMgbm90IHN1aXRh
YmxlLCBiZWNhdXNlIHRpbWViYXNlIGJpdCBkb2VzIG5vdCBjb3JyZXNwb25kIHdpdGgNCnJlYWwg
dGltZS4NCiANCj4gQWxzbywgeW91IGRpc2FibGUgdGhlIHBvd2VyIHNhdmluZyBtb2RlIGlmIHRo
ZSBtYXhpbXVtIGludGVydmFsIGlzDQo+IHNlbGVjdGVkLCANCkl0J3Mgbm90IGRpc2FibGUgdGhl
IHB3MjAgc3RhdGUgb3IgYWx0aXZlYyBpZGxlLCBqdXN0IG1heC1kZWxheSBlbnRyeSB0aW1lLg0K
DQo+YnV0IHRoZSBkb2N1bWVudGF0aW9uIGRvZXNuJ3Qgc2F5IHRoYXQgLS0gYW5kIHRoZSBkb2N1
bWVudGF0aW9uDQo+IHNob3VsZCBiZSBpbiB0aGUgY29kZSAob3Igb3RoZXIgZWFzaWx5IGZpbmRh
YmxlIHBsYWNlKSwgbm90IGp1c3QgaW4gdGhlDQo+IGNvbW1pdCBtZXNzYWdlLg0KPiANCkFsc28g
YWRkIGEgY29tbWVudCBpbiB0aGUgODV4eC9jb21tb24uYyA/DQoNCj4gPiBTaWduZWQtb2ZmLWJ5
OiBXYW5nIERvbmdzaGVuZyA8ZG9uZ3NoZW5nLndhbmdAZnJlZXNjYWxlLmNvbT4NCj4gPiAtLS0N
Cj4gPiBkaWZmIC0tZ2l0IGEvYXJjaC9wb3dlcnBjL2tlcm5lbC9jcHVfc2V0dXBfZnNsX2Jvb2tl
LlMNCj4gPiBiL2FyY2gvcG93ZXJwYy9rZXJuZWwvY3B1X3NldHVwX2ZzbF9ib29rZS5TDQo+ID4g
aW5kZXggNzM4OWQ0OS4uNzM5NWQ3OSAxMDA2NDQNCj4gPiAtLS0gYS9hcmNoL3Bvd2VycGMva2Vy
bmVsL2NwdV9zZXR1cF9mc2xfYm9va2UuUw0KPiA+ICsrKyBiL2FyY2gvcG93ZXJwYy9rZXJuZWwv
Y3B1X3NldHVwX2ZzbF9ib29rZS5TDQo+ID4gQEAgLTUzLDYgKzUzLDIxIEBAIF9HTE9CQUwoX19l
NTAwX2RjYWNoZV9zZXR1cCkNCj4gPiAgCWlzeW5jDQo+ID4gIAlibHINCj4gPg0KPiA+ICtfR0xP
QkFMKGhhc19wdzIwX2FsdGl2ZWNfaWRsZSkNCj4gPiArCS8qIDAgZmFsc2UsIDEgdHJ1ZSAqLw0K
PiA+ICsJbGkJcjMsIDANCj4gPiArDQo+ID4gKwkvKiBQVzIwICYgQWx0aVZlYyBpZGxlIGZlYXR1
cmUgb25seSBleGlzdHMgZm9yIEU2NTAwICovDQo+ID4gKwltZnNwcglyMCwgU1BSTl9QVlINCj4g
PiArCXJsd2lubQlyNCwgcjAsIDE2LCAxNiwgMzENCj4gPiArCWxpcwlyMTIsIDANCj4gPiArCW9y
aQlyMTIsIHIxMiwgUFZSX1ZFUl9FNjUwMEBsDQo+ID4gKwljbXB3CXI0LCByMTINCj4gPiArCWJu
ZQkyZg0KPiA+ICsJbGkJcjMsIDENCj4gPiArMjoNCj4gPiArCWJscg0KPiANCj4gV2h5IGlzIHRo
aXMgaW4gYXNtPyAgQW5kIHNob3VsZG4ndCB0aGlzIGdvIGluIHRoZSBjcHV0YWJsZSBzb21laG93
Pw0KPiANCk5vdCBhIHNwZWNpYWwgcmVhc29uIGZvciB0aGlzLCBqdXN0IGFzbS4uLg0KSSBzZWUg
dGhhdCwgd2UgY2FuIHVzZSBjcHVfc3BlYy0+cHZyX3ZhbHVlIGluIGMgY29kZS4NCg0KPiA+ICtz
dGF0aWMgdm9pZCBxdWVyeV9wd3JtZ3RjcjAodm9pZCAqdmFsKSB7DQo+ID4gKwl1MzIgKnZhbHVl
ID0gdmFsOw0KPiA+ICsNCj4gPiArCSp2YWx1ZSA9IG1mc3ByKFNQUk5fUFdSTUdUQ1IwKTsNCj4g
PiArfQ0KPiA+ICsNCj4gPiArc3RhdGljIHNzaXplX3Qgc2hvd19wdzIwX3N0YXRlKHN0cnVjdCBk
ZXZpY2UgKmRldiwNCj4gPiArCQkJCXN0cnVjdCBkZXZpY2VfYXR0cmlidXRlICphdHRyLCBjaGFy
ICpidWYpIHsNCj4gPiArCXUzMiB2YWx1ZTsNCj4gPiArCXVuc2lnbmVkIGludCBjcHUgPSBkZXYt
PmlkOw0KPiA+ICsNCj4gPiArCXNtcF9jYWxsX2Z1bmN0aW9uX3NpbmdsZShjcHUsIHF1ZXJ5X3B3
cm1ndGNyMCwgJnZhbHVlLCAxKTsNCj4gPiArDQo+ID4gKwl2YWx1ZSAmPSBQV1JNR1RDUjBfUFcy
MF9XQUlUOw0KPiA+ICsNCj4gPiArCXJldHVybiBzcHJpbnRmKGJ1ZiwgIiV1XG4iLCB2YWx1ZSA/
IDEgOiAwKTsgfQ0KPiA+ICsNCj4gPiArc3RhdGljIHZvaWQgY29udHJvbF9wdzIwX3N0YXRlKHZv
aWQgKnZhbCkgew0KPiA+ICsJdTMyICp2YWx1ZSA9IHZhbDsNCj4gPiArCXUzMiBwdzIwX3N0YXRl
Ow0KPiA+ICsNCj4gPiArCXB3MjBfc3RhdGUgPSBtZnNwcihTUFJOX1BXUk1HVENSMCk7DQo+ID4g
Kw0KPiA+ICsJaWYgKCp2YWx1ZSkNCj4gPiArCQlwdzIwX3N0YXRlIHw9IFBXUk1HVENSMF9QVzIw
X1dBSVQ7DQo+ID4gKwllbHNlDQo+ID4gKwkJcHcyMF9zdGF0ZSAmPSB+UFdSTUdUQ1IwX1BXMjBf
V0FJVDsNCj4gPiArDQo+ID4gKwltdHNwcihTUFJOX1BXUk1HVENSMCwgcHcyMF9zdGF0ZSk7DQo+
ID4gK30NCj4gPiArDQo+ID4gK3N0YXRpYyBzc2l6ZV90IHN0b3JlX3B3MjBfc3RhdGUoc3RydWN0
IGRldmljZSAqZGV2LA0KPiA+ICsJCQkJc3RydWN0IGRldmljZV9hdHRyaWJ1dGUgKmF0dHIsDQo+
ID4gKwkJCQljb25zdCBjaGFyICpidWYsIHNpemVfdCBjb3VudCkNCj4gDQo+IFRoZSBkaWZmZXJl
bmNlIGJldHdlZW4gcXVlcnkvc2hvdyBhbmQgY29udHJvbC9zdG9yZSBpcyBhIGJpdCBhd2t3YXJk
Li4uDQo+IEknZCBzL3F1ZXJ5L2RvX3Nob3cvIGFuZCBzL2NvbnRyb2wvZG9fc3RvcmUvIChvciBs
b2NhbF8gb3Igb3RoZXINCj4gYXBwcm9wcmlhdGUgcHJlZml4KS4NCj4gDQpkb19zaG93L2RvX3N0
b3JlIGxvb2tzIGJldHRlciB0aGFuIG90aGVycy4NCg0KPiA+ICtzdGF0aWMgc3NpemVfdCBzaG93
X2FsdGl2ZWNfaWRsZV93YWl0X2VudHJ5X2JpdChzdHJ1Y3QgZGV2aWNlICpkZXYsDQo+ID4gKwkJ
CQlzdHJ1Y3QgZGV2aWNlX2F0dHJpYnV0ZSAqYXR0ciwgY2hhciAqYnVmKSB7DQo+ID4gKwl1MzIg
dmFsdWU7DQo+ID4gKwl1bnNpZ25lZCBpbnQgY3B1ID0gZGV2LT5pZDsNCj4gPiArDQo+ID4gKwlz
bXBfY2FsbF9mdW5jdGlvbl9zaW5nbGUoY3B1LCBxdWVyeV9wd3JtZ3RjcjAsICZ2YWx1ZSwgMSk7
DQo+ID4gKw0KPiA+ICsJdmFsdWUgPSBNQVhfQklUIC0gKCh2YWx1ZSAmIFBXUk1HVENSMF9BVl9J
RExFX0NOVCkgPj4NCj4gPiArCQkJCVBXUk1HVENSMF9BVl9JRExFX0NOVF9TSElGVCk7DQo+ID4g
Kw0KPiA+ICsJcmV0dXJuIHNwcmludGYoYnVmLCAid2FpdCBlbnRyeSBiaXQgaXMgJXVcbiIsIHZh
bHVlKTsgfQ0KPiANCj4gUmVhZGluZyBhIHN5c2ZzIHZhbHVlIHNob3VsZCBqdXN0IHJldHVybiB0
aGUgdmFsdWUsIG5vdCBhIGh1bWFuLXJlYWRhYmxlDQo+IHN0cmluZy4NCj4gDQpUaGFua3MuDQoN
Cj4gPiArc3RhdGljIERFVklDRV9BVFRSKHB3MjBfc3RhdGUsIDA2NDQsIHNob3dfcHcyMF9zdGF0
ZSwNCj4gPiArc3RvcmVfcHcyMF9zdGF0ZSk7IHN0YXRpYyBERVZJQ0VfQVRUUihwdzIwX3dhaXRf
ZW50cnlfYml0LCAwNjQ0LA0KPiBzaG93X3B3MjBfd2FpdF9lbnRyeV9iaXQsDQo+ID4gKwkJCQkJ
CXN0b3JlX3B3MjBfd2FpdF9lbnRyeV9iaXQpOw0KPiA+ICsNCj4gPiArc3RhdGljIERFVklDRV9B
VFRSKGFsdGl2ZWNfaWRsZSwgMDY0NCwgc2hvd19hbHRpdmVjX2lkbGUsDQo+ID4gK3N0b3JlX2Fs
dGl2ZWNfaWRsZSk7IHN0YXRpYyBERVZJQ0VfQVRUUihhbHRpdmVjX2lkbGVfd2FpdF9lbnRyeV9i
aXQsDQo+IDA2NDQsDQo+ID4gKwkJCXNob3dfYWx0aXZlY19pZGxlX3dhaXRfZW50cnlfYml0LA0K
PiA+ICsJCQlzdG9yZV9hbHRpdmVjX2lkbGVfd2FpdF9lbnRyeV9iaXQpOw0KPiANCj4gSSdkIG1h
a2UgdGhlc2UgMDYwMCwgZ2l2ZW4gdGhlaXIgYWJpbGl0eSB0byBzcGFtIG90aGVyIENQVXMgd2l0
aCBJUElzDQo+IGV2ZW4gb24gcmVhZC4NCj4gDQpUaGFua3MuDQoNCj4gPiArc3RhdGljIGludCBf
X2luaXQgY3JlYXRlX3B3MjBfYWx0aXZlY19zeXNmcyh2b2lkKSB7DQo+ID4gKwlpbnQgaTsNCj4g
PiArCXN0cnVjdCBkZXZpY2UgKmNwdV9kZXY7DQo+ID4gKw0KPiA+ICsJaWYgKCFoYXNfcHcyMF9h
bHRpdmVjX2lkbGUoKSkNCj4gPiArCQlyZXR1cm4gLUVOT0RFVjsNCj4gPiArDQo+ID4gKwlmb3Jf
ZWFjaF9wb3NzaWJsZV9jcHUoaSkgew0KPiA+ICsJCWNwdV9kZXYgPSBnZXRfY3B1X2RldmljZShp
KTsNCj4gPiArCQlkZXZpY2VfY3JlYXRlX2ZpbGUoY3B1X2RldiwgJmRldl9hdHRyX3B3MjBfc3Rh
dGUpOw0KPiA+ICsJCWRldmljZV9jcmVhdGVfZmlsZShjcHVfZGV2LCAmZGV2X2F0dHJfcHcyMF93
YWl0X2VudHJ5X2JpdCk7DQo+ID4gKw0KPiA+ICsJCWRldmljZV9jcmVhdGVfZmlsZShjcHVfZGV2
LCAmZGV2X2F0dHJfYWx0aXZlY19pZGxlKTsNCj4gPiArCQlkZXZpY2VfY3JlYXRlX2ZpbGUoY3B1
X2RldiwNCj4gPiArCQkJCQkmZGV2X2F0dHJfYWx0aXZlY19pZGxlX3dhaXRfZW50cnlfYml0KTsN
Cj4gPiArCX0NCj4gPiArDQo+ID4gKwlyZXR1cm4gMDsNCj4gPiArfQ0KPiA+ICtkZXZpY2VfaW5p
dGNhbGwoY3JlYXRlX3B3MjBfYWx0aXZlY19zeXNmcyk7DQo+IA0KPiBJIHRoaW5rIEkgc2FpZCB0
aGlzIGVhcmxpZXIsIGJ1dCBpdCdkIGJlIG5pY2UgdG8gaGF2ZSBhICJsYXRlIGNwdSBzZXR1cCIN
Cj4gY3B1dGFibGUgY2FsbGJhY2sgLS0gYnV0IGZhaWxpbmcgdGhhdCwgdGhpcyBzaG91bGQgYmUg
Y2FsbGVkIGZyb20NCj4gcmVnaXN0ZXJfY3B1X29ubGluZSgpIHdoaWNoIGlzIHdoZXJlIG90aGVy
IENQVSBzeXNmcyBhdHRyaWJ1dGVzIGFyZQ0KPiBjcmVhdGVkLg0KDQpZZXMsIHRoYW5rcy4gDQoN
Ci1kb25nc2hlbmcNCg==

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

* Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
  2013-09-12  3:48     ` Wang Dongsheng-B40534
@ 2013-09-12 18:06       ` Scott Wood
  2013-09-13  2:53         ` Wang Dongsheng-B40534
  0 siblings, 1 reply; 12+ messages in thread
From: Scott Wood @ 2013-09-12 18:06 UTC (permalink / raw)
  To: Wang Dongsheng-B40534; +Cc: Wood Scott-B07421, linuxppc-dev

On Wed, 2013-09-11 at 22:48 -0500, Wang Dongsheng-B40534 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Thursday, September 12, 2013 7:04 AM
> > To: Wang Dongsheng-B40534
> > Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and
> > altivec idle
> > 
> > On Wed, 2013-09-11 at 13:56 +0800, Dongsheng Wang wrote:
> > > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > >
> > > Add a sys interface to enable/diable pw20 state or altivec idle, and
> > > control the wait entry time.
> > >
> > > Enable/Disable interface:
> > > 0, disable. 1, enable.
> > > /sys/devices/system/cpu/cpuX/pw20_state
> > > /sys/devices/system/cpu/cpuX/altivec_idle
> > >
> > > Set wait entry bit interface:
> > > bit value range 0~63, 0 bit is Mintime, 63 bit is Maxtime.
> > > /sys/devices/system/cpu/cpuX/pw20_wait_entry_bit
> > > /sys/devices/system/cpu/cpuX/altivec_idle_wait_entry_bit
> > 
> > I'm no fan of the way powerpc does bit numbering, but don't flip it
> > around here -- you'll just cause confusion.
> > 
> OK. 0 bit is maxtime, 63 bit is mintime.
> 
> > Better yet, this interface should take real time units rather than a
> > timebase bit.
> > 
> I think the real time is not suitable, because timebase bit does not correspond with
> real time.

It's a bit sloppy due to how the hardware works, but you could convert
it like you did in earlier patches.  Semantically it should probably be
the minimum time to wait before entering the low power state.
 
> > Also, you disable the power saving mode if the maximum interval is
> > selected, 
> It's not disable the pw20 state or altivec idle, just max-delay entry time.

No, the code checks for zero to set or clear the enabling bit (e.g.
PW20_WAIT).

> >but the documentation doesn't say that -- and the documentation
> > should be in the code (or other easily findable place), not just in the
> > commit message.
> > 
> Also add a comment in the 85xx/common.c ?

Yes.

> > > +_GLOBAL(has_pw20_altivec_idle)
> > > +	/* 0 false, 1 true */
> > > +	li	r3, 0
> > > +
> > > +	/* PW20 & AltiVec idle feature only exists for E6500 */
> > > +	mfspr	r0, SPRN_PVR
> > > +	rlwinm	r4, r0, 16, 16, 31
> > > +	lis	r12, 0
> > > +	ori	r12, r12, PVR_VER_E6500@l
> > > +	cmpw	r4, r12
> > > +	bne	2f
> > > +	li	r3, 1
> > > +2:
> > > +	blr
> > 
> > Why is this in asm?  And shouldn't this go in the cputable somehow?
> > 
> Not a special reason for this, just asm...

Asm shouldn't be used without a good reason.

-Scott

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

* RE: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
  2013-09-12 18:06       ` Scott Wood
@ 2013-09-13  2:53         ` Wang Dongsheng-B40534
  2013-09-16 21:09           ` Scott Wood
  0 siblings, 1 reply; 12+ messages in thread
From: Wang Dongsheng-B40534 @ 2013-09-13  2:53 UTC (permalink / raw)
  To: Wood Scott-B07421; +Cc: linuxppc-dev

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogRnJpZGF5LCBTZXB0ZW1iZXIgMTMsIDIwMTMgMjowNyBBTQ0KPiBUbzogV2Fu
ZyBEb25nc2hlbmctQjQwNTM0DQo+IENjOiBXb29kIFNjb3R0LUIwNzQyMTsgZ2FsYWtAa2VybmVs
LmNyYXNoaW5nLm9yZzsgbGludXhwcGMtDQo+IGRldkBsaXN0cy5vemxhYnMub3JnDQo+IFN1Ympl
Y3Q6IFJlOiBbUEFUQ0ggdjMgNC80XSBwb3dlcnBjLzg1eHg6IGFkZCBzeXNmcyBmb3IgcHcyMCBz
dGF0ZSBhbmQNCj4gYWx0aXZlYyBpZGxlDQo+IA0KPiBPbiBXZWQsIDIwMTMtMDktMTEgYXQgMjI6
NDggLTA1MDAsIFdhbmcgRG9uZ3NoZW5nLUI0MDUzNCB3cm90ZToNCj4gPg0KPiA+ID4gLS0tLS1P
cmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gPiA+IEZyb206IFdvb2QgU2NvdHQtQjA3NDIxDQo+ID4g
PiBTZW50OiBUaHVyc2RheSwgU2VwdGVtYmVyIDEyLCAyMDEzIDc6MDQgQU0NCj4gPiA+IFRvOiBX
YW5nIERvbmdzaGVuZy1CNDA1MzQNCj4gPiA+IENjOiBnYWxha0BrZXJuZWwuY3Jhc2hpbmcub3Jn
OyBsaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZw0KPiA+ID4gU3ViamVjdDogUmU6IFtQQVRD
SCB2MyA0LzRdIHBvd2VycGMvODV4eDogYWRkIHN5c2ZzIGZvciBwdzIwIHN0YXRlDQo+ID4gPiBh
bmQgYWx0aXZlYyBpZGxlDQo+ID4gPg0KPiA+ID4gT24gV2VkLCAyMDEzLTA5LTExIGF0IDEzOjU2
ICswODAwLCBEb25nc2hlbmcgV2FuZyB3cm90ZToNCj4gPiA+ID4gRnJvbTogV2FuZyBEb25nc2hl
bmcgPGRvbmdzaGVuZy53YW5nQGZyZWVzY2FsZS5jb20+DQo+ID4gPiA+DQo+ID4gPiA+IEFkZCBh
IHN5cyBpbnRlcmZhY2UgdG8gZW5hYmxlL2RpYWJsZSBwdzIwIHN0YXRlIG9yIGFsdGl2ZWMgaWRs
ZSwNCj4gPiA+ID4gYW5kIGNvbnRyb2wgdGhlIHdhaXQgZW50cnkgdGltZS4NCj4gPiA+ID4NCj4g
PiA+ID4gRW5hYmxlL0Rpc2FibGUgaW50ZXJmYWNlOg0KPiA+ID4gPiAwLCBkaXNhYmxlLiAxLCBl
bmFibGUuDQo+ID4gPiA+IC9zeXMvZGV2aWNlcy9zeXN0ZW0vY3B1L2NwdVgvcHcyMF9zdGF0ZQ0K
PiA+ID4gPiAvc3lzL2RldmljZXMvc3lzdGVtL2NwdS9jcHVYL2FsdGl2ZWNfaWRsZQ0KPiA+ID4g
Pg0KPiA+ID4gPiBTZXQgd2FpdCBlbnRyeSBiaXQgaW50ZXJmYWNlOg0KPiA+ID4gPiBiaXQgdmFs
dWUgcmFuZ2UgMH42MywgMCBiaXQgaXMgTWludGltZSwgNjMgYml0IGlzIE1heHRpbWUuDQo+ID4g
PiA+IC9zeXMvZGV2aWNlcy9zeXN0ZW0vY3B1L2NwdVgvcHcyMF93YWl0X2VudHJ5X2JpdA0KPiA+
ID4gPiAvc3lzL2RldmljZXMvc3lzdGVtL2NwdS9jcHVYL2FsdGl2ZWNfaWRsZV93YWl0X2VudHJ5
X2JpdA0KPiA+ID4NCj4gPiA+IEknbSBubyBmYW4gb2YgdGhlIHdheSBwb3dlcnBjIGRvZXMgYml0
IG51bWJlcmluZywgYnV0IGRvbid0IGZsaXAgaXQNCj4gPiA+IGFyb3VuZCBoZXJlIC0tIHlvdSds
bCBqdXN0IGNhdXNlIGNvbmZ1c2lvbi4NCj4gPiA+DQo+ID4gT0suIDAgYml0IGlzIG1heHRpbWUs
IDYzIGJpdCBpcyBtaW50aW1lLg0KPiA+DQo+ID4gPiBCZXR0ZXIgeWV0LCB0aGlzIGludGVyZmFj
ZSBzaG91bGQgdGFrZSByZWFsIHRpbWUgdW5pdHMgcmF0aGVyIHRoYW4gYQ0KPiA+ID4gdGltZWJh
c2UgYml0Lg0KPiA+ID4NCj4gPiBJIHRoaW5rIHRoZSByZWFsIHRpbWUgaXMgbm90IHN1aXRhYmxl
LCBiZWNhdXNlIHRpbWViYXNlIGJpdCBkb2VzIG5vdA0KPiA+IGNvcnJlc3BvbmQgd2l0aCByZWFs
IHRpbWUuDQo+IA0KPiBJdCdzIGEgYml0IHNsb3BweSBkdWUgdG8gaG93IHRoZSBoYXJkd2FyZSB3
b3JrcywgYnV0IHlvdSBjb3VsZCBjb252ZXJ0IGl0DQo+IGxpa2UgeW91IGRpZCBpbiBlYXJsaWVy
IHBhdGNoZXMuICBTZW1hbnRpY2FsbHkgaXQgc2hvdWxkIHByb2JhYmx5IGJlIHRoZQ0KPiBtaW5p
bXVtIHRpbWUgdG8gd2FpdCBiZWZvcmUgZW50ZXJpbmcgdGhlIGxvdyBwb3dlciBzdGF0ZS4NCj4g
DQpCdXQgdGhlcmUgaGFzIGEgcHJvYmxlbSwgd2UgY2FuJ3QgY29udmVydCBiaXQgdG8gdGhlIHJl
YWwgdGltZSB3aGVuIHVzZXIgcmVhZCB0aGlzIHN5c2ZzLg0KTGlrZToNCmVjaG8gMTAwMCh1cykg
PiAvc3lzLyovcHcyMF93YWl0X2VudHJ5X2JpdCwgYWZ0ZXIgY29udmVydCB3ZSBnZXQgYml0IGlz
IDQ5Lg0KY2F0IC9zeXMvKi9wdzIwX3dhaXRfZW50cnlfYml0LCBhZnRlciBjb252ZXJ0IHRoZSB0
aW1lIGlzIDE1OTgodXMpLg0KDQpUaGUgcmVhZCBvdXQgb2YgdGhlIHRpbWUgaXMgbm90IHJlYWwg
dGltZS4gVW5sZXNzIHdlIGRlZmluZSBhIHZhcmlhYmxlIHRvIHNhdmUgdGhlIHJlYWwgdGltZS4N
Cg0KPiA+ID4gQWxzbywgeW91IGRpc2FibGUgdGhlIHBvd2VyIHNhdmluZyBtb2RlIGlmIHRoZSBt
YXhpbXVtIGludGVydmFsIGlzDQo+ID4gPiBzZWxlY3RlZCwNCj4gPiBJdCdzIG5vdCBkaXNhYmxl
IHRoZSBwdzIwIHN0YXRlIG9yIGFsdGl2ZWMgaWRsZSwganVzdCBtYXgtZGVsYXkgZW50cnkNCj4g
dGltZS4NCj4gDQo+IE5vLCB0aGUgY29kZSBjaGVja3MgZm9yIHplcm8gdG8gc2V0IG9yIGNsZWFy
IHRoZSBlbmFibGluZyBiaXQgKGUuZy4NCj4gUFcyMF9XQUlUKS4NCj4gDQpUaGVyZSBoYXMgcHcy
MF9zdGF0ZS9hbHRpdmVjX2lkbGUgc3lzIGludGVyZmFjZSB0byBjb250cm9sICJlbmFibGUvZGlz
YWJsZSIsDQpUaGVyZSBpcyBvbmx5IHRvIGNvbnRyb2wgd2FpdCBiaXQuIERpZCB5b3UgbWVhbiBy
ZW1vdmUgInB3MjBfc3RhdGUvYWx0aXZlY19pZGxlIg0Kc3lzIGludGVyZmFjZSwgYW5kIHJldXNl
ICJwdzIwX3dhaXRfZW50cnlfYml0L2FsdGl2ZWNfaWRsZSoiIHN5cyBpbnRlcmZhY2U/DQpXaGVu
IGVjaG8gemVybyBpbnRvICJwdzIwX3dhaXRfZW50cnlfYml0IiB3ZSBqdXN0IHRvIGRpc2FibGUg
cHcyMCBzdGF0ZSwgSSB0aGluayB0aGF0IGlzIHJlYXNvbmFibGUuIDopDQo=

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

* Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
  2013-09-13  2:53         ` Wang Dongsheng-B40534
@ 2013-09-16 21:09           ` Scott Wood
  2013-09-18  3:35             ` Wang Dongsheng-B40534
  0 siblings, 1 reply; 12+ messages in thread
From: Scott Wood @ 2013-09-16 21:09 UTC (permalink / raw)
  To: Wang Dongsheng-B40534; +Cc: Wood Scott-B07421, linuxppc-dev

On Thu, 2013-09-12 at 21:53 -0500, Wang Dongsheng-B40534 wrote:
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Friday, September 13, 2013 2:07 AM
> > To: Wang Dongsheng-B40534
> > Cc: Wood Scott-B07421; galak@kernel.crashing.org; linuxppc-
> > dev@lists.ozlabs.org
> > Subject: Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and
> > altivec idle
> > 
> > On Wed, 2013-09-11 at 22:48 -0500, Wang Dongsheng-B40534 wrote:
> > >
> > > > -----Original Message-----
> > > > From: Wood Scott-B07421
> > > > Sent: Thursday, September 12, 2013 7:04 AM
> > > > To: Wang Dongsheng-B40534
> > > > Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org
> > > > Subject: Re: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state
> > > > and altivec idle
> > > >
> > > > On Wed, 2013-09-11 at 13:56 +0800, Dongsheng Wang wrote:
> > > > > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > > >
> > > > > Add a sys interface to enable/diable pw20 state or altivec idle,
> > > > > and control the wait entry time.
> > > > >
> > > > > Enable/Disable interface:
> > > > > 0, disable. 1, enable.
> > > > > /sys/devices/system/cpu/cpuX/pw20_state
> > > > > /sys/devices/system/cpu/cpuX/altivec_idle
> > > > >
> > > > > Set wait entry bit interface:
> > > > > bit value range 0~63, 0 bit is Mintime, 63 bit is Maxtime.
> > > > > /sys/devices/system/cpu/cpuX/pw20_wait_entry_bit
> > > > > /sys/devices/system/cpu/cpuX/altivec_idle_wait_entry_bit
> > > >
> > > > I'm no fan of the way powerpc does bit numbering, but don't flip it
> > > > around here -- you'll just cause confusion.
> > > >
> > > OK. 0 bit is maxtime, 63 bit is mintime.
> > >
> > > > Better yet, this interface should take real time units rather than a
> > > > timebase bit.
> > > >
> > > I think the real time is not suitable, because timebase bit does not
> > > correspond with real time.
> > 
> > It's a bit sloppy due to how the hardware works, but you could convert it
> > like you did in earlier patches.  Semantically it should probably be the
> > minimum time to wait before entering the low power state.
> > 
> But there has a problem, we can't convert bit to the real time when user read this sysfs.
> Like:
> echo 1000(us) > /sys/*/pw20_wait_entry_bit, after convert we get bit is 49.
> cat /sys/*/pw20_wait_entry_bit, after convert the time is 1598(us).
> 
> The read out of the time is not real time. Unless we define a variable to save the real time.

It's not the end of the world if the value is different when read back.
It just gets rounded up when you write it.

> > > > Also, you disable the power saving mode if the maximum interval is
> > > > selected,
> > > It's not disable the pw20 state or altivec idle, just max-delay entry
> > time.
> > 
> > No, the code checks for zero to set or clear the enabling bit (e.g.
> > PW20_WAIT).
> > 
> There has pw20_state/altivec_idle sys interface to control "enable/disable",
> There is only to control wait bit. Did you mean remove "pw20_state/altivec_idle"
> sys interface, and reuse "pw20_wait_entry_bit/altivec_idle*" sys interface?
> When echo zero into "pw20_wait_entry_bit" we just to disable pw20 state, I think that is reasonable. :)

Sorry, I misread the patch and didn't realize these were separate
interfaces.

-Scott

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

* RE: [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
  2013-09-16 21:09           ` Scott Wood
@ 2013-09-18  3:35             ` Wang Dongsheng-B40534
  0 siblings, 0 replies; 12+ messages in thread
From: Wang Dongsheng-B40534 @ 2013-09-18  3:35 UTC (permalink / raw)
  To: Wood Scott-B07421; +Cc: linuxppc-dev

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBTY290dC1CMDc0
MjENCj4gU2VudDogVHVlc2RheSwgU2VwdGVtYmVyIDE3LCAyMDEzIDU6MDkgQU0NCj4gVG86IFdh
bmcgRG9uZ3NoZW5nLUI0MDUzNA0KPiBDYzogV29vZCBTY290dC1CMDc0MjE7IGdhbGFrQGtlcm5l
bC5jcmFzaGluZy5vcmc7IGxpbnV4cHBjLQ0KPiBkZXZAbGlzdHMub3psYWJzLm9yZw0KPiBTdWJq
ZWN0OiBSZTogW1BBVENIIHYzIDQvNF0gcG93ZXJwYy84NXh4OiBhZGQgc3lzZnMgZm9yIHB3MjAg
c3RhdGUgYW5kDQo+IGFsdGl2ZWMgaWRsZQ0KPiANCj4gT24gVGh1LCAyMDEzLTA5LTEyIGF0IDIx
OjUzIC0wNTAwLCBXYW5nIERvbmdzaGVuZy1CNDA1MzQgd3JvdGU6DQo+ID4NCj4gPiA+IC0tLS0t
T3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+ID4gPiBGcm9tOiBXb29kIFNjb3R0LUIwNzQyMQ0KPiA+
ID4gU2VudDogRnJpZGF5LCBTZXB0ZW1iZXIgMTMsIDIwMTMgMjowNyBBTQ0KPiA+ID4gVG86IFdh
bmcgRG9uZ3NoZW5nLUI0MDUzNA0KPiA+ID4gQ2M6IFdvb2QgU2NvdHQtQjA3NDIxOyBnYWxha0Br
ZXJuZWwuY3Jhc2hpbmcub3JnOyBsaW51eHBwYy0NCj4gPiA+IGRldkBsaXN0cy5vemxhYnMub3Jn
DQo+ID4gPiBTdWJqZWN0OiBSZTogW1BBVENIIHYzIDQvNF0gcG93ZXJwYy84NXh4OiBhZGQgc3lz
ZnMgZm9yIHB3MjAgc3RhdGUNCj4gPiA+IGFuZCBhbHRpdmVjIGlkbGUNCj4gPiA+DQo+ID4gPiBP
biBXZWQsIDIwMTMtMDktMTEgYXQgMjI6NDggLTA1MDAsIFdhbmcgRG9uZ3NoZW5nLUI0MDUzNCB3
cm90ZToNCj4gPiA+ID4NCj4gPiA+ID4gPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiA+
ID4gPiA+IEZyb206IFdvb2QgU2NvdHQtQjA3NDIxDQo+ID4gPiA+ID4gU2VudDogVGh1cnNkYXks
IFNlcHRlbWJlciAxMiwgMjAxMyA3OjA0IEFNDQo+ID4gPiA+ID4gVG86IFdhbmcgRG9uZ3NoZW5n
LUI0MDUzNA0KPiA+ID4gPiA+IENjOiBnYWxha0BrZXJuZWwuY3Jhc2hpbmcub3JnOyBsaW51eHBw
Yy1kZXZAbGlzdHMub3psYWJzLm9yZw0KPiA+ID4gPiA+IFN1YmplY3Q6IFJlOiBbUEFUQ0ggdjMg
NC80XSBwb3dlcnBjLzg1eHg6IGFkZCBzeXNmcyBmb3IgcHcyMA0KPiA+ID4gPiA+IHN0YXRlIGFu
ZCBhbHRpdmVjIGlkbGUNCj4gPiA+ID4gPg0KPiA+ID4gPiA+IE9uIFdlZCwgMjAxMy0wOS0xMSBh
dCAxMzo1NiArMDgwMCwgRG9uZ3NoZW5nIFdhbmcgd3JvdGU6DQo+ID4gPiA+ID4gPiBGcm9tOiBX
YW5nIERvbmdzaGVuZyA8ZG9uZ3NoZW5nLndhbmdAZnJlZXNjYWxlLmNvbT4NCj4gPiA+ID4gPiA+
DQo+ID4gPiA+ID4gPiBBZGQgYSBzeXMgaW50ZXJmYWNlIHRvIGVuYWJsZS9kaWFibGUgcHcyMCBz
dGF0ZSBvciBhbHRpdmVjDQo+ID4gPiA+ID4gPiBpZGxlLCBhbmQgY29udHJvbCB0aGUgd2FpdCBl
bnRyeSB0aW1lLg0KPiA+ID4gPiA+ID4NCj4gPiA+ID4gPiA+IEVuYWJsZS9EaXNhYmxlIGludGVy
ZmFjZToNCj4gPiA+ID4gPiA+IDAsIGRpc2FibGUuIDEsIGVuYWJsZS4NCj4gPiA+ID4gPiA+IC9z
eXMvZGV2aWNlcy9zeXN0ZW0vY3B1L2NwdVgvcHcyMF9zdGF0ZQ0KPiA+ID4gPiA+ID4gL3N5cy9k
ZXZpY2VzL3N5c3RlbS9jcHUvY3B1WC9hbHRpdmVjX2lkbGUNCj4gPiA+ID4gPiA+DQo+ID4gPiA+
ID4gPiBTZXQgd2FpdCBlbnRyeSBiaXQgaW50ZXJmYWNlOg0KPiA+ID4gPiA+ID4gYml0IHZhbHVl
IHJhbmdlIDB+NjMsIDAgYml0IGlzIE1pbnRpbWUsIDYzIGJpdCBpcyBNYXh0aW1lLg0KPiA+ID4g
PiA+ID4gL3N5cy9kZXZpY2VzL3N5c3RlbS9jcHUvY3B1WC9wdzIwX3dhaXRfZW50cnlfYml0DQo+
ID4gPiA+ID4gPiAvc3lzL2RldmljZXMvc3lzdGVtL2NwdS9jcHVYL2FsdGl2ZWNfaWRsZV93YWl0
X2VudHJ5X2JpdA0KPiA+ID4gPiA+DQo+ID4gPiA+ID4gSSdtIG5vIGZhbiBvZiB0aGUgd2F5IHBv
d2VycGMgZG9lcyBiaXQgbnVtYmVyaW5nLCBidXQgZG9uJ3QgZmxpcA0KPiA+ID4gPiA+IGl0IGFy
b3VuZCBoZXJlIC0tIHlvdSdsbCBqdXN0IGNhdXNlIGNvbmZ1c2lvbi4NCj4gPiA+ID4gPg0KPiA+
ID4gPiBPSy4gMCBiaXQgaXMgbWF4dGltZSwgNjMgYml0IGlzIG1pbnRpbWUuDQo+ID4gPiA+DQo+
ID4gPiA+ID4gQmV0dGVyIHlldCwgdGhpcyBpbnRlcmZhY2Ugc2hvdWxkIHRha2UgcmVhbCB0aW1l
IHVuaXRzIHJhdGhlcg0KPiA+ID4gPiA+IHRoYW4gYSB0aW1lYmFzZSBiaXQuDQo+ID4gPiA+ID4N
Cj4gPiA+ID4gSSB0aGluayB0aGUgcmVhbCB0aW1lIGlzIG5vdCBzdWl0YWJsZSwgYmVjYXVzZSB0
aW1lYmFzZSBiaXQgZG9lcw0KPiA+ID4gPiBub3QgY29ycmVzcG9uZCB3aXRoIHJlYWwgdGltZS4N
Cj4gPiA+DQo+ID4gPiBJdCdzIGEgYml0IHNsb3BweSBkdWUgdG8gaG93IHRoZSBoYXJkd2FyZSB3
b3JrcywgYnV0IHlvdSBjb3VsZA0KPiA+ID4gY29udmVydCBpdCBsaWtlIHlvdSBkaWQgaW4gZWFy
bGllciBwYXRjaGVzLiAgU2VtYW50aWNhbGx5IGl0IHNob3VsZA0KPiA+ID4gcHJvYmFibHkgYmUg
dGhlIG1pbmltdW0gdGltZSB0byB3YWl0IGJlZm9yZSBlbnRlcmluZyB0aGUgbG93IHBvd2VyDQo+
IHN0YXRlLg0KPiA+ID4NCj4gPiBCdXQgdGhlcmUgaGFzIGEgcHJvYmxlbSwgd2UgY2FuJ3QgY29u
dmVydCBiaXQgdG8gdGhlIHJlYWwgdGltZSB3aGVuDQo+IHVzZXIgcmVhZCB0aGlzIHN5c2ZzLg0K
PiA+IExpa2U6DQo+ID4gZWNobyAxMDAwKHVzKSA+IC9zeXMvKi9wdzIwX3dhaXRfZW50cnlfYml0
LCBhZnRlciBjb252ZXJ0IHdlIGdldCBiaXQgaXMNCj4gNDkuDQo+ID4gY2F0IC9zeXMvKi9wdzIw
X3dhaXRfZW50cnlfYml0LCBhZnRlciBjb252ZXJ0IHRoZSB0aW1lIGlzIDE1OTgodXMpLg0KPiA+
DQo+ID4gVGhlIHJlYWQgb3V0IG9mIHRoZSB0aW1lIGlzIG5vdCByZWFsIHRpbWUuIFVubGVzcyB3
ZSBkZWZpbmUgYSB2YXJpYWJsZQ0KPiB0byBzYXZlIHRoZSByZWFsIHRpbWUuDQo+IA0KPiBJdCdz
IG5vdCB0aGUgZW5kIG9mIHRoZSB3b3JsZCBpZiB0aGUgdmFsdWUgaXMgZGlmZmVyZW50IHdoZW4g
cmVhZCBiYWNrLg0KPiBJdCBqdXN0IGdldHMgcm91bmRlZCB1cCB3aGVuIHlvdSB3cml0ZSBpdC4N
Cj4gDQpPaywgbWFrZSBhIHZhcmlhYmxlIHRvIHNhdmUgaXQuDQoNCj4gPiA+ID4gPiBBbHNvLCB5
b3UgZGlzYWJsZSB0aGUgcG93ZXIgc2F2aW5nIG1vZGUgaWYgdGhlIG1heGltdW0gaW50ZXJ2YWwN
Cj4gPiA+ID4gPiBpcyBzZWxlY3RlZCwNCj4gPiA+ID4gSXQncyBub3QgZGlzYWJsZSB0aGUgcHcy
MCBzdGF0ZSBvciBhbHRpdmVjIGlkbGUsIGp1c3QgbWF4LWRlbGF5DQo+ID4gPiA+IGVudHJ5DQo+
ID4gPiB0aW1lLg0KPiA+ID4NCj4gPiA+IE5vLCB0aGUgY29kZSBjaGVja3MgZm9yIHplcm8gdG8g
c2V0IG9yIGNsZWFyIHRoZSBlbmFibGluZyBiaXQgKGUuZy4NCj4gPiA+IFBXMjBfV0FJVCkuDQo+
ID4gPg0KPiA+IFRoZXJlIGhhcyBwdzIwX3N0YXRlL2FsdGl2ZWNfaWRsZSBzeXMgaW50ZXJmYWNl
IHRvIGNvbnRyb2wNCj4gPiAiZW5hYmxlL2Rpc2FibGUiLCBUaGVyZSBpcyBvbmx5IHRvIGNvbnRy
b2wgd2FpdCBiaXQuIERpZCB5b3UgbWVhbg0KPiByZW1vdmUgInB3MjBfc3RhdGUvYWx0aXZlY19p
ZGxlIg0KPiA+IHN5cyBpbnRlcmZhY2UsIGFuZCByZXVzZSAicHcyMF93YWl0X2VudHJ5X2JpdC9h
bHRpdmVjX2lkbGUqIiBzeXMNCj4gaW50ZXJmYWNlPw0KPiA+IFdoZW4gZWNobyB6ZXJvIGludG8g
InB3MjBfd2FpdF9lbnRyeV9iaXQiIHdlIGp1c3QgdG8gZGlzYWJsZSBwdzIwDQo+ID4gc3RhdGUs
IEkgdGhpbmsgdGhhdCBpcyByZWFzb25hYmxlLiA6KQ0KPiANCj4gU29ycnksIEkgbWlzcmVhZCB0
aGUgcGF0Y2ggYW5kIGRpZG4ndCByZWFsaXplIHRoZXNlIHdlcmUgc2VwYXJhdGUNCj4gaW50ZXJm
YWNlcy4NCktlZXAgdGhlICJwdzIwX3N0YXRlL2FsdGl2ZWNfaWRsZSIgaW50ZXJmYWNlcy4NCg0K
LWRvbmdzaGVuZw0K

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

end of thread, other threads:[~2013-09-18  3:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-11  5:56 [PATCH v3 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define Dongsheng Wang
2013-09-11  5:56 ` [PATCH v3 2/4] powerpc/85xx: add hardware automatically enter altivec idle state Dongsheng Wang
2013-09-11 22:42   ` Scott Wood
2013-09-12  2:27     ` Wang Dongsheng-B40534
2013-09-11  5:56 ` [PATCH v3 3/4] powerpc/85xx: add hardware automatically enter pw20 state Dongsheng Wang
2013-09-11  5:56 ` [PATCH v3 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle Dongsheng Wang
2013-09-11 23:04   ` Scott Wood
2013-09-12  3:48     ` Wang Dongsheng-B40534
2013-09-12 18:06       ` Scott Wood
2013-09-13  2:53         ` Wang Dongsheng-B40534
2013-09-16 21:09           ` Scott Wood
2013-09-18  3:35             ` Wang Dongsheng-B40534

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).