[PATCH 01/10] staging: r8188eu: remove unnecessary cast

[PATCH 01/10] staging: r8188eu: remove unnecessary cast

[Martin Kaiser]

name is a const char * by default. This type should be ok for r8188eu.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index e002070f7fba..72556ac10d7d 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -61,7 +61,7 @@ struct rtw_usb_drv {
 };
 
 static struct rtw_usb_drv rtl8188e_usb_drv = {
-	.usbdrv.name = (char *)"r8188eu",
+	.usbdrv.name = "r8188eu",
 	.usbdrv.probe = rtw_drv_init,
 	.usbdrv.disconnect = rtw_dev_remove,
 	.usbdrv.id_table = rtw_usb_id_tbl,
-- 
2.20.1

[PATCH 02/10] staging: r8188eu: remove unused define

[Martin Kaiser]

_HCI_OPS_OS_C_ is not used in the r8188eu driver. Remove it.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 953fa05dc30c..a11a0597e515 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright(c) 2007 - 2011 Realtek Corporation. */
 
-#define _HCI_OPS_OS_C_
-
 #include "../include/osdep_service.h"
 #include "../include/drv_types.h"
 #include "../include/osdep_intf.h"
-- 
2.20.1

[PATCH 03/10] staging: rtl8188eu: use actual request type as parameter

[Martin Kaiser]

At the moment, usbctrl_vendorreq's requesttype parameter must be set to
1 for reading and 0 for writing. It's then converted to the actual
bmRequestType for the USB control request. We can simplify the code and
avoid this conversion if the caller passes the actual bmRequestType.

This patch is an adaptation of commit 788fde031027 ("staging: rtl8188eu:
use actual request type as parameter") for the new r8188eu driver.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 46 ++++++---------------
 1 file changed, 12 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index a11a0597e515..dccb9fd34777 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -15,7 +15,6 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 	struct usb_device *udev = dvobjpriv->pusbdev;
 	unsigned int pipe;
 	int status = 0;
-	u8 reqtype;
 	u8 *pIo_buf;
 	int vendorreq_times = 0;
 
@@ -44,26 +43,24 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
 		memset(pIo_buf, 0, len);
 
-		if (requesttype == 0x01) {
+		if (requesttype == REALTEK_USB_VENQT_READ) {
 			pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
-			reqtype =  REALTEK_USB_VENQT_READ;
 		} else {
 			pipe = usb_sndctrlpipe(udev, 0);/* write_out */
-			reqtype =  REALTEK_USB_VENQT_WRITE;
 			memcpy(pIo_buf, pdata, len);
 		}
 
 		status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
-					 reqtype, value, REALTEK_USB_VENQT_CMD_IDX,
+					 requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
 					 pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
 
 		if (status == len) {   /*  Success this control transfer. */
 			rtw_reset_continual_urb_error(dvobjpriv);
-			if (requesttype == 0x01)
+			if (requesttype == REALTEK_USB_VENQT_READ)
 				memcpy(pdata, pIo_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 == 0x01) ? "read" : "write",
+				value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write",
 				len, status, *(u32 *)pdata, vendorreq_times);
 
 			if (status < 0) {
@@ -75,7 +72,7 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 				}
 			} else { /*  status != len && status >= 0 */
 				if (status > 0) {
-					if (requesttype == 0x01) {
+					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);
 					}
@@ -101,19 +98,16 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 
 static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
 {
-	u8 requesttype;
 	u16 wvalue;
 	u16 len;
 	u8 data = 0;
 
 
 
-	requesttype = 0x01;/* read_in */
-
 	wvalue = (u16)(addr & 0x0000ffff);
 	len = 1;
 
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
+	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
 
 
 
@@ -123,57 +117,49 @@ static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
 
 static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr)
 {
-	u8 requesttype;
 	u16 wvalue;
 	u16 len;
 	__le32 data;
 
-	requesttype = 0x01;/* read_in */
 	wvalue = (u16)(addr & 0x0000ffff);
 	len = 2;
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
+	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
 
 	return (u16)(le32_to_cpu(data) & 0xffff);
 }
 
 static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
 {
-	u8 requesttype;
 	u16 wvalue;
 	u16 len;
 	__le32 data;
 
-	requesttype = 0x01;/* read_in */
-
 	wvalue = (u16)(addr & 0x0000ffff);
 	len = 4;
 
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
+	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
 
 	return le32_to_cpu(data);
 }
 
 static int usb_write8(struct intf_hdl *pintfhdl, u32 addr, u8 val)
 {
-	u8 requesttype;
 	u16 wvalue;
 	u16 len;
 	u8 data;
 	int ret;
 
 
-	requesttype = 0x00;/* write_out */
 	wvalue = (u16)(addr & 0x0000ffff);
 	len = 1;
 	data = val;
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
 
 	return ret;
 }
 
 static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
 {
-	u8 requesttype;
 	u16 wvalue;
 	u16 len;
 	__le32 data;
@@ -181,14 +167,12 @@ static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
 
 
 
-	requesttype = 0x00;/* write_out */
-
 	wvalue = (u16)(addr & 0x0000ffff);
 	len = 2;
 
 	data = cpu_to_le32(val & 0x0000ffff);
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
 
 
 
@@ -197,7 +181,6 @@ static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
 
 static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
 {
-	u8 requesttype;
 	u16 wvalue;
 	u16 len;
 	__le32 data;
@@ -205,13 +188,11 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
 
 
 
-	requesttype = 0x00;/* write_out */
-
 	wvalue = (u16)(addr & 0x0000ffff);
 	len = 4;
 	data = cpu_to_le32(val);
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
 
 
 
@@ -220,7 +201,6 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
 
 static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
 {
-	u8 requesttype;
 	u16 wvalue;
 	u16 len;
 	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
@@ -228,13 +208,11 @@ static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata
 
 
 
-	requesttype = 0x00;/* write_out */
-
 	wvalue = (u16)(addr & 0x0000ffff);
 	len = length;
 	memcpy(buf, pdata, len);
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, requesttype);
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, REALTEK_USB_VENQT_WRITE);
 
 	return ret;
 }
-- 
2.20.1

[PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines

[Martin Kaiser]

Replace the numeric values with USB constants to make their
meaning clearer.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/include/usb_ops.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/r8188eu/include/usb_ops.h b/drivers/staging/r8188eu/include/usb_ops.h
index 5d290199e37c..b6a1cd536adf 100644
--- a/drivers/staging/r8188eu/include/usb_ops.h
+++ b/drivers/staging/r8188eu/include/usb_ops.h
@@ -8,8 +8,8 @@
 #include "drv_types.h"
 #include "osdep_intf.h"
 
-#define REALTEK_USB_VENQT_READ		0xC0
-#define REALTEK_USB_VENQT_WRITE		0x40
+#define REALTEK_USB_VENQT_READ		(USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE)
+#define REALTEK_USB_VENQT_WRITE		(USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE)
 #define REALTEK_USB_VENQT_CMD_REQ	0x05
 #define REALTEK_USB_VENQT_CMD_IDX	0x00
 
-- 
2.20.1

[PATCH 05/10] staging: r8188eu: remove an unused enum

[Martin Kaiser]

The VENDOR_READ and VENDOR_WRITE defines are not used.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/include/usb_ops.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/r8188eu/include/usb_ops.h b/drivers/staging/r8188eu/include/usb_ops.h
index b6a1cd536adf..c53cc54b6b87 100644
--- a/drivers/staging/r8188eu/include/usb_ops.h
+++ b/drivers/staging/r8188eu/include/usb_ops.h
@@ -13,10 +13,6 @@
 #define REALTEK_USB_VENQT_CMD_REQ	0x05
 #define REALTEK_USB_VENQT_CMD_IDX	0x00
 
-enum {
-	VENDOR_WRITE = 0x00,
-	VENDOR_READ = 0x01,
-};
 #define ALIGNMENT_UNIT			16
 #define MAX_VENDOR_REQ_CMD_SIZE	254	/* 8188cu SIE Support */
 #define MAX_USB_IO_CTL_SIZE	(MAX_VENDOR_REQ_CMD_SIZE + ALIGNMENT_UNIT)
-- 
2.20.1

[PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions

[Martin Kaiser]

Remove unnecessary variables, summarize declarations and assignments.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 30 +++++----------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index dccb9fd34777..cb969a200681 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -98,46 +98,30 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 
 static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
 {
-	u16 wvalue;
-	u16 len;
-	u8 data = 0;
-
-
-
-	wvalue = (u16)(addr & 0x0000ffff);
-	len = 1;
-
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
-
+	u16 wvalue = (u16)(addr & 0x0000ffff);
+	u8 data;
 
+	usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ);
 
 	return data;
-
 }
 
 static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr)
 {
-	u16 wvalue;
-	u16 len;
+	u16 wvalue = (u16)(addr & 0x0000ffff);
 	__le32 data;
 
-	wvalue = (u16)(addr & 0x0000ffff);
-	len = 2;
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
+	usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_READ);
 
 	return (u16)(le32_to_cpu(data) & 0xffff);
 }
 
 static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
 {
-	u16 wvalue;
-	u16 len;
+	u16 wvalue = (u16)(addr & 0x0000ffff);
 	__le32 data;
 
-	wvalue = (u16)(addr & 0x0000ffff);
-	len = 4;
-
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
+	usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_READ);
 
 	return le32_to_cpu(data);
 }
-- 
2.20.1

[PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions

[Martin Kaiser]

Remove unnecessary variables, summarize declarations and assignments.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 50 ++++-----------------
 1 file changed, 8 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index cb969a200681..e01f1ac19596 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -128,59 +128,25 @@ static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
 
 static int usb_write8(struct intf_hdl *pintfhdl, u32 addr, u8 val)
 {
-	u16 wvalue;
-	u16 len;
-	u8 data;
-	int ret;
-
-
-	wvalue = (u16)(addr & 0x0000ffff);
-	len = 1;
-	data = val;
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
+	u16 wvalue = (u16)(addr & 0x0000ffff);
 
-	return ret;
+	return usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
 }
 
 static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
 {
-	u16 wvalue;
-	u16 len;
-	__le32 data;
-	int ret;
-
-
-
-	wvalue = (u16)(addr & 0x0000ffff);
-	len = 2;
-
-	data = cpu_to_le32(val & 0x0000ffff);
-
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
-
-
+	u16 wvalue = (u16)(addr & 0x0000ffff);
+	__le32 data = cpu_to_le32(val & 0x0000ffff);
 
-	return ret;
+	return usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
 }
 
 static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
 {
-	u16 wvalue;
-	u16 len;
-	__le32 data;
-	int ret;
-
-
-
-	wvalue = (u16)(addr & 0x0000ffff);
-	len = 4;
-	data = cpu_to_le32(val);
-
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
-
-
+	u16 wvalue = (u16)(addr & 0x0000ffff);
+	__le32 data = cpu_to_le32(val);
 
-	return ret;
+	return usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE);
 }
 
 static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
-- 
2.20.1

[PATCH 08/10] staging: r8188eu: clean up the usb_writeN

[Martin Kaiser]

Remove unnecessary variables, check the length.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index e01f1ac19596..5408383ccec3 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -151,20 +151,15 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
 
 static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
 {
-	u16 wvalue;
-	u16 len;
+	u16 wvalue = (u16)(addr & 0x0000ffff);
 	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
-	int ret;
-
 
+	if (length > VENDOR_CMD_MAX_DATA_LEN)
+		return -EINVAL;
 
-	wvalue = (u16)(addr & 0x0000ffff);
-	len = length;
-	memcpy(buf, pdata, len);
+	memcpy(buf, pdata, length);
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, REALTEK_USB_VENQT_WRITE);
-
-	return ret;
+	return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
 }
 
 static void interrupt_handler_8188eu(struct adapter *adapt, u16 pkt_len, u8 *pbuf)
-- 
2.20.1

[PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops

[Martin Kaiser]

Remove function pointers which are not used by the r8188eu driver.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/include/rtw_io.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index f1b3074fa075..4b41c7b03972 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -100,13 +100,10 @@ struct _io_ops {
 			  u8 *pmem);
 	void (*_write_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
 			   u8 *pmem);
-	void (*_sync_irp_protocol_rw)(struct io_queue *pio_q);
-	u32 (*_read_interrupt)(struct intf_hdl *pintfhdl, u32 addr);
 	u32 (*_read_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
 			  u8 *pmem);
 	u32 (*_write_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
 			   u8 *pmem);
-	u32 (*_write_scsi)(struct intf_hdl *pintfhdl,u32 cnt, u8 *pmem);
 	void (*_read_port_cancel)(struct intf_hdl *pintfhdl);
 	void (*_write_port_cancel)(struct intf_hdl *pintfhdl);
 };
-- 
2.20.1

[PATCH 10/10] staging: r8188eu: set pipe only once

[Martin Kaiser]

Set the pipe for reading or writing in usbctrl_vendorreq only once.
There's no need to set it again for every retry.

This patch is an adaptation of commit 889ed8b5e374 ("staging: rtl8188eu:
set pipe only once") for the new r8188eu driver.

Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 5408383ccec3..5a55ee38d7b8 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -40,15 +40,16 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 		goto release_mutex;
 	}
 
-	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
-		memset(pIo_buf, 0, len);
+	if (requesttype == REALTEK_USB_VENQT_READ)
+		pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
+	else
+		pipe = usb_sndctrlpipe(udev, 0);/* write_out */
 
-		if (requesttype == REALTEK_USB_VENQT_READ) {
-			pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
-		} else {
-			pipe = usb_sndctrlpipe(udev, 0);/* write_out */
+	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
+		if (requesttype == REALTEK_USB_VENQT_READ)
+			memset(pIo_buf, 0, len);
+		else
 			memcpy(pIo_buf, pdata, len);
-		}
 
 		status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
 					 requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
-- 
2.20.1

Re: [PATCH 01/10] staging: r8188eu: remove unnecessary cast

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:
>
> name is a const char * by default. This type should be ok for r8188eu.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index e002070f7fba..72556ac10d7d 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -61,7 +61,7 @@ struct rtw_usb_drv {
>  };
>
>  static struct rtw_usb_drv rtl8188e_usb_drv = {
> -       .usbdrv.name = (char *)"r8188eu",
> +       .usbdrv.name = "r8188eu",
>         .usbdrv.probe = rtw_drv_init,
>         .usbdrv.disconnect = rtw_dev_remove,
>         .usbdrv.id_table = rtw_usb_id_tbl,
> --
> 2.20.1
>

Looks ok to me, thanks. I would consider using a cover letter style
[PATCH 00/10] style approach as an addition in future though, just my
personal opinion.

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 02/10] staging: r8188eu: remove unused define

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:
>
> _HCI_OPS_OS_C_ is not used in the r8188eu driver. Remove it.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/hal/usb_ops_linux.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index 953fa05dc30c..a11a0597e515 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -1,8 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright(c) 2007 - 2011 Realtek Corporation. */
>
> -#define _HCI_OPS_OS_C_
> -
>  #include "../include/osdep_service.h"
>  #include "../include/drv_types.h"
>  #include "../include/osdep_intf.h"
> --
> 2.20.1
>

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 05/10] staging: r8188eu: remove an unused enum

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:
>
> The VENDOR_READ and VENDOR_WRITE defines are not used.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/include/usb_ops.h | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/include/usb_ops.h b/drivers/staging/r8188eu/include/usb_ops.h
> index b6a1cd536adf..c53cc54b6b87 100644
> --- a/drivers/staging/r8188eu/include/usb_ops.h
> +++ b/drivers/staging/r8188eu/include/usb_ops.h
> @@ -13,10 +13,6 @@
>  #define REALTEK_USB_VENQT_CMD_REQ      0x05
>  #define REALTEK_USB_VENQT_CMD_IDX      0x00
>
> -enum {
> -       VENDOR_WRITE = 0x00,
> -       VENDOR_READ = 0x01,
> -};
>  #define ALIGNMENT_UNIT                 16
>  #define MAX_VENDOR_REQ_CMD_SIZE        254     /* 8188cu SIE Support */
>  #define MAX_USB_IO_CTL_SIZE    (MAX_VENDOR_REQ_CMD_SIZE + ALIGNMENT_UNIT)
> --
> 2.20.1
>

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:50, Martin Kaiser <martin@kaiser.cx> wrote:
>
> Remove function pointers which are not used by the r8188eu driver.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/include/rtw_io.h | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
> index f1b3074fa075..4b41c7b03972 100644
> --- a/drivers/staging/r8188eu/include/rtw_io.h
> +++ b/drivers/staging/r8188eu/include/rtw_io.h
> @@ -100,13 +100,10 @@ struct _io_ops {
>                           u8 *pmem);
>         void (*_write_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
>                            u8 *pmem);
> -       void (*_sync_irp_protocol_rw)(struct io_queue *pio_q);
> -       u32 (*_read_interrupt)(struct intf_hdl *pintfhdl, u32 addr);
>         u32 (*_read_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
>                           u8 *pmem);
>         u32 (*_write_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
>                            u8 *pmem);
> -       u32 (*_write_scsi)(struct intf_hdl *pintfhdl,u32 cnt, u8 *pmem);
>         void (*_read_port_cancel)(struct intf_hdl *pintfhdl);
>         void (*_write_port_cancel)(struct intf_hdl *pintfhdl);
>  };
> --
> 2.20.1
>

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:
>
> At the moment, usbctrl_vendorreq's requesttype parameter must be set to
> 1 for reading and 0 for writing. It's then converted to the actual
> bmRequestType for the USB control request. We can simplify the code and
> avoid this conversion if the caller passes the actual bmRequestType.
>
> This patch is an adaptation of commit 788fde031027 ("staging: rtl8188eu:
> use actual request type as parameter") for the new r8188eu driver.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/hal/usb_ops_linux.c | 46 ++++++---------------
>  1 file changed, 12 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index a11a0597e515..dccb9fd34777 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -15,7 +15,6 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>         struct usb_device *udev = dvobjpriv->pusbdev;
>         unsigned int pipe;
>         int status = 0;
> -       u8 reqtype;
>         u8 *pIo_buf;
>         int vendorreq_times = 0;
>
> @@ -44,26 +43,24 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>         while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
>                 memset(pIo_buf, 0, len);
>
> -               if (requesttype == 0x01) {
> +               if (requesttype == REALTEK_USB_VENQT_READ) {
>                         pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
> -                       reqtype =  REALTEK_USB_VENQT_READ;
>                 } else {
>                         pipe = usb_sndctrlpipe(udev, 0);/* write_out */
> -                       reqtype =  REALTEK_USB_VENQT_WRITE;
>                         memcpy(pIo_buf, pdata, len);
>                 }
>
>                 status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
> -                                        reqtype, value, REALTEK_USB_VENQT_CMD_IDX,
> +                                        requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
>                                          pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
>
>                 if (status == len) {   /*  Success this control transfer. */
>                         rtw_reset_continual_urb_error(dvobjpriv);
> -                       if (requesttype == 0x01)
> +                       if (requesttype == REALTEK_USB_VENQT_READ)
>                                 memcpy(pdata, pIo_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 == 0x01) ? "read" : "write",
> +                               value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write",
>                                 len, status, *(u32 *)pdata, vendorreq_times);
>
>                         if (status < 0) {
> @@ -75,7 +72,7 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>                                 }
>                         } else { /*  status != len && status >= 0 */
>                                 if (status > 0) {
> -                                       if (requesttype == 0x01) {
> +                                       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);
>                                         }
> @@ -101,19 +98,16 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>
>  static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
>  {
> -       u8 requesttype;
>         u16 wvalue;
>         u16 len;
>         u8 data = 0;
>
>
>
> -       requesttype = 0x01;/* read_in */
> -
>         wvalue = (u16)(addr & 0x0000ffff);
>         len = 1;
>
> -       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
>
>
>
> @@ -123,57 +117,49 @@ static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
>
>  static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr)
>  {
> -       u8 requesttype;
>         u16 wvalue;
>         u16 len;
>         __le32 data;
>
> -       requesttype = 0x01;/* read_in */
>         wvalue = (u16)(addr & 0x0000ffff);
>         len = 2;
> -       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
>
>         return (u16)(le32_to_cpu(data) & 0xffff);
>  }
>
>  static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
>  {
> -       u8 requesttype;
>         u16 wvalue;
>         u16 len;
>         __le32 data;
>
> -       requesttype = 0x01;/* read_in */
> -
>         wvalue = (u16)(addr & 0x0000ffff);
>         len = 4;
>
> -       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
>
>         return le32_to_cpu(data);
>  }
>
>  static int usb_write8(struct intf_hdl *pintfhdl, u32 addr, u8 val)
>  {
> -       u8 requesttype;
>         u16 wvalue;
>         u16 len;
>         u8 data;
>         int ret;
>
>
> -       requesttype = 0x00;/* write_out */
>         wvalue = (u16)(addr & 0x0000ffff);
>         len = 1;
>         data = val;
> -       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
>
>         return ret;
>  }
>
>  static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
>  {
> -       u8 requesttype;
>         u16 wvalue;
>         u16 len;
>         __le32 data;
> @@ -181,14 +167,12 @@ static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
>
>
>
> -       requesttype = 0x00;/* write_out */
> -
>         wvalue = (u16)(addr & 0x0000ffff);
>         len = 2;
>
>         data = cpu_to_le32(val & 0x0000ffff);
>
> -       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
>
>
>
> @@ -197,7 +181,6 @@ static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
>
>  static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>  {
> -       u8 requesttype;
>         u16 wvalue;
>         u16 len;
>         __le32 data;
> @@ -205,13 +188,11 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>
>
>
> -       requesttype = 0x00;/* write_out */
> -
>         wvalue = (u16)(addr & 0x0000ffff);
>         len = 4;
>         data = cpu_to_le32(val);
>
> -       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
>
>
>
> @@ -220,7 +201,6 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>
>  static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
>  {
> -       u8 requesttype;
>         u16 wvalue;
>         u16 len;
>         u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
> @@ -228,13 +208,11 @@ static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata
>
>
>
> -       requesttype = 0x00;/* write_out */
> -
>         wvalue = (u16)(addr & 0x0000ffff);
>         len = length;
>         memcpy(buf, pdata, len);
>
> -       ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, requesttype);
> +       ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, REALTEK_USB_VENQT_WRITE);
>
>         return ret;
>  }
> --
> 2.20.1
>

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:
>
> Replace the numeric values with USB constants to make their
> meaning clearer.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/include/usb_ops.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/include/usb_ops.h b/drivers/staging/r8188eu/include/usb_ops.h
> index 5d290199e37c..b6a1cd536adf 100644
> --- a/drivers/staging/r8188eu/include/usb_ops.h
> +++ b/drivers/staging/r8188eu/include/usb_ops.h
> @@ -8,8 +8,8 @@
>  #include "drv_types.h"
>  #include "osdep_intf.h"
>
> -#define REALTEK_USB_VENQT_READ         0xC0
> -#define REALTEK_USB_VENQT_WRITE                0x40
> +#define REALTEK_USB_VENQT_READ         (USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE)
> +#define REALTEK_USB_VENQT_WRITE                (USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE)
>  #define REALTEK_USB_VENQT_CMD_REQ      0x05
>  #define REALTEK_USB_VENQT_CMD_IDX      0x00
>
> --
> 2.20.1
>

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:
>
> Remove unnecessary variables, summarize declarations and assignments.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/hal/usb_ops_linux.c | 30 +++++----------------
>  1 file changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index dccb9fd34777..cb969a200681 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -98,46 +98,30 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>
>  static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
>  {
> -       u16 wvalue;
> -       u16 len;
> -       u8 data = 0;
> -
> -
> -
> -       wvalue = (u16)(addr & 0x0000ffff);
> -       len = 1;
> -
> -       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
> -
> +       u16 wvalue = (u16)(addr & 0x0000ffff);
> +       u8 data;
>
> +       usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ);
>
>         return data;
> -
>  }
>
>  static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr)
>  {
> -       u16 wvalue;
> -       u16 len;
> +       u16 wvalue = (u16)(addr & 0x0000ffff);
>         __le32 data;
>
> -       wvalue = (u16)(addr & 0x0000ffff);
> -       len = 2;
> -       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
> +       usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_READ);
>
>         return (u16)(le32_to_cpu(data) & 0xffff);
>  }
>
>  static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
>  {
> -       u16 wvalue;
> -       u16 len;
> +       u16 wvalue = (u16)(addr & 0x0000ffff);
>         __le32 data;
>
> -       wvalue = (u16)(addr & 0x0000ffff);
> -       len = 4;
> -
> -       usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
> +       usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_READ);
>
>         return le32_to_cpu(data);
>  }
> --
> 2.20.1
>

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:
>
> Remove unnecessary variables, summarize declarations and assignments.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/hal/usb_ops_linux.c | 50 ++++-----------------
>  1 file changed, 8 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index cb969a200681..e01f1ac19596 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -128,59 +128,25 @@ static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
>
>  static int usb_write8(struct intf_hdl *pintfhdl, u32 addr, u8 val)
>  {
> -       u16 wvalue;
> -       u16 len;
> -       u8 data;
> -       int ret;
> -
> -
> -       wvalue = (u16)(addr & 0x0000ffff);
> -       len = 1;
> -       data = val;
> -       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
> +       u16 wvalue = (u16)(addr & 0x0000ffff);
>
> -       return ret;
> +       return usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
>  }
>
>  static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
>  {
> -       u16 wvalue;
> -       u16 len;
> -       __le32 data;
> -       int ret;
> -
> -
> -
> -       wvalue = (u16)(addr & 0x0000ffff);
> -       len = 2;
> -
> -       data = cpu_to_le32(val & 0x0000ffff);
> -
> -       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
> -
> -
> +       u16 wvalue = (u16)(addr & 0x0000ffff);
> +       __le32 data = cpu_to_le32(val & 0x0000ffff);
>
> -       return ret;
> +       return usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
>  }
>
>  static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>  {
> -       u16 wvalue;
> -       u16 len;
> -       __le32 data;
> -       int ret;
> -
> -
> -
> -       wvalue = (u16)(addr & 0x0000ffff);
> -       len = 4;
> -       data = cpu_to_le32(val);
> -
> -       ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
> -
> -
> +       u16 wvalue = (u16)(addr & 0x0000ffff);
> +       __le32 data = cpu_to_le32(val);
>
> -       return ret;
> +       return usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE);
>  }
>
>  static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
> --
> 2.20.1
>
Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 08/10] staging: r8188eu: clean up the usb_writeN

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:50, Martin Kaiser <martin@kaiser.cx> wrote:
>
> Remove unnecessary variables, check the length.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index e01f1ac19596..5408383ccec3 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -151,20 +151,15 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>
>  static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
>  {
> -       u16 wvalue;
> -       u16 len;
> +       u16 wvalue = (u16)(addr & 0x0000ffff);
>         u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
> -       int ret;
> -
>
> +       if (length > VENDOR_CMD_MAX_DATA_LEN)
> +               return -EINVAL;
>
> -       wvalue = (u16)(addr & 0x0000ffff);
> -       len = length;
> -       memcpy(buf, pdata, len);
> +       memcpy(buf, pdata, length);
>
> -       ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, REALTEK_USB_VENQT_WRITE);
> -
> -       return ret;
> +       return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
>  }
>
>  static void interrupt_handler_8188eu(struct adapter *adapt, u16 pkt_len, u8 *pbuf)
> --
> 2.20.1
>

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 10/10] staging: r8188eu: set pipe only once

[Phillip Potter]

On Sat, 21 Aug 2021 at 17:50, Martin Kaiser <martin@kaiser.cx> wrote:
>
> Set the pipe for reading or writing in usbctrl_vendorreq only once.
> There's no need to set it again for every retry.
>
> This patch is an adaptation of commit 889ed8b5e374 ("staging: rtl8188eu:
> set pipe only once") for the new r8188eu driver.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>  drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index 5408383ccec3..5a55ee38d7b8 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -40,15 +40,16 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>                 goto release_mutex;
>         }
>
> -       while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
> -               memset(pIo_buf, 0, len);
> +       if (requesttype == REALTEK_USB_VENQT_READ)
> +               pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
> +       else
> +               pipe = usb_sndctrlpipe(udev, 0);/* write_out */
>
> -               if (requesttype == REALTEK_USB_VENQT_READ) {
> -                       pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
> -               } else {
> -                       pipe = usb_sndctrlpipe(udev, 0);/* write_out */
> +       while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
> +               if (requesttype == REALTEK_USB_VENQT_READ)
> +                       memset(pIo_buf, 0, len);
> +               else
>                         memcpy(pIo_buf, pdata, len);
> -               }
>
>                 status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
>                                          requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
> --
> 2.20.1
>

Acked-by: Phillip Potter <phil@philpotter.co.uk>

Regards,
Phil

Re: [PATCH 01/10] staging: r8188eu: remove unnecessary cast

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> name is a const char * by default. This type should be ok for r8188eu.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index e002070f7fba..72556ac10d7d 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -61,7 +61,7 @@ struct rtw_usb_drv {
>   };
>   
>   static struct rtw_usb_drv rtl8188e_usb_drv = {
> -	.usbdrv.name = (char *)"r8188eu",
> +	.usbdrv.name = "r8188eu",
>   	.usbdrv.probe = rtw_drv_init,
>   	.usbdrv.disconnect = rtw_dev_remove,
>   	.usbdrv.id_table = rtw_usb_id_tbl,
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 02/10] staging: r8188eu: remove unused define

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> _HCI_OPS_OS_C_ is not used in the r8188eu driver. Remove it.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/hal/usb_ops_linux.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index 953fa05dc30c..a11a0597e515 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -1,8 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0
>   /* Copyright(c) 2007 - 2011 Realtek Corporation. */
>   
> -#define _HCI_OPS_OS_C_
> -
>   #include "../include/osdep_service.h"
>   #include "../include/drv_types.h"
>   #include "../include/osdep_intf.h"
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> At the moment, usbctrl_vendorreq's requesttype parameter must be set to
> 1 for reading and 0 for writing. It's then converted to the actual
> bmRequestType for the USB control request. We can simplify the code and
> avoid this conversion if the caller passes the actual bmRequestType.
> 
> This patch is an adaptation of commit 788fde031027 ("staging: rtl8188eu:
> use actual request type as parameter") for the new r8188eu driver.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/hal/usb_ops_linux.c | 46 ++++++---------------
>   1 file changed, 12 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index a11a0597e515..dccb9fd34777 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -15,7 +15,6 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>   	struct usb_device *udev = dvobjpriv->pusbdev;
>   	unsigned int pipe;
>   	int status = 0;
> -	u8 reqtype;
>   	u8 *pIo_buf;
>   	int vendorreq_times = 0;
>   
> @@ -44,26 +43,24 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>   	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
>   		memset(pIo_buf, 0, len);
>   
> -		if (requesttype == 0x01) {
> +		if (requesttype == REALTEK_USB_VENQT_READ) {
>   			pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
> -			reqtype =  REALTEK_USB_VENQT_READ;
>   		} else {
>   			pipe = usb_sndctrlpipe(udev, 0);/* write_out */
> -			reqtype =  REALTEK_USB_VENQT_WRITE;
>   			memcpy(pIo_buf, pdata, len);
>   		}
>   
>   		status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
> -					 reqtype, value, REALTEK_USB_VENQT_CMD_IDX,
> +					 requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
>   					 pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
>   
>   		if (status == len) {   /*  Success this control transfer. */
>   			rtw_reset_continual_urb_error(dvobjpriv);
> -			if (requesttype == 0x01)
> +			if (requesttype == REALTEK_USB_VENQT_READ)
>   				memcpy(pdata, pIo_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 == 0x01) ? "read" : "write",
> +				value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write",
>   				len, status, *(u32 *)pdata, vendorreq_times);
>   
>   			if (status < 0) {
> @@ -75,7 +72,7 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>   				}
>   			} else { /*  status != len && status >= 0 */
>   				if (status > 0) {
> -					if (requesttype == 0x01) {
> +					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);
>   					}
> @@ -101,19 +98,16 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>   
>   static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
>   {
> -	u8 requesttype;
>   	u16 wvalue;
>   	u16 len;
>   	u8 data = 0;
>   
>   
>   
> -	requesttype = 0x01;/* read_in */
> -
>   	wvalue = (u16)(addr & 0x0000ffff);
>   	len = 1;
>   
> -	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
>   
>   
>   
> @@ -123,57 +117,49 @@ static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
>   
>   static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr)
>   {
> -	u8 requesttype;
>   	u16 wvalue;
>   	u16 len;
>   	__le32 data;
>   
> -	requesttype = 0x01;/* read_in */
>   	wvalue = (u16)(addr & 0x0000ffff);
>   	len = 2;
> -	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
>   
>   	return (u16)(le32_to_cpu(data) & 0xffff);
>   }
>   
>   static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
>   {
> -	u8 requesttype;
>   	u16 wvalue;
>   	u16 len;
>   	__le32 data;
>   
> -	requesttype = 0x01;/* read_in */
> -
>   	wvalue = (u16)(addr & 0x0000ffff);
>   	len = 4;
>   
> -	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
>   
>   	return le32_to_cpu(data);
>   }
>   
>   static int usb_write8(struct intf_hdl *pintfhdl, u32 addr, u8 val)
>   {
> -	u8 requesttype;
>   	u16 wvalue;
>   	u16 len;
>   	u8 data;
>   	int ret;
>   
>   
> -	requesttype = 0x00;/* write_out */
>   	wvalue = (u16)(addr & 0x0000ffff);
>   	len = 1;
>   	data = val;
> -	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
>   
>   	return ret;
>   }
>   
>   static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
>   {
> -	u8 requesttype;
>   	u16 wvalue;
>   	u16 len;
>   	__le32 data;
> @@ -181,14 +167,12 @@ static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
>   
>   
>   
> -	requesttype = 0x00;/* write_out */
> -
>   	wvalue = (u16)(addr & 0x0000ffff);
>   	len = 2;
>   
>   	data = cpu_to_le32(val & 0x0000ffff);
>   
> -	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
>   
>   
>   
> @@ -197,7 +181,6 @@ static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
>   
>   static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>   {
> -	u8 requesttype;
>   	u16 wvalue;
>   	u16 len;
>   	__le32 data;
> @@ -205,13 +188,11 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>   
>   
>   
> -	requesttype = 0x00;/* write_out */
> -
>   	wvalue = (u16)(addr & 0x0000ffff);
>   	len = 4;
>   	data = cpu_to_le32(val);
>   
> -	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, requesttype);
> +	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
>   
>   
>   
> @@ -220,7 +201,6 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>   
>   static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
>   {
> -	u8 requesttype;
>   	u16 wvalue;
>   	u16 len;
>   	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
> @@ -228,13 +208,11 @@ static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata
>   
>   
>   
> -	requesttype = 0x00;/* write_out */
> -
>   	wvalue = (u16)(addr & 0x0000ffff);
>   	len = length;
>   	memcpy(buf, pdata, len);
>   
> -	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, requesttype);
> +	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, REALTEK_USB_VENQT_WRITE);
>   
>   	return ret;
>   }
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> Replace the numeric values with USB constants to make their
> meaning clearer.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/include/usb_ops.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/usb_ops.h b/drivers/staging/r8188eu/include/usb_ops.h
> index 5d290199e37c..b6a1cd536adf 100644
> --- a/drivers/staging/r8188eu/include/usb_ops.h
> +++ b/drivers/staging/r8188eu/include/usb_ops.h
> @@ -8,8 +8,8 @@
>   #include "drv_types.h"
>   #include "osdep_intf.h"
>   
> -#define REALTEK_USB_VENQT_READ		0xC0
> -#define REALTEK_USB_VENQT_WRITE		0x40
> +#define REALTEK_USB_VENQT_READ		(USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE)
> +#define REALTEK_USB_VENQT_WRITE		(USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE)
>   #define REALTEK_USB_VENQT_CMD_REQ	0x05
>   #define REALTEK_USB_VENQT_CMD_IDX	0x00
>   
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 05/10] staging: r8188eu: remove an unused enum

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> The VENDOR_READ and VENDOR_WRITE defines are not used.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/include/usb_ops.h | 4 ----
>   1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/usb_ops.h b/drivers/staging/r8188eu/include/usb_ops.h
> index b6a1cd536adf..c53cc54b6b87 100644
> --- a/drivers/staging/r8188eu/include/usb_ops.h
> +++ b/drivers/staging/r8188eu/include/usb_ops.h
> @@ -13,10 +13,6 @@
>   #define REALTEK_USB_VENQT_CMD_REQ	0x05
>   #define REALTEK_USB_VENQT_CMD_IDX	0x00
>   
> -enum {
> -	VENDOR_WRITE = 0x00,
> -	VENDOR_READ = 0x01,
> -};
>   #define ALIGNMENT_UNIT			16
>   #define MAX_VENDOR_REQ_CMD_SIZE	254	/* 8188cu SIE Support */
>   #define MAX_USB_IO_CTL_SIZE	(MAX_VENDOR_REQ_CMD_SIZE + ALIGNMENT_UNIT)
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> Remove unnecessary variables, summarize declarations and assignments.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/hal/usb_ops_linux.c | 30 +++++----------------
>   1 file changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index dccb9fd34777..cb969a200681 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -98,46 +98,30 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>   
>   static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
>   {
> -	u16 wvalue;
> -	u16 len;
> -	u8 data = 0;
> -
> -
> -
> -	wvalue = (u16)(addr & 0x0000ffff);
> -	len = 1;
> -
> -	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
> -
> +	u16 wvalue = (u16)(addr & 0x0000ffff);
> +	u8 data;
>   
> +	usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ);
>   
>   	return data;
> -
>   }
>   
>   static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr)
>   {
> -	u16 wvalue;
> -	u16 len;
> +	u16 wvalue = (u16)(addr & 0x0000ffff);
>   	__le32 data;
>   
> -	wvalue = (u16)(addr & 0x0000ffff);
> -	len = 2;
> -	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
> +	usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_READ);
>   
>   	return (u16)(le32_to_cpu(data) & 0xffff);
>   }
>   
>   static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
>   {
> -	u16 wvalue;
> -	u16 len;
> +	u16 wvalue = (u16)(addr & 0x0000ffff);
>   	__le32 data;
>   
> -	wvalue = (u16)(addr & 0x0000ffff);
> -	len = 4;
> -
> -	usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_READ);
> +	usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_READ);
>   
>   	return le32_to_cpu(data);
>   }
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> Remove unnecessary variables, summarize declarations and assignments.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/hal/usb_ops_linux.c | 50 ++++-----------------
>   1 file changed, 8 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index cb969a200681..e01f1ac19596 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -128,59 +128,25 @@ static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
>   
>   static int usb_write8(struct intf_hdl *pintfhdl, u32 addr, u8 val)
>   {
> -	u16 wvalue;
> -	u16 len;
> -	u8 data;
> -	int ret;
> -
> -
> -	wvalue = (u16)(addr & 0x0000ffff);
> -	len = 1;
> -	data = val;
> -	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
> +	u16 wvalue = (u16)(addr & 0x0000ffff);
>   
> -	return ret;
> +	return usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
>   }
>   
>   static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
>   {
> -	u16 wvalue;
> -	u16 len;
> -	__le32 data;
> -	int ret;
> -
> -
> -
> -	wvalue = (u16)(addr & 0x0000ffff);
> -	len = 2;
> -
> -	data = cpu_to_le32(val & 0x0000ffff);
> -
> -	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
> -
> -
> +	u16 wvalue = (u16)(addr & 0x0000ffff);
> +	__le32 data = cpu_to_le32(val & 0x0000ffff);
>   
> -	return ret;
> +	return usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
>   }
>   
>   static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>   {
> -	u16 wvalue;
> -	u16 len;
> -	__le32 data;
> -	int ret;
> -
> -
> -
> -	wvalue = (u16)(addr & 0x0000ffff);
> -	len = 4;
> -	data = cpu_to_le32(val);
> -
> -	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, len, REALTEK_USB_VENQT_WRITE);
> -
> -
> +	u16 wvalue = (u16)(addr & 0x0000ffff);
> +	__le32 data = cpu_to_le32(val);
>   
> -	return ret;
> +	return usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE);
>   }
>   
>   static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 08/10] staging: r8188eu: clean up the usb_writeN

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> Remove unnecessary variables, check the length.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index e01f1ac19596..5408383ccec3 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -151,20 +151,15 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>   
>   static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
>   {
> -	u16 wvalue;
> -	u16 len;
> +	u16 wvalue = (u16)(addr & 0x0000ffff);
>   	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
> -	int ret;
> -
>   
> +	if (length > VENDOR_CMD_MAX_DATA_LEN)
> +		return -EINVAL;
>   
> -	wvalue = (u16)(addr & 0x0000ffff);
> -	len = length;
> -	memcpy(buf, pdata, len);
> +	memcpy(buf, pdata, length);

Hi Martin, shouldn't this be

memcpy(buf, pdata, (length & 0xffff));

>   
> -	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, len, REALTEK_USB_VENQT_WRITE);
> -
> -	return ret;
> +	return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
>   }
>   
>   static void interrupt_handler_8188eu(struct adapter *adapt, u16 pkt_len, u8 *pbuf)
> 

Regards,
Michael

Re: [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> Remove function pointers which are not used by the r8188eu driver.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/include/rtw_io.h | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
> index f1b3074fa075..4b41c7b03972 100644
> --- a/drivers/staging/r8188eu/include/rtw_io.h
> +++ b/drivers/staging/r8188eu/include/rtw_io.h
> @@ -100,13 +100,10 @@ struct _io_ops {
>   			  u8 *pmem);
>   	void (*_write_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
>   			   u8 *pmem);
> -	void (*_sync_irp_protocol_rw)(struct io_queue *pio_q);
> -	u32 (*_read_interrupt)(struct intf_hdl *pintfhdl, u32 addr);
>   	u32 (*_read_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
>   			  u8 *pmem);
>   	u32 (*_write_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
>   			   u8 *pmem);
> -	u32 (*_write_scsi)(struct intf_hdl *pintfhdl,u32 cnt, u8 *pmem);
>   	void (*_read_port_cancel)(struct intf_hdl *pintfhdl);
>   	void (*_write_port_cancel)(struct intf_hdl *pintfhdl);
>   };
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 10/10] staging: r8188eu: set pipe only once

[Michael Straube]

On 8/21/21 6:48 PM, Martin Kaiser wrote:
> Set the pipe for reading or writing in usbctrl_vendorreq only once.
> There's no need to set it again for every retry.
> 
> This patch is an adaptation of commit 889ed8b5e374 ("staging: rtl8188eu:
> set pipe only once") for the new r8188eu driver.
> 
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
>   drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index 5408383ccec3..5a55ee38d7b8 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -40,15 +40,16 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>   		goto release_mutex;
>   	}
>   
> -	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
> -		memset(pIo_buf, 0, len);
> +	if (requesttype == REALTEK_USB_VENQT_READ)
> +		pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
> +	else
> +		pipe = usb_sndctrlpipe(udev, 0);/* write_out */
>   
> -		if (requesttype == REALTEK_USB_VENQT_READ) {
> -			pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
> -		} else {
> -			pipe = usb_sndctrlpipe(udev, 0);/* write_out */
> +	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
> +		if (requesttype == REALTEK_USB_VENQT_READ)
> +			memset(pIo_buf, 0, len);
> +		else
>   			memcpy(pIo_buf, pdata, len);
> -		}
>   
>   		status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
>   					 requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
> 

Looks good to me.

Acked-by: Michael Straube <straube.linux@gmail.com>

Thanks,
Michael

Re: [PATCH 08/10] staging: r8188eu: clean up the usb_writeN

[Martin Kaiser]

Hi Michael,

Thus wrote Michael Straube (straube.linux@gmail.com):

> On 8/21/21 6:48 PM, Martin Kaiser wrote:
> > Remove unnecessary variables, check the length.

> > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > ---
> >   drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 +++++----------
> >   1 file changed, 5 insertions(+), 10 deletions(-)

> > diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> > index e01f1ac19596..5408383ccec3 100644
> > --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> > +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> > @@ -151,20 +151,15 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
> >   static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
> >   {
> > -	u16 wvalue;
> > -	u16 len;
> > +	u16 wvalue = (u16)(addr & 0x0000ffff);
> >   	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
> > -	int ret;
> > -
> > +	if (length > VENDOR_CMD_MAX_DATA_LEN)
> > +		return -EINVAL;
> > -	wvalue = (u16)(addr & 0x0000ffff);
> > -	len = length;
> > -	memcpy(buf, pdata, len);
> > +	memcpy(buf, pdata, length);

> Hi Martin, shouldn't this be

> memcpy(buf, pdata, (length & 0xffff));

I don't think this makes any difference. I've already checked that
length <= VENDOR_CMD_MAX_DATA_LEN, which is 254. memcpy takes a size_t
parameter for the number of bytes to copy. length will not overflow
this.

Best regards,
Martin

Re: [PATCH 01/10] staging: r8188eu: remove unnecessary cast

[Martin Kaiser]

Thus wrote Phillip Potter (phil@philpotter.co.uk):

> On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:

> > name is a const char * by default. This type should be ok for r8188eu.

> > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > ---
> >  drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)

> > diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> > index e002070f7fba..72556ac10d7d 100644
> > --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> > +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> > @@ -61,7 +61,7 @@ struct rtw_usb_drv {
> >  };

> >  static struct rtw_usb_drv rtl8188e_usb_drv = {
> > -       .usbdrv.name = (char *)"r8188eu",
> > +       .usbdrv.name = "r8188eu",
> >         .usbdrv.probe = rtw_drv_init,
> >         .usbdrv.disconnect = rtw_dev_remove,
> >         .usbdrv.id_table = rtw_usb_id_tbl,
> > --
> > 2.20.1

Hi Phil,

> Looks ok to me, thanks. I would consider using a cover letter style
> [PATCH 00/10] style approach as an addition in future though, just my
> personal opinion.

> Acked-by: Phillip Potter <phil@philpotter.co.uk>

Thanks.

This series is a mixed bag of things I found while poking around in the
code. So I didn't think there was anything useful to say in a cover
letter. Still, I see your point, it makes sense for a patch series to
have a cover letter, I'll add one for future patch series.

Best regards,
Martin

Re: [PATCH 08/10] staging: r8188eu: clean up the usb_writeN

[Michael Straube]

On 8/22/21 6:58 PM, Martin Kaiser wrote:
> Hi Michael,
> 
> Thus wrote Michael Straube (straube.linux@gmail.com):
> 
>> On 8/21/21 6:48 PM, Martin Kaiser wrote:
>>> Remove unnecessary variables, check the length.
> 
>>> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
>>> ---
>>>    drivers/staging/r8188eu/hal/usb_ops_linux.c | 15 +++++----------
>>>    1 file changed, 5 insertions(+), 10 deletions(-)
> 
>>> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
>>> index e01f1ac19596..5408383ccec3 100644
>>> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
>>> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
>>> @@ -151,20 +151,15 @@ static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
>>>    static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
>>>    {
>>> -	u16 wvalue;
>>> -	u16 len;
>>> +	u16 wvalue = (u16)(addr & 0x0000ffff);
>>>    	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
>>> -	int ret;
>>> -
>>> +	if (length > VENDOR_CMD_MAX_DATA_LEN)
>>> +		return -EINVAL;
>>> -	wvalue = (u16)(addr & 0x0000ffff);
>>> -	len = length;
>>> -	memcpy(buf, pdata, len);
>>> +	memcpy(buf, pdata, length);
> 
>> Hi Martin, shouldn't this be
> 
>> memcpy(buf, pdata, (length & 0xffff));
> 
> I don't think this makes any difference. I've already checked that
> length <= VENDOR_CMD_MAX_DATA_LEN, which is 254. memcpy takes a size_t
> parameter for the number of bytes to copy. length will not overflow
> this.
> 
> Best regards,
> Martin
> 

Hi Martin,

ah, I see now. You are right.

Acked-by: Michael Straube <straube.linux@gmail.com>


Thanks,
Michael

Re: [PATCH 01/10] staging: r8188eu: remove unnecessary cast

[Phillip Potter]

On Sun, 22 Aug 2021 at 18:04, Martin Kaiser <lists@kaiser.cx> wrote:
>
> Thus wrote Phillip Potter (phil@philpotter.co.uk):
>
> > On Sat, 21 Aug 2021 at 17:49, Martin Kaiser <martin@kaiser.cx> wrote:
>
> > > name is a const char * by default. This type should be ok for r8188eu.
>
> > > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > > ---
> > >  drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> > > diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> > > index e002070f7fba..72556ac10d7d 100644
> > > --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> > > +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> > > @@ -61,7 +61,7 @@ struct rtw_usb_drv {
> > >  };
>
> > >  static struct rtw_usb_drv rtl8188e_usb_drv = {
> > > -       .usbdrv.name = (char *)"r8188eu",
> > > +       .usbdrv.name = "r8188eu",
> > >         .usbdrv.probe = rtw_drv_init,
> > >         .usbdrv.disconnect = rtw_dev_remove,
> > >         .usbdrv.id_table = rtw_usb_id_tbl,
> > > --
> > > 2.20.1
>
> Hi Phil,
>
> > Looks ok to me, thanks. I would consider using a cover letter style
> > [PATCH 00/10] style approach as an addition in future though, just my
> > personal opinion.
>
> > Acked-by: Phillip Potter <phil@philpotter.co.uk>
>
> Thanks.
>
> This series is a mixed bag of things I found while poking around in the
> code. So I didn't think there was anything useful to say in a cover
> letter. Still, I see your point, it makes sense for a patch series to
> have a cover letter, I'll add one for future patch series.
>
> Best regards,
> Martin

Reasonable point for sure - it is your call ultimately, I like your
work in any case, so many thanks :-)

Regards,
Phil