Replace ST USB stack?

[daniel-santos]

Hello @HubertD! I saw you mention this in a pull request and it's something I'm likely going to have to address. I haven't dug into it too deeply but tinyusb is a stack that claims to support many STM32 µCs -- I would have to add STM32G support myself, but it has STM32F support.

Anyway, if any of us start any serious work on this, we can use this issue to track it. Thanks!

EDIT: I forgot to mention, the only real problem with it is that the license pollution by adding the restriction that it cannot be used on non-ST products (which seems a tad bit silly to me anyway). As we don't support any non-ST chips (yet), it's only an issue of the license being non-free.

[fenugrec]

If replacing the USB stack means fixing the weird inexplicable issues, I'm very open to the idea.

[Daniel-Trevitz]

I made an attempt here:
hathach/tinyusb#1657
https://github.com/Daniel-Trevitz/candleLight_fw/tree/tinyusb/src

Still having the same issue as described in the discussion.

The API is /way/ cleaner - though not really documented... I'd recommend it except it currently only transmits one frame and then goes silent.

It's working now. buffer was the wrong size... The tinyusb implementation is pretty clean.

[fenugrec]

The API is /way/ cleaner - though not really documented.

that's a bit off-putting...

license pollution by adding the restriction that it cannot be used on non-ST products

Can you elaborate ? tinyusb seems to be MIT-licensed, and if their low-level ST-specific code is restricted to ST (which is probably only the case if they're directly using files from ST's libraries), that shouldn't prevent us from supporting other mcus ?

[Daniel-Trevitz]

The API is /way/ cleaner - though not really documented.

that's a bit off-putting...

It should be. I found that part not fun; but that said, reading the code was MUCH easier than reading ST's code.

license pollution by adding the restriction that it cannot be used on non-ST products

Can you elaborate ? tinyusb seems to be MIT-licensed, and if their low-level ST-specific code is restricted to ST (which is probably only the case if they're directly using files from ST's libraries), that shouldn't prevent us from supporting other mcus ?

Assuming the OP is correct, simply not compiling the ST code into other microcontrollers would be enough. Taht said, I don't think the OP is correct about this. Looks like a BSD-3 lic to me.

[zigele]

Hi, I am implementing the usb2can protocol for candlelight based on ESP32. esp32's usb stack is tinyusb. currently the open source software tsmaster has successfully enumerated canable devices.
But it doesn't work yet. I am having problems with this function USBD_CtlPrepareRx, I am not sure how this function works (USBD_CtlPrepareRx) and how to implement it with tinyusb.

I would appreciate it if someone could give me some pointers.

static uint8_t USBD_GS_CAN_Config_Request(USBD_HandleTypeDef *pdev, USBD_SetupReqTypedef *req)
{
	USBD_GS_CAN_HandleTypeDef *hcan = (USBD_GS_CAN_HandleTypeDef*) pdev->pClassData.
	enum gs_can_termination_state term_state.
	uint32_t d32. 

	switch (req->bRequest) {

		case GS_USB_BREQ_SET_TERMINATION.
			if (get_term(req->wValue) == GS_CAN_TERMINATION_UNSUPPORTED) {
				USBD_CtlError(pdev, req).
				break.
			}
		// fall-through
		case GS_USB_BREQ_HOST_FORMAT.
		case GS_USB_BREQ_MODE.
		case GS_USB_BREQ_BITTIMING.
		case GS_USB_BREQ_IDENTIFY.
			hcan->last_setup_request = *req.
			USBD_CtlPrepareRx(pdev, hcan->ep0_buf, req->wLength).
			break.
 ```
![image](https://user-images.githubusercontent.com/22033218/235429764-9fc23539-a9f1-4e9a-ab82-ea655032b33c.png)

[marckleinebudde]

I think you have to implement the tud_vendor_control_xfer_cb() callback.

[Daniel-Trevitz]

@zigele, did you reference my branch linked above? Marckleinebudde is correct. See my branch's copy of usb_descriptors.c, main.c, and in particular gs_usb.c

[zigele]

I think you have to implement the tud_vendor_control_xfer_cb() callback.

Yes, I have implemented it. However, what I am very confused about is what data I should return to the host. I am stuck with my work. This is my test code. The problem is this code case GS_USB_BREQ_MODE

bool tud_vendor_control_xfer_cb(uint8_t rhport, uint8_t stage, tusb_control_request_t const *request)
{
    static uint8_t rx_buf[1000];
    uint8_t datae[8] = {0xef, 0xbe, 0, 0, 0, 0x10, 0, 0, 0};
    uint8_t timet[8] = {0, 0, 0, 0, 0, 0, 9, 9};
    uint8_t bufferr[1000] = {2};
    uint8_t ifalt = 0;
    if (stage != CONTROL_STAGE_SETUP)
        return true;
    ESP_LOGI("tud_vendor_control_xfer_cb request", "%d %d %d", request->bmRequestType_bit.type, rhport, stage);
    switch (request->bmRequestType_bit.type)
    {
    case TUSB_REQ_TYPE_VENDOR: // 2 USBD_GS_CAN_Config_Request
        ESP_LOGI("tud_vendor_control_xfer_cb request->bRequest", "%d  %d", request->bRequest, request->wIndex);
        switch (request->bRequest) // return USBD_GS_CAN_Vendor_Request(pdev, req);
        {

            // fall-through
        case GS_USB_BREQ_HOST_FORMAT: // 0
        case GS_USB_BREQ_MODE:        // 2
        case GS_USB_BREQ_BITTIMING:   // 1
        case GS_USB_BREQ_IDENTIFY:    // 7
            //  return tud_control_xfer(rhport, request, (void *)(uintptr_t)datae, 4);
            // return usbd_control_set_request(request);
            return tud_control_status(rhport, request);
            break;
        case GS_USB_BREQ_GET_TERMINATION:
            break;
        case GS_USB_BREQ_DEVICE_CONFIG: // 5
            return tud_control_xfer(rhport, request, (void *)(uintptr_t)USBD_GS_CAN_dconf, request->wLength);
            break;
        case GS_USB_BREQ_BT_CONST: // 4
            return tud_control_xfer(rhport, request, (void *)(uintptr_t)USBD_GS_CAN_btconst, request->wLength);
            break;
        case GS_USB_BREQ_TIMESTAMP:
            // return tud_control_xfer(rhport, request, (void *)(uintptr_t)datae, 4);
            break;
        case VENDOR_REQUEST_WIFI:
            break;
        case VENDOR_REQUEST_MICROSOFT: // 15
            if (request->wIndex == 7)
            {
                ESP_LOGI("TAG", "MS OS 2.0 request");
                // Get Microsoft OS 2.0 compatible descriptor
                uint16_t total_len;
                memcpy(&total_len, desc_ms_os_20 + 8, 2);

                return tud_control_xfer(rhport, request, (void *)(uintptr_t)desc_ms_os_20, total_len);
            }
            else if (request->wIndex == 5)
            {
                ESP_LOGI("TAG", "(request->wIndex == 5)");
                // Get Microsoft OS 2.0 compatible descriptor
                uint16_t total_len;
                memcpy(&total_len, desc_ms_os_20 + 8, 2);

                return tud_control_xfer(rhport, request, (void *)(uintptr_t)desc_ms_os_20, total_len);
            }
            else if (request->wIndex == 4)
            {
                ESP_LOGI("TAG", "(request->wIndex == 4)");
                return tud_control_xfer(rhport, request, (void *)(uintptr_t)USBD_MS_COMP_ID_FEATURE_DESC, request->wLength);
            }
            else
            {
                return false;
            }
            break;

        case GS_USB_BREQ_SET_TERMINATION: // 12

            break;
        default:
            // Stall both IN and OUT control endpoint
            //   dcd_edpt_stall(rhport, EDPT_CTRL_OUT);
            //   dcd_edpt_stall(rhport, EDPT_CTRL_IN);
            break;
        }
        break;
    case TUSB_REQ_TYPE_CLASS: // 1
        if (request->bRequest == 0x22)
        {
        }
        break;
    case TUSB_REQ_TYPE_STANDARD: // 0
    {
        switch (request->bRequest)
        {
        case 0x0AU: // USB_REQ_GET_INTERFACE
            return tud_control_xfer(rhport, request, &ifalt, 1);
            break;
        case 0x0B:
            break;
        default:
            break;
        }
    }
    break;
    case TUSB_REQ_TYPE_INVALID: // 3
    {
      //to do
    }
    break;
    default:
       // tud_control_status(rhport, request);
        break;
    }
    ESP_LOGI("tud_vendor_control_xfer_cb end", "--------------------------------------");
    return false;
}

···

[Daniel-Trevitz]

You're not implementing the CONTROL_STAGE_DATA. See my branch's copy of gs_usb.c, line 265

[zigele]

You're not implementing the CONTROL_STAGE_DATA. See my branch's copy of gs_usb.c, line 265

Thank you very much for the pointers. It worked.
Very much looking forward to candlelight's performance on esp32.

[marckleinebudde]

Please share the code, once it's working.

[zigele]

Of course I will. It may take some time to sort out the code and implement the can driver for ESP32.

[zigele]

Very unfortunate and worrisome results. I implemented the candlelight source code on the ESP32S3 chip, using the tinyusb stack and the cherryusb stack, and got unsatisfactory results. When the CAN bus is running at 1Mbps baud rate and the load exceeds 50%, the usb device endpoint receives a clear feature from the host computer, which causes the bulk in transmission to stop on the device side. I suspect that there is a problem with the driver of the Tsmaster software on the PC side, but I have not found the root cause, which is beyond my capabilities. I am considering dropping the ESP32 and choosing the advance chip instead.
thanks.

[marckleinebudde]

Oh, that's not good. FWIW I would test with the Linux kernel driver, not with a user space driver. Don't know the quality. Can you still upload the code somewhere?

[zigele]

I put the relevant source code in this repository. In order to find some bugs, the code was messed up by me.
https://github.com/zigele/esp32-candlelight_fw
The Windows platform testing software I use is TSmaster.
The USB protocol stack used is CherryUSB.

As shown in the figure, the main problem is that when the baud rate exceeds 1Mbps, a large amount of data data transmission will cause TSmaster to send USB instructions (Clear Feature), causing the USB device to stop working.
Unfortunately I was not able to find a solution.
void IRAM_ATTR usncan_in_callback(uint8_t ep, uint32_t nbytes) { // taskENTER_CRITICAL(&myMutex); // USB_LOG_RAW("actual in len:%d\r\n", nbytes); if ((nbytes % USBCAN_MAX_PACKET_SIZE) == 0 && nbytes) { usbd_ep_start_write(USBCAN_EP_SEND, NULL, 0); } else { xHigherPriorityTaskWoken = pdFALSE; xSemaphoreGiveFromISR(xSemaphore, &xHigherPriorityTaskWoken); portYIELD_FROM_ISR(xHigherPriorityTaskWoken); } // taskEXIT_CRITICAL(&myMutex); } void IRAM_ATTR can0_receive_task(void *arg) { gs_host_frame_t frame; frame.echo_id = 0xFFFFFFFF; // not an echo frame frame.channel = 0; frame.flags = 0; twai_message_t can_mes; frame.reserved = 0; uint8_t frame_len = sizeof(gs_host_frame_t) - 4; while (1) { twai_receive(&can_mes, portMAX_DELAY); frame.can_id = can_mes.identifier; frame.can_dlc = can_mes.data_length_code; memcpy(frame.data, can_mes.data, can_mes.data_length_code); // frame.timestamp_us = esp_timer_get_time(); // count12++; xHigherPriorityTaskWoken = pdFALSE; xSemaphoreTakeFromISR(xSemaphore, &xHigherPriorityTaskWoken); usbd_ep_start_write(USBCAN_EP_SEND, &frame, frame_len); } // vTaskDelay(10 / portTICK_PERIOD_MS); vTaskDelete(NULL); }

[zigele]

I put the relevant source code in this repository. In order to find some bugs, the code was messed up by me. https://github.com/zigele/esp32-candlelight_fw The Windows platform testing software I use is TSmaster. The USB protocol stack used is CherryUSB.

As shown in the figure, the main problem is that when the baud rate exceeds 1Mbps, a large amount of data data transmission will cause TSmaster to send USB instructions (Clear Feature), causing the USB device to stop working. Unfortunately I was not able to find a solution. void IRAM_ATTR usncan_in_callback(uint8_t ep, uint32_t nbytes) { // taskENTER_CRITICAL(&myMutex); // USB_LOG_RAW("actual in len:%d\r\n", nbytes); if ((nbytes % USBCAN_MAX_PACKET_SIZE) == 0 && nbytes) { usbd_ep_start_write(USBCAN_EP_SEND, NULL, 0); } else { xHigherPriorityTaskWoken = pdFALSE; xSemaphoreGiveFromISR(xSemaphore, &xHigherPriorityTaskWoken); portYIELD_FROM_ISR(xHigherPriorityTaskWoken); } // taskEXIT_CRITICAL(&myMutex); } void IRAM_ATTR can0_receive_task(void *arg) { gs_host_frame_t frame; frame.echo_id = 0xFFFFFFFF; // not an echo frame frame.channel = 0; frame.flags = 0; twai_message_t can_mes; frame.reserved = 0; uint8_t frame_len = sizeof(gs_host_frame_t) - 4; while (1) { twai_receive(&can_mes, portMAX_DELAY); frame.can_id = can_mes.identifier; frame.can_dlc = can_mes.data_length_code; memcpy(frame.data, can_mes.data, can_mes.data_length_code); // frame.timestamp_us = esp_timer_get_time(); // count12++; xHigherPriorityTaskWoken = pdFALSE; xSemaphoreTakeFromISR(xSemaphore, &xHigherPriorityTaskWoken); usbd_ep_start_write(USBCAN_EP_SEND, &frame, frame_len); } // vTaskDelay(10 / portTICK_PERIOD_MS); vTaskDelete(NULL); }

TSmaster use the windows usb driver is https://github.com/HubertD/candle_dll/tree/master/src，I guess。

[marckleinebudde]

The problem is the "stall". The USB device says something is broken.

[fenugrec]

TSmaster use the windows usb driver is https://github.com/HubertD/candle_dll/tree/master/src，I guess。

Are you sure ? that driver is nearly unusable as-is. I spent many frustrating hours trying to make it work well at higher bus loads but it seems nearly impossible with just a userland driver that uses winUSB.

If they somehow fixed it to work "better", they should have the source code somewhere since it is LGPL; I'd be interested to see that...

[zigele]

TSmaster use the windows usb driver is https://github.com/HubertD/candle_dll/tree/master/src，I guess。

Are you sure ? that driver is nearly unusable as-is. I spent many frustrating hours trying to make it work well at higher bus loads but it seems nearly impossible with just a userland driver that uses winUSB.

If they somehow fixed it to work "better", they should have the source code somewhere since it is LGPL; I'd be interested to see that...

not sure。By decompiling the cando .dll file in its installation directory, it seems that it has partially identical interfaces. But I'm not sure, maybe I can raise an issue with Tsmaster.

[marckleinebudde]

Maybe you can create a capture with wireshark, so we can analyze it, too. A stall is created by the USB device (here the ESP) if something does unexpectedly wrong. Although the windows driver might not be that good, it smells more like a problem with the USB stack on the ESP.

During porting of the candlelight firmware to the STM32G0B1 we run into a similar problem. Under high load the device was sending a stall, too. It turned out to be a bug in the USB driver. It wanted to do a read-modify-write of one of the registers in the USB controller, but instead it did a read-clear-modify-write. If a USB USB is send from the PC to the STM32G0B1 between the "clear" and the "write", the USB endpoint is not configured and the controller sends a stall.

[vtjballeng]

not sure。By decompiling the cando .dll file in its installation directory, it seems that it has partially identical interfaces. But I'm not sure, maybe I can raise an issue with Tsmaster.

@zigele , did you find out any more on this issue?

Are you sure ? that driver is nearly unusable as-is. I spent many frustrating hours trying to make it work well at higher bus loads but it seems nearly impossible with just a userland driver that uses winUSB.

If they somehow fixed it to work "better", they should have the source code somewhere since it is LGPL; I'd be interested to see that...

@fenugrec, I found that Tsmaster can easily scale in busload so I can have a PCAN FD checking Busload at 87% and the STMF072 candlelight fw board keeps up. It use a cando.dll . I haven't seen any published source which might be avoided by labeling the dll file as cando rather than candle.

Meanwhile, in cangaroo v2.3 using the candle.dll I can't get accurate timing at 50ms or less in the single digit bus load range.

[fenugrec]

I haven't seen any published source which might be avoided by labeling the dll file as cando rather than candle.

Renaming a file doesn't mean they can bypass LGPL requirements...

Meanwhile, in cangaroo v2.3 using the candle.dll

As-is, the publicly available source for candle.dll is unusable, as you have described. I spent some time a while back trying to improve it, tried many things. Never published because it was still unreliable garbage.

But this is getting off-topic; this issue is about replacing the ST USB stack.