linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Larry Finger <Larry.Finger@lwfinger.net>,
	Phillip Potter <phil@philpotter.co.uk>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Pavel Skripkin <paskripkin@gmail.com>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	David Laight <david.Laight@aculab.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Martin Kaiser <martin@kaiser.cx>
Cc: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Subject: [PATCH v8 16/19] staging: r8188eu: call the new usb_read() from rtw_read{8,16,32}()
Date: Mon, 20 Sep 2021 01:53:53 +0200	[thread overview]
Message-ID: <20210919235356.4151-17-fmdefrancesco@gmail.com> (raw)
In-Reply-To: <20210919235356.4151-1-fmdefrancesco@gmail.com>

Create and call new usb_read() instead of usbctrl_vendorreq() from
inside rtw_read{8,16,32}().

In old code, rtw_read{8,16,32}() called usbctrl_vendorreq() which in
turn uses usb_control_msg() from within a "while" loop to build a control
URB, send it off and wait for completion. usbctrl_vendorreq() was used for
both receiving and sending messages, depending on the "requesttype"
argument which was passed by callers.

Compared to usbctrl_vendorreq(), which managed both reads and writes
from and to the USB endpoint, the new usb_read() manages only reads. For
this purpose it uses the newer USB Core usb_control_msg_recv() API. The
latter is preferred according both to a suggestion by Greg Kroah-Hartman
and also to its actual design.

Two noteworthy features of usb_control_msg_recv() are that (1) the data
pointer can be made to a reference on the stack because it does not have
the restriction that usb_control_msg() has where the data pointer must be
to dynamically allocated memory, and that (2) the whole message must be
properly received from the device in order for this function to be
successfuli (if a device returns less than the expected amount of data,
then the function will fail).

usbctrl_vendorreq() uses a "while" loop that we considered unnecessary
so that it is not in the new usb_read(). Furthermore, the latter has no
redundant checking, less obvious comments, and it manages errors before
success cases. All in all, usb_read() is simpler than
usbctrl_vendorreq() and uses less lines of code.

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 | 59 +++++++++++++++++++--
 1 file changed, 56 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/r8188eu/hal/usb_ops_linux.c b/drivers/staging/r8188eu/hal/usb_ops_linux.c
index c378b5740353..53704b7c1059 100644
--- a/drivers/staging/r8188eu/hal/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/hal/usb_ops_linux.c
@@ -94,6 +94,59 @@ static int usbctrl_vendorreq(struct intf_hdl *intfhdl, u16 value, void *data, u1
 	return status;
 }
 
+static int usb_read(struct intf_hdl *intfhdl, u16 value, void *data, u8 size)
+{
+	struct adapter *adapt = intfhdl->padapter;
+	struct dvobj_priv *dvobjpriv = adapter_to_dvobj(adapt);
+	struct usb_device *udev = dvobjpriv->pusbdev;
+	int status;
+	u8 *io_buf; /* Pointer to I/O buffer */
+
+	mutex_lock(&dvobjpriv->usb_vendor_req_mutex);
+
+	if (adapt->bSurpriseRemoved || adapt->pwrctrlpriv.pnp_bstop_trx)
+		return -EPERM;
+
+	io_buf = dvobjpriv->usb_vendor_req_buf;
+
+	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 == -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;
+	}
+
+	if (status < 0) {
+		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;
+	}
+
+	rtw_reset_continual_urb_error(dvobjpriv);
+	memcpy(data, io_buf, size);
+
+mutex_unlock:
+	mutex_unlock(&dvobjpriv->usb_vendor_req_mutex);
+
+	return status;
+}
+
 u8 rtw_read8(struct adapter *adapter, u32 addr)
 {
 	struct io_priv *io_priv = &adapter->iopriv;
@@ -101,7 +154,7 @@ u8 rtw_read8(struct adapter *adapter, u32 addr)
 	u16 value = addr & 0xffff;
 	u8 data;
 
-	usbctrl_vendorreq(intfhdl, value, &data, 1, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, value, &data, 1);
 
 	return data;
 }
@@ -113,7 +166,7 @@ u16 rtw_read16(struct adapter *adapter, u32 addr)
 	u16 value = addr & 0xffff;
 	__le16 data;
 
-	usbctrl_vendorreq(intfhdl, value, &data, 2, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, value, &data, 2);
 
 	return le16_to_cpu(data);
 }
@@ -125,7 +178,7 @@ u32 rtw_read32(struct adapter *adapter, u32 addr)
 	u16 value = addr & 0xffff;
 	__le32 data;
 
-	usbctrl_vendorreq(intfhdl, value, &data, 4, REALTEK_USB_VENQT_READ);
+	usb_read(intfhdl, value, &data, 4);
 
 	return le32_to_cpu(data);
 }
-- 
2.33.0


  parent reply	other threads:[~2021-09-19 23:56 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-19 23:53 [PATCH v8 00/19] staging: r8188eu: shorten and simplify calls chains Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 01/19] staging: r8188eu: clean up symbols usbctrl_vendorreq() Fabio M. De Francesco
2021-09-20 11:46   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 02/19] staging: r8188eu: reorder declarations in usbctrl_vendorreq() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 03/19] staging: r8188eu: remove unnecessary test " Fabio M. De Francesco
2021-09-20 11:47   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 04/19] staging: r8188eu: reorder comments " Fabio M. De Francesco
2021-09-20 11:48   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 05/19] staging: r8188eu: remove unnedeed parentheses " Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 06/19] staging: r8188eu: remove unnecessary space " Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 07/19] staging: r8188eu: remove unnecessary comment " Fabio M. De Francesco
2021-09-20 11:48   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 08/19] staging: r8188eu: fix grammar mistake " Fabio M. De Francesco
2021-09-20 11:49   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 09/19] staging: r8188eu: remove unnecessary braces " Fabio M. De Francesco
2021-09-20 11:49   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 10/19] staging: r8188eu: rename symbols in rtw_read*() and rtw_write*() Fabio M. De Francesco
2021-09-20 11:50   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 11/19] staging: r8188eu: remove unnecessary casts from rtw_{read,write}*() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 12/19] staging: r8188eu: change the type of a variable in rtw_write16() Fabio M. De Francesco
2021-09-20 11:50   ` Greg Kroah-Hartman
2021-09-19 23:53 ` [PATCH v8 13/19] staging: r8188eu: remove an unneeded buffer from rtw_writeN() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 14/19] staging: r8188eu: remove an unnecessary bit AND " Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 15/19] staging: r8188eu: change the type of a variable in rtw_read16() Fabio M. De Francesco
2021-09-20 11:51   ` Greg Kroah-Hartman
2021-09-20 11:56   ` Dan Carpenter
2021-09-20 13:03     ` Fabio M. De Francesco
2021-09-20 13:10       ` Dan Carpenter
2021-09-20 16:17         ` Fabio M. De Francesco
2021-09-20 19:01           ` Dan Carpenter
2021-09-20 19:54             ` Fabio M. De Francesco
2021-09-21  5:35               ` Dan Carpenter
2021-09-19 23:53 ` Fabio M. De Francesco [this message]
2021-09-19 23:53 ` [PATCH v8 17/19] staging: r8188eu: call the new usb_write() from rtw_write{8,16,32,N}() Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 18/19] staging: r8188eu: remove shared buffer for USB requests Fabio M. De Francesco
2021-09-19 23:53 ` [PATCH v8 19/19] staging: r8188eu: remove usb_vendor_req_mutex Fabio M. De Francesco
2021-09-20 11:55 ` [PATCH v8 00/19] staging: r8188eu: shorten and simplify calls chains Greg Kroah-Hartman
2021-09-20 13:44   ` Fabio M. De Francesco
2021-09-20 14:06     ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210919235356.4151-17-fmdefrancesco@gmail.com \
    --to=fmdefrancesco@gmail.com \
    --cc=Larry.Finger@lwfinger.net \
    --cc=dan.carpenter@oracle.com \
    --cc=david.Laight@aculab.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=martin@kaiser.cx \
    --cc=paskripkin@gmail.com \
    --cc=phil@philpotter.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).