| Message ID | fa686aa40903162143j5f641d7dob53b96b681737e07@mail.gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Grant Likely |
| Headers | show |
On Mar 16, 2009, at 11:43 PM, Grant Likely wrote: > On Mon, Mar 16, 2009 at 9:54 PM, Grant Likely <grant.likely@secretlab.ca > > wrote: >> On Mon, Mar 16, 2009 at 4:05 AM, Piotr Ziecik <kosmo@semihalf.com> >> wrote: >>> BestComm, a DMA engine in MPC52xx SoC, requires snooping when >>> CPU caches are enabled to work properly. >>> >>> Adding CPU_FTR_NEED_COHERENT fixes NFS problems on MPC52xx machines >>> introduced by 'powerpc/mm: Fix handling of _PAGE_COHERENT in BAT >>> setup code'. >>> [...] >>> #if defined(CONFIG_SMP) || defined(CONFIG_MPC10X_BRIDGE) \ >>> - || defined(CONFIG_PPC_83xx) || defined(CONFIG_8260) >>> + || defined(CONFIG_PPC_83xx) || defined(CONFIG_8260) \ >>> + || defined(CONFIG_PPC_MPC52xx) >>> #define CPU_FTR_COMMON CPU_FTR_NEED_COHERENT >>> #else >>> #define CPU_FTR_COMMON 0 >> >> Aside from the fact that MPC10X_BRIDGE, PPC83xx and 8260 are already >> doing it, adding the feature bit this way isn't multiplatform >> friendly. Essentially it means that all selected platforms will have >> CPU_FTR_NEED_COHERENT enabled if CONFIG_PPC_MPC52xx is enabled. > > Here's my counter-patch. It contains the change to just G2_LE cores > when MPC52xx is selected. However, this change will also affect some > of the MPC82xx parts. However CPU_FTR_NEED_COHERENT shouldn't > actually hurt anything, so maybe it would be better to just enable it > unconditionally for the G2_LE core. > > Kumar/Ben, thoughts? > > g. > > --- > > diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/ > include/asm/cput > index 4911104..48d7f5f 100644 > --- a/arch/powerpc/include/asm/cputable.h > +++ b/arch/powerpc/include/asm/cputable.h > @@ -348,8 +348,15 @@ extern const char *powerpc_base_platform; > CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX) > #define CPU_FTRS_82XX (CPU_FTR_COMMON | \ > CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB) > + > +#if defined(CONFIG_PPC_MPC52xx) > +#define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \ > + CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP | > CPU_FTR_NEED_COHERENT) > +#else > #define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \ > CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP) > +#endif > + > #define CPU_FTRS_E300 (CPU_FTR_MAYBE_CAN_DOZE | \ > CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP | \ > CPU_FTR_COMMON) Doing this via a static CPU FTR fixup isn't really the best way to handle it. I was thinking about this the other day in my patch to actually make G2/e300 cores respect _PAGE_COHERENT. We really should set this via a platform fixup. Just not sure if that's soon enough. - k
On Tue, Mar 17, 2009 at 6:10 AM, Kumar Gala <kumar.gala@freescale.com> wrote: > On Mar 16, 2009, at 11:43 PM, Grant Likely wrote: >> diff --git a/arch/powerpc/include/asm/cputable.h >> b/arch/powerpc/include/asm/cput >> index 4911104..48d7f5f 100644 >> --- a/arch/powerpc/include/asm/cputable.h >> +++ b/arch/powerpc/include/asm/cputable.h >> @@ -348,8 +348,15 @@ extern const char *powerpc_base_platform; >> CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX) >> #define CPU_FTRS_82XX (CPU_FTR_COMMON | \ >> CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB) >> + >> +#if defined(CONFIG_PPC_MPC52xx) >> +#define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \ >> + CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP | >> CPU_FTR_NEED_COHERENT) >> +#else >> #define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \ >> CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP) >> +#endif >> + >> #define CPU_FTRS_E300 (CPU_FTR_MAYBE_CAN_DOZE | \ >> CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP | \ >> CPU_FTR_COMMON) > > Doing this via a static CPU FTR fixup isn't really the best way to handle > it. I was thinking about this the other day in my patch to actually make > G2/e300 cores respect _PAGE_COHERENT. We really should set this via a > platform fixup. Just not sure if that's soon enough. I agree, but as Ben pointed out last night on IRC, feature fixup (early_init) occurs well before platform probe time. Platform code cannot fix it up until someone does the work of making platform probe time earlier. It's non-trivial. However, I need to get a fix in for this ASAP, otherwise the 5200 will be broken in 2.6.29. I don't see this patch as a final solution, but it works as a stop-gap until platform probing can be reworked. g.
On Mar 17, 2009, at 8:45 AM, Grant Likely wrote: > On Tue, Mar 17, 2009 at 6:10 AM, Kumar Gala > <kumar.gala@freescale.com> wrote: >> On Mar 16, 2009, at 11:43 PM, Grant Likely wrote: >>> diff --git a/arch/powerpc/include/asm/cputable.h >>> b/arch/powerpc/include/asm/cput >>> index 4911104..48d7f5f 100644 >>> --- a/arch/powerpc/include/asm/cputable.h >>> +++ b/arch/powerpc/include/asm/cputable.h >>> @@ -348,8 +348,15 @@ extern const char *powerpc_base_platform; >>> CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX) >>> #define CPU_FTRS_82XX (CPU_FTR_COMMON | \ >>> CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB) >>> + >>> +#if defined(CONFIG_PPC_MPC52xx) >>> +#define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \ >>> + CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP | >>> CPU_FTR_NEED_COHERENT) >>> +#else >>> #define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \ >>> CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP) >>> +#endif >>> + >>> #define CPU_FTRS_E300 (CPU_FTR_MAYBE_CAN_DOZE | \ >>> CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP | \ >>> CPU_FTR_COMMON) >> >> Doing this via a static CPU FTR fixup isn't really the best way to >> handle >> it. I was thinking about this the other day in my patch to >> actually make >> G2/e300 cores respect _PAGE_COHERENT. We really should set this >> via a >> platform fixup. Just not sure if that's soon enough. > > I agree, but as Ben pointed out last night on IRC, feature fixup > (early_init) occurs well before platform probe time. Platform code > cannot fix it up until someone does the work of making platform probe > time earlier. It's non-trivial. > > However, I need to get a fix in for this ASAP, otherwise the 5200 will > be broken in 2.6.29. I don't see this patch as a final solution, but > it works as a stop-gap until platform probing can be reworked. So for .29 lets just go with the easy solution of adding CONFIG_PPC_MPC52xx to the list that set CPU_FTR_NEED_COHERENT (10x, 83xx, etc..). - k
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cput index 4911104..48d7f5f 100644 --- a/arch/powerpc/include/asm/cputable.h +++ b/arch/powerpc/include/asm/cputable.h @@ -348,8 +348,15 @@ extern const char *powerpc_base_platform; CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX) #define CPU_FTRS_82XX (CPU_FTR_COMMON | \ CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_USE_TB) + +#if defined(CONFIG_PPC_MPC52xx) +#define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \ + CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NEED_COHERENT) +#else #define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \ CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP) +#endif + #define CPU_FTRS_E300 (CPU_FTR_MAYBE_CAN_DOZE | \ CPU_FTR_USE_TB | CPU_FTR_MAYBE_CAN_NAP | \ CPU_FTR_COMMON)