From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andre Przywara Subject: Re: [PATCH] xen/arm: midway: implement SMP Date: Tue, 19 Nov 2013 16:24:05 +0100 Message-ID: <528B8295.3080806@linaro.org> References: <1384781260-10429-1-git-send-email-julien.grall@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VinA5-0008VB-OH for xen-devel@lists.xenproject.org; Tue, 19 Nov 2013 15:24:22 +0000 Received: by mail-bk0-f51.google.com with SMTP id 6so1146671bkj.38 for ; Tue, 19 Nov 2013 07:24:19 -0800 (PST) In-Reply-To: <1384781260-10429-1-git-send-email-julien.grall@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: xen-devel@lists.xenproject.org, stefano.stabellini@eu.citrix.com, tim@xen.org, ian.campbell@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 11/18/2013 02:27 PM, Julien Grall wrote: > Signed-off-by: Julien Grall > --- > xen/arch/arm/platforms/midway.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/xen/arch/arm/platforms/midway.c b/xen/arch/arm/platforms/midway.c > index 399056b..3b9fcfc 100644 > --- a/xen/arch/arm/platforms/midway.c > +++ b/xen/arch/arm/platforms/midway.c > @@ -47,6 +47,27 @@ static uint32_t midway_quirks(void) > return PLATFORM_QUIRK_DOM0_MAPPING_11; > } > > +static int __init midway_cpu_up(int cpu) Wouldn't it make more sense to do this initialization in smp_init() instead of here per CPU? init_secondary() is fixed and thus we will not write different values for each core. I guess it does not really matter, I was just wondering whether this would be a saner approach (and I think your first version was this way, right?) Thanks, Andre. P.S. BTW: Any reason we instantiate cpu_up() for all platforms where they are actually NOPs? I think the platform code only calls a function if it is non-NULL, so we could just skip all of this: cpu_up() {return 0;} definition. > +{ > + void __iomem *pens; > + > + pens = ioremap_nocache(0, PAGE_SIZE); > + if ( !pens ) > + { > + dprintk(XENLOG_ERR, "Unable to map midway pens MMIO\n"); > + return -EFAULT; > + } > + > + printk("Set cpu pen %u to %"PRIpaddr" (%p)\n", > + cpu, __pa(init_secondary), init_secondary); > + > + writel(__pa(init_secondary), pens + 0x40 + cpu * 0x10); > + > + iounmap(pens); > + > + return 0; > +} > + > static const char * const midway_dt_compat[] __initconst = > { > "calxeda,ecx-2000", > @@ -57,6 +78,7 @@ PLATFORM_START(midway, "CALXEDA MIDWAY") > .compatible = midway_dt_compat, > .reset = midway_reset, > .quirks = midway_quirks, > + .cpu_up = midway_cpu_up, > PLATFORM_END > > /* >