-
Notifications
You must be signed in to change notification settings - Fork 7.3k
usb: device_next: Add Kconfig to set maximum speed #76255
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
Changes from all commits
c377d24
5b26d84
189d130
0f771a4
c9100dc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,26 @@ module = USBD | |
module-str = usbd | ||
source "subsys/logging/Kconfig.template.log_config" | ||
|
||
choice USBD_MAX_SPEED_CHOICE | ||
prompt "Max supported connection speed" | ||
tmon-nordic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
default USBD_MAX_SPEED_HIGH if UDC_DRIVER_HAS_HIGH_SPEED_SUPPORT | ||
default USBD_MAX_SPEED_FULL | ||
|
||
config USBD_MAX_SPEED_HIGH | ||
bool "High-Speed" | ||
depends on UDC_DRIVER_HAS_HIGH_SPEED_SUPPORT | ||
|
||
config USBD_MAX_SPEED_FULL | ||
bool "Full-Speed" | ||
depends on !UDC_DRIVER_HIGH_SPEED_SUPPORT_ENABLED | ||
|
||
endchoice | ||
|
||
config USBD_MAX_SPEED | ||
int | ||
default 0 if USBD_MAX_SPEED_FULL | ||
default 1 if USBD_MAX_SPEED_HIGH | ||
Comment on lines
+34
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How come this indirection is needed? Why not simple do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The choice here makes it possible to add Super-Speed support later on without much hassle. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense, thanks. I'll take a closer look later today. |
||
|
||
config USBD_SHELL | ||
bool "USB device shell" | ||
depends on SHELL | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,13 +68,13 @@ static K_FIFO_DEFINE(bt_hci_tx_queue); | |
|
||
/* | ||
* Transfers through three endpoints proceed in a synchronous manner, | ||
* with maximum packet size of high speed bulk endpoint. | ||
* with maximum packet size of max supported speed bulk endpoint. | ||
* | ||
* REVISE: global (bulk, interrupt, iso) specific pools would be more | ||
* RAM usage efficient. | ||
*/ | ||
UDC_BUF_POOL_DEFINE(bt_hci_ep_pool, | ||
3, 512, | ||
3, USBD_MAX_BULK_MPS, | ||
sizeof(struct udc_buf_info), NULL); | ||
/* HCI RX/TX threads */ | ||
static K_KERNEL_STACK_DEFINE(rx_thread_stack, CONFIG_BT_HCI_TX_STACK_SIZE); | ||
|
@@ -138,7 +138,8 @@ static uint8_t bt_hci_get_bulk_in(struct usbd_class_data *const c_data) | |
struct bt_hci_data *data = usbd_class_get_private(c_data); | ||
struct usbd_bt_hci_desc *desc = data->desc; | ||
|
||
if (usbd_bus_speed(uds_ctx) == USBD_SPEED_HS) { | ||
if (USBD_SUPPORTS_HIGH_SPEED && | ||
usbd_bus_speed(uds_ctx) == USBD_SPEED_HS) { | ||
Comment on lines
+141
to
+142
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not look good and is easy to forget. I suggest adding a helper macro that could take usbd_bus_speed(), usbd_caps_speed_is_hs() or just speed as argument. diff --git a/include/zephyr/usb/usbd.h b/include/zephyr/usb/usbd.h
index 186f79b991b..8ff45bfe678 100644
--- a/include/zephyr/usb/usbd.h
+++ b/include/zephyr/usb/usbd.h
@@ -966,6 +966,15 @@ int usbd_wakeup_request(struct usbd_context *uds_ctx);
*/
enum usbd_speed usbd_bus_speed(const struct usbd_context *const uds_ctx);
+/**
+ * @brief Helper to check if speed is high speed
+ *
+ * @param[in] fspeed Parameter that expands to usbd_speed
+ *
+ * @return 1 if actual speed is high speed
+ */
+#define usbd_speed_is_hs(fspeed) \
+ (USBD_SUPPORTS_HIGH_SPEED && fspeed == USBD_SPEED_HS)
/**
* @brief Get highest speed supported by the controller
* Maybe also shorthand for usbd_bus_speed()/usbd_caps_speed() +/**
+ * @brief Helper to check if actual device speed is high speed
+ *
+ * @param[in] uds_ctx Pointer to a device context
+ *
+ * @return 1 if actual device speed is high speed
+ */
+#define usbd_bus_speed_is_hs(uds_ctx) \
+ (usbd_speed_is_hs(usbd_bus_speed(uds_ctx))) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tend to disagree. Hiding compile time evaluation conditions behind a macro that looks just like a function is IMHO hiding the fact that the code is supposed to go away when compiled with Full-Speed only configuration. I won't object the macros if you submit the PR yourself, but I just don't think these are justified myself. |
||
return desc->if0_hs_in_ep.bEndpointAddress; | ||
} | ||
|
||
|
@@ -151,7 +152,8 @@ static uint8_t bt_hci_get_bulk_out(struct usbd_class_data *const c_data) | |
struct bt_hci_data *data = usbd_class_get_private(c_data); | ||
struct usbd_bt_hci_desc *desc = data->desc; | ||
|
||
if (usbd_bus_speed(uds_ctx) == USBD_SPEED_HS) { | ||
if (USBD_SUPPORTS_HIGH_SPEED && | ||
usbd_bus_speed(uds_ctx) == USBD_SPEED_HS) { | ||
return desc->if0_hs_out_ep.bEndpointAddress; | ||
} | ||
|
||
|
@@ -449,7 +451,7 @@ static void *bt_hci_get_desc(struct usbd_class_data *const c_data, | |
{ | ||
struct bt_hci_data *data = usbd_class_get_private(c_data); | ||
|
||
if (speed == USBD_SPEED_HS) { | ||
if (USBD_SUPPORTS_HIGH_SPEED && speed == USBD_SPEED_HS) { | ||
return data->hs_desc; | ||
} | ||
|
||
|
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 commit changes too much. It should be limited to introducing the Kconfig option UDC_DRIVER_HAS_HIGH_SPEED_SUPPORT and related changes in the UDC drivers. They are independent of the stack changes. The stack changes should be done in the next commit.
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.
Done.