Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

atsamd usb serial not receiving characters until close #105

Closed
jacobrosenthal opened this issue Oct 20, 2019 · 12 comments · Fixed by #154
Closed

atsamd usb serial not receiving characters until close #105

jacobrosenthal opened this issue Oct 20, 2019 · 12 comments · Fixed by #154

Comments

@jacobrosenthal
Copy link
Member

I have a usb commit in #104 here for pygamer When you open the port with screen /dev/blah 115200 and type nothing is printed on screen and Ive checked and the characters arent even received. I thought it was maybe interrupts, but I implemented the polling to upper case echo example and it has the same behavior.

So maybe my implementation is broken, but wait I noticed as you close screen, the led example will trigger and In the case of the polling to upper case example when you reopen screen you'll see your previous chars printed to screen.

So the act of closing is triggering for some reason. I can't confirm on itsy but I presume its the same. So maybe its my setup for some reason. I checked with stm32-usb and a upper case polling bluepill example and can confirm those characters are echoed immediately using screen so its not my setup.

I have a feeling using the stty as given in directions in Itsy, opens and closes and is thus triggering a flush somehow and so this hasnt been noticed until now.

Im not yet familiar with the usb device implementation but I think it would lie in there somewhere. Any thoughts @sajattack ?

@twitchyliquid64
Copy link
Contributor

Works fine on the itsybitsy_m4, I only implemented 2 weeks or so ago.

but wait I noticed as you close screen

... Laptop screen?

I have a feeling using the stty as given in directions in Itsy, opens and closes and is thus triggering a flush somehow and so this hasnt been noticed until now.

One thing that might be worth doing is setting up usbmon and having a look at the USB packets going to and from the device in the working / not-working scenarios.

@jacobrosenthal
Copy link
Member Author

jacobrosenthal commented Oct 21, 2019 via email

@twitchyliquid64
Copy link
Contributor

Will do when I get back to my hotel room.

If stty works but not screen, I expect it's due to the CDC ACM driver rather than the peripheral driver. Either way I'll give it a go.

@twitchyliquid64
Copy link
Contributor

I couldnt get it working at all with screen. I used minicom instead as that should work.

I used wireshark and the usbmon module to monitor the USB packets coming out of my device. This is my observations:

  • Setting stty thingy then doing the echo thing causes the usb controller to set the line state before and after sending data (this is the scenario that communications work)
  • minicom does not send any control packets except at the beginning
  • Sending data packets only (ie: minicom) hangs interaction till a control packet is sent
  • Hanging up or closing minicom causes the interactions to miraculously start working again (presumably because it results in a control packet being sent).

As such, I think the likely culprit is a code path expecting a control packet and not handling the received non-control packet until that happens.

@twitchyliquid64
Copy link
Contributor

twitchyliquid64 commented Oct 22, 2019

Either way, this is a valid bug somewhere in the stack (possibly in the usb-device crate, possibly in the peripheral driver, possibly in the usbd-serial crate).

I'll be able to better investigate when I get home (travelling atm), where I can get the UART out and start pumping debug messages.

cc @mvirkkunen - does usbd-serial work with minicom et al on the devices you tested?

@jacobrosenthal
Copy link
Member Author

jacobrosenthal commented Oct 22, 2019 via email

@jacobrosenthal
Copy link
Member Author

From @mvirkkunen on matrix

I haven't exactly tested on anything except one device but "picocom" works on Linux and I forgot which terminal I used on Windows but it worked

@jacobrosenthal
Copy link
Member Author

Ive got my debugger up and working to look at this. Nothing obvious yet but Ive never dealt with usb.

it seems like trcpt1 isnt being called to trigger the poll when the char is received
instead when disconnect happens, usb_other triggers the poll, which triggers something internally to finally trigger trcpt1 to be hit, forcing another poll, which finally receives the character

@twitchyliquid64
Copy link
Contributor

twitchyliquid64 commented Jan 16, 2020

There may be a quirk / undocumented behaviour where certain endpoints or states need to be serviced first before other interrupts fire.

A few ideas:

  • Maybe we still need to check each endpoint if we get a suspend or reset interrupt (ie: device doesnt send trcpt1 if there is already a different interrupt pending).
  • Maybe we need to check endpoints before checking suspend / reset interrupts.

Apologies for not helping out here, I have lots of free time but am currently pseudo-homeless, so unable to access my gear.

@jacobrosenthal
Copy link
Member Author

I dont think its poll ordering. I wired up the start of frame interrupt, which seems to get fired on probably on every packet including heartbeats or something, to the poll function, and with that hitting poll constantly the reads come through fine.

So the register is showing the buffer has data and clearing it fine. So it does seem that for some reason the trcpt0 (I said trcpt1 last time, I think I now understand that input is on channel 0) interrupt isnt firing to trigger the poll

I looked through svds and those seem correct around interrupts registers...

@twitchyliquid64
Copy link
Contributor

I spent hours and couldn't figure out why the interrupt wasn't firing. Given I havent found any open-source implementation not using the SOF interrupt, ive just submitted a fix PR that uses SOF. Can confirm it fixes our issues.

@jacobrosenthal
Copy link
Member Author

Ive come to the same conclusion on suspend/resume

The fact is this chip does reset/suspend/resume and more for you. We could presumably be doing more low power stuff, but were not anyway.

Ive spent time rewriting but I still havent gotten the mix of flags. I THINK you need enable the wakeup interrupt when you suspend? But Ive found that damn bit set constantly and havent been able to then distinguish from suspend and not awake and suspend and awake. Would we need a local static variable?

The question comes down to does usb-device heavily profit from suspend/resume notifications? I dont know...

And yeah for interrupt. Ive spend days now. I just dont see anything in other implementations either

Maybe Ill try to disable sof in the arduino and see if it still works?

twitchyliquid64 pushed a commit to twitchyliquid64/atsamd that referenced this issue Jan 20, 2020
twitchyliquid64 pushed a commit to twitchyliquid64/atsamd that referenced this issue Jan 20, 2020
sajattack pushed a commit that referenced this issue Jan 21, 2020
…nd USB reset (#154)

* samd51 UsbBus: Fix interrupts not enabling & missing events around bus reset

Fixes #105.

* Only clear endpoint _bank ready_ bit during configuration/protocol-reset

* Pair programmin jammin (#1)

Co-authored-by: Jacob Rosenthal <[email protected]>

* Cleanup ep init logic & improve comments.

Co-authored-by: Jacob Rosenthal <[email protected]>

* Bump HAL version

* cleanup rxstp interrupts
Co-authored-by: Jacob Rosenthal <[email protected]>

* Remove unnecessary handling of trfail status in poll

Co-authored-by: Jacob Rosenthal <[email protected]>

Co-authored-by: Jacob Rosenthal <[email protected]>
sajattack pushed a commit that referenced this issue Jan 23, 2020
…155)

* samd51 UsbBus: Fix interrupts not enabling & missing events around bus reset

Fixes #105.

* Only clear endpoint _bank ready_ bit during configuration/protocol-reset

* Pair programmin jammin (#1)

Co-authored-by: Jacob Rosenthal <[email protected]>

* Cleanup ep init logic & improve comments.

Co-authored-by: Jacob Rosenthal <[email protected]>

* Bump HAL version

* cleanup rxstp interrupts
Co-authored-by: Jacob Rosenthal <[email protected]>

* Remove unnecessary handling of trfail status in poll

Co-authored-by: Jacob Rosenthal <[email protected]>

* WIP getting USB working on samd21

* samd21 UsbBus: Fix implementation & get itsybitsy_m0 example working

* Bump crate versions

* Remove dangling reference to uart_debug

Co-authored-by: Jacob Rosenthal <[email protected]>
kaizensparc pushed a commit to kaizensparc/atsamd that referenced this issue Dec 24, 2021
…nd USB reset (atsamd-rs#154)

* samd51 UsbBus: Fix interrupts not enabling & missing events around bus reset

Fixes atsamd-rs#105.

* Only clear endpoint _bank ready_ bit during configuration/protocol-reset

* Pair programmin jammin (atsamd-rs#1)

Co-authored-by: Jacob Rosenthal <[email protected]>

* Cleanup ep init logic & improve comments.

Co-authored-by: Jacob Rosenthal <[email protected]>

* Bump HAL version

* cleanup rxstp interrupts
Co-authored-by: Jacob Rosenthal <[email protected]>

* Remove unnecessary handling of trfail status in poll

Co-authored-by: Jacob Rosenthal <[email protected]>

Co-authored-by: Jacob Rosenthal <[email protected]>
kaizensparc pushed a commit to kaizensparc/atsamd that referenced this issue Dec 24, 2021
…tsamd-rs#155)

* samd51 UsbBus: Fix interrupts not enabling & missing events around bus reset

Fixes atsamd-rs#105.

* Only clear endpoint _bank ready_ bit during configuration/protocol-reset

* Pair programmin jammin (atsamd-rs#1)

Co-authored-by: Jacob Rosenthal <[email protected]>

* Cleanup ep init logic & improve comments.

Co-authored-by: Jacob Rosenthal <[email protected]>

* Bump HAL version

* cleanup rxstp interrupts
Co-authored-by: Jacob Rosenthal <[email protected]>

* Remove unnecessary handling of trfail status in poll

Co-authored-by: Jacob Rosenthal <[email protected]>

* WIP getting USB working on samd21

* samd21 UsbBus: Fix implementation & get itsybitsy_m0 example working

* Bump crate versions

* Remove dangling reference to uart_debug

Co-authored-by: Jacob Rosenthal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants