On Wednesday 26 May 2010 01:38:47 Grant Likely wrote: > (cc'ing Josh Boyer and John Linn) > > On Thu, May 20, 2010 at 4:01 AM, Sergey Temerkhanov > > <temerkha...@cifronik.ru> wrote: > > This patch enables support for Xilinx Virtex 4 FX singe-float FPU. > > > > Changelog v1->v2: > > -Added MSR_AP bit definition > > -Renamed CONFIG_XILINX_FPU to CONFIG_XILINX_SOFTFPU, moved it to > > 'Platform support' and made it Virtex4-FX-only. > > -Changed SAVE_FPR/REST_FPR definition style. > > > > Caveats: > > - Hard-float binaries which rely on in-kernel math emulation will > > give wrong results since they expect 64-bit double-precision instead of > > 32-bit single-precision numbers which Xilinx V4-FX Soft FPU produces. > > > > Regards, Sergey Temerkhanov > > Hi Sergey. Comments below. > > First off, see if you can use 'git mail' or some other way to inline > your patches. Patches as attachments are awkward to deal with and the > patch description is getting separated from the patch itself. > > > Signed-off-by: Sergey Temerkhanov<temerkha...@cifronik.ru> > > > > diff -r b59861a64e13 arch/powerpc/include/asm/ppc_asm.h > > --- a/arch/powerpc/include/asm/ppc_asm.h Thu May 20 13:24:53 2010 +0400 > > +++ b/arch/powerpc/include/asm/ppc_asm.h Thu May 20 13:55:10 2010 +0400 > > @@ -85,13 +85,21 @@ > > #define REST_8GPRS(n, base) REST_4GPRS(n, base); REST_4GPRS(n+4, > > base) > > #define REST_10GPRS(n, base) REST_8GPRS(n, base); REST_2GPRS(n+8, > > base) > > > > -#define SAVE_FPR(n, base) stfd n,THREAD_FPR0+8*TS_FPRWIDTH*(n)(base) > > + > > +#ifdef CONFIG_XILINX_SOFTFPU > > +#define SAVE_FPR(n, base) stfs n,THREAD_FPR0+8*TS_FPRWIDTH*(n)(base) > > +#define REST_FPR(n, base) lfs n,THREAD_FPR0+8*TS_FPRWIDTH*(n)(base) > > +#else > > +#define SAVE_FPR(n, base) stfd n,THREAD_FPR0+8*TS_FPRWIDTH*(n)(base) > > +#define REST_FPR(n, base) lfd n,THREAD_FPR0+8*TS_FPRWIDTH*(n)(base) > > +#endif > > + > > Hint: If you don't change the whitespace on the SAVE_FPR() line, then diff > will realize it is unchanged and reviewers will have more context queues > as to what you are doing. > > Otherwise, this looks better.
Agreed, I've missed it somehow. > > > #define SAVE_2FPRS(n, base) SAVE_FPR(n, base); SAVE_FPR(n+1, base) > > #define SAVE_4FPRS(n, base) SAVE_2FPRS(n, base); SAVE_2FPRS(n+2, > > base) > > #define SAVE_8FPRS(n, base) SAVE_4FPRS(n, base); SAVE_4FPRS(n+4, > > base) > > #define SAVE_16FPRS(n, base) SAVE_8FPRS(n, base); SAVE_8FPRS(n+8, > > base) > > #define SAVE_32FPRS(n, base) SAVE_16FPRS(n, base); SAVE_16FPRS(n+16, > > base) -#define REST_FPR(n, > > base) lfd n,THREAD_FPR0+8*TS_FPRWIDTH*(n)(base) + > > #define REST_2FPRS(n, base) REST_FPR(n, base); REST_FPR(n+1, base) > > #define REST_4FPRS(n, base) REST_2FPRS(n, base); REST_2FPRS(n+2, > > base) > > #define REST_8FPRS(n, base) REST_4FPRS(n, base); REST_4FPRS(n+4, > > base) > > diff -r b59861a64e13 arch/powerpc/include/asm/reg.h > > --- a/arch/powerpc/include/asm/reg.h Thu May 20 13:24:53 2010 +0400 > > +++ b/arch/powerpc/include/asm/reg.h Thu May 20 13:55:10 2010 +0400 > > @@ -30,6 +30,7 @@ > > #define MSR_ISF_LG 61 /* Interrupt 64b mode valid on 630 */ > > #define MSR_HV_LG 60 /* Hypervisor state */ > > #define MSR_VEC_LG 25 /* Enable AltiVec */ > > +#define MSR_AP_LG 25 /* Enable APU */ > > #define MSR_VSX_LG 23 /* Enable VSX */ > > #define MSR_POW_LG 18 /* Enable Power Management */ > > #define MSR_WE_LG 18 /* Wait State Enable */ > > @@ -71,6 +72,7 @@ > > #define MSR_HV 0 > > #endif > > > > +#define MSR_AP __MASK(MSR_AP_LG) /* Enable APU */ > > Need to be more specific: "Enable Xilinx Virtex 405 APU". Same goes > for MSR_AP_LG line above. This bit is also defined for PPC405 cores other than Xilinx, so I think it will be better as "Enable PPC405 APU". > > > #define MSR_VEC __MASK(MSR_VEC_LG) /* Enable AltiVec */ > > #define MSR_VSX __MASK(MSR_VSX_LG) /* Enable VSX */ > > #define MSR_POW __MASK(MSR_POW_LG) /* Enable Power > > Management */ > > diff -r b59861a64e13 arch/powerpc/kernel/fpu.S > > --- a/arch/powerpc/kernel/fpu.S Thu May 20 13:24:53 2010 +0400 > > +++ b/arch/powerpc/kernel/fpu.S Thu May 20 13:55:10 2010 +0400 > > @@ -57,6 +57,9 @@ > > _GLOBAL(load_up_fpu) > > mfmsr r5 > > ori r5,r5,MSR_FP > > +#ifdef CONFIG_XILINX_SOFTFPU > > + oris r5,r5,msr...@h > > +#endif > > #ifdef CONFIG_VSX > > BEGIN_FTR_SECTION > > oris r5,r5,msr_...@h > > @@ -85,6 +88,9 @@ > > toreal(r5) > > PPC_LL r4,_MSR-STACK_FRAME_OVERHEAD(r5) > > li r10,MSR_FP|MSR_FE0|MSR_FE1 > > +#ifdef CONFIG_XILINX_SOFTFPU > > + oris r10,r10,msr...@h > > +#endif > > andc r4,r4,r10 /* disable FP for previous task */ > > PPC_STL r4,_MSR-STACK_FRAME_OVERHEAD(r5) > > 1: > > @@ -94,6 +100,9 @@ > > mfspr r5,SPRN_SPRG3 /* current task's THREAD (phys) */ > > lwz r4,THREAD_FPEXC_MODE(r5) > > ori r9,r9,MSR_FP /* enable FP for current */ > > +#ifdef CONFIG_XILINX_SOFTFPU > > + oris r9,r9,msr...@h > > +#endif > > or r9,r9,r4 > > #else > > ld r4,PACACURRENT(r13) > > @@ -124,6 +133,9 @@ > > _GLOBAL(giveup_fpu) > > mfmsr r5 > > ori r5,r5,MSR_FP > > +#ifdef CONFIG_XILINX_SOFTFPU > > + oris r5,r5,msr...@h > > +#endif > > #ifdef CONFIG_VSX > > BEGIN_FTR_SECTION > > oris r5,r5,msr_...@h > > @@ -145,6 +157,9 @@ > > beq 1f > > PPC_LL r4,_MSR-STACK_FRAME_OVERHEAD(r5) > > li r3,MSR_FP|MSR_FE0|MSR_FE1 > > +#ifdef CONFIG_XILINX_SOFTFPU > > + oris r3,r3,msr...@h > > +#endif > > #ifdef CONFIG_VSX > > BEGIN_FTR_SECTION > > oris r3,r3,msr_...@h > > diff -r b59861a64e13 arch/powerpc/kernel/head_40x.S > > --- a/arch/powerpc/kernel/head_40x.S Thu May 20 13:24:53 2010 +0400 > > +++ b/arch/powerpc/kernel/head_40x.S Thu May 20 13:55:10 2010 +0400 > > @@ -420,7 +420,19 @@ > > addi r3,r1,STACK_FRAME_OVERHEAD > > EXC_XFER_STD(0x700, program_check_exception) > > > > +/* 0x0800 - FPU unavailable Exception */ > > +#ifdef CONFIG_PPC_FPU > > + START_EXCEPTION(0x0800, FloatingPointUnavailable) > > + NORMAL_EXCEPTION_PROLOG > > + beq 1f; \ > > + bl load_up_fpu; /* if from user, just load it up */ \ > > + b fast_exception_return; \ > > +1: addi r3,r1,STACK_FRAME_OVERHEAD; \ > > + EXC_XFER_EE_LITE(0x800, kernel_fp_unavailable_exception) > > +#else > > EXCEPTION(0x0800, Trap_08, unknown_exception, EXC_XFER_EE) > > +#endif > > + > > EXCEPTION(0x0900, Trap_09, unknown_exception, EXC_XFER_EE) > > EXCEPTION(0x0A00, Trap_0A, unknown_exception, EXC_XFER_EE) > > EXCEPTION(0x0B00, Trap_0B, unknown_exception, EXC_XFER_EE) > > @@ -432,7 +444,7 @@ > > > > EXCEPTION(0x0D00, Trap_0D, unknown_exception, EXC_XFER_EE) > > EXCEPTION(0x0E00, Trap_0E, unknown_exception, EXC_XFER_EE) > > - EXCEPTION(0x0F00, Trap_0F, unknown_exception, EXC_XFER_EE) > > + EXCEPTION(0x0F20, Trap_0F, unknown_exception, EXC_XFER_EE) > > Is this change required to support the FPU? It looks like something > that belongs in a separate patch. Well, it's not exactly necessary - it's just a stub for "APU Unavailable Exception". I'll move it to another patch. > > > /* 0x1000 - Programmable Interval Timer (PIT) Exception */ > > START_EXCEPTION(0x1000, Decrementer) > > @@ -821,8 +833,10 @@ > > * The PowerPC 4xx family of processors do not have an FPU, so this just > > * returns. > > */ > > +#ifndef CONFIG_PPC_FPU > > _ENTRY(giveup_fpu) > > blr > > +#endif > > > > /* This is where the main kernel code starts. > > */ > > diff -r b59861a64e13 arch/powerpc/platforms/Kconfig > > --- a/arch/powerpc/platforms/Kconfig Thu May 20 13:24:53 2010 +0400 > > +++ b/arch/powerpc/platforms/Kconfig Thu May 20 13:55:10 2010 +0400 > > @@ -333,4 +333,9 @@ > > bool "Xilinx PCI host bridge support" > > depends on PCI && XILINX_VIRTEX > > > > +config XILINX_SOFTFPU > > + bool "Xilinx Soft FPU" > > + select PPC_FPU > > + depends on XILINX_VIRTEX_4_FX && !PPC40x_SIMPLE && !405GP && !405GPR > > + > > Hmmm. There should be a nicer way of doing this, but this will do for now. > > Otherwise, this patch looks good to me. Josh, what do you think? > > Cheers, > g. > -- Regards, Sergey Temerkhanov, Cifronic ZAO _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev