* Re: [PATCH v3 2/3] hvc_init(): Enforce one-time initialization.
From: Rusty Russell @ 2011-11-11 4:30 UTC (permalink / raw)
To: Miche Baker-Harvey, Greg Kroah-Hartman
Cc: Stephen Rothwell, xen-devel, Konrad Rzeszutek Wilk, linux-kernel,
virtualization, Anton Blanchard, Amit Shah, Mike Waychison,
ppc-dev, Eric Northrup
In-Reply-To: <20111108214504.28884.61814.stgit@miche.sea.corp.google.com>
On Tue, 08 Nov 2011 13:45:04 -0800, Miche Baker-Harvey <miche@google.com> wrote:
> hvc_init() must only be called once, and no thread should continue with hvc_alloc()
> until after initialization is complete. The original code does not enforce either
> of these requirements. A new mutex limits entry to hvc_init() to a single thread,
> and blocks all later comers until it has completed.
>
> This patch fixes multiple crash symptoms.
No, it doesn't: not now your previous double-hvc_init patch has been
reverted.
It's easy to show this is the case though: replace mutex_lock() with:
if (!mutex_trylock()) {
WARN(1, "hvc_console: mutex is stopping us!\n");
mutex_lock();
}
If you get that warning, then your mutex is doing something.
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCH v3 1/3] virtio_console: Fix locking of vtermno.
From: Rusty Russell @ 2011-11-11 4:27 UTC (permalink / raw)
To: Miche Baker-Harvey, Greg Kroah-Hartman
Cc: Stephen Rothwell, xen-devel, Konrad Rzeszutek Wilk, linux-kernel,
virtualization, Anton Blanchard, Amit Shah, Mike Waychison,
ppc-dev, Eric Northrup
In-Reply-To: <20111108214458.28884.86759.stgit@miche.sea.corp.google.com>
On Tue, 08 Nov 2011 13:44:58 -0800, Miche Baker-Harvey <miche@google.com> wrote:
> Some modifications of vtermno were not done under the spinlock.
>
> Moved assignment from vtermno and increment of vtermno together,
> putting both under the spinlock. Revert vtermno on failure.
>
> Signed-off-by: Miche Baker-Harvey <miche@google.com>
Does it matter? It's normal not to lock in a function called
"init_XXX", since it's not exposed yet.
Or is it?
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCH 2/7] powerpc/85xx: add HOTPLUG_CPU support
From: Benjamin Herrenschmidt @ 2011-11-11 4:22 UTC (permalink / raw)
To: Zhao Chenhui; +Cc: linuxppc-dev
In-Reply-To: <1320409889-14408-1-git-send-email-chenhui.zhao@freescale.com>
On Fri, 2011-11-04 at 20:31 +0800, Zhao Chenhui wrote:
> #ifdef CONFIG_SMP
> /* When we get here, r24 needs to hold the CPU # */
> .globl __secondary_start
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 7bf2187..12a54f0 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -381,8 +381,14 @@ void generic_cpu_die(unsigned int cpu)
>
> for (i = 0; i < 100; i++) {
> smp_rmb();
> - if (per_cpu(cpu_state, cpu) == CPU_DEAD)
> + if (per_cpu(cpu_state, cpu) == CPU_DEAD) {
> + /*
> + * After another core sets cpu_state to CPU_DEAD,
> + * it needs some time to die.
> + */
> + msleep(10);
> return;
> + }
> msleep(100);
> }
> printk(KERN_ERR "CPU%d didn't die...\n", cpu);
I don't really see why you need to wait. This loop is about waiting for
the CPU to mark itself DEAD, not necessarily to be physically powered
off.
> +struct epapr_entry {
> + u32 addr_h;
> + u32 addr_l;
> + u32 r3_h;
> + u32 r3_l;
> + u32 reserved;
> + u32 pir;
> + u32 r6_h;
> + u32 r6_l;
> +};
This should probably be in a generic location.
> +static int is_corenet;
This global is a bit gross...
...
> +
> +static void __cpuinit smp_85xx_reset_core(int nr)
> +{
> + __iomem u32 *vaddr, *pir_vaddr;
> + u32 val, cpu_mask;
> +
> + /* If CoreNet platform, use BRR as release register. */
> + if (is_corenet) {
> + cpu_mask = 1 << nr;
> + vaddr = ioremap(get_immrbase() + MPC85xx_BRR_OFF, 4);
> + } else {
> + cpu_mask = 1 << (24 + nr);
> + vaddr = ioremap(get_immrbase() + MPC85xx_ECM_EEBPCR_OFF, 4);
> + }
Instead, can't you instead have two functions using a common helper and
pick/hook the right one ?
Also in what context is that reset_core() called ? Doing ioremap isn't
something you can do at any random time...
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] powerpc: Export PIR data through sysfs
From: Benjamin Herrenschmidt @ 2011-11-11 4:18 UTC (permalink / raw)
To: ananth; +Cc: Scott Wood, linuxppc-dev, Anton Blanchard, mahesh
In-Reply-To: <20111110084807.GA16323@in.ibm.com>
On Thu, 2011-11-10 at 14:18 +0530, Ananth N Mavinakayanahalli wrote:
>
> From: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>
> The Processor Identification Register (PIR) on some powerpc platforms
> provides information to decode the processor identification tag.
> Decoding this information is platform specific.
>
> We currently need this information for POWERx processors and hence
> follows a similar model as adopted for the other POWERx specific
> features.
At this rate we're going to end up with no bits left for CPU features
way too quickly... Especially for something we only care about once at
boot time.
Wouldn't CPU_FTR_PPCAS_ARCH_V2 be a good enough test ?
Can you tell us a bit more about the real use for that feature ? I still
don't see what's the point of getting the underlying HW ID.
Cheers,
Ben.
> Signed-off-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> ---
> arch/powerpc/include/asm/cputable.h | 9 +++++----
> arch/powerpc/kernel/sysfs.c | 8 ++++++++
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> Index: linux-3.2-rc1/arch/powerpc/include/asm/cputable.h
> ===================================================================
> --- linux-3.2-rc1.orig/arch/powerpc/include/asm/cputable.h
> +++ linux-3.2-rc1/arch/powerpc/include/asm/cputable.h
> @@ -201,6 +201,7 @@ extern const char *powerpc_base_platform
> #define CPU_FTR_POPCNTB LONG_ASM_CONST(0x0400000000000000)
> #define CPU_FTR_POPCNTD LONG_ASM_CONST(0x0800000000000000)
> #define CPU_FTR_ICSWX LONG_ASM_CONST(0x1000000000000000)
> +#define CPU_FTR_PIR LONG_ASM_CONST(0x2000000000000000)
>
> #ifndef __ASSEMBLY__
>
> @@ -400,7 +401,7 @@ extern const char *powerpc_base_platform
> #define CPU_FTRS_POWER4 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> CPU_FTR_MMCRA | CPU_FTR_CP_USE_DCBTZ | \
> - CPU_FTR_STCX_CHECKS_ADDRESS)
> + CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_PIR)
> #define CPU_FTRS_PPC970 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_201 | \
> CPU_FTR_ALTIVEC_COMP | CPU_FTR_CAN_NAP | CPU_FTR_MMCRA | \
> @@ -408,19 +409,19 @@ extern const char *powerpc_base_platform
> CPU_FTR_HVMODE)
> #define CPU_FTRS_POWER5 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> - CPU_FTR_MMCRA | CPU_FTR_SMT | \
> + CPU_FTR_MMCRA | CPU_FTR_SMT | CPU_FTR_PIR | \
> CPU_FTR_COHERENT_ICACHE | CPU_FTR_PURR | \
> CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB)
> #define CPU_FTRS_POWER6 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> - CPU_FTR_MMCRA | CPU_FTR_SMT | \
> + CPU_FTR_MMCRA | CPU_FTR_SMT | CPU_FTR_PIR | \
> CPU_FTR_COHERENT_ICACHE | \
> CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
> CPU_FTR_DSCR | CPU_FTR_UNALIGNED_LD_STD | \
> CPU_FTR_STCX_CHECKS_ADDRESS | CPU_FTR_POPCNTB | CPU_FTR_CFAR)
> #define CPU_FTRS_POWER7 (CPU_FTR_USE_TB | CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | CPU_FTR_ARCH_206 |\
> - CPU_FTR_MMCRA | CPU_FTR_SMT | \
> + CPU_FTR_MMCRA | CPU_FTR_SMT | CPU_FTR_PIR | \
> CPU_FTR_COHERENT_ICACHE | \
> CPU_FTR_PURR | CPU_FTR_SPURR | CPU_FTR_REAL_LE | \
> CPU_FTR_DSCR | CPU_FTR_SAO | CPU_FTR_ASYM_SMT | \
> Index: linux-3.2-rc1/arch/powerpc/kernel/sysfs.c
> ===================================================================
> --- linux-3.2-rc1.orig/arch/powerpc/kernel/sysfs.c
> +++ linux-3.2-rc1/arch/powerpc/kernel/sysfs.c
> @@ -177,11 +177,13 @@ SYSFS_PMCSETUP(mmcra, SPRN_MMCRA);
> SYSFS_PMCSETUP(purr, SPRN_PURR);
> SYSFS_PMCSETUP(spurr, SPRN_SPURR);
> SYSFS_PMCSETUP(dscr, SPRN_DSCR);
> +SYSFS_PMCSETUP(pir, SPRN_PIR);
>
> static SYSDEV_ATTR(mmcra, 0600, show_mmcra, store_mmcra);
> static SYSDEV_ATTR(spurr, 0600, show_spurr, NULL);
> static SYSDEV_ATTR(dscr, 0600, show_dscr, store_dscr);
> static SYSDEV_ATTR(purr, 0600, show_purr, store_purr);
> +static SYSDEV_ATTR(pir, 0400, show_pir, NULL);
>
> unsigned long dscr_default = 0;
> EXPORT_SYMBOL(dscr_default);
> @@ -392,6 +394,9 @@ static void __cpuinit register_cpu_onlin
>
> if (cpu_has_feature(CPU_FTR_DSCR))
> sysdev_create_file(s, &attr_dscr);
> +
> + if (cpu_has_feature(CPU_FTR_PIR))
> + sysdev_create_file(s, &attr_pir);
> #endif /* CONFIG_PPC64 */
>
> cacheinfo_cpu_online(cpu);
> @@ -462,6 +467,9 @@ static void unregister_cpu_online(unsign
>
> if (cpu_has_feature(CPU_FTR_DSCR))
> sysdev_remove_file(s, &attr_dscr);
> +
> + if (cpu_has_feature(CPU_FTR_PIR))
> + sysdev_remove_file(s, &attr_pir);
> #endif /* CONFIG_PPC64 */
>
> cacheinfo_cpu_offline(cpu);
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH v2 1/5] [ppc] Process dynamic relocations for kernel
From: Benjamin Herrenschmidt @ 2011-11-11 4:11 UTC (permalink / raw)
To: Suzuki Poulose
Cc: Nathan Miller, Josh Poimboeuf, Josh Poimboeuf, Dave Hansen,
Paul Mackerras, Scott Wood, Alan Modra, linuxppc-dev
In-Reply-To: <4EBB3794.9050309@in.ibm.com>
On Thu, 2011-11-10 at 08:01 +0530, Suzuki Poulose wrote:
> Oops ! You are right. We could go back to the clean_dcache_all() or the
> initial approach that you suggested. (dcbst).
>
> I am not sure how do we flush the entire dcache(only). Could you post a
> patch which does the same ?
>
> Another option is to, change the current mapping to 'Write Through' before
> calling the relocate() and revert back to the original setting after relocate().
Why not just keep the dcbst's & icbi's in relocate for now ? (original
patch) Is it noticably slower to boot ? If not I'd say keep it that way,
it will work on all implementations.
Cheers,
Ben.
> >
> >>
> >>
> >>> For i-cache invalidation there's already the (incorrectly named?)
> >>> flush_instruction_cache(). It uses the appropriate platform-specific
> >>> methods (e.g. iccci for 44x) to invalidate the entire i-cache.
> >>
> >> Agreed. The only thing that worries me is the use of KERNELBASE in the
> >> flush_instruction_cache() for CONFIG_4xx. Can we safely assume all 4xx
> >> implementations ignore the arguments passed to iccci ?
> >
> > Good question. I don't know the answer. :-)
> >
> > That also may suggest a bigger can of worms. A grep of the powerpc code
> > shows many uses of KERNELBASE. For a relocatable kernel, nobody should
> > be relying on KERNELBASE except for the early relocation code. Are we
> > sure that all the other usages of KERNELBASE are "safe"?
> >
> I think we could simply replace the occurrences of KERNELBASE (after the relocate())
> with '_stext' which would give the virtual start address of the kernel.
>
> Thanks
> Suzuki
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* [PATCH] powerpc/trace: add a dummy stack frame for trace_hardirqs_off
From: Kevin Hao @ 2011-11-11 2:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev list
The trace_hardirqs_off will use CALLER_ADDR0 and CALLER_ADDR1.
If an exception occurs in user mode, there is only one stack frame
on the stack and accessing the CALLER_ADDR1 will causes the following
call trace. So we create a dummy stack frame to make
trace_hardirqs_off happy.
WARNING: at kernel/smp.c:459
Modules linked in:
NIP: c0093280 LR: c00930a0 CTR: c0010780
REGS: edb87ae0 TRAP: 0700 Not tainted (3.1.0)
MSR: 00021002 <ME,CE> CR: 28002888 XER: 00000000
TASK = edce2ac0[17658] 'mthread-lock-on' THREAD: edb86000 CPU: 5
GPR00: 00000001 edb87b90 edce2ac0 00000005 c0019594 edb87bd8 00000001 00000fe3
GPR08: 00041000 c084138c 4e20120d edb87b90 48002888 1001aa7c 00000000 00000000
GPR16: 48830000 10012a8c 00000000 10000af4 00000001 c0810000 00000000 00000000
GPR24: ee9aa920 c0816a18 00000000 00000005 c0019594 edb87bd8 ee20178c edb87b90
NIP [c0093280] smp_call_function_many+0x214/0x2b4
LR [c00930a0] smp_call_function_many+0x34/0x2b4
Call Trace:
[edb87b90] [c00930a0] smp_call_function_many+0x34/0x2b4 (unreliable)
[edb87bd0] [c00194ec] __flush_tlb_page+0xac/0x100
[edb87c00] [c001957c] flush_tlb_page+0x3c/0x54
[edb87c10] [c00180ac] ptep_set_access_flags+0x74/0x12c
[edb87c40] [c0128068] handle_pte_fault+0x2f0/0x9ac
[edb87cb0] [c0128c3c] handle_mm_fault+0x104/0x1dc
[edb87ce0] [c05f40f4] do_page_fault+0x2dc/0x630
[edb87e50] [c001078c] handle_page_fault+0xc/0x80
--- Exception: 301 at trace_hardirqs_off+0x68/0x150
LR = reenable_mmu+0x24/0x78
[edb87f10] [4902f4f8] 0x4902f4f8 (unreliable)
[edb87f40] [c00101d8] reenable_mmu+0x24/0x78
Instruction dump:
3d20c082 8009916c 2f800000 40befe7c 3d20c081 880968c4 2f800001 41befe6c
3d20c084 3929138c 88090004 68000001 <0f000000> 2f800000 419efe50 38000001
---[ end trace f26d2d7d0a9c89a2 ]---
Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
---
arch/powerpc/kernel/entry_32.S | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 56212bc..4f80cf1 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -215,7 +215,22 @@ reenable_mmu: /* re-enable mmu so we can */
stw r9,8(r1)
stw r11,12(r1)
stw r3,ORIG_GPR3(r1)
+ /*
+ * The trace_hardirqs_off will use CALLER_ADDR0 and CALLER_ADDR1.
+ * If from user mode there is only one stack frame on the stack, and
+ * accessing CALLER_ADDR1 will cause oops. So we need create a dummy
+ * stack frame to make trace_hardirqs_off happy.
+ */
+ andi. r12,r12,MSR_PR
+ beq 11f
+ stwu r1,-16(r1)
+ bl trace_hardirqs_off
+ addi r1,r1,16
+ b 12f
+
+11:
bl trace_hardirqs_off
+12:
lwz r0,GPR0(r1)
lwz r3,ORIG_GPR3(r1)
lwz r4,GPR4(r1)
--
1.7.4.1.433.gcd306
^ permalink raw reply related
* Re: [PATCH 4/5] treewide: Convert uses of ATTRIB_NORETURN to __noreturn
From: Paul Mundt @ 2011-11-11 2:02 UTC (permalink / raw)
To: Joe Perches
Cc: Andrew Morton, Fenghua Yu, linux-m68k, Tony Luck, linux-ia64,
linux-mips, linux-sh, Heiko Carstens, linux-kernel, Ralf Baechle,
David Howells, linux-am33-list, Geert Uytterhoeven, Chris Metcalf,
linux390, Martin Schwidefsky, Paul Mackerras, linux-s390,
Koichi Yasutake, linuxppc-dev
In-Reply-To: <abb1d8b542872ef3bfd695e85d3b8a0fd70645b9.1320917558.git.joe@perches.com>
On Thu, Nov 10, 2011 at 01:41:45AM -0800, Joe Perches wrote:
> Use the more commonly used __noreturn instead of ATTRIB_NORETURN.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> arch/ia64/kernel/machine_kexec.c | 2 +-
> arch/m68k/amiga/config.c | 2 +-
> arch/mips/include/asm/ptrace.h | 2 +-
> arch/mips/kernel/traps.c | 2 +-
> arch/mn10300/include/asm/exceptions.h | 2 +-
> arch/powerpc/kernel/machine_kexec_32.c | 2 +-
> arch/powerpc/kernel/machine_kexec_64.c | 2 +-
> arch/s390/include/asm/processor.h | 2 +-
> arch/sh/kernel/process_32.c | 2 +-
> arch/sh/kernel/process_64.c | 2 +-
> arch/tile/kernel/machine_kexec.c | 2 +-
> include/linux/kernel.h | 6 +++---
> 12 files changed, 14 insertions(+), 14 deletions(-)
>
For the SH bits:
Signed-off-by: Paul Mundt <lethal@linux-sh.org>
^ permalink raw reply
* [PATCH] powerpc kvm: fix kvmppc_start_thread() for CONFIG_SMP=N
From: Michael Neuling @ 2011-11-11 2:03 UTC (permalink / raw)
To: Paul Mackerras, benh, Alexander Graf; +Cc: linuxppc-dev
Currently kvmppc_start_thread() tries to wake other SMT threads via
xics_wake_cpu(). Unfortunately xics_wake_cpu only exists when
CONFIG_SMP=Y so when compiling with CONFIG_SMP=N we get:
arch/powerpc/kvm/built-in.o: In function `.kvmppc_start_thread':
book3s_hv.c:(.text+0xa1e0): undefined reference to `.xics_wake_cpu'
The following should be fine since kvmppc_start_thread() shouldn't
called to start non-zero threads when SMP=N since threads_per_core=1.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
arch/powerpc/kvm/book3s_hv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-ozlabs/arch/powerpc/kvm/book3s_hv.c
===================================================================
--- linux-ozlabs.orig/arch/powerpc/kvm/book3s_hv.c
+++ linux-ozlabs/arch/powerpc/kvm/book3s_hv.c
@@ -538,7 +538,7 @@ static void kvmppc_start_thread(struct k
tpaca->kvm_hstate.napping = 0;
vcpu->cpu = vc->pcpu;
smp_wmb();
-#ifdef CONFIG_PPC_ICP_NATIVE
+#if defined(CONFIG_PPC_ICP_NATIVE) && defined(CONFIG_SMP)
if (vcpu->arch.ptid) {
tpaca->cpu_start = 0x80;
wmb();
^ permalink raw reply
* Re: [RFC PATCH 00/17] powerpc/e500: separate e500 from e500mc
From: Moffett, Kyle D @ 2011-11-11 0:38 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org
Cc: Timur Tabi, linux-kernel@vger.kernel.org, Paul Gortmaker,
Scott Wood
In-Reply-To: <20111110165455.GE11983@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 11:54, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 10:30:41AM -0600, Kumar Gala wrote:
>> On Nov 10, 2011, at 10:17 AM, Moffett, Kyle D wrote:
>>> Furthermore, it looks like there are a couple issues here I missed
>>> before. PPC64 systems all appear to have an L1_CACHE_SHIFT of 7,
>>> except when you turn on the P5020DS board option which magically
>>> changes it to "6" and breaks lord-knows-what. I think my patch
>>> series actually "breaks" that and makes e5500 use 7 as well.
>>=20
>> a value of '6' on E5500 / P5020DS is correct and doesn't break anything.
>> Setting it to 7 is wrong and thus the code is correct today.
>>=20
>>> Are you sure that a kernel built to support E5500 can also run on
>>> other 64-bit PowerPC/POWER systems?
>>=20
>> No it will not. There is not expectation of that as E5500 is an
>> embedded / Book-E class part and uses that ISA version. Book-S
>> (server) 64-bit machines are not OS compatible and we are not trying to
>> make them as such (but we do re-use a lot of code).
>=20
> What about other 64-bit book3e chips? What cache block size does A2 have=
?
Ok, so I've been poking around this code a bunch and as far as I can
tell, the cacheline stuff has basically always been subtly wrong in
twelve different ways and it's only largely coincidence that it works
today.
So PowerPC64 systems have their own "ppc64_caches" structure set up
before start_kernel() is called by parsing the OpenFirmware "cpu" nodes.
That structure is then checked in every piece of 64-bit kernel code
(except xmon) that uses the "dcbXX" and "icbXX" opcodes.
There is an entirely separate mechanism built into the "cputable" that
is used on all PowerPC systems to compute cacheline sizes to pass in via
ELF headers for userspace to use in memset()/memcpy(), etc.
Furthermore, the VDSO gets cacheline sizes stored into it, but on 64-bit
they come from the ppc64_caches structure and on 32-bit they come from
dcache_bsize/icache_bsize copied from the cputable.
Then there's the value in arch/powerpc/include/asm/cache.h which is used
throughout the kernel to figure out how far apart to space CPU-specific
datastructures (EG: __cacheline_aligned_on_smp).
Despite the fact that all PPC64 have an "L1_CACHE_SIZE" value of 128,
the PowerPC A2 and e5500 have {d,i}cache_bsize values of 64 in cputable
and presumably also get correct values from OpenFirmware, so the bogus
constant in asm/cache.h does nothing more than waste a bit of memory
for unnecessary padding.
Unfortunately, lots of PPC32 assembly pretends that the value found in
asm/cache.h is a hard truth and uses it for "dcbz", etc, which is why
there are all of those ugly #ifdefs in asm/cache.h
Based on all of that, my proposal is going to be a patch which does the
following:
(1) Conditionally set L1_CACHE_SHIFT to the maximum value used by any
platform being compiled in for alignment purposes.
(2) Make the ppc64_caches struct apply to ppc32 as well, and
preinitialize it with a minimum value used by any platform being
compiled in (for "dcbXX"/"icbXX" purposes). This is safe because
the pagesize is always a multiple of the cache block size and the
kernel only uses dcbXX/icbXX on whole pages. The only impact is a
temporary small performance hit from flushing or zeroing the same
block 8 times if too small.
(3) Try to initialize the ppc_caches struct on ppc32 from the
OpenFirmware device-tree. If that fails, then use the values we
find in the cputable. After this is initialized any performance
hit in copy_page()/zero_page() will obviously disappear.
(4) Fix all of the PPC32 assembly code that is misusing L1_CACHE_SHIFT
to use the ppc_caches struct instead.
Does that sound like a reasonable approach?
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
^ permalink raw reply
* Re: [PATCH v2 1/5] [ppc] Process dynamic relocations for kernel
From: Josh Poimboeuf @ 2011-11-10 21:44 UTC (permalink / raw)
To: Suzuki Poulose
Cc: Nathan Miller, Josh Poimboeuf, Dave Hansen, Paul Mackerras,
Scott Wood, Alan Modra, linuxppc-dev
In-Reply-To: <4EBB3794.9050309@in.ibm.com>
On Thu, 2011-11-10 at 08:01 +0530, Suzuki Poulose wrote:
> >> How about using clean_dcache_range() to flush the range runtime
> >> address range [ _stext, _end ] ? That would flush the entire
> >> instructions.
> >
> > Wouldn't that result in more cache flushing than the original solution?
> >
> > For example, my kernel is 3.5MB. Assuming a 32 byte cache line size,
> > clean_dcache_range(_stext, _end) would result in about 115,000 dcbst's
> > (3.5MB / 32).
>
> Oops ! You are right. We could go back to the clean_dcache_all() or the
> initial approach that you suggested. (dcbst).
>
> I am not sure how do we flush the entire dcache(only). Could you post a
> patch which does the same ?
It turns out that my original idea of giving a 64k address range to
clean_dcache_range() wouldn't work, because dcbst only flushes the line
if the given address is in the cache already.
Also, I experimented with a simple clean_dcache_all() function in
misc_32.S:
#define L1_CACHE_SIZE (32 * 1024)
#define L1_CACHE_LINES (L1_CACHE_SIZE / L1_CACHE_BYTES)
_GLOBAL(clean_dcache_all)
lis r3, _start@h
ori r3, r3, _start@l
li r4, (L1_CACHE_LINES * 2)
mtctr r4
1: lwz r5, 0(r3)
addi r3, r3, L1_CACHE_BYTES
bdnz 1b
sync
blr
But this approach has some issues:
1. It should probably be made more platform-independent with respect to
d-cache size. I'm not sure the best way to achieve that.
2. The _start address is the kernel virtual address, not the physical
address, but relocate() is running without TLB address translation
enabled. Although we could easily circumvent this problem by clearing
the d-cache directly in relocate() (or in head_44x.S) using physical
addresses.
3. Chicken/egg issue: the _start address might be stale because we
haven't yet flushed the d-cache and invalidated the i-cache.
I also discovered that calling flush_instruction_cache() from relocate()
wouldn't work for all platforms, for similar reasons.
We could overcome these issues with more code, but the added complexity
might not be worth it (premature optimization and all). My original
patch at least has the benefit of being simple.
>
> Another option is to, change the current mapping to 'Write Through' before
> calling the relocate() and revert back to the original setting after relocate().
True, that's another option. Although since TLB handling is
platform-specific, I think it would have to be handled by the caller in
head_44x.S, rather than within relocate().
> > That also may suggest a bigger can of worms. A grep of the powerpc code
> > shows many uses of KERNELBASE. For a relocatable kernel, nobody should
> > be relying on KERNELBASE except for the early relocation code. Are we
> > sure that all the other usages of KERNELBASE are "safe"?
> >
> I think we could simply replace the occurrences of KERNELBASE (after the relocate())
> with '_stext' which would give the virtual start address of the kernel.
Yeah, that would work.
Josh
^ permalink raw reply
* Re: [RFC PATCH 08/17] powerpc/e500: Remove conditional "lwsync" substitution
From: Moffett, Kyle D @ 2011-11-10 20:27 UTC (permalink / raw)
To: Scott Wood
Cc: Baruch Siach, Timur Tabi, linux-kernel@vger.kernel.org,
Paul Gortmaker, Paul Mackerras, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20111110170334.GF11983@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 12:03, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 10:42:25AM -0600, Kumar Gala wrote:
>>=20
>> On Nov 10, 2011, at 10:31 AM, Scott Wood wrote:
>>=20
>>> On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
>>>> Nak, we can run an e500mc in a mode that is compatible with e500v1/v2.
>>>> I see no reason to change the support we have there.
>>>=20
>>> What "mode" do you mean? DCBZ32? We don't support using that currentl=
y,
>>> and I'd imagine the performance implication would be such that you'd
>>> never want to do it unless it's the only way to make some piece of lega=
cy
>>> software work.
>>=20
>> Correct, DCBZ32, we've had customers that go down this path.
>=20
> For running legacy software, or for multiplatform Linux kernels?
>=20
> And if you're willing to toss performance away for this goal, why do you
> need lwsync? :-)
>=20
> DCBZ32 is not a "mode that is compatible with v1/v2", BTW. It only
> affects cache block size (for dcbz/dcba only), not SPE versus FP, not
> changes in power management, not changes in machine check handling, etc.
>=20
> Using DCBZ32 for the kernel would also complicate switching the kernel to
> dcbzl, to support enabling DCBZ32 for certain userspace apps (a more
> likely use case) without making it systemwide.
So, as far as I can tell the kernel doesn't even try to touch DCBZ32.
Even if it did, if you are building a new kernel that includes this patch,
surely you can actually build a proper e500mc kernel instead of trying to
build a new kernel to run on hardware it wasn't designed to run on, right?
I think the bigger issue is the fact that building a PPC_BOOK3E_64 kernel
with both e5500 and PowerPC A2 support turned on will not actually run on
both. Before my v1-patch-series, machine-check handling is messed up for
PowerPC A2, and afterwards cacheline sizes are messed up for e5500.
Does this mean that PPC_BOOK3E_64 needs to be split into two separate
Book 3-III families the same way that 32-bit has been split? Is there
another way around it?
Cheers,
Kyle Moffett
^ permalink raw reply
* Re: [RFC PATCH 08/17] powerpc/e500: Remove conditional "lwsync" substitution
From: Kumar Gala @ 2011-11-10 20:34 UTC (permalink / raw)
To: Moffett, Kyle D
Cc: Baruch Siach, Timur Tabi, linux-kernel@vger.kernel.org,
Paul Gortmaker, Paul Mackerras, Scott Wood,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4567A1A9-51D2-4D21-9080-BD3D78901C1D@boeing.com>
On Nov 10, 2011, at 2:27 PM, Moffett, Kyle D wrote:
> On Nov 10, 2011, at 12:03, Scott Wood wrote:
>> On Thu, Nov 10, 2011 at 10:42:25AM -0600, Kumar Gala wrote:
>>>=20
>>> On Nov 10, 2011, at 10:31 AM, Scott Wood wrote:
>>>=20
>>>> On Thu, Nov 10, 2011 at 07:40:04AM -0600, Kumar Gala wrote:
>>>>> Nak, we can run an e500mc in a mode that is compatible with =
e500v1/v2.
>>>>> I see no reason to change the support we have there.
>>>>=20
>>>> What "mode" do you mean? DCBZ32? We don't support using that =
currently,
>>>> and I'd imagine the performance implication would be such that =
you'd
>>>> never want to do it unless it's the only way to make some piece of =
legacy
>>>> software work.
>>>=20
>>> Correct, DCBZ32, we've had customers that go down this path.
>>=20
>> For running legacy software, or for multiplatform Linux kernels?
>>=20
>> And if you're willing to toss performance away for this goal, why do =
you
>> need lwsync? :-)
>>=20
>> DCBZ32 is not a "mode that is compatible with v1/v2", BTW. It only
>> affects cache block size (for dcbz/dcba only), not SPE versus FP, not
>> changes in power management, not changes in machine check handling, =
etc.
>>=20
>> Using DCBZ32 for the kernel would also complicate switching the =
kernel to
>> dcbzl, to support enabling DCBZ32 for certain userspace apps (a more
>> likely use case) without making it systemwide.
>=20
> So, as far as I can tell the kernel doesn't even try to touch DCBZ32.
Correct, it was my thinking I'd get there an add this one day, that day =
never came.
> Even if it did, if you are building a new kernel that includes this =
patch,
> surely you can actually build a proper e500mc kernel instead of trying =
to
> build a new kernel to run on hardware it wasn't designed to run on, =
right?
>=20
> I think the bigger issue is the fact that building a PPC_BOOK3E_64 =
kernel
> with both e5500 and PowerPC A2 support turned on will not actually run =
on
> both. Before my v1-patch-series, machine-check handling is messed up =
for
> PowerPC A2, and afterwards cacheline sizes are messed up for e5500.
That might be, but who is asking or wanting to run a BOOK3E_64 kernel on =
both. I'm guessing there are a number of issues with this.
> Does this mean that PPC_BOOK3E_64 needs to be split into two separate
> Book 3-III families the same way that 32-bit has been split? Is there
> another way around it?
No idea, we have to ask Ben how much he cares. I don't see any FSL =
customers pushing us to run the same kernel on A2 and P5020 (or future =
FSL devices).
- k=
^ permalink raw reply
* Re: [RFC][PATCH 08/30] powerpc/85xx: Rework P1020 SoC device tree
From: Kumar Gala @ 2011-11-10 20:27 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20111110185818.GB12339@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 12:58 PM, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 12:46:27PM -0600, Kumar Gala wrote:
>>=20
>> On Nov 10, 2011, at 12:05 PM, Scott Wood wrote:
>>=20
>>> On Thu, Nov 10, 2011 at 10:13:51AM -0600, Kumar Gala wrote:
>>>> Split the P1020 SoC device tree into what we can include as a =
'prefix'
>>>> to the board device tree and what needs to be included as a =
'postfix'.
>>>>=20
>>>> This allows use more re-use and less duplication between various =
board
>>>> device tree configurations (32-bit address map vs 36-bit address =
map).
>>>=20
>>> Could you elaborate on the issues that require this pre/post =
approach?
>>=20
>> We utilize two 'features' of dtc to accomplish things. One is =
includes the other is 'merging of labeled nodes'.
>>=20
>> We can using merging to allow the board to specify the "reg" & =
"ranges"
>> values of a SoC node that might vary between physical address maps
>> (32-bit vs 36-bit as example).
>>=20
>> dtc is able to 'merge' multiple labeled nodes as well. So we use =
that
>> to handle board specific values for something like localbus if we =
want
>> to support both a 32-bit and 36-bit address map.
>>=20
>>> What does the SoC part need to specify that has to override or go =
within
>>> something the board specifies?
>>=20
>> Not sure I follow the question.
>=20
> My question is, why does the stuff in post need to be in post, versus =
pre?
>=20
> Why can't pre define the soc node, and let the board dts set ranges?=20=
> Likewise for localbus, pci, etc.
>=20
> -Scott
Gotcha, the reason we have -pre and -post is because of how dtc include =
mechanism works. It builds it's tree in the order things are parsed in =
the file. In addition, "labeled nodes" like we have in post for 'soc', =
'lbc', etc. have to be defined after they are referenced.
- k=
^ permalink raw reply
* Re: [RFC][PATCH 28/30] powerpc/85xx: Rework P1023RDS device tree
From: Kumar Gala @ 2011-11-10 20:24 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20111110185559.GA12339@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 12:55 PM, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 12:42:54PM -0600, Kumar Gala wrote:
>>=20
>> On Nov 10, 2011, at 12:10 PM, Scott Wood wrote:
>>=20
>>> On Thu, Nov 10, 2011 at 10:14:11AM -0600, Kumar Gala wrote:
>>>> - power@e0070{
>>>> - compatible =3D "fsl,mpc8536-pmc", =
"fsl,mpc8548-pmc",
>>>> - "fsl,p1022-pmc";
>>>> - reg =3D <0xe0070 0x20>;
>>>> - etsec1_clk: soc-clk@B0{
>>>> - fsl,pmcdr-mask =3D <0x00000080>;
>>>> - };
>>>> - etsec2_clk: soc-clk@B1{
>>>> - fsl,pmcdr-mask =3D <0x00000040>;
>>>> - };
>>>> - etsec3_clk: soc-clk@B2{
>>>> - fsl,pmcdr-mask =3D <0x00000020>;
>>>> - };
>>>> - };
>>>=20
>>> The compatibles on this node are wrong, but it looks like you're =
removing
>>> it altogether?
>>>=20
>>> -Scott
>>=20
>> Yeah, I think the whole thing needs to be relocated at because there =
are no etsec's on P1023, etc. So will comment about this in commit =
message
>=20
> Ah right, the etsec stuff should go as well. :-)
>=20
> It should look like this:
>=20
> power@e0070 {
> compatible =3D "fsl,p1023-pmc", "fsl,mpc8548-pmc";
> reg =3D <0xe0070 0x20>;
> };
What does being compatible with mpc8548-pmc mean?
- k=
^ permalink raw reply
* Re: [RFC][PATCH 08/30] powerpc/85xx: Rework P1020 SoC device tree
From: Scott Wood @ 2011-11-10 18:58 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <5561C3BD-B4E5-4D82-84C4-B8715191DDE3@kernel.crashing.org>
On Thu, Nov 10, 2011 at 12:46:27PM -0600, Kumar Gala wrote:
>
> On Nov 10, 2011, at 12:05 PM, Scott Wood wrote:
>
> > On Thu, Nov 10, 2011 at 10:13:51AM -0600, Kumar Gala wrote:
> >> Split the P1020 SoC device tree into what we can include as a 'prefix'
> >> to the board device tree and what needs to be included as a 'postfix'.
> >>
> >> This allows use more re-use and less duplication between various board
> >> device tree configurations (32-bit address map vs 36-bit address map).
> >
> > Could you elaborate on the issues that require this pre/post approach?
>
> We utilize two 'features' of dtc to accomplish things. One is includes the other is 'merging of labeled nodes'.
>
> We can using merging to allow the board to specify the "reg" & "ranges"
> values of a SoC node that might vary between physical address maps
> (32-bit vs 36-bit as example).
>
> dtc is able to 'merge' multiple labeled nodes as well. So we use that
> to handle board specific values for something like localbus if we want
> to support both a 32-bit and 36-bit address map.
>
> > What does the SoC part need to specify that has to override or go within
> > something the board specifies?
>
> Not sure I follow the question.
My question is, why does the stuff in post need to be in post, versus pre?
Why can't pre define the soc node, and let the board dts set ranges?
Likewise for localbus, pci, etc.
-Scott
^ permalink raw reply
* Re: [RFC][PATCH 28/30] powerpc/85xx: Rework P1023RDS device tree
From: Scott Wood @ 2011-11-10 18:55 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <B48BB824-D8FF-4D60-B0A5-B6C237074907@kernel.crashing.org>
On Thu, Nov 10, 2011 at 12:42:54PM -0600, Kumar Gala wrote:
>
> On Nov 10, 2011, at 12:10 PM, Scott Wood wrote:
>
> > On Thu, Nov 10, 2011 at 10:14:11AM -0600, Kumar Gala wrote:
> >> - power@e0070{
> >> - compatible = "fsl,mpc8536-pmc", "fsl,mpc8548-pmc",
> >> - "fsl,p1022-pmc";
> >> - reg = <0xe0070 0x20>;
> >> - etsec1_clk: soc-clk@B0{
> >> - fsl,pmcdr-mask = <0x00000080>;
> >> - };
> >> - etsec2_clk: soc-clk@B1{
> >> - fsl,pmcdr-mask = <0x00000040>;
> >> - };
> >> - etsec3_clk: soc-clk@B2{
> >> - fsl,pmcdr-mask = <0x00000020>;
> >> - };
> >> - };
> >
> > The compatibles on this node are wrong, but it looks like you're removing
> > it altogether?
> >
> > -Scott
>
> Yeah, I think the whole thing needs to be relocated at because there are no etsec's on P1023, etc. So will comment about this in commit message
Ah right, the etsec stuff should go as well. :-)
It should look like this:
power@e0070 {
compatible = "fsl,p1023-pmc", "fsl,mpc8548-pmc";
reg = <0xe0070 0x20>;
};
-Scott
^ permalink raw reply
* Re: [RFC PATCH 16/17] powerpc/e500: Make __setup_cpu_{e200, e500, e500mc, e5500} optional
From: Kumar Gala @ 2011-11-10 18:52 UTC (permalink / raw)
To: Scott Wood
Cc: Baruch Siach, Shaohui Xie, Alexandre Bounine, Timur Tabi,
linux-kernel, Paul Gortmaker, Paul Mackerras, Kyle Moffett,
Andrew Morton, linuxppc-dev
In-Reply-To: <20111110164741.GC11983@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 10:47 AM, Scott Wood wrote:
> On Wed, Nov 09, 2011 at 07:07:14PM -0500, Kyle Moffett wrote:
>> Only build the setup functions when the corresponding entries are
>> included in the CPU table.
>>=20
>> Signed-off-by: Kyle Moffett <Kyle.D.Moffett@boeing.com>
>> ---
>> arch/powerpc/kernel/cpu_setup_fsl_booke.S | 14 +++++++++++++-
>> 1 files changed, 13 insertions(+), 1 deletions(-)
>>=20
>> diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S =
b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
>> index 8053db0..77721b2 100644
>> --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
>> +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
>> @@ -17,6 +17,8 @@
>> #include <asm/cputable.h>
>> #include <asm/ppc_asm.h>
>>=20
>> +#if defined(CONFIG_FSL_E500_V1_V2) || defined(CONFIG_FSL_E500MC) \
>> + || defined(CONFIG_FSL_E5500)
>=20
> This seems like the kind of thing that CONFIG_E500 should be retained
> for.
>=20
> -Scott
Agreed, we should define CONFIG_E500 as internal and used for common =
functionality between E500, E500MC, E5500, E6500.
- k=
^ permalink raw reply
* Re: [RFC][PATCH 15/30] powerpc/85xx: Rework P1022DS device tree
From: Kumar Gala @ 2011-11-10 18:50 UTC (permalink / raw)
To: Tabi Timur-B04825; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <CAOZdJXXh571mgF2rFWqfWOVDBLyS10TdKt9Gk86FhwObaiXjOw@mail.gmail.com>
On Nov 10, 2011, at 11:30 AM, Tabi Timur-B04825 wrote:
> On Thu, Nov 10, 2011 at 10:13 AM, Kumar Gala =
<galak@kernel.crashing.org> wrote:
>>=20
>> arch/powerpc/boot/dts/fsl/p1022si-post.dtsi | 235 +++++++++++++++
>> arch/powerpc/boot/dts/fsl/p1022si-pre.dtsi | 68 +++++
>> arch/powerpc/boot/dts/p1022ds.dts | 431 =
+--------------------------
>> 3 files changed, 318 insertions(+), 416 deletions(-)
>> create mode 100644 arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> create mode 100644 arch/powerpc/boot/dts/fsl/p1022si-pre.dtsi
>=20
> I understand the value of removing duplication of DTS contents, so
> that we don't copy-paste the same nodes over and over again, but now
> that you've added pre- and post- dtsi files, I wonder if you're not
> jumping out of the frying pan and into the flame. It seems that the
> DTS files are more fragile than ever, and it's more difficult now to
> figure out which files go where.
I hope that is not true, as the template is the same.
BOARD.dts:
include "fsl/SOCsi-pre.dtsi"
=85
include "fsl/SOCsi-post.dtsi"
[ some boards may do more complicated things like we do on MPC8572DS to =
deal with 32-bit vs 36-bit addr maps, or CAMP use examples ]
All BOARD dts's should follow this pattern given the knowledge of which =
SoC you are utilizing. For a board port, you should NEVER have to touch =
anything in the 'fsl/' directory. That assumes, we've done a good job =
with the .dts struct in general. =46rom time to time I'm sure we'll =
find some pattern or IP block we need to fix.
> Another concern I have is whether this will force all Pxxxx boards to
> have the same CCSR address. I don't think so, because I didn't see
> it, but I just want to make sure.
Nope, we can have as many boards for the same SoC w/different address =
maps and share the same SOC.dtsi files
- k=
^ permalink raw reply
* Re: [RFC][PATCH 08/30] powerpc/85xx: Rework P1020 SoC device tree
From: Kumar Gala @ 2011-11-10 18:46 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20111110180542.GA12203@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 12:05 PM, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 10:13:51AM -0600, Kumar Gala wrote:
>> Split the P1020 SoC device tree into what we can include as a =
'prefix'
>> to the board device tree and what needs to be included as a =
'postfix'.
>>=20
>> This allows use more re-use and less duplication between various =
board
>> device tree configurations (32-bit address map vs 36-bit address =
map).
>=20
> Could you elaborate on the issues that require this pre/post approach?
We utilize two 'features' of dtc to accomplish things. One is includes =
the other is 'merging of labeled nodes'.
We can using merging to allow the board to specify the "reg" & "ranges" =
values of a SoC node that might vary between physical address maps =
(32-bit vs 36-bit as example).
dtc is able to 'merge' multiple labeled nodes as well. So we use that =
to handle board specific values for something like localbus if we want =
to support both a 32-bit and 36-bit address map.
> What does the SoC part need to specify that has to override or go =
within
> something the board specifies?
Not sure I follow the question.
- k=
^ permalink raw reply
* Re: [RFC][PATCH 28/30] powerpc/85xx: Rework P1023RDS device tree
From: Kumar Gala @ 2011-11-10 18:42 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20111110181047.GB12203@schlenkerla.am.freescale.net>
On Nov 10, 2011, at 12:10 PM, Scott Wood wrote:
> On Thu, Nov 10, 2011 at 10:14:11AM -0600, Kumar Gala wrote:
>> - power@e0070{
>> - compatible =3D "fsl,mpc8536-pmc", =
"fsl,mpc8548-pmc",
>> - "fsl,p1022-pmc";
>> - reg =3D <0xe0070 0x20>;
>> - etsec1_clk: soc-clk@B0{
>> - fsl,pmcdr-mask =3D <0x00000080>;
>> - };
>> - etsec2_clk: soc-clk@B1{
>> - fsl,pmcdr-mask =3D <0x00000040>;
>> - };
>> - etsec3_clk: soc-clk@B2{
>> - fsl,pmcdr-mask =3D <0x00000020>;
>> - };
>> - };
>=20
> The compatibles on this node are wrong, but it looks like you're =
removing
> it altogether?
>=20
> -Scott
Yeah, I think the whole thing needs to be relocated at because there are =
no etsec's on P1023, etc. So will comment about this in commit message
- k=
^ permalink raw reply
* Re: [RFC][PATCH 28/30] powerpc/85xx: Rework P1023RDS device tree
From: Scott Wood @ 2011-11-10 18:10 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <1320941653-29797-29-git-send-email-galak@kernel.crashing.org>
On Thu, Nov 10, 2011 at 10:14:11AM -0600, Kumar Gala wrote:
> - power@e0070{
> - compatible = "fsl,mpc8536-pmc", "fsl,mpc8548-pmc",
> - "fsl,p1022-pmc";
> - reg = <0xe0070 0x20>;
> - etsec1_clk: soc-clk@B0{
> - fsl,pmcdr-mask = <0x00000080>;
> - };
> - etsec2_clk: soc-clk@B1{
> - fsl,pmcdr-mask = <0x00000040>;
> - };
> - etsec3_clk: soc-clk@B2{
> - fsl,pmcdr-mask = <0x00000020>;
> - };
> - };
The compatibles on this node are wrong, but it looks like you're removing
it altogether?
-Scott
^ permalink raw reply
* Re: [RFC][PATCH 08/30] powerpc/85xx: Rework P1020 SoC device tree
From: Scott Wood @ 2011-11-10 18:05 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <1320941653-29797-9-git-send-email-galak@kernel.crashing.org>
On Thu, Nov 10, 2011 at 10:13:51AM -0600, Kumar Gala wrote:
> Split the P1020 SoC device tree into what we can include as a 'prefix'
> to the board device tree and what needs to be included as a 'postfix'.
>
> This allows use more re-use and less duplication between various board
> device tree configurations (32-bit address map vs 36-bit address map).
Could you elaborate on the issues that require this pre/post approach?
What does the SoC part need to specify that has to override or go within
something the board specifies?
-Scott
^ permalink raw reply
* Re: [RFC][PATCH 15/30] powerpc/85xx: Rework P1022DS device tree
From: Tabi Timur-B04825 @ 2011-11-10 17:30 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <1320941653-29797-16-git-send-email-galak@kernel.crashing.org>
On Thu, Nov 10, 2011 at 10:13 AM, Kumar Gala <galak@kernel.crashing.org> wr=
ote:
>
> =A0arch/powerpc/boot/dts/fsl/p1022si-post.dtsi | =A0235 +++++++++++++++
> =A0arch/powerpc/boot/dts/fsl/p1022si-pre.dtsi =A0| =A0 68 +++++
> =A0arch/powerpc/boot/dts/p1022ds.dts =A0 =A0 =A0 =A0 =A0 | =A0431 +------=
--------------------
> =A03 files changed, 318 insertions(+), 416 deletions(-)
> =A0create mode 100644 arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> =A0create mode 100644 arch/powerpc/boot/dts/fsl/p1022si-pre.dtsi
I understand the value of removing duplication of DTS contents, so
that we don't copy-paste the same nodes over and over again, but now
that you've added pre- and post- dtsi files, I wonder if you're not
jumping out of the frying pan and into the flame. It seems that the
DTS files are more fragile than ever, and it's more difficult now to
figure out which files go where.
Another concern I have is whether this will force all Pxxxx boards to
have the same CCSR address. I don't think so, because I didn't see
it, but I just want to make sure.
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply
* Re: [RFC PATCH 14/17] powerpc/e500: Don't make kgdb use e500v1/e500v2 registers on e500mc
From: Scott Wood @ 2011-11-10 16:46 UTC (permalink / raw)
To: Kyle Moffett
Cc: Baruch Siach, Timur Tabi, linux-kernel, Paul Gortmaker,
Paul Mackerras, linuxppc-dev
In-Reply-To: <1320883635-17194-15-git-send-email-Kyle.D.Moffett@boeing.com>
BOn Wed, Nov 09, 2011 at 07:07:12PM -0500, Kyle Moffett wrote:
> The only systems which need the sparse PowerPC register map are the
> e500v1/e500v2.
Is this due to SPE?
> +/*
> + * On FreeScale e500v1 or e500v2 processors we need to skip some register
> + * sections, so just add up a list of what we need to store.
> + *
> + * On all other 32-bit PowerPC we can just pick the last needed register.
> + */
The "s" in "Freescale" is not capitalized, BTW.
-Scott
^ permalink raw reply
* Re: [RFC PATCH 00/17] powerpc/e500: separate e500 from e500mc
From: Moffett, Kyle D @ 2011-11-10 16:17 UTC (permalink / raw)
To: Kumar Gala
Cc: Scott Wood, Timur Tabi, linuxppc-dev@lists.ozlabs.org,
linux-kernel@vger.kernel.org, Paul Gortmaker
In-Reply-To: <FF2BF3F3-1158-4747-AEEC-03D6A7C462D9@kernel.crashing.org>
On Nov 10, 2011, at 08:59, Kumar Gala wrote:
> On Nov 9, 2011, at 6:03 PM, Kyle Moffett wrote:
>> I saw Baruch Siach's patch:
>> powerpc: 85xx: separate e500 from e500mc
>>=20
>> Unfortunately, that patch breaks the dependencies for the P5020DS
>> platform and does not fix the underlying code which does not
>> understand what the ambiguous "CONFIG_E500" means.
>>=20
>> In order to fix the issue at the fundamental level, I created the
>> following 17-patch series loosely based on Baruch's patch.
>>=20
>> =3D=3D=3D High-Level Summary =3D=3D=3D
>>=20
>> The e500v1/v2 and e500mc/e5500 CPU families are not compatible with
>> each other, yet they share the same "CONFIG_E500" Kconfig option.
>>=20
>> The following patch series splits the 32-bit CPU support into two
>> separate options: "CONFIG_FSL_E500_V1_V2" and "CONFIG_FSL_E500MC".
>> Additionally, the 64-bit e5500 support is separated to its own config
>> option ("CONFIG_FSL_E5500") which is automatically combined with
>> either 32-bit e500MC or 64-bit Book-3E when the P5020DS board support
>> is enabled.
>=20
> So its clear from the community that there is confusion here and we
> need to clean this up. I guess my attempt to support an kernel that
> ran on both E500v2 and E500mc isn't worth it. However I don't want to
> completely remove the ability to do this.
Well, a kernel built with CONFIG_PPC_E500MC today appears to be
fundamentally broken on E500v1/E500v2:
#if defined(CONFIG_8xx) || defined(CONFIG_403GCX)
#define L1_CACHE_SHIFT 4
#define MAX_COPY_PREFETCH 1
#elif defined(CONFIG_PPC_E500MC)
#define L1_CACHE_SHIFT 6
#define MAX_COPY_PREFETCH 4
#elif defined(CONFIG_PPC32)
#define MAX_COPY_PREFETCH 4
#if defined(CONFIG_PPC_47x)
#define L1_CACHE_SHIFT 7
#else
#define L1_CACHE_SHIFT 5
#endif
#else /* CONFIG_PPC64 */
#define L1_CACHE_SHIFT 7
#endif
E500MC will set L1_CACHE_SHIFT to 6, while regular E500 appears to
want it set to 5. I don't know if that's a mistake or exactly what
code that affects, but it looks very wrong.
Furthermore, it looks like there are a couple issues here I missed
before. PPC64 systems all appear to have an L1_CACHE_SHIFT of 7,
except when you turn on the P5020DS board option which magically
changes it to "6" and breaks lord-knows-what. I think my patch
series actually "breaks" that and makes e5500 use 7 as well.
Are you sure that a kernel built to support E5500 can also run on
other 64-bit PowerPC/POWER systems?
> Towards the cleanup I'd ask for a proposal on what exactly the
> CONFIG_ options we'd end up with would be and their meaning.
> So today we have:
>=20
> CONFIG_E500
> CONFIG_PPC_E500MC
It's actually a bit more complicated than that. There are 3 ways
that the user can configure an e500 kernel today. I'm omitting
the "FSL_SOC_BOOKE" menu that wraps around all of the 85xx/e5500
boards today, because that is set for all of these platforms:
* PPC32 + PPC_85xx + E500 [+ boards]
* PPC64 + BOOK3E_64 + P5020_DS (which adds E500 and PPC_E500MC)
Note that whether or not "PPC_E500MC" is set on PPC32 depends
only on which boards the user picked. So if I am trying to
build an e500v2 kernel and I accidentally also turn on support
for one of the e500mc boards, my kernel mysteriously breaks.
> What do we want to move to? I want to keep the builds such that we
> have only 2 classes: e500V1/V2 and e500mc/e5500/e6500/.../eX500.
> I see no reason to hyper-optimize e500mc vs e5500 vs e6500.
So after my changes, there are the following user-configurable
option sets:
* PPC32 + FSL_E500_V1_V2 [+ e500v1/v2 boards]
* PPC32 + FSL_E500MC [+ e500mc boards]
* PPC64 + BOOK3E_64 + P5020_DS (which adds FSL_E5500)
Since most of the "e500mc"-specific code was in 32-bit-only ASM
or inside of #ifdef PPC32, the new FSL_E500MC option is only
set on 32-bit builds, even if it is running in compat mode on
64-bit e5500 hardware)
Internally the P5020_DS option turns on the hidden FSL_E5500
option for both 32-bit and 64-bit; that config option enables
platform drivers and similar stuff.
Depending on how compatible the AMP processors are, you could
rename the option to be "FSL_E5X00" or add a hidden option for
"FSL_E6500" that is also selected by appropriate boards.
Please let me know if you think!
Cheers,
Kyle Moffett
--
Curious about my work on the Debian powerpcspe port?
I'm keeping a blog here: http://pureperl.blogspot.com/
^ 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