All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain
@ 2021-09-04 15:04 Fabio M. De Francesco
  2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-09-04 15:04 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel
  Cc: Fabio M. De Francesco

io_ops abstraction is useless in this driver, since there is only one ops
registration. Without io_ops we can get rid of indirect calls mess and
shorten the calls chain.

Shorten the calls chain of rtw_read8/16/32() down to the actual reads.
For this purpose unify the three usb_read8/16/32 into the new
usb_read(); make the latter parameterizable with 'size'; embed most of
the code of usbctrl_vendorreq() into usb_read() and use in it the new
usb_control_msg_recv() API of USB Core.

Shorten the calls chain of rtw_write8/16/32() down to the actual writes.
For this purpose unify the four usb_write8/16/32/N() into the new
usb_write(); make the latter parameterizable with 'size'; embed most of
the code of usbctrl_vendorreq() into usb_write() and use in it the new
usb_control_msg_send() API of USB Core.

The code with the modifications was thoroughly tested by Pavel Skripkin 
using a TP-Link TL-WN722N v2 / v3 [Realtek RTL8188EUS]

Fabio M. De Francesco (2):
  staging: r8188eu: Shorten calls chain of rtw_read8/16/32()
  staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N()

Pavel Skripkin (1):
  staging: r8188eu: remove _io_ops structure

 drivers/staging/r8188eu/core/rtw_io.c         | 241 +-----------------
 drivers/staging/r8188eu/hal/usb_halinit.c     |   6 +-
 drivers/staging/r8188eu/hal/usb_ops_linux.c   | 232 ++++++++++-------
 drivers/staging/r8188eu/include/rtw_io.h      |  76 +-----
 drivers/staging/r8188eu/include/usb_ops.h     |   2 -
 .../staging/r8188eu/include/usb_ops_linux.h   |   8 -
 drivers/staging/r8188eu/os_dep/usb_intf.c     |   2 +-
 .../staging/r8188eu/os_dep/usb_ops_linux.c    |  40 +--
 8 files changed, 169 insertions(+), 438 deletions(-)

-- 
2.33.0


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

* [PATCH 1/3] staging: r8188eu: remove _io_ops structure
  2021-09-04 15:04 [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
@ 2021-09-04 15:04 ` Fabio M. De Francesco
  2021-09-06  7:49   ` Dan Carpenter
  2021-09-04 15:04 ` [PATCH 2/3] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco
  2021-09-04 15:04 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco
  2 siblings, 1 reply; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-09-04 15:04 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

io_ops abstraction is useless in this driver, since there is only one ops
registration. Without io_ops we can get rid of indirect calls mess and
shorten the calls chain.

Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/staging/r8188eu/core/rtw_io.c         | 241 +-----------------
 drivers/staging/r8188eu/hal/usb_halinit.c     |   6 +-
 drivers/staging/r8188eu/hal/usb_ops_linux.c   |  70 ++---
 drivers/staging/r8188eu/include/rtw_io.h      |  76 +-----
 drivers/staging/r8188eu/include/usb_ops.h     |   2 -
 .../staging/r8188eu/include/usb_ops_linux.h   |   8 -
 drivers/staging/r8188eu/os_dep/usb_intf.c     |   2 +-
 .../staging/r8188eu/os_dep/usb_ops_linux.c    |  40 +--
 8 files changed, 68 insertions(+), 377 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index cde0205816b1..2d5cc080bf1d 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -34,224 +34,6 @@ jackson@realtek.com.tw
 #define rtw_cpu_to_le16(val)		cpu_to_le16(val)
 #define rtw_cpu_to_le32(val)		cpu_to_le32(val)
 
-u8 _rtw_read8(struct adapter *adapter, u32 addr)
-{
-	u8 r_val;
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl *pintfhdl = &pio_priv->intf;
-	u8 (*_read8)(struct intf_hdl *pintfhdl, u32 addr);
-
-
-	_read8 = pintfhdl->io_ops._read8;
-	r_val = _read8(pintfhdl, addr);
-
-	return r_val;
-}
-
-u16 _rtw_read16(struct adapter *adapter, u32 addr)
-{
-	u16 r_val;
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	u16 (*_read16)(struct intf_hdl *pintfhdl, u32 addr);
-
-	_read16 = pintfhdl->io_ops._read16;
-
-	r_val = _read16(pintfhdl, addr);
-
-	return r_val;
-}
-
-u32 _rtw_read32(struct adapter *adapter, u32 addr)
-{
-	u32 r_val;
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	u32	(*_read32)(struct intf_hdl *pintfhdl, u32 addr);
-
-	_read32 = pintfhdl->io_ops._read32;
-
-	r_val = _read32(pintfhdl, addr);
-
-	return r_val;
-}
-
-int _rtw_write8(struct adapter *adapter, u32 addr, u8 val)
-{
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	int (*_write8)(struct intf_hdl *pintfhdl, u32 addr, u8 val);
-	int ret;
-
-	_write8 = pintfhdl->io_ops._write8;
-
-	ret = _write8(pintfhdl, addr, val);
-
-
-	return RTW_STATUS_CODE(ret);
-}
-
-int _rtw_write16(struct adapter *adapter, u32 addr, u16 val)
-{
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	int (*_write16)(struct intf_hdl *pintfhdl, u32 addr, u16 val);
-	int ret;
-
-	_write16 = pintfhdl->io_ops._write16;
-
-	ret = _write16(pintfhdl, addr, val);
-
-
-	return RTW_STATUS_CODE(ret);
-}
-int _rtw_write32(struct adapter *adapter, u32 addr, u32 val)
-{
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	int (*_write32)(struct intf_hdl *pintfhdl, u32 addr, u32 val);
-	int ret;
-
-	_write32 = pintfhdl->io_ops._write32;
-
-	ret = _write32(pintfhdl, addr, val);
-
-
-	return RTW_STATUS_CODE(ret);
-}
-
-int _rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata)
-{
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl *pintfhdl = (struct intf_hdl *)(&pio_priv->intf);
-	int (*_writeN)(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata);
-	int ret;
-
-	_writeN = pintfhdl->io_ops._writeN;
-
-	ret = _writeN(pintfhdl, addr, length, pdata);
-
-
-	return RTW_STATUS_CODE(ret);
-}
-int _rtw_write8_async(struct adapter *adapter, u32 addr, u8 val)
-{
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	int (*_write8_async)(struct intf_hdl *pintfhdl, u32 addr, u8 val);
-	int ret;
-
-	_write8_async = pintfhdl->io_ops._write8_async;
-
-	ret = _write8_async(pintfhdl, addr, val);
-
-
-	return RTW_STATUS_CODE(ret);
-}
-
-int _rtw_write16_async(struct adapter *adapter, u32 addr, u16 val)
-{
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	int (*_write16_async)(struct intf_hdl *pintfhdl, u32 addr, u16 val);
-	int ret;
-
-	_write16_async = pintfhdl->io_ops._write16_async;
-	ret = _write16_async(pintfhdl, addr, val);
-
-	return RTW_STATUS_CODE(ret);
-}
-
-int _rtw_write32_async(struct adapter *adapter, u32 addr, u32 val)
-{
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	int (*_write32_async)(struct intf_hdl *pintfhdl, u32 addr, u32 val);
-	int ret;
-
-	_write32_async = pintfhdl->io_ops._write32_async;
-	ret = _write32_async(pintfhdl, addr, val);
-
-	return RTW_STATUS_CODE(ret);
-}
-
-void _rtw_read_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem)
-{
-	void (*_read_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem);
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-
-
-	if (adapter->bDriverStopped || adapter->bSurpriseRemoved)
-	     return;
-	_read_mem = pintfhdl->io_ops._read_mem;
-	_read_mem(pintfhdl, addr, cnt, pmem);
-
-}
-
-void _rtw_write_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem)
-{
-	void (*_write_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem);
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-
-
-
-	_write_mem = pintfhdl->io_ops._write_mem;
-
-	_write_mem(pintfhdl, addr, cnt, pmem);
-
-
-}
-
-void _rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem)
-{
-	u32 (*_read_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem);
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-
-
-
-	if (adapter->bDriverStopped || adapter->bSurpriseRemoved)
-	     return;
-
-	_read_port = pintfhdl->io_ops._read_port;
-
-	_read_port(pintfhdl, addr, cnt, pmem);
-
-
-}
-
-void _rtw_read_port_cancel(struct adapter *adapter)
-{
-	void (*_read_port_cancel)(struct intf_hdl *pintfhdl);
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-
-	_read_port_cancel = pintfhdl->io_ops._read_port_cancel;
-
-	if (_read_port_cancel)
-		_read_port_cancel(pintfhdl);
-}
-
-u32 _rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem)
-{
-	u32 (*_write_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem);
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct	intf_hdl		*pintfhdl = &pio_priv->intf;
-	u32 ret = _SUCCESS;
-
-
-
-	_write_port = pintfhdl->io_ops._write_port;
-
-	ret = _write_port(pintfhdl, addr, cnt, pmem);
-
-
-
-	return ret;
-}
-
 u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem, int timeout_ms)
 {
 	int ret = _SUCCESS;
@@ -261,7 +43,7 @@ u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pme
 	rtw_sctx_init(&sctx, timeout_ms);
 	pxmitbuf->sctx = &sctx;
 
-	ret = _rtw_write_port(adapter, addr, cnt, pmem);
+	ret = rtw_write_port(adapter, addr, cnt, pmem);
 
 	if (ret == _SUCCESS)
 		ret = rtw_sctx_wait(&sctx);
@@ -269,31 +51,12 @@ u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pme
 	return ret;
 }
 
-void _rtw_write_port_cancel(struct adapter *adapter)
-{
-	void (*_write_port_cancel)(struct intf_hdl *pintfhdl);
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-
-	_write_port_cancel = pintfhdl->io_ops._write_port_cancel;
-
-	if (_write_port_cancel)
-		_write_port_cancel(pintfhdl);
-}
-
-int rtw_init_io_priv(struct adapter *padapter, void (*set_intf_ops)(struct _io_ops *pops))
+void rtw_init_io_priv(struct adapter *padapter)
 {
 	struct io_priv	*piopriv = &padapter->iopriv;
 	struct intf_hdl *pintf = &piopriv->intf;
 
-	if (!set_intf_ops)
-		return _FAIL;
-
 	piopriv->padapter = padapter;
 	pintf->padapter = padapter;
 	pintf->pintf_dev = adapter_to_dvobj(padapter);
-
-	set_intf_ops(&pintf->io_ops);
-
-	return _SUCCESS;
 }
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 147c51255878..33147cbc55bb 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -1052,11 +1052,7 @@ static unsigned int rtl8188eu_inirp_init(struct adapter *Adapter)
 	u8 i;
 	struct recv_buf *precvbuf;
 	uint	status;
-	struct intf_hdl *pintfhdl = &Adapter->iopriv.intf;
 	struct recv_priv *precvpriv = &Adapter->recvpriv;
-	u32 (*_read_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem);
-
-	_read_port = pintfhdl->io_ops._read_port;
 
 	status = _SUCCESS;
 
@@ -1065,7 +1061,7 @@ static unsigned int rtl8188eu_inirp_init(struct adapter *Adapter)
 	/* issue Rx irp to receive data */
 	precvbuf = (struct recv_buf *)precvpriv->precv_buf;
 	for (i = 0; i < NR_RECVBUFF; i++) {
-		if (!_read_port(pintfhdl, precvpriv->ff_hwaddr, 0, (unsigned char *)precvbuf)) {
+		if (!rtw_read_port(Adapter, precvpriv->ff_hwaddr, 0, (unsigned char *)precvbuf)) {
 			status = _FAIL;
 			goto exit;
 		}
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 0cf69033c529..a87b0d2e87d0 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -97,8 +97,10 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 	return status;
 }
 
-static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
+u8 rtw_read8(struct adapter *adapter, u32 addr)
 {
+	struct io_priv *pio_priv = &adapter->iopriv;
+	struct intf_hdl *pintfhdl = &pio_priv->intf;
 	u16 wvalue = (u16)(addr & 0x0000ffff);
 	u8 data;
 
@@ -107,8 +109,10 @@ static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr)
 	return data;
 }
 
-static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr)
+u16 rtw_read16(struct adapter *adapter, u32 addr)
 {
+	struct io_priv *pio_priv = &adapter->iopriv;
+	struct intf_hdl *pintfhdl = &pio_priv->intf;
 	u16 wvalue = (u16)(addr & 0x0000ffff);
 	__le32 data;
 
@@ -117,8 +121,10 @@ static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr)
 	return (u16)(le32_to_cpu(data) & 0xffff);
 }
 
-static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
+u32 rtw_read32(struct adapter *adapter, u32 addr)
 {
+	struct io_priv *pio_priv = &adapter->iopriv;
+	struct intf_hdl *pintfhdl = &pio_priv->intf;
 	u16 wvalue = (u16)(addr & 0x0000ffff);
 	__le32 data;
 
@@ -127,40 +133,59 @@ static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr)
 	return le32_to_cpu(data);
 }
 
-static int usb_write8(struct intf_hdl *pintfhdl, u32 addr, u8 val)
+int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
 {
+	struct io_priv *pio_priv = &adapter->iopriv;
+	struct intf_hdl *pintfhdl = &pio_priv->intf;
 	u16 wvalue = (u16)(addr & 0x0000ffff);
+	int ret;
 
-	return usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
+
+	return RTW_STATUS_CODE(ret);
 }
 
-static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val)
+int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
 {
+	struct io_priv *pio_priv = &adapter->iopriv;
+	struct intf_hdl *pintfhdl = &pio_priv->intf;
 	u16 wvalue = (u16)(addr & 0x0000ffff);
 	__le32 data = cpu_to_le32(val & 0x0000ffff);
+	int ret;
+
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
 
-	return usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
+	return RTW_STATUS_CODE(ret);
 }
 
-static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val)
+int rtw_write32(struct adapter *adapter, u32 addr, u32 val)
 {
+	struct io_priv *pio_priv = &adapter->iopriv;
+	struct intf_hdl *pintfhdl = &pio_priv->intf;
 	u16 wvalue = (u16)(addr & 0x0000ffff);
 	__le32 data = cpu_to_le32(val);
+	int ret;
 
-	return usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE);
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE);
+
+	return RTW_STATUS_CODE(ret);
 }
 
-static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
+int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata)
 {
+	struct io_priv *pio_priv = &adapter->iopriv;
+	struct intf_hdl *pintfhdl = &pio_priv->intf;
 	u16 wvalue = (u16)(addr & 0x0000ffff);
 	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
+	int ret;
 
 	if (length > VENDOR_CMD_MAX_DATA_LEN)
 		return -EINVAL;
 
 	memcpy(buf, pdata, length);
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
 
-	return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
+	return RTW_STATUS_CODE(ret);
 }
 
 static void interrupt_handler_8188eu(struct adapter *adapt, u16 pkt_len, u8 *pbuf)
@@ -431,11 +456,10 @@ static void usb_read_port_complete(struct urb *purb, struct pt_regs *regs)
 	}
 }
 
-static u32 usb_read_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem)
+u32 rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *rmem)
 {
 	struct urb *purb = NULL;
 	struct recv_buf	*precvbuf = (struct recv_buf *)rmem;
-	struct adapter		*adapter = pintfhdl->padapter;
 	struct dvobj_priv	*pdvobj = adapter_to_dvobj(adapter);
 	struct recv_priv	*precvpriv = &adapter->recvpriv;
 	struct usb_device	*pusbd = pdvobj->pusbdev;
@@ -534,26 +558,6 @@ void rtl8188eu_xmit_tasklet(unsigned long priv)
 	}
 }
 
-void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
-{
-
-	memset((u8 *)pops, 0, sizeof(struct _io_ops));
-	pops->_read8 = &usb_read8;
-	pops->_read16 = &usb_read16;
-	pops->_read32 = &usb_read32;
-	pops->_read_mem = &usb_read_mem;
-	pops->_read_port = &usb_read_port;
-	pops->_write8 = &usb_write8;
-	pops->_write16 = &usb_write16;
-	pops->_write32 = &usb_write32;
-	pops->_writeN = &usb_writeN;
-	pops->_write_mem = &usb_write_mem;
-	pops->_write_port = &usb_write_port;
-	pops->_read_port_cancel = &usb_read_port_cancel;
-	pops->_write_port_cancel = &usb_write_port_cancel;
-
-}
-
 void rtl8188eu_set_hw_type(struct adapter *adapt)
 {
 	adapt->chip_type = RTL8188E;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 4b41c7b03972..9722b76533dc 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -84,30 +84,6 @@ struct intf_priv;
 struct intf_hdl;
 struct io_queue;
 
-struct _io_ops {
-	u8 (*_read8)(struct intf_hdl *pintfhdl, u32 addr);
-	u16 (*_read16)(struct intf_hdl *pintfhdl, u32 addr);
-	u32 (*_read32)(struct intf_hdl *pintfhdl, u32 addr);
-	int (*_write8)(struct intf_hdl *pintfhdl, u32 addr, u8 val);
-	int (*_write16)(struct intf_hdl *pintfhdl, u32 addr, u16 val);
-	int (*_write32)(struct intf_hdl *pintfhdl, u32 addr, u32 val);
-	int (*_writeN)(struct intf_hdl *pintfhdl, u32 addr, u32 length,
-		       u8 *pdata);
-	int (*_write8_async)(struct intf_hdl *pintfhdl, u32 addr, u8 val);
-	int (*_write16_async)(struct intf_hdl *pintfhdl, u32 addr, u16 val);
-	int (*_write32_async)(struct intf_hdl *pintfhdl, u32 addr, u32 val);
-	void (*_read_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
-			  u8 *pmem);
-	void (*_write_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt,
-			   u8 *pmem);
-	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);
-	void (*_read_port_cancel)(struct intf_hdl *pintfhdl);
-	void (*_write_port_cancel)(struct intf_hdl *pintfhdl);
-};
-
 struct io_req {
 	struct list_head list;
 	u32	addr;
@@ -125,7 +101,6 @@ struct io_req {
 struct	intf_hdl {
 	struct adapter *padapter;
 	struct dvobj_priv *pintf_dev;
-	struct _io_ops	io_ops;
 };
 
 struct reg_protocol_rd {
@@ -245,58 +220,34 @@ void unregister_intf_hdl(struct intf_hdl *pintfhdl);
 void _rtw_attrib_read(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
 void _rtw_attrib_write(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
 
-u8 _rtw_read8(struct adapter *adapter, u32 addr);
-u16 _rtw_read16(struct adapter *adapter, u32 addr);
-u32 _rtw_read32(struct adapter *adapter, u32 addr);
-void _rtw_read_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
-void _rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
-void _rtw_read_port_cancel(struct adapter *adapter);
+u8 rtw_read8(struct adapter *adapter, u32 addr);
+u16 rtw_read16(struct adapter *adapter, u32 addr);
+u32 rtw_read32(struct adapter *adapter, u32 addr);
+u32 rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
+void rtw_read_port_cancel(struct adapter *adapter);
 
-int _rtw_write8(struct adapter *adapter, u32 addr, u8 val);
-int _rtw_write16(struct adapter *adapter, u32 addr, u16 val);
-int _rtw_write32(struct adapter *adapter, u32 addr, u32 val);
-int _rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata);
+int rtw_write8(struct adapter *adapter, u32 addr, u8 val);
+int rtw_write16(struct adapter *adapter, u32 addr, u16 val);
+int rtw_write32(struct adapter *adapter, u32 addr, u32 val);
+int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata);
 
 int _rtw_write8_async(struct adapter *adapter, u32 addr, u8 val);
 int _rtw_write16_async(struct adapter *adapter, u32 addr, u16 val);
 int _rtw_write32_async(struct adapter *adapter, u32 addr, u32 val);
 
-void _rtw_write_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
-u32 _rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
+u32 rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
 u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt,
 			     u8 *pmem, int timeout_ms);
-void _rtw_write_port_cancel(struct adapter *adapter);
-
-#define rtw_read8(adapter, addr) _rtw_read8((adapter), (addr))
-#define rtw_read16(adapter, addr) _rtw_read16((adapter), (addr))
-#define rtw_read32(adapter, addr) _rtw_read32((adapter), (addr))
-#define rtw_read_mem(adapter, addr, cnt, mem)				\
-	_rtw_read_mem((adapter), (addr), (cnt), (mem))
-#define rtw_read_port(adapter, addr, cnt, mem)				\
-	_rtw_read_port((adapter), (addr), (cnt), (mem))
-#define rtw_read_port_cancel(adapter) _rtw_read_port_cancel((adapter))
-
-#define  rtw_write8(adapter, addr, val)					\
-	_rtw_write8((adapter), (addr), (val))
-#define  rtw_write16(adapter, addr, val)				\
-	_rtw_write16((adapter), (addr), (val))
-#define  rtw_write32(adapter, addr, val)				\
-	_rtw_write32((adapter), (addr), (val))
-#define  rtw_writeN(adapter, addr, length, data)			\
-	_rtw_writeN((adapter), (addr), (length), (data))
+void rtw_write_port_cancel(struct adapter *adapter);
+
 #define rtw_write8_async(adapter, addr, val)				\
 	_rtw_write8_async((adapter), (addr), (val))
 #define rtw_write16_async(adapter, addr, val)				\
 	_rtw_write16_async((adapter), (addr), (val))
 #define rtw_write32_async(adapter, addr, val)				\
 	_rtw_write32_async((adapter), (addr), (val))
-#define rtw_write_mem(adapter, addr, cnt, mem)				\
-	_rtw_write_mem((adapter), (addr), (cnt), (mem))
-#define rtw_write_port(adapter, addr, cnt, mem)				\
-	_rtw_write_port((adapter), (addr), (cnt), (mem))
 #define rtw_write_port_and_wait(adapter, addr, cnt, mem, timeout_ms)	\
 	_rtw_write_port_and_wait((adapter), (addr), (cnt), (mem), (timeout_ms))
-#define rtw_write_port_cancel(adapter) _rtw_write_port_cancel((adapter))
 
 void rtw_write_scsi(struct adapter *adapter, u32 cnt, u8 *pmem);
 
@@ -340,8 +291,7 @@ void async_write32(struct adapter *adapter, u32 addr, u32 val,
 void async_write_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
 void async_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
 
-int rtw_init_io_priv(struct adapter *padapter,
-		     void (*set_intf_ops)(struct _io_ops *pops));
+void rtw_init_io_priv(struct adapter *padapter);
 
 uint alloc_io_queue(struct adapter *adapter);
 void free_io_queue(struct adapter *adapter);
diff --git a/drivers/staging/r8188eu/include/usb_ops.h b/drivers/staging/r8188eu/include/usb_ops.h
index c53cc54b6b87..1939b781b097 100644
--- a/drivers/staging/r8188eu/include/usb_ops.h
+++ b/drivers/staging/r8188eu/include/usb_ops.h
@@ -21,8 +21,6 @@
 
 void rtl8188eu_set_hw_type(struct adapter *padapter);
 #define hal_set_hw_type rtl8188eu_set_hw_type
-void rtl8188eu_set_intf_ops(struct _io_ops *pops);
-#define usb_set_intf_ops rtl8188eu_set_intf_ops
 
 /*
  * Increase and check if the continual_urb_error of this @param dvobjprivei
diff --git a/drivers/staging/r8188eu/include/usb_ops_linux.h b/drivers/staging/r8188eu/include/usb_ops_linux.h
index c357a3b1560e..641f059ffaf7 100644
--- a/drivers/staging/r8188eu/include/usb_ops_linux.h
+++ b/drivers/staging/r8188eu/include/usb_ops_linux.h
@@ -28,12 +28,4 @@
 
 unsigned int ffaddr2pipehdl(struct dvobj_priv *pdvobj, u32 addr);
 
-void usb_read_mem(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem);
-void usb_write_mem(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem);
-
-void usb_read_port_cancel(struct intf_hdl *pintfhdl);
-
-u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem);
-void usb_write_port_cancel(struct intf_hdl *pintfhdl);
-
 #endif
diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index bb85ab77fd26..78c91b14e637 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -596,7 +596,7 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj,
 	padapter->intf_stop = &usb_intf_stop;
 
 	/* step init_io_priv */
-	rtw_init_io_priv(padapter, usb_set_intf_ops);
+	rtw_init_io_priv(padapter);
 
 	/* step read_chip_version */
 	rtw_hal_read_chip_version(padapter);
diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index 62dd4a131534..f1440a837b97 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -31,24 +31,14 @@ struct zero_bulkout_context {
 	void *padapter;
 };
 
-void usb_read_mem(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem)
-{
-}
-
-void usb_write_mem(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
-{
-}
-
-void usb_read_port_cancel(struct intf_hdl *pintfhdl)
+void rtw_read_port_cancel(struct adapter *adapter)
 {
 	int i;
-	struct recv_buf *precvbuf;
-	struct adapter	*padapter = pintfhdl->padapter;
-	precvbuf = (struct recv_buf *)padapter->recvpriv.precv_buf;
+	struct recv_buf *precvbuf = (struct recv_buf *) adapter->recvpriv.precv_buf;
 
 	DBG_88E("%s\n", __func__);
 
-	padapter->bReadPortCancel = true;
+	adapter->bReadPortCancel = true;
 
 	for (i = 0; i < NR_RECVBUFF; i++) {
 		precvbuf->reuse = true;
@@ -134,22 +124,21 @@ static void usb_write_port_complete(struct urb *purb, struct pt_regs *regs)
 
 }
 
-u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
+u32 rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *wmem)
 {
 	unsigned long irqL;
 	unsigned int pipe;
 	int status;
 	u32 ret = _FAIL;
 	struct urb *purb = NULL;
-	struct adapter *padapter = (struct adapter *)pintfhdl->padapter;
-	struct dvobj_priv	*pdvobj = adapter_to_dvobj(padapter);
-	struct xmit_priv	*pxmitpriv = &padapter->xmitpriv;
+	struct dvobj_priv	*pdvobj = adapter_to_dvobj(adapter);
+	struct xmit_priv	*pxmitpriv = &adapter->xmitpriv;
 	struct xmit_buf *pxmitbuf = (struct xmit_buf *)wmem;
 	struct xmit_frame *pxmitframe = (struct xmit_frame *)pxmitbuf->priv_data;
 	struct usb_device *pusbd = pdvobj->pusbdev;
 
-	if ((padapter->bDriverStopped) || (padapter->bSurpriseRemoved) ||
-	    (padapter->pwrctrlpriv.pnp_bstop_trx)) {
+	if ((adapter->bDriverStopped) || (adapter->bSurpriseRemoved) ||
+	    (adapter->pwrctrlpriv.pnp_bstop_trx)) {
 		rtw_sctx_done_err(&pxmitbuf->sctx, RTW_SCTX_DONE_TX_DENY);
 		goto exit;
 	}
@@ -196,7 +185,7 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
 
 	status = usb_submit_urb(purb, GFP_ATOMIC);
 	if (!status) {
-		struct hal_data_8188e	*haldata = GET_HAL_DATA(padapter);
+		struct hal_data_8188e	*haldata = GET_HAL_DATA(adapter);
 
 		haldata->srestpriv.last_tx_time = jiffies;
 	} else {
@@ -205,7 +194,7 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
 
 		switch (status) {
 		case -ENODEV:
-			padapter->bDriverStopped = true;
+			adapter->bDriverStopped = true;
 			break;
 		default:
 			break;
@@ -224,15 +213,14 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
 	return ret;
 }
 
-void usb_write_port_cancel(struct intf_hdl *pintfhdl)
+void rtw_write_port_cancel(struct adapter *adapter)
 {
 	int i, j;
-	struct adapter	*padapter = pintfhdl->padapter;
-	struct xmit_buf *pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmitbuf;
+	struct xmit_buf *pxmitbuf = (struct xmit_buf *) adapter->xmitpriv.pxmitbuf;
 
 	DBG_88E("%s\n", __func__);
 
-	padapter->bWritePortCancel = true;
+	adapter->bWritePortCancel = true;
 
 	for (i = 0; i < NR_XMITBUFF; i++) {
 		for (j = 0; j < 8; j++) {
@@ -242,7 +230,7 @@ void usb_write_port_cancel(struct intf_hdl *pintfhdl)
 		pxmitbuf++;
 	}
 
-	pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmit_extbuf;
+	pxmitbuf = (struct xmit_buf *) adapter->xmitpriv.pxmit_extbuf;
 	for (i = 0; i < NR_XMIT_EXTBUFF; i++) {
 		for (j = 0; j < 8; j++) {
 			if (pxmitbuf->pxmit_urb[j])
-- 
2.33.0


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

* [PATCH 2/3] staging: r8188eu: Shorten calls chain of rtw_read8/16/32()
  2021-09-04 15:04 [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
  2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco
@ 2021-09-04 15:04 ` Fabio M. De Francesco
  2021-09-04 15:04 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco
  2 siblings, 0 replies; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-09-04 15:04 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel
  Cc: Fabio M. De Francesco, Pavel Skripkin

Shorten the calls chain of rtw_read8/16/32() down to the actual reads.
For this purpose unify the three usb_read8/16/32 into the new
usb_read(); make the latter parameterizable with 'size'; embed most of
the code of usbctrl_vendorreq() into usb_read() and use in it the new
usb_control_msg_recv() API of USB Core.

Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 92 +++++++++++++++++----
 1 file changed, 78 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index a87b0d2e87d0..f9c4fd5a2c53 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -97,38 +97,102 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 	return status;
 }
 
+static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size)
+{
+	u16 value = (u16)(addr & 0x0000ffff);
+	struct adapter *adapt = intfhdl->padapter;
+	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
+	struct usb_device *udev = dvobjpriv->pusbdev;
+	int status;
+	u8 *io_buf;
+	int vendorreq_times = 0;
+
+	if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
+		status = -EPERM;
+		goto exit;
+	}
+
+	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
+
+	/*  Acquire IO memory for vendorreq */
+	io_buf = dvobjpriv->usb_vendor_req_buf;
+
+	if (!io_buf) {
+		DBG_88E("[%s] io_buf == NULL\n", __func__);
+		status = -ENOMEM;
+		goto release_mutex;
+	}
+
+	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
+		status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+					      REALTEK_USB_VENQT_READ, value,
+					      REALTEK_USB_VENQT_CMD_IDX, io_buf,
+					      size, RTW_USB_CONTROL_MSG_TIMEOUT,
+					      GFP_KERNEL);
+		if (!status) {   /*  Success this control transfer. */
+			rtw_reset_continual_urb_error(dvobjpriv);
+			memcpy(data, io_buf, size);
+		} else { /*  error cases */
+			DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n",
+				value, "read", size, status, vendorreq_times);
+
+			if (status == (-ESHUTDOWN) || status == -ENODEV) {
+				adapt->bSurpriseRemoved = true;
+			} else {
+				struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
+
+				haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
+			}
+
+			if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
+				adapt->bSurpriseRemoved = true;
+				break;
+			}
+		}
+
+		/*  firmware download is checksummed, don't retry */
+		if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status)
+			break;
+	}
+
+release_mutex:
+	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
+exit:
+	return status;
+}
+
 u8 rtw_read8(struct adapter *adapter, u32 addr)
 {
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-	u16 wvalue = (u16)(addr & 0x0000ffff);
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
+	u16 value = (u16)(addr & 0x0000ffff);
 	u8 data;
 
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, value, &data, 1);
 
 	return data;
 }
 
 u16 rtw_read16(struct adapter *adapter, u32 addr)
 {
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-	u16 wvalue = (u16)(addr & 0x0000ffff);
-	__le32 data;
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
+	u16 value = (u16)(addr & 0x0000ffff);
+	__le16 data;
 
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, value, &data, 2);
 
-	return (u16)(le32_to_cpu(data) & 0xffff);
+	return le16_to_cpu(data);
 }
 
 u32 rtw_read32(struct adapter *adapter, u32 addr)
 {
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-	u16 wvalue = (u16)(addr & 0x0000ffff);
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
+	u16 value = (u16)(addr & 0x0000ffff);
 	__le32 data;
 
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, value, &data, 4);
 
 	return le32_to_cpu(data);
 }
-- 
2.33.0


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

* [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N()
  2021-09-04 15:04 [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
  2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco
  2021-09-04 15:04 ` [PATCH 2/3] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco
@ 2021-09-04 15:04 ` Fabio M. De Francesco
  2021-09-04 17:09     ` kernel test robot
  2 siblings, 1 reply; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-09-04 15:04 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel
  Cc: Fabio M. De Francesco, Pavel Skripkin

Shorten the calls chain of rtw_write8/16/32() down to the actual writes.
For this purpose unify the four usb_write8/16/32/N() into the new
usb_write(); make the latter parameterizable with 'size'; embed most of
the code of usbctrl_vendorreq() into usb_write() and use in it the new
usb_control_msg_send() API of USB Core.

Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Co-developed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 102 ++++++++------------
 1 file changed, 39 insertions(+), 63 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index f9c4fd5a2c53..8584baca9b8e 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -8,76 +8,51 @@
 #include "../include/recv_osdep.h"
 #include "../include/rtl8188e_hal.h"
 
-static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, u16 len, u8 requesttype)
+static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size)
 {
-	struct adapter	*adapt = pintfhdl->padapter;
-	struct dvobj_priv  *dvobjpriv = adapter_to_dvobj(adapt);
+	u16 value = (u16)(addr & 0x0000ffff);
+	struct adapter *adapt = intfhdl->padapter;
+	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
 	struct usb_device *udev = dvobjpriv->pusbdev;
-	unsigned int pipe;
-	int status = 0;
-	u8 *pIo_buf;
+	int status;
+	u8 *io_buf;
 	int vendorreq_times = 0;
 
-	if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) {
+	if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
 		status = -EPERM;
 		goto exit;
 	}
 
-	if (len > MAX_VENDOR_REQ_CMD_SIZE) {
-		DBG_88E("[%s] Buffer len error ,vendor request failed\n", __func__);
-		status = -EINVAL;
-		goto exit;
-	}
-
-	_enter_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL);
+	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
 
 	/*  Acquire IO memory for vendorreq */
-	pIo_buf = dvobjpriv->usb_vendor_req_buf;
+	io_buf = dvobjpriv->usb_vendor_req_buf;
 
-	if (!pIo_buf) {
-		DBG_88E("[%s] pIo_buf == NULL\n", __func__);
+	if (!io_buf) {
+		DBG_88E("[%s] io_buf == NULL\n", __func__);
 		status = -ENOMEM;
 		goto release_mutex;
 	}
 
-	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,
-					 pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
-
-		if (status == len) {   /*  Success this control transfer. */
+		status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+					      REALTEK_USB_VENQT_READ, value,
+					      REALTEK_USB_VENQT_CMD_IDX, io_buf,
+					      size, RTW_USB_CONTROL_MSG_TIMEOUT,
+					      GFP_KERNEL);
+		if (!status) {   /*  Success this control transfer. */
 			rtw_reset_continual_urb_error(dvobjpriv);
-			if (requesttype == REALTEK_USB_VENQT_READ)
-				memcpy(pdata, pIo_buf,  len);
+			memcpy(data, io_buf, size);
 		} else { /*  error cases */
-			DBG_88E("reg 0x%x, usb %s %u fail, status:%d value=0x%x, vendorreq_times:%d\n",
-				value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write",
-				len, status, *(u32 *)pdata, vendorreq_times);
-
-			if (status < 0) {
-				if (status == (-ESHUTDOWN) || status == -ENODEV) {
-					adapt->bSurpriseRemoved = true;
-				} else {
-					struct hal_data_8188e	*haldata = GET_HAL_DATA(adapt);
-					haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
-				}
-			} else { /*  status != len && status >= 0 */
-				if (status > 0) {
-					if (requesttype == REALTEK_USB_VENQT_READ) {
-						/*  For Control read transfer, we have to copy the read data from pIo_buf to pdata. */
-						memcpy(pdata, pIo_buf,  len);
-					}
-				}
+			DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n",
+				value, "read", size, status, vendorreq_times);
+
+			if (status == (-ESHUTDOWN) || status == -ENODEV) {
+				adapt->bSurpriseRemoved = true;
+			} else {
+				struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
+
+				haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
 			}
 
 			if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
@@ -87,17 +62,18 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 
 		}
 
-		/*  firmware download is checksumed, don't retry */
-		if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || status == len)
+		/*  firmware download is checksummed, don't retry */
+		if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status)
 			break;
 	}
+
 release_mutex:
-	_exit_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL);
+	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
 exit:
 	return status;
 }
 
-static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size)
+static int usb_write(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size)
 {
 	u16 value = (u16)(addr & 0x0000ffff);
 	struct adapter *adapt = intfhdl->padapter;
@@ -123,15 +99,15 @@ static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size)
 		goto release_mutex;
 	}
 
+	memcpy(io_buf, data, size);
 	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
-		status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
-					      REALTEK_USB_VENQT_READ, value,
+		status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+					      REALTEK_USB_VENQT_SEND, value,
 					      REALTEK_USB_VENQT_CMD_IDX, io_buf,
 					      size, RTW_USB_CONTROL_MSG_TIMEOUT,
 					      GFP_KERNEL);
 		if (!status) {   /*  Success this control transfer. */
 			rtw_reset_continual_urb_error(dvobjpriv);
-			memcpy(data, io_buf, size);
 		} else { /*  error cases */
 			DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n",
 				value, "read", size, status, vendorreq_times);
@@ -204,7 +180,7 @@ int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
 	u16 wvalue = (u16)(addr & 0x0000ffff);
 	int ret;
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
+	ret = usb_write(pintfhdl, wvalue, &val, 1);
 
 	return RTW_STATUS_CODE(ret);
 }
@@ -217,7 +193,7 @@ int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
 	__le32 data = cpu_to_le32(val & 0x0000ffff);
 	int ret;
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
+	ret = usb_write(pintfhdl, wvalue, &data, 2);
 
 	return RTW_STATUS_CODE(ret);
 }
@@ -230,7 +206,7 @@ int rtw_write32(struct adapter *adapter, u32 addr, u32 val)
 	__le32 data = cpu_to_le32(val);
 	int ret;
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE);
+	ret = usb_write(pintfhdl, wvalue, &data, 4);
 
 	return RTW_STATUS_CODE(ret);
 }
@@ -247,7 +223,7 @@ int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata)
 		return -EINVAL;
 
 	memcpy(buf, pdata, length);
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
+	ret = usb_write(pintfhdl, wvalue, buf, (length & 0xffff));
 
 	return RTW_STATUS_CODE(ret);
 }
-- 
2.33.0


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

* Re: [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N()
  2021-09-04 15:04 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco
@ 2021-09-04 17:09     ` kernel test robot
  0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-09-04 17:09 UTC (permalink / raw)
  To: Fabio M. De Francesco, Larry Finger, Phillip Potter,
	Greg Kroah-Hartman, linux-staging, linux-kernel
  Cc: kbuild-all, Fabio M. De Francesco, Pavel Skripkin

[-- Attachment #1: Type: text/plain, Size: 4404 bytes --]

Hi "Fabio,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git fde4862d1ac7100028da4371d92454fec6cf3f4f
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a08da7fbe9a78b73da29c82c244b022186aa035c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Fabio-M-De-Francesco/staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010
        git checkout a08da7fbe9a78b73da29c82c244b022186aa035c
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash drivers/staging/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/staging/r8188eu/hal/usb_ops_linux.c: In function 'usb_write':
>> drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: error: 'REALTEK_USB_VENQT_SEND' undeclared (first use in this function); did you mean 'REALTEK_USB_VENQT_READ'?
     105 |                                               REALTEK_USB_VENQT_SEND, value,
         |                                               ^~~~~~~~~~~~~~~~~~~~~~
         |                                               REALTEK_USB_VENQT_READ
   drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: note: each undeclared identifier is reported only once for each function it appears in


vim +105 drivers/staging/r8188eu/hal/usb_ops_linux.c

    75	
    76	static int usb_write(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size)
    77	{
    78		u16 value = (u16)(addr & 0x0000ffff);
    79		struct adapter *adapt = intfhdl->padapter;
    80		struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
    81		struct usb_device *udev = dvobjpriv->pusbdev;
    82		int status;
    83		u8 *io_buf;
    84		int vendorreq_times = 0;
    85	
    86		if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
    87			status = -EPERM;
    88			goto exit;
    89		}
    90	
    91		mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
    92	
    93		/*  Acquire IO memory for vendorreq */
    94		io_buf = dvobjpriv->usb_vendor_req_buf;
    95	
    96		if (!io_buf) {
    97			DBG_88E("[%s] io_buf == NULL\n", __func__);
    98			status = -ENOMEM;
    99			goto release_mutex;
   100		}
   101	
   102		memcpy(io_buf, data, size);
   103		while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
   104			status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
 > 105						      REALTEK_USB_VENQT_SEND, value,
   106						      REALTEK_USB_VENQT_CMD_IDX, io_buf,
   107						      size, RTW_USB_CONTROL_MSG_TIMEOUT,
   108						      GFP_KERNEL);
   109			if (!status) {   /*  Success this control transfer. */
   110				rtw_reset_continual_urb_error(dvobjpriv);
   111			} else { /*  error cases */
   112				DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n",
   113					value, "read", size, status, vendorreq_times);
   114	
   115				if (status == (-ESHUTDOWN) || status == -ENODEV) {
   116					adapt->bSurpriseRemoved = true;
   117				} else {
   118					struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
   119	
   120					haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
   121				}
   122	
   123				if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
   124					adapt->bSurpriseRemoved = true;
   125					break;
   126				}
   127			}
   128	
   129			/*  firmware download is checksummed, don't retry */
   130			if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status)
   131				break;
   132		}
   133	
   134	release_mutex:
   135		mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
   136	exit:
   137		return status;
   138	}
   139	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 55310 bytes --]

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

* Re: [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N()
@ 2021-09-04 17:09     ` kernel test robot
  0 siblings, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-09-04 17:09 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4511 bytes --]

Hi "Fabio,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on staging/staging-testing]

url:    https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git fde4862d1ac7100028da4371d92454fec6cf3f4f
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a08da7fbe9a78b73da29c82c244b022186aa035c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Fabio-M-De-Francesco/staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010
        git checkout a08da7fbe9a78b73da29c82c244b022186aa035c
        # save the attached .config to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash drivers/staging/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/staging/r8188eu/hal/usb_ops_linux.c: In function 'usb_write':
>> drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: error: 'REALTEK_USB_VENQT_SEND' undeclared (first use in this function); did you mean 'REALTEK_USB_VENQT_READ'?
     105 |                                               REALTEK_USB_VENQT_SEND, value,
         |                                               ^~~~~~~~~~~~~~~~~~~~~~
         |                                               REALTEK_USB_VENQT_READ
   drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: note: each undeclared identifier is reported only once for each function it appears in


vim +105 drivers/staging/r8188eu/hal/usb_ops_linux.c

    75	
    76	static int usb_write(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size)
    77	{
    78		u16 value = (u16)(addr & 0x0000ffff);
    79		struct adapter *adapt = intfhdl->padapter;
    80		struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
    81		struct usb_device *udev = dvobjpriv->pusbdev;
    82		int status;
    83		u8 *io_buf;
    84		int vendorreq_times = 0;
    85	
    86		if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
    87			status = -EPERM;
    88			goto exit;
    89		}
    90	
    91		mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
    92	
    93		/*  Acquire IO memory for vendorreq */
    94		io_buf = dvobjpriv->usb_vendor_req_buf;
    95	
    96		if (!io_buf) {
    97			DBG_88E("[%s] io_buf == NULL\n", __func__);
    98			status = -ENOMEM;
    99			goto release_mutex;
   100		}
   101	
   102		memcpy(io_buf, data, size);
   103		while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
   104			status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
 > 105						      REALTEK_USB_VENQT_SEND, value,
   106						      REALTEK_USB_VENQT_CMD_IDX, io_buf,
   107						      size, RTW_USB_CONTROL_MSG_TIMEOUT,
   108						      GFP_KERNEL);
   109			if (!status) {   /*  Success this control transfer. */
   110				rtw_reset_continual_urb_error(dvobjpriv);
   111			} else { /*  error cases */
   112				DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n",
   113					value, "read", size, status, vendorreq_times);
   114	
   115				if (status == (-ESHUTDOWN) || status == -ENODEV) {
   116					adapt->bSurpriseRemoved = true;
   117				} else {
   118					struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
   119	
   120					haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
   121				}
   122	
   123				if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
   124					adapt->bSurpriseRemoved = true;
   125					break;
   126				}
   127			}
   128	
   129			/*  firmware download is checksummed, don't retry */
   130			if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status)
   131				break;
   132		}
   133	
   134	release_mutex:
   135		mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
   136	exit:
   137		return status;
   138	}
   139	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 55310 bytes --]

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

* Re: [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N()
  2021-09-04 17:09     ` kernel test robot
@ 2021-09-04 20:41       ` Fabio M. De Francesco
  -1 siblings, 0 replies; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-09-04 20:41 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, Pavel Skripkin
  Cc: linux-staging, linux-kernel, kernel test robot, kbuild-all

On Saturday, September 4, 2021 7:09:58 PM CEST kernel test robot wrote:
> Hi "Fabio,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on staging/staging-testing]
> 
> url:    https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/
staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 
fde4862d1ac7100028da4371d92454fec6cf3f4f
>
> [...]
>
> All errors (new ones prefixed by >>):
> 
>    drivers/staging/r8188eu/hal/usb_ops_linux.c: In function 'usb_write':
> >> drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: error: 
'REALTEK_USB_VENQT_SEND' undeclared (first use in this function); did you 
mean 'REALTEK_USB_VENQT_READ'?
>      105 |                                               
REALTEK_USB_VENQT_SEND, value,
>          |                                               
^~~~~~~~~~~~~~~~~~~~~~
>
Hello to all,

I apologize for this easily avoidable mistake. I'm about to post the v2 
series.

However, I write this message to affirm that Pavel's tests on this patch are 
still valid and confirmed because his copy *has* the right parameter that is 
REALTEK_USB_VENQT_WRITE.

It's all my fault. Pavel warned me that during the review and test he noticed 
that I had written REALTEK_USB_VENQT_READ and obviously my build was 
successful because it was a symbol known to the compiler.

Pavel successfully changed his local copy and I (independently) changed mine 
without thinking about rebuilding. I was absolutely certain it was the 
expected parameter (perhaps the function name - usb_control_msg_send () - 
played a part in leading me to this stupid mistake).

In summary, first I want to apologize for the noise, secondly I want to 
apologize to Pavel who is co-author and tester of this 3/3 of our series.

Regards,

Fabio




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

* Re: [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N()
@ 2021-09-04 20:41       ` Fabio M. De Francesco
  0 siblings, 0 replies; 10+ messages in thread
From: Fabio M. De Francesco @ 2021-09-04 20:41 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1917 bytes --]

On Saturday, September 4, 2021 7:09:58 PM CEST kernel test robot wrote:
> Hi "Fabio,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on staging/staging-testing]
> 
> url:    https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/
staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git 
fde4862d1ac7100028da4371d92454fec6cf3f4f
>
> [...]
>
> All errors (new ones prefixed by >>):
> 
>    drivers/staging/r8188eu/hal/usb_ops_linux.c: In function 'usb_write':
> >> drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: error: 
'REALTEK_USB_VENQT_SEND' undeclared (first use in this function); did you 
mean 'REALTEK_USB_VENQT_READ'?
>      105 |                                               
REALTEK_USB_VENQT_SEND, value,
>          |                                               
^~~~~~~~~~~~~~~~~~~~~~
>
Hello to all,

I apologize for this easily avoidable mistake. I'm about to post the v2 
series.

However, I write this message to affirm that Pavel's tests on this patch are 
still valid and confirmed because his copy *has* the right parameter that is 
REALTEK_USB_VENQT_WRITE.

It's all my fault. Pavel warned me that during the review and test he noticed 
that I had written REALTEK_USB_VENQT_READ and obviously my build was 
successful because it was a symbol known to the compiler.

Pavel successfully changed his local copy and I (independently) changed mine 
without thinking about rebuilding. I was absolutely certain it was the 
expected parameter (perhaps the function name - usb_control_msg_send () - 
played a part in leading me to this stupid mistake).

In summary, first I want to apologize for the noise, secondly I want to 
apologize to Pavel who is co-author and tester of this 3/3 of our series.

Regards,

Fabio



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

* Re: [PATCH 1/3] staging: r8188eu: remove _io_ops structure
  2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco
@ 2021-09-06  7:49   ` Dan Carpenter
  2021-09-06 14:04     ` Pavel Skripkin
  0 siblings, 1 reply; 10+ messages in thread
From: Dan Carpenter @ 2021-09-06  7:49 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Pavel Skripkin

On Sat, Sep 04, 2021 at 05:04:45PM +0200, Fabio M. De Francesco wrote:
> -static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
> +int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata)
>  {
> +	struct io_priv *pio_priv = &adapter->iopriv;
> +	struct intf_hdl *pintfhdl = &pio_priv->intf;
>  	u16 wvalue = (u16)(addr & 0x0000ffff);
>  	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
> +	int ret;
>  
>  	if (length > VENDOR_CMD_MAX_DATA_LEN)
>  		return -EINVAL;

The caller treats this negative return as success.

>  
>  	memcpy(buf, pdata, length);
> +	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
>  
> -	return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
> +	return RTW_STATUS_CODE(ret);
>  }

[ snip ]


> -void usb_read_port_cancel(struct intf_hdl *pintfhdl)
> +void rtw_read_port_cancel(struct adapter *adapter)
>  {
>  	int i;
> -	struct recv_buf *precvbuf;
> -	struct adapter	*padapter = pintfhdl->padapter;
> -	precvbuf = (struct recv_buf *)padapter->recvpriv.precv_buf;
> +	struct recv_buf *precvbuf = (struct recv_buf *) adapter->recvpriv.precv_buf;
>  
>  	DBG_88E("%s\n", __func__);
>  
> -	padapter->bReadPortCancel = true;
> +	adapter->bReadPortCancel = true;

In these functions it would be better to rename "padapter" as "adapter"
in a follow on function.  Keep it the same for now just to make review
easier.

>  
>  	for (i = 0; i < NR_RECVBUFF; i++) {
>  		precvbuf->reuse = true;
> @@ -134,22 +124,21 @@ static void usb_write_port_complete(struct urb *purb, struct pt_regs *regs)
>  
>  }
>  
> -u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
> +u32 rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *wmem)
>  {
>  	unsigned long irqL;
>  	unsigned int pipe;
>  	int status;
>  	u32 ret = _FAIL;
>  	struct urb *purb = NULL;
> -	struct adapter *padapter = (struct adapter *)pintfhdl->padapter;
> -	struct dvobj_priv	*pdvobj = adapter_to_dvobj(padapter);
> -	struct xmit_priv	*pxmitpriv = &padapter->xmitpriv;
> +	struct dvobj_priv	*pdvobj = adapter_to_dvobj(adapter);
> +	struct xmit_priv	*pxmitpriv = &adapter->xmitpriv;
>  	struct xmit_buf *pxmitbuf = (struct xmit_buf *)wmem;
>  	struct xmit_frame *pxmitframe = (struct xmit_frame *)pxmitbuf->priv_data;
>  	struct usb_device *pusbd = pdvobj->pusbdev;
>  
> -	if ((padapter->bDriverStopped) || (padapter->bSurpriseRemoved) ||
> -	    (padapter->pwrctrlpriv.pnp_bstop_trx)) {
> +	if ((adapter->bDriverStopped) || (adapter->bSurpriseRemoved) ||
> +	    (adapter->pwrctrlpriv.pnp_bstop_trx)) {

Here too etc.  And below.

regards,
dan carpenter

>  		rtw_sctx_done_err(&pxmitbuf->sctx, RTW_SCTX_DONE_TX_DENY);
>  		goto exit;
>  	}
> @@ -196,7 +185,7 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
>  
>  	status = usb_submit_urb(purb, GFP_ATOMIC);
>  	if (!status) {
> -		struct hal_data_8188e	*haldata = GET_HAL_DATA(padapter);
> +		struct hal_data_8188e	*haldata = GET_HAL_DATA(adapter);
>  
>  		haldata->srestpriv.last_tx_time = jiffies;
>  	} else {
> @@ -205,7 +194,7 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
>  
>  		switch (status) {
>  		case -ENODEV:
> -			padapter->bDriverStopped = true;
> +			adapter->bDriverStopped = true;
>  			break;
>  		default:
>  			break;
> @@ -224,15 +213,14 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem)
>  	return ret;
>  }
>  
> -void usb_write_port_cancel(struct intf_hdl *pintfhdl)
> +void rtw_write_port_cancel(struct adapter *adapter)
>  {
>  	int i, j;
> -	struct adapter	*padapter = pintfhdl->padapter;
> -	struct xmit_buf *pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmitbuf;
> +	struct xmit_buf *pxmitbuf = (struct xmit_buf *) adapter->xmitpriv.pxmitbuf;
>  
>  	DBG_88E("%s\n", __func__);
>  
> -	padapter->bWritePortCancel = true;
> +	adapter->bWritePortCancel = true;
>  
>  	for (i = 0; i < NR_XMITBUFF; i++) {
>  		for (j = 0; j < 8; j++) {
> @@ -242,7 +230,7 @@ void usb_write_port_cancel(struct intf_hdl *pintfhdl)
>  		pxmitbuf++;
>  	}
>  
> -	pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmit_extbuf;
> +	pxmitbuf = (struct xmit_buf *) adapter->xmitpriv.pxmit_extbuf;
>  	for (i = 0; i < NR_XMIT_EXTBUFF; i++) {
>  		for (j = 0; j < 8; j++) {
>  			if (pxmitbuf->pxmit_urb[j])
> -- 
> 2.33.0
> 

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

* Re: [PATCH 1/3] staging: r8188eu: remove _io_ops structure
  2021-09-06  7:49   ` Dan Carpenter
@ 2021-09-06 14:04     ` Pavel Skripkin
  0 siblings, 0 replies; 10+ messages in thread
From: Pavel Skripkin @ 2021-09-06 14:04 UTC (permalink / raw)
  To: Dan Carpenter, Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel

On 9/6/21 10:49 AM, Dan Carpenter wrote:
> On Sat, Sep 04, 2021 at 05:04:45PM +0200, Fabio M. De Francesco wrote:
>> -static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata)
>> +int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata)
>>  {
>> +	struct io_priv *pio_priv = &adapter->iopriv;
>> +	struct intf_hdl *pintfhdl = &pio_priv->intf;
>>  	u16 wvalue = (u16)(addr & 0x0000ffff);
>>  	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
>> +	int ret;
>>  
>>  	if (length > VENDOR_CMD_MAX_DATA_LEN)
>>  		return -EINVAL;
> 
> The caller treats this negative return as success.
> 

Oh, good catch, thank you so much for pointing it out... This driver 
uses unusual approach for error handling with private _SUCCESS and _FAIL 
macros and I forgot to call RTW_ERROR_CODE :(

>>  
>>  	memcpy(buf, pdata, length);
>> +	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
>>  
>> -	return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
>> +	return RTW_STATUS_CODE(ret);
>>  }
> 
> [ snip ]
> 
> 
>> -void usb_read_port_cancel(struct intf_hdl *pintfhdl)
>> +void rtw_read_port_cancel(struct adapter *adapter)
>>  {
>>  	int i;
>> -	struct recv_buf *precvbuf;
>> -	struct adapter	*padapter = pintfhdl->padapter;
>> -	precvbuf = (struct recv_buf *)padapter->recvpriv.precv_buf;
>> +	struct recv_buf *precvbuf = (struct recv_buf *) adapter->recvpriv.precv_buf;
>>  
>>  	DBG_88E("%s\n", __func__);
>>  
>> -	padapter->bReadPortCancel = true;
>> +	adapter->bReadPortCancel = true;
> 
> In these functions it would be better to rename "padapter" as "adapter"
> in a follow on function.  Keep it the same for now just to make review
> easier.
> 

Ok, will bring old name back in v4. Thank you for review :)




With regards,
Pavel Skripkin

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

end of thread, other threads:[~2021-09-06 14:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04 15:04 [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco
2021-09-06  7:49   ` Dan Carpenter
2021-09-06 14:04     ` Pavel Skripkin
2021-09-04 15:04 ` [PATCH 2/3] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco
2021-09-04 15:04 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco
2021-09-04 17:09   ` kernel test robot
2021-09-04 17:09     ` kernel test robot
2021-09-04 20:41     ` Fabio M. De Francesco
2021-09-04 20:41       ` Fabio M. De Francesco

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.