Loading optional firmware v4
Subsystems (6)
cover Loading optional firmware v4
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(-)
Signature · 1 line
2.14.1
PATCH 1/9 firmware: some documentation fixes
Including:
- Fixup outdated kernel-doc paths
- Slightly too short title underline
- Some typos
--- 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(-)
Documentation/driver-api/firmware/request_firmware.rst +8 -8
@@ -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
drivers/base/firmware_loader/fallback.c +2 -2
@@ -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 *
drivers/base/firmware_loader/fallback.h +1 -1
@@ -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. *
Signature · 1 line
2.14.1
Greg KH
maintainerRe: [PATCH 1/9] firmware: some documentation fixes
Quoted 11 lines · Andres Rodriguez
Including:
- Fixup outdated kernel-doc paths
- Slightly too short title underline
- Some typosSigned-off-by Andres Rodriguez <[email protected]>--- 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
Greg KH
maintainerRe: [PATCH 1/9] firmware: some documentation fixes
Quoted 13 lines · Greg KH
Quoted 11 lines · Andres Rodriguez
Including:
- Fixup outdated kernel-doc paths
- Slightly too short title underline
- Some typosSigned-off-by Andres Rodriguez <[email protected]>--- 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(-)
Quoted 2 lines
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
PATCH 1/2 firmware: some documentation fixes
Including:
- Fixup outdated kernel-doc paths
- Slightly too short title underline
- Some typos
---
Hey Greg,
Here is the patch rebased on gregkh/driver-core-linus as requested.
Note that there is a patch by Hans de Goede <[email protected]> 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(-)
Documentation/driver-api/firmware/request_firmware.rst +3 -3
@@ -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
drivers/base/firmware_loader/fallback.c +2 -2
@@ -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 *
drivers/base/firmware_loader/fallback.h +1 -1
@@ -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. *
Signature · 1 line
2.14.1
PATCH 2/2 usb: typec: fix formatting errors that cause build breakage
The ASCII art illustration should be marked as a preformatted text block. Otherwise the build will fail when attempting to parse it.
--- Documentation/driver-api/usb/typec.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/driver-api/usb/typec.rst +1 -1
@@ -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 |
Signature · 1 line
2.14.1
Greg KH
maintainerRe: [PATCH 2/2] usb: typec: fix formatting errors that cause build breakage
Quoted 8 lines · Andres Rodriguez
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 <[email protected]>--- 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
Greg KH
maintainerRe: [PATCH 1/2] firmware: some documentation fixes
In reply to Andres RodriguezQuoted 10 lines · Andres Rodriguez
Including:
- Fixup outdated kernel-doc paths
- Slightly too short title underline
- Some typosSigned-off-by Andres Rodriguez <[email protected]>---
Hey Greg,
Quoted 7 lines
Here is the patch rebased on gregkh/driver-core-linus as requested.
Note that there is a patch by Hans de Goede <[email protected]> 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
Luis R. Rodriguez
maintainerRe: [PATCH 1/9] firmware: some documentation fixes
In reply to Andres RodriguezQuoted 6 lines · Andres Rodriguez
Including:
- Fixup outdated kernel-doc paths
- Slightly too short title underline
- Some typosSigned-off-by Andres Rodriguez <[email protected]>
This already got merged.
PATCH 2/9 firmware: wrap FW_OPT_* into an enum
In reply to Andres RodriguezThis should let us associate enum kdoc to these values.
v2: use BIT() macro
--- 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(-)
drivers/base/firmware_loader/fallback.c +6 -6
@@ -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))
drivers/base/firmware_loader/fallback.h +4 -2
@@ -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 */
drivers/base/firmware_loader/firmware.h +10 -8
@@ -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 */
drivers/base/firmware_loader/main.c +3 -3
@@ -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)
Signature · 1 line
2.14.1
Luis R. Rodriguez
maintainerRe: [PATCH 2/9] firmware: wrap FW_OPT_* into an enum
Quoted 3 lines · Andres Rodriguez
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.
Quoted 3 lines
Signed-off-by Andres Rodriguez <[email protected]>---
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
Quoted 172 lines
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(-)drivers/base/firmware_loader/fallback.c +6
-6@@ -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))drivers/base/firmware_loader/fallback.h +4
-2@@ -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 */ drivers/base/firmware_loader/firmware.h +10
-8@@ -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 */ drivers/base/firmware_loader/main.c +3
-3@@ -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)Signature · 1 line
2.14.1
Signature · 1 line
Do not panic
PATCH 3/9 firmware: add kernel-doc for enum fw_opt
In reply to Andres RodriguezSome basic definitions for the FW_OPT_* values
v2: Documentation corrections from Luis.
--- drivers/base/firmware_loader/firmware.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
drivers/base/firmware_loader/firmware.h +20 -0
@@ -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 ),
Signature · 1 line
2.14.1
Luis R. Rodriguez
maintainerRe: [PATCH 3/9] firmware: add kernel-doc for enum fw_opt
Quoted 3 lines · Andres Rodriguez
Some basic definitions for the FW_OPT_* values
v2: Documentation corrections from Luis.
Likewise.
Quoted 10 lines
Signed-off-by Andres Rodriguez <[email protected]>--- drivers/base/firmware_loader/firmware.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)drivers/base/firmware_loader/firmware.h +0
-0
drivers/base/firmware_loader/firmware.h line 19
Quoted 9 lines
drivers/base/firmware_loader/firmware.h +6
-0@@ -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
Quoted 21 lines
drivers/base/firmware_loader/firmware.h +14
-0+* @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 ),Signature · 1 line
2.14.1
Signature · 1 line
Do not panic
PATCH 4/9 firmware: use () to terminate kernel-doc function names
In reply to Andres RodriguezThe kernel-doc spec dictates a function name ends in ().
--- drivers/base/firmware_loader/fallback.c | 8 ++++---- drivers/base/firmware_loader/main.c | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-)
drivers/base/firmware_loader/fallback.c +4 -4
@@ -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
drivers/base/firmware_loader/main.c +11 -11
@@ -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
Signature · 1 line
2.14.1
Luis R. Rodriguez
maintainerRe: [PATCH 4/9] firmware: use () to terminate kernel-doc function names
Quoted 4 lines · Andres Rodriguez
The kernel-doc spec dictates a function name ends in ().
Signed-off-by Andres Rodriguez <[email protected]>Acked-by Randy Dunlap <[email protected]>
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
Quoted 152 lines
--- drivers/base/firmware_loader/fallback.c | 8 ++++---- drivers/base/firmware_loader/main.c | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-)drivers/base/firmware_loader/fallback.c +4
-4@@ -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 drivers/base/firmware_loader/main.c +11
-11@@ -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 Signature · 1 line
2.14.1
Signature · 1 line
Do not panic
PATCH 5/9 firmware: add function to load firmware without warnings v5
In reply to Andres RodriguezCurrently 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
--- .../driver-api/firmware/request_firmware.rst | 5 +++++ drivers/base/firmware_loader/main.c | 24 ++++++++++++++++++++++ include/linux/firmware.h | 2 ++ 3 files changed, 31 insertions(+)
Documentation/driver-api/firmware/request_firmware.rst +5 -0
@@ -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
drivers/base/firmware_loader/main.c +24 -0
@@ -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
include/linux/firmware.h +2 -0
@@ -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,
Signature · 1 line
2.14.1
Luis R. Rodriguez
maintainerRe: [PATCH 5/9] firmware: add function to load firmware without warnings v5
Quoted 47 lines · Andres Rodriguez
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 addingFW_OPT_NO_WARNto the old behaviour.v2: add header prototype, use updated
opt_flags
v3: split debug message to separate patch
added_nowaitvariant
added documentation
v4: fix kernel-doc style
v5: drop_nowaitfor now, since we lack agreement on the API[0]: https://git.kernel.org/linus/c0cc00f250e1
Signed-off-by Andres Rodriguez <[email protected]>--- .../driver-api/firmware/request_firmware.rst | 5 +++++ drivers/base/firmware_loader/main.c | 24 ++++++++++++++++++++++ include/linux/firmware.h | 2 ++ 3 files changed, 31 insertions(+)Documentation/driver-api/firmware/request_firmware.rst +5
-0@@ -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.cdrivers/base/firmware_loader/main.c +0
-0@@ -631,6 +631,30 @@ firmware_request(const struct firmware **firmware_p, const char *name, }
drivers/base/firmware_loader/main.c firmware_request(const struct firmware **firmware_p, const char *name, · line 641
Quoted 10 lines
drivers/base/firmware_loader/main.c +8
-0EXPORT_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
Quoted 36 lines
drivers/base/firmware_loader/main.c +16
-0+**/ +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 include/linux/firmware.h +2
-0@@ -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,Signature · 1 line
2.14.1
Signature · 1 line
Do not panic
Luis R. Rodriguez
maintainerRe: [PATCH 5/9] firmware: add function to load firmware without warnings v5
In reply to Andres Rodriguezinclude/linux/firmware.h struct builtin_fw { · line 49
Quoted 13 lines · Andres Rodriguez
include/linux/firmware.h +2
-0@@ -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 <[email protected]>, and Kees Cook <[email protected]>. 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
PATCH 6/9 firmware: print firmware name on fallback path
In reply to Andres RodriguezPreviously, 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.
--- drivers/base/firmware_loader/fallback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/base/firmware_loader/fallback.c +1 -1
@@ -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); }
Signature · 1 line
2.14.1
Luis R. Rodriguez
maintainerRe: [PATCH 6/9] firmware: print firmware name on fallback path
Quoted 5 lines · Andres Rodriguez
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
Quoted 24 lines
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 <[email protected]>--- drivers/base/firmware_loader/fallback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/base/firmware_loader/fallback.c +1
-1@@ -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); }Signature · 1 line
2.14.1
Signature · 1 line
Do not panic
Re: [PATCH 6/9] firmware: print firmware name on fallback path
Quoted 11 lines · Luis R. Rodriguez
Quoted 5 lines · Andres Rodriguez
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.
Quoted 7 lines
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
Quoted 28 lines
Luis
Quoted 24 lines
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 <[email protected]>--- drivers/base/firmware_loader/fallback.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/base/firmware_loader/fallback.c +1
-1@@ -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); }Signature · 1 line
2.14.1
Luis R. Rodriguez
maintainerRe: [PATCH 6/9] firmware: print firmware name on fallback path
Quoted 29 lines · Andres Rodriguez
Quoted 11 lines · Luis R. Rodriguez
Quoted 5 lines · Andres Rodriguez
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.
Quoted 7 lines
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:
Quoted 8 lines
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_WARNis set and no messages are printed, or
b)FW_OPT_NO_WARNis 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
Re: [PATCH 6/9] firmware: print firmware name on fallback path
In reply to Luis R. Rodriguez(sorry for the delay, this got buried in my inbox)
Quoted 7 lines · Luis R. Rodriguez
Quoted 5 lines · Andres Rodriguez
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.
Quoted 8 lines
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.
Signature · 1 line
Kalle Valo
Luis R. Rodriguez
maintainerRe: [PATCH 6/9] firmware: print firmware name on fallback path
Quoted 18 lines · Kalle Valo
(sorry for the delay, this got buried in my inbox)
Quoted 14 lines · Luis R. Rodriguez
Quoted 5 lines · Andres Rodriguez
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?
Quoted 6 lines
Quoted 1 line
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
PATCH 7/9 firmware: use rename fw_sysfs_fallback to use the firmware_ prefix
In reply to Andres RodriguezUse the correct prefix for symbols exported by firmware_loader(). This is done since firmware_sysfs_fallback() is now exposed through kernel-doc.
--- 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(-)
drivers/base/firmware_loader/fallback.c +4 -4
@@ -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;
drivers/base/firmware_loader/fallback.h +2 -2
@@ -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)
drivers/base/firmware_loader/firmware.h +3 -3
@@ -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
drivers/base/firmware_loader/main.c +1 -1
@@ -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);
Signature · 1 line
2.14.1
Luis R. Rodriguez
maintainerRe: [PATCH 7/9] firmware: use rename fw_sysfs_fallback to use the firmware_ prefix
Quoted 18 lines · Andres Rodriguez
Use the correct prefix for symbols exported by
firmware_loader(). This is done sincefirmware_sysfs_fallback()is now exposed through kernel-doc.Signed-off-by Andres Rodriguez <[email protected]>--- 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(-)drivers/base/firmware_loader/fallback.c +0
-0@@ -661,10 +661,10 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)return fw_force_sysfs_fallback (opt_flags);
drivers/base/firmware_loader/fallback.c static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) · line 667
Quoted 10 lines
drivers/base/firmware_loader/fallback.c +4
-4} -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.
Quoted 30 lines
drivers/base/firmware_loader/fallback.c +0
-0{if (!fw_run_sysfs_fallback (opt_flags))return ret;drivers/base/firmware_loader/fallback.h +2
-2@@ -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)drivers/base/firmware_loader/firmware.h +0
-0
drivers/base/firmware_loader/firmware.h line 19
Quoted 7 lines
drivers/base/firmware_loader/firmware.h +2
-2@@ -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.
Quoted 25 lines
drivers/base/firmware_loader/firmware.h +1
-1* @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 drivers/base/firmware_loader/main.c +1
-1@@ -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);Signature · 1 line
2.14.1
Signature · 1 line
Do not panic
PATCH 8/9 ath10k: use request_firmware_nowarn to load firmware
In reply to Andres RodriguezThis reduces the unnecessary spew when trying to load optional firmware:
"Direct firmware load for ... failed with error -2"
--- drivers/net/wireless/ath/ath10k/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/ath10k/core.c +1 -1
@@ -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);
Signature · 1 line
2.14.1
PATCH 9/9 ath10k: re-enable the firmware fallback mechanism for testmode
In reply to Andres RodriguezThe 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.
--- drivers/net/wireless/ath/ath10k/testmode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/ath/ath10k/testmode.c +1 -1
@@ -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);
Signature · 1 line
2.14.1
Re: [PATCH 9/9] ath10k: re-enable the firmware fallback mechanism for testmode
Quoted 8 lines · Andres Rodriguez
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 <[email protected]>
Thanks!
Signature · 1 line
Kalle Valo