Hello Maxime,
On 2024-07-09 13:09, Maxime Ripard wrote:
On Tue, Jul 09, 2024 at 12:10:51PM GMT, Dragan Simic wrote:
On 2024-07-09 11:10, Andy Yan wrote:
> At 2024-07-09 16:17:06, "Dragan Simic" <dsimic@xxxxxxxxxxx> wrote:
> > On 2024-07-08 09:46, Andy Yan wrote:
> > > At 2024-07-04 18:35:42, "Dragan Simic" <dsimic@xxxxxxxxxxx> wrote:
> > > > On 2024-07-04 04:10, Andy Yan wrote:
> > > > > At 2024-07-04 07:32:02, "Dragan Simic" <dsimic@xxxxxxxxxxx> wrote:
> > > > > > After the additional firmware-related module information was
> > > > > > introduced by
> > > > > > the commit c0677e41a47f ("drm/rockchip: cdn-dp-core: add
> > > > > > MODULE_FIRMWARE
> > > > > > macro"), there's no longer need for the
> > > > > > firmware-loading workarounds
> > > > > > whose
> > > > > > sole purpose was to prevent the missing firmware
> > > > > > blob in an initial
> > > > > > ramdisk
> > > > > > from causing driver initialization to fail. Thus, delete the
> > > > > > workarounds,
> > > > > > which removes a sizable chunk of redundant code.
> > > > >
> > > > > What would happen if there was no ramdisk? And the firmware is in
> > > > > rootfs ？
> > > > >
> > > > > For example： A buildroot based tiny embedded system。
> > > >
> > > > Good point, let me explain, please.
> > > >
> > > > In general, if a driver is built into the kernel, there
> > > > should also be
> > > > an initial ramdisk that contains the related firmware blobs, because
> > > > it's
> > > > unknown is the root filesystem available when the driver is probed.
> > > > If
> > > > a driver is built as a module and there's no initial ramdisk, having
> > > > the related firmware blobs on the root filesystem should be fine,
> > > > because
> > > > the firmware blobs and the kernel module become available at
> > > > the same
> > > > time, through the root filesystem. [1]
> > > >
> > > > Another option for a driver built statically into the kernel, when
> > > > there's
> > > > no initial ramdisk, is to build the required firmware blobs into the
> > > > kernel
> > > > image. [2] Of course, that's feasible only when a kernel image is
> > > > built
> > > > specificially for some device, because otherwise it would become too
> > > > large
> > > > because of too many drivers and their firmware blobs becoming
> > > > included,
> > > > but that seems to fit the Buildroot-based example.
> > > >
> > > > To sum it up, mechanisms already exist in the kernel for various
> > > > scenarios
> > > > when it comes to loading firmware blobs. Even if the deleted
> > > > workaround
> > > > attempts to solve some issue specific to some environment,
> > > > that isn't
> > > > the
> > > > right place or the right way for solving any issues of that kind.
> > > >
> > > > While preparing this patch, I even tried to find another
> > > > kernel driver
> > > > that
> > > > also implements some similar workarounds for firmware loading, to
> > > > justify
> > > > the existence of such workarounds and to possibly move them into the
> > > > kernel's
> > > > firmware-loading interface. Alas, I was unable to find such
> > > > workarounds
> > > > in
> > > > other drivers, which solidified my reasoning behind classifying the
> > > > removed
> > > > code as out-of-place and redundant.
> > >
> > > For some tiny embedded system，there is no such ramdisk，for example：
> > > a buildroot based rootfs，the buildroot only generate rootfs。
> > >
> > > And FYI， there are mainline drivers try to fix such issue by
> > > defer_probe，for example：
> > > smc_abc[0]
> > > There are also some other similar scenario in gpu driver{1}[2]
> > >
> > > [0]https://elixir.bootlin.com/linux/latest/source/drivers/tee/optee/smc_abi.c#L1518
> > > [1]https://patchwork.kernel.org/project/dri-devel/patch/20240109120604.603700-1-javierm@xxxxxxxxxx/
> > > [2]https://lore.kernel.org/dri-devel/87y1918psd.fsf@xxxxxxxxxxxx-host-address-is-not-set/T/
> >
> > Thanks for providing these examples.
> >
> > Before I continue thinking about the possible systemic solution,
> > could you please clarify the way Buildroot builds the kernel and
> > prepares the root filesystem? I'm not familiar with Buildroot,
> > but it seems to me that it builds the drivers statically into the
> > produced kernel image, while it places the related firmware blobs
> > into the produced root filesystem. Am I right there?
>
> in practice we can chose build the drivers statically into the kernel，
> we can also build it as a module。
> And in both case， the firmware blobs are put in rootfs。
> If the drivers is built as a module， the module will also put in
> rootfs，
> so its fine。
> But if a drivers is built into the kernel ，it maybe can't access the
> firmware blob
> before the rootfs is mounted.
> So we can see some drivers try to use DEFER_PROBE to fix this issue.
When Buildroot builds the drivers statically into the kernel image,
can it also be told to build the required firmware blobs into the
kernel image, for which there's already support in the kernel?
Of course, that would be feasible if only a small number of firmware
blobs would end up built into the kernel image, i.e. if the Buildroot
build would be tailored for a specific board.
IIRC, it can, but it's not really convenient from a legal point of
view.
Ah, makes sense. Very different licensing for the same file, etc.
Otherwise...
> > As I already wrote earlier, and as the above-linked discussions
> > conclude, solving these issues doesn't belong to any specific driver.
> > It should be resolved within the kernel's firmware loading mechanism
> > instead, and no driver should be specific in that regard.
>
> IT would be good if it can be resolved within the kernel's firmware
> loading mechanism.
... we'll need this as a systemic solution.
The general policy has been to put drivers that need a firmware as a
module, and just never build them statically.
I totally agree, but if Buildroot builds them statically and provides
no initial ramdisk, we need a better solution than having various
drivers
attempt to implement their own workarounds.
