Re: [PATCH RESEND v2 3/4] ARM: BCM: Add SMP support for Broadcom NSP

From: Florian Fainelli <f.fainelli@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>,
	Kapil Hali <kapilh@broadcom.com>,
	Rob Herring <robh+dt@kernel.org>, Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	Russell King <linux@arm.linux.org.uk>,
	Ray Jui <rjui@broadcom.com>,
	Scott Branden <sbranden@broadcom.com>,
	Jon Mason <jonmason@broadcom.com>
Cc: Gregory Fong <gregory.0xf0@gmail.com>, Lee Jones <lee@kernel.org>,
	Hauke Mehrtens <hauke@hauke-m.de>,
	Heiko Stuebner <heiko@sntech.de>,
	Kever Yang <kever.yang@rock-chips.com>,
	Maxime Ripard <maxime.ripard@free-electrons.com>,
	Olof Johansson <olof@lixom.net>, Paul Walmsley <paul@pwsan.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Chen-Yu Tsai <wens@csie.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	bcm-kernel-feedback-list@broadcom.com
Subject: Re: [PATCH RESEND v2 3/4] ARM: BCM: Add SMP support for Broadcom NSP
Date: Fri, 06 Nov 2015 12:03:49 -0800	[thread overview]
Message-ID: <563D07A5.8090305@gmail.com> (raw)
In-Reply-To: <563D060D.6070606@gmail.com>

On 06/11/15 11:57, Florian Fainelli wrote:
> On 06/11/15 11:49, Kapil Hali wrote:
>> Add SMP support for Broadcom's Northstar Plus SoC
>> cpu enable method. This changes also consolidates
>> iProc family's - BCM NSP and BCM Kona, platform
>> SMP handling in a common file.
>>
>> Northstar Plus SoC is based on ARM Cortex-A9
>> revision r3p0 which requires configuration for ARM
>> Errata 764369 for SMP. This change adds the needed
>> configuration option.
>>
>> Signed-off-by: Kapil Hali <kapilh@broadcom.com>
>> ---
> 
> Technically, this is not quite a RESEND, using the same git format-patch
> --subject command as before maybe?
> 
> [snip]
> 
>> +#ifndef __BCM_NSP_H
>> +#define __BCM_NSP_H
>> +
>> +extern void nsp_secondary_startup(void);
> 
> This does not appear to be needed anymore since you use the standard
> secondary_boot entry point now.
> 
>> +
>> +#endif /* __BCM_NSP_H */
>> diff --git a/arch/arm/mach-bcm/kona_smp.c b/arch/arm/mach-bcm/platsmp.c
>> similarity index 75%
>> rename from arch/arm/mach-bcm/kona_smp.c
>> rename to arch/arm/mach-bcm/platsmp.c
>> index 66a0465..925402f 100644
>> --- a/arch/arm/mach-bcm/kona_smp.c
>> +++ b/arch/arm/mach-bcm/platsmp.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Copyright (C) 2014 Broadcom Corporation
>> + * Copyright (C) 2014-2015 Broadcom Corporation
>>   * Copyright 2014 Linaro Limited
>>   *
>>   * This program is free software; you can redistribute it and/or
>> @@ -12,16 +12,23 @@
>>   * GNU General Public License for more details.
>>   */
>>  
>> -#include <linux/init.h>
>> +#include <linux/cpumask.h>
>> +#include <linux/delay.h>
>>  #include <linux/errno.h>
>> +#include <linux/init.h>
>>  #include <linux/io.h>
>> +#include <linux/jiffies.h>
>>  #include <linux/of.h>
>>  #include <linux/sched.h>
>> +#include <linux/smp.h>
>>  
>> +#include <asm/cacheflush.h>
>>  #include <asm/smp.h>
>>  #include <asm/smp_plat.h>
>>  #include <asm/smp_scu.h>
>>  
>> +#include "bcm_nsp.h"
> 
> Likewise.
> 
>> +
>>  /* Size of mapped Cortex A9 SCU address space */
>>  #define CORTEX_A9_SCU_SIZE	0x58
>>  
>> @@ -75,6 +82,37 @@ static int __init scu_a9_enable(void)
>>  	return 0;
>>  }
>>  
>> +static int nsp_write_lut(void)
>> +{
>> +	void __iomem *sku_rom_lut;
>> +	phys_addr_t secondary_startup_phy;
>> +
>> +	if (!secondary_boot) {
>> +		pr_warn("required secondary boot register not specified\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	sku_rom_lut = ioremap_nocache((phys_addr_t)secondary_boot,
>> +						sizeof(secondary_boot));
> 
> That looks weird to me, are not you intending to get a virtual mapping
> of the SKU ROM LUT base register address here? What would
> sizeof(function) return here?

secondary_boot != secondary_startup, I read it wrong, this is fine.
-- 
Florian