Re: [PATCH] drm/panthor: Add defer probe for firmware load

[Javier Martinez Canillas <javierm@redhat.com>]

Steven Price <steven.price@arm.com> writes:

> Hi Javier,
>
> On 25/04/2024 10:22, Javier Martinez Canillas wrote:
>> Steven Price <steven.price@arm.com> writes:
>> 
>> Hello Steven,
>> 
>>> On 13/04/2024 12:49, Andy Yan wrote:
>>>> From: Andy Yan <andy.yan@rock-chips.com>
>>>>
>>>> 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@redhat.com/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.
>
> So I think we need a conclusion before we start applying point fixes to
> individual drivers.
>

Agreed.

[...]

>> 
>> 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.
>
> That would seem like a good feature if it's agreed that deferring on
> request_firmware is a good idea.
>

Yeah. I didn't attempt to type that patch because didn't get an answer
from Greg and didn't want to spent the time writing and testing a patch
to just be nacked.

>> Since as you mentioned, this isn't specific to panthor and an issue that I also
>> faced with the powervr driver.
>
> I'm not in any way against the idea of deferring the probe until the
> firmware is around - indeed it seems like a very sensible idea in many
> respects. But I don't want panthor to be 'special' in this way.
>
> If the consensus is that the firmware should live with the module (i.e.
> either both in the initramfs or both in the rootfs) then the code is
> fine as it is. That seemed to be the view of Sima in that thread and
> seems reasonable to me - why put the .ko in the initrd if you can't
> actually use it until the rootfs comes along?
>

That's indeed a sensible position for me as well and is what I answered to
the user who reported the powervr issue.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat