diff mbox series

[v2] usb: dwc3: gadget: move cmd_endtransfer to extra function

Message ID 20220303131121.3557729-1-m.grzeschik@pengutronix.de (mailing list archive)
State Superseded
Headers show
Series [v2] usb: dwc3: gadget: move cmd_endtransfer to extra function | expand

Commit Message

Michael Grzeschik March 3, 2022, 1:11 p.m. UTC 
This patch adds the extra function __dwc3_stop_active_transfer to
consolidate the same codepath.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/usb/dwc3/gadget.c | 68 +++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 32 deletions(-)

Comments

Thinh Nguyen March 4, 2022, 2:26 a.m. UTC | #1 
Hi,

Michael Grzeschik wrote:
> This patch adds the extra function __dwc3_stop_active_transfer to
> consolidate the same codepath.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> ---

Add a comment here on what's changed since previous version.

>  drivers/usb/dwc3/gadget.c | 68 +++++++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 37dbf6132731b7..baddf6196ceee2 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1673,6 +1673,39 @@ static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
>  	return DWC3_DSTS_SOFFN(reg);
>  }
>  
> +/**
> + * __dwc3_stop_active_transfer - stop the current active transfer
> + * @dep: isoc endpoint
> + * @force: set forcerm bit in the command
> + * @interrupt: command complete interrupt after End Transfer command
> + *
> + * When setting force, the ForceRM bit will be set. In that case
> + * the controller won't update the TRB progress on command
> + * completion. It also won't clear the HWO bit in the TRB.
> + * The command will also not complete immediatly in that case.

"immediately"

> + */

Remove blank line

> +
> +static void __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool interrupt)

Can we make this return "int"?

> +{
> +	struct dwc3_gadget_ep_cmd_params params;
> +	u32 cmd;
> +	int ret;
> +
> +	cmd = DWC3_DEPCMD_ENDTRANSFER;
> +	cmd |= force ? DWC3_DEPCMD_HIPRI_FORCERM : 0;
> +	cmd |= interrupt ? DWC3_DEPCMD_CMDIOC : 0;
> +	cmd |= DWC3_DEPCMD_PARAM(dep->resource_index);
> +	memset(&params, 0, sizeof(params));
> +	ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
> +	WARN_ON_ONCE(ret);
> +	dep->resource_index = 0;
> +
> +	if (!interrupt)
> +		dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
> +	else if (!ret)
> +		dep->flags |= DWC3_EP_END_TRANSFER_PENDING;

So we can return ret here.

> +}
> +
>  /**
>   * dwc3_gadget_start_isoc_quirk - workaround invalid frame number
>   * @dep: isoc endpoint
> @@ -1842,21 +1875,8 @@ static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
>  	 * status, issue END_TRANSFER command and retry on the next XferNotReady
>  	 * event.
>  	 */
> -	if (ret == -EAGAIN) {
> -		struct dwc3_gadget_ep_cmd_params params;
> -		u32 cmd;
> -
> -		cmd = DWC3_DEPCMD_ENDTRANSFER |
> -			DWC3_DEPCMD_CMDIOC |
> -			DWC3_DEPCMD_PARAM(dep->resource_index);
> -
> -		dep->resource_index = 0;
> -		memset(&params, 0, sizeof(params));
> -
> -		ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
> -		if (!ret)
> -			dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
> -	}
> +	if (ret == -EAGAIN)
> +		__dwc3_stop_active_transfer(dep, false, true);

if (ret == -EAGAIN)
	ret = __dwc3_stop_active_transfer(dep, false, true);

>  
>  	return ret;
>  }
> @@ -3597,10 +3617,6 @@ static void dwc3_reset_gadget(struct dwc3 *dwc)
>  static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
>  	bool interrupt)
>  {
> -	struct dwc3_gadget_ep_cmd_params params;
> -	u32 cmd;
> -	int ret;
> -
>  	if (!(dep->flags & DWC3_EP_TRANSFER_STARTED) ||
>  	    (dep->flags & DWC3_EP_END_TRANSFER_PENDING))
>  		return;
> @@ -3632,19 +3648,7 @@ static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
>  	 * This mode is NOT available on the DWC_usb31 IP.
>  	 */
>  
> -	cmd = DWC3_DEPCMD_ENDTRANSFER;
> -	cmd |= force ? DWC3_DEPCMD_HIPRI_FORCERM : 0;
> -	cmd |= interrupt ? DWC3_DEPCMD_CMDIOC : 0;
> -	cmd |= DWC3_DEPCMD_PARAM(dep->resource_index);
> -	memset(&params, 0, sizeof(params));
> -	ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
> -	WARN_ON_ONCE(ret);
> -	dep->resource_index = 0;
> -
> -	if (!interrupt)
> -		dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
> -	else
> -		dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
> +	__dwc3_stop_active_transfer(dep, force, interrupt);
>  }
>  
>  static void dwc3_clear_stall_all_ep(struct dwc3 *dwc)

Thanks,
Thinh
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 37dbf6132731b7..baddf6196ceee2 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1673,6 +1673,39 @@  static int __dwc3_gadget_get_frame(struct dwc3 *dwc)
 	return DWC3_DSTS_SOFFN(reg);
 }
 
+/**
+ * __dwc3_stop_active_transfer - stop the current active transfer
+ * @dep: isoc endpoint
+ * @force: set forcerm bit in the command
+ * @interrupt: command complete interrupt after End Transfer command
+ *
+ * When setting force, the ForceRM bit will be set. In that case
+ * the controller won't update the TRB progress on command
+ * completion. It also won't clear the HWO bit in the TRB.
+ * The command will also not complete immediatly in that case.
+ */
+
+static void __dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool interrupt)
+{
+	struct dwc3_gadget_ep_cmd_params params;
+	u32 cmd;
+	int ret;
+
+	cmd = DWC3_DEPCMD_ENDTRANSFER;
+	cmd |= force ? DWC3_DEPCMD_HIPRI_FORCERM : 0;
+	cmd |= interrupt ? DWC3_DEPCMD_CMDIOC : 0;
+	cmd |= DWC3_DEPCMD_PARAM(dep->resource_index);
+	memset(&params, 0, sizeof(params));
+	ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
+	WARN_ON_ONCE(ret);
+	dep->resource_index = 0;
+
+	if (!interrupt)
+		dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
+	else if (!ret)
+		dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
+}
+
 /**
  * dwc3_gadget_start_isoc_quirk - workaround invalid frame number
  * @dep: isoc endpoint
@@ -1842,21 +1875,8 @@  static int __dwc3_gadget_start_isoc(struct dwc3_ep *dep)
 	 * status, issue END_TRANSFER command and retry on the next XferNotReady
 	 * event.
 	 */
-	if (ret == -EAGAIN) {
-		struct dwc3_gadget_ep_cmd_params params;
-		u32 cmd;
-
-		cmd = DWC3_DEPCMD_ENDTRANSFER |
-			DWC3_DEPCMD_CMDIOC |
-			DWC3_DEPCMD_PARAM(dep->resource_index);
-
-		dep->resource_index = 0;
-		memset(&params, 0, sizeof(params));
-
-		ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
-		if (!ret)
-			dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
-	}
+	if (ret == -EAGAIN)
+		__dwc3_stop_active_transfer(dep, false, true);
 
 	return ret;
 }
@@ -3597,10 +3617,6 @@  static void dwc3_reset_gadget(struct dwc3 *dwc)
 static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
 	bool interrupt)
 {
-	struct dwc3_gadget_ep_cmd_params params;
-	u32 cmd;
-	int ret;
-
 	if (!(dep->flags & DWC3_EP_TRANSFER_STARTED) ||
 	    (dep->flags & DWC3_EP_END_TRANSFER_PENDING))
 		return;
@@ -3632,19 +3648,7 @@  static void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force,
 	 * This mode is NOT available on the DWC_usb31 IP.
 	 */
 
-	cmd = DWC3_DEPCMD_ENDTRANSFER;
-	cmd |= force ? DWC3_DEPCMD_HIPRI_FORCERM : 0;
-	cmd |= interrupt ? DWC3_DEPCMD_CMDIOC : 0;
-	cmd |= DWC3_DEPCMD_PARAM(dep->resource_index);
-	memset(&params, 0, sizeof(params));
-	ret = dwc3_send_gadget_ep_cmd(dep, cmd, &params);
-	WARN_ON_ONCE(ret);
-	dep->resource_index = 0;
-
-	if (!interrupt)
-		dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
-	else
-		dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
+	__dwc3_stop_active_transfer(dep, force, interrupt);
 }
 
 static void dwc3_clear_stall_all_ep(struct dwc3 *dwc)