LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 01/10] staging: r8188eu: remove unnecessary cast
@ 2021-08-21 16:48 Martin Kaiser
  2021-08-21 16:48 ` [PATCH 02/10] staging: r8188eu: remove unused define Martin Kaiser
                   ` (10 more replies)
  0 siblings, 11 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 02/10] staging: r8188eu: remove unused define
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:27   ` Phillip Potter
  2021-08-22 11:56   ` Michael Straube
  2021-08-21 16:48 ` [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter Martin Kaiser
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
  2021-08-21 16:48 ` [PATCH 02/10] staging: r8188eu: remove unused define Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:44   ` Phillip Potter
  2021-08-22 12:08   ` Michael Straube
  2021-08-21 16:48 ` [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines Martin Kaiser
                   ` (8 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
  2021-08-21 16:48 ` [PATCH 02/10] staging: r8188eu: remove unused define Martin Kaiser
  2021-08-21 16:48 ` [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:45   ` Phillip Potter
  2021-08-22 12:18   ` Michael Straube
  2021-08-21 16:48 ` [PATCH 05/10] staging: r8188eu: remove an unused enum Martin Kaiser
                   ` (7 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 05/10] staging: r8188eu: remove an unused enum
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
                   ` (2 preceding siblings ...)
  2021-08-21 16:48 ` [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:29   ` Phillip Potter
  2021-08-22 12:19   ` Michael Straube
  2021-08-21 16:48 ` [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions Martin Kaiser
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
                   ` (3 preceding siblings ...)
  2021-08-21 16:48 ` [PATCH 05/10] staging: r8188eu: remove an unused enum Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:45   ` Phillip Potter
  2021-08-22 12:25   ` Michael Straube
  2021-08-21 16:48 ` [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions Martin Kaiser
                   ` (5 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
                   ` (4 preceding siblings ...)
  2021-08-21 16:48 ` [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:46   ` Phillip Potter
  2021-08-22 12:27   ` Michael Straube
  2021-08-21 16:48 ` [PATCH 08/10] staging: r8188eu: clean up the usb_writeN Martin Kaiser
                   ` (4 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 08/10] staging: r8188eu: clean up the usb_writeN
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
                   ` (5 preceding siblings ...)
  2021-08-21 16:48 ` [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:47   ` Phillip Potter
  2021-08-22 13:24   ` Michael Straube
  2021-08-21 16:48 ` [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops Martin Kaiser
                   ` (3 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
                   ` (6 preceding siblings ...)
  2021-08-21 16:48 ` [PATCH 08/10] staging: r8188eu: clean up the usb_writeN Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:29   ` Phillip Potter
  2021-08-22 13:28   ` Michael Straube
  2021-08-21 16:48 ` [PATCH 10/10] staging: r8188eu: set pipe only once Martin Kaiser
                   ` (2 subsequent siblings)
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* [PATCH 10/10] staging: r8188eu: set pipe only once
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
                   ` (7 preceding siblings ...)
  2021-08-21 16:48 ` [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops Martin Kaiser
@ 2021-08-21 16:48 ` Martin Kaiser
  2021-08-21 17:47   ` Phillip Potter
  2021-08-22 14:05   ` Michael Straube
  2021-08-21 17:27 ` [PATCH 01/10] staging: r8188eu: remove unnecessary cast Phillip Potter
  2021-08-22 11:54 ` Michael Straube
  10 siblings, 2 replies; 34+ messages in thread
From: Martin Kaiser @ 2021-08-21 16:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, Michael Straube,
	linux-staging, linux-kernel, 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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 01/10] staging: r8188eu: remove unnecessary cast
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
                   ` (8 preceding siblings ...)
  2021-08-21 16:48 ` [PATCH 10/10] staging: r8188eu: set pipe only once Martin Kaiser
@ 2021-08-21 17:27 ` Phillip Potter
  2021-08-22 17:04   ` Martin Kaiser
  2021-08-22 11:54 ` Michael Straube
  10 siblings, 1 reply; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:27 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 02/10] staging: r8188eu: remove unused define
  2021-08-21 16:48 ` [PATCH 02/10] staging: r8188eu: remove unused define Martin Kaiser
@ 2021-08-21 17:27   ` Phillip Potter
  2021-08-22 11:56   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:27 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 05/10] staging: r8188eu: remove an unused enum
  2021-08-21 16:48 ` [PATCH 05/10] staging: r8188eu: remove an unused enum Martin Kaiser
@ 2021-08-21 17:29   ` Phillip Potter
  2021-08-22 12:19   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:29 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops
  2021-08-21 16:48 ` [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops Martin Kaiser
@ 2021-08-21 17:29   ` Phillip Potter
  2021-08-22 13:28   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:29 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter
  2021-08-21 16:48 ` [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter Martin Kaiser
@ 2021-08-21 17:44   ` Phillip Potter
  2021-08-22 12:08   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:44 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines
  2021-08-21 16:48 ` [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines Martin Kaiser
@ 2021-08-21 17:45   ` Phillip Potter
  2021-08-22 12:18   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:45 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions
  2021-08-21 16:48 ` [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions Martin Kaiser
@ 2021-08-21 17:45   ` Phillip Potter
  2021-08-22 12:25   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:45 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions
  2021-08-21 16:48 ` [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions Martin Kaiser
@ 2021-08-21 17:46   ` Phillip Potter
  2021-08-22 12:27   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:46 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 08/10] staging: r8188eu: clean up the usb_writeN
  2021-08-21 16:48 ` [PATCH 08/10] staging: r8188eu: clean up the usb_writeN Martin Kaiser
@ 2021-08-21 17:47   ` Phillip Potter
  2021-08-22 13:24   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:47 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 10/10] staging: r8188eu: set pipe only once
  2021-08-21 16:48 ` [PATCH 10/10] staging: r8188eu: set pipe only once Martin Kaiser
@ 2021-08-21 17:47   ` Phillip Potter
  2021-08-22 14:05   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-21 17:47 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 01/10] staging: r8188eu: remove unnecessary cast
  2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
                   ` (9 preceding siblings ...)
  2021-08-21 17:27 ` [PATCH 01/10] staging: r8188eu: remove unnecessary cast Phillip Potter
@ 2021-08-22 11:54 ` Michael Straube
  10 siblings, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 11:54 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 02/10] staging: r8188eu: remove unused define
  2021-08-21 16:48 ` [PATCH 02/10] staging: r8188eu: remove unused define Martin Kaiser
  2021-08-21 17:27   ` Phillip Potter
@ 2021-08-22 11:56   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 11:56 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter
  2021-08-21 16:48 ` [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter Martin Kaiser
  2021-08-21 17:44   ` Phillip Potter
@ 2021-08-22 12:08   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 12:08 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines
  2021-08-21 16:48 ` [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines Martin Kaiser
  2021-08-21 17:45   ` Phillip Potter
@ 2021-08-22 12:18   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 12:18 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 05/10] staging: r8188eu: remove an unused enum
  2021-08-21 16:48 ` [PATCH 05/10] staging: r8188eu: remove an unused enum Martin Kaiser
  2021-08-21 17:29   ` Phillip Potter
@ 2021-08-22 12:19   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 12:19 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions
  2021-08-21 16:48 ` [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions Martin Kaiser
  2021-08-21 17:45   ` Phillip Potter
@ 2021-08-22 12:25   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 12:25 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions
  2021-08-21 16:48 ` [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions Martin Kaiser
  2021-08-21 17:46   ` Phillip Potter
@ 2021-08-22 12:27   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 12:27 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 08/10] staging: r8188eu: clean up the usb_writeN
  2021-08-21 16:48 ` [PATCH 08/10] staging: r8188eu: clean up the usb_writeN Martin Kaiser
  2021-08-21 17:47   ` Phillip Potter
@ 2021-08-22 13:24   ` Michael Straube
  2021-08-22 16:58     ` Martin Kaiser
  1 sibling, 1 reply; 34+ messages in thread
From: Michael Straube @ 2021-08-22 13:24 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops
  2021-08-21 16:48 ` [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops Martin Kaiser
  2021-08-21 17:29   ` Phillip Potter
@ 2021-08-22 13:28   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 13:28 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 10/10] staging: r8188eu: set pipe only once
  2021-08-21 16:48 ` [PATCH 10/10] staging: r8188eu: set pipe only once Martin Kaiser
  2021-08-21 17:47   ` Phillip Potter
@ 2021-08-22 14:05   ` Michael Straube
  1 sibling, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 14:05 UTC (permalink / raw)
  To: Martin Kaiser, Greg Kroah-Hartman
  Cc: Larry Finger, Phillip Potter, Dan Carpenter, linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 08/10] staging: r8188eu: clean up the usb_writeN
  2021-08-22 13:24   ` Michael Straube
@ 2021-08-22 16:58     ` Martin Kaiser
  2021-08-22 17:17       ` Michael Straube
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Kaiser @ 2021-08-22 16:58 UTC (permalink / raw)
  To: Michael Straube
  Cc: Greg Kroah-Hartman, Larry Finger, Phillip Potter, Dan Carpenter,
	linux-staging, linux-kernel

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 01/10] staging: r8188eu: remove unnecessary cast
  2021-08-21 17:27 ` [PATCH 01/10] staging: r8188eu: remove unnecessary cast Phillip Potter
@ 2021-08-22 17:04   ` Martin Kaiser
  2021-08-22 23:49     ` Phillip Potter
  0 siblings, 1 reply; 34+ messages in thread
From: Martin Kaiser @ 2021-08-22 17:04 UTC (permalink / raw)
  To: Phillip Potter
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	linux-staging, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 08/10] staging: r8188eu: clean up the usb_writeN
  2021-08-22 16:58     ` Martin Kaiser
@ 2021-08-22 17:17       ` Michael Straube
  0 siblings, 0 replies; 34+ messages in thread
From: Michael Straube @ 2021-08-22 17:17 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Phillip Potter, Dan Carpenter,
	linux-staging, linux-kernel



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


^ permalink raw reply	[flat|nested] 34+ messages in thread

* Re: [PATCH 01/10] staging: r8188eu: remove unnecessary cast
  2021-08-22 17:04   ` Martin Kaiser
@ 2021-08-22 23:49     ` Phillip Potter
  0 siblings, 0 replies; 34+ messages in thread
From: Phillip Potter @ 2021-08-22 23:49 UTC (permalink / raw)
  To: Martin Kaiser
  Cc: Greg Kroah-Hartman, Larry Finger, Dan Carpenter, Michael Straube,
	open list:STAGING SUBSYSTEM, Linux Kernel Mailing List

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

^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2021-08-22 23:50 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-21 16:48 [PATCH 01/10] staging: r8188eu: remove unnecessary cast Martin Kaiser
2021-08-21 16:48 ` [PATCH 02/10] staging: r8188eu: remove unused define Martin Kaiser
2021-08-21 17:27   ` Phillip Potter
2021-08-22 11:56   ` Michael Straube
2021-08-21 16:48 ` [PATCH 03/10] staging: rtl8188eu: use actual request type as parameter Martin Kaiser
2021-08-21 17:44   ` Phillip Potter
2021-08-22 12:08   ` Michael Straube
2021-08-21 16:48 ` [PATCH 04/10] staging: r8188eu: rewrite usb vendor request defines Martin Kaiser
2021-08-21 17:45   ` Phillip Potter
2021-08-22 12:18   ` Michael Straube
2021-08-21 16:48 ` [PATCH 05/10] staging: r8188eu: remove an unused enum Martin Kaiser
2021-08-21 17:29   ` Phillip Potter
2021-08-22 12:19   ` Michael Straube
2021-08-21 16:48 ` [PATCH 06/10] staging: r8188eu: clean up the usb_readXY functions Martin Kaiser
2021-08-21 17:45   ` Phillip Potter
2021-08-22 12:25   ` Michael Straube
2021-08-21 16:48 ` [PATCH 07/10] staging: r8188eu: clean up the usb_writeXY functions Martin Kaiser
2021-08-21 17:46   ` Phillip Potter
2021-08-22 12:27   ` Michael Straube
2021-08-21 16:48 ` [PATCH 08/10] staging: r8188eu: clean up the usb_writeN Martin Kaiser
2021-08-21 17:47   ` Phillip Potter
2021-08-22 13:24   ` Michael Straube
2021-08-22 16:58     ` Martin Kaiser
2021-08-22 17:17       ` Michael Straube
2021-08-21 16:48 ` [PATCH 09/10] staging: r8188eu: remove unused members of struct _io_ops Martin Kaiser
2021-08-21 17:29   ` Phillip Potter
2021-08-22 13:28   ` Michael Straube
2021-08-21 16:48 ` [PATCH 10/10] staging: r8188eu: set pipe only once Martin Kaiser
2021-08-21 17:47   ` Phillip Potter
2021-08-22 14:05   ` Michael Straube
2021-08-21 17:27 ` [PATCH 01/10] staging: r8188eu: remove unnecessary cast Phillip Potter
2021-08-22 17:04   ` Martin Kaiser
2021-08-22 23:49     ` Phillip Potter
2021-08-22 11:54 ` Michael Straube

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).