Steven Price <steven.price@xxxxxxx> writes: Hello Steven, > On 13/04/2024 12:49, Andy Yan wrote: >> From: Andy Yan <andy.yan@xxxxxxxxxxxxxx> >> >> The firmware in the rootfs will not be accessible until we >> are in the SYSTEM_RUNNING state, so return EPROBE_DEFER until >> that point. >> This let the driver can load firmware when it is builtin. > > The usual solution is that the firmware should be placed in the > initrd/initramfs if the module is included there (or built-in). The same > issue was brought up regarding the powervr driver: > > https://lore.kernel.org/dri-devel/20240109120604.603700-1-javierm@xxxxxxxxxx/T/ > > I'm not sure if that ever actually reached a conclusion though. The > question was deferred to Greg KH but I didn't see Greg actually getting > involved in the thread. > Correct, there was not conclusion reached in that thread. >> Signed-off-by: Andy Yan <andy.yan@xxxxxxxxxxxxxx> >> --- >> >> drivers/gpu/drm/panthor/panthor_fw.c | 11 ++++++++++- >> 1 file changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c b/drivers/gpu/drm/panthor/panthor_fw.c >> index 33c87a59834e..25e375f8333c 100644 >> --- a/drivers/gpu/drm/panthor/panthor_fw.c >> +++ b/drivers/gpu/drm/panthor/panthor_fw.c >> @@ -1336,8 +1336,17 @@ int panthor_fw_init(struct panthor_device *ptdev) >> } >> >> ret = panthor_fw_load(ptdev); >> - if (ret) >> + if (ret) { >> + /* >> + * The firmware in the rootfs will not be accessible until we >> + * are in the SYSTEM_RUNNING state, so return EPROBE_DEFER until >> + * that point. >> + */ >> + if (system_state < SYSTEM_RUNNING) > > This should really only be in the case where ret == -ENOENT - any other > error and the firmware is apparently present but broken in some way, so > there's no point deferring. > > I also suspect we'd need some change in panthor_fw_load() to quieten > error messages if we're going to defer on this, in which case it might > make more sense to move this logic into that function. > In the thread you referenced I suggested to add that logic in request_firmware() (or add a new request_firmware_defer() helper function) that changes the request firmare behaviour to return -EPROBE_DEFER instead of -ENOENT. Since as you mentioned, this isn't specific to panthor and an issue that I also faced with the powervr driver. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
