From e8b30f3489eb98b2cde404bbc00fa8e6ef4f0127 Mon Sep 17 00:00:00 2001 From: Jared Boone Date: Thu, 18 Oct 2012 19:42:36 -0700 Subject: [PATCH 1/4] Oops. Request handlers called from setup handler could access the IN side of the endpoint, which does not get a copy of the SETUP bytes. TODO: Make a single copy of the SETUP bytes, and provide a clean way to access those bytes regardless of whether you're holding the IN or OUT endpoint. (This was a problem in the IN complete handler, and probably other places, too.) --- firmware/usb_performance/usb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/firmware/usb_performance/usb.c b/firmware/usb_performance/usb.c index 535cb52d..2737201e 100644 --- a/firmware/usb_performance/usb.c +++ b/firmware/usb_performance/usb.c @@ -557,6 +557,9 @@ static void usb_check_for_setup_events() { ); if( endpoint && endpoint->setup_complete ) { copy_setup(&endpoint->setup, usb_queue_head(endpoint->address)->setup); + // TODO: Clean up this duplicated effort by providing + // a cleaner way to get the SETUP data. + copy_setup(&endpoint->in->setup, usb_queue_head(endpoint->address)->setup); usb_clear_endpoint_setup_status(endptsetupstat_bit); endpoint->setup_complete(endpoint); } else { From 526a8e9e7a1cc66cbd180349b151570e297a892f Mon Sep 17 00:00:00 2001 From: Jared Boone Date: Thu, 18 Oct 2012 19:47:25 -0700 Subject: [PATCH 2/4] Add typed request return value that indicates request is OK or requires endpoint STALL. Changed vendor request to a lookup table, instead of an ever-growing switch statement. --- firmware/usb_performance/usb_performance.c | 116 ++++++++--------- firmware/usb_performance/usb_request.c | 7 +- firmware/usb_performance/usb_request.h | 7 +- .../usb_performance/usb_standard_request.c | 121 +++++++++--------- .../usb_performance/usb_standard_request.h | 2 +- 5 files changed, 121 insertions(+), 132 deletions(-) diff --git a/firmware/usb_performance/usb_performance.c b/firmware/usb_performance/usb_performance.c index a7ecac7f..4c74acf7 100644 --- a/firmware/usb_performance/usb_performance.c +++ b/firmware/usb_performance/usb_performance.c @@ -196,7 +196,7 @@ void set_transceiver_mode(const transceiver_mode_t new_transceiver_mode) { sgpio_cpld_stream_enable(); } -bool usb_vendor_request_set_transceiver_mode( +usb_request_status_t usb_vendor_request_set_transceiver_mode( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { @@ -205,22 +205,22 @@ bool usb_vendor_request_set_transceiver_mode( case 1: set_transceiver_mode(TRANSCEIVER_MODE_RX); usb_endpoint_schedule_ack(endpoint->in); - return true; + return USB_REQUEST_STATUS_OK; case 2: set_transceiver_mode(TRANSCEIVER_MODE_TX); usb_endpoint_schedule_ack(endpoint->in); - return true; + return USB_REQUEST_STATUS_OK; default: - return false; + return USB_REQUEST_STATUS_STALL; } } else { - return true; + return USB_REQUEST_STATUS_OK; } } -bool usb_vendor_request_write_max2837( +usb_request_status_t usb_vendor_request_write_max2837( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { @@ -229,16 +229,16 @@ bool usb_vendor_request_write_max2837( if( endpoint->setup.value < 0x3ff ) { max2837_reg_write(endpoint->setup.index, endpoint->setup.value); usb_endpoint_schedule_ack(endpoint->in); - return true; + return USB_REQUEST_STATUS_OK; } } - return false; + return USB_REQUEST_STATUS_STALL; } else { - return true; + return USB_REQUEST_STATUS_OK; } } -bool usb_vendor_request_read_max2837( +usb_request_status_t usb_vendor_request_read_max2837( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { @@ -249,15 +249,15 @@ bool usb_vendor_request_read_max2837( endpoint->buffer[1] = value >> 8; usb_endpoint_schedule(endpoint->in, &endpoint->buffer, 2); usb_endpoint_schedule_ack(endpoint->out); - return true; + return USB_REQUEST_STATUS_OK; } - return false; + return USB_REQUEST_STATUS_STALL; } else { - return true; + return USB_REQUEST_STATUS_OK; } } -bool usb_vendor_request_write_si5351c( +usb_request_status_t usb_vendor_request_write_si5351c( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { @@ -266,16 +266,16 @@ bool usb_vendor_request_write_si5351c( if( endpoint->setup.value < 256 ) { si5351c_write_single(endpoint->setup.index, endpoint->setup.value); usb_endpoint_schedule_ack(endpoint->in); - return true; + return USB_REQUEST_STATUS_OK; } } - return false; + return USB_REQUEST_STATUS_STALL; } else { - return true; + return USB_REQUEST_STATUS_OK; } } -bool usb_vendor_request_read_si5351c( +usb_request_status_t usb_vendor_request_read_si5351c( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { @@ -285,15 +285,15 @@ bool usb_vendor_request_read_si5351c( endpoint->buffer[0] = value; usb_endpoint_schedule(endpoint->in, &endpoint->buffer, 1); usb_endpoint_schedule_ack(endpoint->out); - return true; + return USB_REQUEST_STATUS_OK; } - return false; + return USB_REQUEST_STATUS_STALL; } else { - return true; + return USB_REQUEST_STATUS_OK; } } -bool usb_vendor_request_set_sample_rate( +usb_request_status_t usb_vendor_request_set_sample_rate( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { @@ -301,15 +301,15 @@ bool usb_vendor_request_set_sample_rate( const uint32_t sample_rate = (endpoint->setup.index << 16) | endpoint->setup.value; if( sample_rate_set(sample_rate) ) { usb_endpoint_schedule_ack(endpoint->in); - return true; + return USB_REQUEST_STATUS_OK; } - return false; + return USB_REQUEST_STATUS_STALL; } else { - return true; + return USB_REQUEST_STATUS_OK; } } -bool usb_vendor_request_set_baseband_filter_bandwidth( +usb_request_status_t usb_vendor_request_set_baseband_filter_bandwidth( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { @@ -317,56 +317,42 @@ bool usb_vendor_request_set_baseband_filter_bandwidth( const uint32_t bandwidth = (endpoint->setup.index << 16) | endpoint->setup.value; if( baseband_filter_bandwidth_set(bandwidth) ) { usb_endpoint_schedule_ack(endpoint->in); - return true; + return USB_REQUEST_STATUS_OK; } - return false; + return USB_REQUEST_STATUS_STALL; } else { - return true; + return USB_REQUEST_STATUS_OK; } } -void usb_vendor_request( +static const usb_request_handler_fn vendor_request_handler[] = { + NULL, + usb_vendor_request_set_transceiver_mode, + usb_vendor_request_write_max2837, + usb_vendor_request_read_max2837, + usb_vendor_request_write_si5351c, + usb_vendor_request_read_si5351c, + usb_vendor_request_set_sample_rate, + usb_vendor_request_set_baseband_filter_bandwidth, +}; + +static const uint32_t vendor_request_handler_count = + sizeof(vendor_request_handler) / sizeof(vendor_request_handler[0]); + +usb_request_status_t usb_vendor_request( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { - bool success = false; + usb_request_status_t status = USB_REQUEST_STATUS_STALL; - switch(endpoint->setup.request) { - case 1: - success = usb_vendor_request_set_transceiver_mode(endpoint, stage); - break; - - case 2: - success = usb_vendor_request_write_max2837(endpoint, stage); - break; - - case 3: - success = usb_vendor_request_read_max2837(endpoint, stage); - break; - - case 4: - success = usb_vendor_request_write_si5351c(endpoint, stage); - break; - - case 5: - success = usb_vendor_request_read_si5351c(endpoint, stage); - break; - - case 6: - success = usb_vendor_request_set_sample_rate(endpoint, stage); - break; - - case 7: - success = usb_vendor_request_set_baseband_filter_bandwidth(endpoint, stage); - break; - - default: - break; + if( endpoint->setup.request < vendor_request_handler_count ) { + usb_request_handler_fn handler = vendor_request_handler[endpoint->setup.request]; + if( handler ) { + status = handler(endpoint, stage); + } } - if( success != true ) { - usb_endpoint_stall(endpoint); - } + return status; } const usb_request_handlers_t usb_request_handlers = { diff --git a/firmware/usb_performance/usb_request.c b/firmware/usb_performance/usb_request.c index bfa4ea75..25132fd4 100644 --- a/firmware/usb_performance/usb_request.c +++ b/firmware/usb_performance/usb_request.c @@ -28,6 +28,7 @@ static void usb_request( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { + usb_request_status_t status = USB_REQUEST_STATUS_STALL; usb_request_handler_fn handler = 0; switch( endpoint->setup.request_type & USB_SETUP_REQUEST_TYPE_mask ) { @@ -49,8 +50,10 @@ static void usb_request( } if( handler ) { - handler(endpoint, stage); - } else { + status = handler(endpoint, stage); + } + + if( status != USB_REQUEST_STATUS_OK ) { // USB 2.0 section 9.2.7 "Request Error" usb_endpoint_stall(endpoint); } diff --git a/firmware/usb_performance/usb_request.h b/firmware/usb_performance/usb_request.h index 2d32bcea..9c1ef84f 100644 --- a/firmware/usb_performance/usb_request.h +++ b/firmware/usb_performance/usb_request.h @@ -37,7 +37,12 @@ typedef enum { USB_TRANSFER_STAGE_STATUS, } usb_transfer_stage_t; -typedef void (*usb_request_handler_fn)( +typedef enum { + USB_REQUEST_STATUS_OK = 0, + USB_REQUEST_STATUS_STALL = 1, +} usb_request_status_t; + +typedef usb_request_status_t (*usb_request_handler_fn)( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ); diff --git a/firmware/usb_performance/usb_standard_request.c b/firmware/usb_performance/usb_standard_request.c index 286d783b..71a38232 100644 --- a/firmware/usb_performance/usb_standard_request.c +++ b/firmware/usb_performance/usb_standard_request.c @@ -63,7 +63,7 @@ extern bool usb_set_configuration( const uint_fast8_t configuration_number ); -static void usb_send_descriptor( +static usb_request_status_t usb_send_descriptor( usb_endpoint_t* const endpoint, uint8_t* const descriptor_data ) { @@ -77,113 +77,110 @@ static void usb_send_descriptor( descriptor_data, (setup_length > descriptor_length) ? descriptor_length : setup_length ); + usb_endpoint_schedule_ack(endpoint->out); + return USB_REQUEST_STATUS_OK; } -static void usb_send_descriptor_string( +static usb_request_status_t usb_send_descriptor_string( usb_endpoint_t* const endpoint ) { uint_fast8_t index = endpoint->setup.value_l; for( uint_fast8_t i=0; usb_descriptor_strings[i] != 0; i++ ) { if( i == index ) { - usb_send_descriptor(endpoint, usb_descriptor_strings[i]); - return; + return usb_send_descriptor(endpoint, usb_descriptor_strings[i]); } } - usb_endpoint_stall(endpoint); + return USB_REQUEST_STATUS_STALL; } -static void usb_standard_request_get_descriptor_setup( +static usb_request_status_t usb_standard_request_get_descriptor_setup( usb_endpoint_t* const endpoint ) { switch( endpoint->setup.value_h ) { case USB_DESCRIPTOR_TYPE_DEVICE: - usb_send_descriptor(endpoint, usb_descriptor_device); - break; + return usb_send_descriptor(endpoint, usb_descriptor_device); case USB_DESCRIPTOR_TYPE_CONFIGURATION: // TODO: Duplicated code. Refactor. if( usb_speed(endpoint->device) == USB_SPEED_HIGH ) { - usb_send_descriptor(endpoint, usb_descriptor_configuration_high_speed); + return usb_send_descriptor(endpoint, usb_descriptor_configuration_high_speed); } else { - usb_send_descriptor(endpoint, usb_descriptor_configuration_full_speed); + return usb_send_descriptor(endpoint, usb_descriptor_configuration_full_speed); } - break; case USB_DESCRIPTOR_TYPE_DEVICE_QUALIFIER: - usb_send_descriptor(endpoint, usb_descriptor_device_qualifier); - break; + return usb_send_descriptor(endpoint, usb_descriptor_device_qualifier); case USB_DESCRIPTOR_TYPE_OTHER_SPEED_CONFIGURATION: // TODO: Duplicated code. Refactor. if( usb_speed(endpoint->device) == USB_SPEED_HIGH ) { - usb_send_descriptor(endpoint, usb_descriptor_configuration_full_speed); + return usb_send_descriptor(endpoint, usb_descriptor_configuration_full_speed); } else { - usb_send_descriptor(endpoint, usb_descriptor_configuration_high_speed); + return usb_send_descriptor(endpoint, usb_descriptor_configuration_high_speed); } - break; case USB_DESCRIPTOR_TYPE_STRING: - usb_send_descriptor_string(endpoint); - break; + return usb_send_descriptor_string(endpoint); case USB_DESCRIPTOR_TYPE_INTERFACE: case USB_DESCRIPTOR_TYPE_ENDPOINT: default: - usb_endpoint_stall(endpoint); - break; + return USB_REQUEST_STATUS_STALL; } } -static void usb_standard_request_get_descriptor( +static usb_request_status_t usb_standard_request_get_descriptor( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { switch( stage ) { case USB_TRANSFER_STAGE_SETUP: - usb_standard_request_get_descriptor_setup(endpoint); - usb_endpoint_schedule_ack(endpoint->out); - break; + return usb_standard_request_get_descriptor_setup(endpoint); case USB_TRANSFER_STAGE_DATA: - break; - case USB_TRANSFER_STAGE_STATUS: - break; + return USB_REQUEST_STATUS_OK; + default: + return USB_REQUEST_STATUS_STALL; } } /*********************************************************************/ -static void usb_standard_request_set_address( +static usb_request_status_t usb_standard_request_set_address_setup( + usb_endpoint_t* const endpoint +) { + usb_set_address_deferred(endpoint->device, endpoint->setup.value_l); + usb_endpoint_schedule_ack(endpoint->in); + return USB_REQUEST_STATUS_OK; +} + +static usb_request_status_t usb_standard_request_set_address( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { switch( stage ) { case USB_TRANSFER_STAGE_SETUP: - usb_set_address_deferred(endpoint->device, endpoint->setup.value_l); - usb_endpoint_schedule_ack(endpoint->in); - break; + return usb_standard_request_set_address_setup(endpoint); case USB_TRANSFER_STAGE_DATA: - break; - case USB_TRANSFER_STAGE_STATUS: /* NOTE: Not necessary to set address here, as DEVICEADR.USBADRA bit * will cause controller to automatically perform set address * operation on IN ACK. */ - break; + return USB_REQUEST_STATUS_OK; default: - break; + return USB_REQUEST_STATUS_STALL; } } /*********************************************************************/ -static void usb_standard_request_set_configuration_setup( +static usb_request_status_t usb_standard_request_set_configuration_setup( usb_endpoint_t* const endpoint ) { const uint8_t usb_configuration = endpoint->setup.value_l; @@ -193,32 +190,32 @@ static void usb_standard_request_set_configuration_setup( usb_set_address_immediate(endpoint->device, 0); } usb_endpoint_schedule_ack(endpoint->in); + return USB_REQUEST_STATUS_OK; } else { - usb_endpoint_stall(endpoint); + return USB_REQUEST_STATUS_STALL; } } -static void usb_standard_request_set_configuration( +static usb_request_status_t usb_standard_request_set_configuration( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { switch( stage ) { case USB_TRANSFER_STAGE_SETUP: - usb_standard_request_set_configuration_setup(endpoint); - break; + return usb_standard_request_set_configuration_setup(endpoint); case USB_TRANSFER_STAGE_DATA: - break; - case USB_TRANSFER_STAGE_STATUS: - break; + return USB_REQUEST_STATUS_OK; + default: + return USB_REQUEST_STATUS_STALL; } } /*********************************************************************/ -static void usb_standard_request_get_configuration_setup( +static usb_request_status_t usb_standard_request_get_configuration_setup( usb_endpoint_t* const endpoint ) { if( endpoint->setup.length == 1 ) { @@ -227,52 +224,50 @@ static void usb_standard_request_get_configuration_setup( endpoint->buffer[0] = endpoint->device->configuration->number; } usb_endpoint_schedule(endpoint->in, &endpoint->buffer, 1); + usb_endpoint_schedule_ack(endpoint->out); + return USB_REQUEST_STATUS_OK; } else { - usb_endpoint_stall(endpoint); + return USB_REQUEST_STATUS_STALL; } } -static void usb_standard_request_get_configuration( +static usb_request_status_t usb_standard_request_get_configuration( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { switch( stage ) { case USB_TRANSFER_STAGE_SETUP: - usb_standard_request_get_configuration_setup(endpoint); - usb_endpoint_schedule_ack(endpoint->out); - break; + return usb_standard_request_get_configuration_setup(endpoint); case USB_TRANSFER_STAGE_DATA: - break; - case USB_TRANSFER_STAGE_STATUS: - break; - + return USB_REQUEST_STATUS_OK; + + default: + return USB_REQUEST_STATUS_STALL; } } /*********************************************************************/ -void usb_standard_request( +usb_request_status_t usb_standard_request( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ) { switch( endpoint->setup.request ) { case USB_STANDARD_REQUEST_GET_DESCRIPTOR: - usb_standard_request_get_descriptor(endpoint, stage); - break; + return usb_standard_request_get_descriptor(endpoint, stage); case USB_STANDARD_REQUEST_SET_ADDRESS: - usb_standard_request_set_address(endpoint, stage); - break; + return usb_standard_request_set_address(endpoint, stage); case USB_STANDARD_REQUEST_SET_CONFIGURATION: - usb_standard_request_set_configuration(endpoint, stage); - break; + return usb_standard_request_set_configuration(endpoint, stage); case USB_STANDARD_REQUEST_GET_CONFIGURATION: - usb_standard_request_get_configuration(endpoint, stage); - break; - + return usb_standard_request_get_configuration(endpoint, stage); + + default: + return USB_REQUEST_STATUS_STALL; } } diff --git a/firmware/usb_performance/usb_standard_request.h b/firmware/usb_performance/usb_standard_request.h index e96dba21..5a0bdac6 100644 --- a/firmware/usb_performance/usb_standard_request.h +++ b/firmware/usb_performance/usb_standard_request.h @@ -25,7 +25,7 @@ #include "usb_type.h" #include "usb_request.h" -void usb_standard_request( +usb_request_status_t usb_standard_request( usb_endpoint_t* const endpoint, const usb_transfer_stage_t stage ); From 413b34eb8677f17c69dcd6f1c8b8305d4d1e407f Mon Sep 17 00:00:00 2001 From: Jared Boone Date: Thu, 1 Nov 2012 22:48:45 -0700 Subject: [PATCH 3/4] Add #includes required for Linux. --- host/libhackrf/examples/hackrf_transfer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/host/libhackrf/examples/hackrf_transfer.c b/host/libhackrf/examples/hackrf_transfer.c index 089fe823..36ef463b 100644 --- a/host/libhackrf/examples/hackrf_transfer.c +++ b/host/libhackrf/examples/hackrf_transfer.c @@ -27,6 +27,8 @@ #include #include +#include +#include #include #include #include From cc5f1c61c714b5e3c1da3818b6ef20c6b2cd54b2 Mon Sep 17 00:00:00 2001 From: Jared Boone Date: Fri, 2 Nov 2012 22:34:43 -0700 Subject: [PATCH 4/4] Fix clock edge for TX mode. TX data was completely crapped up due to skew on my Jellybean/Lemondrop board. Hopefully, this also applies to Jawbreaker. I'll recheck once I have hardware in-hand. --- firmware/common/sgpio.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/firmware/common/sgpio.c b/firmware/common/sgpio.c index d53c3f87..a4b74c53 100644 --- a/firmware/common/sgpio.c +++ b/firmware/common/sgpio.c @@ -167,6 +167,7 @@ void sgpio_configure( const bool input_slice = (i == 0) && (transceiver_mode == TRANSCEIVER_MODE_RX); const uint_fast8_t concat_order = (input_slice || single_slice) ? 0 : 3; const uint_fast8_t concat_enable = (input_slice || single_slice) ? 0 : 1; + const uint_fast8_t clk_capture_mode = (transceiver_mode == TRANSCEIVER_MODE_RX) ? 1 : 0; SGPIO_MUX_CFG(slice_index) = SGPIO_MUX_CFG_CONCAT_ORDER(concat_order) @@ -185,7 +186,7 @@ void sgpio_configure( | SGPIO_SLICE_MUX_CFG_DATA_CAPTURE_MODE(0) | SGPIO_SLICE_MUX_CFG_INV_OUT_CLK(0) | SGPIO_SLICE_MUX_CFG_CLKGEN_MODE(1) - | SGPIO_SLICE_MUX_CFG_CLK_CAPTURE_MODE(1) + | SGPIO_SLICE_MUX_CFG_CLK_CAPTURE_MODE(clk_capture_mode) | SGPIO_SLICE_MUX_CFG_MATCH_MODE(0) ;