LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv3 0/2] sched: arch_scale_smt_powers
From: Joel Schopp @ 2010-01-28 23:20 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: ego, linux-kernel, Ingo Molnar, linuxppc-dev
In-Reply-To: <1264548479.12239.54.camel@jschopp-laptop>

The new Power7 processor has 4 way SMT.  This 4 way SMT benefits from
dynamic power updates that arch_scale_smt_power was designed to provide.
The first patch fixes a generic scheduler bug necessary for arch_scale_smt
to properly function.  The second patch implements arch_scale_smt_power 
for powerpc, and in particular for Power7 processors.

Version 2 changes:
- Drop Patch 1 from the original series since it's in the -tip tree now
- Move enabling the cpu feature into it's own patch (now patch 1)
- Add stubbing out broken x86 implementation to patch 1
- clean up coding style in patch 2

Version 3 changes:
- Added a comment to Patch 2

---
Joel Schopp (2):
powerpc: implement arch_scale_smt_power for Power7
sched: enable ARCH_POWER

 arch/powerpc/include/asm/cputable.h |    3 +
 arch/powerpc/kernel/smp.c           |   56
++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/sched.c         |    6 +--
 kernel/sched_features.h             |    2 -
 4 files changed, 61 insertions(+), 6 deletions(-)

^ permalink raw reply

* [PATCHv3 1/2] sched: enable ARCH_POWER
From: Joel Schopp @ 2010-01-28 23:20 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: ego, linux-kernel, Ingo Molnar, linuxppc-dev
In-Reply-To: <1264548486.12239.55.camel@jschopp-laptop>

Enable the scheduler feature that allows use of arch_scale_smt_power.  Stub out
the broken x86 implementation.

Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
---
Index: linux-2.6.git/kernel/sched_features.h
===================================================================
--- linux-2.6.git.orig/kernel/sched_features.h
+++ linux-2.6.git/kernel/sched_features.h
@@ -102,7 +102,7 @@ SCHED_FEAT(CACHE_HOT_BUDDY, 1)
 /*
  * Use arch dependent cpu power functions
  */
-SCHED_FEAT(ARCH_POWER, 0)
+SCHED_FEAT(ARCH_POWER, 1)
 
 SCHED_FEAT(HRTICK, 0)
 SCHED_FEAT(DOUBLE_TICK, 0)
Index: linux-2.6.git/arch/x86/kernel/cpu/sched.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/cpu/sched.c
+++ linux-2.6.git/arch/x86/kernel/cpu/sched.c
@@ -44,11 +44,9 @@ unsigned long arch_scale_freq_power(stru
 unsigned long arch_scale_smt_power(struct sched_domain *sd, int cpu)
 {
 	/*
-	 * aperf/mperf already includes the smt gain
+	 * aperf/mperf already includes the smt gain, but represents capacity
+	 * as 0 when idle.  So for now just return default.
 	 */
-	if (boot_cpu_has(X86_FEATURE_APERFMPERF))
-		return SCHED_LOAD_SCALE;
-
 	return default_scale_smt_power(sd, cpu);
 }

^ permalink raw reply

* [PATCHv3 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Joel Schopp @ 2010-01-28 23:20 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: ego, linux-kernel, Ingo Molnar, linuxppc-dev
In-Reply-To: <1264548495.12239.56.camel@jschopp-laptop>

On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
there is performance benefit to idling the higher numbered threads in
the core.  

This patch implements arch_scale_smt_power to dynamically update smt
thread power in these idle cases in order to prefer threads 0,1 over
threads 2,3 within a core.


Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
---
Version 2 addresses style and optimization, same basic functionality
Version 3 adds a comment
On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
there is performance benefit to idling the higher numbered threads in
the core.  

This patch implements arch_scale_smt_power to dynamically update smt
thread power in these idle cases in order to prefer threads 0,1 over
threads 2,3 within a core.

Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
---
Version 2 addresses style and optimization, same basic functionality
Index: linux-2.6.git/arch/powerpc/kernel/smp.c
===================================================================
--- linux-2.6.git.orig/arch/powerpc/kernel/smp.c
+++ linux-2.6.git/arch/powerpc/kernel/smp.c
@@ -620,3 +620,59 @@ void __cpu_die(unsigned int cpu)
 		smp_ops->cpu_die(cpu);
 }
 #endif
+
+unsigned long arch_scale_smt_power(struct sched_domain *sd, int cpu)
+{
+	int sibling;
+	int idle_count = 0;
+	int thread;
+
+	/* Setup the default weight and smt_gain used by most cpus for SMT
+	 * Power.  Doing this right away covers the default case and can be
+	 * used by cpus that modify it dynamically.
+	 */
+	struct cpumask *sibling_map = sched_domain_span(sd);
+	unsigned long weight = cpumask_weight(sibling_map);
+	unsigned long smt_gain = sd->smt_gain;
+
+
+	if (cpu_has_feature(CPU_FTR_ASYNC_SMT4) && weight == 4) {
+		for_each_cpu(sibling, sibling_map) {
+			if (idle_cpu(sibling))
+				idle_count++;
+		}
+
+		/* the following section attempts to tweak cpu power based
+		 * on current idleness of the threads dynamically at runtime
+		 */
+		if (idle_count > 1) {
+			thread = cpu_thread_in_core(cpu);
+			if (thread < 2) {
+				/* add 75 % to thread power */
+				smt_gain += (smt_gain >> 1) + (smt_gain >> 2);
+			} else {
+				 /* subtract 75 % to thread power */
+				smt_gain = smt_gain >> 2;
+			}
+		}
+	}
+
+	/* default smt gain is 1178, weight is # of SMT threads */
+	switch (weight) {
+	case 1:
+		/*divide by 1, do nothing*/
+		break;
+	case 2:
+		smt_gain = smt_gain >> 1;
+		break;
+	case 4:
+		smt_gain = smt_gain >> 2;
+		break;
+	default:
+		smt_gain /= weight;
+		break;
+	}
+
+	return smt_gain;
+
+}
Index: linux-2.6.git/arch/powerpc/include/asm/cputable.h
===================================================================
--- linux-2.6.git.orig/arch/powerpc/include/asm/cputable.h
+++ linux-2.6.git/arch/powerpc/include/asm/cputable.h
@@ -195,6 +195,7 @@ extern const char *powerpc_base_platform
 #define CPU_FTR_SAO			LONG_ASM_CONST(0x0020000000000000)
 #define CPU_FTR_CP_USE_DCBTZ		LONG_ASM_CONST(0x0040000000000000)
 #define CPU_FTR_UNALIGNED_LD_STD	LONG_ASM_CONST(0x0080000000000000)
+#define CPU_FTR_ASYNC_SMT4		LONG_ASM_CONST(0x0100000000000000)
 
 #ifndef __ASSEMBLY__
 
@@ -409,7 +410,7 @@ extern const char *powerpc_base_platform
 	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
 	    CPU_FTR_COHERENT_ICACHE | CPU_FTR_LOCKLESS_TLBIE | \
 	    CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
-	    CPU_FTR_DSCR | CPU_FTR_SAO)
+	    CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYNC_SMT4)
 #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
 	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
 	    CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \

^ permalink raw reply

* [PATCH] powerpc: Dynamically allocate pacas
From: Michael Ellerman @ 2010-01-28 23:23 UTC (permalink / raw)
  To: linuxppc-dev

On 64-bit kernels we currently have a 512 byte struct paca_struct for
each cpu (usually just called "the paca"). Currently they are statically
allocated, which means a kernel built for a large number of cpus will
waste a lot of space if it's booted on a machine with few cpus.

We can avoid that by only allocating the number of pacas we need at
boot. However this is complicated by the fact that we need to access
the paca before we know how many cpus there are in the system.

The solution is to dynamically allocate enough space for NR_CPUS pacas,
but then later in boot when we know how many cpus we have, we free any
unused pacas.

Lightly booted on Legacy iSeries & pSeries LPAR.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
 arch/powerpc/include/asm/paca.h            |   18 +++++-
 arch/powerpc/kernel/head_64.S              |    6 +-
 arch/powerpc/kernel/paca.c                 |   93 +++++++++++++++++++++-------
 arch/powerpc/kernel/prom.c                 |    3 +
 arch/powerpc/kernel/setup-common.c         |    3 +
 arch/powerpc/kernel/setup_64.c             |   12 ++--
 arch/powerpc/platforms/iseries/exception.S |   25 +++++---
 7 files changed, 118 insertions(+), 42 deletions(-)


v3: Really fixed the 32-bit build.

diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
index 5e9b4ef..fbcd003 100644
--- a/arch/powerpc/include/asm/paca.h
+++ b/arch/powerpc/include/asm/paca.h
@@ -14,6 +14,9 @@
 #define _ASM_POWERPC_PACA_H
 #ifdef __KERNEL__
 
+#ifdef CONFIG_PPC64
+
+#include <linux/init.h>
 #include <asm/types.h>
 #include <asm/lppaca.h>
 #include <asm/mmu.h>
@@ -140,8 +143,19 @@ struct paca_struct {
 #endif
 };
 
-extern struct paca_struct paca[];
-extern void initialise_pacas(void);
+extern struct paca_struct *paca;
+extern __initdata struct paca_struct boot_paca;
+extern void initialise_paca(struct paca_struct *new_paca, int cpu);
+
+extern void allocate_pacas(void);
+extern void free_unused_pacas(void);
+
+#else /* CONFIG_PPC64 */
+
+static inline void allocate_pacas(void) { };
+static inline void free_unused_pacas(void) { };
+
+#endif /* CONFIG_PPC64 */
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_PACA_H */
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index 9258074..b24b700 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -219,7 +219,8 @@ generic_secondary_common_init:
 	 * physical cpu id in r24, we need to search the pacas to find
 	 * which logical id maps to our physical one.
 	 */
-	LOAD_REG_ADDR(r13, paca)	/* Get base vaddr of paca array	 */
+	LOAD_REG_ADDR(r13, paca)	/* Load paca pointer		 */
+	ld	r13,0(r13)		/* Get base vaddr of paca array	 */
 	li	r5,0			/* logical cpu id                */
 1:	lhz	r6,PACAHWCPUID(r13)	/* Load HW procid from paca      */
 	cmpw	r6,r24			/* Compare to our id             */
@@ -536,7 +537,8 @@ _GLOBAL(pmac_secondary_start)
 	mtmsrd	r3			/* RI on */
 
 	/* Set up a paca value for this processor. */
-	LOAD_REG_ADDR(r4,paca)		/* Get base vaddr of paca array	*/
+	LOAD_REG_ADDR(r4,paca)		/* Load paca pointer		*/
+	ld	r4,0(r4)		/* Get base vaddr of paca array	*/
 	mulli	r13,r24,PACA_SIZE	/* Calculate vaddr of right paca */
 	add	r13,r13,r4		/* for this processor.		*/
 	mtspr	SPRN_SPRG_PACA,r13	/* Save vaddr of paca in an SPRG*/
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index d16b1ea..0c40c6f 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -9,11 +9,15 @@
 
 #include <linux/threads.h>
 #include <linux/module.h>
+#include <linux/lmb.h>
 
+#include <asm/firmware.h>
 #include <asm/lppaca.h>
 #include <asm/paca.h>
 #include <asm/sections.h>
 #include <asm/pgtable.h>
+#include <asm/iseries/lpar_map.h>
+#include <asm/iseries/hv_types.h>
 
 /* This symbol is provided by the linker - let it fill in the paca
  * field correctly */
@@ -70,37 +74,82 @@ struct slb_shadow slb_shadow[] __cacheline_aligned = {
  * processors.  The processor VPD array needs one entry per physical
  * processor (not thread).
  */
-struct paca_struct paca[NR_CPUS];
+struct paca_struct *paca;
 EXPORT_SYMBOL(paca);
 
-void __init initialise_pacas(void)
-{
-	int cpu;
+struct paca_struct boot_paca;
 
-	/* The TOC register (GPR2) points 32kB into the TOC, so that 64kB
-	 * of the TOC can be addressed using a single machine instruction.
-	 */
+void __init initialise_paca(struct paca_struct *new_paca, int cpu)
+{
+       /* The TOC register (GPR2) points 32kB into the TOC, so that 64kB
+	* of the TOC can be addressed using a single machine instruction.
+	*/
 	unsigned long kernel_toc = (unsigned long)(&__toc_start) + 0x8000UL;
 
-	/* Can't use for_each_*_cpu, as they aren't functional yet */
-	for (cpu = 0; cpu < NR_CPUS; cpu++) {
-		struct paca_struct *new_paca = &paca[cpu];
-
 #ifdef CONFIG_PPC_BOOK3S
-		new_paca->lppaca_ptr = &lppaca[cpu];
+	new_paca->lppaca_ptr = &lppaca[cpu];
 #else
-		new_paca->kernel_pgd = swapper_pg_dir;
+	new_paca->kernel_pgd = swapper_pg_dir;
 #endif
-		new_paca->lock_token = 0x8000;
-		new_paca->paca_index = cpu;
-		new_paca->kernel_toc = kernel_toc;
-		new_paca->kernelbase = (unsigned long) _stext;
-		new_paca->kernel_msr = MSR_KERNEL;
-		new_paca->hw_cpu_id = 0xffff;
-		new_paca->__current = &init_task;
+	new_paca->lock_token = 0x8000;
+	new_paca->paca_index = cpu;
+	new_paca->kernel_toc = kernel_toc;
+	new_paca->kernelbase = (unsigned long) _stext;
+	new_paca->kernel_msr = MSR_KERNEL;
+	new_paca->hw_cpu_id = 0xffff;
+	new_paca->__current = &init_task;
 #ifdef CONFIG_PPC_STD_MMU_64
-		new_paca->slb_shadow_ptr = &slb_shadow[cpu];
+	new_paca->slb_shadow_ptr = &slb_shadow[cpu];
 #endif /* CONFIG_PPC_STD_MMU_64 */
+}
+
+static int __initdata paca_size;
+
+void __init allocate_pacas(void)
+{
+	int nr_cpus, cpu, limit;
+
+	/*
+	 * We can't take SLB misses on the paca, and we want to access them
+	 * in real mode, so allocate them within the RMA and also within
+	 * the first segment. On iSeries they must be within the area mapped
+	 * by the HV, which is HvPagesToMap * HVPAGESIZE bytes.
+	 */
+	limit = min(0x10000000ULL, lmb.rmo_size);
+	if (firmware_has_feature(FW_FEATURE_ISERIES))
+		limit = min(limit, HvPagesToMap * HVPAGESIZE);
+
+	nr_cpus = NR_CPUS;
+	/* On iSeries we know we can never have more than 64 cpus */
+	if (firmware_has_feature(FW_FEATURE_ISERIES))
+		nr_cpus = min(64, nr_cpus);
+
+	paca_size = PAGE_ALIGN(sizeof(struct paca_struct) * nr_cpus);
+
+	paca = __va(lmb_alloc_base(paca_size, PAGE_SIZE, limit));
+	memset(paca, 0, paca_size);
+
+	printk(KERN_DEBUG "Allocated %u bytes for %d pacas at %p\n",
+		paca_size, nr_cpus, paca);
+
+	/* Can't use for_each_*_cpu, as they aren't functional yet */
+	for (cpu = 0; cpu < nr_cpus; cpu++)
+		initialise_paca(&paca[cpu], cpu);
+}
+
+void __init free_unused_pacas(void)
+{
+	int new_size;
+
+	new_size = PAGE_ALIGN(sizeof(struct paca_struct) * num_possible_cpus());
+
+	if (new_size >= paca_size)
+		return;
+
+	lmb_free(__pa(paca) + new_size, paca_size - new_size);
+
+	printk(KERN_DEBUG "Freed %u bytes for unused pacas\n",
+		paca_size - new_size);
 
-	}
+	paca_size = new_size;
 }
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4ec3008..6e0cab0 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -43,6 +43,7 @@
 #include <asm/smp.h>
 #include <asm/system.h>
 #include <asm/mmu.h>
+#include <asm/paca.h>
 #include <asm/pgtable.h>
 #include <asm/pci.h>
 #include <asm/iommu.h>
@@ -1224,6 +1225,8 @@ void __init early_init_devtree(void *params)
 	 * FIXME .. and the initrd too? */
 	move_device_tree();
 
+	allocate_pacas();
+
 	DBG("Scanning CPUs ...\n");
 
 	/* Retreive CPU related informations from the flat tree
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 03dd6a2..48f0a00 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -36,6 +36,7 @@
 #include <linux/lmb.h>
 #include <linux/of_platform.h>
 #include <asm/io.h>
+#include <asm/paca.h>
 #include <asm/prom.h>
 #include <asm/processor.h>
 #include <asm/vdso_datapage.h>
@@ -493,6 +494,8 @@ void __init smp_setup_cpu_maps(void)
 	 * here will have to be reworked
 	 */
 	cpu_init_thread_core_maps(nthreads);
+
+	free_unused_pacas();
 }
 #endif /* CONFIG_SMP */
 
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 6568406..6354739 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -144,9 +144,9 @@ early_param("smt-enabled", early_smt_enabled);
 #endif /* CONFIG_SMP */
 
 /* Put the paca pointer into r13 and SPRG_PACA */
-void __init setup_paca(int cpu)
+static void __init setup_paca(struct paca_struct *new_paca)
 {
-	local_paca = &paca[cpu];
+	local_paca = new_paca;
 	mtspr(SPRN_SPRG_PACA, local_paca);
 #ifdef CONFIG_PPC_BOOK3E
 	mtspr(SPRN_SPRG_TLB_EXFRAME, local_paca->extlb);
@@ -176,14 +176,12 @@ void __init early_setup(unsigned long dt_ptr)
 {
 	/* -------- printk is _NOT_ safe to use here ! ------- */
 
-	/* Fill in any unititialised pacas */
-	initialise_pacas();
-
 	/* Identify CPU type */
 	identify_cpu(0, mfspr(SPRN_PVR));
 
 	/* Assume we're on cpu 0 for now. Don't write to the paca yet! */
-	setup_paca(0);
+	initialise_paca(&boot_paca, 0);
+	setup_paca(&boot_paca);
 
 	/* Initialize lockdep early or else spinlocks will blow */
 	lockdep_init();
@@ -203,7 +201,7 @@ void __init early_setup(unsigned long dt_ptr)
 	early_init_devtree(__va(dt_ptr));
 
 	/* Now we know the logical id of our boot cpu, setup the paca. */
-	setup_paca(boot_cpuid);
+	setup_paca(&paca[boot_cpuid]);
 
 	/* Fix up paca fields required for the boot cpu */
 	get_paca()->cpu_start = 1;
diff --git a/arch/powerpc/platforms/iseries/exception.S b/arch/powerpc/platforms/iseries/exception.S
index 5369653..fba5bf9 100644
--- a/arch/powerpc/platforms/iseries/exception.S
+++ b/arch/powerpc/platforms/iseries/exception.S
@@ -43,17 +43,14 @@ system_reset_iSeries:
 	LOAD_REG_ADDR(r23, alpaca)
 	li	r0,ALPACA_SIZE
 	sub	r23,r13,r23
-	divdu	r23,r23,r0		/* r23 has cpu number */
-	LOAD_REG_ADDR(r13, paca)
-	mulli	r0,r23,PACA_SIZE
-	add	r13,r13,r0
-	mtspr	SPRN_SPRG_PACA,r13	/* Save it away for the future */
-	mfmsr	r24
-	ori	r24,r24,MSR_RI
-	mtmsrd	r24			/* RI on */
-	mr	r24,r23
+	divdu	r24,r23,r0		/* r24 has cpu number */
 	cmpwi	0,r24,0			/* Are we processor 0? */
 	bne	1f
+	LOAD_REG_ADDR(r13, boot_paca)
+	mtspr	SPRN_SPRG_PACA,r13	/* Save it away for the future */
+	mfmsr	r23
+	ori	r23,r23,MSR_RI
+	mtmsrd	r23			/* RI on */
 	b	.__start_initialization_iSeries	/* Start up the first processor */
 1:	mfspr	r4,SPRN_CTRLF
 	li	r5,CTRL_RUNLATCH	/* Turn off the run light */
@@ -86,6 +83,16 @@ system_reset_iSeries:
 #endif
 
 2:
+	/* Load our paca now that it's been allocated */
+	LOAD_REG_ADDR(r13, paca)
+	ld	r13,0(r13)
+	mulli	r0,r24,PACA_SIZE
+	add	r13,r13,r0
+	mtspr	SPRN_SPRG_PACA,r13	/* Save it away for the future */
+	mfmsr	r23
+	ori	r23,r23,MSR_RI
+	mtmsrd	r23			/* RI on */
+
 	HMT_LOW
 #ifdef CONFIG_SMP
 	lbz	r23,PACAPROCSTART(r13)	/* Test if this processor
-- 
1.6.3.3

^ permalink raw reply related

* [PATCHv3 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Joel Schopp @ 2010-01-28 23:24 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Ingo Molnar, linuxppc-dev, linux-kernel, ego
In-Reply-To: <1264720855.9660.22.camel@jschopp-laptop>

On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
there is performance benefit to idling the higher numbered threads in
the core.  

This patch implements arch_scale_smt_power to dynamically update smt
thread power in these idle cases in order to prefer threads 0,1 over
threads 2,3 within a core.


Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
---
Version 2 addresses style and optimization, same basic functionality
Version 3 adds a comment, resent due to mailing format error
Index: linux-2.6.git/arch/powerpc/kernel/smp.c
===================================================================
--- linux-2.6.git.orig/arch/powerpc/kernel/smp.c
+++ linux-2.6.git/arch/powerpc/kernel/smp.c
@@ -620,3 +620,59 @@ void __cpu_die(unsigned int cpu)
 		smp_ops->cpu_die(cpu);
 }
 #endif
+
+unsigned long arch_scale_smt_power(struct sched_domain *sd, int cpu)
+{
+	int sibling;
+	int idle_count = 0;
+	int thread;
+
+	/* Setup the default weight and smt_gain used by most cpus for SMT
+	 * Power.  Doing this right away covers the default case and can be
+	 * used by cpus that modify it dynamically.
+	 */
+	struct cpumask *sibling_map = sched_domain_span(sd);
+	unsigned long weight = cpumask_weight(sibling_map);
+	unsigned long smt_gain = sd->smt_gain;
+
+
+	if (cpu_has_feature(CPU_FTR_ASYNC_SMT4) && weight == 4) {
+		for_each_cpu(sibling, sibling_map) {
+			if (idle_cpu(sibling))
+				idle_count++;
+		}
+
+		/* the following section attempts to tweak cpu power based
+		 * on current idleness of the threads dynamically at runtime
+		 */
+		if (idle_count > 1) {
+			thread = cpu_thread_in_core(cpu);
+			if (thread < 2) {
+				/* add 75 % to thread power */
+				smt_gain += (smt_gain >> 1) + (smt_gain >> 2);
+			} else {
+				 /* subtract 75 % to thread power */
+				smt_gain = smt_gain >> 2;
+			}
+		}
+	}
+
+	/* default smt gain is 1178, weight is # of SMT threads */
+	switch (weight) {
+	case 1:
+		/*divide by 1, do nothing*/
+		break;
+	case 2:
+		smt_gain = smt_gain >> 1;
+		break;
+	case 4:
+		smt_gain = smt_gain >> 2;
+		break;
+	default:
+		smt_gain /= weight;
+		break;
+	}
+
+	return smt_gain;
+
+}
Index: linux-2.6.git/arch/powerpc/include/asm/cputable.h
===================================================================
--- linux-2.6.git.orig/arch/powerpc/include/asm/cputable.h
+++ linux-2.6.git/arch/powerpc/include/asm/cputable.h
@@ -195,6 +195,7 @@ extern const char *powerpc_base_platform
 #define CPU_FTR_SAO			LONG_ASM_CONST(0x0020000000000000)
 #define CPU_FTR_CP_USE_DCBTZ		LONG_ASM_CONST(0x0040000000000000)
 #define CPU_FTR_UNALIGNED_LD_STD	LONG_ASM_CONST(0x0080000000000000)
+#define CPU_FTR_ASYNC_SMT4		LONG_ASM_CONST(0x0100000000000000)
 
 #ifndef __ASSEMBLY__
 
@@ -409,7 +410,7 @@ extern const char *powerpc_base_platform
 	    CPU_FTR_MMCRA | CPU_FTR_SMT | \
 	    CPU_FTR_COHERENT_ICACHE | CPU_FTR_LOCKLESS_TLBIE | \
 	    CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
-	    CPU_FTR_DSCR | CPU_FTR_SAO)
+	    CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYNC_SMT4)
 #define CPU_FTRS_CELL	(CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
 	    CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
 	    CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \

^ permalink raw reply

* Re: [PATCH 1/2] eeh: Fixing a bug when pci structure is null
From: Benjamin Herrenschmidt @ 2010-01-29  0:03 UTC (permalink / raw)
  To: leitao; +Cc: linuxppc-dev, Linas Vepstas
In-Reply-To: <0642ead63df1b9fdced24750eb0aea940f0408b7.1264617281.git.root@sanx1002.austin.ibm.com>

On Wed, 2010-01-27 at 12:43 -0600, leitao@linux.vnet.ibm.com wrote:

> diff --git a/arch/powerpc/platforms/pseries/eeh_driver.c b/arch/powerpc/platforms/pseries/eeh_driver.c
> index ef8e454..afdddf6 100644
> --- a/arch/powerpc/platforms/pseries/eeh_driver.c
> +++ b/arch/powerpc/platforms/pseries/eeh_driver.c
> @@ -41,6 +41,13 @@ static inline const char * pcid_name (struct pci_dev *pdev)
>  	return "";
>  }
>  
> +inline const char *eeh_pci_name(struct pci_dev *pdev)
> +{
> +        if (NULL==pdev)
> +                return "<null>";
> +        return pci_name(pdev);
> +}
> +

Coding style gack ... just make it static inline in the header file.

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCH 2/2] eeh: fixing pci_dev dependency
From: Benjamin Herrenschmidt @ 2010-01-29  0:04 UTC (permalink / raw)
  To: leitao; +Cc: linuxppc-dev, Linas Vepstas
In-Reply-To: <930155d7427ae670cd2add220649c6490f68d0d4.1264617281.git.root@sanx1002.austin.ibm.com>

On Wed, 2010-01-27 at 12:43 -0600, leitao@linux.vnet.ibm.com wrote:
> Currently pci_dev can be null when EEH is in action. This patch
> just assure that we pci_dev is not NULL before calling pci_dev_put.

Like all variants of *_put(), it already checks for a NULL argument
afaik. So that patch should be unnecessary.

Cheers,
Ben.
 
> Signed-off-by: Breno Leitao <leitao@linux.vnet.ibm.com>
> Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>
> ---
>  arch/powerpc/platforms/pseries/eeh_event.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/eeh_event.c b/arch/powerpc/platforms/pseries/eeh_event.c
> index ec5df8f..7956e46 100644
> --- a/arch/powerpc/platforms/pseries/eeh_event.c
> +++ b/arch/powerpc/platforms/pseries/eeh_event.c
> @@ -85,7 +85,8 @@ static int eeh_event_handler(void * dummy)
>  	pdn = handle_eeh_events(event);
>  
>  	eeh_clear_slot(event->dn, EEH_MODE_RECOVERING);
> -	pci_dev_put(event->dev);
> +	if (event->dev)
> +		pci_dev_put(event->dev);
>  	kfree(event);
>  	mutex_unlock(&eeh_event_mutex);
>  

^ permalink raw reply

* Re: [PATCHv3 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Benjamin Herrenschmidt @ 2010-01-29  1:23 UTC (permalink / raw)
  To: Joel Schopp; +Cc: linux-kernel, Ingo Molnar, linuxppc-dev, Peter Zijlstra, ego
In-Reply-To: <1264721088.10385.1.camel@jschopp-laptop>

On Thu, 2010-01-28 at 17:24 -0600, Joel Schopp wrote:
> On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
> there is performance benefit to idling the higher numbered threads in
> the core.  
> 
> This patch implements arch_scale_smt_power to dynamically update smt
> thread power in these idle cases in order to prefer threads 0,1 over
> threads 2,3 within a core.

Almost there :-) Joel, Peter, can you help me figure something out tho ?

On machine that don't have SMT, I would like to avoid calling
arch_scale_smt_power() at all if possible (in addition to not compiling
it in if SMT is not enabled in .config).

Now, I must say I'm utterly confused by how the domains are setup and I
haven't quite managed to sort it out... it looks to me that
SD_SHARE_CPUPOWER is always going to be set on all CPUs when the config
option is set (though each CPU will have its own domain) or am I
misguided ? IE. Is there any sense in having at least a fast exit path
out of arch_scale_smt_power() for non-SMT CPUs ?

Joel, can you look at compiling it out when SMT is not set ? We don't
want to bloat SMP kernels for 32-bit non-SMT embedded platforms.

Oh, and one minor nit:

> Signed-off-by: Joel Schopp <jschopp@austin.ibm.com>
> ---
> Version 2 addresses style and optimization, same basic functionality
> Version 3 adds a comment, resent due to mailing format error
> Index: linux-2.6.git/arch/powerpc/kernel/smp.c
> ===================================================================
> --- linux-2.6.git.orig/arch/powerpc/kernel/smp.c
> +++ linux-2.6.git/arch/powerpc/kernel/smp.c
> @@ -620,3 +620,59 @@ void __cpu_die(unsigned int cpu)
>  		smp_ops->cpu_die(cpu);
>  }
>  #endif

 ^^^ Please add the /* CONFIG_CPU_HOTPLUG */ (or whatever it is) that's
missing after that #endif :-)

Cheers,
Ben.

^ permalink raw reply

* Re: [PATCHv2 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Benjamin Herrenschmidt @ 2010-01-29  1:23 UTC (permalink / raw)
  To: Joel Schopp; +Cc: linux-kernel, Ingo Molnar, linuxppc-dev, Peter Zijlstra, ego
In-Reply-To: <4B621213.2010805@austin.ibm.com>


> I've tested it with manually offlined threads and it behaves as I'd like 
> it to.

Which is ? IE. I'm happy that you like how it behaves, but I'd like to u
understand how that is so I can make sure I'm also happy with it :-)

Cheers,
Ben.

^ permalink raw reply

* Re: [RFC PATCH] PCI-E broken on PPC (regression)
From: Matthew Wilcox @ 2010-01-29  3:45 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Linux PCI, Jay Vosburgh, Jesse Barnes, David Miller, Ron Mercer,
	kaneshige.kenji, linuxppc-dev, Breno Leitao
In-Reply-To: <1264558256.3601.153.camel@pasglop>

On Wed, Jan 27, 2010 at 01:10:56PM +1100, Benjamin Herrenschmidt wrote:
> 
> > Cc'ing Ben for PPC.  Ben, should PPC use pci_scan_device when probing
> > its root busses?  Sounds like it just uses pci_device_add for each one
> > it finds instead?
> > 
> > If you don't actually need scanning (though what about hotplug?) we can
> > move the call to device_add instead...
> 
> Ok so I looked at the code and the problem goes way beyond root busses.
> 
> Basically, powerpc can use the code in arch/powerpc/kernel/pci_of_scan.c
> to "generate" the pci_dev without using config space probing or at least
> using as little of it as possible, using the firmware device-tree
> information instead.
> 
> This is also probably going to be moved to a more generic place and
> extended to be used optionally by other architectures.

Yes, having it under drivers/pci/ somewhere would be a big improvement,
that way we'd actually see it when trying to do cleanups and wouldn't
accidentally break your architectures.

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply

* Re: [RFC PATCH] PCI-E broken on PPC (regression)
From: Benjamin Herrenschmidt @ 2010-01-29  3:54 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Linux PCI, Jay Vosburgh, Jesse Barnes, David Miller, Ron Mercer,
	kaneshige.kenji, linuxppc-dev, Breno Leitao
In-Reply-To: <20100129034524.GA13385@parisc-linux.org>

On Thu, 2010-01-28 at 20:45 -0700, Matthew Wilcox wrote:
> > This is also probably going to be moved to a more generic place and
> > extended to be used optionally by other architectures.
> 
> Yes, having it under drivers/pci/ somewhere would be a big improvement,
> that way we'd actually see it when trying to do cleanups and wouldn't
> accidentally break your architectures. 

Yup, and you'll notice that I didn't complain about the breakage for
that precise reason :-)

Cheers,
Ben.

^ permalink raw reply

* Freescale MPC8313 TSEC SGMII problem.
From: Johnny Hung @ 2010-01-29  6:29 UTC (permalink / raw)
  To: linux-embedded, linuxppc-dev

Hi All,

I have a Freescale MPC8313 customized board and want to confiugre
TSEC1 to SGMII. Would you please help me for this question,

1). Is it possible for MPC8313 TSEC1 connect to Ethernet Switch by
SGMII (without phy), like below diagram.

[MPC8313 TSEC1]  <====SGMII====> [Ethernet Switch]

2). If yes in 1), how to configure it in MPC8313, as I know it can set
TSEC1 to SGMII in LTIB uboot setting. How about in Linux Kernel driver
? Is it need extra setting or patch?

BRs, H. Johnny

^ permalink raw reply

* Re: i2c_powermac: Kernel access of bad area
From: Jean Delvare @ 2010-01-29  8:18 UTC (permalink / raw)
  To: Christian Kujau, Benjamin Herrenschmidt
  Cc: linuxppc-dev, Michael Hanselmann, LKML, Stelian Pop
In-Reply-To: <20100107171738.2c83b13e@hyperion.delvare>

Ben, Christian,

On Thu, 7 Jan 2010 17:17:38 +0100, Jean Delvare wrote:
> On Wed, 6 Jan 2010 19:41:05 -0800 (PST), Christian Kujau wrote:
> > Hi Jean,
> > 
> > On Wed, 6 Jan 2010 at 17:37, Jean Delvare wrote:
> > > I think that sysfs files creation should be moved to the end of
> > > probe_thermostat() and sysfs files removal should be moved to the
> > > beginning of remove_thermostat(). Something like the totally untested
> > > patch below (no ppc machine at hand):
> > 
> > I applied your patch to 2.6.33-rc3, and was able to unload i2c-powermac and 
> > then reading the files left in /sys/devices/temperatures. I even tried to 
> > read the non-existant files (e.g. sensor1_fan_speed, etc...), but the 
> > kernel just wouldn't oops :)
> > 
> > So the initial oops is gone - yeah!

Ben, what about applying this patch of mine, as Christian reported it
fixed his oops?

> > However, the "Badness" remains when I try to modprobe i2c-powermac again:
> > 
> > [  442.148222] PowerMac i2c bus pmu 2 registered
> > [  442.148792] PowerMac i2c bus pmu 1 registered
> > [  442.149299] PowerMac i2c bus mac-io 0 registered
> > [  442.163573] adt746x: ADT7467 initializing
> > [  442.170072] adt746x: Lowering max temperatures from 73, 80, 109 to 67, 47, 67
> > [  442.176559] PowerMac i2c bus uni-n 1 registered
> > [  442.227115] sysfs: cannot create duplicate filename '/devices/ams'
> > [  442.227697] ------------[ cut here ]------------
> > [  442.228176] Badness at fs/sysfs/dir.c:487
> > [  442.228642] NIP: c00eb71c LR: c00eb71c CTR: 00000000
> > [  442.229117] REGS: eea0fa50 TRAP: 0700   Not tainted  (2.6.33-rc3)
> > [  442.229592] MSR: 00029032 <EE,ME,CE,IR,DR>  CR: 42008444  XER: 00000000
> > [  442.230151] TASK = eea10000[2821] 'modprobe' THREAD: eea0e000
> > [  442.230191] GPR00: c00eb71c eea0fb00 eea10000 0000004c 000064c6 ffffffff ffffffff 00000000 
> > [  442.230758] GPR08: efa71740 c03e0000 00000000 000064c6 44008428 10020390 100e0000 100df49c 
> > [  442.231326] GPR16: 100b54c0 100df49c 100ddd20 1018fb08 100b5340 c03e674c c03e6720 c03ea044 
> > [  442.231902] GPR24: 00000000 24008422 ffffffea eea0fb58 ef0e9000 ef0e9000 ef0a9ea0 ffffffef 
> > [  442.233187] NIP [c00eb71c] sysfs_add_one+0x94/0xc0
> > [  442.233695] LR [c00eb71c] sysfs_add_one+0x94/0xc0
> > [  442.234363] Call Trace:
> > 
> > I've put the whole dmesg on:
> > 
> >    http://nerdbynature.de/bits/2.6.33-rc2/i2c_powermac/r1/
> 
> Hmm. Looks like a different but somewhat similar problem in the ams
> driver: some code that is in ams_exit() (the module exit code) should
> instead be called when the device (not module) is removed. It probably
> doesn't make much of a difference in the PMU case, but in the I2C case
> it does matter.
> 
> The following, totally untested patch may fix it. I make no guarantee
> that my code isn't racy though, I'm not familiar enough with the ams
> driver code to tell for sure.
> 
> ---
>  drivers/hwmon/ams/ams-core.c |   11 +++++++----
>  drivers/hwmon/ams/ams-i2c.c  |    2 ++
>  drivers/hwmon/ams/ams-pmu.c  |    2 ++
>  drivers/hwmon/ams/ams.h      |    1 +
>  4 files changed, 12 insertions(+), 4 deletions(-)
> 
> --- linux-2.6.33-rc3.orig/drivers/hwmon/ams/ams-core.c	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.33-rc3/drivers/hwmon/ams/ams-core.c	2010-01-07 17:14:25.000000000 +0100
> @@ -213,7 +213,7 @@ int __init ams_init(void)
>  	return -ENODEV;
>  }
>  
> -void ams_exit(void)
> +void ams_sensor_detach(void)
>  {
>  	/* Remove input device */
>  	ams_input_exit();
> @@ -221,9 +221,6 @@ void ams_exit(void)
>  	/* Remove attributes */
>  	device_remove_file(&ams_info.of_dev->dev, &dev_attr_current);
>  
> -	/* Shut down implementation */
> -	ams_info.exit();
> -
>  	/* Flush interrupt worker
>  	 *
>  	 * We do this after ams_info.exit(), because an interrupt might
> @@ -239,6 +236,12 @@ void ams_exit(void)
>  	pmf_unregister_irq_client(&ams_freefall_client);
>  }
>  
> +static void __exit ams_exit(void)
> +{
> +	/* Shut down implementation */
> +	ams_info.exit();
> +}
> +
>  MODULE_AUTHOR("Stelian Pop, Michael Hanselmann");
>  MODULE_DESCRIPTION("Apple Motion Sensor driver");
>  MODULE_LICENSE("GPL");
> --- linux-2.6.33-rc3.orig/drivers/hwmon/ams/ams-i2c.c	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.33-rc3/drivers/hwmon/ams/ams-i2c.c	2010-01-07 17:12:46.000000000 +0100
> @@ -238,6 +238,8 @@ static int ams_i2c_probe(struct i2c_clie
>  static int ams_i2c_remove(struct i2c_client *client)
>  {
>  	if (ams_info.has_device) {
> +		ams_sensor_detach();
> +
>  		/* Disable interrupts */
>  		ams_i2c_set_irq(AMS_IRQ_ALL, 0);
>  
> --- linux-2.6.33-rc3.orig/drivers/hwmon/ams/ams-pmu.c	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.33-rc3/drivers/hwmon/ams/ams-pmu.c	2010-01-07 17:13:47.000000000 +0100
> @@ -133,6 +133,8 @@ static void ams_pmu_get_xyz(s8 *x, s8 *y
>  
>  static void ams_pmu_exit(void)
>  {
> +	ams_sensor_detach();
> +
>  	/* Disable interrupts */
>  	ams_pmu_set_irq(AMS_IRQ_ALL, 0);
>  
> --- linux-2.6.33-rc3.orig/drivers/hwmon/ams/ams.h	2009-06-10 05:05:27.000000000 +0200
> +++ linux-2.6.33-rc3/drivers/hwmon/ams/ams.h	2010-01-07 17:11:43.000000000 +0100
> @@ -61,6 +61,7 @@ extern struct ams ams_info;
>  
>  extern void ams_sensors(s8 *x, s8 *y, s8 *z);
>  extern int ams_sensor_attach(void);
> +extern void ams_sensor_detach(void);
>  
>  extern int ams_pmu_init(struct device_node *np);
>  extern int ams_i2c_init(struct device_node *np);

Christian, did you ever test this second patch of mine? If you did,
what was the outcome?

-- 
Jean Delvare

^ permalink raw reply

* Re: [PATCHv3 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Peter Zijlstra @ 2010-01-29 10:13 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Ingo Molnar, linuxppc-dev, linux-kernel, ego
In-Reply-To: <1264728185.20211.34.camel@pasglop>

On Fri, 2010-01-29 at 12:23 +1100, Benjamin Herrenschmidt wrote:
> On machine that don't have SMT, I would like to avoid calling
> arch_scale_smt_power() at all if possible (in addition to not compiling
> it in if SMT is not enabled in .config).
> 
> Now, I must say I'm utterly confused by how the domains are setup and I
> haven't quite managed to sort it out... it looks to me that
> SD_SHARE_CPUPOWER is always going to be set on all CPUs when the config
> option is set (though each CPU will have its own domain) or am I
> misguided ? IE. Is there any sense in having at least a fast exit path
> out of arch_scale_smt_power() for non-SMT CPUs ?

The sched_domain creation code is a f'ing stink pile that hurts
everybody's brain.

The AMD magny-cours people sort of cleaned it up a bit but didn't go
nearly far enough. Doing so is somewhere on my todo list, but sadly that
thing is way larger than my spare time.

Now SD_SHARE_CPUPOWER _should_ only be set for SMT domains, because only
SMT siblings share cpupower.

SD_SHARE_PKG_RESOURCES _should_ be set for both SMT and MC, because they
all share the same cache domain.

If it all works out that way in practice on powerpc is another question
entirely ;-)

That said, I'm still not entirely convinced I like this usage of
cpupower, its supposed to be a normalization scale for load-balancing,
not a placement hook.

I'd be much happier with a SD_GROUP_ORDER or something like that, that
works together with SD_PREFER_SIBLING to pack active tasks to cpus in
ascending group order.

^ permalink raw reply

* Proc/CpuInfo - Timebase
From: Ajay Jain @ 2010-01-29 11:23 UTC (permalink / raw)
  To: linuxppc-dev

Hi,

On a powerpc system running linux, when I see cat /proc/cpuinfo, I
find the value of timebase. Can somebody suggest on what is that, and
how is it used?

Regards,
Ajay.

^ permalink raw reply

* RE: Proc/CpuInfo - Timebase
From: Jenkins, Clive @ 2010-01-29 11:35 UTC (permalink / raw)
  To: Ajay Jain, linuxppc-dev
In-Reply-To: <b92b327b1001290323g4e586804ta6aad8fba098dc2a@mail.gmail.com>

> On a powerpc system running linux, when I see cat /proc/cpuinfo, I
> find the value of timebase. Can somebody suggest on what is that, and
> how is it used?

It is the frequency (in Hz) of the clock that increments the Time Base
Register (TBR).

After reading TBR (or calculating a difference between two TBR values)
you can use "timebase" to convert to real time units, such as
microseconds/nanoseconds/picoseconds.

For debugging I log events together with a TBR reading, which I can
later convert to a time from reset (or a time from the moment when the
kernel initialised TBR to zero, which happens in some kernels).

Clive

^ permalink raw reply

* Re: [PATCHv3 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Gabriel Paubert @ 2010-01-29 12:25 UTC (permalink / raw)
  To: Joel Schopp; +Cc: linux-kernel, Ingo Molnar, linuxppc-dev, Peter Zijlstra, ego
In-Reply-To: <1264720855.9660.22.camel@jschopp-laptop>

On Thu, Jan 28, 2010 at 05:20:55PM -0600, Joel Schopp wrote:
> On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
> there is performance benefit to idling the higher numbered threads in
> the core.  
> 

Really 2, 3, or 4? When you have 4 idle threads out of 4, performance
becomes a minor concern, no? ;-)

	Gabriel

^ permalink raw reply

* Re: [PATCH 2/2] eeh: fixing pci_dev dependency
From: Linas Vepstas @ 2010-01-29 15:42 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, leitao
In-Reply-To: <1264723487.20211.17.camel@pasglop>

On 28 January 2010 18:04, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2010-01-27 at 12:43 -0600, leitao@linux.vnet.ibm.com wrote:
>> Currently pci_dev can be null when EEH is in action. This patch
>> just assure that we pci_dev is not NULL before calling pci_dev_put.
>
> Like all variants of *_put(), it already checks for a NULL argument
> afaik. So that patch should be unnecessary.

Ah, OK, I paniced when I saw that and assumed the worst

--linas

^ permalink raw reply

* Re: [PATCH v4 1/3] i2c-mpc: use __devinit[data] for initialization functions and data
From: Wolfram Sang @ 2010-01-29 16:15 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Wolfgang Grandegger, Devicetree-discuss, Linuxppc-dev, Linux-i2c,
	Ben Dooks
In-Reply-To: <1264685141-26391-2-git-send-email-wg@grandegger.com>

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On Thu, Jan 28, 2010 at 02:25:39PM +0100, Wolfgang Grandegger wrote:
> From: Wolfgang Grandegger <wg@denx.de>
> 
> "__devinit[data]" has not yet been used for all initialization functions
> and data. To avoid truncating lines, the struct "mpc_i2c_match_data" has
> been renamed to "mpc_i2c_data", which is even the better name.
> 
> Signed-off-by: Wolfgang Grandegger <wg@denx.de>

Tested-by: Wolfram Sang <w.sang@pengutronix.de>

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCH v4 2/3] i2c-mpc: add support for the MPC512x processors from Freescale
From: Wolfram Sang @ 2010-01-29 16:24 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Wolfgang Grandegger, Devicetree-discuss, Linuxppc-dev, Linux-i2c,
	Ben Dooks
In-Reply-To: <1264685141-26391-3-git-send-email-wg@grandegger.com>

[-- Attachment #1: Type: text/plain, Size: 10730 bytes --]

On Thu, Jan 28, 2010 at 02:25:40PM +0100, Wolfgang Grandegger wrote:
> From: Wolfgang Grandegger <wg@denx.de>
> 
> The "setclock" initialization functions have been renamed to "setup"
> because I2C interrupts must be enabled for the MPC512x. This requires
> to handle "fsl,preserve-clocking" in a slighly different way. Also,
> the old settings are now reported calling dev_dbg(). For the MPC512x
> the clock setup function of the MPC52xx can be re-used. Furthermore,
> the Kconfig help has been updated and corrected.
> 
> Signed-off-by: Wolfgang Grandegger <wg@denx.de>

One minor thing and you can add my

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

> ---
>  drivers/i2c/busses/Kconfig   |    7 +-
>  drivers/i2c/busses/i2c-mpc.c |  127 ++++++++++++++++++++++++++++++------------
>  2 files changed, 94 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 5f318ce..5477e41 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -418,13 +418,12 @@ config I2C_IXP2000
>  	  instead.
>  
>  config I2C_MPC
> -	tristate "MPC107/824x/85xx/52xx/86xx"
> +	tristate "MPC107/824x/85xx/512x/52xx/83xx/86xx"
>  	depends on PPC32
>  	help
>  	  If you say yes to this option, support will be included for the
> -	  built-in I2C interface on the MPC107/Tsi107/MPC8240/MPC8245 and
> -	  MPC85xx/MPC8641 family processors. The driver may also work on 52xx
> -	  family processors, though interrupts are known not to work.
> +	  built-in I2C interface on the MPC107, Tsi107, MPC512x, MPC52xx,
> +	  MPC8240, MPC8245, MPC83xx, MPC85xx and MPC8641 family processors.
>  
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called i2c-mpc.
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index 275ebe6..1af0730 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -31,6 +31,9 @@
>  
>  #define DRV_NAME "mpc-i2c"
>  
> +#define MPC_I2C_CLOCK_SAFE     0

For clarity, I think this define should also be used when checking for the
condition (instead of using !clock).

> +#define MPC_I2C_CLOCK_PRESERVE (~0U)
> +
>  #define MPC_I2C_FDR   0x04
>  #define MPC_I2C_CR    0x08
>  #define MPC_I2C_SR    0x0c
> @@ -67,9 +70,8 @@ struct mpc_i2c_divider {
>  };
>  
>  struct mpc_i2c_data {
> -	void (*setclock)(struct device_node *node,
> -			 struct mpc_i2c *i2c,
> -			 u32 clock, u32 prescaler);
> +	void (*setup)(struct device_node *node, struct mpc_i2c *i2c,
> +		      u32 clock, u32 prescaler);
>  	u32 prescaler;
>  };
>  
> @@ -164,7 +166,7 @@ static int i2c_wait(struct mpc_i2c *i2c, unsigned timeout, int writing)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_PPC_MPC52xx
> +#if defined(CONFIG_PPC_MPC52xx) || defined(CONFIG_PPC_MPC512x)
>  static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_52xx[] = {
>  	{20, 0x20}, {22, 0x21}, {24, 0x22}, {26, 0x23},
>  	{28, 0x24}, {30, 0x01}, {32, 0x25}, {34, 0x02},
> @@ -216,12 +218,18 @@ static int __devinit mpc_i2c_get_fdr_52xx(struct device_node *node, u32 clock,
>  	return div ? (int)div->fdr : -EINVAL;
>  }
>  
> -static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> -					    struct mpc_i2c *i2c,
> -					    u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
> +					 struct mpc_i2c *i2c,
> +					 u32 clock, u32 prescaler)
>  {
>  	int ret, fdr;
>  
> +	if (clock == MPC_I2C_CLOCK_PRESERVE) {
> +		dev_dbg(i2c->dev, "using fdr %d\n",
> +			readb(i2c->base + MPC_I2C_FDR));
> +		return;
> +	}
> +
>  	ret = mpc_i2c_get_fdr_52xx(node, clock, prescaler);
>  	fdr = (ret >= 0) ? ret : 0x3f; /* backward compatibility */
>  
> @@ -230,13 +238,49 @@ static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
>  	if (ret >= 0)
>  		dev_info(i2c->dev, "clock %d Hz (fdr=%d)\n", clock, fdr);
>  }
> -#else /* !CONFIG_PPC_MPC52xx */
> -static void __devinit mpc_i2c_setclock_52xx(struct device_node *node,
> -					    struct mpc_i2c *i2c,
> -					    u32 clock, u32 prescaler)
> +#else /* !(CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x) */
> +static void __devinit mpc_i2c_setup_52xx(struct device_node *node,
> +					 struct mpc_i2c *i2c,
> +					 u32 clock, u32 prescaler)
> +{
> +}
> +#endif /* CONFIG_PPC_MPC52xx || CONFIG_PPC_MPC512x */
> +
> +#ifdef CONFIG_PPC_MPC512x
> +static void __devinit mpc_i2c_setup_512x(struct device_node *node,
> +					 struct mpc_i2c *i2c,
> +					 u32 clock, u32 prescaler)
> +{
> +	struct device_node *node_ctrl;
> +	void __iomem *ctrl;
> +	const u32 *pval;
> +	u32 idx;
> +
> +	/* Enable I2C interrupts for mpc5121 */
> +	node_ctrl = of_find_compatible_node(NULL, NULL,
> +					    "fsl,mpc5121-i2c-ctrl");
> +	if (node_ctrl) {
> +		ctrl = of_iomap(node_ctrl, 0);
> +		if (ctrl) {
> +			/* Interrupt enable bits for i2c-0/1/2: bit 24/26/28 */
> +			pval = of_get_property(node, "reg", NULL);
> +			idx = (*pval & 0xff) / 0x20;
> +			setbits32(ctrl, 1 << (24 + idx * 2));
> +			iounmap(ctrl);
> +		}
> +		of_node_put(node_ctrl);
> +	}
> +
> +	/* The clock setup for the 52xx works also fine for the 512x */
> +	mpc_i2c_setup_52xx(node, i2c, clock, prescaler);
> +}
> +#else /* CONFIG_PPC_MPC512x */
> +static void __devinit mpc_i2c_setup_512x(struct device_node *node,
> +					 struct mpc_i2c *i2c,
> +					 u32 clock, u32 prescaler)
>  {
>  }
> -#endif /* CONFIG_PPC_MPC52xx*/
> +#endif /* CONFIG_PPC_MPC512x */
>  
>  #ifdef CONFIG_FSL_SOC
>  static const struct __devinitdata mpc_i2c_divider mpc_i2c_dividers_8xxx[] = {
> @@ -322,12 +366,19 @@ static int __devinit mpc_i2c_get_fdr_8xxx(struct device_node *node, u32 clock,
>  	return div ? (int)div->fdr : -EINVAL;
>  }
>  
> -static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
> -					    struct mpc_i2c *i2c,
> -					    u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setup_8xxx(struct device_node *node,
> +					 struct mpc_i2c *i2c,
> +					 u32 clock, u32 prescaler)
>  {
>  	int ret, fdr;
>  
> +	if (clock == MPC_I2C_CLOCK_PRESERVE) {
> +		dev_dbg(i2c->dev, "using dfsrr %d, fdr %d\n",
> +			readb(i2c->base + MPC_I2C_DFSRR),
> +			readb(i2c->base + MPC_I2C_FDR));
> +		return;
> +	}
> +
>  	ret = mpc_i2c_get_fdr_8xxx(node, clock, prescaler);
>  	fdr = (ret >= 0) ? ret : 0x1031; /* backward compatibility */
>  
> @@ -340,9 +391,9 @@ static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
>  }
>  
>  #else /* !CONFIG_FSL_SOC */
> -static void __devinit mpc_i2c_setclock_8xxx(struct device_node *node,
> -					    struct mpc_i2c *i2c,
> -					    u32 clock, u32 prescaler)
> +static void __devinit mpc_i2c_setup_8xxx(struct device_node *node,
> +					 struct mpc_i2c *i2c,
> +					 u32 clock, u32 prescaler)
>  {
>  }
>  #endif /* CONFIG_FSL_SOC */
> @@ -496,7 +547,7 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
>  {
>  	struct mpc_i2c *i2c;
>  	const u32 *prop;
> -	u32 clock = 0;
> +	u32 clock = MPC_I2C_CLOCK_SAFE;
>  	int result = 0;
>  	int plen;
>  
> @@ -525,21 +576,21 @@ static int __devinit fsl_i2c_probe(struct of_device *op,
>  		}
>  	}
>  
> -	if (!of_get_property(op->node, "fsl,preserve-clocking", NULL)) {
> +	if (of_get_property(op->node, "fsl,preserve-clocking", NULL)) {
> +		clock = MPC_I2C_CLOCK_PRESERVE;
> +	} else {
>  		prop = of_get_property(op->node, "clock-frequency", &plen);
>  		if (prop && plen == sizeof(u32))
>  			clock = *prop;
> +	}
>  
> -		if (match->data) {
> -			struct mpc_i2c_data *data =
> -				(struct mpc_i2c_data *)match->data;
> -			data->setclock(op->node, i2c, clock, data->prescaler);
> -		} else {
> -			/* Backwards compatibility */
> -			if (of_get_property(op->node, "dfsrr", NULL))
> -				mpc_i2c_setclock_8xxx(op->node, i2c,
> -						      clock, 0);
> -		}
> +	if (match->data) {
> +		struct mpc_i2c_data *data = match->data;
> +		data->setup(op->node, i2c, clock, data->prescaler);
> +	} else {
> +		/* Backwards compatibility */
> +		if (of_get_property(op->node, "dfsrr", NULL))
> +			mpc_i2c_setup_8xxx(op->node, i2c, clock, 0);
>  	}
>  
>  	dev_set_drvdata(&op->dev, i2c);
> @@ -584,21 +635,25 @@ static int __devexit fsl_i2c_remove(struct of_device *op)
>  	return 0;
>  };
>  
> +static struct mpc_i2c_data __devinitdata mpc_i2c_data_512x = {
> +	.setup = mpc_i2c_setup_512x,
> +};
> +
>  static struct mpc_i2c_data __devinitdata mpc_i2c_data_52xx = {
> -	.setclock = mpc_i2c_setclock_52xx,
> +	.setup = mpc_i2c_setup_52xx,
>  };
>  
>  static struct mpc_i2c_data __devinitdata mpc_i2c_data_8313 = {
> -	.setclock = mpc_i2c_setclock_8xxx,
> +	.setup = mpc_i2c_setup_8xxx,
>  };
>  
>  static struct mpc_i2c_data __devinitdata mpc_i2c_data_8543 = {
> -	.setclock = mpc_i2c_setclock_8xxx,
> +	.setup = mpc_i2c_setup_8xxx,
>  	.prescaler = 2,
>  };
>  
>  static struct mpc_i2c_data __devinitdata mpc_i2c_data_8544 = {
> -	.setclock = mpc_i2c_setclock_8xxx,
> +	.setup = mpc_i2c_setup_8xxx,
>  	.prescaler = 3,
>  };
>  
> @@ -606,6 +661,7 @@ static const struct of_device_id mpc_i2c_of_match[] = {
>  	{.compatible = "mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
>  	{.compatible = "fsl,mpc5200b-i2c", .data = &mpc_i2c_data_52xx, },
>  	{.compatible = "fsl,mpc5200-i2c", .data = &mpc_i2c_data_52xx, },
> +	{.compatible = "fsl,mpc5121-i2c", .data = &mpc_i2c_data_512x, },
>  	{.compatible = "fsl,mpc8313-i2c", .data = &mpc_i2c_data_8313, },
>  	{.compatible = "fsl,mpc8543-i2c", .data = &mpc_i2c_data_8543, },
>  	{.compatible = "fsl,mpc8544-i2c", .data = &mpc_i2c_data_8544, },
> @@ -615,7 +671,6 @@ static const struct of_device_id mpc_i2c_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, mpc_i2c_of_match);
>  
> -
>  /* Structure for a device driver */
>  static struct of_platform_driver mpc_i2c_driver = {
>  	.match_table	= mpc_i2c_of_match,
> @@ -648,5 +703,5 @@ module_exit(fsl_i2c_exit);
>  
>  MODULE_AUTHOR("Adrian Cox <adrian@humboldt.co.uk>");
>  MODULE_DESCRIPTION("I2C-Bus adapter for MPC107 bridge and "
> -		   "MPC824x/85xx/52xx processors");
> +		   "MPC824x/85xx/512x/52xx processors");
>  MODULE_LICENSE("GPL");
> -- 
> 1.6.2.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCHv3 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Joel Schopp @ 2010-01-29 16:26 UTC (permalink / raw)
  To: Gabriel Paubert
  Cc: linux-kernel, Ingo Molnar, linuxppc-dev, Peter Zijlstra, ego
In-Reply-To: <20100129122536.GA11156@iram.es>

Gabriel Paubert wrote:
> On Thu, Jan 28, 2010 at 05:20:55PM -0600, Joel Schopp wrote:
>   
>> On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
>> there is performance benefit to idling the higher numbered threads in
>> the core.  
>>
>>     
>
> Really 2, 3, or 4? When you have 4 idle threads out of 4, performance
> becomes a minor concern, no? ;-)
>
> 	Gabriel
>   
Yes, but going from 4 idle to 3 idle you want to keep the slanted 
weights.  If you ignored 4 you'd place wrong and then correct it after 
the fact.

^ permalink raw reply

* Re: [PATCH v4 3/3] powerpc: doc/dts-bindings: update doc of FSL I2C bindings
From: Wolfram Sang @ 2010-01-29 16:26 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Ben Dooks, Devicetree-discuss, Linuxppc-dev, Linux-i2c,
	Wolfgang Grandegger
In-Reply-To: <1264685141-26391-4-git-send-email-wg@grandegger.com>

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Thu, Jan 28, 2010 at 02:25:41PM +0100, Wolfgang Grandegger wrote:
> From: Wolfgang Grandegger <wg@denx.de>
> 
> This patch adds the MPC5121 to the list of supported devices,
> enhances the doc of the "clock-frequency" property and removes
> the obsolete "cell-index" property from the example nodes.
> Furthermore and example for the MPC5121 has been added.
> 
> Signed-off-by: Wolfgang Grandegger <wg@denx.de>

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply

* Re: [PATCHv3 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Joel Schopp @ 2010-01-29 18:34 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Ingo Molnar, linuxppc-dev, linux-kernel, ego
In-Reply-To: <1264760027.4283.2164.camel@laptop>


> That said, I'm still not entirely convinced I like this usage of
> cpupower, its supposed to be a normalization scale for load-balancing,
> not a placement hook.
>   
Even if you do a placement hook you'll need to address it in the load 
balancing as well.  Consider a single 4 thread SMT core with 4 running 
tasks.  If 2 of them exit the remaining 2 will need to be load balanced 
within the core in a way that takes into account the dynamic nature of 
the thread power.  This patch does that.
> I'd be much happier with a SD_GROUP_ORDER or something like that, that
> works together with SD_PREFER_SIBLING to pack active tasks to cpus in
> ascending group order.
>
>   
I don't see this load-balancing patch as mutually exclusive with a patch 
to fix placement.  But even if it is a mutually exclusive solution there 
is no reason we can't fix things now with this patch and then later take 
it out when it's fixed another way.  This patch series is 
straightforward, non-intrusive, and without it the scheduler is broken 
on this processor. 

^ permalink raw reply

* Re: [PATCHv3 2/2] powerpc: implement arch_scale_smt_power for Power7
From: Joel Schopp @ 2010-01-29 18:41 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: linux-kernel, Ingo Molnar, linuxppc-dev, Peter Zijlstra, ego
In-Reply-To: <1264728185.20211.34.camel@pasglop>

Benjamin Herrenschmidt wrote:
> On Thu, 2010-01-28 at 17:24 -0600, Joel Schopp wrote:
>   
>> On Power7 processors running in SMT4 mode with 2, 3, or 4 idle threads 
>> there is performance benefit to idling the higher numbered threads in
>> the core.  
>>
>> This patch implements arch_scale_smt_power to dynamically update smt
>> thread power in these idle cases in order to prefer threads 0,1 over
>> threads 2,3 within a core.
>>     
>
> Almost there :-) Joel, Peter, can you help me figure something out tho ?
>
> On machine that don't have SMT, I would like to avoid calling
> arch_scale_smt_power() at all if possible (in addition to not compiling
> it in if SMT is not enabled in .config).
>
> Now, I must say I'm utterly confused by how the domains are setup and I
> haven't quite managed to sort it out... it looks to me that
> SD_SHARE_CPUPOWER is always going to be set on all CPUs when the config
> option is set (though each CPU will have its own domain) or am I
> misguided ? IE. Is there any sense in having at least a fast exit path
> out of arch_scale_smt_power() for non-SMT CPUs ?
>
> Joel, can you look at compiling it out when SMT is not set ? We don't
> want to bloat SMP kernels for 32-bit non-SMT embedded platforms.
>   
I can wrap the powerpc definition of arch_scale_smt in an #ifdef, if 
it's not there the scheduler uses the default, which is the same as it 
uses if SMT isn't compiled. 

^ permalink raw reply

* Re: i2c_powermac: Kernel access of bad area
From: Benjamin Herrenschmidt @ 2010-01-29 21:34 UTC (permalink / raw)
  To: Jean Delvare
  Cc: Christian Kujau, linuxppc-dev, Michael Hanselmann, LKML,
	Stelian Pop
In-Reply-To: <20100129091857.3b7228db@hyperion.delvare>


> Ben, what about applying this patch of mine, as Christian reported it
> fixed his oops?

Sure. I never quite know with i2c which ones you will apply directly and
which ones you want to go through my tree :-)

Hopefully they should still be referened on patchwork, I'll dig there
and pick them up.

Cheers,
Ben.

> > > However, the "Badness" remains when I try to modprobe i2c-powermac again:
> > > 
> > > [  442.148222] PowerMac i2c bus pmu 2 registered
> > > [  442.148792] PowerMac i2c bus pmu 1 registered
> > > [  442.149299] PowerMac i2c bus mac-io 0 registered
> > > [  442.163573] adt746x: ADT7467 initializing
> > > [  442.170072] adt746x: Lowering max temperatures from 73, 80, 109 to 67, 47, 67
> > > [  442.176559] PowerMac i2c bus uni-n 1 registered
> > > [  442.227115] sysfs: cannot create duplicate filename '/devices/ams'
> > > [  442.227697] ------------[ cut here ]------------
> > > [  442.228176] Badness at fs/sysfs/dir.c:487
> > > [  442.228642] NIP: c00eb71c LR: c00eb71c CTR: 00000000
> > > [  442.229117] REGS: eea0fa50 TRAP: 0700   Not tainted  (2.6.33-rc3)
> > > [  442.229592] MSR: 00029032 <EE,ME,CE,IR,DR>  CR: 42008444  XER: 00000000
> > > [  442.230151] TASK = eea10000[2821] 'modprobe' THREAD: eea0e000
> > > [  442.230191] GPR00: c00eb71c eea0fb00 eea10000 0000004c 000064c6 ffffffff ffffffff 00000000 
> > > [  442.230758] GPR08: efa71740 c03e0000 00000000 000064c6 44008428 10020390 100e0000 100df49c 
> > > [  442.231326] GPR16: 100b54c0 100df49c 100ddd20 1018fb08 100b5340 c03e674c c03e6720 c03ea044 
> > > [  442.231902] GPR24: 00000000 24008422 ffffffea eea0fb58 ef0e9000 ef0e9000 ef0a9ea0 ffffffef 
> > > [  442.233187] NIP [c00eb71c] sysfs_add_one+0x94/0xc0
> > > [  442.233695] LR [c00eb71c] sysfs_add_one+0x94/0xc0
> > > [  442.234363] Call Trace:
> > > 
> > > I've put the whole dmesg on:
> > > 
> > >    http://nerdbynature.de/bits/2.6.33-rc2/i2c_powermac/r1/
> > 
> > Hmm. Looks like a different but somewhat similar problem in the ams
> > driver: some code that is in ams_exit() (the module exit code) should
> > instead be called when the device (not module) is removed. It probably
> > doesn't make much of a difference in the PMU case, but in the I2C case
> > it does matter.
> > 
> > The following, totally untested patch may fix it. I make no guarantee
> > that my code isn't racy though, I'm not familiar enough with the ams
> > driver code to tell for sure.
> > 
> > ---
> >  drivers/hwmon/ams/ams-core.c |   11 +++++++----
> >  drivers/hwmon/ams/ams-i2c.c  |    2 ++
> >  drivers/hwmon/ams/ams-pmu.c  |    2 ++
> >  drivers/hwmon/ams/ams.h      |    1 +
> >  4 files changed, 12 insertions(+), 4 deletions(-)
> > 
> > --- linux-2.6.33-rc3.orig/drivers/hwmon/ams/ams-core.c	2009-06-10 05:05:27.000000000 +0200
> > +++ linux-2.6.33-rc3/drivers/hwmon/ams/ams-core.c	2010-01-07 17:14:25.000000000 +0100
> > @@ -213,7 +213,7 @@ int __init ams_init(void)
> >  	return -ENODEV;
> >  }
> >  
> > -void ams_exit(void)
> > +void ams_sensor_detach(void)
> >  {
> >  	/* Remove input device */
> >  	ams_input_exit();
> > @@ -221,9 +221,6 @@ void ams_exit(void)
> >  	/* Remove attributes */
> >  	device_remove_file(&ams_info.of_dev->dev, &dev_attr_current);
> >  
> > -	/* Shut down implementation */
> > -	ams_info.exit();
> > -
> >  	/* Flush interrupt worker
> >  	 *
> >  	 * We do this after ams_info.exit(), because an interrupt might
> > @@ -239,6 +236,12 @@ void ams_exit(void)
> >  	pmf_unregister_irq_client(&ams_freefall_client);
> >  }
> >  
> > +static void __exit ams_exit(void)
> > +{
> > +	/* Shut down implementation */
> > +	ams_info.exit();
> > +}
> > +
> >  MODULE_AUTHOR("Stelian Pop, Michael Hanselmann");
> >  MODULE_DESCRIPTION("Apple Motion Sensor driver");
> >  MODULE_LICENSE("GPL");
> > --- linux-2.6.33-rc3.orig/drivers/hwmon/ams/ams-i2c.c	2009-06-10 05:05:27.000000000 +0200
> > +++ linux-2.6.33-rc3/drivers/hwmon/ams/ams-i2c.c	2010-01-07 17:12:46.000000000 +0100
> > @@ -238,6 +238,8 @@ static int ams_i2c_probe(struct i2c_clie
> >  static int ams_i2c_remove(struct i2c_client *client)
> >  {
> >  	if (ams_info.has_device) {
> > +		ams_sensor_detach();
> > +
> >  		/* Disable interrupts */
> >  		ams_i2c_set_irq(AMS_IRQ_ALL, 0);
> >  
> > --- linux-2.6.33-rc3.orig/drivers/hwmon/ams/ams-pmu.c	2009-06-10 05:05:27.000000000 +0200
> > +++ linux-2.6.33-rc3/drivers/hwmon/ams/ams-pmu.c	2010-01-07 17:13:47.000000000 +0100
> > @@ -133,6 +133,8 @@ static void ams_pmu_get_xyz(s8 *x, s8 *y
> >  
> >  static void ams_pmu_exit(void)
> >  {
> > +	ams_sensor_detach();
> > +
> >  	/* Disable interrupts */
> >  	ams_pmu_set_irq(AMS_IRQ_ALL, 0);
> >  
> > --- linux-2.6.33-rc3.orig/drivers/hwmon/ams/ams.h	2009-06-10 05:05:27.000000000 +0200
> > +++ linux-2.6.33-rc3/drivers/hwmon/ams/ams.h	2010-01-07 17:11:43.000000000 +0100
> > @@ -61,6 +61,7 @@ extern struct ams ams_info;
> >  
> >  extern void ams_sensors(s8 *x, s8 *y, s8 *z);
> >  extern int ams_sensor_attach(void);
> > +extern void ams_sensor_detach(void);
> >  
> >  extern int ams_pmu_init(struct device_node *np);
> >  extern int ams_i2c_init(struct device_node *np);
> 
> Christian, did you ever test this second patch of mine? If you did,
> what was the outcome?
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox