Re: [PATCH 1/2] usb: dwc3: gadget: Remove descriptor arguments to ep_enable
On 11/10/2016 3:15 AM, Felipe Balbi wrote:
>
> Hi,
>
> John Youn 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 ?
I did mean shift since bit 0 is the direction. But that also works.
Regards,
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: dwc3: gadget: Remove descriptor arguments to ep_enable
Hi,
John Youn 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
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
[felipe.ba...@linux.intel.com : minor improvements]
Signed-off-by: Felipe Balbi
---
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(¶ms, 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_enabl
[PATCH 1/2] 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
---
drivers/usb/dwc3/core.h | 1 -
drivers/usb/dwc3/gadget.c | 40 +++-
2 files changed, 23 insertions(+), 18 deletions(-)
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;
@@ -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.desc = NULL;
+ dep->endpoint.comp_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 (
