* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver [not found] <20170329174751.13184-1-hdegoede@redhat.com> @ 2017-03-29 17:54 ` Hans de Goede 2017-03-30 1:20 ` Larry Finger 2017-04-04 18:31 ` Larry Finger 2 siblings, 0 replies; 18+ messages in thread From: Hans de Goede @ 2017-03-29 17:54 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Bastien Nocera, Larry Finger, Jes Sorensen, linux-wireless Hi All, I forgot to add --compose so I could add the cover letter I had already written, so here it goes: Hi Greg, all, I'm hereby submitting the rtl8723bs driver which Bastien Nocera and Larry Finger have been maintaining at: https://github.com/hadess/rtl8723bs For inclusion into staging, like most of the other rtl* drivers in staging this is a cleaned up version of Realtek's own driver code for Linux. I would like to see this added to staging because the rtl8723bs is found on quite a few systems used by Linux users, such as on Atom systems (Intel Computestick and various other Atom based devices) and on many (budget) ARM boards such as the CHIP. The plan moving forward with this is for the new clean, written from scratch, rtl8xxxu driver to eventually gain support for sdio devices. But there is no clear timeline for that as Jes is doing that in his spare time, so I would like to see this driver included in staging for now. Regards, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver [not found] <20170329174751.13184-1-hdegoede@redhat.com> 2017-03-29 17:54 ` [PATCH] staging: Add rtl8723bs sdio wifi driver Hans de Goede @ 2017-03-30 1:20 ` Larry Finger 2017-03-30 7:15 ` Greg Kroah-Hartman 2017-04-04 18:31 ` Larry Finger 2 siblings, 1 reply; 18+ messages in thread From: Larry Finger @ 2017-03-30 1:20 UTC (permalink / raw) To: Hans de Goede, Greg Kroah-Hartman Cc: Bastien Nocera, Jes Sorensen, linux-wireless On 03/29/2017 12:47 PM, Hans de Goede wrote: > The rtl8723bs is found on quite a few systems used by Linux users, > such as on Atom systems (Intel Computestick and various other > Atom based devices) and on many (budget) ARM boards such as > the CHIP. > > The plan moving forward with this is for the new clean, > written from scratch, rtl8xxxu driver to eventually gain > support for sdio devices. But there is no clear timeline > for that, so lets add this driver included in staging for now. > > Cc: Bastien Nocera <hadess@hadess.net> > Cc: Larry Finger <Larry.Finger@lwfinger.net> > Cc: Jes Sorensen <jes.sorensen@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Hans, I am still building the driver on my CherryTrail tablet, but I did see some warnings and errors listed by Smatch as follows: finger@linux-4v1g:~/wireless-drivers-next> make CHECK=~/smatch/smatch C=2 drivers/staging/rtl8723bs/ CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h CHECK arch/x86/purgatory/purgatory.c CHECK arch/x86/purgatory/sha256.c CHECK arch/x86/purgatory/string.c CHK include/generated/bounds.h CHK include/generated/timeconst.h CHK include/generated/asm-offsets.h CALL scripts/checksyscalls.sh CHECK scripts/mod/empty.c CHECK drivers/staging/rtl8723bs/core/rtw_ap.c drivers/staging/rtl8723bs/core/rtw_ap.c:382 expire_timeout_chk() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/core/rtw_btcoex.c CHECK drivers/staging/rtl8723bs/core/rtw_cmd.c CHECK drivers/staging/rtl8723bs/core/rtw_debug.c drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() error: we previously assumed 'phead' could be null (see line 453) drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() warn: variable dereferenced before check 'phead' (see line 454) CHECK drivers/staging/rtl8723bs/core/rtw_efuse.c CHECK drivers/staging/rtl8723bs/core/rtw_io.c CHECK drivers/staging/rtl8723bs/core/rtw_ioctl_set.c CHECK drivers/staging/rtl8723bs/core/rtw_ieee80211.c drivers/staging/rtl8723bs/core/rtw_ieee80211.c:83 rtw_is_cckrates_included() warn: if statement not indented drivers/staging/rtl8723bs/core/rtw_ieee80211.c:98 rtw_is_cckratesonly_included() warn: if statement not indented CHECK drivers/staging/rtl8723bs/core/rtw_mlme.c drivers/staging/rtl8723bs/core/rtw_mlme.c:862 rtw_survey_event_callback() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_mlme.c:1102 rtw_free_assoc_resources() warn: if statement not indented drivers/staging/rtl8723bs/core/rtw_mlme.c:1165 rtw_indicate_disconnect() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_mlme.c:2830 rtw_restructure_ht_ie() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_mlme.c:2991 rtw_update_ht_cap() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/core/rtw_mlme_ext.c drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:525 _mgt_dispatcher() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:1595 OnAssocReq() error: buffer overflow 'pstapriv->sta_aid' 32 <= 32 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:2391 dump_mgntframe_and_wait() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:2420 dump_mgntframe_and_wait_ack() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:4969 process_80211d() error: testing array offset 'i' after use. drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:5738 linked_status_chk() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:6459 sitesurvey_cmd_hdl() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/core/rtw_odm.c drivers/staging/rtl8723bs/core/rtw_odm.c:109 rtw_odm_dbg_comp_msg() warn: if statement not indented drivers/staging/rtl8723bs/core/rtw_odm.c:146 rtw_odm_ability_msg() warn: if statement not indented CHECK drivers/staging/rtl8723bs/core/rtw_pwrctrl.c drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:641 LeaveAllPowerSaveModeDirect() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/core/rtw_recv.c drivers/staging/rtl8723bs/core/rtw_recv.c:598 portctrl() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_recv.c:838 sta2sta_data_frame() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_recv.c:1547 validate_recv_frame() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/core/rtw_rf.c CHECK drivers/staging/rtl8723bs/core/rtw_security.c drivers/staging/rtl8723bs/core/rtw_security.c:266 rtw_wep_encrypt() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:433 rtw_seccalctkipmic() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:749 rtw_tkip_encrypt() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:865 rtw_tkip_decrypt() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:1383 aes_cipher() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:1415 aes_cipher() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:1430 aes_cipher() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:1582 rtw_aes_encrypt() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:1651 aes_decipher() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:1739 aes_decipher() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_security.c:1792 aes_decipher() warn: curly braces intended? drivers/staging/rtl8723bs/core/rtw_security.c:1809 aes_decipher() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/core/rtw_sta_mgt.c drivers/staging/rtl8723bs/core/rtw_sta_mgt.c:25 _rtw_init_stainfo() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/core/rtw_wlan_util.c drivers/staging/rtl8723bs/core/rtw_wlan_util.c:67 cckrates_included() warn: if statement not indented drivers/staging/rtl8723bs/core/rtw_wlan_util.c:81 cckratesonly_included() warn: if statement not indented drivers/staging/rtl8723bs/core/rtw_wlan_util.c:815 rtw_camid_alloc() warn: should '1 << (cam_id)' be a 64 bit type? CHECK drivers/staging/rtl8723bs/core/rtw_xmit.c drivers/staging/rtl8723bs/core/rtw_xmit.c:277 _rtw_init_xmit_priv() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_xmit.c:294 _rtw_free_xmit_priv() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_xmit.c:295 _rtw_free_xmit_priv() warn: inconsistent indenting drivers/staging/rtl8723bs/core/rtw_xmit.c:946 xmitframe_addmic() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/hal/hal_intf.c CHECK drivers/staging/rtl8723bs/hal/hal_com.c drivers/staging/rtl8723bs/hal/hal_com.c:1734 rtw_bb_rf_gain_offset() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/hal/hal_com_phycfg.c drivers/staging/rtl8723bs/hal/hal_com_phycfg.c:2090 Hal_ChannelPlanToRegulation() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/hal/hal_btcoex.c CHECK drivers/staging/rtl8723bs/hal/hal_sdio.c CHECK drivers/staging/rtl8723bs/hal/Hal8723BPwrSeq.c CHECK drivers/staging/rtl8723bs/hal/HalPhyRf.c CHECK drivers/staging/rtl8723bs/hal/HalPwrSeqCmd.c CHECK drivers/staging/rtl8723bs/hal/odm.c CHECK drivers/staging/rtl8723bs/hal/odm_CfoTracking.c CHECK drivers/staging/rtl8723bs/hal/odm_debug.c CHECK drivers/staging/rtl8723bs/hal/odm_DIG.c CHECK drivers/staging/rtl8723bs/hal/odm_DynamicBBPowerSaving.c CHECK drivers/staging/rtl8723bs/hal/odm_DynamicTxPower.c CHECK drivers/staging/rtl8723bs/hal/odm_EdcaTurboCheck.c CHECK drivers/staging/rtl8723bs/hal/odm_HWConfig.c CHECK drivers/staging/rtl8723bs/hal/odm_NoiseMonitor.c CHECK drivers/staging/rtl8723bs/hal/odm_PathDiv.c CHECK drivers/staging/rtl8723bs/hal/odm_RegConfig8723B.c CHECK drivers/staging/rtl8723bs/hal/odm_RTL8723B.c CHECK drivers/staging/rtl8723bs/hal/rtl8723b_cmd.c CHECK drivers/staging/rtl8723bs/hal/rtl8723b_dm.c CHECK drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c:518 rtl8723b_FirmwareDownload() error: we previously assumed 'pFirmware' could be null (see line 382) CHECK drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c CHECK drivers/staging/rtl8723bs/hal/rtl8723b_rf6052.c CHECK drivers/staging/rtl8723bs/hal/rtl8723b_rxdesc.c CHECK drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c CHECK drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c CHECK drivers/staging/rtl8723bs/hal/sdio_halinit.c CHECK drivers/staging/rtl8723bs/hal/sdio_ops.c CHECK drivers/staging/rtl8723bs/hal/HalBtc8723b1Ant.c CHECK drivers/staging/rtl8723bs/hal/HalBtc8723b2Ant.c CHECK drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:314 ODM_ReadAndConfig_MP_8723B_AGC_TAB() warn: for statement not indented drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:583 ODM_ReadAndConfig_MP_8723B_PHY_REG() warn: for statement not indented drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:586 ODM_ReadAndConfig_MP_8723B_PHY_REG() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/hal/HalHWImg8723B_MAC.c CHECK drivers/staging/rtl8723bs/hal/HalHWImg8723B_RF.c CHECK drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c CHECK drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:470 rtw_cfg80211_ibss_indicate_connect() error: we previously assumed 'scanned' could be null (see line 466) drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:942 rtw_cfg80211_set_encryption() warn: inconsistent indenting drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:955 rtw_cfg80211_set_encryption() error: buffer overflow 'psecuritypriv->dot11DefKey' 4 <= 4 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1017 rtw_cfg80211_set_encryption() error: buffer overflow 'padapter->securitypriv.dot118021XGrpKey' 5 <= 5 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1216 cfg80211_rtw_set_default_key() warn: inconsistent indenting drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2498 rtw_cfg80211_monitor_if_xmit_entry() error: we previously assumed 'skb' could be null (see line 2495) drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2850 cfg80211_rtw_start_ap() warn: if statement not indented drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2860 cfg80211_rtw_start_ap() warn: if statement not indented drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3417 rtw_cfg80211_preinit_wiphy() warn: inconsistent indenting drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3547 rtw_wdev_alloc() info: ignoring unreachable code. CHECK drivers/staging/rtl8723bs/os_dep/ioctl_linux.c CHECK drivers/staging/rtl8723bs/os_dep/mlme_linux.c drivers/staging/rtl8723bs/os_dep/mlme_linux.c:149 rtw_os_indicate_disconnect() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/os_dep/osdep_service.c CHECK drivers/staging/rtl8723bs/os_dep/os_intfs.c drivers/staging/rtl8723bs/os_dep/os_intfs.c:1082 ips_netdrv_open() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/os_dep/recv_linux.c drivers/staging/rtl8723bs/os_dep/recv_linux.c:353 rtw_recv_indicatepkt() warn: variable dereferenced before check 'precv_frame' (see line 312) CHECK drivers/staging/rtl8723bs/os_dep/rtw_proc.c drivers/staging/rtl8723bs/os_dep/rtw_proc.c:102 rtw_drv_proc_open() warn: inconsistent indenting CHECK drivers/staging/rtl8723bs/os_dep/sdio_intf.c CHECK drivers/staging/rtl8723bs/os_dep/sdio_ops_linux.c CHECK drivers/staging/rtl8723bs/os_dep/wifi_regd.c CHECK drivers/staging/rtl8723bs/os_dep/xmit_linux.c drivers/staging/rtl8723bs/os_dep/xmit_linux.c:42 _rtw_pktfile_read() warn: inconsistent indenting The indent problems can probably be ignored, but many of the others are serious enough to be addressed now. Once this driver is merged into staging, the Outreachy folks will be really busy. Thanks for posting this driver, Larry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-03-30 1:20 ` Larry Finger @ 2017-03-30 7:15 ` Greg Kroah-Hartman 2017-03-30 14:06 ` Jes Sorensen 0 siblings, 1 reply; 18+ messages in thread From: Greg Kroah-Hartman @ 2017-03-30 7:15 UTC (permalink / raw) To: Larry Finger; +Cc: Hans de Goede, Bastien Nocera, Jes Sorensen, linux-wireless On Wed, Mar 29, 2017 at 08:20:19PM -0500, Larry Finger wrote: > On 03/29/2017 12:47 PM, Hans de Goede wrote: > > The rtl8723bs is found on quite a few systems used by Linux users, > > such as on Atom systems (Intel Computestick and various other > > Atom based devices) and on many (budget) ARM boards such as > > the CHIP. > > > > The plan moving forward with this is for the new clean, > > written from scratch, rtl8xxxu driver to eventually gain > > support for sdio devices. But there is no clear timeline > > for that, so lets add this driver included in staging for now. > > > > Cc: Bastien Nocera <hadess@hadess.net> > > Cc: Larry Finger <Larry.Finger@lwfinger.net> > > Cc: Jes Sorensen <jes.sorensen@gmail.com> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Hans, > > I am still building the driver on my CherryTrail tablet, but I did see some > warnings and errors listed by Smatch as follows: > > finger@linux-4v1g:~/wireless-drivers-next> make CHECK=~/smatch/smatch C=2 > drivers/staging/rtl8723bs/ > CHK include/config/kernel.release > CHK include/generated/uapi/linux/version.h > CHK include/generated/utsrelease.h > CHECK arch/x86/purgatory/purgatory.c > CHECK arch/x86/purgatory/sha256.c > CHECK arch/x86/purgatory/string.c > CHK include/generated/bounds.h > CHK include/generated/timeconst.h > CHK include/generated/asm-offsets.h > CALL scripts/checksyscalls.sh > CHECK scripts/mod/empty.c > CHECK drivers/staging/rtl8723bs/core/rtw_ap.c > drivers/staging/rtl8723bs/core/rtw_ap.c:382 expire_timeout_chk() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/core/rtw_btcoex.c > CHECK drivers/staging/rtl8723bs/core/rtw_cmd.c > CHECK drivers/staging/rtl8723bs/core/rtw_debug.c > drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() error: > we previously assumed 'phead' could be null (see line 453) > drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() warn: > variable dereferenced before check 'phead' (see line 454) > CHECK drivers/staging/rtl8723bs/core/rtw_efuse.c > CHECK drivers/staging/rtl8723bs/core/rtw_io.c > CHECK drivers/staging/rtl8723bs/core/rtw_ioctl_set.c > CHECK drivers/staging/rtl8723bs/core/rtw_ieee80211.c > drivers/staging/rtl8723bs/core/rtw_ieee80211.c:83 rtw_is_cckrates_included() > warn: if statement not indented > drivers/staging/rtl8723bs/core/rtw_ieee80211.c:98 > rtw_is_cckratesonly_included() warn: if statement not indented > CHECK drivers/staging/rtl8723bs/core/rtw_mlme.c > drivers/staging/rtl8723bs/core/rtw_mlme.c:862 rtw_survey_event_callback() > warn: inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_mlme.c:1102 rtw_free_assoc_resources() > warn: if statement not indented > drivers/staging/rtl8723bs/core/rtw_mlme.c:1165 rtw_indicate_disconnect() > warn: inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_mlme.c:2830 rtw_restructure_ht_ie() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_mlme.c:2991 rtw_update_ht_cap() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/core/rtw_mlme_ext.c > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:525 _mgt_dispatcher() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:1595 OnAssocReq() error: > buffer overflow 'pstapriv->sta_aid' 32 <= 32 > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:2391 dump_mgntframe_and_wait() > warn: inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:2420 > dump_mgntframe_and_wait_ack() warn: inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:4969 process_80211d() error: > testing array offset 'i' after use. > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:5738 linked_status_chk() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_mlme_ext.c:6459 sitesurvey_cmd_hdl() > warn: inconsistent indenting > CHECK drivers/staging/rtl8723bs/core/rtw_odm.c > drivers/staging/rtl8723bs/core/rtw_odm.c:109 rtw_odm_dbg_comp_msg() warn: if > statement not indented > drivers/staging/rtl8723bs/core/rtw_odm.c:146 rtw_odm_ability_msg() warn: if > statement not indented > CHECK drivers/staging/rtl8723bs/core/rtw_pwrctrl.c > drivers/staging/rtl8723bs/core/rtw_pwrctrl.c:641 > LeaveAllPowerSaveModeDirect() warn: inconsistent indenting > CHECK drivers/staging/rtl8723bs/core/rtw_recv.c > drivers/staging/rtl8723bs/core/rtw_recv.c:598 portctrl() warn: inconsistent > indenting > drivers/staging/rtl8723bs/core/rtw_recv.c:838 sta2sta_data_frame() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_recv.c:1547 validate_recv_frame() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/core/rtw_rf.c > CHECK drivers/staging/rtl8723bs/core/rtw_security.c > drivers/staging/rtl8723bs/core/rtw_security.c:266 rtw_wep_encrypt() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:433 rtw_seccalctkipmic() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:749 rtw_tkip_encrypt() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:865 rtw_tkip_decrypt() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:1383 aes_cipher() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:1415 aes_cipher() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:1430 aes_cipher() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:1582 rtw_aes_encrypt() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:1651 aes_decipher() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:1739 aes_decipher() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_security.c:1792 aes_decipher() warn: > curly braces intended? > drivers/staging/rtl8723bs/core/rtw_security.c:1809 aes_decipher() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/core/rtw_sta_mgt.c > drivers/staging/rtl8723bs/core/rtw_sta_mgt.c:25 _rtw_init_stainfo() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/core/rtw_wlan_util.c > drivers/staging/rtl8723bs/core/rtw_wlan_util.c:67 cckrates_included() warn: > if statement not indented > drivers/staging/rtl8723bs/core/rtw_wlan_util.c:81 cckratesonly_included() > warn: if statement not indented > drivers/staging/rtl8723bs/core/rtw_wlan_util.c:815 rtw_camid_alloc() warn: > should '1 << (cam_id)' be a 64 bit type? > CHECK drivers/staging/rtl8723bs/core/rtw_xmit.c > drivers/staging/rtl8723bs/core/rtw_xmit.c:277 _rtw_init_xmit_priv() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_xmit.c:294 _rtw_free_xmit_priv() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_xmit.c:295 _rtw_free_xmit_priv() warn: > inconsistent indenting > drivers/staging/rtl8723bs/core/rtw_xmit.c:946 xmitframe_addmic() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/hal/hal_intf.c > CHECK drivers/staging/rtl8723bs/hal/hal_com.c > drivers/staging/rtl8723bs/hal/hal_com.c:1734 rtw_bb_rf_gain_offset() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/hal/hal_com_phycfg.c > drivers/staging/rtl8723bs/hal/hal_com_phycfg.c:2090 > Hal_ChannelPlanToRegulation() warn: inconsistent indenting > CHECK drivers/staging/rtl8723bs/hal/hal_btcoex.c > CHECK drivers/staging/rtl8723bs/hal/hal_sdio.c > CHECK drivers/staging/rtl8723bs/hal/Hal8723BPwrSeq.c > CHECK drivers/staging/rtl8723bs/hal/HalPhyRf.c > CHECK drivers/staging/rtl8723bs/hal/HalPwrSeqCmd.c > CHECK drivers/staging/rtl8723bs/hal/odm.c > CHECK drivers/staging/rtl8723bs/hal/odm_CfoTracking.c > CHECK drivers/staging/rtl8723bs/hal/odm_debug.c > CHECK drivers/staging/rtl8723bs/hal/odm_DIG.c > CHECK drivers/staging/rtl8723bs/hal/odm_DynamicBBPowerSaving.c > CHECK drivers/staging/rtl8723bs/hal/odm_DynamicTxPower.c > CHECK drivers/staging/rtl8723bs/hal/odm_EdcaTurboCheck.c > CHECK drivers/staging/rtl8723bs/hal/odm_HWConfig.c > CHECK drivers/staging/rtl8723bs/hal/odm_NoiseMonitor.c > CHECK drivers/staging/rtl8723bs/hal/odm_PathDiv.c > CHECK drivers/staging/rtl8723bs/hal/odm_RegConfig8723B.c > CHECK drivers/staging/rtl8723bs/hal/odm_RTL8723B.c > CHECK drivers/staging/rtl8723bs/hal/rtl8723b_cmd.c > CHECK drivers/staging/rtl8723bs/hal/rtl8723b_dm.c > CHECK drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c > drivers/staging/rtl8723bs/hal/rtl8723b_hal_init.c:518 > rtl8723b_FirmwareDownload() error: we previously assumed 'pFirmware' could > be null (see line 382) > CHECK drivers/staging/rtl8723bs/hal/rtl8723b_phycfg.c > CHECK drivers/staging/rtl8723bs/hal/rtl8723b_rf6052.c > CHECK drivers/staging/rtl8723bs/hal/rtl8723b_rxdesc.c > CHECK drivers/staging/rtl8723bs/hal/rtl8723bs_recv.c > CHECK drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c > CHECK drivers/staging/rtl8723bs/hal/sdio_halinit.c > CHECK drivers/staging/rtl8723bs/hal/sdio_ops.c > CHECK drivers/staging/rtl8723bs/hal/HalBtc8723b1Ant.c > CHECK drivers/staging/rtl8723bs/hal/HalBtc8723b2Ant.c > CHECK drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c > drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:314 > ODM_ReadAndConfig_MP_8723B_AGC_TAB() warn: for statement not indented > drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:583 > ODM_ReadAndConfig_MP_8723B_PHY_REG() warn: for statement not indented > drivers/staging/rtl8723bs/hal/HalHWImg8723B_BB.c:586 > ODM_ReadAndConfig_MP_8723B_PHY_REG() warn: inconsistent indenting > CHECK drivers/staging/rtl8723bs/hal/HalHWImg8723B_MAC.c > CHECK drivers/staging/rtl8723bs/hal/HalHWImg8723B_RF.c > CHECK drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c > CHECK drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:470 > rtw_cfg80211_ibss_indicate_connect() error: we previously assumed 'scanned' > could be null (see line 466) > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:942 > rtw_cfg80211_set_encryption() warn: inconsistent indenting > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:955 > rtw_cfg80211_set_encryption() error: buffer overflow > 'psecuritypriv->dot11DefKey' 4 <= 4 > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1017 > rtw_cfg80211_set_encryption() error: buffer overflow > 'padapter->securitypriv.dot118021XGrpKey' 5 <= 5 > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:1216 > cfg80211_rtw_set_default_key() warn: inconsistent indenting > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2498 > rtw_cfg80211_monitor_if_xmit_entry() error: we previously assumed 'skb' > could be null (see line 2495) > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2850 > cfg80211_rtw_start_ap() warn: if statement not indented > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:2860 > cfg80211_rtw_start_ap() warn: if statement not indented > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3417 > rtw_cfg80211_preinit_wiphy() warn: inconsistent indenting > drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c:3547 rtw_wdev_alloc() > info: ignoring unreachable code. > CHECK drivers/staging/rtl8723bs/os_dep/ioctl_linux.c > CHECK drivers/staging/rtl8723bs/os_dep/mlme_linux.c > drivers/staging/rtl8723bs/os_dep/mlme_linux.c:149 > rtw_os_indicate_disconnect() warn: inconsistent indenting > CHECK drivers/staging/rtl8723bs/os_dep/osdep_service.c > CHECK drivers/staging/rtl8723bs/os_dep/os_intfs.c > drivers/staging/rtl8723bs/os_dep/os_intfs.c:1082 ips_netdrv_open() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/os_dep/recv_linux.c > drivers/staging/rtl8723bs/os_dep/recv_linux.c:353 rtw_recv_indicatepkt() > warn: variable dereferenced before check 'precv_frame' (see line 312) > CHECK drivers/staging/rtl8723bs/os_dep/rtw_proc.c > drivers/staging/rtl8723bs/os_dep/rtw_proc.c:102 rtw_drv_proc_open() warn: > inconsistent indenting > CHECK drivers/staging/rtl8723bs/os_dep/sdio_intf.c > CHECK drivers/staging/rtl8723bs/os_dep/sdio_ops_linux.c > CHECK drivers/staging/rtl8723bs/os_dep/wifi_regd.c > CHECK drivers/staging/rtl8723bs/os_dep/xmit_linux.c > drivers/staging/rtl8723bs/os_dep/xmit_linux.c:42 _rtw_pktfile_read() warn: > inconsistent indenting > > The indent problems can probably be ignored, but many of the others are > serious enough to be addressed now. > > Once this driver is merged into staging, the Outreachy folks will be really busy. > > Thanks for posting this driver, So, no objection from you? Great, I'll go queue it up later today... thanks, greg k-h ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-03-30 7:15 ` Greg Kroah-Hartman @ 2017-03-30 14:06 ` Jes Sorensen 2017-03-30 15:22 ` poma 0 siblings, 1 reply; 18+ messages in thread From: Jes Sorensen @ 2017-03-30 14:06 UTC (permalink / raw) To: Greg Kroah-Hartman, Larry Finger Cc: Hans de Goede, Bastien Nocera, linux-wireless On 03/30/2017 03:15 AM, Greg Kroah-Hartman wrote: > On Wed, Mar 29, 2017 at 08:20:19PM -0500, Larry Finger wrote: >> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>> The rtl8723bs is found on quite a few systems used by Linux users, >>> such as on Atom systems (Intel Computestick and various other >>> Atom based devices) and on many (budget) ARM boards such as >>> the CHIP. >>> >>> The plan moving forward with this is for the new clean, >>> written from scratch, rtl8xxxu driver to eventually gain >>> support for sdio devices. But there is no clear timeline >>> for that, so lets add this driver included in staging for now. >>> >>> Cc: Bastien Nocera <hadess@hadess.net> >>> Cc: Larry Finger <Larry.Finger@lwfinger.net> >>> Cc: Jes Sorensen <jes.sorensen@gmail.com> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> >> Hans, >> >> I am still building the driver on my CherryTrail tablet, but I did see some >> warnings and errors listed by Smatch as follows: >> [snip] >> The indent problems can probably be ignored, but many of the others are >> serious enough to be addressed now. >> >> Once this driver is merged into staging, the Outreachy folks will be really busy. >> >> Thanks for posting this driver, > > So, no objection from you? Great, I'll go queue it up later today... > > thanks, > > greg k-h > As much as I hate adding more Realtek vendor code to the kernel, I do agree this is the right thing to do in the interim. Not sure when I'll get to SDIO support in rtl8xxxu, but I will process patches, hint hint :) Jes ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-03-30 14:06 ` Jes Sorensen @ 2017-03-30 15:22 ` poma 0 siblings, 0 replies; 18+ messages in thread From: poma @ 2017-03-30 15:22 UTC (permalink / raw) To: Jes Sorensen, Greg Kroah-Hartman, Larry Finger Cc: Hans de Goede, Bastien Nocera, linux-wireless On 30.03.2017 16:06, Jes Sorensen wrote: > On 03/30/2017 03:15 AM, Greg Kroah-Hartman wrote: >> On Wed, Mar 29, 2017 at 08:20:19PM -0500, Larry Finger wrote: >>> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>>> The rtl8723bs is found on quite a few systems used by Linux users, >>>> such as on Atom systems (Intel Computestick and various other >>>> Atom based devices) and on many (budget) ARM boards such as >>>> the CHIP. >>>> >>>> The plan moving forward with this is for the new clean, >>>> written from scratch, rtl8xxxu driver to eventually gain >>>> support for sdio devices. But there is no clear timeline >>>> for that, so lets add this driver included in staging for now. >>>> >>>> Cc: Bastien Nocera <hadess@hadess.net> >>>> Cc: Larry Finger <Larry.Finger@lwfinger.net> >>>> Cc: Jes Sorensen <jes.sorensen@gmail.com> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> >>> Hans, >>> >>> I am still building the driver on my CherryTrail tablet, but I did see some >>> warnings and errors listed by Smatch as follows: >>> > [snip] >>> The indent problems can probably be ignored, but many of the others are >>> serious enough to be addressed now. >>> >>> Once this driver is merged into staging, the Outreachy folks will be really busy. >>> >>> Thanks for posting this driver, >> >> So, no objection from you? Great, I'll go queue it up later today... >> >> thanks, >> >> greg k-h >> > > As much as I hate adding more Realtek vendor code to the kernel, I do > agree this is the right thing to do in the interim. Not sure when I'll > get to SDIO support in rtl8xxxu, but I will process patches, hint hint :) > > Jes > A day after tomorrow? :) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver [not found] <20170329174751.13184-1-hdegoede@redhat.com> 2017-03-29 17:54 ` [PATCH] staging: Add rtl8723bs sdio wifi driver Hans de Goede 2017-03-30 1:20 ` Larry Finger @ 2017-04-04 18:31 ` Larry Finger 2017-04-04 18:53 ` Hans de Goede 2 siblings, 1 reply; 18+ messages in thread From: Larry Finger @ 2017-04-04 18:31 UTC (permalink / raw) To: Hans de Goede; +Cc: Bastien Nocera, Jes Sorensen, linux-wireless On 03/29/2017 12:47 PM, Hans de Goede wrote: > The rtl8723bs is found on quite a few systems used by Linux users, > such as on Atom systems (Intel Computestick and various other > Atom based devices) and on many (budget) ARM boards such as > the CHIP. > > The plan moving forward with this is for the new clean, > written from scratch, rtl8xxxu driver to eventually gain > support for sdio devices. But there is no clear timeline > for that, so lets add this driver included in staging for now. Hans, I started looking at the Smatch errors. This one may be the result of a serious problem: CHECK drivers/staging/rtl8723bs/core/rtw_debug.c drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() error: we previously assumed 'phead' could be null (see line 453) drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() warn: variable dereferenced before check 'phead' (see line 454) A snippet of the code in question is as follows: spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); phead = get_list_head(queue); plist = phead ? get_next(phead) : NULL; plist = get_next(phead); if ((!phead) || (!plist)) { spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); return 0; } This code comes directly from the hadess repo, but I am suspicious of the double call to get_next(phead). I cannot imagine any valid reason to skip every other entry on that list. Larry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-04 18:31 ` Larry Finger @ 2017-04-04 18:53 ` Hans de Goede 2017-04-04 19:47 ` Larry Finger 2017-04-04 21:38 ` Arend Van Spriel 0 siblings, 2 replies; 18+ messages in thread From: Hans de Goede @ 2017-04-04 18:53 UTC (permalink / raw) To: Larry Finger; +Cc: Bastien Nocera, Jes Sorensen, linux-wireless Hi, On 04/04/2017 08:31 PM, Larry Finger wrote: > On 03/29/2017 12:47 PM, Hans de Goede wrote: >> The rtl8723bs is found on quite a few systems used by Linux users, >> such as on Atom systems (Intel Computestick and various other >> Atom based devices) and on many (budget) ARM boards such as >> the CHIP. >> >> The plan moving forward with this is for the new clean, >> written from scratch, rtl8xxxu driver to eventually gain >> support for sdio devices. But there is no clear timeline >> for that, so lets add this driver included in staging for now. > > Hans, > > I started looking at the Smatch errors. This one may be the result of a serious problem: > > CHECK drivers/staging/rtl8723bs/core/rtw_debug.c > drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() error: we previously assumed 'phead' could be null (see line 453) > drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() warn: variable dereferenced before check 'phead' (see line 454) > > A snippet of the code in question is as follows: > > spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); > phead = get_list_head(queue); > plist = phead ? get_next(phead) : NULL; > plist = get_next(phead); > if ((!phead) || (!plist)) { > spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); > return 0; > } > > This code comes directly from the hadess repo, but I am suspicious of the double call to get_next(phead). I cannot imagine any valid reason to skip every other entry on that list. If you look closer and simplify the first getnext line what is written is: plist = get_next(phead); plist = get_next(phead); Which indeed looks like it could use improvement, but I don't think it is seriously broken. Regards, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-04 18:53 ` Hans de Goede @ 2017-04-04 19:47 ` Larry Finger 2017-04-04 21:38 ` Arend Van Spriel 1 sibling, 0 replies; 18+ messages in thread From: Larry Finger @ 2017-04-04 19:47 UTC (permalink / raw) To: Hans de Goede; +Cc: Bastien Nocera, Jes Sorensen, linux-wireless On 04/04/2017 01:53 PM, Hans de Goede wrote: > Hi, > > On 04/04/2017 08:31 PM, Larry Finger wrote: >> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>> The rtl8723bs is found on quite a few systems used by Linux users, >>> such as on Atom systems (Intel Computestick and various other >>> Atom based devices) and on many (budget) ARM boards such as >>> the CHIP. >>> >>> The plan moving forward with this is for the new clean, >>> written from scratch, rtl8xxxu driver to eventually gain >>> support for sdio devices. But there is no clear timeline >>> for that, so lets add this driver included in staging for now. >> >> Hans, >> >> I started looking at the Smatch errors. This one may be the result of a >> serious problem: >> >> CHECK drivers/staging/rtl8723bs/core/rtw_debug.c >> drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() error: >> we previously assumed 'phead' could be null (see line 453) >> drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() warn: >> variable dereferenced before check 'phead' (see line 454) >> >> A snippet of the code in question is as follows: >> >> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >> phead = get_list_head(queue); >> plist = phead ? get_next(phead) : NULL; >> plist = get_next(phead); >> if ((!phead) || (!plist)) { >> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >> return 0; >> } >> >> This code comes directly from the hadess repo, but I am suspicious of the >> double call to get_next(phead). I cannot imagine any valid reason to skip >> every other entry on that list. > > If you look closer and simplify the first getnext line what is written is: > > plist = get_next(phead); > plist = get_next(phead); > > Which indeed looks like it could use improvement, but I don't > think it is seriously broken. My problem was that I thought that get_next() took an entry off the list. Now that I actually looked, I see that it just gets the next pointer. Sorry for the noise, Larry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-04 18:53 ` Hans de Goede 2017-04-04 19:47 ` Larry Finger @ 2017-04-04 21:38 ` Arend Van Spriel 2017-04-04 21:53 ` Hans de Goede 1 sibling, 1 reply; 18+ messages in thread From: Arend Van Spriel @ 2017-04-04 21:38 UTC (permalink / raw) To: Hans de Goede, Larry Finger; +Cc: Bastien Nocera, Jes Sorensen, linux-wireless On 4-4-2017 20:53, Hans de Goede wrote: > Hi, > > On 04/04/2017 08:31 PM, Larry Finger wrote: >> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>> The rtl8723bs is found on quite a few systems used by Linux users, >>> such as on Atom systems (Intel Computestick and various other >>> Atom based devices) and on many (budget) ARM boards such as >>> the CHIP. >>> >>> The plan moving forward with this is for the new clean, >>> written from scratch, rtl8xxxu driver to eventually gain >>> support for sdio devices. But there is no clear timeline >>> for that, so lets add this driver included in staging for now. >> >> Hans, >> >> I started looking at the Smatch errors. This one may be the result of >> a serious problem: >> >> CHECK drivers/staging/rtl8723bs/core/rtw_debug.c >> drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() >> error: we previously assumed 'phead' could be null (see line 453) >> drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() >> warn: variable dereferenced before check 'phead' (see line 454) >> >> A snippet of the code in question is as follows: >> >> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >> phead = get_list_head(queue); >> plist = phead ? get_next(phead) : NULL; >> plist = get_next(phead); >> if ((!phead) || (!plist)) { >> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >> return 0; >> } >> >> This code comes directly from the hadess repo, but I am suspicious of >> the double call to get_next(phead). I cannot imagine any valid reason >> to skip every other entry on that list. > > If you look closer and simplify the first getnext line what is written is: > > plist = get_next(phead); > plist = get_next(phead); > > Which indeed looks like it could use improvement, but I don't > think it is seriously broken. So get_list_head(queue) can never return NULL? Otherwise I don't know how close I need to get for a simplified look ;-) Gr. AvS > Regards, > > Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-04 21:38 ` Arend Van Spriel @ 2017-04-04 21:53 ` Hans de Goede 2017-04-04 23:41 ` Larry Finger 0 siblings, 1 reply; 18+ messages in thread From: Hans de Goede @ 2017-04-04 21:53 UTC (permalink / raw) To: Arend Van Spriel, Larry Finger Cc: Bastien Nocera, Jes Sorensen, linux-wireless Hi, On 04/04/2017 11:38 PM, Arend Van Spriel wrote: > > > On 4-4-2017 20:53, Hans de Goede wrote: >> Hi, >> >> On 04/04/2017 08:31 PM, Larry Finger wrote: >>> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>>> The rtl8723bs is found on quite a few systems used by Linux users, >>>> such as on Atom systems (Intel Computestick and various other >>>> Atom based devices) and on many (budget) ARM boards such as >>>> the CHIP. >>>> >>>> The plan moving forward with this is for the new clean, >>>> written from scratch, rtl8xxxu driver to eventually gain >>>> support for sdio devices. But there is no clear timeline >>>> for that, so lets add this driver included in staging for now. >>> >>> Hans, >>> >>> I started looking at the Smatch errors. This one may be the result of >>> a serious problem: >>> >>> CHECK drivers/staging/rtl8723bs/core/rtw_debug.c >>> drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() >>> error: we previously assumed 'phead' could be null (see line 453) >>> drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() >>> warn: variable dereferenced before check 'phead' (see line 454) >>> >>> A snippet of the code in question is as follows: >>> >>> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >>> phead = get_list_head(queue); >>> plist = phead ? get_next(phead) : NULL; >>> plist = get_next(phead); >>> if ((!phead) || (!plist)) { >>> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >>> return 0; >>> } >>> >>> This code comes directly from the hadess repo, but I am suspicious of >>> the double call to get_next(phead). I cannot imagine any valid reason >>> to skip every other entry on that list. >> >> If you look closer and simplify the first getnext line what is written is: >> >> plist = get_next(phead); >> plist = get_next(phead); >> >> Which indeed looks like it could use improvement, but I don't >> think it is seriously broken. > > So get_list_head(queue) can never return NULL? I don't know. > Otherwise I don't know > how close I need to get for a simplified look ;-) I simplified things so as to make clear that Larry's worry was not really a problem, I do agree the smatch complaint is valid. Regards, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-04 21:53 ` Hans de Goede @ 2017-04-04 23:41 ` Larry Finger 2017-04-05 9:36 ` Hans de Goede 0 siblings, 1 reply; 18+ messages in thread From: Larry Finger @ 2017-04-04 23:41 UTC (permalink / raw) To: Hans de Goede, Arend Van Spriel Cc: Bastien Nocera, Jes Sorensen, linux-wireless On 04/04/2017 04:53 PM, Hans de Goede wrote: > Hi, > > On 04/04/2017 11:38 PM, Arend Van Spriel wrote: >> >> >> On 4-4-2017 20:53, Hans de Goede wrote: >>> Hi, >>> >>> On 04/04/2017 08:31 PM, Larry Finger wrote: >>>> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>>>> The rtl8723bs is found on quite a few systems used by Linux users, >>>>> such as on Atom systems (Intel Computestick and various other >>>>> Atom based devices) and on many (budget) ARM boards such as >>>>> the CHIP. >>>>> >>>>> The plan moving forward with this is for the new clean, >>>>> written from scratch, rtl8xxxu driver to eventually gain >>>>> support for sdio devices. But there is no clear timeline >>>>> for that, so lets add this driver included in staging for now. >>>> >>>> Hans, >>>> >>>> I started looking at the Smatch errors. This one may be the result of >>>> a serious problem: >>>> >>>> CHECK drivers/staging/rtl8723bs/core/rtw_debug.c >>>> drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() >>>> error: we previously assumed 'phead' could be null (see line 453) >>>> drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() >>>> warn: variable dereferenced before check 'phead' (see line 454) >>>> >>>> A snippet of the code in question is as follows: >>>> >>>> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >>>> phead = get_list_head(queue); >>>> plist = phead ? get_next(phead) : NULL; >>>> plist = get_next(phead); >>>> if ((!phead) || (!plist)) { >>>> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >>>> return 0; >>>> } >>>> >>>> This code comes directly from the hadess repo, but I am suspicious of >>>> the double call to get_next(phead). I cannot imagine any valid reason >>>> to skip every other entry on that list. >>> >>> If you look closer and simplify the first getnext line what is written is: >>> >>> plist = get_next(phead); >>> plist = get_next(phead); >>> >>> Which indeed looks like it could use improvement, but I don't >>> think it is seriously broken. >> >> So get_list_head(queue) can never return NULL? > > I don't know. > >> Otherwise I don't know >> how close I need to get for a simplified look ;-) > > I simplified things so as to make clear that Larry's worry was > not really a problem, I do agree the smatch complaint is valid. I think the following fixes the problem: diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c b/drivers/staging/rtl8723bs/core/rtw_debug.c index d34747b29309..51cef55d3f76 100644 --- a/drivers/staging/rtl8723bs/core/rtw_debug.c +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c @@ -451,7 +451,6 @@ int proc_get_survey_info(struct seq_file *m, void *v) spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); phead = get_list_head(queue); plist = phead ? get_next(phead) : NULL; - plist = get_next(phead); if ((!phead) || (!plist)) { spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); return 0; Pointer plist is set in the 3rd line, thus the second set of it can be removed. Larry ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-04 23:41 ` Larry Finger @ 2017-04-05 9:36 ` Hans de Goede 2017-04-05 16:32 ` Larry Finger 0 siblings, 1 reply; 18+ messages in thread From: Hans de Goede @ 2017-04-05 9:36 UTC (permalink / raw) To: Larry Finger, Arend Van Spriel Cc: Bastien Nocera, Jes Sorensen, linux-wireless Hi, On 05-04-17 01:41, Larry Finger wrote: > On 04/04/2017 04:53 PM, Hans de Goede wrote: >> Hi, >> >> On 04/04/2017 11:38 PM, Arend Van Spriel wrote: >>> >>> >>> On 4-4-2017 20:53, Hans de Goede wrote: >>>> Hi, >>>> >>>> On 04/04/2017 08:31 PM, Larry Finger wrote: >>>>> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>>>>> The rtl8723bs is found on quite a few systems used by Linux users, >>>>>> such as on Atom systems (Intel Computestick and various other >>>>>> Atom based devices) and on many (budget) ARM boards such as >>>>>> the CHIP. >>>>>> >>>>>> The plan moving forward with this is for the new clean, >>>>>> written from scratch, rtl8xxxu driver to eventually gain >>>>>> support for sdio devices. But there is no clear timeline >>>>>> for that, so lets add this driver included in staging for now. >>>>> >>>>> Hans, >>>>> >>>>> I started looking at the Smatch errors. This one may be the result of >>>>> a serious problem: >>>>> >>>>> CHECK drivers/staging/rtl8723bs/core/rtw_debug.c >>>>> drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() >>>>> error: we previously assumed 'phead' could be null (see line 453) >>>>> drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() >>>>> warn: variable dereferenced before check 'phead' (see line 454) >>>>> >>>>> A snippet of the code in question is as follows: >>>>> >>>>> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >>>>> phead = get_list_head(queue); >>>>> plist = phead ? get_next(phead) : NULL; >>>>> plist = get_next(phead); >>>>> if ((!phead) || (!plist)) { >>>>> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >>>>> return 0; >>>>> } >>>>> >>>>> This code comes directly from the hadess repo, but I am suspicious of >>>>> the double call to get_next(phead). I cannot imagine any valid reason >>>>> to skip every other entry on that list. >>>> >>>> If you look closer and simplify the first getnext line what is written is: >>>> >>>> plist = get_next(phead); >>>> plist = get_next(phead); >>>> >>>> Which indeed looks like it could use improvement, but I don't >>>> think it is seriously broken. >>> >>> So get_list_head(queue) can never return NULL? >> >> I don't know. >> >>> Otherwise I don't know >>> how close I need to get for a simplified look ;-) >> >> I simplified things so as to make clear that Larry's worry was >> not really a problem, I do agree the smatch complaint is valid. > > I think the following fixes the problem: > > diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c b/drivers/staging/rtl8723bs/core/rtw_debug.c > index d34747b29309..51cef55d3f76 100644 > --- a/drivers/staging/rtl8723bs/core/rtw_debug.c > +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c > @@ -451,7 +451,6 @@ int proc_get_survey_info(struct seq_file *m, void *v) > spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); > phead = get_list_head(queue); > plist = phead ? get_next(phead) : NULL; > - plist = get_next(phead); > if ((!phead) || (!plist)) { > spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); > return 0; > > Pointer plist is set in the 3rd line, thus the second set of it can be removed. Agreed, when I've some time I plan to do a follow-up patch to my initial submission fixing all the Smatch errors. But feel free to beat me to it. Greg, if I understood you correctly you plan to merge my initial submission as is and then we can fix this (and other things) with follow up patches, right ? Regards, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-05 9:36 ` Hans de Goede @ 2017-04-05 16:32 ` Larry Finger 2017-04-06 6:55 ` Hans de Goede 0 siblings, 1 reply; 18+ messages in thread From: Larry Finger @ 2017-04-05 16:32 UTC (permalink / raw) To: Hans de Goede, Arend Van Spriel Cc: Bastien Nocera, Jes Sorensen, linux-wireless On 04/05/2017 04:36 AM, Hans de Goede wrote: > Hi, > > On 05-04-17 01:41, Larry Finger wrote: >> On 04/04/2017 04:53 PM, Hans de Goede wrote: >>> Hi, >>> >>> On 04/04/2017 11:38 PM, Arend Van Spriel wrote: >>>> >>>> >>>> On 4-4-2017 20:53, Hans de Goede wrote: >>>>> Hi, >>>>> >>>>> On 04/04/2017 08:31 PM, Larry Finger wrote: >>>>>> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>>>>>> The rtl8723bs is found on quite a few systems used by Linux users, >>>>>>> such as on Atom systems (Intel Computestick and various other >>>>>>> Atom based devices) and on many (budget) ARM boards such as >>>>>>> the CHIP. >>>>>>> >>>>>>> The plan moving forward with this is for the new clean, >>>>>>> written from scratch, rtl8xxxu driver to eventually gain >>>>>>> support for sdio devices. But there is no clear timeline >>>>>>> for that, so lets add this driver included in staging for now. >>>>>> >>>>>> Hans, >>>>>> >>>>>> I started looking at the Smatch errors. This one may be the result of >>>>>> a serious problem: >>>>>> >>>>>> CHECK drivers/staging/rtl8723bs/core/rtw_debug.c >>>>>> drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() >>>>>> error: we previously assumed 'phead' could be null (see line 453) >>>>>> drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() >>>>>> warn: variable dereferenced before check 'phead' (see line 454) >>>>>> >>>>>> A snippet of the code in question is as follows: >>>>>> >>>>>> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >>>>>> phead = get_list_head(queue); >>>>>> plist = phead ? get_next(phead) : NULL; >>>>>> plist = get_next(phead); >>>>>> if ((!phead) || (!plist)) { >>>>>> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >>>>>> return 0; >>>>>> } >>>>>> >>>>>> This code comes directly from the hadess repo, but I am suspicious of >>>>>> the double call to get_next(phead). I cannot imagine any valid reason >>>>>> to skip every other entry on that list. >>>>> >>>>> If you look closer and simplify the first getnext line what is written is: >>>>> >>>>> plist = get_next(phead); >>>>> plist = get_next(phead); >>>>> >>>>> Which indeed looks like it could use improvement, but I don't >>>>> think it is seriously broken. >>>> >>>> So get_list_head(queue) can never return NULL? >>> >>> I don't know. >>> >>>> Otherwise I don't know >>>> how close I need to get for a simplified look ;-) >>> >>> I simplified things so as to make clear that Larry's worry was >>> not really a problem, I do agree the smatch complaint is valid. >> >> I think the following fixes the problem: >> >> diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c >> b/drivers/staging/rtl8723bs/core/rtw_debug.c >> index d34747b29309..51cef55d3f76 100644 >> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c >> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c >> @@ -451,7 +451,6 @@ int proc_get_survey_info(struct seq_file *m, void *v) >> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >> phead = get_list_head(queue); >> plist = phead ? get_next(phead) : NULL; >> - plist = get_next(phead); >> if ((!phead) || (!plist)) { >> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >> return 0; >> >> Pointer plist is set in the 3rd line, thus the second set of it can be removed. > > Agreed, when I've some time I plan to do a follow-up patch > to my initial submission fixing all the Smatch errors. But feel > free to beat me to it. > > Greg, if I understood you correctly you plan to merge my initial > submission as is and then we can fix this (and other things) with > follow up patches, right ? Hans, I took GregKH out of the Cc list when I started the discussion of the Smatch errors/warnings, and he probably has not seen the recent E-mails in this thread. It it was my understanding that he planned to apply your submission in the form you sent. I have several patches nearly ready to fix most of the Smatch warnings and errors. I will send them as soon as the original submission is applied. Larry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-05 16:32 ` Larry Finger @ 2017-04-06 6:55 ` Hans de Goede 2017-04-06 9:04 ` Bastien Nocera 0 siblings, 1 reply; 18+ messages in thread From: Hans de Goede @ 2017-04-06 6:55 UTC (permalink / raw) To: Larry Finger, Arend Van Spriel Cc: Bastien Nocera, Jes Sorensen, linux-wireless Hi, On 05-04-17 18:32, Larry Finger wrote: > On 04/05/2017 04:36 AM, Hans de Goede wrote: >> Hi, >> >> On 05-04-17 01:41, Larry Finger wrote: >>> On 04/04/2017 04:53 PM, Hans de Goede wrote: >>>> Hi, >>>> >>>> On 04/04/2017 11:38 PM, Arend Van Spriel wrote: >>>>> >>>>> >>>>> On 4-4-2017 20:53, Hans de Goede wrote: >>>>>> Hi, >>>>>> >>>>>> On 04/04/2017 08:31 PM, Larry Finger wrote: >>>>>>> On 03/29/2017 12:47 PM, Hans de Goede wrote: >>>>>>>> The rtl8723bs is found on quite a few systems used by Linux users, >>>>>>>> such as on Atom systems (Intel Computestick and various other >>>>>>>> Atom based devices) and on many (budget) ARM boards such as >>>>>>>> the CHIP. >>>>>>>> >>>>>>>> The plan moving forward with this is for the new clean, >>>>>>>> written from scratch, rtl8xxxu driver to eventually gain >>>>>>>> support for sdio devices. But there is no clear timeline >>>>>>>> for that, so lets add this driver included in staging for now. >>>>>>> >>>>>>> Hans, >>>>>>> >>>>>>> I started looking at the Smatch errors. This one may be the result of >>>>>>> a serious problem: >>>>>>> >>>>>>> CHECK drivers/staging/rtl8723bs/core/rtw_debug.c >>>>>>> drivers/staging/rtl8723bs/core/rtw_debug.c:454 proc_get_survey_info() >>>>>>> error: we previously assumed 'phead' could be null (see line 453) >>>>>>> drivers/staging/rtl8723bs/core/rtw_debug.c:455 proc_get_survey_info() >>>>>>> warn: variable dereferenced before check 'phead' (see line 454) >>>>>>> >>>>>>> A snippet of the code in question is as follows: >>>>>>> >>>>>>> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >>>>>>> phead = get_list_head(queue); >>>>>>> plist = phead ? get_next(phead) : NULL; >>>>>>> plist = get_next(phead); >>>>>>> if ((!phead) || (!plist)) { >>>>>>> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> This code comes directly from the hadess repo, but I am suspicious of >>>>>>> the double call to get_next(phead). I cannot imagine any valid reason >>>>>>> to skip every other entry on that list. >>>>>> >>>>>> If you look closer and simplify the first getnext line what is written is: >>>>>> >>>>>> plist = get_next(phead); >>>>>> plist = get_next(phead); >>>>>> >>>>>> Which indeed looks like it could use improvement, but I don't >>>>>> think it is seriously broken. >>>>> >>>>> So get_list_head(queue) can never return NULL? >>>> >>>> I don't know. >>>> >>>>> Otherwise I don't know >>>>> how close I need to get for a simplified look ;-) >>>> >>>> I simplified things so as to make clear that Larry's worry was >>>> not really a problem, I do agree the smatch complaint is valid. >>> >>> I think the following fixes the problem: >>> >>> diff --git a/drivers/staging/rtl8723bs/core/rtw_debug.c >>> b/drivers/staging/rtl8723bs/core/rtw_debug.c >>> index d34747b29309..51cef55d3f76 100644 >>> --- a/drivers/staging/rtl8723bs/core/rtw_debug.c >>> +++ b/drivers/staging/rtl8723bs/core/rtw_debug.c >>> @@ -451,7 +451,6 @@ int proc_get_survey_info(struct seq_file *m, void *v) >>> spin_lock_bh(&(pmlmepriv->scanned_queue.lock)); >>> phead = get_list_head(queue); >>> plist = phead ? get_next(phead) : NULL; >>> - plist = get_next(phead); >>> if ((!phead) || (!plist)) { >>> spin_unlock_bh(&(pmlmepriv->scanned_queue.lock)); >>> return 0; >>> >>> Pointer plist is set in the 3rd line, thus the second set of it can be removed. >> >> Agreed, when I've some time I plan to do a follow-up patch >> to my initial submission fixing all the Smatch errors. But feel >> free to beat me to it. >> >> Greg, if I understood you correctly you plan to merge my initial >> submission as is and then we can fix this (and other things) with >> follow up patches, right ? > > Hans, > > I took GregKH out of the Cc list when I started the discussion of the Smatch errors/warnings, and he probably has not seen the recent E-mails in this thread. Ah I missed that (clearly I did). > It it was my understanding that he planned to apply your submission in the form you sent. That is my understanding too. > I have several patches nearly ready to fix most of the Smatch warnings and errors. I will send them as soon as the original submission is applied. Good thank you. So what is the plan with the github version ? Note that my submission contains a few small fixes on top of the github version, for which I intended to submit a pull-req but I've not gotten around to that yet, I've done so now: https://github.com/hadess/rtl8723bs/pull/125 But do we want to keep maintaining the github version (for a while at least) I wonder, as that does mean double work? Regards, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-06 6:55 ` Hans de Goede @ 2017-04-06 9:04 ` Bastien Nocera 2017-04-06 9:49 ` Hans de Goede 0 siblings, 1 reply; 18+ messages in thread From: Bastien Nocera @ 2017-04-06 9:04 UTC (permalink / raw) To: Hans de Goede, Larry Finger, Arend Van Spriel Cc: Jes Sorensen, linux-wireless On Thu, 2017-04-06 at 08:55 +0200, Hans de Goede wrote: > <snip> > Good thank you. So what is the plan with the github version ? > > Note that my submission contains a few small fixes on top of > the github version, for which I intended to submit a pull-req > but I've not gotten around to that yet, I've done so now: > > https://github.com/hadess/rtl8723bs/pull/125 > > But do we want to keep maintaining the github version (for a while > at least) I wonder, as that does mean double work? My plan is to remove everything and point to the upstream tree as soon as the support has landed in Linus' tree. While there is some use in keeping it around for older kernels, we keep getting asked to support even older kernels than reasonable, or have users complaining about non-working machines once they use the driver, which simply uncovers kernel bugs that were fixed upstream. I don't think it's a good use of our time to carry on supporting this out-of-tree. I'll however make sure to try and document the migration in a way that's helpful to users. Cheers ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-06 9:04 ` Bastien Nocera @ 2017-04-06 9:49 ` Hans de Goede 2017-04-06 18:32 ` Larry Finger 0 siblings, 1 reply; 18+ messages in thread From: Hans de Goede @ 2017-04-06 9:49 UTC (permalink / raw) To: Bastien Nocera, Larry Finger, Arend Van Spriel Cc: Jes Sorensen, linux-wireless Hi, On 06-04-17 11:04, Bastien Nocera wrote: > On Thu, 2017-04-06 at 08:55 +0200, Hans de Goede wrote: >> > <snip> >> Good thank you. So what is the plan with the github version ? >> >> Note that my submission contains a few small fixes on top of >> the github version, for which I intended to submit a pull-req >> but I've not gotten around to that yet, I've done so now: >> >> https://github.com/hadess/rtl8723bs/pull/125 >> >> But do we want to keep maintaining the github version (for a while >> at least) I wonder, as that does mean double work? > > My plan is to remove everything and point to the upstream tree as soon > as the support has landed in Linus' tree. > > While there is some use in keeping it around for older kernels, we keep > getting asked to support even older kernels than reasonable, or have > users complaining about non-working machines once they use the driver, > which simply uncovers kernel bugs that were fixed upstream. > > I don't think it's a good use of our time to carry on supporting this > out-of-tree. I'll however make sure to try and document the migration > in a way that's helpful to users. Ok, that works for me. Regards, Hans ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-06 9:49 ` Hans de Goede @ 2017-04-06 18:32 ` Larry Finger 2017-04-06 18:36 ` Jes Sorensen 0 siblings, 1 reply; 18+ messages in thread From: Larry Finger @ 2017-04-06 18:32 UTC (permalink / raw) To: Hans de Goede, Bastien Nocera, Arend Van Spriel Cc: Jes Sorensen, linux-wireless On 04/06/2017 04:49 AM, Hans de Goede wrote: > Hi, > > On 06-04-17 11:04, Bastien Nocera wrote: >> On Thu, 2017-04-06 at 08:55 +0200, Hans de Goede wrote: >>> >> <snip> >>> Good thank you. So what is the plan with the github version ? >>> >>> Note that my submission contains a few small fixes on top of >>> the github version, for which I intended to submit a pull-req >>> but I've not gotten around to that yet, I've done so now: >>> >>> https://github.com/hadess/rtl8723bs/pull/125 >>> >>> But do we want to keep maintaining the github version (for a while >>> at least) I wonder, as that does mean double work? >> >> My plan is to remove everything and point to the upstream tree as soon >> as the support has landed in Linus' tree. >> >> While there is some use in keeping it around for older kernels, we keep >> getting asked to support even older kernels than reasonable, or have >> users complaining about non-working machines once they use the driver, >> which simply uncovers kernel bugs that were fixed upstream. >> >> I don't think it's a good use of our time to carry on supporting this >> out-of-tree. I'll however make sure to try and document the migration >> in a way that's helpful to users. > > Ok, that works for me. I agree. I have enough to do. Larry ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] staging: Add rtl8723bs sdio wifi driver 2017-04-06 18:32 ` Larry Finger @ 2017-04-06 18:36 ` Jes Sorensen 0 siblings, 0 replies; 18+ messages in thread From: Jes Sorensen @ 2017-04-06 18:36 UTC (permalink / raw) To: Larry Finger, Hans de Goede, Bastien Nocera, Arend Van Spriel Cc: linux-wireless On 04/06/2017 02:32 PM, Larry Finger wrote: > On 04/06/2017 04:49 AM, Hans de Goede wrote: >> Hi, >> >> On 06-04-17 11:04, Bastien Nocera wrote: >>> On Thu, 2017-04-06 at 08:55 +0200, Hans de Goede wrote: >>>> >>> <snip> >>>> Good thank you. So what is the plan with the github version ? >>>> >>>> Note that my submission contains a few small fixes on top of >>>> the github version, for which I intended to submit a pull-req >>>> but I've not gotten around to that yet, I've done so now: >>>> >>>> https://github.com/hadess/rtl8723bs/pull/125 >>>> >>>> But do we want to keep maintaining the github version (for a while >>>> at least) I wonder, as that does mean double work? >>> >>> My plan is to remove everything and point to the upstream tree as soon >>> as the support has landed in Linus' tree. >>> >>> While there is some use in keeping it around for older kernels, we keep >>> getting asked to support even older kernels than reasonable, or have >>> users complaining about non-working machines once they use the driver, >>> which simply uncovers kernel bugs that were fixed upstream. >>> >>> I don't think it's a good use of our time to carry on supporting this >>> out-of-tree. I'll however make sure to try and document the migration >>> in a way that's helpful to users. >> >> Ok, that works for me. > > I agree. I have enough to do. You could simply break the build, and have it spit out a warning saying the git repo is kept there to track the old source. It might be useful to have a repository of the original code to go look at. Cheers, Jes ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2017-04-06 18:36 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20170329174751.13184-1-hdegoede@redhat.com> 2017-03-29 17:54 ` [PATCH] staging: Add rtl8723bs sdio wifi driver Hans de Goede 2017-03-30 1:20 ` Larry Finger 2017-03-30 7:15 ` Greg Kroah-Hartman 2017-03-30 14:06 ` Jes Sorensen 2017-03-30 15:22 ` poma 2017-04-04 18:31 ` Larry Finger 2017-04-04 18:53 ` Hans de Goede 2017-04-04 19:47 ` Larry Finger 2017-04-04 21:38 ` Arend Van Spriel 2017-04-04 21:53 ` Hans de Goede 2017-04-04 23:41 ` Larry Finger 2017-04-05 9:36 ` Hans de Goede 2017-04-05 16:32 ` Larry Finger 2017-04-06 6:55 ` Hans de Goede 2017-04-06 9:04 ` Bastien Nocera 2017-04-06 9:49 ` Hans de Goede 2017-04-06 18:32 ` Larry Finger 2017-04-06 18:36 ` Jes Sorensen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.