staging: r8188eu: call new usb_write() from rtw_write{8,16,32,N}()

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>
Link: https://lore.kernel.org/r/20210924122705.3781-15-fmdefrancesco@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index a3dcd366c00cad30f05ba9d1d89010dfff9f1729..88db7488b3a2c8819f0c0541c35408c415475479 100644 (file)
--- 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);
 }