* [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain @ 2021-09-04 15:04 Fabio M. De Francesco 2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Fabio M. De Francesco @ 2021-09-04 15:04 UTC (permalink / raw) To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging, linux-kernel Cc: Fabio M. De Francesco io_ops abstraction is useless in this driver, since there is only one ops registration. Without io_ops we can get rid of indirect calls mess and shorten the calls chain. Shorten the calls chain of rtw_read8/16/32() down to the actual reads. For this purpose unify the three usb_read8/16/32 into the new usb_read(); make the latter parameterizable with 'size'; embed most of the code of usbctrl_vendorreq() into usb_read() and use in it the new usb_control_msg_recv() API of USB Core. Shorten the calls chain of rtw_write8/16/32() down to the actual writes. For this purpose unify the four usb_write8/16/32/N() into the new usb_write(); make the latter parameterizable with 'size'; embed most of the code of usbctrl_vendorreq() into usb_write() and use in it the new usb_control_msg_send() API of USB Core. The code with the modifications was thoroughly tested by Pavel Skripkin using a TP-Link TL-WN722N v2 / v3 [Realtek RTL8188EUS] Fabio M. De Francesco (2): staging: r8188eu: Shorten calls chain of rtw_read8/16/32() staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Pavel Skripkin (1): staging: r8188eu: remove _io_ops structure drivers/staging/r8188eu/core/rtw_io.c | 241 +----------------- drivers/staging/r8188eu/hal/usb_halinit.c | 6 +- drivers/staging/r8188eu/hal/usb_ops_linux.c | 232 ++++++++++------- drivers/staging/r8188eu/include/rtw_io.h | 76 +----- drivers/staging/r8188eu/include/usb_ops.h | 2 - .../staging/r8188eu/include/usb_ops_linux.h | 8 - drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +- .../staging/r8188eu/os_dep/usb_ops_linux.c | 40 +-- 8 files changed, 169 insertions(+), 438 deletions(-) -- 2.33.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] staging: r8188eu: remove _io_ops structure 2021-09-04 15:04 [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco @ 2021-09-04 15:04 ` Fabio M. De Francesco 2021-09-06 7:49 ` Dan Carpenter 2021-09-04 15:04 ` [PATCH 2/3] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco 2021-09-04 15:04 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco 2 siblings, 1 reply; 11+ messages in thread From: Fabio M. De Francesco @ 2021-09-04 15:04 UTC (permalink / raw) To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging, linux-kernel Cc: Pavel Skripkin, Fabio M . De Francesco From: Pavel Skripkin <paskripkin@gmail.com> io_ops abstraction is useless in this driver, since there is only one ops registration. Without io_ops we can get rid of indirect calls mess and shorten the calls chain. Co-developed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> --- drivers/staging/r8188eu/core/rtw_io.c | 241 +----------------- drivers/staging/r8188eu/hal/usb_halinit.c | 6 +- drivers/staging/r8188eu/hal/usb_ops_linux.c | 70 ++--- drivers/staging/r8188eu/include/rtw_io.h | 76 +----- drivers/staging/r8188eu/include/usb_ops.h | 2 - .../staging/r8188eu/include/usb_ops_linux.h | 8 - drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +- .../staging/r8188eu/os_dep/usb_ops_linux.c | 40 +-- 8 files changed, 68 insertions(+), 377 deletions(-) diff --git a/drivers/staging/r8188eu/core/rtw_io.c b/drivers/staging/r8188eu/core/rtw_io.c index cde0205816b1..2d5cc080bf1d 100644 --- a/drivers/staging/r8188eu/core/rtw_io.c +++ b/drivers/staging/r8188eu/core/rtw_io.c @@ -34,224 +34,6 @@ jackson@realtek.com.tw #define rtw_cpu_to_le16(val) cpu_to_le16(val) #define rtw_cpu_to_le32(val) cpu_to_le32(val) -u8 _rtw_read8(struct adapter *adapter, u32 addr) -{ - u8 r_val; - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - u8 (*_read8)(struct intf_hdl *pintfhdl, u32 addr); - - - _read8 = pintfhdl->io_ops._read8; - r_val = _read8(pintfhdl, addr); - - return r_val; -} - -u16 _rtw_read16(struct adapter *adapter, u32 addr) -{ - u16 r_val; - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - u16 (*_read16)(struct intf_hdl *pintfhdl, u32 addr); - - _read16 = pintfhdl->io_ops._read16; - - r_val = _read16(pintfhdl, addr); - - return r_val; -} - -u32 _rtw_read32(struct adapter *adapter, u32 addr) -{ - u32 r_val; - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - u32 (*_read32)(struct intf_hdl *pintfhdl, u32 addr); - - _read32 = pintfhdl->io_ops._read32; - - r_val = _read32(pintfhdl, addr); - - return r_val; -} - -int _rtw_write8(struct adapter *adapter, u32 addr, u8 val) -{ - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - int (*_write8)(struct intf_hdl *pintfhdl, u32 addr, u8 val); - int ret; - - _write8 = pintfhdl->io_ops._write8; - - ret = _write8(pintfhdl, addr, val); - - - return RTW_STATUS_CODE(ret); -} - -int _rtw_write16(struct adapter *adapter, u32 addr, u16 val) -{ - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - int (*_write16)(struct intf_hdl *pintfhdl, u32 addr, u16 val); - int ret; - - _write16 = pintfhdl->io_ops._write16; - - ret = _write16(pintfhdl, addr, val); - - - return RTW_STATUS_CODE(ret); -} -int _rtw_write32(struct adapter *adapter, u32 addr, u32 val) -{ - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - int (*_write32)(struct intf_hdl *pintfhdl, u32 addr, u32 val); - int ret; - - _write32 = pintfhdl->io_ops._write32; - - ret = _write32(pintfhdl, addr, val); - - - return RTW_STATUS_CODE(ret); -} - -int _rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata) -{ - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = (struct intf_hdl *)(&pio_priv->intf); - int (*_writeN)(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata); - int ret; - - _writeN = pintfhdl->io_ops._writeN; - - ret = _writeN(pintfhdl, addr, length, pdata); - - - return RTW_STATUS_CODE(ret); -} -int _rtw_write8_async(struct adapter *adapter, u32 addr, u8 val) -{ - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - int (*_write8_async)(struct intf_hdl *pintfhdl, u32 addr, u8 val); - int ret; - - _write8_async = pintfhdl->io_ops._write8_async; - - ret = _write8_async(pintfhdl, addr, val); - - - return RTW_STATUS_CODE(ret); -} - -int _rtw_write16_async(struct adapter *adapter, u32 addr, u16 val) -{ - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - int (*_write16_async)(struct intf_hdl *pintfhdl, u32 addr, u16 val); - int ret; - - _write16_async = pintfhdl->io_ops._write16_async; - ret = _write16_async(pintfhdl, addr, val); - - return RTW_STATUS_CODE(ret); -} - -int _rtw_write32_async(struct adapter *adapter, u32 addr, u32 val) -{ - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - int (*_write32_async)(struct intf_hdl *pintfhdl, u32 addr, u32 val); - int ret; - - _write32_async = pintfhdl->io_ops._write32_async; - ret = _write32_async(pintfhdl, addr, val); - - return RTW_STATUS_CODE(ret); -} - -void _rtw_read_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem) -{ - void (*_read_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem); - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - - - if (adapter->bDriverStopped || adapter->bSurpriseRemoved) - return; - _read_mem = pintfhdl->io_ops._read_mem; - _read_mem(pintfhdl, addr, cnt, pmem); - -} - -void _rtw_write_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem) -{ - void (*_write_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem); - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - - - - _write_mem = pintfhdl->io_ops._write_mem; - - _write_mem(pintfhdl, addr, cnt, pmem); - - -} - -void _rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem) -{ - u32 (*_read_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem); - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - - - - if (adapter->bDriverStopped || adapter->bSurpriseRemoved) - return; - - _read_port = pintfhdl->io_ops._read_port; - - _read_port(pintfhdl, addr, cnt, pmem); - - -} - -void _rtw_read_port_cancel(struct adapter *adapter) -{ - void (*_read_port_cancel)(struct intf_hdl *pintfhdl); - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - - _read_port_cancel = pintfhdl->io_ops._read_port_cancel; - - if (_read_port_cancel) - _read_port_cancel(pintfhdl); -} - -u32 _rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem) -{ - u32 (*_write_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem); - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - u32 ret = _SUCCESS; - - - - _write_port = pintfhdl->io_ops._write_port; - - ret = _write_port(pintfhdl, addr, cnt, pmem); - - - - return ret; -} - u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem, int timeout_ms) { int ret = _SUCCESS; @@ -261,7 +43,7 @@ u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pme rtw_sctx_init(&sctx, timeout_ms); pxmitbuf->sctx = &sctx; - ret = _rtw_write_port(adapter, addr, cnt, pmem); + ret = rtw_write_port(adapter, addr, cnt, pmem); if (ret == _SUCCESS) ret = rtw_sctx_wait(&sctx); @@ -269,31 +51,12 @@ u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pme return ret; } -void _rtw_write_port_cancel(struct adapter *adapter) -{ - void (*_write_port_cancel)(struct intf_hdl *pintfhdl); - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - - _write_port_cancel = pintfhdl->io_ops._write_port_cancel; - - if (_write_port_cancel) - _write_port_cancel(pintfhdl); -} - -int rtw_init_io_priv(struct adapter *padapter, void (*set_intf_ops)(struct _io_ops *pops)) +void rtw_init_io_priv(struct adapter *padapter) { struct io_priv *piopriv = &padapter->iopriv; struct intf_hdl *pintf = &piopriv->intf; - if (!set_intf_ops) - return _FAIL; - piopriv->padapter = padapter; pintf->padapter = padapter; pintf->pintf_dev = adapter_to_dvobj(padapter); - - set_intf_ops(&pintf->io_ops); - - return _SUCCESS; } diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c index 147c51255878..33147cbc55bb 100644 --- a/drivers/staging/r8188eu/hal/usb_halinit.c +++ b/drivers/staging/r8188eu/hal/usb_halinit.c @@ -1052,11 +1052,7 @@ static unsigned int rtl8188eu_inirp_init(struct adapter *Adapter) u8 i; struct recv_buf *precvbuf; uint status; - struct intf_hdl *pintfhdl = &Adapter->iopriv.intf; struct recv_priv *precvpriv = &Adapter->recvpriv; - u32 (*_read_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *pmem); - - _read_port = pintfhdl->io_ops._read_port; status = _SUCCESS; @@ -1065,7 +1061,7 @@ static unsigned int rtl8188eu_inirp_init(struct adapter *Adapter) /* issue Rx irp to receive data */ precvbuf = (struct recv_buf *)precvpriv->precv_buf; for (i = 0; i < NR_RECVBUFF; i++) { - if (!_read_port(pintfhdl, precvpriv->ff_hwaddr, 0, (unsigned char *)precvbuf)) { + if (!rtw_read_port(Adapter, precvpriv->ff_hwaddr, 0, (unsigned char *)precvbuf)) { status = _FAIL; goto exit; } diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c index 0cf69033c529..a87b0d2e87d0 100644 --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c @@ -97,8 +97,10 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, return status; } -static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr) +u8 rtw_read8(struct adapter *adapter, u32 addr) { + struct io_priv *pio_priv = &adapter->iopriv; + struct intf_hdl *pintfhdl = &pio_priv->intf; u16 wvalue = (u16)(addr & 0x0000ffff); u8 data; @@ -107,8 +109,10 @@ static u8 usb_read8(struct intf_hdl *pintfhdl, u32 addr) return data; } -static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr) +u16 rtw_read16(struct adapter *adapter, u32 addr) { + struct io_priv *pio_priv = &adapter->iopriv; + struct intf_hdl *pintfhdl = &pio_priv->intf; u16 wvalue = (u16)(addr & 0x0000ffff); __le32 data; @@ -117,8 +121,10 @@ static u16 usb_read16(struct intf_hdl *pintfhdl, u32 addr) return (u16)(le32_to_cpu(data) & 0xffff); } -static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr) +u32 rtw_read32(struct adapter *adapter, u32 addr) { + struct io_priv *pio_priv = &adapter->iopriv; + struct intf_hdl *pintfhdl = &pio_priv->intf; u16 wvalue = (u16)(addr & 0x0000ffff); __le32 data; @@ -127,40 +133,59 @@ static u32 usb_read32(struct intf_hdl *pintfhdl, u32 addr) return le32_to_cpu(data); } -static int usb_write8(struct intf_hdl *pintfhdl, u32 addr, u8 val) +int rtw_write8(struct adapter *adapter, u32 addr, u8 val) { + struct io_priv *pio_priv = &adapter->iopriv; + struct intf_hdl *pintfhdl = &pio_priv->intf; u16 wvalue = (u16)(addr & 0x0000ffff); + int ret; - return usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE); + ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE); + + return RTW_STATUS_CODE(ret); } -static int usb_write16(struct intf_hdl *pintfhdl, u32 addr, u16 val) +int rtw_write16(struct adapter *adapter, u32 addr, u16 val) { + struct io_priv *pio_priv = &adapter->iopriv; + struct intf_hdl *pintfhdl = &pio_priv->intf; u16 wvalue = (u16)(addr & 0x0000ffff); __le32 data = cpu_to_le32(val & 0x0000ffff); + int ret; + + ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE); - return usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE); + return RTW_STATUS_CODE(ret); } -static int usb_write32(struct intf_hdl *pintfhdl, u32 addr, u32 val) +int rtw_write32(struct adapter *adapter, u32 addr, u32 val) { + struct io_priv *pio_priv = &adapter->iopriv; + struct intf_hdl *pintfhdl = &pio_priv->intf; u16 wvalue = (u16)(addr & 0x0000ffff); __le32 data = cpu_to_le32(val); + int ret; - return usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE); + ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE); + + return RTW_STATUS_CODE(ret); } -static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata) +int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata) { + struct io_priv *pio_priv = &adapter->iopriv; + struct intf_hdl *pintfhdl = &pio_priv->intf; u16 wvalue = (u16)(addr & 0x0000ffff); u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0}; + int ret; if (length > VENDOR_CMD_MAX_DATA_LEN) return -EINVAL; memcpy(buf, pdata, length); + ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE); - return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE); + return RTW_STATUS_CODE(ret); } static void interrupt_handler_8188eu(struct adapter *adapt, u16 pkt_len, u8 *pbuf) @@ -431,11 +456,10 @@ static void usb_read_port_complete(struct urb *purb, struct pt_regs *regs) } } -static u32 usb_read_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem) +u32 rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *rmem) { struct urb *purb = NULL; struct recv_buf *precvbuf = (struct recv_buf *)rmem; - struct adapter *adapter = pintfhdl->padapter; struct dvobj_priv *pdvobj = adapter_to_dvobj(adapter); struct recv_priv *precvpriv = &adapter->recvpriv; struct usb_device *pusbd = pdvobj->pusbdev; @@ -534,26 +558,6 @@ void rtl8188eu_xmit_tasklet(unsigned long priv) } } -void rtl8188eu_set_intf_ops(struct _io_ops *pops) -{ - - memset((u8 *)pops, 0, sizeof(struct _io_ops)); - pops->_read8 = &usb_read8; - pops->_read16 = &usb_read16; - pops->_read32 = &usb_read32; - pops->_read_mem = &usb_read_mem; - pops->_read_port = &usb_read_port; - pops->_write8 = &usb_write8; - pops->_write16 = &usb_write16; - pops->_write32 = &usb_write32; - pops->_writeN = &usb_writeN; - pops->_write_mem = &usb_write_mem; - pops->_write_port = &usb_write_port; - pops->_read_port_cancel = &usb_read_port_cancel; - pops->_write_port_cancel = &usb_write_port_cancel; - -} - void rtl8188eu_set_hw_type(struct adapter *adapt) { adapt->chip_type = RTL8188E; diff --git a/drivers/staging/r8188eu/include/rtw_io.h b/drivers/staging/r8188eu/include/rtw_io.h index 4b41c7b03972..9722b76533dc 100644 --- a/drivers/staging/r8188eu/include/rtw_io.h +++ b/drivers/staging/r8188eu/include/rtw_io.h @@ -84,30 +84,6 @@ struct intf_priv; struct intf_hdl; struct io_queue; -struct _io_ops { - u8 (*_read8)(struct intf_hdl *pintfhdl, u32 addr); - u16 (*_read16)(struct intf_hdl *pintfhdl, u32 addr); - u32 (*_read32)(struct intf_hdl *pintfhdl, u32 addr); - int (*_write8)(struct intf_hdl *pintfhdl, u32 addr, u8 val); - int (*_write16)(struct intf_hdl *pintfhdl, u32 addr, u16 val); - int (*_write32)(struct intf_hdl *pintfhdl, u32 addr, u32 val); - int (*_writeN)(struct intf_hdl *pintfhdl, u32 addr, u32 length, - u8 *pdata); - int (*_write8_async)(struct intf_hdl *pintfhdl, u32 addr, u8 val); - int (*_write16_async)(struct intf_hdl *pintfhdl, u32 addr, u16 val); - int (*_write32_async)(struct intf_hdl *pintfhdl, u32 addr, u32 val); - void (*_read_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, - u8 *pmem); - void (*_write_mem)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, - u8 *pmem); - u32 (*_read_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, - u8 *pmem); - u32 (*_write_port)(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, - u8 *pmem); - void (*_read_port_cancel)(struct intf_hdl *pintfhdl); - void (*_write_port_cancel)(struct intf_hdl *pintfhdl); -}; - struct io_req { struct list_head list; u32 addr; @@ -125,7 +101,6 @@ struct io_req { struct intf_hdl { struct adapter *padapter; struct dvobj_priv *pintf_dev; - struct _io_ops io_ops; }; struct reg_protocol_rd { @@ -245,58 +220,34 @@ void unregister_intf_hdl(struct intf_hdl *pintfhdl); void _rtw_attrib_read(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); void _rtw_attrib_write(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); -u8 _rtw_read8(struct adapter *adapter, u32 addr); -u16 _rtw_read16(struct adapter *adapter, u32 addr); -u32 _rtw_read32(struct adapter *adapter, u32 addr); -void _rtw_read_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); -void _rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); -void _rtw_read_port_cancel(struct adapter *adapter); +u8 rtw_read8(struct adapter *adapter, u32 addr); +u16 rtw_read16(struct adapter *adapter, u32 addr); +u32 rtw_read32(struct adapter *adapter, u32 addr); +u32 rtw_read_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); +void rtw_read_port_cancel(struct adapter *adapter); -int _rtw_write8(struct adapter *adapter, u32 addr, u8 val); -int _rtw_write16(struct adapter *adapter, u32 addr, u16 val); -int _rtw_write32(struct adapter *adapter, u32 addr, u32 val); -int _rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata); +int rtw_write8(struct adapter *adapter, u32 addr, u8 val); +int rtw_write16(struct adapter *adapter, u32 addr, u16 val); +int rtw_write32(struct adapter *adapter, u32 addr, u32 val); +int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata); int _rtw_write8_async(struct adapter *adapter, u32 addr, u8 val); int _rtw_write16_async(struct adapter *adapter, u32 addr, u16 val); int _rtw_write32_async(struct adapter *adapter, u32 addr, u32 val); -void _rtw_write_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); -u32 _rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); +u32 rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); u32 _rtw_write_port_and_wait(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem, int timeout_ms); -void _rtw_write_port_cancel(struct adapter *adapter); - -#define rtw_read8(adapter, addr) _rtw_read8((adapter), (addr)) -#define rtw_read16(adapter, addr) _rtw_read16((adapter), (addr)) -#define rtw_read32(adapter, addr) _rtw_read32((adapter), (addr)) -#define rtw_read_mem(adapter, addr, cnt, mem) \ - _rtw_read_mem((adapter), (addr), (cnt), (mem)) -#define rtw_read_port(adapter, addr, cnt, mem) \ - _rtw_read_port((adapter), (addr), (cnt), (mem)) -#define rtw_read_port_cancel(adapter) _rtw_read_port_cancel((adapter)) - -#define rtw_write8(adapter, addr, val) \ - _rtw_write8((adapter), (addr), (val)) -#define rtw_write16(adapter, addr, val) \ - _rtw_write16((adapter), (addr), (val)) -#define rtw_write32(adapter, addr, val) \ - _rtw_write32((adapter), (addr), (val)) -#define rtw_writeN(adapter, addr, length, data) \ - _rtw_writeN((adapter), (addr), (length), (data)) +void rtw_write_port_cancel(struct adapter *adapter); + #define rtw_write8_async(adapter, addr, val) \ _rtw_write8_async((adapter), (addr), (val)) #define rtw_write16_async(adapter, addr, val) \ _rtw_write16_async((adapter), (addr), (val)) #define rtw_write32_async(adapter, addr, val) \ _rtw_write32_async((adapter), (addr), (val)) -#define rtw_write_mem(adapter, addr, cnt, mem) \ - _rtw_write_mem((adapter), (addr), (cnt), (mem)) -#define rtw_write_port(adapter, addr, cnt, mem) \ - _rtw_write_port((adapter), (addr), (cnt), (mem)) #define rtw_write_port_and_wait(adapter, addr, cnt, mem, timeout_ms) \ _rtw_write_port_and_wait((adapter), (addr), (cnt), (mem), (timeout_ms)) -#define rtw_write_port_cancel(adapter) _rtw_write_port_cancel((adapter)) void rtw_write_scsi(struct adapter *adapter, u32 cnt, u8 *pmem); @@ -340,8 +291,7 @@ void async_write32(struct adapter *adapter, u32 addr, u32 val, void async_write_mem(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); void async_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *pmem); -int rtw_init_io_priv(struct adapter *padapter, - void (*set_intf_ops)(struct _io_ops *pops)); +void rtw_init_io_priv(struct adapter *padapter); uint alloc_io_queue(struct adapter *adapter); void free_io_queue(struct adapter *adapter); diff --git a/drivers/staging/r8188eu/include/usb_ops.h b/drivers/staging/r8188eu/include/usb_ops.h index c53cc54b6b87..1939b781b097 100644 --- a/drivers/staging/r8188eu/include/usb_ops.h +++ b/drivers/staging/r8188eu/include/usb_ops.h @@ -21,8 +21,6 @@ void rtl8188eu_set_hw_type(struct adapter *padapter); #define hal_set_hw_type rtl8188eu_set_hw_type -void rtl8188eu_set_intf_ops(struct _io_ops *pops); -#define usb_set_intf_ops rtl8188eu_set_intf_ops /* * Increase and check if the continual_urb_error of this @param dvobjprivei diff --git a/drivers/staging/r8188eu/include/usb_ops_linux.h b/drivers/staging/r8188eu/include/usb_ops_linux.h index c357a3b1560e..641f059ffaf7 100644 --- a/drivers/staging/r8188eu/include/usb_ops_linux.h +++ b/drivers/staging/r8188eu/include/usb_ops_linux.h @@ -28,12 +28,4 @@ unsigned int ffaddr2pipehdl(struct dvobj_priv *pdvobj, u32 addr); -void usb_read_mem(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem); -void usb_write_mem(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem); - -void usb_read_port_cancel(struct intf_hdl *pintfhdl); - -u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem); -void usb_write_port_cancel(struct intf_hdl *pintfhdl); - #endif diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c index bb85ab77fd26..78c91b14e637 100644 --- a/drivers/staging/r8188eu/os_dep/usb_intf.c +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c @@ -596,7 +596,7 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv *dvobj, padapter->intf_stop = &usb_intf_stop; /* step init_io_priv */ - rtw_init_io_priv(padapter, usb_set_intf_ops); + rtw_init_io_priv(padapter); /* step read_chip_version */ rtw_hal_read_chip_version(padapter); diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c index 62dd4a131534..f1440a837b97 100644 --- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c +++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c @@ -31,24 +31,14 @@ struct zero_bulkout_context { void *padapter; }; -void usb_read_mem(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *rmem) -{ -} - -void usb_write_mem(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) -{ -} - -void usb_read_port_cancel(struct intf_hdl *pintfhdl) +void rtw_read_port_cancel(struct adapter *adapter) { int i; - struct recv_buf *precvbuf; - struct adapter *padapter = pintfhdl->padapter; - precvbuf = (struct recv_buf *)padapter->recvpriv.precv_buf; + struct recv_buf *precvbuf = (struct recv_buf *) adapter->recvpriv.precv_buf; DBG_88E("%s\n", __func__); - padapter->bReadPortCancel = true; + adapter->bReadPortCancel = true; for (i = 0; i < NR_RECVBUFF; i++) { precvbuf->reuse = true; @@ -134,22 +124,21 @@ static void usb_write_port_complete(struct urb *purb, struct pt_regs *regs) } -u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) +u32 rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *wmem) { unsigned long irqL; unsigned int pipe; int status; u32 ret = _FAIL; struct urb *purb = NULL; - struct adapter *padapter = (struct adapter *)pintfhdl->padapter; - struct dvobj_priv *pdvobj = adapter_to_dvobj(padapter); - struct xmit_priv *pxmitpriv = &padapter->xmitpriv; + struct dvobj_priv *pdvobj = adapter_to_dvobj(adapter); + struct xmit_priv *pxmitpriv = &adapter->xmitpriv; struct xmit_buf *pxmitbuf = (struct xmit_buf *)wmem; struct xmit_frame *pxmitframe = (struct xmit_frame *)pxmitbuf->priv_data; struct usb_device *pusbd = pdvobj->pusbdev; - if ((padapter->bDriverStopped) || (padapter->bSurpriseRemoved) || - (padapter->pwrctrlpriv.pnp_bstop_trx)) { + if ((adapter->bDriverStopped) || (adapter->bSurpriseRemoved) || + (adapter->pwrctrlpriv.pnp_bstop_trx)) { rtw_sctx_done_err(&pxmitbuf->sctx, RTW_SCTX_DONE_TX_DENY); goto exit; } @@ -196,7 +185,7 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) status = usb_submit_urb(purb, GFP_ATOMIC); if (!status) { - struct hal_data_8188e *haldata = GET_HAL_DATA(padapter); + struct hal_data_8188e *haldata = GET_HAL_DATA(adapter); haldata->srestpriv.last_tx_time = jiffies; } else { @@ -205,7 +194,7 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) switch (status) { case -ENODEV: - padapter->bDriverStopped = true; + adapter->bDriverStopped = true; break; default: break; @@ -224,15 +213,14 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) return ret; } -void usb_write_port_cancel(struct intf_hdl *pintfhdl) +void rtw_write_port_cancel(struct adapter *adapter) { int i, j; - struct adapter *padapter = pintfhdl->padapter; - struct xmit_buf *pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmitbuf; + struct xmit_buf *pxmitbuf = (struct xmit_buf *) adapter->xmitpriv.pxmitbuf; DBG_88E("%s\n", __func__); - padapter->bWritePortCancel = true; + adapter->bWritePortCancel = true; for (i = 0; i < NR_XMITBUFF; i++) { for (j = 0; j < 8; j++) { @@ -242,7 +230,7 @@ void usb_write_port_cancel(struct intf_hdl *pintfhdl) pxmitbuf++; } - pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmit_extbuf; + pxmitbuf = (struct xmit_buf *) adapter->xmitpriv.pxmit_extbuf; for (i = 0; i < NR_XMIT_EXTBUFF; i++) { for (j = 0; j < 8; j++) { if (pxmitbuf->pxmit_urb[j]) -- 2.33.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] staging: r8188eu: remove _io_ops structure 2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco @ 2021-09-06 7:49 ` Dan Carpenter 2021-09-06 14:04 ` Pavel Skripkin 0 siblings, 1 reply; 11+ messages in thread From: Dan Carpenter @ 2021-09-06 7:49 UTC (permalink / raw) To: Fabio M. De Francesco Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging, linux-kernel, Pavel Skripkin On Sat, Sep 04, 2021 at 05:04:45PM +0200, Fabio M. De Francesco wrote: > -static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata) > +int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata) > { > + struct io_priv *pio_priv = &adapter->iopriv; > + struct intf_hdl *pintfhdl = &pio_priv->intf; > u16 wvalue = (u16)(addr & 0x0000ffff); > u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0}; > + int ret; > > if (length > VENDOR_CMD_MAX_DATA_LEN) > return -EINVAL; The caller treats this negative return as success. > > memcpy(buf, pdata, length); > + ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE); > > - return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE); > + return RTW_STATUS_CODE(ret); > } [ snip ] > -void usb_read_port_cancel(struct intf_hdl *pintfhdl) > +void rtw_read_port_cancel(struct adapter *adapter) > { > int i; > - struct recv_buf *precvbuf; > - struct adapter *padapter = pintfhdl->padapter; > - precvbuf = (struct recv_buf *)padapter->recvpriv.precv_buf; > + struct recv_buf *precvbuf = (struct recv_buf *) adapter->recvpriv.precv_buf; > > DBG_88E("%s\n", __func__); > > - padapter->bReadPortCancel = true; > + adapter->bReadPortCancel = true; In these functions it would be better to rename "padapter" as "adapter" in a follow on function. Keep it the same for now just to make review easier. > > for (i = 0; i < NR_RECVBUFF; i++) { > precvbuf->reuse = true; > @@ -134,22 +124,21 @@ static void usb_write_port_complete(struct urb *purb, struct pt_regs *regs) > > } > > -u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) > +u32 rtw_write_port(struct adapter *adapter, u32 addr, u32 cnt, u8 *wmem) > { > unsigned long irqL; > unsigned int pipe; > int status; > u32 ret = _FAIL; > struct urb *purb = NULL; > - struct adapter *padapter = (struct adapter *)pintfhdl->padapter; > - struct dvobj_priv *pdvobj = adapter_to_dvobj(padapter); > - struct xmit_priv *pxmitpriv = &padapter->xmitpriv; > + struct dvobj_priv *pdvobj = adapter_to_dvobj(adapter); > + struct xmit_priv *pxmitpriv = &adapter->xmitpriv; > struct xmit_buf *pxmitbuf = (struct xmit_buf *)wmem; > struct xmit_frame *pxmitframe = (struct xmit_frame *)pxmitbuf->priv_data; > struct usb_device *pusbd = pdvobj->pusbdev; > > - if ((padapter->bDriverStopped) || (padapter->bSurpriseRemoved) || > - (padapter->pwrctrlpriv.pnp_bstop_trx)) { > + if ((adapter->bDriverStopped) || (adapter->bSurpriseRemoved) || > + (adapter->pwrctrlpriv.pnp_bstop_trx)) { Here too etc. And below. regards, dan carpenter > rtw_sctx_done_err(&pxmitbuf->sctx, RTW_SCTX_DONE_TX_DENY); > goto exit; > } > @@ -196,7 +185,7 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) > > status = usb_submit_urb(purb, GFP_ATOMIC); > if (!status) { > - struct hal_data_8188e *haldata = GET_HAL_DATA(padapter); > + struct hal_data_8188e *haldata = GET_HAL_DATA(adapter); > > haldata->srestpriv.last_tx_time = jiffies; > } else { > @@ -205,7 +194,7 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) > > switch (status) { > case -ENODEV: > - padapter->bDriverStopped = true; > + adapter->bDriverStopped = true; > break; > default: > break; > @@ -224,15 +213,14 @@ u32 usb_write_port(struct intf_hdl *pintfhdl, u32 addr, u32 cnt, u8 *wmem) > return ret; > } > > -void usb_write_port_cancel(struct intf_hdl *pintfhdl) > +void rtw_write_port_cancel(struct adapter *adapter) > { > int i, j; > - struct adapter *padapter = pintfhdl->padapter; > - struct xmit_buf *pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmitbuf; > + struct xmit_buf *pxmitbuf = (struct xmit_buf *) adapter->xmitpriv.pxmitbuf; > > DBG_88E("%s\n", __func__); > > - padapter->bWritePortCancel = true; > + adapter->bWritePortCancel = true; > > for (i = 0; i < NR_XMITBUFF; i++) { > for (j = 0; j < 8; j++) { > @@ -242,7 +230,7 @@ void usb_write_port_cancel(struct intf_hdl *pintfhdl) > pxmitbuf++; > } > > - pxmitbuf = (struct xmit_buf *)padapter->xmitpriv.pxmit_extbuf; > + pxmitbuf = (struct xmit_buf *) adapter->xmitpriv.pxmit_extbuf; > for (i = 0; i < NR_XMIT_EXTBUFF; i++) { > for (j = 0; j < 8; j++) { > if (pxmitbuf->pxmit_urb[j]) > -- > 2.33.0 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] staging: r8188eu: remove _io_ops structure 2021-09-06 7:49 ` Dan Carpenter @ 2021-09-06 14:04 ` Pavel Skripkin 0 siblings, 0 replies; 11+ messages in thread From: Pavel Skripkin @ 2021-09-06 14:04 UTC (permalink / raw) To: Dan Carpenter, Fabio M. De Francesco Cc: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging, linux-kernel On 9/6/21 10:49 AM, Dan Carpenter wrote: > On Sat, Sep 04, 2021 at 05:04:45PM +0200, Fabio M. De Francesco wrote: >> -static int usb_writeN(struct intf_hdl *pintfhdl, u32 addr, u32 length, u8 *pdata) >> +int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata) >> { >> + struct io_priv *pio_priv = &adapter->iopriv; >> + struct intf_hdl *pintfhdl = &pio_priv->intf; >> u16 wvalue = (u16)(addr & 0x0000ffff); >> u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0}; >> + int ret; >> >> if (length > VENDOR_CMD_MAX_DATA_LEN) >> return -EINVAL; > > The caller treats this negative return as success. > Oh, good catch, thank you so much for pointing it out... This driver uses unusual approach for error handling with private _SUCCESS and _FAIL macros and I forgot to call RTW_ERROR_CODE :( >> >> memcpy(buf, pdata, length); >> + ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE); >> >> - return usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE); >> + return RTW_STATUS_CODE(ret); >> } > > [ snip ] > > >> -void usb_read_port_cancel(struct intf_hdl *pintfhdl) >> +void rtw_read_port_cancel(struct adapter *adapter) >> { >> int i; >> - struct recv_buf *precvbuf; >> - struct adapter *padapter = pintfhdl->padapter; >> - precvbuf = (struct recv_buf *)padapter->recvpriv.precv_buf; >> + struct recv_buf *precvbuf = (struct recv_buf *) adapter->recvpriv.precv_buf; >> >> DBG_88E("%s\n", __func__); >> >> - padapter->bReadPortCancel = true; >> + adapter->bReadPortCancel = true; > > In these functions it would be better to rename "padapter" as "adapter" > in a follow on function. Keep it the same for now just to make review > easier. > Ok, will bring old name back in v4. Thank you for review :) With regards, Pavel Skripkin ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() 2021-09-04 15:04 [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco 2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco @ 2021-09-04 15:04 ` Fabio M. De Francesco 2021-09-04 15:04 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco 2 siblings, 0 replies; 11+ messages in thread From: Fabio M. De Francesco @ 2021-09-04 15:04 UTC (permalink / raw) To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging, linux-kernel Cc: Fabio M. De Francesco, Pavel Skripkin Shorten the calls chain of rtw_read8/16/32() down to the actual reads. For this purpose unify the three usb_read8/16/32 into the new usb_read(); make the latter parameterizable with 'size'; embed most of the code of usbctrl_vendorreq() into usb_read() and use in it the new usb_control_msg_recv() API of USB Core. Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Co-developed-by: Pavel Skripkin <paskripkin@gmail.com> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- drivers/staging/r8188eu/hal/usb_ops_linux.c | 92 +++++++++++++++++---- 1 file changed, 78 insertions(+), 14 deletions(-) diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c index a87b0d2e87d0..f9c4fd5a2c53 100644 --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c @@ -97,38 +97,102 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, return status; } +static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) +{ + u16 value = (u16)(addr & 0x0000ffff); + struct adapter *adapt = intfhdl->padapter; + struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt); + struct usb_device *udev = dvobjpriv->pusbdev; + int status; + u8 *io_buf; + int vendorreq_times = 0; + + if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) { + status = -EPERM; + goto exit; + } + + mutex_lock(&dvobjpriv->usb_vendor_req_mutex); + + /* Acquire IO memory for vendorreq */ + io_buf = dvobjpriv->usb_vendor_req_buf; + + if (!io_buf) { + DBG_88E("[%s] io_buf == NULL\n", __func__); + status = -ENOMEM; + goto release_mutex; + } + + while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) { + status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ, + REALTEK_USB_VENQT_READ, value, + REALTEK_USB_VENQT_CMD_IDX, io_buf, + size, RTW_USB_CONTROL_MSG_TIMEOUT, + GFP_KERNEL); + if (!status) { /* Success this control transfer. */ + rtw_reset_continual_urb_error(dvobjpriv); + memcpy(data, io_buf, size); + } else { /* error cases */ + DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n", + value, "read", size, status, vendorreq_times); + + if (status == (-ESHUTDOWN) || status == -ENODEV) { + adapt->bSurpriseRemoved = true; + } else { + struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); + + haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL; + } + + if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) { + adapt->bSurpriseRemoved = true; + break; + } + } + + /* firmware download is checksummed, don't retry */ + if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status) + break; + } + +release_mutex: + mutex_unlock(&dvobjpriv->usb_vendor_req_mutex); +exit: + return status; +} + u8 rtw_read8(struct adapter *adapter, u32 addr) { - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - u16 wvalue = (u16)(addr & 0x0000ffff); + struct io_priv *io_priv = &adapter->iopriv; + struct intf_hdl *intfhdl = &io_priv->intf; + u16 value = (u16)(addr & 0x0000ffff); u8 data; - usbctrl_vendorreq(pintfhdl, wvalue, &data, 1, REALTEK_USB_VENQT_READ); + usb_read(intfhdl, value, &data, 1); return data; } u16 rtw_read16(struct adapter *adapter, u32 addr) { - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - u16 wvalue = (u16)(addr & 0x0000ffff); - __le32 data; + struct io_priv *io_priv = &adapter->iopriv; + struct intf_hdl *intfhdl = &io_priv->intf; + u16 value = (u16)(addr & 0x0000ffff); + __le16 data; - usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_READ); + usb_read(intfhdl, value, &data, 2); - return (u16)(le32_to_cpu(data) & 0xffff); + return le16_to_cpu(data); } u32 rtw_read32(struct adapter *adapter, u32 addr) { - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - u16 wvalue = (u16)(addr & 0x0000ffff); + struct io_priv *io_priv = &adapter->iopriv; + struct intf_hdl *intfhdl = &io_priv->intf; + u16 value = (u16)(addr & 0x0000ffff); __le32 data; - usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_READ); + usb_read(intfhdl, value, &data, 4); return le32_to_cpu(data); } -- 2.33.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() 2021-09-04 15:04 [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco 2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco 2021-09-04 15:04 ` [PATCH 2/3] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco @ 2021-09-04 15:04 ` Fabio M. De Francesco 2021-09-04 17:09 ` kernel test robot 2 siblings, 1 reply; 11+ messages in thread From: Fabio M. De Francesco @ 2021-09-04 15:04 UTC (permalink / raw) To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging, linux-kernel Cc: Fabio M. De Francesco, Pavel Skripkin Shorten the calls chain of rtw_write8/16/32() down to the actual writes. For this purpose unify the four usb_write8/16/32/N() into the new usb_write(); make the latter parameterizable with 'size'; embed most of the code of usbctrl_vendorreq() into usb_write() and use in it the new usb_control_msg_send() API of USB Core. Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Co-developed-by: Pavel Skripkin <paskripkin@gmail.com> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com> --- drivers/staging/r8188eu/hal/usb_ops_linux.c | 102 ++++++++------------ 1 file changed, 39 insertions(+), 63 deletions(-) diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c index f9c4fd5a2c53..8584baca9b8e 100644 --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c @@ -8,76 +8,51 @@ #include "../include/recv_osdep.h" #include "../include/rtl8188e_hal.h" -static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, u16 len, u8 requesttype) +static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) { - struct adapter *adapt = pintfhdl->padapter; - struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt); + u16 value = (u16)(addr & 0x0000ffff); + struct adapter *adapt = intfhdl->padapter; + struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt); struct usb_device *udev = dvobjpriv->pusbdev; - unsigned int pipe; - int status = 0; - u8 *pIo_buf; + int status; + u8 *io_buf; int vendorreq_times = 0; - if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) { + if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) { status = -EPERM; goto exit; } - if (len > MAX_VENDOR_REQ_CMD_SIZE) { - DBG_88E("[%s] Buffer len error ,vendor request failed\n", __func__); - status = -EINVAL; - goto exit; - } - - _enter_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL); + mutex_lock(&dvobjpriv->usb_vendor_req_mutex); /* Acquire IO memory for vendorreq */ - pIo_buf = dvobjpriv->usb_vendor_req_buf; + io_buf = dvobjpriv->usb_vendor_req_buf; - if (!pIo_buf) { - DBG_88E("[%s] pIo_buf == NULL\n", __func__); + if (!io_buf) { + DBG_88E("[%s] io_buf == NULL\n", __func__); status = -ENOMEM; goto release_mutex; } - if (requesttype == REALTEK_USB_VENQT_READ) - pipe = usb_rcvctrlpipe(udev, 0);/* read_in */ - else - pipe = usb_sndctrlpipe(udev, 0);/* write_out */ - while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) { - if (requesttype == REALTEK_USB_VENQT_READ) - memset(pIo_buf, 0, len); - else - memcpy(pIo_buf, pdata, len); - - status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ, - requesttype, value, REALTEK_USB_VENQT_CMD_IDX, - pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT); - - if (status == len) { /* Success this control transfer. */ + status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ, + REALTEK_USB_VENQT_READ, value, + REALTEK_USB_VENQT_CMD_IDX, io_buf, + size, RTW_USB_CONTROL_MSG_TIMEOUT, + GFP_KERNEL); + if (!status) { /* Success this control transfer. */ rtw_reset_continual_urb_error(dvobjpriv); - if (requesttype == REALTEK_USB_VENQT_READ) - memcpy(pdata, pIo_buf, len); + memcpy(data, io_buf, size); } else { /* error cases */ - DBG_88E("reg 0x%x, usb %s %u fail, status:%d value=0x%x, vendorreq_times:%d\n", - value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write", - len, status, *(u32 *)pdata, vendorreq_times); - - if (status < 0) { - if (status == (-ESHUTDOWN) || status == -ENODEV) { - adapt->bSurpriseRemoved = true; - } else { - struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); - haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL; - } - } else { /* status != len && status >= 0 */ - if (status > 0) { - if (requesttype == REALTEK_USB_VENQT_READ) { - /* For Control read transfer, we have to copy the read data from pIo_buf to pdata. */ - memcpy(pdata, pIo_buf, len); - } - } + DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n", + value, "read", size, status, vendorreq_times); + + if (status == (-ESHUTDOWN) || status == -ENODEV) { + adapt->bSurpriseRemoved = true; + } else { + struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); + + haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL; } if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) { @@ -87,17 +62,18 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, } - /* firmware download is checksumed, don't retry */ - if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || status == len) + /* firmware download is checksummed, don't retry */ + if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status) break; } + release_mutex: - _exit_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL); + mutex_unlock(&dvobjpriv->usb_vendor_req_mutex); exit: return status; } -static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) +static int usb_write(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) { u16 value = (u16)(addr & 0x0000ffff); struct adapter *adapt = intfhdl->padapter; @@ -123,15 +99,15 @@ static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) goto release_mutex; } + memcpy(io_buf, data, size); while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) { - status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ, - REALTEK_USB_VENQT_READ, value, + status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ, + REALTEK_USB_VENQT_SEND, value, REALTEK_USB_VENQT_CMD_IDX, io_buf, size, RTW_USB_CONTROL_MSG_TIMEOUT, GFP_KERNEL); if (!status) { /* Success this control transfer. */ rtw_reset_continual_urb_error(dvobjpriv); - memcpy(data, io_buf, size); } else { /* error cases */ DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n", value, "read", size, status, vendorreq_times); @@ -204,7 +180,7 @@ int rtw_write8(struct adapter *adapter, u32 addr, u8 val) u16 wvalue = (u16)(addr & 0x0000ffff); int ret; - ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE); + ret = usb_write(pintfhdl, wvalue, &val, 1); return RTW_STATUS_CODE(ret); } @@ -217,7 +193,7 @@ int rtw_write16(struct adapter *adapter, u32 addr, u16 val) __le32 data = cpu_to_le32(val & 0x0000ffff); int ret; - ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE); + ret = usb_write(pintfhdl, wvalue, &data, 2); return RTW_STATUS_CODE(ret); } @@ -230,7 +206,7 @@ int rtw_write32(struct adapter *adapter, u32 addr, u32 val) __le32 data = cpu_to_le32(val); int ret; - ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE); + ret = usb_write(pintfhdl, wvalue, &data, 4); return RTW_STATUS_CODE(ret); } @@ -247,7 +223,7 @@ int rtw_writeN(struct adapter *adapter, u32 addr, u32 length, u8 *pdata) return -EINVAL; memcpy(buf, pdata, length); - ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE); + ret = usb_write(pintfhdl, wvalue, buf, (length & 0xffff)); return RTW_STATUS_CODE(ret); } -- 2.33.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() 2021-09-04 15:04 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco @ 2021-09-04 17:09 ` kernel test robot 0 siblings, 0 replies; 11+ messages in thread From: kernel test robot @ 2021-09-04 17:09 UTC (permalink / raw) To: Fabio M. De Francesco, Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging, linux-kernel Cc: kbuild-all, Fabio M. De Francesco, Pavel Skripkin [-- Attachment #1: Type: text/plain, Size: 4404 bytes --] Hi "Fabio, Thank you for the patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] url: https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git fde4862d1ac7100028da4371d92454fec6cf3f4f config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/a08da7fbe9a78b73da29c82c244b022186aa035c git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Fabio-M-De-Francesco/staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010 git checkout a08da7fbe9a78b73da29c82c244b022186aa035c # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash drivers/staging/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/staging/r8188eu/hal/usb_ops_linux.c: In function 'usb_write': >> drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: error: 'REALTEK_USB_VENQT_SEND' undeclared (first use in this function); did you mean 'REALTEK_USB_VENQT_READ'? 105 | REALTEK_USB_VENQT_SEND, value, | ^~~~~~~~~~~~~~~~~~~~~~ | REALTEK_USB_VENQT_READ drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: note: each undeclared identifier is reported only once for each function it appears in vim +105 drivers/staging/r8188eu/hal/usb_ops_linux.c 75 76 static int usb_write(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) 77 { 78 u16 value = (u16)(addr & 0x0000ffff); 79 struct adapter *adapt = intfhdl->padapter; 80 struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt); 81 struct usb_device *udev = dvobjpriv->pusbdev; 82 int status; 83 u8 *io_buf; 84 int vendorreq_times = 0; 85 86 if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) { 87 status = -EPERM; 88 goto exit; 89 } 90 91 mutex_lock(&dvobjpriv->usb_vendor_req_mutex); 92 93 /* Acquire IO memory for vendorreq */ 94 io_buf = dvobjpriv->usb_vendor_req_buf; 95 96 if (!io_buf) { 97 DBG_88E("[%s] io_buf == NULL\n", __func__); 98 status = -ENOMEM; 99 goto release_mutex; 100 } 101 102 memcpy(io_buf, data, size); 103 while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) { 104 status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ, > 105 REALTEK_USB_VENQT_SEND, value, 106 REALTEK_USB_VENQT_CMD_IDX, io_buf, 107 size, RTW_USB_CONTROL_MSG_TIMEOUT, 108 GFP_KERNEL); 109 if (!status) { /* Success this control transfer. */ 110 rtw_reset_continual_urb_error(dvobjpriv); 111 } else { /* error cases */ 112 DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n", 113 value, "read", size, status, vendorreq_times); 114 115 if (status == (-ESHUTDOWN) || status == -ENODEV) { 116 adapt->bSurpriseRemoved = true; 117 } else { 118 struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); 119 120 haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL; 121 } 122 123 if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) { 124 adapt->bSurpriseRemoved = true; 125 break; 126 } 127 } 128 129 /* firmware download is checksummed, don't retry */ 130 if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status) 131 break; 132 } 133 134 release_mutex: 135 mutex_unlock(&dvobjpriv->usb_vendor_req_mutex); 136 exit: 137 return status; 138 } 139 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 55310 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() @ 2021-09-04 17:09 ` kernel test robot 0 siblings, 0 replies; 11+ messages in thread From: kernel test robot @ 2021-09-04 17:09 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 4511 bytes --] Hi "Fabio, Thank you for the patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] url: https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git fde4862d1ac7100028da4371d92454fec6cf3f4f config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/a08da7fbe9a78b73da29c82c244b022186aa035c git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Fabio-M-De-Francesco/staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010 git checkout a08da7fbe9a78b73da29c82c244b022186aa035c # save the attached .config to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash drivers/staging/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): drivers/staging/r8188eu/hal/usb_ops_linux.c: In function 'usb_write': >> drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: error: 'REALTEK_USB_VENQT_SEND' undeclared (first use in this function); did you mean 'REALTEK_USB_VENQT_READ'? 105 | REALTEK_USB_VENQT_SEND, value, | ^~~~~~~~~~~~~~~~~~~~~~ | REALTEK_USB_VENQT_READ drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: note: each undeclared identifier is reported only once for each function it appears in vim +105 drivers/staging/r8188eu/hal/usb_ops_linux.c 75 76 static int usb_write(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) 77 { 78 u16 value = (u16)(addr & 0x0000ffff); 79 struct adapter *adapt = intfhdl->padapter; 80 struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt); 81 struct usb_device *udev = dvobjpriv->pusbdev; 82 int status; 83 u8 *io_buf; 84 int vendorreq_times = 0; 85 86 if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) { 87 status = -EPERM; 88 goto exit; 89 } 90 91 mutex_lock(&dvobjpriv->usb_vendor_req_mutex); 92 93 /* Acquire IO memory for vendorreq */ 94 io_buf = dvobjpriv->usb_vendor_req_buf; 95 96 if (!io_buf) { 97 DBG_88E("[%s] io_buf == NULL\n", __func__); 98 status = -ENOMEM; 99 goto release_mutex; 100 } 101 102 memcpy(io_buf, data, size); 103 while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) { 104 status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ, > 105 REALTEK_USB_VENQT_SEND, value, 106 REALTEK_USB_VENQT_CMD_IDX, io_buf, 107 size, RTW_USB_CONTROL_MSG_TIMEOUT, 108 GFP_KERNEL); 109 if (!status) { /* Success this control transfer. */ 110 rtw_reset_continual_urb_error(dvobjpriv); 111 } else { /* error cases */ 112 DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n", 113 value, "read", size, status, vendorreq_times); 114 115 if (status == (-ESHUTDOWN) || status == -ENODEV) { 116 adapt->bSurpriseRemoved = true; 117 } else { 118 struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); 119 120 haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL; 121 } 122 123 if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) { 124 adapt->bSurpriseRemoved = true; 125 break; 126 } 127 } 128 129 /* firmware download is checksummed, don't retry */ 130 if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status) 131 break; 132 } 133 134 release_mutex: 135 mutex_unlock(&dvobjpriv->usb_vendor_req_mutex); 136 exit: 137 return status; 138 } 139 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 55310 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() 2021-09-04 17:09 ` kernel test robot @ 2021-09-04 20:41 ` Fabio M. De Francesco -1 siblings, 0 replies; 11+ messages in thread From: Fabio M. De Francesco @ 2021-09-04 20:41 UTC (permalink / raw) To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, Pavel Skripkin Cc: linux-staging, linux-kernel, kernel test robot, kbuild-all On Saturday, September 4, 2021 7:09:58 PM CEST kernel test robot wrote: > Hi "Fabio, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on staging/staging-testing] > > url: https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/ staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010 > base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git fde4862d1ac7100028da4371d92454fec6cf3f4f > > [...] > > All errors (new ones prefixed by >>): > > drivers/staging/r8188eu/hal/usb_ops_linux.c: In function 'usb_write': > >> drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: error: 'REALTEK_USB_VENQT_SEND' undeclared (first use in this function); did you mean 'REALTEK_USB_VENQT_READ'? > 105 | REALTEK_USB_VENQT_SEND, value, > | ^~~~~~~~~~~~~~~~~~~~~~ > Hello to all, I apologize for this easily avoidable mistake. I'm about to post the v2 series. However, I write this message to affirm that Pavel's tests on this patch are still valid and confirmed because his copy *has* the right parameter that is REALTEK_USB_VENQT_WRITE. It's all my fault. Pavel warned me that during the review and test he noticed that I had written REALTEK_USB_VENQT_READ and obviously my build was successful because it was a symbol known to the compiler. Pavel successfully changed his local copy and I (independently) changed mine without thinking about rebuilding. I was absolutely certain it was the expected parameter (perhaps the function name - usb_control_msg_send () - played a part in leading me to this stupid mistake). In summary, first I want to apologize for the noise, secondly I want to apologize to Pavel who is co-author and tester of this 3/3 of our series. Regards, Fabio ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() @ 2021-09-04 20:41 ` Fabio M. De Francesco 0 siblings, 0 replies; 11+ messages in thread From: Fabio M. De Francesco @ 2021-09-04 20:41 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1917 bytes --] On Saturday, September 4, 2021 7:09:58 PM CEST kernel test robot wrote: > Hi "Fabio, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on staging/staging-testing] > > url: https://github.com/0day-ci/linux/commits/Fabio-M-De-Francesco/ staging-r8188eu-Shorten-and-simplify-calls-chain/20210904-231010 > base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git fde4862d1ac7100028da4371d92454fec6cf3f4f > > [...] > > All errors (new ones prefixed by >>): > > drivers/staging/r8188eu/hal/usb_ops_linux.c: In function 'usb_write': > >> drivers/staging/r8188eu/hal/usb_ops_linux.c:105:47: error: 'REALTEK_USB_VENQT_SEND' undeclared (first use in this function); did you mean 'REALTEK_USB_VENQT_READ'? > 105 | REALTEK_USB_VENQT_SEND, value, > | ^~~~~~~~~~~~~~~~~~~~~~ > Hello to all, I apologize for this easily avoidable mistake. I'm about to post the v2 series. However, I write this message to affirm that Pavel's tests on this patch are still valid and confirmed because his copy *has* the right parameter that is REALTEK_USB_VENQT_WRITE. It's all my fault. Pavel warned me that during the review and test he noticed that I had written REALTEK_USB_VENQT_READ and obviously my build was successful because it was a symbol known to the compiler. Pavel successfully changed his local copy and I (independently) changed mine without thinking about rebuilding. I was absolutely certain it was the expected parameter (perhaps the function name - usb_control_msg_send () - played a part in leading me to this stupid mistake). In summary, first I want to apologize for the noise, secondly I want to apologize to Pavel who is co-author and tester of this 3/3 of our series. Regards, Fabio ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 0/3] staging: r8188eu: Shorten and simplify calls chain @ 2021-09-04 21:27 Fabio M. De Francesco 2021-09-04 21:27 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco 0 siblings, 1 reply; 11+ messages in thread From: Fabio M. De Francesco @ 2021-09-04 21:27 UTC (permalink / raw) To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, Pavel Skripkin, linux-staging, linux-kernel Cc: Fabio M. De Francesco io_ops abstraction is useless in this driver, since there is only one ops registration. Without io_ops we can get rid of indirect calls mess and shorten the calls chain. Shorten the calls chain of rtw_read8/16/32() down to the actual reads. For this purpose unify the three usb_read8/16/32 into the new usb_read(); make the latter parameterizable with 'size'; embed most of the code of usbctrl_vendorreq() into usb_read() and use in it the new usb_control_msg_recv() API of USB Core. Shorten the calls chain of rtw_write8/16/32() down to the actual writes. For this purpose unify the four usb_write8/16/32/N() into the new usb_write(); make the latter parameterizable with 'size'; embed most of the code of usbctrl_vendorreq() into usb_write() and use in it the new usb_control_msg_send() API of USB Core. The code with the modifications was thoroughly tested by Pavel Skripkin using a TP-Link TL-WN722N v2 / v3 [Realtek RTL8188EUS] Fabio M. De Francesco (2): staging: r8188eu: Shorten calls chain of rtw_read8/16/32() staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Pavel Skripkin (1): staging: r8188eu: remove _io_ops structure drivers/staging/r8188eu/core/rtw_io.c | 241 +---------------- drivers/staging/r8188eu/hal/usb_halinit.c | 6 +- drivers/staging/r8188eu/hal/usb_ops_linux.c | 242 +++++++++++------- drivers/staging/r8188eu/include/rtw_io.h | 76 +----- drivers/staging/r8188eu/include/usb_ops.h | 2 - .../staging/r8188eu/include/usb_ops_linux.h | 8 - drivers/staging/r8188eu/os_dep/usb_intf.c | 2 +- .../staging/r8188eu/os_dep/usb_ops_linux.c | 40 +-- 8 files changed, 174 insertions(+), 443 deletions(-) -- 2.33.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() 2021-09-04 21:27 [PATCH v2 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco @ 2021-09-04 21:27 ` Fabio M. De Francesco 0 siblings, 0 replies; 11+ messages in thread From: Fabio M. De Francesco @ 2021-09-04 21:27 UTC (permalink / raw) To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, Pavel Skripkin, linux-staging, linux-kernel Cc: Fabio M. De Francesco 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> --- 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 | 130 ++++++++------------ 1 file changed, 53 insertions(+), 77 deletions(-) diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c index f9c4fd5a2c53..e31d1b1fdb12 100644 --- a/drivers/staging/r8188eu/hal/usb_ops_linux.c +++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c @@ -8,76 +8,51 @@ #include "../include/recv_osdep.h" #include "../include/rtl8188e_hal.h" -static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, u16 len, u8 requesttype) +static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) { - struct adapter *adapt = pintfhdl->padapter; - struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt); + u16 value = (u16)(addr & 0x0000ffff); + struct adapter *adapt = intfhdl->padapter; + struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt); struct usb_device *udev = dvobjpriv->pusbdev; - unsigned int pipe; - int status = 0; - u8 *pIo_buf; + int status; + u8 *io_buf; int vendorreq_times = 0; - if ((adapt->bSurpriseRemoved) || (adapt->pwrctrlpriv.pnp_bstop_trx)) { + if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx) { status = -EPERM; goto exit; } - if (len > MAX_VENDOR_REQ_CMD_SIZE) { - DBG_88E("[%s] Buffer len error ,vendor request failed\n", __func__); - status = -EINVAL; - goto exit; - } - - _enter_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL); + mutex_lock(&dvobjpriv->usb_vendor_req_mutex); /* Acquire IO memory for vendorreq */ - pIo_buf = dvobjpriv->usb_vendor_req_buf; + io_buf = dvobjpriv->usb_vendor_req_buf; - if (!pIo_buf) { - DBG_88E("[%s] pIo_buf == NULL\n", __func__); + if (!io_buf) { + DBG_88E("[%s] io_buf == NULL\n", __func__); status = -ENOMEM; goto release_mutex; } - if (requesttype == REALTEK_USB_VENQT_READ) - pipe = usb_rcvctrlpipe(udev, 0);/* read_in */ - else - pipe = usb_sndctrlpipe(udev, 0);/* write_out */ - while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) { - if (requesttype == REALTEK_USB_VENQT_READ) - memset(pIo_buf, 0, len); - else - memcpy(pIo_buf, pdata, len); - - status = usb_control_msg(udev, pipe, REALTEK_USB_VENQT_CMD_REQ, - requesttype, value, REALTEK_USB_VENQT_CMD_IDX, - pIo_buf, len, RTW_USB_CONTROL_MSG_TIMEOUT); - - if (status == len) { /* Success this control transfer. */ + status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ, + REALTEK_USB_VENQT_READ, value, + REALTEK_USB_VENQT_CMD_IDX, io_buf, + size, RTW_USB_CONTROL_MSG_TIMEOUT, + GFP_KERNEL); + if (!status) { /* Success this control transfer. */ rtw_reset_continual_urb_error(dvobjpriv); - if (requesttype == REALTEK_USB_VENQT_READ) - memcpy(pdata, pIo_buf, len); + memcpy(data, io_buf, size); } else { /* error cases */ - DBG_88E("reg 0x%x, usb %s %u fail, status:%d value=0x%x, vendorreq_times:%d\n", - value, (requesttype == REALTEK_USB_VENQT_READ) ? "read" : "write", - len, status, *(u32 *)pdata, vendorreq_times); - - if (status < 0) { - if (status == (-ESHUTDOWN) || status == -ENODEV) { - adapt->bSurpriseRemoved = true; - } else { - struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); - haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL; - } - } else { /* status != len && status >= 0 */ - if (status > 0) { - if (requesttype == REALTEK_USB_VENQT_READ) { - /* For Control read transfer, we have to copy the read data from pIo_buf to pdata. */ - memcpy(pdata, pIo_buf, len); - } - } + DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n", + value, "read", size, status, vendorreq_times); + + if (status == (-ESHUTDOWN) || status == -ENODEV) { + adapt->bSurpriseRemoved = true; + } else { + struct hal_data_8188e *haldata = GET_HAL_DATA(adapt); + + haldata->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL; } if (rtw_inc_and_chk_continual_urb_error(dvobjpriv)) { @@ -87,17 +62,18 @@ static int usbctrl_vendorreq(struct intf_hdl *pintfhdl, u16 value, void *pdata, } - /* firmware download is checksumed, don't retry */ - if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || status == len) + /* firmware download is checksummed, don't retry */ + if ((value >= FW_8188E_START_ADDRESS && value <= FW_8188E_END_ADDRESS) || !status) break; } + release_mutex: - _exit_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL); + mutex_unlock(&dvobjpriv->usb_vendor_req_mutex); exit: return status; } -static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) +static int usb_write(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) { u16 value = (u16)(addr & 0x0000ffff); struct adapter *adapt = intfhdl->padapter; @@ -123,15 +99,15 @@ static int usb_read(struct intf_hdl *intfhdl, u32 addr, void *data, u8 size) goto release_mutex; } + memcpy(io_buf, data, size); while (++vendorreq_times <= MAX_USBCTRL_VENDORREQ_TIMES) { - status = usb_control_msg_recv(udev, 0, REALTEK_USB_VENQT_CMD_REQ, - REALTEK_USB_VENQT_READ, value, + status = usb_control_msg_send(udev, 0, REALTEK_USB_VENQT_CMD_REQ, + REALTEK_USB_VENQT_WRITE, value, REALTEK_USB_VENQT_CMD_IDX, io_buf, size, RTW_USB_CONTROL_MSG_TIMEOUT, GFP_KERNEL); if (!status) { /* Success this control transfer. */ rtw_reset_continual_urb_error(dvobjpriv); - memcpy(data, io_buf, size); } else { /* error cases */ DBG_88E("reg 0x%x, usb %s %u fail, status:%d vendorreq_times:%d\n", value, "read", size, status, vendorreq_times); @@ -199,55 +175,55 @@ u32 rtw_read32(struct adapter *adapter, u32 addr) 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); + struct io_priv *io_priv = &adapter->iopriv; + struct intf_hdl *intfhdl = &io_priv->intf; + u16 value = (u16)(addr & 0x0000ffff); int ret; - ret = usbctrl_vendorreq(pintfhdl, wvalue, &val, 1, REALTEK_USB_VENQT_WRITE); + ret = usb_write(intfhdl, value, &val, 1); 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); + struct io_priv *io_priv = &adapter->iopriv; + struct intf_hdl *intfhdl = &io_priv->intf; + u16 value = (u16)(addr & 0x0000ffff); __le32 data = cpu_to_le32(val & 0x0000ffff); int ret; - ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 2, REALTEK_USB_VENQT_WRITE); + ret = usb_write(intfhdl, value, &data, 2); 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); + struct io_priv *io_priv = &adapter->iopriv; + struct intf_hdl *intfhdl = &io_priv->intf; + u16 value = (u16)(addr & 0x0000ffff); __le32 data = cpu_to_le32(val); int ret; - ret = usbctrl_vendorreq(pintfhdl, wvalue, &data, 4, REALTEK_USB_VENQT_WRITE); + ret = usb_write(intfhdl, value, &data, 4); 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 length, u8 *data) { - struct io_priv *pio_priv = &adapter->iopriv; - struct intf_hdl *pintfhdl = &pio_priv->intf; - u16 wvalue = (u16)(addr & 0x0000ffff); + struct io_priv *io_priv = &adapter->iopriv; + struct intf_hdl *intfhdl = &io_priv->intf; + u16 value = (u16)(addr & 0x0000ffff); u8 buf[VENDOR_CMD_MAX_DATA_LEN] = {0}; int ret; if (length > VENDOR_CMD_MAX_DATA_LEN) return -EINVAL; - memcpy(buf, pdata, length); - ret = usbctrl_vendorreq(pintfhdl, wvalue, buf, (length & 0xffff), REALTEK_USB_VENQT_WRITE); + memcpy(buf, data, length); + ret = usb_write(intfhdl, value, buf, (length & 0xffff)); return RTW_STATUS_CODE(ret); } -- 2.33.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-09-06 14:04 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-04 15:04 [PATCH 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco 2021-09-04 15:04 ` [PATCH 1/3] staging: r8188eu: remove _io_ops structure Fabio M. De Francesco 2021-09-06 7:49 ` Dan Carpenter 2021-09-06 14:04 ` Pavel Skripkin 2021-09-04 15:04 ` [PATCH 2/3] staging: r8188eu: Shorten calls chain of rtw_read8/16/32() Fabio M. De Francesco 2021-09-04 15:04 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco 2021-09-04 17:09 ` kernel test robot 2021-09-04 17:09 ` kernel test robot 2021-09-04 20:41 ` Fabio M. De Francesco 2021-09-04 20:41 ` Fabio M. De Francesco 2021-09-04 21:27 [PATCH v2 0/3] staging: r8188eu: Shorten and simplify calls chain Fabio M. De Francesco 2021-09-04 21:27 ` [PATCH 3/3] staging: r8188eu: Shorten calls chain of rtw_write8/16/32/N() Fabio M. De Francesco
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.