phdussud/pico-dirtyJtag

Any plans for a UART device?

davidthings opened this issue · 16 comments

I'd love for this codebase to also support a UART over the same connection.

Are there any plans for this?

Would a PR for this functionality be interesting?

Hello David,
I don't have plans for this but a PR would be welcome. The constraint we are under is that this device needs to be compatible with the existing dirtyJtag. We can't change the interface numbers and PID/VID of the device. The new interface has to use a different number. I don't remember if the CDC spec imposes fixed interface numbers
I am raising this issue because all of the application code that interfaces with DirtyJtag claims interfaces by number (i.e they don't use an enumeration and pick approach)

Understood. I'll poke around.

Hacking around a little I added the CDC serial to the end of the interfaces and endpoints, and made CDC task just do an echo back. On Ubuntu 22, Echo back works and openFPGALoader reports that it sees the jtag device.

Cautiously optimistic?

Here's the relevant lsusb -v section, in case anything looks messed up. Full disclosure, I am a non-expert in USB and RP2040.

Bus 003 Device 025: ID 1209:c0ca Generic Jean THOMAS DirtyJTAG

Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x1209 Generic
  idProduct          0xc0ca Jean THOMAS DirtyJTAG
  bcdDevice            1.10
  iManufacturer           1 Jean THOMAS
  iProduct                2 DirtyJTAG
  iSerial                 3 DC6168411724432D
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength       0x0062
    bNumInterfaces          3
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower              100mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass       255 Vendor Specific Class
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
    Interface Association:
      bLength                 8
      bDescriptorType        11
      bFirstInterface         1
      bInterfaceCount         2
      bFunctionClass          2 Communications
      bFunctionSubClass       2 Abstract (modem)
      bFunctionProtocol       0 
      iFunction               0 
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         2 Communications
      bInterfaceSubClass      2 Abstract (modem)
      bInterfaceProtocol      0 
      iInterface              4 
      CDC Header:
        bcdCDC               1.20
      CDC Call Management:
        bmCapabilities       0x00
        bDataInterface          2
      CDC ACM:
        bmCapabilities       0x02
          line coding and serial state
      CDC Union:
        bMasterInterface        1
        bSlaveInterface         2 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval              16
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass        10 CDC Data
      bInterfaceSubClass      0 
      bInterfaceProtocol      0 
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x84  EP 4 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval               0

OK. we should find out if it works on Windows. The implementation of USB can be different

Will check.

I also have a different project that can be used for this. I ported blackmagic to Pico, including a fairly high perf comm port

Fancy! I'll get my slow one limping and then take a look.

Everything is looking good (UART loopback looking great). I'm putting a testbed together to confirm that the program - monitor - program - etc. cycle works for my setup, then it will be Windows test time.

Looks like it works under Window. The UART device shows up as a COM port, and appears to work, for example I can open Putty on it, and talk to my target board. As for the DirtyJTAG device, it appears in DevMan, but I have no ability / desire to check that.

Next step is to create the PR, I guess. Would you mind if I don't add your fancy UART code? I think the slow UART is a good start.

Sounds good. I would ask that you conditionalize the source with #ifdef UART_PORT or something like this, defined in dirtyjtag.c
This way people can choose to omit it if desired, but let's include it by default.

Good idea. Since this will be a definition needed in several files (dirtyJtag.c, usb_descriptors.c, etc), is it ok if it appears in a header file (like dirtyJtagConfig.h, or something)?

Are you wanting to make sure none of the new stuff is included in the build (to minimize code size, for example), or just that the USB machinery is as before in the case of UART_PORT false?

Yes, a config header is the right thing to do. I would like to eliminate all of the new stuff when UART_PORT is false. It is good for code size and code modularity (no need to concern oneself non functional code).

Understood

Good news, bad news. On the bad side, even in tight loopback, the USB CDC implementation on the RP2040 seems prone to dropping characters, so meh-level performance might be our experience. On the good side, as part of the debugging process, I dropped your cute DMA CDC-UART interface in, rather than my more plodding implementation. Seems to be working pretty well. I'm about to do some ifdef'ing (for conditional compilation) and then I'll send up the PR.

good!