diff mbox series

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

Message ID 20181112160143.4459-1-l.stach@pengutronix.de (mailing list archive)
State Not Applicable
Headers show
Series [v2,1/2] firmware: add nowarn variant of request_firmware_nowait() | expand

Commit Message

Lucas Stach Nov. 12, 2018, 4:01 p.m. UTC 
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(-)

Comments

Luis Chamberlain Nov. 19, 2018, 8:07 p.m. UTC | #1 
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
diff mbox series

Patch

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)
 {
 }