diff mbox series

[4/4] staging: r8188eu: always process urb status

Message ID 20230110205626.183516-5-martin@kaiser.cx
State New
Headers
Series staging: r8188eu: clean up usb_write_port_complete |

Commit Message

Martin Kaiser Jan. 10, 2023, 8:56 p.m. UTC 
  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 <martin@kaiser.cx>
---
 drivers/staging/r8188eu/os_dep/usb_ops_linux.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Pavel Skripkin Jan. 11, 2023, 5:24 p.m. UTC | #1 
Hi Martin,

Martin Kaiser <martin@kaiser.cx> 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 <martin@kaiser.cx>
> ---
>   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
Martin Kaiser Jan. 23, 2023, 8:04 p.m. UTC | #2 
Hi Pavel,

Thus wrote Pavel Skripkin (paskripkin@gmail.com):

> 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
diff mbox series

Patch

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