[U-Boot] [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards
Li.Xiubo at freescale.com
Li.Xiubo at freescale.com
Fri Nov 21 02:55:29 CET 2014
Hi Albert,
If there hasn't any other problem, I will send out the V4 series.
Thanks very much,
BRs
Xiubo
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
> Sent: Thursday, November 20, 2014 8:07 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 Wed, 19 Nov 2014 07:21:26 +0000, Li.Xiubo at freescale.com
> <Li.Xiubo at freescale.com> wrote:
> > Hi Albert,
> >
> > > -----Original Message-----
> > > From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
> > > Sent: Tuesday, November 18, 2014 3:18 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 Tue, 18 Nov 2014 02:01:02 +0000, Li.Xiubo at freescale.com
> > > <Li.Xiubo at 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.
> > >
> >
> > Yes, I do think a strong version will be okay.
> >
> > In file arch/arm/cpu/armv7/ls102xa/cpu.c, add the strong version:
> >
> > +/* Release the secondary core from holdoff state and boot it */
> > +void smp_kick_all_cpus(void)
> > +{
> > + struct ccsr_gur __iomem *gur = (void *)(CONFIG_SYS_FSL_GUTS_ADDR);
> > +
> > + out_be32(&gur->brrl, 0x2);
> > +}
> > +
> > Is this okay ?
>
> Yes, thanks!
>
> > I have test the holdoff release in two boards(including the old one before
> > I used) for 37 times and all has passed. I have a check the before failed
> logs,
> > It is another issue led to the failure. And also get confirmation that the
> > Holdoff release will do reset and then boot the secondary core.
>
> Good -- this makes smp_kick_all_cpus() the right home for holdoff
> releast.
>
> > Thanks,
>
> Thank you for your patience. :)
>
> > BRs
> > Xiubo
>
> Amicalement,
> --
> Albert.
More information about the U-Boot
mailing list