From 2e2dc7bdc5928dfa027291ca8c6d6792698e5b9a Mon Sep 17 00:00:00 2001 From: Jeremiah McCarthy Date: Wed, 7 Apr 2021 17:05:04 -0400 Subject: [PATCH] Revise per initial comments Returns the RT driver to the function state of previous iteration, which did not support the will_detach. Behavior should be fine without this feature. This removes much of the added bloat to track state, and handle requests in the APP_DETACH state which is no longer required. Removes the optional bloat added to the RT driver, such as responding to GETSTATE requests. Fixes the DFU Mode to extract the attr bits from the functional descriptor when opened. Fixes some incorrect bitwise if checks. Also, updates some naming of functions to be consistent with the rest of the library. --- src/class/dfu/dfu_mode_device.c | 19 ++++--- src/class/dfu/dfu_mode_device.h | 8 +-- src/class/dfu/dfu_rt_and_mode_device.c | 18 +++--- src/class/dfu/dfu_rt_and_mode_device.h | 10 ++-- src/class/dfu/dfu_rt_device.c | 76 +++----------------------- src/class/dfu/dfu_rt_device.h | 35 ------------ src/class/vendor/vendor_device.c | 2 +- src/common/tusb_types.h | 18 +++--- src/device/usbd.c | 16 +++--- 9 files changed, 55 insertions(+), 147 deletions(-) diff --git a/src/class/dfu/dfu_mode_device.c b/src/class/dfu/dfu_mode_device.c index 0b80b103d..7ad0f5168 100644 --- a/src/class/dfu/dfu_mode_device.c +++ b/src/class/dfu/dfu_mode_device.c @@ -65,11 +65,11 @@ static bool dfu_mode_state_machine(uint8_t rhport, tusb_control_request_t const //--------------------------------------------------------------------+ // USBD Driver API //--------------------------------------------------------------------+ -void dfu_mode_init(void) +void dfu_moded_init(void) { _dfu_state_ctx.state = APP_DETACH; // After init, reset will occur. We want to be in APP_DETACH to move to DFU_IDLE _dfu_state_ctx.status = DFU_STATUS_OK; - _dfu_state_ctx.attrs = tud_dfu_mode_init_attrs_cb(); + _dfu_state_ctx.attrs = 0; _dfu_state_ctx.blk_transfer_in_proc = false; _dfu_state_ctx.last_block_num = 0; _dfu_state_ctx.last_transfer_len = 0; @@ -77,7 +77,7 @@ void dfu_mode_init(void) dfu_debug_print_context(); } -void dfu_mode_reset(uint8_t rhport) +void dfu_moded_reset(uint8_t rhport) { if (_dfu_state_ctx.state == APP_DETACH) { @@ -118,14 +118,13 @@ void dfu_mode_reset(uint8_t rhport) } _dfu_state_ctx.status = DFU_STATUS_OK; - _dfu_state_ctx.attrs = tud_dfu_mode_init_attrs_cb(); _dfu_state_ctx.blk_transfer_in_proc = false; _dfu_state_ctx.last_block_num = 0; _dfu_state_ctx.last_transfer_len = 0; dfu_debug_print_context(); } -uint16_t dfu_mode_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len) +uint16_t dfu_moded_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len) { (void) rhport; (void) max_len; @@ -139,6 +138,9 @@ uint16_t dfu_mode_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, u if ( TUSB_DESC_FUNCTIONAL == tu_desc_type(p_desc) ) { + tusb_desc_dfu_functional_t *dfu_desc = (tusb_desc_dfu_functional_t *)p_desc; + _dfu_state_ctx.attrs = (uint8_t)dfu_desc->bAttributes; + drv_len += tu_desc_len(p_desc); p_desc = tu_desc_next(p_desc); } @@ -149,7 +151,7 @@ uint16_t dfu_mode_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, u // Invoked when a control transfer occurred on an interface of this class // Driver response accordingly to the request and the transfer stage (setup/data/ack) // return false to stall control endpoint (e.g unsupported request) -bool dfu_mode_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request) +bool dfu_moded_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request) { if ( (stage == CONTROL_STAGE_DATA) && (request->bRequest == DFU_DNLOAD_SYNC) ) { @@ -266,7 +268,7 @@ void tud_dfu_mode_poll_timeout_done() _dfu_state_ctx.state = DFU_DNLOAD_SYNC; } else if (_dfu_state_ctx.state == DFU_MANIFEST) { - _dfu_state_ctx.state = ((_dfu_state_ctx.attrs & DFU_FUNC_ATTR_MANIFESTATION_TOLERANT_BITMASK) == 0) + _dfu_state_ctx.state = ((_dfu_state_ctx.attrs & DFU_FUNC_ATTR_MANIFESTATION_TOLERANT_BITMASK) != 0) ? DFU_MANIFEST_WAIT_RESET : DFU_MANIFEST_SYNC; } } @@ -394,7 +396,6 @@ static bool dfu_mode_state_machine(uint8_t rhport, tusb_control_request_t const { _dfu_state_ctx.state = DFU_DNLOAD_SYNC; _dfu_state_ctx.blk_transfer_in_proc = true; - dfu_req_dnload_setup(rhport, request); } else { if ( tud_dfu_mode_device_data_done_check_cb() ) @@ -447,7 +448,7 @@ static bool dfu_mode_state_machine(uint8_t rhport, tusb_control_request_t const { case DFU_REQUEST_GETSTATUS: { - if ((_dfu_state_ctx.attrs & DFU_FUNC_ATTR_MANIFESTATION_TOLERANT_BITMASK) == 0) + if ((_dfu_state_ctx.attrs & DFU_FUNC_ATTR_MANIFESTATION_TOLERANT_BITMASK) != 0) { _dfu_state_ctx.state = DFU_MANIFEST; dfu_req_getstatus_reply(rhport, request); diff --git a/src/class/dfu/dfu_mode_device.h b/src/class/dfu/dfu_mode_device.h index 11b748c3b..aacb1911c 100644 --- a/src/class/dfu/dfu_mode_device.h +++ b/src/class/dfu/dfu_mode_device.h @@ -118,10 +118,10 @@ TU_ATTR_WEAK uint8_t tud_dfu_mode_get_status_desc_table_index_cb(); //--------------------------------------------------------------------+ // Internal Class Driver API //--------------------------------------------------------------------+ -void dfu_mode_init(void); -void dfu_mode_reset(uint8_t rhport); -uint16_t dfu_mode_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len); -bool dfu_mode_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request); +void dfu_moded_init(void); +void dfu_moded_reset(uint8_t rhport); +uint16_t dfu_moded_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len); +bool dfu_moded_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request); #ifdef __cplusplus diff --git a/src/class/dfu/dfu_rt_and_mode_device.c b/src/class/dfu/dfu_rt_and_mode_device.c index d555a1d58..8b6a8a766 100644 --- a/src/class/dfu/dfu_rt_and_mode_device.c +++ b/src/class/dfu/dfu_rt_and_mode_device.c @@ -62,9 +62,9 @@ static dfu_protocol_type_t mode = 0x00; //--------------------------------------------------------------------+ // USBD Driver API //--------------------------------------------------------------------+ -void dfu_init(void) +void dfu_d_init(void) { - mode = dfu_init_in_mode_cb(); + mode = tud_dfu_init_in_mode_cb(); switch (mode) { @@ -79,10 +79,10 @@ void dfu_init(void) case DFU_PROTOCOL_DFU: { - ctx.init = dfu_mode_init; - ctx.reset = dfu_mode_reset; - ctx.open = dfu_mode_open; - ctx.control_xfer_cb = dfu_mode_control_xfer_cb; + ctx.init = dfu_moded_init; + ctx.reset = dfu_moded_reset; + ctx.open = dfu_moded_open; + ctx.control_xfer_cb = dfu_moded_control_xfer_cb; } break; @@ -96,17 +96,17 @@ void dfu_init(void) ctx.init(); } -void dfu_reset(uint8_t rhport) +void dfu_d_reset(uint8_t rhport) { ctx.reset(rhport); } -uint16_t dfu_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len) +uint16_t dfu_d_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len) { return ctx.open(rhport, itf_desc, max_len); } -bool dfu_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request) +bool dfu_d_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request) { return ctx.control_xfer_cb(rhport, stage, request); } diff --git a/src/class/dfu/dfu_rt_and_mode_device.h b/src/class/dfu/dfu_rt_and_mode_device.h index 9734a031c..b6eafbba4 100644 --- a/src/class/dfu/dfu_rt_and_mode_device.h +++ b/src/class/dfu/dfu_rt_and_mode_device.h @@ -39,15 +39,15 @@ // Application Callback API (weak is optional) //--------------------------------------------------------------------+ // Invoked when the driver needs to determine the callbacks to use -dfu_protocol_type_t dfu_init_in_mode_cb(); +dfu_protocol_type_t tud_dfu_init_in_mode_cb(); //--------------------------------------------------------------------+ // Internal Class Driver API //--------------------------------------------------------------------+ -void dfu_init(void); -void dfu_reset(uint8_t rhport); -uint16_t dfu_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len); -bool dfu_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request); +void dfu_d_init(void); +void dfu_d_reset(uint8_t rhport); +uint16_t dfu_d_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len); +bool dfu_d_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request); #ifdef __cplusplus diff --git a/src/class/dfu/dfu_rt_device.c b/src/class/dfu/dfu_rt_device.c index d1d028d69..729b8e3e0 100644 --- a/src/class/dfu/dfu_rt_device.c +++ b/src/class/dfu/dfu_rt_device.c @@ -38,41 +38,17 @@ //--------------------------------------------------------------------+ // INTERNAL OBJECT & FUNCTION DECLARATION //--------------------------------------------------------------------+ -typedef struct TU_ATTR_PACKED -{ - dfu_mode_device_status_t status; - dfu_mode_state_t state; - uint8_t attrs; -} dfu_rt_state_ctx_t; - -// Only a single dfu state is allowed -CFG_TUSB_MEM_SECTION static dfu_rt_state_ctx_t _dfu_state_ctx; - -static void dfu_rt_getstatus_reply(uint8_t rhport, tusb_control_request_t const * request); //--------------------------------------------------------------------+ // USBD Driver API //--------------------------------------------------------------------+ void dfu_rtd_init(void) { - _dfu_state_ctx.state = APP_IDLE; - _dfu_state_ctx.status = DFU_STATUS_OK; - _dfu_state_ctx.attrs = tud_dfu_runtime_init_attrs_cb(); - dfu_debug_print_context(); } void dfu_rtd_reset(uint8_t rhport) { - if (((_dfu_state_ctx.attrs & DFU_FUNC_ATTR_WILL_DETACH_BITMASK) == 0) - && (_dfu_state_ctx.state == DFU_REQUEST_DETACH)) - { - tud_dfu_runtime_reboot_to_dfu_cb(); - } - - _dfu_state_ctx.state = APP_IDLE; - _dfu_state_ctx.status = DFU_STATUS_OK; - _dfu_state_ctx.attrs = tud_dfu_runtime_init_attrs_cb(); - dfu_debug_print_context(); + (void) rhport; } uint16_t dfu_rtd_open(uint8_t rhport, tusb_desc_interface_t const * itf_desc, uint16_t max_len) @@ -117,67 +93,29 @@ bool dfu_rtd_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request // Handle class request only from here TU_VERIFY(request->bmRequestType_bit.type == TUSB_REQ_TYPE_CLASS); - TU_LOG2(" DFU Request: %s\r\n", tu_lookup_find(&_dfu_request_table, request->bRequest)); + TU_LOG2(" DFU RT Request: %s\r\n", tu_lookup_find(&_dfu_request_table, request->bRequest)); switch (request->bRequest) { case DFU_REQUEST_DETACH: { - if (_dfu_state_ctx.state == APP_IDLE) - { - _dfu_state_ctx.state = APP_DETACH; - if ((_dfu_state_ctx.attrs & DFU_FUNC_ATTR_WILL_DETACH_BITMASK) == 1) - { - tud_dfu_runtime_reboot_to_dfu_cb(); - } else { - tud_dfu_runtime_detach_start_timer_cb(request->wValue); - } - } else { - TU_LOG2(" DFU Unexpected request during state %s: %u\r\n", tu_lookup_find(&_dfu_mode_state_table, _dfu_state_ctx.state), request->bRequest); - return false; - } + tud_control_status(rhport, request); + tud_dfu_runtime_reboot_to_dfu_cb(); } break; case DFU_REQUEST_GETSTATUS: { dfu_status_req_payload_t resp; - resp.bStatus = _dfu_state_ctx.status; - memset((uint8_t *)&resp.bwPollTimeout, 0x00, 3); // Value is ignored - resp.bState = _dfu_state_ctx.state; - resp.iString = ( tud_dfu_runtime_get_status_desc_table_index_cb ) ? tud_dfu_runtime_get_status_desc_table_index_cb() : 0; - + // Status = OK, Poll timeout is ignored during RT, State = APP_IDLE, IString = 0 + memset(&resp, 0x00, sizeof(dfu_status_req_payload_t)); tud_control_xfer(rhport, request, &resp, sizeof(dfu_status_req_payload_t)); } break; - case DFU_REQUEST_GETSTATE: - { - tud_control_xfer(rhport, request, &_dfu_state_ctx.state, 1); - } - break; - - default: - { - TU_LOG2(" DFU Nonstandard Runtime Request: %u\r\n", request->bRequest); - return ( tud_dfu_runtime_req_nonstandard_cb ) ? tud_dfu_runtime_req_nonstandard_cb(rhport, stage, request) : false; - } - break; + default: return false; // stall unsupported request } return true; } -void tud_dfu_runtime_set_status(dfu_mode_device_status_t status) -{ - _dfu_state_ctx.status = status; -} - -void tud_dfu_runtime_detach_timer_elapsed() -{ - if (_dfu_state_ctx.state == DFU_REQUEST_DETACH) - { - _dfu_state_ctx.state = APP_IDLE; - } -} - #endif diff --git a/src/class/dfu/dfu_rt_device.h b/src/class/dfu/dfu_rt_device.h index 23703862d..465bf9d99 100644 --- a/src/class/dfu/dfu_rt_device.h +++ b/src/class/dfu/dfu_rt_device.h @@ -38,44 +38,9 @@ //--------------------------------------------------------------------+ // Application Callback API (weak is optional) //--------------------------------------------------------------------+ -// Allow the application to update the status as required. -// Is set to DFU_STATUS_OK during internal initialization and USB reset -// Value is not checked to allow for custom statuses to be used -void tud_dfu_runtime_set_status(dfu_mode_device_status_t status); - // Invoked when a DFU_DETACH request is received and bitWillDetach is set void tud_dfu_runtime_reboot_to_dfu_cb(); -// Invoked when a DFU_DETACH request is received and bitWillDetach is not set -// This should start a timer for wTimeout ms -// When the timer has elapsed, the app must call tud_dfu_runtime_detach_timer_elapsed -// If a USB reset is called while the timer is running, the class will call -// tud_dfu_runtime_reboot_to_dfu_cb. -// NOTE: This callback should return immediately, and not implement the delay -// internally, as this can will hold up the USB stack -TU_ATTR_WEAK void tud_dfu_runtime_detach_start_timer_cb(uint16_t wTimeout); - -// Invoke when the dfu runtime detach timer has elapsed -void tud_dfu_runtime_detach_timer_elapsed(); - -// Invoked when a nonstandard request is received -// Use may be vendor specific. -// Return false to stall -TU_ATTR_WEAK bool tud_dfu_runtime_req_nonstandard_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const * request); - -// Invoked during initialization of the dfu driver to set attributes -// Return byte set with bitmasks: -// DFU_FUNC_ATTR_CAN_DOWNLOAD_BITMASK -// DFU_FUNC_ATTR_CAN_UPLOAD_BITMASK -// DFU_FUNC_ATTR_MANIFESTATION_TOLERANT_BITMASK -// DFU_FUNC_ATTR_WILL_DETACH_BITMASK -// Note: This should match the USB descriptor -uint8_t tud_dfu_runtime_init_attrs_cb(); - -// Invoked during a DFU_GETSTATUS request to get for the string index -// to the status description string table. -TU_ATTR_WEAK uint8_t tud_dfu_runtime_get_status_desc_table_index_cb(); - //--------------------------------------------------------------------+ // Internal Class Driver API //--------------------------------------------------------------------+ diff --git a/src/class/vendor/vendor_device.c b/src/class/vendor/vendor_device.c index 3fcea89c4..d39830a6a 100644 --- a/src/class/vendor/vendor_device.c +++ b/src/class/vendor/vendor_device.c @@ -1,4 +1,4 @@ -/* +/* * The MIT License (MIT) * * Copyright (c) 2019 Ha Thach (tinyusb.org) diff --git a/src/common/tusb_types.h b/src/common/tusb_types.h index a462b498f..7703ebb7b 100644 --- a/src/common/tusb_types.h +++ b/src/common/tusb_types.h @@ -427,13 +427,17 @@ typedef struct TU_ATTR_PACKED uint8_t bLength; uint8_t bDescriptorType; - struct TU_ATTR_PACKED { - uint8_t bitCanDnload : 1; - uint8_t bitCanUpload : 1; - uint8_t bitManifestationTolerant : 1; - uint8_t bitWillDetach : 1; - uint8_t reserved : 4; - } bmAttributes; + union { + struct TU_ATTR_PACKED { + uint8_t bitCanDnload : 1; + uint8_t bitCanUpload : 1; + uint8_t bitManifestationTolerant : 1; + uint8_t bitWillDetach : 1; + uint8_t reserved : 4; + } bmAttributes; + + uint8_t bAttributes; + }; uint16_t wDetachTimeOut; uint16_t wTransferSize; diff --git a/src/device/usbd.c b/src/device/usbd.c index 6f2262feb..92e521ec9 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -190,10 +190,10 @@ static usbd_class_driver_t const _usbd_driver[] = #if CFG_TUD_DFU_MODE { DRIVER_NAME("DFU-MODE") - .init = dfu_mode_init, - .reset = dfu_mode_reset, - .open = dfu_mode_open, - .control_xfer_cb = dfu_mode_control_xfer_cb, + .init = dfu_moded_init, + .reset = dfu_moded_reset, + .open = dfu_moded_open, + .control_xfer_cb = dfu_moded_control_xfer_cb, .xfer_cb = NULL, .sof = NULL }, @@ -202,10 +202,10 @@ static usbd_class_driver_t const _usbd_driver[] = #if CFG_TUD_DFU_RUNTIME_AND_MODE { DRIVER_NAME("DFU-RT-MODE") - .init = dfu_init, - .reset = dfu_reset, - .open = dfu_open, - .control_xfer_cb = dfu_control_xfer_cb, + .init = dfu_d_init, + .reset = dfu_d_reset, + .open = dfu_d_open, + .control_xfer_cb = dfu_d_control_xfer_cb, .xfer_cb = NULL, .sof = NULL },