LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/9] Loading optional firmware v4
@ 2018-04-23 20:11 Andres Rodriguez
  2018-04-23 20:11 ` [PATCH 1/9] firmware: some documentation fixes Andres Rodriguez
                   ` (8 more replies)
  0 siblings, 9 replies; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

Hey Everyone,

This is a trimmed down version of v3 that contains the changes for which we
have agreement. Changes that raised concern are being shifted to a followup
series, mainly changes to the async firmware API.

Inline feedback I received in these patches has also been addressed, thanks
everyone that chimed in.

I'll be following up with some proposals for the async API. Hopefully we can
end up with something that is not "the least bad" option.

This series can be found in the for-luis-v4 branch of:
https://github.com/lostgoat/linux.git

Andres Rodriguez (9):
  firmware: some documentation fixes
  firmware: wrap FW_OPT_* into an enum
  firmware: add kernel-doc for enum fw_opt
  firmware: use () to terminate kernel-doc function names
  firmware: add function to load firmware without warnings v5
  firmware: print firmware name on fallback path
  firmware: use rename fw_sysfs_fallback to use the firmware_ prefix
  ath10k: use request_firmware_nowarn to load firmware
  ath10k: re-enable the firmware fallback mechanism for testmode

 .../driver-api/firmware/request_firmware.rst       | 21 +++++----
 drivers/base/firmware_loader/fallback.c            | 30 ++++++------
 drivers/base/firmware_loader/fallback.h            | 12 +++--
 drivers/base/firmware_loader/firmware.h            | 38 +++++++++++----
 drivers/base/firmware_loader/main.c                | 54 ++++++++++++++++------
 drivers/net/wireless/ath/ath10k/core.c             |  2 +-
 drivers/net/wireless/ath/ath10k/testmode.c         |  2 +-
 include/linux/firmware.h                           |  2 +
 8 files changed, 108 insertions(+), 53 deletions(-)

-- 
2.14.1

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

* [PATCH 1/9] firmware: some documentation fixes
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
@ 2018-04-23 20:11 ` Andres Rodriguez
  2018-04-25 15:25   ` Greg KH
  2018-05-03 23:31   ` [PATCH 1/9] " Luis R. Rodriguez
  2018-04-23 20:11 ` [PATCH 2/9] firmware: wrap FW_OPT_* into an enum Andres Rodriguez
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

Including:
 - Fixup outdated kernel-doc paths
 - Slightly too short title underline
 - Some typos

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 Documentation/driver-api/firmware/request_firmware.rst | 16 ++++++++--------
 drivers/base/firmware_loader/fallback.c                |  4 ++--
 drivers/base/firmware_loader/fallback.h                |  2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index 9305bf4db38e..7632f8807472 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -17,17 +17,17 @@ an error is returned.
 
 firmware_request
 ----------------
-.. kernel-doc:: drivers/base/firmware_class.c
+.. kernel-doc:: drivers/base/firmware_loader/main.c
    :functions: firmware_request
 
 firmware_request_direct
 -----------------------
-.. kernel-doc:: drivers/base/firmware_class.c
+.. kernel-doc:: drivers/base/firmware_loader/main.c
    :functions: firmware_request_direct
 
 firmware_request_into_buf
 -------------------------
-.. kernel-doc:: drivers/base/firmware_class.c
+.. kernel-doc:: drivers/base/firmware_loader/main.c
    :functions: firmware_request_into_buf
 
 Asynchronous firmware requests
@@ -41,7 +41,7 @@ in atomic contexts.
 
 firmware_request_nowait
 -----------------------
-.. kernel-doc:: drivers/base/firmware_class.c
+.. kernel-doc:: drivers/base/firmware_loader/main.c
    :functions: firmware_request_nowait
 
 Special optimizations on reboot
@@ -50,12 +50,12 @@ Special optimizations on reboot
 Some devices have an optimization in place to enable the firmware to be
 retained during system reboot. When such optimizations are used the driver
 author must ensure the firmware is still available on resume from suspend,
-this can be done with firmware_request_cache() insted of requesting for the
-firmare to be loaded.
+this can be done with firmware_request_cache() instead of requesting for the
+firmware to be loaded.
 
 firmware_request_cache()
------------------------
-.. kernel-doc:: drivers/base/firmware_class.c
+------------------------
+.. kernel-doc:: drivers/base/firmware_loader/main.c
    :functions: firmware_request_cache
 
 request firmware API expected driver use
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 71f529de5243..da97fc26119c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -536,8 +536,8 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 }
 
 /**
- * fw_load_sysfs_fallback - load a firmware via the syfs fallback mechanism
- * @fw_sysfs: firmware syfs information for the firmware to load
+ * 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/fallback.h b/drivers/base/firmware_loader/fallback.h
index dfebc644ed35..f8255670a663 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -6,7 +6,7 @@
 #include <linux/device.h>
 
 /**
- * struct firmware_fallback_config - firmware fallback configuratioon settings
+ * struct firmware_fallback_config - firmware fallback configuration settings
  *
  * Helps describe and fine tune the fallback mechanism.
  *
-- 
2.14.1

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

* [PATCH 2/9] firmware: wrap FW_OPT_* into an enum
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
  2018-04-23 20:11 ` [PATCH 1/9] firmware: some documentation fixes Andres Rodriguez
@ 2018-04-23 20:11 ` Andres Rodriguez
  2018-05-03 23:35   ` Luis R. Rodriguez
  2018-04-23 20:11 ` [PATCH 3/9] firmware: add kernel-doc for enum fw_opt Andres Rodriguez
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

This should let us associate enum kdoc to these values.

v2: use BIT() macro

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/base/firmware_loader/fallback.c | 12 ++++++------
 drivers/base/firmware_loader/fallback.h |  6 ++++--
 drivers/base/firmware_loader/firmware.h | 18 ++++++++++--------
 drivers/base/firmware_loader/main.c     |  6 +++---
 4 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index da97fc26119c..ecc49a619298 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -511,7 +511,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;
@@ -544,7 +544,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;
@@ -598,7 +598,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;
@@ -639,7 +639,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;
@@ -648,7 +648,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");
@@ -663,7 +663,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 d11d37db370b..b252bfa82295 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,14 @@
 
 #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 {
+	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 +112,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 5812148694b6..5baadad3012d 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
 _firmware_request(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 firmware_request_work_func(struct work_struct *work)
-- 
2.14.1

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

* [PATCH 3/9] firmware: add kernel-doc for enum fw_opt
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
  2018-04-23 20:11 ` [PATCH 1/9] firmware: some documentation fixes Andres Rodriguez
  2018-04-23 20:11 ` [PATCH 2/9] firmware: wrap FW_OPT_* into an enum Andres Rodriguez
@ 2018-04-23 20:11 ` Andres Rodriguez
  2018-05-03 23:36   ` Luis R. Rodriguez
  2018-04-23 20:12 ` [PATCH 4/9] firmware: use () to terminate kernel-doc function names Andres Rodriguez
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:11 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

Some basic definitions for the FW_OPT_* values

v2: Documentation corrections from Luis.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/base/firmware_loader/firmware.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index b252bfa82295..a405d400a925 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -11,6 +11,26 @@
 
 #include <generated/utsrelease.h>
 
+/**
+ * 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),
-- 
2.14.1

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

* [PATCH 4/9] firmware: use () to terminate kernel-doc function names
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
                   ` (2 preceding siblings ...)
  2018-04-23 20:11 ` [PATCH 3/9] firmware: add kernel-doc for enum fw_opt Andres Rodriguez
@ 2018-04-23 20:12 ` Andres Rodriguez
  2018-05-03 23:37   ` Luis R. Rodriguez
  2018-04-23 20:12 ` [PATCH 5/9] firmware: add function to load firmware without warnings v5 Andres Rodriguez
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

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>
---
 drivers/base/firmware_loader/fallback.c |  8 ++++----
 drivers/base/firmware_loader/main.c     | 22 +++++++++++-----------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index ecc49a619298..e75928458489 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -124,7 +124,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
@@ -238,7 +238,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
@@ -430,7 +430,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
@@ -536,7 +536,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 5baadad3012d..d028cd3257f7 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -597,7 +597,7 @@ _firmware_request(const struct firmware **firmware_p, const char *name,
 }
 
 /**
- * firmware_request: - send firmware request and wait for it
+ * firmware_request() - 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
@@ -632,7 +632,7 @@ firmware_request(const struct firmware **firmware_p, const char *name,
 EXPORT_SYMBOL(firmware_request);
 
 /**
- * firmware_request_direct: - load firmware directly without usermode helper
+ * firmware_request_direct() - load firmware directly without usermode helper
  * @firmware_p: pointer to firmware image
  * @name: name of firmware file
  * @device: device for which firmware is being loaded
@@ -657,7 +657,7 @@ int firmware_request_direct(const struct firmware **firmware_p,
 EXPORT_SYMBOL_GPL(firmware_request_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);
 
 /**
- * firmware_request_into_buf - load firmware into a previously allocated buffer
+ * firmware_request_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 @@ firmware_request_into_buf(const struct firmware **firmware_p, const char *name,
 EXPORT_SYMBOL(firmware_request_into_buf);
 
 /**
- * firmware_release: - release the resource associated with a firmware image
+ * firmware_release() - release the resource associated with a firmware image
  * @fw: firmware resource to release
  **/
 void firmware_release(const struct firmware *fw)
@@ -755,7 +755,7 @@ static void firmware_request_work_func(struct work_struct *work)
 }
 
 /**
- * firmware_request_nowait - asynchronous version of firmware_request
+ * firmware_request_nowait() - asynchronous version of firmware_request
  * @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(firmware_request_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 firmware_request 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.14.1

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

* [PATCH 5/9] firmware: add function to load firmware without warnings v5
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
                   ` (3 preceding siblings ...)
  2018-04-23 20:12 ` [PATCH 4/9] firmware: use () to terminate kernel-doc function names Andres Rodriguez
@ 2018-04-23 20:12 ` Andres Rodriguez
  2018-05-03 23:40   ` Luis R. Rodriguez
  2018-05-04  0:00   ` Luis R. Rodriguez
  2018-04-23 20:12 ` [PATCH 6/9] firmware: print firmware name on fallback path Andres Rodriguez
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

Currently the firmware loader only exposes one silent path for querying
optional firmware, and that is firmware_request_direct(). This function
also disables the fallback path, which might not always be the
desired behaviour. [0]

This patch introduces a variations of firmware_request() that enable the
caller to disable the undesired warning messages. This is equivalent to
adding FW_OPT_NO_WARN to the old behaviour.

v2: add header prototype, use updated opt_flags
v3: split debug message to separate patch
    added _nowait variant
    added documentation
v4: fix kernel-doc style
v5: drop _nowait for now, since we lack agreement on the API

[0]: https://git.kernel.org/linus/c0cc00f250e1

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 .../driver-api/firmware/request_firmware.rst       |  5 +++++
 drivers/base/firmware_loader/main.c                | 24 ++++++++++++++++++++++
 include/linux/firmware.h                           |  2 ++
 3 files changed, 31 insertions(+)

diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index 7632f8807472..c8bddbdcfd10 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -20,6 +20,11 @@ firmware_request
 .. kernel-doc:: drivers/base/firmware_loader/main.c
    :functions: firmware_request
 
+firmware_request_nowarn
+-----------------------
+.. kernel-doc:: drivers/base/firmware_loader/main.c
+   :functions: firmware_request_nowarn
+
 firmware_request_direct
 -----------------------
 .. kernel-doc:: drivers/base/firmware_loader/main.c
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index d028cd3257f7..58aaadf81e12 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -631,6 +631,30 @@ firmware_request(const struct firmware **firmware_p, const char *name,
 }
 EXPORT_SYMBOL(firmware_request);
 
+/**
+ * firmware_request_nowarn() - request for an optional fw module
+ * @firmware: pointer to firmware image
+ * @name: name of firmware file
+ * @device: device for which firmware is being loaded
+ *
+ * This function is similar in behaviour to firmware_request(), except
+ * it doesn't produce warning messages when the file is not found.
+ **/
+int
+firmware_request_nowarn(const struct firmware **firmware, const char *name,
+			struct device *device)
+{
+	int ret;
+
+	/* Need to pin this module until return */
+	__module_get(THIS_MODULE);
+	ret = _firmware_request(firmware, name, device, NULL, 0,
+				FW_OPT_UEVENT | FW_OPT_NO_WARN);
+	module_put(THIS_MODULE);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(firmware_request_nowarn);
+
 /**
  * firmware_request_direct() - load firmware directly without usermode helper
  * @firmware_p: pointer to firmware image
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index db8351a42405..a34e16f77f20 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -42,6 +42,8 @@ struct builtin_fw {
 #if defined(CONFIG_FW_LOADER) || (defined(CONFIG_FW_LOADER_MODULE) && defined(MODULE))
 int firmware_request(const struct firmware **fw, const char *name,
 		     struct device *device);
+int firmware_request_nowarn(const struct firmware **fw, const char *name,
+			    struct device *device);
 int firmware_request_nowait(
 	struct module *module, bool uevent,
 	const char *name, struct device *device, gfp_t gfp, void *context,
-- 
2.14.1

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

* [PATCH 6/9] firmware: print firmware name on fallback path
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
                   ` (4 preceding siblings ...)
  2018-04-23 20:12 ` [PATCH 5/9] firmware: add function to load firmware without warnings v5 Andres Rodriguez
@ 2018-04-23 20:12 ` Andres Rodriguez
  2018-05-03 23:42   ` Luis R. Rodriguez
  2018-04-23 20:12 ` [PATCH 7/9] firmware: use rename fw_sysfs_fallback to use the firmware_ prefix Andres Rodriguez
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

Previously, one could assume the firmware name from the preceding
message: "Direct firmware load for {name} failed with error %d".

However, with the new firmware_request_nowarn() entrypoint, the message
outlined above will not always be printed.

Therefore, we add the firmware name to the fallback path spew in order
to associate it with the appropriate request.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/base/firmware_loader/fallback.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index e75928458489..1a47ddc70c31 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -669,6 +669,6 @@ int fw_sysfs_fallback(struct firmware *fw, const char *name,
 	if (!fw_run_sysfs_fallback(opt_flags))
 		return ret;
 
-	dev_warn(device, "Falling back to user helper\n");
+	dev_warn(device, "Falling back to user helper for %s\n", name);
 	return fw_load_from_user_helper(fw, name, device, opt_flags);
 }
-- 
2.14.1

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

* [PATCH 7/9] firmware: use rename fw_sysfs_fallback to use the firmware_ prefix
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
                   ` (5 preceding siblings ...)
  2018-04-23 20:12 ` [PATCH 6/9] firmware: print firmware name on fallback path Andres Rodriguez
@ 2018-04-23 20:12 ` Andres Rodriguez
  2018-05-03 23:44   ` Luis R. Rodriguez
  2018-04-23 20:12 ` [PATCH 8/9] ath10k: use request_firmware_nowarn to load firmware Andres Rodriguez
  2018-04-23 20:12 ` [PATCH 9/9] ath10k: re-enable the firmware fallback mechanism for testmode Andres Rodriguez
  8 siblings, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

Use the correct prefix for symbols exported by firmware_loader(). This
is done since firmware_sysfs_fallback() is now exposed through
kernel-doc.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/base/firmware_loader/fallback.c | 8 ++++----
 drivers/base/firmware_loader/fallback.h | 4 ++--
 drivers/base/firmware_loader/firmware.h | 6 +++---
 drivers/base/firmware_loader/main.c     | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 1a47ddc70c31..fc186b5bccce 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -661,10 +661,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_sysfs_fallback(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..8cfaa3299bb7 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -31,7 +31,7 @@ struct firmware_fallback_config {
 };
 
 #ifdef CONFIG_FW_LOADER_USER_HELPER
-int fw_sysfs_fallback(struct firmware *fw, const char *name,
+int firmware_sysfs_fallback(struct firmware *fw, const char *name,
 		      struct device *device,
 		      enum fw_opt opt_flags,
 		      int ret);
@@ -43,7 +43,7 @@ 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,
+static inline int firmware_sysfs_fallback(struct firmware *fw, const char *name,
 				    struct device *device,
 				    enum fw_opt opt_flags,
 				    int ret)
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index a405d400a925..59836d50c5bd 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -15,12 +15,12 @@
  * 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.
+ *                 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().
+ *                     For details refer to firmware_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
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 58aaadf81e12..f009566acd35 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -581,7 +581,7 @@ _firmware_request(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_sysfs_fallback(fw, name, device, opt_flags, ret);
 	} else
 		ret = assign_fw(fw, device, opt_flags);
 
-- 
2.14.1

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

* [PATCH 8/9] ath10k: use request_firmware_nowarn to load firmware
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
                   ` (6 preceding siblings ...)
  2018-04-23 20:12 ` [PATCH 7/9] firmware: use rename fw_sysfs_fallback to use the firmware_ prefix Andres Rodriguez
@ 2018-04-23 20:12 ` Andres Rodriguez
  2018-04-23 20:12 ` [PATCH 9/9] ath10k: re-enable the firmware fallback mechanism for testmode Andres Rodriguez
  8 siblings, 0 replies; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

This reduces the unnecessary spew when trying to load optional firmware:
"Direct firmware load for ... failed with error -2"

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Kalle Valo <kvalo@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index f3ec13b80b20..9a225b7ad2d7 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -652,7 +652,7 @@ static const struct firmware *ath10k_fetch_fw_file(struct ath10k *ar,
 		dir = ".";
 
 	snprintf(filename, sizeof(filename), "%s/%s", dir, file);
-	ret = request_firmware(&fw, filename, ar->dev);
+	ret = request_firmware_nowarn(&fw, filename, ar->dev);
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n",
 		   filename, ret);
 
-- 
2.14.1

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

* [PATCH 9/9] ath10k: re-enable the firmware fallback mechanism for testmode
  2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
                   ` (7 preceding siblings ...)
  2018-04-23 20:12 ` [PATCH 8/9] ath10k: use request_firmware_nowarn to load firmware Andres Rodriguez
@ 2018-04-23 20:12 ` Andres Rodriguez
  2018-04-24  5:29   ` Kalle Valo
  8 siblings, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-23 20:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

The ath10k testmode uses request_firmware_direct() in order to avoid
producing firmware load warnings. Disabling the fallback mechanism was a
side effect of disabling warnings.

We now have a new API that allows us to avoid warnings while keeping the
fallback mechanism enabled. So use that instead.

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/net/wireless/ath/ath10k/testmode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/testmode.c b/drivers/net/wireless/ath/ath10k/testmode.c
index 568810b41657..0beb91be1b1c 100644
--- a/drivers/net/wireless/ath/ath10k/testmode.c
+++ b/drivers/net/wireless/ath/ath10k/testmode.c
@@ -157,7 +157,7 @@ static int ath10k_tm_fetch_utf_firmware_api_1(struct ath10k *ar,
 		 ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE);
 
 	/* load utf firmware image */
-	ret = request_firmware_direct(&fw_file->firmware, filename, ar->dev);
+	ret = request_firmware_nowarn(&fw_file->firmware, filename, ar->dev);
 	ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode fw request '%s': %d\n",
 		   filename, ret);
 
-- 
2.14.1

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

* Re: [PATCH 9/9] ath10k: re-enable the firmware fallback mechanism for testmode
  2018-04-23 20:12 ` [PATCH 9/9] ath10k: re-enable the firmware fallback mechanism for testmode Andres Rodriguez
@ 2018-04-24  5:29   ` Kalle Valo
  0 siblings, 0 replies; 31+ messages in thread
From: Kalle Valo @ 2018-04-24  5:29 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

Andres Rodriguez <andresx7@gmail.com> writes:

> The ath10k testmode uses request_firmware_direct() in order to avoid
> producing firmware load warnings. Disabling the fallback mechanism was a
> side effect of disabling warnings.
>
> We now have a new API that allows us to avoid warnings while keeping the
> fallback mechanism enabled. So use that instead.
>
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>

Thanks!

Acked-by: Kalle Valo <kvalo@codeaurora.org>

-- 
Kalle Valo

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

* Re: [PATCH 1/9] firmware: some documentation fixes
  2018-04-23 20:11 ` [PATCH 1/9] firmware: some documentation fixes Andres Rodriguez
@ 2018-04-25 15:25   ` Greg KH
  2018-04-25 15:26     ` Greg KH
  2018-05-03 23:31   ` [PATCH 1/9] " Luis R. Rodriguez
  1 sibling, 1 reply; 31+ messages in thread
From: Greg KH @ 2018-04-25 15:25 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

On Mon, Apr 23, 2018 at 04:11:57PM -0400, Andres Rodriguez wrote:
> Including:
>  - Fixup outdated kernel-doc paths
>  - Slightly too short title underline
>  - Some typos
> 
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---
>  Documentation/driver-api/firmware/request_firmware.rst | 16 ++++++++--------
>  drivers/base/firmware_loader/fallback.c                |  4 ++--
>  drivers/base/firmware_loader/fallback.h                |  2 +-
>  3 files changed, 11 insertions(+), 11 deletions(-)

This fails to apply to my driver-core-next branch.  Can you please
rebase it and resend?

thanks,

greg k-h

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

* Re: [PATCH 1/9] firmware: some documentation fixes
  2018-04-25 15:25   ` Greg KH
@ 2018-04-25 15:26     ` Greg KH
  2018-04-25 16:25       ` [PATCH 1/2] " Andres Rodriguez
  0 siblings, 1 reply; 31+ messages in thread
From: Greg KH @ 2018-04-25 15:26 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

On Wed, Apr 25, 2018 at 05:25:26PM +0200, Greg KH wrote:
> On Mon, Apr 23, 2018 at 04:11:57PM -0400, Andres Rodriguez wrote:
> > Including:
> >  - Fixup outdated kernel-doc paths
> >  - Slightly too short title underline
> >  - Some typos
> > 
> > Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> > ---
> >  Documentation/driver-api/firmware/request_firmware.rst | 16 ++++++++--------
> >  drivers/base/firmware_loader/fallback.c                |  4 ++--
> >  drivers/base/firmware_loader/fallback.h                |  2 +-
> >  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> This fails to apply to my driver-core-next branch.  Can you please
> rebase it and resend?

Oops, I meant the driver-core-linus branch, which is where "fixes" would
go at the moment.

thanks,

greg k-h

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

* [PATCH 1/2] firmware: some documentation fixes
  2018-04-25 15:26     ` Greg KH
@ 2018-04-25 16:25       ` Andres Rodriguez
  2018-04-25 16:25         ` [PATCH 2/2] usb: typec: fix formatting errors that cause build breakage Andres Rodriguez
  2018-04-25 16:36         ` [PATCH 1/2] firmware: some documentation fixes Greg KH
  0 siblings, 2 replies; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-25 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

Including:
 - Fixup outdated kernel-doc paths
 - Slightly too short title underline
 - Some typos

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---

Hey Greg,

Here is the patch rebased on gregkh/driver-core-linus as requested.

Note that there is a patch by Hans de Goede <hdegoede@redhat.com> that contains
some of the path fixes as well.

I also included an extra patch to fix the htmldocs build. I think we are in time
to squash the fix into the offending commit.

Regards,
Andres

 Documentation/driver-api/firmware/request_firmware.rst | 6 +++---
 drivers/base/firmware_loader/fallback.c                | 4 ++--
 drivers/base/firmware_loader/fallback.h                | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
index 20f21ed427a5..d5ec95a7195b 100644
--- a/Documentation/driver-api/firmware/request_firmware.rst
+++ b/Documentation/driver-api/firmware/request_firmware.rst
@@ -50,11 +50,11 @@ Special optimizations on reboot
 Some devices have an optimization in place to enable the firmware to be
 retained during system reboot. When such optimizations are used the driver
 author must ensure the firmware is still available on resume from suspend,
-this can be done with firmware_request_cache() insted of requesting for the
-firmare to be loaded.
+this can be done with firmware_request_cache() instead of requesting for the
+firmware to be loaded.
 
 firmware_request_cache()
------------------------
+------------------------
 .. kernel-doc:: drivers/base/firmware_loader/main.c
    :functions: firmware_request_cache
 
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 31b5015b59fe..358354148dec 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -537,8 +537,8 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
 }
 
 /**
- * fw_load_sysfs_fallback - load a firmware via the syfs fallback mechanism
- * @fw_sysfs: firmware syfs information for the firmware to load
+ * 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/fallback.h b/drivers/base/firmware_loader/fallback.h
index dfebc644ed35..f8255670a663 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -6,7 +6,7 @@
 #include <linux/device.h>
 
 /**
- * struct firmware_fallback_config - firmware fallback configuratioon settings
+ * struct firmware_fallback_config - firmware fallback configuration settings
  *
  * Helps describe and fine tune the fallback mechanism.
  *
-- 
2.14.1

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

* [PATCH 2/2] usb: typec: fix formatting errors that cause build breakage
  2018-04-25 16:25       ` [PATCH 1/2] " Andres Rodriguez
@ 2018-04-25 16:25         ` Andres Rodriguez
  2018-04-25 16:35           ` Greg KH
  2018-04-25 16:36         ` [PATCH 1/2] firmware: some documentation fixes Greg KH
  1 sibling, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-25 16:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

The ASCII art illustration should be marked as a preformatted text
block. Otherwise the build will fail when attempting to parse it.

Fixes: bdecb33af34f ( "usb: typec: API for controlling USB ..." )
Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 Documentation/driver-api/usb/typec.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/driver-api/usb/typec.rst b/Documentation/driver-api/usb/typec.rst
index feb31946490b..48ff58095f11 100644
--- a/Documentation/driver-api/usb/typec.rst
+++ b/Documentation/driver-api/usb/typec.rst
@@ -210,7 +210,7 @@ If the connector is dual-role capable, there may also be a switch for the data
 role. USB Type-C Connector Class does not supply separate API for them. The
 port drivers can use USB Role Class API with those.
 
-Illustration of the muxes behind a connector that supports an alternate mode:
+Illustration of the muxes behind a connector that supports an alternate mode::
 
                      ------------------------
                      |       Connector      |
-- 
2.14.1

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

* Re: [PATCH 2/2] usb: typec: fix formatting errors that cause build breakage
  2018-04-25 16:25         ` [PATCH 2/2] usb: typec: fix formatting errors that cause build breakage Andres Rodriguez
@ 2018-04-25 16:35           ` Greg KH
  0 siblings, 0 replies; 31+ messages in thread
From: Greg KH @ 2018-04-25 16:35 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

On Wed, Apr 25, 2018 at 12:25:40PM -0400, Andres Rodriguez wrote:
> The ASCII art illustration should be marked as a preformatted text
> block. Otherwise the build will fail when attempting to parse it.
> 
> Fixes: bdecb33af34f ( "usb: typec: API for controlling USB ..." )
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---
>  Documentation/driver-api/usb/typec.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This is already in my usb tree and will go to Linus in a day or so.

thanks,

greg k-h

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

* Re: [PATCH 1/2] firmware: some documentation fixes
  2018-04-25 16:25       ` [PATCH 1/2] " Andres Rodriguez
  2018-04-25 16:25         ` [PATCH 2/2] usb: typec: fix formatting errors that cause build breakage Andres Rodriguez
@ 2018-04-25 16:36         ` Greg KH
  1 sibling, 0 replies; 31+ messages in thread
From: Greg KH @ 2018-04-25 16:36 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, mcgrof, alexdeucher, christian.koenig, kvalo,
	arend.vanspriel, linux-wireless, ath10k, hdegoede

On Wed, Apr 25, 2018 at 12:25:39PM -0400, Andres Rodriguez wrote:
> Including:
>  - Fixup outdated kernel-doc paths
>  - Slightly too short title underline
>  - Some typos
> 
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---
> 
> Hey Greg,
> 
> Here is the patch rebased on gregkh/driver-core-linus as requested.
> 
> Note that there is a patch by Hans de Goede <hdegoede@redhat.com> that contains
> some of the path fixes as well.
> 
> I also included an extra patch to fix the htmldocs build. I think we are in time
> to squash the fix into the offending commit.

We can't "squash" commits as our trees are not rebasable :)

thanks,

greg k-h

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

* Re: [PATCH 1/9] firmware: some documentation fixes
  2018-04-23 20:11 ` [PATCH 1/9] firmware: some documentation fixes Andres Rodriguez
  2018-04-25 15:25   ` Greg KH
@ 2018-05-03 23:31   ` Luis R. Rodriguez
  1 sibling, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-03 23:31 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede,
	Kees Cook

On Mon, Apr 23, 2018 at 04:11:57PM -0400, Andres Rodriguez wrote:
> Including:
>  - Fixup outdated kernel-doc paths
>  - Slightly too short title underline
>  - Some typos
> 
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>

This already got merged.

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

* Re: [PATCH 2/9] firmware: wrap FW_OPT_* into an enum
  2018-04-23 20:11 ` [PATCH 2/9] firmware: wrap FW_OPT_* into an enum Andres Rodriguez
@ 2018-05-03 23:35   ` Luis R. Rodriguez
  0 siblings, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-03 23:35 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede,
	Kees Cook

On Mon, Apr 23, 2018 at 04:11:58PM -0400, Andres Rodriguez wrote:
> This should let us associate enum kdoc to these values.
> 
> v2: use BIT() macro

No need to keep the changelog of series here, best to put them below as
I note.
> 
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---

Here is good to add changelog for series.

Anyway this patch can be merged with your next patch. I'll do
this myself and take it in my queue.

  Luis

>  drivers/base/firmware_loader/fallback.c | 12 ++++++------
>  drivers/base/firmware_loader/fallback.h |  6 ++++--
>  drivers/base/firmware_loader/firmware.h | 18 ++++++++++--------
>  drivers/base/firmware_loader/main.c     |  6 +++---
>  4 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index da97fc26119c..ecc49a619298 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -511,7 +511,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;
> @@ -544,7 +544,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;
> @@ -598,7 +598,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;
> @@ -639,7 +639,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;
> @@ -648,7 +648,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");
> @@ -663,7 +663,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 d11d37db370b..b252bfa82295 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,14 @@
>  
>  #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 {
> +	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 +112,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 5812148694b6..5baadad3012d 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
>  _firmware_request(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 firmware_request_work_func(struct work_struct *work)
> -- 
> 2.14.1
> 
> 

-- 
Do not panic

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

* Re: [PATCH 3/9] firmware: add kernel-doc for enum fw_opt
  2018-04-23 20:11 ` [PATCH 3/9] firmware: add kernel-doc for enum fw_opt Andres Rodriguez
@ 2018-05-03 23:36   ` Luis R. Rodriguez
  0 siblings, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-03 23:36 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede

On Mon, Apr 23, 2018 at 04:11:59PM -0400, Andres Rodriguez wrote:
> Some basic definitions for the FW_OPT_* values
> 
> v2: Documentation corrections from Luis.

Likewise.

> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---
>  drivers/base/firmware_loader/firmware.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
> index b252bfa82295..a405d400a925 100644
> --- a/drivers/base/firmware_loader/firmware.h
> +++ b/drivers/base/firmware_loader/firmware.h
> @@ -11,6 +11,26 @@
>  
>  #include <generated/utsrelease.h>
>  
> +/**
> + * 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.

The style here is a bit off. I'll just merge this patch with the last one
an change the style a bit to match expectations.

  Luis

> + * @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),
> -- 
> 2.14.1
> 
> 

-- 
Do not panic

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

* Re: [PATCH 4/9] firmware: use () to terminate kernel-doc function names
  2018-04-23 20:12 ` [PATCH 4/9] firmware: use () to terminate kernel-doc function names Andres Rodriguez
@ 2018-05-03 23:37   ` Luis R. Rodriguez
  0 siblings, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-03 23:37 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede,
	Kees Cook

On Mon, Apr 23, 2018 at 04:12:00PM -0400, Andres Rodriguez wrote:
> 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>

0-day never got back to me about my full sweep API rename so I think your
changes are best to go in first, I'll do some small changes to account for this
and resubmit to Greg once 0-day gives me its blessing.

  Luis

> ---
>  drivers/base/firmware_loader/fallback.c |  8 ++++----
>  drivers/base/firmware_loader/main.c     | 22 +++++++++++-----------
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index ecc49a619298..e75928458489 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -124,7 +124,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
> @@ -238,7 +238,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
> @@ -430,7 +430,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
> @@ -536,7 +536,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 5baadad3012d..d028cd3257f7 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -597,7 +597,7 @@ _firmware_request(const struct firmware **firmware_p, const char *name,
>  }
>  
>  /**
> - * firmware_request: - send firmware request and wait for it
> + * firmware_request() - 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
> @@ -632,7 +632,7 @@ firmware_request(const struct firmware **firmware_p, const char *name,
>  EXPORT_SYMBOL(firmware_request);
>  
>  /**
> - * firmware_request_direct: - load firmware directly without usermode helper
> + * firmware_request_direct() - load firmware directly without usermode helper
>   * @firmware_p: pointer to firmware image
>   * @name: name of firmware file
>   * @device: device for which firmware is being loaded
> @@ -657,7 +657,7 @@ int firmware_request_direct(const struct firmware **firmware_p,
>  EXPORT_SYMBOL_GPL(firmware_request_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);
>  
>  /**
> - * firmware_request_into_buf - load firmware into a previously allocated buffer
> + * firmware_request_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 @@ firmware_request_into_buf(const struct firmware **firmware_p, const char *name,
>  EXPORT_SYMBOL(firmware_request_into_buf);
>  
>  /**
> - * firmware_release: - release the resource associated with a firmware image
> + * firmware_release() - release the resource associated with a firmware image
>   * @fw: firmware resource to release
>   **/
>  void firmware_release(const struct firmware *fw)
> @@ -755,7 +755,7 @@ static void firmware_request_work_func(struct work_struct *work)
>  }
>  
>  /**
> - * firmware_request_nowait - asynchronous version of firmware_request
> + * firmware_request_nowait() - asynchronous version of firmware_request
>   * @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(firmware_request_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 firmware_request 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.14.1
> 
> 

-- 
Do not panic

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

* Re: [PATCH 5/9] firmware: add function to load firmware without warnings v5
  2018-04-23 20:12 ` [PATCH 5/9] firmware: add function to load firmware without warnings v5 Andres Rodriguez
@ 2018-05-03 23:40   ` Luis R. Rodriguez
  2018-05-04  0:00   ` Luis R. Rodriguez
  1 sibling, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-03 23:40 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede,
	Kees Cook

On Mon, Apr 23, 2018 at 04:12:01PM -0400, Andres Rodriguez wrote:
> Currently the firmware loader only exposes one silent path for querying
> optional firmware, and that is firmware_request_direct(). This function
> also disables the fallback path, which might not always be the
> desired behaviour. [0]
> 
> This patch introduces a variations of firmware_request() that enable the
> caller to disable the undesired warning messages. This is equivalent to
> adding FW_OPT_NO_WARN to the old behaviour.
> 
> v2: add header prototype, use updated opt_flags
> v3: split debug message to separate patch
>     added _nowait variant
>     added documentation
> v4: fix kernel-doc style
> v5: drop _nowait for now, since we lack agreement on the API
> 
> [0]: https://git.kernel.org/linus/c0cc00f250e1
> 
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---
>  .../driver-api/firmware/request_firmware.rst       |  5 +++++
>  drivers/base/firmware_loader/main.c                | 24 ++++++++++++++++++++++
>  include/linux/firmware.h                           |  2 ++
>  3 files changed, 31 insertions(+)
> 
> diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst
> index 7632f8807472..c8bddbdcfd10 100644
> --- a/Documentation/driver-api/firmware/request_firmware.rst
> +++ b/Documentation/driver-api/firmware/request_firmware.rst
> @@ -20,6 +20,11 @@ firmware_request
>  .. kernel-doc:: drivers/base/firmware_loader/main.c
>     :functions: firmware_request
>  
> +firmware_request_nowarn
> +-----------------------
> +.. kernel-doc:: drivers/base/firmware_loader/main.c
> +   :functions: firmware_request_nowarn
> +
>  firmware_request_direct
>  -----------------------
>  .. kernel-doc:: drivers/base/firmware_loader/main.c
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index d028cd3257f7..58aaadf81e12 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -631,6 +631,30 @@ firmware_request(const struct firmware **firmware_p, const char *name,
>  }
>  EXPORT_SYMBOL(firmware_request);
>  
> +/**
> + * firmware_request_nowarn() - request for an optional fw module
> + * @firmware: pointer to firmware image
> + * @name: name of firmware file
> + * @device: device for which firmware is being loaded
> + *
> + * This function is similar in behaviour to firmware_request(), except
> + * it doesn't produce warning messages when the file is not found.

It doesn't explain the difference between this and firmware_request_direct()
I'll go ahead and add this and expand also on firmware_request_direct() to be
clearer on the differences to a reader.

I'll queue this up and resubmit myself.

  Luis

> + **/
> +int
> +firmware_request_nowarn(const struct firmware **firmware, const char *name,
> +			struct device *device)
> +{
> +	int ret;
> +
> +	/* Need to pin this module until return */
> +	__module_get(THIS_MODULE);
> +	ret = _firmware_request(firmware, name, device, NULL, 0,
> +				FW_OPT_UEVENT | FW_OPT_NO_WARN);
> +	module_put(THIS_MODULE);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(firmware_request_nowarn);
> +
>  /**
>   * firmware_request_direct() - load firmware directly without usermode helper
>   * @firmware_p: pointer to firmware image
> diff --git a/include/linux/firmware.h b/include/linux/firmware.h
> index db8351a42405..a34e16f77f20 100644
> --- a/include/linux/firmware.h
> +++ b/include/linux/firmware.h
> @@ -42,6 +42,8 @@ struct builtin_fw {
>  #if defined(CONFIG_FW_LOADER) || (defined(CONFIG_FW_LOADER_MODULE) && defined(MODULE))
>  int firmware_request(const struct firmware **fw, const char *name,
>  		     struct device *device);
> +int firmware_request_nowarn(const struct firmware **fw, const char *name,
> +			    struct device *device);
>  int firmware_request_nowait(
>  	struct module *module, bool uevent,
>  	const char *name, struct device *device, gfp_t gfp, void *context,
> -- 
> 2.14.1
> 
> 

-- 
Do not panic

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

* Re: [PATCH 6/9] firmware: print firmware name on fallback path
  2018-04-23 20:12 ` [PATCH 6/9] firmware: print firmware name on fallback path Andres Rodriguez
@ 2018-05-03 23:42   ` Luis R. Rodriguez
  2018-05-05  2:57     ` Andres Rodriguez
  2018-05-12  8:03     ` Kalle Valo
  0 siblings, 2 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-03 23:42 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede,
	Kees Cook, Mimi Zohar

On Mon, Apr 23, 2018 at 04:12:02PM -0400, Andres Rodriguez wrote:
> Previously, one could assume the firmware name from the preceding
> message: "Direct firmware load for {name} failed with error %d".
> 
> However, with the new firmware_request_nowarn() entrypoint, the message
> outlined above will not always be printed.

I though the whole point was to not print an error message. What if
we want later to disable this error message? This would prove a bit
pointless.

Let's discuss the exact semantics desired here. Why would only the
fallback be desirable here?

Andres, Kalle?

After we address this I'll address resubmitting this lat patch
along with the last one. For now I'll skip it.

  Luis

> Therefore, we add the firmware name to the fallback path spew in order
> to associate it with the appropriate request.
> 
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---
>  drivers/base/firmware_loader/fallback.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index e75928458489..1a47ddc70c31 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -669,6 +669,6 @@ int fw_sysfs_fallback(struct firmware *fw, const char *name,
>  	if (!fw_run_sysfs_fallback(opt_flags))
>  		return ret;
>  
> -	dev_warn(device, "Falling back to user helper\n");
> +	dev_warn(device, "Falling back to user helper for %s\n", name);
>  	return fw_load_from_user_helper(fw, name, device, opt_flags);
>  }
> -- 
> 2.14.1
> 
> 

-- 
Do not panic

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

* Re: [PATCH 7/9] firmware: use rename fw_sysfs_fallback to use the firmware_ prefix
  2018-04-23 20:12 ` [PATCH 7/9] firmware: use rename fw_sysfs_fallback to use the firmware_ prefix Andres Rodriguez
@ 2018-05-03 23:44   ` Luis R. Rodriguez
  0 siblings, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-03 23:44 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede,
	Kees Cook

On Mon, Apr 23, 2018 at 04:12:03PM -0400, Andres Rodriguez wrote:
> Use the correct prefix for symbols exported by firmware_loader(). This
> is done since firmware_sysfs_fallback() is now exposed through
> kernel-doc.
> 
> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
> ---
>  drivers/base/firmware_loader/fallback.c | 8 ++++----
>  drivers/base/firmware_loader/fallback.h | 4 ++--
>  drivers/base/firmware_loader/firmware.h | 6 +++---
>  drivers/base/firmware_loader/main.c     | 2 +-
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index 1a47ddc70c31..fc186b5bccce 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -661,10 +661,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_sysfs_fallback(struct firmware *fw, const char *name,
> +			    struct device *device,
> +			    enum fw_opt opt_flags,
> +			    int ret)

Since we may get more than one fallback later I'll usee the firmware_fallback_sysfs() here.
I'll do this change and resubmit myself.

>  {
>  	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..8cfaa3299bb7 100644
> --- a/drivers/base/firmware_loader/fallback.h
> +++ b/drivers/base/firmware_loader/fallback.h
> @@ -31,7 +31,7 @@ struct firmware_fallback_config {
>  };
>  
>  #ifdef CONFIG_FW_LOADER_USER_HELPER
> -int fw_sysfs_fallback(struct firmware *fw, const char *name,
> +int firmware_sysfs_fallback(struct firmware *fw, const char *name,
>  		      struct device *device,
>  		      enum fw_opt opt_flags,
>  		      int ret);
> @@ -43,7 +43,7 @@ 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,
> +static inline int firmware_sysfs_fallback(struct firmware *fw, const char *name,
>  				    struct device *device,
>  				    enum fw_opt opt_flags,
>  				    int ret)
> diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
> index a405d400a925..59836d50c5bd 100644
> --- a/drivers/base/firmware_loader/firmware.h
> +++ b/drivers/base/firmware_loader/firmware.h
> @@ -15,12 +15,12 @@
>   * 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.
> + *                 when the firmware is not found. Userspace is in charge
> + *                 to load the firmware using the sysfs loading facility.

This change was really not part of this patch, so I'll merge it in with the
other patch.

>   * @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().
> + *                     For details refer to firmware_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
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index 58aaadf81e12..f009566acd35 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -581,7 +581,7 @@ _firmware_request(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_sysfs_fallback(fw, name, device, opt_flags, ret);
>  	} else
>  		ret = assign_fw(fw, device, opt_flags);
>  
> -- 
> 2.14.1
> 
> 

-- 
Do not panic

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

* Re: [PATCH 5/9] firmware: add function to load firmware without warnings v5
  2018-04-23 20:12 ` [PATCH 5/9] firmware: add function to load firmware without warnings v5 Andres Rodriguez
  2018-05-03 23:40   ` Luis R. Rodriguez
@ 2018-05-04  0:00   ` Luis R. Rodriguez
  1 sibling, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-04  0:00 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: linux-kernel, gregkh, mcgrof, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede,
	Kees Cook, Mimi Zohar

On Mon, Apr 23, 2018 at 04:12:01PM -0400, Andres Rodriguez wrote:
> diff --git a/include/linux/firmware.h b/include/linux/firmware.h
> index db8351a42405..a34e16f77f20 100644
> --- a/include/linux/firmware.h
> +++ b/include/linux/firmware.h
> @@ -42,6 +42,8 @@ struct builtin_fw {
>  #if defined(CONFIG_FW_LOADER) || (defined(CONFIG_FW_LOADER_MODULE) && defined(MODULE))
>  int firmware_request(const struct firmware **fw, const char *name,
>  		     struct device *device);
> +int firmware_request_nowarn(const struct firmware **fw, const char *name,
> +			    struct device *device);
>  int firmware_request_nowait(
>  	struct module *module, bool uevent,
>  	const char *name, struct device *device, gfp_t gfp, void *context,

You also missed the firmware_request_nowarn() call on the #else. I'll add it
and re-submit myself.

In future patches about firmware please also Cc Mimi Zohar <zohar@linux.vnet.ibm.com>,
and Kees Cook <keescook@chromium.org>. You can also use the long list (modulo, not
the EFI list) that Hans used on his EFI patches. I realize its long but its just
to ensure enough folks get to review and eybeball the code.

  Luis

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

* Re: [PATCH 6/9] firmware: print firmware name on fallback path
  2018-05-03 23:42   ` Luis R. Rodriguez
@ 2018-05-05  2:57     ` Andres Rodriguez
  2018-05-08  0:20       ` Luis R. Rodriguez
  2018-05-12  8:03     ` Kalle Valo
  1 sibling, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-05-05  2:57 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: andresx7, linux-kernel, gregkh, alexdeucher, christian.koenig,
	kvalo, arend.vanspriel, linux-wireless, ath10k, hdegoede,
	Kees Cook, Mimi Zohar



On 2018-05-03 07:42 PM, Luis R. Rodriguez wrote:
> On Mon, Apr 23, 2018 at 04:12:02PM -0400, Andres Rodriguez wrote:
>> Previously, one could assume the firmware name from the preceding
>> message: "Direct firmware load for {name} failed with error %d".
>>
>> However, with the new firmware_request_nowarn() entrypoint, the message
>> outlined above will not always be printed.
> 
> I though the whole point was to not print an error message. What if
> we want later to disable this error message? This would prove a bit
> pointless.
> 
> Let's discuss the exact semantics desired here. Why would only the
> fallback be desirable here?
> 
> Andres, Kalle?
> 
> After we address this I'll address resubmitting this lat patch
> along with the last one. For now I'll skip it.

You are correct. I initially thought it would be useful to know that the 
usermode fallback was being triggered. And for that message to be useful 
we would need a fw name.

But now that you point it out, this behaviour is inconsistent with the 
_nowarn() definition. We shouldn't have a message in the first place.

So it might be better to instead have:

if (!(opt_flags & FW_OPT_NO_WARN) )
     dev_warn(device, "Falling back to user helper\n");

No need to add the firmware name, cause we either:
     a) FW_OPT_NO_WARN is set and no messages are printed, or
     b) FW_OPT_NO_WARN is not set and we get both messages.

Yay, nay?

Regards,
Andres

> 
>    Luis
> 
>> Therefore, we add the firmware name to the fallback path spew in order
>> to associate it with the appropriate request.
>>
>> Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
>> ---
>>   drivers/base/firmware_loader/fallback.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
>> index e75928458489..1a47ddc70c31 100644
>> --- a/drivers/base/firmware_loader/fallback.c
>> +++ b/drivers/base/firmware_loader/fallback.c
>> @@ -669,6 +669,6 @@ int fw_sysfs_fallback(struct firmware *fw, const char *name,
>>   	if (!fw_run_sysfs_fallback(opt_flags))
>>   		return ret;
>>   
>> -	dev_warn(device, "Falling back to user helper\n");
>> +	dev_warn(device, "Falling back to user helper for %s\n", name);
>>   	return fw_load_from_user_helper(fw, name, device, opt_flags);
>>   }
>> -- 
>> 2.14.1
>>
>>
> 

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

* Re: [PATCH 6/9] firmware: print firmware name on fallback path
  2018-05-05  2:57     ` Andres Rodriguez
@ 2018-05-08  0:20       ` Luis R. Rodriguez
  0 siblings, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-08  0:20 UTC (permalink / raw)
  To: Andres Rodriguez
  Cc: Luis R. Rodriguez, linux-kernel, gregkh, alexdeucher,
	christian.koenig, kvalo, arend.vanspriel, linux-wireless, ath10k,
	hdegoede, Kees Cook, Mimi Zohar

On Fri, May 04, 2018 at 10:57:26PM -0400, Andres Rodriguez wrote:
> On 2018-05-03 07:42 PM, Luis R. Rodriguez wrote:
> > On Mon, Apr 23, 2018 at 04:12:02PM -0400, Andres Rodriguez wrote:
> > > Previously, one could assume the firmware name from the preceding
> > > message: "Direct firmware load for {name} failed with error %d".
> > > 
> > > However, with the new firmware_request_nowarn() entrypoint, the message
> > > outlined above will not always be printed.
> > 
> > I though the whole point was to not print an error message. What if
> > we want later to disable this error message? This would prove a bit
> > pointless.
> > 
> > Let's discuss the exact semantics desired here. Why would only the
> > fallback be desirable here?
> > 
> > Andres, Kalle?
> > 
> > After we address this I'll address resubmitting this lat patch
> > along with the last one. For now I'll skip it.
> 
> You are correct. I initially thought it would be useful to know that the
> usermode fallback was being triggered. And for that message to be useful we
> would need a fw name.
> 
> But now that you point it out, this behaviour is inconsistent with the
> _nowarn() definition. We shouldn't have a message in the first place.
> 
> So it might be better to instead have:
> 
> if (!(opt_flags & FW_OPT_NO_WARN) )
>     dev_warn(device, "Falling back to user helper\n");
> 
> No need to add the firmware name, cause we either:
>     a) FW_OPT_NO_WARN is set and no messages are printed, or
>     b) FW_OPT_NO_WARN is not set and we get both messages.
> 
> Yay, nay?

I welcome such a new warning but not for any of the reasons stated.

It make sense if FW_OPT_NO_WARN is not set and only because the fallback
mechanism can fail for a slew of different firmware files, and just getting
informed a failure with a fallback occurred does not tell us for which file it
failed for.

I'll add such a patch to my queue and send it off soon prior to your own
new API nowarn call.

  Luis

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

* Re: [PATCH 6/9] firmware: print firmware name on fallback path
  2018-05-03 23:42   ` Luis R. Rodriguez
  2018-05-05  2:57     ` Andres Rodriguez
@ 2018-05-12  8:03     ` Kalle Valo
  2018-05-12  9:06       ` Luis R. Rodriguez
  1 sibling, 1 reply; 31+ messages in thread
From: Kalle Valo @ 2018-05-12  8:03 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: Andres Rodriguez, linux-kernel, gregkh, alexdeucher,
	christian.koenig, arend.vanspriel, linux-wireless, ath10k,
	hdegoede, Kees Cook, Mimi Zohar

(sorry for the delay, this got buried in my inbox)

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

> On Mon, Apr 23, 2018 at 04:12:02PM -0400, Andres Rodriguez wrote:
>> Previously, one could assume the firmware name from the preceding
>> message: "Direct firmware load for {name} failed with error %d".
>> 
>> However, with the new firmware_request_nowarn() entrypoint, the message
>> outlined above will not always be printed.
>
> I though the whole point was to not print an error message. What if
> we want later to disable this error message? This would prove a bit
> pointless.
>
> Let's discuss the exact semantics desired here. Why would only the
> fallback be desirable here?
>
> Andres, Kalle?

So from ath10k point of view we do not want to have any messages printed
when calling firmware_request_nowarn(). The warnings get users really
confused when ath10k is checking if an optional firmware file is
available or not.

-- 
Kalle Valo

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

* Re: [PATCH 6/9] firmware: print firmware name on fallback path
  2018-05-12  8:03     ` Kalle Valo
@ 2018-05-12  9:06       ` Luis R. Rodriguez
  0 siblings, 0 replies; 31+ messages in thread
From: Luis R. Rodriguez @ 2018-05-12  9:06 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Luis R. Rodriguez, Andres Rodriguez, linux-kernel, gregkh,
	alexdeucher, christian.koenig, arend.vanspriel, linux-wireless,
	ath10k, hdegoede, Kees Cook, Mimi Zohar

On Sat, May 12, 2018 at 11:03:52AM +0300, Kalle Valo wrote:
> (sorry for the delay, this got buried in my inbox)
> 
> "Luis R. Rodriguez" <mcgrof@kernel.org> writes:
> 
> > On Mon, Apr 23, 2018 at 04:12:02PM -0400, Andres Rodriguez wrote:
> >> Previously, one could assume the firmware name from the preceding
> >> message: "Direct firmware load for {name} failed with error %d".
> >> 
> >> However, with the new firmware_request_nowarn() entrypoint, the message
> >> outlined above will not always be printed.
> >
> > I though the whole point was to not print an error message. What if
> > we want later to disable this error message? This would prove a bit
> > pointless.
> >
> > Let's discuss the exact semantics desired here. Why would only the
> > fallback be desirable here?
> >
> > Andres, Kalle?
> 
> So from ath10k point of view we do not want to have any messages printed
> when calling firmware_request_nowarn(). The warnings get users really
> confused when ath10k is checking if an optional firmware file is
> available or not.

I figured, that is the intended functionality now.

  Luis

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

* Re: [PATCH 4/9] firmware: use () to terminate kernel-doc function names
  2018-04-17 15:33 ` [PATCH 4/9] firmware: use () to terminate kernel-doc function names Andres Rodriguez
@ 2018-04-17 20:56   ` Randy Dunlap
  0 siblings, 0 replies; 31+ messages in thread
From: Randy Dunlap @ 2018-04-17 20:56 UTC (permalink / raw)
  To: Andres Rodriguez, linux-kernel
  Cc: gregkh, mcgrof, alexdeucher, ckoenig.leichtzumerken, kvalo,
	arend.vanspriel

On 04/17/18 08:33, Andres Rodriguez wrote:
> 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>

Thanks.

> ---
>  drivers/base/firmware_loader/fallback.c |  8 ++++----
>  drivers/base/firmware_loader/main.c     | 22 +++++++++++-----------
>  2 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
> index ecc49a619298..e75928458489 100644
> --- a/drivers/base/firmware_loader/fallback.c
> +++ b/drivers/base/firmware_loader/fallback.c
> @@ -124,7 +124,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
> @@ -238,7 +238,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
> @@ -430,7 +430,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
> @@ -536,7 +536,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 5baadad3012d..d028cd3257f7 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -597,7 +597,7 @@ _firmware_request(const struct firmware **firmware_p, const char *name,
>  }
>  
>  /**
> - * firmware_request: - send firmware request and wait for it
> + * firmware_request() - 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
> @@ -632,7 +632,7 @@ firmware_request(const struct firmware **firmware_p, const char *name,
>  EXPORT_SYMBOL(firmware_request);
>  
>  /**
> - * firmware_request_direct: - load firmware directly without usermode helper
> + * firmware_request_direct() - load firmware directly without usermode helper
>   * @firmware_p: pointer to firmware image
>   * @name: name of firmware file
>   * @device: device for which firmware is being loaded
> @@ -657,7 +657,7 @@ int firmware_request_direct(const struct firmware **firmware_p,
>  EXPORT_SYMBOL_GPL(firmware_request_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);
>  
>  /**
> - * firmware_request_into_buf - load firmware into a previously allocated buffer
> + * firmware_request_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 @@ firmware_request_into_buf(const struct firmware **firmware_p, const char *name,
>  EXPORT_SYMBOL(firmware_request_into_buf);
>  
>  /**
> - * firmware_release: - release the resource associated with a firmware image
> + * firmware_release() - release the resource associated with a firmware image
>   * @fw: firmware resource to release
>   **/
>  void firmware_release(const struct firmware *fw)
> @@ -755,7 +755,7 @@ static void firmware_request_work_func(struct work_struct *work)
>  }
>  
>  /**
> - * firmware_request_nowait - asynchronous version of firmware_request
> + * firmware_request_nowait() - asynchronous version of firmware_request
>   * @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(firmware_request_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 firmware_request 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
> 


-- 
~Randy

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

* [PATCH 4/9] firmware: use () to terminate kernel-doc function names
  2018-04-17 15:32 [PATCH 0/9] Loading optional firmware v3 Andres Rodriguez
@ 2018-04-17 15:33 ` Andres Rodriguez
  2018-04-17 20:56   ` Randy Dunlap
  0 siblings, 1 reply; 31+ messages in thread
From: Andres Rodriguez @ 2018-04-17 15:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: andresx7, gregkh, mcgrof, alexdeucher, ckoenig.leichtzumerken,
	kvalo, arend.vanspriel

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

Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
---
 drivers/base/firmware_loader/fallback.c |  8 ++++----
 drivers/base/firmware_loader/main.c     | 22 +++++++++++-----------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index ecc49a619298..e75928458489 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -124,7 +124,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
@@ -238,7 +238,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
@@ -430,7 +430,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
@@ -536,7 +536,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 5baadad3012d..d028cd3257f7 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -597,7 +597,7 @@ _firmware_request(const struct firmware **firmware_p, const char *name,
 }
 
 /**
- * firmware_request: - send firmware request and wait for it
+ * firmware_request() - 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
@@ -632,7 +632,7 @@ firmware_request(const struct firmware **firmware_p, const char *name,
 EXPORT_SYMBOL(firmware_request);
 
 /**
- * firmware_request_direct: - load firmware directly without usermode helper
+ * firmware_request_direct() - load firmware directly without usermode helper
  * @firmware_p: pointer to firmware image
  * @name: name of firmware file
  * @device: device for which firmware is being loaded
@@ -657,7 +657,7 @@ int firmware_request_direct(const struct firmware **firmware_p,
 EXPORT_SYMBOL_GPL(firmware_request_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);
 
 /**
- * firmware_request_into_buf - load firmware into a previously allocated buffer
+ * firmware_request_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 @@ firmware_request_into_buf(const struct firmware **firmware_p, const char *name,
 EXPORT_SYMBOL(firmware_request_into_buf);
 
 /**
- * firmware_release: - release the resource associated with a firmware image
+ * firmware_release() - release the resource associated with a firmware image
  * @fw: firmware resource to release
  **/
 void firmware_release(const struct firmware *fw)
@@ -755,7 +755,7 @@ static void firmware_request_work_func(struct work_struct *work)
 }
 
 /**
- * firmware_request_nowait - asynchronous version of firmware_request
+ * firmware_request_nowait() - asynchronous version of firmware_request
  * @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(firmware_request_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 firmware_request 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.14.1

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

end of thread, other threads:[~2018-05-12  9:06 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-23 20:11 [PATCH 0/9] Loading optional firmware v4 Andres Rodriguez
2018-04-23 20:11 ` [PATCH 1/9] firmware: some documentation fixes Andres Rodriguez
2018-04-25 15:25   ` Greg KH
2018-04-25 15:26     ` Greg KH
2018-04-25 16:25       ` [PATCH 1/2] " Andres Rodriguez
2018-04-25 16:25         ` [PATCH 2/2] usb: typec: fix formatting errors that cause build breakage Andres Rodriguez
2018-04-25 16:35           ` Greg KH
2018-04-25 16:36         ` [PATCH 1/2] firmware: some documentation fixes Greg KH
2018-05-03 23:31   ` [PATCH 1/9] " Luis R. Rodriguez
2018-04-23 20:11 ` [PATCH 2/9] firmware: wrap FW_OPT_* into an enum Andres Rodriguez
2018-05-03 23:35   ` Luis R. Rodriguez
2018-04-23 20:11 ` [PATCH 3/9] firmware: add kernel-doc for enum fw_opt Andres Rodriguez
2018-05-03 23:36   ` Luis R. Rodriguez
2018-04-23 20:12 ` [PATCH 4/9] firmware: use () to terminate kernel-doc function names Andres Rodriguez
2018-05-03 23:37   ` Luis R. Rodriguez
2018-04-23 20:12 ` [PATCH 5/9] firmware: add function to load firmware without warnings v5 Andres Rodriguez
2018-05-03 23:40   ` Luis R. Rodriguez
2018-05-04  0:00   ` Luis R. Rodriguez
2018-04-23 20:12 ` [PATCH 6/9] firmware: print firmware name on fallback path Andres Rodriguez
2018-05-03 23:42   ` Luis R. Rodriguez
2018-05-05  2:57     ` Andres Rodriguez
2018-05-08  0:20       ` Luis R. Rodriguez
2018-05-12  8:03     ` Kalle Valo
2018-05-12  9:06       ` Luis R. Rodriguez
2018-04-23 20:12 ` [PATCH 7/9] firmware: use rename fw_sysfs_fallback to use the firmware_ prefix Andres Rodriguez
2018-05-03 23:44   ` Luis R. Rodriguez
2018-04-23 20:12 ` [PATCH 8/9] ath10k: use request_firmware_nowarn to load firmware Andres Rodriguez
2018-04-23 20:12 ` [PATCH 9/9] ath10k: re-enable the firmware fallback mechanism for testmode Andres Rodriguez
2018-04-24  5:29   ` Kalle Valo
  -- strict thread matches above, loose matches on Subject: below --
2018-04-17 15:32 [PATCH 0/9] Loading optional firmware v3 Andres Rodriguez
2018-04-17 15:33 ` [PATCH 4/9] firmware: use () to terminate kernel-doc function names Andres Rodriguez
2018-04-17 20:56   ` Randy Dunlap

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).