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

Ley Foon Tan lftan.linux at gmail.com
Thu May 10 08:45:54 UTC 2018


On Tue, May 8, 2018 at 5:21 PM, Marek Vasut <marex at 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 at 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 at intel.com>
>>>> Signed-off-by: Chin Liang See <chin.liang.see at 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.

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

Regards
Ley Foon


More information about the U-Boot mailing list