[U-Boot] [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards

[Albert ARIBAUD <albert.u.boot@aribaud.net>]

Hello Li.Xiubo at freescale.com,

On Mon, 17 Nov 2014 02:16:11 +0000, Li.Xiubo at freescale.com
<Li.Xiubo@freescale.com> wrote:
> Hi Albert,
> 
> 
> > -----Original Message-----
> > From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
> > Sent: Friday, November 14, 2014 7:45 PM
> > To: Xiubo Li-B47053
> > Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot at lists.denx.de
> > Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards
> > 
> > Hello Li.Xiubo at freescale.com,
> > 
> > On Fri, 14 Nov 2014 09:06:13 +0000, Li.Xiubo at freescale.com
> > <Li.Xiubo@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)?

> Thanks,
> 
> BRs
> Xiubo

Amicalement,
-- 
Albert.