Subject: Re: [PATCH 1/2] ARM: oxnas: Add OX820 SMP support
To: Arnd Bergmann <arnd@arndb.de>, linux-arm-kernel@lists.infradead.org
References: <20161017084303.20078-1-narmstrong@baylibre.com>
 <20161017084303.20078-2-narmstrong@baylibre.com>
 <14513531.isB50DxNJf@wuerfel>
Cc: linux-kernel@vger.kernel.org, linux-oxnas@lists.tuxfamily.org
From: Neil Armstrong <narmstrong@baylibre.com>
Organization: Baylibre
Message-ID: <9124cda0-d16b-0992-2552-8a617aeb5d40@baylibre.com>
Date: Mon, 17 Oct 2016 11:34:32 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.3.0
MIME-Version: 1.0
In-Reply-To: <14513531.isB50DxNJf@wuerfel>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1644
Lines: 51

On 10/17/2016 11:06 AM, Arnd Bergmann wrote:
> On Monday, October 17, 2016 10:43:02 AM CEST Neil Armstrong wrote:
>> +
>> +       /*
>> +        * This is really belt and braces; we hold unintended secondary
>> +        * CPUs in the holding pen until we're ready for them.  However,
>> +        * since we haven't sent them a soft interrupt, they shouldn't
>> +        * be there.
>> +        */
>> +       write_pen_release(cpu);
>> +
>> +       /*
>> +        * Enable GIC cpu interface in CPU Interface Control Register
>> +        */
>> +       writel(GIC_CPU_CTRL_ENABLE,
>> +               gic_cpu_ctrl + GIC_NCPU_OFFSET(cpu) + GIC_CPU_CTRL);
>> +
>> +       /*
>> +        * Send the secondary CPU a soft interrupt, thereby causing
>> +        * the boot monitor to read the system wide flags register,
>> +        * and branch to the address found there.
>> +        */
>> +
>> +       arch_send_wakeup_ipi_mask(cpumask_of(cpu));
>> +       timeout = jiffies + (1 * HZ);
>> +       while (time_before(jiffies, timeout)) {
>> +               smp_rmb();
>> +               if (read_pen_release() == -1)
>> +                       break;
>> +
>> +               udelay(10);
>> +       }
>>
> 
> This seems to have been copied from plat-versatile, but is really
> not needed here since you apparently have proper hardware support for
> starting up the CPUs.
Yes it seems.

> 
> Any reason you can't just write to the cpu_ctrl register
> once and keep going without that whole holding_pen loop
> and spinlock?
I suppose but I did not find any good examples except the plat-versatile code.
I will try some simpler code.

> 
> 	Arnd
> 

Neil