linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6] firmware_loader: cleanups for v4.18
@ 2018-05-04 17:43 Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 1/6] firmware: wrap FW_OPT_* into an enum Luis R. Rodriguez
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
	kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, khc, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev,
	Luis R. Rodriguez

Greg,

I've reviewed the pending patches for the firmware_laoder and as for
v4.18, the following 3 patches from Andres have been iterated enough
that they're ready after I made some final minor changes, mostly just
style fixes and re-arrangements in terms of order. The new API he was
suggesting to add requires just a bit more review.

The last 3 patches are my own and are new, so I'd like further review
from others on them, but considering the changes Hans de Goede is having
us consider, I think this will prove useful to his work in terms of
splitting up code and documenting things properly. One thing that was
clear -- is our Kconfig entries for FW_LOADER were *extremely* outdated,
as such I've gone ahead and updated them.

The kconfig wording update for FW_LOADER includes prior conclusions made
to help justify keeping the split of the firmware fallback sysfs
interface in terms of size which was discussed with Josh Triplett a
while ago. It also includes modern recommendations, which would otherwise
get lost, on what to do about corner case firmware situations on
provisioning situations which folks have brought to my attention before
and architectural solutions based on firmwared [0] for a few years now.
Finally this work also reveals that a couple of candidate drivers could
likely move to staging considering their age, *or* we could just remove
the respective firmware build options. SCSI folks? Networking folks? To
my surprise *nothing* has been done about PREVENT_FIRMWARE_BUILD for
them since pre-git days!  These sneaky litte buggers are:

  * CONFIG_WANXL --> CONFIG_WANXL_BUILD_FIRMWARE
  * CONFIG_SCSI_AIC79XX --> CONFIG_AIC79XX_BUILD_FIRMWARE

To this day both of these drivers are building driver *firmwares* when
the option CONFIG_PREVENT_FIRMWARE_BUILD is disabled, and they don't
even make use of the firmware API at all. I find it *highly unlikely*
pre-git day drivers are raging in new radical firmware updates these
days. I'll go put a knife into some of that unless I hear back from
SCSI or networking folks that WANXL_BUILD_FIRMWARE and
AIC79XX_BUILD_FIRMWARE are still hip and very much needed.

On my radar as well are Mimi's latest firmware_loader proposed changes,
but I think those need considerable review and attention from more security
folks, Android folks, and the linux-wireless community, our own
scattered random folks of firmware reviewer folks.

These patches are based on top of linux-next next-20180504, they are
also available in a respective git branch, both for linux-next [1] and
linux [2].

Question, and specially rants are greatly appreciated, and of course...
may the 4th be with you.

[0] https://github.com/teg/firmwared
[1] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20180504-firmware_loader
[2] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=20180504-firmware_loader

  Luis

Andres Rodriguez (3):
  firmware: wrap FW_OPT_* into an enum
  firmware: use () to terminate kernel-doc function names
  firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs()

Luis R. Rodriguez (3):
  firmware_loader: document firmware_sysfs_fallback()
  firmware_loader: enhance Kconfig documentation over FW_LOADER
  firmware_loader: move kconfig FW_LOADER entries to its own file

 drivers/base/Kconfig                    |  90 +++-----------
 drivers/base/firmware_loader/Kconfig    | 149 ++++++++++++++++++++++++
 drivers/base/firmware_loader/fallback.c |  46 +++++---
 drivers/base/firmware_loader/fallback.h |  18 +--
 drivers/base/firmware_loader/firmware.h |  37 ++++--
 drivers/base/firmware_loader/main.c     |  28 ++---
 6 files changed, 252 insertions(+), 116 deletions(-)
 create mode 100644 drivers/base/firmware_loader/Kconfig

-- 
2.17.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v5 1/6] firmware: wrap FW_OPT_* into an enum
  2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
@ 2018-05-04 17:43 ` Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 2/6] firmware: use () to terminate kernel-doc function names Luis R. Rodriguez
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
	kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, khc, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev,
	Luis R . Rodriguez

From: Andres Rodriguez <andresx7@gmail.com>

This should let us associate enum kdoc to these values.
While at it, kdocify the fw_opt.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: coding style fixes, merge kdoc with enum move]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c | 12 ++++----
 drivers/base/firmware_loader/fallback.h |  6 ++--
 drivers/base/firmware_loader/firmware.h | 37 +++++++++++++++++++------
 drivers/base/firmware_loader/main.c     |  6 ++--
 4 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..b57a7b3b4122 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -512,7 +512,7 @@ static const struct attribute_group *fw_dev_attr_groups[] = {
 
 static struct fw_sysfs *
 fw_create_instance(struct firmware *firmware, const char *fw_name,
-		   struct device *device, unsigned int opt_flags)
+		   struct device *device, enum fw_opt opt_flags)
 {
 	struct fw_sysfs *fw_sysfs;
 	struct device *f_dev;
@@ -545,7 +545,7 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
  * In charge of constructing a sysfs fallback interface for firmware loading.
  **/
 static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
-				  unsigned int opt_flags, long timeout)
+				  enum fw_opt opt_flags, long timeout)
 {
 	int retval = 0;
 	struct device *f_dev = &fw_sysfs->dev;
@@ -599,7 +599,7 @@ static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
 
 static int fw_load_from_user_helper(struct firmware *firmware,
 				    const char *name, struct device *device,
-				    unsigned int opt_flags)
+				    enum fw_opt opt_flags)
 {
 	struct fw_sysfs *fw_sysfs;
 	long timeout;
@@ -640,7 +640,7 @@ static int fw_load_from_user_helper(struct firmware *firmware,
 	return ret;
 }
 
-static bool fw_force_sysfs_fallback(unsigned int opt_flags)
+static bool fw_force_sysfs_fallback(enum fw_opt opt_flags)
 {
 	if (fw_fallback_config.force_sysfs_fallback)
 		return true;
@@ -649,7 +649,7 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
 	return true;
 }
 
-static bool fw_run_sysfs_fallback(unsigned int opt_flags)
+static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
 {
 	if (fw_fallback_config.ignore_sysfs_fallback) {
 		pr_info_once("Ignoring firmware sysfs fallback due to sysctl knob\n");
@@ -664,7 +664,7 @@ static bool fw_run_sysfs_fallback(unsigned int opt_flags)
 
 int fw_sysfs_fallback(struct firmware *fw, const char *name,
 		      struct device *device,
-		      unsigned int opt_flags,
+		      enum fw_opt opt_flags,
 		      int ret)
 {
 	if (!fw_run_sysfs_fallback(opt_flags))
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index f8255670a663..a3b73a09db6c 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -5,6 +5,8 @@
 #include <linux/firmware.h>
 #include <linux/device.h>
 
+#include "firmware.h"
+
 /**
  * struct firmware_fallback_config - firmware fallback configuration settings
  *
@@ -31,7 +33,7 @@ struct firmware_fallback_config {
 #ifdef CONFIG_FW_LOADER_USER_HELPER
 int fw_sysfs_fallback(struct firmware *fw, const char *name,
 		      struct device *device,
-		      unsigned int opt_flags,
+		      enum fw_opt opt_flags,
 		      int ret);
 void kill_pending_fw_fallback_reqs(bool only_kill_custom);
 
@@ -43,7 +45,7 @@ void unregister_sysfs_loader(void);
 #else /* CONFIG_FW_LOADER_USER_HELPER */
 static inline int fw_sysfs_fallback(struct firmware *fw, const char *name,
 				    struct device *device,
-				    unsigned int opt_flags,
+				    enum fw_opt opt_flags,
 				    int ret)
 {
 	/* Keep carrying over the same error */
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 64acbb1a392c..4f433b447367 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -2,6 +2,7 @@
 #ifndef __FIRMWARE_LOADER_H
 #define __FIRMWARE_LOADER_H
 
+#include <linux/bitops.h>
 #include <linux/firmware.h>
 #include <linux/types.h>
 #include <linux/kref.h>
@@ -10,13 +11,33 @@
 
 #include <generated/utsrelease.h>
 
-/* firmware behavior options */
-#define FW_OPT_UEVENT			(1U << 0)
-#define FW_OPT_NOWAIT			(1U << 1)
-#define FW_OPT_USERHELPER		(1U << 2)
-#define FW_OPT_NO_WARN			(1U << 3)
-#define FW_OPT_NOCACHE			(1U << 4)
-#define FW_OPT_NOFALLBACK		(1U << 5)
+/**
+ * enum fw_opt - options to control firmware loading behaviour
+ *
+ * @FW_OPT_UEVENT: Enables the fallback mechanism to send a kobject uevent
+ *	when the firmware is not found. Userspace is in charge to load the
+ *	firmware using the sysfs loading facility.
+ * @FW_OPT_NOWAIT: Used to describe the firmware request is asynchronous.
+ * @FW_OPT_USERHELPER: Enable the fallback mechanism, in case the direct
+ *	filesystem lookup fails at finding the firmware.  For details refer to
+ *	fw_sysfs_fallback().
+ * @FW_OPT_NO_WARN: Quiet, avoid printing warning messages.
+ * @FW_OPT_NOCACHE: Disables firmware caching. Firmware caching is used to
+ *	cache the firmware upon suspend, so that upon resume races against the
+ *	firmware file lookup on storage is avoided. Used for calls where the
+ *	file may be too big, or where the driver takes charge of its own
+ *	firmware caching mechanism.
+ * @FW_OPT_NOFALLBACK: Disable the fallback mechanism. Takes precedence over
+ *	&FW_OPT_UEVENT and &FW_OPT_USERHELPER.
+ */
+enum fw_opt {
+	FW_OPT_UEVENT =         BIT(0),
+	FW_OPT_NOWAIT =         BIT(1),
+	FW_OPT_USERHELPER =     BIT(2),
+	FW_OPT_NO_WARN =        BIT(3),
+	FW_OPT_NOCACHE =        BIT(4),
+	FW_OPT_NOFALLBACK =     BIT(5),
+};
 
 enum fw_status {
 	FW_STATUS_UNKNOWN,
@@ -110,6 +131,6 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
 }
 
 int assign_fw(struct firmware *fw, struct device *device,
-	      unsigned int opt_flags);
+	      enum fw_opt opt_flags);
 
 #endif /* __FIRMWARE_LOADER_H */
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index eb34089e4299..9919f0e6a7cc 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -443,7 +443,7 @@ static int fw_add_devm_name(struct device *dev, const char *name)
 #endif
 
 int assign_fw(struct firmware *fw, struct device *device,
-	      unsigned int opt_flags)
+	      enum fw_opt opt_flags)
 {
 	struct fw_priv *fw_priv = fw->priv;
 	int ret;
@@ -558,7 +558,7 @@ static void fw_abort_batch_reqs(struct firmware *fw)
 static int
 _request_firmware(const struct firmware **firmware_p, const char *name,
 		  struct device *device, void *buf, size_t size,
-		  unsigned int opt_flags)
+		  enum fw_opt opt_flags)
 {
 	struct firmware *fw = NULL;
 	int ret;
@@ -734,7 +734,7 @@ struct firmware_work {
 	struct device *device;
 	void *context;
 	void (*cont)(const struct firmware *fw, void *context);
-	unsigned int opt_flags;
+	enum fw_opt opt_flags;
 };
 
 static void request_firmware_work_func(struct work_struct *work)
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 2/6] firmware: use () to terminate kernel-doc function names
  2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 1/6] firmware: wrap FW_OPT_* into an enum Luis R. Rodriguez
@ 2018-05-04 17:43 ` Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 3/6] firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs() Luis R. Rodriguez
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
	kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, khc, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev,
	Luis R . Rodriguez

From: Andres Rodriguez <andresx7@gmail.com>

The kernel-doc spec dictates a function name ends in ().

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c |  8 ++++----
 drivers/base/firmware_loader/main.c     | 20 ++++++++++----------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index b57a7b3b4122..529f7013616f 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -125,7 +125,7 @@ static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
 }
 
 /**
- * firmware_timeout_store - set number of seconds to wait for firmware
+ * firmware_timeout_store() - set number of seconds to wait for firmware
  * @class: device class pointer
  * @attr: device attribute pointer
  * @buf: buffer to scan for timeout value
@@ -239,7 +239,7 @@ static int map_fw_priv_pages(struct fw_priv *fw_priv)
 }
 
 /**
- * firmware_loading_store - set value in the 'loading' control file
+ * firmware_loading_store() - set value in the 'loading' control file
  * @dev: device pointer
  * @attr: device attribute pointer
  * @buf: buffer to scan for loading control value
@@ -431,7 +431,7 @@ static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
 }
 
 /**
- * firmware_data_write - write method for firmware
+ * firmware_data_write() - write method for firmware
  * @filp: open sysfs file
  * @kobj: kobject for the device
  * @bin_attr: bin_attr structure
@@ -537,7 +537,7 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 }
 
 /**
- * fw_load_sysfs_fallback - load a firmware via the sysfs fallback mechanism
+ * fw_load_sysfs_fallback() - load a firmware via the sysfs fallback mechanism
  * @fw_sysfs: firmware sysfs information for the firmware to load
  * @opt_flags: flags of options, FW_OPT_*
  * @timeout: timeout to wait for the load
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 9919f0e6a7cc..7f2bc7e8e3c0 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -597,7 +597,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 }
 
 /**
- * request_firmware: - send firmware request and wait for it
+ * request_firmware() - send firmware request and wait for it
  * @firmware_p: pointer to firmware image
  * @name: name of firmware file
  * @device: device for which firmware is being loaded
@@ -657,7 +657,7 @@ int request_firmware_direct(const struct firmware **firmware_p,
 EXPORT_SYMBOL_GPL(request_firmware_direct);
 
 /**
- * firmware_request_cache: - cache firmware for suspend so resume can use it
+ * firmware_request_cache() - cache firmware for suspend so resume can use it
  * @name: name of firmware file
  * @device: device for which firmware should be cached for
  *
@@ -681,7 +681,7 @@ int firmware_request_cache(struct device *device, const char *name)
 EXPORT_SYMBOL_GPL(firmware_request_cache);
 
 /**
- * request_firmware_into_buf - load firmware into a previously allocated buffer
+ * request_firmware_into_buf() - load firmware into a previously allocated buffer
  * @firmware_p: pointer to firmware image
  * @name: name of firmware file
  * @device: device for which firmware is being loaded and DMA region allocated
@@ -713,7 +713,7 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
 EXPORT_SYMBOL(request_firmware_into_buf);
 
 /**
- * release_firmware: - release the resource associated with a firmware image
+ * release_firmware() - release the resource associated with a firmware image
  * @fw: firmware resource to release
  **/
 void release_firmware(const struct firmware *fw)
@@ -755,7 +755,7 @@ static void request_firmware_work_func(struct work_struct *work)
 }
 
 /**
- * request_firmware_nowait - asynchronous version of request_firmware
+ * 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.
@@ -824,7 +824,7 @@ EXPORT_SYMBOL(request_firmware_nowait);
 static ASYNC_DOMAIN_EXCLUSIVE(fw_cache_domain);
 
 /**
- * cache_firmware - cache one firmware image in kernel memory space
+ * cache_firmware() - cache one firmware image in kernel memory space
  * @fw_name: the firmware image name
  *
  * Cache firmware in kernel memory so that drivers can use it when
@@ -866,7 +866,7 @@ static struct fw_priv *lookup_fw_priv(const char *fw_name)
 }
 
 /**
- * uncache_firmware - remove one cached firmware image
+ * uncache_firmware() - remove one cached firmware image
  * @fw_name: the firmware image name
  *
  * Uncache one firmware image which has been cached successfully
@@ -1042,7 +1042,7 @@ static void __device_uncache_fw_images(void)
 }
 
 /**
- * device_cache_fw_images - cache devices' firmware
+ * device_cache_fw_images() - cache devices' firmware
  *
  * If one device called request_firmware or its nowait version
  * successfully before, the firmware names are recored into the
@@ -1075,7 +1075,7 @@ static void device_cache_fw_images(void)
 }
 
 /**
- * device_uncache_fw_images - uncache devices' firmware
+ * device_uncache_fw_images() - uncache devices' firmware
  *
  * uncache all firmwares which have been cached successfully
  * by device_uncache_fw_images earlier
@@ -1092,7 +1092,7 @@ static void device_uncache_fw_images_work(struct work_struct *work)
 }
 
 /**
- * device_uncache_fw_images_delay - uncache devices firmwares
+ * device_uncache_fw_images_delay() - uncache devices firmwares
  * @delay: number of milliseconds to delay uncache device firmwares
  *
  * uncache all devices's firmwares which has been cached successfully
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 3/6] firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs()
  2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 1/6] firmware: wrap FW_OPT_* into an enum Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 2/6] firmware: use () to terminate kernel-doc function names Luis R. Rodriguez
@ 2018-05-04 17:43 ` Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 4/6] firmware_loader: document firmware_sysfs_fallback() Luis R. Rodriguez
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
	kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, khc, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev,
	Luis R . Rodriguez

From: Andres Rodriguez <andresx7@gmail.com>

This is done since this call is now exposed through kernel-doc,
and since this also paves the way for different future types of
fallback mechanims.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: small coding style changes]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c |  8 ++++----
 drivers/base/firmware_loader/fallback.h | 16 ++++++++--------
 drivers/base/firmware_loader/firmware.h |  2 +-
 drivers/base/firmware_loader/main.c     |  2 +-
 4 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 529f7013616f..3db9e0f225ac 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -662,10 +662,10 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
 	return fw_force_sysfs_fallback(opt_flags);
 }
 
-int fw_sysfs_fallback(struct firmware *fw, const char *name,
-		      struct device *device,
-		      enum fw_opt opt_flags,
-		      int ret)
+int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+			    struct device *device,
+			    enum fw_opt opt_flags,
+			    int ret)
 {
 	if (!fw_run_sysfs_fallback(opt_flags))
 		return ret;
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index a3b73a09db6c..21063503e4ea 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -31,10 +31,10 @@ struct firmware_fallback_config {
 };
 
 #ifdef CONFIG_FW_LOADER_USER_HELPER
-int fw_sysfs_fallback(struct firmware *fw, const char *name,
-		      struct device *device,
-		      enum fw_opt opt_flags,
-		      int ret);
+int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+			    struct device *device,
+			    enum fw_opt opt_flags,
+			    int ret);
 void kill_pending_fw_fallback_reqs(bool only_kill_custom);
 
 void fw_fallback_set_cache_timeout(void);
@@ -43,10 +43,10 @@ void fw_fallback_set_default_timeout(void);
 int register_sysfs_loader(void);
 void unregister_sysfs_loader(void);
 #else /* CONFIG_FW_LOADER_USER_HELPER */
-static inline int fw_sysfs_fallback(struct firmware *fw, const char *name,
-				    struct device *device,
-				    enum fw_opt opt_flags,
-				    int ret)
+static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+					  struct device *device,
+					  enum fw_opt opt_flags,
+					  int ret)
 {
 	/* Keep carrying over the same error */
 	return ret;
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 4f433b447367..4c1395f8e7ed 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -20,7 +20,7 @@
  * @FW_OPT_NOWAIT: Used to describe the firmware request is asynchronous.
  * @FW_OPT_USERHELPER: Enable the fallback mechanism, in case the direct
  *	filesystem lookup fails at finding the firmware.  For details refer to
- *	fw_sysfs_fallback().
+ *	firmware_fallback_sysfs().
  * @FW_OPT_NO_WARN: Quiet, avoid printing warning messages.
  * @FW_OPT_NOCACHE: Disables firmware caching. Firmware caching is used to
  *	cache the firmware upon suspend, so that upon resume races against the
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 7f2bc7e8e3c0..d951af29017a 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -581,7 +581,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 			dev_warn(device,
 				 "Direct firmware load for %s failed with error %d\n",
 				 name, ret);
-		ret = fw_sysfs_fallback(fw, name, device, opt_flags, ret);
+		ret = firmware_fallback_sysfs(fw, name, device, opt_flags, ret);
 	} else
 		ret = assign_fw(fw, device, opt_flags);
 
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 4/6] firmware_loader: document firmware_sysfs_fallback()
  2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
                   ` (2 preceding siblings ...)
  2018-05-04 17:43 ` [PATCH v5 3/6] firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs() Luis R. Rodriguez
@ 2018-05-04 17:43 ` Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 5/6] firmware_loader: enhance Kconfig documentation over FW_LOADER Luis R. Rodriguez
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
	kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, khc, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev,
	Luis R. Rodriguez

This also sets the expecations for future fallback interfaces, even
if they are not exported.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/firmware_loader/fallback.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 3db9e0f225ac..9169e7b9800c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -662,6 +662,26 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
 	return fw_force_sysfs_fallback(opt_flags);
 }
 
+/**
+ * firmware_fallback_sysfs() - use the fallback mechanism to find firmware
+ * @fw: pointer to firmware image
+ * @name: name of firmware file to look for
+ * @device: device for which firmware is being loaded
+ * @opt_flags: options to control firmware loading behaviour
+ * @ret: return value from direct lookup which triggered the fallback mechanism
+ *
+ * This function is called if direct lookup for the firmware failed, it enables
+ * a fallback mechanism through userspace by exposing a sysfs loading
+ * interface. Userspace is in charge of loading the firmware through the syfs
+ * loading interface. This syfs fallback mechanism may be disabled completely
+ * on a system by setting the proc sysctl value ignore_sysfs_fallback to true.
+ * If this false we check if the internal API caller set the @FW_OPT_NOFALLBACK
+ * flag, if so it would also disable the fallback mechanism. A system may want
+ * to enfoce the sysfs fallback mechanism at all times, it can do this by
+ * setting ignore_sysfs_fallback to false and force_sysfs_fallback to true.
+ * Enabling force_sysfs_fallback is functionally equivalent to build a kernel
+ * with CONFIG_FW_LOADER_USER_HELPER_FALLBACK.
+ **/
 int firmware_fallback_sysfs(struct firmware *fw, const char *name,
 			    struct device *device,
 			    enum fw_opt opt_flags,
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 5/6] firmware_loader: enhance Kconfig documentation over FW_LOADER
  2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
                   ` (3 preceding siblings ...)
  2018-05-04 17:43 ` [PATCH v5 4/6] firmware_loader: document firmware_sysfs_fallback() Luis R. Rodriguez
@ 2018-05-04 17:43 ` Luis R. Rodriguez
  2018-05-04 17:43 ` [PATCH v5 6/6] firmware_loader: move kconfig FW_LOADER entries to its own file Luis R. Rodriguez
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
	kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, khc, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev,
	Luis R. Rodriguez

If you try to read FW_LOADER today it speaks of old riddles and
unless you have been following development closely you will loose
track of what is what. Even the documentation for PREVENT_FIRMWARE_BUILD
is a bit fuzzy and how it fits into this big picture.

Give the FW_LOADER kconfig documentation some love with more up to
date developments and recommendations. While at it, wrap the FW_LOADER
code into its own menu to compartamentalize and make it clearer which
components really are part of the FW_LOADER. This should also make
it easier to later move these kconfig entries into the firmware_loader/
directory later.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/Kconfig | 160 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 126 insertions(+), 34 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 29b0eb452b3a..bf2d464b0e2c 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -70,39 +70,64 @@ config STANDALONE
 	  If unsure, say Y.
 
 config PREVENT_FIRMWARE_BUILD
-	bool "Prevent firmware from being built"
+	bool "Disable drivers features which enable custom firmware building"
 	default y
 	help
-	  Say yes to avoid building firmware. Firmware is usually shipped
-	  with the driver and only when updating the firmware should a
-	  rebuild be made.
-	  If unsure, say Y here.
+	  Say yes to disable driver features which enable building a custom
+	  driver firmwar at kernel build time. These drivers do not use the
+	  kernel firmware API to load firmware (CONFIG_FW_LOADER), instead they
+	  use their own custom loading mechanism. The required firmware is
+	  usually shipped with the driver, building the driver firmware
+	  should only be needed if you have an updated firmware source.
+
+	  Firmware should not be being built as part of kernel, these days
+	  you should always prevent this and say Y here. There are only two
+	  old drivers which enable building of its firmware at kernel build
+	  time:
+
+	    o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
+	    o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
+
+menu "Firmware loader"
 
 config FW_LOADER
-	tristate "Userspace firmware loading support" if EXPERT
+	tristate "Firmware loading facility" if EXPERT
 	default y
 	---help---
-	  This option is provided for the case where none of the in-tree modules
-	  require userspace firmware loading support, but a module built
-	  out-of-tree does.
+	  This enables the firmware loading facility in the kernel. The kernel
+	  will first look for built-in firmware, if it has any. Next, it will
+	  look for the requested firmware in a series of filesystem paths:
+
+		o firmware_class path module parameter or kernel boot param
+		o /lib/firmware/updates/UTS_RELEASE
+		o /lib/firmware/updates
+		o /lib/firmware/UTS_RELEASE
+		o /lib/firmware
+
+	  Enabling this feature only increases your kernel image by about
+	  828 bytes, enable this option unless you are certain you don't
+	  need firmware.
+
+	  You typically want this built-in (=y) but you can also enable this
+	  as a module, in which case the firmware_class module will be built.
+	  You also want to be sure to enable this built-in if you are going to
+	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
+
+if FW_LOADER
 
 config EXTRA_FIRMWARE
-	string "External firmware blobs to build into the kernel binary"
-	depends on FW_LOADER
+	string "Build these firmware blobs into the kernel binary"
 	help
-	  Various drivers in the kernel source tree may require firmware,
-	  which is generally available in your distribution's linux-firmware
-	  package.
+	  Device drivers which require firmware can typically deal with
+	  having the kernel load firmware from the various supported
+	  /lib/firmware/ paths. This option enables you to build into the
+	  kernel firmware files. Built-in firmware searches are preceeded
+	  over firmware lookups using your filesystem over the supported
+	  /lib/firmware paths documented on CONFIG_FW_LOADER.
 
-	  The linux-firmware package should install firmware into
-	  /lib/firmware/ on your system, so they can be loaded by userspace
-	  helpers on request.
-
-	  This option allows firmware to be built into the kernel for the case
-	  where the user either cannot or doesn't want to provide it from
-	  userspace at runtime (for example, when the firmware in question is
-	  required for accessing the boot device, and the user doesn't want to
-	  use an initrd).
+	  This may be useful for testing or if the firmware is required early on
+	  in boot and cannot rely on the firmware being placed in an initrd or
+	  initramfs.
 
 	  This option is a string and takes the (space-separated) names of the
 	  firmware files -- the same names that appear in MODULE_FIRMWARE()
@@ -113,7 +138,7 @@ config EXTRA_FIRMWARE
 	  For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
 	  the usb8388.bin file into /lib/firmware, and build the kernel. Then
 	  any request_firmware("usb8388.bin") will be satisfied internally
-	  without needing to call out to userspace.
+	  inside the kernel without ever looking at your filesystem at runtime.
 
 	  WARNING: If you include additional firmware files into your binary
 	  kernel image that are not available under the terms of the GPL,
@@ -130,22 +155,89 @@ config EXTRA_FIRMWARE_DIR
 	  looks for the firmware files listed in the EXTRA_FIRMWARE option.
 
 config FW_LOADER_USER_HELPER
-	bool
+	bool "Enable the firmware sysfs fallback mechanism"
+	help
+	  This option enables a sysfs loading facility to enable firmware
+	  loading to the kernel through userspace as a fallback mechanism
+	  if and only if the kernel's direct filesystem lookup for the
+	  firmware failed using the different /lib/firmware/ paths, or the
+	  path specified in the firmware_class path module parameter, or the
+	  firmware_class path kernel boot parameter if the firmware_class is
+	  built-in. For details on how to work with the sysfs fallback mechanism
+	  refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
+
+	  The direct filesystem lookup for firwmare is always used first now.
+
+	  If the kernel's direct filesystem lookup for firware fails to find
+	  the requested firmware a sysfs fallback loading facility is made
+	  available and userspace is informed about this through uevents.
+	  The uevent can be supressed if the driver explicitly requested it,
+	  this is known as the driver using the custom fallback mechanism.
+	  If the custom fallback mechanism is used userspace must always
+	  acknowledge failure to find firmware as the timeout for the fallback
+	  mechanism is disabled, and failed requests will linger forever.
+
+	  This used to be the default firmware loading facility, and udev used
+	  listen for uvents to load firmware for the kernel. The firmware
+	  loading facility functionality in udev has been removed, as such it
+	  can no longer be relied upon as a fallback mechanism. Linux no longer
+	  relies on or uses a fallback mechanism in userspace.
+
+	  Since this was the default firmware loading facility at one point,
+	  old userspace may exist which relies upon it, and as such this
+	  mechanism can never be removed from the kernel.
+
+	  You should only enable this functionality if you are certain you
+	  require a fallback mechanism and have a userspace mechanism ready to
+	  load firmware in case it is not found. Another reason kernels may
+	  have this feature enabled is to support a driver which explicitly
+	  relies on this fallback mechanism. Only two drivers need this today:
+
+	    o CONFIG_LEDS_LP55XX_COMMON
+	    o CONFIG_DELL_RBU
+
+	  Outside of supporting the above drivers, another reason for needing
+	  this may be that your firmware resides outside of the paths the kernel
+	  looks for and cannot possibily be specified using the firmware_class
+	  path module parameter or kernel firmware_class path boot parameter
+	  if firmware_class is built-in.
+
+	  A modern use case may be to temporarily mount a custom partition
+	  during provisioning which is only accessible to userspace, and then
+	  to use it to look for and fetch the required firmware. Such type of
+	  driver functionality may not even ever be desirable upstream by
+	  vendors, and as such is only required to be supported as an interface
+	  for provisioning. Since udev's firmware loading facility has been
+	  removed you can use firmwared or a fork of it to customize how you
+	  want to load firmware based on uevents issued:
+	  https://github.com/teg/firmwared
+
+	  Enabling this option will increase your kernel image size by about
+	  13436 bytes.
+
+	  If you are unsure about this, say N here, unless you are Linux
+	  distribution and need to support the above two drivers, or you are
+	  certain you need to support some really custom firmware loading
+	  facility in userspace.
 
 config FW_LOADER_USER_HELPER_FALLBACK
-	bool "Fallback user-helper invocation for firmware loading"
-	depends on FW_LOADER
-	select FW_LOADER_USER_HELPER
+	bool "Force the firmware sysfs fallback mechanism when possible"
+	depends on FW_LOADER_USER_HELPER
 	help
-	  This option enables / disables the invocation of user-helper
-	  (e.g. udev) for loading firmware files as a fallback after the
-	  direct file loading in kernel fails.  The user-mode helper is
-	  no longer required unless you have a special firmware file that
-	  resides in a non-standard path. Moreover, the udev support has
-	  been deprecated upstream.
+	  Enabling this option forces a sysfs userspace fallback mechanism
+	  to be used for all firmware requests which explicitly do not disable a
+	  a fallback mechanism. Firmware calls which do prohibit a fallback
+	  mechanism is request_firmware_direct(). This option is kept for
+          backward compatibility purposes given this precise mechanism can also
+	  be enabled by setting the proc sysctl value to true:
+
+	       /proc/sys/kernel/firmware_config/force_sysfs_fallback
 
 	  If you are unsure about this, say N here.
 
+endif # FW_LOADER
+endmenu
+
 config WANT_DEV_COREDUMP
 	bool
 	help
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 6/6] firmware_loader: move kconfig FW_LOADER entries to its own file
  2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
                   ` (4 preceding siblings ...)
  2018-05-04 17:43 ` [PATCH v5 5/6] firmware_loader: enhance Kconfig documentation over FW_LOADER Luis R. Rodriguez
@ 2018-05-04 17:43 ` Luis R. Rodriguez
  2018-05-04 19:17 ` [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Krzysztof Halasa
  2018-05-08 15:24 ` Luis R. Rodriguez
  7 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
  To: gregkh
  Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
	kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
	arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, khc, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev,
	Luis R. Rodriguez

This will make it easier to track and easier to understand
what components and features are part of the FW_LOADER. There
are some components related to firmware which have *nothing* to
do with the FW_LOADER, souch as PREVENT_FIRMWARE_BUILD.

Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
 drivers/base/Kconfig                 | 150 +--------------------------
 drivers/base/firmware_loader/Kconfig | 149 ++++++++++++++++++++++++++
 2 files changed, 150 insertions(+), 149 deletions(-)
 create mode 100644 drivers/base/firmware_loader/Kconfig

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index bf2d464b0e2c..06d6e27784fa 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -88,155 +88,7 @@ config PREVENT_FIRMWARE_BUILD
 	    o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
 	    o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
 
-menu "Firmware loader"
-
-config FW_LOADER
-	tristate "Firmware loading facility" if EXPERT
-	default y
-	---help---
-	  This enables the firmware loading facility in the kernel. The kernel
-	  will first look for built-in firmware, if it has any. Next, it will
-	  look for the requested firmware in a series of filesystem paths:
-
-		o firmware_class path module parameter or kernel boot param
-		o /lib/firmware/updates/UTS_RELEASE
-		o /lib/firmware/updates
-		o /lib/firmware/UTS_RELEASE
-		o /lib/firmware
-
-	  Enabling this feature only increases your kernel image by about
-	  828 bytes, enable this option unless you are certain you don't
-	  need firmware.
-
-	  You typically want this built-in (=y) but you can also enable this
-	  as a module, in which case the firmware_class module will be built.
-	  You also want to be sure to enable this built-in if you are going to
-	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
-
-if FW_LOADER
-
-config EXTRA_FIRMWARE
-	string "Build these firmware blobs into the kernel binary"
-	help
-	  Device drivers which require firmware can typically deal with
-	  having the kernel load firmware from the various supported
-	  /lib/firmware/ paths. This option enables you to build into the
-	  kernel firmware files. Built-in firmware searches are preceeded
-	  over firmware lookups using your filesystem over the supported
-	  /lib/firmware paths documented on CONFIG_FW_LOADER.
-
-	  This may be useful for testing or if the firmware is required early on
-	  in boot and cannot rely on the firmware being placed in an initrd or
-	  initramfs.
-
-	  This option is a string and takes the (space-separated) names of the
-	  firmware files -- the same names that appear in MODULE_FIRMWARE()
-	  and request_firmware() in the source. These files should exist under
-	  the directory specified by the EXTRA_FIRMWARE_DIR option, which is
-	  /lib/firmware by default.
-
-	  For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
-	  the usb8388.bin file into /lib/firmware, and build the kernel. Then
-	  any request_firmware("usb8388.bin") will be satisfied internally
-	  inside the kernel without ever looking at your filesystem at runtime.
-
-	  WARNING: If you include additional firmware files into your binary
-	  kernel image that are not available under the terms of the GPL,
-	  then it may be a violation of the GPL to distribute the resulting
-	  image since it combines both GPL and non-GPL work. You should
-	  consult a lawyer of your own before distributing such an image.
-
-config EXTRA_FIRMWARE_DIR
-	string "Firmware blobs root directory"
-	depends on EXTRA_FIRMWARE != ""
-	default "/lib/firmware"
-	help
-	  This option controls the directory in which the kernel build system
-	  looks for the firmware files listed in the EXTRA_FIRMWARE option.
-
-config FW_LOADER_USER_HELPER
-	bool "Enable the firmware sysfs fallback mechanism"
-	help
-	  This option enables a sysfs loading facility to enable firmware
-	  loading to the kernel through userspace as a fallback mechanism
-	  if and only if the kernel's direct filesystem lookup for the
-	  firmware failed using the different /lib/firmware/ paths, or the
-	  path specified in the firmware_class path module parameter, or the
-	  firmware_class path kernel boot parameter if the firmware_class is
-	  built-in. For details on how to work with the sysfs fallback mechanism
-	  refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
-
-	  The direct filesystem lookup for firwmare is always used first now.
-
-	  If the kernel's direct filesystem lookup for firware fails to find
-	  the requested firmware a sysfs fallback loading facility is made
-	  available and userspace is informed about this through uevents.
-	  The uevent can be supressed if the driver explicitly requested it,
-	  this is known as the driver using the custom fallback mechanism.
-	  If the custom fallback mechanism is used userspace must always
-	  acknowledge failure to find firmware as the timeout for the fallback
-	  mechanism is disabled, and failed requests will linger forever.
-
-	  This used to be the default firmware loading facility, and udev used
-	  listen for uvents to load firmware for the kernel. The firmware
-	  loading facility functionality in udev has been removed, as such it
-	  can no longer be relied upon as a fallback mechanism. Linux no longer
-	  relies on or uses a fallback mechanism in userspace.
-
-	  Since this was the default firmware loading facility at one point,
-	  old userspace may exist which relies upon it, and as such this
-	  mechanism can never be removed from the kernel.
-
-	  You should only enable this functionality if you are certain you
-	  require a fallback mechanism and have a userspace mechanism ready to
-	  load firmware in case it is not found. Another reason kernels may
-	  have this feature enabled is to support a driver which explicitly
-	  relies on this fallback mechanism. Only two drivers need this today:
-
-	    o CONFIG_LEDS_LP55XX_COMMON
-	    o CONFIG_DELL_RBU
-
-	  Outside of supporting the above drivers, another reason for needing
-	  this may be that your firmware resides outside of the paths the kernel
-	  looks for and cannot possibily be specified using the firmware_class
-	  path module parameter or kernel firmware_class path boot parameter
-	  if firmware_class is built-in.
-
-	  A modern use case may be to temporarily mount a custom partition
-	  during provisioning which is only accessible to userspace, and then
-	  to use it to look for and fetch the required firmware. Such type of
-	  driver functionality may not even ever be desirable upstream by
-	  vendors, and as such is only required to be supported as an interface
-	  for provisioning. Since udev's firmware loading facility has been
-	  removed you can use firmwared or a fork of it to customize how you
-	  want to load firmware based on uevents issued:
-	  https://github.com/teg/firmwared
-
-	  Enabling this option will increase your kernel image size by about
-	  13436 bytes.
-
-	  If you are unsure about this, say N here, unless you are Linux
-	  distribution and need to support the above two drivers, or you are
-	  certain you need to support some really custom firmware loading
-	  facility in userspace.
-
-config FW_LOADER_USER_HELPER_FALLBACK
-	bool "Force the firmware sysfs fallback mechanism when possible"
-	depends on FW_LOADER_USER_HELPER
-	help
-	  Enabling this option forces a sysfs userspace fallback mechanism
-	  to be used for all firmware requests which explicitly do not disable a
-	  a fallback mechanism. Firmware calls which do prohibit a fallback
-	  mechanism is request_firmware_direct(). This option is kept for
-          backward compatibility purposes given this precise mechanism can also
-	  be enabled by setting the proc sysctl value to true:
-
-	       /proc/sys/kernel/firmware_config/force_sysfs_fallback
-
-	  If you are unsure about this, say N here.
-
-endif # FW_LOADER
-endmenu
+source "drivers/base/firmware_loader/Kconfig"
 
 config WANT_DEV_COREDUMP
 	bool
diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
new file mode 100644
index 000000000000..b4cf42d6fe51
--- /dev/null
+++ b/drivers/base/firmware_loader/Kconfig
@@ -0,0 +1,149 @@
+menu "Firmware loader"
+
+config FW_LOADER
+	tristate "Firmware loading facility" if EXPERT
+	default y
+	---help---
+	  This enables the firmware loading facility in the kernel. The kernel
+	  will first look for built-in firmware, if it has any. Next, it will
+	  look for the requested firmware in a series of filesystem paths:
+
+		o firmware_class path module parameter or kernel boot param
+		o /lib/firmware/updates/UTS_RELEASE
+		o /lib/firmware/updates
+		o /lib/firmware/UTS_RELEASE
+		o /lib/firmware
+
+	  Enabling this feature only increases your kernel image by about
+	  828 bytes, enable this option unless you are certain you don't
+	  need firmware.
+
+	  You typically want this built-in (=y) but you can also enable this
+	  as a module, in which case the firmware_class module will be built.
+	  You also want to be sure to enable this built-in if you are going to
+	  enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
+
+if FW_LOADER
+
+config EXTRA_FIRMWARE
+	string "Build these firmware blobs into the kernel binary"
+	help
+	  Device drivers which require firmware can typically deal with
+	  having the kernel load firmware from the various supported
+	  /lib/firmware/ paths. This option enables you to build into the
+	  kernel firmware files. Built-in firmware searches are preceeded
+	  over firmware lookups using your filesystem over the supported
+	  /lib/firmware paths documented on CONFIG_FW_LOADER.
+
+	  This may be useful for testing or if the firmware is required early on
+	  in boot and cannot rely on the firmware being placed in an initrd or
+	  initramfs.
+
+	  This option is a string and takes the (space-separated) names of the
+	  firmware files -- the same names that appear in MODULE_FIRMWARE()
+	  and request_firmware() in the source. These files should exist under
+	  the directory specified by the EXTRA_FIRMWARE_DIR option, which is
+	  /lib/firmware by default.
+
+	  For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
+	  the usb8388.bin file into /lib/firmware, and build the kernel. Then
+	  any request_firmware("usb8388.bin") will be satisfied internally
+	  inside the kernel without ever looking at your filesystem at runtime.
+
+	  WARNING: If you include additional firmware files into your binary
+	  kernel image that are not available under the terms of the GPL,
+	  then it may be a violation of the GPL to distribute the resulting
+	  image since it combines both GPL and non-GPL work. You should
+	  consult a lawyer of your own before distributing such an image.
+
+config EXTRA_FIRMWARE_DIR
+	string "Firmware blobs root directory"
+	depends on EXTRA_FIRMWARE != ""
+	default "/lib/firmware"
+	help
+	  This option controls the directory in which the kernel build system
+	  looks for the firmware files listed in the EXTRA_FIRMWARE option.
+
+config FW_LOADER_USER_HELPER
+	bool "Enable the firmware sysfs fallback mechanism"
+	help
+	  This option enables a sysfs loading facility to enable firmware
+	  loading to the kernel through userspace as a fallback mechanism
+	  if and only if the kernel's direct filesystem lookup for the
+	  firmware failed using the different /lib/firmware/ paths, or the
+	  path specified in the firmware_class path module parameter, or the
+	  firmware_class path kernel boot parameter if the firmware_class is
+	  built-in. For details on how to work with the sysfs fallback mechanism
+	  refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
+
+	  The direct filesystem lookup for firwmare is always used first now.
+
+	  If the kernel's direct filesystem lookup for firware fails to find
+	  the requested firmware a sysfs fallback loading facility is made
+	  available and userspace is informed about this through uevents.
+	  The uevent can be supressed if the driver explicitly requested it,
+	  this is known as the driver using the custom fallback mechanism.
+	  If the custom fallback mechanism is used userspace must always
+	  acknowledge failure to find firmware as the timeout for the fallback
+	  mechanism is disabled, and failed requests will linger forever.
+
+	  This used to be the default firmware loading facility, and udev used
+	  listen for uvents to load firmware for the kernel. The firmware
+	  loading facility functionality in udev has been removed, as such it
+	  can no longer be relied upon as a fallback mechanism. Linux no longer
+	  relies on or uses a fallback mechanism in userspace.
+
+	  Since this was the default firmware loading facility at one point,
+	  old userspace may exist which relies upon it, and as such this
+	  mechanism can never be removed from the kernel.
+
+	  You should only enable this functionality if you are certain you
+	  require a fallback mechanism and have a userspace mechanism ready to
+	  load firmware in case it is not found. Another reason kernels may
+	  have this feature enabled is to support a driver which explicitly
+	  relies on this fallback mechanism. Only two drivers need this today:
+
+	    o CONFIG_LEDS_LP55XX_COMMON
+	    o CONFIG_DELL_RBU
+
+	  Outside of supporting the above drivers, another reason for needing
+	  this may be that your firmware resides outside of the paths the kernel
+	  looks for and cannot possibily be specified using the firmware_class
+	  path module parameter or kernel firmware_class path boot parameter
+	  if firmware_class is built-in.
+
+	  A modern use case may be to temporarily mount a custom partition
+	  during provisioning which is only accessible to userspace, and then
+	  to use it to look for and fetch the required firmware. Such type of
+	  driver functionality may not even ever be desirable upstream by
+	  vendors, and as such is only required to be supported as an interface
+	  for provisioning. Since udev's firmware loading facility has been
+	  removed you can use firmwared or a fork of it to customize how you
+	  want to load firmware based on uevents issued:
+	  https://github.com/teg/firmwared
+
+	  Enabling this option will increase your kernel image size by about
+	  13436 bytes.
+
+	  If you are unsure about this, say N here, unless you are Linux
+	  distribution and need to support the above two drivers, or you are
+	  certain you need to support some really custom firmware loading
+	  facility in userspace.
+
+config FW_LOADER_USER_HELPER_FALLBACK
+	bool "Force the firmware sysfs fallback mechanism when possible"
+	depends on FW_LOADER_USER_HELPER
+	help
+	  Enabling this option forces a sysfs userspace fallback mechanism
+	  to be used for all firmware requests which explicitly do not disable a
+	  a fallback mechanism. Firmware calls which do prohibit a fallback
+	  mechanism is request_firmware_direct(). This option is kept for
+          backward compatibility purposes given this precise mechanism can also
+	  be enabled by setting the proc sysctl value to true:
+
+	       /proc/sys/kernel/firmware_config/force_sysfs_fallback
+
+	  If you are unsure about this, say N here.
+
+endif # FW_LOADER
+endmenu
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/6] firmware_loader: cleanups for v4.18
  2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
                   ` (5 preceding siblings ...)
  2018-05-04 17:43 ` [PATCH v5 6/6] firmware_loader: move kconfig FW_LOADER entries to its own file Luis R. Rodriguez
@ 2018-05-04 19:17 ` Krzysztof Halasa
  2018-05-04 19:58   ` Luis R. Rodriguez
  2018-05-08 15:24 ` Luis R. Rodriguez
  7 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Halasa @ 2018-05-04 19:17 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: gregkh, akpm, keescook, josh, teg, wagi, hdegoede, andresx7,
	zohar, kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai,
	kvalo, arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev

"Luis R. Rodriguez" <mcgrof@kernel.org> writes:

>   * CONFIG_WANXL --> CONFIG_WANXL_BUILD_FIRMWARE
>   * CONFIG_SCSI_AIC79XX --> CONFIG_AIC79XX_BUILD_FIRMWARE
>
> To this day both of these drivers are building driver *firmwares* when
> the option CONFIG_PREVENT_FIRMWARE_BUILD is disabled, and they don't
> even make use of the firmware API at all.

Don't know for sure about Adaptec, but wanXL firmware fits every
definition of "stable". BTW it's a 1997 or so early PCI card, built
around the PLX PCI9060 bus mastering PCI bridge and Motorola 68360
(the original QUICC) processor. Maximum bit rate of 2 Mb/s on each sync
serial port.

It's more about delivering the .S source for the firmware, I guess.
Nobody is expected to build it. The fw is about 2.5 KB and is directly
linked with the driver.
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/6] firmware_loader: cleanups for v4.18
  2018-05-04 19:17 ` [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Krzysztof Halasa
@ 2018-05-04 19:58   ` Luis R. Rodriguez
  2018-05-05 20:26     ` Krzysztof Halasa
  0 siblings, 1 reply; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04 19:58 UTC (permalink / raw)
  To: Krzysztof Halasa
  Cc: Luis R. Rodriguez, gregkh, akpm, keescook, josh, teg, wagi,
	hdegoede, andresx7, zohar, kubakici, shuah, mfuzzey, dhowells,
	pali.rohar, tiwai, kvalo, arend.vanspriel, zajec5, nbroeking,
	markivx, broonie, dmitry.torokhov, dwmw2, torvalds,
	Abhay_Salunke, bjorn.andersson, jewalt, oneukum, cantabile.desu,
	ast, hare, jejb, martin.petersen, davem, maco, arve, tkjos,
	linux-kernel, linux-fsdevel, linux-scsi, linux-wireless, netdev

On Fri, May 04, 2018 at 09:17:08PM +0200, Krzysztof Halasa wrote:
> "Luis R. Rodriguez" <mcgrof@kernel.org> writes:
> 
> >   * CONFIG_WANXL --> CONFIG_WANXL_BUILD_FIRMWARE
> >   * CONFIG_SCSI_AIC79XX --> CONFIG_AIC79XX_BUILD_FIRMWARE
> >
> > To this day both of these drivers are building driver *firmwares* when
> > the option CONFIG_PREVENT_FIRMWARE_BUILD is disabled, and they don't
> > even make use of the firmware API at all.
> 
> Don't know for sure about Adaptec, but wanXL firmware fits every
> definition of "stable". BTW it's a 1997 or so early PCI card, built
> around the PLX PCI9060 bus mastering PCI bridge and Motorola 68360
> (the original QUICC) processor. Maximum bit rate of 2 Mb/s on each sync
> serial port.

So we can nuke CONFIG_WANXL_BUILD_FIRMWARE now?

> It's more about delivering the .S source for the firmware, I guess.
> Nobody is expected to build it. The fw is about 2.5 KB and is directly
> linked with the driver.

:P Future work I guess would be to just use the firmware API and stuff
it into linux-firmware?

  Luis

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/6] firmware_loader: cleanups for v4.18
  2018-05-04 19:58   ` Luis R. Rodriguez
@ 2018-05-05 20:26     ` Krzysztof Halasa
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Halasa @ 2018-05-05 20:26 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: gregkh, akpm, keescook, josh, teg, wagi, hdegoede, andresx7,
	zohar, kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai,
	kvalo, arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev

"Luis R. Rodriguez" <mcgrof@kernel.org> writes:

> So we can nuke CONFIG_WANXL_BUILD_FIRMWARE now?

I'm uncertain I understand why do you want it, or maybe what are you
trying to do at all.

And what use would wanxlfw.S (the assembly source) have if the option is
removed?

>> It's more about delivering the .S source for the firmware, I guess.
>> Nobody is expected to build it. The fw is about 2.5 KB and is directly
>> linked with the driver.
>
> :P Future work I guess would be to just use the firmware API and stuff
> it into linux-firmware?

Who's going to make it happen?
The last time I checked (several years ago), wanXL worked. Who's going
to test it after the change?

I assume linux-firmware could include fw source and there would be means
to build the binary.

Just to be sure: the wanXL firmware has exactly nothing to do with FW
loader, nothing depends on it (nor the other way around), it's just
(with the rest of the wanXL code) an old piece of a driver for an old
card.

The question is, what do we gain by messing with it?
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/6] firmware_loader: cleanups for v4.18
  2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
                   ` (6 preceding siblings ...)
  2018-05-04 19:17 ` [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Krzysztof Halasa
@ 2018-05-08 15:24 ` Luis R. Rodriguez
  7 siblings, 0 replies; 11+ messages in thread
From: Luis R. Rodriguez @ 2018-05-08 15:24 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: gregkh, akpm, keescook, josh, teg, wagi, hdegoede, andresx7,
	zohar, kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai,
	kvalo, arend.vanspriel, zajec5, nbroeking, markivx, broonie,
	dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
	jewalt, oneukum, cantabile.desu, ast, hare, jejb,
	martin.petersen, khc, davem, maco, arve, tkjos, linux-kernel,
	linux-fsdevel, linux-scsi, linux-wireless, netdev

On Fri, May 04, 2018 at 10:43:49AM -0700, Luis R. Rodriguez wrote:
> Greg,
> 
> I've reviewed the pending patches for the firmware_laoder and as for
> v4.18, the following 3 patches from Andres have been iterated enough
> that they're ready after I made some final minor changes, mostly just
> style fixes and re-arrangements in terms of order. The new API he was
> suggesting to add requires just a bit more review.

We're done with review of the new API for the sync no warn call,
so I'll just re-issue another patch series with those patches
in a new series.

Coming right up.

  Luis

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-05-08 15:24 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04 17:43 [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Luis R. Rodriguez
2018-05-04 17:43 ` [PATCH v5 1/6] firmware: wrap FW_OPT_* into an enum Luis R. Rodriguez
2018-05-04 17:43 ` [PATCH v5 2/6] firmware: use () to terminate kernel-doc function names Luis R. Rodriguez
2018-05-04 17:43 ` [PATCH v5 3/6] firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs() Luis R. Rodriguez
2018-05-04 17:43 ` [PATCH v5 4/6] firmware_loader: document firmware_sysfs_fallback() Luis R. Rodriguez
2018-05-04 17:43 ` [PATCH v5 5/6] firmware_loader: enhance Kconfig documentation over FW_LOADER Luis R. Rodriguez
2018-05-04 17:43 ` [PATCH v5 6/6] firmware_loader: move kconfig FW_LOADER entries to its own file Luis R. Rodriguez
2018-05-04 19:17 ` [PATCH v5 0/6] firmware_loader: cleanups for v4.18 Krzysztof Halasa
2018-05-04 19:58   ` Luis R. Rodriguez
2018-05-05 20:26     ` Krzysztof Halasa
2018-05-08 15:24 ` Luis R. Rodriguez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).