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

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

[Lucas Stach]

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@pengutronix.de>
---
v2: Add missing documentation, as pointed out by 0-day bot.
---
 drivers/base/firmware_loader/main.c | 92 ++++++++++++++++++++---------
 include/linux/firmware.h            | 12 ++++
 2 files changed, 77 insertions(+), 27 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 8e9213b36e31..9fa4ee154147 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -790,34 +790,11 @@ static void request_firmware_work_func(struct work_struct *work)
 	kfree(fw_work);
 }
 
-/**
- * request_firmware_nowait() - asynchronous version of request_firmware
- * @module: module requesting the firmware
- * @uevent: sends uevent to copy the firmware image if this flag
- *	is non-zero else the firmware copy must be done manually.
- * @name: name of firmware file
- * @device: device for which firmware is being loaded
- * @gfp: allocation flags
- * @context: will be passed over to @cont, and
- *	@fw may be %NULL if firmware request fails.
- * @cont: function will be called asynchronously when the firmware
- *	request is over.
- *
- *	Caller must hold the reference count of @device.
- *
- *	Asynchronous variant of request_firmware() for user contexts:
- *		- sleep for as small periods as possible since it may
- *		  increase kernel boot time of built-in device drivers
- *		  requesting firmware in their ->probe() methods, if
- *		  @gfp is GFP_KERNEL.
- *
- *		- can't sleep at all if @gfp is GFP_ATOMIC.
- **/
-int
-request_firmware_nowait(
+
+static int _request_firmware_nowait(
 	struct module *module, bool uevent,
 	const char *name, struct device *device, gfp_t gfp, void *context,
-	void (*cont)(const struct firmware *fw, void *context))
+	void (*cont)(const struct firmware *fw, void *context), bool nowarn)
 {
 	struct firmware_work *fw_work;
 
@@ -835,7 +812,8 @@ request_firmware_nowait(
 	fw_work->context = context;
 	fw_work->cont = cont;
 	fw_work->opt_flags = FW_OPT_NOWAIT |
-		(uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER);
+		(uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER) |
+		(nowarn ? FW_OPT_NO_WARN : 0);
 
 	if (!uevent && fw_cache_is_setup(device, name)) {
 		kfree_const(fw_work->name);
@@ -854,8 +832,68 @@ request_firmware_nowait(
 	schedule_work(&fw_work->work);
 	return 0;
 }
+
+/**
+ * request_firmware_nowait() - asynchronous version of request_firmware
+ * @module: module requesting the firmware
+ * @uevent: sends uevent to copy the firmware image if this flag
+ *	is non-zero else the firmware copy must be done manually.
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded
+ * @gfp: allocation flags
+ * @context: will be passed over to @cont, and
+ *	@fw may be %NULL if firmware request fails.
+ * @cont: function will be called asynchronously when the firmware
+ *	request is over.
+ *
+ *	Caller must hold the reference count of @device.
+ *
+ *	Asynchronous variant of request_firmware() for user contexts:
+ *		- sleep for as small periods as possible since it may
+ *		  increase kernel boot time of built-in device drivers
+ *		  requesting firmware in their ->probe() methods, if
+ *		  @gfp is GFP_KERNEL.
+ *
+ *		- can't sleep at all if @gfp is GFP_ATOMIC.
+ **/
+int request_firmware_nowait(
+	struct module *module, bool uevent,
+	const char *name, struct device *device, gfp_t gfp, void *context,
+	void (*cont)(const struct firmware *fw, void *context))
+{
+	return _request_firmware_nowait(module, uevent, name, device, gfp,
+					context, cont, false);
+
+}
 EXPORT_SYMBOL(request_firmware_nowait);
 
+/**
+ * request_firmware_nowait_nowarn() - async version of request_firmware_nowarn
+ * @module: module requesting the firmware
+ * @uevent: sends uevent to copy the firmware image if this flag
+ *	is non-zero else the firmware copy must be done manually.
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded
+ * @gfp: allocation flags
+ * @context: will be passed over to @cont, and
+ *	@fw may be %NULL if firmware request fails.
+ * @cont: function will be called asynchronously when the firmware
+ *	request is over.
+ *
+ * Similar in function to request_firmware_nowait(), but doesn't print a warning
+ * when the firmware file could not be found.
+ */
+int request_firmware_nowait_nowarn(
+	struct module *module, bool uevent,
+	const char *name, struct device *device, gfp_t gfp, void *context,
+	void (*cont)(const struct firmware *fw, void *context))
+{
+	return _request_firmware_nowait(module, uevent, name, device, gfp,
+					context, cont, true);
+
+}
+EXPORT_SYMBOL(request_firmware_nowait_nowarn);
+
 #ifdef CONFIG_PM_SLEEP
 static ASYNC_DOMAIN_EXCLUSIVE(fw_cache_domain);
 
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index 2dd566c91d44..a6d0bc8273a4 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -48,6 +48,10 @@ int request_firmware_nowait(
 	struct module *module, bool uevent,
 	const char *name, struct device *device, gfp_t gfp, void *context,
 	void (*cont)(const struct firmware *fw, void *context));
+int request_firmware_nowait_nowarn(
+	struct module *module, bool uevent,
+	const char *name, struct device *device, gfp_t gfp, void *context,
+	void (*cont)(const struct firmware *fw, void *context));
 int request_firmware_direct(const struct firmware **fw, const char *name,
 			    struct device *device);
 int request_firmware_into_buf(const struct firmware **firmware_p,
@@ -77,6 +81,14 @@ static inline int request_firmware_nowait(
 	return -EINVAL;
 }
 
+static inline int request_firmware_nowait_nowarn(
+	struct module *module, bool uevent,
+	const char *name, struct device *device, gfp_t gfp, void *context,
+	void (*cont)(const struct firmware *fw, void *context))
+{
+	return -EINVAL;
+}
+
 static inline void release_firmware(const struct firmware *fw)
 {
 }
-- 
2.19.1

[PATCH v2 2/2] dmaengine: imx-sdma: don't print warning when firmware is absent

[Lucas Stach]

The SDMA firmware is optional and a usable fallabck to the internal
ROM firmware is present in the driver. There is already a message
printed informing the user that the internal firmware is used, so
there is no need to print a scary warning for what is normal operation
on most systems.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/dma/imx-sdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index b4ec2d20e661..c2e7819cecb8 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1784,7 +1784,7 @@ static int sdma_get_firmware(struct sdma_engine *sdma,
 {
 	int ret;
 
-	ret = request_firmware_nowait(THIS_MODULE,
+	ret = request_firmware_nowait_nowarn(THIS_MODULE,
 			FW_ACTION_HOTPLUG, fw_name, sdma->dev,
 			GFP_KERNEL, sdma, sdma_load_firmware);
 
-- 
2.19.1

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

[Luis Chamberlain]

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@pengutronix.de>

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@wotan.suse.de/T/#u
[1] https://lkml.kernel.org/r/20180421173650.GW14440@wotan.suse.de              
[2] https://lkml.kernel.org/r/20180422202609.GX14440@wotan.suse.de 

  Luis