Re: [PATCH v2 1/2] firmware: add nowarn variant of request_firmware_nowait()

From: Luis Chamberlain
Date: Mon Nov 19 2018 - 15:07:15 EST


On Mon, Nov 12, 2018 at 05:01:42PM +0100, Lucas Stach wrote:
> Device drivers with optional firmware may still want to use the
> asynchronous firmware loading interface. To avoid printing a
> warining into the kernel log when the optional firmware is
> absent, add a nowarn variant of this interface.
>
> Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>

Thanks for the patch Lucas!

> +EXPORT_SYMBOL(request_firmware_nowait_nowarn);

New symbols should use firmware_* prefix, so use:

* firmware_request_nowait_nowarn()

Also, please make new functionality EXPORT_SYMBOL_GPL(), the old functioanlity
must be kept as-is, so in this caseEXPORT_SYMBOL().

Other than this, you should be aware that there has been significant
discussion over how to properly evolve the API of the firmware API since
last year, you may want to read those threads. The short and skinny of
it though is that the firmware API has two main diverging modes of
operation:

o async
o sync

The async functionality diverges from the synchronous functionality in
that it is data driven. The synchronous functionality is functional, and
experience shows that while data driven can avoid collateral evolutions
we *don't prefer it in the kernel*. So we should break down the async
API to match the sync functional design.

Internally we can use flags for small modifications, as we use them now,
but since we don't expose flags for the sync case lets try to keep
parity for this API then.

A good example of what we need to do. The uevent flag is only set to
false by only two drivers:

o CONFIG_LEDS_LP55XX_COMMON
o CONFIG_DELL_RBU

As such, this functionality should just be wrapped into its own single
functional call eventually.

The conversion of the async API to functional does not need to happen
for your changes, but new async API should follow the functional driven
approach. So please make your call work as functional.

Please let me know if this makes sense or if you have any quetsions!

[0] https://lore.kernel.org/lkml/20180628031332.GE21242@xxxxxxxxxxxxx/T/#u
[1] https://lkml.kernel.org/r/20180421173650.GW14440@xxxxxxxxxxxxx
[2] https://lkml.kernel.org/r/20180422202609.GX14440@xxxxxxxxxxxxx

Luis