Hi Minas,
On 12/14/21 7:22 AM, Minas Harutyunyan wrote:
Hi Amelie,
On 12/7/2021 5:01 PM, Amelie Delaunay wrote:
Calling dwc2_hsotg_ep_disable on ep0 (in/out) will lead to the following
logs before returning -EINVAL:
dwc2 49000000.usb-otg: dwc2_hsotg_ep_disable: called for ep0
dwc2 49000000.usb-otg: dwc2_hsotg_ep_disable: called for ep0
This messages printed for EP0 OUT which can't be disabled, but EP0 IN
can and should be disabled. Your patch exclude EP0 IN from disabling flow.
Thanks for the review. I wonder why then in dwc2_hsotg_udc_stop and
dwc2_hsotg_core_init_disconnected EP0 IN is not disabled (loop starts
from EP1) ?
Due to:
/* Same dwc2_hsotg_ep is used in both directions for ep0 */
hsotg->eps_out[0] = hsotg->eps_in[0];
the condition in dwc2_hsotg_ep_disable to display the error is always
true for EP0, whatever the direction, so the log appears for EP0 IN & OUT:
if (ep == &hsotg->eps_out[0]->ep) {
dev_err(hsotg->dev, "%s: called for ep0\n", __func__);
return -EINVAL;
}
Should all loops need to be fixed to start loop from EP0 but exclude EP0
OUT from being disabled, so that EP0 IN can be disabled ? e.g. for
dwc2_hsotg_suspend:
$ git diff drivers/usb/dwc2/gadget.c
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 11d85a6e0b0d..0c12219bfbf4 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4231,7 +4231,7 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
dev_dbg(hsotg->dev, "%s(ep %p)\n", __func__, ep);
- if (ep == &hsotg->eps_out[0]->ep) {
+ if (ep == &hsotg->eps_out[0]->ep && !dir_in) {
dev_err(hsotg->dev, "%s: called for ep0\n", __func__);
return -EINVAL;
}
@@ -5077,7 +5077,7 @@ int dwc2_hsotg_suspend(struct dwc2_hsotg *hsotg)
for (ep = 0; ep < hsotg->num_of_eps; ep++) {
if (hsotg->eps_in[ep])
dwc2_hsotg_ep_disable_lock(&hsotg->eps_in[ep]->ep);
- if (hsotg->eps_out[ep])
+ if (ep > 0 && hsotg->eps_out[ep])
dwc2_hsotg_ep_disable_lock(&hsotg->eps_out[ep]->ep);
}
}
Regards,
Amelie
Thanks,
Minas
To avoid these two logs while suspending, start disabling the endpoint
from the index 1, as done in dwc2_hsotg_udc_stop:
/* all endpoints should be shutdown */
for (ep = 1; ep < hsotg->num_of_eps; ep++) {
if (hsotg->eps_in[ep])
dwc2_hsotg_ep_disable_lock(&hsotg->eps_in[ep]->ep);
if (hsotg->eps_out[ep])
dwc2_hsotg_ep_disable_lock(&hsotg->eps_out[ep]->ep);
}
Signed-off-by: Amelie Delaunay <amelie.delaunay@xxxxxxxxxxx>
---
drivers/usb/dwc2/gadget.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index b884a83b26a6..ee31f9a328da 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -5086,7 +5086,7 @@ int dwc2_hsotg_suspend(struct dwc2_hsotg *hsotg)
hsotg->gadget.speed = USB_SPEED_UNKNOWN;
spin_unlock_irqrestore(&hsotg->lock, flags);
- for (ep = 0; ep < hsotg->num_of_eps; ep++) {
+ for (ep = 1; ep < hsotg->num_of_eps; ep++) {
if (hsotg->eps_in[ep])
dwc2_hsotg_ep_disable_lock(&hsotg->eps_in[ep]->ep);
if (hsotg->eps_out[ep])
