So here are a few dwc2 fixes that I've been using with HiKey.
I'm not totally sure these are all ideal, but they avoid edge case
issues that we have been running into with switching between
gadget mode and host mode.
I'd guess the first two are potentially -stable material, and
the last might be worth sending to -stable too, as its a relatively
simple fix, but to my understanding the UDC state tracking has
always been broken so its not really a regression. But still.
I'd love to get some feedback on the patches and consideration
to be merged upstream.
thanks
-john
Cc: Wei Xu <[email protected]>
Cc: Guodong Xu <[email protected]>
Cc: Amit Pundir <[email protected]>
Cc: YongQin Liu <[email protected]>
Cc: John Youn <[email protected]>
Cc: Minas Harutyunyan <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
John Stultz (3):
usb: dwc2: Improve gadget state disconnection handling
usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode
usb: dwc2: Fix UDC state tracking
drivers/usb/dwc2/gadget.c | 7 +++++++
drivers/usb/dwc2/hcd.c | 8 ++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
--
2.7.4
We've found that while in host mode, using Android, if one runs
the command:
stop adbd
The existing usb devices being utilized in host mode are disconnected.
This is most visible with usb networking devices.
This seems to be due to adbd closing the file:
/dev/usb-ffs/adb/ep0
Which calls ffs_ep0_release() and the following backtrace:
[<ffffff800875a430>] dwc2_hsotg_ep_disable+0x148/0x150
[<ffffff800875a498>] dwc2_hsotg_udc_stop+0x60/0x110
[<ffffff8008787950>] usb_gadget_remove_driver+0x58/0x78
[<ffffff80087879e4>] usb_gadget_unregister_driver+0x74/0xe8
[<ffffff80087850c0>] unregister_gadget+0x28/0x58
[<ffffff800878511c>] unregister_gadget_item+0x2c/0x40
[<ffffff8008790ea8>] ffs_data_clear+0xe8/0xf8
[<ffffff8008790ed8>] ffs_data_reset+0x20/0x58
[<ffffff8008793218>] ffs_data_closed+0x98/0xe8
[<ffffff80087932d8>] ffs_ep0_release+0x20/0x30
Then when dwc2_hsotg_ep_disable() is called, we call
kill_all_requests() which causes a bunch of the following
messages:
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
dwc2 f72c0000.usb: Mode Mismatch Interrupt: currently in Host mode
init: Service 'adbd' (pid 1915) killed by signal 9
init: Sending signal 9 to service 'adbd' (pid 1915) process group...
init: Successfully killed process cgroup uid 0 pid 1915 in 0ms
init: processing action (init.svc.adbd=stopped) from (/init.usb.configfs.rc:15)
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 8 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 12 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 15 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 3 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_hc_chhltd_intr_dma: Channel 4 - ChHltd set, but reason is unknown
dwc2 f72c0000.usb: hcint 0x00000002, intsts 0x04200029
dwc2 f72c0000.usb: dwc2_update_urb_state_abn(): trimming xfer length
And the usb devices connected are basically hung at this point.
It seems like if we're in host mode, we probably shouldn't run
the dwc2_hostg_ep_disable logic, so this patch returns an error
in that case.
With this patch (along with the two previous patches mailed out
earlier:
https://lkml.org/lkml/2017/8/3/1008
https://lkml.org/lkml/2017/8/3/1010
), we avoid the mismatched interrupts and connected usb devices
continue to function.
I'm not sure if some other solution would be better here, but this seems
to work, so I wanted to send it out for input on what the right approach
should be.
Cc: Wei Xu <[email protected]>
Cc: Guodong Xu <[email protected]>
Cc: Amit Pundir <[email protected]>
Cc: YongQin Liu <[email protected]>
Cc: John Youn <[email protected]>
Cc: Minas Harutyunyan <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Reported-by: YongQin Liu <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
drivers/usb/dwc2/gadget.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0d8e09c..7fd0e38 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4004,6 +4004,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
return -EINVAL;
}
+ if (hsotg->op_state != OTG_STATE_B_PERIPHERAL) {
+ dev_err(hsotg->dev, "%s: called in host mode?\n", __func__);
+ return -EINVAL;
+ }
+
epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
spin_lock_irqsave(&hsotg->lock, flags);
--
2.7.4
It has been noticed that the dwc2 udc state reporting doesn't
seem to work (at least on HiKey boards). Where after the initial
setup, the sysfs /sys/class/udc/f72c0000.usb/state file would
report "configured" no matter the state of the OTG port.
This patch adds a call so that we report to the UDC layer when
the gadget device is disconnected.
Cc: Wei Xu <[email protected]>
Cc: Guodong Xu <[email protected]>
Cc: Amit Pundir <[email protected]>
Cc: YongQin Liu <[email protected]>
Cc: John Youn <[email protected]>
Cc: Minas Harutyunyan <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Reported-by: Amit Pundir <[email protected]>
Signed-off-by: John Stultz <[email protected]>
---
drivers/usb/dwc2/gadget.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7fd0e38..603c216 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -3202,6 +3202,8 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg)
call_gadget(hsotg, disconnect);
hsotg->lx_state = DWC2_L3;
+
+ usb_gadget_set_state(&hsotg->gadget, USB_STATE_NOTATTACHED);
}
/**
--
2.7.4
In the earlier commit dad3f793f20f ("usb: dwc2: Make sure we
disconnect the gadget state"), I was trying to fix up the
fact that we somehow weren't disconnecting the gadget state,
so that when the OTG port was plugged in the second time we
would get warnings about the state tracking being wrong.
The fix there was somewhat simple, as it just made sure to
call dwc2_hsotg_disconnect() before we connected things up
in OTG mode.
But in looking at a different issue I was seeing with UDC
state handling, I realized that it would be much better
to call dwc2_hsotg_disconnect when we get the state change
signal moving to host mode.
Thus, this patch removes the earlier disconnect call I added
and moves it (and the needed locking) to the host mode
transition.
Cc: Wei Xu <[email protected]>
Cc: Guodong Xu <[email protected]>
Cc: Amit Pundir <[email protected]>
Cc: YongQin Liu <[email protected]>
Cc: John Youn <[email protected]>
Cc: Minas Harutyunyan <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Chen Yu <[email protected]>
Cc: Felipe Balbi <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: [email protected]
Signed-off-by: John Stultz <[email protected]>
---
drivers/usb/dwc2/hcd.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index c263114..df54428 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3277,7 +3277,6 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
dwc2_core_init(hsotg, false);
dwc2_enable_global_interrupts(hsotg);
spin_lock_irqsave(&hsotg->lock, flags);
- dwc2_hsotg_disconnect(hsotg);
dwc2_hsotg_core_init_disconnected(hsotg, false);
spin_unlock_irqrestore(&hsotg->lock, flags);
dwc2_hsotg_core_connect(hsotg);
@@ -3296,8 +3295,13 @@ static void dwc2_conn_id_status_change(struct work_struct *work)
if (count > 250)
dev_err(hsotg->dev,
"Connection id status change timed out\n");
- hsotg->op_state = OTG_STATE_A_HOST;
+ spin_lock_irqsave(&hsotg->lock, flags);
+ dwc2_hsotg_disconnect(hsotg);
+ dwc2_hsotg_core_init_disconnected(hsotg, false);
+ spin_unlock_irqrestore(&hsotg->lock, flags);
+
+ hsotg->op_state = OTG_STATE_A_HOST;
/* Initialize the Core for Host mode */
dwc2_core_init(hsotg, false);
dwc2_enable_global_interrupts(hsotg);
--
2.7.4
On 09/20/2017 12:57 PM, John Stultz wrote:
> So here are a few dwc2 fixes that I've been using with HiKey.
> I'm not totally sure these are all ideal, but they avoid edge case
> issues that we have been running into with switching between
> gadget mode and host mode.
>
> I'd guess the first two are potentially -stable material, and
> the last might be worth sending to -stable too, as its a relatively
> simple fix, but to my understanding the UDC state tracking has
> always been broken so its not really a regression. But still.
>
> I'd love to get some feedback on the patches and consideration
> to be merged upstream.
>
> thanks
> -john
>
> Cc: Wei Xu <[email protected]>
> Cc: Guodong Xu <[email protected]>
> Cc: Amit Pundir <[email protected]>
> Cc: YongQin Liu <[email protected]>
> Cc: John Youn <[email protected]>
> Cc: Minas Harutyunyan <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Cc: Chen Yu <[email protected]>
> Cc: Felipe Balbi <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]
>
> John Stultz (3):
> usb: dwc2: Improve gadget state disconnection handling
> usb: dwc2: Error out of dwc2_hsotg_ep_disable() if we're in host mode
> usb: dwc2: Fix UDC state tracking
>
> drivers/usb/dwc2/gadget.c | 7 +++++++
> drivers/usb/dwc2/hcd.c | 8 ++++++--
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
Hi John,
I think we have something that fixes these issues.
Minas,
Could you take a look at this? I was not able to find the patches we
talked about. If possible, please post them so that John can try them
out.
Thanks,
John