-
Notifications
You must be signed in to change notification settings - Fork 5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4.14] Fix GCC 6.4.0 and 7.3.0 warnings #2413
[4.14] Fix GCC 6.4.0 and 7.3.0 warnings #2413
Conversation
We've actually picked a newer source of the rtl8192cu patches in later kernels, so this patch won't affect them, but otherwise no objections. |
FWIW with the Linaro toolchain gcc-linaro-7.2.1-2017.11, i'm getting more warnings (didn't check for false-positive):
|
@lategoodbye Yeah GCC 7.x will have newer warnings, I'd be open to fixing those as well if they are. |
@popcornmix just FYI, I just did a build off of the 4.16.y branch and both patches apply and are relevant. |
Okay, the 8192cu change might be best submitted here: https://github.com/pvaret/rtl8192cu-fixes |
Perfect, will go ahead and take a look at submitting them there! |
@popcornmix Just so I am clear about the flow cycle here so I don't waste your time in the future, should I be basing patches on the 4.16.y branch and then porting the relevant ones back to the older branches? Which branches are active and worthy of these fixes? I want to make you guys do the least amount of work since these aren't mission critical fixes. |
1784ba0
to
db5a6c2
Compare
sound/soc/bcm/allo-piano-dac-plus.c
Outdated
@@ -708,7 +708,7 @@ static int snd_allo_piano_dac_init(struct snd_soc_pcm_runtime *rtd) | |||
if (!glb_ptr) | |||
return -ENOMEM; | |||
|
|||
memset(glb_ptr, 0x00, sizeof(glb_ptr)); | |||
memset(glb_ptr, 0x00, sizeof(*glb_ptr)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is correct, but it would be better to replace the kmalloc
above with kzalloc
and delete this line.
All fine apart from me preferring a kmalloc -> kzalloc fix. |
db5a6c2
to
d5a60f2
Compare
@pelwell, done, thank you! |
4.14 will be the next kernel used by raspbian and is LTS so we are likely to stick with it as stable kernel for some time. (Most likely until some new feature, perhaps in the drm/vc4 area which evolves quite rapidly makes the switch worthwhile). So I'd suggest PR-ing to 4.14 in general. Ignore kernels older than 4.14 - they are unlikely to get further updates. The newer kernels are mostly for bleeding edge testers and are used by some other distributions, and obviously will be used when we finally move to a newer stable kernel. Most kernels aren't LTS and we typically will stop updating the tree when kernel.org marks it as EOL. If the PR to 4.14 applies cleanly to newer kernel trees, just let us know and we'll cherry pick changes. If it's more complicated then separate PRs are useful. |
@@ -2360,9 +2360,6 @@ phy_TxPwrIdxToDbm( | |||
Offset = -7; | |||
break; | |||
|
|||
case WIRELESS_MODE_G: | |||
case WIRELESS_MODE_N_24G: | |||
Offset = -8; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original code is showing that WIRELESS_MODE_G and WIRELESS_MODE_N_24G have been handled explicitly and need Offset = -8
. The default case generally means we'll pick a sane value, but it's less clear if it will be correct if new modes are added.
I think the real bug is the missing break statement after the assignment, and fixing it that way makes it clear that WIRELESS_MODE_G/WIRELESS_MODE_N_24G are being handled explicitly.
The compiler will generate the same code is all cases, but add the break rather than removing the labels provides more information to a programmer reading the code.
This also applies to the following two similar patches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will push that change here shortly.
Ditto for the other two similar instances. |
d5a60f2
to
b12dc59
Compare
All three instances fixed properly. |
b12dc59
to
470b351
Compare
This warning appears with GCC 6.4.0 from toolchains.bootlin.com: ../sound/soc/bcm/allo-piano-dac-plus.c: In function ‘snd_allo_piano_dac_init’: ../sound/soc/bcm/allo-piano-dac-plus.c:711:30: warning: argument to ‘sizeof’ in ‘memset’ call is the same expression as the destination; did you mean to dereference it? [-Wsizeof-pointer-memaccess] memset(glb_ptr, 0x00, sizeof(glb_ptr)); ^ Suggested-by: Phil Elwell <[email protected]> Signed-off-by: Nathan Chancellor <[email protected]>
This warning appears with GCC 6.4.0 from toolchains.bootlin.com: ../drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In function ‘vchiq_open’: ../drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1735:7: warning: unused variable ‘ret’ [-Wunused-variable] int ret; ^~~ This variable's usage was removed by commit 3c98026 ("staging: vchiq_arm: Make debugfs failure non-fatal"), making it useless. Signed-off-by: Nathan Chancellor <[email protected]>
These warnings appear with GCC 6.4.0 from toolchains.bootlin.com: ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_security.c: In function ‘aes_cipher’: ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_security.c:1504:5: warning: this ‘for’ clause does not guard... [-Wmisleading-indentation] for (j = 0; j < 8; j++) ^~~ ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_security.c:1507:2: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘for’ payload_index = hdrlen + 8; ^~~~~~~~~~~~~ ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_security.c: In function ‘aes_decipher’: ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_security.c:1878:5: warning: this ‘for’ clause does not guard... [-Wmisleading-indentation] for (j = 0; j < 8; j++) ^~~ ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_security.c:1881:2: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘for’ payload_index = hdrlen + 8; ^~~~~~~~~~~~~ ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_mlme_ext.c:5666:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation] if( _rtw_memcmp(pwdinfo->rx_prov_disc_info.peerDevAddr, empty_addr, ETH_ALEN) ); ^~ ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_mlme_ext.c:5667:6: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the ‘if’ _rtw_memcpy(pwdinfo->rx_prov_disc_info.peerDevAddr, GetAddr2Ptr(pframe), ETH_ALEN); ^~~~~~~~~~~ It appears to be due to tabs versus spaces. Signed-off-by: Nathan Chancellor <[email protected]>
These warnings appear with GCC 7.3.0 from toolchains.bootlin.com: ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_mlme_ext.c: In function ‘mgt_dispatcher’: ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_mlme_ext.c:734:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if(check_fwstate(pmlmepriv, WIFI_AP_STATE) == _TRUE) ^ ../drivers/net/wireless/realtek/rtl8192cu/core/rtw_mlme_ext.c:739:3: note: here case WIFI_ASSOCREQ: ^~~~ ../drivers/net/wireless/realtek/rtl8192cu/hal/rtl8192c/rtl8192c_phycfg.c: In function ‘phy_TxPwrIdxToDbm’: ../drivers/net/wireless/realtek/rtl8192cu/hal/rtl8192c/rtl8192c_phycfg.c:2365:10: warning: this statement may fall through [-Wimplicit-fallthrough=] Offset = -8; ~~~~~~~^~~~ ../drivers/net/wireless/realtek/rtl8192cu/hal/rtl8192c/rtl8192c_phycfg.c:2366:2: note: here default: ^~~~~~~ ../drivers/net/wireless/realtek/rtl8192cu/hal/rtl8192c/usb/usb_halinit.c: In function ‘GetHwReg8192CU’: ../drivers/net/wireless/realtek/rtl8192cu/hal/rtl8192c/usb/usb_halinit.c:5694:20: warning: this statement may fall through [-Wimplicit-fallthrough=] *((u16 *)(val)) = pHalData->BasicRateSet; ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ ../drivers/net/wireless/realtek/rtl8192cu/hal/rtl8192c/usb/usb_halinit.c:5695:3: note: here case HW_VAR_TXPAUSE: ^~~~ ../drivers/net/wireless/realtek/rtl8192cu/os_dep/linux/ioctl_linux.c: In function ‘set_group_key’: ../drivers/net/wireless/realtek/rtl8192cu/os_dep/linux/ioctl_linux.c:7383:11: warning: this statement may fall through [-Wimplicit-fallthrough=] keylen = 16; ~~~~~~~^~~~ ../drivers/net/wireless/realtek/rtl8192cu/os_dep/linux/ioctl_linux.c:7384:3: note: here default: ^~~~~~~ ../drivers/net/wireless/realtek/rtl8192cu/os_dep/linux/ioctl_cfg80211.c: In function ‘set_group_key’: ../drivers/net/wireless/realtek/rtl8192cu/os_dep/linux/ioctl_cfg80211.c:822:11: warning: this statement may fall through [-Wimplicit-fallthrough=] keylen = 16; ~~~~~~~^~~~ ../drivers/net/wireless/realtek/rtl8192cu/os_dep/linux/ioctl_cfg80211.c:823:3: note: here default: ^~~~~~~ None of them appear to be a real issue but it is trivial to make the warnings go away. Signed-off-by: Nathan Chancellor <[email protected]>
This warning appears with GCC 7.3.0 from toolchains.bootlin.com: ../drivers/usb/host/dwc_otg/dwc_otg_fiq_fsm.c: In function ‘fiq_fsm_update_hs_isoc’: ../drivers/usb/host/dwc_otg/dwc_otg_fiq_fsm.c:595:61: warning: statement will never be executed [-Wswitch-unreachable] st->hctsiz_copy.b.xfersize = nrpackets * st->hcchar_copy.b.mps; ~~~~~~~~~~~~~~~~~^~~~ Signed-off-by: Nathan Chancellor <[email protected]>
470b351
to
ec0b94d
Compare
Thanks! |
kernel: Fix GCC 6.4.0 and 7.3.0 warnings See: raspberrypi/linux#2413 kernel: audioinjector-octo: Add continuous clock feature See: raspberrypi/linux#2409 kernel: overlays: Add 'upstream' overlay See: raspberrypi/linux#2393 kernel: overlays: Add overlay for PiBell soundcard See: https://www.raspberrypi.org/forums/viewtopic.php?f=44&t=99784&p=1279490#p1278971 kernel: Removing (broken) RaspiDac3 support kernel: i2c: bcm2835: Set up the rising/falling edge delays See: raspberrypi/linux#2407 kernel: overlays: Add updated mmc1 alias to sdio overlays kernel: config: Enable CONFIG_GPIO_MOCKUP module See: raspberrypi/linux#2410 kernel: overlays: Rework sdio overlays to allow polling See: raspberrypi/linux#2401 kernel: firmware/raspberrypi: Add a get_throttled sysfs file See: raspberrypi/linux#2397 firmware: dtoverlay: Also allow fragment-<n> in overlays firmware: i2c_gpio: Optimise and run clients faster
kernel: Fix GCC 6.4.0 and 7.3.0 warnings See: raspberrypi/linux#2413 kernel: audioinjector-octo: Add continuous clock feature See: raspberrypi/linux#2409 kernel: overlays: Add 'upstream' overlay See: raspberrypi/linux#2393 kernel: overlays: Add overlay for PiBell soundcard See: https://www.raspberrypi.org/forums/viewtopic.php?f=44&t=99784&p=1279490#p1278971 kernel: Removing (broken) RaspiDac3 support kernel: i2c: bcm2835: Set up the rising/falling edge delays See: raspberrypi/linux#2407 kernel: overlays: Add updated mmc1 alias to sdio overlays kernel: config: Enable CONFIG_GPIO_MOCKUP module See: raspberrypi/linux#2410 kernel: overlays: Rework sdio overlays to allow polling See: raspberrypi/linux#2401 kernel: firmware/raspberrypi: Add a get_throttled sysfs file See: raspberrypi/linux#2397 firmware: dtoverlay: Also allow fragment-<n> in overlays firmware: i2c_gpio: Optimise and run clients faster
commit 950ac33 upstream. The STM32MP1 RTC may have 2 clocks, the pclk and the rtc_ck. If clk_prepare_enable() fails for the second clock (rtc_ck) we must only call clk_disable_unprepare() for the first clock (pclk) but currently we call it on both leading to a WARN: [ 15.629568] WARNING: CPU: 0 PID: 146 at drivers/clk/clk.c:958 clk_core_disable+0xb0/0xc8 [ 15.637620] ck_rtc already disabled [ 15.663322] CPU: 0 PID: 146 Comm: systemd-udevd Not tainted 5.4.77-pknbsp-svn5759-atag-v5.4.77-204-gea4235203137-dirty #2413 [ 15.674510] Hardware name: STM32 (Device Tree Support) [ 15.679658] [<c0111148>] (unwind_backtrace) from [<c010c0b8>] (show_stack+0x10/0x14) [ 15.687371] [<c010c0b8>] (show_stack) from [<c0ab3d28>] (dump_stack+0xc0/0xe0) [ 15.694574] [<c0ab3d28>] (dump_stack) from [<c012360c>] (__warn+0xc8/0xf0) [ 15.701428] [<c012360c>] (__warn) from [<c0123694>] (warn_slowpath_fmt+0x60/0x94) [ 15.708894] [<c0123694>] (warn_slowpath_fmt) from [<c053b518>] (clk_core_disable+0xb0/0xc8) [ 15.717230] [<c053b518>] (clk_core_disable) from [<c053c190>] (clk_core_disable_lock+0x18/0x24) [ 15.725924] [<c053c190>] (clk_core_disable_lock) from [<bf0adc44>] (stm32_rtc_probe+0x124/0x5e4 [rtc_stm32]) [ 15.735739] [<bf0adc44>] (stm32_rtc_probe [rtc_stm32]) from [<c05f7d4c>] (platform_drv_probe+0x48/0x98) [ 15.745095] [<c05f7d4c>] (platform_drv_probe) from [<c05f5cec>] (really_probe+0x1f0/0x458) [ 15.753338] [<c05f5cec>] (really_probe) from [<c05f61c4>] (driver_probe_device+0x70/0x1c4) [ 15.761584] [<c05f61c4>] (driver_probe_device) from [<c05f6580>] (device_driver_attach+0x58/0x60) [ 15.770439] [<c05f6580>] (device_driver_attach) from [<c05f6654>] (__driver_attach+0xcc/0x170) [ 15.779032] [<c05f6654>] (__driver_attach) from [<c05f40d8>] (bus_for_each_dev+0x58/0x7c) [ 15.787191] [<c05f40d8>] (bus_for_each_dev) from [<c05f4ffc>] (bus_add_driver+0xdc/0x1f8) [ 15.795352] [<c05f4ffc>] (bus_add_driver) from [<c05f6ed8>] (driver_register+0x7c/0x110) [ 15.803425] [<c05f6ed8>] (driver_register) from [<c01027bc>] (do_one_initcall+0x70/0x1b8) [ 15.811588] [<c01027bc>] (do_one_initcall) from [<c01a1094>] (do_init_module+0x58/0x1f8) [ 15.819660] [<c01a1094>] (do_init_module) from [<c01a0074>] (load_module+0x1e58/0x23c8) [ 15.827646] [<c01a0074>] (load_module) from [<c01a0860>] (sys_finit_module+0xa0/0xd4) [ 15.835459] [<c01a0860>] (sys_finit_module) from [<c01011e0>] (__sys_trace_return+0x0/0x20) Signed-off-by: Martin Fuzzey <[email protected]> Fixes: 4e64350 ("rtc: add STM32 RTC driver") Cc: [email protected] Reviewed-by: Nobuhiro Iwamatsu <[email protected]> Signed-off-by: Alexandre Belloni <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 950ac33 upstream. The STM32MP1 RTC may have 2 clocks, the pclk and the rtc_ck. If clk_prepare_enable() fails for the second clock (rtc_ck) we must only call clk_disable_unprepare() for the first clock (pclk) but currently we call it on both leading to a WARN: [ 15.629568] WARNING: CPU: 0 PID: 146 at drivers/clk/clk.c:958 clk_core_disable+0xb0/0xc8 [ 15.637620] ck_rtc already disabled [ 15.663322] CPU: 0 PID: 146 Comm: systemd-udevd Not tainted 5.4.77-pknbsp-svn5759-atag-v5.4.77-204-gea4235203137-dirty #2413 [ 15.674510] Hardware name: STM32 (Device Tree Support) [ 15.679658] [<c0111148>] (unwind_backtrace) from [<c010c0b8>] (show_stack+0x10/0x14) [ 15.687371] [<c010c0b8>] (show_stack) from [<c0ab3d28>] (dump_stack+0xc0/0xe0) [ 15.694574] [<c0ab3d28>] (dump_stack) from [<c012360c>] (__warn+0xc8/0xf0) [ 15.701428] [<c012360c>] (__warn) from [<c0123694>] (warn_slowpath_fmt+0x60/0x94) [ 15.708894] [<c0123694>] (warn_slowpath_fmt) from [<c053b518>] (clk_core_disable+0xb0/0xc8) [ 15.717230] [<c053b518>] (clk_core_disable) from [<c053c190>] (clk_core_disable_lock+0x18/0x24) [ 15.725924] [<c053c190>] (clk_core_disable_lock) from [<bf0adc44>] (stm32_rtc_probe+0x124/0x5e4 [rtc_stm32]) [ 15.735739] [<bf0adc44>] (stm32_rtc_probe [rtc_stm32]) from [<c05f7d4c>] (platform_drv_probe+0x48/0x98) [ 15.745095] [<c05f7d4c>] (platform_drv_probe) from [<c05f5cec>] (really_probe+0x1f0/0x458) [ 15.753338] [<c05f5cec>] (really_probe) from [<c05f61c4>] (driver_probe_device+0x70/0x1c4) [ 15.761584] [<c05f61c4>] (driver_probe_device) from [<c05f6580>] (device_driver_attach+0x58/0x60) [ 15.770439] [<c05f6580>] (device_driver_attach) from [<c05f6654>] (__driver_attach+0xcc/0x170) [ 15.779032] [<c05f6654>] (__driver_attach) from [<c05f40d8>] (bus_for_each_dev+0x58/0x7c) [ 15.787191] [<c05f40d8>] (bus_for_each_dev) from [<c05f4ffc>] (bus_add_driver+0xdc/0x1f8) [ 15.795352] [<c05f4ffc>] (bus_add_driver) from [<c05f6ed8>] (driver_register+0x7c/0x110) [ 15.803425] [<c05f6ed8>] (driver_register) from [<c01027bc>] (do_one_initcall+0x70/0x1b8) [ 15.811588] [<c01027bc>] (do_one_initcall) from [<c01a1094>] (do_init_module+0x58/0x1f8) [ 15.819660] [<c01a1094>] (do_init_module) from [<c01a0074>] (load_module+0x1e58/0x23c8) [ 15.827646] [<c01a0074>] (load_module) from [<c01a0860>] (sys_finit_module+0xa0/0xd4) [ 15.835459] [<c01a0860>] (sys_finit_module) from [<c01011e0>] (__sys_trace_return+0x0/0x20) Signed-off-by: Martin Fuzzey <[email protected]> Fixes: 4e64350 ("rtc: add STM32 RTC driver") Cc: [email protected] Reviewed-by: Nobuhiro Iwamatsu <[email protected]> Signed-off-by: Alexandre Belloni <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
commit 950ac33 upstream. The STM32MP1 RTC may have 2 clocks, the pclk and the rtc_ck. If clk_prepare_enable() fails for the second clock (rtc_ck) we must only call clk_disable_unprepare() for the first clock (pclk) but currently we call it on both leading to a WARN: [ 15.629568] WARNING: CPU: 0 PID: 146 at drivers/clk/clk.c:958 clk_core_disable+0xb0/0xc8 [ 15.637620] ck_rtc already disabled [ 15.663322] CPU: 0 PID: 146 Comm: systemd-udevd Not tainted 5.4.77-pknbsp-svn5759-atag-v5.4.77-204-gea4235203137-dirty #2413 [ 15.674510] Hardware name: STM32 (Device Tree Support) [ 15.679658] [<c0111148>] (unwind_backtrace) from [<c010c0b8>] (show_stack+0x10/0x14) [ 15.687371] [<c010c0b8>] (show_stack) from [<c0ab3d28>] (dump_stack+0xc0/0xe0) [ 15.694574] [<c0ab3d28>] (dump_stack) from [<c012360c>] (__warn+0xc8/0xf0) [ 15.701428] [<c012360c>] (__warn) from [<c0123694>] (warn_slowpath_fmt+0x60/0x94) [ 15.708894] [<c0123694>] (warn_slowpath_fmt) from [<c053b518>] (clk_core_disable+0xb0/0xc8) [ 15.717230] [<c053b518>] (clk_core_disable) from [<c053c190>] (clk_core_disable_lock+0x18/0x24) [ 15.725924] [<c053c190>] (clk_core_disable_lock) from [<bf0adc44>] (stm32_rtc_probe+0x124/0x5e4 [rtc_stm32]) [ 15.735739] [<bf0adc44>] (stm32_rtc_probe [rtc_stm32]) from [<c05f7d4c>] (platform_drv_probe+0x48/0x98) [ 15.745095] [<c05f7d4c>] (platform_drv_probe) from [<c05f5cec>] (really_probe+0x1f0/0x458) [ 15.753338] [<c05f5cec>] (really_probe) from [<c05f61c4>] (driver_probe_device+0x70/0x1c4) [ 15.761584] [<c05f61c4>] (driver_probe_device) from [<c05f6580>] (device_driver_attach+0x58/0x60) [ 15.770439] [<c05f6580>] (device_driver_attach) from [<c05f6654>] (__driver_attach+0xcc/0x170) [ 15.779032] [<c05f6654>] (__driver_attach) from [<c05f40d8>] (bus_for_each_dev+0x58/0x7c) [ 15.787191] [<c05f40d8>] (bus_for_each_dev) from [<c05f4ffc>] (bus_add_driver+0xdc/0x1f8) [ 15.795352] [<c05f4ffc>] (bus_add_driver) from [<c05f6ed8>] (driver_register+0x7c/0x110) [ 15.803425] [<c05f6ed8>] (driver_register) from [<c01027bc>] (do_one_initcall+0x70/0x1b8) [ 15.811588] [<c01027bc>] (do_one_initcall) from [<c01a1094>] (do_init_module+0x58/0x1f8) [ 15.819660] [<c01a1094>] (do_init_module) from [<c01a0074>] (load_module+0x1e58/0x23c8) [ 15.827646] [<c01a0074>] (load_module) from [<c01a0860>] (sys_finit_module+0xa0/0xd4) [ 15.835459] [<c01a0860>] (sys_finit_module) from [<c01011e0>] (__sys_trace_return+0x0/0x20) Signed-off-by: Martin Fuzzey <[email protected]> Fixes: 4e64350 ("rtc: add STM32 RTC driver") Cc: [email protected] Reviewed-by: Nobuhiro Iwamatsu <[email protected]> Signed-off-by: Alexandre Belloni <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Greg Kroah-Hartman <[email protected]>
I ran into these warnings on the latest 4.14 branch.
Pretty simple to fix and now
make > /dev/null
is clean.