LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain
@ 2021-09-13 18:09 Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 01/18] staging: r8188eu: remove usb_{read,write}_mem Fabio M. De Francesco
                   ` (18 more replies)
  0 siblings, 19 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  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] and by Fabio M.
De Francesco using a ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano].

Fabio M. De Francesco (4):
  staging: r8188eu: hal: Clean up usbctrl_vendorreq()
  staging: r8188eu: hal: Clean up rtw_read*() and rtw_write*()
  staging: r8188eu: Shorten calls chain of rtw_read8/16/32()
  staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N()

Pavel Skripkin (14):
  staging: r8188eu: remove usb_{read,write}_mem
  staging: r8188eu: remove the helpers of rtw_read8()
  staging: r8188eu: remove the helpers of rtw_read16()
  staging: r8188eu: remove the helpers of rtw_read32()
  staging: r8188eu: remove the helpers of usb_write8
  staging: r8188eu: remove the helpers of usb_write16
  staging: r8188eu: remove the helpers of usb_write32
  staging: r8188eu: remove the helpers of usb_writeN
  staging: r8188eu: remove the helpers of usb_read_port
  staging: r8188eu: remove the helpers of usb_write_port
  staging: r8188eu: remove the helpers of usb_read_port_cancel
  staging: r8188eu: remove the helpers of usb_write_port_cancel
  staging: r8188eu: remove core/rtw_io.c
  staging: remove struct _io_ops

 drivers/staging/r8188eu/Makefile              |   1 -
 drivers/staging/r8188eu/core/rtw_io.c         | 299 ------------------
 drivers/staging/r8188eu/hal/usb_halinit.c     |   6 +-
 drivers/staging/r8188eu/hal/usb_ops_linux.c   | 250 ++++++++-------
 drivers/staging/r8188eu/include/rtw_io.h      |  89 +-----
 drivers/staging/r8188eu/include/usb_ops.h     |   2 -
 .../staging/r8188eu/include/usb_ops_linux.h   |   8 -
 drivers/staging/r8188eu/os_dep/usb_intf.c     |   8 +-
 .../staging/r8188eu/os_dep/usb_ops_linux.c    |  20 +-
 9 files changed, 157 insertions(+), 526 deletions(-)
 delete mode 100644 drivers/staging/r8188eu/core/rtw_io.c

-- 
2.33.0


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

* [PATCH v4 01/18] staging: r8188eu: remove usb_{read,write}_mem
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 02/18] staging: r8188eu: remove the helpers of rtw_read8() Fabio M. De Francesco
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove usb_{read,write}_mem() because they are unused.

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>
---

v3->v4:
	Split a patch into fourteen.
v2->v3:
	No changes.
v1->v2:
	No changes.

 drivers/staging/r8188eu/core/rtw_io.c         | 29 -------------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c   |  2 --
 drivers/staging/r8188eu/include/rtw_io.h      |  4 ---
 .../staging/r8188eu/include/usb_ops_linux.h   |  3 --
 .../staging/r8188eu/os_dep/usb_ops_linux.c    |  8 -----
 5 files changed, 46 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index cde0205816b1..e6f377377ab2 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -175,35 +175,6 @@ int _rtw_write32_async(struct adapter *adapter, u32 addr, u32 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);
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 065b0d8e030a..7f30b00b3ce6 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -541,13 +541,11 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 	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;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 263a37d49b6e..5ef89c72cc83 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -270,8 +270,6 @@ 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))
@@ -290,8 +288,6 @@ void _rtw_write_port_cancel(struct adapter *adapter);
 	_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)	\
diff --git a/drivers/staging/r8188eu/include/usb_ops_linux.h b/drivers/staging/r8188eu/include/usb_ops_linux.h
index c357a3b1560e..37e0614fd15c 100644
--- a/drivers/staging/r8188eu/include/usb_ops_linux.h
+++ b/drivers/staging/r8188eu/include/usb_ops_linux.h
@@ -28,9 +28,6 @@
 
 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);
diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index 928730158450..9afb4df71969 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -31,14 +31,6 @@ 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)
 {
 	int i;
-- 
2.33.0


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

* [PATCH v4 02/18] staging: r8188eu: remove the helpers of rtw_read8()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 01/18] staging: r8188eu: remove usb_{read,write}_mem Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 03/18] staging: r8188eu: remove the helpers of rtw_read16() Fabio M. De Francesco
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_read8() and usb_read8() and embed their
code into the caller (i.e., rtw_read8()).

_rtw_read8() is a mere redefinition of rtw_read8() and it is unneeded.
usb_read8() was the only functions assigned to (*_usb_read8) pointer,
so we can simply remove it and make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c       | 14 --------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c |  5 +++--
 drivers/staging/r8188eu/include/rtw_io.h    |  3 +--
 3 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index e6f377377ab2..4c43b6d00178 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -34,20 +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;
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 7f30b00b3ce6..8389deeb1182 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;
 
@@ -538,7 +540,6 @@ 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_port = &usb_read_port;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 5ef89c72cc83..9dc32f7bcae8 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -245,7 +245,7 @@ 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);
+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);
@@ -267,7 +267,6 @@ 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_port(adapter, addr, cnt, mem)				\
-- 
2.33.0


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

* [PATCH v4 03/18] staging: r8188eu: remove the helpers of rtw_read16()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 01/18] staging: r8188eu: remove usb_{read,write}_mem Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 02/18] staging: r8188eu: remove the helpers of rtw_read8() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 04/18] staging: r8188eu: remove the helpers of rtw_read32() Fabio M. De Francesco
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_read16() and usb_read16() and embed their
code into the caller (i.e., rtw_read16()).

_rtw_read16() is a mere redefinition of rtw_read16() and it is unneeded.
usb_read16() was the only functions assigned to the (*_usb_read16) pointer,
so we can simply remove it and make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c       | 14 --------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c |  5 +++--
 drivers/staging/r8188eu/include/rtw_io.h    |  3 +--
 3 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index 4c43b6d00178..b5d1c8e52b22 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -34,20 +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)
 
-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;
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 8389deeb1182..8b4fc014d93a 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -109,8 +109,10 @@ u8 rtw_read8(struct adapter *adapter, 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;
 
@@ -540,7 +542,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 {
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
-	pops->_read16 = &usb_read16;
 	pops->_read32 = &usb_read32;
 	pops->_read_port = &usb_read_port;
 	pops->_write8 = &usb_write8;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 9dc32f7bcae8..527503d3ecc8 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -246,7 +246,7 @@ 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);
+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);
@@ -267,7 +267,6 @@ 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_read16(adapter, addr) _rtw_read16((adapter), (addr))
 #define rtw_read32(adapter, addr) _rtw_read32((adapter), (addr))
 #define rtw_read_port(adapter, addr, cnt, mem)				\
 	_rtw_read_port((adapter), (addr), (cnt), (mem))
-- 
2.33.0


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

* [PATCH v4 04/18] staging: r8188eu: remove the helpers of rtw_read32()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (2 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 03/18] staging: r8188eu: remove the helpers of rtw_read16() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 05/18] staging: r8188eu: remove the helpers of usb_write8 Fabio M. De Francesco
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_readr32() and usb_read32() and embed their
code into the caller (i.e., rtw_read32()).

_rtw_read32() is a mere redefinition of rtw_read32() and it is unneeded.
usb_read32() was the only functions assigned to the (*_usb_read32) pointer,
so we can simply remove it and make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c       | 14 --------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c |  5 +++--
 drivers/staging/r8188eu/include/rtw_io.h    |  3 +--
 3 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index b5d1c8e52b22..cb24500cbc6e 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -34,20 +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)
 
-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;
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 8b4fc014d93a..39fd9994787d 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -121,8 +121,10 @@ u16 rtw_read16(struct adapter *adapter, 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;
 
@@ -542,7 +544,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 {
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
-	pops->_read32 = &usb_read32;
 	pops->_read_port = &usb_read_port;
 	pops->_write8 = &usb_write8;
 	pops->_write16 = &usb_write16;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 527503d3ecc8..c53d9c8bd9a7 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -247,7 +247,7 @@ 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);
+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);
@@ -267,7 +267,6 @@ 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_read32(adapter, addr) _rtw_read32((adapter), (addr))
 #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))
-- 
2.33.0


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

* [PATCH v4 05/18] staging: r8188eu: remove the helpers of usb_write8
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (3 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 04/18] staging: r8188eu: remove the helpers of rtw_read32() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 06/18] staging: r8188eu: remove the helpers of usb_write16() Fabio M. De Francesco
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_write8() and usb_write8() and embed their
code into the caller (i.e., rtw_write8()).

_rtw_write8() is a mere redefinition of rtw_write8() and it is unneeded.
usb_write8() was the only functions assigned to the (*_usb_write8) pointer,
so we can simply remove it and make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c       | 15 ---------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 10 +++++++---
 drivers/staging/r8188eu/include/rtw_io.h    |  4 +---
 3 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index cb24500cbc6e..69ab6e24a4b7 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -34,21 +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)
 
-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;
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 39fd9994787d..69e64863a5d1 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -133,11 +133,16 @@ u32 rtw_read32(struct adapter *adapter, 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;
+
+	ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
 
-	return 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)
@@ -545,7 +550,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
 	pops->_read_port = &usb_read_port;
-	pops->_write8 = &usb_write8;
 	pops->_write16 = &usb_write16;
 	pops->_write32 = &usb_write32;
 	pops->_writeN = &usb_writeN;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index c53d9c8bd9a7..3b229027f094 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -252,7 +252,7 @@ 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);
 
-int _rtw_write8(struct adapter *adapter, u32 addr, u8 val);
+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);
@@ -271,8 +271,6 @@ void _rtw_write_port_cancel(struct adapter *adapter);
 	_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)				\
-- 
2.33.0


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

* [PATCH v4 06/18] staging: r8188eu: remove the helpers of usb_write16()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (4 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 05/18] staging: r8188eu: remove the helpers of usb_write8 Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 07/18] staging: r8188eu: remove the helpers of usb_write32() Fabio M. De Francesco
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_write16() and usb_write16() and embed their
code into the caller (i.e., rtw_write16()).

_rtw_write16() is a mere redefinition of rtw_write16() and it is unneeded.
usb_write16() was the only functions assigned to the (*_usb_write16)
pointer, so we can simply remove it and make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c       | 14 --------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 10 +++++++---
 drivers/staging/r8188eu/include/rtw_io.h    |  4 +---
 3 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index 69ab6e24a4b7..7547e25d870b 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -34,20 +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)
 
-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;
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 69e64863a5d1..128cdb178abc 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -145,12 +145,17 @@ int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
 	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;
 
-	return usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
+	ret = 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)
@@ -550,7 +555,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
 	pops->_read_port = &usb_read_port;
-	pops->_write16 = &usb_write16;
 	pops->_write32 = &usb_write32;
 	pops->_writeN = &usb_writeN;
 	pops->_write_port = &usb_write_port;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 3b229027f094..0cb1f626a549 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -253,7 +253,7 @@ void _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_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);
 
@@ -271,8 +271,6 @@ void _rtw_write_port_cancel(struct adapter *adapter);
 	_rtw_read_port((adapter), (addr), (cnt), (mem))
 #define rtw_read_port_cancel(adapter) _rtw_read_port_cancel((adapter))
 
-#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)			\
-- 
2.33.0


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

* [PATCH v4 07/18] staging: r8188eu: remove the helpers of usb_write32()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (5 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 06/18] staging: r8188eu: remove the helpers of usb_write16() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 08/18] staging: r8188eu: remove the helpers of usb_writeN() Fabio M. De Francesco
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_write32() and usb_write32() and embed their
code into the caller (i.e., rtw_write32()).

_rtw_write32() is a mere redefinition of rtw_write32() and it is unneeded.
usb_write32() was the only functions assigned to the (*_usb_write32)
pointer, so we can simply remove it and make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c       | 15 ---------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 10 +++++++---
 drivers/staging/r8188eu/include/rtw_io.h    |  4 +---
 3 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index 7547e25d870b..98c9823fe53b 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -34,21 +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)
 
-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;
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 128cdb178abc..73f90b21364e 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -158,12 +158,17 @@ int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
 	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)
@@ -555,7 +560,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
 	pops->_read_port = &usb_read_port;
-	pops->_write32 = &usb_write32;
 	pops->_writeN = &usb_writeN;
 	pops->_write_port = &usb_write_port;
 	pops->_read_port_cancel = &usb_read_port_cancel;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 0cb1f626a549..83e2ed13b667 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -254,7 +254,7 @@ 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_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);
@@ -271,8 +271,6 @@ void _rtw_write_port_cancel(struct adapter *adapter);
 	_rtw_read_port((adapter), (addr), (cnt), (mem))
 #define rtw_read_port_cancel(adapter) _rtw_read_port_cancel((adapter))
 
-#define  rtw_write32(adapter, addr, val)				\
-	_rtw_write32((adapter), (addr), (val))
 #define  rtw_writeN(adapter, addr, length, data)			\
 	_rtw_writeN((adapter), (addr), (length), (data))
 #define rtw_write8_async(adapter, addr, val)				\
-- 
2.33.0


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

* [PATCH v4 08/18] staging: r8188eu: remove the helpers of usb_writeN()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (6 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 07/18] staging: r8188eu: remove the helpers of usb_write32() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 09/18] staging: r8188eu: remove the helpers of rtw_read_port() Fabio M. De Francesco
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_writeN() and usb_writeN() and embed their
code into the caller (i.e., rtw_writeN()).

_rtw_writeN() is a mere redefinition of rtw_writeN() and it is unneeded.
usb_writeN() was the only functions assigned to the (*_usb_writeN) pointer,
so we can simply remove it and make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c       | 14 --------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c | 11 +++++++----
 drivers/staging/r8188eu/include/rtw_io.h    |  4 +---
 3 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index 98c9823fe53b..b2e41fa48d81 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -34,20 +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)
 
-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;
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 73f90b21364e..90fb46c75159 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -171,17 +171,21 @@ int rtw_write32(struct adapter *adapter, u32 addr, u32 val)
 	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;
+		return _FAIL;
 
 	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)
@@ -560,7 +564,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
 	pops->_read_port = &usb_read_port;
-	pops->_writeN = &usb_writeN;
 	pops->_write_port = &usb_write_port;
 	pops->_read_port_cancel = &usb_read_port_cancel;
 	pops->_write_port_cancel = &usb_write_port_cancel;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 83e2ed13b667..1123017cac65 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -255,7 +255,7 @@ 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_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);
@@ -271,8 +271,6 @@ void _rtw_write_port_cancel(struct adapter *adapter);
 	_rtw_read_port((adapter), (addr), (cnt), (mem))
 #define rtw_read_port_cancel(adapter) _rtw_read_port_cancel((adapter))
 
-#define  rtw_writeN(adapter, addr, length, data)			\
-	_rtw_writeN((adapter), (addr), (length), (data))
 #define rtw_write8_async(adapter, addr, val)				\
 	_rtw_write8_async((adapter), (addr), (val))
 #define rtw_write16_async(adapter, addr, val)				\
-- 
2.33.0


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

* [PATCH v4 09/18] staging: r8188eu: remove the helpers of rtw_read_port()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (7 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 08/18] staging: r8188eu: remove the helpers of usb_writeN() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 10/18] staging: r8188eu: remove the helpers of rtw_write_port() Fabio M. De Francesco
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_read_port() and usb_read_port() and embed their
code into the caller (i.e., rtw_read_port()).

_rtw_read_port() is a mere redefinition of rtw_read_port() and it is
unneeded. usb_read_port() was the only functions assigned to the
(*_usb_read_port) pointer, so we can simply remove it and make a direct
call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c       | 18 ------------------
 drivers/staging/r8188eu/hal/usb_halinit.c   |  6 +-----
 drivers/staging/r8188eu/hal/usb_ops_linux.c |  4 +---
 drivers/staging/r8188eu/include/rtw_io.h    |  4 +---
 4 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index b2e41fa48d81..ac72f894da75 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -75,24 +75,6 @@ int _rtw_write32_async(struct adapter *adapter, u32 addr, u32 val)
 	return RTW_STATUS_CODE(ret);
 }
 
-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);
diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
index 0c317a8723ef..59a7af696121 100644
--- a/drivers/staging/r8188eu/hal/usb_halinit.c
+++ b/drivers/staging/r8188eu/hal/usb_halinit.c
@@ -1047,11 +1047,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;
 
@@ -1060,7 +1056,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 90fb46c75159..a104e3fac7d1 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -456,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;
@@ -563,7 +562,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 {
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
-	pops->_read_port = &usb_read_port;
 	pops->_write_port = &usb_write_port;
 	pops->_read_port_cancel = &usb_read_port_cancel;
 	pops->_write_port_cancel = &usb_write_port_cancel;
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 1123017cac65..600c6e7a375b 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -249,7 +249,7 @@ 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);
+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);
@@ -267,8 +267,6 @@ 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_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_async(adapter, addr, val)				\
-- 
2.33.0


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

* [PATCH v4 10/18] staging: r8188eu: remove the helpers of rtw_write_port()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (8 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 09/18] staging: r8188eu: remove the helpers of rtw_read_port() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 11/18] staging: r8188eu: remove the helpers of usb_read_port_cancel() Fabio M. De Francesco
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_write_port() and usb_write_port() and embed
their code into the caller (i.e., rtw_write_port()).

_rtw_write_port() is a mere redefinition of rtw_write_port() and it is
unneeded. usb_write_port() was the only functions assigned to the
(*_usb_write_port) pointer, so we can simply remove it and make a direct
call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c         | 20 +------------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c   |  1 -
 drivers/staging/r8188eu/include/rtw_io.h      |  4 +---
 .../staging/r8188eu/include/usb_ops_linux.h   |  1 -
 .../staging/r8188eu/os_dep/usb_ops_linux.c    |  3 +--
 5 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index ac72f894da75..3a5e9dbfcb12 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -87,24 +87,6 @@ void _rtw_read_port_cancel(struct adapter *adapter)
 		_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;
@@ -114,7 +96,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);
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index a104e3fac7d1..4fea21c0f7af 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -562,7 +562,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 {
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
-	pops->_write_port = &usb_write_port;
 	pops->_read_port_cancel = &usb_read_port_cancel;
 	pops->_write_port_cancel = &usb_write_port_cancel;
 
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 600c6e7a375b..f2b1978b6e80 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -262,7 +262,7 @@ 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);
@@ -275,8 +275,6 @@ void _rtw_write_port_cancel(struct adapter *adapter);
 	_rtw_write16_async((adapter), (addr), (val))
 #define rtw_write32_async(adapter, addr, val)				\
 	_rtw_write32_async((adapter), (addr), (val))
-#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))
diff --git a/drivers/staging/r8188eu/include/usb_ops_linux.h b/drivers/staging/r8188eu/include/usb_ops_linux.h
index 37e0614fd15c..bdc596fe5854 100644
--- a/drivers/staging/r8188eu/include/usb_ops_linux.h
+++ b/drivers/staging/r8188eu/include/usb_ops_linux.h
@@ -30,7 +30,6 @@ unsigned int ffaddr2pipehdl(struct dvobj_priv *pdvobj, u32 addr);
 
 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_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index 9afb4df71969..36ef06f88fdd 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -124,14 +124,13 @@ 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 *padapter, 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 xmit_buf *pxmitbuf = (struct xmit_buf *)wmem;
-- 
2.33.0


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

* [PATCH v4 11/18] staging: r8188eu: remove the helpers of usb_read_port_cancel()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (9 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 10/18] staging: r8188eu: remove the helpers of rtw_write_port() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 12/18] staging: r8188eu: remove the helpers of rtw_write_port_cancel() Fabio M. De Francesco
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_read_port_cancel() and usb_read_port_cancel()
and embed their code into the caller (i.e., rtw_read_port_cancel()).

_rtw_read_port_cancel() is a mere redefinition of rtw_read_port_cancel()
and it is unneeded. usb_read_port_cancel() was the only functions assigned
to the (*_usb_read_port_cancel) pointer, so we can simply remove it and
make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c           | 11 -----------
 drivers/staging/r8188eu/hal/usb_ops_linux.c     |  1 -
 drivers/staging/r8188eu/include/rtw_io.h        |  3 +--
 drivers/staging/r8188eu/include/usb_ops_linux.h |  2 --
 drivers/staging/r8188eu/os_dep/usb_ops_linux.c  |  6 ++----
 5 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index 3a5e9dbfcb12..a57742057a65 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -75,17 +75,6 @@ int _rtw_write32_async(struct adapter *adapter, u32 addr, u32 val)
 	return RTW_STATUS_CODE(ret);
 }
 
-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_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem, int timeout_ms)
 {
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 4fea21c0f7af..1865a26142bc 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -562,7 +562,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 {
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
-	pops->_read_port_cancel = &usb_read_port_cancel;
 	pops->_write_port_cancel = &usb_write_port_cancel;
 
 }
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index f2b1978b6e80..56e17e2a7ee2 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -250,7 +250,7 @@ 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);
 u32 rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem);
-void _rtw_read_port_cancel(struct adapter *adapter);
+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);
@@ -267,7 +267,6 @@ 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_read_port_cancel(adapter) _rtw_read_port_cancel((adapter))
 
 #define rtw_write8_async(adapter, addr, val)				\
 	_rtw_write8_async((adapter), (addr), (val))
diff --git a/drivers/staging/r8188eu/include/usb_ops_linux.h b/drivers/staging/r8188eu/include/usb_ops_linux.h
index bdc596fe5854..186c6b7628dc 100644
--- a/drivers/staging/r8188eu/include/usb_ops_linux.h
+++ b/drivers/staging/r8188eu/include/usb_ops_linux.h
@@ -28,8 +28,6 @@
 
 unsigned int ffaddr2pipehdl(struct dvobj_priv *pdvobj, u32 addr);
 
-void usb_read_port_cancel(struct intf_hdl *pintfhdl);
-
 void usb_write_port_cancel(struct intf_hdl *pintfhdl);
 
 #endif
diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index 36ef06f88fdd..a98ffdf92ed4 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -31,12 +31,10 @@ struct zero_bulkout_context {
 	void *padapter;
 };
 
-void usb_read_port_cancel(struct intf_hdl *pintfhdl)
+void rtw_read_port_cancel(struct adapter *padapter)
 {
 	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 *)padapter->recvpriv.precv_buf;
 
 	DBG_88E("%s\n", __func__);
 
-- 
2.33.0


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

* [PATCH v4 12/18] staging: r8188eu: remove the helpers of rtw_write_port_cancel()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (10 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 11/18] staging: r8188eu: remove the helpers of usb_read_port_cancel() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 13/18] staging: r8188eu: remove core/rtw_io.c Fabio M. De Francesco
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Remove the unnecessary _rtw_write_port_cancel() and usb_write_port_cancel()
and embed their code into the caller (i.e., rtw_write_port_cancel()).

_rtw_write_port_cancel() is a mere redefinition of rtw_write_port_cancel()
and it is unneeded. usb_write_port_cancel() was the only functions
assigned to the (*_usb_write_port_cancel) pointer, so we can simply remove
it and make a direct call.

This patch is in preparation for the _io_ops structure removal.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/core/rtw_io.c           | 12 ------------
 drivers/staging/r8188eu/hal/usb_ops_linux.c     |  2 --
 drivers/staging/r8188eu/include/rtw_io.h        |  3 +--
 drivers/staging/r8188eu/include/usb_ops_linux.h |  2 --
 drivers/staging/r8188eu/os_dep/usb_ops_linux.c  |  3 +--
 5 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
index a57742057a65..74b02ff8e44d 100644
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ b/drivers/staging/r8188eu/core/rtw_io.c
@@ -93,18 +93,6 @@ 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))
 {
 	struct io_priv	*piopriv = &padapter->iopriv;
diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 1865a26142bc..2516cfc464a9 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -562,8 +562,6 @@ void rtl8188eu_set_intf_ops(struct _io_ops	*pops)
 {
 
 	memset((u8 *)pops, 0, sizeof(struct _io_ops));
-	pops->_write_port_cancel = &usb_write_port_cancel;
-
 }
 
 void rtl8188eu_set_hw_type(struct adapter *adapt)
diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 56e17e2a7ee2..4f4678a55687 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -265,7 +265,7 @@ 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_and_wait(struct adapter *adapter, u32 addr, u32 cnt,
 			     u8 *pmem, int timeout_ms);
-void _rtw_write_port_cancel(struct adapter *adapter);
+void rtw_write_port_cancel(struct adapter *adapter);
 
 
 #define rtw_write8_async(adapter, addr, val)				\
@@ -276,7 +276,6 @@ void _rtw_write_port_cancel(struct adapter *adapter);
 	_rtw_write32_async((adapter), (addr), (val))
 #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))
 
 /* ioreq */
 void ioreq_read8(struct adapter *adapter, u32 addr, u8 *pval);
diff --git a/drivers/staging/r8188eu/include/usb_ops_linux.h b/drivers/staging/r8188eu/include/usb_ops_linux.h
index 186c6b7628dc..641f059ffaf7 100644
--- a/drivers/staging/r8188eu/include/usb_ops_linux.h
+++ b/drivers/staging/r8188eu/include/usb_ops_linux.h
@@ -28,6 +28,4 @@
 
 unsigned int ffaddr2pipehdl(struct dvobj_priv *pdvobj, u32 addr);
 
-void usb_write_port_cancel(struct intf_hdl *pintfhdl);
-
 #endif
diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index a98ffdf92ed4..4085f3e6067d 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -211,10 +211,9 @@ u32 rtw_write_port(struct adapter *padapter, u32 addr, u32 cnt, u8 *wmem)
 	return ret;
 }
 
-void usb_write_port_cancel(struct intf_hdl *pintfhdl)
+void rtw_write_port_cancel(struct adapter *padapter)
 {
 	int i, j;
-	struct adapter	*padapter = pintfhdl->padapter;
 	struct xmit_buf *pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmitbuf;
 
 	DBG_88E("%s\n", __func__);
-- 
2.33.0


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

* [PATCH v4 13/18] staging: r8188eu: remove core/rtw_io.c
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (11 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 12/18] staging: r8188eu: remove the helpers of rtw_write_port_cancel() Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 14/18] staging: remove struct _io_ops Fabio M. De Francesco
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

There are only unused functions and macros and one function which can be
open-coded.

So, remove core/rtw_io.c at all, remove core/rtw_io.c from Makefile
and open-code rtw_init_io_priv() in rtw_usb_if1_init().

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/Makefile          |   1 -
 drivers/staging/r8188eu/core/rtw_io.c     | 111 ----------------------
 drivers/staging/r8188eu/include/rtw_io.h  |  19 +---
 drivers/staging/r8188eu/os_dep/usb_intf.c |   8 +-
 4 files changed, 8 insertions(+), 131 deletions(-)
 delete mode 100644 drivers/staging/r8188eu/core/rtw_io.c

diff --git a/drivers/staging/r8188eu/Makefile b/drivers/staging/r8188eu/Makefile
index aebaf29990fd..4ca48fe628fd 100644
--- a/drivers/staging/r8188eu/Makefile
+++ b/drivers/staging/r8188eu/Makefile
@@ -78,7 +78,6 @@ rtk_core :=				\
 		core/rtw_debug.o	\
 		core/rtw_efuse.o	\
 		core/rtw_ieee80211.o	\
-		core/rtw_io.o		\
 		core/rtw_ioctl_set.o	\
 		core/rtw_iol.o		\
 		core/rtw_led.o		\
diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c
deleted file mode 100644
index 74b02ff8e44d..000000000000
--- a/drivers/staging/r8188eu/core/rtw_io.c
+++ /dev/null
@@ -1,111 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/* Copyright(c) 2007 - 2011 Realtek Corporation. */
-
-/*
-
-The purpose of rtw_io.c
-
-a. provides the API
-
-b. provides the protocol engine
-
-c. provides the software interface between caller and the hardware interface
-
-Compiler Flag Option:
-
-USB:
-   a. USE_ASYNC_IRP: Both sync/async operations are provided.
-
-Only sync read/rtw_write_mem operations are provided.
-
-jackson@realtek.com.tw
-
-*/
-
-#define _RTW_IO_C_
-#include "../include/osdep_service.h"
-#include "../include/drv_types.h"
-#include "../include/rtw_io.h"
-#include "../include/osdep_intf.h"
-#include "../include/usb_ops.h"
-
-#define rtw_le16_to_cpu(val)		le16_to_cpu(val)
-#define rtw_le32_to_cpu(val)		le32_to_cpu(val)
-#define rtw_cpu_to_le16(val)		cpu_to_le16(val)
-#define rtw_cpu_to_le32(val)		cpu_to_le32(val)
-
-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);
-}
-
-
-u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem, int timeout_ms)
-{
-	int ret = _SUCCESS;
-	struct xmit_buf *pxmitbuf = (struct xmit_buf *)pmem;
-	struct submit_ctx sctx;
-
-	rtw_sctx_init(&sctx, timeout_ms);
-	pxmitbuf->sctx = &sctx;
-
-	ret = rtw_write_port(adapter, addr, cnt, pmem);
-
-	if (ret == _SUCCESS)
-		ret = rtw_sctx_wait(&sctx);
-
-	return ret;
-}
-
-int rtw_init_io_priv(struct adapter *padapter, void (*set_intf_ops)(struct _io_ops *pops))
-{
-	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/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h
index 4f4678a55687..2b9b64f1ac80 100644
--- a/drivers/staging/r8188eu/include/rtw_io.h
+++ b/drivers/staging/r8188eu/include/rtw_io.h
@@ -257,25 +257,11 @@ 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_and_wait(struct adapter *adapter, u32 addr, u32 cnt,
-			     u8 *pmem, int timeout_ms);
 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_port_and_wait(adapter, addr, cnt, mem, timeout_ms)	\
-	_rtw_write_port_and_wait((adapter), (addr), (cnt), (mem), (timeout_ms))
+void rtw_write_scsi(struct adapter *adapter, u32 cnt, u8 *pmem);
 
 /* ioreq */
 void ioreq_read8(struct adapter *adapter, u32 addr, u8 *pval);
@@ -317,9 +303,6 @@ 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));
-
 uint alloc_io_queue(struct adapter *adapter);
 void free_io_queue(struct adapter *adapter);
 void async_bus_io(struct io_queue *pio_q);
diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index 78c857d97e8b..f38d1b267384 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -507,6 +507,8 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj,
 	struct adapter *padapter = NULL;
 	struct net_device *pnetdev = NULL;
 	int status = _FAIL;
+	struct io_priv *piopriv;
+	struct intf_hdl *pintf;
 
 	padapter = vzalloc(sizeof(*padapter));
 	if (!padapter)
@@ -538,7 +540,11 @@ 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);
+	piopriv = &padapter->iopriv;
+	pintf = &piopriv->intf;
+	piopriv->padapter = padapter;
+	pintf->padapter = padapter;
+	pintf->pintf_dev = adapter_to_dvobj(padapter);
 
 	/* step read_chip_version */
 	rtl8188e_read_chip_version(padapter);
-- 
2.33.0


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

* [PATCH v4 14/18] staging: remove struct _io_ops
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (12 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 13/18] staging: r8188eu: remove core/rtw_io.c Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-13 18:09 ` [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq() Fabio M. De Francesco
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin, Fabio M . De Francesco

From: Pavel Skripkin <paskripkin@gmail.com>

Finally struct _io_ops is unused, so remove it.

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>
---

v3->v4:
        Split a patch into fourteen.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/hal/usb_ops_linux.c |  6 -----
 drivers/staging/r8188eu/include/rtw_io.h    | 25 ---------------------
 drivers/staging/r8188eu/include/usb_ops.h   |  2 --
 3 files changed, 33 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 2516cfc464a9..117213c9f984 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -558,12 +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));
-}
-
 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 2b9b64f1ac80..c6a078210eeb 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 {
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
-- 
2.33.0


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

* [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (13 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 14/18] staging: remove struct _io_ops Fabio M. De Francesco
@ 2021-09-13 18:09 ` Fabio M. De Francesco
  2021-09-14  9:24   ` Dan Carpenter
  2021-09-13 18:10 ` [PATCH v4 16/18] staging: r8188eu: hal: Clean up rtw_read*() and rtw_write*() Fabio M. De Francesco
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:09 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Fabio M. De Francesco, Pavel Skripkin

Clean up usbctrl_vendorreq () in usb_ops_linux.c because some
of its code will be reused in this series. This cleanup is in
preparation for shortening the call chains of rtw_read{8,16,32}()
and rtw_write{8,16,32,N}(). More insights about the reasons why at
https://lore.kernel.org/lkml/5319192.FrU0QrjFp7@localhost.localdomain/

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>
---

v3->v4:
        Make this patch.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/hal/usb_ops_linux.c | 62 +++++++++------------
 1 file changed, 27 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 117213c9f984..2c7e92085a6e 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -8,15 +8,15 @@
 #include "../include/recv_osdep.h"
 #include "../include/rtl8188e_hal.h"
 
-static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, u16 len, u8 requesttype)
+static int usbctrl_vendorreq(struct intf_hdl *intfhdl, u16 value, void *data, u16 len, u8 requesttype)
 {
-	struct adapter	*adapt = pintfhdl->padapter;
-	struct dvobj_priv  *dvobjpriv = adapter_to_dvobj(adapt);
-	struct usb_device *udev = dvobjpriv->pusbdev;
-	unsigned int pipe;
 	int status = 0;
-	u8 *pIo_buf;
+	u8 *io_buf;
+	unsigned int pipe;
 	int vendorreq_times = 0;
+	struct adapter *adapt = intfhdl->padapter;
+	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
+	struct usb_device *udev = dvobjpriv->pusbdev;
 
 	if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) {
 		status = -EPERM;
@@ -32,51 +32,44 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
 
 	/*  Acquire IO memory for vendorreq */
-	pIo_buf = dvobjpriv->usb_vendor_req_buf;
-
-	if (!pIo_buf) {
-		DBG_88E("[%s] pIo_buf == NULL\n", __func__);
-		status = -ENOMEM;
-		goto release_mutex;
-	}
+	io_buf = dvobjpriv->usb_vendor_req_buf;
 
 	if (requesttype == REALTEK_USB_VENQT_READ)
-		pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
+		/* read in */
+		pipe = usb_rcvctrlpipe(udev, 0);
 	else
-		pipe = usb_sndctrlpipe(udev, 0);/* write_out */
+		/* write out */
+		pipe = usb_sndctrlpipe(udev, 0);
 
 	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
 		if (requesttype == REALTEK_USB_VENQT_READ)
-			memset(pIo_buf, 0, len);
+			memset(io_buf, 0, len);
 		else
-			memcpy(pIo_buf, pdata, len);
+			memcpy(io_buf, data, len);
 
 		status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
 					 requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
-					 pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
+					 io_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
 
-		if (status == len) {   /*  Success this control transfer. */
+		if (status == len) {
+			/*  success */
 			rtw_reset_continual_urb_error(dvobjpriv);
 			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 == REALTEK_USB_VENQT_READ) ? "read" : "write",
-				len, status, *(u32 *)pdata, vendorreq_times);
-
+				memcpy(data, io_buf,  len);
+		} else {
+			/* errors */
 			if (status < 0) {
-				if (status == (-ESHUTDOWN) || status == -ENODEV) {
+				if (status == (-ESHUTDOWN || -ENODEV)) {
 					adapt->bSurpriseRemoved = true;
 				} else {
-					struct hal_data_8188e	*haldata = GET_HAL_DATA(adapt);
+					struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
 					haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
 				}
-			} else { /*  status != len && status >= 0 */
+			} else {
+				/*  status != len && status >= 0 */
 				if (status > 0) {
-					if (requesttype == REALTEK_USB_VENQT_READ) {
-						/*  For Control read transfer, we have to copy the read data from pIo_buf to pdata. */
-						memcpy(pdata, pIo_buf,  len);
-					}
+					if (requesttype == REALTEK_USB_VENQT_READ)
+						memcpy(data, io_buf,  len);
 				}
 			}
 
@@ -86,12 +79,11 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
 			}
 
 		}
-
-		/*  firmware download is checksumed, don't retry */
+		/*  firmware download is checksummed, don't retry */
 		if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || status == len)
 			break;
 	}
-release_mutex:
+
 	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
 exit:
 	return status;
-- 
2.33.0


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

* [PATCH v4 16/18] staging: r8188eu: hal: Clean up rtw_read*() and rtw_write*()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (14 preceding siblings ...)
  2021-09-13 18:09 ` [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq() Fabio M. De Francesco
@ 2021-09-13 18:10 ` Fabio M. De Francesco
  2021-09-14 14:33   ` David Laight
  2021-09-13 18:10 ` [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco
                   ` (2 subsequent siblings)
  18 siblings, 1 reply; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:10 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Fabio M. De Francesco, Pavel Skripkin

Clean up rtw_read{8,16,32}() and rtw_write{8,16,32,N}() in usb_ops_linux.c
because some of their code will be reused in the following two patches of
this series. This cleanup is in preparation for shortening the call chains
of rtw_read{8,16,32}() and rtw_write{8,16,32,N}().
More insights can be found at
https://lore.kernel.org/lkml/5319192.FrU0QrjFp7@localhost.localdomain/

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>
---

v3->v4:
        Make this patch.
v2->v3:
        No changes.
v1->v2:
        No changes.

 drivers/staging/r8188eu/hal/usb_ops_linux.c | 71 +++++++++++----------
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 2c7e92085a6e..04402bab805e 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -91,91 +91,92 @@ static int usbctrl_vendorreq(struct intf_hdl *intfhdl, u16 value, void *data, u1
 
 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;
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
+	u16 address = (u16)(addr & 0xffff);
 
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ);
+	usbctrl_vendorreq(intfhdl, address, &data, 1, REALTEK_USB_VENQT_READ);
 
 	return data;
 }
 
 u16 rtw_read16(struct adapter *adapter, u32 addr)
 {
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-	u16 wvalue = (u16)(addr & 0x0000ffff);
-	__le32 data;
+	__le16 data;
+	u16 address = (u16)(addr & 0xffff);
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_READ);
+	usbctrl_vendorreq(intfhdl, address, &data, 2, REALTEK_USB_VENQT_READ);
 
-	return (u16)(le32_to_cpu(data) & 0xffff);
+	return le16_to_cpu(data);
 }
 
 u32 rtw_read32(struct adapter *adapter, u32 addr)
 {
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-	u16 wvalue = (u16)(addr & 0x0000ffff);
 	__le32 data;
+	u16 address = (u16)(addr & 0xffff);
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_READ);
+	usbctrl_vendorreq(intfhdl, address, &data, 4, REALTEK_USB_VENQT_READ);
 
 	return le32_to_cpu(data);
 }
 
 int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
 {
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-	u16 wvalue = (u16)(addr & 0x0000ffff);
 	int ret;
+	u16 address = (u16)(addr & 0xffff);
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE);
+	ret = usbctrl_vendorreq(intfhdl, address, &val, 1, REALTEK_USB_VENQT_WRITE);
 
 	return RTW_STATUS_CODE(ret);
 }
 
 int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
 {
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-	u16 wvalue = (u16)(addr & 0x0000ffff);
-	__le32 data = cpu_to_le32(val & 0x0000ffff);
 	int ret;
+	u16 address = (u16)(addr & 0xffff);
+	__le16 data = cpu_to_le16(val);
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE);
+	ret = usbctrl_vendorreq(intfhdl, address, &data, 2, REALTEK_USB_VENQT_WRITE);
 
 	return RTW_STATUS_CODE(ret);
 }
 
 int rtw_write32(struct adapter *adapter, u32 addr, u32 val)
 {
-	struct io_priv *pio_priv = &adapter->iopriv;
-	struct intf_hdl *pintfhdl = &pio_priv->intf;
-	u16 wvalue = (u16)(addr & 0x0000ffff);
-	__le32 data = cpu_to_le32(val);
 	int ret;
+	u16 address = (u16)(addr & 0xffff);
+	__le32 data = cpu_to_le32(val);
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE);
+	ret = usbctrl_vendorreq(intfhdl, address, &data, 4, REALTEK_USB_VENQT_WRITE);
 
 	return RTW_STATUS_CODE(ret);
 }
 
-int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata)
+int rtw_writeN(struct adapter *adapter, u32 addr, u32 len, u8 *data)
 {
-	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;
+	u16 address = (u16)(addr & 0xffff);
+	u16 length = (u16)(len & 0xffff);
+	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
+	struct io_priv *io_priv = &adapter->iopriv;
+	struct intf_hdl *intfhdl = &io_priv->intf;
 
 	if (length > VENDOR_CMD_MAX_DATA_LEN)
 		return _FAIL;
 
-	memcpy(buf, pdata, length);
-	ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE);
+	memcpy(buf, data, length);
+	ret = usbctrl_vendorreq(intfhdl, address, buf, length, REALTEK_USB_VENQT_WRITE);
 
 	return RTW_STATUS_CODE(ret);
 }
-- 
2.33.0


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

* [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (15 preceding siblings ...)
  2021-09-13 18:10 ` [PATCH v4 16/18] staging: r8188eu: hal: Clean up rtw_read*() and rtw_write*() Fabio M. De Francesco
@ 2021-09-13 18:10 ` Fabio M. De Francesco
  2021-09-13 20:19   ` Pavel Skripkin
  2021-09-14  9:32   ` Dan Carpenter
  2021-09-13 18:10 ` [PATCH v4 18/18] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco
  2021-09-13 20:35 ` [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
  18 siblings, 2 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:10 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  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>
---

v3->v4: 
	Make some changes according to a first review of Greg
	Kroah-Hartman; remove unnecessary while loop and a couple of
	'if' test; handle the errors returned by usb_control_msg_recv()
v2->v3: 
	No changes.
v1->v2: 
	No changes.

 drivers/staging/r8188eu/hal/usb_ops_linux.c | 56 +++++++++++++++++++--
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 04402bab805e..75475b0083db 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -89,6 +89,56 @@ static int usbctrl_vendorreq(struct intf_hdl *intfhdl, u16 value, void *data, u1
 	return status;
 }
 
+static int usb_read(struct intf_hdl *intfhdl, u16 addr, void *data, u8 size)
+{
+	int status;
+	u8 *io_buf; /* pointer to I/O buffer */
+	struct adapter *adapt = intfhdl->padapter;
+	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
+	struct usb_device *udev = dvobjpriv->pusbdev;
+
+	if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
+		status = -EPERM;
+		goto exit;
+	}
+
+	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
+
+	io_buf = dvobjpriv->usb_vendor_req_buf;
+
+	status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+				      REALTEK_USB_VENQT_READ, addr,
+				      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);
+		goto mutex_unlock;
+	}
+	/*  error cases */
+	if (status == (-ESHUTDOWN || -ENODEV || -ENOENT)) {
+		/*
+		 * device or controller has been disabled due to
+		 * some problem that could not be worked around,
+		 * device or bus doesn’t exist, endpoint does not
+		 * exist or is not enabled.
+		 */
+		adapt->bSurpriseRemoved = true;
+			goto mutex_unlock;
+	}
+	GET_HAL_DATA(adapt)->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
+	if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
+		adapt->bSurpriseRemoved = true;
+		goto mutex_unlock;
+	}
+mutex_unlock:
+	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
+exit:
+	return status;
+}
+
 u8 rtw_read8(struct adapter *adapter, u32 addr)
 {
 	u8 data;
@@ -96,7 +146,7 @@ u8 rtw_read8(struct adapter *adapter, u32 addr)
 	struct intf_hdl *intfhdl = &io_priv->intf;
 	u16 address = (u16)(addr & 0xffff);
 
-	usbctrl_vendorreq(intfhdl, address, &data, 1, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, address, &data, 1);
 
 	return data;
 }
@@ -108,7 +158,7 @@ u16 rtw_read16(struct adapter *adapter, u32 addr)
 	struct io_priv *io_priv = &adapter->iopriv;
 	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	usbctrl_vendorreq(intfhdl, address, &data, 2, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, address, &data, 2);
 
 	return le16_to_cpu(data);
 }
@@ -120,7 +170,7 @@ u32 rtw_read32(struct adapter *adapter, u32 addr)
 	struct io_priv *io_priv = &adapter->iopriv;
 	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	usbctrl_vendorreq(intfhdl, address, &data, 4, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, address, &data, 4);
 
 	return le32_to_cpu(data);
 }
-- 
2.33.0


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

* [PATCH v4 18/18] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N()
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (16 preceding siblings ...)
  2021-09-13 18:10 ` [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco
@ 2021-09-13 18:10 ` Fabio M. De Francesco
  2021-09-13 20:35 ` [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 18:10 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  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>
---

v3->v4:
        Make some changes according to a first review of Greg
        Kroah-Hartman; remove unnecessary 'while' loop and a couple of
        'if' test; handle the errors returned by usb_control_msg_send().

v2->v3: Fix the version number of the patch.
 
v1->v2: Replace parameter REALTEK_USB_VENQT_READ with REALTEK_USB_VENQT_WRITE
	in usb_control_msg_send(). More in-depth explanation at
	https://lore.kernel.org/lkml/2791328.7pjKATJfGa@localhost.localdomain/T/#m1fc1ab2f7c1f463049ad88d5df5bb1b107b37260

 drivers/staging/r8188eu/hal/usb_ops_linux.c | 111 +++++++-------------
 1 file changed, 39 insertions(+), 72 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index 75475b0083db..cf27c84c3c90 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -8,88 +8,57 @@
 #include "../include/recv_osdep.h"
 #include "../include/rtl8188e_hal.h"
 
-static int usbctrl_vendorreq(struct intf_hdl *intfhdl, u16 value, void *data, u16 len, u8 requesttype)
+static int usb_read(struct intf_hdl *intfhdl, u16 addr, void *data, u8 size)
 {
-	int status = 0;
-	u8 *io_buf;
-	unsigned int pipe;
-	int vendorreq_times = 0;
+	int status;
+	u8 *io_buf; /* pointer to I/O buffer */
 	struct adapter *adapt = intfhdl->padapter;
 	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
 	struct usb_device *udev = dvobjpriv->pusbdev;
 
-	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;
-	}
-
 	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
 
-	/*  Acquire IO memory for vendorreq */
 	io_buf = dvobjpriv->usb_vendor_req_buf;
 
-	if (requesttype == REALTEK_USB_VENQT_READ)
-		/* read in */
-		pipe = usb_rcvctrlpipe(udev, 0);
-	else
-		/* write out */
-		pipe = usb_sndctrlpipe(udev, 0);
-
-	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
-		if (requesttype == REALTEK_USB_VENQT_READ)
-			memset(io_buf, 0, len);
-		else
-			memcpy(io_buf, data, len);
-
-		status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
-					 requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
-					 io_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
-
-		if (status == len) {
-			/*  success */
-			rtw_reset_continual_urb_error(dvobjpriv);
-			if (requesttype == REALTEK_USB_VENQT_READ)
-				memcpy(data, io_buf,  len);
-		} else {
-			/* errors */
-			if (status < 0) {
-				if (status == (-ESHUTDOWN || -ENODEV)) {
-					adapt->bSurpriseRemoved = true;
-				} else {
-					struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
-					haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
-				}
-			} else {
-				/*  status != len && status >= 0 */
-				if (status > 0) {
-					if (requesttype == REALTEK_USB_VENQT_READ)
-						memcpy(data, io_buf,  len);
-				}
-			}
-
-			if (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 == len)
-			break;
+	status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+				      REALTEK_USB_VENQT_READ, addr,
+				      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);
+		goto mutex_unlock;
 	}
-
+	/*  error cases */
+	if (status == (-ESHUTDOWN || -ENODEV || -ENOENT)) {
+		/*
+		 * device or controller has been disabled due to
+		 * some problem that could not be worked around,
+		 * device or bus doesn’t exist, endpoint does not
+		 * exist or is not enabled.
+		 */
+		adapt->bSurpriseRemoved = true;
+			goto mutex_unlock;
+	}
+	GET_HAL_DATA(adapt)->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
+	if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
+		adapt->bSurpriseRemoved = true;
+		goto mutex_unlock;
+	}
+mutex_unlock:
 	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
 exit:
 	return status;
 }
 
-static int usb_read(struct intf_hdl *intfhdl, u16 addr, void *data, u8 size)
+static int usb_write(struct intf_hdl *intfhdl, u16 addr, void *data, u8 size)
 {
 	int status;
 	u8 *io_buf; /* pointer to I/O buffer */
@@ -105,16 +74,16 @@ static int usb_read(struct intf_hdl *intfhdl, u16 addr, void *data, u8 size)
 	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
 
 	io_buf = dvobjpriv->usb_vendor_req_buf;
+	memcpy(io_buf, data, size);
 
-	status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
-				      REALTEK_USB_VENQT_READ, addr,
+	status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
+				      REALTEK_USB_VENQT_WRITE, addr,
 				      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);
 		goto mutex_unlock;
 	}
 	/*  error cases */
@@ -182,7 +151,7 @@ int rtw_write8(struct adapter *adapter, u32 addr, u8 val)
 	struct io_priv *io_priv = &adapter->iopriv;
 	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	ret = usbctrl_vendorreq(intfhdl, address, &val, 1, REALTEK_USB_VENQT_WRITE);
+	ret = usb_write(intfhdl, address, &val, 1);
 
 	return RTW_STATUS_CODE(ret);
 }
@@ -195,7 +164,7 @@ int rtw_write16(struct adapter *adapter, u32 addr, u16 val)
 	struct io_priv *io_priv = &adapter->iopriv;
 	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	ret = usbctrl_vendorreq(intfhdl, address, &data, 2, REALTEK_USB_VENQT_WRITE);
+	ret = usb_write(intfhdl, address, &data, 2);
 
 	return RTW_STATUS_CODE(ret);
 }
@@ -208,7 +177,7 @@ int rtw_write32(struct adapter *adapter, u32 addr, u32 val)
 	struct io_priv *io_priv = &adapter->iopriv;
 	struct intf_hdl *intfhdl = &io_priv->intf;
 
-	ret = usbctrl_vendorreq(intfhdl, address, &data, 4, REALTEK_USB_VENQT_WRITE);
+	ret = usb_write(intfhdl, address, &data, 4);
 
 	return RTW_STATUS_CODE(ret);
 }
@@ -218,15 +187,13 @@ int rtw_writeN(struct adapter *adapter, u32 addr, u32 len, u8 *data)
 	int ret;
 	u16 address = (u16)(addr & 0xffff);
 	u16 length = (u16)(len & 0xffff);
-	u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0};
 	struct io_priv *io_priv = &adapter->iopriv;
 	struct intf_hdl *intfhdl = &io_priv->intf;
 
 	if (length > VENDOR_CMD_MAX_DATA_LEN)
 		return _FAIL;
 
-	memcpy(buf, data, length);
-	ret = usbctrl_vendorreq(intfhdl, address, buf, length, REALTEK_USB_VENQT_WRITE);
+	ret = usb_write(intfhdl, address, data, length);
 
 	return RTW_STATUS_CODE(ret);
 }
-- 
2.33.0


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

* Re: [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32()
  2021-09-13 18:10 ` [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco
@ 2021-09-13 20:19   ` Pavel Skripkin
  2021-09-14  9:32   ` Dan Carpenter
  1 sibling, 0 replies; 31+ messages in thread
From: Pavel Skripkin @ 2021-09-13 20:19 UTC (permalink / raw)
  To: Fabio M. De Francesco, Larry Finger, Phillip Potter,
	Greg Kroah-Hartman, linux-staging, linux-kernel, Michael Straube

On 9/13/21 21:10, Fabio M. De Francesco wrote:
> 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>
> ---
> 
> v3->v4:
> 	Make some changes according to a first review of Greg
> 	Kroah-Hartman; remove unnecessary while loop and a couple of
> 	'if' test; handle the errors returned by usb_control_msg_recv()
> v2->v3:
> 	No changes.
> v1->v2:
> 	No changes.
> 

Hi, maintainers and reviewers!

We have just noticed, that 17 and 18 patches in this series contain 
logic error, so, please, don't waste time reviewing them.

v5 will be posted soon :)


With regards,
Pavel Skripkin

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

* Re: [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain
  2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
                   ` (17 preceding siblings ...)
  2021-09-13 18:10 ` [PATCH v4 18/18] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco
@ 2021-09-13 20:35 ` Fabio M. De Francesco
  18 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-13 20:35 UTC (permalink / raw)
  To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, Pavel Skripkin
  Cc: linux-staging, linux-kernel, Michael Straube

On Monday, September 13, 2021 8:09:44 PM CEST Fabio M. De Francesco wrote:
> 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] and by Fabio M.
> De Francesco using a ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano].

Hi all,

Please disregard this series. I've made a mistake, so tomorrow I'll send v5.

Pavel Skripkin made me notice a logical error where the code checks errors in 
17/18 and 18/18. So, two lines must be rewritten.

Thanks,

Fabio



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

* Re: [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq()
  2021-09-13 18:09 ` [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq() Fabio M. De Francesco
@ 2021-09-14  9:24   ` Dan Carpenter
  2021-09-14 11:18     ` Fabio M. De Francesco
  2021-09-14 13:24     ` Fabio M. De Francesco
  0 siblings, 2 replies; 31+ messages in thread
From: Dan Carpenter @ 2021-09-14  9:24 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Mon, Sep 13, 2021 at 08:09:59PM +0200, Fabio M. De Francesco wrote:
> Clean up usbctrl_vendorreq () in usb_ops_linux.c because some
> of its code will be reused in this series. This cleanup is in
> preparation for shortening the call chains of rtw_read{8,16,32}()
> and rtw_write{8,16,32,N}(). More insights about the reasons why at
> https://lore.kernel.org/lkml/5319192.FrU0QrjFp7@localhost.localdomain/
> 

This commit message is quite bad.

This patch has nothing to do with reusing the code or shortening call
chains.

Don't use a link like that in the commit message especially when it's a
link to an email you wrote.  If it's someone else's email you can say,
somethink like "As <name> points out in <his/her> email <url>.  Blah
blah blah."  That way you give credit to the other person but all the
information is in the commit message.

A better commit message would be:

    Clean up usbctrl_vendorreq().

    1) Remove impossible NULL check.
    2) Rename variables:
        pintfhdl => intfhdl
        pdata => data
        pIo_buf => io_buf
    3) Edit the comments.

> 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>
> ---
> 
> v3->v4:
>         Make this patch.
> v2->v3:
>         No changes.
> v1->v2:
>         No changes.
> 
>  drivers/staging/r8188eu/hal/usb_ops_linux.c | 62 +++++++++------------
>  1 file changed, 27 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index 117213c9f984..2c7e92085a6e 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -8,15 +8,15 @@
>  #include "../include/recv_osdep.h"
>  #include "../include/rtl8188e_hal.h"
>  
> -static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, u16 len, u8 requesttype)
> +static int usbctrl_vendorreq(struct intf_hdl *intfhdl, u16 value, void *data, u16 len, u8 requesttype)
>  {
> -	struct adapter	*adapt = pintfhdl->padapter;
> -	struct dvobj_priv  *dvobjpriv = adapter_to_dvobj(adapt);
> -	struct usb_device *udev = dvobjpriv->pusbdev;
> -	unsigned int pipe;
>  	int status = 0;
> -	u8 *pIo_buf;
> +	u8 *io_buf;
> +	unsigned int pipe;
>  	int vendorreq_times = 0;
> +	struct adapter *adapt = intfhdl->padapter;
> +	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
> +	struct usb_device *udev = dvobjpriv->pusbdev;

I don't understand why you moved these from the top to the bottom.
But the original was better.  In networking code declarations are
normally written in Reverse Christmas Tree format, longest to shortest,
like this:

	long long long_name;
	medium name;
	u8 short;

>  
>  	if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) {
>  		status = -EPERM;
> @@ -32,51 +32,44 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata,
>  	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
>  
>  	/*  Acquire IO memory for vendorreq */
> -	pIo_buf = dvobjpriv->usb_vendor_req_buf;
> -
> -	if (!pIo_buf) {
> -		DBG_88E("[%s] pIo_buf == NULL\n", __func__);
> -		status = -ENOMEM;
> -		goto release_mutex;
> -	}
> +	io_buf = dvobjpriv->usb_vendor_req_buf;
>  
>  	if (requesttype == REALTEK_USB_VENQT_READ)
> -		pipe = usb_rcvctrlpipe(udev, 0);/* read_in */
> +		/* read in */
> +		pipe = usb_rcvctrlpipe(udev, 0);
>  	else
> -		pipe = usb_sndctrlpipe(udev, 0);/* write_out */
> +		/* write out */
> +		pipe = usb_sndctrlpipe(udev, 0);

Use {} curly braces around multi-line indents.  But in this case the
comments are worthless.  Just delete them.


>  
>  	while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) {
>  		if (requesttype == REALTEK_USB_VENQT_READ)
> -			memset(pIo_buf, 0, len);
> +			memset(io_buf, 0, len);
>  		else
> -			memcpy(pIo_buf, pdata, len);
> +			memcpy(io_buf, data, len);
>  
>  		status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ,
>  					 requesttype, value, REALTEK_USB_VENQT_CMD_IDX,
> -					 pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
> +					 io_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT);
>  
> -		if (status == len) {   /*  Success this control transfer. */
> +		if (status == len) {
> +			/*  success */

Delete this comment.

>  			rtw_reset_continual_urb_error(dvobjpriv);
>  			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 == REALTEK_USB_VENQT_READ) ? "read" : "write",
> -				len, status, *(u32 *)pdata, vendorreq_times);
> -
> +				memcpy(data, io_buf,  len);
> +		} else {
> +			/* errors */

Delete.


>  			if (status < 0) {
> -				if (status == (-ESHUTDOWN) || status == -ENODEV) {
> +				if (status == (-ESHUTDOWN || -ENODEV)) {
>  					adapt->bSurpriseRemoved = true;
>  				} else {
> -					struct hal_data_8188e	*haldata = GET_HAL_DATA(adapt);
> +					struct hal_data_8188e *haldata = GET_HAL_DATA(adapt);
>  					haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
>  				}
> -			} else { /*  status != len && status >= 0 */
> +			} else {
> +				/*  status != len && status >= 0 */

Delete.

>  				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);
> -					}
> +					if (requesttype == REALTEK_USB_VENQT_READ)
> +						memcpy(data, io_buf,  len);
>  				}
>  			}
>  

regards,
dan carpenter

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

* Re: [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32()
  2021-09-13 18:10 ` [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco
  2021-09-13 20:19   ` Pavel Skripkin
@ 2021-09-14  9:32   ` Dan Carpenter
  2021-09-14 12:55     ` Fabio M. De Francesco
  1 sibling, 1 reply; 31+ messages in thread
From: Dan Carpenter @ 2021-09-14  9:32 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Mon, Sep 13, 2021 at 08:10:01PM +0200, Fabio M. De Francesco wrote:
> diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> index 04402bab805e..75475b0083db 100644
> --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> @@ -89,6 +89,56 @@ static int usbctrl_vendorreq(struct intf_hdl *intfhdl, u16 value, void *data, u1
>  	return status;
>  }
>  
> +static int usb_read(struct intf_hdl *intfhdl, u16 addr, void *data, u8 size)
> +{
> +	int status;
> +	u8 *io_buf; /* pointer to I/O buffer */
> +	struct adapter *adapt = intfhdl->padapter;
> +	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
> +	struct usb_device *udev = dvobjpriv->pusbdev;
> +
> +	if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) {
> +		status = -EPERM;
> +		goto exit;
> +	}

Just return directly.

	if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx)
		return -EPERM;

> +
> +	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
> +
> +	io_buf = dvobjpriv->usb_vendor_req_buf;
> +
> +	status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
> +				      REALTEK_USB_VENQT_READ, addr,
> +				      REALTEK_USB_VENQT_CMD_IDX, io_buf,
> +				      size, RTW_USB_CONTROL_MSG_TIMEOUT,
> +				      GFP_KERNEL);
> +	if (!status) {
> +		/*  Success this control transfer. */

It's always better to do error handling instead of success handling.

	if (status) {

Remove the comment because now it's in the standard format.

> +		rtw_reset_continual_urb_error(dvobjpriv);
> +		memcpy(data, io_buf, size);
> +		goto mutex_unlock;
> +	}
> +	/*  error cases */
> +	if (status == (-ESHUTDOWN || -ENODEV || -ENOENT)) {

	if (status == -ESHUTDOWN ||
	    status == -ENODEV ||
	    status == -ENOENT) {


> +		/*
> +		 * device or controller has been disabled due to
> +		 * some problem that could not be worked around,
> +		 * device or bus doesn’t exist, endpoint does not
> +		 * exist or is not enabled.
> +		 */
> +		adapt->bSurpriseRemoved = true;
> +			goto mutex_unlock;

Indented wrong.

> +	}
> +	GET_HAL_DATA(adapt)->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
> +	if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
> +		adapt->bSurpriseRemoved = true;
> +		goto mutex_unlock;
> +	}
> +mutex_unlock:
> +	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
> +exit:
> +	return status;
> +}

regards,
dan carpenter


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

* Re: [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq()
  2021-09-14  9:24   ` Dan Carpenter
@ 2021-09-14 11:18     ` Fabio M. De Francesco
  2021-09-14 12:20       ` Dan Carpenter
  2021-09-14 13:24     ` Fabio M. De Francesco
  1 sibling, 1 reply; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-14 11:18 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Tuesday, September 14, 2021 11:24:05 AM CEST Dan Carpenter wrote:
> On Mon, Sep 13, 2021 at 08:09:59PM +0200, Fabio M. De Francesco wrote:
> > Clean up usbctrl_vendorreq () in usb_ops_linux.c because some
> > of its code will be reused in this series. This cleanup is in
> > preparation for shortening the call chains of rtw_read{8,16,32}()
> > and rtw_write{8,16,32,N}(). More insights about the reasons why at
> > https://lore.kernel.org/lkml/5319192.FrU0QrjFp7@localhost.localdomain/
> > 
> 
> This commit message is quite bad.
> 
> This patch has nothing to do with reusing the code or shortening call
> chains.

It has to do, in a certain sense. Let me explain please...

Some days ago, David Laight made the review of "Shorten calls chain of 
rtw_write8/16/32/n()" version 3. 

In that patch he noticed some lines of usb_read() that I had created with the 
help of reusing some lines of the code of usbctrl_vendorreq() that is deleted 
in the same patch. 

He thought that they were clean-ups and renames and so he suggested to make 
those "clean-ups" in a separate patch.

However they were _not_ renames or other clean-ups, because usb_read() was 
not touched in that patch and, above all, it was a new function. 

I am sure that when I write new functions I can use whatever name of 
variables I like, even if people may think I'm renaming the variables that 
were in a old function that now is deleted. Am I not permitted?

However, because I also think that readability of the diffs matters, I 
decided to do some clean-up of the code I'm about to reuse in the new 
functions. It improves readability of the above-mentioned patch that is also 
the 18/18 of this series.

That is the reason why I'm cleaning up a function that is going to be deleted 
in the last patch of the series.

> Don't use a link like that in the commit message especially when it's a
> link to an email you wrote.  If it's someone else's email you can say,
> something like "As <name> points out in <his/her> email <url>.  Blah
> blah blah."  That way you give credit to the other person but all the
> information is in the commit message.

I agree with you. I'll redo the commit message for in order to summarize in 
few lines why I'm doing clean-ups of functions that must be deleted in 18/18.
The same for 16/18. I think that a short explanation like the one that I gave 
you above should suffice (much shorter, obviously).

I hope that I've been clear now. Please let me know if you have more 
suggestions about this patch and the next (16/18).

Regards,

Fabio



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

* Re: [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq()
  2021-09-14 11:18     ` Fabio M. De Francesco
@ 2021-09-14 12:20       ` Dan Carpenter
  0 siblings, 0 replies; 31+ messages in thread
From: Dan Carpenter @ 2021-09-14 12:20 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Tue, Sep 14, 2021 at 01:18:01PM +0200, Fabio M. De Francesco wrote:
> On Tuesday, September 14, 2021 11:24:05 AM CEST Dan Carpenter wrote:
> > On Mon, Sep 13, 2021 at 08:09:59PM +0200, Fabio M. De Francesco wrote:
> > > Clean up usbctrl_vendorreq () in usb_ops_linux.c because some
> > > of its code will be reused in this series. This cleanup is in
> > > preparation for shortening the call chains of rtw_read{8,16,32}()
> > > and rtw_write{8,16,32,N}(). More insights about the reasons why at
> > > https://lore.kernel.org/lkml/5319192.FrU0QrjFp7@localhost.localdomain/ 
> > > 
> > 
> > This commit message is quite bad.
> > 
> > This patch has nothing to do with reusing the code or shortening call
> > chains.
> 
> It has to do, in a certain sense. Let me explain please...

I guess I can see that all this stuff is related in your mind, but next
time please leave it out.  Keep the commit messages as simple to
understand as possible.

Just say you are cleaning up the function.  That's a simple motivation.
A good motivation.

regards,
dan carpenter


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

* Re: [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32()
  2021-09-14  9:32   ` Dan Carpenter
@ 2021-09-14 12:55     ` Fabio M. De Francesco
  2021-09-14 13:01       ` Dan Carpenter
  0 siblings, 1 reply; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-14 12:55 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Tuesday, September 14, 2021 11:32:58 AM CEST Dan Carpenter wrote:
> On Mon, Sep 13, 2021 at 08:10:01PM +0200, Fabio M. De Francesco wrote:
> > diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/
staging/r8188eu/hal/usb_ops_linux.c
> > index 04402bab805e..75475b0083db 100644
> > --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
> > +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
> > @@ -89,6 +89,56 @@ static int usbctrl_vendorreq(struct intf_hdl *intfhdl, 
u16 value, void *data, u1
> >  	return status;
> >  }
> >  
> > +static int usb_read(struct intf_hdl *intfhdl, u16 addr, void *data, u8 
size)
> > +{
> > +	int status;
> > +	u8 *io_buf; /* pointer to I/O buffer */
> > +	struct adapter *adapt = intfhdl->padapter;
> > +	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
> > +	struct usb_device *udev = dvobjpriv->pusbdev;
> > +
> > +	if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) 
{
> > +		status = -EPERM;
> > +		goto exit;
> > +	}
> 
> Just return directly.
> 
> 	if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx)
> 		return -EPERM;

Dear Dan,

I agree with you, I'll return it directly in v5.

> > +
> > +	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
> > +
> > +	io_buf = dvobjpriv->usb_vendor_req_buf;
> > +
> > +	status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ,
> > +				      REALTEK_USB_VENQT_READ, 
addr,
> > +				      REALTEK_USB_VENQT_CMD_IDX, 
io_buf,
> > +				      size, 
RTW_USB_CONTROL_MSG_TIMEOUT,
> > +				      GFP_KERNEL);
> > +	if (!status) {
> > +		/*  Success this control transfer. */
> 
> It's always better to do error handling instead of success handling.

Yes, you're right again. I blindly followed the style that was in the parts 
of the code of usbctrl_vendorreq() that I'm reusing here.
 
> 	if (status) {
> 
> Remove the comment because now it's in the standard format.

OK, those comments are not necessary: I'll remove them.

> > +		rtw_reset_continual_urb_error(dvobjpriv);
> > +		memcpy(data, io_buf, size);
> > +		goto mutex_unlock;
> > +	}
> > +	/*  error cases */
> > +	if (status == (-ESHUTDOWN || -ENODEV || -ENOENT)) {
> 
> 	if (status == -ESHUTDOWN ||
> 	    status == -ENODEV ||
> 	    status == -ENOENT) {

This is a stupid mistake and Pavel soon noticed it. Yesterday I sent a 
message to ask reviewers for disregarding v4 and wait for v5 with the fix of 
this test. :(

However, I noticed that usb_control_msg_recv() might return in "status" some 
recoverable errors (like -ENOMEM and others); so I guess that the code must 
retry in a while loop (exactly as it did with usb_control_msg() in 
usbctrl_vendorreq()). 

Actually I'm not 100% sure that the "while" loop is needed. What I know is 
that I've been using this version (with _no_ loop) for about a week, not less 
than 12 hours a day and I didn't notice any problem...

Can you please tell if a loop for retries is *really* necessary?

> 
> > +		/*
> > +		 * device or controller has been disabled due to
> > +		 * some problem that could not be worked around,
> > +		 * device or bus doesn’t exist, endpoint does not
> > +		 * exist or is not enabled.
> > +		 */
> > +		adapt->bSurpriseRemoved = true;
> > +			goto mutex_unlock;
> 
> Indented wrong.

Yes, it must be fixed.

Regards,

Fabio

> 
> > +	}
> > +	GET_HAL_DATA(adapt)->srestpriv.wifi_error_status = 
USB_VEN_REQ_CMD_FAIL;
> > +	if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) {
> > +		adapt->bSurpriseRemoved = true;
> > +		goto mutex_unlock;
> > +	}
> > +mutex_unlock:
> > +	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
> > +exit:
> > +	return status;
> > +}
> 
> regards,
> dan carpenter
>



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

* Re: [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32()
  2021-09-14 12:55     ` Fabio M. De Francesco
@ 2021-09-14 13:01       ` Dan Carpenter
  0 siblings, 0 replies; 31+ messages in thread
From: Dan Carpenter @ 2021-09-14 13:01 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Tue, Sep 14, 2021 at 02:55:45PM +0200, Fabio M. De Francesco wrote:
> On Tuesday, September 14, 2021 11:32:58 AM CEST Dan Carpenter wrote:
> > On Mon, Sep 13, 2021 at 08:10:01PM +0200, Fabio M. De Francesco wrote:
> > > +	if (status == (-ESHUTDOWN || -ENODEV || -ENOENT)) {
> > 
> > 	if (status == -ESHUTDOWN ||
> > 	    status == -ENODEV ||
> > 	    status == -ENOENT) {
> 
> This is a stupid mistake and Pavel soon noticed it. Yesterday I sent a 
> message to ask reviewers for disregarding v4 and wait for v5 with the fix of 
> this test. :(

There wasn't enough information in the email to know what issue you had
seen.  I had already started reviewing it when I saw the email.

> 
> However, I noticed that usb_control_msg_recv() might return in "status" some 
> recoverable errors (like -ENOMEM and others); so I guess that the code must 
> retry in a while loop (exactly as it did with usb_control_msg() in 
> usbctrl_vendorreq()). 

I would not add a while loop unless testing shows it is required.

regards,
dan carpenter


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

* Re: [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq()
  2021-09-14  9:24   ` Dan Carpenter
  2021-09-14 11:18     ` Fabio M. De Francesco
@ 2021-09-14 13:24     ` Fabio M. De Francesco
  2021-09-14 13:30       ` Dan Carpenter
  1 sibling, 1 reply; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-14 13:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Tuesday, September 14, 2021 11:24:05 AM CEST Dan Carpenter wrote:
> I don't understand why you moved these from the top to the bottom.
> But the original was better.  In networking code declarations are
> normally written in Reverse Christmas Tree format, longest to shortest,
> like this:
> 
> 	long long long_name;
> 	medium name;
> 	u8 short;

Dear Dan,

I'm sorry that I forgot to thank you for the reviews in the other messages I 
sent in reply. :(

I also forgot to answer to the above question...

I changed the order of the declarations because David Laight wrote "I think 
you'll need 'reverse xmas tree' ordering as well." (copy-paste from his 
message).

As far as I know you are both experienced kernel developers, so I took his 
words for truth. Is it a matter of personal taste or Reverse/Non Reverse Xmas 
Trees are strictly required by the Linux kernel coding style guidelines?

I thank you very much for the time you spent for reviewing.

Regards,

Fabio 



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

* Re: [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq()
  2021-09-14 13:24     ` Fabio M. De Francesco
@ 2021-09-14 13:30       ` Dan Carpenter
  2021-09-14 14:05         ` Fabio M. De Francesco
  0 siblings, 1 reply; 31+ messages in thread
From: Dan Carpenter @ 2021-09-14 13:30 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Tue, Sep 14, 2021 at 03:24:06PM +0200, Fabio M. De Francesco wrote:
> On Tuesday, September 14, 2021 11:24:05 AM CEST Dan Carpenter wrote:
> > I don't understand why you moved these from the top to the bottom.
> > But the original was better.  In networking code declarations are
> > normally written in Reverse Christmas Tree format, longest to shortest,
> > like this:
> > 
> > 	long long long_name;
> > 	medium name;
> > 	u8 short;
> 
> Dear Dan,
> 
> I'm sorry that I forgot to thank you for the reviews in the other messages I 
> sent in reply. :(
> 
> I also forgot to answer to the above question...
> 
> I changed the order of the declarations because David Laight wrote "I think 
> you'll need 'reverse xmas tree' ordering as well." (copy-paste from his 
> message).
> 
> As far as I know you are both experienced kernel developers, so I took his 
> words for truth. Is it a matter of personal taste or Reverse/Non Reverse Xmas 
> Trees are strictly required by the Linux kernel coding style guidelines?

David and I are saying the same thing.  "Reverse Christmas Tree" vs
"reverse xmas tree".  It's like an upside down pine tree.

	asfklajsdfljasldf
	asdflkjasldfjal
	asdflkjalsdfj
	asldfkjalsdf
	asldfkjl98
	lwkejrlw
	ljklkjl
	lkjkll
	kjld
	asdf
	x

regards,
dan carpenter


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

* Re: [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq()
  2021-09-14 13:30       ` Dan Carpenter
@ 2021-09-14 14:05         ` Fabio M. De Francesco
  0 siblings, 0 replies; 31+ messages in thread
From: Fabio M. De Francesco @ 2021-09-14 14:05 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube, Pavel Skripkin

On Tuesday, September 14, 2021 3:30:07 PM CEST Dan Carpenter wrote:
> On Tue, Sep 14, 2021 at 03:24:06PM +0200, Fabio M. De Francesco wrote:
> > [...]
> David and I are saying the same thing.  "Reverse Christmas Tree" vs
> "reverse xmas tree".  It's like an upside down pine tree.
> 
> 	asfklajsdfljasldf
> 	asdflkjasldfjal
> 	asdflkjalsdfj
> 	asldfkjalsdf
> 	asldfkjl98
> 	lwkejrlw
> 	ljklkjl
> 	lkjkll
> 	kjld
> 	asdf
> 	x
> 
> regards,
> dan carpenter

Oh, sorry. I interpreted David's message the other way because it seemed to 
me that the code already had this RXT style (with the only exception of the 
"wvalue" variable that now is gone forever). Now I see that I got it wrong 
and that something got lost in translation.

Thank you again,

Fabio




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

* RE: [PATCH v4 16/18] staging: r8188eu: hal: Clean up rtw_read*() and rtw_write*()
  2021-09-13 18:10 ` [PATCH v4 16/18] staging: r8188eu: hal: Clean up rtw_read*() and rtw_write*() Fabio M. De Francesco
@ 2021-09-14 14:33   ` David Laight
  0 siblings, 0 replies; 31+ messages in thread
From: David Laight @ 2021-09-14 14:33 UTC (permalink / raw)
  To: 'Fabio M. De Francesco',
	Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
	linux-kernel, Michael Straube
  Cc: Pavel Skripkin

From: Fabio M. De Francesco
> Sent: 13 September 2021 19:10
...
> +	u16 address = (u16)(addr & 0xffff);
> 
> -	usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ);
> +	usbctrl_vendorreq(intfhdl, address, &data, 1, REALTEK_USB_VENQT_READ);

You really don't need the (u16) cast or the '& 0xffff'.
The assignment will just truncate.

But do you even need to truncate the value at all?
It rather depends on what happens inside usbctrl_vendorreq()
and whether the parameter to this code is already constrained.

I think modern gcc are better, but I have seen code where
the '& 0xffff' masked the value, then the (u16) masked the
value and finally the low 16 bits were written into a structure.

(Oh the other patches are now readable - a lot of junk down the pan.)

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


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

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

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 18:09 [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 01/18] staging: r8188eu: remove usb_{read,write}_mem Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 02/18] staging: r8188eu: remove the helpers of rtw_read8() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 03/18] staging: r8188eu: remove the helpers of rtw_read16() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 04/18] staging: r8188eu: remove the helpers of rtw_read32() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 05/18] staging: r8188eu: remove the helpers of usb_write8 Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 06/18] staging: r8188eu: remove the helpers of usb_write16() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 07/18] staging: r8188eu: remove the helpers of usb_write32() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 08/18] staging: r8188eu: remove the helpers of usb_writeN() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 09/18] staging: r8188eu: remove the helpers of rtw_read_port() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 10/18] staging: r8188eu: remove the helpers of rtw_write_port() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 11/18] staging: r8188eu: remove the helpers of usb_read_port_cancel() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 12/18] staging: r8188eu: remove the helpers of rtw_write_port_cancel() Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 13/18] staging: r8188eu: remove core/rtw_io.c Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 14/18] staging: remove struct _io_ops Fabio M. De Francesco
2021-09-13 18:09 ` [PATCH v4 15/18] staging: r8188eu: hal: Clean up usbctrl_vendorreq() Fabio M. De Francesco
2021-09-14  9:24   ` Dan Carpenter
2021-09-14 11:18     ` Fabio M. De Francesco
2021-09-14 12:20       ` Dan Carpenter
2021-09-14 13:24     ` Fabio M. De Francesco
2021-09-14 13:30       ` Dan Carpenter
2021-09-14 14:05         ` Fabio M. De Francesco
2021-09-13 18:10 ` [PATCH v4 16/18] staging: r8188eu: hal: Clean up rtw_read*() and rtw_write*() Fabio M. De Francesco
2021-09-14 14:33   ` David Laight
2021-09-13 18:10 ` [PATCH v4 17/18] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco
2021-09-13 20:19   ` Pavel Skripkin
2021-09-14  9:32   ` Dan Carpenter
2021-09-14 12:55     ` Fabio M. De Francesco
2021-09-14 13:01       ` Dan Carpenter
2021-09-13 18:10 ` [PATCH v4 18/18] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco
2021-09-13 20:35 ` [PATCH v4 00/18] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco

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).