Re: [PATCH] Remove CPU_FTR_NEED_COHERENT for 7448.

So, like, the other day Benjamin Herrenschmidt mumbled:

I still maintain it should be a runtime thing tho :-)

Ben.

So, could you comment on my proposed solution doing
things exactly this way?  Speifically, would folks
prefer the dynamic 

    number_of_cpus() == 1 

and possibly checking the device tree or SMP map, or the 

    #ifndef SMP

test guarding the removal of the CPU_FTR_NEED_COHERENT bit?

Thanks,
jdl

On Fri, 2007-05-04 at 10:16 -0500, Jon Loeliger wrote:

So, like, the other day Benjamin Herrenschmidt mumbled:

quoted

I still maintain it should be a runtime thing tho :-)

Ben.

So, could you comment on my proposed solution doing
things exactly this way?  Speifically, would folks
prefer the dynamic 

    number_of_cpus() == 1 

Sorry I don't remember the actual patch, must have missed it... I
suppose we could have generic code in early_init_devtree set the default
for this based on cpu_possible_map() containing more than one bit and
have platforms using one of those broken bridges force the bit in from
their probe routine.

Ben.

On Sat, 2007-05-05 at 08:25 +1000, Benjamin Herrenschmidt wrote:

On Fri, 2007-05-04 at 10:16 -0500, Jon Loeliger wrote:

quoted

So, could you comment on my proposed solution doing
things exactly this way?  Speifically, would folks
prefer the dynamic 

    number_of_cpus() == 1 

Sorry I don't remember the actual patch, must have missed it... I
suppose we could have generic code in early_init_devtree set the default
for this based on cpu_possible_map() containing more than one bit and
have platforms using one of those broken bridges force the bit in from
their probe routine.

Having looked into it further, the MPC106/7 are probably unique in
containing an internal cache which must be coherent with the CPU.
Presumably the designers intended it as a performance enhancement for
other PCI bus masters accessing PowerPC memory, but for most
applications the cost of turning on coherent memory would have
outweighed it.

So I'm now happy about removing CPU_FTR_NEED_COHERENT from the 7448. I
also agree that platforms with this quirk should turn it on during
probing, but there aren't any boards in arch/powerpc that need this. The
static method based on CONFIG_MPC10X_BRIDGE is probably good enough for
arch/ppc. 

-- 
Adrian Cox [off-list ref]

Sorry I don't remember the actual patch, must have missed it... I
suppose we could have generic code in early_init_devtree set=20
the default
for this based on cpu_possible_map() containing more than one bit and
have platforms using one of those broken bridges force the bit in from
their probe routine.
=20
Ben.

Hi Ben,

Yeah, I had no actual intention of retrofitting
This into all the existing boards.  I was more
Wanting to leave them alone and simply un-set the
Bit on the one board where I wanted it done better.

My proposed solution in the _probe() routine was:

        static int __init mpc86xx_hpcn_probe(void)
        {
        	unsigned long root =3D of_get_flat_dt_root();
       =20
        	if (of_flat_dt_is_compatible(root, "mpc86xx")) {
        		/* get number_of_cpus() from somewhere */
        		if (number_of_cpus() =3D=3D 1) {
        			cur_cpu_spec->cpu_features &=3D
~CPU_FTR_NEED_COHERENT;
        		}
        		return 1;	/* Looks good */
        	}
       =20
        	return 0;
        }

Or perhaps:

        static int __init mpc86xx_hpcn_probe(void)
        {
        	unsigned long root =3D of_get_flat_dt_root();
       =20
        	if (of_flat_dt_is_compatible(root, "mpc86xx")) {
	  #ifndef CONFIG_SMP
        		cur_cpu_spec->cpu_features &=3D
~CPU_FTR_NEED_COHERENT;
	  #endif
        		return 1;	/* Looks good */
        	}
       =20
        	return 0;
        }

Specifically, which notion of multi-CPU-ness do
We want to honor here?

Thanks,
Jdl