Re: [PATCH v2 08/10] firmware_loader: move builtin build helper to shared library

[Luis Chamberlain]

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@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> 
> 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