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.b...@aribaud.net] > Sent: Thursday, November 20, 2014 8:07 PM > To: Xiubo Li-B47053 > Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot@lists.denx.de > Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa boards > > Hello li.xi...@freescale.com, > > On Wed, 19 Nov 2014 07:21:26 +0000, li.xi...@freescale.com > <li.xi...@freescale.com> wrote: > > Hi Albert, > > > > > -----Original Message----- > > > From: Albert ARIBAUD [mailto:albert.u.b...@aribaud.net] > > > Sent: Tuesday, November 18, 2014 3:18 PM > > > To: Xiubo Li-B47053 > > > Cc: Sun York-R58495; Jin Zhengxiong-R64188; u-boot@lists.denx.de > > > Subject: Re: [PATCH v3 3/5] ls102xa: HYP/non-sec: support for ls102xa > boards > > > > > > 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. > > > > > > > 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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
