Re: [PATCH 1/2] usb: dwc3: gadget: Remove descriptor arguments to ep_enable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

John Youn <johnyoun@xxxxxxxxxxxx> writes:
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 2322863..b9903c6 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -539,7 +539,6 @@ struct dwc3_ep {
>  
>  	struct dwc3_trb		*trb_pool;
>  	dma_addr_t		trb_pool_dma;
> -	const struct usb_ss_ep_comp_descriptor *comp_desc;
>  	struct dwc3		*dwc;
>  
>  	u32			saved_state;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 7e465ea..0e73383 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -576,13 +576,13 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 *dwc, struct dwc3_ep *dep)
>   * Caller should take care of locking
>   */
>  static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> -		const struct usb_endpoint_descriptor *desc,
> -		const struct usb_ss_ep_comp_descriptor *comp_desc,
>  		bool modify, bool restore)
>  {
>  	struct dwc3		*dwc = dep->dwc;
>  	u32			reg;
>  	int			ret;
> +	struct usb_ep		*ep;
> +	const struct usb_endpoint_descriptor *desc;
>  
>  	if (!(dep->flags & DWC3_EP_ENABLED)) {
>  		ret = dwc3_gadget_start_config(dwc, dep);
> @@ -590,8 +590,10 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>  			return ret;
>  	}
>  
> -	ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, modify,
> -			restore);
> +	ep = &dep->endpoint;
> +	desc = ep->desc;
> +	ret = dwc3_gadget_set_ep_config(dwc, dep, desc, ep->comp_desc,
> +			modify, restore);
>  	if (ret)
>  		return ret;

this can be improved (see new version below).

> @@ -713,11 +713,15 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
>  	dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
>  
>  	dep->stream_capable = false;
> -	dep->endpoint.desc = NULL;
> -	dep->comp_desc = NULL;
>  	dep->type = 0;
>  	dep->flags &= DWC3_EP_END_TRANSFER_PENDING;
>  
> +	/* Clear out the ep descriptors for non-ep0 */
> +	if (dep->number >> 1) {

Do you mean dep->number > 1 ?

> @@ -1891,6 +1893,12 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 *dwc,
>  				(epnum & 1) ? "in" : "out");
>  
>  		dep->endpoint.name = dep->name;
> +
> +		if (!(dep->number >> 1)) {

ditto

new version follows:

8<------------------------------------------------------------------------

From 3df1dffa98f1c40f3a67bb621317e43f3c29820f Mon Sep 17 00:00:00 2001
From: John Youn <johnyoun@xxxxxxxxxxxx>
Date: Wed, 9 Nov 2016 16:36:28 -0800
Subject: [PATCH v2] usb: dwc3: gadget: Remove descriptor arguments to ep_enable

The __dwc3_gadget_endpoint_enable() function has access to the endpoint
descriptors via the usb_ep. So we don't need to pass them in as
arguments. The descriptors should be set by the caller prior to calling
usb_ep_enable().

Signed-off-by: John Youn <johnyoun@xxxxxxxxxxxx>

[felipe.balbi@xxxxxxxxxxxxxxx : minor improvements]

Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
---
 drivers/usb/dwc3/core.h   |  1 -
 drivers/usb/dwc3/gadget.c | 44 +++++++++++++++++++++++++-------------------
 2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 2322863e3cf7..b9903c6c3de4 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -539,7 +539,6 @@ struct dwc3_ep {
 
 	struct dwc3_trb		*trb_pool;
 	dma_addr_t		trb_pool_dma;
-	const struct usb_ss_ep_comp_descriptor *comp_desc;
 	struct dwc3		*dwc;
 
 	u32			saved_state;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 7e465ea6a211..22ccc346af2f 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -488,16 +488,19 @@ static int dwc3_gadget_start_config(struct dwc3 *dwc, struct dwc3_ep *dep)
 }
 
 static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep,
-		const struct usb_endpoint_descriptor *desc,
-		const struct usb_ss_ep_comp_descriptor *comp_desc,
 		bool modify, bool restore)
 {
+	const struct usb_ss_ep_comp_descriptor *comp_desc;
+	const struct usb_endpoint_descriptor *desc;
 	struct dwc3_gadget_ep_cmd_params params;
 
 	if (dev_WARN_ONCE(dwc->dev, modify && restore,
 					"Can't modify and restore\n"))
 		return -EINVAL;
 
+	comp_desc = dep->endpoint.comp_desc;
+	desc = dep->endpoint.desc;
+
 	memset(&params, 0x00, sizeof(params));
 
 	params.param0 = DWC3_DEPCFG_EP_TYPE(usb_endpoint_type(desc))
@@ -576,11 +579,11 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3 *dwc, struct dwc3_ep *dep)
  * Caller should take care of locking
  */
 static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
-		const struct usb_endpoint_descriptor *desc,
-		const struct usb_ss_ep_comp_descriptor *comp_desc,
 		bool modify, bool restore)
 {
+	const struct usb_endpoint_descriptor *desc = dep->endpoint.desc;
 	struct dwc3		*dwc = dep->dwc;
+
 	u32			reg;
 	int			ret;
 
@@ -590,8 +593,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
 			return ret;
 	}
 
-	ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, modify,
-			restore);
+	ret = dwc3_gadget_set_ep_config(dwc, dep, modify, restore);
 	if (ret)
 		return ret;
 
@@ -599,8 +601,6 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
 		struct dwc3_trb	*trb_st_hw;
 		struct dwc3_trb	*trb_link;
 
-		dep->endpoint.desc = desc;
-		dep->comp_desc = comp_desc;
 		dep->type = usb_endpoint_type(desc);
 		dep->flags |= DWC3_EP_ENABLED;
 		dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
@@ -713,11 +713,15 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
 	dwc3_writel(dwc->regs, DWC3_DALEPENA, reg);
 
 	dep->stream_capable = false;
-	dep->endpoint.desc = NULL;
-	dep->comp_desc = NULL;
 	dep->type = 0;
 	dep->flags &= DWC3_EP_END_TRANSFER_PENDING;
 
+	/* Clear out the ep descriptors for non-ep0 */
+	if (dep->number > 1) {
+		dep->endpoint.comp_desc = NULL;
+		dep->endpoint.desc = NULL;
+	}
+
 	return 0;
 }
 
@@ -763,7 +767,7 @@ static int dwc3_gadget_ep_enable(struct usb_ep *ep,
 		return 0;
 
 	spin_lock_irqsave(&dwc->lock, flags);
-	ret = __dwc3_gadget_ep_enable(dep, desc, ep->comp_desc, false, false);
+	ret = __dwc3_gadget_ep_enable(dep, false, false);
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	return ret;
@@ -1741,16 +1745,14 @@ static int __dwc3_gadget_start(struct dwc3 *dwc)
 	dwc3_gadget_ep0_desc.wMaxPacketSize = cpu_to_le16(512);
 
 	dep = dwc->eps[0];
-	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false,
-			false);
+	ret = __dwc3_gadget_ep_enable(dep, false, false);
 	if (ret) {
 		dev_err(dwc->dev, "failed to enable %s\n", dep->name);
 		goto err0;
 	}
 
 	dep = dwc->eps[1];
-	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, false,
-			false);
+	ret = __dwc3_gadget_ep_enable(dep, false, false);
 	if (ret) {
 		dev_err(dwc->dev, "failed to enable %s\n", dep->name);
 		goto err1;
@@ -1891,6 +1893,12 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 *dwc,
 				(epnum & 1) ? "in" : "out");
 
 		dep->endpoint.name = dep->name;
+
+		if (!(dep->number > 1)) {
+			dep->endpoint.desc = &dwc3_gadget_ep0_desc;
+			dep->endpoint.comp_desc = NULL;
+		}
+
 		spin_lock_init(&dep->lock);
 
 		if (epnum == 0 || epnum == 1) {
@@ -2579,16 +2587,14 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 	}
 
 	dep = dwc->eps[0];
-	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, true,
-			false);
+	ret = __dwc3_gadget_ep_enable(dep, true, false);
 	if (ret) {
 		dev_err(dwc->dev, "failed to enable %s\n", dep->name);
 		return;
 	}
 
 	dep = dwc->eps[1];
-	ret = __dwc3_gadget_ep_enable(dep, &dwc3_gadget_ep0_desc, NULL, true,
-			false);
+	ret = __dwc3_gadget_ep_enable(dep, true, false);
 	if (ret) {
 		dev_err(dwc->dev, "failed to enable %s\n", dep->name);
 		return;
-- 
2.10.1



-- 
balbi

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux