[PATCH] powerpc/85xx: don't init the mpic ipi for the SoC which has doorbell support

[Kevin Hao]

On Fri, Nov 08, 2013 at 03:16:12PM -0600, Scott Wood wrote:
> OK...  Why are you splitting out smp_85xx_basic_setup()?

In the current implementation of smp_85xx_setup_cpu(), we only invoke
the function mpic_setup_this_cpu() when the smp_85xx_ops.probe is set 
to smp_mpic_probe(). So if we set smp_85xx_ops.probe to NULL when doorbell
is available, we must make sure that the mpic_setup_this_cpu() is also invoked
when there does have a mpic. The smp_85xx_basic_setup() is for the board which
has no mpic.

  static void smp_85xx_setup_cpu(int cpu_nr)
  {
  	if (smp_85xx_ops.probe == smp_mpic_probe)
  		mpic_setup_this_cpu();
  
  	if (cpu_has_feature(CPU_FTR_DBELL))
  		doorbell_setup_this_cpu();
  }

>  Where do you
> call it other than from smp_85xx_setup_cpu()?

We would set the .setup_cpu() to smp_85xx_basic_setup() if it is a
non-mpic board. The following is quoted form the patch:
   	np = of_find_node_by_type(NULL, "open-pic");
   	if (np) {
   		smp_85xx_ops.probe = smp_mpic_probe;
  +		smp_85xx_ops.setup_cpu = smp_85xx_setup_cpu;
   		smp_85xx_ops.message_pass = smp_mpic_message_pass;
  -	}
  +	} else
  +		smp_85xx_ops.setup_cpu = smp_85xx_basic_setup;

>  Couldn't you have just
> removed the conditional without splitting up the function?

This will break the non-mpic board.

>  The change
> log says it's "to check if we need to invoke mpic_setup_this_cpu()"
> which doesn't make sense since we always want to call
> mpic_setup_this_cpu() if we have an MPIC.

If we have a mpic, we will call mpic_setup_this_cpu(). But if not,
we would set the .setup_cpu to smp_85xx_basic_setup() to avoid
the invoking of mpic_setup_this_cpu().

Thanks,
Kevin
> 
> -Scott
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20131109/a2a56a97/attachment.sig>