Received-SPF: pass (google.com: domain of linux-kernel+bounces-158735-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
From: Javier Martinez Canillas <javierm@redhat.com>
To: Steven Price <steven.price@arm.com>, Andy Yan <andyshrk@163.com>,
 boris.brezillon@collabora.com
Cc: daniel@ffwll.ch, airlied@gmail.com, liviu.dudau@arm.com,
 maarten.lankhorst@linux.intel.com, tzimmermann@suse.de,
 dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Andy Yan
 <andy.yan@rock-chips.com>
Subject: Re: [PATCH] drm/panthor: Add defer probe for firmware load
In-Reply-To: <501d18c9-3116-4cd1-8091-b9f552e9fb5a@arm.com>
References: <20240413114938.740631-1-andyshrk@163.com>
 <54e4a174-dea7-4588-b8a6-0310c210ffce@arm.com>
 <87frv9zthu.fsf@minerva.mail-host-address-is-not-set>
 <501d18c9-3116-4cd1-8091-b9f552e9fb5a@arm.com>
Date: Thu, 25 Apr 2024 16:45:06 +0200
Message-ID: <87y1918psd.fsf@minerva.mail-host-address-is-not-set>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

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