Merge pull request #1531 from pigrew/usbtmc_highspeed

USBTMC: Fixes and support for high-speed USB

examples/device/usbtmc/src/usb_descriptors.c

@@ -37,6 +37,9 @@
 #define USB_PID           (0x4000 | _PID_MAP(CDC, 0) | _PID_MAP(MSC, 1) | _PID_MAP(HID, 2) | \
                            _PID_MAP(MIDI, 3) | _PID_MAP(VENDOR, 4) )
 
+#define USB_VID   0xCafe
+#define USB_BCD   0x0200
+
 //--------------------------------------------------------------------+
 // Device Descriptors
 //--------------------------------------------------------------------+
@@ -51,9 +54,9 @@ tusb_desc_device_t const desc_device =
 
     .bMaxPacketSize0    = CFG_TUD_ENDPOINT0_SIZE,
 
-    .idVendor           = 0xCafe,
+    .idVendor           = USB_VID,
     .idProduct          = USB_PID,
-    .bcdDevice          = 0x0100,
+    .bcdDevice          = USB_BCD,
 
     .iManufacturer      = 0x01,
     .iProduct           = 0x02,
@@ -75,22 +78,22 @@ uint8_t const * tud_descriptor_device_cb(void)
 
 #if defined(CFG_TUD_USBTMC)
 
-#  define TUD_USBTMC_DESC_MAIN(_itfnum,_bNumEndpoints) \
+#  define TUD_USBTMC_DESC_MAIN(_itfnum,_bNumEndpoints, _bulkMaxPacketLength) \
      TUD_USBTMC_IF_DESCRIPTOR(_itfnum, _bNumEndpoints,  /*_stridx = */ 4u, TUD_USBTMC_PROTOCOL_USB488), \
-     TUD_USBTMC_BULK_DESCRIPTORS(/* OUT = */0x01, /* IN = */ 0x81, /* packet size = */USBTMCD_MAX_PACKET_SIZE)
+     TUD_USBTMC_BULK_DESCRIPTORS(/* OUT = */0x01, /* IN = */ 0x81, /* packet size = */_bulkMaxPacketLength)
 
 #if CFG_TUD_USBTMC_ENABLE_INT_EP
 // USBTMC Interrupt xfer always has length of 2, but we use epMaxSize=8 for
 //  compatibility with mcus that only allow 8, 16, 32 or 64 for FS endpoints
-#  define TUD_USBTMC_DESC(_itfnum) \
-     TUD_USBTMC_DESC_MAIN(_itfnum, /* _epCount = */ 3), \
+#  define TUD_USBTMC_DESC(_itfnum, _bulkMaxPacketLength) \
+     TUD_USBTMC_DESC_MAIN(_itfnum, /* _epCount = */ 3, _bulkMaxPacketLength), \
      TUD_USBTMC_INT_DESCRIPTOR(/* INT ep # */ 0x82, /* epMaxSize = */ 8, /* bInterval = */16u )
 #  define TUD_USBTMC_DESC_LEN (TUD_USBTMC_IF_DESCRIPTOR_LEN + TUD_USBTMC_BULK_DESCRIPTORS_LEN + TUD_USBTMC_INT_DESCRIPTOR_LEN)
 
 #else
 
-#  define TUD_USBTMC_DESC(_itfnum) \
-     TUD_USBTMC_DESC_MAIN(_itfnum, /* _epCount = */ 2u)
+#  define TUD_USBTMC_DESC(_itfnum, _bulkMaxPacketLength) \
+     TUD_USBTMC_DESC_MAIN(_itfnum, /* _epCount = */ 2u, _bulkMaxPacketLength)
 #  define TUD_USBTMC_DESC_LEN (TUD_USBTMC_IF_DESCRIPTOR_LEN + TUD_USBTMC_BULK_DESCRIPTORS_LEN)
 
 #endif /* CFG_TUD_USBTMC_ENABLE_INT_EP */
@@ -119,21 +122,66 @@ enum
 #endif
 
 
-uint8_t const desc_configuration[] =
+uint8_t const desc_fs_configuration[] =
 {
   // Config number, interface count, string index, total length, attribute, power in mA
   TUD_CONFIG_DESCRIPTOR(1, ITF_NUM_TOTAL, 0, CONFIG_TOTAL_LEN, 0x00, 100),
 
-  TUD_USBTMC_DESC(ITF_NUM_USBTMC),
+  TUD_USBTMC_DESC(ITF_NUM_USBTMC, /* _bulkMaxPacketLength = */ 64),
 };
 
+#if TUD_OPT_HIGH_SPEED
+
+uint8_t const desc_hs_configuration[] =
+{
+  // Config number, interface count, string index, total length, attribute, power in mA
+  TUD_CONFIG_DESCRIPTOR(1, ITF_NUM_TOTAL, 0, CONFIG_TOTAL_LEN, 0x00, 100),
+
+  TUD_USBTMC_DESC(ITF_NUM_USBTMC, /* _bulkMaxPacketLength = */ 512),
+};
+
+// other speed configuration
+uint8_t desc_other_speed_config[CONFIG_TOTAL_LEN];
+
+// device qualifier is mostly similar to device descriptor since we don't change configuration based on speed
+tusb_desc_device_qualifier_t const desc_device_qualifier =
+{
+  .bLength            = sizeof(tusb_desc_device_qualifier_t),
+  .bDescriptorType    = TUSB_DESC_DEVICE_QUALIFIER,
+  .bcdUSB             = USB_BCD,
+
+  .bDeviceClass       = 0x00,
+  .bDeviceSubClass    = 0x00,
+  .bDeviceProtocol    = 0x00,
+
+  .bMaxPacketSize0    = CFG_TUD_ENDPOINT0_SIZE,
+  .bNumConfigurations = 0x01,
+  .bReserved          = 0x00
+};
+
+// Invoked when received GET DEVICE QUALIFIER DESCRIPTOR request
+// Application return pointer to descriptor, whose contents must exist long enough for transfer to complete.
+// device_qualifier descriptor describes information about a high-speed capable device that would
+// change if the device were operating at the other speed. If not highspeed capable stall this request.
+uint8_t const* tud_descriptor_device_qualifier_cb(void)
+{
+  return (uint8_t const*) &desc_device_qualifier;
+}
+
+#endif
+
 // Invoked when received GET CONFIGURATION DESCRIPTOR
 // Application return pointer to descriptor
 // Descriptor contents must exist long enough for transfer to complete
 uint8_t const * tud_descriptor_configuration_cb(uint8_t index)
 {
   (void) index; // for multiple configurations
-  return desc_configuration;
+#if TUD_OPT_HIGH_SPEED
+  // Although we are highspeed, host may be fullspeed.
+  return (tud_speed_get() == TUSB_SPEED_HIGH) ?  desc_hs_configuration : desc_fs_configuration;
+#else
+  return desc_fs_configuration;
+#endif
 }
 
 //--------------------------------------------------------------------+

examples/device/usbtmc/visaQuery.py

@@ -1,6 +1,6 @@
 #!/usr/bin/env python3
 
-import visa
+import pyvisa
 import time
 import sys
 
@@ -54,9 +54,9 @@ def test_srq():
 	assert (inst.read_stb() == 0)
 	inst.write("123")
 	
-	#inst.enable_event(visa.constants.VI_EVENT_SERVICE_REQ, visa.constants.VI_QUEUE)
-	#waitrsp = inst.wait_on_event(visa.constants.VI_EVENT_SERVICE_REQ, 5000)
-	#inst.discard_events(visa.constants.VI_EVENT_SERVICE_REQ, visa.constants.VI_QUEUE)
+	#inst.enable_event(pyvisa.constants.VI_EVENT_SERVICE_REQ, pyvisa.constants.VI_QUEUE)
+	#waitrsp = inst.wait_on_event(pyvisa.constants.VI_EVENT_SERVICE_REQ, 5000)
+	#inst.discard_events(pyvisa.constants.VI_EVENT_SERVICE_REQ, pyvisa.constants.VI_QUEUE)
 	#inst.wait_for_srq()
 	time.sleep(0.3)
 	stb = inst.read_stb()
@@ -77,8 +77,8 @@ def test_read_timeout():
 	t0 = time.monotonic()
 	try:
 		rsp = inst.read()
-		assert(false), "Read should have resulted in timeout"
-	except visa.VisaIOError:
+		assert(False), "Read should have resulted in timeout"
+	except pyvisa.VisaIOError:
 		print("  Got expected exception")
 	t = time.monotonic() - t0
 	assert ((t*1000.0) > (inst.timeout - 300))
@@ -99,23 +99,27 @@ def test_abort_in():
 	t0 = time.monotonic()
 	try:
 		rsp = inst.read()
-		assert(false), "Read should have resulted in timeout"
-	except visa.VisaIOError:
+		assert(False), "Read should have resulted in timeout"
+	except pyvisa.VisaIOError:
 		print("  Got expected exception")
 	t = time.monotonic() - t0
 	assert ((t*1000.0) > (inst.timeout - 300))
 	assert ((t*1000.0) < (inst.timeout + 300))
 	print(f"  Delay was {t:0.3}")
-	# Response is still in queue, so send a clear (to be more helpful to the next test)
+	# Response is still in queue, so read it out (to be more helpful to the next test)
 	inst.timeout = 800
 	y = inst.read()
 	assert(y == "xxx\r\n")
 	
 def test_indicate():
 	# perform indicator pulse
-	usb_iface = inst.get_visa_attribute(visa.constants.VI_ATTR_USB_INTFC_NUM)
+	usb_iface = inst.get_visa_attribute(pyvisa.constants.VI_ATTR_USB_INTFC_NUM)
 	retv = inst.control_in(request_type_bitmap_field=0xA1, request_id=64, request_value=0x0000, index=usb_iface, length=0x0001)
-	assert((retv[1] == visa.constants.StatusCode(0)) and (retv[0] == b'\x01')), f"indicator pulse failed: retv={retv}"
+	# pyvisa used to return (statuscode,bytes), but now only returns bytes, so we need to handle both cases
+	if(isinstance(retv,bytes)):
+		assert(retv == b'\x01')
+	else:
+		assert((retv[1] == pyvisa.constants.StatusCode(0)) and (retv[0] == b'\x01')), f"indicator pulse failed: retv={retv}"
 	
 	
 def test_multi_read():
@@ -131,19 +135,19 @@ def test_multi_read():
 	#inst.chunk_size = old_chunk_size
 	
 def test_stall_ep0():
-	usb_iface = inst.get_visa_attribute(visa.constants.VI_ATTR_USB_INTFC_NUM)
+	usb_iface = inst.get_visa_attribute(pyvisa.constants.VI_ATTR_USB_INTFC_NUM)
 	inst.read_stb()
 	# This is an invalid request, should create stall.
 	try:
 		retv = inst.control_in(request_type_bitmap_field=0xA1, request_id=60, request_value=0x0000, index=usb_iface, length=0x0001)
-		assert false
-	except visa.VisaIOError:
+		assert(False)
+	except pyvisa.VisaIOError:
 		pass
 	
 	assert (inst.read_stb() == 0)
 
 
-rm = visa.ResourceManager()
+rm = pyvisa.ResourceManager()
 reslist = rm.list_resources("USB?::?*::INSTR")
 print(reslist)
 
@@ -167,7 +171,6 @@ inst.timeout = 2000
 print("+ multi read")
 test_multi_read()
 
-
 print("+ echo delay=0")
 inst.write("delay 0")
 test_echo(1,175)

src/class/usbtmc/usbtmc.h

@@ -189,7 +189,10 @@ typedef enum {
   USBTMC_STATUS_FAILED = 0x80,
   USBTMC_STATUS_TRANSFER_NOT_IN_PROGRESS = 0x81,
   USBTMC_STATUS_SPLIT_NOT_IN_PROGRESS = 0x82,
-  USBTMC_STATUS_SPLIT_IN_PROGRESS  = 0x83
+  USBTMC_STATUS_SPLIT_IN_PROGRESS  = 0x83,
+
+  /****** USBTMC 488 *************/
+  USB488_STATUS_INTERRUPT_IN_BUSY = 0x20
 } usbtmc_status_enum;
 
 /************************************************************

src/class/usbtmc/usbtmc_device.c

@@ -64,7 +64,8 @@
 // USBTMC 3.2.2 error conditions not strictly followed
 // No local lock-out, REN, or GTL.
 // Clear message available status byte at the correct time? (488 4.3.1.3)
-
+// Ability to defer status byte transmission
+// Transmission of status byte in response to USB488 SRQ condition
 
 #include "tusb_option.h"
 
@@ -80,6 +81,11 @@
 static char logMsg[150];
 #endif
 
+// Buffer size must be an exact multiple of the max packet size for both
+// bulk  (up to 64 bytes for FS, 512 bytes for HS). In addation, this driver
+// imposes a minimum buffer size of 32 bytes. 
+#define USBTMCD_BUFFER_SIZE (TUD_OPT_HIGH_SPEED ? 512 : 64)
+
 /*
  * The state machine does not allow simultaneous reading and writing. This is
  * consistent with USBTMC.
@@ -120,9 +126,12 @@ typedef struct
   uint8_t ep_int_in;
   // IN buffer is only used for first packet, not the remainder
   // in order to deal with prepending header
-  CFG_TUSB_MEM_ALIGN uint8_t ep_bulk_in_buf[USBTMCD_MAX_PACKET_SIZE];
+  CFG_TUSB_MEM_ALIGN uint8_t ep_bulk_in_buf[USBTMCD_BUFFER_SIZE];
+  uint32_t ep_bulk_in_wMaxPacketSize;
   // OUT buffer receives one packet at a time
-  CFG_TUSB_MEM_ALIGN uint8_t ep_bulk_out_buf[USBTMCD_MAX_PACKET_SIZE];
+  CFG_TUSB_MEM_ALIGN uint8_t ep_bulk_out_buf[USBTMCD_BUFFER_SIZE];
+  uint32_t ep_bulk_out_wMaxPacketSize;
+
   uint32_t transfer_size_remaining; // also used for requested length for bulk IN.
   uint32_t transfer_size_sent;      // To keep track of data bytes that have been queued in FIFO (not header bytes)
 
@@ -139,11 +148,8 @@ CFG_TUSB_MEM_SECTION static usbtmc_interface_state_t usbtmc_state =
     .itf_id = 0xFF,
 };
 
-// We need all headers to fit in a single packet in this implementation.
-TU_VERIFY_STATIC(USBTMCD_MAX_PACKET_SIZE >= 32u,"USBTMC dev EP packet size too small");
-TU_VERIFY_STATIC(
-    (sizeof(usbtmc_state.ep_bulk_in_buf) % USBTMCD_MAX_PACKET_SIZE) == 0,
-    "packet buffer must be a multiple of the packet size");
+// We need all headers to fit in a single packet in this implementation, 32 bytes will fit all standard USBTMC headers
+TU_VERIFY_STATIC(USBTMCD_BUFFER_SIZE >= 32u,"USBTMC dev buffer size too small");
 
 static bool handle_devMsgOutStart(uint8_t rhport, void *data, size_t len);
 static bool handle_devMsgOut(uint8_t rhport, void *data, size_t len, size_t packetLen);
@@ -151,7 +157,6 @@ static bool handle_devMsgOut(uint8_t rhport, void *data, size_t len, size_t pack
 static uint8_t termChar;
 static uint8_t termCharRequested = false;
 
-
 osal_mutex_def_t usbtmcLockBuffer;
 static osal_mutex_t usbtmcLock;
 
@@ -282,12 +287,15 @@ uint16_t usbtmcd_open_cb(uint8_t rhport, tusb_desc_interface_t const * itf_desc,
       tusb_desc_endpoint_t const *ep_desc = (tusb_desc_endpoint_t const *)p_desc;
       switch(ep_desc->bmAttributes.xfer) {
         case TUSB_XFER_BULK:
-          TU_ASSERT(tu_edpt_packet_size(ep_desc) == USBTMCD_MAX_PACKET_SIZE, 0);
+          // Ensure  buffer is an exact multiple of the maxPacketSize
+          TU_ASSERT((USBTMCD_BUFFER_SIZE % tu_edpt_packet_size(ep_desc)) == 0, 0);
           if (tu_edpt_dir(ep_desc->bEndpointAddress) == TUSB_DIR_IN)
           {
             usbtmc_state.ep_bulk_in = ep_desc->bEndpointAddress;
+            usbtmc_state.ep_bulk_in_wMaxPacketSize = tu_edpt_packet_size(ep_desc);
           } else {
             usbtmc_state.ep_bulk_out = ep_desc->bEndpointAddress;
+            usbtmc_state.ep_bulk_out_wMaxPacketSize = tu_edpt_packet_size(ep_desc);
           }
 
           break;
@@ -395,7 +403,7 @@ static bool handle_devMsgOut(uint8_t rhport, void *data, size_t len, size_t pack
   // return true upon failure, as we can assume error is being handled elsewhere.
   TU_VERIFY(usbtmc_state.state == STATE_RCV,true);
 
-  bool shortPacket = (packetLen < USBTMCD_MAX_PACKET_SIZE);
+  bool shortPacket = (packetLen < usbtmc_state.ep_bulk_out_wMaxPacketSize);
 
   // Packet is to be considered complete when we get enough data or at a short packet.
   bool atEnd = false;
@@ -522,7 +530,7 @@ bool usbtmcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint
       break;
 
     case STATE_TX_INITIATED:
-      if(usbtmc_state.transfer_size_remaining >=sizeof(usbtmc_state.ep_bulk_in_buf))
+      if(usbtmc_state.transfer_size_remaining >= sizeof(usbtmc_state.ep_bulk_in_buf))
       {
         // FIXME! This removes const below!
         TU_VERIFY( usbd_edpt_xfer(rhport, usbtmc_state.ep_bulk_in,
@@ -539,7 +547,7 @@ bool usbtmcd_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t result, uint
         usbtmc_state.transfer_size_remaining = 0;
         usbtmc_state.devInBuffer = NULL;
         TU_VERIFY( usbd_edpt_xfer(rhport, usbtmc_state.ep_bulk_in, usbtmc_state.ep_bulk_in_buf, (uint16_t)packetLen) );
-        if(((packetLen % USBTMCD_MAX_PACKET_SIZE) != 0) || (packetLen == 0 ))
+        if(((packetLen % usbtmc_state.ep_bulk_in_wMaxPacketSize) != 0) || (packetLen == 0 ))
         {
           usbtmc_state.state = STATE_TX_SHORTED;
         }
@@ -589,18 +597,31 @@ bool usbtmcd_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request
   {
     uint32_t ep_addr = (request->wIndex);
 
+    // At this point, a transfer MAY be in progress. Based on USB spec, when clearing bulk EP HALT,
+    // the EP transfer buffer needs to be cleared and DTOG needs to be reset, even if 
+    // the EP is not halted. The only USBD API interface to do this is to stall and then unstall the EP.
     if(ep_addr == usbtmc_state.ep_bulk_out)
     {
       criticalEnter();
+      usbd_edpt_stall(rhport, (uint8_t)ep_addr);
+      usbd_edpt_clear_stall(rhport, (uint8_t)ep_addr);
       usbtmc_state.state = STATE_NAK; // USBD core has placed EP in NAK state for us
       criticalLeave();
       tud_usbtmc_bulkOut_clearFeature_cb();
     }
     else if (ep_addr == usbtmc_state.ep_bulk_in)
     {
+      usbd_edpt_stall(rhport, (uint8_t)ep_addr);
+      usbd_edpt_clear_stall(rhport, (uint8_t)ep_addr);
       tud_usbtmc_bulkIn_clearFeature_cb();
     }
-    else
+    else if ((usbtmc_state.ep_int_in != 0) && (ep_addr == usbtmc_state.ep_int_in))
+    {
+      // Clearing interrupt in EP
+      usbd_edpt_stall(rhport, (uint8_t)ep_addr);
+      usbd_edpt_clear_stall(rhport, (uint8_t)ep_addr);
+    }
+     else
     {
       return false;
     }
@@ -680,7 +701,7 @@ bool usbtmcd_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request
     usbtmc_state.transfer_size_remaining = 0u;
       // Check if we've queued a short packet
       criticalEnter();
-      usbtmc_state.state = ((usbtmc_state.transfer_size_sent % USBTMCD_MAX_PACKET_SIZE) == 0) ?
+      usbtmc_state.state = ((usbtmc_state.transfer_size_sent % usbtmc_state.ep_bulk_in_wMaxPacketSize) == 0) ?
               STATE_ABORTING_BULK_IN : STATE_ABORTING_BULK_IN_SHORTED;
       criticalLeave();
       if(usbtmc_state.transfer_size_sent  == 0)
@@ -810,25 +831,32 @@ bool usbtmcd_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request
 
       bTag = request->wValue & 0x7F;
       TU_VERIFY(request->bmRequestType == 0xA1);
-      TU_VERIFY((request->wValue & (~0x7F)) == 0u); // Other bits are required to be zero
+      TU_VERIFY((request->wValue & (~0x7F)) == 0u); // Other bits are required to be zero (USB488v1.0 Table 11)
       TU_VERIFY(bTag >= 0x02 && bTag <= 127);
       TU_VERIFY(request->wIndex == usbtmc_state.itf_id);
       TU_VERIFY(request->wLength == 0x0003);
       rsp.bTag = (uint8_t)bTag;
       if(usbtmc_state.ep_int_in != 0)
       {
-        rsp.USBTMC_status = USBTMC_STATUS_SUCCESS;
-        rsp.statusByte = 0x00; // Use interrupt endpoint, instead.
-
-        usbtmc_read_stb_interrupt_488_t intMsg =
+        rsp.statusByte = 0x00; // Use interrupt endpoint, instead. Must be 0x00 (USB488v1.0 4.3.1.2)
+        if(usbd_edpt_busy(rhport, usbtmc_state.ep_int_in))
+        {
+          rsp.USBTMC_status = USB488_STATUS_INTERRUPT_IN_BUSY;
+        }
+        else
         {
-          .bNotify1 = {
-              .one = 1,
-              .bTag = bTag & 0x7Fu,
-          },
-          .StatusByte = tud_usbtmc_get_stb_cb(&(rsp.USBTMC_status))
-        };
-        usbd_edpt_xfer(rhport, usbtmc_state.ep_int_in, (void*)&intMsg, sizeof(intMsg));
+          rsp.USBTMC_status = USBTMC_STATUS_SUCCESS;
+          usbtmc_read_stb_interrupt_488_t intMsg =
+          {
+            .bNotify1 = {
+                .one = 1,
+                .bTag = bTag & 0x7Fu,
+            },
+            .StatusByte = tud_usbtmc_get_stb_cb(&(rsp.USBTMC_status))
+          };
+          // Must be queued before control request response sent (USB488v1.0 4.3.1.2)
+          usbd_edpt_xfer(rhport, usbtmc_state.ep_int_in, (void*)&intMsg, sizeof(intMsg));
+        }
       }
       else
       {

src/class/usbtmc/usbtmc_device.h

@@ -35,10 +35,6 @@
 #define CFG_TUD_USBTMC_ENABLE_488 (1)
 #endif
 
-// USB spec says that full-speed must be 8,16,32, or 64.
-// However, this driver implementation requires it to be >=32
-#define USBTMCD_MAX_PACKET_SIZE (TUD_OPT_HIGH_SPEED ? 512 : 64)
-
 /***********************************************
  *  Functions to be implemeted by the class implementation
  */