| Message ID | 20191211192252.35024-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ath11k: Remove unnecessary enum scan_priority | expand |
On Wed, Dec 11, 2019 at 11:23 AM Nathan Chancellor <natechancellor@gmail.com> wrote: > > Clang warns: > > drivers/net/wireless/ath/ath11k/wmi.c:1827:23: warning: implicit > conversion from enumeration type 'enum wmi_scan_priority' to different > enumeration type 'enum scan_priority' [-Wenum-conversion] > arg->scan_priority = WMI_SCAN_PRIORITY_LOW; > ~ ^~~~~~~~~~~~~~~~~~~~~ > 1 warning generated. > > wmi_scan_priority and scan_priority have the same values but the wmi one > has WMI prefixed to the names. Since that enum is already being used, > get rid of scan_priority and switch its one use to wmi_scan_priority to > fix this warning. > > Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") > Link: https://github.com/ClangBuiltLinux/linux/issues/808 > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> Further, it looks like the member `scan_priority` in `struct wmi_start_scan_arg` and `struct wmi_start_scan_cmd` should probably use `enum wmi_scan_priority`, rather than `u32`. Also, I don't know if the more concisely named enum is preferable? Either way, thanks for the patch. Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > --- > drivers/net/wireless/ath/ath11k/wmi.h | 11 +---------- > 1 file changed, 1 insertion(+), 10 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h > index 4a518d406bc5..756101656391 100644 > --- a/drivers/net/wireless/ath/ath11k/wmi.h > +++ b/drivers/net/wireless/ath/ath11k/wmi.h > @@ -2896,15 +2896,6 @@ struct wmi_bcn_offload_ctrl_cmd { > u32 bcn_ctrl_op; > } __packed; > > -enum scan_priority { > - SCAN_PRIORITY_VERY_LOW, > - SCAN_PRIORITY_LOW, > - SCAN_PRIORITY_MEDIUM, > - SCAN_PRIORITY_HIGH, > - SCAN_PRIORITY_VERY_HIGH, > - SCAN_PRIORITY_COUNT, > -}; > - > enum scan_dwelltime_adaptive_mode { > SCAN_DWELL_MODE_DEFAULT = 0, > SCAN_DWELL_MODE_CONSERVATIVE = 1, > @@ -3056,7 +3047,7 @@ struct scan_req_params { > u32 scan_req_id; > u32 vdev_id; > u32 pdev_id; > - enum scan_priority scan_priority; > + enum wmi_scan_priority scan_priority; > union { > struct { > u32 scan_ev_started:1, > -- > 2.24.0 > > --
Nathan Chancellor <natechancellor@gmail.com> wrote: > Clang warns: > > drivers/net/wireless/ath/ath11k/wmi.c:1827:23: warning: implicit > conversion from enumeration type 'enum wmi_scan_priority' to different > enumeration type 'enum scan_priority' [-Wenum-conversion] > arg->scan_priority = WMI_SCAN_PRIORITY_LOW; > ~ ^~~~~~~~~~~~~~~~~~~~~ > 1 warning generated. > > wmi_scan_priority and scan_priority have the same values but the wmi one > has WMI prefixed to the names. Since that enum is already being used, > get rid of scan_priority and switch its one use to wmi_scan_priority to > fix this warning. > > Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") > Link: https://github.com/ClangBuiltLinux/linux/issues/808 > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to ath-next branch of ath.git, thanks. 509421acab69 ath11k: Remove unnecessary enum scan_priority
Nick Desaulniers <ndesaulniers@google.com> writes: > On Wed, Dec 11, 2019 at 11:23 AM Nathan Chancellor > <natechancellor@gmail.com> wrote: >> >> Clang warns: >> >> drivers/net/wireless/ath/ath11k/wmi.c:1827:23: warning: implicit >> conversion from enumeration type 'enum wmi_scan_priority' to different >> enumeration type 'enum scan_priority' [-Wenum-conversion] >> arg->scan_priority = WMI_SCAN_PRIORITY_LOW; >> ~ ^~~~~~~~~~~~~~~~~~~~~ >> 1 warning generated. >> >> wmi_scan_priority and scan_priority have the same values but the wmi one >> has WMI prefixed to the names. Since that enum is already being used, >> get rid of scan_priority and switch its one use to wmi_scan_priority to >> fix this warning. >> >> Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") >> Link: https://github.com/ClangBuiltLinux/linux/issues/808 >> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > Further, it looks like the member `scan_priority` in `struct > wmi_start_scan_arg` and `struct wmi_start_scan_cmd` should probably > use `enum wmi_scan_priority`, rather than `u32`. struct wmi_start_scan_cmd is sent to firmware and that's why it has u32 to make sure that the size is exactly 32 bits. > Also, I don't know if the more concisely named enum is preferable? I didn't get this comment.
On Thu, Dec 19, 2019 at 5:32 AM Kalle Valo <kvalo@codeaurora.org> wrote: > > Nick Desaulniers <ndesaulniers@google.com> writes: > > > On Wed, Dec 11, 2019 at 11:23 AM Nathan Chancellor > > <natechancellor@gmail.com> wrote: > >> wmi_scan_priority and scan_priority have the same values but the wmi one > >> has WMI prefixed to the names. Since that enum is already being used, > >> get rid of scan_priority and switch its one use to wmi_scan_priority to > >> fix this warning. > >> > > Also, I don't know if the more concisely named enum is preferable? > > I didn't get this comment. Given two enums with the same values: enum scan_priority enum wmi_scan_priority wouldn't you prefer to type wmi_ a few times less? Doesn't really matter, but that was the point I was making.
Nick Desaulniers <ndesaulniers@google.com> writes: > On Thu, Dec 19, 2019 at 5:32 AM Kalle Valo <kvalo@codeaurora.org> wrote: >> >> Nick Desaulniers <ndesaulniers@google.com> writes: >> >> > On Wed, Dec 11, 2019 at 11:23 AM Nathan Chancellor >> > <natechancellor@gmail.com> wrote: >> >> wmi_scan_priority and scan_priority have the same values but the wmi one >> >> has WMI prefixed to the names. Since that enum is already being used, >> >> get rid of scan_priority and switch its one use to wmi_scan_priority to >> >> fix this warning. >> >> >> > Also, I don't know if the more concisely named enum is preferable? >> >> I didn't get this comment. > > Given two enums with the same values: > enum scan_priority > enum wmi_scan_priority > wouldn't you prefer to type wmi_ a few times less? Doesn't really > matter, but that was the point I was making. Ah, now I got it :) This enum is part of firmware interface (WMI) so yes, I prefer to use the wmi_ prefix to make that obvious.
diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h index 4a518d406bc5..756101656391 100644 --- a/drivers/net/wireless/ath/ath11k/wmi.h +++ b/drivers/net/wireless/ath/ath11k/wmi.h @@ -2896,15 +2896,6 @@ struct wmi_bcn_offload_ctrl_cmd { u32 bcn_ctrl_op; } __packed; -enum scan_priority { - SCAN_PRIORITY_VERY_LOW, - SCAN_PRIORITY_LOW, - SCAN_PRIORITY_MEDIUM, - SCAN_PRIORITY_HIGH, - SCAN_PRIORITY_VERY_HIGH, - SCAN_PRIORITY_COUNT, -}; - enum scan_dwelltime_adaptive_mode { SCAN_DWELL_MODE_DEFAULT = 0, SCAN_DWELL_MODE_CONSERVATIVE = 1, @@ -3056,7 +3047,7 @@ struct scan_req_params { u32 scan_req_id; u32 vdev_id; u32 pdev_id; - enum scan_priority scan_priority; + enum wmi_scan_priority scan_priority; union { struct { u32 scan_ev_started:1,
Clang warns: drivers/net/wireless/ath/ath11k/wmi.c:1827:23: warning: implicit conversion from enumeration type 'enum wmi_scan_priority' to different enumeration type 'enum scan_priority' [-Wenum-conversion] arg->scan_priority = WMI_SCAN_PRIORITY_LOW; ~ ^~~~~~~~~~~~~~~~~~~~~ 1 warning generated. wmi_scan_priority and scan_priority have the same values but the wmi one has WMI prefixed to the names. Since that enum is already being used, get rid of scan_priority and switch its one use to wmi_scan_priority to fix this warning. Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices") Link: https://github.com/ClangBuiltLinux/linux/issues/808 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/net/wireless/ath/ath11k/wmi.h | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-)