usb: device_next: allow string descriptor index to be updated
Support for multiple instances of a class implementation,
and the ability to register an instance to a configuration
at runtime, requires a mechanism to add a string descriptor
and update its index based on the total number of descriptors.
We also need to handle some special string descriptors like
Product or Serial Number provided by the application.
Marked as such by using specific macros, these descriptors
can be sorted out by the stack and the device descriptor
indexes are updated automatically.
Signed-off-by: Johann Fischer <johann.fischer@nordicsemi.no>
diff --git a/include/zephyr/usb/usbd.h b/include/zephyr/usb/usbd.h
index c060411..12ce411 100644
--- a/include/zephyr/usb/usbd.h
+++ b/include/zephyr/usb/usbd.h
@@ -50,9 +50,14 @@
*/
#define USB_STRING_DESCRIPTOR_LENGTH(s) (sizeof(s) * 2)
-#define USBD_DESC_MANUFACTURER_IDX 1
-#define USBD_DESC_PRODUCT_IDX 2
-#define USBD_DESC_SERIAL_NUMBER_IDX 3
+/* Used internally to keep descriptors in order */
+enum usbd_desc_usage_type {
+ USBD_DUT_STRING_LANG,
+ USBD_DUT_STRING_MANUFACTURER,
+ USBD_DUT_STRING_PRODUCT,
+ USBD_DUT_STRING_SERIAL_NUMBER,
+ USBD_DUT_STRING_INTERFACE,
+};
/**
* Descriptor node
@@ -65,6 +70,8 @@
sys_dnode_t node;
/** Descriptor index, required for string descriptors */
unsigned int idx : 8;
+ /** Descriptor usage type (not bDescriptorType) */
+ unsigned int utype : 8;
/** If not set, string descriptor must be converted to UTF16LE */
unsigned int utf16le : 1;
/** If not set, device stack obtains SN using the hwinfo API */
@@ -312,7 +319,19 @@
.desc = &cfg_desc_##name, \
}
-
+/**
+ * @brief Create a string descriptor node and language string descriptor
+ *
+ * This macro defines a descriptor node and a string descriptor that,
+ * when added to the device context, is automatically used as the language
+ * string descriptor zero. Both descriptor node and descriptor are defined with
+ * static-storage-class specifier. Default and currently only supported
+ * language ID is 0x0409 English (United States).
+ * If string descriptors are used, it is necessary to add this descriptor
+ * as the first one to the USB device context.
+ *
+ * @param name Language string descriptor node identifier.
+ */
#define USBD_DESC_LANG_DEFINE(name) \
static struct usb_string_descriptor \
string_desc_##name = { \
@@ -322,10 +341,11 @@
}; \
static struct usbd_desc_node name = { \
.idx = 0, \
+ .utype = USBD_DUT_STRING_LANG, \
.desc = &string_desc_##name, \
}
-#define USBD_DESC_STRING_DEFINE(d_name, d_string, d_idx) \
+#define USBD_DESC_STRING_DEFINE(d_name, d_string, d_utype) \
struct usb_string_descriptor_##d_name { \
uint8_t bLength; \
uint8_t bDescriptorType; \
@@ -337,20 +357,53 @@
.bDescriptorType = USB_DESC_STRING, \
.bString = d_string, \
}; \
- BUILD_ASSERT(d_idx != 0, "Index 0 is not allowed"); \
static struct usbd_desc_node d_name = { \
- .idx = d_idx, \
+ .utype = d_utype, \
.desc = &string_desc_##d_name, \
}
+/**
+ * @brief Create a string descriptor node and manufacturer string descriptor
+ *
+ * This macro defines a descriptor node and a string descriptor that,
+ * when added to the device context, is automatically used as the manufacturer
+ * string descriptor. Both descriptor node and descriptor are defined with
+ * static-storage-class specifier.
+ *
+ * @param d_name String descriptor node identifier.
+ * @param d_string ASCII7 encoded manufacturer string literal
+ */
#define USBD_DESC_MANUFACTURER_DEFINE(d_name, d_string) \
- USBD_DESC_STRING_DEFINE(d_name, d_string, USBD_DESC_MANUFACTURER_IDX)
+ USBD_DESC_STRING_DEFINE(d_name, d_string, USBD_DUT_STRING_MANUFACTURER)
+/**
+ * @brief Create a string descriptor node and product string descriptor
+ *
+ * This macro defines a descriptor node and a string descriptor that,
+ * when added to the device context, is automatically used as the product
+ * string descriptor. Both descriptor node and descriptor are defined with
+ * static-storage-class specifier.
+ *
+ * @param d_name String descriptor node identifier.
+ * @param d_string ASCII7 encoded product string literal
+ */
#define USBD_DESC_PRODUCT_DEFINE(d_name, d_string) \
- USBD_DESC_STRING_DEFINE(d_name, d_string, USBD_DESC_PRODUCT_IDX)
+ USBD_DESC_STRING_DEFINE(d_name, d_string, USBD_DUT_STRING_PRODUCT)
+/**
+ * @brief Create a string descriptor node and serial number string descriptor
+ *
+ * This macro defines a descriptor node and a string descriptor that,
+ * when added to the device context, is automatically used as the serial number
+ * string descriptor. The string literal parameter is used as a placeholder,
+ * the unique number is obtained from hwinfo. Both descriptor node and descriptor
+ * are defined with static-storage-class specifier.
+ *
+ * @param d_name String descriptor node identifier.
+ * @param d_string ASCII7 encoded serial number string literal placeholder
+ */
#define USBD_DESC_SERIAL_NUMBER_DEFINE(d_name, d_string) \
- USBD_DESC_STRING_DEFINE(d_name, d_string, USBD_DESC_SERIAL_NUMBER_IDX)
+ USBD_DESC_STRING_DEFINE(d_name, d_string, USBD_DUT_STRING_SERIAL_NUMBER)
#define USBD_DEFINE_CLASS(class_name, class_api, class_data) \
static STRUCT_SECTION_ITERABLE(usbd_class_node, class_name) = { \
diff --git a/subsys/usb/device_next/class/usbd_cdc_ecm.c b/subsys/usb/device_next/class/usbd_cdc_ecm.c
index 00525bc..6f7200c 100644
--- a/subsys/usb/device_next/class/usbd_cdc_ecm.c
+++ b/subsys/usb/device_next/class/usbd_cdc_ecm.c
@@ -24,12 +24,6 @@
#define CDC_ECM_EP_MPS_INT 16
#define CDC_ECM_EP_INTERVAL_INT 0x0A
-/*
- * REVISE: We can change usbd_add_descriptor() so that hardcoded
- * indexes are not necessary.
- */
-#define CDC_ECM_IMAC_BASE 4
-
enum {
CDC_ECM_IFACE_UP,
CDC_ECM_CLASS_ENABLED,
@@ -416,12 +410,24 @@
LOG_DBG("CDC ECM class initialized");
if (usbd_add_descriptor(c_nd->data->uds_ctx, data->mac_desc_nd)) {
- LOG_ERR("Failed to add iMACAddress");
+ LOG_ERR("Failed to add iMACAddress string descriptor");
+ } else {
+ desc->if0_ecm.iMACAddress = data->mac_desc_nd->idx;
}
return 0;
}
+static void usbd_cdc_ecm_shutdown(struct usbd_class_node *const c_nd)
+{
+ struct usbd_cdc_ecm_desc *desc = c_nd->data->desc;
+ const struct device *dev = c_nd->data->priv;
+ struct cdc_ecm_eth_data *const data = dev->data;
+
+ desc->if0_ecm.iMACAddress = 0;
+ sys_dlist_remove(&data->mac_desc_nd->node);
+}
+
static int cdc_ecm_send(const struct device *dev, struct net_pkt *const pkt)
{
struct cdc_ecm_eth_data *const data = dev->data;
@@ -573,6 +579,7 @@
.resumed = usbd_cdc_ecm_resumed,
.control_to_dev = usbd_cdc_ecm_ctd,
.init = usbd_cdc_ecm_init,
+ .shutdown = usbd_cdc_ecm_shutdown,
};
static const struct ethernet_api cdc_ecm_eth_api = {
@@ -629,7 +636,7 @@
.bFunctionLength = sizeof(struct cdc_ecm_descriptor), \
.bDescriptorType = USB_DESC_CS_INTERFACE, \
.bDescriptorSubtype = ETHERNET_FUNC_DESC, \
- .iMACAddress = CDC_ECM_IMAC_BASE + n, \
+ .iMACAddress = 0, \
.bmEthernetStatistics = sys_cpu_to_le32(0), \
.wMaxSegmentSize = sys_cpu_to_le16(NET_ETH_MAX_FRAME_SIZE), \
.wNumberMCFilters = sys_cpu_to_le16(0), \
@@ -697,7 +704,7 @@
CDC_ECM_DEFINE_DESCRIPTOR(n); \
USBD_DESC_STRING_DEFINE(mac_desc_nd_##n, \
DT_INST_PROP(n, remote_mac_address), \
- CDC_ECM_IMAC_BASE + n); \
+ USBD_DUT_STRING_INTERFACE); \
\
static struct usbd_class_data usbd_cdc_ecm_data_##n; \
\
diff --git a/subsys/usb/device_next/usbd_desc.c b/subsys/usb/device_next/usbd_desc.c
index 2f20614..2e8cb1c 100644
--- a/subsys/usb/device_next/usbd_desc.c
+++ b/subsys/usb/device_next/usbd_desc.c
@@ -104,6 +104,73 @@
return 0;
}
+static inline bool desc_type_equal(const struct usbd_desc_node *const a,
+ const struct usbd_desc_node *const b)
+{
+ const struct usb_desc_header *const head_a = a->desc;
+ const struct usb_desc_header *const head_b = b->desc;
+
+ return head_a->bDescriptorType == head_b->bDescriptorType;
+}
+
+/*
+ * Add descriptor node to the descriptor list in ascending order by index
+ * and sorted by bDescriptorType. For the string descriptors, the function
+ * does not care about index zero for the language string descriptor,
+ * so if it is not added first, the device will be non-compliant.
+ */
+static int desc_add_and_update_idx(struct usbd_contex *const uds_ctx,
+ struct usbd_desc_node *const new_nd)
+{
+ struct usbd_desc_node *tmp_nd;
+
+ SYS_DLIST_FOR_EACH_CONTAINER(&uds_ctx->descriptors, tmp_nd, node) {
+ struct usbd_desc_node *next_nd;
+
+ if (!desc_type_equal(tmp_nd, new_nd)) {
+ continue;
+ }
+
+ next_nd = SYS_DLIST_PEEK_NEXT_CONTAINER(&uds_ctx->descriptors,
+ tmp_nd,
+ node);
+
+ if (next_nd == NULL) {
+ /* Last node of the same bDescriptorType or tail */
+ new_nd->idx = tmp_nd->idx + 1;
+ sys_dlist_append(&uds_ctx->descriptors, &new_nd->node);
+ LOG_DBG("Add %u behind %u", new_nd->idx, tmp_nd->idx);
+
+ return 0;
+ }
+
+ if (!desc_type_equal(next_nd, new_nd)) {
+ /* Last node of the same bDescriptorType */
+ new_nd->idx = tmp_nd->idx + 1;
+ sys_dlist_insert(&next_nd->node, &new_nd->node);
+ LOG_DBG("Add %u before %u", new_nd->idx, next_nd->idx);
+
+ return 0;
+ }
+
+ if (tmp_nd->idx != (next_nd->idx - 1)) {
+ /* Add between nodes of the same bDescriptorType */
+ new_nd->idx = tmp_nd->idx + 1;
+ sys_dlist_insert(&next_nd->node, &new_nd->node);
+ LOG_DBG("Add %u between %u and %u",
+ tmp_nd->idx, next_nd->idx, new_nd->idx);
+ return 0;
+ }
+ }
+
+ /* If there are none of same bDescriptorType, node idx is set to 0. */
+ new_nd->idx = 0;
+ sys_dlist_append(&uds_ctx->descriptors, &new_nd->node);
+ LOG_DBG("Added first descriptor node (usage type %u)", new_nd->utype);
+
+ return 0;
+}
+
void *usbd_get_descriptor(struct usbd_contex *const uds_ctx,
const uint8_t type, const uint8_t idx)
{
@@ -136,12 +203,17 @@
int usbd_add_descriptor(struct usbd_contex *const uds_ctx,
struct usbd_desc_node *const desc_nd)
{
+ struct usb_device_descriptor *dev_desc = uds_ctx->desc;
struct usb_desc_header *head;
- uint8_t type;
int ret = 0;
usbd_device_lock(uds_ctx);
+ if (dev_desc == NULL || usbd_is_initialized(uds_ctx)) {
+ ret = -EPERM;
+ goto add_descriptor_error;
+ }
+
/* Check if descriptor list is initialized */
if (!sys_dnode_is_linked(&uds_ctx->descriptors)) {
LOG_DBG("Initialize descriptors list");
@@ -149,28 +221,28 @@
}
head = desc_nd->desc;
- type = head->bDescriptorType;
if (sys_dnode_is_linked(&desc_nd->node)) {
ret = -EALREADY;
goto add_descriptor_error;
}
- if (type == USB_DESC_STRING) {
- struct usb_device_descriptor *dev_desc = uds_ctx->desc;
+ ret = desc_add_and_update_idx(uds_ctx, desc_nd);
+ if (ret) {
+ ret = -EINVAL;
+ goto add_descriptor_error;
+ }
- if (dev_desc == NULL) {
- ret = -EPERM;
- goto add_descriptor_error;
- }
-
- switch (desc_nd->idx) {
- case USBD_DESC_MANUFACTURER_IDX:
+ if (head->bDescriptorType == USB_DESC_STRING) {
+ switch (desc_nd->utype) {
+ case USBD_DUT_STRING_LANG:
+ break;
+ case USBD_DUT_STRING_MANUFACTURER:
dev_desc->iManufacturer = desc_nd->idx;
break;
- case USBD_DESC_PRODUCT_IDX:
+ case USBD_DUT_STRING_PRODUCT:
dev_desc->iProduct = desc_nd->idx;
break;
- case USBD_DESC_SERIAL_NUMBER_IDX:
+ case USBD_DUT_STRING_SERIAL_NUMBER:
if (!desc_nd->custom_sn) {
ret = usbd_get_sn_from_hwid(desc_nd);
desc_nd->utf16le = false;
@@ -187,8 +259,6 @@
}
}
- sys_dlist_append(&uds_ctx->descriptors, &desc_nd->node);
-
add_descriptor_error:
usbd_device_unlock(uds_ctx);
return ret;
