diff mbox series

wifi: rtw89: Fix clang -Wimplicit-fallthrough in rtw89_query_sar()

Message ID 20230822-rtw89-tas-clang-implicit-fallthrough-v1-1-5cb73f0fa976@kernel.org
State New
Headers show
Series wifi: rtw89: Fix clang -Wimplicit-fallthrough in rtw89_query_sar() | expand

Commit Message

Nathan Chancellor Aug. 22, 2023, 3:27 p.m. UTC 
clang warns (or errors with CONFIG_WERROR=y):

  drivers/net/wireless/realtek/rtw89/sar.c:216:3: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
    216 |                 case RTW89_TAS_STATE_DPR_FORBID:
        |                 ^
  drivers/net/wireless/realtek/rtw89/sar.c:216:3: note: insert 'break;' to avoid fall-through
    216 |                 case RTW89_TAS_STATE_DPR_FORBID:
        |                 ^
        |                 break;
  1 error generated.

Clang is a little more pedantic than GCC, which does not warn when
falling through to a case that is just break or return. Clang's version
is more in line with the kernel's own stance in deprecated.rst, which
states that all switch/case blocks must end in either break,
fallthrough, continue, goto, or return. Add the missing break to silence
the warning.

Closes: https://github.com/ClangBuiltLinux/linux/issues/1921
Fixes: eb2624f55ad1 ("wifi: rtw89: Introduce Time Averaged SAR (TAS) feature")
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 drivers/net/wireless/realtek/rtw89/sar.c | 1 +
 1 file changed, 1 insertion(+)


---
base-commit: f585f4ab0b998578b4ef3610ccfc08e207fc3499
change-id: 20230822-rtw89-tas-clang-implicit-fallthrough-aafed8e950fc

Best regards,

Comments

Nick Desaulniers Aug. 22, 2023, 3:35 p.m. UTC | #1 
On Tue, Aug 22, 2023 at 8:27 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> clang warns (or errors with CONFIG_WERROR=y):
>
>   drivers/net/wireless/realtek/rtw89/sar.c:216:3: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
>     216 |                 case RTW89_TAS_STATE_DPR_FORBID:
>         |                 ^
>   drivers/net/wireless/realtek/rtw89/sar.c:216:3: note: insert 'break;' to avoid fall-through
>     216 |                 case RTW89_TAS_STATE_DPR_FORBID:
>         |                 ^
>         |                 break;
>   1 error generated.
>
> Clang is a little more pedantic than GCC, which does not warn when
> falling through to a case that is just break or return. Clang's version
> is more in line with the kernel's own stance in deprecated.rst, which
> states that all switch/case blocks must end in either break,
> fallthrough, continue, goto, or return. Add the missing break to silence
> the warning.
>
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1921
> Fixes: eb2624f55ad1 ("wifi: rtw89: Introduce Time Averaged SAR (TAS) feature")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  drivers/net/wireless/realtek/rtw89/sar.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/realtek/rtw89/sar.c b/drivers/net/wireless/realtek/rtw89/sar.c
> index fafc7a0cfe97..715bf5c2f56a 100644
> --- a/drivers/net/wireless/realtek/rtw89/sar.c
> +++ b/drivers/net/wireless/realtek/rtw89/sar.c
> @@ -213,6 +213,7 @@ s8 rtw89_query_sar(struct rtw89_dev *rtwdev)
>                 case RTW89_TAS_STATE_DPR_ON:
>                         delta = rtw89_txpwr_tas_to_sar(sar_hdl, tas->delta);
>                         cfg -= delta;
> +                       break;
>                 case RTW89_TAS_STATE_DPR_FORBID:
>                 default:
>                         break;
>
> ---
> base-commit: f585f4ab0b998578b4ef3610ccfc08e207fc3499
> change-id: 20230822-rtw89-tas-clang-implicit-fallthrough-aafed8e950fc
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>
Ping-Ke Shih Aug. 23, 2023, 1:44 a.m. UTC | #2 
> -----Original Message-----
> From: Nathan Chancellor <nathan@kernel.org>
> Sent: Tuesday, August 22, 2023 11:27 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; kvalo@kernel.org
> Cc: ndesaulniers@google.com; trix@redhat.com; Damon Chen <damon.chen@realtek.com>;
> linux-wireless@vger.kernel.org; patches@lists.linux.dev; llvm@lists.linux.dev; Nathan Chancellor
> <nathan@kernel.org>
> Subject: [PATCH] wifi: rtw89: Fix clang -Wimplicit-fallthrough in rtw89_query_sar()
> 
> clang warns (or errors with CONFIG_WERROR=y):
> 
>   drivers/net/wireless/realtek/rtw89/sar.c:216:3: error: unannotated fall-through between switch labels
> [-Werror,-Wimplicit-fallthrough]
>     216 |                 case RTW89_TAS_STATE_DPR_FORBID:
>         |                 ^
>   drivers/net/wireless/realtek/rtw89/sar.c:216:3: note: insert 'break;' to avoid fall-through
>     216 |                 case RTW89_TAS_STATE_DPR_FORBID:
>         |                 ^
>         |                 break;
>   1 error generated.
> 
> Clang is a little more pedantic than GCC, which does not warn when
> falling through to a case that is just break or return. Clang's version
> is more in line with the kernel's own stance in deprecated.rst, which
> states that all switch/case blocks must end in either break,
> fallthrough, continue, goto, or return. Add the missing break to silence
> the warning.
> 
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1921
> Fixes: eb2624f55ad1 ("wifi: rtw89: Introduce Time Averaged SAR (TAS) feature")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>
Kalle Valo Aug. 25, 2023, 10 a.m. UTC | #3 
Nathan Chancellor <nathan@kernel.org> wrote:

> clang warns (or errors with CONFIG_WERROR=y):
> 
>   drivers/net/wireless/realtek/rtw89/sar.c:216:3: error: unannotated fall-through between switch labels [-Werror,-Wimplicit-fallthrough]
>     216 |                 case RTW89_TAS_STATE_DPR_FORBID:
>         |                 ^
>   drivers/net/wireless/realtek/rtw89/sar.c:216:3: note: insert 'break;' to avoid fall-through
>     216 |                 case RTW89_TAS_STATE_DPR_FORBID:
>         |                 ^
>         |                 break;
>   1 error generated.
> 
> Clang is a little more pedantic than GCC, which does not warn when
> falling through to a case that is just break or return. Clang's version
> is more in line with the kernel's own stance in deprecated.rst, which
> states that all switch/case blocks must end in either break,
> fallthrough, continue, goto, or return. Add the missing break to silence
> the warning.
> 
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1921
> Fixes: eb2624f55ad1 ("wifi: rtw89: Introduce Time Averaged SAR (TAS) feature")
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>

Patch applied to wireless-next.git, thanks.

78d84f35d2c3 wifi: rtw89: Fix clang -Wimplicit-fallthrough in rtw89_query_sar()
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw89/sar.c b/drivers/net/wireless/realtek/rtw89/sar.c
index fafc7a0cfe97..715bf5c2f56a 100644
--- a/drivers/net/wireless/realtek/rtw89/sar.c
+++ b/drivers/net/wireless/realtek/rtw89/sar.c
@@ -213,6 +213,7 @@  s8 rtw89_query_sar(struct rtw89_dev *rtwdev)
 		case RTW89_TAS_STATE_DPR_ON:
 			delta = rtw89_txpwr_tas_to_sar(sar_hdl, tas->delta);
 			cfg -= delta;
+			break;
 		case RTW89_TAS_STATE_DPR_FORBID:
 		default:
 			break;