Rockbox Technical Forums

Rockbox Development => Starting Development and Compiling => Topic started by: pamaury on August 10, 2009, 09:38:49 AM

Title: [SOLVED] usb_core_request_endpoint: does order matter ?
Post by: pamaury on August 10, 2009, 09:38:49 AM
Hello,
I have been using rockbox for some time now but recently I start hacking rockbox and I am more precisely interested in USB code. I tried to add a usb driver that would eventually implement the CDC-ACM class (I know there is a GSoC to implement usb classes in plugins but it's not ready yet).
The main point is that CDC-ACM requires two pipes:
1) One interrupt pipe (dir IN)
2) One bulk pipe (dir IN/OUT)

So I naturally wrote this code:
Code: [Select]
int usb_cdc_acm_request_endpoints(struct usb_class_driver *drv)
{
    /* Communication Class interrupt endpoint */
    comm_ep_int=usb_core_request_endpoint(USB_ENDPOINT_XFER_INT,USB_DIR_IN,drv);
    if(comm_ep_int<0)
    {
        logf("cdc_acm: unable to request interrupt endpoint");
        return -1;
    }

    /* Data Class bulk endpoints */
    data_ep_in=usb_core_request_endpoint(USB_ENDPOINT_XFER_BULK,USB_DIR_IN,drv);
    if(data_ep_in<0)
    {
        logf("cdc_acm: unable to request bulk in endpoint");
        usb_core_release_endpoint(comm_ep_int);
        return -1;
    }

    data_ep_out=usb_core_request_endpoint(USB_ENDPOINT_XFER_BULK,USB_DIR_OUT,drv);
    if(data_ep_out<0)
    {
        logf("cdc_acm: unable to request bulk out endpoint");
        usb_core_release_endpoint(comm_ep_int);
        usb_core_release_endpoint(data_ep_in);
        return -1;
    }
   
    return 0;
}

But it failed to work on my Sansa e200. It took me some time to understand that sansa e200 had only 3 endpoints and that I had to deactivate other drivers in order to make it works.
But even after that, it continue to failed.
I finally decided to reverse the order:

Code: [Select]
int usb_cdc_acm_request_endpoints(struct usb_class_driver *drv)
{
    /* Data Class bulk endpoints */
    data_ep_in=usb_core_request_endpoint(USB_ENDPOINT_XFER_BULK,USB_DIR_IN,drv);
    if(data_ep_in<0)
    {
        logf("cdc_acm: unable to request bulk in endpoint");
        usb_core_release_endpoint(comm_ep_int);
        return -1;
    }

    data_ep_out=usb_core_request_endpoint(USB_ENDPOINT_XFER_BULK,USB_DIR_OUT,drv);
    if(data_ep_out<0)
    {
        logf("cdc_acm: unable to request bulk out endpoint");
        usb_core_release_endpoint(comm_ep_int);
        usb_core_release_endpoint(data_ep_in);
        return -1;
    }
   
    /* Communication Class interrupt endpoint */
    comm_ep_int=usb_core_request_endpoint(USB_ENDPOINT_XFER_INT,USB_DIR_IN,drv);
    if(comm_ep_int<0)
    {
        logf("cdc_acm: unable to request interrupt endpoint");
        return -1;
    }
   
    return 0;
}

And...it worked !

I'm wondering why it fails when I first request the INT endpoint whereas it suceeds when I first request the BULK one. Does the order matter or is it driver specific ?

Thank you
Title: Re: usb_core_request_endpoint: does order matter ?
Post by: gevaerts on August 10, 2009, 10:32:57 AM
What exactly fails?

I suspect that this is a driver bug : either the allocation is buggy (i.e. the bug would be in usb_drv_request_endpoint()), or the actual setting up of endpoints is buggy.

I think I saw this problem as well at some point, but I haven't looked at it very hard.
Title: Re: usb_core_request_endpoint: does order matter ?
Post by: pamaury on August 10, 2009, 10:42:32 AM
It fails at this request:
Code: [Select]
data_ep_out=usb_core_request_endpoint(USB_ENDPOINT_XFER_BULK,USB_DIR_OUT,drv);

This is strange because the bulk in request succeeds if I'm correct, so this request should be easily satisfied. Perhaps it comes from the driver, I will investigate more.

Thank you

--------------
EDIT
--------------

I think I have found the culprit: it's usb_drv_request_endpoint is usb-drv-arc.c (the sansa e200 usb driver). I might be specific to this driver as far as I have check in other drivers.
Here is the code:
Code: [Select]
int usb_drv_request_endpoint(int type, int dir)
{
    int ep_num, ep_dir;
    short ep_type;

    /* Safety */
    ep_dir = EP_DIR(dir);
    ep_type = type & USB_ENDPOINT_XFERTYPE_MASK;

    logf("req: %s %s", XFER_DIR_STR(ep_dir), XFER_TYPE_STR(ep_type));

    /* Find an available ep/dir pair */
    for (ep_num=1;ep_num<USB_NUM_ENDPOINTS;ep_num++) {
        usb_endpoint_t* endpoint=&endpoints[ep_num];
        int other_dir=(ep_dir ? 0:1);

        if (endpoint->allocated[ep_dir])
            continue;

        if (endpoint->allocated[other_dir] &&
                endpoint->type[other_dir] != ep_type) {
            logf("ep of different type!");
            return -1; // <---- strange line !
        }


        endpoint->allocated[ep_dir] = 1;
        endpoint->type[ep_dir] = ep_type;

        log_ep(ep_num, ep_dir, "add");
        return (ep_num | (dir & USB_ENDPOINT_DIR_MASK));
    }

    return -1;
}

The function seems to fail each type it finds an endpoint with the other direction allocated but of the wrong type. This is kind of absurd and explains why it fails in my case. It should be a "continue" instead of a "return -1", perhaps a typo.