Clean up and simplify the usb_write_port_complete callback function. Yet
again, this is based on the previous patches I submitted.
Martin Kaiser (4):
staging: r8188eu: refactor status handling in usb_write_port_complete
staging: r8188eu: reformat usb_write_port_complete
staging: r8188eu: remove unused function parameter
staging: r8188eu: always process urb status
.../staging/r8188eu/include/usb_ops_linux.h | 2 -
.../staging/r8188eu/os_dep/usb_ops_linux.c | 46 ++++++++-----------
2 files changed, 18 insertions(+), 30 deletions(-)
--
2.30.2
Remove the if clause in usb_write_port_complete and process the urb
status regardless of bSurpriseRemoved, bDriverStopped and
bWritePortCancel.
The only possible results of urb status processing are updates to
bSurpriseRemoved and bDriverStopped. All of the three status variable are
set to true only if the whole USB processing has to be stopped (when the
driver is unloaded or when the system goes to sleep).
It's no problem if one of the "stop everything" variables is already set
and the urb status processing sets another one.
This patch removes the last goto in usb_write_port_complete. It's also
part of the ongoing effort to limit the use of the "stop everything"
variables.
Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/os_dep/usb_ops_linux.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index 3fd080091340..62106d2f82ad 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -44,9 +44,6 @@ static void usb_write_port_complete(struct urb *purb)
if (pxmitbuf->flags == HIGH_QUEUE_INX)
rtw_chk_hi_queue_cmd(padapter);
- if (padapter->bSurpriseRemoved || padapter->bDriverStopped || padapter->bWritePortCancel)
- goto check_completion;
-
switch (purb->status) {
case 0:
case -EINPROGRESS:
@@ -63,7 +60,6 @@ static void usb_write_port_complete(struct urb *purb)
break;
}
-check_completion:
rtw_sctx_done_err(&pxmitbuf->sctx,
purb->status ? RTW_SCTX_DONE_WRITE_PORT_ERR : RTW_SCTX_DONE_SUCCESS);
rtw_free_xmitbuf(pxmitpriv, pxmitbuf);
--
2.30.2
This trivial patch reformats the usb_write_port_complete function.
Hopefully, this makes the code a bit easier to read.
Signed-off-by: Martin Kaiser <[email protected]>
---
drivers/staging/r8188eu/os_dep/usb_ops_linux.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index 8494b80a08e5..7da13b87d726 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -38,14 +38,13 @@ void rtw_read_port_cancel(struct adapter *padapter)
static void usb_write_port_complete(struct urb *purb, struct pt_regs *regs)
{
struct xmit_buf *pxmitbuf = (struct xmit_buf *)purb->context;
- struct adapter *padapter = pxmitbuf->padapter;
- struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
+ struct adapter *padapter = pxmitbuf->padapter;
+ struct xmit_priv *pxmitpriv = &padapter->xmitpriv;
if (pxmitbuf->flags == HIGH_QUEUE_INX)
rtw_chk_hi_queue_cmd(padapter);
- if (padapter->bSurpriseRemoved || padapter->bDriverStopped ||
- padapter->bWritePortCancel)
+ if (padapter->bSurpriseRemoved || padapter->bDriverStopped || padapter->bWritePortCancel)
goto check_completion;
switch (purb->status) {
@@ -66,13 +65,9 @@ static void usb_write_port_complete(struct urb *purb, struct pt_regs *regs)
check_completion:
rtw_sctx_done_err(&pxmitbuf->sctx,
- purb->status ? RTW_SCTX_DONE_WRITE_PORT_ERR :
- RTW_SCTX_DONE_SUCCESS);
-
+ purb->status ? RTW_SCTX_DONE_WRITE_PORT_ERR : RTW_SCTX_DONE_SUCCESS);
rtw_free_xmitbuf(pxmitpriv, pxmitbuf);
-
tasklet_hi_schedule(&pxmitpriv->xmit_tasklet);
-
}
u32 rtw_write_port(struct adapter *padapter, u32 addr, u32 cnt, u8 *wmem)
--
2.30.2
Refactor the satus handling in usb_write_port_complete. Make it clearer
what happens for each status and avoid all the goto statements.
Signed-off-by: Martin Kaiser <[email protected]>
---
.../staging/r8188eu/os_dep/usb_ops_linux.c | 29 +++++++++----------
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
index 257bcf496012..8494b80a08e5 100644
--- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
@@ -48,21 +48,20 @@ static void usb_write_port_complete(struct urb *purb, struct pt_regs *regs)
padapter->bWritePortCancel)
goto check_completion;
- if (purb->status) {
- if (purb->status == -EINPROGRESS) {
- goto check_completion;
- } else if (purb->status == -ENOENT) {
- goto check_completion;
- } else if (purb->status == -ECONNRESET) {
- goto check_completion;
- } else if (purb->status == -ESHUTDOWN) {
- padapter->bDriverStopped = true;
- goto check_completion;
- } else if ((purb->status != -EPIPE) && (purb->status != -EPROTO)) {
- padapter->bSurpriseRemoved = true;
-
- goto check_completion;
- }
+ switch (purb->status) {
+ case 0:
+ case -EINPROGRESS:
+ case -ENOENT:
+ case -ECONNRESET:
+ case -EPIPE:
+ case -EPROTO:
+ break;
+ case -ESHUTDOWN:
+ padapter->bDriverStopped = true;
+ break;
+ default:
+ padapter->bSurpriseRemoved = true;
+ break;
}
check_completion:
--
2.30.2
On 1/10/23 21:56, Martin Kaiser wrote:
> Clean up and simplify the usb_write_port_complete callback function. Yet
> again, this is based on the previous patches I submitted.
>
> Martin Kaiser (4):
> staging: r8188eu: refactor status handling in usb_write_port_complete
> staging: r8188eu: reformat usb_write_port_complete
> staging: r8188eu: remove unused function parameter
> staging: r8188eu: always process urb status
>
> .../staging/r8188eu/include/usb_ops_linux.h | 2 -
> .../staging/r8188eu/os_dep/usb_ops_linux.c | 46 ++++++++-----------
> 2 files changed, 18 insertions(+), 30 deletions(-)
>
Tested-by: Philipp Hortmann <[email protected]> # Edimax N150
Hi Martin,
Martin Kaiser <[email protected]> says:
> Remove the if clause in usb_write_port_complete and process the urb
> status regardless of bSurpriseRemoved, bDriverStopped and
> bWritePortCancel.
>
> The only possible results of urb status processing are updates to
> bSurpriseRemoved and bDriverStopped. All of the three status variable are
> set to true only if the whole USB processing has to be stopped (when the
> driver is unloaded or when the system goes to sleep).
>
Not sure if it matters but we still have that weird rule that after 5
failed usb read/writes bSurpriseRemoved will be set to true
Maybe also worth removing above logic?
> It's no problem if one of the "stop everything" variables is already set
> and the urb status processing sets another one.
>
> This patch removes the last goto in usb_write_port_complete. It's also
> part of the ongoing effort to limit the use of the "stop everything"
> variables.
>
> Signed-off-by: Martin Kaiser <[email protected]>
> ---
> drivers/staging/r8188eu/os_dep/usb_ops_linux.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
> index 3fd080091340..62106d2f82ad 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_ops_linux.c
> @@ -44,9 +44,6 @@ static void usb_write_port_complete(struct urb *purb)
> if (pxmitbuf->flags == HIGH_QUEUE_INX)
> rtw_chk_hi_queue_cmd(padapter);
>
> - if (padapter->bSurpriseRemoved || padapter->bDriverStopped || padapter->bWritePortCancel)
> - goto check_completion;
> -
> switch (purb->status) {
> case 0:
> case -EINPROGRESS:
> @@ -63,7 +60,6 @@ static void usb_write_port_complete(struct urb *purb)
> break;
> }
>
> -check_completion:
> rtw_sctx_done_err(&pxmitbuf->sctx,
> purb->status ? RTW_SCTX_DONE_WRITE_PORT_ERR : RTW_SCTX_DONE_SUCCESS);
> rtw_free_xmitbuf(pxmitpriv, pxmitbuf);
With regards,
Pavel Skripkin
Hi Pavel,
Thus wrote Pavel Skripkin ([email protected]):
> Not sure if it matters but we still have that weird rule that after 5 failed
> usb read/writes bSurpriseRemoved will be set to true
> Maybe also worth removing above logic?
thanks for making we aware of this retry counter. It does really look
strange. Still, I'm not sure that I understand this well enough to
submit a patch for removal. I'll play around with this as time
permits...
Best regards,
Martin