On 11/03/24 10:34 am, Anwar, Md Danish wrote:
>
>
> On 3/7/2024 6:16 PM, Tom Rini wrote:
>> On Wed, Feb 28, 2024 at 05:36:45PM +0530, MD Danish Anwar wrote:
>>> Add APIs to set a firmware_name to a rproc and boot the rproc with the
>>
>>> same firmware.
>>>
>>> Clients can call rproc_set_firmware() API to set firmware_name for a rproc
>>> whereas rproc_boot() will load the firmware set by rproc_set_firmware() to
>>> a buffer by calling request_firmware_into_buf(). rproc_boot() will then
>>> load the firmware file to the remote processor and start the remote
>>> processor.
>>>
>>> Also include "fs-loader.h" and make remoteproc driver select FS_LOADER in
>>> Kconfig so that we can call request_firmware_into_buf() from remoteproc
>>> driver.
>>>
>>> Signed-off-by: MD Danish Anwar <danishan...@ti.com>
>>> Acked-by: Ravi Gunasekaran <r-gunaseka...@ti.com>
>>> Reviewed-by: Roger Quadros <rog...@kernel.org>
>>
>> This breaks building on am64x_evm_r5 am65x_evm_r5_usbdfu
>> am65x_evm_r5_usbmsc in next currently, thanks.
>>
> I will work on fixing this build error and re-spin the patch.
>
Hi Tom, Roger,
This patch adds "request_firmware_into_buf()" in the rproc driver. To
use this API, FS_LOADER is needed. So I am adding "select FS_LOADER" in
REMOTEPROC Kconfig option. As a result whenever REMOTEPROC is enabled,
FS_LOADER also gets enabled.
Now arch/arm/mach-k3/common.c [1] and arch/arm/mach-omap2/boot-common.c
[2] has a "load_firmware()" API which calls fs-loader APIs and they have
below if condition before calling fs-loader APIs.
if (!IS_ENABLED(CONFIG_FS_LOADER))
return 0;
Till now, CONFIG_FS_LOADER was not set as a result the load_firmware()
API in above mentioned files, was returning 0.
Now as this patch enables CONFIG_FS_LOADER, as a result the code after
the if check starts getting executed and it tries to look for
get_fs_loader() and other fs-loader APIs but this is done at SPL and at
this time FS_LOADER is not built yet as a result we see below error.
The if checks only checks for CONFIG_FS_LOADER but not for
CONFIG_SPL_FS_LOADER.
AR spl/boot/built-in.o
LD spl/u-boot-spl
arm-none-linux-gnueabihf-ld.bfd: arch/arm/mach-k3/common.o: in function
`load_firmware':
/home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:184: undefined
reference to `get_fs_loader'
arm-none-linux-gnueabihf-ld.bfd:
/home/danish/workspace/u-boot/arch/arm/mach-k3/common.c:185: undefined
reference to `request_firmware_into_buf'
make[2]: *** [/home/danish/workspace/u-boot/scripts/Makefile.spl:527:
spl/u-boot-spl] Error 1
make[1]: *** [/home/danish/workspace/u-boot/Makefile:2055:
spl/u-boot-spl] Error 2
make[1]: Leaving directory '/home/danish/uboot_images/am64x/r5'
make: *** [Makefile:177: sub-make] Error 2
This bug has always been there but as CONFIG_FS_LOADER was never
enabled, this build error was never seen as the load_firmware() API will
return 0 without calling fs-loader APIs.
Now that this patch enables CONFIG_FS_LOADER, the bug gets exposed and
build error is seen.
My opinion here would be, to check for CONFIG_IS_ENABLED(FS_LOADER)
instead of IS_ENABLED(CONFIG_FS_LOADER) as the former will check for the
appropriate config option (CONFIG_SPL_FS_LOADER / CONFIG_FS_LOADER)
based on the build stage.
I tested with the below diff and I don't see build errors with
am64x_evm_r5, am65x_evm_r5_usbdfu, am65x_evm_r5_usbmsc configs.
diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
index f411366778..6792ff7467 100644
--- a/arch/arm/mach-k3/common.c
+++ b/arch/arm/mach-k3/common.c
@@ -162,7 +162,7 @@ int load_firmware(char *name_fw, char
*name_loadaddr, u32 *loadaddr)
char *name = NULL;
int size = 0;
- if (!IS_ENABLED(CONFIG_FS_LOADER))
+ if (!CONFIG_IS_ENABLED(FS_LOADER))
return 0;
*loadaddr = 0;
diff --git a/arch/arm/mach-omap2/boot-common.c
b/arch/arm/mach-omap2/boot-common.c
index 57917da25c..aa0ab13d5f 100644
--- a/arch/arm/mach-omap2/boot-common.c
+++ b/arch/arm/mach-omap2/boot-common.c
@@ -190,7 +190,7 @@ int load_firmware(char *name_fw, u32 *loadaddr)
struct udevice *fsdev;
int size = 0;
- if (!IS_ENABLED(CONFIG_FS_LOADER))
+ if (!CONFIG_IS_ENABLED(FS_LOADER))
return 0;
if (!*loadaddr)
Tom, Roger, Please let me know if this looks ok.
If it's ok, I will post this diff as a separate patch and once that is
merged Tom can merge this patch or I can send a v7 if needed.
[1]
https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-k3/common.c#L159
[2]
https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-omap2/boot-common.c#L188
--
Thanks and Regards,
Danish
