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

[Florian Fainelli <f.fainelli@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