* [PATCH v9 01/16] staging: r8188eu: clean up symbols in usbctrl_vendorreq()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 02/16] staging: r8188eu: reorder declarations " Fabio M. De Francesco
` (14 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Clean up symbol names in usbctrl_vendorreq():
pdata => data;
pio_priv => io_priv;
pintfhdl => intf.
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 28 ++++++++++-----------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 6de99079e117..3e7bc510197e 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -8,14 +8,14 @@
#include "../include/recv_osdep.h"
#include "../include/rtl8188e_hal.h"
-static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, u16 len, u8 requesttype)
+static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 len, u8 requesttype)
{
- struct adapter *adapt = pintfhdl->padapter;
- struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
+ struct adapter *adapt = intf->padapter;
+ struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
struct usb_device *udev = dvobjpriv->pusbdev;
unsigned int pipe;
int status = 0;
- u8 *pIo_buf;
+ u8 *io_buf;
int vendorreq_times = 0;
if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) {
@@ -32,10 +32,10 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
/* Acquire IO memory for vendorreq */
- pIo_buf = dvobjpriv->usb_vendor_req_buf;
+ io_buf = dvobjpriv->usb_vendor_req_buf;
- if (!pIo_buf) {
- DBG_88E("[%s] pIo_buf == NULL\n", __func__);
+ if (!io_buf) {
+ DBG_88E("[%s] io_buf == NULL\n", __func__);
status = -ENOMEM;
goto release_mutex;
}
@@ -47,22 +47,22 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
if (requesttype == REALTEK_USB_VENQT_READ)
- memset(pIo_buf, 0, len);
+ memset(io_buf, 0, len);
else
- memcpy(pIo_buf, pdata, len);
+ memcpy(io_buf, data, len);
status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
- pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
+ io_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
if (status == len) { /* Success this control transfer. */
rtw_reset_continual_urb_error(dvobjpriv);
if (requesttype == REALTEK_USB_VENQT_READ)
- memcpy(pdata, pIo_buf, len);
+ memcpy(data, io_buf, len);
} else { /* error cases */
DBG_88E("reg 0x%x, usb %s %u fail, status:%d value=0x%x, vendorreq_times:%d\n",
value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write",
- len, status, *(u32 *)pdata, vendorreq_times);
+ len, status, *(u32 *)data, vendorreq_times);
if (status < 0) {
if (status == -ESHUTDOWN || status == -ENODEV) {
@@ -74,8 +74,8 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
} else { /* status != len && status >= 0 */
if (status > 0) {
if (requesttype == REALTEK_USB_VENQT_READ) {
- /* For Control read transfer, we have to copy the read data from pIo_buf to pdata. */
- memcpy(pdata, pIo_buf, len);
+ /* For Control read transfer, we have to copy the read data from io_buf to data. */
+ memcpy(data, io_buf, len);
}
}
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 02/16] staging: r8188eu: reorder declarations in usbctrl_vendorreq()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 01/16] staging: r8188eu: clean up symbols in usbctrl_vendorreq() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 03/16] staging: r8188eu: remove unnecessary test " Fabio M. De Francesco
` (13 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Reorder variables declarations according to the "Reverse Xmas Tree"
style.
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 3e7bc510197e..84ec7c1346b1 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -13,10 +13,10 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
struct adapter *adapt = intf->padapter;
struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
struct usb_device *udev = dvobjpriv->pusbdev;
+ int vendorreq_times = 0;
unsigned int pipe;
int status = 0;
u8 *io_buf;
- int vendorreq_times = 0;
if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) {
status = -EPERM;
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 03/16] staging: r8188eu: remove unnecessary test in usbctrl_vendorreq()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 01/16] staging: r8188eu: clean up symbols in usbctrl_vendorreq() Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 02/16] staging: r8188eu: reorder declarations " Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 04/16] staging: r8188eu: reorder comments " Fabio M. De Francesco
` (12 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Remove unnecessary test for "!io_buf" in usbctrl_vendorreq(). This test
is not necessary because io_buf has been assigned with the address of
a region of dinamically allocated memory (dvobj->usb_alloc_vendor_req_buf)
by rtw_init_intf_priv() in os_dep/usb_intf.c.
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 84ec7c1346b1..61a016e3608f 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -34,12 +34,6 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
/* Acquire IO memory for vendorreq */
io_buf = dvobjpriv->usb_vendor_req_buf;
- if (!io_buf) {
- DBG_88E("[%s] io_buf == NULL\n", __func__);
- status = -ENOMEM;
- goto release_mutex;
- }
-
if (requesttype == REALTEK_USB_VENQT_READ)
pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
else
@@ -91,7 +85,7 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || status == len)
break;
}
-release_mutex:
+
mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
exit:
return status;
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 04/16] staging: r8188eu: reorder comments in usbctrl_vendorreq()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (2 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 03/16] staging: r8188eu: remove unnecessary test " Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 05/16] staging: r8188eu: remove a comment from usbctrl_vendorreq() Fabio M. De Francesco
` (11 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Reorder comments in usbctrl_vendorreq() to follow Linux coding style.
Delete two of them because they are "obvious".
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 61a016e3608f..35d268c5cd7f 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -35,9 +35,9 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
io_buf = dvobjpriv->usb_vendor_req_buf;
if (requesttype == REALTEK_USB_VENQT_READ)
- pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
+ pipe = usb_rcvctrlpipe(udev, 0);
else
- pipe = usb_sndctrlpipe(udev, 0);/* write_out */
+ pipe = usb_sndctrlpipe(udev, 0);
while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
if (requesttype == REALTEK_USB_VENQT_READ)
@@ -49,11 +49,13 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
io_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
- if (status == len) { /* Success this control transfer. */
+ if (status == len) {
+ /* success */
rtw_reset_continual_urb_error(dvobjpriv);
if (requesttype == REALTEK_USB_VENQT_READ)
memcpy(data, io_buf, len);
- } else { /* error cases */
+ } else {
+ /* error cases */
DBG_88E("reg 0x%x, usb %s %u fail, status:%d value=0x%x, vendorreq_times:%d\n",
value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write",
len, status, *(u32 *)data, vendorreq_times);
@@ -65,7 +67,8 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
}
- } else { /* status != len && status >= 0 */
+ } else {
+ /* status != len && status >= 0 */
if (status > 0) {
if (requesttype == REALTEK_USB_VENQT_READ) {
/* For Control read transfer, we have to copy the read data from io_buf to data. */
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 05/16] staging: r8188eu: remove a comment from usbctrl_vendorreq()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (3 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 04/16] staging: r8188eu: reorder comments " Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 06/16] staging: r8188eu: rename symbols in rtw_read*() and rtw_write*() Fabio M. De Francesco
` (10 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Remove an unnecessary comment from usbctrl_vendorreq().
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 35d268c5cd7f..5c9613cc2415 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -70,10 +70,8 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
} else {
/* status != len && status >= 0 */
if (status > 0) {
- if (requesttype == REALTEK_USB_VENQT_READ) {
- /* For Control read transfer, we have to copy the read data from io_buf to data. */
+ if (requesttype == REALTEK_USB_VENQT_READ)
memcpy(data, io_buf, len);
- }
}
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 06/16] staging: r8188eu: rename symbols in rtw_read*() and rtw_write*()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (4 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 05/16] staging: r8188eu: remove a comment from usbctrl_vendorreq() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 07/16] staging: r8188eu: remove casts from rtw_{read,write}*() Fabio M. De Francesco
` (9 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Rename names of variables in rtw_read{8.16.32}() and in
rtw_write{8,16,32,N}() because of unnecessary 'p' (that probably stand
for "pointer to") and 'w' (that probably stands for "word"):
pio_priv => io_priv;
pintfhdl => intf;
wvalue => value.
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 60 ++++++++++-----------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 5c9613cc2415..9ff57e114b04 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -94,91 +94,91 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
u8 rtw_read8(struct adapter *adapter, u32 addr)
{
- struct io_priv *pio_priv = &adapter->iopriv;
- struct intf_hdl *pintfhdl = &pio_priv->intf;
- u16 wvalue = (u16)(addr & 0x0000ffff);
+ struct io_priv *io_priv = &adapter->iopriv;
+ struct intf_hdl *intf = &io_priv->intf;
+ u16 value = (u16)(addr & 0x0000ffff);
u8 data;
- usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ);
+ usbctrl_vendorreq(intf, value, &data, 1, REALTEK_USB_VENQT_READ);
return data;
}
u16 rtw_read16(struct adapter *adapter, u32 addr)
{
- struct io_priv *pio_priv = &adapter->iopriv;
- struct intf_hdl *pintfhdl = &pio_priv->intf;
- u16 wvalue = (u16)(addr & 0x0000ffff);
+ struct io_priv *io_priv = &adapter->iopriv;
+ struct intf_hdl *intf = &io_priv->intf;
+ u16 value = (u16)(addr & 0x0000ffff);
__le32 data;
- usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_READ);
+ usbctrl_vendorreq(intf, value, &data, 2, REALTEK_USB_VENQT_READ);
return (u16)(le32_to_cpu(data) & 0xffff);
}
u32 rtw_read32(struct adapter *adapter, u32 addr)
{
- struct io_priv *pio_priv = &adapter->iopriv;
- struct intf_hdl *pintfhdl = &pio_priv->intf;
- u16 wvalue = (u16)(addr & 0x0000ffff);
+ struct io_priv *io_priv = &adapter->iopriv;
+ struct intf_hdl *intf = &io_priv->intf;
+ u16 value = (u16)(addr & 0x0000ffff);
__le32 data;
- usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_READ);
+ usbctrl_vendorreq(intf, value, &data, 4, REALTEK_USB_VENQT_READ);
return le32_to_cpu(data);
}
int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
{
- struct io_priv *pio_priv = &adapter->iopriv;
- struct intf_hdl *pintfhdl = &pio_priv->intf;
- u16 wvalue = (u16)(addr & 0x0000ffff);
+ struct io_priv *io_priv = &adapter->iopriv;
+ struct intf_hdl *intf = &io_priv->intf;
+ u16 value = (u16)(addr & 0x0000ffff);
int ret;
- ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
+ ret = usbctrl_vendorreq(intf, value, &val, 1, REALTEK_USB_VENQT_WRITE);
return RTW_STATUS_CODE(ret);
}
int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
{
- struct io_priv *pio_priv = &adapter->iopriv;
- struct intf_hdl *pintfhdl = &pio_priv->intf;
- u16 wvalue = (u16)(addr & 0x0000ffff);
+ struct io_priv *io_priv = &adapter->iopriv;
+ struct intf_hdl *intf = &io_priv->intf;
+ u16 value = (u16)(addr & 0x0000ffff);
__le32 data = cpu_to_le32(val & 0x0000ffff);
int ret;
- ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
+ ret = usbctrl_vendorreq(intf, value, &data, 2, REALTEK_USB_VENQT_WRITE);
return RTW_STATUS_CODE(ret);
}
int rtw_write32(struct adapter *adapter, u32 addr, u32 val)
{
- struct io_priv *pio_priv = &adapter->iopriv;
- struct intf_hdl *pintfhdl = &pio_priv->intf;
- u16 wvalue = (u16)(addr & 0x0000ffff);
+ struct io_priv *io_priv = &adapter->iopriv;
+ struct intf_hdl *intf = &io_priv->intf;
+ u16 value = (u16)(addr & 0x0000ffff);
__le32 data = cpu_to_le32(val);
int ret;
- ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE);
+ ret = usbctrl_vendorreq(intf, value, &data, 4, REALTEK_USB_VENQT_WRITE);
return RTW_STATUS_CODE(ret);
}
-int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata)
+int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *data)
{
- struct io_priv *pio_priv = &adapter->iopriv;
- struct intf_hdl *pintfhdl = &pio_priv->intf;
- u16 wvalue = (u16)(addr & 0x0000ffff);
+ struct io_priv *io_priv = &adapter->iopriv;
+ struct intf_hdl *intf = &io_priv->intf;
+ u16 value = (u16)(addr & 0x0000ffff);
u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
int ret;
if (length > VENDOR_CMD_MAX_DATA_LEN)
return _FAIL;
- memcpy(buf, pdata, length);
- ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
+ memcpy(buf, data, length);
+ ret = usbctrl_vendorreq(intf, value, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
return RTW_STATUS_CODE(ret);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 07/16] staging: r8188eu: remove casts from rtw_{read,write}*()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (5 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 06/16] staging: r8188eu: rename symbols in rtw_read*() and rtw_write*() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 08/16] staging: r8188eu: change the type of a variable in rtw_write16() Fabio M. De Francesco
` (8 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Remove unnecessary casts from rtw_read{8,16,32}() and from
rtw_write{8,16,32,N}().
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 9ff57e114b04..95cd7a6bc28b 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -96,7 +96,7 @@ u8 rtw_read8(struct adapter *adapter, u32 addr)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
- u16 value = (u16)(addr & 0x0000ffff);
+ u16 value = addr & 0xffff;
u8 data;
usbctrl_vendorreq(intf, value, &data, 1, REALTEK_USB_VENQT_READ);
@@ -108,7 +108,7 @@ u16 rtw_read16(struct adapter *adapter, u32 addr)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
- u16 value = (u16)(addr & 0x0000ffff);
+ u16 value = addr & 0xffff;
__le32 data;
usbctrl_vendorreq(intf, value, &data, 2, REALTEK_USB_VENQT_READ);
@@ -120,7 +120,7 @@ u32 rtw_read32(struct adapter *adapter, u32 addr)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
- u16 value = (u16)(addr & 0x0000ffff);
+ u16 value = addr & 0xffff;
__le32 data;
usbctrl_vendorreq(intf, value, &data, 4, REALTEK_USB_VENQT_READ);
@@ -132,7 +132,7 @@ int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
- u16 value = (u16)(addr & 0x0000ffff);
+ u16 value = addr & 0xffff;
int ret;
ret = usbctrl_vendorreq(intf, value, &val, 1, REALTEK_USB_VENQT_WRITE);
@@ -144,7 +144,7 @@ int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
- u16 value = (u16)(addr & 0x0000ffff);
+ u16 value = addr & 0xffff;
__le32 data = cpu_to_le32(val & 0x0000ffff);
int ret;
@@ -157,7 +157,7 @@ int rtw_write32(struct adapter *adapter, u32 addr, u32 val)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
- u16 value = (u16)(addr & 0x0000ffff);
+ u16 value = addr & 0xffff;
__le32 data = cpu_to_le32(val);
int ret;
@@ -170,7 +170,7 @@ int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *data)
{
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
- u16 value = (u16)(addr & 0x0000ffff);
+ u16 value = addr & 0xffff;
u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
int ret;
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 08/16] staging: r8188eu: change the type of a variable in rtw_write16()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (6 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 07/16] staging: r8188eu: remove casts from rtw_{read,write}*() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 09/16] staging: r8188eu: remove an buffer from rtw_writeN() Fabio M. De Francesco
` (7 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Change the type of "data" from __le32 to __le16 in rtw_write16(). The
argument "val", which is u16, after being conditionally swapped to little
endian, is assigned to "data"; therefore, __le16 is the most suitable type
for "data". Remove the bitwise AND of "val" with 0xffff because it is
redundant. Use cpu_to_le16() because "val" is u16.
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 95cd7a6bc28b..5dcab1ee4be0 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -145,7 +145,7 @@ int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
u16 value = addr & 0xffff;
- __le32 data = cpu_to_le32(val & 0x0000ffff);
+ __le16 data = cpu_to_le16(val);
int ret;
ret = usbctrl_vendorreq(intf, value, &data, 2, REALTEK_USB_VENQT_WRITE);
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 09/16] staging: r8188eu: remove an buffer from rtw_writeN()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (7 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 08/16] staging: r8188eu: change the type of a variable in rtw_write16() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 10/16] staging: r8188eu: remove a bitwise AND " Fabio M. De Francesco
` (6 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Remove the unnecessary buffer "u8 buf[VENDOR_CMD_MAX_DATA_LEN]" and
pass "data" directly to usbctrl_vendorreq().
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 5dcab1ee4be0..8e4e578ed60b 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -171,14 +171,12 @@ int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *data)
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
u16 value = addr & 0xffff;
- u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
int ret;
if (length > VENDOR_CMD_MAX_DATA_LEN)
return _FAIL;
- memcpy(buf, data, length);
- ret = usbctrl_vendorreq(intf, value, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
+ ret = usbctrl_vendorreq(intf, value, data, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
return RTW_STATUS_CODE(ret);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 10/16] staging: r8188eu: remove a bitwise AND from rtw_writeN()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (8 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 09/16] staging: r8188eu: remove an buffer from rtw_writeN() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 11/16] staging: r8188eu: change the type of a variable in rtw_read16() Fabio M. De Francesco
` (5 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Remove an unnecessary bitwise AND because "length" can never be greater
than 0xffff since VENDOR_CMD_MAX_DATA_LEN is defined as '254'.
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 8e4e578ed60b..3b50d2b5c0e3 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -176,7 +176,7 @@ int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *data)
if (length > VENDOR_CMD_MAX_DATA_LEN)
return _FAIL;
- ret = usbctrl_vendorreq(intf, value, data, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
+ ret = usbctrl_vendorreq(intf, value, data, length, REALTEK_USB_VENQT_WRITE);
return RTW_STATUS_CODE(ret);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 11/16] staging: r8188eu: change the type of a variable in rtw_read16()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (9 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 10/16] staging: r8188eu: remove a bitwise AND " Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 12/16] staging: r8188eu: Remove a test from usbctrl_vendorreq() Fabio M. De Francesco
` (4 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Change the type of "data" from __le32 to __le16. The size of the data
that usbctrl_vendorreq() will read is two bytes in little endian order,
so the most suitable type is __le16.
With the old code, since the two most significant bytes of data are not
initialized, KMSan can likely detect the reading of uninitialized data,
so this change can prevent the checker from complaining.
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 3b50d2b5c0e3..04a878c1a87d 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -109,11 +109,11 @@ u16 rtw_read16(struct adapter *adapter, u32 addr)
struct io_priv *io_priv = &adapter->iopriv;
struct intf_hdl *intf = &io_priv->intf;
u16 value = addr & 0xffff;
- __le32 data;
+ __le16 data;
usbctrl_vendorreq(intf, value, &data, 2, REALTEK_USB_VENQT_READ);
- return (u16)(le32_to_cpu(data) & 0xffff);
+ return le16_to_cpu(data);
}
u32 rtw_read32(struct adapter *adapter, u32 addr)
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 12/16] staging: r8188eu: Remove a test from usbctrl_vendorreq()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (10 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 11/16] staging: r8188eu: change the type of a variable in rtw_read16() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 13/16] staging: r8188eu: call new usb_read() from rtw_read{8,16,32}() Fabio M. De Francesco
` (3 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco
Remove an unnecessary 'if' test from usbctrl_vendorreq() because
"length" is never greater than MAX_VENDOR_REQ_CMD_SIZE.
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 04a878c1a87d..b3f8a76b5db2 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -23,12 +23,6 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
goto exit;
}
- if (len > MAX_VENDOR_REQ_CMD_SIZE) {
- DBG_88E("[%s] Buffer len error ,vendor request failed\n", __func__);
- status = -EINVAL;
- goto exit;
- }
-
mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
/* Acquire IO memory for vendorreq */
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 13/16] staging: r8188eu: call new usb_read() from rtw_read{8,16,32}()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (11 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 12/16] staging: r8188eu: Remove a test from usbctrl_vendorreq() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 14/16] staging: r8188eu: call new usb_write() from rtw_write{8,16,32,N}() Fabio M. De Francesco
` (2 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco, Greg Kroah-Hartman
Create and call new usb_read() instead of usbctrl_vendorreq() from
inside rtw_read{8,16,32}().
In old code, rtw_read{8,16,32}() called usbctrl_vendorreq() which in
turn uses usb_control_msg() from within a "while" loop to build a control
URB, send it off and wait for completion. usbctrl_vendorreq() was used for
both receiving and sending messages, depending on the "requesttype"
argument which was passed by callers.
Compared to usbctrl_vendorreq(), which managed both reads and writes
from and to the USB endpoint, the new usb_read() manages only reads. For
this purpose it uses the newer USB Core usb_control_msg_recv() API. The
latter is preferred according both to a suggestion by Greg Kroah-Hartman
and also to its actual design.
Two noteworthy features of usb_control_msg_recv() are that (1) the data
pointer can be made to a reference on the stack because it does not have
the restriction that usb_control_msg() has where the data pointer must be
to dynamically allocated memory, and that (2) the whole message must be
properly received from the device in order for this function to be
successfuli (if a device returns less than the expected amount of data,
then the function will fail).
usbctrl_vendorreq() uses a "while" loop that we considered unnecessary
so that it is not in the new usb_read(). Furthermore, the latter has no
redundant checking, less obvious comments, and it manages errors before
success cases. All in all, usb_read() is simpler than
usbctrl_vendorreq() and uses less lines of code.
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 59 +++++++++++++++++++--
1 file changed, 56 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index b3f8a76b5db2..a3dcd366c00c 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -86,6 +86,59 @@ static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 l
return status;
}
+static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
+{
+ struct adapter *adapt = intf->padapter;
+ struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
+ struct usb_device *udev = dvobjpriv->pusbdev;
+ int status;
+ u8 *io_buf; /* Pointer to I/O buffer */
+
+ mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
+
+ if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx)
+ return -EPERM;
+
+ io_buf = dvobjpriv->usb_vendor_req_buf;
+
+ status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+ REALTEK_USB_VENQT_READ, value,
+ REALTEK_USB_VENQT_CMD_IDX, io_buf,
+ size, RTW_USB_CONTROL_MSG_TIMEOUT,
+ GFP_KERNEL);
+
+ if (status == -ESHUTDOWN ||
+ status == -ENODEV ||
+ status == -ENOENT) {
+ /*
+ * device or controller has been disabled due to
+ * some problem that could not be worked around,
+ * device or bus doesn’t exist, endpoint does not
+ * exist or is not enabled.
+ */
+ adapt->bSurpriseRemoved = true;
+ goto mutex_unlock;
+ }
+
+ if (status < 0) {
+ GET_HAL_DATA(adapt)->srestpriv.wifi_error_status =
+ USB_VEN_REQ_CMD_FAIL;
+
+ if (rtw_inc_and_chk_continual_urb_error(dvobjpriv))
+ adapt->bSurpriseRemoved = true;
+
+ goto mutex_unlock;
+ }
+
+ rtw_reset_continual_urb_error(dvobjpriv);
+ memcpy(data, io_buf, size);
+
+mutex_unlock:
+ mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
+
+ return status;
+}
+
u8 rtw_read8(struct adapter *adapter, u32 addr)
{
struct io_priv *io_priv = &adapter->iopriv;
@@ -93,7 +146,7 @@ u8 rtw_read8(struct adapter *adapter, u32 addr)
u16 value = addr & 0xffff;
u8 data;
- usbctrl_vendorreq(intf, value, &data, 1, REALTEK_USB_VENQT_READ);
+ usb_read(intf, value, &data, 1);
return data;
}
@@ -105,7 +158,7 @@ u16 rtw_read16(struct adapter *adapter, u32 addr)
u16 value = addr & 0xffff;
__le16 data;
- usbctrl_vendorreq(intf, value, &data, 2, REALTEK_USB_VENQT_READ);
+ usb_read(intf, value, &data, 2);
return le16_to_cpu(data);
}
@@ -117,7 +170,7 @@ u32 rtw_read32(struct adapter *adapter, u32 addr)
u16 value = addr & 0xffff;
__le32 data;
- usbctrl_vendorreq(intf, value, &data, 4, REALTEK_USB_VENQT_READ);
+ usb_read(intf, value, &data, 4);
return le32_to_cpu(data);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 14/16] staging: r8188eu: call new usb_write() from rtw_write{8,16,32,N}()
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (12 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 13/16] staging: r8188eu: call new usb_read() from rtw_read{8,16,32}() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 15/16] staging: r8188eu: remove shared buffer for USB requests Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex Fabio M. De Francesco
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M. De Francesco, Greg Kroah-Hartman
Create and call new usb_write() instead of usbctrl_vendorreq() from
inside rtw_write{8,16,32,N}().
In old code, rtw_write{8,16,32,N}() called usbctrl_vendorreq() which in
turn uses usb_control_msg() from within a "while" loop to build a control
URB, send it off and wait for completion. usbctrl_vendorreq() was used
for both receiving and sending messages, depending on the "requesttype"
argument which is passed by callers.
Compared to usbctrl_vendorreq(), which manages both reads and writes
from and to the USB endpoint, the new usb_write() manages only writes.
For this purpose it uses the newer USB Core usb_control_msg_send() API.
The latter is preferred according both to suggestions by Greg Kroah-Hartman
and also to its actual design.
A noteworthy feature of usb_control_msg_send() is that the data pointer
can be made to a reference on the stack because it does not have the
restriction that usb_control_msg() has where the data pointer must be to
dynamically allocated memory.
usbctrl_vendorreq() used a "while" loop that we considered unnecessary
so that it is not in the new usb_write(). Furthermore, the latter has no
redundant checking, less obvious comments, no debug prints, and it manages
errors before success case. All in all, usb_write() is simpler than
usbctrl_vendorreq() and uses less lines of code.
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 108 ++++++++------------
1 file changed, 43 insertions(+), 65 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index a3dcd366c00c..88db7488b3a2 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -8,85 +8,63 @@
#include "../include/recv_osdep.h"
#include "../include/rtl8188e_hal.h"
-static int usbctrl_vendorreq(struct intf_hdl *intf, u16 value, void *data, u16 len, u8 requesttype)
+static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
{
struct adapter *adapt = intf->padapter;
struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
struct usb_device *udev = dvobjpriv->pusbdev;
- int vendorreq_times = 0;
- unsigned int pipe;
- int status = 0;
- u8 *io_buf;
+ int status;
+ u8 *io_buf; /* Pointer to I/O buffer */
- if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) {
+ mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
+
+ if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
status = -EPERM;
- goto exit;
+ goto mutex_unlock;
}
- mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
-
/* Acquire IO memory for vendorreq */
io_buf = dvobjpriv->usb_vendor_req_buf;
- if (requesttype == REALTEK_USB_VENQT_READ)
- pipe = usb_rcvctrlpipe(udev, 0);
- else
- pipe = usb_sndctrlpipe(udev, 0);
-
- while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
- if (requesttype == REALTEK_USB_VENQT_READ)
- memset(io_buf, 0, len);
- else
- memcpy(io_buf, data, len);
-
- status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
- requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
- io_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
+ status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+ REALTEK_USB_VENQT_READ, value,
+ REALTEK_USB_VENQT_CMD_IDX, io_buf,
+ size, RTW_USB_CONTROL_MSG_TIMEOUT,
+ GFP_KERNEL);
- if (status == len) {
- /* success */
- rtw_reset_continual_urb_error(dvobjpriv);
- if (requesttype == REALTEK_USB_VENQT_READ)
- memcpy(data, io_buf, len);
- } else {
- /* error cases */
- DBG_88E("reg 0x%x, usb %s %u fail, status:%d value=0x%x, vendorreq_times:%d\n",
- value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write",
- len, status, *(u32 *)data, vendorreq_times);
-
- if (status < 0) {
- if (status == -ESHUTDOWN || status == -ENODEV) {
- adapt->bSurpriseRemoved = true;
- } else {
- struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
- haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
- }
- } else {
- /* status != len && status >= 0 */
- if (status > 0) {
- if (requesttype == REALTEK_USB_VENQT_READ)
- memcpy(data, io_buf, len);
- }
- }
+ if (status == -ESHUTDOWN ||
+ status == -ENODEV ||
+ status == -ENOENT) {
+ /*
+ * device or controller has been disabled due to
+ * some problem that could not be worked around,
+ * device or bus doesn’t exist, endpoint does not
+ * exist or is not enabled.
+ */
+ adapt->bSurpriseRemoved = true;
+ goto mutex_unlock;
+ }
- if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
- adapt->bSurpriseRemoved = true;
- break;
- }
+ if (status < 0) {
+ GET_HAL_DATA(adapt)->srestpriv.wifi_error_status =
+ USB_VEN_REQ_CMD_FAIL;
- }
+ if (rtw_inc_and_chk_continual_urb_error(dvobjpriv))
+ adapt->bSurpriseRemoved = true;
- /* firmware download is checksumed, don't retry */
- if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || status == len)
- break;
+ goto mutex_unlock;
}
+ rtw_reset_continual_urb_error(dvobjpriv);
+ memcpy(data, io_buf, size);
+
+mutex_unlock:
mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
-exit:
+
return status;
}
-static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
+static int usb_write(struct intf_hdl *intf, u16 value, void *data, u8 size)
{
struct adapter *adapt = intf->padapter;
struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
@@ -101,8 +79,9 @@ static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
io_buf = dvobjpriv->usb_vendor_req_buf;
- status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
- REALTEK_USB_VENQT_READ, value,
+ memcpy(io_buf, data, size);
+ status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+ REALTEK_USB_VENQT_WRITE, value,
REALTEK_USB_VENQT_CMD_IDX, io_buf,
size, RTW_USB_CONTROL_MSG_TIMEOUT,
GFP_KERNEL);
@@ -131,7 +110,6 @@ static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
}
rtw_reset_continual_urb_error(dvobjpriv);
- memcpy(data, io_buf, size);
mutex_unlock:
mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
@@ -182,7 +160,7 @@ int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
u16 value = addr & 0xffff;
int ret;
- ret = usbctrl_vendorreq(intf, value, &val, 1, REALTEK_USB_VENQT_WRITE);
+ ret = usb_write(intf, value, &val, 1);
return RTW_STATUS_CODE(ret);
}
@@ -195,7 +173,7 @@ int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
__le16 data = cpu_to_le16(val);
int ret;
- ret = usbctrl_vendorreq(intf, value, &data, 2, REALTEK_USB_VENQT_WRITE);
+ ret = usb_write(intf, value, &data, 2);
return RTW_STATUS_CODE(ret);
}
@@ -208,7 +186,7 @@ int rtw_write32(struct adapter *adapter, u32 addr, u32 val)
__le32 data = cpu_to_le32(val);
int ret;
- ret = usbctrl_vendorreq(intf, value, &data, 4, REALTEK_USB_VENQT_WRITE);
+ ret = usb_write(intf, value, &data, 4);
return RTW_STATUS_CODE(ret);
}
@@ -223,7 +201,7 @@ int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *data)
if (length > VENDOR_CMD_MAX_DATA_LEN)
return _FAIL;
- ret = usbctrl_vendorreq(intf, value, data, length, REALTEK_USB_VENQT_WRITE);
+ ret = usb_write(intf, value, data, length);
return RTW_STATUS_CODE(ret);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 15/16] staging: r8188eu: remove shared buffer for USB requests
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (13 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 14/16] staging: r8188eu: call new usb_write() from rtw_write{8,16,32,N}() Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-21 18:18 ` [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex Fabio M. De Francesco
15 siblings, 0 replies; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M . De Francesco
From: Pavel Skripkin <paskripkin@gmail.com>
Remove the shared buffer for USB requests because it is not necessary
and use a local on stack variable since the new Core USB API does not
have the constraints of usb_control_msg().
Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 ++++++---------
drivers/staging/r8188eu/include/drv_types.h | 3 ---
drivers/staging/r8188eu/os_dep/usb_intf.c | 14 +-------------
3 files changed, 7 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 88db7488b3a2..3ede93cd68d6 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -14,7 +14,7 @@ static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
struct usb_device *udev = dvobjpriv->pusbdev;
int status;
- u8 *io_buf; /* Pointer to I/O buffer */
+ u8 io_buf[4];
mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
@@ -23,9 +23,6 @@ static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
goto mutex_unlock;
}
- /* Acquire IO memory for vendorreq */
- io_buf = dvobjpriv->usb_vendor_req_buf;
-
status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
REALTEK_USB_VENQT_READ, value,
REALTEK_USB_VENQT_CMD_IDX, io_buf,
@@ -70,14 +67,14 @@ static int usb_write(struct intf_hdl *intf, u16 value, void *data, u8 size)
struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
struct usb_device *udev = dvobjpriv->pusbdev;
int status;
- u8 *io_buf; /* Pointer to I/O buffer */
+ u8 io_buf[VENDOR_CMD_MAX_DATA_LEN];
mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
- if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx)
- return -EPERM;
-
- io_buf = dvobjpriv->usb_vendor_req_buf;
+ if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
+ status = -EPERM;
+ goto mutex_unlock;
+ }
memcpy(io_buf, data, size);
status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
diff --git a/drivers/staging/r8188eu/include/drv_types.h b/drivers/staging/r8188eu/include/drv_types.h
index c96a33c8c621..6d63429d06c6 100644
--- a/drivers/staging/r8188eu/include/drv_types.h
+++ b/drivers/staging/r8188eu/include/drv_types.h
@@ -168,9 +168,6 @@ struct dvobj_priv {
struct semaphore usb_suspend_sema;
struct mutex usb_vendor_req_mutex;
- u8 *usb_alloc_vendor_req_buf;
- u8 *usb_vendor_req_buf;
-
struct usb_interface *pusbintf;
struct usb_device *pusbdev;
diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index 11a584cbe9d8..5ab42d55207f 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -75,24 +75,12 @@ static struct rtw_usb_drv *usb_drv = &rtl8188e_usb_drv;
static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
{
- u8 rst = _SUCCESS;
-
mutex_init(&dvobj->usb_vendor_req_mutex);
-
- dvobj->usb_alloc_vendor_req_buf = kzalloc(MAX_USB_IO_CTL_SIZE, GFP_KERNEL);
- if (!dvobj->usb_alloc_vendor_req_buf) {
- DBG_88E("alloc usb_vendor_req_buf failed... /n");
- rst = _FAIL;
- goto exit;
- }
- dvobj->usb_vendor_req_buf = (u8 *)N_BYTE_ALIGMENT((size_t)(dvobj->usb_alloc_vendor_req_buf), ALIGNMENT_UNIT);
-exit:
- return rst;
+ return _SUCCESS;
}
static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
{
- kfree(dvobj->usb_alloc_vendor_req_buf);
mutex_destroy(&dvobj->usb_vendor_req_mutex);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex
2021-09-21 18:18 [PATCH v9 00/16] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
` (14 preceding siblings ...)
2021-09-21 18:18 ` [PATCH v9 15/16] staging: r8188eu: remove shared buffer for USB requests Fabio M. De Francesco
@ 2021-09-21 18:18 ` Fabio M. De Francesco
2021-09-22 13:21 ` Pavel Skripkin
15 siblings, 1 reply; 22+ messages in thread
From: Fabio M. De Francesco @ 2021-09-21 18:18 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Pavel Skripkin, linux-staging,
linux-kernel, David Laight, Dan Carpenter, Martin Kaiser
Cc: Fabio M . De Francesco
From: Pavel Skripkin <paskripkin@gmail.com>
This mutex was used to protect shared buffer for USB requests. Since
buffer was removed in previous patch we can remove this mutex as well.
Furthermore, because it was used to serialize the calls to the Core USB
API, we thoroughly tested the enabling of concurrent firing of USB requests
without the mutex and found no problems of any kind in common use cases.
Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
drivers/staging/r8188eu/hal/usb_ops_linux.c | 30 ++++++---------------
drivers/staging/r8188eu/os_dep/usb_intf.c | 22 ++-------------
2 files changed, 10 insertions(+), 42 deletions(-)
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 3ede93cd68d6..c15a75513c8c 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -16,12 +16,8 @@ static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
int status;
u8 io_buf[4];
- mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
-
- if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
- status = -EPERM;
- goto mutex_unlock;
- }
+ if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx)
+ return -EPERM;
status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
REALTEK_USB_VENQT_READ, value,
@@ -39,7 +35,7 @@ static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
* exist or is not enabled.
*/
adapt->bSurpriseRemoved = true;
- goto mutex_unlock;
+ return status;
}
if (status < 0) {
@@ -49,15 +45,12 @@ static int usb_read(struct intf_hdl *intf, u16 value, void *data, u8 size)
if (rtw_inc_and_chk_continual_urb_error(dvobjpriv))
adapt->bSurpriseRemoved = true;
- goto mutex_unlock;
+ return status;
}
rtw_reset_continual_urb_error(dvobjpriv);
memcpy(data, io_buf, size);
-mutex_unlock:
- mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
-
return status;
}
@@ -69,12 +62,8 @@ static int usb_write(struct intf_hdl *intf, u16 value, void *data, u8 size)
int status;
u8 io_buf[VENDOR_CMD_MAX_DATA_LEN];
- mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
-
- if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
- status = -EPERM;
- goto mutex_unlock;
- }
+ if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx)
+ return -EPERM;
memcpy(io_buf, data, size);
status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
@@ -93,7 +82,7 @@ static int usb_write(struct intf_hdl *intf, u16 value, void *data, u8 size)
* exist or is not enabled.
*/
adapt->bSurpriseRemoved = true;
- goto mutex_unlock;
+ return status;
}
if (status < 0) {
@@ -103,14 +92,11 @@ static int usb_write(struct intf_hdl *intf, u16 value, void *data, u8 size)
if (rtw_inc_and_chk_continual_urb_error(dvobjpriv))
adapt->bSurpriseRemoved = true;
- goto mutex_unlock;
+ return status;
}
rtw_reset_continual_urb_error(dvobjpriv);
-mutex_unlock:
- mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
-
return status;
}
diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index 5ab42d55207f..2e6e6070c304 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -73,21 +73,9 @@ static struct rtw_usb_drv rtl8188e_usb_drv = {
static struct rtw_usb_drv *usb_drv = &rtl8188e_usb_drv;
-static u8 rtw_init_intf_priv(struct dvobj_priv *dvobj)
-{
- mutex_init(&dvobj->usb_vendor_req_mutex);
- return _SUCCESS;
-}
-
-static void rtw_deinit_intf_priv(struct dvobj_priv *dvobj)
-{
- mutex_destroy(&dvobj->usb_vendor_req_mutex);
-}
-
static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)
{
int i;
- int status = _FAIL;
struct dvobj_priv *pdvobjpriv;
struct usb_host_config *phost_conf;
struct usb_config_descriptor *pconf_desc;
@@ -146,19 +134,13 @@ static struct dvobj_priv *usb_dvobj_init(struct usb_interface *usb_intf)
DBG_88E("NON USB_SPEED_HIGH\n");
}
- if (rtw_init_intf_priv(pdvobjpriv) == _FAIL)
- goto free_dvobj;
-
/* 3 misc */
sema_init(&pdvobjpriv->usb_suspend_sema, 0);
rtw_reset_continual_urb_error(pdvobjpriv);
usb_get_dev(pusbd);
- status = _SUCCESS;
-
-free_dvobj:
- if (status != _SUCCESS && pdvobjpriv) {
+ if (pdvobjpriv) {
usb_set_intfdata(usb_intf, NULL);
kfree(pdvobjpriv);
pdvobjpriv = NULL;
@@ -188,7 +170,7 @@ static void usb_dvobj_deinit(struct usb_interface *usb_intf)
usb_reset_device(interface_to_usbdev(usb_intf));
}
}
- rtw_deinit_intf_priv(dvobj);
+
kfree(dvobj);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex
2021-09-21 18:18 ` [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex Fabio M. De Francesco
@ 2021-09-22 13:21 ` Pavel Skripkin
2021-09-23 8:47 ` Pavel Skripkin
0 siblings, 1 reply; 22+ messages in thread
From: Pavel Skripkin @ 2021-09-22 13:21 UTC (permalink / raw)
To: Fabio M. De Francesco, Larry Finger, Phillip Potter,
linux-staging, linux-kernel, David Laight, Dan Carpenter,
Martin Kaiser
On 9/21/21 21:18, Fabio M. De Francesco wrote:
> From: Pavel Skripkin <paskripkin@gmail.com>
>
> This mutex was used to protect shared buffer for USB requests. Since
> buffer was removed in previous patch we can remove this mutex as well.
>
> Furthermore, because it was used to serialize the calls to the Core USB
> API, we thoroughly tested the enabling of concurrent firing of USB requests
> without the mutex and found no problems of any kind in common use cases.
>
> Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Hi, Greg!
If all is OK with previous 15 patches, please, do not take this one, it
causes problems with connection... :)
I don't understand what went wrong after v8, but anyway, this one should
not be applied for now, since it's broken
Thank you
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex
2021-09-22 13:21 ` Pavel Skripkin
@ 2021-09-23 8:47 ` Pavel Skripkin
2021-09-23 10:12 ` Pavel Skripkin
0 siblings, 1 reply; 22+ messages in thread
From: Pavel Skripkin @ 2021-09-23 8:47 UTC (permalink / raw)
To: Fabio M. De Francesco, Larry Finger, Phillip Potter,
linux-staging, linux-kernel, David Laight, Dan Carpenter,
Martin Kaiser
On 9/22/21 16:21, Pavel Skripkin wrote:
> On 9/21/21 21:18, Fabio M. De Francesco wrote:
>> From: Pavel Skripkin <paskripkin@gmail.com>
>>
>> This mutex was used to protect shared buffer for USB requests. Since
>> buffer was removed in previous patch we can remove this mutex as well.
>>
>> Furthermore, because it was used to serialize the calls to the Core USB
>> API, we thoroughly tested the enabling of concurrent firing of USB requests
>> without the mutex and found no problems of any kind in common use cases.
>>
>> Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>
> Hi, Greg!
>
> If all is OK with previous 15 patches, please, do not take this one, it
> causes problems with connection... :)
>
> I don't understand what went wrong after v8, but anyway, this one should
> not be applied for now, since it's broken
>
>
> Thank you
>
>
Just to be clear: previous 15 patches _are_ tested and do not cause any
misbehavior or bugs.
I guess, the stack buffer maybe the problem here, since it's the only
change on this side since v8. I didn't have a chance to take a closer
look, but I will do it on weekends, I hope :)
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex
2021-09-23 8:47 ` Pavel Skripkin
@ 2021-09-23 10:12 ` Pavel Skripkin
2021-09-23 11:07 ` Greg KH
0 siblings, 1 reply; 22+ messages in thread
From: Pavel Skripkin @ 2021-09-23 10:12 UTC (permalink / raw)
To: Fabio M. De Francesco, Larry Finger, Phillip Potter,
linux-staging, linux-kernel, David Laight, Dan Carpenter,
Martin Kaiser
On 9/23/21 11:47, Pavel Skripkin wrote:
> On 9/22/21 16:21, Pavel Skripkin wrote:
>> On 9/21/21 21:18, Fabio M. De Francesco wrote:
>>> From: Pavel Skripkin <paskripkin@gmail.com>
>>>
>>> This mutex was used to protect shared buffer for USB requests. Since
>>> buffer was removed in previous patch we can remove this mutex as well.
>>>
>>> Furthermore, because it was used to serialize the calls to the Core USB
>>> API, we thoroughly tested the enabling of concurrent firing of USB requests
>>> without the mutex and found no problems of any kind in common use cases.
>>>
>>> Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>>> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>>> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>>
>> Hi, Greg!
>>
>> If all is OK with previous 15 patches, please, do not take this one, it
>> causes problems with connection... :)
>>
>> I don't understand what went wrong after v8, but anyway, this one should
>> not be applied for now, since it's broken
>>
>>
>> Thank you
>>
>>
>
>
> Just to be clear: previous 15 patches _are_ tested and do not cause any
> misbehavior or bugs.
>
> I guess, the stack buffer maybe the problem here, since it's the only
> change on this side since v8. I didn't have a chance to take a closer
> look, but I will do it on weekends, I hope :)
>
Oh, I found the problem by just looking at the code with clear mind:
> -free_dvobj:
> - if (status != _SUCCESS && pdvobjpriv) {
> + if (pdvobjpriv) {
> usb_set_intfdata(usb_intf, NULL);
> kfree(pdvobjpriv);
> pdvobjpriv = NULL;
This if should be deleted completely, because we don't want to fail on
every probe :)
Sorry for noise... :(
Greg, can you take first 15 patches, if they look good and then I will
send fixed version of 16? AFAIU, you are ok with taking part of the series
Thank you
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex
2021-09-23 10:12 ` Pavel Skripkin
@ 2021-09-23 11:07 ` Greg KH
2021-09-23 11:13 ` Pavel Skripkin
0 siblings, 1 reply; 22+ messages in thread
From: Greg KH @ 2021-09-23 11:07 UTC (permalink / raw)
To: Pavel Skripkin
Cc: Fabio M. De Francesco, Larry Finger, Phillip Potter,
linux-staging, linux-kernel, David Laight, Dan Carpenter,
Martin Kaiser
On Thu, Sep 23, 2021 at 01:12:53PM +0300, Pavel Skripkin wrote:
> On 9/23/21 11:47, Pavel Skripkin wrote:
> > On 9/22/21 16:21, Pavel Skripkin wrote:
> > > On 9/21/21 21:18, Fabio M. De Francesco wrote:
> > > > From: Pavel Skripkin <paskripkin@gmail.com>
> > > >
> > > > This mutex was used to protect shared buffer for USB requests. Since
> > > > buffer was removed in previous patch we can remove this mutex as well.
> > > >
> > > > Furthermore, because it was used to serialize the calls to the Core USB
> > > > API, we thoroughly tested the enabling of concurrent firing of USB requests
> > > > without the mutex and found no problems of any kind in common use cases.
> > > >
> > > > Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> > >
> > > Hi, Greg!
> > >
> > > If all is OK with previous 15 patches, please, do not take this one, it
> > > causes problems with connection... :)
> > >
> > > I don't understand what went wrong after v8, but anyway, this one should
> > > not be applied for now, since it's broken
> > >
> > >
> > > Thank you
> > >
> > >
> >
> >
> > Just to be clear: previous 15 patches _are_ tested and do not cause any
> > misbehavior or bugs.
> >
> > I guess, the stack buffer maybe the problem here, since it's the only
> > change on this side since v8. I didn't have a chance to take a closer
> > look, but I will do it on weekends, I hope :)
> >
>
> Oh, I found the problem by just looking at the code with clear mind:
>
> > -free_dvobj:
> > - if (status != _SUCCESS && pdvobjpriv) {
> > + if (pdvobjpriv) {
> > usb_set_intfdata(usb_intf, NULL);
> > kfree(pdvobjpriv);
> > pdvobjpriv = NULL;
>
> This if should be deleted completely, because we don't want to fail on every
> probe :)
>
> Sorry for noise... :(
>
> Greg, can you take first 15 patches, if they look good and then I will send
> fixed version of 16? AFAIU, you are ok with taking part of the series
Please fix up and resend the whole series as our tools work best by
taking the whole thing.
That way I "know" you tested them all :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v9 16/16] staging: r8188eu: remove usb_vendor_req_mutex
2021-09-23 11:07 ` Greg KH
@ 2021-09-23 11:13 ` Pavel Skripkin
0 siblings, 0 replies; 22+ messages in thread
From: Pavel Skripkin @ 2021-09-23 11:13 UTC (permalink / raw)
To: Greg KH
Cc: Fabio M. De Francesco, Larry Finger, Phillip Potter,
linux-staging, linux-kernel, David Laight, Dan Carpenter,
Martin Kaiser
On 9/23/21 14:07, Greg KH wrote:
> On Thu, Sep 23, 2021 at 01:12:53PM +0300, Pavel Skripkin wrote:
>> On 9/23/21 11:47, Pavel Skripkin wrote:
>> > On 9/22/21 16:21, Pavel Skripkin wrote:
>> > > On 9/21/21 21:18, Fabio M. De Francesco wrote:
>> > > > From: Pavel Skripkin <paskripkin@gmail.com>
>> > > >
>> > > > This mutex was used to protect shared buffer for USB requests. Since
>> > > > buffer was removed in previous patch we can remove this mutex as well.
>> > > >
>> > > > Furthermore, because it was used to serialize the calls to the Core USB
>> > > > API, we thoroughly tested the enabling of concurrent firing of USB requests
>> > > > without the mutex and found no problems of any kind in common use cases.
>> > > >
>> > > > Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> > > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> > > > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
>> > >
>> > > Hi, Greg!
>> > >
>> > > If all is OK with previous 15 patches, please, do not take this one, it
>> > > causes problems with connection... :)
>> > >
>> > > I don't understand what went wrong after v8, but anyway, this one should
>> > > not be applied for now, since it's broken
>> > >
>> > >
>> > > Thank you
>> > >
>> > >
>> >
>> >
>> > Just to be clear: previous 15 patches _are_ tested and do not cause any
>> > misbehavior or bugs.
>> >
>> > I guess, the stack buffer maybe the problem here, since it's the only
>> > change on this side since v8. I didn't have a chance to take a closer
>> > look, but I will do it on weekends, I hope :)
>> >
>>
>> Oh, I found the problem by just looking at the code with clear mind:
>>
>> > -free_dvobj:
>> > - if (status != _SUCCESS && pdvobjpriv) {
>> > + if (pdvobjpriv) {
>> > usb_set_intfdata(usb_intf, NULL);
>> > kfree(pdvobjpriv);
>> > pdvobjpriv = NULL;
>>
>> This if should be deleted completely, because we don't want to fail on every
>> probe :)
>>
>> Sorry for noise... :(
>>
>> Greg, can you take first 15 patches, if they look good and then I will send
>> fixed version of 16? AFAIU, you are ok with taking part of the series
>
> Please fix up and resend the whole series as our tools work best by
> taking the whole thing.
>
> That way I "know" you tested them all :)
It is true, but there was small error on testing side from me this time.
I am testing with qemu and I forgot to copy new driver to qemu's shared
folder (it was busy day or I was already asleep, idk). It resulted to
testing old version of the driver. Next day I've tested new changes on
top of this series and found the problem :)
I am sorry for kind of lying, i've already fixed it locally, so it won't
happen again
With regards,
Pavel Skripkin
^ permalink raw reply [flat|nested] 22+ messages in thread