"enum fw_opt" is not used as an enum. Change fw_opt to a u32 and the FW_OPT_* values inside the enum to defines.
Signed-off-by: Scott Branden scott.branden@broadcom.com --- drivers/base/firmware_loader/fallback.c | 12 +++++------ drivers/base/firmware_loader/fallback.h | 8 +++---- .../base/firmware_loader/fallback_platform.c | 2 +- drivers/base/firmware_loader/firmware.h | 21 ++++++++----------- drivers/base/firmware_loader/main.c | 11 +++++----- 5 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 1e9c96e3ed63..af1aa3d69920 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -460,7 +460,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, enum fw_opt opt_flags) + struct device *device, u32 opt_flags) { struct fw_sysfs *fw_sysfs; struct device *f_dev; @@ -493,7 +493,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, - enum fw_opt opt_flags, long timeout) + u32 opt_flags, long timeout) { int retval = 0; struct device *f_dev = &fw_sysfs->dev; @@ -547,7 +547,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, - enum fw_opt opt_flags) + u32 opt_flags) { struct fw_sysfs *fw_sysfs; long timeout; @@ -588,7 +588,7 @@ static int fw_load_from_user_helper(struct firmware *firmware, return ret; }
-static bool fw_force_sysfs_fallback(enum fw_opt opt_flags) +static bool fw_force_sysfs_fallback(u32 opt_flags) { if (fw_fallback_config.force_sysfs_fallback) return true; @@ -597,7 +597,7 @@ static bool fw_force_sysfs_fallback(enum fw_opt opt_flags) return true; }
-static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) +static bool fw_run_sysfs_fallback(u32 opt_flags) { int ret;
@@ -640,7 +640,7 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) **/ int firmware_fallback_sysfs(struct firmware *fw, const char *name, struct device *device, - enum fw_opt opt_flags, + u32 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 06f4577733a8..2afdb6adb23f 100644 --- a/drivers/base/firmware_loader/fallback.h +++ b/drivers/base/firmware_loader/fallback.h @@ -33,7 +33,7 @@ struct firmware_fallback_config { #ifdef CONFIG_FW_LOADER_USER_HELPER int firmware_fallback_sysfs(struct firmware *fw, const char *name, struct device *device, - enum fw_opt opt_flags, + u32 opt_flags, int ret); void kill_pending_fw_fallback_reqs(bool only_kill_custom);
@@ -45,7 +45,7 @@ void unregister_sysfs_loader(void); #else /* CONFIG_FW_LOADER_USER_HELPER */ static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name, struct device *device, - enum fw_opt opt_flags, + u32 opt_flags, int ret) { /* Keep carrying over the same error */ @@ -67,10 +67,10 @@ static inline void unregister_sysfs_loader(void) #endif /* CONFIG_FW_LOADER_USER_HELPER */
#ifdef CONFIG_EFI_EMBEDDED_FIRMWARE -int firmware_fallback_platform(struct fw_priv *fw_priv, enum fw_opt opt_flags); +int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags); #else static inline int firmware_fallback_platform(struct fw_priv *fw_priv, - enum fw_opt opt_flags) + u32 opt_flags) { return -ENOENT; } diff --git a/drivers/base/firmware_loader/fallback_platform.c b/drivers/base/firmware_loader/fallback_platform.c index c88c745590fe..cdd2c9a9f38a 100644 --- a/drivers/base/firmware_loader/fallback_platform.c +++ b/drivers/base/firmware_loader/fallback_platform.c @@ -8,7 +8,7 @@ #include "fallback.h" #include "firmware.h"
-int firmware_fallback_platform(struct fw_priv *fw_priv, enum fw_opt opt_flags) +int firmware_fallback_platform(struct fw_priv *fw_priv, u32 opt_flags) { const u8 *data; size_t size; diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h index 25836a6afc9f..cf6dc96a84a2 100644 --- a/drivers/base/firmware_loader/firmware.h +++ b/drivers/base/firmware_loader/firmware.h @@ -12,7 +12,7 @@ #include <generated/utsrelease.h>
/** - * enum fw_opt - options to control firmware loading behaviour + * 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 @@ -33,15 +33,13 @@ * the platform's main firmware. If both this fallback and the sysfs * fallback are enabled, then this fallback will be tried first. */ -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_SYSFS = BIT(5), - FW_OPT_FALLBACK_PLATFORM = BIT(6), -}; +#define FW_OPT_UEVENT BIT(0) +#define FW_OPT_NOWAIT BIT(1) +#define FW_OPT_USERHELPER BIT(2) +#define FW_OPT_NO_WARN BIT(3) +#define FW_OPT_NOCACHE BIT(4) +#define FW_OPT_NOFALLBACK_SYSFS BIT(5) +#define FW_OPT_FALLBACK_PLATFORM BIT(6)
enum fw_status { FW_STATUS_UNKNOWN, @@ -136,8 +134,7 @@ static inline void fw_state_done(struct fw_priv *fw_priv) __fw_state_set(fw_priv, FW_STATUS_DONE); }
-int assign_fw(struct firmware *fw, struct device *device, - enum fw_opt opt_flags); +int assign_fw(struct firmware *fw, struct device *device, u32 opt_flags);
#ifdef CONFIG_FW_LOADER_PAGED_BUF void fw_free_paged_buf(struct fw_priv *fw_priv); diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 76f79913916d..2ca6b4735979 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -210,7 +210,7 @@ static struct fw_priv *__lookup_fw_priv(const char *fw_name) static int alloc_lookup_fw_priv(const char *fw_name, struct firmware_cache *fwc, struct fw_priv **fw_priv, void *dbuf, - size_t size, enum fw_opt opt_flags) + size_t size, u32 opt_flags) { struct fw_priv *tmp;
@@ -635,8 +635,7 @@ static int fw_add_devm_name(struct device *dev, const char *name) } #endif
-int assign_fw(struct firmware *fw, struct device *device, - enum fw_opt opt_flags) +int assign_fw(struct firmware *fw, struct device *device, u32 opt_flags) { struct fw_priv *fw_priv = fw->priv; int ret; @@ -687,7 +686,7 @@ int assign_fw(struct firmware *fw, struct device *device, static int _request_firmware_prepare(struct firmware **firmware_p, const char *name, struct device *device, void *dbuf, size_t size, - enum fw_opt opt_flags) + u32 opt_flags) { struct firmware *firmware; struct fw_priv *fw_priv; @@ -753,7 +752,7 @@ static void fw_abort_batch_reqs(struct firmware *fw) static int _request_firmware(const struct firmware **firmware_p, const char *name, struct device *device, void *buf, size_t size, - enum fw_opt opt_flags) + u32 opt_flags) { struct firmware *fw = NULL; int ret; @@ -990,7 +989,7 @@ struct firmware_work { struct device *device; void *context; void (*cont)(const struct firmware *fw, void *context); - enum fw_opt opt_flags; + u32 opt_flags; };
static void request_firmware_work_func(struct work_struct *work)
On Fri, May 22, 2020 at 02:46:58PM -0700, Scott Branden wrote:
/**
- enum fw_opt - options to control firmware loading behaviour
- 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
@@ -33,15 +33,13 @@
- the platform's main firmware. If both this fallback and the sysfs
fallback are enabled, then this fallback will be tried first.*/ -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_SYSFS = BIT(5),
- FW_OPT_FALLBACK_PLATFORM = BIT(6),
-}; +#define FW_OPT_UEVENT BIT(0) +#define FW_OPT_NOWAIT BIT(1) +#define FW_OPT_USERHELPER BIT(2) +#define FW_OPT_NO_WARN BIT(3) +#define FW_OPT_NOCACHE BIT(4) +#define FW_OPT_NOFALLBACK_SYSFS BIT(5) +#define FW_OPT_FALLBACK_PLATFORM BIT(6)
Everything looked good up to here. The enum defines each flag. We just want to use an enum for *one* flag represetnation, not a bundle.
Luis
Hi Luis,
On 2020-05-22 3:45 p.m., Luis Chamberlain wrote:
On Fri, May 22, 2020 at 02:46:58PM -0700, Scott Branden wrote:
/**
- enum fw_opt - options to control firmware loading behaviour
- 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
@@ -33,15 +33,13 @@
- the platform's main firmware. If both this fallback and the sysfs
fallback are enabled, then this fallback will be tried first.*/ -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_SYSFS = BIT(5),
- FW_OPT_FALLBACK_PLATFORM = BIT(6),
-}; +#define FW_OPT_UEVENT BIT(0) +#define FW_OPT_NOWAIT BIT(1) +#define FW_OPT_USERHELPER BIT(2) +#define FW_OPT_NO_WARN BIT(3) +#define FW_OPT_NOCACHE BIT(4) +#define FW_OPT_NOFALLBACK_SYSFS BIT(5) +#define FW_OPT_FALLBACK_PLATFORM BIT(6)
Everything looked good up to here. The enum defines each flag. We just want to use an enum for *one* flag represetnation, not a bundle.
I do not know exactly what you are looking for then. The FW_OPT_* values are OR'd together in the code. You still want the fw_opt enum above left in place entirely and then the values used in OR'd together?
Luis
On Fri, May 22, 2020 at 03:49:05PM -0700, Scott Branden wrote:
Hi Luis,
On 2020-05-22 3:45 p.m., Luis Chamberlain wrote:
On Fri, May 22, 2020 at 02:46:58PM -0700, Scott Branden wrote:
/**
- enum fw_opt - options to control firmware loading behaviour
- 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
@@ -33,15 +33,13 @@
- the platform's main firmware. If both this fallback and the sysfs
fallback are enabled, then this fallback will be tried first.*/ -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_SYSFS = BIT(5),
- FW_OPT_FALLBACK_PLATFORM = BIT(6),
-}; +#define FW_OPT_UEVENT BIT(0) +#define FW_OPT_NOWAIT BIT(1) +#define FW_OPT_USERHELPER BIT(2) +#define FW_OPT_NO_WARN BIT(3) +#define FW_OPT_NOCACHE BIT(4) +#define FW_OPT_NOFALLBACK_SYSFS BIT(5) +#define FW_OPT_FALLBACK_PLATFORM BIT(6)
Everything looked good up to here. The enum defines each flag. We just want to use an enum for *one* flag represetnation, not a bundle.
I do not know exactly what you are looking for then. The FW_OPT_* values are OR'd together in the code. You still want the fw_opt enum above left in place entirely and then the values used in OR'd together?
Yes, exactly. If they are combined then we just use a u32 to represent these are then flags combined. An enum represents just *one* flag, not a set which are OR'd together. Let me know if this is still not clear.
Luis
On Fri, May 22, 2020 at 11:00:25PM +0000, Luis Chamberlain wrote:
On Fri, May 22, 2020 at 03:49:05PM -0700, Scott Branden wrote:
Hi Luis,
On 2020-05-22 3:45 p.m., Luis Chamberlain wrote:
On Fri, May 22, 2020 at 02:46:58PM -0700, Scott Branden wrote:
/**
- enum fw_opt - options to control firmware loading behaviour
- 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
@@ -33,15 +33,13 @@
- the platform's main firmware. If both this fallback and the sysfs
fallback are enabled, then this fallback will be tried first.*/ -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_SYSFS = BIT(5),
- FW_OPT_FALLBACK_PLATFORM = BIT(6),
-}; +#define FW_OPT_UEVENT BIT(0) +#define FW_OPT_NOWAIT BIT(1) +#define FW_OPT_USERHELPER BIT(2) +#define FW_OPT_NO_WARN BIT(3) +#define FW_OPT_NOCACHE BIT(4) +#define FW_OPT_NOFALLBACK_SYSFS BIT(5) +#define FW_OPT_FALLBACK_PLATFORM BIT(6)
Everything looked good up to here. The enum defines each flag. We just want to use an enum for *one* flag represetnation, not a bundle.
I do not know exactly what you are looking for then. The FW_OPT_* values are OR'd together in the code. You still want the fw_opt enum above left in place entirely and then the values used in OR'd together?
Yes, exactly. If they are combined then we just use a u32 to represent these are then flags combined. An enum represents just *one* flag, not a set which are OR'd together. Let me know if this is still not clear.
If this is the case, why have an enum at all? What difference is it from a #define like was done here?
thanks,
greg k-h
On Sat, May 23, 2020 at 08:05:48AM +0200, Greg Kroah-Hartman wrote:
On Fri, May 22, 2020 at 11:00:25PM +0000, Luis Chamberlain wrote:
On Fri, May 22, 2020 at 03:49:05PM -0700, Scott Branden wrote:
Hi Luis,
On 2020-05-22 3:45 p.m., Luis Chamberlain wrote:
On Fri, May 22, 2020 at 02:46:58PM -0700, Scott Branden wrote:
/**
- enum fw_opt - options to control firmware loading behaviour
- 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
@@ -33,15 +33,13 @@
- the platform's main firmware. If both this fallback and the sysfs
fallback are enabled, then this fallback will be tried first.*/ -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_SYSFS = BIT(5),
- FW_OPT_FALLBACK_PLATFORM = BIT(6),
-}; +#define FW_OPT_UEVENT BIT(0) +#define FW_OPT_NOWAIT BIT(1) +#define FW_OPT_USERHELPER BIT(2) +#define FW_OPT_NO_WARN BIT(3) +#define FW_OPT_NOCACHE BIT(4) +#define FW_OPT_NOFALLBACK_SYSFS BIT(5) +#define FW_OPT_FALLBACK_PLATFORM BIT(6)
Everything looked good up to here. The enum defines each flag. We just want to use an enum for *one* flag represetnation, not a bundle.
I do not know exactly what you are looking for then. The FW_OPT_* values are OR'd together in the code. You still want the fw_opt enum above left in place entirely and then the values used in OR'd together?
Yes, exactly. If they are combined then we just use a u32 to represent these are then flags combined. An enum represents just *one* flag, not a set which are OR'd together. Let me know if this is still not clear.
If this is the case, why have an enum at all? What difference is it from a #define like was done here?
If used on a switch to test for cases the compiler will error if you miss one, and it is easier to use kdoc for them.
Luis
linux-kselftest-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Luis Chamberlain -
Scott Branden