hathach
host/dwc2: cleanup transfer on device close
Describe the PR Fix queued xfer of previous failed enumeration mess with next enumeration.
[1:1] Get Descriptor: 80 06 03 03 09 04 00 01
on EP 00 with 8 bytes: OK
on EP 80 with 40 bytes: OK
[1:1] Control data:
0000: 28 03 4D 00 61 00 73 00 73 00 20 00 53 00 74 00 |(.M.a.s.s. .S.t.|
0010: 6F 00 72 00 61 00 67 00 65 00 20 00 44 00 65 00 |o.r.a.g.e. .D.e.|
0020: 76 00 69 00 63 00 65 00 |v.i.c.e.|
[1:0:0] USBH DEVICE REMOVED
[1:0:0] unplugged address = 1
Device removed, address = 1
[1:] USBH Device Attach
High Speed
[1:0] Open EP0 with Size = 8
Get 8 byte of Device Descriptor
[1:0] Get Descriptor: 80 06 00 01 00 00 08 00
on EP 00 with 8 bytes: OK <------- Missed by one
on EP 00 with 8 bytes: OK
[1:0] Control data:
0000: 12 01 00 02 00 00 00 40 |.......@|
on EP 80 with 8 bytes: OK
Set Address = 1
[1:0] Set Address: 00 05 01 00 00 00 00 00
on EP 00 with 0 bytes: OK
on EP 00 with 8 bytes: OK
[1:1] Open EP0 with Size = 64
Get Device Descriptor
[1:1] Get Descriptor: 80 06 00 01 00 00 12 00
on EP 80 with 8 bytes: OK
hcd_edpt_xfer 655: ASSERT FAILED
While I can see the issue described here on my side as well, it seems to me this is not the proper fix, but rather a workaround - with its own side-effects. Why wasn't this xfer cleared? And what if is currently ongoing?
There seem to be some race conditions between handling some stuff in the interrupt handler, and deferring some stuff to the tuh_task(). This is probably the root of (some of) these problems...
it seems to me this is not the proper fix, but rather a workaround - with its own side-effects.
Could you detail more, what you mean sie-effects ?
my bad, I think we cannot simply clear data here. We should mark endpoint for removal then disable channel first, within the halted interrupt check the flag and clear the endpoint.
Ah yes we should close the channel properly, especially when a hub is used.
I've changed it use channel_disable(), I think there is no need to wait until the channel is disabled ?
I've changed it use
channel_disable(), I think there is no need to wait until the channel is disabled ?
I am on vacation, will try to check this out afterwards (next week or so).
@HiFiPhile I make more changes to PR, also add closing flag for endpoint to de-alloc() it when chanel is halted with closing flags. Also put the check into chanel_disable(). Let me know if these make sense to you.
@hathach It looks good but I don't remember some details. Tried plugging in/out a bad contact USB stick on H7RS and no assert is hit.
@HiFiPhile why skipping these examples for h7rs, I see it compile just fine with minsize.
@hathach With LOG=2 IAR debug builds needs > 70k for these examples, cdc_msc_hid_freertos needs 66k even on MinRel build.
@hathach With LOG=2 IAR debug builds needs > 70k for these examples, cdc_msc_hid_freertos needs 66k even on MinRel build.
as long as minsize with LOG=0 compile, we can leave it enabled since that is what ci build, also user can test with those example as well. LOG > 1 include lots of constant file/func/line for assert and increase the footprint qutie a bit.
I've just dropped that commit, maybe later we can enhance skip/only mechanism for debug variant.