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

General USB fixes ported from the new mmu bootloader #323

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions lib/Descriptors.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@
#include "lufa/LUFA/Drivers/USB/USB.h"

/* Macros: */
/** Endpoint address of the CDC device-to-host notification IN endpoint. */
#define CDC_NOTIFICATION_EPADDR (ENDPOINT_DIR_IN | 2)

/** 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)
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.


/** Endpoint address of the CDC host-to-device data OUT endpoint. */
#define CDC_RX_EPADDR (ENDPOINT_DIR_OUT | 4)
#define CDC_RX_EPADDR (ENDPOINT_DIR_OUT | 2)

/** Endpoint address of the CDC device-to-host notification IN endpoint. */
#define CDC_NOTIFICATION_EPADDR (ENDPOINT_DIR_IN | 3)

/** Size in bytes of the CDC device-to-host notification IN endpoint. */
#define CDC_NOTIFICATION_EPSIZE 8
Expand Down
6 changes: 3 additions & 3 deletions lib/lufa_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,16 @@

#define USB_DEVICE_ONLY
#define DEVICE_STATE_AS_GPIOR 0
// #define ORDERED_EP_CONFIG
#define ORDERED_EP_CONFIG
#define FIXED_CONTROL_ENDPOINT_SIZE 8
#define FIXED_NUM_CONFIGURATIONS 1
#define INTERRUPT_CONTROL_ENDPOINT
#define USE_FLASH_DESCRIPTORS
#define USE_STATIC_OPTIONS (USB_DEVICE_OPT_FULLSPEED | USB_OPT_REG_ENABLED | USB_OPT_AUTO_PLL)
#define USE_STATIC_OPTIONS (USB_DEVICE_OPT_FULLSPEED | USB_OPT_REG_ENABLED | USB_OPT_MANUAL_PLL)
#define NO_INTERNAL_SERIAL
#define NO_DEVICE_SELF_POWER
#define NO_DEVICE_REMOTE_WAKEUP
// #define NO_SOF_EVENTS
#define NO_SOF_EVENTS
Copy link
Collaborator

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...

Copy link
Collaborator Author

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?

#define F_USB F_CPU
#define DEVICE_VID 0x2C99
#define DEVICE_PID 0x0004
12 changes: 10 additions & 2 deletions src/modules/usb_cdc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ USB_ClassInfo_CDC_Device_t VirtualSerial_CDC_Interface = {
.Address = CDC_TX_EPADDR,
.Size = CDC_TXRX_EPSIZE,
.Type = EP_TYPE_BULK,
.Banks = 2,
.Banks = 1,
},
.DataOUTEndpoint = {
.Address = CDC_RX_EPADDR,
.Size = CDC_TXRX_EPSIZE,
.Type = EP_TYPE_BULK,
.Banks = 2,
.Banks = 1,
},
.NotificationEndpoint = {
.Address = CDC_NOTIFICATION_EPADDR,
Expand Down Expand Up @@ -90,6 +90,14 @@ namespace usb {
CDC cdc;

void CDC::Init() {
#if defined(USE_STATIC_OPTIONS) && (USE_STATIC_OPTIONS & USB_OPT_MANUAL_PLL)
#if defined(USB_SERIES_4_AVR)
PLLFRQ = ((1 << PLLUSB) | (1 << PDIV3) | (1 << PDIV1));
#endif
USB_PLL_On();
while (!(USB_PLL_IsReady()));
#endif

USB_Init();

/* Create a regular character stream for the interface so that it can be used with the stdio.h functions */
Expand Down
Loading