* [PATCH 0/4] staging: r8188eu: add error handling of usb read errors
@ 2022-05-18 22:11 Pavel Skripkin
2022-05-18 22:11 ` [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8 Pavel Skripkin
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Pavel Skripkin @ 2022-05-18 22:11 UTC (permalink / raw)
To: gregkh, Larry.Finger, phil, straube.linux, dan.carpenter, fmdefrancesco
Cc: linux-kernel, linux-staging, Pavel Skripkin
Hi,
it's reincarnation of my old series for adding sane error handling in
r8818eu.
*Problem*
Old code was returning just stack variable in case of read error. It's
not the best approach, since passing around stack data might cause
device misconfiguration or even kernel data leakage
To solve this I've changed rtw_read{8,16,32} prototypes to return an error via
return value and data via passed pointer. Some work should be done to
propogate an error down to calltrace, but it's good way to at least
start doing sane I/O error handling
Tested locally on qemu with TP-Link TL-WN722N v2/v3 [Realtek RTL8188EUS]
device. More testing is welcomed, of course :)
Series is based on top of staging-testing branch
Pavel Skripkin (4):
staging: r8188eu: add error handling of rtw_read8
staging: r8188eu: add error handling of rtw_read16
staging: r8188eu: add error handling of rtw_read32
MAINTAINERS: add myself as r8188eu reviewer
MAINTAINERS | 1 +
drivers/staging/r8188eu/core/rtw_cmd.c | 15 +-
drivers/staging/r8188eu/core/rtw_efuse.c | 32 ++-
drivers/staging/r8188eu/core/rtw_fw.c | 62 ++++-
drivers/staging/r8188eu/core/rtw_led.c | 20 +-
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 62 ++++-
drivers/staging/r8188eu/core/rtw_pwrctrl.c | 9 +-
drivers/staging/r8188eu/core/rtw_wlan_util.c | 20 +-
.../r8188eu/hal/Hal8188ERateAdaptive.c | 21 +-
drivers/staging/r8188eu/hal/HalPhyRf_8188e.c | 20 +-
drivers/staging/r8188eu/hal/HalPwrSeqCmd.c | 9 +-
drivers/staging/r8188eu/hal/hal_com.c | 24 +-
drivers/staging/r8188eu/hal/rtl8188e_cmd.c | 37 ++-
drivers/staging/r8188eu/hal/rtl8188e_dm.c | 6 +-
.../staging/r8188eu/hal/rtl8188e_hal_init.c | 117 +++++++--
drivers/staging/r8188eu/hal/rtl8188e_phycfg.c | 30 ++-
drivers/staging/r8188eu/hal/usb_halinit.c | 239 +++++++++++++++---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 33 ++-
drivers/staging/r8188eu/include/rtw_io.h | 6 +-
drivers/staging/r8188eu/os_dep/ioctl_linux.c | 61 ++++-
drivers/staging/r8188eu/os_dep/os_intfs.c | 19 +-
21 files changed, 681 insertions(+), 162 deletions(-)
--
2.36.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8
2022-05-18 22:11 [PATCH 0/4] staging: r8188eu: add error handling of usb read errors Pavel Skripkin
@ 2022-05-18 22:11 ` Pavel Skripkin
2022-05-19 1:34 ` kernel test robot
2022-05-19 4:33 ` Dan Carpenter
2022-05-18 22:11 ` [PATCH 2/4] staging: r8188eu: add error handling of rtw_read16 Pavel Skripkin
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Pavel Skripkin @ 2022-05-18 22:11 UTC (permalink / raw)
To: gregkh, Larry.Finger, phil, straube.linux, dan.carpenter, fmdefrancesco
Cc: linux-kernel, linux-staging, Pavel Skripkin
rtw_read8() reads data from device via USB API which may fail. In case
of any failure previous code returned stack data to callers, which is
wrong.
Fix it by changing rtw_read8() prototype and prevent caller from
touching random stack data
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/staging/r8188eu/core/rtw_efuse.c | 13 +-
drivers/staging/r8188eu/core/rtw_fw.c | 46 ++++-
drivers/staging/r8188eu/core/rtw_led.c | 20 ++-
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 49 +++++-
drivers/staging/r8188eu/core/rtw_wlan_util.c | 20 ++-
drivers/staging/r8188eu/hal/HalPhyRf_8188e.c | 17 +-
drivers/staging/r8188eu/hal/HalPwrSeqCmd.c | 9 +-
drivers/staging/r8188eu/hal/hal_com.c | 24 ++-
drivers/staging/r8188eu/hal/rtl8188e_cmd.c | 37 +++-
drivers/staging/r8188eu/hal/rtl8188e_dm.c | 6 +-
.../staging/r8188eu/hal/rtl8188e_hal_init.c | 69 ++++++--
drivers/staging/r8188eu/hal/rtl8188e_phycfg.c | 10 +-
drivers/staging/r8188eu/hal/usb_halinit.c | 159 ++++++++++++++----
drivers/staging/r8188eu/hal/usb_ops_linux.c | 7 +-
drivers/staging/r8188eu/include/rtw_io.h | 2 +-
drivers/staging/r8188eu/os_dep/ioctl_linux.c | 18 +-
16 files changed, 407 insertions(+), 99 deletions(-)
diff --git a/drivers/staging/r8188eu/core/rtw_efuse.c b/drivers/staging/r8188eu/core/rtw_efuse.c
index 0e0e60638880..a2691c7f96f6 100644
--- a/drivers/staging/r8188eu/core/rtw_efuse.c
+++ b/drivers/staging/r8188eu/core/rtw_efuse.c
@@ -28,14 +28,21 @@ ReadEFuseByte(
u32 value32;
u8 readbyte;
u16 retry;
+ int res;
/* Write Address */
rtw_write8(Adapter, EFUSE_CTRL + 1, (_offset & 0xff));
- readbyte = rtw_read8(Adapter, EFUSE_CTRL + 2);
+ res = rtw_read8(Adapter, EFUSE_CTRL + 2, &readbyte);
+ if (res)
+ return;
+
rtw_write8(Adapter, EFUSE_CTRL + 2, ((_offset >> 8) & 0x03) | (readbyte & 0xfc));
/* Write bit 32 0 */
- readbyte = rtw_read8(Adapter, EFUSE_CTRL + 3);
+ res = rtw_read8(Adapter, EFUSE_CTRL + 3, &readbyte);
+ if (res)
+ return;
+
rtw_write8(Adapter, EFUSE_CTRL + 3, (readbyte & 0x7f));
/* Check bit 32 read-ready */
@@ -54,6 +61,8 @@ ReadEFuseByte(
value32 = rtw_read32(Adapter, EFUSE_CTRL);
*pbuf = (u8)(value32 & 0xff);
+
+ /* FIXME: return an error to caller */
}
/*-----------------------------------------------------------------------------
diff --git a/drivers/staging/r8188eu/core/rtw_fw.c b/drivers/staging/r8188eu/core/rtw_fw.c
index bf077876ed3d..701b033830bc 100644
--- a/drivers/staging/r8188eu/core/rtw_fw.c
+++ b/drivers/staging/r8188eu/core/rtw_fw.c
@@ -44,18 +44,28 @@ static_assert(sizeof(struct rt_firmware_hdr) == 32);
static void fw_download_enable(struct adapter *padapter, bool enable)
{
u8 tmp;
+ int res;
if (enable) {
/* MCU firmware download enable. */
- tmp = rtw_read8(padapter, REG_MCUFWDL);
+ res = rtw_read8(padapter, REG_MCUFWDL, &tmp);
+ if (res)
+ return;
+
rtw_write8(padapter, REG_MCUFWDL, tmp | 0x01);
/* 8051 reset */
- tmp = rtw_read8(padapter, REG_MCUFWDL + 2);
+ res = rtw_read8(padapter, REG_MCUFWDL + 2, &tmp);
+ if (res)
+ return;
+
rtw_write8(padapter, REG_MCUFWDL + 2, tmp & 0xf7);
} else {
/* MCU firmware download disable. */
- tmp = rtw_read8(padapter, REG_MCUFWDL);
+ res = rtw_read8(padapter, REG_MCUFWDL, &tmp);
+ if (res)
+ return;
+
rtw_write8(padapter, REG_MCUFWDL, tmp & 0xfe);
/* Reserved for fw extension. */
@@ -125,8 +135,13 @@ static int page_write(struct adapter *padapter, u32 page, u8 *buffer, u32 size)
{
u8 value8;
u8 u8Page = (u8)(page & 0x07);
+ int res;
- value8 = (rtw_read8(padapter, REG_MCUFWDL + 2) & 0xF8) | u8Page;
+ res = rtw_read8(padapter, REG_MCUFWDL + 2, &value8);
+ if (res)
+ return res;
+
+ value8 = (value8 & 0xF8) | u8Page;
rtw_write8(padapter, REG_MCUFWDL + 2, value8);
return block_write(padapter, buffer, size);
@@ -165,8 +180,12 @@ static int write_fw(struct adapter *padapter, u8 *buffer, u32 size)
void rtw_reset_8051(struct adapter *padapter)
{
u8 val8;
+ int res;
+
+ res = rtw_read8(padapter, REG_SYS_FUNC_EN + 1, &val8);
+ if (res)
+ return;
- val8 = rtw_read8(padapter, REG_SYS_FUNC_EN + 1);
rtw_write8(padapter, REG_SYS_FUNC_EN + 1, val8 & (~BIT(2)));
rtw_write8(padapter, REG_SYS_FUNC_EN + 1, val8 | (BIT(2)));
}
@@ -240,12 +259,14 @@ int rtl8188e_firmware_download(struct adapter *padapter)
{
int ret = _SUCCESS;
u8 write_fw_retry = 0;
+ u8 reg;
unsigned long fwdl_timeout;
struct dvobj_priv *dvobj = adapter_to_dvobj(padapter);
struct device *device = dvobj_to_dev(dvobj);
struct rt_firmware_hdr *fwhdr = NULL;
u8 *fw_data;
u32 fw_size;
+ int res;
if (!dvobj->firmware.data)
ret = load_firmware(&dvobj->firmware, device);
@@ -269,7 +290,11 @@ int rtl8188e_firmware_download(struct adapter *padapter)
/* Suggested by Filen. If 8051 is running in RAM code, driver should inform Fw to reset by itself, */
/* or it will cause download Fw fail. 2010.02.01. by tynli. */
- if (rtw_read8(padapter, REG_MCUFWDL) & RAM_DL_SEL) { /* 8051 RAM code */
+ res = rtw_read8(padapter, REG_MCUFWDL, ®);
+ if (res)
+ goto exit;
+
+ if (reg & RAM_DL_SEL) { /* 8051 RAM code */
rtw_write8(padapter, REG_MCUFWDL, 0x00);
rtw_reset_8051(padapter);
}
@@ -278,7 +303,14 @@ int rtl8188e_firmware_download(struct adapter *padapter)
fwdl_timeout = jiffies + msecs_to_jiffies(500);
while (1) {
/* reset the FWDL chksum */
- rtw_write8(padapter, REG_MCUFWDL, rtw_read8(padapter, REG_MCUFWDL) | FWDL_CHKSUM_RPT);
+ res = rtw_read8(padapter, REG_MCUFWDL, ®);
+ if (res == -ENODEV)
+ break;
+
+ if (res)
+ continue;
+
+ rtw_write8(padapter, REG_MCUFWDL, reg | FWDL_CHKSUM_RPT);
ret = write_fw(padapter, fw_data, fw_size);
diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
index 2f3000428af7..b532e614c5b6 100644
--- a/drivers/staging/r8188eu/core/rtw_led.c
+++ b/drivers/staging/r8188eu/core/rtw_led.c
@@ -34,28 +34,38 @@ static void ResetLedStatus(struct LED_871x *pLed)
static void SwLedOn(struct adapter *padapter, struct LED_871x *pLed)
{
- u8 LedCfg;
+ u8 LedCfg;
+ int res;
if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
return;
- LedCfg = rtw_read8(padapter, REG_LEDCFG2);
+ res = rtw_read8(padapter, REG_LEDCFG2, &LedCfg);
+ if (res)
+ return;
+
rtw_write8(padapter, REG_LEDCFG2, (LedCfg & 0xf0) | BIT(5) | BIT(6)); /* SW control led0 on. */
pLed->bLedOn = true;
}
static void SwLedOff(struct adapter *padapter, struct LED_871x *pLed)
{
- u8 LedCfg;
+ u8 LedCfg;
+ int res;
if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
goto exit;
- LedCfg = rtw_read8(padapter, REG_LEDCFG2);/* 0x4E */
+ res = rtw_read8(padapter, REG_LEDCFG2, &LedCfg);/* 0x4E */
+ if (res)
+ goto exit;
LedCfg &= 0x90; /* Set to software control. */
rtw_write8(padapter, REG_LEDCFG2, (LedCfg | BIT(3)));
- LedCfg = rtw_read8(padapter, REG_MAC_PINMUX_CFG);
+ res = rtw_read8(padapter, REG_MAC_PINMUX_CFG, &LedCfg);
+ if (res)
+ goto exit;
+
LedCfg &= 0xFE;
rtw_write8(padapter, REG_MAC_PINMUX_CFG, LedCfg);
exit:
diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index 848b5051aa13..d4e59fab367c 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -5672,14 +5672,28 @@ unsigned int send_beacon(struct adapter *padapter)
bool get_beacon_valid_bit(struct adapter *adapter)
{
+ int res;
+ u8 reg;
+
+ res = rtw_read8(adapter, REG_TDECTRL + 2, ®);
+ if (res)
+ return false;
+
/* BIT(16) of REG_TDECTRL = BIT(0) of REG_TDECTRL+2 */
- return BIT(0) & rtw_read8(adapter, REG_TDECTRL + 2);
+ return BIT(0) & reg;
}
void clear_beacon_valid_bit(struct adapter *adapter)
{
+ int res;
+ u8 reg;
+
+ res = rtw_read8(adapter, REG_TDECTRL + 2, ®);
+ if (res)
+ return;
+
/* BIT(16) of REG_TDECTRL = BIT(0) of REG_TDECTRL+2, write 1 to clear, Clear by sw */
- rtw_write8(adapter, REG_TDECTRL + 2, rtw_read8(adapter, REG_TDECTRL + 2) | BIT(0));
+ rtw_write8(adapter, REG_TDECTRL + 2, reg | BIT(0));
}
/****************************************************************************
@@ -6007,7 +6021,8 @@ static void rtw_set_bssid(struct adapter *adapter, u8 *bssid)
static void mlme_join(struct adapter *adapter, int type)
{
struct mlme_priv *mlmepriv = &adapter->mlmepriv;
- u8 retry_limit = 0x30;
+ u8 retry_limit = 0x30, reg;
+ int res;
switch (type) {
case 0:
@@ -6032,7 +6047,11 @@ static void mlme_join(struct adapter *adapter, int type)
case 2:
/* sta add event call back */
/* enable update TSF */
- rtw_write8(adapter, REG_BCN_CTRL, rtw_read8(adapter, REG_BCN_CTRL) & (~BIT(4)));
+ res = rtw_read8(adapter, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(adapter, REG_BCN_CTRL, reg & (~BIT(4)));
if (check_fwstate(mlmepriv, WIFI_ADHOC_STATE | WIFI_ADHOC_MASTER_STATE))
retry_limit = 0x7;
@@ -6753,6 +6772,9 @@ void mlmeext_sta_add_event_callback(struct adapter *padapter, struct sta_info *p
static void mlme_disconnect(struct adapter *adapter)
{
+ int res;
+ u8 reg;
+
/* Set RCR to not to receive data frame when NO LINK state */
/* reject all data frames */
rtw_write16(adapter, REG_RXFLTMAP2, 0x00);
@@ -6761,7 +6783,12 @@ static void mlme_disconnect(struct adapter *adapter)
rtw_write8(adapter, REG_DUAL_TSF_RST, (BIT(0) | BIT(1)));
/* disable update TSF */
- rtw_write8(adapter, REG_BCN_CTRL, rtw_read8(adapter, REG_BCN_CTRL) | BIT(4));
+
+ res = rtw_read8(adapter, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(adapter, REG_BCN_CTRL, reg | BIT(4));
}
void mlmeext_sta_del_event_callback(struct adapter *padapter)
@@ -6818,11 +6845,15 @@ static u8 chk_ap_is_alive(struct sta_info *psta)
static void rtl8188e_sreset_linked_status_check(struct adapter *padapter)
{
u32 rx_dma_status = rtw_read32(padapter, REG_RXDMA_STATUS);
+ int res;
+ u8 reg;
if (rx_dma_status != 0x00)
rtw_write32(padapter, REG_RXDMA_STATUS, rx_dma_status);
- rtw_read8(padapter, REG_FMETHR);
+ /* FIXME: should this read be removed? */
+ res = rtw_read8(padapter, REG_FMETHR, ®);
+ (void)res;
}
void linked_status_chk(struct adapter *padapter)
@@ -7224,6 +7255,7 @@ u8 disconnect_hdl(struct adapter *padapter, unsigned char *pbuf)
struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
struct wlan_bssid_ex *pnetwork = (struct wlan_bssid_ex *)(&pmlmeinfo->network);
u8 val8;
+ int res;
if (is_client_associated_to_ap(padapter))
issue_deauth_ex(padapter, pnetwork->MacAddress, WLAN_REASON_DEAUTH_LEAVING, param->deauth_timeout_ms / 100, 100);
@@ -7236,7 +7268,10 @@ u8 disconnect_hdl(struct adapter *padapter, unsigned char *pbuf)
if (((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE) || ((pmlmeinfo->state & 0x03) == WIFI_FW_AP_STATE)) {
/* Stop BCN */
- val8 = rtw_read8(padapter, REG_BCN_CTRL);
+ res = rtw_read8(padapter, REG_BCN_CTRL, &val8);
+ if (res)
+ return H2C_DROPPED;
+
rtw_write8(padapter, REG_BCN_CTRL, val8 & (~(EN_BCN_FUNCTION | EN_TXBCN_RPT)));
}
diff --git a/drivers/staging/r8188eu/core/rtw_wlan_util.c b/drivers/staging/r8188eu/core/rtw_wlan_util.c
index 27035eac6e61..f5002c88a5ac 100644
--- a/drivers/staging/r8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/r8188eu/core/rtw_wlan_util.c
@@ -279,8 +279,13 @@ void Restore_DM_Func_Flag(struct adapter *padapter)
void Set_MSR(struct adapter *padapter, u8 type)
{
u8 val8;
+ int res;
- val8 = rtw_read8(padapter, MSR) & 0x0c;
+ res = rtw_read8(padapter, MSR, &val8);
+ if (res)
+ return;
+
+ val8 &= 0x0c;
val8 |= type;
rtw_write8(padapter, MSR, val8);
}
@@ -505,7 +510,11 @@ int WMM_param_handler(struct adapter *padapter, struct ndis_802_11_var_ie *pIE)
static void set_acm_ctrl(struct adapter *adapter, u8 acm_mask)
{
- u8 acmctrl = rtw_read8(adapter, REG_ACMHWCTRL);
+ u8 acmctrl;
+ int res = rtw_read8(adapter, REG_ACMHWCTRL, &acmctrl);
+
+ if (res)
+ return;
if (acm_mask > 1)
acmctrl = acmctrl | 0x1;
@@ -763,6 +772,7 @@ void HT_info_handler(struct adapter *padapter, struct ndis_802_11_var_ie *pIE)
static void set_min_ampdu_spacing(struct adapter *adapter, u8 spacing)
{
u8 sec_spacing;
+ int res;
if (spacing <= 7) {
switch (adapter->securitypriv.dot11PrivacyAlgrthm) {
@@ -784,8 +794,12 @@ static void set_min_ampdu_spacing(struct adapter *adapter, u8 spacing)
if (spacing < sec_spacing)
spacing = sec_spacing;
+ res = rtw_read8(adapter, REG_AMPDU_MIN_SPACE, &sec_spacing);
+ if (res)
+ return;
+
rtw_write8(adapter, REG_AMPDU_MIN_SPACE,
- (rtw_read8(adapter, REG_AMPDU_MIN_SPACE) & 0xf8) | spacing);
+ (sec_spacing & 0xf8) | spacing);
}
}
diff --git a/drivers/staging/r8188eu/hal/HalPhyRf_8188e.c b/drivers/staging/r8188eu/hal/HalPhyRf_8188e.c
index b944c8071a3b..aa4b4459329e 100644
--- a/drivers/staging/r8188eu/hal/HalPhyRf_8188e.c
+++ b/drivers/staging/r8188eu/hal/HalPhyRf_8188e.c
@@ -470,9 +470,17 @@ static void _PHY_SaveMACRegisters(
)
{
u32 i;
+ int res;
- for (i = 0; i < (IQK_MAC_REG_NUM - 1); i++)
- MACBackup[i] = rtw_read8(adapt, MACReg[i]);
+ for (i = 0; i < (IQK_MAC_REG_NUM - 1); i++) {
+ u8 reg;
+
+ res = rtw_read8(adapt, MACReg[i], ®);
+ if (res)
+ return;
+
+ MACBackup[i] = reg;
+ }
MACBackup[i] = rtw_read32(adapt, MACReg[i]);
}
@@ -739,9 +747,12 @@ static void phy_LCCalibrate_8188E(struct adapter *adapt)
{
u8 tmpreg;
u32 RF_Amode = 0, LC_Cal;
+ int res;
/* Check continuous TX and Packet TX */
- tmpreg = rtw_read8(adapt, 0xd03);
+ res = rtw_read8(adapt, 0xd03, &tmpreg);
+ if (res)
+ return;
if ((tmpreg & 0x70) != 0) /* Deal with contisuous TX case */
rtw_write8(adapt, 0xd03, tmpreg & 0x8F); /* disable all continuous TX */
diff --git a/drivers/staging/r8188eu/hal/HalPwrSeqCmd.c b/drivers/staging/r8188eu/hal/HalPwrSeqCmd.c
index 5b91aec6a7e3..fe2fe63dbc18 100644
--- a/drivers/staging/r8188eu/hal/HalPwrSeqCmd.c
+++ b/drivers/staging/r8188eu/hal/HalPwrSeqCmd.c
@@ -34,6 +34,7 @@ u8 HalPwrSeqCmdParsing(struct adapter *padapter, struct wl_pwr_cfg pwrseqcmd[])
u32 offset = 0;
u32 poll_count = 0; /* polling autoload done. */
u32 max_poll_count = 5000;
+ int res;
do {
pwrcfgcmd = pwrseqcmd[aryidx];
@@ -43,7 +44,9 @@ u8 HalPwrSeqCmdParsing(struct adapter *padapter, struct wl_pwr_cfg pwrseqcmd[])
offset = GET_PWR_CFG_OFFSET(pwrcfgcmd);
/* Read the value from system register */
- value = rtw_read8(padapter, offset);
+ res = rtw_read8(padapter, offset, &value);
+ if (res)
+ return false;
value &= ~(GET_PWR_CFG_MASK(pwrcfgcmd));
value |= (GET_PWR_CFG_VALUE(pwrcfgcmd) & GET_PWR_CFG_MASK(pwrcfgcmd));
@@ -55,7 +58,9 @@ u8 HalPwrSeqCmdParsing(struct adapter *padapter, struct wl_pwr_cfg pwrseqcmd[])
poll_bit = false;
offset = GET_PWR_CFG_OFFSET(pwrcfgcmd);
do {
- value = rtw_read8(padapter, offset);
+ res = rtw_read8(padapter, offset, &value);
+ if (res)
+ return false;
value &= GET_PWR_CFG_MASK(pwrcfgcmd);
if (value == (GET_PWR_CFG_VALUE(pwrcfgcmd) & GET_PWR_CFG_MASK(pwrcfgcmd)))
diff --git a/drivers/staging/r8188eu/hal/hal_com.c b/drivers/staging/r8188eu/hal/hal_com.c
index 910cc07f656c..7717ee722cce 100644
--- a/drivers/staging/r8188eu/hal/hal_com.c
+++ b/drivers/staging/r8188eu/hal/hal_com.c
@@ -297,13 +297,15 @@ s32 c2h_evt_read(struct adapter *adapter, u8 *buf)
{
s32 ret = _FAIL;
struct c2h_evt_hdr *c2h_evt;
- int i;
+ int i, res;
u8 trigger;
if (!buf)
goto exit;
- trigger = rtw_read8(adapter, REG_C2HEVT_CLEAR);
+ res = rtw_read8(adapter, REG_C2HEVT_CLEAR, &trigger);
+ if (res)
+ return _FAIL;
if (trigger == C2H_EVT_HOST_CLOSE)
goto exit; /* Not ready */
@@ -314,13 +316,21 @@ s32 c2h_evt_read(struct adapter *adapter, u8 *buf)
memset(c2h_evt, 0, 16);
- *buf = rtw_read8(adapter, REG_C2HEVT_MSG_NORMAL);
- *(buf + 1) = rtw_read8(adapter, REG_C2HEVT_MSG_NORMAL + 1);
+ res = rtw_read8(adapter, REG_C2HEVT_MSG_NORMAL, buf);
+ if (res)
+ return _FAIL;
+
+ res = rtw_read8(adapter, REG_C2HEVT_MSG_NORMAL + 1, buf + 1);
+ if (res)
+ return _FAIL;
/* Read the content */
- for (i = 0; i < c2h_evt->plen; i++)
- c2h_evt->payload[i] = rtw_read8(adapter, REG_C2HEVT_MSG_NORMAL +
- sizeof(*c2h_evt) + i);
+ for (i = 0; i < c2h_evt->plen; i++) {
+ res = rtw_read8(adapter, REG_C2HEVT_MSG_NORMAL +
+ sizeof(*c2h_evt) + i, c2h_evt->payload + i);
+ if (res)
+ return _FAIL;
+ }
ret = _SUCCESS;
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
index 475650dc7301..b01ee1695fee 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_cmd.c
@@ -18,13 +18,18 @@
static u8 _is_fw_read_cmd_down(struct adapter *adapt, u8 msgbox_num)
{
- u8 read_down = false;
+ u8 read_down = false, reg;
int retry_cnts = 100;
+ int res;
u8 valid;
do {
- valid = rtw_read8(adapt, REG_HMETFR) & BIT(msgbox_num);
+ res = rtw_read8(adapt, REG_HMETFR, ®);
+ if (res)
+ continue;
+
+ valid = reg & BIT(msgbox_num);
if (0 == valid)
read_down = true;
} while ((!read_down) && (retry_cnts--));
@@ -533,6 +538,8 @@ void rtl8188e_set_FwJoinBssReport_cmd(struct adapter *adapt, u8 mstatus)
bool bcn_valid = false;
u8 DLBcnCount = 0;
u32 poll = 0;
+ u8 reg;
+ int res;
if (mstatus == 1) {
/* We should set AID, correct TSF, HW seq enable before set JoinBssReport to Fw in 88/92C. */
@@ -547,8 +554,17 @@ void rtl8188e_set_FwJoinBssReport_cmd(struct adapter *adapt, u8 mstatus)
/* Disable Hw protection for a time which revserd for Hw sending beacon. */
/* Fix download reserved page packet fail that access collision with the protection time. */
/* 2010.05.11. Added by tynli. */
- rtw_write8(adapt, REG_BCN_CTRL, rtw_read8(adapt, REG_BCN_CTRL) & (~BIT(3)));
- rtw_write8(adapt, REG_BCN_CTRL, rtw_read8(adapt, REG_BCN_CTRL) | BIT(4));
+ res = rtw_read8(adapt, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(adapt, REG_BCN_CTRL, reg & (~BIT(3)));
+
+ res = rtw_read8(adapt, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(adapt, REG_BCN_CTRL, reg | BIT(4));
if (haldata->RegFwHwTxQCtrl & BIT(6))
bSendBeacon = true;
@@ -581,8 +597,17 @@ void rtl8188e_set_FwJoinBssReport_cmd(struct adapter *adapt, u8 mstatus)
/* */
/* Enable Bcn */
- rtw_write8(adapt, REG_BCN_CTRL, rtw_read8(adapt, REG_BCN_CTRL) | BIT(3));
- rtw_write8(adapt, REG_BCN_CTRL, rtw_read8(adapt, REG_BCN_CTRL) & (~BIT(4)));
+ res = rtw_read8(adapt, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(adapt, REG_BCN_CTRL, reg | BIT(3));
+
+ res = rtw_read8(adapt, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(adapt, REG_BCN_CTRL, reg & (~BIT(4)));
/* To make sure that if there exists an adapter which would like to send beacon. */
/* If exists, the origianl value of 0x422[6] will be 1, we should check this to */
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_dm.c b/drivers/staging/r8188eu/hal/rtl8188e_dm.c
index 6d28e3dc0d26..0399872c4546 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_dm.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_dm.c
@@ -12,8 +12,12 @@
static void dm_InitGPIOSetting(struct adapter *Adapter)
{
u8 tmp1byte;
+ int res;
+
+ res = rtw_read8(Adapter, REG_GPIO_MUXCFG, &tmp1byte);
+ if (res)
+ return;
- tmp1byte = rtw_read8(Adapter, REG_GPIO_MUXCFG);
tmp1byte &= (GPIOSEL_GPIO | ~GPIOSEL_ENBT);
rtw_write8(Adapter, REG_GPIO_MUXCFG, tmp1byte);
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
index e17375a74f17..e67ecbd1ba79 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
@@ -13,10 +13,14 @@
static void iol_mode_enable(struct adapter *padapter, u8 enable)
{
u8 reg_0xf0 = 0;
+ int res;
if (enable) {
/* Enable initial offload */
- reg_0xf0 = rtw_read8(padapter, REG_SYS_CFG);
+ res = rtw_read8(padapter, REG_SYS_CFG, ®_0xf0);
+ if (res)
+ return;
+
rtw_write8(padapter, REG_SYS_CFG, reg_0xf0 | SW_OFFLOAD_EN);
if (!padapter->bFWReady)
@@ -24,7 +28,10 @@ static void iol_mode_enable(struct adapter *padapter, u8 enable)
} else {
/* disable initial offload */
- reg_0xf0 = rtw_read8(padapter, REG_SYS_CFG);
+ res = rtw_read8(padapter, REG_SYS_CFG, ®_0xf0);
+ if (res)
+ return;
+
rtw_write8(padapter, REG_SYS_CFG, reg_0xf0 & ~SW_OFFLOAD_EN);
}
}
@@ -34,17 +41,31 @@ static s32 iol_execute(struct adapter *padapter, u8 control)
s32 status = _FAIL;
u8 reg_0x88 = 0;
unsigned long timeout;
+ int res;
control = control & 0x0f;
- reg_0x88 = rtw_read8(padapter, REG_HMEBOX_E0);
+ res = rtw_read8(padapter, REG_HMEBOX_E0, ®_0x88);
+ if (res)
+ return _FAIL;
+
rtw_write8(padapter, REG_HMEBOX_E0, reg_0x88 | control);
timeout = jiffies + msecs_to_jiffies(1000);
- while ((reg_0x88 = rtw_read8(padapter, REG_HMEBOX_E0)) & control &&
- time_before(jiffies, timeout))
- ;
- reg_0x88 = rtw_read8(padapter, REG_HMEBOX_E0);
+ do {
+ res = rtw_read8(padapter, REG_HMEBOX_E0, ®_0x88);
+ if (res)
+ continue;
+
+ if (!(reg_0x88 & control))
+ break;
+
+ } while (time_before(jiffies, timeout));
+
+ res = rtw_read8(padapter, REG_HMEBOX_E0, ®_0x88);
+ if (res)
+ return _FAIL;
+
status = (reg_0x88 & control) ? _FAIL : _SUCCESS;
if (reg_0x88 & control << 4)
status = _FAIL;
@@ -190,13 +211,18 @@ static void efuse_read_phymap_from_txpktbuf(
u16 dbg_addr = 0;
__le32 lo32 = 0, hi32 = 0;
u16 len = 0, count = 0;
- int i = 0;
+ int i = 0, res;
u16 limit = *size;
-
+ u8 reg;
u8 *pos = content;
- if (bcnhead < 0) /* if not valid */
- bcnhead = rtw_read8(adapter, REG_TDECTRL + 1);
+ if (bcnhead < 0) { /* if not valid */
+ res = rtw_read8(adapter, REG_TDECTRL + 1, ®);
+ if (res)
+ return;
+
+ bcnhead = reg;
+ }
rtw_write8(adapter, REG_PKT_BUFF_ACCESS_CTRL, TXPKT_BUF_SELECT);
@@ -207,8 +233,16 @@ static void efuse_read_phymap_from_txpktbuf(
rtw_write8(adapter, REG_TXPKTBUF_DBG, 0);
timeout = jiffies + msecs_to_jiffies(1000);
- while (!rtw_read8(adapter, REG_TXPKTBUF_DBG) && time_before(jiffies, timeout))
+ do {
+ res = rtw_read8(adapter, REG_TXPKTBUF_DBG, ®);
+ if (res)
+ continue;
+
+ if (reg)
+ break;
+
rtw_usleep_os(100);
+ } while (time_before(jiffies, timeout));
/* data from EEPROM needs to be in LE */
lo32 = cpu_to_le32(rtw_read32(adapter, REG_PKTBUF_DBG_DATA_L));
@@ -525,10 +559,17 @@ void rtl8188e_SetHalODMVar(struct adapter *Adapter, void *pValue1, bool bSet)
void hal_notch_filter_8188e(struct adapter *adapter, bool enable)
{
+ int res;
+ u8 reg;
+
+ res = rtw_read8(adapter, rOFDM0_RxDSP + 1, ®);
+ if (res)
+ return;
+
if (enable)
- rtw_write8(adapter, rOFDM0_RxDSP + 1, rtw_read8(adapter, rOFDM0_RxDSP + 1) | BIT(1));
+ rtw_write8(adapter, rOFDM0_RxDSP + 1, reg | BIT(1));
else
- rtw_write8(adapter, rOFDM0_RxDSP + 1, rtw_read8(adapter, rOFDM0_RxDSP + 1) & ~BIT(1));
+ rtw_write8(adapter, rOFDM0_RxDSP + 1, reg & ~BIT(1));
}
/* */
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c b/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
index 4864dafd887b..985339a974fc 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
@@ -594,6 +594,7 @@ _PHY_SetBWMode92C(
struct hal_data_8188e *pHalData = &Adapter->haldata;
u8 regBwOpMode;
u8 regRRSR_RSC;
+ int res;
if (Adapter->bDriverStopped)
return;
@@ -602,8 +603,13 @@ _PHY_SetBWMode92C(
/* 3<1>Set MAC register */
/* 3 */
- regBwOpMode = rtw_read8(Adapter, REG_BWOPMODE);
- regRRSR_RSC = rtw_read8(Adapter, REG_RRSR + 2);
+ res = rtw_read8(Adapter, REG_BWOPMODE, ®BwOpMode);
+ if (res)
+ return;
+
+ res = rtw_read8(Adapter, REG_RRSR + 2, ®RRSR_RSC);
+ if (res)
+ return;
switch (pHalData->CurrentChannelBW) {
case HT_CHANNEL_WIDTH_20:
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index b62ebd011886..6e3c8af5c4e7 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -81,6 +81,7 @@ static void _InitInterrupt(struct adapter *Adapter)
{
u32 imr, imr_ex;
u8 usb_opt;
+ int res;
/* HISR write one to clear */
rtw_write32(Adapter, REG_HISR_88E, 0xFFFFFFFF);
@@ -94,7 +95,9 @@ static void _InitInterrupt(struct adapter *Adapter)
/* REG_USB_SPECIAL_OPTION - BIT(4) */
/* 0; Use interrupt endpoint to upload interrupt pkt */
/* 1; Use bulk endpoint to upload interrupt pkt, */
- usb_opt = rtw_read8(Adapter, REG_USB_SPECIAL_OPTION);
+ res = rtw_read8(Adapter, REG_USB_SPECIAL_OPTION, &usb_opt);
+ if (res)
+ return;
if (adapter_to_dvobj(Adapter)->pusbdev->speed == USB_SPEED_HIGH)
usb_opt = usb_opt | (INT_BULK_SEL);
@@ -363,8 +366,12 @@ static void _InitEDCA(struct adapter *Adapter)
static void _InitRetryFunction(struct adapter *Adapter)
{
u8 value8;
+ int res;
+
+ res = rtw_read8(Adapter, REG_FWHW_TXQ_CTRL, &value8);
+ if (res)
+ return;
- value8 = rtw_read8(Adapter, REG_FWHW_TXQ_CTRL);
value8 |= EN_AMPDU_RTY_NEW;
rtw_write8(Adapter, REG_FWHW_TXQ_CTRL, value8);
@@ -423,9 +430,15 @@ usb_AggSettingRxUpdate(
{
u8 valueDMA;
u8 valueUSB;
+ int res;
- valueDMA = rtw_read8(Adapter, REG_TRXDMA_CTRL);
- valueUSB = rtw_read8(Adapter, REG_USB_SPECIAL_OPTION);
+ res = rtw_read8(Adapter, REG_TRXDMA_CTRL, &valueDMA);
+ if (res)
+ return;
+
+ res = rtw_read8(Adapter, REG_USB_SPECIAL_OPTION, &valueUSB);
+ if (res)
+ return;
valueDMA |= RXDMA_AGG_EN;
valueUSB &= ~USB_AGG_EN;
@@ -449,6 +462,7 @@ static void InitUsbAggregationSetting(struct adapter *Adapter)
static void _InitBeaconParameters(struct adapter *Adapter)
{
struct hal_data_8188e *haldata = &Adapter->haldata;
+ int res;
rtw_write16(Adapter, REG_BCN_CTRL, 0x1010);
@@ -461,9 +475,10 @@ static void _InitBeaconParameters(struct adapter *Adapter)
/* beacause test chip does not contension before sending beacon. by tynli. 2009.11.03 */
rtw_write16(Adapter, REG_BCNTCFG, 0x660F);
- haldata->RegFwHwTxQCtrl = rtw_read8(Adapter, REG_FWHW_TXQ_CTRL + 2);
- haldata->RegReg542 = rtw_read8(Adapter, REG_TBTT_PROHIBIT + 2);
- haldata->RegCR_1 = rtw_read8(Adapter, REG_CR + 1);
+ /* FIXME: return an error to caller */
+ res = rtw_read8(Adapter, REG_FWHW_TXQ_CTRL + 2, &haldata->RegFwHwTxQCtrl);
+ res = rtw_read8(Adapter, REG_TBTT_PROHIBIT + 2, &haldata->RegReg542);
+ res = rtw_read8(Adapter, REG_CR + 1, &haldata->RegCR_1);
}
static void _BeaconFunctionEnable(struct adapter *Adapter,
@@ -514,6 +529,7 @@ u32 rtl8188eu_hal_init(struct adapter *Adapter)
u16 value16;
u8 txpktbuf_bndy;
u32 status = _SUCCESS;
+ int res;
struct hal_data_8188e *haldata = &Adapter->haldata;
struct pwrctrl_priv *pwrctrlpriv = &Adapter->pwrctrlpriv;
struct registry_priv *pregistrypriv = &Adapter->registrypriv;
@@ -620,7 +636,10 @@ u32 rtl8188eu_hal_init(struct adapter *Adapter)
/* Enable TX Report */
/* Enable Tx Report Timer */
- value8 = rtw_read8(Adapter, REG_TX_RPT_CTRL);
+ res = rtw_read8(Adapter, REG_TX_RPT_CTRL, &value8);
+ if (res)
+ return _FAIL;
+
rtw_write8(Adapter, REG_TX_RPT_CTRL, (value8 | BIT(1) | BIT(0)));
/* Set MAX RPT MACID */
rtw_write8(Adapter, REG_TX_RPT_CTRL + 1, 2);/* FOR sta mode ,0: bc/mc ,1:AP */
@@ -714,9 +733,13 @@ static void CardDisableRTL8188EU(struct adapter *Adapter)
{
u8 val8;
struct hal_data_8188e *haldata = &Adapter->haldata;
+ int res;
/* Stop Tx Report Timer. 0x4EC[Bit1]=b'0 */
- val8 = rtw_read8(Adapter, REG_TX_RPT_CTRL);
+ res = rtw_read8(Adapter, REG_TX_RPT_CTRL, &val8);
+ if (res)
+ return;
+
rtw_write8(Adapter, REG_TX_RPT_CTRL, val8 & (~BIT(1)));
/* stop rx */
@@ -727,10 +750,16 @@ static void CardDisableRTL8188EU(struct adapter *Adapter)
/* 2. 0x1F[7:0] = 0 turn off RF */
- val8 = rtw_read8(Adapter, REG_MCUFWDL);
+ res = rtw_read8(Adapter, REG_MCUFWDL, &val8);
+ if (res)
+ return;
+
if ((val8 & RAM_DL_SEL) && Adapter->bFWReady) { /* 8051 RAM code */
/* Reset MCU 0x2[10]=0. */
- val8 = rtw_read8(Adapter, REG_SYS_FUNC_EN + 1);
+ res = rtw_read8(Adapter, REG_SYS_FUNC_EN + 1, &val8);
+ if (res)
+ return;
+
val8 &= ~BIT(2); /* 0x2[10], FEN_CPUEN */
rtw_write8(Adapter, REG_SYS_FUNC_EN + 1, val8);
}
@@ -740,26 +769,45 @@ static void CardDisableRTL8188EU(struct adapter *Adapter)
/* YJ,add,111212 */
/* Disable 32k */
- val8 = rtw_read8(Adapter, REG_32K_CTRL);
+ res = rtw_read8(Adapter, REG_32K_CTRL, &val8);
+ if (res)
+ return;
+
rtw_write8(Adapter, REG_32K_CTRL, val8 & (~BIT(0)));
/* Card disable power action flow */
HalPwrSeqCmdParsing(Adapter, Rtl8188E_NIC_DISABLE_FLOW);
/* Reset MCU IO Wrapper */
- val8 = rtw_read8(Adapter, REG_RSV_CTRL + 1);
+ res = rtw_read8(Adapter, REG_RSV_CTRL + 1, &val8);
+ if (res)
+ return;
+
rtw_write8(Adapter, REG_RSV_CTRL + 1, (val8 & (~BIT(3))));
- val8 = rtw_read8(Adapter, REG_RSV_CTRL + 1);
+
+ res = rtw_read8(Adapter, REG_RSV_CTRL + 1, &val8);
+ if (res)
+ return;
+
rtw_write8(Adapter, REG_RSV_CTRL + 1, val8 | BIT(3));
/* YJ,test add, 111207. For Power Consumption. */
- val8 = rtw_read8(Adapter, GPIO_IN);
+ res = rtw_read8(Adapter, GPIO_IN, &val8);
+ if (res)
+ return;
+
rtw_write8(Adapter, GPIO_OUT, val8);
rtw_write8(Adapter, GPIO_IO_SEL, 0xFF);/* Reg0x46 */
- val8 = rtw_read8(Adapter, REG_GPIO_IO_SEL);
+ res = rtw_read8(Adapter, REG_GPIO_IO_SEL, &val8);
+ if (res)
+ return;
+
rtw_write8(Adapter, REG_GPIO_IO_SEL, (val8 << 4));
- val8 = rtw_read8(Adapter, REG_GPIO_IO_SEL + 1);
+ res = rtw_read8(Adapter, REG_GPIO_IO_SEL + 1, &val8);
+ if (res)
+ return;
+
rtw_write8(Adapter, REG_GPIO_IO_SEL + 1, val8 | 0x0F);/* Reg0x43 */
rtw_write32(Adapter, REG_BB_PAD_CTRL, 0x00080808);/* set LNA ,TRSW,EX_PA Pin to output mode */
haldata->bMacPwrCtrlOn = false;
@@ -830,9 +878,13 @@ void ReadAdapterInfo8188EU(struct adapter *Adapter)
struct eeprom_priv *eeprom = &Adapter->eeprompriv;
struct led_priv *ledpriv = &Adapter->ledpriv;
u8 eeValue;
+ int res;
/* check system boot selection */
- eeValue = rtw_read8(Adapter, REG_9346CR);
+ res = rtw_read8(Adapter, REG_9346CR, &eeValue);
+ if (res)
+ return;
+
eeprom->EepromOrEfuse = (eeValue & BOOT_FROM_EEPROM);
eeprom->bautoload_fail_flag = !(eeValue & EEPROM_EN);
@@ -887,12 +939,21 @@ static void hw_var_set_opmode(struct adapter *Adapter, u8 *val)
{
u8 val8;
u8 mode = *((u8 *)val);
+ int res;
/* disable Port0 TSF update */
- rtw_write8(Adapter, REG_BCN_CTRL, rtw_read8(Adapter, REG_BCN_CTRL) | BIT(4));
+ res = rtw_read8(Adapter, REG_BCN_CTRL, &val8);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_BCN_CTRL, val8 | BIT(4));
/* set net_type */
- val8 = rtw_read8(Adapter, MSR) & 0x0c;
+ res = rtw_read8(Adapter, MSR, &val8);
+ if (res)
+ return;
+
+ val8 &= 0x0c;
val8 |= mode;
rtw_write8(Adapter, MSR, val8);
@@ -927,14 +988,22 @@ static void hw_var_set_opmode(struct adapter *Adapter, u8 *val)
rtw_write8(Adapter, REG_DUAL_TSF_RST, BIT(0));
/* BIT(3) - If set 0, hw will clr bcnq when tx becon ok/fail or port 0 */
- rtw_write8(Adapter, REG_MBID_NUM, rtw_read8(Adapter, REG_MBID_NUM) | BIT(3) | BIT(4));
+ res = rtw_read8(Adapter, REG_MBID_NUM, &val8);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_MBID_NUM, val8 | BIT(3) | BIT(4));
/* enable BCN0 Function for if1 */
/* don't enable update TSF0 for if1 (due to TSF update when beacon/probe rsp are received) */
rtw_write8(Adapter, REG_BCN_CTRL, (DIS_TSF_UDT0_NORMAL_CHIP | EN_BCN_FUNCTION | BIT(1)));
/* dis BCN1 ATIM WND if if2 is station */
- rtw_write8(Adapter, REG_BCN_CTRL_1, rtw_read8(Adapter, REG_BCN_CTRL_1) | BIT(0));
+ res = rtw_read8(Adapter, REG_BCN_CTRL_1, &val8);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_BCN_CTRL_1, val8 | BIT(0));
}
}
@@ -943,6 +1012,8 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
struct hal_data_8188e *haldata = &Adapter->haldata;
struct dm_priv *pdmpriv = &haldata->dmpriv;
struct odm_dm_struct *podmpriv = &haldata->odmpriv;
+ u8 reg;
+ int res;
switch (variable) {
case HW_VAR_SET_OPMODE:
@@ -970,7 +1041,11 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
/* Set RRSR rate table. */
rtw_write8(Adapter, REG_RRSR, BrateCfg & 0xff);
rtw_write8(Adapter, REG_RRSR + 1, (BrateCfg >> 8) & 0xff);
- rtw_write8(Adapter, REG_RRSR + 2, rtw_read8(Adapter, REG_RRSR + 2) & 0xf0);
+ res = rtw_read8(Adapter, REG_RRSR + 2, ®);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_RRSR + 2, reg & 0xf0);
/* Set RTS initial rate */
while (BrateCfg > 0x1) {
@@ -994,13 +1069,21 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
StopTxBeacon(Adapter);
/* disable related TSF function */
- rtw_write8(Adapter, REG_BCN_CTRL, rtw_read8(Adapter, REG_BCN_CTRL) & (~BIT(3)));
+ res = rtw_read8(Adapter, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_BCN_CTRL, reg & (~BIT(3)));
rtw_write32(Adapter, REG_TSFTR, tsf);
rtw_write32(Adapter, REG_TSFTR + 4, tsf >> 32);
/* enable related TSF function */
- rtw_write8(Adapter, REG_BCN_CTRL, rtw_read8(Adapter, REG_BCN_CTRL) | BIT(3));
+ res = rtw_read8(Adapter, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_BCN_CTRL, reg | BIT(3));
if (((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE) || ((pmlmeinfo->state & 0x03) == WIFI_FW_AP_STATE))
ResumeTxBeacon(Adapter);
@@ -1016,7 +1099,11 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
rtw_write16(Adapter, REG_RXFLTMAP2, 0x00);
/* disable update TSF */
- rtw_write8(Adapter, REG_BCN_CTRL, rtw_read8(Adapter, REG_BCN_CTRL) | BIT(4));
+ res = rtw_read8(Adapter, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_BCN_CTRL, reg | BIT(4));
} else { /* sitesurvey done */
struct mlme_ext_priv *pmlmeext = &Adapter->mlmeextpriv;
struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
@@ -1027,11 +1114,19 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
rtw_write16(Adapter, REG_RXFLTMAP2, 0xFFFF);
/* enable update TSF */
- rtw_write8(Adapter, REG_BCN_CTRL, rtw_read8(Adapter, REG_BCN_CTRL) & (~BIT(4)));
+ res = rtw_read8(Adapter, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_BCN_CTRL, reg & (~BIT(4)));
} else if ((pmlmeinfo->state & 0x03) == WIFI_FW_AP_STATE) {
rtw_write16(Adapter, REG_RXFLTMAP2, 0xFFFF);
/* enable update TSF */
- rtw_write8(Adapter, REG_BCN_CTRL, rtw_read8(Adapter, REG_BCN_CTRL) & (~BIT(4)));
+ res = rtw_read8(Adapter, REG_BCN_CTRL, ®);
+ if (res)
+ return;
+
+ rtw_write8(Adapter, REG_BCN_CTRL, reg & (~BIT(4)));
}
rtw_write32(Adapter, REG_RCR, rtw_read32(Adapter, REG_RCR) | RCR_CBSSID_BCN);
}
@@ -1194,6 +1289,8 @@ void SetBeaconRelatedRegisters8188EUsb(struct adapter *adapt)
struct mlme_ext_priv *pmlmeext = &adapt->mlmeextpriv;
struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
u32 bcn_ctrl_reg = REG_BCN_CTRL;
+ int res;
+ u8 reg;
/* reset TSF, enable update TSF, correcting TSF On Beacon */
/* BCN interval */
@@ -1219,7 +1316,11 @@ void SetBeaconRelatedRegisters8188EUsb(struct adapter *adapt)
ResumeTxBeacon(adapt);
- rtw_write8(adapt, bcn_ctrl_reg, rtw_read8(adapt, bcn_ctrl_reg) | BIT(1));
+ res = rtw_read8(adapt, bcn_ctrl_reg, ®);
+ if (res)
+ return;
+
+ rtw_write8(adapt, bcn_ctrl_reg, reg | BIT(1));
}
void rtl8188eu_init_default_value(struct adapter *adapt)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index d5e674542a78..f399a7fd8b97 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -94,16 +94,13 @@ static int usb_write(struct intf_hdl *intf, u16 value, void *data, u8 size)
return status;
}
-u8 rtw_read8(struct adapter *adapter, u32 addr)
+int __must_check rtw_read8(struct adapter *adapter, u32 addr, u8 *data)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
u16 value = addr & 0xffff;
- u8 data;
- usb_read(intf, value, &data, 1);
-
- return data;
+ return usb_read(intf, value, data, 1);
}
u16 rtw_read16(struct adapter *adapter, u32 addr)
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 6910e2b430e2..1198d3850a6d 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -220,7 +220,7 @@ void unregister_intf_hdl(struct intf_hdl *pintfhdl);
void _rtw_attrib_read(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
void _rtw_attrib_write(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
-u8 rtw_read8(struct adapter *adapter, u32 addr);
+int __must_check rtw_read8(struct adapter *adapter, u32 addr, u8 *data);
u16 rtw_read16(struct adapter *adapter, u32 addr);
u32 rtw_read32(struct adapter *adapter, u32 addr);
void _rtw_read_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
index 42cb79cee2ae..66aac2cbe3a9 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -1912,8 +1912,11 @@ static int rtw_wx_read32(struct net_device *dev,
switch (bytes) {
case 1:
- data32 = rtw_read8(padapter, addr);
- sprintf(extra, "0x%02X", data32);
+ ret = rtw_read8(padapter, addr, (u8 *) &data32);
+ if (ret)
+ goto err_free_ptmp;
+
+ sprintf(extra, "0x%02X", data32 & 0xff);
break;
case 2:
data32 = rtw_read16(padapter, addr);
@@ -3259,6 +3262,7 @@ static void rtw_set_dynamic_functions(struct adapter *adapter, u8 dm_func)
{
struct hal_data_8188e *haldata = &adapter->haldata;
struct odm_dm_struct *odmpriv = &haldata->odmpriv;
+ int res;
switch (dm_func) {
case 0:
@@ -3274,7 +3278,9 @@ static void rtw_set_dynamic_functions(struct adapter *adapter, u8 dm_func)
if (!(odmpriv->SupportAbility & DYNAMIC_BB_DIG)) {
struct rtw_dig *digtable = &odmpriv->DM_DigTable;
- digtable->CurIGValue = rtw_read8(adapter, 0xc50);
+ res = rtw_read8(adapter, 0xc50, &digtable->CurIGValue);
+ (void) res;
+ /* FIXME: return an error to caller */
}
odmpriv->SupportAbility = DYNAMIC_ALL_FUNC_ENABLE;
break;
@@ -3410,8 +3416,9 @@ static int rtw_dbg_port(struct net_device *dev,
u16 reg = arg;
u16 start_value = 0;
u32 write_num = extra_arg;
- int i;
+ int i, res;
struct xmit_frame *xmit_frame;
+ u8 val8;
xmit_frame = rtw_IOL_accquire_xmit_frame(padapter);
if (!xmit_frame) {
@@ -3424,7 +3431,8 @@ static int rtw_dbg_port(struct net_device *dev,
if (rtl8188e_IOL_exec_cmds_sync(padapter, xmit_frame, 5000, 0) != _SUCCESS)
ret = -EPERM;
- rtw_read8(padapter, reg);
+ /* FIXME: is this read necessary? */
+ res = rtw_read8(padapter, reg, &val8);
}
break;
--
2.36.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/4] staging: r8188eu: add error handling of rtw_read16
2022-05-18 22:11 [PATCH 0/4] staging: r8188eu: add error handling of usb read errors Pavel Skripkin
2022-05-18 22:11 ` [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8 Pavel Skripkin
@ 2022-05-18 22:11 ` Pavel Skripkin
2022-05-19 4:47 ` Dan Carpenter
2022-05-18 22:12 ` [PATCH 3/4] staging: r8188eu: add error handling of rtw_read32 Pavel Skripkin
2022-05-18 22:12 ` [PATCH 4/4] MAINTAINERS: add myself as r8188eu reviewer Pavel Skripkin
3 siblings, 1 reply; 13+ messages in thread
From: Pavel Skripkin @ 2022-05-18 22:11 UTC (permalink / raw)
To: gregkh, Larry.Finger, phil, straube.linux, dan.carpenter, fmdefrancesco
Cc: linux-kernel, linux-staging, Pavel Skripkin
rtw_read16() reads data from device via USB API which may fail. In case
of any failure previous code returned stack data to callers, which is
wrong.
Fix it by changing rtw_read16() prototype and prevent caller from
touching random stack data
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
.../staging/r8188eu/hal/rtl8188e_hal_init.c | 21 ++++++++++++---
drivers/staging/r8188eu/hal/rtl8188e_phycfg.c | 8 ++++--
drivers/staging/r8188eu/hal/usb_halinit.c | 27 ++++++++++++++++---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 13 ++++++---
drivers/staging/r8188eu/include/rtw_io.h | 2 +-
drivers/staging/r8188eu/os_dep/ioctl_linux.c | 11 +++++---
drivers/staging/r8188eu/os_dep/os_intfs.c | 6 ++++-
7 files changed, 69 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
index e67ecbd1ba79..22661c66cc18 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
@@ -249,11 +249,14 @@ static void efuse_read_phymap_from_txpktbuf(
hi32 = cpu_to_le32(rtw_read32(adapter, REG_PKTBUF_DBG_DATA_H));
if (i == 0) {
+ int res;
+ u16 reg;
/* Although lenc is only used in a debug statement,
* do not remove it as the rtw_read16() call consumes
* 2 bytes from the EEPROM source.
*/
- rtw_read16(adapter, REG_PKTBUF_DBG_DATA_L);
+ res = rtw_read16(adapter, REG_PKTBUF_DBG_DATA_L, ®);
+ (void) res;
len = le32_to_cpu(lo32) & 0x0000ffff;
@@ -355,25 +358,35 @@ int rtl8188e_IOL_exec_cmds_sync(struct adapter *adapter, struct xmit_frame *xmit
void rtl8188e_EfusePowerSwitch(struct adapter *pAdapter, u8 PwrState)
{
u16 tmpV16;
+ int res;
if (PwrState) {
rtw_write8(pAdapter, REG_EFUSE_ACCESS, EFUSE_ACCESS_ON);
/* 1.2V Power: From VDDON with Power Cut(0x0000h[15]), defualt valid */
- tmpV16 = rtw_read16(pAdapter, REG_SYS_ISO_CTRL);
+ res = rtw_read16(pAdapter, REG_SYS_ISO_CTRL, &tmpV16);
+ if (res)
+ return;
+
if (!(tmpV16 & PWC_EV12V)) {
tmpV16 |= PWC_EV12V;
rtw_write16(pAdapter, REG_SYS_ISO_CTRL, tmpV16);
}
/* Reset: 0x0000h[28], default valid */
- tmpV16 = rtw_read16(pAdapter, REG_SYS_FUNC_EN);
+ res = rtw_read16(pAdapter, REG_SYS_FUNC_EN, &tmpV16);
+ if (res)
+ return;
+
if (!(tmpV16 & FEN_ELDR)) {
tmpV16 |= FEN_ELDR;
rtw_write16(pAdapter, REG_SYS_FUNC_EN, tmpV16);
}
/* Clock: Gated(0x0008h[5]) 8M(0x0008h[1]) clock from ANA, default valid */
- tmpV16 = rtw_read16(pAdapter, REG_SYS_CLKR);
+ res = rtw_read16(pAdapter, REG_SYS_CLKR, &tmpV16);
+ if (res)
+ return;
+
if ((!(tmpV16 & LOADER_CLK_EN)) || (!(tmpV16 & ANA8M))) {
tmpV16 |= (LOADER_CLK_EN | ANA8M);
rtw_write16(pAdapter, REG_SYS_CLKR, tmpV16);
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c b/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
index 985339a974fc..298c3d9bc7be 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
@@ -484,13 +484,17 @@ PHY_BBConfig8188E(
{
int rtStatus = _SUCCESS;
struct hal_data_8188e *pHalData = &Adapter->haldata;
- u32 RegVal;
+ u16 RegVal;
u8 CrystalCap;
+ int res;
phy_InitBBRFRegisterDefinition(Adapter);
/* Enable BB and RF */
- RegVal = rtw_read16(Adapter, REG_SYS_FUNC_EN);
+ res = rtw_read16(Adapter, REG_SYS_FUNC_EN, &RegVal);
+ if (res)
+ return _FAIL;
+
rtw_write16(Adapter, REG_SYS_FUNC_EN, (u16)(RegVal | BIT(13) | BIT(0) | BIT(1)));
/* 20090923 Joseph: Advised by Steven and Jenyu. Power sequence before init RF. */
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 6e3c8af5c4e7..1a68e4f19dc2 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -52,6 +52,8 @@ void rtl8188eu_interface_configure(struct adapter *adapt)
u32 rtl8188eu_InitPowerOn(struct adapter *adapt)
{
u16 value16;
+ int res;
+
/* HW Power on sequence */
struct hal_data_8188e *haldata = &adapt->haldata;
if (haldata->bMacPwrCtrlOn)
@@ -65,7 +67,10 @@ u32 rtl8188eu_InitPowerOn(struct adapter *adapt)
rtw_write16(adapt, REG_CR, 0x00); /* suggseted by zhouzhou, by page, 20111230 */
/* Enable MAC DMA/WMAC/SCHEDULE/SEC block */
- value16 = rtw_read16(adapt, REG_CR);
+ res = rtw_read16(adapt, REG_CR, &value16);
+ if (res)
+ return _FAIL;
+
value16 |= (HCI_TXDMA_EN | HCI_RXDMA_EN | TXDMA_EN | RXDMA_EN
| PROTOCOL_EN | SCHEDULE_EN | ENSEC | CALTMR_EN);
/* for SDIO - Set CR bit10 to enable 32k calibration. Suggested by SD1 Gimmy. Added by tynli. 2011.08.31. */
@@ -166,7 +171,14 @@ static void _InitNormalChipRegPriority(struct adapter *Adapter, u16 beQ,
u16 bkQ, u16 viQ, u16 voQ, u16 mgtQ,
u16 hiQ)
{
- u16 value16 = (rtw_read16(Adapter, REG_TRXDMA_CTRL) & 0x7);
+ u16 value16;
+ int res;
+
+ res = rtw_read16(Adapter, REG_TRXDMA_CTRL, &value16);
+ if (res)
+ return;
+
+ value16 &= 0x7;
value16 |= _TXDMA_BEQ_MAP(beQ) | _TXDMA_BKQ_MAP(bkQ) |
_TXDMA_VIQ_MAP(viQ) | _TXDMA_VOQ_MAP(voQ) |
@@ -630,7 +642,10 @@ u32 rtl8188eu_hal_init(struct adapter *Adapter)
/* Hw bug which Hw initials RxFF boundary size to a value which is larger than the real Rx buffer size in 88E. */
/* */
/* Enable MACTXEN/MACRXEN block */
- value16 = rtw_read16(Adapter, REG_CR);
+ res = rtw_read16(Adapter, REG_CR, &value16);
+ if (res)
+ return _FAIL;
+
value16 |= (MACTXEN | MACRXEN);
rtw_write8(Adapter, REG_CR, value16);
@@ -703,7 +718,11 @@ u32 rtl8188eu_hal_init(struct adapter *Adapter)
rtw_write16(Adapter, REG_TX_RPT_TIME, 0x3DF0);
/* enable tx DMA to drop the redundate data of packet */
- rtw_write16(Adapter, REG_TXDMA_OFFSET_CHK, (rtw_read16(Adapter, REG_TXDMA_OFFSET_CHK) | DROP_DATA_EN));
+ res = rtw_read16(Adapter, REG_TXDMA_OFFSET_CHK, &value16);
+ if (res)
+ return _FAIL;
+
+ rtw_write16(Adapter, REG_TXDMA_OFFSET_CHK, (value16 | DROP_DATA_EN));
/* 2010/08/26 MH Merge from 8192CE. */
if (pwrctrlpriv->rf_pwrstate == rf_on) {
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index f399a7fd8b97..7d62f1f3d26e 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -103,16 +103,21 @@ int __must_check rtw_read8(struct adapter *adapter, u32 addr, u8 *data)
return usb_read(intf, value, data, 1);
}
-u16 rtw_read16(struct adapter *adapter, u32 addr)
+int __must_check rtw_read16(struct adapter *adapter, u32 addr, u16 *data)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
u16 value = addr & 0xffff;
- __le16 data;
+ __le16 le_data;
+ int res;
- usb_read(intf, value, &data, 2);
+ res = usb_read(intf, value, &le_data, 2);
+ if (res)
+ return res;
- return le16_to_cpu(data);
+ *data = le16_to_cpu(le_data);
+
+ return 0;
}
u32 rtw_read32(struct adapter *adapter, u32 addr)
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 1198d3850a6d..ce3369e33d66 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -221,7 +221,7 @@ void _rtw_attrib_read(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
void _rtw_attrib_write(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
int __must_check rtw_read8(struct adapter *adapter, u32 addr, u8 *data);
-u16 rtw_read16(struct adapter *adapter, u32 addr);
+int __must_check rtw_read16(struct adapter *adapter, u32 addr, u16 *data);
u32 rtw_read32(struct adapter *adapter, u32 addr);
void _rtw_read_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
u32 rtw_read_port(struct adapter *adapter, u8 *pmem);
diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
index 66aac2cbe3a9..1b35951a53cb 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -1919,7 +1919,10 @@ static int rtw_wx_read32(struct net_device *dev,
sprintf(extra, "0x%02X", data32 & 0xff);
break;
case 2:
- data32 = rtw_read16(padapter, addr);
+ ret = rtw_read16(padapter, addr, (u16 *) &data32);
+ if (ret)
+ goto err_free_ptmp;
+
sprintf(extra, "0x%04X", data32);
break;
case 4:
@@ -3441,8 +3444,9 @@ static int rtw_dbg_port(struct net_device *dev,
u16 reg = arg;
u16 start_value = 200;
u32 write_num = extra_arg;
+ u16 val16;
- int i;
+ int i, res;
struct xmit_frame *xmit_frame;
xmit_frame = rtw_IOL_accquire_xmit_frame(padapter);
@@ -3456,7 +3460,8 @@ static int rtw_dbg_port(struct net_device *dev,
if (rtl8188e_IOL_exec_cmds_sync(padapter, xmit_frame, 5000, 0) != _SUCCESS)
ret = -EPERM;
- rtw_read16(padapter, reg);
+ /* FIXME: is this read necessary? */
+ res = rtw_read16(padapter, reg, &val16);
}
break;
case 0x08: /* continuous write dword test */
diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index 891c85b088ca..d9325ef6ac28 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -740,12 +740,16 @@ static void rtw_fifo_cleanup(struct adapter *adapter)
{
struct pwrctrl_priv *pwrpriv = &adapter->pwrctrlpriv;
u8 trycnt = 100;
+ int res;
/* pause tx */
rtw_write8(adapter, REG_TXPAUSE, 0xff);
/* keep sn */
- adapter->xmitpriv.nqos_ssn = rtw_read16(adapter, REG_NQOS_SEQ);
+ /* FIXME: return an error to caller */
+ res = rtw_read16(adapter, REG_NQOS_SEQ, &adapter->xmitpriv.nqos_ssn);
+ if (res)
+ return;
if (!pwrpriv->bkeepfwalive) {
/* RX DMA stop */
--
2.36.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/4] staging: r8188eu: add error handling of rtw_read32
2022-05-18 22:11 [PATCH 0/4] staging: r8188eu: add error handling of usb read errors Pavel Skripkin
2022-05-18 22:11 ` [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8 Pavel Skripkin
2022-05-18 22:11 ` [PATCH 2/4] staging: r8188eu: add error handling of rtw_read16 Pavel Skripkin
@ 2022-05-18 22:12 ` Pavel Skripkin
2022-05-19 5:43 ` Dan Carpenter
2022-05-18 22:12 ` [PATCH 4/4] MAINTAINERS: add myself as r8188eu reviewer Pavel Skripkin
3 siblings, 1 reply; 13+ messages in thread
From: Pavel Skripkin @ 2022-05-18 22:12 UTC (permalink / raw)
To: gregkh, Larry.Finger, phil, straube.linux, dan.carpenter, fmdefrancesco
Cc: linux-kernel, linux-staging, Pavel Skripkin
rtw_read32() reads data from device via USB API which may fail. In case
of any failure previous code returned stack data to callers, which is
wrong.
Fix it by changing rtw_read32() prototype and prevent caller from
touching random stack data
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/staging/r8188eu/core/rtw_cmd.c | 15 +++++-
drivers/staging/r8188eu/core/rtw_efuse.c | 19 +++++--
drivers/staging/r8188eu/core/rtw_fw.c | 16 ++++--
drivers/staging/r8188eu/core/rtw_mlme_ext.c | 13 ++++-
drivers/staging/r8188eu/core/rtw_pwrctrl.c | 9 +++-
.../r8188eu/hal/Hal8188ERateAdaptive.c | 21 ++++++--
drivers/staging/r8188eu/hal/HalPhyRf_8188e.c | 3 +-
.../staging/r8188eu/hal/rtl8188e_hal_init.c | 27 ++++++++--
drivers/staging/r8188eu/hal/rtl8188e_phycfg.c | 12 ++++-
drivers/staging/r8188eu/hal/usb_halinit.c | 53 ++++++++++++++++---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 13 +++--
drivers/staging/r8188eu/include/rtw_io.h | 2 +-
drivers/staging/r8188eu/os_dep/ioctl_linux.c | 32 ++++++++---
drivers/staging/r8188eu/os_dep/os_intfs.c | 13 ++++-
14 files changed, 204 insertions(+), 44 deletions(-)
diff --git a/drivers/staging/r8188eu/core/rtw_cmd.c b/drivers/staging/r8188eu/core/rtw_cmd.c
index 06523d91939a..5b6a891b5d67 100644
--- a/drivers/staging/r8188eu/core/rtw_cmd.c
+++ b/drivers/staging/r8188eu/core/rtw_cmd.c
@@ -898,8 +898,12 @@ static void traffic_status_watchdog(struct adapter *padapter)
static void rtl8188e_sreset_xmit_status_check(struct adapter *padapter)
{
u32 txdma_status;
+ int res;
+
+ res = rtw_read32(padapter, REG_TXDMA_STATUS, &txdma_status);
+ if (res)
+ return;
- txdma_status = rtw_read32(padapter, REG_TXDMA_STATUS);
if (txdma_status != 0x00)
rtw_write32(padapter, REG_TXDMA_STATUS, txdma_status);
/* total xmit irp = 4 */
@@ -1177,7 +1181,14 @@ u8 rtw_ps_cmd(struct adapter *padapter)
static bool rtw_is_hi_queue_empty(struct adapter *adapter)
{
- return (rtw_read32(adapter, REG_HGQ_INFORMATION) & 0x0000ff00) == 0;
+ int res;
+ u32 reg;
+
+ res = rtw_read32(adapter, REG_HGQ_INFORMATION, ®);
+ if (res)
+ return false;
+
+ return (reg & 0x0000ff00) == 0;
}
static void rtw_chk_hi_queue_hdl(struct adapter *padapter)
diff --git a/drivers/staging/r8188eu/core/rtw_efuse.c b/drivers/staging/r8188eu/core/rtw_efuse.c
index a2691c7f96f6..7105122c2ba0 100644
--- a/drivers/staging/r8188eu/core/rtw_efuse.c
+++ b/drivers/staging/r8188eu/core/rtw_efuse.c
@@ -47,9 +47,18 @@ ReadEFuseByte(
/* Check bit 32 read-ready */
retry = 0;
- value32 = rtw_read32(Adapter, EFUSE_CTRL);
- while (!(((value32 >> 24) & 0xff) & 0x80) && (retry < 10000)) {
- value32 = rtw_read32(Adapter, EFUSE_CTRL);
+ res = rtw_read32(Adapter, EFUSE_CTRL, &value32);
+ if (res)
+ return;
+
+ while (retry < 10000) {
+ res = rtw_read32(Adapter, EFUSE_CTRL, &value32);
+ if (res)
+ continue;
+
+ if (((value32 >> 24) & 0xff) & 0x80)
+ break;
+
retry++;
}
@@ -58,7 +67,9 @@ ReadEFuseByte(
/* Designer says that there shall be some delay after ready bit is set, or the */
/* result will always stay on last data we read. */
udelay(50);
- value32 = rtw_read32(Adapter, EFUSE_CTRL);
+ res = rtw_read32(Adapter, EFUSE_CTRL, &value32);
+ if (res)
+ return;
*pbuf = (u8)(value32 & 0xff);
diff --git a/drivers/staging/r8188eu/core/rtw_fw.c b/drivers/staging/r8188eu/core/rtw_fw.c
index 701b033830bc..602b959437db 100644
--- a/drivers/staging/r8188eu/core/rtw_fw.c
+++ b/drivers/staging/r8188eu/core/rtw_fw.c
@@ -194,10 +194,14 @@ static int fw_free_to_go(struct adapter *padapter)
{
u32 counter = 0;
u32 value32;
+ int res;
/* polling CheckSum report */
do {
- value32 = rtw_read32(padapter, REG_MCUFWDL);
+ res = rtw_read32(padapter, REG_MCUFWDL, &value32);
+ if (res)
+ continue;
+
if (value32 & FWDL_CHKSUM_RPT)
break;
} while (counter++ < POLLING_READY_TIMEOUT_COUNT);
@@ -205,7 +209,10 @@ static int fw_free_to_go(struct adapter *padapter)
if (counter >= POLLING_READY_TIMEOUT_COUNT)
return _FAIL;
- value32 = rtw_read32(padapter, REG_MCUFWDL);
+ res = rtw_read32(padapter, REG_MCUFWDL, &value32);
+ if (res)
+ return _FAIL;
+
value32 |= MCUFWDL_RDY;
value32 &= ~WINTINI_RDY;
rtw_write32(padapter, REG_MCUFWDL, value32);
@@ -215,7 +222,10 @@ static int fw_free_to_go(struct adapter *padapter)
/* polling for FW ready */
counter = 0;
do {
- value32 = rtw_read32(padapter, REG_MCUFWDL);
+ res = rtw_read32(padapter, REG_MCUFWDL, &value32);
+ if (res)
+ continue;
+
if (value32 & WINTINI_RDY)
return _SUCCESS;
udelay(5);
diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
index d4e59fab367c..e54d4139466d 100644
--- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
@@ -6023,6 +6023,7 @@ static void mlme_join(struct adapter *adapter, int type)
struct mlme_priv *mlmepriv = &adapter->mlmepriv;
u8 retry_limit = 0x30, reg;
int res;
+ u32 reg32;
switch (type) {
case 0:
@@ -6030,8 +6031,12 @@ static void mlme_join(struct adapter *adapter, int type)
/* enable to rx data frame, accept all data frame */
rtw_write16(adapter, REG_RXFLTMAP2, 0xFFFF);
+ res = rtw_read32(adapter, REG_RCR, ®32);
+ if (res)
+ return;
+
rtw_write32(adapter, REG_RCR,
- rtw_read32(adapter, REG_RCR) | RCR_CBSSID_DATA | RCR_CBSSID_BCN);
+ reg32 | RCR_CBSSID_DATA | RCR_CBSSID_BCN);
if (check_fwstate(mlmepriv, WIFI_STATION_STATE)) {
retry_limit = 48;
@@ -6844,10 +6849,14 @@ static u8 chk_ap_is_alive(struct sta_info *psta)
static void rtl8188e_sreset_linked_status_check(struct adapter *padapter)
{
- u32 rx_dma_status = rtw_read32(padapter, REG_RXDMA_STATUS);
+ u32 rx_dma_status;
int res;
u8 reg;
+ res = rtw_read32(padapter, REG_RXDMA_STATUS, &rx_dma_status);
+ if (res)
+ return;
+
if (rx_dma_status != 0x00)
rtw_write32(padapter, REG_RXDMA_STATUS, rx_dma_status);
diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 6990808ef353..1fe3d3d9cfb9 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -229,6 +229,9 @@ void rtw_set_ps_mode(struct adapter *padapter, u8 ps_mode, u8 smart_ps, u8 bcn_a
static bool lps_rf_on(struct adapter *adapter)
{
+ int res;
+ u32 reg;
+
/* When we halt NIC, we should check if FW LPS is leave. */
if (adapter->pwrctrlpriv.rf_pwrstate == rf_off) {
/* If it is in HW/SW Radio OFF or IPS state, we do not check Fw LPS Leave, */
@@ -236,7 +239,11 @@ static bool lps_rf_on(struct adapter *adapter)
return true;
}
- if (rtw_read32(adapter, REG_RCR) & 0x00070000)
+ res = rtw_read32(adapter, REG_RCR, ®);
+ if (res)
+ return false;
+
+ if (reg & 0x00070000)
return false;
return true;
diff --git a/drivers/staging/r8188eu/hal/Hal8188ERateAdaptive.c b/drivers/staging/r8188eu/hal/Hal8188ERateAdaptive.c
index 57e8f5573846..3cefdf90d6e0 100644
--- a/drivers/staging/r8188eu/hal/Hal8188ERateAdaptive.c
+++ b/drivers/staging/r8188eu/hal/Hal8188ERateAdaptive.c
@@ -279,6 +279,7 @@ static int odm_ARFBRefresh_8188E(struct odm_dm_struct *dm_odm, struct odm_ra_inf
{ /* Wilson 2011/10/26 */
u32 MaskFromReg;
s8 i;
+ int res;
switch (pRaInfo->RateID) {
case RATR_INX_WIRELESS_NGB:
@@ -303,19 +304,31 @@ static int odm_ARFBRefresh_8188E(struct odm_dm_struct *dm_odm, struct odm_ra_inf
pRaInfo->RAUseRate = (pRaInfo->RateMask) & 0x0000000d;
break;
case 12:
- MaskFromReg = rtw_read32(dm_odm->Adapter, REG_ARFR0);
+ res = rtw_read32(dm_odm->Adapter, REG_ARFR0, &MaskFromReg);
+ if (res)
+ return res;
+
pRaInfo->RAUseRate = (pRaInfo->RateMask) & MaskFromReg;
break;
case 13:
- MaskFromReg = rtw_read32(dm_odm->Adapter, REG_ARFR1);
+ res = rtw_read32(dm_odm->Adapter, REG_ARFR1, &MaskFromReg);
+ if (res)
+ return res;
+
pRaInfo->RAUseRate = (pRaInfo->RateMask) & MaskFromReg;
break;
case 14:
- MaskFromReg = rtw_read32(dm_odm->Adapter, REG_ARFR2);
+ res = rtw_read32(dm_odm->Adapter, REG_ARFR2, &MaskFromReg);
+ if (res)
+ return res;
+
pRaInfo->RAUseRate = (pRaInfo->RateMask) & MaskFromReg;
break;
case 15:
- MaskFromReg = rtw_read32(dm_odm->Adapter, REG_ARFR3);
+ res = rtw_read32(dm_odm->Adapter, REG_ARFR3, &MaskFromReg);
+ if (res)
+ return res;
+
pRaInfo->RAUseRate = (pRaInfo->RateMask) & MaskFromReg;
break;
default:
diff --git a/drivers/staging/r8188eu/hal/HalPhyRf_8188e.c b/drivers/staging/r8188eu/hal/HalPhyRf_8188e.c
index aa4b4459329e..a97d9c3da16d 100644
--- a/drivers/staging/r8188eu/hal/HalPhyRf_8188e.c
+++ b/drivers/staging/r8188eu/hal/HalPhyRf_8188e.c
@@ -482,7 +482,8 @@ static void _PHY_SaveMACRegisters(
MACBackup[i] = reg;
}
- MACBackup[i] = rtw_read32(adapt, MACReg[i]);
+ /* FIXME: return an error to caller */
+ res = rtw_read32(adapt, MACReg[i], MACBackup + i);
}
static void reload_adda_reg(struct adapter *adapt, u32 *ADDAReg, u32 *ADDABackup, u32 RegiesterNum)
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
index 22661c66cc18..f1dd60b30533 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
@@ -215,6 +215,7 @@ static void efuse_read_phymap_from_txpktbuf(
u16 limit = *size;
u8 reg;
u8 *pos = content;
+ u32 reg32;
if (bcnhead < 0) { /* if not valid */
res = rtw_read8(adapter, REG_TDECTRL + 1, ®);
@@ -245,8 +246,18 @@ static void efuse_read_phymap_from_txpktbuf(
} while (time_before(jiffies, timeout));
/* data from EEPROM needs to be in LE */
- lo32 = cpu_to_le32(rtw_read32(adapter, REG_PKTBUF_DBG_DATA_L));
- hi32 = cpu_to_le32(rtw_read32(adapter, REG_PKTBUF_DBG_DATA_H));
+ res = rtw_read32(adapter, REG_PKTBUF_DBG_DATA_L, ®32);
+ if (res)
+ return;
+
+ lo32 = cpu_to_le32(reg32);
+
+
+ res = rtw_read32(adapter, REG_PKTBUF_DBG_DATA_H, ®32);
+ if (res)
+ return;
+
+ hi32 = cpu_to_le32(reg32);
if (i == 0) {
int res;
@@ -544,8 +555,12 @@ void rtl8188e_read_chip_version(struct adapter *padapter)
u32 value32;
struct HAL_VERSION ChipVersion;
struct hal_data_8188e *pHalData = &padapter->haldata;
+ int res;
+
+ res = rtw_read32(padapter, REG_SYS_CFG, &value32);
+ if (res)
+ return;
- value32 = rtw_read32(padapter, REG_SYS_CFG);
ChipVersion.ChipType = ((value32 & RTL_ID) ? TEST_CHIP : NORMAL_CHIP);
ChipVersion.VendorType = ((value32 & VENDOR_ID) ? CHIP_VENDOR_UMC : CHIP_VENDOR_TSMC);
@@ -596,12 +611,16 @@ static s32 _LLTWrite(struct adapter *padapter, u32 address, u32 data)
s32 count = 0;
u32 value = _LLT_INIT_ADDR(address) | _LLT_INIT_DATA(data) | _LLT_OP(_LLT_WRITE_ACCESS);
u16 LLTReg = REG_LLT_INIT;
+ int res;
rtw_write32(padapter, LLTReg, value);
/* polling */
do {
- value = rtw_read32(padapter, LLTReg);
+ res = rtw_read32(padapter, LLTReg, &value);
+ if (res)
+ continue;
+
if (_LLT_NO_ACTIVE == _LLT_OP_VALUE(value))
break;
diff --git a/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c b/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
index 298c3d9bc7be..dea6d915a1f4 100644
--- a/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
+++ b/drivers/staging/r8188eu/hal/rtl8188e_phycfg.c
@@ -56,8 +56,12 @@ rtl8188e_PHY_QueryBBReg(
)
{
u32 ReturnValue = 0, OriginalValue, BitShift;
+ int res;
+
+ res = rtw_read32(Adapter, RegAddr, &OriginalValue);
+ if (res)
+ return 0;
- OriginalValue = rtw_read32(Adapter, RegAddr);
BitShift = phy_CalculateBitShift(BitMask);
ReturnValue = (OriginalValue & BitMask) >> BitShift;
return ReturnValue;
@@ -84,9 +88,13 @@ rtl8188e_PHY_QueryBBReg(
void rtl8188e_PHY_SetBBReg(struct adapter *Adapter, u32 RegAddr, u32 BitMask, u32 Data)
{
u32 OriginalValue, BitShift;
+ int res;
if (BitMask != bMaskDWord) { /* if not "double word" write */
- OriginalValue = rtw_read32(Adapter, RegAddr);
+ res = rtw_read32(Adapter, RegAddr, &OriginalValue);
+ if (res)
+ return;
+
BitShift = phy_CalculateBitShift(BitMask);
Data = ((OriginalValue & (~BitMask)) | (Data << BitShift));
}
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 1a68e4f19dc2..6bc04950b2c6 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -297,8 +297,12 @@ static void _InitQueuePriority(struct adapter *Adapter)
static void _InitNetworkType(struct adapter *Adapter)
{
u32 value32;
+ int res;
+
+ res = rtw_read32(Adapter, REG_CR, &value32);
+ if (res)
+ return;
- value32 = rtw_read32(Adapter, REG_CR);
/* TODO: use the other function to set network type */
value32 = (value32 & ~MASK_NETTYPE) | _NETTYPE(NT_LINK_AP);
@@ -338,9 +342,13 @@ static void _InitAdaptiveCtrl(struct adapter *Adapter)
{
u16 value16;
u32 value32;
+ int res;
/* Response Rate Set */
- value32 = rtw_read32(Adapter, REG_RRSR);
+ res = rtw_read32(Adapter, REG_RRSR, &value32);
+ if (res)
+ return;
+
value32 &= ~RATE_BITMAP_ALL;
value32 |= RATE_RRSR_CCK_ONLY_1M;
rtw_write32(Adapter, REG_RRSR, value32);
@@ -409,11 +417,15 @@ static void _InitRetryFunction(struct adapter *Adapter)
static void usb_AggSettingTxUpdate(struct adapter *Adapter)
{
u32 value32;
+ int res;
if (Adapter->registrypriv.wifi_spec)
return;
- value32 = rtw_read32(Adapter, REG_TDECTRL);
+ res = rtw_read32(Adapter, REG_TDECTRL, &value32);
+ if (res)
+ return;
+
value32 = value32 & ~(BLK_DESC_NUM_MASK << BLK_DESC_NUM_SHIFT);
value32 |= ((USB_TXAGG_DESC_NUM & BLK_DESC_NUM_MASK) << BLK_DESC_NUM_SHIFT);
@@ -511,11 +523,17 @@ static void _BBTurnOnBlock(struct adapter *Adapter)
static void _InitAntenna_Selection(struct adapter *Adapter)
{
struct hal_data_8188e *haldata = &Adapter->haldata;
+ int res;
+ u32 reg;
if (haldata->AntDivCfg == 0)
return;
- rtw_write32(Adapter, REG_LEDCFG0, rtw_read32(Adapter, REG_LEDCFG0) | BIT(23));
+ res = rtw_read32(Adapter, REG_LEDCFG0, ®);
+ if (res)
+ return;
+
+ rtw_write32(Adapter, REG_LEDCFG0, reg | BIT(23));
rtl8188e_PHY_SetBBReg(Adapter, rFPGA0_XAB_RFParameter, BIT(13), 0x01);
if (rtl8188e_PHY_QueryBBReg(Adapter, rFPGA0_XA_RFInterfaceOE, 0x300) == Antenna_A)
@@ -545,6 +563,7 @@ u32 rtl8188eu_hal_init(struct adapter *Adapter)
struct hal_data_8188e *haldata = &Adapter->haldata;
struct pwrctrl_priv *pwrctrlpriv = &Adapter->pwrctrlpriv;
struct registry_priv *pregistrypriv = &Adapter->registrypriv;
+ u32 reg;
if (Adapter->pwrctrlpriv.bkeepfwalive) {
if (haldata->odmpriv.RFCalibrateInfo.bIQKInitialized) {
@@ -742,7 +761,11 @@ u32 rtl8188eu_hal_init(struct adapter *Adapter)
rtw_write8(Adapter, REG_USB_HRPWM, 0);
/* ack for xmit mgmt frames. */
- rtw_write32(Adapter, REG_FWHW_TXQ_CTRL, rtw_read32(Adapter, REG_FWHW_TXQ_CTRL) | BIT(12));
+ res = rtw_read32(Adapter, REG_FWHW_TXQ_CTRL, ®);
+ if (res)
+ return _FAIL;
+
+ rtw_write32(Adapter, REG_FWHW_TXQ_CTRL, reg | BIT(12));
exit:
return status;
@@ -1111,7 +1134,12 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
case HW_VAR_MLME_SITESURVEY:
if (*((u8 *)val)) { /* under sitesurvey */
/* config RCR to receive different BSSID & not to receive data frame */
- u32 v = rtw_read32(Adapter, REG_RCR);
+ u32 v;
+
+ res = rtw_read32(Adapter, REG_RCR, &v);
+ if (res)
+ return;
+
v &= ~(RCR_CBSSID_BCN);
rtw_write32(Adapter, REG_RCR, v);
/* reject all data frame */
@@ -1126,6 +1154,7 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
} else { /* sitesurvey done */
struct mlme_ext_priv *pmlmeext = &Adapter->mlmeextpriv;
struct mlme_ext_info *pmlmeinfo = &pmlmeext->mlmext_info;
+ u32 reg32;
if ((is_client_associated_to_ap(Adapter)) ||
((pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE)) {
@@ -1147,7 +1176,12 @@ void SetHwReg8188EU(struct adapter *Adapter, u8 variable, u8 *val)
rtw_write8(Adapter, REG_BCN_CTRL, reg & (~BIT(4)));
}
- rtw_write32(Adapter, REG_RCR, rtw_read32(Adapter, REG_RCR) | RCR_CBSSID_BCN);
+
+ res = rtw_read32(Adapter, REG_RCR, ®32);
+ if (res)
+ return;
+
+ rtw_write32(Adapter, REG_RCR, reg32 | RCR_CBSSID_BCN);
}
break;
case HW_VAR_SLOT_TIME:
@@ -1320,7 +1354,10 @@ void SetBeaconRelatedRegisters8188EUsb(struct adapter *adapt)
rtw_write8(adapt, REG_SLOT, 0x09);
- value32 = rtw_read32(adapt, REG_TCR);
+ res = rtw_read32(adapt, REG_TCR, &value32);
+ if (res)
+ return;
+
value32 &= ~TSFRST;
rtw_write32(adapt, REG_TCR, value32);
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 7d62f1f3d26e..c1a4d023f627 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -120,16 +120,21 @@ int __must_check rtw_read16(struct adapter *adapter, u32 addr, u16 *data)
return 0;
}
-u32 rtw_read32(struct adapter *adapter, u32 addr)
+int __must_check rtw_read32(struct adapter *adapter, u32 addr, u32 *data)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
u16 value = addr & 0xffff;
- __le32 data;
+ __le32 le_data;
+ int res;
- usb_read(intf, value, &data, 4);
+ res = usb_read(intf, value, &le_data, 4);
+ if (res)
+ return res;
- return le32_to_cpu(data);
+ *data = le32_to_cpu(le_data);
+
+ return 0;
}
int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index ce3369e33d66..1c6097367a67 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -222,7 +222,7 @@ void _rtw_attrib_write(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
int __must_check rtw_read8(struct adapter *adapter, u32 addr, u8 *data);
int __must_check rtw_read16(struct adapter *adapter, u32 addr, u16 *data);
-u32 rtw_read32(struct adapter *adapter, u32 addr);
+int __must_check rtw_read32(struct adapter *adapter, u32 addr, u32 *data);
void _rtw_read_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
u32 rtw_read_port(struct adapter *adapter, u8 *pmem);
void rtw_read_port_cancel(struct adapter *adapter);
diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
index 1b35951a53cb..00d1ba62c248 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -1926,7 +1926,10 @@ static int rtw_wx_read32(struct net_device *dev,
sprintf(extra, "0x%04X", data32);
break;
case 4:
- data32 = rtw_read32(padapter, addr);
+ ret = rtw_read32(padapter, addr, &data32);
+ if (ret)
+ goto err_free_ptmp;
+
sprintf(extra, "0x%08X", data32);
break;
default:
@@ -3213,18 +3216,29 @@ static int rtw_rereg_nd_name(struct net_device *dev,
static void mac_reg_dump(struct adapter *padapter)
{
int i, j = 1;
+ u32 reg;
+ int res;
+
pr_info("\n ======= MAC REG =======\n");
for (i = 0x0; i < 0x300; i += 4) {
if (j % 4 == 1)
pr_info("0x%02x", i);
- pr_info(" 0x%08x ", rtw_read32(padapter, i));
+
+ res = rtw_read32(padapter, i, ®);
+ if (!res)
+ pr_info(" 0x%08x ", reg);
+
if ((j++) % 4 == 0)
pr_info("\n");
}
for (i = 0x400; i < 0x800; i += 4) {
if (j % 4 == 1)
pr_info("0x%02x", i);
- pr_info(" 0x%08x ", rtw_read32(padapter, i));
+
+ res = rtw_read32(padapter, i, ®);
+ if (!res)
+ pr_info(" 0x%08x ", reg);
+
if ((j++) % 4 == 0)
pr_info("\n");
}
@@ -3232,13 +3246,18 @@ static void mac_reg_dump(struct adapter *padapter)
static void bb_reg_dump(struct adapter *padapter)
{
- int i, j = 1;
+ int i, j = 1, res;
+ u32 reg;
+
pr_info("\n ======= BB REG =======\n");
for (i = 0x800; i < 0x1000; i += 4) {
if (j % 4 == 1)
pr_info("0x%02x", i);
- pr_info(" 0x%08x ", rtw_read32(padapter, i));
+ res = rtw_read32(padapter, i, ®);
+ if (!res)
+ pr_info(" 0x%08x ", reg);
+
if ((j++) % 4 == 0)
pr_info("\n");
}
@@ -3484,7 +3503,8 @@ static int rtw_dbg_port(struct net_device *dev,
if (rtl8188e_IOL_exec_cmds_sync(padapter, xmit_frame, 5000, 0) != _SUCCESS)
ret = -EPERM;
- rtw_read32(padapter, reg);
+ /* FIXME: is this read necessary? */
+ ret = rtw_read32(padapter, reg, &write_num);
}
break;
}
diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index d9325ef6ac28..cac9553666e6 100644
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -741,6 +741,7 @@ static void rtw_fifo_cleanup(struct adapter *adapter)
struct pwrctrl_priv *pwrpriv = &adapter->pwrctrlpriv;
u8 trycnt = 100;
int res;
+ u32 reg;
/* pause tx */
rtw_write8(adapter, REG_TXPAUSE, 0xff);
@@ -753,10 +754,18 @@ static void rtw_fifo_cleanup(struct adapter *adapter)
if (!pwrpriv->bkeepfwalive) {
/* RX DMA stop */
+ res = rtw_read32(adapter, REG_RXPKT_NUM, ®);
+ if (res)
+ return;
+
rtw_write32(adapter, REG_RXPKT_NUM,
- (rtw_read32(adapter, REG_RXPKT_NUM) | RW_RELEASE_EN));
+ (reg | RW_RELEASE_EN));
do {
- if (!(rtw_read32(adapter, REG_RXPKT_NUM) & RXDMA_IDLE))
+ res = rtw_read32(adapter, REG_RXPKT_NUM, ®);
+ if (res)
+ continue;
+
+ if (!(reg & RXDMA_IDLE))
break;
} while (trycnt--);
--
2.36.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/4] MAINTAINERS: add myself as r8188eu reviewer
2022-05-18 22:11 [PATCH 0/4] staging: r8188eu: add error handling of usb read errors Pavel Skripkin
` (2 preceding siblings ...)
2022-05-18 22:12 ` [PATCH 3/4] staging: r8188eu: add error handling of rtw_read32 Pavel Skripkin
@ 2022-05-18 22:12 ` Pavel Skripkin
2022-05-19 5:46 ` Dan Carpenter
3 siblings, 1 reply; 13+ messages in thread
From: Pavel Skripkin @ 2022-05-18 22:12 UTC (permalink / raw)
To: gregkh, Larry.Finger, phil, straube.linux, dan.carpenter, fmdefrancesco
Cc: linux-kernel, linux-staging, Pavel Skripkin
I was reviewing r8188eu patches for a while, but I am missing some of
them, since I am not in CC list. I want to be CC'ed to help reviewing
and testing more patches.
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 0a635f2ae5b9..bd38a2e465cc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18716,6 +18716,7 @@ F: drivers/staging/olpc_dcon/
STAGING - REALTEK RTL8188EU DRIVERS
M: Larry Finger <Larry.Finger@lwfinger.net>
M: Phillip Potter <phil@philpotter.co.uk>
+R: Pavel Skripkin <paskripkin@gmail.com>
S: Supported
F: drivers/staging/r8188eu/
--
2.36.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8
2022-05-18 22:11 ` [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8 Pavel Skripkin
@ 2022-05-19 1:34 ` kernel test robot
2022-05-19 4:33 ` Dan Carpenter
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2022-05-19 1:34 UTC (permalink / raw)
To: Pavel Skripkin, gregkh, Larry.Finger, phil, straube.linux,
dan.carpenter, fmdefrancesco
Cc: kbuild-all, linux-kernel, linux-staging, Pavel Skripkin
Hi Pavel,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
url: https://github.com/intel-lab-lkp/linux/commits/Pavel-Skripkin/staging-r8188eu-add-error-handling-of-usb-read-errors/20220519-061342
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git e41f7a5521d7f03dca99e3207633df71740569dd
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20220519/202205190916.GRvUby7c-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/f8c9bd199a25d7a1d8f6fed1b0d5da9cec1f8faa
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Pavel-Skripkin/staging-r8188eu-add-error-handling-of-usb-read-errors/20220519-061342
git checkout f8c9bd199a25d7a1d8f6fed1b0d5da9cec1f8faa
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/staging/r8188eu/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/staging/r8188eu/hal/usb_halinit.c: In function '_InitBeaconParameters':
>> drivers/staging/r8188eu/hal/usb_halinit.c:465:13: warning: variable 'res' set but not used [-Wunused-but-set-variable]
465 | int res;
| ^~~
--
drivers/staging/r8188eu/os_dep/ioctl_linux.c: In function 'rtw_dbg_port':
>> drivers/staging/r8188eu/os_dep/ioctl_linux.c:3419:32: warning: variable 'res' set but not used [-Wunused-but-set-variable]
3419 | int i, res;
| ^~~
vim +/res +465 drivers/staging/r8188eu/hal/usb_halinit.c
461
462 static void _InitBeaconParameters(struct adapter *Adapter)
463 {
464 struct hal_data_8188e *haldata = &Adapter->haldata;
> 465 int res;
466
467 rtw_write16(Adapter, REG_BCN_CTRL, 0x1010);
468
469 /* TODO: Remove these magic number */
470 rtw_write16(Adapter, REG_TBTT_PROHIBIT, 0x6404);/* ms */
471 rtw_write8(Adapter, REG_DRVERLYINT, DRIVER_EARLY_INT_TIME);/* 5ms */
472 rtw_write8(Adapter, REG_BCNDMATIM, BCN_DMA_ATIME_INT_TIME); /* 2ms */
473
474 /* Suggested by designer timchen. Change beacon AIFS to the largest number */
475 /* beacause test chip does not contension before sending beacon. by tynli. 2009.11.03 */
476 rtw_write16(Adapter, REG_BCNTCFG, 0x660F);
477
478 /* FIXME: return an error to caller */
479 res = rtw_read8(Adapter, REG_FWHW_TXQ_CTRL + 2, &haldata->RegFwHwTxQCtrl);
480 res = rtw_read8(Adapter, REG_TBTT_PROHIBIT + 2, &haldata->RegReg542);
481 res = rtw_read8(Adapter, REG_CR + 1, &haldata->RegCR_1);
482 }
483
--
0-DAY CI Kernel Test Service
https://01.org/lkp
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8
2022-05-18 22:11 ` [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8 Pavel Skripkin
2022-05-19 1:34 ` kernel test robot
@ 2022-05-19 4:33 ` Dan Carpenter
2022-05-19 5:43 ` Pavel Skripkin
1 sibling, 1 reply; 13+ messages in thread
From: Dan Carpenter @ 2022-05-19 4:33 UTC (permalink / raw)
To: Pavel Skripkin
Cc: gregkh, Larry.Finger, phil, straube.linux, fmdefrancesco,
linux-kernel, linux-staging
On Thu, May 19, 2022 at 01:11:51AM +0300, Pavel Skripkin wrote:
> @@ -240,12 +259,14 @@ int rtl8188e_firmware_download(struct adapter *padapter)
> {
> int ret = _SUCCESS;
> u8 write_fw_retry = 0;
> + u8 reg;
> unsigned long fwdl_timeout;
> struct dvobj_priv *dvobj = adapter_to_dvobj(padapter);
> struct device *device = dvobj_to_dev(dvobj);
> struct rt_firmware_hdr *fwhdr = NULL;
> u8 *fw_data;
> u32 fw_size;
> + int res;
>
> if (!dvobj->firmware.data)
> ret = load_firmware(&dvobj->firmware, device);
> @@ -269,7 +290,11 @@ int rtl8188e_firmware_download(struct adapter *padapter)
>
> /* Suggested by Filen. If 8051 is running in RAM code, driver should inform Fw to reset by itself, */
> /* or it will cause download Fw fail. 2010.02.01. by tynli. */
> - if (rtw_read8(padapter, REG_MCUFWDL) & RAM_DL_SEL) { /* 8051 RAM code */
> + res = rtw_read8(padapter, REG_MCUFWDL, ®);
> + if (res)
> + goto exit;
You didn't introduce this bug, but this path needs to have an error code
set. Also we really need to get rid of the _FAIL garbage. When I saw
this, I got "ret" and "res" mixed up so I thought we were returning
negative error codes instead of _FAIL. That would But then I saw we
are returning success.
> +
> + if (reg & RAM_DL_SEL) { /* 8051 RAM code */
> rtw_write8(padapter, REG_MCUFWDL, 0x00);
> rtw_reset_8051(padapter);
> }
> @@ -278,7 +303,14 @@ int rtl8188e_firmware_download(struct adapter *padapter)
> fwdl_timeout = jiffies + msecs_to_jiffies(500);
> while (1) {
> /* reset the FWDL chksum */
> - rtw_write8(padapter, REG_MCUFWDL, rtw_read8(padapter, REG_MCUFWDL) | FWDL_CHKSUM_RPT);
> + res = rtw_read8(padapter, REG_MCUFWDL, ®);
> + if (res == -ENODEV)
> + break;
> +
> + if (res)
> + continue;
This continue is wrong. If res = -EPERM then it's a forever loop.
Let's just break for every error.
> +
> + rtw_write8(padapter, REG_MCUFWDL, reg | FWDL_CHKSUM_RPT);
>
> ret = write_fw(padapter, fw_data, fw_size);
>
> diff --git a/drivers/staging/r8188eu/core/rtw_led.c b/drivers/staging/r8188eu/core/rtw_led.c
> index 2f3000428af7..b532e614c5b6 100644
> --- a/drivers/staging/r8188eu/core/rtw_led.c
> +++ b/drivers/staging/r8188eu/core/rtw_led.c
> @@ -34,28 +34,38 @@ static void ResetLedStatus(struct LED_871x *pLed)
>
> static void SwLedOn(struct adapter *padapter, struct LED_871x *pLed)
> {
> - u8 LedCfg;
> + u8 LedCfg;
Please don't make unrelated changes.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] staging: r8188eu: add error handling of rtw_read16
2022-05-18 22:11 ` [PATCH 2/4] staging: r8188eu: add error handling of rtw_read16 Pavel Skripkin
@ 2022-05-19 4:47 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2022-05-19 4:47 UTC (permalink / raw)
To: Pavel Skripkin
Cc: gregkh, Larry.Finger, phil, straube.linux, fmdefrancesco,
linux-kernel, linux-staging
On Thu, May 19, 2022 at 01:11:56AM +0300, Pavel Skripkin wrote:
> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
> index e67ecbd1ba79..22661c66cc18 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188e_hal_init.c
> @@ -249,11 +249,14 @@ static void efuse_read_phymap_from_txpktbuf(
> hi32 = cpu_to_le32(rtw_read32(adapter, REG_PKTBUF_DBG_DATA_H));
>
> if (i == 0) {
> + int res;
> + u16 reg;
> /* Although lenc is only used in a debug statement,
Blank line after declarations.
I think it's better to put "int res" declarations at the start of the
function. That's where people will expect to see it.
> * do not remove it as the rtw_read16() call consumes
> * 2 bytes from the EEPROM source.
> */
> - rtw_read16(adapter, REG_PKTBUF_DBG_DATA_L);
> + res = rtw_read16(adapter, REG_PKTBUF_DBG_DATA_L, ®);
> + (void) res;
>
> len = le32_to_cpu(lo32) & 0x0000ffff;
>
[ snip ]
> diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
> index 1198d3850a6d..ce3369e33d66 100644
> --- a/drivers/staging/r8188eu/include/rtw_io.h
> +++ b/drivers/staging/r8188eu/include/rtw_io.h
> @@ -221,7 +221,7 @@ void _rtw_attrib_read(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
> void _rtw_attrib_write(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
>
> int __must_check rtw_read8(struct adapter *adapter, u32 addr, u8 *data);
> -u16 rtw_read16(struct adapter *adapter, u32 addr);
> +int __must_check rtw_read16(struct adapter *adapter, u32 addr, u16 *data);
> u32 rtw_read32(struct adapter *adapter, u32 addr);
> void _rtw_read_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
> u32 rtw_read_port(struct adapter *adapter, u8 *pmem);
> diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> index 66aac2cbe3a9..1b35951a53cb 100644
> --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
> @@ -1919,7 +1919,10 @@ static int rtw_wx_read32(struct net_device *dev,
> sprintf(extra, "0x%02X", data32 & 0xff);
> break;
> case 2:
> - data32 = rtw_read16(padapter, addr);
> + ret = rtw_read16(padapter, addr, (u16 *) &data32);
Checkpatch.
I have an unpublished Smatch warning for casts like this. You can't
pass a data32 pointer to something which is takes a u16 pointer and
expect it to work. The last two bytes are uninitialized.
And even if you zero out the bytes, it is a bug on big endian systems.
> + if (ret)
> + goto err_free_ptmp;
> +
> sprintf(extra, "0x%04X", data32);
> break;
> case 4:
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8
2022-05-19 4:33 ` Dan Carpenter
@ 2022-05-19 5:43 ` Pavel Skripkin
2022-05-19 5:49 ` Dan Carpenter
0 siblings, 1 reply; 13+ messages in thread
From: Pavel Skripkin @ 2022-05-19 5:43 UTC (permalink / raw)
To: Dan Carpenter
Cc: gregkh, Larry.Finger, phil, straube.linux, fmdefrancesco,
linux-kernel, linux-staging
[-- Attachment #1.1: Type: text/plain, Size: 2777 bytes --]
Hi Dan,
On 5/19/22 07:33, Dan Carpenter wrote:
> On Thu, May 19, 2022 at 01:11:51AM +0300, Pavel Skripkin wrote:
>> @@ -240,12 +259,14 @@ int rtl8188e_firmware_download(struct adapter *padapter)
>> {
>> int ret = _SUCCESS;
>> u8 write_fw_retry = 0;
>> + u8 reg;
>> unsigned long fwdl_timeout;
>> struct dvobj_priv *dvobj = adapter_to_dvobj(padapter);
>> struct device *device = dvobj_to_dev(dvobj);
>> struct rt_firmware_hdr *fwhdr = NULL;
>> u8 *fw_data;
>> u32 fw_size;
>> + int res;
>>
>> if (!dvobj->firmware.data)
>> ret = load_firmware(&dvobj->firmware, device);
>> @@ -269,7 +290,11 @@ int rtl8188e_firmware_download(struct adapter *padapter)
>>
>> /* Suggested by Filen. If 8051 is running in RAM code, driver should inform Fw to reset by itself, */
>> /* or it will cause download Fw fail. 2010.02.01. by tynli. */
>> - if (rtw_read8(padapter, REG_MCUFWDL) & RAM_DL_SEL) { /* 8051 RAM code */
>> + res = rtw_read8(padapter, REG_MCUFWDL, ®);
>> + if (res)
>> + goto exit;
>
> You didn't introduce this bug, but this path needs to have an error code
> set. Also we really need to get rid of the _FAIL garbage. When I saw
> this, I got "ret" and "res" mixed up so I thought we were returning
> negative error codes instead of _FAIL. That would But then I saw we
> are returning success.
>
I see now, that 'res' and 'ret' got mixed up in my mind too. Will fix up
>> +
>> + if (reg & RAM_DL_SEL) { /* 8051 RAM code */
>> rtw_write8(padapter, REG_MCUFWDL, 0x00);
>> rtw_reset_8051(padapter);
>> }
>> @@ -278,7 +303,14 @@ int rtl8188e_firmware_download(struct adapter *padapter)
>> fwdl_timeout = jiffies + msecs_to_jiffies(500);
>> while (1) {
>> /* reset the FWDL chksum */
>> - rtw_write8(padapter, REG_MCUFWDL, rtw_read8(padapter, REG_MCUFWDL) | FWDL_CHKSUM_RPT);
>> + res = rtw_read8(padapter, REG_MCUFWDL, ®);
>> + if (res == -ENODEV)
>> + break;
>> +
>> + if (res)
>> + continue;
>
> This continue is wrong. If res = -EPERM then it's a forever loop.
> Let's just break for every error.
>
I was trying to avoid strict breaking the loop on any error, since I am
afraid this might break the driver.
What about:
do {
/* reset the FWDL chksum */
ret = rtw_read8(padapter, REG_MCUFWDL, ®);
if (ret == -ENODEV || ret == -EPERM)
break;
if (ret) {
ret == _FAIL;
continue;
}
rtw_write8(padapter, REG_MCUFWDL, reg | FWDL_CHKSUM_RPT);
ret = write_fw(padapter, fw_data, fw_size);
} while (!(ret == _SUCCESS ||
(time_after(jiffies, fwdl_timeout) && write_fw_retry++ >= 3)))
The idea is to break only on fatal errors to make things less strict
With regards,
Pavel Skripkin
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] staging: r8188eu: add error handling of rtw_read32
2022-05-18 22:12 ` [PATCH 3/4] staging: r8188eu: add error handling of rtw_read32 Pavel Skripkin
@ 2022-05-19 5:43 ` Dan Carpenter
2022-05-19 5:48 ` Pavel Skripkin
0 siblings, 1 reply; 13+ messages in thread
From: Dan Carpenter @ 2022-05-19 5:43 UTC (permalink / raw)
To: Pavel Skripkin
Cc: gregkh, Larry.Finger, phil, straube.linux, fmdefrancesco,
linux-kernel, linux-staging
On Thu, May 19, 2022 at 01:12:01AM +0300, Pavel Skripkin wrote:
> diff --git a/drivers/staging/r8188eu/core/rtw_efuse.c b/drivers/staging/r8188eu/core/rtw_efuse.c
> index a2691c7f96f6..7105122c2ba0 100644
> --- a/drivers/staging/r8188eu/core/rtw_efuse.c
> +++ b/drivers/staging/r8188eu/core/rtw_efuse.c
> @@ -47,9 +47,18 @@ ReadEFuseByte(
>
> /* Check bit 32 read-ready */
> retry = 0;
> - value32 = rtw_read32(Adapter, EFUSE_CTRL);
> - while (!(((value32 >> 24) & 0xff) & 0x80) && (retry < 10000)) {
> - value32 = rtw_read32(Adapter, EFUSE_CTRL);
> + res = rtw_read32(Adapter, EFUSE_CTRL, &value32);
> + if (res)
> + return;
> +
> + while (retry < 10000) {
> + res = rtw_read32(Adapter, EFUSE_CTRL, &value32);
> + if (res)
> + continue;
Forever loop. Always put the ++ in side the while (). Apparently,
Smatch does not catch this. #Idea #Oppurtunity
> +
> + if (((value32 >> 24) & 0xff) & 0x80)
> + break;
> +
> retry++;
> }
[ snip ]
> @@ -215,7 +222,10 @@ static int fw_free_to_go(struct adapter *padapter)
> /* polling for FW ready */
> counter = 0;
> do {
> - value32 = rtw_read32(padapter, REG_MCUFWDL);
> + res = rtw_read32(padapter, REG_MCUFWDL, &value32);
> + if (res)
> + continue;
> +
> if (value32 & WINTINI_RDY)
> return _SUCCESS;
> udelay(5);
You really want to do this delay on each iteration. So write it like
so:
res = rtw_read32(padapter, REG_MCUFWDL, &value32);
if (!res && value32 & WINTINI_RDY)
return _SUCCESS;
udelay(5);
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme_ext.c b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> index d4e59fab367c..e54d4139466d 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme_ext.c
> @@ -6023,6 +6023,7 @@ static void mlme_join(struct adapter *adapter, int type)
> struct mlme_priv *mlmepriv = &adapter->mlmepriv;
> u8 retry_limit = 0x30, reg;
> int res;
> + u32 reg32;
The reg32 should got before the res so it's in reverse Christmas tree
order.
[ snip ]
> @@ -245,8 +246,18 @@ static void efuse_read_phymap_from_txpktbuf(
> } while (time_before(jiffies, timeout));
>
> /* data from EEPROM needs to be in LE */
> - lo32 = cpu_to_le32(rtw_read32(adapter, REG_PKTBUF_DBG_DATA_L));
> - hi32 = cpu_to_le32(rtw_read32(adapter, REG_PKTBUF_DBG_DATA_H));
> + res = rtw_read32(adapter, REG_PKTBUF_DBG_DATA_L, ®32);
> + if (res)
> + return;
> +
> + lo32 = cpu_to_le32(reg32);
> +
> +
Double blank line. Checkpatch?
> @@ -596,12 +611,16 @@ static s32 _LLTWrite(struct adapter *padapter, u32 address, u32 data)
> s32 count = 0;
> u32 value = _LLT_INIT_ADDR(address) | _LLT_INIT_DATA(data) | _LLT_OP(_LLT_WRITE_ACCESS);
> u16 LLTReg = REG_LLT_INIT;
> + int res;
>
> rtw_write32(padapter, LLTReg, value);
>
> /* polling */
> do {
> - value = rtw_read32(padapter, LLTReg);
> + res = rtw_read32(padapter, LLTReg, &value);
> + if (res)
> + continue;
This continue has the potential to lead to a forever loop. The limit
check needs to be a part of the do while() condition. Probably send
that patch first, by itself as a clean up before adding this continue.
> +
> if (_LLT_NO_ACTIVE == _LLT_OP_VALUE(value))
> break;
>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] MAINTAINERS: add myself as r8188eu reviewer
2022-05-18 22:12 ` [PATCH 4/4] MAINTAINERS: add myself as r8188eu reviewer Pavel Skripkin
@ 2022-05-19 5:46 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2022-05-19 5:46 UTC (permalink / raw)
To: Pavel Skripkin
Cc: gregkh, Larry.Finger, phil, straube.linux, fmdefrancesco,
linux-kernel, linux-staging
On Thu, May 19, 2022 at 01:12:06AM +0300, Pavel Skripkin wrote:
> I was reviewing r8188eu patches for a while, but I am missing some of
> them, since I am not in CC list. I want to be CC'ed to help reviewing
> and testing more patches.
>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0a635f2ae5b9..bd38a2e465cc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18716,6 +18716,7 @@ F: drivers/staging/olpc_dcon/
> STAGING - REALTEK RTL8188EU DRIVERS
> M: Larry Finger <Larry.Finger@lwfinger.net>
> M: Phillip Potter <phil@philpotter.co.uk>
> +R: Pavel Skripkin <paskripkin@gmail.com>
> S: Supported
> F: drivers/staging/r8188eu/
Acked-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] staging: r8188eu: add error handling of rtw_read32
2022-05-19 5:43 ` Dan Carpenter
@ 2022-05-19 5:48 ` Pavel Skripkin
0 siblings, 0 replies; 13+ messages in thread
From: Pavel Skripkin @ 2022-05-19 5:48 UTC (permalink / raw)
To: Dan Carpenter
Cc: gregkh, Larry.Finger, phil, straube.linux, fmdefrancesco,
linux-kernel, linux-staging
[-- Attachment #1.1: Type: text/plain, Size: 1225 bytes --]
Hi Dan,
On 5/19/22 08:43, Dan Carpenter wrote:
> On Thu, May 19, 2022 at 01:12:01AM +0300, Pavel Skripkin wrote:
>> diff --git a/drivers/staging/r8188eu/core/rtw_efuse.c b/drivers/staging/r8188eu/core/rtw_efuse.c
>> index a2691c7f96f6..7105122c2ba0 100644
>> --- a/drivers/staging/r8188eu/core/rtw_efuse.c
>> +++ b/drivers/staging/r8188eu/core/rtw_efuse.c
>> @@ -47,9 +47,18 @@ ReadEFuseByte(
>>
>> /* Check bit 32 read-ready */
>> retry = 0;
>> - value32 = rtw_read32(Adapter, EFUSE_CTRL);
>> - while (!(((value32 >> 24) & 0xff) & 0x80) && (retry < 10000)) {
>> - value32 = rtw_read32(Adapter, EFUSE_CTRL);
>> + res = rtw_read32(Adapter, EFUSE_CTRL, &value32);
>> + if (res)
>> + return;
>> +
>> + while (retry < 10000) {
>> + res = rtw_read32(Adapter, EFUSE_CTRL, &value32);
>> + if (res)
>> + continue;
>
> Forever loop. Always put the ++ in side the while (). Apparently,
> Smatch does not catch this. #Idea #Oppurtunity
>
I have missed it every single loop... :(
That's why I don't like 'while' loops, 'for' suits much better for this
kind of things.
Thanks you for your review! Will address your comments in next version
With regards,
Pavel Skripkin
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8
2022-05-19 5:43 ` Pavel Skripkin
@ 2022-05-19 5:49 ` Dan Carpenter
0 siblings, 0 replies; 13+ messages in thread
From: Dan Carpenter @ 2022-05-19 5:49 UTC (permalink / raw)
To: Pavel Skripkin
Cc: gregkh, Larry.Finger, phil, straube.linux, fmdefrancesco,
linux-kernel, linux-staging
On Thu, May 19, 2022 at 08:43:23AM +0300, Pavel Skripkin wrote:
> > > +
> > > + if (reg & RAM_DL_SEL) { /* 8051 RAM code */
> > > rtw_write8(padapter, REG_MCUFWDL, 0x00);
> > > rtw_reset_8051(padapter);
> > > }
> > > @@ -278,7 +303,14 @@ int rtl8188e_firmware_download(struct adapter *padapter)
> > > fwdl_timeout = jiffies + msecs_to_jiffies(500);
> > > while (1) {
> > > /* reset the FWDL chksum */
> > > - rtw_write8(padapter, REG_MCUFWDL, rtw_read8(padapter, REG_MCUFWDL) | FWDL_CHKSUM_RPT);
> > > + res = rtw_read8(padapter, REG_MCUFWDL, ®);
> > > + if (res == -ENODEV)
> > > + break;
> > > +
> > > + if (res)
> > > + continue;
> >
> > This continue is wrong. If res = -EPERM then it's a forever loop.
> > Let's just break for every error.
> >
>
> I was trying to avoid strict breaking the loop on any error, since I am
> afraid this might break the driver.
>
> What about:
>
> do {
> /* reset the FWDL chksum */
> ret = rtw_read8(padapter, REG_MCUFWDL, ®);
> if (ret == -ENODEV || ret == -EPERM)
> break;
>
> if (ret) {
> ret == _FAIL;
> continue;
> }
>
> rtw_write8(padapter, REG_MCUFWDL, reg | FWDL_CHKSUM_RPT);
>
> ret = write_fw(padapter, fw_data, fw_size);
> } while (!(ret == _SUCCESS ||
> (time_after(jiffies, fwdl_timeout) && write_fw_retry++ >= 3)))
>
> The idea is to break only on fatal errors to make things less strict
>
This is too complicated.
Treat all the errors the same, and use one time out condition. Either
based on the jiffies or the retry count.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2022-05-19 5:51 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-18 22:11 [PATCH 0/4] staging: r8188eu: add error handling of usb read errors Pavel Skripkin
2022-05-18 22:11 ` [PATCH 1/4] staging: r8188eu: add error handling of rtw_read8 Pavel Skripkin
2022-05-19 1:34 ` kernel test robot
2022-05-19 4:33 ` Dan Carpenter
2022-05-19 5:43 ` Pavel Skripkin
2022-05-19 5:49 ` Dan Carpenter
2022-05-18 22:11 ` [PATCH 2/4] staging: r8188eu: add error handling of rtw_read16 Pavel Skripkin
2022-05-19 4:47 ` Dan Carpenter
2022-05-18 22:12 ` [PATCH 3/4] staging: r8188eu: add error handling of rtw_read32 Pavel Skripkin
2022-05-19 5:43 ` Dan Carpenter
2022-05-19 5:48 ` Pavel Skripkin
2022-05-18 22:12 ` [PATCH 4/4] MAINTAINERS: add myself as r8188eu reviewer Pavel Skripkin
2022-05-19 5:46 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).