[U-Boot] [PATCH v1 08/16] arm: socfpga: stratix10: Add mailbox support for Stratix10 SoC

Ley Foon Tan
2018-05-11T05:45:13Z

On Thu, May 10, 2018 at 6:09 PM, Marek Vasut <marex@denx.de> wrote:
> On 05/10/2018 10:45 AM, Ley Foon Tan wrote:
>> On Tue, May 8, 2018 at 5:21 PM, Marek Vasut <marex@denx.de> wrote:
>>> On 05/08/2018 08:49 AM, Ley Foon Tan wrote:
>>>> On Thu, Apr 19, 2018 at 10:53 AM, Marek Vasut <marex@denx.de> wrote:
>>>>> On 04/19/2018 11:50 AM, Ley Foon Tan wrote:
>>>>>> Add mailbox support for Stratix SoC
>>>>>>
>>>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>>>>> Signed-off-by: Chin Liang See <chin.liang.see@intel.com>
>>>>>> ---
>>>
>>> [...]
>>>
>>>>>> +static __always_inline int mbox_polling_resp(u32 rout)
>>>>>> +{
>>>>>> + static const struct socfpga_mailbox *mbox_base =
>>>>>> + (void *)SOCFPGA_MAILBOX_ADDRESS;
>>>>>> + u32 rin;
>>>>>> + unsigned long i = ~0;
>>>>>> +
>>>>>> + while (i) {
>>>>>> + rin = readl(&mbox_base->rin);
>>>>>> + if (rout != rin)
>>>>>> + return 0;
>>>>>
>>>>> This looks like include/wait_bit.h reimplementation
>>>> The reason we use this method instead of using wait_bit because this
>>>> function will use in Secure Monitor Call (SMC) call as well.
>>>> When run in secure section, timer is not available. So, we can't use
>>>> wait_for_bit.
>>>
>>> Can't you just do timer_init() then ?
>> We can't use timer in secure region. This SMC code is resident in
>> onchip memory (secure region) while Linux is running.
>> So, timer might be used by Linux.
>
> So pieces of U-Boot remain in memory and get called by Linux, right ?
Normal U-boot (in DDR) is destroyed after Linux is running, only SMC
code Onchip memory will remain.
The existing U-boot have support this SMC feature, platform just need
to configure the CONFIG_ARMV8_SECURE_BASE for the secure section.
Of course, we need to enable CONFIG_ARMV8_PSCI too.

>
> I am CCing a few more people , since this might need further discussion.
>
> btw does this work when Linux is compiled in AArch32 mode while U-Boot
> is AArch64 ? I think it might not.
It should work. PSCI handle checks ESR_EL3.EC field to know whether it
is SMC32 or SMC64.
See code in handle_sync near line 230 in
http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv8/psci.S

>
>>>>>> +/* Send command only without waiting for responses from SDM */
>>>>>> +static __always_inline int __mbox_send_cmd_only(u8 id, u32 cmd,
>>>>>> + u8 is_indirect, u32 len,
>>>>>> + u32 *arg)
>>>>>> +{
>>>>>> + int ret = mbox_prepare_cmd_only(id, cmd, is_indirect, len, arg);
>>>>>> + /* write doorbell */
>>>>>> + writel(1, MBOX_DOORBELL_TO_SDM_REG);
>>>>>> +
>>>>>> + return ret;
>>>>>> +}
>>>>>> +
>>>>>> +/* Return number of responses received in buffer */
>>>>>> +static __always_inline int __mbox_rcv_resp(u32 *resp_buf, u32 resp_buf_max_len)
>>>>>
>>>>> __always_inline is nonsense, drop it. Let the compiler do it's thing.
>>>> This function used in SMC call as well, so it needs to be inline when
>>>> in secure section.
>>>
>>> Why ?
>> These functions needed for normal U-boot and SMC (secure section in
>> OCM) and we need to copy the whole function to secure section, not
>> just the API call.
>> This also avoid code duplication for U-boot and SMC.
>
> Thanks for the explanation!
>
> --

Regards
Ley Foon