* Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms
From: Josh Boyer @ 2009-12-10 17:07 UTC (permalink / raw)
To: Dave Kleikamp
Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210155721.6697.40863.sendpatchset@norville.austin.ibm.com>
On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
>diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
>index 9eed29e..1393307 100644
>--- a/arch/powerpc/include/asm/processor.h
>+++ b/arch/powerpc/include/asm/processor.h
>@@ -161,9 +161,35 @@ struct thread_struct {
> #ifdef CONFIG_PPC32
> void *pgdir; /* root of page-table tree */
> #endif
>-#if defined(CONFIG_4xx) || defined (CONFIG_BOOKE)
>- unsigned long dbcr0; /* debug control register values */
>+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
>+ /*
>+ * The following help to manage the use of Debug Control Registers
>+ * om the BookE platforms.
>+ */
>+ unsigned long dbcr0;
> unsigned long dbcr1;
>+ unsigned long dbcr2;
This is wrong for 405. 405 only has dbcr0 and dbcr1. I don't see why you'd
change the #define values to be more explicit and then include things that
don't make sense.
>+ /*
>+ * The stored value of the DBSR register will be the value at the
>+ * last debug interrupt. This register can only be read from the
>+ * user (will never be written to) and has value while helping to
>+ * describe the reason for the last debug trap. Torez
>+ */
>+ unsigned long dbsr;
>+ /*
>+ * The following will contain addresses used by debug applications
>+ * to help trace and trap on particular address locations.
>+ * The bits in the Debug Control Registers above help define which
>+ * of the following registers will contain valid data and/or addresses.
>+ */
>+ unsigned long iac1;
>+ unsigned long iac2;
>+ unsigned long iac3;
>+ unsigned long iac4;
>+ unsigned long dac1;
>+ unsigned long dac2;
>+ unsigned long dvc1;
>+ unsigned long dvc2;
> #endif
Without digging much, I'm wondering if we could just use a pointer to a debug
register structure here instead of growing struct thread more.
> /* FP and VSX 0-31 register set */
> double fpr[32][TS_FPRWIDTH];
>diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
>index 3bf7835..7f8c71f 100644
>--- a/arch/powerpc/include/asm/reg_booke.h
>+++ b/arch/powerpc/include/asm/reg_booke.h
>@@ -248,6 +248,8 @@
> #define DBSR_RET 0x00008000 /* Return Debug Event */
> #define DBSR_CIRPT 0x00000040 /* Critical Interrupt Taken Event */
> #define DBSR_CRET 0x00000020 /* Critical Return Debug Event */
>+#define DBSR_IAC12ATS 0x00000002 /* Instr Address Compare 1/2 Toggle */
>+#define DBSR_IAC34ATS 0x00000001 /* Instr Address Compare 3/4 Toggle */
> #endif
> #ifdef CONFIG_40x
> #define DBSR_IC 0x80000000 /* Instruction Completion */
>@@ -294,25 +296,68 @@
> #define DBCR0_IC 0x08000000 /* Instruction Completion */
> #define DBCR0_ICMP DBCR0_IC
> #define DBCR0_BT 0x04000000 /* Branch Taken */
>-#define DBCR0_BRT DBCR0_BT
> #define DBCR0_EDE 0x02000000 /* Exception Debug Event */
> #define DBCR0_IRPT DBCR0_EDE
> #define DBCR0_TDE 0x01000000 /* TRAP Debug Event */
>-#define DBCR0_IA1 0x00800000 /* Instr Addr compare 1 enable */
>-#define DBCR0_IAC1 DBCR0_IA1
>-#define DBCR0_IA2 0x00400000 /* Instr Addr compare 2 enable */
>-#define DBCR0_IAC2 DBCR0_IA2
>-#define DBCR0_IA12 0x00200000 /* Instr Addr 1-2 range enable */
>-#define DBCR0_IA12X 0x00100000 /* Instr Addr 1-2 range eXclusive */
>-#define DBCR0_IA3 0x00080000 /* Instr Addr compare 3 enable */
>-#define DBCR0_IAC3 DBCR0_IA3
>-#define DBCR0_IA4 0x00040000 /* Instr Addr compare 4 enable */
>-#define DBCR0_IAC4 DBCR0_IA4
>-#define DBCR0_IA34 0x00020000 /* Instr Addr 3-4 range Enable */
>-#define DBCR0_IA34X 0x00010000 /* Instr Addr 3-4 range eXclusive */
>-#define DBCR0_IA12T 0x00008000 /* Instr Addr 1-2 range Toggle */
>-#define DBCR0_IA34T 0x00004000 /* Instr Addr 3-4 range Toggle */
>+#define DBCR0_IAC1 0x00800000 /* Instr Addr compare 1 enable */
>+#define DBCR0_IAC2 0x00400000 /* Instr Addr compare 2 enable */
>+#define DBCR0_IAC12M 0x00300000 /* Instr Addr 1-2 range enable */
>+#define DBCR0_IAC12M_R 0x00100000 /* Instr Addr 1-2 Reserved state */
>+#define DBCR0_IAC12M_I 0x00200000 /* Instr Addr 1-2 range Inclusive */
>+#define DBCR0_IAC12M_X 0x00300000 /* Instr Addr 1-2 range eXclusive */
>+#define DBCR0_IAC3 0x00080000 /* Instr Addr compare 3 enable */
>+#define DBCR0_IAC4 0x00040000 /* Instr Addr compare 4 enable */
>+#define DBCR0_IAC34 0x00020000 /* Instr Addr 3-4 range Enable */
>+#define DBCR0_IAC34X 0x00010000 /* Instr Addr 3-4 range eXclusive */
>+#define DBCR0_IAC12T 0x00008000 /* Instr Addr 1-2 range Toggle */
>+#define DBCR0_IAC34T 0x00004000 /* Instr Addr 3-4 range Toggle */
A lot of this seems to just be cleanup... would it be possible to factor out
the cleanup parts so that the additions are easier to review?
> #define DBCR0_FT 0x00000001 /* Freeze Timers on debug event */
>+
>+#define DBCR0_USER_DEBUG (DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
>+ DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
>+#define DBCR0_BASE_REG_VALUE 0
>+
>+#define dbcr_iac_range(task) ((task)->thread.dbcr0)
>+#define DBCR_IAC12M DBCR0_IAC12M
>+#define DBCR_IAC12M_I DBCR0_IAC12M_I
>+#define DBCR_IAC12M_X DBCR0_IAC12M_X
>+#define DBCR_IAC34M DBCR0_IAC34M
>+#define DBCR_IAC34M_I DBCR0_IAC34M_I
>+#define DBCR_IAC34M_X DBCR0_IAC34M_X
>+
>+/* Bit definitions related to the DBCR1. */
>+#define DBCR1_D1R 0x80000000 /* DAC1 Read Debug Event */
>+#define DBCR1_DAC1R DBCR1_D1R
>+#define DBCR1_D2R 0x40000000 /* DAC2 Read Debug Event */
>+#define DBCR1_DAC2R DBCR1_D2R
>+#define DBCR1_D1W 0x20000000 /* DAC1 Write Debug Event */
>+#define DBCR1_DAC1W DBCR1_D1W
>+#define DBCR1_D2W 0x10000000 /* DAC2 Write Debug Event */
>+#define DBCR1_DAC2W DBCR1_D2W
>+
>+#define DBCR1_USER_DEBUG (DBCR1_DAC1R | DBCR1_DAC2R | DBCR1_DAC1W | \
>+ DBCR1_DAC2W)
>+#define DBCR1_BASE_REG_VALUE 0
>+
>+#define dbcr_dac(task) ((task)->thread.dbcr1)
>+#define DBCR_DAC1R DBCR1_DAC1R
>+#define DBCR_DAC1W DBCR1_DAC1W
>+#define DBCR_DAC2R DBCR1_DAC2R
>+#define DBCR_DAC2W DBCR1_DAC2W
>+
>+#define DBCR2_USER_DEBUG 0
>+#define DBCR2_BASE_REG_VALUE 0
Why are these defined for 405?
>+/*
>+ * Are there any active Debug Events represented in the
>+ * Debug Control Registers?
>+ */
>+#define DBCR0_ACTIVE_EVENTS (DBCR0_ICMP | DBCR0_IAC1 | DBCR0_IAC2 | \
>+ DBCR0_IAC3 | DBCR0_IAC4)
>+#define DBCR1_ACTIVE_EVENTS (DBCR1_D1R | DBCR1_D2R | DBCR1_D1W | DBCR1_D2W)
>+#define DBCR_ACTIVE_EVENTS(dbcr0, dbcr1) (((dbcr0) & DBCR0_ACTIVE_EVENTS) || \
>+ ((dbcr1) & DBCR1_ACTIVE_EVENTS))
>+
> #elif defined(CONFIG_BOOKE)
> #define DBCR0_EDM 0x80000000 /* External Debug Mode */
> #define DBCR0_IDM 0x40000000 /* Internal Debug Mode */
>@@ -324,8 +369,7 @@
> #define DBCR0_RST_NONE 0x00000000 /* No Reset */
> #define DBCR0_ICMP 0x08000000 /* Instruction Completion */
> #define DBCR0_IC DBCR0_ICMP
>-#define DBCR0_BRT 0x04000000 /* Branch Taken */
>-#define DBCR0_BT DBCR0_BRT
>+#define DBCR0_BT 0x04000000 /* Branch Taken */
This seems like just cleanup of DBCR0_BRT?
> #define DBCR0_IRPT 0x02000000 /* Exception Debug Event */
> #define DBCR0_TDE 0x01000000 /* TRAP Debug Event */
> #define DBCR0_TIE DBCR0_TDE
>@@ -342,19 +386,99 @@
> #define DBCR0_CRET 0x00000020 /* Critical Return Debug Event */
> #define DBCR0_FT 0x00000001 /* Freeze Timers on debug event */
>
>+#define DBCR0_USER_DEBUG (DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
>+ DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | \
>+ DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | \
>+ DBCR0_DAC2W)
>+#define DBCR0_BASE_REG_VALUE 0
>+
>+#define dbcr_dac(task) ((task)->thread.dbcr0)
>+#define DBCR_DAC1R DBCR0_DAC1R
>+#define DBCR_DAC1W DBCR0_DAC1W
>+#define DBCR_DAC2R DBCR0_DAC2R
>+#define DBCR_DAC2W DBCR0_DAC2W
>+
> /* Bit definitions related to the DBCR1. */
I'll try to review these a bit later. Changing #defines makes for hard patch
review :)
josh
^ permalink raw reply
* Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
From: Josh Boyer @ 2009-12-10 17:27 UTC (permalink / raw)
To: Dave Kleikamp
Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210155727.6697.74672.sendpatchset@norville.austin.ibm.com>
On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
>powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
>
>From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>
>This patch defines context switch and trap related functionality
>for BookE specific Debug Registers. It adds support to ptrace()
>for setting and getting BookE related Debug Registers
>
>Signed-off-by: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
>Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
>Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
>Cc: David Gibson <dwg@au1.ibm.com>
>Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
>---
>
> arch/powerpc/include/asm/system.h | 2
> arch/powerpc/kernel/process.c | 109 ++++++++-
> arch/powerpc/kernel/ptrace.c | 435 ++++++++++++++++++++++++++++++++++---
> arch/powerpc/kernel/signal.c | 6 -
> arch/powerpc/kernel/signal_32.c | 8 +
> arch/powerpc/kernel/traps.c | 86 ++++++-
> 6 files changed, 564 insertions(+), 82 deletions(-)
>
>
>diff --git a/arch/powerpc/include/asm/system.h b/arch/powerpc/include/asm/system.h
>index bb8e006..474bf23 100644
>--- a/arch/powerpc/include/asm/system.h
>+++ b/arch/powerpc/include/asm/system.h
>@@ -114,6 +114,8 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
> extern int set_dabr(unsigned long dabr);
> extern void do_dabr(struct pt_regs *regs, unsigned long address,
> unsigned long error_code);
>+extern void do_send_trap(struct pt_regs *regs, unsigned long address,
>+ unsigned long error_code, int signal_code, int errno);
> extern void print_backtrace(unsigned long *);
> extern void show_regs(struct pt_regs * regs);
> extern void flush_instruction_cache(void);
>diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>index c930ac3..a0dbb09 100644
>--- a/arch/powerpc/kernel/process.c
>+++ b/arch/powerpc/kernel/process.c
>@@ -245,6 +245,24 @@ void discard_lazy_cpu_state(void)
> }
> #endif /* CONFIG_SMP */
>
>+void do_send_trap(struct pt_regs *regs, unsigned long address,
>+ unsigned long error_code, int signal_code, int errno)
>+{
>+ siginfo_t info;
>+
>+ if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>+ 11, SIGSEGV) == NOTIFY_STOP)
>+ return;
>+
>+ /* Deliver the signal to userspace */
>+ info.si_signo = SIGTRAP;
>+ info.si_errno = errno;
>+ info.si_code = signal_code;
>+ info.si_addr = (void __user *)address;
>+ force_sig_info(SIGTRAP, &info, current);
>+}
>+
>+#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
> void do_dabr(struct pt_regs *regs, unsigned long address,
> unsigned long error_code)
> {
>@@ -257,12 +275,6 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
> if (debugger_dabr_match(regs))
> return;
>
>- /* Clear the DAC and struct entries. One shot trigger */
>-#if defined(CONFIG_BOOKE)
>- mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
>- | DBCR0_IDM));
>-#endif
>-
> /* Clear the DABR */
> set_dabr(0);
>
>@@ -273,9 +285,71 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
> info.si_addr = (void __user *)address;
> force_sig_info(SIGTRAP, &info, current);
> }
>+#endif
>
> static DEFINE_PER_CPU(unsigned long, current_dabr);
>
>+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
>+/*
>+ * Set the debug registers back to their default "safe" values.
>+ */
>+static void set_debug_reg_defaults(struct thread_struct *thread)
>+{
>+ thread->iac1 = thread->iac2 = thread->iac3 = thread->iac4 = 0;
>+ thread->dac1 = thread->dac2 = 0;
>+ thread->dvc1 = thread->dvc2 = 0;
>+ /*
>+ * reset the DBCR0, DBCR1 and DBCR2 registers. All bits with
>+ * the exception of the reserved bits should be cleared out
>+ * and set to 0.
>+ *
>+ * For the DBCR0 register, the reserved bits are bits 17:30.
>+ * Reserved bits for DBCR1 are bits 10:14 and bits 26:30.
>+ * And, bits 10:11 for DBCR2.
>+ */
>+ thread->dbcr0 = DBCR0_BASE_REG_VALUE;
>+ /*
>+ * First clear all "non reserved" bits from DBCR1 then initialize reg
>+ * to force User/Supervisor bits to b11 (user-only MSR[PR]=1) and
>+ * Effective/Real * bits to b10 (trap only if IS==0)
>+ */
>+ thread->dbcr1 = DBCR1_BASE_REG_VALUE;
>+ /*
>+ * Force Data Address Compare User/Supervisor bits to be User-only
>+ * (0b11 MSR[PR]=1) and set all other bits in DBCR2 register to be 0.
>+ * This sets the Data Address Compare Effective/Real bits to be 0b00
>+ * (Effective, MSR[DS]=don't care).
>+ */
>+ thread->dbcr2 = DBCR2_BASE_REG_VALUE;
>+}
>+
>+static void prime_debug_regs(struct thread_struct *thread)
>+{
>+ mtspr(SPRN_IAC1, thread->iac1);
>+ mtspr(SPRN_IAC2, thread->iac2);
>+ mtspr(SPRN_IAC3, thread->iac3);
>+ mtspr(SPRN_IAC4, thread->iac4);
>+ mtspr(SPRN_DAC1, thread->dac1);
>+ mtspr(SPRN_DAC2, thread->dac2);
>+ mtspr(SPRN_DVC1, thread->dvc1);
>+ mtspr(SPRN_DVC2, thread->dvc2);
>+ mtspr(SPRN_DBCR0, thread->dbcr0);
>+ mtspr(SPRN_DBCR1, thread->dbcr1);
>+ mtspr(SPRN_DBCR2, thread->dbcr2);
405 has no DBCR2, so I doubt setting it via mtspr would be good. Does this
compile for 40x and did you test it at all?
Also, looking at the current kernel it seems SPRN_DBCR2 is only defined for
CONFIG_PPC_BOOK3E_64 which is also inaccurate. I don't see that fixed in any
of the other patches. Are you missing a patch, or was this generated against
and old kernel or something?
>@@ -514,7 +586,7 @@ void show_regs(struct pt_regs * regs)
> printk(" CR: %08lx XER: %08lx\n", regs->ccr, regs->xer);
> trap = TRAP(regs);
> if (trap == 0x300 || trap == 0x600)
>-#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
> #else
> printk("DAR: "REG", DSISR: "REG"\n", regs->dar, regs->dsisr);
This hunk seems unnecessary.
I need to find some time to sit down and look over the rest still. I do have
some questions on how/if this will interact correctly with an older GDB still,
and some of the things I've pointed out make me wonder how much this has all
been tested and on what platforms.
josh
^ permalink raw reply
* Re: Ping does not work on my MPC8544 board, using linux2.6.23
From: Scott Wood @ 2009-12-10 17:39 UTC (permalink / raw)
To: 林志平; +Cc: linuxppc-dev
In-Reply-To: <8148211e0912100505r46a813dcgc907a79e153ca8e@mail.gmail.com>
林志平 wrote:
> Yes mcp85xx_defconfig does not work, I can see mpc8540ads in the
> menuconfig, but no mpc8544ds.
>
>
> mpc8544ds is supported by CONFIG_MPC85xx_DS.
> CONFIG_MPC85XX_DS does not exist. Only mpc85xx_defconfig in
> arch/powerpc/configs
I'm looking right at it, in arch/powerpc/platforms/85xx/Kconfig in 2.6.31.
mpc85xx_defconfig selects this option.
-Scott
^ permalink raw reply
* [PATCH 0/5] More unreachable() conversions.
From: David Daney @ 2009-12-10 17:25 UTC (permalink / raw)
To: Andrew Morton, David Howells, Koichi Yasutake, linux-am33-list,
Kyle McMartin, Helge Deller, linux-parisc, Benjamin Herrenschmidt,
Paul Mackerras, linuxppc-dev, Richard Henderson, Ivan Kokshaysky,
Matt Turner, linux-alpha, Mike Frysinger, uclinux-dist-devel
Cc: linux-arch, Linus Torvalds, Linux Kernel Mailing List
Back in commit 38938c879eb0c39edf85d5164aa0cffe2874304c the new macro
unreachable() was added. We can use it in BUG instead of a variety of
ad hoc endless loops.
This patch set consists of the patches from the original set that were
never Acked-by anyone. Although they are completely untested, they
seem plausible.
I don't plan on pushing these things out any more, so if you
like them please merge them via your architecture trees.
I will reply with the 5 patches.
David Daney (5):
mn10300: Convert BUG() to use unreachable()
parisc: Convert BUG() to use unreachable()
powerpc: Convert BUG() to use unreachable()
alpha: Convert BUG() to use unreachable()
blackfin: Convert BUG() to use unreachable()
arch/alpha/include/asm/bug.h | 3 ++-
arch/blackfin/include/asm/bug.h | 2 +-
arch/mn10300/include/asm/bug.h | 3 ++-
arch/parisc/include/asm/bug.h | 4 ++--
arch/powerpc/include/asm/bug.h | 2 +-
5 files changed, 8 insertions(+), 6 deletions(-)
^ permalink raw reply
* [PATCH 3/5] powerpc: Convert BUG() to use unreachable()
From: David Daney @ 2009-12-10 17:28 UTC (permalink / raw)
To: linux-kernel, linux-arch
Cc: David Daney, linuxppc-dev, Paul Mackerras, akpm, torvalds
In-Reply-To: <4B212EEC.3060808@caviumnetworks.com>
Use the new unreachable() macro instead of for(;;);
Signed-off-by: David Daney <ddaney@caviumnetworks.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: linuxppc-dev@ozlabs.org
---
arch/powerpc/include/asm/bug.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 64e1fdc..2c15212 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -68,7 +68,7 @@
_EMIT_BUG_ENTRY \
: : "i" (__FILE__), "i" (__LINE__), \
"i" (0), "i" (sizeof(struct bug_entry))); \
- for(;;) ; \
+ unreachable(); \
} while (0)
#define BUG_ON(x) do { \
--
1.6.2.5
^ permalink raw reply related
* Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
From: Josh Boyer @ 2009-12-10 17:41 UTC (permalink / raw)
To: Dave Kleikamp
Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210172711.GV2937@zod.rchland.ibm.com>
On Thu, Dec 10, 2009 at 12:27:11PM -0500, Josh Boyer wrote:
>On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
>>+static void prime_debug_regs(struct thread_struct *thread)
>>+{
>>+ mtspr(SPRN_IAC1, thread->iac1);
>>+ mtspr(SPRN_IAC2, thread->iac2);
>>+ mtspr(SPRN_IAC3, thread->iac3);
>>+ mtspr(SPRN_IAC4, thread->iac4);
>>+ mtspr(SPRN_DAC1, thread->dac1);
>>+ mtspr(SPRN_DAC2, thread->dac2);
>>+ mtspr(SPRN_DVC1, thread->dvc1);
>>+ mtspr(SPRN_DVC2, thread->dvc2);
>>+ mtspr(SPRN_DBCR0, thread->dbcr0);
>>+ mtspr(SPRN_DBCR1, thread->dbcr1);
>>+ mtspr(SPRN_DBCR2, thread->dbcr2);
>
>405 has no DBCR2, so I doubt setting it via mtspr would be good. Does this
>compile for 40x and did you test it at all?
>
>Also, looking at the current kernel it seems SPRN_DBCR2 is only defined for
>CONFIG_PPC_BOOK3E_64 which is also inaccurate.
I mis-read the code on this part. SPRN_DBCR2 gets defined unconditionally,
so Book-E should be fine. Still seems broken on 405 though.
josh
^ permalink raw reply
* Re: [PATCH v3] cpm2_pic: Allow correct flow_types for port C interrupts
From: Scott Wood @ 2009-12-10 17:41 UTC (permalink / raw)
To: avorontsov; +Cc: Linuxppc-dev Development, Mark Ware
In-Reply-To: <20091210145808.GA16421@oksana.dev.rtsoft.ru>
Anton Vorontsov wrote:
> On Thu, Dec 10, 2009 at 08:52:01AM -0600, Kumar Gala wrote:
>> On Dec 10, 2009, at 5:14 AM, Mark Ware wrote:
>>
>>> Port C interrupts can be either falling edge, or either edge.
>>> Other external interrupts are either falling edge or active low.
>>> Tested on a custom 8280 based board.
>>>
>>> Signed-off-by: Mark Ware <mware@elphinstone.net>
>>> ---
>>> Changed in v3:
>>> - Cosmetic improvements as suggested by Anton and Scott
>>> - Added tested note to changelog
>>>
>>> arch/powerpc/sysdev/cpm2_pic.c | 28 +++++++++++++++++++++-------
>>> 1 files changed, 21 insertions(+), 7 deletions(-)
>> Scott, Anton do you want to add an Ack to this version?
>
> Acked-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Scott Wood <scottwood@freescale.com>
-Scott
^ permalink raw reply
* [PATCH v2 0/3] powerpc/83xx: Sleep and deep sleep support for MPC8315E-RDB
From: Anton Vorontsov @ 2009-12-10 17:59 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev
Hi all,
This is quite late resend, sorry.
Only the third patch has changed, i.e. I got rid of sleep-nexus
stuff per Scott and Benjamin suggestions.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply
* [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume
From: Anton Vorontsov @ 2009-12-10 18:00 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210175957.GA30682@oksana.dev.rtsoft.ru>
Currently 83xx PMC driver clears deep_sleeping variable very early,
before devices are resumed. This makes fsl_deep_sleep() unusable in
drivers' resume() callback.
Sure, drivers can store fsl_deep_sleep() value on suspend and use
the stored value on resume. But a better solution is to postpone
clearing the deep_sleeping variable, i.e. move it into finish()
callback.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Acked-by: Scott Wood <scottwood@freescale.com>
---
arch/powerpc/platforms/83xx/suspend.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index d306f07..b0c2619 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -194,7 +194,7 @@ out:
return ret;
}
-static void mpc83xx_suspend_finish(void)
+static void mpc83xx_suspend_end(void)
{
deep_sleeping = 0;
}
@@ -278,7 +278,7 @@ static struct platform_suspend_ops mpc83xx_suspend_ops = {
.valid = mpc83xx_suspend_valid,
.begin = mpc83xx_suspend_begin,
.enter = mpc83xx_suspend_enter,
- .finish = mpc83xx_suspend_finish,
+ .end = mpc83xx_suspend_end,
};
static int pmc_probe(struct of_device *ofdev,
--
1.6.3.3
^ permalink raw reply related
* [PATCH 2/3] powerpc/83xx/suspend: Save and restore SICRL, SICRH and SCCR
From: Anton Vorontsov @ 2009-12-10 18:00 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210175957.GA30682@oksana.dev.rtsoft.ru>
We need to save SICRL, SICRH and SCCR registers on suspend, and restore
them on resume. Otherwise, we lose IO and clocks setup on MPC8315E-RDB
boards when ULPI USB PHY is used (non-POR setup).
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/platforms/83xx/suspend.c | 48 +++++++++++++++++++++++++++++++++
1 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c
index b0c2619..4380534 100644
--- a/arch/powerpc/platforms/83xx/suspend.c
+++ b/arch/powerpc/platforms/83xx/suspend.c
@@ -32,6 +32,7 @@
#define PMCCR1_NEXT_STATE 0x0C /* Next state for power management */
#define PMCCR1_NEXT_STATE_SHIFT 2
#define PMCCR1_CURR_STATE 0x03 /* Current state for power management*/
+#define IMMR_SYSCR_OFFSET 0x100
#define IMMR_RCW_OFFSET 0x900
#define RCW_PCI_HOST 0x80000000
@@ -78,6 +79,22 @@ struct mpc83xx_clock {
u32 sccr;
};
+struct mpc83xx_syscr {
+ __be32 sgprl;
+ __be32 sgprh;
+ __be32 spridr;
+ __be32 :32;
+ __be32 spcr;
+ __be32 sicrl;
+ __be32 sicrh;
+};
+
+struct mpc83xx_saved {
+ u32 sicrl;
+ u32 sicrh;
+ u32 sccr;
+};
+
struct pmc_type {
int has_deep_sleep;
};
@@ -87,6 +104,8 @@ static int has_deep_sleep, deep_sleeping;
static int pmc_irq;
static struct mpc83xx_pmc __iomem *pmc_regs;
static struct mpc83xx_clock __iomem *clock_regs;
+static struct mpc83xx_syscr __iomem *syscr_regs;
+static struct mpc83xx_saved saved_regs;
static int is_pci_agent, wake_from_pci;
static phys_addr_t immrbase;
static int pci_pm_state;
@@ -137,6 +156,20 @@ static irqreturn_t pmc_irq_handler(int irq, void *dev_id)
return ret;
}
+static void mpc83xx_suspend_restore_regs(void)
+{
+ out_be32(&syscr_regs->sicrl, saved_regs.sicrl);
+ out_be32(&syscr_regs->sicrh, saved_regs.sicrh);
+ out_be32(&clock_regs->sccr, saved_regs.sccr);
+}
+
+static void mpc83xx_suspend_save_regs(void)
+{
+ saved_regs.sicrl = in_be32(&syscr_regs->sicrl);
+ saved_regs.sicrh = in_be32(&syscr_regs->sicrh);
+ saved_regs.sccr = in_be32(&clock_regs->sccr);
+}
+
static int mpc83xx_suspend_enter(suspend_state_t state)
{
int ret = -EAGAIN;
@@ -166,6 +199,8 @@ static int mpc83xx_suspend_enter(suspend_state_t state)
*/
if (deep_sleeping) {
+ mpc83xx_suspend_save_regs();
+
out_be32(&pmc_regs->mask, PMCER_ALL);
out_be32(&pmc_regs->config1,
@@ -179,6 +214,8 @@ static int mpc83xx_suspend_enter(suspend_state_t state)
in_be32(&pmc_regs->config1) & ~PMCCR1_POWER_OFF);
out_be32(&pmc_regs->mask, PMCER_PMCI);
+
+ mpc83xx_suspend_restore_regs();
} else {
out_be32(&pmc_regs->mask, PMCER_PMCI);
@@ -333,12 +370,23 @@ static int pmc_probe(struct of_device *ofdev,
goto out_pmc;
}
+ if (has_deep_sleep) {
+ syscr_regs = ioremap(immrbase + IMMR_SYSCR_OFFSET,
+ sizeof(*syscr_regs));
+ if (!syscr_regs) {
+ ret = -ENOMEM;
+ goto out_syscr;
+ }
+ }
+
if (is_pci_agent)
mpc83xx_set_agent();
suspend_set_ops(&mpc83xx_suspend_ops);
return 0;
+out_syscr:
+ iounmap(clock_regs);
out_pmc:
iounmap(pmc_regs);
out:
--
1.6.3.3
^ permalink raw reply related
* [PATCH 3/3] powerpc/83xx: Add power management support for MPC8315E-RDB boards
From: Anton Vorontsov @ 2009-12-10 18:01 UTC (permalink / raw)
To: Kumar Gala; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210175957.GA30682@oksana.dev.rtsoft.ru>
- Add nodes for PMC and GTM controllers. GTM4 can be used as a wakeup
source;
- Add fsl,magic-packet properties to eTSEC nodes, i.e. wake-on-lan
support. Unlike MPC8313 processors, MPC8315 can resume from deep
sleep upon magic packet reception.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
arch/powerpc/boot/dts/mpc8315erdb.dts | 27 +++++++++++++++++++++++++++
1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/boot/dts/mpc8315erdb.dts b/arch/powerpc/boot/dts/mpc8315erdb.dts
index 32e10f5..8a3a4f3 100644
--- a/arch/powerpc/boot/dts/mpc8315erdb.dts
+++ b/arch/powerpc/boot/dts/mpc8315erdb.dts
@@ -204,6 +204,7 @@
interrupt-parent = <&ipic>;
tbi-handle = <&tbi0>;
phy-handle = < &phy0 >;
+ fsl,magic-packet;
mdio@520 {
#address-cells = <1>;
@@ -246,6 +247,7 @@
interrupt-parent = <&ipic>;
tbi-handle = <&tbi1>;
phy-handle = < &phy1 >;
+ fsl,magic-packet;
mdio@520 {
#address-cells = <1>;
@@ -309,6 +311,22 @@
interrupt-parent = <&ipic>;
};
+ gtm1: timer@500 {
+ compatible = "fsl,mpc8315-gtm", "fsl,gtm";
+ reg = <0x500 0x100>;
+ interrupts = <90 8 78 8 84 8 72 8>;
+ interrupt-parent = <&ipic>;
+ clock-frequency = <133333333>;
+ };
+
+ timer@600 {
+ compatible = "fsl,mpc8315-gtm", "fsl,gtm";
+ reg = <0x600 0x100>;
+ interrupts = <91 8 79 8 85 8 73 8>;
+ interrupt-parent = <&ipic>;
+ clock-frequency = <133333333>;
+ };
+
/* IPIC
* interrupts cell = <intr #, sense>
* sense values match linux IORESOURCE_IRQ_* defines:
@@ -337,6 +355,15 @@
0x59 0x8>;
interrupt-parent = < &ipic >;
};
+
+ pmc: power@b00 {
+ compatible = "fsl,mpc8315-pmc", "fsl,mpc8313-pmc",
+ "fsl,mpc8349-pmc";
+ reg = <0xb00 0x100 0xa00 0x100>;
+ interrupts = <80 8>;
+ interrupt-parent = <&ipic>;
+ fsl,mpc8313-wakeup-timer = <>m1>;
+ };
};
pci0: pci@e0008500 {
--
1.6.3.3
^ permalink raw reply related
* Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
From: Dave Kleikamp @ 2009-12-10 18:05 UTC (permalink / raw)
To: Josh Boyer
Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210174135.GW2937@zod.rchland.ibm.com>
On Thu, 2009-12-10 at 12:41 -0500, Josh Boyer wrote:
> On Thu, Dec 10, 2009 at 12:27:11PM -0500, Josh Boyer wrote:
> >On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
> >>+static void prime_debug_regs(struct thread_struct *thread)
> >>+{
> >>+ mtspr(SPRN_IAC1, thread->iac1);
> >>+ mtspr(SPRN_IAC2, thread->iac2);
> >>+ mtspr(SPRN_IAC3, thread->iac3);
> >>+ mtspr(SPRN_IAC4, thread->iac4);
> >>+ mtspr(SPRN_DAC1, thread->dac1);
> >>+ mtspr(SPRN_DAC2, thread->dac2);
> >>+ mtspr(SPRN_DVC1, thread->dvc1);
> >>+ mtspr(SPRN_DVC2, thread->dvc2);
> >>+ mtspr(SPRN_DBCR0, thread->dbcr0);
> >>+ mtspr(SPRN_DBCR1, thread->dbcr1);
> >>+ mtspr(SPRN_DBCR2, thread->dbcr2);
> >
> >405 has no DBCR2, so I doubt setting it via mtspr would be good. Does this
> >compile for 40x and did you test it at all?
> >
> >Also, looking at the current kernel it seems SPRN_DBCR2 is only defined for
> >CONFIG_PPC_BOOK3E_64 which is also inaccurate.
>
> I mis-read the code on this part. SPRN_DBCR2 gets defined unconditionally,
> so Book-E should be fine. Still seems broken on 405 though.
Agreed. I'll rushed through the 405 stuff a bit. I'll scrutinize and
test it before I post again.
Thanks,
Shaggy
--
David Kleikamp
IBM Linux Technology Center
^ permalink raw reply
* Re: [PATCH v2 0/3] powerpc/83xx: Sleep and deep sleep support for MPC8315E-RDB
From: Scott Wood @ 2009-12-10 18:07 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev
In-Reply-To: <20091210175957.GA30682@oksana.dev.rtsoft.ru>
Anton Vorontsov wrote:
> Hi all,
>
> This is quite late resend, sorry.
>
> Only the third patch has changed, i.e. I got rid of sleep-nexus
> stuff per Scott and Benjamin suggestions.
Acked-by: Scott Wood <scottwood@freescale.com>
-Scott
^ permalink raw reply
* Floating point in the kernel
From: Sean MacLennan @ 2009-12-10 18:13 UTC (permalink / raw)
To: linuxppc-dev
One of our drivers has code that was originally running on a DSP. The
code makes heavy use of floating point. We have isolated all the
floating point to one kthread in the driver. Using enable_kernel_fp()
this has worked well.
But under a specific heavy RTP load, we started getting kernel panics.
To make a long story short, the scheduler disables FP when you are
context switched out. When you come back and access a FP instruction,
you trap and call load_up_fpu() and everything is fine..... unless you
are in the kernel. If you are in the kernel, like our kthread is, you
get a "kernel FP unavailable exception".
Basically we got away with it for two years because the thread is at
high priority (-20) and tries very hard to finish within 1ms. But the
RTP high load causes us to context switch out and crash. The following
patch fixes this:
diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index 50504ae..3476de9 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -383,7 +383,7 @@ label:
#define FP_UNAVAILABLE_EXCEPTION \
START_EXCEPTION(FloatingPointUnavailable) \
NORMAL_EXCEPTION_PROLOG; \
- beq 1f; \
+ /* SAM beq 1f; */ \
bl load_up_fpu; /* if from user, just load it up */ \
b fast_exception_return; \
1: addi r3,r1,STACK_FRAME_OVERHEAD; \
With the patch we run fine, at the expense that we lose the ability to
catch real FP unavailable exceptions in the kernel. It is because of
this loss that I have not submitted this patch.
We also hit another problem under high RTP load... and this is the
patch that fixes it:
diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
index fc8f5b1..051a02c 100644
--- a/arch/powerpc/kernel/fpu.S
+++ b/arch/powerpc/kernel/fpu.S
@@ -83,6 +83,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
stfd fr0,THREAD_FPSCR(r4)
PPC_LL r5,PT_REGS(r4)
toreal(r5)
+
+ /* Under heavy RTP load the hsp thread can have a NULL pt_regs. */
+ PPC_LCMPI 0,r5,0
+ beq 1f
+
PPC_LL r4,_MSR-STACK_FRAME_OVERHEAD(r5)
li r10,MSR_FP|MSR_FE0|MSR_FE1
andc r4,r4,r10 /* disable FP for previous task */
So, if you are still reading this far, I am just looking for any
suggestions. Are there better ways of handling this? Have I
missed something? Anybody know why pt_regs might be NULL?
Cheers,
Sean
^ permalink raw reply related
* Re: [PATCH v3] cpm2_pic: Allow correct flow_types for port C interrupts
From: Kumar Gala @ 2009-12-10 19:48 UTC (permalink / raw)
To: Mark Ware; +Cc: Scott Wood, Linuxppc-dev Development, avorontsov
In-Reply-To: <4B20D81A.7010503@elphinstone.net>
On Dec 10, 2009, at 5:14 AM, Mark Ware wrote:
> Port C interrupts can be either falling edge, or either edge.
> Other external interrupts are either falling edge or active low.
> Tested on a custom 8280 based board.
>
> Signed-off-by: Mark Ware <mware@elphinstone.net>
> ---
> Changed in v3:
> - Cosmetic improvements as suggested by Anton and Scott
> - Added tested note to changelog
>
> arch/powerpc/sysdev/cpm2_pic.c | 28 +++++++++++++++++++++-------
> 1 files changed, 21 insertions(+), 7 deletions(-)
Applied to next
- k
^ permalink raw reply
* Re: Floating point in the kernel
From: Benjamin Herrenschmidt @ 2009-12-10 20:19 UTC (permalink / raw)
To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20091210131311.78cab78c@lappy.seanm.ca>
On Thu, 2009-12-10 at 13:13 -0500, Sean MacLennan wrote:
> One of our drivers has code that was originally running on a DSP. The
> code makes heavy use of floating point. We have isolated all the
> floating point to one kthread in the driver. Using enable_kernel_fp()
> this has worked well.
>
> But under a specific heavy RTP load, we started getting kernel panics.
> To make a long story short, the scheduler disables FP when you are
> context switched out. When you come back and access a FP instruction,
> you trap and call load_up_fpu() and everything is fine..... unless you
> are in the kernel. If you are in the kernel, like our kthread is, you
> get a "kernel FP unavailable exception".
Right, you must not use floating point in the kernel -and- expect it to
survive schedule. You should use preempt_disable() and ensure you don't
schedule() around a block using the FP.
Note that you may also lose the FP register content if you schedule.
> Basically we got away with it for two years because the thread is at
> high priority (-20) and tries very hard to finish within 1ms. But the
> RTP high load causes us to context switch out and crash. The following
> patch fixes this:
>
> diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
> index 50504ae..3476de9 100644
> --- a/arch/powerpc/kernel/head_booke.h
> +++ b/arch/powerpc/kernel/head_booke.h
> @@ -383,7 +383,7 @@ label:
> #define FP_UNAVAILABLE_EXCEPTION \
> START_EXCEPTION(FloatingPointUnavailable) \
> NORMAL_EXCEPTION_PROLOG; \
> - beq 1f; \
> + /* SAM beq 1f; */ \
> bl load_up_fpu; /* if from user, just load it up */ \
> b fast_exception_return; \
> 1: addi r3,r1,STACK_FRAME_OVERHEAD; \
>
> With the patch we run fine, at the expense that we lose the ability to
> catch real FP unavailable exceptions in the kernel. It is because of
> this loss that I have not submitted this patch.
I'm not sure that will work in all cases, you are playing a bit with
fire :-) I suppose I could think it through after breakfast but my first
thought is "don't do that !". Among other things you may not have a
pt_regs to save the registers to.
> We also hit another problem under high RTP load... and this is the
> patch that fixes it:
>
> diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
> index fc8f5b1..051a02c 100644
> --- a/arch/powerpc/kernel/fpu.S
> +++ b/arch/powerpc/kernel/fpu.S
> @@ -83,6 +83,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
> stfd fr0,THREAD_FPSCR(r4)
> PPC_LL r5,PT_REGS(r4)
> toreal(r5)
> +
> + /* Under heavy RTP load the hsp thread can have a NULL pt_regs. */
> + PPC_LCMPI 0,r5,0
> + beq 1f
> +
Right and that means you just lost the content of your FP registers.
> PPC_LL r4,_MSR-STACK_FRAME_OVERHEAD(r5)
> li r10,MSR_FP|MSR_FE0|MSR_FE1
> andc r4,r4,r10 /* disable FP for previous task */
>
> So, if you are still reading this far, I am just looking for any
> suggestions. Are there better ways of handling this? Have I
> missed something? Anybody know why pt_regs might be NULL?
Just don't schedule when you enable_kernel_fp() or move your workload to
userspace :-)
Cheers,
Ben.
> Cheers,
> Sean
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: Floating point in the kernel
From: arnd @ 2009-12-10 20:32 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Sean MacLennan
In-Reply-To: <20091210131311.78cab78c@lappy.seanm.ca>
On Thursday 10 December 2009 18:13:11 Sean MacLennan wrote:
> One of our drivers has code that was originally running on a DSP. The
> code makes heavy use of floating point. We have isolated all the
> floating point to one kthread in the driver. Using enable_kernel_fp()
> this has worked well.
>
> But under a specific heavy RTP load, we started getting kernel panics.
> To make a long story short, the scheduler disables FP when you are
> context switched out. When you come back and access a FP instruction,
> you trap and call load_up_fpu() and everything is fine..... unless you
> are in the kernel. If you are in the kernel, like our kthread is, you
> get a "kernel FP unavailable exception".
I think the rule here is that you have to disable preemption and must not
call any potentially blocking functions like kmalloc when enable_kernel_fp
is set. The kernel has good control over whether a thread get context switched
or not, so it should be able to prevent these problems.
Arnd <><
^ permalink raw reply
* Re: Floating point in the kernel
From: Sean MacLennan @ 2009-12-10 20:33 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev
In-Reply-To: <1260476379.16132.224.camel@pasglop>
On Fri, 11 Dec 2009 07:19:39 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> I'm not sure that will work in all cases, you are playing a bit with
> fire :-) I suppose I could think it through after breakfast but my
> first thought is "don't do that !". Among other things you may not
> have a pt_regs to save the registers to.
Actually, we usually do have pt_regs, or we are stealing some ;)
> > We also hit another problem under high RTP load... and this is the
> > patch that fixes it:
> >
> > diff --git a/arch/powerpc/kernel/fpu.S b/arch/powerpc/kernel/fpu.S
> > index fc8f5b1..051a02c 100644
> > --- a/arch/powerpc/kernel/fpu.S
> > +++ b/arch/powerpc/kernel/fpu.S
> > @@ -83,6 +83,11 @@ END_FTR_SECTION_IFSET(CPU_FTR_VSX)
> > stfd fr0,THREAD_FPSCR(r4)
> > PPC_LL r5,PT_REGS(r4)
> > toreal(r5)
> > +
> > + /* Under heavy RTP load the hsp thread can have a NULL
> > pt_regs. */
> > + PPC_LCMPI 0,r5,0
> > + beq 1f
> > +
>
> Right and that means you just lost the content of your FP registers.
This only happens once in a blue moon, even under heavy RTP load. But I
agree, this could be a real problem.
> > PPC_LL r4,_MSR-STACK_FRAME_OVERHEAD(r5)
> > li r10,MSR_FP|MSR_FE0|MSR_FE1
> > andc r4,r4,r10 /* disable FP for previous
> > task */
> >
> > So, if you are still reading this far, I am just looking for any
> > suggestions. Are there better ways of handling this? Have I
> > missed something? Anybody know why pt_regs might be NULL?
>
> Just don't schedule when you enable_kernel_fp() or move your workload
> to userspace :-)
To be honest, I can't find *why* we are scheduling :( They only way we
give up the CPU is with locking... and none of the locks where hit
during the problem. We also never get near our timeslice... the longest
run I saw when the problem happened was 670us.
Is there a way to disable scheduling? We currently do not have preempt
enabled... but may in the future.
Cheers,
Sean
^ permalink raw reply
* Re: Deprecating of_platform, the path from here...
From: Grant Likely @ 2009-12-10 20:47 UTC (permalink / raw)
To: David Miller; +Cc: devicetree-discuss, linuxppc-dev, paulus, jk
In-Reply-To: <20091209.162121.256573183.davem@davemloft.net>
Hi David,
On Wed, Dec 9, 2009 at 5:21 PM, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 09 Dec 2009 16:15:50 -0800 (PST)
>
>> From: Grant Likely <grant.likely@secretlab.ca>
>> Date: Wed, 9 Dec 2009 15:06:29 -0700
>>
>>> 1) of_platform will be deprecated in preference of the platform bus.
>>
>> What a shame, it's one of the cleanest driver probing models
>> in the tree.
>
> And BTW, have you folks who "decided" this considered at all the fact
> that it is much easier to describe represent platform devices using
> of OF devices rather than the other way around?
Yup. I also think of_platform is a cleaner implementation than
platform, but the fact remains that there are far more platform
drivers than there are of_platform. So, as nice as of_platform is, it
still does pretty much exactly the same job as platform. I'd rather
see of_platform features migrated to platform than creating drivers
with dual registrations to be used on both OF and non-OF platforms.
Trying to go the other way around (deprecate platform and encouraging
of_platform instead) I don't think will gain much traction; whereas I
think bringing of_platform features into platform will be an easier
sell. I'm trying to be pragmatic here.
> The platform device pdata mechanism requires data structure changes
> and is not dynamically extensible, whereas OF devices are
> fundamentally so.
>
> I don't like the idea to get rid of of_platform devices at all.
There are no plans to actually remove of_platform. I certainly don't
plan to try and force SPARC to switch from of_platform to platform
bus. But on PowerPC (and probably Microblaze) the plan is to move
away from of_platform for all the reasons discussed, and I'm not be
bringing of_platform over as I work on ARM support.
> OF devices are really clean, much like netlink messages, where
> arbitrary named attributes can be added or removed without any data
> structure changes at all.
No argument here.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: Floating point in the kernel
From: Benjamin Herrenschmidt @ 2009-12-10 20:56 UTC (permalink / raw)
To: Sean MacLennan; +Cc: linuxppc-dev
In-Reply-To: <20091210153359.3a480488@lappy.seanm.ca>
On Thu, 2009-12-10 at 15:33 -0500, Sean MacLennan wrote:
> To be honest, I can't find *why* we are scheduling :( They only way we
> give up the CPU is with locking... and none of the locks where hit
> during the problem. We also never get near our timeslice... the
> longest
> run I saw when the problem happened was 670us.
>
> Is there a way to disable scheduling? We currently do not have preempt
> enabled... but may in the future.
Well, that would be interesting to see where you schedule indeed.
Outside of preempt and an explicit lock or memory allocation I don't
see ... Or get/put_user ?
Cheers,
Ben.
^ permalink raw reply
* Re: Deprecating of_platform, the path from here...
From: Benjamin Herrenschmidt @ 2009-12-10 21:30 UTC (permalink / raw)
To: David Miller; +Cc: linuxppc-dev, devicetree-discuss, paulus, jk
In-Reply-To: <1260409535.16132.109.camel@pasglop>
On Thu, 2009-12-10 at 12:45 +1100, Benjamin Herrenschmidt wrote:
> I don't agree with grant idea however that just converting the content
> of the device node into properties is the way to go.
And here of course I meant " converting the content of the device node
into into pdata" ...
> I do prefer your proposed approach (from our IRC discussion) which is
> instead to allocate a struct device-node, convert pdata into properties,
> and modify the drier to use these properties.
>
> The main difference thus between the two type of conversions (convert to
> of_platform vs convert to platform) is that in the first case, you have
> to convert the driver to use properties -and- convert all platforms in
> all archs including gory ARM cell phone stuff you really don't want to
> go anywhere near. In the second case, you still convert the driver to
> use properties natively, but you keep a "wart" to turn pdata into a
> device-node -inside the driver-, protected by a CONFIG option maybe, so
> that those archs can be left alone until it becomes so obvious to
> everybody what approach is better that they'll end up being converted
> too and the wart can go.
>
> I believe the second approach, while less "clean" in the absolute is a
> more realistic path to take.
>
> Now, orthogonally to that, I do believe it's still nice to provide a way
> to statically lay out a device node in platform code, to allow archs
> that don't otherwise have the device-tree to replace pdata with
> something nicer and get rid of the wart quicker.
>
> We could either find a way with macros to layout an actual struct
> device_node statically along with all the properties etc... but that
> sounds a tad hard.
>
> We could have something that convert an entirely ASCII representation
> into a struct device_node, but that would be akin of having dtc in the
> kernel, might be a bit bloated no ? Though it could be made simpler and
> more restrictive.
>
> Or we could find an in-between .. .A different struct type that is more
> suitable for being laid out statically (a name, a type, and an enum of
> structs for various property "types", ie, strings, words, bytes, ...)
> with a little helper function that conver that into a device node at
> runtime ?
>
> What do you think ?
>
> Cheers,
> Ben.
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: Floating point in the kernel
From: Arnd Bergmann @ 2009-12-10 21:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linuxppc-dev, Sean MacLennan
In-Reply-To: <20091210153359.3a480488@lappy.seanm.ca>
On Thursday 10 December 2009, Sean MacLennan wrote:
> To be honest, I can't find why we are scheduling :( They only way we
> give up the CPU is with locking... and none of the locks where hit
> during the problem. We also never get near our timeslice... the longest
> run I saw when the problem happened was 670us.
>
> Is there a way to disable scheduling? We currently do not have preempt
> enabled... but may in the future.
If you do preempt_disable(), it should give you a nice oops with a backtrace
at the point where an actual schedule happens.
The sequence should be something like
preempt_disable();
enable_kernel_fp();
/* use FP here */
preempt_enable();
/* may schedule again */
Arnd <><
^ permalink raw reply
* Re: Deprecating of_platform, the path from here...
From: Grant Likely @ 2009-12-10 21:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: devicetree-discuss, linuxppc-dev, paulus, jk, David Miller
In-Reply-To: <1260409535.16132.109.camel@pasglop>
Hey guys, some more thoughts below...
On Wed, Dec 9, 2009 at 6:45 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> First the probing because that's the real important issue, I believe the
> other one is mostly academic and can be dealt on a per driver basis
> (I'll discuss it later too).
>
> I'm not totally sold on the idea of the table that Grant proposed. I
> think I proposed it initially in a discussion we had on IRC as one
> possible option and in fact it was suggested by Paulus so don't
> completely blame Grant for it though. It does mean that either platform
> code or core code will have some kind of big table that associates a
> whole pile of of_device_id with a platform device name. It somewhat
> sucks when I think more about it and it does feel like a step backward
> from of_platform_device.
For the record, I don't like it either. But it is the pragmatic thing
to do because it is simple to implement, and can easily be replaced
later with a better scheme with little impact on drivers. Basically,
it's a migration plan.
> If we could make platform_driver grow an of_device_id match list it
> would be indeed nicer. Of course you'll reply to that "let's just use
> of_platform" instead :-)
>
> I would then argue that I don't like having two different bus types,
> platform and of_platform for "generic platform" devices that aren't
> typed on a bus type. There's a lot of existing "platform device" and it
> would be quite hard to convert them all. Especially since a lot of them
> are used today on archs that don't have device trees and may never grow
> one.
>
> IE. I believe it's going to be an easier path to grow platform_device
> into something that has the of_device probing functionality rather than
> turn everything into of_platform. (I'm not talking here about the pdata
> and retrieval of informations from the tree, this is the second part of
> the discussion, discussed below).
>
> The main problem with moving existing platform_device to of_platform is
> how do we deal with archs that don't have the device-tree infrastructure
> and use those drivers today ?
>
> We could I suppose create a helper that looks a lot like the current
> platform device creation one, which would create an of_platform device
> instead, along with a struct device_node attached (which isn't part of a
> tree) to it, and create a single "compatible" property whose content is
> the platform data name.
>
> But that means that for every driver we want to be able to use a
> device-tree probing for, we would have to convert all archs & platforms
> that may instanciate it to use the new helpers, in addition to replacing
> whatever pdata they have statically stored into a device node (see the
> second part of the discussion).
>
> It's possible I suppose. I just feel that it's going to be a tougher
> sell to the rest of the world.
>
> There's one nit to be careful of. Some drivers (sadly) have the fact
> that they appear under /sys/bus/platform/ as a userspace ABI thingy. Sad
> but a fact. This is one of the reasons why rather than actually
> converting to of_platform I'd rather find a way to add the of_device_id
> match mechanism to the existing platform_device and deprecate the pdata
> over time.
>
> It will also provide with an easier transition. Basically transform
> platform_device into of_platform_device by first adding the missing bits
> and -then- trimming the crufty remains.
I 100% agree with you here on the matching problem.
> Now let's move to the second part of the discussion which is the
> retrieval of various configuration informations by the driver.
>
> Here too, our model is better, I think there's little argument there. A
> named list of properties is just so much more flexible than a statically
> data structure that has to be in sync between the driver and all
> platforms using it leaves little room for improvement or adding platform
> specific attributes which some drivers might need, etc...
>
> Grant proposal is to have drivers create the pdata from the device-tree.
> This is something I believe we both disagree with, though you more
> vehemently than me I suppose :-)
See my pdata comment lower down.
> There are various things at play here:
>
> First, let me make it clear that imho, the device type
> (of_platform_device vs. platform_device) is irrelevant to that aspect of
> the problem since nowadays we have the ability to carry a device node
> pointer in any descendant of struct device (and we use that heavily for
> devices using specific bus types already).
And even if we kept of_platform; the same problem of parsing device
tree data exists for all other bus types. Places where of_platform
isn't used today (i2c, spi, pci, mdio, etc). It makes sense to
establish a pattern for matching and probing with device tree data
that is usable by all bus types.
> If you take an existing platform driver that you want to use on a
> device-tree enabled platform (other than just creating it and pdata from
> arch code which we all agree sucks), the two choices have different
> consequences:
>
> In one case, converting to of_platform_device, you pretty much _have_ to
> get rid of pdata, and convert the driver into using of_get_property()
> instead. This is probably not a bad thing in the long run, except that
> this means you also -have- to convert all platforms in all archs that
> use that specific platform driver to also generate a device node
> (possibly statically in many cases).
>
> This can be a lot of code churn deep into platform code for things like
> ARM which can be pretty nasty, for which none of us have any way to test
> on the relevant hardware. Other arch people will (maybe rightfully so)
> protest especially if they have no intent to use the device-tree stuff
> in their architecture or not yet anyways. And that for each platform
> driver involved.
>
> On the other case, converting to platform_device, adding the device
> node, we have the ability to do an easier transition and easier to sell.
> yes, we do take the risk of getting in that limbo land where drivers end
> up forever in the "transition" state though. That's a con of this
> approach, I do admit.
>
> I don't agree with grant idea however that just converting the content
> of the device node into properties is the way to go.
s/properties/pdata
I'm being pragmatic here. pdata sucks, but the lowest impact path to
getting device tree data into a lot of existing drivers is to populate
the data source it is already using. However, at least by putting the
translator into the driver itself, the pdata isn't an anonymous
pointer anymore and the compiler can correctly type check it.
But this is a minor point. It really is contained within the driver
with no external impact. pdata can be eliminated one driver at a
time.
> I do prefer your proposed approach (from our IRC discussion) which is
> instead to allocate a struct device-node, convert pdata into properties,
> and modify the drier to use these properties.
I like this approach. It would actually be quite easy to convert on a
driver-by-driver basis from .pdata to device nodes. Even platforms
that statically declare their platform devices could be changed by
statically declaring the sub tree for the device. For (a completely
bogus) example:
static struct platform_device omap3beagle_nand_device = {
.name = "omap2-nand",
.id = -1,
- .dev = {
- .platform_data = &omap3beagle_nand_data,
- },
- .num_resources = 1,
- .resource = &omap3beagle_nand_resource,
+ .device_node = (struct device_node *) {
+ .name = "nand",
+ .properties = (struct property *) {
+ .name = "blah",
+ .length = sizeof("foo"),
+ .value = "foo"
+ .next = (struct property *)
+ {
+ .name = "bar",
+ .length = sizeof("wazzit"),
+ .value = "wazzit"
+ } }
+ },
};
Naturally this example is brutally ugly, and it could be done in a far
cleaner manner; but you get the idea. As you and I just discussed on
IRC, the device_node structure isn't really designed for static
declarations, but it could work. Basically it would allow the
migration to a better model that ugly, non-type-checked anonymous
pdata pointers.
> The main difference thus between the two type of conversions (convert to
> of_platform vs convert to platform) is that in the first case, you have
> to convert the driver to use properties -and- convert all platforms in
> all archs including gory ARM cell phone stuff you really don't want to
> go anywhere near. In the second case, you still convert the driver to
> use properties natively, but you keep a "wart" to turn pdata into a
> device-node -inside the driver-, protected by a CONFIG option maybe, so
> that those archs can be left alone until it becomes so obvious to
> everybody what approach is better that they'll end up being converted
> too and the wart can go.
>
> I believe the second approach, while less "clean" in the absolute is a
> more realistic path to take.
>
> Now, orthogonally to that, I do believe it's still nice to provide a way
> to statically lay out a device node in platform code, to allow archs
> that don't otherwise have the device-tree to replace pdata with
> something nicer and get rid of the wart quicker.
>
> We could either find a way with macros to layout an actual struct
> device_node statically along with all the properties etc... but that
> sounds a tad hard.
Heh. I hacked out my example above before reading this far.
> We could have something that convert an entirely ASCII representation
> into a struct device_node, but that would be akin of having dtc in the
> kernel, might be a bit bloated no ? Though it could be made simpler and
> more restrictive.
Ewh. Ugly.
> Or we could find an in-between .. .A different struct type that is more
> suitable for being laid out statically (a name, a type, and an enum of
> structs for various property "types", ie, strings, words, bytes, ...)
> with a little helper function that conver that into a device node at
> runtime ?
I think this bears some investigation. A slightly different layout
(ie, using arrays instead of linked list for properties) would be
easier to prepare. Worth some investigation.
> What do you think ?
I liked what I read.
Cheers,
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: Deprecating of_platform, the path from here...
From: David Miller @ 2009-12-10 21:56 UTC (permalink / raw)
To: grant.likely; +Cc: devicetree-discuss, linuxppc-dev, paulus, jk
In-Reply-To: <fa686aa40912101247k55d56783ge0163dd1da073b49@mail.gmail.com>
From: Grant Likely <grant.likely@secretlab.ca>
Date: Thu, 10 Dec 2009 13:47:33 -0700
> Trying to go the other way around (deprecate platform and encouraging
> of_platform instead) I don't think will gain much traction; whereas I
> think bringing of_platform features into platform will be an easier
> sell. I'm trying to be pragmatic here.
When people use words like "traction" and "pragmatic" that means they
are making decisions for reasons other than technical ones. :-)
^ permalink raw reply
* Re: Deprecating of_platform, the path from here...
From: Grant Likely @ 2009-12-10 22:03 UTC (permalink / raw)
To: David Miller; +Cc: devicetree-discuss, linuxppc-dev, paulus, jk
In-Reply-To: <20091210.135609.124000757.davem@davemloft.net>
On Thu, Dec 10, 2009 at 2:56 PM, David Miller <davem@davemloft.net> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Thu, 10 Dec 2009 13:47:33 -0700
>
>> Trying to go the other way around (deprecate platform and encouraging
>> of_platform instead) I don't think will gain much traction; whereas I
>> think bringing of_platform features into platform will be an easier
>> sell. =A0I'm trying to be pragmatic here.
>
> When people use words like "traction" and "pragmatic" that means they
> are making decisions for reasons other than technical ones. :-)
100% true. :-)
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox