From 5856d5e13b1f59187f1a73d1bda8075cba7daaa8 Mon Sep 17 00:00:00 2001 From: Stefan Kerkmann Date: Fri, 27 Oct 2023 18:44:58 +0200 Subject: [Maintenance] USB HID control packet as struct (#21688) * ChibiOS: USB HID control request as dedicated struct Instead of accessing the raw USB setup packet and documenting the values as the corresponding USB HID control request fields we introduce a struct that allows direct access to the fields. This is safer and self documenting. * Rename usb_request.h to usb_types.h In the future all shared USB data types can live in this file. --- tmk_core/protocol/chibios/usb_main.c | 151 ++++++++++++++--------------------- 1 file changed, 59 insertions(+), 92 deletions(-) (limited to 'tmk_core/protocol/chibios/usb_main.c') diff --git a/tmk_core/protocol/chibios/usb_main.c b/tmk_core/protocol/chibios/usb_main.c index f2158fd009..66f9ad0318 100644 --- a/tmk_core/protocol/chibios/usb_main.c +++ b/tmk_core/protocol/chibios/usb_main.c @@ -43,6 +43,7 @@ #include "usb_device_state.h" #include "usb_descriptor.h" #include "usb_driver.h" +#include "usb_types.h" #ifdef NKRO_ENABLE # include "keycode_config.h" @@ -103,30 +104,18 @@ union { NULL, /* SETUP buffer (not a SETUP endpoint) */ #endif -/* HID specific constants */ -#define HID_GET_REPORT 0x01 -#define HID_GET_IDLE 0x02 -#define HID_GET_PROTOCOL 0x03 -#define HID_SET_REPORT 0x09 -#define HID_SET_IDLE 0x0A -#define HID_SET_PROTOCOL 0x0B - -/* - * Handles the GET_DESCRIPTOR callback - * - * Returns the proper descriptor - */ static const USBDescriptor *usb_get_descriptor_cb(USBDriver *usbp, uint8_t dtype, uint8_t dindex, uint16_t wIndex) { - (void)usbp; - static USBDescriptor desc; - uint16_t wValue = ((uint16_t)dtype << 8) | dindex; - uint16_t wLength = ((uint16_t)usbp->setup[7] << 8) | usbp->setup[6]; - desc.ud_string = NULL; - desc.ud_size = get_usb_descriptor(wValue, wIndex, wLength, (const void **const) & desc.ud_string); - if (desc.ud_string == NULL) + usb_control_request_t *setup = (usb_control_request_t *)usbp->setup; + + static USBDescriptor descriptor; + descriptor.ud_string = NULL; + descriptor.ud_size = get_usb_descriptor(setup->wValue.word, setup->wIndex, setup->wLength, (const void **const) & descriptor.ud_string); + + if (descriptor.ud_string == NULL) { return NULL; - else - return &desc; + } + + return &descriptor; } /* @@ -497,8 +486,7 @@ void usb_event_queue_task(void) { } } -/* Handles the USB driver global events - * TODO: maybe disable some things when connection is lost? */ +/* Handles the USB driver global events. */ static void usb_event_cb(USBDriver *usbp, usbevent_t event) { switch (event) { case USB_EVENT_ADDRESS: @@ -570,16 +558,6 @@ static void usb_event_cb(USBDriver *usbp, usbevent_t event) { } } -/* Function used locally in os/hal/src/usb.c for getting descriptors - * need it here for HID descriptor */ -static uint16_t get_hword(uint8_t *p) { - uint16_t hw; - - hw = (uint16_t)*p++; - hw |= (uint16_t)*p << 8U; - return hw; -} - /* * Appendix G: HID Request Support Requirements * @@ -596,7 +574,9 @@ static uint16_t get_hword(uint8_t *p) { static uint8_t set_report_buf[2] __attribute__((aligned(4))); static void set_led_transfer_cb(USBDriver *usbp) { - if (usbp->setup[6] == 2) { /* LSB(wLength) */ + usb_control_request_t *setup = (usb_control_request_t *)usbp->setup; + + if (setup->wLength == 2) { uint8_t report_id = set_report_buf[0]; if ((report_id == REPORT_ID_KEYBOARD) || (report_id == REPORT_ID_NKRO)) { keyboard_led_state = set_report_buf[1]; @@ -606,24 +586,16 @@ static void set_led_transfer_cb(USBDriver *usbp) { } } -/* Callback for SETUP request on the endpoint 0 (control) */ -static bool usb_request_hook_cb(USBDriver *usbp) { - const USBDescriptor *dp; - - /* usbp->setup fields: - * 0: bmRequestType (bitmask) - * 1: bRequest - * 2,3: (LSB,MSB) wValue - * 4,5: (LSB,MSB) wIndex - * 6,7: (LSB,MSB) wLength (number of bytes to transfer if there is a data phase) */ +static bool usb_requests_hook_cb(USBDriver *usbp) { + usb_control_request_t *setup = (usb_control_request_t *)usbp->setup; /* Handle HID class specific requests */ - if (((usbp->setup[0] & USB_RTYPE_TYPE_MASK) == USB_RTYPE_TYPE_CLASS) && ((usbp->setup[0] & USB_RTYPE_RECIPIENT_MASK) == USB_RTYPE_RECIPIENT_INTERFACE)) { - switch (usbp->setup[0] & USB_RTYPE_DIR_MASK) { + if ((setup->bmRequestType & (USB_RTYPE_TYPE_MASK | USB_RTYPE_RECIPIENT_MASK)) == (USB_RTYPE_TYPE_CLASS | USB_RTYPE_RECIPIENT_INTERFACE)) { + switch (setup->bmRequestType & USB_RTYPE_DIR_MASK) { case USB_RTYPE_DIR_DEV2HOST: - switch (usbp->setup[1]) { /* bRequest */ - case HID_GET_REPORT: - switch (usbp->setup[4]) { /* LSB(wIndex) (check MSB==0?) */ + switch (setup->bRequest) { + case HID_REQ_GetReport: + switch (setup->wIndex) { #ifndef KEYBOARD_SHARED_EP case KEYBOARD_INTERFACE: usbSetupTransfer(usbp, (uint8_t *)&keyboard_report_sent, KEYBOARD_REPORT_SIZE, NULL); @@ -639,59 +611,54 @@ static bool usb_request_hook_cb(USBDriver *usbp) { #ifdef SHARED_EP_ENABLE case SHARED_INTERFACE: # ifdef KEYBOARD_SHARED_EP - if (usbp->setup[2] == REPORT_ID_KEYBOARD) { + if (setup->wValue.lbyte == REPORT_ID_KEYBOARD) { usbSetupTransfer(usbp, (uint8_t *)&keyboard_report_sent, KEYBOARD_REPORT_SIZE, NULL); - return TRUE; - break; + return true; } # endif # ifdef MOUSE_SHARED_EP - if (usbp->setup[2] == REPORT_ID_MOUSE) { + if (setup->wValue.lbyte == REPORT_ID_MOUSE) { usbSetupTransfer(usbp, (uint8_t *)&mouse_report_sent, sizeof(mouse_report_sent), NULL); - return TRUE; - break; + return true; } # endif #endif /* SHARED_EP_ENABLE */ default: - universal_report_blank.report_id = usbp->setup[2]; - usbSetupTransfer(usbp, (uint8_t *)&universal_report_blank, usbp->setup[6], NULL); - return TRUE; - break; + universal_report_blank.report_id = setup->wValue.lbyte; + usbSetupTransfer(usbp, (uint8_t *)&universal_report_blank, setup->wLength, NULL); + return true; } break; - case HID_GET_PROTOCOL: - if ((usbp->setup[4] == KEYBOARD_INTERFACE) && (usbp->setup[5] == 0)) { /* wIndex */ - usbSetupTransfer(usbp, &keyboard_protocol, 1, NULL); - return TRUE; + case HID_REQ_GetProtocol: + if (setup->wIndex == KEYBOARD_INTERFACE) { + usbSetupTransfer(usbp, &keyboard_protocol, sizeof(uint8_t), NULL); + return true; } break; - case HID_GET_IDLE: - usbSetupTransfer(usbp, &keyboard_idle, 1, NULL); - return TRUE; - break; + case HID_REQ_GetIdle: + usbSetupTransfer(usbp, &keyboard_idle, sizeof(uint8_t), NULL); + return true; } break; case USB_RTYPE_DIR_HOST2DEV: - switch (usbp->setup[1]) { /* bRequest */ - case HID_SET_REPORT: - switch (usbp->setup[4]) { /* LSB(wIndex) (check MSB==0?) */ + switch (setup->bRequest) { + case HID_REQ_SetReport: + switch (setup->wIndex) { case KEYBOARD_INTERFACE: #if defined(SHARED_EP_ENABLE) && !defined(KEYBOARD_SHARED_EP) case SHARED_INTERFACE: #endif usbSetupTransfer(usbp, set_report_buf, sizeof(set_report_buf), set_led_transfer_cb); - return TRUE; - break; + return true; } break; - case HID_SET_PROTOCOL: - if ((usbp->setup[4] == KEYBOARD_INTERFACE) && (usbp->setup[5] == 0)) { /* wIndex */ - keyboard_protocol = ((usbp->setup[2]) != 0x00); /* LSB(wValue) */ + case HID_REQ_SetProtocol: + if (setup->wIndex == KEYBOARD_INTERFACE) { + keyboard_protocol = setup->wValue.word; #ifdef NKRO_ENABLE if (!keyboard_protocol && keyboard_idle) { #else /* NKRO_ENABLE */ @@ -704,12 +671,11 @@ static bool usb_request_hook_cb(USBDriver *usbp) { } } usbSetupTransfer(usbp, NULL, 0, NULL); - return TRUE; - break; + return true; - case HID_SET_IDLE: - keyboard_idle = usbp->setup[3]; /* MSB(wValue) */ - /* arm the timer */ + case HID_REQ_SetIdle: + keyboard_idle = setup->wValue.hbyte; + /* arm the timer */ #ifdef NKRO_ENABLE if (!keymap_config.nkro && keyboard_idle) { #else /* NKRO_ENABLE */ @@ -720,19 +686,21 @@ static bool usb_request_hook_cb(USBDriver *usbp) { osalSysUnlockFromISR(); } usbSetupTransfer(usbp, NULL, 0, NULL); - return TRUE; - break; + return true; } break; } } - /* Handle the Get_Descriptor Request for HID class (not handled by the default hook) */ - if ((usbp->setup[0] == 0x81) && (usbp->setup[1] == USB_REQ_GET_DESCRIPTOR)) { - dp = usbp->config->get_descriptor_cb(usbp, usbp->setup[3], usbp->setup[2], get_hword(&usbp->setup[4])); - if (dp == NULL) return FALSE; - usbSetupTransfer(usbp, (uint8_t *)dp->ud_string, dp->ud_size, NULL); - return TRUE; + /* Handle the Get_Descriptor Request for HID class, which is not handled by + * the ChibiOS USB driver */ + if (((setup->bmRequestType & (USB_RTYPE_DIR_MASK | USB_RTYPE_RECIPIENT_MASK)) == (USB_RTYPE_DIR_DEV2HOST | USB_RTYPE_RECIPIENT_INTERFACE)) && (setup->bRequest == USB_REQ_GET_DESCRIPTOR)) { + const USBDescriptor *descriptor = usbp->config->get_descriptor_cb(usbp, setup->wValue.lbyte, setup->wValue.hbyte, setup->wIndex); + if (descriptor == NULL) { + return false; + } + usbSetupTransfer(usbp, (uint8_t *)descriptor->ud_string, descriptor->ud_size, NULL); + return true; } for (int i = 0; i < NUM_USB_DRIVERS; i++) { @@ -742,10 +710,9 @@ static bool usb_request_hook_cb(USBDriver *usbp) { } } - return FALSE; + return false; } -/* Start-of-frame callback */ static void usb_sof_cb(USBDriver *usbp) { osalSysLockFromISR(); for (int i = 0; i < NUM_USB_DRIVERS; i++) { @@ -758,7 +725,7 @@ static void usb_sof_cb(USBDriver *usbp) { static const USBConfig usbcfg = { usb_event_cb, /* USB events callback */ usb_get_descriptor_cb, /* Device GET_DESCRIPTOR request callback */ - usb_request_hook_cb, /* Requests hook callback */ + usb_requests_hook_cb, /* Requests hook callback */ usb_sof_cb /* Start Of Frame callback */ }; -- cgit v1.2.3