Hello li.xi...@freescale.com,
On Tue, 18 Nov 2014 02:01:02 +0000, li.xi...@freescale.com
<li.xi...@freescale.com> wrote:
> Hi Albert,
>
>
> > > > > > > +#if defined(CONFIG_ARMV7_NONSEC) || defined(CONFIG_ARMV7_VIRT)
> > > > > > > +/* Setting the address at which secondary cores start from.*/
> > > > > > > +void smp_set_core_boot_addr(unsigned long addr, int corenr)
> > > > > > > +{
> > > > > > > + struct ccsr_gur __iomem *gur = (void
> > > > > > > *)(CONFIG_SYS_FSL_GUTS_ADDR);
> > > > > > > +
> > > > > > > + /*
> > > > > > > + * After setting the secondary cores start address,
> > > > > > > + * just release them to boot.
> > > > > > > + */
> > > > > > > + out_be32(&gur->scratchrw[0], addr);
> > > > > > > + out_be32(&gur->brrl, 0x2);
> > > > > > > +}
> > > > > >
> > > > > > This function does not exactly "[set] the address at which secondary
> > > > > > cores start from"; it sets *a* secondary core's boot address, and
> > > > > > then
> > > > > > it *boots* it.
> > > > > >
> > > > >
> > > > > Okay, I will fix it later.
> > > > >
> > > > > > Why does this version of smp_set_core_boot_addr() need to boot the
> > core
> > > > > > in addition to setting the address, whereas the existing ones in
> > > > > > virt_v7, vexpress_common and arndale don't boot the cores?
> > > > > >
> > > > >
> > > > > Yes, they don't doing the release operation.
> > > > >
> > > > > For Low Power Management requirement, maybe only one core will be
> > > > > used,
> > and
> > > > then
> > > > > We also make sure that the secondary core must be in low power and
> > > > > deep
> > > > sleep
> > > > > mode(using wfi). So I just release it here, to make sure that the wfi
> > > > instruction
> > > > > will be executed as early as possible.
> > > >
> > > > Right after smp_set_core_boot_addr() is called, kick_all_cpus() isgoing
> > > > to be called. Wouldn't that boot your CPUs just as well?
> > > >
> > >
> > > Yes, it will.
> > >
> > > But before that we must do the holdoff bit set operation as the SoC's
> > requirement.
> > >
> > > The BRR contains control bits for enabling boot for each core. On exiting
> > HRESET or
> > > PORESET, the RCW BOOT_HO field optionally allows for logical core 0 to be
> > released
> > > for booting or to remain in boot holdoff. All other cores remain in boot
> > holdoff until their
> > > corresponding bit is set.
> > >
> > > Maybe the comment is not very clear and a bit confusing.
> >
> > Before I'm lost entirely, do you mean that the comment:
> >
> > > > > > > + /*
> > > > > > > + * After setting the secondary cores start address,
> > > > > > > + * just release them to boot.
> > > > > > > + */
> >
> > Is actually wrong, and the instructions that follow it do not actually
> > boot the secondary core(s)?
> >
>
> The comment should be:
> /*
> * After setting the secondary core's start address,
> * just release it from holdoff.
> */
> From my tests, for most time the release instructions will boot the secondary
> core(s) without smp_kick_all_cpus(). One time has failed.
>
> So I think the release can not make sure that it will boot the secondary
> core(s).
Thanks for clarifying.
If a holdoff release is the right way to boot a secondary core for you,
then I think the right place to do it is not smp_set_core_boot_addr()
but smp_kick_all_cpus(), of which you could make a strong version which
would do the holdoff release instead of whatever the weak version does.
That is, unless you also need the weak version to run, in which case
I /still/ think you should find a way for all of this to happen at the
'kick" stage (maybe with a .weakref in arch/arm/cpu/armv8/start.S, to
give the weak smp_kick_all_cpus symbol another name that could be
referred to by the strong version).
> Thanks,
>
> BRs
> Xiubo
Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
