[PATCH] firmware: Minor optimization in _scratch_init()

Atish Patra atishp at atishpatra.org
Fri Jul 9 09:50:36 PDT 2021


On Thu, Jul 8, 2021 at 6:35 AM Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Before entering _scratch_init(), register t3 already holds a copy
> of the firmware end address, hence there is no need to calculate
> it again. This reduces 3 instructions in each _scratch_init() loop.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
>  firmware/fw_base.S | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index a5ce946..e35f18e 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -287,10 +287,7 @@ _scratch_init:
>         /* Initialize scratch space */
>         /* Store fw_start and fw_size in scratch space */
>         lla     a4, _fw_start
> -       lla     a5, _fw_end
> -       mul     t0, s7, s8
> -       add     a5, a5, t0
> -       sub     a5, a5, a4
> +       sub     a5, t3, a4

It saves 3 instructions during boot but uses a temporary register (t3)
which was not computed in this block.
The first instruction of the loop also uses t3 to save tp. Can you add
a comment in the beginning of the loop
saying that all the registers that are critical for _scratch_init loop ?

This will remind the future developers that they shouldn't touch these
register inside the loop.


>         REG_S   a4, SBI_SCRATCH_FW_START_OFFSET(tp)
>         REG_S   a5, SBI_SCRATCH_FW_SIZE_OFFSET(tp)
>         /* Store next arg1 in scratch space */
> --
> 2.25.1
>
>
> --
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi

Otherwise, looks good to me.

Reviewed-by: Atish Patra <atish.patra at wdc.com>

--
Regards,
Atish



More information about the opensbi mailing list