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
>
>
>
pgp2rdgzRxr73.pgp
Description: PGP signature
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
