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

Marek Vasut
2018-05-10T10:09:07Z

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 ?

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.

>>>>> +/* 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!

--
Best regards,
Marek Vasut