Date:   Fri, 22 Oct 2021 10:15:05 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     bp@...e.de, akpm@...ux-foundation.org, josh@...htriplett.org,
        rishabhb@...eaurora.org, kubakici@...pl, maco@...roid.com,
        david.brown@...aro.org, bjorn.andersson@...aro.org,
        linux-wireless@...r.kernel.org, keescook@...omium.org,
        shuah@...nel.org, mfuzzey@...keon.com, zohar@...ux.vnet.ibm.com,
        dhowells@...hat.com, pali.rohar@...il.com, tiwai@...e.de,
        arend.vanspriel@...adcom.com, zajec5@...il.com, nbroeking@...com,
        broonie@...nel.org, dmitry.torokhov@...il.com, dwmw2@...radead.org,
        torvalds@...ux-foundation.org, Abhay_Salunke@...l.com,
        jewalt@...innovations.com, cantabile.desu@...il.com, ast@...com,
        andresx7@...il.com, dan.rue@...aro.org, brendanhiggins@...gle.com,
        yzaikin@...gle.com, sfr@...b.auug.org.au, rdunlap@...radead.org,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2 08/10] firmware_loader: move builtin build helper to
 shared library

On Fri, Oct 22, 2021 at 02:13:38PM +0200, Greg KH wrote:
> On Thu, Oct 21, 2021 at 08:58:41AM -0700, Luis R. Rodriguez wrote:
> > From: Luis Chamberlain <mcgrof@...nel.org>
> > 
> > If we wanted to use a different directory for building target
> > builtin firmware it is easier if we just have a shared library
> > Makefile, and each target directory can then just include it and
> > populate the respective needed variables. This reduces clutter,
> > makes things easier to read, and also most importantly allows
> > us to not have to try to magically adjust only one target
> > kconfig symbol for built-in firmware files. Trying to do this
> > can easily end up causing odd build issues if the user is not
> > careful.
> > 
> > As an example issue, if we are going to try to extend the
> > FW_LOADER_BUILTIN_FILES list and FW_LOADER_BUILTIN_DIR in case
> > of a new test firmware builtin support currently our only option
> > would be modify the defaults of each of these in case test firmware
> > builtin support was enabled. Defaults however won't augment a prior
> > setting, and so if FW_LOADER_BUILTIN_DIR="/lib/firmware" and you
> > and want this to be changed to something like
> > FW_LOADER_BUILTIN_DIR="drivers/base/firmware_loader/test-builtin"
> > the change will not take effect as a prior build already had it
> > set, and the build would fail. Trying to augment / append the
> > variables in the Makefile just makes this very difficult to
> > read.
> > 
> > Using a library let's us split up possible built-in targets so
> > that the user does not have to be involved. This will be used
> > in a subsequent patch which will add another user to this
> > built-in firmware library Makefile and demo how to use it outside
> > of the default FW_LOADER_BUILTIN_DIR and FW_LOADER_BUILTIN_FILES.
> > 
> > Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
> 
> I'm sorry, but I do not understand the need for this change at all.  You
> are now building this as a library, but what uses this library?  The
> patches after this series are just testing patches, to verify that the
> code previous in this series is working correctly, it should not depend
> on a new library that only the testing code requires, right?

The last patch adds support to test built-in firmware, but most kernels
will have and do want EXTRA_FIRMWARE="", and so there cannot be anything
that can be tested. And so we need aother two pair of kconfig symbols
which are independent to test built-in firmware. The reason for this is
explained in the commit log, if we try to augment the EXTRA_FIRMWARE
when enabling testing built-in firmware we can easily end up with odd
build issues.

So this patch moves the logic to enable us to re-use the same built-in
magic for two independent kconfig test symbols.

  Luis

Powered by blists - more mailing lists