[PATCH v4 6/9] arm: Get bloblist from boot arguments

[Michal Simek]

Hi Raymond,

On 1/3/24 18:01, Raymond Mao wrote:
> Hi Michal,
> 
> On Wed, 3 Jan 2024 at 05:46, Michal Simek <michal.simek at amd.com 
> <mailto:michal.simek at amd.com>> wrote:
> 
> 
> 
>     On 1/2/24 23:12, Raymond Mao wrote:
>      > Add arch custom function to get bloblist from boot arguments.
>      > Check whether boot arguments aligns with the register conventions
>      > defined in FW Handoff spec v0.9.
>      > Add bloblist related options into qemu-arm default config.
>      >
>      > Signed-off-by: Raymond Mao <raymond.mao at linaro.org
>     <mailto:raymond.mao at linaro.org>>
>      > ---
>      > Changes in v2
>      > - Remove low level code for copying boot arguments.
>      > - Refactor board_fdt_blob_setup() and remove direct access of gd->bloblist.
>      > Changes in v3
>      > - Optimize board_bloblist_from_boot_arg().
>      > Changes in v4
>      > - Move the function as an Arm-arch library instead of a board-specific one.
>      >
>      >   arch/arm/lib/xferlist.c      | 38 ++++++++++++++++++++++++++++++++++++
>      >   configs/qemu_arm64_defconfig |  3 +++
>      >   3 files changed, 45 insertions(+)
>      >   create mode 100644 arch/arm/lib/xferlist.c
>      >
>      > diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
>      > index b1bcd37466..0023c55d0d 100644
>      > --- a/arch/arm/lib/Makefile
>      > +++ b/arch/arm/lib/Makefile
>      > @@ -85,6 +85,10 @@ obj-y      += psci-dt.o
>      >
>      >   obj-$(CONFIG_DEBUG_LL)      += debug.o
>      >
>      > +ifdef CONFIG_BLOBLIST
>      > +obj-$(CONFIG_OF_BOARD)  += xferlist.o
>      > +endif
>      > +
>      >   # For EABI conformant tool chains, provide eabi_compat()
>      >   ifneq (,$(findstring -mabi=aapcs-linux,$(PLATFORM_CPPFLAGS)))
>      >   extra-y     += eabi_compat.o
>      > diff --git a/arch/arm/lib/xferlist.c b/arch/arm/lib/xferlist.c
>      > new file mode 100644
>      > index 0000000000..163d20c11c
>      > --- /dev/null
>      > +++ b/arch/arm/lib/xferlist.c
>      > @@ -0,0 +1,38 @@
>      > +// SPDX-License-Identifier: GPL-2.0+
>      > +/*
>      > + * Copyright (C) 2023 Linaro Limited
>      > + * Author: Raymond Mao <raymond.mao at linaro.org
>     <mailto:raymond.mao at linaro.org>>
>      > + */
>      > +#include <linux/types.h>
>      > +#include <errno.h>
>      > +#include <bloblist.h>
>      > +
>      > +/*
>      > + * Boot parameters saved from start.S
>      > + * saved_args[0]: FDT base address
>      > + * saved_args[1]: Bloblist signature
>      > + * saved_args[2]: must be 0
>      > + * saved_args[3]: Bloblist base address
>      > + */
>      > +extern unsigned long saved_args[];
> 
>     you should put this to bloblist.h. I would say it should be the part of 4/9
>     where you define saved_args for armv7.
> 
> Not sure if I understand your point but this part is for Arm only (v7 and v8) 
> and should
> not be in bloblist.h. It works following the spec from Arm. Other arches should 
> implement
> their own way if they want to load the bloblist from the previous loader.
> 

I am just saying that you shouldn't really have extern in C files. If you need 
to have reference to it put it to .h file.
If this is generic or arch specific that's up to you.

Thanks,
Michal