Subject: Re: [PATCH v4 20/28] ARM: owl: Implement CPU enable-method for S500
To: Arnd Bergmann <arnd@arndb.de>,
        Thomas Liau <thomas.liau@actions-semi.com>
Cc: Linux ARM <linux-arm-kernel@lists.infradead.org>,
        mp-cs@actions-semi.com,
        =?UTF-8?B?5byg5Lic6aOO?= <zhangdf@actions-semi.com>,
        =?UTF-8?B?5YiY54Kc?= <liuwei@actions-semi.com>,
        =?UTF-8?B?5byg5aSp55uK?= <tyzhang@actions-semi.com>,
        96boards@ucrobotics.com, support@lemaker.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Russell King <linux@armlinux.org.uk>
References: <20170606005426.26446-1-afaerber@suse.de>
 <20170606005426.26446-21-afaerber@suse.de>
 <CAK8P3a1fLABzSc7kej6uw751uoZma1Qvpm7c3mkLB45rcBNQTA@mail.gmail.com>
 <CAK8P3a3Pg2rm8nSPUm_E61F7+zSr3gWV6sNqhC0Et3Zh2xPtmA@mail.gmail.com>
From: =?UTF-8?Q?Andreas_F=c3=a4rber?= <afaerber@suse.de>
Organization: SUSE Linux GmbH
Message-ID: <6d9d0d63-113e-b050-a54e-62746ebc6013@suse.de>
Date: Thu, 29 Jun 2017 17:22:28 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.0
MIME-Version: 1.0
In-Reply-To: <CAK8P3a3Pg2rm8nSPUm_E61F7+zSr3gWV6sNqhC0Et3Zh2xPtmA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1903
Lines: 57

Am 29.06.2017 um 17:07 schrieb Arnd Bergmann:
>>> +static int s500_smp_boot_secondary(unsigned int cpu, struct task_struct *idle)
>>> +{
>>> +       unsigned long timeout;
>>> +       int ret;
>>> +
>>> +       ret = s500_wakeup_secondary(cpu);
>>> +       if (ret)
>>> +               return ret;
>>> +
>>> +       udelay(10);
>>> +
>>> +       spin_lock(&boot_lock);
>>> +
>>> +       /*
>>> +        * The secondary processor is waiting to be released from
>>> +        * the holding pen - release it, then wait for it to flag
>>> +        * that it has been released by resetting pen_release.
>>> +        */
>>> +       write_pen_release(cpu_logical_map(cpu));
>>> +       smp_send_reschedule(cpu);
>>> +
>>> +       timeout = jiffies + (1 * HZ);
>>> +       while (time_before(jiffies, timeout)) {
>>> +               if (pen_release == -1)
>>> +                       break;
>>> +       }
>>> +
>>> +       writel(0, timer_base_addr + OWL_CPU1_ADDR + (cpu - 1) * 4);
>>> +       writel(0, timer_base_addr + OWL_CPU1_FLAG + (cpu - 1) * 4);
>>> +
>>> +       spin_unlock(&boot_lock);
>>> +
>>> +       return pen_release != -1 ? -ENOSYS : 0;
>>> +}
>>
>> This looks more complicated than necessary. Why do you need the holding
>> pen when you have a register to start up the CPU?
>>
> 
> It seems you missed my question here. Can you please follow up, and
> if possible send a patch to remove the pen_release logic that appears
> to be unnecessary here?

I do not have any documentation on these registers, only the downstream
code that I forward-ported here. If you tell me what you mean exactly, I
can do some testing and if it still works submit a patch to simplify it.

Comments from the so far quiet Actions Semi side would help, too.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)