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

SAMD21: one endpoint pair for MSC now instead of two #4239

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

dhalbert
Copy link
Collaborator

A long time ago when I was working on HID, I found that MSC on the SAMD21 needed to use separate endpoint pairs for IN and OUT. I discovered this only empirically. There is no erratum that would dictate needing two endpoints. At the time we were using the ASF4 USB stack. I was suspicious it was due to ASF4, but could not be sure.

Now that we are using TinyUSB, I tried this again, and, after some mild testing, I didn't see any problem with using just one endpoint pair with MSC on SAMD21. Neither do the TinyUSB MSC examples.

This PR undoes the special treatment for SAMD21. Theoretically, now, secondary CDC is available on SAMD21. Practically speaking, there is no room on many builds. When turned on, it does not fit on non-Express boards, and only barely fits on smaller translations on SAMD21 Express boards. So I have left it turned off. However, custom builds with secondary CDC should now be possible on SAMD21.

@hathach Just tagging you for interest.

@dhalbert dhalbert requested review from jepler and tannewt February 21, 2021 21:25
@jepler
Copy link
Member

jepler commented Feb 22, 2021

I don't know enough to really comment, and I don't think I have any interesting hosts to test on (just linux). but if it was always superstition and you feel you've done adequate testing, go for it.

@hathach
Copy link
Member

hathach commented Feb 22, 2021

I have never seen any issues with SAMD21 one pair endpoints with MSC, have been running it with simple examples in my tinyusb repo as well as Arduino TinyUSB lib without any issues. It is probably other issue such as Controller 2-bytes overflown and/or else.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me! Thanks for going back and testing this. I agree it's best to leave it off. We'll likely need that space for code re-org for existing stuff.

@tannewt tannewt merged commit 2262af0 into adafruit:main Feb 22, 2021
@dhalbert dhalbert deleted the samd21-msc-one-endpoint-pair branch February 24, 2021 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants