-
Notifications
You must be signed in to change notification settings - Fork 19
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
General USB fixes ported from the new mmu bootloader #323
base: main
Are you sure you want to change the base?
Conversation
All values in bytes. Δ Delta to base
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well on my end. I flashed multiple times and also downgraded/upgraded multiple times. No issues.
/** Endpoint address of the CDC device-to-host data IN endpoint. */ | ||
#define CDC_TX_EPADDR (ENDPOINT_DIR_IN | 3) | ||
#define CDC_TX_EPADDR (ENDPOINT_DIR_IN | 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 just a thought - can't we run into mysterious issues with flashing the MMU board on some platforms? Hopefully not, USB arbitration should be hidden from the avr-dude, but who knows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the endpoint structure is hidden from avrdude. It's the OS CDC class driver that handles this. As for whether it can be a problem, I very much doubt it. There is no hardcoded order in which endpoints have to be allocated, so any driver should handle this. The only maybe risky driver is the custom one on Windows that applies the MMU product name instead of a generic name to the virtual com port, but that seems to be working just fine on my end, so I doubt there will be any issue caused by this.
@@ -11,7 +11,7 @@ | |||
#define NO_INTERNAL_SERIAL | |||
#define NO_DEVICE_SELF_POWER | |||
#define NO_DEVICE_REMOTE_WAKEUP | |||
// #define NO_SOF_EVENTS | |||
#define NO_SOF_EVENTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 this sounds dangerous, we had tons of trouble with SOF events on Buddy platform...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SOF is not used by the CDC class. In device mode, only stuff that does isochronous streams of data cares about SOF, such as UAC and UVC. So disabling the handling of the flag in the interrupt has no adverse effect. We don't care about the SOF callback.
On the other hand, SOF is generated by the host for any class and is part of the frame structure of USB. Maybe this is what you are remembering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, saving 114 bytes just by stripping some (potentially) unneeded features from the USB lib sounds nice, but I have no idea what may break, esp. considering various platforms we are connecting to. Remember, people are reporting "cannot flash the FW" occasionally a we haven't been able to reproduce these issues at all.
I'm not against merging it, it just needs tons of testing.
SOF events are not used by the CDC class. The reordered endpoints should be fine since the descriptor was updated. In any case, this change is just so that the mmu firmware config matches the one in the bootloader. |
This seems to be caused by the transition from the MMU fw to the bootloader. So when the usb device disconnects and then reconnects. This PR doesn't touch that in any way. I have yet to identify what exactly causes that problem since I can't reproduce the problem consistently. Any attempt to cause it in rapid succession makes the problem go away completely.
What do you propose for testing this? I've tested updating the firmware on windows and that worked correctly. I can also run tests on macOS on my thinkpad and on linux. Other than that I don't know what else to test. |
Saves quite a bit of flash and the usb still works fully.